Allow configuring cgroup2 path #8085
Conversation
marvin-tigera
added
release-note-required
|
|
|
@amrut-asm the change lgtm. Would you also consider an operator change so that Talos users are not limited to using manifest installation? |
Sure |
|
@tomastigera The relevant change in operator tigera/operator#2919 |
|
|
||
| GlobalPinDir = DefaultBPFfsPath + "/tc/globals/" | ||
| ObjectDir = "/usr/lib/calico/bpf" | ||
| ) | ||
|
|
||
| func GetCgroupV2Path() string { |
There was a problem hiding this comment.
@tomastigera I think we will want to expose this using a proper FelixConfiguration parameter rather than an environment variable, no? Is there precedence for this in the BPF data plane code?
There was a problem hiding this comment.
I think it would be tricky as this needs to get into an init container. After some discussion at that time, this was an acceptable solution for this problem.
There was a problem hiding this comment.
Ah ok, this code isn't executed in felix proper?
It could in theory still be done via FelixConfiguration (in addition to this env var on the init container) - the operator already reads FelixConfiguration and uses it to configure various components. The flow would look like this:
- Set this field in FelixConfiguration
- tigera/operator reads FelixConfiguration, sets and env var for this container based on the value it finds
That would mean if we ever do need this value inside felix proper, it would already be available and we wouldn't need two separate configuration options. But I am not well positioned to say how likely that is.
There was a problem hiding this comment.
That sounds like a great suggestion to me 👍
There was a problem hiding this comment.
Thanks, I will give it a shot
|
Only one unrelated failure/flake in BPF tests |
…v3.27 [release-v3.27] Auto pick #8085: Allow custom cgroup2 path
Description
Currently, the mount-bpffs init container tries to create the folder /run/calico/cgroup for mounting the cgroupv2 fs
This causes issues on distros like Talos Linux as described in the issue #7892
This can be a configurable path instead
Related issues/PRs
#7892
Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*label.docs-pr-required: This change requires a change to the documentation that has not been completed yet.docs-completed: This change has all necessary documentation completed.docs-not-required: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*label.release-note-required: This PR has user-facing changes. Most PRs should have this label.release-note-not-required: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.