Skip to content

Simplify cleanup-failure recovery during revert #32

Open

Description

@wadewilliams

Context

PR #31 made every failed deploy revert all app services, including rolling services that had already shipped and drain-first services, back to the image each was running. Most of the revert is simple:

  • restore the git checkout
  • roll back shipped rolling services
  • restore drain-first services at their replica count

The hard part is what happens when cleanup itself fails partway through. Several paths try to recover cleverly:

  • _roll_back handles a previous-image container that "survived" a cutover whose docker stop/rm failed: it removes the new containers and scales back to the previous count.
  • _remove_stopped removes stopped leftovers (a stop that worked, then an rm that didn't) before a revert's compose up --scale, because compose would otherwise restart them. It runs in rollback and drain-first restore, keeping previous-image leftovers there.
  • _Run.aborted tracks the rolling service whose own abort kept its old container, so the revert can remove a stopped new container that abort left behind.

Each came from a real failure mode found in review, but together they are the hardest code in deploy.py to reason about. They also multiply the paths that lead to exit 3.

Motivation

The adversarial pre-merge review of #31 (Codex) ranked this the highest-value simplification. We deferred it so we wouldn't rewrite revert code right before merge.

Proposal

When cleanup fails during a revert, stop and exit 3; an operator looks, instead of attempting further recovery. Concretely:

  • Drop survivor handling in _roll_back: a failed cutover cleanup is already reported, so stop there.
  • Replace _remove_stopped with a check: if a service has stopped leftovers, say so and exit 3 rather than removing them.
  • Drop _Run.aborted: an abort whose own cleanup failed already exits 3.
  • Afterwards, list the remaining exit-3 causes in one place (SPEC §4.1) and make sure each has one clear message.

The guarantees from #31 stay: revert on failure and on signal, drain-first ordering, tag-based reverts, and exit 3 whenever something wasn't put back.

Size

About 80–120 lines of src/flow_deploy/deploy.py, plus the tests that pin the removed recovery paths (tests/test_deploy.py, tests/test_drain_first.py, tests/test_deploy_failures.py) and the FakeHost's stopped-container restart model. Medium risk: it touches the revert path, so it needs the full integration suite and a rehearsal on a staging host.

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