Skip to content

Dep bump to runc 1.1.0, cadvisor 0.44.0#109029

Merged
k8s-ci-robot merged 2 commits intokubernetes:masterfrom
ehashman:runc-1.1.0
Mar 29, 2022
Merged

Dep bump to runc 1.1.0, cadvisor 0.44.0#109029
k8s-ci-robot merged 2 commits intokubernetes:masterfrom
ehashman:runc-1.1.0

Conversation

@ehashman
Copy link
Copy Markdown
Member

@ehashman ehashman commented Mar 25, 2022

What type of PR is this?

/kind cleanup
/priority important-soon
/sig node
/cc @odinuge @dims @mrunalp

What this PR does / why we need it:

Non-draft rebased follow-up to #107149

This updates vendored runc/libcontainer to 1.1.0,
and google/cadvisor to a version updated to runc 1.1.0
(google/cadvisor#3048).

Changes in vendor are generated by (roughly):

    ./hack/pin-dependency.sh github.com/google/cadvisor v0.44.0
    ./hack/pin-dependency.sh github.com/opencontainers/runc v1.1.0
    ./hack/update-vendor.sh
    ./hack/lint-dependencies.sh # And follow all its recommendations.
    ./hack/update-vendor.sh
    ./hack/update-internal-modules.sh
    ./hack/lint-dependencies.sh # Re-check everything again.

The changes (mostly in pkg/kubelet/cm) are there to adopt changed
runc 1.1 API, and simplify things a bit. In particular:

  1. simplify cgroup manager instantiation, using a new, easier way of
    libcontainers/cgroups/manager.New;

  2. replace libcontainerAdapter with a boolean variable (all it did
    was passing on whether systemd manager should be used);

  3. trivial change due to removed cgroupfs.HugePageSizes and added
    cgroups.HugePageSizes();

  4. do not calculate cgroup paths in update / destroy, since libcontainer
    cgroup managers now calculate the paths upon creation (previously,
    they were doing that only in Apply, so using e.g. Set or Destroy right
    after creation was impossible without specifying paths).

We currently still calculate cgroup paths in Exists -- this is to be
addressed separately.

Which issue(s) this PR fixes:

none

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Update runc to 1.1.0
Update cadvisor to 0.44.0

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

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/node Categorizes an issue or PR as relevant to SIG Node. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 25, 2022
@k8s-ci-robot k8s-ci-robot added area/dependency Issues or PRs related to dependency changes area/kubectl area/kubelet area/test sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Mar 25, 2022
ehashman
ehashman commented Mar 25, 2022
View reviewed changes
Comment thread go.mod Outdated
Copy link
Copy Markdown
Member Author

@ehashman ehashman Mar 25, 2022

Choose a reason for hiding this comment

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

This one appears to be unavoidable... runc has a direct dep on it https://github.com/opencontainers/runc/blob/86d6898f3052acba1ebcf83aa2eae3f6cc5fb471/go.mod#L17

Copy link
Copy Markdown
Member

@dims dims Mar 25, 2022

Choose a reason for hiding this comment

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

Ack

Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin Mar 27, 2022

Choose a reason for hiding this comment

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

Yes, libseccomp-golang did not made a release since 2019, and the released version is no longer adequate (mostly due to seccomp-notify functionality which runc now supports/requires)

@ehashman
Copy link
Copy Markdown
Member Author

ehashman commented Mar 25, 2022

/test pull-kubernetes-node-kubelet-serial-containerd

@ehashman
Copy link
Copy Markdown
Member Author

ehashman commented Mar 25, 2022

/milestone v1.24

@k8s-ci-robot k8s-ci-robot added this to the v1.24 milestone Mar 25, 2022
@dims
Copy link
Copy Markdown
Member

dims commented Mar 25, 2022

The dependency change LGTM. Please assign to Jordan when this is green as i don't have root OWNERS approval

thanks!

@bobbypage
Copy link
Copy Markdown
Member

bobbypage commented Mar 25, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 25, 2022
@bobbypage
Copy link
Copy Markdown
Member

bobbypage commented Mar 25, 2022

Thanks for rebasing this @ehashman and @kolyshkin for the update!

@bobbypage bobbypage mentioned this pull request Mar 25, 2022
Closed
odinuge
odinuge approved these changes Mar 25, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@odinuge odinuge left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good!

/lgtm

@ehashman
Copy link
Copy Markdown
Member Author

ehashman commented Mar 28, 2022

/test pull-kubernetes-node-e2e-containerd

@ehashman
Copy link
Copy Markdown
Member Author

ehashman commented Mar 28, 2022

Flake was #109082 - filed bug

@liggitt
Copy link
Copy Markdown
Member

liggitt commented Mar 28, 2022 •
edited
Loading

/approve
for dependency mechanics, since linking to the cgroups package containing the init() panic possibility is limited to kubelet in production binaries... if at all possible, it would be good to pick up 1.1.1 with that bit dropped before release

/lgtm
based on node reviewer lgtms above (@bobbypage @mrunalp)

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 28, 2022
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Mar 28, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ehashman, liggitt, mrunalp, odinuge

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 28, 2022
@liggitt liggitt added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 28, 2022
ehashman and others added 2 commits March 28, 2022 16:23
@ehashman @kolyshkin
deps: update runc to 1.1.0
07af1ba 
This updates vendored runc/libcontainer to 1.1.0,
and google/cadvisor to a version updated to runc 1.1.0
(google/cadvisor#3048).

Changes in vendor are generated by (roughly):

        ./hack/pin-dependency.sh github.com/google/cadvisor v0.44.0
        ./hack/pin-dependency.sh github.com/opencontainers/runc v1.1.0
        ./hack/update-vendor.sh
        ./hack/lint-dependencies.sh # And follow all its recommendations.
        ./hack/update-vendor.sh
        ./hack/update-internal-modules.sh
        ./hack/lint-dependencies.sh # Re-check everything again.

Co-Authored-By: Kir Kolyshkin <[email protected]>
@kolyshkin @ehashman
pkg/kubelet: changes to update runc to 1.1.0
37761a3 
The changes (mostly in pkg/kubelet/cm) are there to adopt changed
runc 1.1 API, and simplify things a bit. In particular:

1. simplify cgroup manager instantiation, using a new, easier way of
   libcontainers/cgroups/manager.New;

2. replace libcontainerAdapter with a boolean variable (all it did
   was passing on whether systemd manager should be used);

3. trivial change due to removed cgroupfs.HugePageSizes and added
    cgroups.HugePageSizes();

4. do not calculate cgroup paths in update / destroy, since libcontainer
   cgroup managers now calculate the paths upon creation (previously,
   they were doing that only in Apply, so using e.g. Set or Destroy right
   after creation was impossible without specifying paths).

We currently still calculate cgroup paths in Exists -- this is to be
addressed separately.

Co-Authored-By: Elana Hashman <[email protected]>
@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 28, 2022
@mrunalp
Copy link
Copy Markdown
Contributor

mrunalp commented Mar 28, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 28, 2022
@ehashman
Copy link
Copy Markdown
Member Author

ehashman commented Mar 29, 2022

/retest-required

@kolyshkin kolyshkin mentioned this pull request Mar 29, 2022
Closed
@kolyshkin
Copy link
Copy Markdown
Contributor

kolyshkin commented Mar 29, 2022

OK, runc 1.1.1 is now out, and this is a PR to replace this one (no changes other than rebase and use runc 1.1.1 instead of 1.1.0): #109091

@k8s-ci-robot k8s-ci-robot merged commit fe8a663 into kubernetes:master Mar 29, 2022
@kolyshkin kolyshkin mentioned this pull request Mar 29, 2022
Merged
@pacoxu pacoxu mentioned this pull request Mar 31, 2022
Closed
This was referenced Apr 2, 2022
Closed
Closed
@stevbev stevbev mentioned this pull request Apr 2, 2022
Closed
3 tasks
@github-actions github-actions Bot mentioned this pull request Apr 5, 2022
Open
@PushkarJ PushkarJ mentioned this pull request Apr 5, 2022
Closed
@rkojedzinszky
Copy link
Copy Markdown

rkojedzinszky commented Apr 21, 2022

Will it be backported to 1.22 and 1.23 branches?

@111andre111 111andre111 mentioned this pull request Sep 22, 2022
Closed
@wackxu wackxu mentioned this pull request May 6, 2023
Open
@zafs23 zafs23 mentioned this pull request Jul 25, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@liggitt liggitt liggitt left review comments

@dims dims Awaiting requested review from dims

+3 more reviewers

@mrunalp mrunalp mrunalp approved these changes

@kolyshkin kolyshkin kolyshkin left review comments

@odinuge odinuge odinuge approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mrunalp mrunalp

@bobbypage bobbypage

@liggitt liggitt

@odinuge odinuge

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/kubectl area/kubelet area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. 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 Denotes a PR that will be considered when it comes time to generate release notes. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

SIG Node: code and documentation PRs
Archived in project

Milestone

v1.24

Development

Successfully merging this pull request may close these issues.

9 participants

@ehashman @dims @bobbypage @liggitt @mrunalp @k8s-ci-robot @kolyshkin @rkojedzinszky @odinuge