Skip to content
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

Add cgroupv2 support for linux shellouts #256

Merged
merged 9 commits into from
Feb 25, 2025
Merged

Conversation

ImanolBarba
Copy link
Contributor

@ImanolBarba ImanolBarba commented Feb 17, 2025

Description

This PR aims to implement cgroupv2 support for shellouts, allowing the user to specify under which cgroup should the command be executed.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • If Gemfile.lock has changed, I have used --conservative to do it and included the full output in the Description above.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

If `cgroup` attribute is provided, it will execute the command within a specific cgroupv2, creating it if missing

Signed-off-by: Imanol-Mikel Barba Sabariego <imanol@imanolbarba.net>
Signed-off-by: Imanol-Mikel Barba Sabariego <imanol@imanolbarba.net>
@ImanolBarba ImanolBarba requested review from a team as code owners February 17, 2025 12:00
Signed-off-by: Imanol-Mikel Barba Sabariego <imanol@imanolbarba.net>
Not only check there's _a_ cgroupv2 tree mounted, but check it's also in the expected path

Signed-off-by: Imanol-Mikel Barba Sabariego <imanol@imanolbarba.net>
Signed-off-by: Imanol-Mikel Barba Sabariego <imanol@imanolbarba.net>
Copy link
Contributor

@tpowell-progress tpowell-progress left a comment

Choose a reason for hiding this comment

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

Consider .match? instead to simplify the logic and reading of the code.

@tpowell-progress tpowell-progress added the Status: Waiting on Contributor A pull request that has unresolved requested actions from the author. label Feb 18, 2025
Signed-off-by: Imanol-Mikel Barba Sabariego <imanol@imanolbarba.net>
@ImanolBarba
Copy link
Contributor Author

Looks like I broke the Windows tests despite adding :linux_only, gonna attempt to repro and fix

ImanolBarba and others added 2 commits February 20, 2025 00:27
As cgroups is a linux concept, so it makes sense to run the cgroup tests only on this platform

Signed-off-by: Imanol-Mikel Barba Sabariego <imanol@ima.lol>
Signed-off-by: Imanol-Mikel Barba Sabariego <imanol@imanolbarba.net>
@ImanolBarba
Copy link
Contributor Author

@tpowell-progress ping for review :)

@jaymzh jaymzh merged commit 18db5df into chef:main Feb 25, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Waiting on Contributor A pull request that has unresolved requested actions from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants