Skip to content
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

chore(host-ctr): Update containerd dependency #3062

Closed
wants to merge 1 commit into from

Conversation

gazal-k
Copy link

@gazal-k gazal-k commented Apr 27, 2023

Issue number:

Related to emicklei/go-restful#489 & containerd/containerd#8271

Description of changes:
Updates containerd to the latest 1.6.x release; https://github.com/containerd/containerd/releases/tag/v1.6.20

Testing done:

$ go mod tidy
$ go mod vendor
$ go build ./...
$ ./host-ctr --help
NAME:
   host-ctr - manage host containers
   ...

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Copy link
Contributor

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for this PR!!

@@ -16,16 +16,6 @@ require (
)

replace (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to run go mod tidy to get things cleaned up since we don't need the multiline replace anymore. Go should re-format it for you as:

// containerd still uses the v1alpha2 APIs in k8s.io/cri-api.
// These were removed in v0.26 of cri-api resulting in a dependency conflict.
// TODO: Remove this `replace` when containerd uses v1 of the cri-api
replace k8s.io/cri-api => k8s.io/cri-api v0.25.5

@@ -359,8 +359,7 @@ github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25Kn
github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153 h1:yUdfgN0XgIJw7foRItutHYUIhlcKzcSf5vDpdhQAKTc=
github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc=
github.com/emicklei/go-restful v0.0.0-20170410110728-ff4f55a20633/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
github.com/emicklei/go-restful v2.16.0+incompatible h1:rgqiKNjTnFQA6kkhFe16D8epTksy9HQ1MyrbDXSdYhM=
github.com/emicklei/go-restful v2.16.0+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, this line looks like it's adding github.com/emicklei/go-restful v2.9.5 back into the mix.

And it does look like v2.9.5 of go restful is still being brought in somewhere in containerd:
https://github.com/containerd/containerd/blob/2806fc1057397dbaeefbea0e4e17bddfbd388f38/go.sum#L352-L353

Where and how this is being pulled in on the upstream requires more investigation upstream.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😞 it seems to be present even in 1.7.0. https://github.com/containerd/containerd/blob/v1.7.0/go.sum

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm yeah, just noticed that as well: containerd/containerd#8168

We'll want to follow that upstream. But for now, I'll close this and we can keep an eye on what containerd does

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gazal-k - if you want to open a subsequent PR for just the containerd upgrades (without replacing go-restful), I'd be happy to review that

@jpmcb jpmcb closed this Apr 28, 2023
@gazal-k gazal-k deleted the host-ctr-containerd branch April 28, 2023 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants