Skip to content

[WIP] update runc to 1.1.0-rc.1#107016

Closed
kolyshkin wants to merge 2 commits intokubernetes:masterfrom
kolyshkin:runc-1.1.0-rc.1
Closed

[WIP] update runc to 1.1.0-rc.1#107016
kolyshkin wants to merge 2 commits intokubernetes:masterfrom
kolyshkin:runc-1.1.0-rc.1

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin commented Dec 14, 2021 •
edited
Loading

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This updates vendored runc/libcontainer to 1.1.0-rc.1
(release notes at https://github.com/opencontainers/runc/releases/tag/v1.1.0-rc.1)
and google/cadvisor to a version in google/cadvisor#3031.

Changes in vendor are generated by (roughly):

  ./hack/pin-dependency.sh github.com/google/cadvisor=github.com/kolyshkin/cadvisor runc-1.1.0-rc1
  ./hack/pin-dependency.sh github.com/opencontainers/runc v1.1.0-rc.1
  ./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.

Changes in pkg/kubelet/cm are to adopt new runc 1.1 libcontainer/cgroups
APIs and build on its impovements. In particular:

  • simplify cgroup manager instantiation, using a new, easier way
    (i.e. libcontainers/cgroups/manager.New);

  • the fact that 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).

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

Which issue(s) this PR fixes:

none

Special notes for your reviewer:

With this PR, I am also trying to untangle the mess of both setting cgroups.Path
(or name+parent in case of systemd) and providing pre-calculated cgroups paths,
which confuses anyone reading the code.

Previously, this was required since cgroup managers were only calculating the paths
in Apply, and so using Set or Destroy right after instantiation was impossible. This was
solved in opencontainers/runc#3216 and so it is not required
to specify paths.

Does this PR introduce a user-facing change?

Switch to opencontainers/runc/libcontainer v1.1.

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

None

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Dec 14, 2021

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

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. 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. size/L Denotes a PR that changes 100-499 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 Dec 14, 2021
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Dec 14, 2021

@kolyshkin: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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 the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Dec 14, 2021
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Dec 14, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kolyshkin
To complete the pull request process, please assign liggitt after the PR has been reviewed.
You can assign the PR to them by writing /assign @liggitt in a comment when ready.

The full list of commands accepted by this bot can be found 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 area/apiserver area/cloudprovider area/code-generation area/dependency Issues or PRs related to dependency changes area/kubectl 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/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 14, 2021
@kolyshkin kolyshkin mentioned this pull request Dec 14, 2021
Closed
@kolyshkin kolyshkin mentioned this pull request Dec 14, 2021
Merged
@roycaihw
Copy link
Copy Markdown
Member

roycaihw commented Dec 14, 2021

/remove-sig api-machinery

@k8s-ci-robot k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Dec 14, 2021
@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Dec 15, 2021
@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Dec 15, 2021

/remove-sig api-machinery

@k8s-ci-robot k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Dec 15, 2021
@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Dec 15, 2021

/retest

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Dec 20, 2021
@kolyshkin kolyshkin mentioned this pull request Dec 20, 2021
Closed
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 21, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 21, 2021
@fedebongio
Copy link
Copy Markdown
Contributor

fedebongio commented Dec 21, 2021

/remove-sig api-machinery

@k8s-ci-robot k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Dec 21, 2021
@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Dec 21, 2021

/retest

@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Dec 21, 2021

/retest

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Dec 21, 2021
@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Dec 21, 2021

/retest

@kolyshkin
[WIP] update runc to 1.1.0-rc.1
f39b8e2 
This updates vendored runc/libcontainer to 1.1.0-rc.1,
and google/cadvisor to a version in
google/cadvisor#3031.

Changes in vendor are generated by (roughly):

  ./hack/pin-dependency.sh github.com/google/cadvisor=github.com/kolyshkin/cadvisor runc-1.1.0-rc1
  ./hack/pin-dependency.sh github.com/opencontainers/runc v1.1.0-rc.1
  ./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.

Changes in pkg/kubelet/cm are to adopt new runc 1.1 libcontainer/cgroups
APIs and build on its impovements. In particular:

 - simplify cgroup manager instantiation, using a new, easier way
   (i.e. libcontainers/cgroups/manager.New);

 - the fact that 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).

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

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin
WIP debug
ee22f6d 
Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Dec 22, 2021

/retest

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Dec 22, 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 Required Rerun command
pull-kubernetes-integration ee22f6d link true /test pull-kubernetes-integration
pull-kubernetes-verify ee22f6d link true /test pull-kubernetes-verify

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.

@leilajal
Copy link
Copy Markdown
Contributor

leilajal commented Jan 4, 2022

/remove-sig api-machinery

@k8s-ci-robot k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jan 4, 2022
@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented Jan 11, 2022

Closing in favor of #107149 which lay things out in a more digestible way.

@kolyshkin kolyshkin closed this Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brianpursley brianpursley Awaiting requested review from brianpursley

@caesarxuchao caesarxuchao Awaiting requested review from caesarxuchao

Assignees

No one assigned

Labels

area/apiserver area/cloudprovider area/code-generation area/dependency Issues or PRs related to dependency changes area/kubectl area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

SIG Node: code and documentation PRs
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@kolyshkin @k8s-ci-robot @roycaihw @fedebongio @leilajal @ritazh @enj @SergeyKanzhelev