diff --git a/release-notes/unreleased/174-rollback-drain-deadline.md b/release-notes/unreleased/174-rollback-drain-deadline.md new file mode 100644 index 0000000..b2af54c --- /dev/null +++ b/release-notes/unreleased/174-rollback-drain-deadline.md @@ -0,0 +1,49 @@ +# Release Notes for Issue #174: Rolled-back versions keep their drain delay + +## Bug Fix + +### What Changed + +A version that is rolled back to — or reintroduced with an identical spec — no longer +carries a stale removal deadline into its next rollout. + +When a version is superseded, the operator stamps `restate.dev/remove-version-at` on its +ReplicaSet to schedule teardown after `spec.restate.drainDelaySeconds`. Because +ReplicaSets and Services are named by a content hash of the pod template, rolling back to +a previous template re-adopts that exact ReplicaSet rather than creating a new one — and +the stamp came with it. Nothing removed it: the annotation is written under its own field +manager, so the operator's other patches cannot prune it, and the cleanup pass skips +whichever version is currently latest. + +The next time that version was superseded, its deadline had already passed, so it was +scaled to zero on the first reconcile instead of being given its drain window. + +The operator now clears the annotation when it re-adopts a ReplicaSet as the latest +version. + +### Why This Matters + +The drain delay exists so that invocations already pinned to a version can finish on it. +A version that skipped the delay was scaled to zero while that work was still in flight, +which surfaces as invocations retrying against a version with no pods behind it until they +are re-pinned or time out. + +The window for this was narrow but not exotic: it needed a rollback (or a re-applied +identical spec) followed by another rollout, which is the ordinary shape of "revert, fix +forward". + +### Impact on Users + +- **Existing deployments:** no configuration change. A ReplicaSet currently holding a + stale deadline has it cleared the next time it is reconciled as the latest version. +- **New deployments:** no impact. +- Rollback followed by a further rollout now takes `drainDelaySeconds` longer to tear the + intermediate version down — that delay is the fix, not a regression. + +### Migration Guidance + +None required. + +### Related Issues + +- Issue #174: Support rolling RestateDeployments back to a previously registered revision diff --git a/src/controllers/restatedeployment/controller.rs b/src/controllers/restatedeployment/controller.rs index 3e58dc5..f99770a 100644 --- a/src/controllers/restatedeployment/controller.rs +++ b/src/controllers/restatedeployment/controller.rs @@ -53,7 +53,8 @@ use crate::controllers::restatedeployment::cleanup::{ use crate::controllers::restatedeployment::reconcilers; use super::reconcilers::replicaset::{ - POD_TEMPLATE_HASH_LABEL, RESTATE_POD_TEMPLATE_ANNOTATION, RESTATE_TUNNEL_NAME_ANNOTATION, + POD_TEMPLATE_HASH_LABEL, RESTATE_POD_TEMPLATE_ANNOTATION, RESTATE_REMOVE_VERSION_AT_ANNOTATION, + RESTATE_TUNNEL_NAME_ANNOTATION, }; pub(super) const RESTATE_DEPLOYMENT_ID_ANNOTATION: &str = "restate.dev/deployment-id"; @@ -397,6 +398,38 @@ impl RestateDeployment { ) .await?; + // A ReplicaSet re-adopted here was previously superseded, so it may still + // carry the removal deadline cleanup stamped on it while it drained. Nothing + // else clears it: the patch above is a different field manager (so server-side + // apply won't prune it), and `cleanup_old_replicasets` skips the current + // version via `except_rs`. Left in place, a deadline that has already passed + // makes the *next* rollout tear this version down immediately instead of + // giving it drainDelaySeconds. + if existing_replicaset + .annotations() + .contains_key(RESTATE_REMOVE_VERSION_AT_ANNOTATION) + { + debug!( + "Unscheduling removal of re-adopted ReplicaSet {versioned_name} in namespace {namespace}", + ); + + rs_api + .patch_metadata( + &versioned_name, + &PatchParams::apply("restate-operator/remove-version-at").force(), + &Patch::Apply(serde_json::json!({ + "apiVersion": ReplicaSet::api_version(&()), + "kind": ReplicaSet::kind(&()), + "metadata": { + "annotations": { + RESTATE_REMOVE_VERSION_AT_ANNOTATION: null, + } + } + })), + ) + .await?; + } + existing_replicaset } else { debug!(