fix(installer): make operator readiness honest in the chart and the install gate - #3466

condition=Available answers whether the Deployment sits within the
slack its rollout strategy allows, not whether the operator came up.
That is the wrong question for an install gate, and it stays wrong
whatever the strategy is: any maxUnavailable equal to replicas drives
the minimum available replica count to 0 and the condition then reports
True with no running pod. Measured against an operator stuck in
ImagePullBackOff, the old check passed in 156 ms, and the install went
on to fail two minutes later on the CRD wait, which pointed the
investigation at the wrong component.

rollout status reads updated and available replicas directly and only
succeeds once the updated pod is available, which holds under any
rollout strategy. Dump the deployment, pods, operator logs and events
on failure so the cause is legible where it is detected.

Drop --wait from the helm step. Helm 3 decides a Deployment is ready
with the same replicas - maxUnavailable arithmetic, so on the current
strategy it is satisfied at zero ready pods and gates nothing; were the
strategy tightened it would instead fail the install on its own 2m
budget before this gate runs, hiding the diagnostics behind a bare Helm
timeout. That arithmetic is specific to Helm 3, since Helm 4 reworked
--wait into a wait strategy with a different default, so treat the
reason as version-bound. The conclusion is not: one gate that reports
what it saw is better than two that can disagree about whose budget
expired first, whichever major is installed. Hooks are waited for with
or without the flag. The 5m budget here covers scheduling and a cold
pull of an image that is not in the prepull set.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

At replicas: 1 with maxUnavailable: 1 the Deployment controller takes
the minimum available replica count as 1 - 1 = 0, so the Available
condition reports True with no running pod at all. That reading is what
every consumer of the status sees: an alert on Available, a health
dashboard, or a person reading kubectl get deploy all show green while
the operator is dead.

The replacement is not a tuned pair, it is the platform default written
down. At replicas: 1 the default 25% / 25% resolves to maxSurge 1 and
maxUnavailable 0, because the surge rounds up and the unavailability
rounds down. So this change stops overriding the default rather than
introducing a configuration of its own, and Kubernetes rejects
maxUnavailable: 0 together with maxSurge: 0, so the surge could not
have gone the other way regardless.

The overridden pair was carried over from the earlier cozystack
Deployment, where a darkhttpd sidecar published a host port and
hostNetwork left a surge pod unschedulable on a single-node cluster. It
arrived here in two steps: by the commit before this file was created
the sidecar was already gone and no port was declared, yet the pair
stayed, so it had outlived its own reason before it was ever copied.
That is why looking for a justification in this file's history finds
none. With no port declared anywhere in the pod there is no hostNetwork
port conflict left, so the surge pod schedules beside the outgoing one;
declaring one reintroduces exactly the scheduling failure the pair was
added for. Not exercised on a live cluster in this rework.

Stay on RollingUpdate rather than move to Recreate, which would fix the
condition too, and by a shorter route: MaxUnavailable() reports 0 for
any strategy that is not RollingUpdate, so the minimum available
replica count becomes replicas - 0 = 1 and the condition is honest
without any surge pod at all.
The transition was not attempted, and the failure this project fixed in
ouroboros 0.8.1 does not transfer to it. There the rollingUpdate block
was the one the apiserver defaults onto a Deployment created without
one, so it belonged to no applier and outlived the switch to Recreate.
This Deployment has carried an explicit block since the commit that
created it, and nothing applies it server-side, so whether Recreate
would upgrade cleanly is an open question about the field-ownership
history of the live object rather than a known rejection. What keeps
RollingUpdate here is availability rather than apply semantics:
maxUnavailable: 0 leaves the outgoing operator running when the
replacement never starts, where Recreate deletes it first and leaves
none. Editing the numbers inside the block is
upgrade-safe from every state the field can already be in.

The surge introduces new windows, and leader election covers less of
them than it looks. The operator runs four privileged install steps
before mgr.Start(), and the lease is only acquired inside mgr.Start():
the CRD install, the Flux install, the platform source resource and the
platform PackageSource. All four therefore run in the incoming pod
outside leader election entirely. What the lease does cover is the
reconcile loops, the part that would otherwise conflict continuously
instead of once at startup.

That leaves three seams, and only the first two happen on a healthy
upgrade. The routine one is the incoming pod's install steps against
the outgoing pod's live reconcile loops, since the outgoing pod ran its
own installs at its startup and has been inside mgr.Start() ever since.
Those do not contend for the same fields: the installs write as field
manager cozystack-operator, while the reconcilers write as
cozystack-packagesource-controller and cozystack-package-controller and
go mostly to status subresources. Different owners, different objects,
so this seam is safe by separation rather than by any shared manager.

The second is version skew rather than field contention, and maxSurge:
1 widens it rather than opening it. Under maxSurge: 0 the surge budget
counts desired replicas rather than live pods, so the incoming pod is
created while the outgoing one may still be Terminating - the overlap
existed. What it did not have was a live counterparty: scale-down
sends SIGTERM before the new pod is created, and that cancels the
manager context, so the old loops were already unwinding. Here the
outgoing pod is not signalled until the incoming one is available, so
the overlap runs against a fully live reconciler for a termination
grace period instead of a vestigial one. The incoming pod reports
Ready the instant its
container starts, because there is no readiness probe, so its CRD
install runs while the outgoing binary's reconcile loops are still live
and now writing against a schema the incoming pod has already moved.
Old code against a new schema, which no field manager separates. It is
bounded rather than benign: the outgoing pod exits on SIGTERM within
seconds, a rejected write is transient and the next reconcile repairs
it, and no object is left corrupt. Widening the skew, by adding a probe
that delays Ready or by a migration the old code cannot round-trip, is
what would make this seam matter.

The third seam is install against install, and it needs the outgoing
pod to be restarting, since a crashlooping pod re-runs its four steps
on every restart, which is exactly the broken operator this gate exists
to catch. Three of the four steps use server-side apply under the same
field manager, so they never conflict and resolve last-writer-wins
whenever the two pods carry different manifests. That ordering matters
in one direction, and it is accepted here rather than guarded against:
the CRD install and the Flux install both apply with Force: true, so an
outgoing pod that writes last silently overwrites what the incoming pod
has just applied, with no conflict and no error to exit on. The
incoming pod is inside mgr.Start() by then and will not apply again
until it restarts, so that revert of the upgrade's install phase
persists rather than healing itself. Reaching it needs a restart
landing in the seconds between the incoming pod's installs and the
outgoing pod's SIGTERM. installPlatformSourceResource is the remaining
exception, a read-modify-write of a Get followed by Create or by Update
with the observed resourceVersion, so two pods running it at once can
collide on AlreadyExists or on a conflict; that failure is loud and
self-healing, since any error there exits the process and the pod
restarts, and the object is not left corrupt.

Pin the strategy and all four preconditions the surge makes
load-bearing in a unit test: no container or initContainer declares a
port, both listener arguments stay disabled, leader election stays on,
and no probe uses httpGet. They fail differently and none is visible to
the others, since a declared port stops the scheduler, a bound one only
fails once the incoming process starts, and a second manager without a
lease fails nothing at all until it writes. The httpGet precondition
follows from the two disabled listeners leaving no port to target, and
it is the worst of the four to get wrong: the other three wedge an
upgrade, while a livenessProbe against a disabled listener crashloops
the pod on every cluster, including a fresh install where no surge
happens. Only the httpGet field is forbidden rather than the probes
themselves, because an exec probe stays legitimate. All four are
checked against all three variants, because the template branches on
variant inside the container spec and a port added under one branch
renders for that variant alone. The strategy type is pinned alongside
its two values, since maxSurge and maxUnavailable say nothing on their
own once the type is no longer RollingUpdate.

The listener preconditions are pinned less tightly than the other two,
and the test says so rather than implying coverage it does not have. A
membership assert has no notion of position while Go's flag package
keeps the last occurrence, so a duplicate after the disabled one wins
and the assert still passes, while a duplicate before it loses and the
assert passes just the same - the test cannot tell those two apart.
Moving the disabled flag last fixes the process without making the
assert catch anything. Rejecting the two addresses this
binary defaults to closes the shape a values-gated block to enable
metrics would take, and nothing available here closes an arbitrary
address, since helm-unittest cannot express that a flag appears at most
once.

Note in hack/helm-unit-tests.sh that the loop covers packages/core,
which its comment omitted. The suite added here lives there, so a later
edit reconciling the loop with the stale comment would have silently
stopped running it.

Split the rationale by audience rather than keeping it all in shipped
comments. What an operator applying the rendered manifest has to respect
stays in plain comments and travels into the install artifacts: that the
pair is the default, why maxUnavailable must stay 0, that no port may be
declared or bound, that no probe may use httpGet, and that leader
election has to stay on. Most of what stays in the template comment is
what the chart's own suite pins, so it cannot drift out of step with the
chart without a test going red: the four preconditions and which assert
covers each, and why the variant loop is needed. Three things there are
deliberate exceptions, kept because anyone editing the strategy has to
know them and no assert can carry them - that the webhook server
configured on 9443 is not a port binder, since controller-runtime adds
it to the runnables only inside GetWebhookServer() and this binary never
calls it; that two overlaps between the pods sit outside the lease; and
why RollingUpdate is kept rather than Recreate. Those name what exists.
The seam analysis above, which is how each one works, deliberately stays
here instead. It describes another file's internals, nothing pins it,
and a commit message is fixed in time, so it cannot quietly fall out of
date the way a comment can.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>