Skip to content

vendor: bump runc to v1.0.1#103743

Merged
k8s-ci-robot merged 2 commits intokubernetes:masterfrom
kolyshkin:runc-1.0.1
Jul 23, 2021
Merged

vendor: bump runc to v1.0.1#103743
k8s-ci-robot merged 2 commits intokubernetes:masterfrom
kolyshkin:runc-1.0.1

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin commented Jul 16, 2021

What type of PR is this?

/kind bug

What this PR does / why we need it:

Update runc/libcontainer to v1.0.1, fixing a few bugs, including the one that got 1.0.0 bump reverted
(see #103483).

For runc release notes, see

Which issue(s) this PR fixes:

Fixes: #102676

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

none

kolyshkin added 2 commits July 16, 2021 12:36
@kolyshkin
vendor: bump runc to 1.0.1
eb5df86 
The commands used were (roughly):

	hack/pin-dependency.sh github.com/opencontainers/runc v1.0.1
	hack/lint-dependencies.sh
	# Follow its recommendations.
	hack/pin-dependency.sh github.com/cilium/ebpf v0.6.2
	hack/pin-dependency.sh github.com/opencontainers/selinux v1.8.2
	hack/pin-dependency.sh github.com/sirupsen/logrus v1.8.1
	# Recheck.
	hack/lint-dependencies.sh
	GO111MODULE=on go mod edit -dropreplace github.com/willf/bitset
	hack/update-vendor.sh
	# Recheck.
	hack/lint-dependencies.sh
	hack/update-internal-modules.sh
	# Recheck.
	hack/lint-dependencies.sh

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin
kubelet/cm: don't set Devices
e5b434e 
Since runc 1.0.0 it is now sufficient to have SkipDevices: true.

Signed-off-by: Kir Kolyshkin <[email protected]>
@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/bug Categorizes issue or PR as related to a bug. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 16, 2021
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jul 16, 2021

Hi @kolyshkin. Thanks for your PR.

I'm waiting for a kubernetes 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.

@k8s-ci-robot k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 16, 2021
@k8s-ci-robot k8s-ci-robot requested review from a team, andrewsykim and deads2k July 16, 2021 19:51
@k8s-ci-robot k8s-ci-robot added area/apiserver area/cloudprovider area/dependency Issues or PRs related to dependency changes area/kubelet sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 16, 2021
@kolyshkin kolyshkin mentioned this pull request Jul 16, 2021
Closed
ehashman
ehashman reviewed Jul 16, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@ehashman ehashman left a comment

Choose a reason for hiding this comment

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

/ok-to-test
/triage accepted
/priority critical-urgent

/test pull-kubernetes-node-crio-cgrpv2-e2e
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv1
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv2
/test pull-kubernetes-node-kubelet-serial

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 16, 2021
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 23, 2021
@dims
Copy link
Copy Markdown
Member

dims commented Jul 23, 2021

the CI failures look ok (agree with @endocrimes). We would need an ok from sig-node approver like @liggitt said.

thanks,
Dims

@palnabarun palnabarun mentioned this pull request Jul 23, 2021
Merged
@mrunalp
Copy link
Copy Markdown
Contributor

mrunalp commented Jul 23, 2021

/approve

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jul 23, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, kolyshkin, liggitt, mrunalp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dims
Copy link
Copy Markdown
Member

dims commented Jul 23, 2021

/skip
/hold cancel
(@mrunalp is a node approver)

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 23, 2021
@dims
Copy link
Copy Markdown
Member

dims commented Jul 23, 2021

@kolyshkin please file a cherry-pick to 1.22 release branch.

@rphillips
Copy link
Copy Markdown
Member

rphillips commented Jul 23, 2021

/test pull-kubernetes-e2e-gce-ubuntu-containerd

@rphillips rphillips mentioned this pull request Jul 23, 2021
Merged
@kolyshkin kolyshkin mentioned this pull request Jul 23, 2021
Closed
@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Jul 23, 2021

@kolyshkin please file a cherry-pick to 1.22 release branch.

Created: #103884

For 1.21, see #103746

@ehashman
Copy link
Copy Markdown
Member

ehashman commented Jul 23, 2021

flake #103512

/test pull-kubernetes-integration

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jul 23, 2021 •
edited
Loading

@kolyshkin: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Rerun command
pull-kubernetes-node-kubelet-serial-crio-cgroupv1 e5b434e link /test pull-kubernetes-node-kubelet-serial-crio-cgroupv1
pull-kubernetes-node-kubelet-serial-crio-cgroupv2 e5b434e link /test pull-kubernetes-node-kubelet-serial-crio-cgroupv2
pull-kubernetes-node-kubelet-serial e5b434e link /test pull-kubernetes-node-kubelet-serial

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.

@rphillips
Copy link
Copy Markdown
Member

rphillips commented Jul 23, 2021

/test pull-kubernetes-integration

@k8s-ci-robot k8s-ci-robot merged commit 47e1df8 into kubernetes:master Jul 23, 2021
k8s-ci-robot added a commit that referenced this pull request Jul 26, 2021
@k8s-ci-robot
Merge pull request #103882 from rphillips/automated-cherry-pick-of-#1…
f9ff525 
…03743-upstream-release-1.22

Automated cherry pick of #103743: vendor: bump runc to 1.0.1
This was referenced Aug 9, 2021
Closed
Closed
Closed
@manugupt1
Copy link
Copy Markdown
Contributor

manugupt1 commented Aug 9, 2021

/lgtm

@hakman
Copy link
Copy Markdown
Member

hakman commented Aug 10, 2021

I see this change is being backported to all supported releases, so just wanted to mention that we noticed some instability in kOps periodic tests. When systemd cgroups were used with older versions of containerd (without runc 1.0.1) some random tests were failing. This was fixed by switching to a containerd version using runc 1.0.1, but would be nice to mention it in the release notes.

If you scroll back to 07-30, you should see how tests started failing once this was merged and then went back to green once kOps started using containerd 1.4.9:
https://testgrid.k8s.io/kops-misc#kops-aws-misc-amd64-conformance

@chrischdi chrischdi mentioned this pull request Aug 10, 2021
Closed
@odinuge
Copy link
Copy Markdown
Member

odinuge commented Aug 10, 2021

Hi,

Sorry that I haven't had time to review before now. Have had some time AFK after delivering my thesis.

I see this change is being backported to all supported releases, so just wanted to mention that we noticed some instability in kOps periodic tests. When systemd cgroups were used with older versions of containerd (without runc 1.0.1) some random tests were failing. This was fixed by switching to a containerd version using runc 1.0.1, but would be nice to mention it in the release notes.

If you scroll back to 07-30, you should see how tests started failing once this was merged and then went back to green once kOps started using containerd 1.4.9:
https://testgrid.k8s.io/kops-misc#kops-aws-misc-amd64-conformance

Yeah. K8s with systemd cgroup driver is still "broken" with this PR, since it does freeze the control groups, possibly causing strange behavior. And yeah, when doing that, runc pre v1.0.1 will permanently freeze containers, causing even more strange behavior. More in depth analysis here: #102676 (comment).

But thanks for reporting @hakman.

opencontainers/runc#3143 should mitigate this problem, but we still need to get it fixed in runc and then get a release there, before merging it here... And yeah, that can potentially take some time.... :/

@odinuge odinuge mentioned this pull request Aug 10, 2021
Closed
This was referenced Aug 26, 2021
Merged
Closed
@liggitt liggitt mentioned this pull request Aug 27, 2021
Merged
@liggitt liggitt added the kind/regression Categorizes issue or PR as related to a regression from a prior release. label Apr 27, 2022
@liggitt liggitt removed the kind/regression Categorizes issue or PR as related to a regression from a prior release. label Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BenTheElder BenTheElder BenTheElder left review comments

@andrewsykim andrewsykim Awaiting requested review from andrewsykim

@deads2k deads2k Awaiting requested review from deads2k

+1 more reviewer

@ehashman ehashman ehashman left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@dims dims

@endocrimes endocrimes

@odinuge odinuge

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver area/cloudprovider area/dependency Issues or PRs related to dependency changes area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/node Categorizes an issue or PR as relevant to SIG Node. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

SIG Auth
Archived in project
SIG Node: code and documentation PRs
Archived in project

Milestone

v1.22

Development

Successfully merging this pull request may close these issues.

Pod stuck in ContainerCreating: Unit ...slice already exists