Skip to content

feat(cilium): stage bandwidth manager with BBR#2608

Open
devantler wants to merge 4 commits into
mainfrom
codex/cilium-bbr-stage-2607
Open

feat(cilium): stage bandwidth manager with BBR#2608
devantler wants to merge 4 commits into
mainfrom
codex/cilium-bbr-stage-2607

Conversation

@devantler

@devantler devantler commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

🤖 Generated by the Daily AI Engineer

Summary

  • add a Hetzner-scoped Kustomize component for Cilium bandwidth manager + host-only BBR
  • keep the component unreferenced by default
  • add an offline render regression test and run it in the manifest CI gate

Why

Cilium 1.20 is now staged, so the first performance slice from #2029 is actionable. Enabling bandwidth management directly would roll every Cilium agent because rollOutCiliumPods: true; this PR lands the configuration behind a coarse default-off gate before any production activation.

Safety and rollout

  • committed local and production renders remain byte-for-byte unchanged
  • temporary opt-in rendering proves bandwidthManager.enabled: true, bandwidthManager.bbr: true, bandwidthManager.bbrHostNamespaceOnly: true, and existing WireGuard settings remain present
  • comments document the Linux 5.18+ prerequisite, qdisc and agent-roll impact, low-traffic activation, verification, and rollback
  • bpf.masquerade and netkit stay out of scope
  • pod BBR stays disabled until host-routing prerequisite Stage eBPF host-routing prerequisites for pod BBR #2609 merges and soaks
  • the component stays default-off until device selection is homogeneous under Normalize Cilium devices before bandwidth-manager activation #2610
  • activation and live soak remain a separate follow-up after both prerequisites close

Validation

  • python3 -m unittest scripts/tests/test_cilium_bandwidth_manager_component.py -v
  • ksail workload validate
  • ksail --config ksail.prod.yaml workload validate
  • ksail workload scan --framework nsa --compliance-threshold 85
  • python3 scripts/validate-embedded-json.py
  • python3 scripts/validate-naming.py
  • actionlint .github/workflows/ci.yaml
  • git diff --check
  • local render SHA-256 matches origin/main: deed105d7ab3babe2bf4eefa2b4102c5b9869a40bb1b15942bd72f9dde28d8f3
  • production render SHA-256 matches origin/main: 237da4ca4bc6f4c7af28e4ec38fadc9cc6b724ee14282a15e35aa9909f00ee7c

Follow-ups

Fixes #2607
Part of #2029

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Engineer

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a3880bbbc3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/tests/test_cilium_bandwidth_manager_component.py Outdated

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Engineer

@codex review

Fresh exact-head review requested after resolving the initial findings and adding the host-only BBR activation guard.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: d8c9c59ea3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/tests/test_cilium_bandwidth_manager_component.py Outdated
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Engineer

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a1f812017

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/tests/test-cilium-bandwidth-manager-component.sh
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Engineer

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 5719b84709

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@devantler devantler marked this pull request as ready for review July 13, 2026 08:47
@devantler devantler added this pull request to the merge queue Jul 13, 2026
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Engineer

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

@devantler I'll review the current state of the PR now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8c31cfe0-db04-4776-abad-e7e2c7924ff0

📥 Commits

Reviewing files that changed from the base of the PR and between 91341a4 and 5719b84.

📒 Files selected for processing (5)
  • .github/workflows/ci.yaml
  • k8s/providers/hetzner/infrastructure/controllers/cilium/components/bandwidth-manager-bbr/kustomization.yaml
  • k8s/providers/hetzner/infrastructure/controllers/kustomization.yaml
  • scripts/tests/test-cilium-bandwidth-manager-component.sh
  • tests/cilium-bandwidth-manager-bbr/kustomization.yaml
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (1)
k8s/**/kustomization.yaml

📄 CodeRabbit inference engine (AGENTS.md)

Keep the Kustomize build file named exactly kustomization.yaml (kustomize.config.k8s.io); do not rename it for Flux resources.

Files:

  • k8s/providers/hetzner/infrastructure/controllers/cilium/components/bandwidth-manager-bbr/kustomization.yaml
  • k8s/providers/hetzner/infrastructure/controllers/kustomization.yaml
🧠 Learnings (1)
📚 Learning: 2026-07-01T21:13:36.950Z
Learnt from: devantler
Repo: devantler-tech/platform PR: 2359
File: k8s/bases/apps/actual-budget/helm-release.yaml:62-111
Timestamp: 2026-07-01T21:13:36.950Z
Learning: When reviewing Kustomize/Helm YAML in this repo, keep the base vs provider overlay split: `k8s/bases/apps/**` and `k8s/bases/infrastructure/**` should contain each app’s full, environment-agnostic configuration (including base-level postRenderer Kustomize patches such as deployment strategy, topology spread, probes, and env injection). `k8s/providers/{docker,hetzner}/**` should only add small provider-specific deltas (e.g., `interval`, `persistence.size`) via patch files (like `k8s/providers/<provider>/apps/<app>/patches/helm-release-patch.yaml`). If configuration is identical across providers (e.g., OIDC/OAuth env vars where `${domain}` is resolved per cluster via envsubst), it belongs in the base and must not be duplicated into provider overlays.

Applied to files:

  • k8s/providers/hetzner/infrastructure/controllers/cilium/components/bandwidth-manager-bbr/kustomization.yaml
  • k8s/providers/hetzner/infrastructure/controllers/kustomization.yaml
🔇 Additional comments (7)
k8s/providers/hetzner/infrastructure/controllers/cilium/components/bandwidth-manager-bbr/kustomization.yaml (1)

1-51: LGTM!

k8s/providers/hetzner/infrastructure/controllers/kustomization.yaml (1)

69-74: LGTM!

tests/cilium-bandwidth-manager-bbr/kustomization.yaml (1)

1-7: LGTM!

scripts/tests/test-cilium-bandwidth-manager-component.sh (3)

15-53: LGTM on the extractor and its drain-behavior regression test — the "drain to EOF" design correctly avoids a SIGPIPE/pipefail race with the producer.

Also applies to: 73-90


91-103: LGTM! Single-line, order-independent checks — robust against Kustomize serialization details.


110-125: 🎯 Functional Correctness

No issue here. The opt-in component only adds bandwidthManager values, and this repo doesn’t set bpf.masquerade in the rendered Cilium values, so the current text checks are sufficient.

			> Likely an incorrect or invalid review comment.
.github/workflows/ci.yaml (1)

44-45: LGTM! Confirmed kubectl ships on the standard ubuntu-latest GitHub Actions runner image, so running this step before the KSail setup step is safe.

Also applies to: 80-86


📝 Walkthrough

Walkthrough

Adds a Hetzner-scoped, default-off Kustomize component enabling Cilium bandwidth manager with host-namespace-only BBR. A dedicated fixture composes the component for opt-in rendering tests. A Bash script validates default-off production output, opt-in BBR settings, encryption and WireGuard preservation, and excluded masquerading. CI detects the new test paths and runs the validation during manifest checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: staging Cilium bandwidth manager with BBR.
Description check ✅ Passed The description is directly related to the PR and describes the staged component, test, and rollout intent.
Linked Issues check ✅ Passed The PR meets #2607 by adding a default-off Hetzner component with the required bandwidthManager and BBR settings, plus validation and comments.
Out of Scope Changes check ✅ Passed The changes stay within the staged Cilium bandwidth-manager/BBR work and its validation wiring, with no unrelated scope apparent.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Comment @coderabbitai help to get the list of available commands.

@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 13, 2026
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Codex Daily AI Engineer instance.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

@devantler I'll review the current state of the PR.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: 🫴 Ready

Development

Successfully merging this pull request may close these issues.

Stage default-off Cilium bandwidth manager with BBR

1 participant