Bound the installer's wait for The Combine to come up - #4356
imnasnainaec wants to merge 7 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults 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 |
wait-for-combine only ever read one deployment's READY count. Collapsing the kubectl table with an unquoted expansion put every row on one line, so grep matched the whole line for each name and the greedy sed captured the last N/1 in it -- the alphabetically last deployment, maintenance. Replace the parsing with kubectl wait, and check the database first, since everything else depends on it and it is the slowest to come up on a first install. Then wait for the database to record a completed semantic domain import. The installer stops k3s next, which SIGKILLs pods because the k3s unit is patched to KillMode=mixed, and an interrupted import is redone on the next start; so without this the shutdown could silently cost the user another import. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every other step that records a follow-on state checks SINGLE_STEP before continuing, but Wait-for-combine fell straight through into Shutdown-combine, so single-step could not be used to inspect a running cluster before the installer stopped it. Setting STATE directly rather than calling next-state leaves the recorded state in place, so the next run resumes at Shutdown-combine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"kubectl wait" returns at once, rather than blocking for its timeout, when the API server is unreachable or the deployment is gone, so its retry loop had nothing but an echo in it and could spin, pegging a core and flooding the terminal. Pace the retries. Neither wait had an upper bound either, so a deployment that was never going to come up produced no error, ever. Give each stage a deadline, default one hour and overridable with WAIT_TIMEOUT_SECONDS, and print the pods in the namespace before exiting so there is somewhere to start looking. Rerunning the installer resumes the wait. Check the semantic domain import every 30 seconds rather than every 10; each check starts a mongosh inside the database container, competing with the import it is waiting on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One deadline covers the wait for all four deployments, so "Timed out after 3600s waiting for deployment/maintenance" claimed a wait that had mostly elapsed on the database. Say what the wait was still waiting for instead, and describe the timeout in the header comment as the budget for the deployments and then for the import, rather than one per deployment. The semantic domain import runs from the database pod's postStart hook, so rerunning the installer does not start an import that never ran; give that wait a hint that names the manual import instead of the default advice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The database is not available until its postStart hook has imported the semantic domains, so the import elapses inside the wait for the deployments, not inside the wait that follows it. That second wait is a guard for a database pod that an install left running, which therefore never ran the hook. Comments now use one space after a period rather than two, and some blocks are rewrapped to the width of the file around them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
wait-for-semantic-domains had the same hour-long budget as the wait for the deployments, but readiness is gated on the same record, so it either passes on its first check or is in the case where no pod will ever write the record. That spent an hour polling before printing the manual-import hint that is the actual remedy; give it a short budget of its own. check-wait-deadline now reports the budget of the wait that set it, so the two cannot drift. WAIT_TIMEOUT_SECONDS went into arithmetic unchecked, where bash reads a non-number as 0 and a leading zero as octal: "abc" became a deadline that had already passed, "007" became seven seconds, and "3600s" aborted with a bare arithmetic error. Require a whole number greater than zero, as the timeout option already does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WAIT_TIMEOUT_SECONDS was checked at the top of the script, so a bad value left in the environment stopped an uninstall, which never waits for anything. Check it with the other install-only setup, where the state is known, as the version number already is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2689bfd to
617aeff
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## fix/database-first-boot #4356 +/- ##
========================================================
Coverage 75.96% 75.96%
========================================================
Files 305 305
Lines 11384 11384
Branches 1411 1411
========================================================
Hits 8648 8648
Misses 2332 2332
Partials 404 404
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Split out of #4352, part 4 of 4 — see
#4352 (comment) for the split and how it was verified.
Stacked on #4355. The base is
fix/database-first-boot, notmaster:wait-for-semantic-domainspollsCombineDatabase.SemanticDomainImportStatus, and nothing writes that record until #4355 lands. On its own againstmasterthis would fail a first install afterIMPORT_CHECK_TIMEOUT_SECONDSwith the manual-import hint. Review thedeploy/scripts/install-combine.shcommits here; the four files from #4355 show up in the diff until that merges.wait-for-combineonly ever read one deployment's READY count. Collapsing thekubectltable with an unquotedexpansion put every row on one line, so
grepmatched the whole line for each name and the greedysedcaptured thelast
N/1in it — the alphabetically last deployment,maintenance. So the installer could stop k3s while the restwere still coming up, and stopping k3s SIGKILLs pods, since the unit is patched to
KillMode=mixed.kubectl wait, checking the database first: everything else depends on it and it is theslowest to come up on a first install.
import does not silently cost the user another one.
kubectl waitreturns at once rather than blocking when the API is unreachable or thedeployment is gone, so the retry loop could spin, pegging a core and flooding the terminal; and neither wait had an
upper bound, so a deployment that was never going to come up produced no error, ever. Each stage now has a deadline,
prints the pods in the namespace before exiting, and resumes if the installer is rerun.
WAIT_TIMEOUT_SECONDS, which went into arithmetic unchecked: bash reads a non-number as 0 and a leadingzero as octal, so
abcbecame a deadline already passed,007seven seconds, and3600sa bare arithmetic error.The check sits with the other install-only setup, so a bad value in the environment cannot stop an uninstall.
single-stepstop afterWait-for-combine, so a running cluster can be inspected before the installer stops it.Scope
One file:
deploy/scripts/install-combine.sh, and all of its hunks except theShutdown-combineone, which is in#4353. Verified: the only difference between this branch's copy of the script and #4352's is that hunk.
The
timeoutrow ofinstaller/README.mdwent to #4355 and theupdaterow to #4353, so nothing is left here forthat file. Verified conflict-free against #4353 and #4354.
QA and prod
None.
install-combine.shships incombine-installer.run; QA and prod are deployed by.github/actions/combine-deploy-update, which runssetup_combine.pydirectly. The installer's ownserveroptionis the only server-shaped path it touches, and that path skips
Shutdown-combineentirely.Testing
Not yet exercised on hardware. The waits are the whole substance here, so this wants a real first install — including
the timeout paths, which are easiest to reach by setting
WAIT_TIMEOUT_SECONDSlow.🤖 Generated with Claude Code
This change is