diff --git a/api/v1alpha1/seinode_types.go b/api/v1alpha1/seinode_types.go index 433a856d..da02469b 100644 --- a/api/v1alpha1/seinode_types.go +++ b/api/v1alpha1/seinode_types.go @@ -97,6 +97,7 @@ import ( // and it needs nodeConfig, because only that node's plans run the reset. // +kubebuilder:validation:XValidation:rule="!has(self.dataResetGeneration) || has(self.nodeConfig)",message="spec.dataResetGeneration needs spec.nodeConfig: only a node that reads its config from ConfigMaps runs the declarative data reset" // +kubebuilder:validation:XValidation:rule="!has(oldSelf.dataResetGeneration) || (has(self.dataResetGeneration) && self.dataResetGeneration >= oldSelf.dataResetGeneration)",message="spec.dataResetGeneration can only increase: lowering or removing it would rearm a data wipe; to undo a reset commit, revert the config and keep the counter" +// +kubebuilder:validation:XValidation:rule="!has(self.maintenance) || !has(self.maintenance.hold) || has(self.nodeConfig)",message="spec.maintenance.hold needs spec.nodeConfig: only a node that reads its config from ConfigMaps runs the maintenance hold" type SeiNodeSpec struct { // ChainID of the chain this node belongs to. // Constrained to DNS-1123 label characters because the controller composes @@ -286,6 +287,44 @@ type SeiNodeSpec struct { // +kubebuilder:validation:Minimum=0 // +optional DataResetGeneration int64 `json:"dataResetGeneration,omitempty"` + + // Maintenance holds seid at the sidecar start gate with the pod alive and + // the data volume mounted, for work through kubectl exec. Requires + // spec.nodeConfig. + // +optional + Maintenance *MaintenanceSpec `json:"maintenance,omitempty"` +} + +// MaintenanceHold is a maintenance hold mode. +// +kubebuilder:validation:Enum=Immediate;AfterExit +type MaintenanceHold string + +const ( + // MaintenanceHoldImmediate closes the start gate and stops seid now. + MaintenanceHoldImmediate MaintenanceHold = "Immediate" + // MaintenanceHoldAfterExit closes the start gate and leaves seid running; + // when seid exits on its own, for example at halt-height, it parks. On a + // node parked by an Immediate hold it starts seid once first. + MaintenanceHoldAfterExit MaintenanceHold = "AfterExit" +) + +// MaintenanceSpec is the maintenance request on a SeiNode. +type MaintenanceSpec struct { + // Hold keeps seid from starting while it is set. Immediate stops seid now; + // AfterExit lets it run until it exits. Remove it to release seid. While + // held, the controller still applies the StatefulSet, so a template change + // rolls the pod and the new pod stays parked. A hold set on a new node parks + // it before seid first runs. + // +optional + Hold MaintenanceHold `json:"hold,omitempty"` +} + +// HoldRequested returns the requested maintenance hold, or "" for none. +func (s *SeiNodeSpec) HoldRequested() MaintenanceHold { + if s.Maintenance == nil { + return "" + } + return s.Maintenance.Hold } // Resources overrides the seid-container footprint in pod-resource shape. @@ -754,6 +793,12 @@ const ( // a merge it can still describe the previous reset. ConditionDataResetInProgress = "DataResetInProgress" + // ConditionMaintenanceInProgress reports the maintenance hold + // (spec.maintenance.hold). InProgress-style and always-present: False is + // the steady state. True/Held means seid is parked, so exec work on the data + // is safe. + ConditionMaintenanceInProgress = "MaintenanceInProgress" + // ConditionDataVolumeResizeInProgress reports whether the node's data volume // is still growing toward spec.dataVolume.storage.resources.requests.storage. // InProgress-style and always-present: True is the exception, False the @@ -764,6 +809,23 @@ const ( ConditionDataVolumeResizeInProgress = "DataVolumeResizeInProgress" ) +// Reasons for the MaintenanceInProgress condition. Stable enum (public API for +// alerting/runbooks per CLAUDE.md "Conditions"). +const ( + // ReasonMaintenanceNotApplicable: the node has no spec.nodeConfig. + ReasonMaintenanceNotApplicable = "NotApplicable" + // ReasonNotHeld: no hold is requested or in effect. + ReasonNotHeld = "NotHeld" + // ReasonHoldPending: the requested hold differs from the hold in effect, + // and its plan has not finished. + ReasonHoldPending = "HoldPending" + // ReasonHeld: an Immediate hold is in effect; seid is parked. + ReasonHeld = "Held" + // ReasonArmed: an AfterExit hold is in effect; the gate is closed and seid + // parks at its next exit. + ReasonArmed = "Armed" +) + // Reasons for the DataResetInProgress condition. Stable enum (public API for // alerting/runbooks per CLAUDE.md "Conditions"). const ( @@ -1031,6 +1093,14 @@ type SeiNodeStatus struct { // the spec value, so creating a node never wipes it. // +optional DataResetGeneration int64 `json:"dataResetGeneration,omitempty"` + + // MaintenanceHold is the hold in effect: Immediate means seid is parked by + // the hold, AfterExit means the start gate is closed and seid may still run, + // empty means no hold acts on the node. The controller writes it when a hold, + // release, or held reset plan completes, and compares it with + // spec.maintenance.hold to decide the next plan. + // +optional + MaintenanceHold MaintenanceHold `json:"maintenanceHold,omitempty"` } // NodeEndpointStatus carries the in-cluster URLs this SeiNode serves, derived diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 33907c85..80dbae05 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -770,6 +770,21 @@ func (in *LabelPeerSource) DeepCopy() *LabelPeerSource { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MaintenanceSpec) DeepCopyInto(out *MaintenanceSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MaintenanceSpec. +func (in *MaintenanceSpec) DeepCopy() *MaintenanceSpec { + if in == nil { + return nil + } + out := new(MaintenanceSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MarkReadyPayload) DeepCopyInto(out *MarkReadyPayload) { *out = *in @@ -1494,6 +1509,11 @@ func (in *SeiNodeSpec) DeepCopyInto(out *SeiNodeSpec) { *out = new(SeedSpec) (*in).DeepCopyInto(*out) } + if in.Maintenance != nil { + in, out := &in.Maintenance, &out.Maintenance + *out = new(MaintenanceSpec) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SeiNodeSpec. diff --git a/config/crd/sei.io_seinodes.yaml b/config/crd/sei.io_seinodes.yaml index f9d1b90e..538f1628 100644 --- a/config/crd/sei.io_seinodes.yaml +++ b/config/crd/sei.io_seinodes.yaml @@ -591,6 +591,24 @@ spec: maxLength: 512 minLength: 1 type: string + maintenance: + description: |- + Maintenance holds seid at the sidecar start gate with the pod alive and + the data volume mounted, for work through kubectl exec. Requires + spec.nodeConfig. + properties: + hold: + description: |- + Hold keeps seid from starting while it is set. Immediate stops seid now; + AfterExit lets it run until it exits. Remove it to release seid. While + held, the controller still applies the StatefulSet, so a template change + rolls the pod and the new pod stays parked. A hold set on a new node parks + it before seid first runs. + enum: + - Immediate + - AfterExit + type: string + type: object nodeConfig: description: |- NodeConfig supplies this node's seid config files from existing @@ -1496,6 +1514,9 @@ spec: and keep the counter' rule: '!has(oldSelf.dataResetGeneration) || (has(self.dataResetGeneration) && self.dataResetGeneration >= oldSelf.dataResetGeneration)' + - message: 'spec.maintenance.hold needs spec.nodeConfig: only a node that + reads its config from ConfigMaps runs the maintenance hold' + rule: '!has(self.maintenance) || !has(self.maintenance.hold) || has(self.nodeConfig)' status: description: SeiNodeStatus defines the observed state of a SeiNode. properties: @@ -1677,6 +1698,17 @@ spec: leave the listener closed. type: string type: object + maintenanceHold: + description: |- + MaintenanceHold is the hold in effect: Immediate means seid is parked by + the hold, AfterExit means the start gate is closed and seid may still run, + empty means no hold acts on the node. The controller writes it when a hold, + release, or held reset plan completes, and compares it with + spec.maintenance.hold to decide the next plan. + enum: + - Immediate + - AfterExit + type: string phase: description: Phase is the high-level lifecycle state. enum: diff --git a/docs/specs/010-maintenance-hold/spec.md b/docs/specs/010-maintenance-hold/spec.md index a1423d1e..7e8a1ef6 100644 --- a/docs/specs/010-maintenance-hold/spec.md +++ b/docs/specs/010-maintenance-hold/spec.md @@ -228,7 +228,7 @@ reaches `Running` with seid parked. #### Acceptance Criteria 1. WHEN the hold is removed from a held node and no reset is pending, THE controller SHALL build a plan that marks the sidecar ready. -2. WHEN the hold is removed and a reset is pending, THE controller SHALL build the reset plan with its final `mark-ready`. +2. WHEN the hold is removed and a reset is pending, THE controller SHALL build the reset plan with its `mark-ready`, followed by a step that clears `status.maintenanceHold`. 3. WHEN the plan that marks the sidecar ready completes, THE controller SHALL clear `status.maintenanceHold`. ### Requirement 5: The controller reports the hold @@ -240,8 +240,8 @@ reaches `Running` with seid parked. #### Acceptance Criteria 1. The controller SHALL seed a `MaintenanceInProgress` condition on every SeiNode. `True` is the exception, `False` is the steady state. -2. WHILE no hold is set, THE condition SHALL be `False` with reason `NotHeld`, or `NotApplicable` on a node without `spec.nodeConfig`. -3. WHILE a hold is set and differs from `status.maintenanceHold`, WHILE a hold plan runs, or WHILE the sidecar reports the gate open under a hold, THE condition SHALL be `True` with reason `HoldPending`. +2. WHILE no hold is set and none is in effect, THE condition SHALL be `False` with reason `NotHeld`, or `NotApplicable` on a node without `spec.nodeConfig`. +3. WHILE the hold set differs from `status.maintenanceHold` (a removed hold included), WHILE a plan that changes the hold in effect runs, or WHILE the sidecar reports the gate open under a hold, THE condition SHALL be `True` with reason `HoldPending`. 4. WHILE an `Immediate` hold is in effect, THE condition SHALL be `True` with reason `Held`. 5. WHILE an `AfterExit` hold is in effect, THE condition SHALL be `True` with reason `Armed`. 6. WHEN the hold takes effect or the node is released, THE controller SHALL record an event on the SeiNode. diff --git a/internal/controller/node/controller.go b/internal/controller/node/controller.go index de31a9ef..da65ce8f 100644 --- a/internal/controller/node/controller.go +++ b/internal/controller/node/controller.go @@ -130,13 +130,8 @@ func (r *SeiNodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re observedPhase := node.Status.Phase prevSidecar := apimeta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionSidecarReady) prevStateSync := apimeta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionStateSyncReady) - // A copy, not the pointer FindStatusCondition returns: SetStatusCondition - // mutates the slice element in place, which would erase the transition. - var prevDataReset *metav1.Condition - if c := apimeta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionDataResetInProgress); c != nil { - cp := *c - prevDataReset = &cp - } + prevDataReset := conditionSnapshot(node, seiv1alpha1.ConditionDataResetInProgress) + prevMaintenance := conditionSnapshot(node, seiv1alpha1.ConditionMaintenanceInProgress) setNodePausedCondition(node) @@ -253,6 +248,7 @@ func (r *SeiNodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re // it: spec-derived plus the persisted plan, so it rides the flush on every // path, including Paused, where a pending reset waits. planner.ResolveDataReset(node) + planner.ResolveMaintenance(node) // Failed is terminal — flush any condition updates and exit. if node.Status.Phase == seiv1alpha1.PhaseFailed { @@ -344,6 +340,7 @@ func (r *SeiNodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re } r.emitDataResetEvent(node, prevDataReset) + r.emitMaintenanceEvent(node, prevMaintenance) r.observeCommittedHeight(ctx, node, suppressDrift) if err := flushStatus(); err != nil { @@ -654,3 +651,36 @@ func (r *SeiNodeReconciler) emitDataResetEvent(node *seiv1alpha1.SeiNode, prev * r.Recorder.Event(node, corev1.EventTypeWarning, "DataResetFailed", cur.Message) } } + +// conditionSnapshot copies a condition, or returns nil when it is absent. A +// copy, not the pointer FindStatusCondition returns: SetStatusCondition mutates +// the slice element in place, which would erase the transition. +func conditionSnapshot(node *seiv1alpha1.SeiNode, condType string) *metav1.Condition { + c := apimeta.FindStatusCondition(node.Status.Conditions, condType) + if c == nil { + return nil + } + cp := *c + return &cp +} + +// emitMaintenanceEvent records when a hold takes effect and when seid is +// released (spec 010 Requirement 5). +func (r *SeiNodeReconciler) emitMaintenanceEvent(node *seiv1alpha1.SeiNode, prev *metav1.Condition) { + cur := apimeta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionMaintenanceInProgress) + if cur == nil || r.Recorder == nil || (prev != nil && prev.Reason == cur.Reason) { + return + } + switch cur.Reason { + case seiv1alpha1.ReasonHeld: + r.Recorder.Event(node, corev1.EventTypeNormal, "MaintenanceHeld", cur.Message) + case seiv1alpha1.ReasonArmed: + r.Recorder.Event(node, corev1.EventTypeNormal, "MaintenanceArmed", cur.Message) + case seiv1alpha1.ReasonNotHeld: + // A release runs a plan, so the condition passes through HoldPending + // on its way to NotHeld; any True state before NotHeld is a release. + if prev != nil && prev.Status == metav1.ConditionTrue { + r.Recorder.Event(node, corev1.EventTypeNormal, "MaintenanceReleased", cur.Message) + } + } +} diff --git a/internal/controller/node/datareset_events_test.go b/internal/controller/node/datareset_events_test.go index 4c32e4a5..b4fba220 100644 --- a/internal/controller/node/datareset_events_test.go +++ b/internal/controller/node/datareset_events_test.go @@ -46,3 +46,38 @@ func TestEmitDataResetEvent(t *testing.T) { }) } } + +// 010 Req 5.6 (harbor e2e finding): a release reaches NotHeld from HoldPending, +// because the release plan runs first, and still records MaintenanceReleased. +func TestEmitMaintenanceEvent(t *testing.T) { + cond := func(status metav1.ConditionStatus, reason string) *metav1.Condition { + return &metav1.Condition{Type: seiv1alpha1.ConditionMaintenanceInProgress, Status: status, Reason: reason, Message: reason} + } + cases := []struct { + name string + prev, cur *metav1.Condition + want string + }{ + {"held", cond(metav1.ConditionTrue, seiv1alpha1.ReasonHoldPending), cond(metav1.ConditionTrue, seiv1alpha1.ReasonHeld), "MaintenanceHeld"}, + {"armed", cond(metav1.ConditionTrue, seiv1alpha1.ReasonHoldPending), cond(metav1.ConditionTrue, seiv1alpha1.ReasonArmed), "MaintenanceArmed"}, + {"released via HoldPending", cond(metav1.ConditionTrue, seiv1alpha1.ReasonHoldPending), cond(metav1.ConditionFalse, seiv1alpha1.ReasonNotHeld), "MaintenanceReleased"}, + {"released from Held", cond(metav1.ConditionTrue, seiv1alpha1.ReasonHeld), cond(metav1.ConditionFalse, seiv1alpha1.ReasonNotHeld), "MaintenanceReleased"}, + {"seeded, never held", nil, cond(metav1.ConditionFalse, seiv1alpha1.ReasonNotHeld), ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + rec := record.NewFakeRecorder(4) + r := &SeiNodeReconciler{Recorder: rec} + node := &seiv1alpha1.SeiNode{} + apimeta.SetStatusCondition(&node.Status.Conditions, *tc.cur) + + r.emitMaintenanceEvent(node, tc.prev) + if tc.want == "" { + g.Expect(rec.Events).To(BeEmpty()) + return + } + g.Expect(rec.Events).To(Receive(ContainSubstring(tc.want))) + }) + } +} diff --git a/internal/controller/node/envtest/maintenance_validation_test.go b/internal/controller/node/envtest/maintenance_validation_test.go new file mode 100644 index 00000000..dfd3b4d4 --- /dev/null +++ b/internal/controller/node/envtest/maintenance_validation_test.go @@ -0,0 +1,63 @@ +//go:build envtest + +package envtest_test + +import ( + "testing" + + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + seiv1alpha1 "github.com/sei-protocol/sei-k8s-controller/api/v1alpha1" +) + +// Admission coverage of spec.maintenance.hold (spec 010 Requirement 1, +// SC-001). These cases need no controller. + +// 010 Req 1.1: both values are accepted on a nodeConfig node. +func TestMaintenanceHold_Values_Accepted(t *testing.T) { + for _, hold := range []seiv1alpha1.MaintenanceHold{seiv1alpha1.MaintenanceHoldImmediate, seiv1alpha1.MaintenanceHoldAfterExit} { + t.Run(string(hold), func(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + node := nodeConfigNode(ns, "hold-ok") + node.Spec.Maintenance = &seiv1alpha1.MaintenanceSpec{Hold: hold} + g.Expect(testCli.Create(testCtx, node)).To(Succeed()) + }) + } +} + +// 010 Req 1.2: a hold needs spec.nodeConfig. +func TestMaintenanceHold_RequiresNodeConfig(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + node := nodeConfigNode(ns, "hold-no-nc") + node.Spec.NodeConfig = nil + node.Spec.Maintenance = &seiv1alpha1.MaintenanceSpec{Hold: seiv1alpha1.MaintenanceHoldImmediate} + + err := testCli.Create(testCtx, node) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("needs spec.nodeConfig")) +} + +// 010 Req 1.1: any other value is rejected by the enum. +func TestMaintenanceHold_UnknownValue_Rejected(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + obj := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "sei.io/v1alpha1", + "kind": "SeiNode", + "metadata": map[string]any{"name": "hold-bad", "namespace": ns}, + "spec": map[string]any{ + "chainId": "envtest-1", + "image": "sei:latest", + "fullNode": map[string]any{}, + "nodeConfig": map[string]any{ + "configRef": map[string]any{"name": "rpc-config-v1"}, + "appRef": map[string]any{"name": "rpc-app-v1"}, + }, + "maintenance": map[string]any{"hold": "Forever"}, + }, + }} + g.Expect(testCli.Create(testCtx, obj)).To(HaveOccurred()) +} diff --git a/internal/controller/nodetask/controller_test.go b/internal/controller/nodetask/controller_test.go index e8aa87d2..9ced12dc 100644 --- a/internal/controller/nodetask/controller_test.go +++ b/internal/controller/nodetask/controller_test.go @@ -713,6 +713,12 @@ func TestReconcile_MarkReady_EndToEnd(t *testing.T) { g.Expect(fakeSC.getCalls).To(Equal(0)) } +// nodeConfigRefs is the ConfigMap pair the start-guard tests give a validator. +var nodeConfigRefs = seiv1alpha1.NodeConfig{ + ConfigRef: seiv1alpha1.ConfigFileRef{Name: "val-config"}, + AppRef: seiv1alpha1.ConfigFileRef{Name: "val-config"}, +} + // 009 Req 3.3 / SC-005: while a data reset is pending, a MarkReady task fails // and submits nothing, so it cannot release seid onto data the reset has not // cleared. @@ -722,10 +728,8 @@ func TestReconcile_MarkReady_RefusedWhileResetPending(t *testing.T) { t0 := time.Now() cr := newMarkReadyTask() node := newRunningNode() - node.Spec.NodeConfig = &seiv1alpha1.NodeConfig{ - ConfigRef: seiv1alpha1.ConfigFileRef{Name: "val-config"}, - AppRef: seiv1alpha1.ConfigFileRef{Name: "val-config"}, - } + refs := nodeConfigRefs + node.Spec.NodeConfig = &refs node.Spec.DataResetGeneration = 1 fakeSC := newFakeSidecarClient() @@ -745,6 +749,33 @@ func TestReconcile_MarkReady_RefusedWhileResetPending(t *testing.T) { g.Expect(fakeSC.submitted).To(BeEmpty()) } +// 010 Req 2.4 / SC-003: while a maintenance hold is set, a MarkReady task fails +// and submits nothing, so it cannot start seid out from under the hold. +func TestReconcile_MarkReady_RefusedWhileHeld(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + t0 := time.Now() + cr := newMarkReadyTask() + node := newRunningNode() + refs := nodeConfigRefs + node.Spec.NodeConfig = &refs + node.Spec.Maintenance = &seiv1alpha1.MaintenanceSpec{Hold: seiv1alpha1.MaintenanceHoldImmediate} + fakeSC := newFakeSidecarClient() + + r, c := newReconcilerWithSidecar(t, t0, fakeSC, cr, node) + for range 2 { + _, err := r.Reconcile(ctx, req()) + g.Expect(err).NotTo(HaveOccurred()) + } + + got := getTask(t, ctx, c) + g.Expect(got.Status.Phase).To(Equal(seiv1alpha1.SeiNodeTaskPhaseFailed)) + g.Expect(got.Status.Task.Err).To(ContainSubstring("maintenance hold")) + fakeSC.mu.Lock() + defer fakeSC.mu.Unlock() + g.Expect(fakeSC.submitted).To(BeEmpty()) +} + // --------------------------------------------------------------------------- // RestartSeid // --------------------------------------------------------------------------- diff --git a/internal/planner/data_reset.go b/internal/planner/data_reset.go index fd4979ec..c33aaa87 100644 --- a/internal/planner/data_reset.go +++ b/internal/planner/data_reset.go @@ -177,18 +177,21 @@ func observeTerminalDataResetPlan(node *seiv1alpha1.SeiNode, plan *seiv1alpha1.T } // startDeferred reports whether plan failed only because the start guard -// refused its final mark-ready, after every earlier task completed. +// refused its mark-ready, after every task before it completed. func startDeferred(plan *seiv1alpha1.TaskPlan) bool { d := plan.FailedTaskDetail if d == nil || d.Type != TaskMarkReady || !strings.Contains(d.Error, task.StartGuardRefusal) { return false } for _, t := range plan.Tasks { - if t.Type != TaskMarkReady && t.Status != seiv1alpha1.TaskComplete { + if t.Type == TaskMarkReady { + return true + } + if t.Status != seiv1alpha1.TaskComplete { return false } } - return true + return false } func setDataResetCondition(node *seiv1alpha1.SeiNode, status metav1.ConditionStatus, reason, message string) { diff --git a/internal/planner/maintenance.go b/internal/planner/maintenance.go new file mode 100644 index 00000000..9ba54a5b --- /dev/null +++ b/internal/planner/maintenance.go @@ -0,0 +1,216 @@ +package planner + +import ( + "fmt" + "slices" + + "github.com/google/uuid" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + seiv1alpha1 "github.com/sei-protocol/sei-k8s-controller/api/v1alpha1" + "github.com/sei-protocol/sei-k8s-controller/internal/noderesource" + "github.com/sei-protocol/sei-k8s-controller/internal/task" + sidecar "github.com/sei-protocol/sei-k8s-controller/sidecarapi/client" +) + +// Spec 010 (maintenance hold). spec.maintenance.hold is the request; +// status.maintenanceHold is the hold in effect. Every plan that changes what +// the hold does to seid ends with record-maintenance-hold, so the two compare +// the same way spec 009 compares its counters. + +// planStep is one task of a plan under construction. +type planStep struct { + taskType string + params any +} + +// buildHoldPlan returns the plan that moves the hold in effect from have to +// want, or nil when they agree and the start gate is where the hold says. +// +// want=Immediate mark-not-ready -> stop-seid -> record(Immediate) +// want=AfterExit, have="" mark-not-ready -> record(AfterExit) +// want=AfterExit, have=Imm. observe-image -> start-seid-once -> +// await-seid-start -> mark-not-ready -> record(AfterExit) +// want="" (release) observe-image -> mark-ready -> record("") +// +// start-seid-once starts a parked seid once; mark-not-ready closes the gate +// again a few seconds after seid starts, long before it loads its state, so it +// parks at its next exit. observe-image waits for any rollout, so the sidecar +// tasks reach the pod that runs the current template. +// +// A hold in effect is level-triggered against the observed gate: when a hold is +// requested and the sidecar reports ready (the gate is open) under the hold in +// effect, the hold in effect is applied again first, as if from none. That +// closes a gate a failed start-once plan left open, whether or not the request +// still differs; the next plan then moves the hold with the gate closed. +func buildHoldPlan(node *seiv1alpha1.SeiNode, want, have seiv1alpha1.MaintenanceHold) (*seiv1alpha1.TaskPlan, error) { + switch { + case want != "" && have != "" && sidecarGateOpen(node): + want, have = have, "" + case want == have: + return nil, nil + } + observe := planStep{task.TaskTypeObserveImage, task.ObserveImageParams{NodeName: node.Name, Namespace: node.Namespace}} + stopUpCheck := noderesource.UpCheckForNode(node) + + steps := make([]planStep, 0, 5) + switch { + case want == seiv1alpha1.MaintenanceHoldImmediate: + steps = append(steps, + planStep{taskTypeMarkNotReady, sidecar.MarkNotReadyTask{}}, + planStep{taskTypeStopSeid, sidecar.StopSeidTask{UpCheck: &stopUpCheck}}, + ) + case want == seiv1alpha1.MaintenanceHoldAfterExit && have == seiv1alpha1.MaintenanceHoldImmediate: + steps = append(steps, + observe, + planStep{task.TaskTypeStartSeidOnce, sidecar.MarkReadyTask{}}, + planStep{sidecar.TaskTypeAwaitSeidStart, sidecar.AwaitSeidStartTask{}}, + planStep{taskTypeMarkNotReady, sidecar.MarkNotReadyTask{}}, + ) + case want == seiv1alpha1.MaintenanceHoldAfterExit: + steps = append(steps, planStep{taskTypeMarkNotReady, sidecar.MarkNotReadyTask{}}) + default: // release + steps = append(steps, observe, planStep{TaskMarkReady, sidecar.MarkReadyTask{}}) + } + steps = append(steps, recordHoldStep(want)) + return assembleSteps(steps) +} + +func recordHoldStep(hold seiv1alpha1.MaintenanceHold) planStep { + return planStep{task.TaskTypeRecordMaintenanceHold, task.RecordMaintenanceHoldParams{Hold: hold}} +} + +// assembleSteps turns steps into an Active Running-phase plan. FailedPhase +// stays empty: a failed hold or release keeps the node Running and the planner +// builds the plan again. +func assembleSteps(steps []planStep) (*seiv1alpha1.TaskPlan, error) { + planID := uuid.New().String() + tasks := make([]seiv1alpha1.PlannedTask, 0, len(steps)) + for i, s := range steps { + t, err := buildPlannedTask(planID, s.taskType, i, s.params) + if err != nil { + return nil, err + } + tasks = append(tasks, t) + } + return &seiv1alpha1.TaskPlan{ + ID: planID, + Phase: seiv1alpha1.TaskPlanActive, + Tasks: tasks, + TargetPhase: seiv1alpha1.PhaseRunning, + }, nil +} + +// parkInsteadOfRelease ends a plan with seid parked rather than started, for a +// node whose hold is requested: the trailing mark-ready becomes +// record-maintenance-hold(Immediate). The reset plan and the init plan use it. +func parkInsteadOfRelease(plan *seiv1alpha1.TaskPlan) error { + last := len(plan.Tasks) - 1 + if last < 0 || plan.Tasks[last].Type != TaskMarkReady { + return fmt.Errorf("plan %s does not end in %s; cannot park it", plan.ID, TaskMarkReady) + } + step := recordHoldStep(seiv1alpha1.MaintenanceHoldImmediate) + t, err := buildPlannedTask(plan.ID, step.taskType, last, step.params) + if err != nil { + return err + } + plan.Tasks[last] = t + return nil +} + +// clearHoldAfterRelease ends a plan that releases seid with +// record-maintenance-hold(""), for a node whose hold was removed while still +// in effect. The reset plan uses it when the release and a reset arrive +// together, so the plan that marks the sidecar ready clears the hold, as the +// release plan does. +func clearHoldAfterRelease(plan *seiv1alpha1.TaskPlan) error { + last := len(plan.Tasks) - 1 + if last < 0 || plan.Tasks[last].Type != TaskMarkReady { + return fmt.Errorf("plan %s does not end in %s; cannot release from it", plan.ID, TaskMarkReady) + } + step := recordHoldStep("") + t, err := buildPlannedTask(plan.ID, step.taskType, last+1, step.params) + if err != nil { + return err + } + plan.Tasks = append(plan.Tasks, t) + return nil +} + +// withoutMarkReady drops mark-ready from a plan built for a held node, so an +// image roll under a hold leaves the new pod parked. +func withoutMarkReady(plan *seiv1alpha1.TaskPlan) { + plan.Tasks = slices.DeleteFunc(plan.Tasks, func(t seiv1alpha1.PlannedTask) bool { + return t.Type == TaskMarkReady + }) +} + +// sidecarGateOpen reports whether the last sidecar probe found the start gate +// open (healthz 200). An unreachable sidecar is not evidence either way. +func sidecarGateOpen(node *seiv1alpha1.SeiNode) bool { + c := meta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionSidecarReady) + return c != nil && c.Status == metav1.ConditionTrue +} + +// isMaintenancePlan reports whether plan changes the hold in effect. +func isMaintenancePlan(plan *seiv1alpha1.TaskPlan) bool { + return plan != nil && slices.ContainsFunc(plan.Tasks, func(t seiv1alpha1.PlannedTask) bool { + return t.Type == task.TaskTypeRecordMaintenanceHold + }) +} + +// ResolveMaintenance keeps the always-present MaintenanceInProgress condition +// current from the requested hold and the hold in effect. The reconciler calls +// it on every path, before the Failed and Paused early returns. +func ResolveMaintenance(node *seiv1alpha1.SeiNode) { + if node.Spec.NodeConfig == nil { + setMaintenanceCondition(node, metav1.ConditionFalse, seiv1alpha1.ReasonMaintenanceNotApplicable, + "spec.nodeConfig is not set; the maintenance hold applies only to ConfigMap-configured nodes") + return + } + want, have := node.Spec.HoldRequested(), node.Status.MaintenanceHold + plan := node.Status.Plan + switch { + case isMaintenancePlan(plan) && plan.Phase == seiv1alpha1.TaskPlanActive: + // seid's state is changing; Held or Armed would invite exec work. + setMaintenanceCondition(node, metav1.ConditionTrue, seiv1alpha1.ReasonHoldPending, + fmt.Sprintf("hold plan running; requested: %s, in effect: %s", holdOrNone(want), holdOrNone(have))) + case want == "" && have == "": + setMaintenanceCondition(node, metav1.ConditionFalse, seiv1alpha1.ReasonNotHeld, "no maintenance hold") + case want != have: + // A release counts: seid stays parked until the release plan runs, but + // exec work must stop now. + message := fmt.Sprintf("hold %s requested; in effect: %s", holdOrNone(want), holdOrNone(have)) + if want == "" { + message = fmt.Sprintf("hold %s in effect; release pending", have) + } + setMaintenanceCondition(node, metav1.ConditionTrue, seiv1alpha1.ReasonHoldPending, message) + case sidecarGateOpen(node): + setMaintenanceCondition(node, metav1.ConditionTrue, seiv1alpha1.ReasonHoldPending, + fmt.Sprintf("hold %s in effect but the start gate is open; closing it", want)) + default: + reason := seiv1alpha1.ReasonHeld + if have == seiv1alpha1.MaintenanceHoldAfterExit { + reason = seiv1alpha1.ReasonArmed + } + setMaintenanceCondition(node, metav1.ConditionTrue, reason, fmt.Sprintf("hold %s in effect", have)) + } +} + +func holdOrNone(h seiv1alpha1.MaintenanceHold) string { + if h == "" { + return "none" + } + return string(h) +} + +func setMaintenanceCondition(node *seiv1alpha1.SeiNode, status metav1.ConditionStatus, reason, message string) { + meta.SetStatusCondition(&node.Status.Conditions, metav1.Condition{ + Type: seiv1alpha1.ConditionMaintenanceInProgress, + Status: status, + Reason: reason, + Message: message, + ObservedGeneration: node.Generation, + }) +} diff --git a/internal/planner/maintenance_test.go b/internal/planner/maintenance_test.go new file mode 100644 index 00000000..cf7d4b30 --- /dev/null +++ b/internal/planner/maintenance_test.go @@ -0,0 +1,435 @@ +package planner + +import ( + "context" + "encoding/json" + "testing" + + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + seiv1alpha1 "github.com/sei-protocol/sei-k8s-controller/api/v1alpha1" + "github.com/sei-protocol/sei-k8s-controller/internal/task" + sidecar "github.com/sei-protocol/sei-k8s-controller/sidecarapi/client" +) + +// Spec 010 (maintenance hold): planner-side coverage. Each test names the +// requirement it covers. + +const ( + holdImmediate = seiv1alpha1.MaintenanceHoldImmediate + holdAfterExit = seiv1alpha1.MaintenanceHoldAfterExit +) + +func heldNode(want, have seiv1alpha1.MaintenanceHold) *seiv1alpha1.SeiNode { + node := withNodeConfig(runningFullNode()) + if want != "" { + node.Spec.Maintenance = &seiv1alpha1.MaintenanceSpec{Hold: want} + } + node.Status.MaintenanceHold = have + return node +} + +func recordedHold(t *testing.T, plan *seiv1alpha1.TaskPlan) seiv1alpha1.MaintenanceHold { + t.Helper() + last := plan.Tasks[len(plan.Tasks)-1] + if last.Type != task.TaskTypeRecordMaintenanceHold { + t.Fatalf("plan ends with %s, want %s", last.Type, task.TaskTypeRecordMaintenanceHold) + } + var p task.RecordMaintenanceHoldParams + if err := json.Unmarshal(last.Params.Raw, &p); err != nil { + t.Fatal(err) + } + return p.Hold +} + +// 010 Req 2.1-2.3, 2.7, 4.1, 4.3 / SC-002: each move between the requested +// hold and the hold in effect builds its own plan, ending with the record. +func TestHoldPlans(t *testing.T) { + cases := []struct { + name string + want, have seiv1alpha1.MaintenanceHold + types []string + }{ + {"hold now", holdImmediate, "", []string{ + taskTypeMarkNotReady, taskTypeStopSeid, task.TaskTypeRecordMaintenanceHold}}, + {"arm a running seid", holdAfterExit, "", []string{ + taskTypeMarkNotReady, task.TaskTypeRecordMaintenanceHold}}, + {"start a parked seid once", holdAfterExit, holdImmediate, []string{ + task.TaskTypeObserveImage, task.TaskTypeStartSeidOnce, sidecar.TaskTypeAwaitSeidStart, + taskTypeMarkNotReady, task.TaskTypeRecordMaintenanceHold}}, + {"stop an armed seid now", holdImmediate, holdAfterExit, []string{ + taskTypeMarkNotReady, taskTypeStopSeid, task.TaskTypeRecordMaintenanceHold}}, + {"release a parked seid", "", holdImmediate, []string{ + task.TaskTypeObserveImage, TaskMarkReady, task.TaskTypeRecordMaintenanceHold}}, + {"release an armed seid", "", holdAfterExit, []string{ + task.TaskTypeObserveImage, TaskMarkReady, task.TaskTypeRecordMaintenanceHold}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + node := heldNode(tc.want, tc.have) + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).NotTo(BeNil()) + g.Expect(planTaskTypes(node.Status.Plan)).To(Equal(tc.types)) + g.Expect(recordedHold(t, node.Status.Plan)).To(Equal(tc.want)) + g.Expect(node.Status.Plan.FailedPhase).To(BeEmpty()) + }) + } +} + +// 010 Req 2.8: a hold already in effect plans nothing; a pod that lost +// readiness under it is not re-marked ready (Req 2.5). +func TestHoldInEffect_NoPlanNoReapproval(t *testing.T) { + for _, hold := range []seiv1alpha1.MaintenanceHold{holdImmediate, holdAfterExit} { + t.Run(string(hold), func(t *testing.T) { + g := NewWithT(t) + node := heldNode(hold, hold) + setSidecarReadyCondition(node, metav1.ConditionFalse, "NotReady", "pod rolled") + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).To(BeNil()) + }) + } +} + +// 010 Req 2.5, 2.6: an image change under a hold rolls the pod but leaves it +// parked: the update plan carries no mark-ready. +func TestHold_UpdatePlanCarriesNoMarkReady(t *testing.T) { + g := NewWithT(t) + node := heldNode(holdImmediate, holdImmediate) + node.Spec.Image = testImageV2 + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).NotTo(BeNil()) + types := planTaskTypes(node.Status.Plan) + g.Expect(types).To(ContainElement(task.TaskTypeObserveImage)) + g.Expect(types).NotTo(ContainElement(TaskMarkReady)) +} + +// 010 Req 3 / SC-004: a reset on a held node parks instead of releasing, and +// the hold in effect becomes Immediate. +func TestHold_ResetParks(t *testing.T) { + g := NewWithT(t) + node := heldNode(holdAfterExit, holdAfterExit) + node.Spec.DataResetGeneration = 1 + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + plan := node.Status.Plan + g.Expect(isDataResetPlan(plan)).To(BeTrue(), "the reset comes before any hold plan") + g.Expect(planTaskTypes(plan)).NotTo(ContainElement(TaskMarkReady)) + g.Expect(recordedHold(t, plan)).To(Equal(holdImmediate)) +} + +// 010 Req 4.2: release with a reset pending runs the full reset plan through +// its mark-ready, then clears the hold in effect. +func TestRelease_WithResetPending(t *testing.T) { + g := NewWithT(t) + node := heldNode("", holdImmediate) + node.Spec.DataResetGeneration = 1 + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + types := planTaskTypes(node.Status.Plan) + g.Expect(types[len(types)-2:]).To(Equal([]string{TaskMarkReady, task.TaskTypeRecordMaintenanceHold})) + g.Expect(recordedHold(t, node.Status.Plan)).To(BeEmpty()) +} + +// 010 Req 2.9 / User Story 5: a node created with a hold initializes without +// starting seid, and reaches Running with the hold in effect. +func TestHold_InitPlanParks(t *testing.T) { + for _, mode := range staticModes { + t.Run(mode.name, func(t *testing.T) { + g := NewWithT(t) + node := withNodeConfig(pendingNode(mode.configure)) + node.Spec.Maintenance = &seiv1alpha1.MaintenanceSpec{Hold: holdImmediate} + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).NotTo(BeNil()) + g.Expect(planTaskTypes(node.Status.Plan)).NotTo(ContainElement(TaskMarkReady)) + g.Expect(recordedHold(t, node.Status.Plan)).To(Equal(holdImmediate)) + g.Expect(node.Status.Plan.TargetPhase).To(Equal(seiv1alpha1.PhaseRunning)) + }) + } +} + +// 010 Req 2.4 / SC-003: a plan built before the hold cannot release seid; the +// next plan is the hold plan. +func TestHold_StaleUpdatePlanCannotReleaseGate(t *testing.T) { + g := NewWithT(t) + s := testScheme(t) + node := withNodeConfig(runningFullNode()) + node.Spec.Image = testImageV2 + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + plan := node.Status.Plan + completeTasksBefore(plan, len(plan.Tasks)-1) + + node.Spec.Maintenance = &seiv1alpha1.MaintenanceSpec{Hold: holdImmediate} + + mock := &mockSidecarClient{} + _, err := nodeExecutor(fake.NewClientBuilder().WithScheme(s), s, mock).ExecutePlan(context.Background(), node, plan) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(plan.Phase).To(Equal(seiv1alpha1.TaskPlanFailed)) + g.Expect(plan.FailedTaskDetail.Error).To(ContainSubstring("maintenance hold")) + g.Expect(mock.submitted).To(BeEmpty()) + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(isMaintenancePlan(node.Status.Plan)).To(BeTrue()) +} + +// 010 Req 2.3, 2.4: the start-once step passes the guard under a hold, and the +// plan records AfterExit once mark-not-ready closes the gate again. +func TestHold_StartOncePassesGuard(t *testing.T) { + g := NewWithT(t) + s := testScheme(t) + node := heldNode(holdAfterExit, holdImmediate) + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + plan := node.Status.Plan + completeTasksBefore(plan, 1) // observe-image done + + mock := &mockSidecarClient{} + _, err := nodeExecutor(fake.NewClientBuilder().WithScheme(s), s, mock).ExecutePlan(context.Background(), node, plan) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(plan.Phase).To(Equal(seiv1alpha1.TaskPlanActive), "await-seid-start is polled") + g.Expect(mock.submitted).NotTo(BeEmpty()) + g.Expect(mock.submitted[0].Type).To(Equal(sidecar.TaskTypeMarkReady), "start-seid-once submits mark-ready") +} + +// 010 Req 5 / SC-005: the condition follows the requested hold and the hold in +// effect. +func TestResolveMaintenance(t *testing.T) { + cases := []struct { + name string + nodeConfig bool + want, have seiv1alpha1.MaintenanceHold + status metav1.ConditionStatus + reason string + }{ + {"no nodeConfig", false, "", "", metav1.ConditionFalse, seiv1alpha1.ReasonMaintenanceNotApplicable}, + {"not held", true, "", "", metav1.ConditionFalse, seiv1alpha1.ReasonNotHeld}, + {"hold requested", true, holdImmediate, "", metav1.ConditionTrue, seiv1alpha1.ReasonHoldPending}, + {"held", true, holdImmediate, holdImmediate, metav1.ConditionTrue, seiv1alpha1.ReasonHeld}, + {"armed", true, holdAfterExit, holdAfterExit, metav1.ConditionTrue, seiv1alpha1.ReasonArmed}, + {"start once pending", true, holdAfterExit, holdImmediate, metav1.ConditionTrue, seiv1alpha1.ReasonHoldPending}, + {"release pending", true, "", holdImmediate, metav1.ConditionTrue, seiv1alpha1.ReasonHoldPending}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + node := heldNode(tc.want, tc.have) + if !tc.nodeConfig { + node.Spec.NodeConfig = nil + } + ResolveMaintenance(node) + cond := meta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionMaintenanceInProgress) + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Status).To(Equal(tc.status)) + g.Expect(cond.Reason).To(Equal(tc.reason)) + }) + } +} + +// Review finding: a hold set after the init plan was built must not fail the +// node. The init plan's mark-ready passes, the node reaches Running, and the +// next plan is the hold plan. +func TestHold_SetMidInitDoesNotFailNode(t *testing.T) { + g := NewWithT(t) + s := testScheme(t) + node := withNodeConfig(pendingNode(func(n *seiv1alpha1.SeiNode) { n.Spec.FullNode = &seiv1alpha1.FullNodeSpec{} })) + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + plan := node.Status.Plan + g.Expect(plan.FailedPhase).To(Equal(seiv1alpha1.PhaseFailed), "an init plan failure is terminal, which is why this matters") + completeTasksBefore(plan, len(plan.Tasks)-1) + + node.Spec.Maintenance = &seiv1alpha1.MaintenanceSpec{Hold: holdImmediate} + + mock := &mockSidecarClient{} + _, err := nodeExecutor(fake.NewClientBuilder().WithScheme(s), s, mock).ExecutePlan(context.Background(), node, plan) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(plan.Phase).To(Equal(seiv1alpha1.TaskPlanComplete)) + g.Expect(node.Status.Phase).To(Equal(seiv1alpha1.PhaseRunning)) + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(isMaintenancePlan(node.Status.Plan)).To(BeTrue()) +} + +// seidroid #595 blocker 1: the operator changes AfterExit back to Immediate +// while the start-once plan runs. The start-once step refuses, nothing starts +// seid, and with the gate still closed the planner has nothing left to do. +func TestHold_StartOnceRefusedAfterFlipToImmediate(t *testing.T) { + g := NewWithT(t) + s := testScheme(t) + node := heldNode(holdAfterExit, holdImmediate) + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + plan := node.Status.Plan + completeTasksBefore(plan, 1) // observe-image done + + node.Spec.Maintenance.Hold = holdImmediate + + mock := &mockSidecarClient{} + _, err := nodeExecutor(fake.NewClientBuilder().WithScheme(s), s, mock).ExecutePlan(context.Background(), node, plan) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(plan.Phase).To(Equal(seiv1alpha1.TaskPlanFailed)) + g.Expect(plan.FailedTaskDetail.Type).To(Equal(task.TaskTypeStartSeidOnce)) + g.Expect(mock.submitted).To(BeEmpty(), "start-seid-once must not reach the sidecar") + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).To(BeNil(), "the gate never opened, so the Immediate hold still holds") +} + +// seidroid #595 blocker 2: a hold in effect whose gate the sidecar reports +// open (a start-once plan failed after mark-ready) is rebuilt, so the gate +// closes again even though the requested and in-effect holds agree. +func TestHold_OpenGateUnderHoldIsClosed(t *testing.T) { + cases := []struct { + name string + want, have seiv1alpha1.MaintenanceHold + types []string + records seiv1alpha1.MaintenanceHold + }{ + {"Immediate in effect", holdImmediate, holdImmediate, + []string{taskTypeMarkNotReady, taskTypeStopSeid, task.TaskTypeRecordMaintenanceHold}, holdImmediate}, + {"AfterExit in effect", holdAfterExit, holdAfterExit, + []string{taskTypeMarkNotReady, task.TaskTypeRecordMaintenanceHold}, holdAfterExit}, + // seidroid #595 follow-up: a start-once plan failed after opening the + // gate. The Immediate hold in effect is applied again first; start-once + // is retried on the next plan, with the gate closed. + {"start-once failed, gate open", holdAfterExit, holdImmediate, + []string{taskTypeMarkNotReady, taskTypeStopSeid, task.TaskTypeRecordMaintenanceHold}, holdImmediate}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + node := heldNode(tc.want, tc.have) + setSidecarReadyCondition(node, metav1.ConditionTrue, "Ready", "sidecar returned 200") + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).NotTo(BeNil()) + g.Expect(planTaskTypes(node.Status.Plan)).To(Equal(tc.types)) + g.Expect(recordedHold(t, node.Status.Plan)).To(Equal(tc.records)) + }) + } +} + +// seidroid #595: the condition reads HoldPending, never Held or Armed, while a +// hold plan runs or while the gate is open under a hold. +func TestResolveMaintenance_NotHeldWhileChanging(t *testing.T) { + g := NewWithT(t) + node := heldNode(holdAfterExit, holdImmediate) + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + node.Spec.Maintenance.Hold = holdImmediate // want == have, but a plan runs + + ResolveMaintenance(node) + cond := meta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionMaintenanceInProgress) + g.Expect(cond.Reason).To(Equal(seiv1alpha1.ReasonHoldPending)) + + node.Status.Plan = nil + setSidecarReadyCondition(node, metav1.ConditionTrue, "Ready", "sidecar returned 200") + ResolveMaintenance(node) + cond = meta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionMaintenanceInProgress) + g.Expect(cond.Reason).To(Equal(seiv1alpha1.ReasonHoldPending)) + g.Expect(cond.Message).To(ContainSubstring("gate is open")) +} + +// seidroid #595 nit: a held init plan keeps its "init" metric label. +func TestClassifyPlan_HeldInitIsInit(t *testing.T) { + g := NewWithT(t) + node := withNodeConfig(pendingNode(func(n *seiv1alpha1.SeiNode) { n.Spec.FullNode = &seiv1alpha1.FullNodeSpec{} })) + node.Spec.Maintenance = &seiv1alpha1.MaintenanceSpec{Hold: holdImmediate} + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(classifyPlan(node.Status.Plan)).To(Equal("init")) +} + +// 009 Req 5.9, 010 Req 3: a hold that arrives while a reset runs makes the +// guard refuse the reset plan's final mark-ready. The wipe ran and was +// recorded, so the reset reads ResetComplete with the start deferred, and the +// next plan is the hold plan. +func TestHold_ArrivesMidReset(t *testing.T) { + g := NewWithT(t) + s := testScheme(t) + node := heldNode("", "") + node.Spec.DataResetGeneration = 1 + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + plan := node.Status.Plan + g.Expect(isDataResetPlan(plan)).To(BeTrue()) + completeTasksBefore(plan, 4) + + node.Spec.Maintenance = &seiv1alpha1.MaintenanceSpec{Hold: holdImmediate} + + mock := &mockSidecarClient{} + _, err := nodeExecutor(fake.NewClientBuilder().WithScheme(s), s, mock).ExecutePlan(context.Background(), node, plan) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(plan.Phase).To(Equal(seiv1alpha1.TaskPlanFailed)) + g.Expect(node.Status.DataResetGeneration).To(Equal(int64(1))) + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + cond := dataResetCondition(node) + g.Expect(cond.Reason).To(Equal(seiv1alpha1.ReasonResetComplete)) + g.Expect(cond.Message).To(ContainSubstring("maintenance hold")) + g.Expect(isMaintenancePlan(node.Status.Plan)).To(BeTrue()) +} + +// seidroid #595 blocker 3, 010 Req 4.2, 4.3, 5.3: removing the hold in the same +// commit that raises the counter builds the reset plan with its mark-ready, then +// clears the hold in effect. The condition reads HoldPending while the wipe runs +// and seid starts, never Held, and NotHeld once the plan completes. +func TestHold_ReleasedWithReset(t *testing.T) { + g := NewWithT(t) + s := testScheme(t) + node := heldNode("", holdImmediate) + node.Spec.DataResetGeneration = 1 + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + plan := node.Status.Plan + g.Expect(isDataResetPlan(plan)).To(BeTrue()) + g.Expect(plan.Tasks[len(plan.Tasks)-2].Type).To(Equal(TaskMarkReady)) + g.Expect(recordedHold(t, plan)).To(BeEmpty()) + + ResolveMaintenance(node) + cond := meta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionMaintenanceInProgress) + g.Expect(cond.Reason).To(Equal(seiv1alpha1.ReasonHoldPending)) + + completeTasksBefore(plan, 4) // the wipe ran; record-data-reset is next + mock := &mockSidecarClient{} + exec := nodeExecutor(fake.NewClientBuilder().WithScheme(s), s, mock) + for i := 0; i < 3 && plan.Phase == seiv1alpha1.TaskPlanActive; i++ { + _, err := exec.ExecutePlan(context.Background(), node, plan) + g.Expect(err).NotTo(HaveOccurred()) + } + g.Expect(plan.Phase).To(Equal(seiv1alpha1.TaskPlanComplete)) + g.Expect(node.Status.DataResetGeneration).To(Equal(int64(1))) + g.Expect(node.Status.MaintenanceHold).To(BeEmpty()) + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).To(BeNil(), "the reset plan released the hold; no release plan follows") + ResolveMaintenance(node) + cond = meta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionMaintenanceInProgress) + g.Expect(cond.Reason).To(Equal(seiv1alpha1.ReasonNotHeld)) +} + +// A reset plan that also clears a hold keeps the deferred-start reading: a +// newer counter makes the guard refuse mark-ready, the trailing record step +// never runs, the hold stays in effect, and the next reset releases it. +func TestHold_ReleasedWithReset_StartDeferred(t *testing.T) { + g := NewWithT(t) + s := testScheme(t) + node := heldNode("", holdImmediate) + node.Spec.DataResetGeneration = 1 + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + plan := node.Status.Plan + completeTasksBefore(plan, 4) + + node.Spec.DataResetGeneration = 2 // a newer reset commit lands mid-plan + + mock := &mockSidecarClient{} + _, err := nodeExecutor(fake.NewClientBuilder().WithScheme(s), s, mock).ExecutePlan(context.Background(), node, plan) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(plan.Phase).To(Equal(seiv1alpha1.TaskPlanFailed)) + g.Expect(startDeferred(plan)).To(BeTrue(), "the pending record step must not turn a refused start into a failed reset") + g.Expect(node.Status.MaintenanceHold).To(Equal(holdImmediate), "seid never started, so the hold stays in effect") + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(isDataResetPlan(node.Status.Plan)).To(BeTrue()) + g.Expect(recordedHold(t, node.Status.Plan)).To(BeEmpty()) +} diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 6920d98e..0447d76a 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -316,6 +316,11 @@ func classifyPlan(plan *seiv1alpha1.TaskPlan) string { if isDataResetPlan(plan) { return "data-reset" } + if isMaintenancePlan(plan) && !slices.ContainsFunc(plan.Tasks, func(t seiv1alpha1.PlannedTask) bool { + return t.Type == task.TaskTypeEnsureDataPVC // a held init plan is still an init plan + }) { + return "maintenance-hold" + } for _, t := range plan.Tasks { switch t.Type { case task.TaskTypeObserveImage: diff --git a/internal/planner/static_config.go b/internal/planner/static_config.go index f4ca9bd6..86eaaa31 100644 --- a/internal/planner/static_config.go +++ b/internal/planner/static_config.go @@ -94,36 +94,70 @@ func (p *staticConfigPlanner) Validate(node *seiv1alpha1.SeiNode) error { return p.base.Validate(node) } -// BuildPlan delegates every arm but Running to the mode planner. +// BuildPlan delegates every arm but Running to the mode planner. A node created +// with a maintenance hold initializes but does not start: the init plan ends +// with the hold in effect instead of mark-ready (spec 010 Req 2.9). func (p *staticConfigPlanner) BuildPlan(node *seiv1alpha1.SeiNode) (*seiv1alpha1.TaskPlan, error) { if node.Status.Phase == seiv1alpha1.PhaseRunning { return p.buildRunningPlan(node) } - return p.base.BuildPlan(node) + plan, err := p.base.BuildPlan(node) + if err != nil || plan == nil || node.Spec.HoldRequested() == "" { + return plan, err + } + if err := parkInsteadOfRelease(plan); err != nil { + return nil, err + } + return plan, nil } // buildRunningPlan returns the next plan for a Running node, or nil if none is -// needed. A pending data reset comes first: the reset plan also waits for any -// rollout, so it serves a reset commit that changes the template too. There is -// no configValues arm: the CRD rejects configValues alongside nodeConfig. +// needed. The order is the safety order: +// +// 1. a pending data reset (spec 009), whose plan also waits for any rollout, +// so it serves a reset commit that changes the template too; +// 2. a change to the maintenance hold (spec 010); +// 3. pod-template drift; +// 4. a readiness reapproval, never while a hold is requested. +// +// While a hold is requested no plan releases seid: the reset plan parks +// instead, and the update plan carries no mark-ready. A reset plan that +// releases a hold still in effect clears it after mark-ready. There is no configValues +// arm: the CRD rejects configValues alongside nodeConfig. func (p *staticConfigPlanner) buildRunningPlan(node *seiv1alpha1.SeiNode) (*seiv1alpha1.TaskPlan, error) { + held := node.Spec.HoldRequested() != "" if task.DataResetPending(node) { plan, err := buildDataResetPlan(node) if err != nil { return nil, err } + switch { + case held: + err = parkInsteadOfRelease(plan) + case node.Status.MaintenanceHold != "": + err = clearHoldAfterRelease(plan) + } + if err != nil { + return nil, err + } markDataResetStarted(node) return plan, nil } + if plan, err := buildHoldPlan(node, node.Spec.HoldRequested(), node.Status.MaintenanceHold); err != nil || plan != nil { + return plan, err + } if podTemplateDrifted(node, p.platform) { plan, err := p.buildUpdatePlan(node) if err != nil { return nil, err } + if held { + withoutMarkReady(plan) + } setNodeUpdateCondition(node, metav1.ConditionTrue, "UpdateStarted", podTemplateDriftMessage(node, p.platform)) return plan, nil } - if sidecarNeedsReapproval(node) { + if sidecarNeedsReapproval(node) && !held { return buildMarkReadyPlan(node) } return nil, nil diff --git a/internal/task/record_maintenance_hold.go b/internal/task/record_maintenance_hold.go new file mode 100644 index 00000000..00a09af0 --- /dev/null +++ b/internal/task/record_maintenance_hold.go @@ -0,0 +1,55 @@ +package task + +import ( + "context" + "encoding/json" + "fmt" + + seiv1alpha1 "github.com/sei-protocol/sei-k8s-controller/api/v1alpha1" +) + +const TaskTypeRecordMaintenanceHold = "record-maintenance-hold" + +// RecordMaintenanceHoldParams carries the hold now in effect: Immediate when +// the plan left seid parked, AfterExit when it left the gate armed, "" when it +// released seid. +type RecordMaintenanceHoldParams struct { + Hold seiv1alpha1.MaintenanceHold `json:"hold"` +} + +type recordMaintenanceHoldExecution struct { + taskBase + params RecordMaintenanceHoldParams + cfg ExecutionConfig +} + +func deserializeRecordMaintenanceHold(id string, params json.RawMessage, cfg ExecutionConfig) (TaskExecution, error) { + var p RecordMaintenanceHoldParams + if len(params) > 0 { + if err := json.Unmarshal(params, &p); err != nil { + return nil, fmt.Errorf("deserializing record-maintenance-hold params: %w", err) + } + } + return &recordMaintenanceHoldExecution{ + taskBase: taskBase{id: id, status: ExecutionRunning}, + params: p, + cfg: cfg, + }, nil +} + +// Execute sets status.maintenanceHold in memory. It is the last step of every +// plan that changes what the hold does to seid, so the status names the hold in +// effect only after the gate and seid are in that state. +func (e *recordMaintenanceHoldExecution) Execute(_ context.Context) error { + node, err := ResourceAs[*seiv1alpha1.SeiNode](e.cfg) + if err != nil { + return Terminal(err) + } + node.Status.MaintenanceHold = e.params.Hold + e.complete() + return nil +} + +func (e *recordMaintenanceHoldExecution) Status(_ context.Context) ExecutionStatus { + return e.DefaultStatus() +} diff --git a/internal/task/start_guard.go b/internal/task/start_guard.go index b7a4b3dc..b0cc7aad 100644 --- a/internal/task/start_guard.go +++ b/internal/task/start_guard.go @@ -22,8 +22,26 @@ func DataResetPending(node *seiv1alpha1.SeiNode) bool { // "" when mark-ready may reach the sidecar. It is the start guard: the one rule // every path that can start seid obeys, so a plan built before the spec changed, // or a MarkReady SeiNodeTask, cannot release seid onto data a reset has not -// cleared yet. +// cleared yet, or out from under a maintenance hold. +// +// The hold half acts only on a Running node. An init plan fails the node +// terminally (FailedPhase=Failed), so refusing its mark-ready because a hold +// arrived mid-init would destroy the node; instead seid may start once, and the +// hold plan stops it when the node reaches Running. A hold set before the init +// plan is built parks the node without starting it (parkInsteadOfRelease). func StartBlocked(node *seiv1alpha1.SeiNode) string { + if reason := resetBlocks(node); reason != "" { + return reason + } + if hold := node.Spec.HoldRequested(); hold != "" && node.Status.Phase == seiv1alpha1.PhaseRunning { + return fmt.Sprintf("maintenance hold %s is set", hold) + } + return "" +} + +// resetBlocks is the reset half of the start guard. The hold's own start-once +// step obeys only this half: it is how an AfterExit hold starts a parked seid. +func resetBlocks(node *seiv1alpha1.SeiNode) string { if DataResetPending(node) { return fmt.Sprintf("data reset pending: spec.dataResetGeneration=%d, status.dataResetGeneration=%d", node.Spec.DataResetGeneration, node.Status.DataResetGeneration) @@ -31,12 +49,42 @@ func StartBlocked(node *seiv1alpha1.SeiNode) string { return "" } +// TaskTypeStartSeidOnce is the maintenance hold's start-once step. It submits +// the sidecar's mark-ready, like the mark-ready plan task, but an AfterExit hold +// does not block it: an AfterExit hold on a parked node starts seid once, waits +// for await-seid-start, and closes the gate again with mark-not-ready. A pending +// reset, or any request other than AfterExit, blocks it. +const TaskTypeStartSeidOnce = "start-seid-once" + // deserializeMarkReady wraps the mark-ready sidecar task in the start guard. // Both callers resolve the same SeiNode into cfg.Resource: the plan executor // (the reconciled node) and the SeiNodeTask controller (the target node). A // resource that is not a SeiNode (a SeiNetwork group plan) has no gate of its // own, so it passes through unguarded. func deserializeMarkReady(id string, params json.RawMessage, cfg ExecutionConfig) (TaskExecution, error) { + return deserializeGuardedMarkReady(id, params, cfg, StartBlocked) +} + +func deserializeStartSeidOnce(id string, params json.RawMessage, cfg ExecutionConfig) (TaskExecution, error) { + return deserializeGuardedMarkReady(id, params, cfg, startOnceBlocked) +} + +// startOnceBlocked is the start-once step's guard: a pending reset blocks it, +// and so does a request that is no longer AfterExit. A plan already running +// when the operator changed the hold back to Immediate must not start seid. +func startOnceBlocked(node *seiv1alpha1.SeiNode) string { + if reason := resetBlocks(node); reason != "" { + return reason + } + if hold := node.Spec.HoldRequested(); hold != seiv1alpha1.MaintenanceHoldAfterExit { + return fmt.Sprintf("start once needs maintenance hold AfterExit, requested now: %q", hold) + } + return "" +} + +func deserializeGuardedMarkReady( + id string, params json.RawMessage, cfg ExecutionConfig, blocked func(*seiv1alpha1.SeiNode) string, +) (TaskExecution, error) { inner, err := deserializeSidecar[sidecar.MarkReadyTask](id, params, cfg.BuildSidecarClient, true) if err != nil { return nil, err @@ -45,7 +93,7 @@ func deserializeMarkReady(id string, params json.RawMessage, cfg ExecutionConfig if !ok { return inner, nil } - return &startGuardedExecution{TaskExecution: inner, node: node}, nil + return &startGuardedExecution{TaskExecution: inner, node: node, blocked: blocked}, nil } // StartGuardRefusal prefixes the error of a mark-ready the start guard refused. @@ -59,12 +107,13 @@ const StartGuardRefusal = "mark-ready refused by the start guard" // instead would hold the node's only plan slot and block that reset. type startGuardedExecution struct { TaskExecution - node *seiv1alpha1.SeiNode - err error + node *seiv1alpha1.SeiNode + blocked func(*seiv1alpha1.SeiNode) string + err error } func (e *startGuardedExecution) Execute(ctx context.Context) error { - if reason := StartBlocked(e.node); reason != "" { + if reason := e.blocked(e.node); reason != "" { e.err = fmt.Errorf("%s: %s", StartGuardRefusal, reason) return Terminal(e.err) } diff --git a/internal/task/start_guard_test.go b/internal/task/start_guard_test.go index 49512d7b..af73edbc 100644 --- a/internal/task/start_guard_test.go +++ b/internal/task/start_guard_test.go @@ -27,7 +27,7 @@ func (m *countingSidecar) SubmitTask(ctx context.Context, req sidecar.TaskReques } func nodeConfigNode(spec, handled int64) *seiv1alpha1.SeiNode { - return &seiv1alpha1.SeiNode{ + node := &seiv1alpha1.SeiNode{ ObjectMeta: metav1.ObjectMeta{Name: "rpc-0", Namespace: "default"}, Spec: seiv1alpha1.SeiNodeSpec{ NodeConfig: &seiv1alpha1.NodeConfig{ @@ -36,8 +36,9 @@ func nodeConfigNode(spec, handled int64) *seiv1alpha1.SeiNode { }, DataResetGeneration: spec, }, - Status: seiv1alpha1.SeiNodeStatus{DataResetGeneration: handled}, + Status: seiv1alpha1.SeiNodeStatus{DataResetGeneration: handled, Phase: seiv1alpha1.PhaseRunning}, } + return node } // 009 Req 3: the start guard blocks only while a reset is pending. @@ -130,3 +131,76 @@ func TestRecordDataReset(t *testing.T) { }) } } + +// 010 Req 2.4: a hold blocks mark-ready but not the hold's own start-once +// step; a pending reset blocks both. +func TestStartGuard_Hold(t *testing.T) { + cases := []struct { + name string + hold seiv1alpha1.MaintenanceHold + resetPending bool + taskType string + wantSubmitted bool + }{ + {"mark-ready under hold", seiv1alpha1.MaintenanceHoldImmediate, false, sidecar.TaskTypeMarkReady, false}, + {"start-once under hold", seiv1alpha1.MaintenanceHoldAfterExit, false, task.TaskTypeStartSeidOnce, true}, + {"start-once under reset", seiv1alpha1.MaintenanceHoldAfterExit, true, task.TaskTypeStartSeidOnce, false}, + {"mark-ready released", "", false, sidecar.TaskTypeMarkReady, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + node := nodeConfigNode(0, 0) + if tc.resetPending { + node.Spec.DataResetGeneration = 1 + } + if tc.hold != "" { + node.Spec.Maintenance = &seiv1alpha1.MaintenanceSpec{Hold: tc.hold} + } + mock := &countingSidecar{} + cfg := task.ExecutionConfig{ + BuildSidecarClient: func() (task.SidecarClient, error) { return mock, nil }, + Resource: node, + } + exec, err := task.Deserialize(tc.taskType, task.DeterministicTaskID("p", tc.taskType, 0), nil, cfg) + g.Expect(err).NotTo(HaveOccurred()) + err = exec.Execute(context.Background()) + if tc.wantSubmitted { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(mock.submits).To(Equal(1)) + return + } + var terminal *task.TerminalError + g.Expect(errors.As(err, &terminal)).To(BeTrue()) + g.Expect(mock.submits).To(Equal(0)) + }) + } +} + +// 010 Req 2.7: record-maintenance-hold writes the hold in effect, including +// the empty value a release records. +func TestRecordMaintenanceHold(t *testing.T) { + for _, hold := range []seiv1alpha1.MaintenanceHold{seiv1alpha1.MaintenanceHoldImmediate, ""} { + g := NewWithT(t) + node := nodeConfigNode(0, 0) + node.Status.MaintenanceHold = seiv1alpha1.MaintenanceHoldAfterExit + params := []byte(`{"hold":"` + string(hold) + `"}`) + exec, err := task.Deserialize(task.TaskTypeRecordMaintenanceHold, "id", params, task.ExecutionConfig{Resource: node}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(exec.Execute(context.Background())).To(Succeed()) + g.Expect(node.Status.MaintenanceHold).To(Equal(hold)) + } +} + +// Review finding: a hold set while an init plan runs must not fail the node. +// The hold half of the guard acts only on a Running node. +func TestStartGuard_HoldInertBeforeRunning(t *testing.T) { + g := NewWithT(t) + node := nodeConfigNode(0, 0) + node.Spec.Maintenance = &seiv1alpha1.MaintenanceSpec{Hold: seiv1alpha1.MaintenanceHoldImmediate} + node.Status.Phase = seiv1alpha1.PhaseInitializing + g.Expect(task.StartBlocked(node)).To(BeEmpty()) + + node.Status.Phase = seiv1alpha1.PhaseRunning + g.Expect(task.StartBlocked(node)).To(ContainSubstring("maintenance hold")) +} diff --git a/internal/task/task.go b/internal/task/task.go index 55273e6a..cf407568 100644 --- a/internal/task/task.go +++ b/internal/task/task.go @@ -211,6 +211,7 @@ var registry = map[string]taskDeserializer{ sidecar.TaskTypeMarkNotReady: sidecarTask[sidecar.MarkNotReadyTask](false), sidecar.TaskTypeStopSeid: sidecarTask[sidecar.StopSeidTask](false), sidecar.TaskTypeResetData: sidecarTask[sidecar.ResetDataTask](false), + sidecar.TaskTypeAwaitSeidStart: sidecarTask[sidecar.AwaitSeidStartTask](false), sidecar.TaskTypeResetDataKeepSignState: sidecarTask[sidecar.ResetDataKeepSignStateTask](false), sidecar.TaskTypeGenerateIdentity: sidecarTask[sidecar.GenerateIdentityTask](false), sidecar.TaskTypeGenerateGentx: sidecarTask[sidecar.GenerateGentxTask](false), @@ -235,6 +236,8 @@ var registry = map[string]taskDeserializer{ TaskTypeReplacePod: deserializeReplacePod, TaskTypeObserveImage: deserializeObserveImage, TaskTypeRecordDataReset: deserializeRecordDataReset, + TaskTypeRecordMaintenanceHold: deserializeRecordMaintenanceHold, + TaskTypeStartSeidOnce: deserializeStartSeidOnce, // start-guarded by resets and a non-AfterExit request: start_guard.go TaskTypeUpdateNodeImage: deserializeUpdateNodeImage, TaskTypeValidateSigningKey: deserializeValidateSigningKey, TaskTypeValidateNodeKey: deserializeValidateNodeKey, diff --git a/manifests/sei.io_seinodes.yaml b/manifests/sei.io_seinodes.yaml index f9d1b90e..538f1628 100644 --- a/manifests/sei.io_seinodes.yaml +++ b/manifests/sei.io_seinodes.yaml @@ -591,6 +591,24 @@ spec: maxLength: 512 minLength: 1 type: string + maintenance: + description: |- + Maintenance holds seid at the sidecar start gate with the pod alive and + the data volume mounted, for work through kubectl exec. Requires + spec.nodeConfig. + properties: + hold: + description: |- + Hold keeps seid from starting while it is set. Immediate stops seid now; + AfterExit lets it run until it exits. Remove it to release seid. While + held, the controller still applies the StatefulSet, so a template change + rolls the pod and the new pod stays parked. A hold set on a new node parks + it before seid first runs. + enum: + - Immediate + - AfterExit + type: string + type: object nodeConfig: description: |- NodeConfig supplies this node's seid config files from existing @@ -1496,6 +1514,9 @@ spec: and keep the counter' rule: '!has(oldSelf.dataResetGeneration) || (has(self.dataResetGeneration) && self.dataResetGeneration >= oldSelf.dataResetGeneration)' + - message: 'spec.maintenance.hold needs spec.nodeConfig: only a node that + reads its config from ConfigMaps runs the maintenance hold' + rule: '!has(self.maintenance) || !has(self.maintenance.hold) || has(self.nodeConfig)' status: description: SeiNodeStatus defines the observed state of a SeiNode. properties: @@ -1677,6 +1698,17 @@ spec: leave the listener closed. type: string type: object + maintenanceHold: + description: |- + MaintenanceHold is the hold in effect: Immediate means seid is parked by + the hold, AfterExit means the start gate is closed and seid may still run, + empty means no hold acts on the node. The controller writes it when a hold, + release, or held reset plan completes, and compares it with + spec.maintenance.hold to decide the next plan. + enum: + - Immediate + - AfterExit + type: string phase: description: Phase is the high-level lifecycle state. enum: diff --git a/sidecar/engine/types.go b/sidecar/engine/types.go index f1e64913..01d4f476 100644 --- a/sidecar/engine/types.go +++ b/sidecar/engine/types.go @@ -40,6 +40,7 @@ const ( TaskGovUpdateInstantiateConfig = wire.TaskGovUpdateInstantiateConfig TaskMarkNotReady = wire.TaskMarkNotReady TaskStopSeid = wire.TaskStopSeid + TaskAwaitSeidStart = wire.TaskAwaitSeidStart TaskResetDataKeepSignState = wire.TaskResetDataKeepSignState TaskResetData = wire.TaskResetData TaskEVMDigest = wire.TaskEVMDigest diff --git a/sidecar/serve.go b/sidecar/serve.go index 81a0e9cb..73e4f69b 100644 --- a/sidecar/serve.go +++ b/sidecar/serve.go @@ -125,6 +125,7 @@ var serveCmd = cli.Command{ engine.TaskMarkNotReady: tasks.NewMarkNotReadier(store).Handler(), engine.TaskRestartSeid: tasks.NewRestartSeider().Handler(), engine.TaskStopSeid: tasks.NewStopSeider().Handler(), + engine.TaskAwaitSeidStart: tasks.NewSeidStartAwaiter().Handler(), engine.TaskResetData: tasks.NewResetDataer(homeDir).Handler(), engine.TaskResetDataKeepSignState: tasks.NewResetDataer(homeDir).Handler(), engine.TaskConfigureGenesis: tasks.NewGenesisFetcher(homeDir, chainID, genesisBucket, genesisRegion, nil).Handler(), diff --git a/sidecar/tasks/await_seid_start.go b/sidecar/tasks/await_seid_start.go new file mode 100644 index 00000000..51716bdb --- /dev/null +++ b/sidecar/tasks/await_seid_start.go @@ -0,0 +1,70 @@ +package tasks + +import ( + "context" + "fmt" + "time" + + "github.com/sei-protocol/seilog" + + "github.com/sei-protocol/sei-k8s-controller/sidecar/engine" +) + +var awaitSeidStartLog = seilog.NewLogger("seictl", "task", "await-seid-start") + +// awaitSeidStartTimeout bounds the wait. The start gate's wait loop sees +// mark-ready within about 5s, so a seid that has not started after this long +// never will on this pod: the pod rolled, or the gate closed again. The task +// fails, the plan fails, and the planner builds the start-once plan again +// against the current pod, rather than holding the node's only plan slot. +const awaitSeidStartTimeout = 2 * time.Minute + +// awaitSeidStartPollInterval is how often the task looks for `seid start`. +// The start gate's wait loop polls healthz every 5s, so seid starts within +// about 5s of mark-ready; a short poll here closes the gate again soon after. +const awaitSeidStartPollInterval = 250 * time.Millisecond + +// SeidStartAwaiter completes once a `seid start` process runs in the pod. The +// maintenance hold's start-once step runs mark-ready, this task, then +// mark-not-ready: seid starts once, and the gate is closed again before seid +// has loaded its state, so seid parks at its next exit. It only reads /proc. +type SeidStartAwaiter struct { + find func() bool + pollInterval time.Duration + timeout time.Duration +} + +// NewSeidStartAwaiter builds a SeidStartAwaiter over the pod's /proc. +func NewSeidStartAwaiter() *SeidStartAwaiter { + return &SeidStartAwaiter{ + find: func() bool { + _, err := seidStartFinder{}.FindPID(restartSeidProcess) + return err == nil + }, + pollInterval: awaitSeidStartPollInterval, + timeout: awaitSeidStartTimeout, + } +} + +// Handler returns an engine.TaskHandler for the await-seid-start task type. +// Params are empty. It returns when seid runs, and fails after the timeout or +// when the context ends. +func (a *SeidStartAwaiter) Handler() engine.TaskHandler { + return engine.TypedHandler(func(ctx context.Context, _ struct{}) error { + ctx, cancel := context.WithTimeout(ctx, a.timeout) + defer cancel() + ticker := time.NewTicker(a.pollInterval) + defer ticker.Stop() + for { + if a.find() { + awaitSeidStartLog.Info("seid start is running") + return nil + } + select { + case <-ctx.Done(): + return fmt.Errorf("await-seid-start: seid start not running after %s: %w", a.timeout, ctx.Err()) + case <-ticker.C: + } + } + }) +} diff --git a/sidecar/tasks/await_seid_start_test.go b/sidecar/tasks/await_seid_start_test.go new file mode 100644 index 00000000..61cb26c4 --- /dev/null +++ b/sidecar/tasks/await_seid_start_test.go @@ -0,0 +1,41 @@ +package tasks + +import ( + "context" + "testing" + "time" +) + +// 010 Req 2.3: the start-once step waits until seid runs. +func TestAwaitSeidStart_CompletesWhenSeidRuns(t *testing.T) { + calls := 0 + a := &SeidStartAwaiter{ + find: func() bool { calls++; return calls >= 3 }, + pollInterval: time.Millisecond, + timeout: time.Minute, + } + if _, err := a.Handler()(context.Background(), nil); err != nil { + t.Fatalf("await-seid-start: %v", err) + } + if calls != 3 { + t.Errorf("find called %d times, want 3", calls) + } +} + +func TestAwaitSeidStart_StopsOnContext(t *testing.T) { + a := &SeidStartAwaiter{find: func() bool { return false }, pollInterval: time.Millisecond, timeout: time.Minute} + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) + defer cancel() + if _, err := a.Handler()(ctx, nil); err == nil { + t.Fatal("expected the context error while seid never starts") + } +} + +// Review finding: an unbounded wait held the plan slot forever when the pod +// rolled before seid started. The wait now fails after its timeout. +func TestAwaitSeidStart_FailsAfterTimeout(t *testing.T) { + a := &SeidStartAwaiter{find: func() bool { return false }, pollInterval: time.Millisecond, timeout: 10 * time.Millisecond} + if _, err := a.Handler()(context.Background(), nil); err == nil { + t.Fatal("expected a timeout failure while seid never starts") + } +} diff --git a/sidecarapi/client/tasks.go b/sidecarapi/client/tasks.go index a97dcf5e..9520fa14 100644 --- a/sidecarapi/client/tasks.go +++ b/sidecarapi/client/tasks.go @@ -66,6 +66,7 @@ const ( TaskTypeStopSeid = string(wire.TaskStopSeid) TaskTypeResetData = string(wire.TaskResetData) + TaskTypeAwaitSeidStart = string(wire.TaskAwaitSeidStart) TaskTypeResetDataKeepSignState = string(wire.TaskResetDataKeepSignState) TaskTypeEVMDigest = string(wire.TaskEVMDigest) TaskTypeUnjail = string(wire.TaskUnjail) @@ -337,6 +338,17 @@ func (t StopSeidTask) ToTaskRequest() TaskRequest { return upCheckTaskRequest(t.TaskType(), t.UpCheck) } +// AwaitSeidStartTask completes once a `seid start` process runs in the pod. It +// reads /proc and changes nothing. Consumers poll it to a terminal state. +type AwaitSeidStartTask struct{} + +func (t AwaitSeidStartTask) TaskType() string { return TaskTypeAwaitSeidStart } +func (t AwaitSeidStartTask) Validate() error { return nil } + +func (t AwaitSeidStartTask) ToTaskRequest() TaskRequest { + return TaskRequest{Type: t.TaskType()} +} + // ResetDataKeepSignStateTask is ResetDataTask under the type name that only a // sidecar which keeps the sign state accepts. Use it wherever a validator's // data may be reset: an older sidecar fails the submission instead of running diff --git a/sidecarapi/wire/wire.go b/sidecarapi/wire/wire.go index e9780e57..8b3b5f25 100644 --- a/sidecarapi/wire/wire.go +++ b/sidecarapi/wire/wire.go @@ -44,6 +44,11 @@ const ( TaskStopSeid TaskType = "stop-seid" TaskResetData TaskType = "reset-data" + // TaskAwaitSeidStart completes once a `seid start` process runs in the + // pod. The maintenance hold's start-once step uses it to close the start + // gate again right after seid starts. Read-only. + TaskAwaitSeidStart TaskType = "await-seid-start" + // TaskResetDataKeepSignState is reset-data under a name that promises the // sign state survives. The declarative reset (spec 009) uses it so that a // sidecar built before that guarantee rejects the type as unknown, and the