[Backport release-1.5] fix(velero): add startupProbe so slow startup does not crashloop the install gate - #3533
Merged
Conversation
…install gate Velero binds its metrics/health endpoint (:8085, http-monitoring) only after the server loads its plugins and connects to the API server. Under a heavy parallel platform install that can take minutes, but the upstream liveness probe begins checking /metrics at initialDelaySeconds=10 and kills the container after failureThreshold=5 (~130s total) before :8085 is bound. The kubelet SIGTERMs velero (exitCode 0/Completed), it BackOff-restarts, and never escapes the loop inside the install-gate window. Because backupstrategy-controller hard-depends on velero, this blocks the gate. Add a startupProbe to the velero server container. While it is failing the kubelet holds off liveness and readiness entirely, granting failureThreshold * periodSeconds = 30 * 10s = 300s of startup grace, then hands over to the unchanged, tight upstream liveness (10s / 30s / 5) for steady-state crash detection. This is strictly better than loosening liveness, which would also blunt steady-state detection. The upstream velero chart (12.0.3) does not render a startupProbe, so the package patches the deployment template via patches/add-startup-probe.patch (re-applied after re-vendor by the Makefile update target, matching the seaweedfs/ingress-nginx convention) and supplies the startupProbe through the package values override. Pin the startupProbe budget and the unchanged liveness with a helm-unittest regression guard. Backport adaptations for release-1.5, which vendors velero chart 11.0.0 (appVersion 1.17.0) rather than main's 12.0.3 (1.18.1): - The chart version pin stays at the version this branch already vendors; only the patch re-application is added to `update`. The `helm pull` was previously unpinned, which would have let `update` float to a chart the patch does not apply to, so it is pinned to 11.0.0 to keep the new patch step deterministic. - Added the `test:` target, which exists on main but not here, so the shipped regression guard actually runs. - The image assertions pin this branch's own vendored versions (velero v1.17.0, plugin-for-aws v1.12.1, kubevirt plugin v0.8.0), and the upgrade-crds rationale is restated for chart 11.0.0, where that Job still runs a kubectl image. The startupProbe hunk applies to chart 11.0.0's deployment template unchanged, in the same position inside the metrics.enabled gate. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> (cherry picked from commit f7271b8) Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Hand-backport of #3138 to
release-1.5. Thebackportlabel was applied at merge time but no backport PR was ever opened, so the change never reached this branch — found by auditing the branch withcmd/backport-audit, which reported it asMISSING. Like the other items in that set, it merged before #3155 fixed the bot'sconflict_resolutioninput, so the conflicting cherry-pick was dropped with no draft and no failing check.Velero binds its metrics/health endpoint (
:8085) only after loading plugins and connecting to the API server. Under a heavy parallel platform install the upstream liveness probe starts checking atinitialDelaySeconds=10and kills the container afterfailureThreshold=5, so velero BackOff-restarts and never escapes the loop inside the install-gate window. Becausebackupstrategy-controllerhard-depends on velero, this blocks the gate. The fix adds astartupProbe, which holds liveness and readiness off entirely while it is failing, granting30 * 10s = 300sof startup grace and then handing over to the unchanged upstream liveness for steady-state crash detection.Backport adaptations
This branch vendors velero chart 11.0.0 (appVersion 1.17.0) rather than
main's 12.0.3 (1.18.1), so three things differ from the original. All are recorded in the commit message as well.update. Thehelm pullhere was previously unpinned, which would have letupdatefloat to a chart the patch does not apply to, so it is pinned to11.0.0— the version already vendored. That makes the new patch step deterministic and leaves the vendored tree byte-identical.test:target is added. It exists onmainbut not here, so without it the shipped regression guard would never run.v1.17.0,plugin-for-awsv1.12.1, kubevirt pluginv0.8.0), and theupgrade-crdsrationale is restated for chart 11.0.0, where that Job still runs a kubectl image —main's comment about the Job running the velero image natively is true for 12.x only.The
startupProbehunk itself applies to chart 11.0.0's deployment template unchanged, in the same position inside themetrics.enabledgate.Verification
helm unittest .passes 5/5 inpackages/system/velero. The assertions are mutually corroborating rather than vacuous: theequalassertions prove thestartupProbepath actually renders, and the same paths are asserted absent undervelero.metrics.enabled: false, proving it follows the same gate as liveness and readiness so disabling metrics can never leave astartupProbepointed at an unbound port.Screenshots
Not applicable — no UI change.
Release note