Skip to content

ob secrets push replaces all replicas at once, losing all capacity when the new containers are unhealthy #165

Description

@vishr

What happens

ob secrets push replaces every replica of a workload simultaneously and only
checks health afterwards, so a secret value that prevents startup takes the
workload from N replicas to zero.

recreateRoleForRelease (internal/engine/recreate.go:30-96), called from
internal/engine/secretspush.go:757:

  1. sends the drain signal to all containers of the service and waits for them
    all to exit (recreate.go:42-62),
  2. runs a single docker compose up -d --no-deps --force-recreate --scale <svc>=<n> <svc>
    (recreate.go:64-72), recreating every replica at once,
  3. then loops over the new container IDs waiting for health (recreate.go:81-96).

Every old container is gone before the first health check runs. The restore path calls the same function — recoverSecretGeneration
(secretspush.go:683-693) reaches the same forceSecretGeneration — so
recovery also replaces the whole fleet at once.

Correction to the original framing: recovery is not powerless in general. If
the new secret value is what prevents startup, recreating onto the previous
generation does restore health. What is unconditionally true is that the forward
attempt has already destroyed every serving replica and spent up to the full
within window (role.ReadyTiming(), recreate.go:81-84) at zero capacity
before any evidence exists, and recovery then recreates the fleet a second time,
adding another gap. Recovery fails identically only when the startup failure is
unrelated to the secret — which is what happened here — ending in
SecretRecoveryIncompleteError with the checkpoint left in SecretRecovering
(secretspush.go:684-689, :326-328). The harm is that serving replicas are
destroyed before any evidence is gathered, not that rollback lacks a smarter
path.

Expected

For a workload declaring strategy: rolling, replace one replica at a time and
abort on the first unhealthy container, leaving the remaining replicas serving —
the same per-replica gate the deploy path provides. A consuming project documents
that guarantee as a reason its deploys can run unattended: "the per-replica health
gate that deletes an unhealthy new container and leaves the old ones serving."
The secret-generation path does not honour it.

Observed

goal/production, workload server, replicas: 3, strategy: rolling,
2026-09-09:

⟳ waiting 8cd2037532f3 → healthy
⟳ waiting 9ec784c24ddd → healthy
✗ ob: secret generation recovery for release 20260907-030436-54af549-deploy-cd39fce9b20f
  is incomplete; retry `ob secrets push`:
  replace server on secret generation sg-58458104ffca1fc03117854f:
    timeout waiting for 8cd2037532f3 to be healthy (last: unhealthy)
  restore server to old secret generation:
    timeout waiting for 9ec784c24ddd to be healthy (last: unhealthy)

Afterwards all three replicas were down, and the public API returned 404s for
the duration (with no healthy backend the router is dropped and requests fall
through to another workload's catch-all).

In this case the containers were failing to start for an unrelated,
pre-existing reason rather than because of the new secret. That is what makes the
bug visible: the push cannot discover a startup failure of any kind before it has
already destroyed every replica. A rolling replacement would have failed the push
with two replicas still serving, and no outage.

Suggested fix

In the secrets-push path, dispatch on the workload's declared strategy the way
the deploy path already does (deploy.go:362-366, :693-697,
recovery.go:247-260). forceSecretGeneration currently calls
recreateRoleForRelease unconditionally (secretspush.go:757) and consults
Mode() nowhere. For a rolling workload it should roll: surge one, health-gate
it, retire one old, repeat — deleting the newcomer and leaving survivors on
failure, exactly as roll.go:113-143 already does. Recreate-mode workloads keep
today's behaviour, which is their declared contract (recreate.go:15-19).

RollRole needs one change to be reusable here: it derives the project dir and
release from the compose path (roll.go:72-73), which is wrong for
<release>/.ob-secret-generations/<gen>/compose.yaml — the same reason
recreateRoleForRelease exists at all (recreate.go:25-29). It also needs to
narrow "newcomer" by ob.secret-generation, because every container in a
rotation shares the same ob.release label, so the release label alone cannot
tell old from new.

Worth making forceSecretGeneration idempotent at the same time: after a roll
whose unhealthy newcomer was removed, every container is already on the old
generation, and recovery should recommit and verify rather than fail the
"identity did not change" check (secretspush.go:770-778).

Can replicas run on different secret generations mid-roll?

This is the obvious objection to rolling here, and the answer is that nothing at
the tool level forbids it:

  • env_file is read by Compose at container creation only — the repo states
    this itself (app/workload_contract.go:44-47). A container's secrets are
    baked in at create time.
  • Both generation directories coexist for the whole transition: the new one is
    installed before the checkpoint (secretspush.go:551-573), and the old is
    deleted only in finishSecretCleanup after verification (:663-680).
  • Generation uniformity is asserted only after replacement
    (verifySecretGeneration, :811-822), which a completed roll satisfies.
  • ob deploy already rolls env changes this way.

Application-level coherence — a shared signing key, a password rotated
externally — is precisely what strategy is for. Recreate does not make those
cases safer; it makes every replica fail at once.

Possible follow-up (separate, lower value)

ob job run is attached over SSH (internal/engine/gate.go:131,
internal/engine/plan.go:524), so a long manual job dies with the terminal,
while the same job run through ob schedule run is detached under systemd with
the declared timeout. Consider a detached mode for manual runs, or a warning when
a manual job's declared timeout is long.

Ordering

Depends on #167. Both change RollRole's surge loop and signature, so they
should land sequentially rather than in parallel — and #167 first, because
otherwise the new secrets rolling path is born with that wedge: a stopped
replica mid-rotation gives scale up produced no new container, recovery rolls
into the same wedge, and the push ends in SecretRecoveryIncompleteError.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions