feat(operator): gate readiness on installation completion

[aweingarten]

📌 Summary

Closes #3419 — gives apl-operator a readiness signal that reflects platform installation, so helm install --wait and kubectl wait --for=condition=Available stop returning before the platform has converged.

On the auth.<domainSuffix>/ready suggestion

@j-zimnowoda thanks — that endpoint (oauth2-proxy behind the ingress) is a genuinely useful external smoke check, and I've documented it in this PR as the complement to the in-cluster gate. It isn't a substitute for one, for four reasons:

1. It doesn't fix the reported bug. helm install --wait still returns in ~30s, because the operator's readinessProbe was pgrep -f 'apl-operator' — "the process exists". Every adopter still has to bolt a bespoke external poll loop onto their bootstrap.
2. It needs things the platform doesn't control. Public DNS resolution and a trusted certificate. Bootstrap runners on private networks, and anyone on a custom CA or ACME staging, either can't reach it or must drop to curl -k, which weakens what the 200 proves.
3. Ambiguous failure modes. A non-200 / NXDOMAIN can't distinguish "platform not ready" from a DNS, cert, or ingress problem — exactly the distinction bootstrap automation needs to decide between "keep waiting" and "fail loudly".
4. It has no notion of which revision converged. After pushing a values change, auth/ready stays 200 throughout. There's no way to know the operator applied your change — the AplStatus-shaped ask in the issue.

So: good liveness check for the ingress/auth chain, not a convergence signal. This PR adds the latter using state the operator already tracks.

What changed

The operator writes a /tmp/ready marker at the single point where the installation phase has resolved to completed — fresh install, recovery install, or a restart on an already-installed cluster — and the readinessProbe tests for that marker:

# now blocks until the helmfile pipeline has actually converged
kubectl wait --for=condition=Available deployment/apl-operator -n apl-operator --timeout=30m
helm install apl … --wait --timeout 30m

• src/operator/k8s.ts — markInstallationComplete() alongside the existing updateHeartbeatFile(); same /tmp emptyDir pattern, so the marker clears on restart and is re-written once the operator re-confirms status.
• src/operator/main.ts — one call, after the install/recovery/already-installed branches converge.
• chart/apl — readinessProbe switched from pgrep to test -f /tmp/ready; progressDeadlineSeconds raised to 3600.
• charts/apl-operator — same gate (it had no readinessProbe at all).
• EXECUTION_FLOW.md — documents the contract, including the already existing apl-installation-status / apl-operator-state ConfigMaps, which answer "did the operator apply my commit?" via commitHash + status. That covered most of the issue's ask already; it just wasn't documented as a contract.

Three deliberate properties

• Readiness latches. It is not cleared while a later apply runs. The reconcile loop applies every ~5 minutes in steady state; flapping Available on each pass would make the condition useless as a gate. Per-apply status stays in apl-operator-state.
• It fails closed. A marker that can't be written, or an installation that keeps retrying, leaves the pod NotReady. The signal never claims a convergence that didn't happen — --wait times out loudly instead of returning early.
• progressDeadlineSeconds: 3600. Otherwise kubectl rollout status reports ProgressDeadlineExceeded at the 600s default while a perfectly healthy first install is still running.

🔍 Reviewer Notes

The default is the one thing worth arguing about. With operator.readiness.gateOnInstallationComplete: true (default here), helm install --wait blocks for the real 10-15 minute install instead of ~30 seconds. Anyone using --wait with helm's 5m default timeout will now see it time out — and with --atomic, roll back. That's the honest behaviour and the point of the issue, but it is a behaviour change, so it's switchable in one value and NOTES.txt prints the --timeout guidance. Happy to flip the default to false (opt-in) if you'd rather not change what existing --wait callers see; say the word and I'll push the one-line change.

Two smaller notes:

• Both operator Deployments (chart/apl for install, charts/apl-operator via helmfile) got the gate so their pod specs don't disagree about readiness across the handover.
• A first-class AplStatus CR with conditions and lastSuccessfulReconcile is still the better long-term answer, and would let kubectl wait --for=condition=Converged work per-revision. This PR deliberately doesn't introduce a CRD — it makes the existing signal truthful. Glad to follow up with the CR if you want it.

🧹 Checklist

• Code is readable, maintainable, and robust.
• Unit tests added/updated — 4 new cases in src/operator/k8s.test.ts (marker written with timestamp, idempotent re-mark on restart, never throws when unwritable so the pod stays NotReady, default path matches the probe). Full operator suite: 89 passed / 1 skipped.

Also verified locally: tsc --noEmit clean, eslint clean (33 pre-existing warnings, 0 errors), helm lint chart/apl passes, and both charts render correctly with the gate on and off. lint:hf was not run — no helmfile binary in my environment; no helmfile templates are touched.

[CasLubbers]

Hi @aweingarten, thanks for your contribution! I will start reviewing and testing your PR soon. Since this touches the installation process, the review will take a bit more time than usual.

[CasLubbers]

So this got me thinking what does it actually mean that the apl-operator is 'ready'. With the current place of markOperatorReady (markInstallationComplete) it only installed the core application through helmfile. If the operator would stop working from this point the platform would not work.
So it would be better to move the execution of this function to: https://github.com/linode/apl-core/blob/main/src/operator/apl-operator.ts#L99. There it would mark the operator ready after it did its first apply run. So all the argocd application exists. From there the platform will always try to heal itself through argocd.
Additionally the function is idempotent. So its fine to call it multiple times.

[CasLubbers]

This flag is not necessary. If we have the marker in code which will always run why not always use it?

[aweingarten]

Agreed — flag removed from both charts. The marker is always written now, so there was nothing left to gate on.

[CasLubbers]

There is already a default in the values.yaml

Suggested change

	{{- $readiness := .Values.operator.readiness | default dict }}
	{{- $readiness := .Values.operator.readiness }}

[aweingarten]

Done, taken as suggested: {{- $readiness := .Values.operator.readiness }}.

[CasLubbers]

There is already a default in the values.yaml

Suggested change

	progressDeadlineSeconds: {{ $readiness.progressDeadlineSeconds | default 3600 }}
	progressDeadlineSeconds: {{ $readiness.progressDeadlineSeconds }}

[aweingarten]

Done, taken as suggested.

[CasLubbers]

This comment is cluttering. One line on why we increased the limit would be better.

[aweingarten]

Cut to one line: # The rollout stays Progressing until the operator is ready, which is longer than the 600s default.

[CasLubbers]

Lets remove the if else block and keep this command. Additionally we can remove the comment. It's pretty clear what it does.

[aweingarten]

Done — if/else gone, test -f /tmp/ready kept, comment removed.

[CasLubbers]

This is not entirely true. When apl-operator is done installing the platform is not completly ready. We still have ArgoCD that is applying stuff. It only means that the operator is finished and started is reconcile loop.

[aweingarten]

Corrected — this was the most important note in the review. The section now says explicitly that ArgoCD is still working through the Applications when the operator reports Ready, and that the gate means the operator is finished and its reconcile loop has started. The "What this is not" paragraph was reworded the same way: the gate covers creating the Applications, not whether they have synced and gone Healthy.

[CasLubbers]

The name speaks for itself, we can remove the above comment

[aweingarten]

Removed.

[CasLubbers]

I would rename this function so it states what it actually does:

Suggested change

	export function markInstallationComplete(filePath: string = READINESS_FILE): void {
	export function markOperatorReady(filePath: string = READINESS_FILE): void {

[aweingarten]

Renamed to markOperatorReady, taken as suggested — and the rename is what made moving the call site read correctly, since it is no longer claiming anything about installation.

[CasLubbers]

This comment can be removed. It does not tell much.

[aweingarten]

Removed.

[CasLubbers]

The comment can be removed if we rename the function. Then from the function name it is clear what this function does. That is the clean code we want.

[aweingarten]

Removed. What is left is only the part the name cannot carry: that it is idempotent and that readiness latches so the reconcile loop does not flap the Deployment.

[CasLubbers]

I still need to find a way how I can easily deploy and test your fork.

[Copilot]

Pull request overview

This PR makes apl-operator readiness reflect platform installation completion (helmfile pipeline convergence) instead of mere process liveness, so helm install --wait and kubectl wait --for=condition=Available can be used as reliable bootstrap gates.

Changes:

• Add a /tmp/ready marker written once installation reaches completed, and call it from the post-install convergence point in the operator startup flow.
• Gate operator readinessProbe on the marker file (both the bootstrap chart/apl Deployment and the charts/apl-operator Deployment), and extend rollout budget for the bootstrap Deployment.
• Document the readiness/convergence contract and improve CI workflow output behavior for fork PRs.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/operator/main.ts	Marks installation completion at the single post-install convergence point.
src/operator/k8s.ts	Adds READINESS_FILE and markInstallationComplete() to write /tmp/ready safely.
src/operator/k8s.test.ts	Adds unit coverage for readiness marker behavior and error handling.
src/operator/EXECUTION_FLOW.md	Documents readiness gate semantics and ConfigMap-based introspection contract.
charts/apl-operator/values.yaml	Introduces operator.readiness.gateOnInstallationComplete defaulting to true.
charts/apl-operator/templates/deployment.yaml	Adds readinessProbe gated on /tmp/ready for the helmfile-managed operator Deployment.
chart/apl/values.yaml	Adds readiness gating + rollout budget knobs for the bootstrap operator Deployment.
chart/apl/templates/NOTES.txt	Surfaces the new “wait for Available with a longer timeout” guidance.
chart/apl/templates/deployment.yaml	Switches readiness from pgrep to /tmp/ready (configurable), and raises progressDeadlineSeconds.
.github/workflows/svcaplbot-run-dyff.yml	Always publishes compare output to job summary; adds fork-safe behavior for missing token.

[aweingarten]

Good catch. The summary write now checks the file size and, when it exceeds the limit, appends a closing ``` plus an explicit truncation notice, so the fence can no longer be left open.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

chart/apl/values.yaml:143

• The comment for operator.readiness.progressDeadlineSeconds says it’s “only relevant when the readiness gate is on”, but progressDeadlineSeconds affects Deployment rollout reporting regardless of readinessProbe configuration. If the intent is “primarily needed when the gate is on”, the wording should be adjusted to avoid misleading operators who disable the gate.

    # Rollout budget for the install window. Only relevant when the readiness
    # gate is on — `kubectl rollout status` fails once it is exceeded.
    progressDeadlineSeconds: 3600

src/operator/k8s.ts:52

• The JSDoc claims the /tmp emptyDir marker is “cleared on every restart”, but emptyDir volumes persist for the lifetime of the Pod (container restarts don’t clear them). This reads as if a container crash/restart would force re-marking, which isn’t necessarily true; please clarify that the marker is cleared when the Pod is recreated (e.g., reschedule/rollout).

 * The marker lives on the pod's /tmp emptyDir, so it is cleared on every
 * restart and re-created as soon as the operator re-confirms the installation
 * status from the apl-installation-status ConfigMap.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/operator/k8s.ts:52

• The comment says the /tmp emptyDir marker is "cleared on every restart", but emptyDir persists for the lifetime of the pod and will survive container restarts within the same pod. If the intent is "cleared when the pod is recreated", the wording should be updated to avoid overstating the guarantee.

 * The marker lives on the pod's /tmp emptyDir, so it is cleared on every
 * restart and re-created as soon as the operator re-confirms the installation
 * status from the apl-installation-status ConfigMap.

charts/apl-operator/templates/deployment.yaml:80

• The readiness gate can keep this Deployment NotReady for 10–15 minutes on a first install, but this chart does not raise spec.progressDeadlineSeconds. With the default 600s deadline, Kubernetes will mark the rollout ProgressDeadlineExceeded while installation is still legitimately running, and Helm/kubectl wait/rollout commands may fail early even though the operator is healthy. Consider adding a higher progressDeadlineSeconds (or making it configurable) when gateOnInstallationComplete is enabled, similar to chart/apl.

          {{- if ne (.Values.operator.readiness | default dict).gateOnInstallationComplete false }}
          # /tmp/ready is written by the operator once the platform installation
          # reaches the 'completed' state — the Deployment reports Available only
          # after the helmfile pipeline has converged, not merely once the process
          # is up. Readiness latches, so steady-state reconciles do not flap it.

[aweingarten]

Thanks for the review — the framing question was the right one to ask, and you are right.

Moved the mark to after the first apply run. markOperatorReady() (renamed from markInstallationComplete) now runs in AplOperator.runApplyIfNotBusy, right after the apply succeeds and before the state ConfigMap is updated. That run is what creates the ArgoCD Applications, so past it the platform can heal itself through ArgoCD — which is a defensible meaning for "the operator is ready" in a way that "helmfile finished" was not. Since it is idempotent, it just runs on every apply.

Two consequences worth naming:

• It now fails closed on a failed apply, not just a failed install. If the apply throws, the marker is never written and the pod stays NotReady. Added a test asserting that, alongside one asserting the marker is written on success.
• A pod restart now needs a full apply cycle before it reports Ready again, where before it only needed the installation status to read completed. On the bootstrap chart/apl Deployment that is covered by progressDeadlineSeconds: 1800. The helmfile-managed charts/apl-operator Deployment has no such raise and takes the default 600s — if a steady-state apply on a large cluster runs longer than that, a restart there would report ProgressDeadlineExceeded while the operator is healthy. I have not added a progressDeadlineSeconds to that chart because you did not ask for one and I would rather not add config you did not want; say the word and it is a one-line change.

Everything else in the review is applied: flag removed from both charts, defaults no longer re-stated in the templates, failureThreshold back to a plain 3, progressDeadlineSeconds down to 1800, the if-blocks and the comments that restated their own code all gone, and EXECUTION_FLOW.md no longer claims the platform is fully up at that point.

I still need to find a way how I can easily deploy and test your fork.

The chart change is self-contained if that helps — helm template chart/apl renders the new probe and deadline without needing the operator image, and the behaviour change is the single markOperatorReady() call in apl-operator.ts.

[aweingarten]

Re: your CHANGES_REQUESTED review (#3464 (review)) — implemented as asked, in 85d09a9.

markOperatorReady() now runs in AplOperator.runApplyIfNotBusy, immediately after the apply completes and before the succeeded state write — the spot you linked. It is left idempotent and called on every apply, as you suggested.

Since "the operator never goes Ready" is a much worse failure than the one this PR fixes, I traced the paths that reach it rather than assuming:

• Every install branch reaches it. isInstalled, recovery and standard all fall through to operator.start() in main.ts — there is no early return that would skip the operator loop and strand the pod NotReady.
• The first apply is prompt. reconcile() calls runApplyIfNotBusy at i = 0, before the first scheduleNextAttempt, so it does not wait out a reconcileIntervalMs before marking.
• It fails closed. The call sits inside the try, so a throwing apply skips it and the pod stays NotReady. That is a genuine widening from the previous version, which only failed closed on a failed install — now a failed apply holds readiness back too. Both directions have tests.
• The concurrent poll loop is not a hole. pollAndApplyGitChanges and reconcile run under the same Promise.all, so whichever wins the isApplying race marks readiness — they share the code path.

One consequence of the move worth your call, repeating it here so it is not buried: a pod restart now needs a full apply cycle before reporting Ready, where before it only needed the installation status to read completed. The bootstrap chart/apl Deployment covers that with progressDeadlineSeconds: 1800. The helmfile-managed charts/apl-operator Deployment has no such raise and takes the default 600s, so a slow steady-state apply on a large cluster could report ProgressDeadlineExceeded while the operator is perfectly healthy. I did not add a progressDeadlineSeconds there because you did not ask for one — say the word and it is a one-line change.

The rest of your line comments are all applied; replies are on each thread.

[aweingarten]

Closing out the last two items on this PR, both from Copilot's suppressed comments — pushed in 83df2ff.

1. charts/apl-operator had no rollout budget. I had been leaving this for you to call, but on reflection it is not really a judgement call any more: moving the mark to after the first apply is exactly what made it bite. A recreated pod now needs a full apply cycle before it reports Ready, and on a large cluster that can outrun the default 600s and report ProgressDeadlineExceeded on a perfectly healthy operator. Added progressDeadlineSeconds: 1800 there, mirroring the value you asked for on chart/apl.

To be clear this is not the flag coming back — it is a fixed value, same as the one you specified, no toggle. Trivially revertible if you disagree.

2. The marker's lifetime is now stated accurately. Copilot was right that the original comment overclaimed: emptyDir persists for the life of the pod, so it survives a container restart and is cleared only when the pod is recreated. That JSDoc is gone (you asked for it to go), but the property still matters, so EXECUTION_FLOW.md's "it latches" bullet now says it plainly — a container restart keeps the marker, a rescheduled or rolled-out pod goes NotReady until it completes an apply of its own.

That is every review comment on this PR addressed. 89 operator tests pass; helm template renders both charts with the new deadline.

[aweingarten]

Rebased onto main (8e2a4e4) — the five merge commits are gone, so this is now 5 clean commits. All checks green: build-test-cache, coverage, check-commit, run-compare, Validate PR title.

Worth flagging one thing the rebase exposed, since it raises the value of this PR: six of my other open PRs are now red on run-compare, all for the single reason this PR fixes — gh pr comment exits 4 because secrets.BOT_TOKEN is not available to pull requests from a fork. The compare itself succeeds every time; only the comment-posting step fails.

They were not all red before, but not because they were passing. Their heads were Merge branch 'main' into … commits, and check-commit skips the job on a merge commit:

elif [[ $COMMIT_MSG == "Merge branch"* && $EVENT_ACTION != "ready_for_review" ]]; then
  echo "Skipping - merge commit"

So run-compare was being skipped rather than passing — e.g. #3466's last "green" run on 2026-08-04 was run-compare: skipped. Rebasing removed the merge commits, the job now actually runs, and the fork-token failure it was masking is visible.

Nothing to fix on those PRs; they will go green once this one merges. I have deliberately not cherry-picked the workflow change into them, since that would put an unrelated CI edit in seven PRs and conflict on merge.

Affected: #3466, #3467, #3469, #3470, #3473, #3474.