Skip to content

Commit da0ae1b

Browse files
bdchathamclaude
andauthored
refactor: remove MarkNodesReady from deployment — SeiNode owns sidecar lifecycle (#95)
The SeiNodeDeployment controller no longer reaches into node sidecars to submit mark-ready tasks. The SeiNode controller's NodeUpdate plan handles the full update lifecycle (apply-statefulset, observe-image, mark-ready). The deployment controller observes completion via AwaitSpecUpdate, which polls status.currentImage — a signal that is only set after the SeiNode's NodeUpdate plan fully completes, including sidecar re-initialization. Removed: - MarkNodesReady from in-place deployment plan (UpdateNodeSpecs → AwaitSpecUpdate) - MarkNodesReady task implementation, params type, constant, registry entry - MarkNodesReady tests Updated: - AwaitSpecUpdate comment documenting the boundary contract - Deployment plan test (2 tasks, not 3) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ccc7684 commit da0ae1b

6 files changed

Lines changed: 6 additions & 138 deletions

File tree

internal/planner/deployment.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,6 @@ func (p *inPlaceDeploymentPlanner) BuildPlan(
133133
Namespace: ns,
134134
NodeNames: nodeNames,
135135
}},
136-
{task.TaskTypeMarkNodesReady, &task.MarkNodesReadyParams{
137-
Namespace: ns,
138-
NodeNames: nodeNames,
139-
}},
140136
}
141137

142138
tasks := make([]seiv1alpha1.PlannedTask, len(prog))

internal/planner/deployment_test.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,10 @@ func TestInPlacePlan_ThreeTasks(t *testing.T) {
3030
g.Expect(err).NotTo(HaveOccurred())
3131

3232
g.Expect(plan.Phase).To(Equal(seiv1alpha1.TaskPlanActive))
33-
g.Expect(plan.Tasks).To(HaveLen(3))
33+
g.Expect(plan.Tasks).To(HaveLen(2))
3434

3535
g.Expect(plan.Tasks[0].Type).To(Equal(task.TaskTypeUpdateNodeSpecs))
3636
g.Expect(plan.Tasks[1].Type).To(Equal(task.TaskTypeAwaitSpecUpdate))
37-
g.Expect(plan.Tasks[2].Type).To(Equal(task.TaskTypeMarkNodesReady))
3837

3938
for i, pt := range plan.Tasks {
4039
g.Expect(pt.Status).To(Equal(seiv1alpha1.TaskPending), "task[%d] should be Pending", i)
@@ -52,9 +51,4 @@ func TestInPlacePlan_ThreeTasks(t *testing.T) {
5251
g.Expect(json.Unmarshal(plan.Tasks[1].Params.Raw, &awaitParams)).To(Succeed())
5352
g.Expect(awaitParams.Namespace).To(Equal("pacific-1"))
5453
g.Expect(awaitParams.NodeNames).To(Equal([]string{"wave-group-0", "wave-group-1", "wave-group-2"}))
55-
56-
var markParams task.MarkNodesReadyParams
57-
g.Expect(json.Unmarshal(plan.Tasks[2].Params.Raw, &markParams)).To(Succeed())
58-
g.Expect(markParams.Namespace).To(Equal("pacific-1"))
59-
g.Expect(markParams.NodeNames).To(Equal([]string{"wave-group-0", "wave-group-1", "wave-group-2"}))
6054
}

internal/task/deployment.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ const (
77
TaskTypeCreateEntrantNodes = "create-entrant-nodes"
88
TaskTypeUpdateNodeSpecs = "update-node-specs"
99
TaskTypeAwaitSpecUpdate = "await-spec-update"
10-
TaskTypeMarkNodesReady = "mark-nodes-ready"
1110
TaskTypeSubmitHaltSignal = "submit-halt-signal"
1211
TaskTypeAwaitNodesAtHeight = "await-nodes-at-height"
1312
TaskTypeAwaitNodesCaughtUp = "await-nodes-caught-up"
@@ -74,13 +73,6 @@ type AwaitSpecUpdateParams struct {
7473
NodeNames []string `json:"nodeNames"`
7574
}
7675

77-
// MarkNodesReadyParams holds parameters for submitting mark-ready to
78-
// each node's sidecar after an InPlace rollout completes.
79-
type MarkNodesReadyParams struct {
80-
Namespace string `json:"namespace"`
81-
NodeNames []string `json:"nodeNames"`
82-
}
83-
8476
// TeardownNodesParams holds parameters for deleting incumbent SeiNode resources.
8577
type TeardownNodesParams struct {
8678
Namespace string `json:"namespace"`

internal/task/deployment_update.go

Lines changed: 5 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"encoding/json"
66
"fmt"
77

8-
sidecar "github.com/sei-protocol/seictl/sidecar/client"
98
"k8s.io/apimachinery/pkg/types"
109
"sigs.k8s.io/controller-runtime/pkg/log"
1110

@@ -70,7 +69,11 @@ func (e *updateNodeSpecsExecution) Status(_ context.Context) ExecutionStatus {
7069
return e.status
7170
}
7271

73-
// --- AwaitSpecUpdate: waits for StatefulSet rollout to complete ---
72+
// --- AwaitSpecUpdate: waits for each node's image update to complete ---
73+
// This task polls status.currentImage on each SeiNode. The SeiNode
74+
// controller's NodeUpdate plan handles the full rollout lifecycle
75+
// (apply-statefulset, observe-image, mark-ready) and stamps currentImage
76+
// only after the rollout is complete and the sidecar is re-initialized.
7477

7578
type awaitSpecUpdateExecution struct {
7679
taskBase
@@ -114,72 +117,3 @@ func (e *awaitSpecUpdateExecution) Status(ctx context.Context) ExecutionStatus {
114117
e.complete()
115118
return ExecutionComplete
116119
}
117-
118-
// --- MarkNodesReady: submits mark-ready to each node's sidecar ---
119-
120-
type markNodesReadyExecution struct {
121-
taskBase
122-
params MarkNodesReadyParams
123-
cfg ExecutionConfig
124-
marked map[string]bool
125-
}
126-
127-
func deserializeMarkNodesReady(id string, params json.RawMessage, cfg ExecutionConfig) (TaskExecution, error) {
128-
var p MarkNodesReadyParams
129-
if len(params) > 0 {
130-
if err := json.Unmarshal(params, &p); err != nil {
131-
return nil, fmt.Errorf("deserializing mark-nodes-ready params: %w", err)
132-
}
133-
}
134-
return &markNodesReadyExecution{
135-
taskBase: taskBase{id: id, status: ExecutionRunning},
136-
params: p,
137-
cfg: cfg,
138-
marked: make(map[string]bool, len(p.NodeNames)),
139-
}, nil
140-
}
141-
142-
func (e *markNodesReadyExecution) Execute(_ context.Context) error { return nil }
143-
144-
func (e *markNodesReadyExecution) Status(ctx context.Context) ExecutionStatus {
145-
if s, done := e.isTerminal(); done {
146-
return s
147-
}
148-
logger := log.FromContext(ctx)
149-
150-
allReady := true
151-
for _, name := range e.params.NodeNames {
152-
if e.marked[name] {
153-
continue
154-
}
155-
node := &seiv1alpha1.SeiNode{}
156-
if err := e.cfg.KubeClient.Get(ctx, types.NamespacedName{Name: name, Namespace: e.params.Namespace}, node); err != nil {
157-
allReady = false
158-
continue
159-
}
160-
sc, err := sidecarClientForNode(node)
161-
if err != nil {
162-
allReady = false
163-
continue
164-
}
165-
resp, err := sc.Status(ctx)
166-
if err != nil {
167-
allReady = false
168-
continue
169-
}
170-
if resp.Status == sidecar.Ready {
171-
e.marked[name] = true
172-
continue
173-
}
174-
if _, err := sc.SubmitTask(ctx, sidecar.TaskRequest{Type: sidecar.TaskTypeMarkReady}); err != nil {
175-
logger.V(1).Info("mark-ready submission failed", "node", name, "error", err)
176-
}
177-
allReady = false
178-
}
179-
180-
if allReady {
181-
e.complete()
182-
return ExecutionComplete
183-
}
184-
return ExecutionRunning
185-
}

internal/task/deployment_update_test.go

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -237,50 +237,3 @@ func TestAwaitSpecUpdate_RunningWhenNodeNotFound(t *testing.T) {
237237
t.Fatalf("expected Running for missing node, got %s", exec.Status(context.Background()))
238238
}
239239
}
240-
241-
// --- MarkNodesReady ---
242-
243-
func TestMarkNodesReady_Deserializes(t *testing.T) {
244-
group := testDeploymentGroup()
245-
cfg := testDeploymentCfg(t, group)
246-
247-
params := MarkNodesReadyParams{
248-
Namespace: "sei",
249-
NodeNames: []string{"wave-0", "wave-1"},
250-
}
251-
raw, _ := json.Marshal(params)
252-
exec, err := deserializeMarkNodesReady("id-3", raw, cfg)
253-
if err != nil {
254-
t.Fatalf("deserialize: %v", err)
255-
}
256-
257-
mnr, ok := exec.(*markNodesReadyExecution)
258-
if !ok {
259-
t.Fatal("expected *markNodesReadyExecution")
260-
}
261-
if len(mnr.params.NodeNames) != 2 {
262-
t.Errorf("nodeNames len = %d, want 2", len(mnr.params.NodeNames))
263-
}
264-
if mnr.params.Namespace != "sei" {
265-
t.Errorf("namespace = %q, want %q", mnr.params.Namespace, "sei")
266-
}
267-
}
268-
269-
func TestMarkNodesReady_StartsRunning(t *testing.T) {
270-
group := testDeploymentGroup()
271-
cfg := testDeploymentCfg(t, group)
272-
273-
params := MarkNodesReadyParams{
274-
Namespace: "sei",
275-
NodeNames: []string{"wave-0"},
276-
}
277-
raw, _ := json.Marshal(params)
278-
exec, err := deserializeMarkNodesReady("id-3", raw, cfg)
279-
if err != nil {
280-
t.Fatalf("deserialize: %v", err)
281-
}
282-
283-
if exec.Status(context.Background()) != ExecutionRunning {
284-
t.Fatalf("expected initial status Running, got %s", exec.Status(context.Background()))
285-
}
286-
}

internal/task/task.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,6 @@ var registry = map[string]taskDeserializer{
213213
// Controller-side deployment tasks
214214
TaskTypeUpdateNodeSpecs: deserializeUpdateNodeSpecs,
215215
TaskTypeAwaitSpecUpdate: deserializeAwaitSpecUpdate,
216-
TaskTypeMarkNodesReady: deserializeMarkNodesReady,
217216
TaskTypeCreateEntrantNodes: deserializeCreateEntrantNodes,
218217
TaskTypeSubmitHaltSignal: deserializeSubmitHaltSignal,
219218
TaskTypeAwaitNodesAtHeight: deserializeAwaitNodesAtHeight,

0 commit comments

Comments
 (0)