bump google.golang.org/grpc v1.20.1 by thaJeztah · Pull Request #3192 · containerd/containerd

thaJeztah · 2019-04-06T09:09:00Z

Bumping in two steps, so that I can cherry-pick the first one into the 1.2 release (assuming we don't want to bump too many versions in that branch);

bump google.golang.org/grpc v1.12.2

full diff: grpc/grpc-go@v1.12.0...v1.12.2

transport/server: fix race between writing status and header grpc/grpc-go#2074 transport/server: fix race between writing status and header
- fix Possible race sending headers from server while receiving message over size limit grpc/grpc-go#1972 Possible race sending headers from server while receiving message over size limit
transport/server: fix race between writing status and header grpc/grpc-go#2074 transport: account for user configured small io write buffer
- fix Server abruptly terminates connections if write buffer is small enough grpc/grpc-go#2089 Server abruptly terminates connections if write buffer is small enough

bump google.golang.org/grpc v1.20.1

full diff: grpc/grpc-go@v1.12.2...v1.20.1

includes grpc/grpc-go#2695 transport: do not close channel that can lead to panic

thaJeztah · 2019-04-06T09:10:11Z

@AkihiroSuda PTAL #3186 (comment)

thaJeztah · 2019-04-06T09:46:22Z

Ah, let me update;

client.go:105:4:warning: grpc.WithDialer is deprecated: use WithContextDialer instead  (SA1019) (staticcheck)
98client.go:228:79:warning: grpc.FailFast is deprecated: use WaitForReady.  (SA1019) (staticcheck)
99cmd\containerd\command\publish.go:98:3:warning: grpc.WithDialer is deprecated: use WithContextDialer instead  (SA1019) (staticcheck)
100services\server\server.go:374:3:warning: grpc.WithDialer is deprecated: use WithContextDialer instead  (SA1019) (staticcheck)

codecov-io · 2019-04-06T10:58:44Z

Codecov Report

Merging #3192 into master will increase coverage by 0.28%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #3192      +/-   ##
==========================================
+ Coverage   44.66%   44.95%   +0.28%     
==========================================
  Files         113      113              
  Lines       12156    12233      +77     
==========================================
+ Hits         5430     5499      +69     
+ Misses       5893     5891       -2     
- Partials      833      843      +10

Flag	Coverage Δ
#linux	`48.68% <0%> (ø)`	⬆️
#windows	`39.8% <0%> (-0.1%)`	⬇️

Impacted Files	Coverage Δ
services/server/server.go	`1.26% <0%> (ø)`	⬆️
remotes/docker/resolver.go	`66.96% <0%> (+6.81%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d71c7ad...2ed8e60. Read the comment docs.

AkihiroSuda · 2019-04-06T11:21:31Z

pkg/dialer/dialer.go

can we support ctx.Cancel?

My current approach is a bit dirty yeah; I'm not too familiar with these, so better suggestions are definitely welcome (was also thinking of reversing the logic and use context as the default)

If you can show a quick example, I'm happy to add it 😅

thaJeztah · 2019-04-11T19:39:43Z

updated to v1.20 to include includes grpc/grpc-go#2695 transport: do not close channel that can lead to panic (also see moby/moby#39053)

thaJeztah · 2019-04-11T21:05:01Z

@AkihiroSuda @dmcgowan @tonistiigi PTAL

vendor.conf

thaJeztah · 2019-04-12T15:27:42Z

Don't think failure is related, but I'll re-push to trigger CI;

Details

    --- FAIL: TestContentClient/CommitErrorState (4.61s)
1324        testsuite.go:117: Cleanup failed: unknown
1325            github.com/containerd/containerd/errdefs.init.ializers
1326            	C:/gopath/src/github.com/containerd/containerd/errdefs/errors.go:39
1327            runtime.main
1328            	C:/go/src/runtime/proc.go:188
1329            runtime.goexit
1330            	C:/go/src/runtime/asm_amd64.s:1337
1331            remove C:\Program Files\containerd\root-test\io.containerd.content.v1.content\ingest\9101c5f249b4cb38a50dddda86a787c09f31099573c87c525fc50774ac21d05d\.tmp-updatedat224900915: The process cannot access the file because it is being used by another process.
1332            github.com/containerd/containerd/errdefs.FromGRPC
1333            	C:/gopath/src/github.com/containerd/containerd/errdefs/grpc.go:98
1334            github.com/containerd/containerd/content/proxy.(*proxyContentStore).Abort
1335            	C:/gopath/src/github.com/containerd/containerd/content/proxy/content_store.go:187
1336            github.com/containerd/containerd.newContentStore.func2
1337            	C:/gopath/src/github.com/containerd/containerd/content_test.go:60
1338            github.com/containerd/containerd/content/testsuite.makeTest.func1.1
1339            	C:/gopath/src/github.com/containerd/containerd/content/testsuite/testsuite.go:116
1340            github.com/containerd/containerd/content/testsuite.makeTest.func1
1341            	C:/gopath/src/github.com/containerd/containerd/content/testsuite/testsuite.go:137
1342            testing.tRunner
1343            	C:/go/src/testing/testing.go:865
1344            runtime.goexit
1345            	C:/go/src/runtime/asm_amd64.s:1337
1346            failed to abort c1-commiterror-state
1347            github.com/containerd/containerd.newContentStore.func2
1348            	C:/gopath/src/github.com/containerd/containerd/content_test.go:61
1349            github.com/containerd/containerd/content/testsuite.makeTest.func1.1
1350            	C:/gopath/src/github.com/containerd/containerd/content/testsuite/testsuite.go:116
1351            github.com/containerd/containerd/content/testsuite.makeTest.func1
1352            	C:/gopath/src/github.com/containerd/containerd/content/testsuite/testsuite.go:137
1353            testing.tRunner
1354            	C:/go/src/testing/testing.go:865
1355            runtime.goexit
1356            	C:/go/src/runtime/asm_amd64.s:1337
1357=== RUN   TestOCIExport
1358--- SKIP: TestOCIExport (0.00s)
1359

AkihiroSuda · 2019-04-13T13:13:09Z

Seems to break BuildKit, though it might not be problematic for containerd per se: moby/buildkit#936

full diff: grpc/grpc-go@v1.12.0...v1.12.2 - grpc/grpc-go#2074 transport/server: fix race between writing status and header - fix grpc/grpc-go#1972 Possible race sending headers from server while receiving message over size limit - grpc/grpc-go#2074 transport: account for user configured small io write buffer - fix grpc/grpc-go#2089 Server abruptly terminates connections if write buffer is small enough Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

full diff: grpc/grpc-go@v1.12.2...v1.20.1 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

fuweid · 2019-05-07T09:10:31Z

I think the failed case is flaky one. @thaJeztah could you repush it again?

fuweid · 2019-05-07T10:17:09Z

ping @AkihiroSuda is it the same issue to the buildkitd one?

crosbymichael · 2019-05-16T21:57:24Z

LGTM

dmcgowan

LGTM

thaJeztah mentioned this pull request Apr 6, 2019

Bump dependencies #3186

Merged

thaJeztah force-pushed the bump_grpc_1.19.1 branch from c0f4f17 to 45be0e1 Compare April 6, 2019 10:39

AkihiroSuda reviewed Apr 6, 2019

View reviewed changes

thaJeztah force-pushed the bump_grpc_1.19.1 branch from 45be0e1 to a890d4b Compare April 11, 2019 19:38

thaJeztah changed the title ~~bump google.golang.org/grpc v1.19.1~~ bump google.golang.org/grpc v1.20.0 Apr 11, 2019

thaJeztah commented Apr 11, 2019

View reviewed changes

vendor.conf Outdated Show resolved Hide resolved

thaJeztah force-pushed the bump_grpc_1.19.1 branch from a890d4b to 9c88ee1 Compare April 11, 2019 21:07

thaJeztah force-pushed the bump_grpc_1.19.1 branch from 9c88ee1 to e37ef00 Compare April 12, 2019 15:28

thaJeztah added 2 commits May 5, 2019 12:38

bump google.golang.org/grpc v1.20.1

2ed8e60

full diff: grpc/grpc-go@v1.12.2...v1.20.1 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the bump_grpc_1.19.1 branch from e37ef00 to 2ed8e60 Compare May 5, 2019 19:39

thaJeztah changed the title ~~bump google.golang.org/grpc v1.20.0~~ bump google.golang.org/grpc v1.20.1 May 5, 2019

dmcgowan approved these changes May 22, 2019

View reviewed changes

dmcgowan merged commit 25daa73 into containerd:master May 22, 2019

thaJeztah deleted the bump_grpc_1.19.1 branch May 22, 2019 19:38

jterry75 mentioned this pull request May 22, 2019

Update grpc to 1.20.1 microsoft/hcsshim#616

Merged

estesp mentioned this pull request Aug 27, 2019

[release/1.2] Cherry-pick update to gRPC 1.23.0 #3586

Merged

Conversation

thaJeztah commented Apr 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Apr 6, 2019

Uh oh!

thaJeztah commented Apr 6, 2019

Uh oh!

codecov-io commented Apr 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

AkihiroSuda Apr 6, 2019

Choose a reason for hiding this comment

Uh oh!

thaJeztah Apr 6, 2019

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Apr 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Apr 11, 2019

Uh oh!

Uh oh!

thaJeztah commented Apr 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AkihiroSuda commented Apr 13, 2019

Uh oh!

fuweid commented May 7, 2019

Uh oh!

fuweid commented May 7, 2019

Uh oh!

crosbymichael commented May 16, 2019

Uh oh!

dmcgowan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

thaJeztah commented Apr 6, 2019 •

edited

Loading

codecov-io commented Apr 6, 2019 •

edited

Loading

thaJeztah commented Apr 11, 2019 •

edited

Loading

thaJeztah commented Apr 12, 2019 •

edited

Loading