Skip to content

[WIP][release/1.5] sync 1.5 branch go.mod with main / update runc v1.0.0#5638

Closed
dims wants to merge 3 commits intocontainerd:release/1.5from
dims:sync-go-mod-from-main
Closed

[WIP][release/1.5] sync 1.5 branch go.mod with main / update runc v1.0.0#5638
dims wants to merge 3 commits intocontainerd:release/1.5from
dims:sync-go-mod-from-main

Conversation

@dims
Copy link
Copy Markdown
Member

@dims dims commented Jun 22, 2021

Signed-off-by: Davanum Srinivas [email protected]

@k8s-ci-robot
Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Signed-off-by: Davanum Srinivas <[email protected]>
@dims dims force-pushed the sync-go-mod-from-main branch from e5ca35e to d045c10 Compare June 22, 2021 19:23
@dims
Copy link
Copy Markdown
Member Author

dims commented Jun 22, 2021

/test all

@k8s-ci-robot
Copy link
Copy Markdown

@dims: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-containerd-build d045c10 link /test pull-containerd-build
pull-containerd-node-e2e d045c10 link /test pull-containerd-node-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@dims dims changed the title sync 1.5 branch go.mod with main [WIP][release/1.5] sync 1.5 branch go.mod with main Jun 22, 2021
Signed-off-by: Davanum Srinivas <[email protected]>
@dims dims changed the title [WIP][release/1.5] sync 1.5 branch go.mod with main [WIP][release/1.5] sync 1.5 branch go.mod with main / update runc v1.0.0 Jun 22, 2021
Signed-off-by: Davanum Srinivas <[email protected]>
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jun 22, 2021

Build succeeded.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jun 22, 2021

Build succeeded.

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

Unless there's an important change in any of these that affects containerd, I'd rather keep this for v1.6. This is a huge diff in dependencies, and no changes in the runc code (except for removing dead code for windows, and some minor linting issues).

We keep release branches, so that we can backport specific (focussed) fixes, without having to include other changes from the main branch.

#5587 (comment)

This will also be updating to a new major version, so we should be careful picking this, unless we have a specific reason to update the dependency (I think this is coming in because this PR updates runc v1.0.0-rc95, but we only use a single package (libcontainer/user from runc), so we should no longer have a need to update it, unless there were changes in that package

#5587 (review)

I'm a bit cautious with some of the updates; we should be careful updating these in a patch release if there's no specific reason to require these changes. Especially with go modules, updating the version here will also force any consumer of containerd go module to update their dependencies to these newer versions, so there's a risk that containerd itself is not affected by the updated version (it may not be using the code), but consumers of containerd may be, and will now be forced to accept (possibly breaking) changes.

@kzys
Copy link
Copy Markdown
Member

kzys commented Jun 22, 2021

I agree with @thaJeztah. We should not update dependencies during patch releases except CVEs, to make patch releases safer for clients.

@dims
Copy link
Copy Markdown
Member Author

dims commented Jun 22, 2021

+1 @kzys @thaJeztah

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.

4 participants