Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Default cgroup_parent path no longer respects cgroup version #23896

Open
marvinchin opened this issue Aug 30, 2024 · 2 comments
Open

Default cgroup_parent path no longer respects cgroup version #23896

marvinchin opened this issue Aug 30, 2024 · 2 comments
Assignees
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/numa type/bug

Comments

@marvinchin
Copy link
Contributor

Nomad version

Nomad 1.7.7 (but issue also exists at tip)

Operating system and Environment details

Unix

Issue

The default value for the cgroup_parent config used to be based on the cgroup version inferred by nomad. However, in 1.7.7 this seems to have become a hardcoded value. This causes the default value to be incorrect on cgroup v1 systems.

The issue seems to be introduced by this commit: a4cc76b#diff-30de430fcaf9ccc69c864d5e78db52b7c2741787c3382839547d60d0c9000267L798-R797

I think reverting this to the original behavior should be straightforward just by using defaultParent in place of the hardcoded value:

func defaultParent() string {

Though, we might need to teach defaultParent to handle the "off" case better (right now it falls through to the default case, which assumes cgroup v2).

I don't quite know how to reason about the broader impact of this change, so I'm just reporting the issue for now.

Reproduction steps

On a cgroup v1 system:

# Start the agent
$ nomad agent -dev

# Show that the default value is incorrect
$ ./nomad node status -json -self | jq .CgroupParent
"nomad.slice"

Expected Result

Default value should be /nomad

Actual Result

Default value is nomad.slice

@marvinchin
Copy link
Contributor Author

I tried to work around this by setting the cgroup_parent field in the agent config, but that config field doesn't seem to be adhered to either (at least, I don't see it being merged in convertClientConfig:

func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) {

I'm happy to file a separate issue for this too, if you prefer.

@Juanadelacuesta
Copy link
Member

Hi @marvinchin! Thank you for reporting this issue, it looks like a regression with the introduction of NUMA support, we will be taking a look at it.

@Juanadelacuesta Juanadelacuesta added the stage/accepted Confirmed, and intend to work on. No timeline committment though. label Sep 4, 2024
@Juanadelacuesta Juanadelacuesta moved this from Needs Triage to Needs Roadmapping in Nomad - Community Issues Triage Sep 4, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/numa type/bug
Projects
Status: Needs Roadmapping
Development

No branches or pull requests

3 participants