Skip to content

[release/1.6] Bump google.golang.org/grpc to v1.58.3#9408

Merged
estesp merged 2 commits intocontainerd:release/1.6from
austinvazquez:update-grpc
Dec 1, 2023
Merged

[release/1.6] Bump google.golang.org/grpc to v1.58.3#9408
estesp merged 2 commits intocontainerd:release/1.6from
austinvazquez:update-grpc

Conversation

@austinvazquez
Copy link
Copy Markdown
Member

Issue: N/A

Description:

Trying my hand to see if I can make some progress on updating gRPC in release/1.6.

Backports #7325
Backports #9281
Related to #9285

Testing:
Fork CI run: https://github.com/austinvazquez/containerd/actions/runs/6946804473

Kazuyoshi Kato and others added 2 commits November 21, 2023 16:16
The package has multiple improvements and bug fixes.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
(cherry picked from commit d063597)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
Upgrade google.golang.org/grpc to v1.58.3 in preparation for
upgrading OTel, which has a dependency on the latest version.

See also: containerd#9281.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
(cherry picked from commit 856d105)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
@k8s-ci-robot
Copy link
Copy Markdown

Hi @austinvazquez. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@austinvazquez austinvazquez marked this pull request as ready for review November 21, 2023 17:28
@austinvazquez
Copy link
Copy Markdown
Member Author

austinvazquez commented Nov 21, 2023

@thaJeztah, I wanted to get your thoughts on this. Should the change also include intermediate updates as well? e.g. should I backport the update of google.golang.org/grpc to v1.56.3?

@estesp
Copy link
Copy Markdown
Member

estesp commented Nov 21, 2023

curious why @thaJeztah hit some issue or needed to change the replace rules, but this set of commits seems to be passing CI without those changes? Other than that this seems like the right set of updates to get to latest gRPC with the CVE fixes

@thaJeztah
Copy link
Copy Markdown
Member

Good question; I just tried on my branch if I put back the replace rules that I removed, and that resulted in;

go mod tidy
github.com/containerd/containerd/services/server imports
	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc tested by
	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.test imports
	google.golang.org/grpc/interop imports
	golang.org/x/oauth2/google imports
	cloud.google.com/go/compute/metadata: ambiguous import: found package cloud.google.com/go/compute/metadata in multiple modules:
	cloud.google.com/go v0.81.0 (/go/pkg/mod/cloud.google.com/go@v0.81.0/compute/metadata)
	cloud.google.com/go/compute/metadata v0.2.3 (/go/pkg/mod/cloud.google.com/go/compute/metadata@v0.2.3)

Full log;

Details
go mod tidy
go: downloading github.com/opencontainers/go-digest v1.0.0
go: downloading github.com/gogo/protobuf v1.3.2
go: downloading google.golang.org/grpc v1.58.3
go: downloading github.com/containerd/typeurl v1.0.2
go: downloading github.com/gogo/googleapis v1.3.2
go: downloading github.com/containerd/fifo v1.0.0
go: downloading github.com/moby/sys/signal v0.6.0
go: downloading github.com/opencontainers/image-spec v1.1.0-rc2.0.20221005185240-3a7f492d3f1b
go: downloading github.com/sirupsen/logrus v1.9.3
go: downloading github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417
go: downloading github.com/opencontainers/selinux v1.10.1
go: downloading golang.org/x/sync v0.3.0
go: downloading github.com/containerd/ttrpc v1.1.2
go: downloading github.com/Microsoft/hcsshim v0.9.10
go: downloading github.com/containerd/continuity v0.3.0
go: downloading golang.org/x/sys v0.13.0
go: downloading github.com/klauspost/compress v1.15.9
go: downloading github.com/Microsoft/go-winio v0.5.2
go: downloading github.com/google/go-cmp v0.5.9
go: downloading gotest.tools/v3 v3.5.0
go: downloading github.com/containerd/aufs v1.0.0
go: downloading github.com/containerd/zfs v1.1.0
go: downloading github.com/containerd/go-runc v1.0.0
go: downloading github.com/docker/go-metrics v0.0.1
go: downloading github.com/urfave/cli v1.22.1
go: downloading github.com/coreos/go-systemd/v22 v22.3.2
go: downloading github.com/pelletier/go-toml v1.9.5
go: downloading github.com/containerd/console v1.0.3
go: downloading github.com/docker/go-units v0.4.0
go: downloading github.com/containerd/go-cni v1.1.6
go: downloading github.com/containerd/cgroups v1.0.4
go: downloading github.com/stretchr/testify v1.8.4
go: downloading github.com/AdaLogics/go-fuzz-headers v0.0.0-20210715213245-6c3934b029d8
go: downloading go.etcd.io/bbolt v1.3.7
go: downloading k8s.io/cri-api v0.25.0
go: downloading github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c
go: downloading golang.org/x/net v0.17.0
go: downloading github.com/containernetworking/cni v1.1.1
go: downloading k8s.io/component-base v0.22.5
go: downloading k8s.io/klog/v2 v2.30.0
go: downloading k8s.io/utils v0.0.0-20210930125809-cb0fa318a74b
go: downloading k8s.io/apimachinery v0.22.5
go: downloading github.com/containerd/log v0.1.0
go: downloading github.com/prometheus/client_golang v1.11.1
go: downloading github.com/moby/sys/mountinfo v0.6.2
go: downloading github.com/opencontainers/runc v1.1.5
go: downloading github.com/containerd/imgcrypt v1.1.4
go: downloading github.com/containerd/nri v0.1.1
go: downloading github.com/containernetworking/plugins v1.1.1
go: downloading github.com/davecgh/go-spew v1.1.1
go: downloading github.com/fsnotify/fsnotify v1.4.9
go: downloading github.com/intel/goresctrl v0.2.0
go: downloading github.com/vishvananda/netlink v1.1.1-0.20210330154013-f5de75959ad5
go: downloading k8s.io/client-go v0.22.5
go: downloading google.golang.org/protobuf v1.31.0
go: downloading github.com/tchap/go-patricia v2.2.6+incompatible
go: downloading github.com/emicklei/go-restful/v3 v3.10.1
go: downloading k8s.io/apiserver v0.22.5
go: downloading k8s.io/api v0.22.5
go: downloading github.com/moby/sys/symlink v0.2.0
go: downloading github.com/hashicorp/go-multierror v1.1.1
go: downloading github.com/moby/locker v1.0.1
go: downloading github.com/google/uuid v1.3.0
go: downloading github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
go: downloading github.com/emicklei/go-restful v2.16.0+incompatible
go: downloading github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
go: downloading go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.28.0
go: downloading github.com/imdario/mergo v0.3.12
go: downloading github.com/containerd/btrfs v1.0.0
go: downloading go.opentelemetry.io/otel v1.3.0
go: downloading go.opentelemetry.io/otel/trace v1.3.0
go: downloading go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.3.0
go: downloading go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.3.0
go: downloading go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.3.0
go: downloading go.opentelemetry.io/otel/sdk v1.3.0
go: downloading github.com/pkg/errors v0.9.1
go: downloading google.golang.org/genproto v0.0.0-20200224152610-e50cd9704f63
go: downloading github.com/prometheus/procfs v0.7.3
go: downloading github.com/cpuguy83/go-md2man/v2 v2.0.0
go: downloading github.com/pmezard/go-difflib v1.0.0
go: downloading gopkg.in/yaml.v3 v3.0.1
go: downloading github.com/godbus/dbus/v5 v5.0.6
go: downloading github.com/cilium/ebpf v0.7.0
go: downloading github.com/golang/protobuf v1.5.3
go: downloading github.com/go-logr/logr v1.2.2
go: downloading github.com/beorn7/perks v1.0.1
go: downloading github.com/cespare/xxhash/v2 v2.2.0
go: downloading github.com/prometheus/client_model v0.2.0
go: downloading github.com/prometheus/common v0.30.0
go: downloading github.com/vishvananda/netns v0.0.0-20210104183010-2eb08e3e575f
go: downloading github.com/containers/ocicrypt v1.1.3
go: downloading github.com/google/gofuzz v1.2.0
go: downloading sigs.k8s.io/yaml v1.2.0
go: downloading github.com/spf13/pflag v1.0.5
go: downloading gopkg.in/inf.v0 v0.9.1
go: downloading github.com/moby/spdystream v0.2.0
go: downloading go.opencensus.io v0.24.0
go: downloading github.com/hashicorp/errwrap v1.1.0
go: downloading github.com/json-iterator/go v1.1.12
go: downloading gopkg.in/yaml.v2 v2.4.0
go: downloading github.com/go-logr/stdr v1.2.2
go: downloading go.opentelemetry.io/proto/otlp v0.11.0
go: downloading go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.3.0
go: downloading github.com/mistifyio/go-zfs/v3 v3.0.1
go: downloading github.com/russross/blackfriday/v2 v2.0.1
go: downloading go.uber.org/goleak v1.1.12
go: downloading github.com/onsi/ginkgo/v2 v2.1.3
go: downloading github.com/onsi/gomega v1.17.0
go: downloading github.com/onsi/ginkgo v1.16.4
go: downloading github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153
go: downloading github.com/satori/go.uuid v1.2.0
go: downloading github.com/stretchr/objx v0.5.0
go: downloading gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
go: downloading github.com/frankban/quicktest v1.11.3
go: downloading github.com/matttproud/golang_protobuf_extensions v1.0.4
go: downloading golang.org/x/crypto v0.14.0
go: downloading golang.org/x/term v0.13.0
go: downloading sigs.k8s.io/structured-merge-diff/v4 v4.1.2
go: downloading github.com/modern-go/reflect2 v1.0.2
go: downloading github.com/cncf/xds/go v0.0.0-20230607035331-e9ce68804cb4
go: downloading golang.org/x/oauth2 v0.10.0
go: downloading github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da
go: downloading github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd
go: downloading github.com/grpc-ecosystem/grpc-gateway v1.16.0
go: downloading github.com/cenkalti/backoff/v4 v4.1.2
go: downloading github.com/shurcooL/sanitized_anchor_name v1.0.0
go: downloading github.com/kr/pretty v0.2.1
go: downloading golang.org/x/text v0.13.0
go: downloading github.com/miekg/pkcs11 v1.1.1
go: downloading github.com/stefanberger/go-pkcs11uri v0.0.0-20201008174630-78d3cae3a980
go: downloading gopkg.in/square/go-jose.v2 v2.5.1
go: downloading go.mozilla.org/pkcs7 v0.0.0-20200128120323-432b2356ecb1
go: downloading github.com/blang/semver v3.5.1+incompatible
go: downloading github.com/envoyproxy/protoc-gen-validate v1.0.2
go: downloading golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac
go: downloading github.com/kr/text v0.2.0
go: downloading cloud.google.com/go/compute/metadata v0.2.3
go: downloading google.golang.org/appengine v1.6.7
go: downloading github.com/nxadm/tail v1.4.8
go: downloading cloud.google.com/go/compute v1.21.0
go: downloading gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7
go: downloading cloud.google.com/go v0.81.0
github.com/containerd/containerd/services/server imports
	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc tested by
	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.test imports
	google.golang.org/grpc/interop imports
	golang.org/x/oauth2/google imports
	cloud.google.com/go/compute/metadata: ambiguous import: found package cloud.google.com/go/compute/metadata in multiple modules:
	cloud.google.com/go v0.81.0 (/go/pkg/mod/cloud.google.com/go@v0.81.0/compute/metadata)
	cloud.google.com/go/compute/metadata v0.2.3 (/go/pkg/mod/cloud.google.com/go/compute/metadata@v0.2.3)

The starting point of my PR had some differences though, and some dependencies were updated since, so I wonder if any of those changes (or perhaps changes elsewhere) fixed that issue, or perhaps the newer grpc version made that problem go away?

diff --git a/old-go.mod b/go.mod
index ac7e55731..b764f7e07 100644
--- a/old-go.mod
+++ b/go.mod
@@ -16,7 +16,7 @@ require (
        github.com/containerd/go-runc v1.0.0
        github.com/containerd/imgcrypt v1.1.4
        github.com/containerd/log v0.1.0
-       github.com/containerd/nri v0.1.0
+       github.com/containerd/nri v0.1.1
        github.com/containerd/ttrpc v1.1.2
        github.com/containerd/typeurl v1.0.2
        github.com/containerd/zfs v1.1.0
@@ -63,9 +63,9 @@ require (
        go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.3.0
        go.opentelemetry.io/otel/sdk v1.3.0
        go.opentelemetry.io/otel/trace v1.3.0
-       golang.org/x/net v0.13.0
+       golang.org/x/net v0.17.0
        golang.org/x/sync v0.1.0
-       golang.org/x/sys v0.10.0
+       golang.org/x/sys v0.13.0
        google.golang.org/grpc v1.50.1
        google.golang.org/protobuf v1.28.1
        gotest.tools/v3 v3.5.0
@@ -87,7 +87,7 @@ require (
        github.com/cilium/ebpf v0.7.0 // indirect
        github.com/containers/ocicrypt v1.1.3 // indirect
        github.com/cpuguy83/go-md2man/v2 v2.0.0 // indirect
-       github.com/emicklei/go-restful v2.9.5+incompatible // indirect
+       github.com/emicklei/go-restful v2.16.0+incompatible // indirect
        github.com/go-logr/logr v1.2.2 // indirect
        github.com/go-logr/stdr v1.2.2 // indirect
        github.com/godbus/dbus/v5 v5.0.6 // indirect
@@ -117,10 +117,10 @@ require (
        go.opencensus.io v0.23.0 // indirect
        go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.3.0 // indirect
        go.opentelemetry.io/proto/otlp v0.11.0 // indirect
-       golang.org/x/crypto v0.11.0 // indirect
+       golang.org/x/crypto v0.14.0 // indirect
        golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f // indirect
-       golang.org/x/term v0.10.0 // indirect
-       golang.org/x/text v0.11.0 // indirect
+       golang.org/x/term v0.13.0 // indirect
+       golang.org/x/text v0.13.0 // indirect
        golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect
        google.golang.org/appengine v1.6.7 // indirect
        google.golang.org/genproto v0.0.0-20220502173005-c8bf987b8c21 // indirect

@thaJeztah
Copy link
Copy Markdown
Member

Should the change also include intermediate updates as well? e.g. should I backport the update of google.golang.org/grpc to v1.56.3?

It's not strictly needed; the reason I usually prefer (where possible) to backport changes from master;

  1. To preserve commit-messages; commit messages can have relevant information related to the update.
  2. To provide traces to related changes on master (cherry-picked from <original commit>); this is also sometimes related to trace back the original changes and PR which may contain imporant information (why was the update needed?)
  3. Related to the above; cherry-picking individual updates "forces" you to look at the original PR: was the module updated in isolation, or were related changes needed (which may be in code, or CI).
  4. To a lesser extent; make sure that updates go into main before they go into release branches.
  5. To accredit the original contributor in release branches as well.

Especially 3. can be quite important; missing the related changes may not always result in a compilation failure, or even a failure in CI (perhaps tests are not yet in the release branch). Having to look at the original PR reduces the risk that such changes are overlooked. 4. can be relevant as well (although it becomes more complicated the more an (LTS) branch diverges from main); I've have multiple cases where a fix went into a release branch, but was forgotten in the main branch. As a result the "next release" effectively had a regression ("Oh! This fix never went into main").

@austinvazquez
Copy link
Copy Markdown
Member Author

Good question; I just tried on my branch if I put back the replace rules that I removed, and that resulted in;

I hit this as well and needed to follow https://stackoverflow.com/a/74689094/633961 to resolve the ambiguous import error.

It's not strictly needed; the reason I usually prefer (where possible) to backport changes from master;

Thanks that makes sense. I think this would check those boxes, but let me know if you would like more paper trail here. Happy to help run it down more.

  1. can be relevant as well (although it becomes more complicated the more an (LTS) branch diverges from main);

+1, definitely felt that here with the dependencies. It eventually shook out but was not a simple apply diff in the beginning. 😅

@thaJeztah
Copy link
Copy Markdown
Member

I hit this as well and needed to follow https://stackoverflow.com/a/74689094/633961 to resolve the ambiguous import error.

Oh! That's a good one, yes, I recall now that I had other repositories where I ran into something like this.

The TL;DR is that go mod tidy (or go modules overall) can be pretty bad at removing dependencies from go.mod. Some of that is because it does not / cannot "downgrade" dependencies; when a minimum version of a dependency is "bumped" because of another dependency (which may introduce other transitive dependencies), and that dependency is removed, it now considers the "updated version" to be a manually specified version, so even if it may now be able to downgrade the minimum version (and remove indirect dependencies), those transitive will never be removed.

Sometimes the trick is to remove all //indirect dependencies from go.mod, then re-run go mod tidy to have it re-resolve dependencies. After that manually revert downgrades.

@mikebrow
Copy link
Copy Markdown
Member

/ok-to-test

@estesp
Copy link
Copy Markdown
Member

estesp commented Nov 29, 2023

Opened #9438 which forced gRPC update to 1.58.x; leaving in draft as I think we should merge this and then I can rebase as I was trying to isolate the OTel backport from release/1.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants