From 6e88d3170ac2db3e99e07e533ae3a79a9c33efab Mon Sep 17 00:00:00 2001 From: bdchatham Date: Mon, 13 Jul 2026 15:53:41 -0700 Subject: [PATCH] =?UTF-8?q?feat(workflow):=20StateSync=20recipe=20complete?= =?UTF-8?q?s=20at=20release=20=E2=80=94=20mark-ready=20is=20terminal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The StateSync recipe ends at mark-ready: Complete means every mutation was performed and the node was released to re-bootstrap. Catch-up is verified node-side (sdk WaitCaughtUp, RPC, alerts) by whoever triggered the workflow. Why: catching_up is a proxy for health the workflow cannot own -- the old final await-condition(catchingUp) held the node's single adoption slot for the full catch-up (hours at real chain height) with no deadline anywhere (the sidecar task polls unbounded, the plan executor has no per-task clock), and the only exit from a wedged wait was the force-delete ceremony, which the docs framed as Failed-workflow recovery only. Completing at release frees the slot in minutes, deletes the unbounded-wait failure mode outright, and separates execution (the workflow's job) from verification (the node's health surfaces). Two properties this buys, both now test-pinned: - Failed <=> held: mark-ready is the terminal step and it is fire-and-forget (success == submitted), so a Failed workflow always means the node is still held. The old shape could fail AFTER release (await failing on a slow-but-healthy sync), which made "Failed holds the node" conditional. - The adoption slot frees at release, so drift work and queued workflows resume promptly. Accepted costs, on the record (peer-reviewed with kubernetes-specialist + systems-engineer, chain-side behavior verified by sei-network-specialist): - WorkflowInProgress clears at release, so height-lag alerts can fire during legitimate catch-up. Traced against the platform alert rules: at most 1-2 auto-resolving warning tickets on protocol-low, no pages. - Drift suppression lifts at release, so drift work (an image roll, a config reconcile) can interrupt catch-up at any point in the window, not only at the tail. Verified against sei-chain: the giga startup safety checks key on SS version markers written only at the END of a snapshot restore, so an interrupted restore re-enters state-sync and converges. One sub-second wedge window exists between the sequential cosmos/evm marker writes at the restore tail; recovery is re-running the recipe (runbook note, negligible probability). - The adoption slot frees at release, so a queued workflow can adopt and re-wipe a node mid-catch-up. Wasteful, never corrupting (the recipe quiesces before it wipes); operators sequence workflows per node. - Config drift cannot undo the migration flags: managed nodes have no config-drift detector, and the only running-node config surface (the update progression's p2p patch) never touches app.toml state-store keys. Test changes: planner progression expectations end at mark-ready with an explicit no-await pin; envtest lifecycle stall point moves to configure-state-sync (mark-ready is fire-and-forget, so GetTask-based stalls cannot hold it); AdoptAndComplete pins the terminal-step shape; workflow_test task counts 6 -> 5. sdk/sei's StateSyncWorkflow doc now states the release semantics (callers verify catch-up via WaitCaughtUp). No CRD schema change (make manifests generate produced no config/manifests diff). The seictl UX/docs companion (success handoff print, 15m default timeout, README) follows in sei-protocol/seictl. Co-Authored-By: Claude Fable 5 --- api/v1alpha1/seinodetaskworkflow_types.go | 8 ++++++-- .../node/envtest/workflow_lifecycle_test.go | 16 +++++++++++++--- internal/controller/node/workflow_test.go | 4 ++-- internal/planner/workflow.go | 13 +++++++++---- internal/planner/workflow_test.go | 5 +++-- sdk/sei/workflow.go | 6 ++++-- test/integration/workflow_test.go | 13 ++++++++----- 7 files changed, 45 insertions(+), 20 deletions(-) diff --git a/api/v1alpha1/seinodetaskworkflow_types.go b/api/v1alpha1/seinodetaskworkflow_types.go index 5bafbfcd..6c0a22f4 100644 --- a/api/v1alpha1/seinodetaskworkflow_types.go +++ b/api/v1alpha1/seinodetaskworkflow_types.go @@ -14,8 +14,12 @@ type SeiNodeTaskWorkflowKind string const ( // SeiNodeTaskWorkflowKindStateSync re-bootstraps the target node through // CometBFT state sync: mark-not-ready -> stop-seid -> reset-data -> - // config-patch -> configure-state-sync -> mark-ready -> - // await-condition(catchingUp). It is the paved road for STO-624. + // config-patch -> configure-state-sync -> mark-ready. The recipe ends at + // mark-ready, so Complete means every mutation was performed and the node + // was released to re-bootstrap, NOT that it caught up; catch-up is + // verified node-side (sdk WaitCaughtUp, RPC, alerts) by whoever triggered + // the workflow. The clean consequence: Failed always means the node is + // still held. It is the paved road for STO-624. SeiNodeTaskWorkflowKindStateSync SeiNodeTaskWorkflowKind = "StateSync" ) diff --git a/internal/controller/node/envtest/workflow_lifecycle_test.go b/internal/controller/node/envtest/workflow_lifecycle_test.go index 61f8062d..03a95864 100644 --- a/internal/controller/node/envtest/workflow_lifecycle_test.go +++ b/internal/controller/node/envtest/workflow_lifecycle_test.go @@ -236,6 +236,16 @@ func TestWorkflowLifecycle_AdoptAndComplete(t *testing.T) { g.Expect(ready.Status).To(Equal(metav1.ConditionTrue)) g.Expect(w.Finalizers).NotTo(ContainElement(seiv1alpha1.SeiNodeTaskWorkflowFinalizer)) + // Complete-at-release semantics pin: the recipe ends at mark-ready and + // carries no catch-up gate. Complete means the node was released, not + // that it caught up; catch-up verification is node-side (STO-624). + g.Expect(w.Status.Plan).NotTo(BeNil()) + g.Expect(w.Status.Plan.Tasks).NotTo(BeEmpty()) + g.Expect(w.Status.Plan.Tasks[len(w.Status.Plan.Tasks)-1].Type).To(Equal(sidecar.TaskTypeMarkReady)) + for _, tk := range w.Status.Plan.Tasks { + g.Expect(tk.Type).NotTo(Equal(sidecar.TaskTypeAwaitCondition)) + } + n := getNode(g, cli, "lc-complete") g.Expect(n.Status.AdoptedWorkflow).To(BeNil()) wip := apimeta.FindStatusCondition(n.Status.Conditions, seiv1alpha1.ConditionWorkflowInProgress) @@ -278,7 +288,7 @@ func TestWorkflowLifecycle_FailurePath(t *testing.T) { func TestWorkflowLifecycle_ReadoptByUIDAfterRestart(t *testing.T) { g := NewWithT(t) - fake := newFakeSidecar().stallAt(sidecar.TaskTypeAwaitCondition) + fake := newFakeSidecar().stallAt(sidecar.TaskTypeConfigureStateSync) cli, cancel := startNodeManagerC(t, fake) node := lifecycleNode("lc-restart") @@ -343,7 +353,7 @@ func TestWorkflowLifecycle_QueueThenSequential(t *testing.T) { func TestWorkflowLifecycle_FinalizerBlocksDeleteThenForce(t *testing.T) { g := NewWithT(t) - fake := newFakeSidecar().stallAt(sidecar.TaskTypeAwaitCondition) + fake := newFakeSidecar().stallAt(sidecar.TaskTypeConfigureStateSync) cli := startNodeManager(t, fake) node := lifecycleNode("lc-final") @@ -491,7 +501,7 @@ func forceDelete(cli client.Client, wf *seiv1alpha1.SeiNodeTaskWorkflow) { // takes effect. func TestWorkflowLifecycle_PauseFreezesForceDelete(t *testing.T) { g := NewWithT(t) - fake := newFakeSidecar().stallAt(sidecar.TaskTypeAwaitCondition) + fake := newFakeSidecar().stallAt(sidecar.TaskTypeConfigureStateSync) cli := startNodeManager(t, fake) node := lifecycleNode("lc-pausedel") diff --git a/internal/controller/node/workflow_test.go b/internal/controller/node/workflow_test.go index 5f67287f..a1c55f12 100644 --- a/internal/controller/node/workflow_test.go +++ b/internal/controller/node/workflow_test.go @@ -116,7 +116,7 @@ func TestReconcileWorkflow_AdoptsOldestPending(t *testing.T) { adopted := getWorkflow(t, c, "ss-0") g.Expect(adopted.Finalizers).To(ContainElement(seiv1alpha1.SeiNodeTaskWorkflowFinalizer)) g.Expect(adopted.Status.Plan).NotTo(BeNil()) - g.Expect(adopted.Status.Plan.Tasks).To(HaveLen(6)) // config-patch omitted (no Migration in fixture) + g.Expect(adopted.Status.Plan.Tasks).To(HaveLen(5)) // config-patch omitted (no Migration in fixture); mark-ready is terminal g.Expect(adopted.Status.Phase).To(Equal(seiv1alpha1.SeiNodeTaskWorkflowPhaseRunning)) } @@ -265,7 +265,7 @@ func TestReconcileWorkflow_ReadoptsByUIDAfterRestart(t *testing.T) { // The interrupted plan is rebuilt and persisted deterministically. resumed := getWorkflow(t, c, "ss-0") g.Expect(resumed.Status.Plan).NotTo(BeNil()) - g.Expect(resumed.Status.Plan.Tasks).To(HaveLen(6)) + g.Expect(resumed.Status.Plan.Tasks).To(HaveLen(5)) // The deletion gate is (re-)ensured on the drive step, not just at adoption: // an adoption interrupted before the finalizer add must not resume into // destructive execution ungated (guards the 1a regression). diff --git a/internal/planner/workflow.go b/internal/planner/workflow.go index dc4af525..f823e028 100644 --- a/internal/planner/workflow.go +++ b/internal/planner/workflow.go @@ -65,7 +65,13 @@ func (p *stateSyncWorkflowPlanner) Validate(node *seiv1alpha1.SeiNode, wf *seiv1 // BuildPlan compiles the StateSync progression: // // mark-not-ready -> stop-seid -> reset-data -> config-patch -> -// configure-state-sync -> mark-ready -> await-condition(catchingUp) +// configure-state-sync -> mark-ready +// +// The recipe ends at mark-ready: Complete means every mutation was performed +// and the node was released to re-bootstrap. Catch-up is verified node-side +// (sdk WaitCaughtUp, RPC, alerts) by whoever triggered the workflow. Because +// mark-ready is the terminal step, a Failed workflow always leaves the node +// held, and the adoption slot frees at release. // // TargetPhase and FailedPhase are left empty: a workflow never drives the // node's phase (a failure parks the node held, it does not fail the node). @@ -104,8 +110,8 @@ func (p *stateSyncWorkflowPlanner) BuildPlan(node *seiv1alpha1.SeiNode, wf *seiv // gate and purges mark-ready records; stop-seid brings seid down; only then // does reset-data run (it refuses if seid's RPC still answers). The release // step's mark-ready always executes fresh because mark-not-ready purged the - // prior record. The final catchingUp await-condition carries no - // targetHeight (the live head is unknown at plan-build time). + // prior record, and it is the terminal step, so the workflow completes at + // release. type step struct { taskType string params any @@ -125,7 +131,6 @@ func (p *stateSyncWorkflowPlanner) BuildPlan(node *seiv1alpha1.SeiNode, wf *seiv steps = append(steps, step{TaskConfigureStateSync, cfgSS}, step{TaskMarkReady, sidecar.MarkReadyTask{}}, - step{TaskAwaitCondition, sidecar.AwaitConditionTask{Condition: sidecar.ConditionCatchingUp}}, ) tasks := make([]seiv1alpha1.PlannedTask, 0, len(steps)) diff --git a/internal/planner/workflow_test.go b/internal/planner/workflow_test.go index 92a33d8d..ed0c87f3 100644 --- a/internal/planner/workflow_test.go +++ b/internal/planner/workflow_test.go @@ -87,8 +87,10 @@ func TestStateSyncWorkflow_Progression(t *testing.T) { TaskConfigPatch, TaskConfigureStateSync, TaskMarkReady, - TaskAwaitCondition, })) + // mark-ready is the terminal step: Complete means the node was released, + // and catch-up is verified node-side, never by the recipe (STO-624). + g.Expect(gotTypes).NotTo(ContainElement(TaskAwaitCondition)) g.Expect(plan.Phase).To(Equal(seiv1alpha1.TaskPlanActive)) // A workflow never drives the node's phase. @@ -119,7 +121,6 @@ func TestStateSyncWorkflow_OmitsConfigPatchWhenEmpty(t *testing.T) { taskTypeResetData, TaskConfigureStateSync, TaskMarkReady, - TaskAwaitCondition, })) g.Expect(gotTypes).NotTo(ContainElement(TaskConfigPatch)) } diff --git a/sdk/sei/workflow.go b/sdk/sei/workflow.go index d3299ae8..6e75be28 100644 --- a/sdk/sei/workflow.go +++ b/sdk/sei/workflow.go @@ -60,8 +60,10 @@ type WorkflowSpec struct { // StateSyncWorkflow is the payload for WorkflowStateSync — re-bootstrap the // target through CometBFT state sync (quiesce -> wipe data/ -> re-configure -> -// resync -> await caught-up). The recipe is a paved road; its step ordering and -// guardrails live in the controller, not here. +// release). Complete means every mutation was performed and the node was +// released to re-bootstrap; the resync runs after Complete, so callers verify +// catch-up node-side (WaitCaughtUp). The recipe is a paved road; its step +// ordering and guardrails live in the controller, not here. type StateSyncWorkflow struct { // Migration, when set, runs a named seid config migration inside the // re-bootstrap (the controller materializes it into the config-patch step). diff --git a/test/integration/workflow_test.go b/test/integration/workflow_test.go index 00e960cb..79eca2ee 100644 --- a/test/integration/workflow_test.go +++ b/test/integration/workflow_test.go @@ -25,8 +25,9 @@ import ( // memiavl baseline for the network in this suite, it reaches BOTH witnesses: it // is applied to the network (so the genesis validator inherits it) and to the // rpc follower via provision's storageConfig, so both witnesses serve chunks. -// Without it, the resync has nothing to sync FROM and the workflow's -// await-caught-up step never clears. +// Without it, the resync has nothing to sync FROM and the node-side +// WaitCaughtUp assertion after workflow Complete never clears (the workflow +// itself completes at mark-ready and does not gate on catch-up). // // Keys are the sei-config storage.* overrides that sei-config.SnapshotGenerationOverrides // emits (storage.snapshot_interval / storage.snapshot_keep_recent, plus @@ -316,9 +317,11 @@ func bringUpStateSyncFollower(ctx context.Context, t *testing.T, c *sei.Client, } // workflowWaitTimeout is the TIGHT child budget for one resync workflow's -// WaitTerminal — well under the 60m scenario ctx so a wedged recipe (a giga boot -// that refuses to start, or a step whose await carries no timeout) fails FAST and -// legibly with the recorded status instead of stalling to the scenario deadline. +// WaitTerminal — well under the 60m scenario ctx so a wedged recipe step fails +// FAST and legibly with the recorded status instead of stalling to the scenario +// deadline. The recipe ends at mark-ready (Complete == released, catch-up is +// asserted node-side below), so a healthy run completes in minutes and this +// budget bounds only the mutation steps, of which reset-data is the slowest. const workflowWaitTimeout = 15 * time.Minute // awaitWorkflowComplete blocks on wf.WaitTerminal under a tight child timeout and