feat: optional bpf mount #24161

frezbo · 2023-03-03T16:30:44Z

On distributions that already mount the bpffs filesystem at /sys/fs/bpf this is a good way to optionally disable the bpf mount init container and have no pods running with securityContext.privilged and also reduced the number of init containers that needs to be run.

This option was previously used when containerRuntime.integration=crio
helm value was set, since this is not just only specific to crio,
deprecate the containerRuntime.integration=crio option to skip
mounting bpffs filesystem in favour of bpf.autoMount.enabled which
is similar to how cgroupv2 mounts are disabled (cgroup.autoMount.enabled).

Eg: On Talos both cgroupv2 and bpffs filesystems are already mounted and using a values yaml like below helps reduce the number of init containers by a factor of two.

partial-values.yaml

cgroup:
  autoMount:
    enabled: false
  hostRoot: /sys/fs/cgroup
bpf:
  autoMount:
    enabled: false

aanm

@frezbo Thank you for the PR. Have you checked all other places that have the comment CRI-O already mounts the BPF filesystem in the install/kubernetes/cilium/templates/cilium-agent/daemonset.yaml file? Might make sense to also add the bpf.autoMount.enabled there.

frezbo · 2023-03-03T19:28:23Z

@frezbo Thank you for the PR. Have you checked all other places that have the comment CRI-O already mounts the BPF filesystem in the install/kubernetes/cilium/templates/cilium-agent/daemonset.yaml file? Might make sense to also add the bpf.autoMount.enabled there.

I can update that too, good point

frezbo · 2023-03-03T19:56:13Z

@aanm quick question, should i move from containerRuntime.integration and use bpf.autoMount.enabled instead and update the release notes or keep both?

frezbo · 2023-03-03T19:56:53Z

I've tested with the mount paths and the init container removed and all cilium connectivity test passes.

aanm · 2023-03-03T19:59:14Z

@aanm quick question, should i move from containerRuntime.integration and use bpf.autoMount.enabled instead and update the release notes or keep both?

@frezbo I would suggest to keep both and mark the containerRuntime.integration as deprecated because AFAIK the only reason why it's used is for the bpf mounting points.

frezbo · 2023-03-04T07:35:29Z

I've updated the PR and also removed some old references to un-used containerRuntime.integration values and removed the un-used containerRuntime.socketPath option. Also updated the upgrade notes to mention bpf.autoMount.enabled will be the default going forward and the old containerRuntime.integration will be deprecated. I hope this clears things up.

aanm · 2023-03-06T08:31:03Z

/test

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathConfig MonitorAggregation Checks that monitor aggregation flags send notifications

Failure Output

FAIL: Error creating resource /home/jenkins/workspace/Cilium-PR-K8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/k8s/manifests/l3-policy-demo.yaml: Cannot retrieve "cilium-bd9rl"'s policy revision: cannot get policy revision: ""

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

kaworu

Hi @frezbo and thanks for the PR! Patch LGTM but doc update seems missing.

Documentation/operations/upgrade.rst

qmonnet

Please also provide context for the deprecation in the commit description. As it stands, without reading the discussion in the PR, the change looks unrelated to the autoMount addition and it's hard to understand why it's part of the commit.

frezbo · 2023-03-06T18:12:56Z

Updated the extra documentation and commit message.

qmonnet

Looks good, thank you!

install/kubernetes/cilium/values.yaml.tmpl

kaworu

Thank you @frezbo, patch LGTM besides @qmonnet's comment.

On distributions that already mount the `bpffs` filesystem at `/sys/fs/bpf` this is a good way to optionally disable the bpf mount init container and have no pods running with `securityContext.privilged` and also reduced the number of init containers that needs to be run. This option was previously used when `containerRuntime.integration=crio` helm value was set, since this is not just only specific to crio, deprecate the `containerRuntime.integration=crio` option to skip mounting `bpffs` filesystem in favour of `bpf.autoMount.enabled` which is similar to how `cgroupv2` mounts are disabled (`cgroup.autoMount.enabled`). Eg: On [Talos](https://www.talos.dev/) both `cgroupv2` and `bpffs` filesystems are already mounted and using a values yaml like below helps reduce the number of init containers by a factor of two. `partial-values.yaml` ```yaml cgroup: autoMount: enabled: false hostRoot: /sys/fs/cgroup bpf: autoMount: enabled: false ``` Signed-off-by: Noel Georgi <git@frezbo.dev>

qmonnet

Perfect, thanks a lot!

qmonnet · 2023-03-07T16:14:29Z

/test

Relates: #24161 Signed-off-by: Tam Mach <tam.mach@cilium.io>

frezbo requested review from a team as code owners March 3, 2023 16:30

frezbo requested review from tommyp1ckles and kaworu March 3, 2023 16:30

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 3, 2023

github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Mar 3, 2023

aanm reviewed Mar 3, 2023

View reviewed changes

aanm self-requested a review March 3, 2023 19:16

aanm added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Mar 3, 2023

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 3, 2023

tommyp1ckles approved these changes Mar 4, 2023

View reviewed changes

frezbo force-pushed the feat/optional-bpf-mount branch from 2ec1a3f to 313c46b Compare March 4, 2023 07:33

frezbo requested a review from a team as a code owner March 4, 2023 07:33

frezbo requested a review from qmonnet March 4, 2023 07:33

aanm approved these changes Mar 6, 2023

View reviewed changes

kaworu requested changes Mar 6, 2023

View reviewed changes

Documentation/operations/upgrade.rst Show resolved Hide resolved

qmonnet requested changes Mar 6, 2023

View reviewed changes

frezbo force-pushed the feat/optional-bpf-mount branch from 313c46b to f04982a Compare March 6, 2023 18:12

qmonnet approved these changes Mar 7, 2023

View reviewed changes

qmonnet requested a review from kaworu March 7, 2023 10:34

qmonnet reviewed Mar 7, 2023

View reviewed changes

install/kubernetes/cilium/values.yaml.tmpl Show resolved Hide resolved

frezbo force-pushed the feat/optional-bpf-mount branch from f04982a to 2a82132 Compare March 7, 2023 13:55

qmonnet requested changes Mar 7, 2023

View reviewed changes

install/kubernetes/cilium/values.yaml.tmpl Outdated Show resolved Hide resolved

kaworu approved these changes Mar 7, 2023

View reviewed changes

kaworu added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/helm Impacts helm charts and user deployment experience labels Mar 7, 2023

frezbo force-pushed the feat/optional-bpf-mount branch from 2a82132 to 21c6023 Compare March 7, 2023 15:06

qmonnet approved these changes Mar 7, 2023

View reviewed changes

maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 7, 2023

borkmann merged commit 4274616 into cilium:master Mar 7, 2023

sayboras added a commit that referenced this pull request Apr 13, 2024

helm: Remove deprecated option containerRuntime.integration

ebfb8ee

Relates: #24161 Signed-off-by: Tam Mach <tam.mach@cilium.io>

sayboras mentioned this pull request Apr 13, 2024

helm: Remove deprecated option containerRuntime.integration #31942

Merged

sayboras added a commit that referenced this pull request Apr 13, 2024

helm: Remove deprecated option containerRuntime.integration

dafdba4

Relates: #24161 Signed-off-by: Tam Mach <tam.mach@cilium.io>

sayboras added a commit that referenced this pull request Apr 13, 2024

helm: Remove deprecated option containerRuntime.integration

303a619

Relates: #24161 Signed-off-by: Tam Mach <tam.mach@cilium.io>

github-merge-queue bot pushed a commit that referenced this pull request Apr 16, 2024

helm: Remove deprecated option containerRuntime.integration

e8b940f

Relates: #24161 Signed-off-by: Tam Mach <tam.mach@cilium.io>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: optional bpf mount #24161

feat: optional bpf mount #24161

frezbo commented Mar 3, 2023 •

edited

aanm left a comment

frezbo commented Mar 3, 2023

frezbo commented Mar 3, 2023

frezbo commented Mar 3, 2023

aanm commented Mar 3, 2023

frezbo commented Mar 4, 2023

aanm commented Mar 6, 2023 •

edited by maintainer-s-little-helper bot

Test Name

Failure Output

kaworu left a comment

qmonnet left a comment

frezbo commented Mar 6, 2023

qmonnet left a comment

kaworu left a comment

qmonnet left a comment

qmonnet commented Mar 7, 2023

feat: optional bpf mount #24161

feat: optional bpf mount #24161

Conversation

frezbo commented Mar 3, 2023 • edited

aanm left a comment

Choose a reason for hiding this comment

frezbo commented Mar 3, 2023

frezbo commented Mar 3, 2023

frezbo commented Mar 3, 2023

aanm commented Mar 3, 2023

frezbo commented Mar 4, 2023

aanm commented Mar 6, 2023 • edited by maintainer-s-little-helper bot

Test Name

Failure Output

kaworu left a comment

Choose a reason for hiding this comment

qmonnet left a comment

Choose a reason for hiding this comment

frezbo commented Mar 6, 2023

qmonnet left a comment

Choose a reason for hiding this comment

kaworu left a comment

Choose a reason for hiding this comment

qmonnet left a comment

Choose a reason for hiding this comment

qmonnet commented Mar 7, 2023

frezbo commented Mar 3, 2023 •

edited

aanm commented Mar 6, 2023 •

edited by maintainer-s-little-helper bot