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.
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
[release/1.5] upgrade containerd/cgroups to v1.0.3 #6961
Conversation
Skipping CI for Draft Pull Request. |
d52cb44
to
34374b2
Compare
runtime/opts/opts_linux.go
Outdated
@@ -29,7 +29,7 @@ func WithNamespaceCgroupDeletion(ctx context.Context, i *namespaces.DeleteInfo) | |||
if cgroups.Mode() == cgroups.Unified { | |||
cg, err := cgroupsv2.LoadManager("/sys/fs/cgroup", i.Name) | |||
if err != nil { | |||
if err == cgroupsv2.ErrCgroupDeleted { | |||
if err == cgroups.ErrCgroupDeleted { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because of containerd/cgroups#201
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v2: remove errors that are never returned
Unlike v1, the v2 package never returns these errors.
Do we need this if
check at all then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Seems it only returns ErrInvalidGroupPath. Let me update the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was the discussion on #5739
(sorry for short comment; typing from my phone)
This commit brings the resource leak fix below. containerd/cgroups#212 - The original upgrade PR was containerd#6498. I didn't cherry-pick the commit due to conflicts. - ErrCgroupDeleted check is removed since LoadManager won't return the error (see containerd#5739) and the variable was removed in 1.0.3. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This commit brings the resource leak fix below.
containerd/cgroups#212
The upgrade PR in main was #6498. I didn't cherry-pick the commit due
to conflicts.
Signed-off-by: Kazuyoshi Kato katokazu@amazon.com