Skip to content

Use cgroups.AddProc() for cgroups v1#5738

Merged
AkihiroSuda merged 1 commit intocontainerd:mainfrom
thaJeztah:simplify_cgroup_add
Mar 25, 2022
Merged

Use cgroups.AddProc() for cgroups v1#5738
AkihiroSuda merged 1 commit intocontainerd:mainfrom
thaJeztah:simplify_cgroup_add

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Jul 15, 2021

depends on containerd/cgroups#200 containerd/cgroups#202

All occurrences only passed a PID, so we can use this utility to make the code more symmetrical with their cgroups v2 counterparts.

@k8s-ci-robot
Copy link
Copy Markdown

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

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jul 15, 2021

Build succeeded.

@thaJeztah thaJeztah closed this Jul 15, 2021
@thaJeztah thaJeztah reopened this Jul 15, 2021
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jul 15, 2021

Build succeeded.

@thaJeztah
Copy link
Copy Markdown
Member Author

Failure looks consistent across multiple jobs;

=== FAIL: . TestShimOOMScore (unknown)
panic: test timed out after 10m0s

Don't think that's related to my changes in containerd/cgroups, but this PR brings in some other changes, so perhaps that's related.

@thaJeztah
Copy link
Copy Markdown
Member Author

opened #5740 to test just the cgroups update

@thaJeztah thaJeztah force-pushed the simplify_cgroup_add branch from fd3c11d to b44b9b3 Compare July 15, 2021 11:36
@thaJeztah
Copy link
Copy Markdown
Member Author

Ah.. I'm stupid; I didn't copy the replace rule to the client/integration package, and that of course doesn't inherit the replace.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jul 15, 2021

Build succeeded.

@thaJeztah thaJeztah force-pushed the simplify_cgroup_add branch 2 times, most recently from c8a5bbd to fd7470e Compare July 15, 2021 16:41
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jul 15, 2021

Build succeeded.

@thaJeztah thaJeztah force-pushed the simplify_cgroup_add branch from fd7470e to d21b5bc Compare March 23, 2022 13:59
@thaJeztah thaJeztah changed the title [WIP] Use cgroups.AddProc() for cgroups v1 Use cgroups.AddProc() for cgroups v1 Mar 23, 2022
All occurrences only passed a PID, so we can use this utility to make
the code more symmetrical with their cgroups v2 counterparts.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the simplify_cgroup_add branch from d21b5bc to c091d48 Compare March 23, 2022 14:25
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Mar 23, 2022

Build succeeded.

@estesp
Copy link
Copy Markdown
Member

estesp commented Mar 23, 2022

Is this still supposed to be in draft or ready to review now @thaJeztah ?

@thaJeztah thaJeztah marked this pull request as ready for review March 23, 2022 15:50
@thaJeztah
Copy link
Copy Markdown
Member Author

Ready now! Was waiting for CI to finish after I rebased 😅

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Copy Markdown
Member Author

@AkihiroSuda this LGTY?

Copy link
Copy Markdown
Member

@kzys kzys left a comment

Choose a reason for hiding this comment

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

LGTM

Ping @AkihiroSuda

@AkihiroSuda AkihiroSuda merged commit eaf7929 into containerd:main Mar 25, 2022
@thaJeztah thaJeztah deleted the simplify_cgroup_add branch March 25, 2022 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants