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.

Sign up for GitHub

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

Jump to bottom

vendor: update github.com/opencontainers/runc #92862

Merged
merged 2 commits into from
Jul 12, 2020
Merged

vendor: update github.com/opencontainers/runc #92862

merged 2 commits into from
Jul 12, 2020

Conversation

giuseppe
Copy link
Member

@giuseppe giuseppe commented Jul 7, 2020

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind bug

What this PR does / why we need it:

when the systemd cgroup manager is used, controllers not handled by systemd are created manually afterwards.

libcontainer didn't correctly cleanup these cgroups that were leakedon cgroup v1.

Which issue(s) this PR fixes:

Fixes #92766

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.:

@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/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jul 7, 2020
@k8s-ci-robot k8s-ci-robot requested review from dchen1107, matthyx and a team July 7, 2020 12:45
@k8s-ci-robot k8s-ci-robot added the area/dependency Issues or PRs related to dependency changes label Jul 7, 2020
@giuseppe
Copy link
Member Author

giuseppe commented Jul 7, 2020

/sig-node

@k8s-ci-robot k8s-ci-robot added area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 7, 2020
@giuseppe giuseppe mentioned this pull request Jul 7, 2020
Closed
@giuseppe giuseppe force-pushed the cgroup-fix-leaks branch from b6bb1b7 to 52bf6e5 Compare July 7, 2020 13:46
@liggitt
Copy link
Member

liggitt commented Jul 7, 2020

vendor mechanics lgtm

cc @kubernetes/sig-node-pr-reviews for runc and ebpf changes

@sjenning
Copy link
Contributor

sjenning commented Jul 7, 2020

/assign

@sjenning
Copy link
Contributor

sjenning commented Jul 7, 2020

/lgtm

needs top level approver

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 7, 2020
@sjenning
Copy link
Contributor

sjenning commented Jul 7, 2020

/assign @liggitt

@liggitt
Copy link
Member

liggitt commented Jul 7, 2020

there's a gofmt error that needs fixing

@sjenning
Copy link
Contributor

sjenning commented Jul 7, 2020

ah yes 52bf6e5#diff-20728a5825a9e01d8bf61ad43ab2b7e4R128

Verifying verify-gofmt.sh

+++ Running case: verify.gofmt 
+++ working dir: /home/prow/go/src/k8s.io/kubernetes
+++ command: bash "hack/make-rules/../../hack/verify-gofmt.sh"
diff -u ./pkg/kubelet/dockershim/cm/container_manager_linux.go.orig ./pkg/kubelet/dockershim/cm/container_manager_linux.go
--- ./pkg/kubelet/dockershim/cm/container_manager_linux.go.orig	2020-07-07 15:02:04.734181675 +0000
+++ ./pkg/kubelet/dockershim/cm/container_manager_linux.go	2020-07-07 15:02:04.734181675 +0000
@@ -123,8 +123,8 @@
 		Parent: "/",
 		Name:   name,
 		Resources: &configs.Resources{
-			Memory:     int64(memoryLimit),
-			MemorySwap: -1,
+			Memory:      int64(memoryLimit),
+			MemorySwap:  -1,
 			SkipDevices: true,
 			Devices: []*configs.DeviceRule{
 				{

Run ./hack/update-gofmt.sh
+++ exit code: 1
+++ error: 1
�[0;31mFAILED�[0m   verify-gofmt.sh	24s

@giuseppe giuseppe force-pushed the cgroup-fix-leaks branch from 52bf6e5 to 1289531 Compare July 7, 2020 18:22
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 7, 2020
@mrunalp
Copy link
Contributor

mrunalp commented Jul 7, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 7, 2020
@liggitt
Copy link
Member

liggitt commented Jul 7, 2020

/approve
/retest

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, liggitt

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

The pull request process is described here

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

@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 7, 2020
kolyshkin
kolyshkin approved these changes Jul 7, 2020
View reviewed changes
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM (not a maintainer)

@haircommander
Copy link
Contributor

/retest

@kolyshkin kolyshkin mentioned this pull request Jul 8, 2020
Merged
@haircommander
Copy link
Contributor

hm is prow stalled or is 24 hours waiting for it to retest normal

@liggitt liggitt added this to the v1.19 milestone Jul 8, 2020
@liggitt liggitt added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Jul 8, 2020
@k8s-ci-robot k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jul 8, 2020
@liggitt liggitt added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. labels Jul 8, 2020
giuseppe added 2 commits July 9, 2020 09:37
@giuseppe
vendor: update github.com/opencontainers/runc
c655a5b 
when the systemd cgroup manager is used, controllers not handled by
systemd are created manually afterwards.
libcontainer didn't correctly cleanup these cgroups that were leaked
on cgroup v1.

Closes: kubernetes#92766

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe
kubelet: skip setting the devices cgroup
0d2a493 
use the new libcontainer feature of skipping setting the devices
cgroup.  This is necessary on cgroup v2 to avoid leaking a eBPF
program every time the cgroup is re-configured.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe giuseppe force-pushed the cgroup-fix-leaks branch from 1289531 to 0d2a493 Compare July 9, 2020 07:38
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 9, 2020
@giuseppe
Copy link
Member Author

giuseppe commented Jul 9, 2020

had to rebase. Pushed a new version

@liggitt
Copy link
Member

liggitt commented Jul 9, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 9, 2020
@liggitt
Copy link
Member

liggitt commented Jul 9, 2020

/retest

2 similar comments
@haircommander
Copy link
Contributor

/retest

@haircommander
Copy link
Contributor

/retest

This was referenced Jul 10, 2020
Merged
Closed
@k8s-ci-robot k8s-ci-robot merged commit 63926cf into kubernetes:master Jul 12, 2020
@skeeey skeeey mentioned this pull request Aug 17, 2020
Closed
@kolyshkin kolyshkin mentioned this pull request May 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kolyshkin kolyshkin kolyshkin approved these changes

@dchen1107 dchen1107 Awaiting requested review from dchen1107

@matthyx matthyx Awaiting requested review from matthyx

Assignees

@mrunalp mrunalp

@sjenning sjenning

@liggitt liggitt

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/node Categorizes an issue or PR as relevant to SIG Node. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.19
Development

Successfully merging this pull request may close these issues.

kubelet consistently timing out on attempting to Destroy cgroups
7 participants
@giuseppe @liggitt @sjenning @mrunalp @k8s-ci-robot @haircommander @kolyshkin