-
Notifications
You must be signed in to change notification settings - Fork 26
sync up with containerd 1.5 GA #47
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
Merged
AkihiroSuda
merged 1 commit into
containerd:master
from
Priyankasaggu11929:psaggu-bump-runc-version-to-v1_0_0_rc94
May 12, 2021
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
git commit -a -s --amend)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.
Hi @AkihiroSuda,
Thank you so much for the pointers. I have pushed a signed commit.
Re:
I ran the following two commands, to update runc & containerd modules:
And the rest of the stuff was added as part of the update.
I'm not sure if this is wrong way of bumping version. Could you please verify? Thank you!
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.
This may produce more cleaner result
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.
This repo no longer uses vendoring, so the
go mod vendorstep can be skipped; also runc is an indirect dependency of containerd, so no need togo get -uthat.I tried;
go.mod, and changev1.5.0-beta.3tov1.5.0go mod tidyThe above gave me this diff:
Make sure you're using a recent version of go though (I used go 1.16), because older versions of go may produce a different
go.mod(and include more//indirectlines)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.
Thank you @thaJeztah. To confirm, the go version on my local is
go1.16.3I followed the same steps as you shared above & the diff matches to the one linked.
One question:
As you pointed
runcis an indirect dependency of containerd. So, which means when I editedgo.modto changev1.5.0-beta.3tov1.5.0, followed bygo mod tidy, it should also update the runc version?In the above diff, the runc version is
v1.0.0-rc93.Thank you!
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.
Ah; I think the difference is that I tried it using current
masteras a starting point. Looks likego mod tidydoes not remove// indirectlines fromgo.modonce they're their (in some cases?)Can you try editing
go.mod, and remove all the// indirectlines? Should look like;And then run
go mod tidyagain?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.
Wondering if we also need to copy the relevant replace rules from containerd in plugin
go.modfiles though 🤔https://github.com/containerd/containerd/blob/v1.5.0/go.mod#L70-L77
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.
Following on the above:
go.modto exactly look like the one you shared abovego mod tidyI get the same diff as to the one you've linked couple of comments above.
This is my diff: diff.txt
I'm just confused if I need to run
go get -u github.com/containerd/containerd@v1.5.0as well after editing go.mod. Because that is when I see runc getting bumped to versionrunc v1.0.0-rc94Diff after the
go get -u github.com/containerd/containerd@v1.5.0command:diff2.txt
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.
containerd v1.5.0 does not have v1.0.0-rc94 yet as a dependency in go modules (it's still on rc93). I don't think we need to update beyond that, because the only parts of runc used by containerd (as module) are in the libcontainer/user package. There's some changes in that package, but they should not be important enough to "force" a more recent version (rc93 "or above" is fine).
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.
@thaJeztah, I've cleaned the changes (following the suggestions above) & updated the PR.
This discussion was really helpful. Thank you so much. :)