New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Update to latest sha/tag of dependencies #4970
Conversation
de73b82
to
5da3950
Compare
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
5da3950
to
bb98d93
Compare
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
bb98d93
to
d611274
Compare
Build succeeded.
|
@mikebrow WDYT? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple comments thx @dims
github.com/containerd/go-runc v0.0.0-20200220073739-7016d3ce2328 | ||
github.com/containerd/imgcrypt v1.0.1 | ||
github.com/containerd/nri v0.0.0-20201007170849-eb1350a75164 | ||
github.com/containerd/go-runc v0.0.0-20201020171139-16b287bc67d0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs updated but probably need to update go-runc first.. to grab the latest runtime spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it's already on latest runtime spec (v1.0.2); https://github.com/containerd/go-runc/blob/16b287bc67d0/go.mod#L7
That said, it should currently pick the runtime we specified in our go.mod, because we use vendoring (in which case the vendored version is used, not "multi version" modules).
Also had a quick look, because some changes in this dependency update are related to #4526. Unfortunately those aren't the last commits (otherwise it would've been good to pick a slightly older commit, and include the "last few commits" in the related (#4526) pull request
github.com/containerd/ttrpc v1.0.2 | ||
github.com/containerd/typeurl v1.0.1 | ||
github.com/containerd/zfs v0.0.0-20200918131355-0a33824f23a2 | ||
github.com/containernetworking/plugins v0.8.6 | ||
github.com/containernetworking/plugins v0.9.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's do this one in a separate pr and put something in the description about containernetworking/plugins#540 which is a behavior change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! And thanks for the nice table!
Left some comments inline. Also wondering if we should try to split to individual commits; there's a lot of dependencies being updated, and in case there's a regression, having separate commits could potentially help doing a git bisect
As to your comments;
gogo/protobuf as it leads to generated code changes
Agreed; better to keep separate
google.golang.org/grpc because k/k will break with newer grpc (long story ... etcd needs to be updated first etcd-io/etcd#12470)
Also agreed; best kept separate in general, as it's potentially risky to update
we'll probably need to hold back syndtr/gocapability as one of the CI jobs is broken with it (as it adds CAP_PERFMON)
That one is tricky indeed; see opencontainers/runtime-spec#1071 and moby/moby#41563
There's also an existing PR to update it #4919
github.com/containerd/go-runc v0.0.0-20200220073739-7016d3ce2328 | ||
github.com/containerd/imgcrypt v1.0.1 | ||
github.com/containerd/nri v0.0.0-20201007170849-eb1350a75164 | ||
github.com/containerd/go-runc v0.0.0-20201020171139-16b287bc67d0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it's already on latest runtime spec (v1.0.2); https://github.com/containerd/go-runc/blob/16b287bc67d0/go.mod#L7
That said, it should currently pick the runtime we specified in our go.mod, because we use vendoring (in which case the vendored version is used, not "multi version" modules).
Also had a quick look, because some changes in this dependency update are related to #4526. Unfortunately those aren't the last commits (otherwise it would've been good to pick a slightly older commit, and include the "last few commits" in the related (#4526) pull request
github.com/imdario/mergo v0.3.10 | ||
github.com/klauspost/compress v1.11.3 | ||
github.com/hashicorp/go-multierror v1.1.0 | ||
github.com/imdario/mergo v0.3.11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a heads-up (diff looks ok); we had an issue reported with a panic in this dependency; not sure if the version we're using here currently is affected docker/cli#2916 (haven't dug into the issue yet, so really just a heads up)
github.com/urfave/cli v1.22.2 | ||
github.com/stretchr/testify v1.7.0 | ||
github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 | ||
github.com/tchap/go-patricia v2.3.0+incompatible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks to only include a new feature, no bug fixes, so not "strictly" needed if we want to reduce code-churn.
github.com/stretchr/testify v1.7.0 | ||
github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 | ||
github.com/tchap/go-patricia v2.3.0+incompatible | ||
github.com/urfave/cli v1.22.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there's a regression in v1.22.2 and up; see https://github.com/opencontainers/runc/blob/cb269306807dcfd7f742c8444aca2e638bd81550/go.mod#L22-L23
Might affect ctr
as well (if so, we may want to pin to the older version). I have a PR pending urfave/cli#1135, but time got the best of me to revisit it
Looks like we are lagging behind a bunch of dependencies. Do we want to bump all of them? some of them? In this PR, i've held back
gogo/protobuf
as it leads to generated code changesgoogle.golang.org/grpc
because k/k will break with newer grpc (long story ... etcd needs to be updated first Please upgrade grpc version used in etcd go.mod etcd-io/etcd#12470)syndtr/gocapability
as one of the CI jobs is broken with it (as it addsCAP_PERFMON
)Updating:
Not yet ready for the following as Kubernetes and etcd will break: