deps: update runc to 1.1.0-rc.1 #3031
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @kolyshkin. Thanks for your PR. I'm waiting for a google member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
kolyshkin
force-pushed
the
runc-1.1.0-rc1
branch
from
c6698fa
to
d8900b9
Compare
|
/ok-to-test |
This was referenced Dec 14, 2021
|
Note I am also testing this in kubernetes: kubernetes/kubernetes#107016. |
kolyshkin
added a commit
to kolyshkin/kubernetes
that referenced
this pull request
Dec 22, 2021
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 <kolyshkin@gmail.com>
|
LGTM |
iwankgb
approved these changes
Dec 27, 2021
|
This looks good, but there's a few conflicts in go.{sum,mod} files |
The changes to the code are: - resctrl: use intelrtd.Root instead of GetIntelRdtPath (which was removed); - resctrl: test: set cgroups.TestMode = true in init(), explain why; - container/libcontainer/helpers: modify NewCgroupManager for the updated fs[2].NewManager API. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
force-pushed
the
runc-1.1.0-rc1
branch
from
d8900b9
to
86ff3d9
Compare
|
Rebased; PTAL @bobbypage |
|
LGTM, thanks! |
ehashman
pushed a commit
to ehashman/kubernetes
that referenced
this pull request
Feb 10, 2022
There's no need to call m.Update (which will create another instance of libcontainer cgroup manager, convert all the resources and then set them). All this is already done here, except for Set(). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: rm dup code Commit ecd6361 added setting PidsLimit to Create and Update. Commit bce9d5f added setting PidsLimit to m.toResources. Now, PidsLimit is assigned twice. Remove the duplicate. Fixes: bce9d5f Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: ToSystemd: nit Remove the second condition as it can't ever be true (this was obviously written by a C programmer). Fixes: b230fb8 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm/cgroup_manager: simplify setting hugetlb Commit 79be8be made hugetlb settings optional if cgroup v2 is used and hugetlb is not available, fixing issue 92933. Note at that time this was only needed for v2, because for v1 the resources were set one-by-one, and only for supported resources. Commit d312ef7 switched the code to using Set from runc/libcontainer cgroups manager, and expanded the check to cgroup v1 as well. Move this check earlier, to inside m.toResources, so instead of converting all hugetlb resources from ResourceConfig to libcontainers's Resources.HugetlbLimit, and then setting it to nil, we can skip the conversion entirely if hugetlb is not supported, thus not doing the work that is not needed. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: move common code to libctCgroupConfig Instead of doing (almost) the same thing from the three different methods (Create, Update, Destroy), move the functionality to libctCgroupConfig, replacing updateSystemdCgroupInfo. The needResources bool is needed because we do not need resources during Destroy, so we skip the unneeded resource conversion. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: simplify enforceExistingCgroup 1. Move the rl == nil check to before we dereference it. 2. Remove Exists check before Update, since it is complicated, and Update will return a meaningful error anyway in case the cgroup does not exist. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> deps: update runc to 1.1.0 This updates vendored runc/libcontainer to 1.1.0, and google/cadvisor to a version updated to runc 1.1.0-rc1 (google/cadvisor#3031). Changes in vendor are generated by (roughly): ./hack/pin-dependency.sh github.com/google/cadvisor a3b5f4f6c501d5a4ac67530bc973a05c1ebd7d8c ./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. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
bwplotka
pushed a commit
to bwplotka/kubernetes
that referenced
this pull request
Feb 24, 2022
There's no need to call m.Update (which will create another instance of libcontainer cgroup manager, convert all the resources and then set them). All this is already done here, except for Set(). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: rm dup code Commit ecd6361 added setting PidsLimit to Create and Update. Commit bce9d5f added setting PidsLimit to m.toResources. Now, PidsLimit is assigned twice. Remove the duplicate. Fixes: bce9d5f Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: ToSystemd: nit Remove the second condition as it can't ever be true (this was obviously written by a C programmer). Fixes: b230fb8 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm/cgroup_manager: simplify setting hugetlb Commit 79be8be made hugetlb settings optional if cgroup v2 is used and hugetlb is not available, fixing issue 92933. Note at that time this was only needed for v2, because for v1 the resources were set one-by-one, and only for supported resources. Commit d312ef7 switched the code to using Set from runc/libcontainer cgroups manager, and expanded the check to cgroup v1 as well. Move this check earlier, to inside m.toResources, so instead of converting all hugetlb resources from ResourceConfig to libcontainers's Resources.HugetlbLimit, and then setting it to nil, we can skip the conversion entirely if hugetlb is not supported, thus not doing the work that is not needed. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: move common code to libctCgroupConfig Instead of doing (almost) the same thing from the three different methods (Create, Update, Destroy), move the functionality to libctCgroupConfig, replacing updateSystemdCgroupInfo. The needResources bool is needed because we do not need resources during Destroy, so we skip the unneeded resource conversion. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: simplify enforceExistingCgroup 1. Move the rl == nil check to before we dereference it. 2. Remove Exists check before Update, since it is complicated, and Update will return a meaningful error anyway in case the cgroup does not exist. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> deps: update runc to 1.1.0 This updates vendored runc/libcontainer to 1.1.0, and google/cadvisor to a version updated to runc 1.1.0-rc1 (google/cadvisor#3031). Changes in vendor are generated by (roughly): ./hack/pin-dependency.sh github.com/google/cadvisor a3b5f4f6c501d5a4ac67530bc973a05c1ebd7d8c ./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. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
bwplotka
pushed a commit
to bwplotka/kubernetes
that referenced
this pull request
Mar 4, 2022
There's no need to call m.Update (which will create another instance of libcontainer cgroup manager, convert all the resources and then set them). All this is already done here, except for Set(). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: rm dup code Commit ecd6361 added setting PidsLimit to Create and Update. Commit bce9d5f added setting PidsLimit to m.toResources. Now, PidsLimit is assigned twice. Remove the duplicate. Fixes: bce9d5f Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: ToSystemd: nit Remove the second condition as it can't ever be true (this was obviously written by a C programmer). Fixes: b230fb8 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm/cgroup_manager: simplify setting hugetlb Commit 79be8be made hugetlb settings optional if cgroup v2 is used and hugetlb is not available, fixing issue 92933. Note at that time this was only needed for v2, because for v1 the resources were set one-by-one, and only for supported resources. Commit d312ef7 switched the code to using Set from runc/libcontainer cgroups manager, and expanded the check to cgroup v1 as well. Move this check earlier, to inside m.toResources, so instead of converting all hugetlb resources from ResourceConfig to libcontainers's Resources.HugetlbLimit, and then setting it to nil, we can skip the conversion entirely if hugetlb is not supported, thus not doing the work that is not needed. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: move common code to libctCgroupConfig Instead of doing (almost) the same thing from the three different methods (Create, Update, Destroy), move the functionality to libctCgroupConfig, replacing updateSystemdCgroupInfo. The needResources bool is needed because we do not need resources during Destroy, so we skip the unneeded resource conversion. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> pkg/kubelet/cm: simplify enforceExistingCgroup 1. Move the rl == nil check to before we dereference it. 2. Remove Exists check before Update, since it is complicated, and Update will return a meaningful error anyway in case the cgroup does not exist. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> deps: update runc to 1.1.0 This updates vendored runc/libcontainer to 1.1.0, and google/cadvisor to a version updated to runc 1.1.0-rc1 (google/cadvisor#3031). Changes in vendor are generated by (roughly): ./hack/pin-dependency.sh github.com/google/cadvisor a3b5f4f6c501d5a4ac67530bc973a05c1ebd7d8c ./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. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
added a commit
to kolyshkin/kubernetes
that referenced
this pull request
Mar 23, 2022
This updates vendored runc/libcontainer to 1.1.0, and google/cadvisor to a version updated to runc 1.1.0-rc1 (google/cadvisor#3031). 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. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
runc 1.1.0-rc.1 was just released. This PR updates vendored runc/libcontainer
and modifies the code according to the changes in the libcontainer API.
Since the final 1.1.0 is supposed to be very similar (if not identical), I guess this could
be merged as is (if no issues are found).
The changes to the code are:
resctrl: use intelrtd.Root instead of GetIntelRdtPath (which was
removed);
resctrl: test: set cgroups.TestMode = true in TestGetPids (since
by default cgroup drivers check they open cgroupfs files);
container/libcontainer/helpers: modify NewCgroupManager for the
updated fs[2].NewManager API.