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

make musl cgroupv2 identifying same as other target_envs #127

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

nrxus
Copy link

@nrxus nrxus commented Sep 21, 2023

I've been noticing an issue where creating a cgroup instance with an auto hierarchy fails and the error seems to come from:

cgroups-rs/src/cgroup.rs

Lines 556 to 560 in 8d29c19

if !fp.exists() {
if let Err(e) = std::fs::create_dir(fp.clone()) {
return Err(Error::with_cause(ErrorKind::FsError, e));
}
}

.I believe this is because cgroups-rs thinks that the machine is using cgroup v2 where it isn't. This is happening at a customer site so I can't be sure 100% but we do compile all of our binaries with musl as the target as we want static builds. The system is an older aws endpoint with a 4.14 kernel so I'd assume it's using a hybrid cgroup (where systemd makes a fake unified cgroup but it's still cgroup v1 in reality) instead of a pure cgroup v2 system so that may be where the musl specific cgroupv2 check fails. I think we can avoid this musl specifc code now that the crate is using v0.25 of nix as the needed constant was added in this version: nix-rust/nix@23f1787

This PR just removes the cfg gates so that musl targets hit the same code as other targets.

I also considered updating nix while i was at it since it is a couple of versions behind but I wasn't sure what your policy was for upgrading crates that cause an MSRV bump.

@Tim-Zhang Tim-Zhang requested a review from liubin October 23, 2023 11:58
@Tim-Zhang
Copy link
Member

@liubin I'm not familiar with this so please take a look at this.

Copy link
Member

@liubin liubin left a comment

Choose a reason for hiding this comment

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

@nrxus Now nix is 0.25.0, so it could compile.

@liubin liubin closed this Oct 24, 2023
@liubin liubin reopened this Oct 24, 2023
@liubin
Copy link
Member

liubin commented Oct 24, 2023

Close and reopen to trigger the CI.

@Tim-Zhang
Copy link
Member

@nrxus CI was failed, you can fix it follow the error message, thanks

When compiling for a musl target, use the same CGROUP2_SUPER_MAGIC
constant that we use for other linux targets

Signed-off-by: Andrés Medina <[email protected]>
@nrxus
Copy link
Author

nrxus commented Oct 24, 2023

@Tim-Zhang I have fixed the commit message to match the expectations from CI (as per the contributing guidelines)

Copy link
Member

@Tim-Zhang Tim-Zhang left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @nrxus

@Tim-Zhang Tim-Zhang merged commit 4f1fe13 into kata-containers:main Oct 25, 2023
6 checks passed
Tim-Zhang added a commit to Tim-Zhang/cgroups-rs that referenced this pull request Oct 25, 2023
@Tim-Zhang Tim-Zhang mentioned this pull request Oct 25, 2023
Tim-Zhang added a commit to Tim-Zhang/cgroups-rs that referenced this pull request Oct 25, 2023
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.

3 participants