Skip to content

Bound the installer's wait for The Combine to come up - #4356

Draft
imnasnainaec wants to merge 7 commits into
fix/database-first-bootfrom
fix/installer-waits
Draft

imnasnainaec wants to merge 7 commits into
fix/database-first-bootfrom
fix/installer-waits

Conversation

@imnasnainaec

@imnasnainaec imnasnainaec commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

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, not master: wait-for-semantic-domains polls
CombineDatabase.SemanticDomainImportStatus, and nothing writes that record until #4355 lands. On its own against
master this would fail a first install after IMPORT_CHECK_TIMEOUT_SECONDS with the manual-import hint. Review the
deploy/scripts/install-combine.sh commits here; the four files from #4355 show up in the diff until that merges.

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. So the installer could stop k3s while the rest
were still coming up, and stopping k3s SIGKILLs pods, since the unit is patched to KillMode=mixed.

  • Replace the parsing with kubectl wait, checking the database first: everything else depends on it and it is the
    slowest to come up on a first install.
  • Wait for the database to record a completed semantic domain import before the shutdown, so that an interrupted
    import does not silently cost the user another one.
  • Pace and bound both waits. kubectl wait returns at once rather than blocking when the API is unreachable or the
    deployment 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.
  • Validate WAIT_TIMEOUT_SECONDS, which went into arithmetic unchecked: bash reads a non-number as 0 and a leading
    zero as octal, so abc became a deadline already passed, 007 seven seconds, and 3600s a bare arithmetic error.
    The check sits with the other install-only setup, so a bad value in the environment cannot stop an uninstall.
  • Let single-step stop after Wait-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 the Shutdown-combine one, which is in
#4353. Verified: the only difference between this branch's copy of the script and #4352's is that hunk.

The timeout row of installer/README.md went to #4355 and the update row to #4353, so nothing is left here for
that file. Verified conflict-free against #4353 and #4354.

QA and prod

None. install-combine.sh ships in combine-installer.run; QA and prod are deployed by
.github/actions/combine-deploy-update, which runs setup_combine.py directly. The installer's own server option
is the only server-shaped path it touches, and that path skips Shutdown-combine entirely.

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_SECONDS low.

🤖 Generated with Claude Code


This change is Reviewable

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 03ce12f2-c5a7-43d8-8b65-5d75bfb56297

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

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

imnasnainaec and others added 7 commits August 21, 2026 14:44
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>
@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.96%. Comparing base (7191cb2) to head (617aeff).

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           
Flag Coverage Δ
backend 87.22% <ø> (ø)
frontend 66.88% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant