Skip to content

Conversation

Spimtav
Copy link
Contributor

@Spimtav Spimtav commented Nov 15, 2024

Changes proposed by this PR

cgroups v1 and v2 use completely different filenames and directory structures. Currently, this k8s-topgun container limits test can only handle cgroups v1 formats, and will fail on cgroups v2 containers due to "file not found" errors. The changes in this PR fix the implementation so that it can handle both cgroups v1 and v2-style formats, regardless of which is present.

Note: the changes in this PR were largely based on these fixes to the equivalent test in the testflight package:

Notes to reviewer

Testing this commit requires access to the k8s-topgun cluster, which is complex and time-consuming. Probably easier if i demo live.

Signed-off-by: claire tinati <mrt59@cornell.edu>
@Spimtav Spimtav requested a review from a team as a code owner November 15, 2024 19:18
@taylorsilva taylorsilva added the release/undocumented This didn't warrant being documented or put in release notes. label Nov 15, 2024
@Spimtav Spimtav added misc and removed release/undocumented This didn't warrant being documented or put in release notes. labels Nov 15, 2024
@taylorsilva taylorsilva added the release/undocumented This didn't warrant being documented or put in release notes. label Nov 15, 2024
@taylorsilva
Copy link
Member

tag war! Hi 👋

Copy link
Member

@taylorsilva taylorsilva left a comment

Choose a reason for hiding this comment

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

Preemptively approving since it's exactly the same as what fixed testflight

@Spimtav
Copy link
Contributor Author

Spimtav commented Nov 15, 2024

Dang that's some funny timing 😆. Sorry for stepping on your toes with the label a second ago!

@Spimtav Spimtav merged commit f033220 into master Nov 15, 2024
11 checks passed
@Spimtav Spimtav deleted the cgroup_refactor branch November 15, 2024 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
misc release/undocumented This didn't warrant being documented or put in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants