Repository navigation
fix(monitoring-agents): relax fluent-bit probes for loaded installs - #3045
Conversation
The fluent-bit DaemonSet crash-loops on a loaded install, timing out its HelmRelease and cascading into velero and backupstrategy-controller DependencyNotReady failures. The subchart default liveness/readiness probes (initialDelaySeconds 0, timeoutSeconds 1, failureThreshold 3) target the HTTP server on :2020, but that server does not bind until AFTER the kubernetes filter's 'testing connectivity with API server' step. Under a concurrent install the kube-API can be unreachable for ~10s, delaying the :2020 bind, while the single-threaded HTTP server can miss a 1s timeout when the engine is busy. The probes then fire into a connection-refused port and liveness-kill fluent-bit into a restart loop. Give both probes load headroom (initialDelaySeconds 15, timeoutSeconds 5, failureThreshold 6) so the API-gated late bind and busy-engine blips are tolerated. The httpGet target is unchanged (path / on port http = :2020). Add a helm-unittest pinning the relaxed timing and the preserved target. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
📝 WalkthroughWalkthroughUpdates Fluent Bit probe settings in the monitoring-agents values file, adding a liveness probe and completing the readiness probe with explicit HTTP port and timing fields. Adds YAML tests that assert the generated DaemonSet uses the ChangesFluent Bit probe settings
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
area/uncategorized
dosubot
Bot
added
area/monitoring
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a race condition where the fluent-bit monitoring agent enters a crash-loop during concurrent cluster installations. By relaxing the liveness and readiness probe thresholds, the agent is given sufficient time to bind its HTTP server even when the Kubernetes API is temporarily unreachable or under heavy load. These changes ensure stability during the deployment phase without altering the agent's core monitoring functionality. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request relaxes the liveness and readiness probe timings for Fluent Bit in the monitoring-agents package to prevent crash loops during loaded installations, and adds corresponding regression tests. The feedback suggests also applying the relaxed initialDelaySeconds and periodSeconds configurations to the readiness probe (and updating the test assertions accordingly) to ensure consistency and prevent premature connection failures during startup.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| port: http | ||
| timeoutSeconds: 5 | ||
| failureThreshold: 6 |
There was a problem hiding this comment.
The PR description states that both probes are relaxed with initialDelaySeconds: 15 and periodSeconds: 10. However, these parameters were only added to livenessProbe and are missing from readinessProbe. Adding them to readinessProbe ensures consistency and prevents unnecessary connection attempts/failures during the initial startup phase.
port: http
initialDelaySeconds: 15
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 6| - equal: | ||
| path: spec.template.spec.containers[0].readinessProbe.timeoutSeconds | ||
| value: 5 | ||
| - equal: | ||
| path: spec.template.spec.containers[0].readinessProbe.failureThreshold | ||
| value: 6 |
There was a problem hiding this comment.
Update the readiness probe assertions to include the relaxed initialDelaySeconds and periodSeconds values to match the suggested configuration in values.yaml.
- equal:
path: spec.template.spec.containers[0].readinessProbe.initialDelaySeconds
value: 15
- equal:
path: spec.template.spec.containers[0].readinessProbe.periodSeconds
value: 10
- equal:
path: spec.template.spec.containers[0].readinessProbe.timeoutSeconds
value: 5
- equal:
path: spec.template.spec.containers[0].readinessProbe.failureThreshold
value: 6![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/system/monitoring-agents/values.yaml`:
- Around line 306-311: The readinessProbe override is only partially relaxed
because it still relies on the subchart defaults for initialDelaySeconds and
periodSeconds. Update the readinessProbe block in values.yaml so the
monitoring-agents configuration explicitly sets those timing fields alongside
timeoutSeconds and failureThreshold, matching the intended relaxed startup
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 67a2b37c-f946-485e-84c1-1bf63c6153f1
📒 Files selected for processing (2)
packages/system/monitoring-agents/tests/fluent-bit_probes_test.yamlpackages/system/monitoring-agents/values.yaml
| readinessProbe: | ||
| httpGet: | ||
| path: / | ||
| port: http | ||
| timeoutSeconds: 5 | ||
| failureThreshold: 6 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Readiness probe relaxation is incomplete.
readinessProbe still omits initialDelaySeconds and periodSeconds, so those remain at subchart defaults. That does not fully match the stated relaxed timing contract and can still probe too early during loaded installs.
Suggested fix
readinessProbe:
httpGet:
path: /
port: http
+ initialDelaySeconds: 15
+ periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 6📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| readinessProbe: | |
| httpGet: | |
| path: / | |
| port: http | |
| timeoutSeconds: 5 | |
| failureThreshold: 6 | |
| readinessProbe: | |
| httpGet: | |
| path: / | |
| port: http | |
| initialDelaySeconds: 15 | |
| periodSeconds: 10 | |
| timeoutSeconds: 5 | |
| failureThreshold: 6 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/system/monitoring-agents/values.yaml` around lines 306 - 311, The
readinessProbe override is only partially relaxed because it still relies on the
subchart defaults for initialDelaySeconds and periodSeconds. Update the
readinessProbe block in values.yaml so the monitoring-agents configuration
explicitly sets those timing fields alongside timeoutSeconds and
failureThreshold, matching the intended relaxed startup behavior.
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
Reviewed the diff. Values-only override of the subchart, vendored sources untouched. Tests pin both httpGet target and timings. Non-blocking thought: a startupProbe would canonically separate cold-start headroom from steady-state aggressiveness without widening the dead-process zombie window to 60-75s, worth considering if probe flake reappears. LGTM.
2 participants
What this PR does
Problem
On a loaded install the
monitoring-agentsfluent-bit DaemonSet crash-loops and its HelmRelease times out (timeout waiting for: [DaemonSet/.../monitoring-agents-fluent-bit status: 'InProgress']), cascading into velero → backupstrategy-controllerDependencyNotReadyand failing the whole install.Root cause: fluent-bit's HTTP server on
:2020(the liveness/readiness target) binds only AFTER thefilter:kubernetes"testing connectivity with API server" step. During a concurrent install the kube-API can be unreachable for ~10s (observedconnection #144 timeout after 10 seconds to: kubernetes.default.svc:443), which delays the:2020bind. The vendored subchart's default probes (initialDelaySeconds 0, timeoutSeconds 1, failureThreshold 3) fire into a connection-refused port immediately, and the single-threaded HTTP server can't always meet a 1s timeout while the engine is busy — so liveness SIGTERMs fluent-bit (exitCode 0, not OOM) into a restart loop.Fix
Relax both probes in the first-party umbrella
values.yamlfluent-bit:block:initialDelaySeconds 15, periodSeconds 10, timeoutSeconds 5, failureThreshold 6(~75s tolerance from container start, ~60s steady-state). ThehttpGettarget is unchanged — path/on porthttp(:2020). The change is values-only; the vendored subchart is not touched.Why load-triggered (not always-broken)
A steady-state cluster runs the identical probe config with the DaemonSet 3/3 Ready, 0 restarts, 26 days. The probe only fails when the
:2020bind is delayed by API unreachability under install-time load. Steady-state-stable + flaky-only-under-load = probe-too-aggressive, not a target or config error.Test
Added a helm-unittest (
tests/fluent-bit_probes_test.yaml) pinning both the relaxed timing and the preserved:2020httpGet target (path/, porthttp) on the rendered DaemonSet, so a future change that re-tightens the probes or breaks the path/port is caught.Screenshots
N/A — no UI changes.
Release note
Summary by CodeRabbit