From 07e241f3a7b74917179b1c51996bdd17312f2393 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 17 Sep 2026 09:03:29 +0200 Subject: [PATCH 1/4] feat(api): spec.nodeConfig serves config.toml and app.toml verbatim from ConfigMaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A SeiNode may name ConfigMaps supplying its config.toml and app.toml. Both mount read-only over the seid config directory, and what the operator wrote is what seid reads. The controller adds nothing on top. A node with the field takes a plan progression carrying no config task at all. That is what keeps the mount attached: a rename onto a mounted path from another mount namespace succeeds and detaches the mount, after which seid reads the writer's file with nothing reporting the swap. The kernel's EBUSY guard is is_local_mountpoint(), scoped to the calling namespace, and vfs_rename() calls detach_mounts(). Verified on Linux 6.12.68. staticConfigPlanner wraps the mode planner rather than replacing it, so each mode keeps its own Validate and its own init plan. It owns the Running arm, which the mode planners route through an assembler that force-inserts config-apply whenever the configValues baseline is unobserved. MountsNodeConfig is the predicate everywhere: the union of spec.nodeConfig and status.currentNodeConfig, because a node mid-revert still has the mount while its spec no longer names a ConfigMap. config-validate is stripped too. It reports on a file the operator owns, through sei-config's legacy reader, which falls back to mode "full" when app.toml carries no [sei] mode — so on a validator it passes a config seid refuses. replace-pod parses both ConfigMaps before it deletes a pod, and that is the check that matters. Rejected beside the field, each because its only route to seid was a config task: configValues, overrides, peers, externalAddress. peers is the sharpest — status.resolvedPeers would keep updating and keep looking correct while config.toml stayed as written. Refused by staticConfigPlanner.Validate, each because it learns config at run time that no ConfigMap can hold: a bootstrap Job (its pod holds the same PVC and rewrites config.toml there), a state-sync snapshot source, a genesis ceremony, and consensus engine Autobahn. Co-Authored-By: Claude Opus 5 (1M context) --- api/v1alpha1/seinode_types.go | 79 ++++ api/v1alpha1/zz_generated.deepcopy.go | 42 +++ config/crd/sei.io_seinodes.yaml | 134 +++++++ .../envtest/nodeconfig_validation_test.go | 144 ++++++++ internal/controller/node/workflow.go | 11 + internal/noderesource/node_config_test.go | 138 +++++++ internal/noderesource/noderesource.go | 76 +++- internal/planner/doc.go | 32 +- internal/planner/planner.go | 69 +++- internal/planner/static_config.go | 217 +++++++++++ internal/planner/static_config_test.go | 341 ++++++++++++++++++ internal/planner/workflow.go | 6 + internal/task/bootstrap_resources_test.go | 28 ++ internal/task/observe_image.go | 7 +- internal/task/replace_pod.go | 66 ++++ internal/task/replace_pod_test.go | 62 ++++ manifests/sei.io_seinodes.yaml | 134 +++++++ 17 files changed, 1574 insertions(+), 12 deletions(-) create mode 100644 internal/controller/node/envtest/nodeconfig_validation_test.go create mode 100644 internal/noderesource/node_config_test.go create mode 100644 internal/planner/static_config.go create mode 100644 internal/planner/static_config_test.go diff --git a/api/v1alpha1/seinode_types.go b/api/v1alpha1/seinode_types.go index a1f019e7..41d8e384 100644 --- a/api/v1alpha1/seinode_types.go +++ b/api/v1alpha1/seinode_types.go @@ -58,6 +58,15 @@ import ( // requests.memory, and the controller derives the limit from the request, so the // footprint is frozen by freezing requests. // +kubebuilder:validation:XValidation:rule="(!has(self.resources) && !has(oldSelf.resources)) || (has(self.resources) && has(oldSelf.resources) && (has(self.resources.requests) == has(oldSelf.resources.requests)) && (!has(self.resources.requests) || ((('cpu' in self.resources.requests) == ('cpu' in oldSelf.resources.requests)) && (('memory' in self.resources.requests) == ('memory' in oldSelf.resources.requests)) && (!('cpu' in self.resources.requests) || !('cpu' in oldSelf.resources.requests) || quantity(string(self.resources.requests['cpu'])).compareTo(quantity(string(oldSelf.resources.requests['cpu']))) == 0) && (!('memory' in self.resources.requests) || !('memory' in oldSelf.resources.requests) || quantity(string(self.resources.requests['memory'])).compareTo(quantity(string(oldSelf.resources.requests['memory']))) == 0))))",message="spec.resources is create-only: the footprint is fixed at creation (a change is not rolled onto a running pod — the StatefulSet is OnDelete and drift detection is image-only), so replace the node to resize" +// A node with spec.nodeConfig runs no task that writes config.toml or +// app.toml, so every field whose only route to seid was one of those tasks is +// rejected beside it. Accepting one would report success on an edit that never +// reached the node — and for peers, status.resolvedPeers would keep updating +// and keep looking correct while config.toml stayed as the operator wrote it. +// +kubebuilder:validation:XValidation:rule="!has(self.nodeConfig) || !has(self.configValues)",message="spec.nodeConfig and spec.configValues cannot both be set: a node reading its config from ConfigMaps runs no config-patch task, so the values would never reach seid; put them in the ConfigMap" +// +kubebuilder:validation:XValidation:rule="!has(self.nodeConfig) || !has(self.overrides)",message="spec.nodeConfig and spec.overrides cannot both be set: a node reading its config from ConfigMaps runs no config-apply task, so the overrides would never reach seid; put them in the ConfigMap" +// +kubebuilder:validation:XValidation:rule="!has(self.nodeConfig) || !has(self.peers)",message="spec.nodeConfig and spec.peers cannot both be set: nothing carries a resolved peer set into config.toml on a node reading its config from ConfigMaps; write p2p.persistent-peers in the ConfigMap" +// +kubebuilder:validation:XValidation:rule="!has(self.nodeConfig) || !has(self.externalAddress)",message="spec.nodeConfig and spec.externalAddress cannot both be set: nothing carries it into config.toml on a node reading its config from ConfigMaps; write p2p.external-address in the ConfigMap" type SeiNodeSpec struct { // ChainID of the chain this node belongs to. // Constrained to DNS-1123 label characters because the controller composes @@ -122,6 +131,41 @@ type SeiNodeSpec struct { // +listMapKey=key ConfigValues []ConfigValue `json:"configValues,omitempty"` + // NodeConfig supplies this node's seid config files from existing + // ConfigMaps. The files mount read-only over the seid config directory, so + // they replace whatever the data volume already holds. + // + // A node with this field set takes the static-config plan: no task in its + // plan writes either file on the production pod. That is what keeps the + // mount attached. A rename onto a mounted path from another container + // detaches the mount, and seid then reads the writer's file. + // + // The operator owns both files verbatim. The controller supplies nothing: + // not the mode's base configuration, not persistent-peers, not + // external-address, not the freeze height, not the snapshot-generation + // keys. It does not validate them either — replace-pod parses both files + // before it deletes a pod, and that is the only check. + // + // The fields whose only route to seid was a config task are rejected + // beside this one: configValues, overrides, peers, externalAddress. + // + // The references are the unit of change. Kubelet pins a subPath mount at + // pod start, so editing a ConfigMap in place does not reach a running pod. + // Publish under a new name and the node rolls. + // + // Not supported with a bootstrap Job, a state-sync snapshot source, a + // genesis ceremony, or consensus engine Autobahn. Each of those writes + // config.toml at run time, and the plan is refused. + // + // The StatefulSet carries the references as soon as they are set, before + // any plan runs. A reference that does not resolve leaves the template + // unmountable: the controller will not replace the pod itself, but a + // drain, an eviction, or a manual delete recreates it into + // ContainerCreating, and StatefulSets are OnDelete so nothing rolls it + // back. Create the ConfigMaps first. + // +optional + NodeConfig *NodeConfig `json:"nodeConfig,omitempty"` + // Scheduling configures worker-node isolation. // +optional Scheduling *SchedulingConfig `json:"scheduling,omitempty"` @@ -347,6 +391,31 @@ func (s *SeiNodeSpec) SnapshotSource() *SnapshotSource { } } +// NodeConfig supplies a node's seid config files from existing ConfigMaps. +// Both references are required: a node that takes its config.toml from a +// ConfigMap and its app.toml from the controller would have two owners of one +// directory, and the controller's writer would detach the mount delivering the +// other file. +type NodeConfig struct { + // ConfigRef holds config.toml. + ConfigRef ConfigFileRef `json:"configRef"` + + // AppRef holds app.toml. + AppRef ConfigFileRef `json:"appRef"` +} + +// ConfigFileRef names the ConfigMap holding one seid config file. Both +// references may name the same ConfigMap. +type ConfigFileRef struct { + // Name of an existing ConfigMap in the SeiNode's namespace. The file is + // read from the key matching its own name, config.toml or app.toml. + // Kubelet refuses the mount when that key is absent, so the pod stays in + // ContainerCreating and `kubectl describe pod` names the missing key. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + Name string `json:"name"` +} + // NodeKeySecret returns the Secret supplying this node's P2P identity // (node_key.json), or nil when the node has none and `seid init` generates one // onto the data volume. @@ -781,6 +850,16 @@ type SeiNodeStatus struct { // +optional CurrentNodeIsolation NodeIsolation `json:"currentNodeIsolation,omitempty"` + // CurrentNodeConfig is the spec.nodeConfig the owned StatefulSet's pod was + // last rolled with, stamped jointly with CurrentImage on rollout + // completion. Unset means the pod mounts no operator-supplied config. + // + // Unset means the pod mounts no operator-supplied config. Unlike the fields + // above, unset is a real observation and not "not yet observed". It records + // the references, never the ConfigMaps' contents. + // +optional + CurrentNodeConfig *NodeConfig `json:"currentNodeConfig,omitempty"` + // +listType=map // +listMapKey=type // +optional diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 0826b606..a9734a2f 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -162,6 +162,21 @@ func (in *AwaitNodesAtHeightPayload) DeepCopy() *AwaitNodesAtHeightPayload { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ConfigFileRef) DeepCopyInto(out *ConfigFileRef) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigFileRef. +func (in *ConfigFileRef) DeepCopy() *ConfigFileRef { + if in == nil { + return nil + } + out := new(ConfigFileRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ConfigMigration) DeepCopyInto(out *ConfigMigration) { *out = *in @@ -741,6 +756,23 @@ func (in *NetworkConsensusSpec) DeepCopy() *NetworkConsensusSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NodeConfig) DeepCopyInto(out *NodeConfig) { + *out = *in + out.ConfigRef = in.ConfigRef + out.AppRef = in.AppRef +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeConfig. +func (in *NodeConfig) DeepCopy() *NodeConfig { + if in == nil { + return nil + } + out := new(NodeConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodeEndpoint) DeepCopyInto(out *NodeEndpoint) { *out = *in @@ -1380,6 +1412,11 @@ func (in *SeiNodeSpec) DeepCopyInto(out *SeiNodeSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.NodeConfig != nil { + in, out := &in.NodeConfig, &out.NodeConfig + *out = new(NodeConfig) + **out = **in + } if in.Scheduling != nil { in, out := &in.Scheduling, &out.Scheduling *out = new(SchedulingConfig) @@ -1447,6 +1484,11 @@ func (in *SeiNodeSpec) DeepCopy() *SeiNodeSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SeiNodeStatus) DeepCopyInto(out *SeiNodeStatus) { *out = *in + if in.CurrentNodeConfig != nil { + in, out := &in.CurrentNodeConfig, &out.CurrentNodeConfig + *out = new(NodeConfig) + **out = **in + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]metav1.Condition, len(*in)) diff --git a/config/crd/sei.io_seinodes.yaml b/config/crd/sei.io_seinodes.yaml index 3c02be84..707d08a2 100644 --- a/config/crd/sei.io_seinodes.yaml +++ b/config/crd/sei.io_seinodes.yaml @@ -86,6 +86,11 @@ spec: limits is not compared here: the equality rule already pins limits.memory to requests.memory, and the controller derives the limit from the request, so the footprint is frozen by freezing requests. + A node with spec.nodeConfig runs no task that writes config.toml or + app.toml, so every field whose only route to seid was one of those tasks is + rejected beside it. Accepting one would report success on an edit that never + reached the node — and for peers, status.resolvedPeers would keep updating + and keep looking correct while config.toml stayed as the operator wrote it. properties: archive: description: Archive configures an archive node with full history @@ -557,6 +562,75 @@ spec: maxLength: 512 minLength: 1 type: string + nodeConfig: + description: |- + NodeConfig supplies this node's seid config files from existing + ConfigMaps. The files mount read-only over the seid config directory, so + they replace whatever the data volume already holds. + + A node with this field set takes the static-config plan: no task in its + plan writes either file on the production pod. That is what keeps the + mount attached. A rename onto a mounted path from another container + detaches the mount, and seid then reads the writer's file. + + The operator owns both files verbatim. The controller supplies nothing: + not the mode's base configuration, not persistent-peers, not + external-address, not the freeze height, not the snapshot-generation + keys. It does not validate them either — replace-pod parses both files + before it deletes a pod, and that is the only check. + + The fields whose only route to seid was a config task are rejected + beside this one: configValues, overrides, peers, externalAddress. + + The references are the unit of change. Kubelet pins a subPath mount at + pod start, so editing a ConfigMap in place does not reach a running pod. + Publish under a new name and the node rolls. + + Not supported with a bootstrap Job, a state-sync snapshot source, a + genesis ceremony, or consensus engine Autobahn. Each of those writes + config.toml at run time, and the plan is refused. + + The StatefulSet carries the references as soon as they are set, before + any plan runs. A reference that does not resolve leaves the template + unmountable: the controller will not replace the pod itself, but a + drain, an eviction, or a manual delete recreates it into + ContainerCreating, and StatefulSets are OnDelete so nothing rolls it + back. Create the ConfigMaps first. + properties: + appRef: + description: AppRef holds app.toml. + properties: + name: + description: |- + Name of an existing ConfigMap in the SeiNode's namespace. The file is + read from the key matching its own name, config.toml or app.toml. + Kubelet refuses the mount when that key is absent, so the pod stays in + ContainerCreating and `kubectl describe pod` names the missing key. + maxLength: 253 + minLength: 1 + type: string + required: + - name + type: object + configRef: + description: ConfigRef holds config.toml. + properties: + name: + description: |- + Name of an existing ConfigMap in the SeiNode's namespace. The file is + read from the key matching its own name, config.toml or app.toml. + Kubelet refuses the mount when that key is absent, so the pod stays in + ContainerCreating and `kubectl describe pod` names the missing key. + maxLength: 253 + minLength: 1 + type: string + required: + - name + type: object + required: + - appRef + - configRef + type: object overrides: additionalProperties: type: string @@ -1377,6 +1451,22 @@ spec: == 0) && (!('memory' in self.resources.requests) || !('memory' in oldSelf.resources.requests) || quantity(string(self.resources.requests['memory'])).compareTo(quantity(string(oldSelf.resources.requests['memory']))) == 0)))) + - message: 'spec.nodeConfig and spec.configValues cannot both be set: + a node reading its config from ConfigMaps runs no config-patch task, + so the values would never reach seid; put them in the ConfigMap' + rule: '!has(self.nodeConfig) || !has(self.configValues)' + - message: 'spec.nodeConfig and spec.overrides cannot both be set: a node + reading its config from ConfigMaps runs no config-apply task, so the + overrides would never reach seid; put them in the ConfigMap' + rule: '!has(self.nodeConfig) || !has(self.overrides)' + - message: 'spec.nodeConfig and spec.peers cannot both be set: nothing + carries a resolved peer set into config.toml on a node reading its + config from ConfigMaps; write p2p.persistent-peers in the ConfigMap' + rule: '!has(self.nodeConfig) || !has(self.peers)' + - message: 'spec.nodeConfig and spec.externalAddress cannot both be set: + nothing carries it into config.toml on a node reading its config from + ConfigMaps; write p2p.external-address in the ConfigMap' + rule: '!has(self.nodeConfig) || !has(self.externalAddress)' status: description: SeiNodeStatus defines the observed state of a SeiNode. properties: @@ -1495,6 +1585,50 @@ spec: Parent controllers compare this against spec.image to determine whether a spec change has been fully actuated. type: string + currentNodeConfig: + description: |- + CurrentNodeConfig is the spec.nodeConfig the owned StatefulSet's pod was + last rolled with, stamped jointly with CurrentImage on rollout + completion. Unset means the pod mounts no operator-supplied config. + + Unset means the pod mounts no operator-supplied config. Unlike the fields + above, unset is a real observation and not "not yet observed". It records + the references, never the ConfigMaps' contents. + properties: + appRef: + description: AppRef holds app.toml. + properties: + name: + description: |- + Name of an existing ConfigMap in the SeiNode's namespace. The file is + read from the key matching its own name, config.toml or app.toml. + Kubelet refuses the mount when that key is absent, so the pod stays in + ContainerCreating and `kubectl describe pod` names the missing key. + maxLength: 253 + minLength: 1 + type: string + required: + - name + type: object + configRef: + description: ConfigRef holds config.toml. + properties: + name: + description: |- + Name of an existing ConfigMap in the SeiNode's namespace. The file is + read from the key matching its own name, config.toml or app.toml. + Kubelet refuses the mount when that key is absent, so the pod stays in + ContainerCreating and `kubectl describe pod` names the missing key. + maxLength: 253 + minLength: 1 + type: string + required: + - name + type: object + required: + - appRef + - configRef + type: object currentNodeIsolation: description: |- CurrentNodeIsolation is the effective node isolation the owned diff --git a/internal/controller/node/envtest/nodeconfig_validation_test.go b/internal/controller/node/envtest/nodeconfig_validation_test.go new file mode 100644 index 00000000..86cc7fef --- /dev/null +++ b/internal/controller/node/envtest/nodeconfig_validation_test.go @@ -0,0 +1,144 @@ +//go:build envtest + +package envtest_test + +import ( + "testing" + + . "github.com/onsi/gomega" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + seiv1alpha1 "github.com/sei-protocol/sei-k8s-controller/api/v1alpha1" +) + +// Admission-level coverage of spec.nodeConfig: which shapes the API server +// accepts, and which it rejects. +// +// These cases need no controller. A failure here is a CRD-contract defect and +// never a reconcile bug. + +func nodeConfigNode(ns, name string) *seiv1alpha1.SeiNode { + return &seiv1alpha1.SeiNode{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}, + Spec: seiv1alpha1.SeiNodeSpec{ + ChainID: "envtest-1", + Image: "sei:latest", + FullNode: &seiv1alpha1.FullNodeSpec{}, + NodeConfig: &seiv1alpha1.NodeConfig{ + ConfigRef: seiv1alpha1.ConfigFileRef{Name: "rpc-config-v1"}, + AppRef: seiv1alpha1.ConfigFileRef{Name: "rpc-app-v1"}, + }, + }, + } +} + +func TestNodeConfig_BothRefs_Accepted(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + + g.Expect(testCli.Create(testCtx, nodeConfigNode(ns, "nc-both"))).To(Succeed()) + + // One ConfigMap carrying both keys is the common case and equally valid. + same := nodeConfigNode(ns, "nc-same") + same.Spec.NodeConfig.AppRef.Name = same.Spec.NodeConfig.ConfigRef.Name + g.Expect(testCli.Create(testCtx, same)).To(Succeed()) +} + +// Both files are required. A node taking config.toml from a ConfigMap and +// app.toml from the controller would have two owners of one directory, and the +// controller's writer would detach the mount delivering the other file. +func TestNodeConfig_OneRefMissing_Rejected(t *testing.T) { + cases := []struct { + name string + clear func(*seiv1alpha1.NodeConfig) + }{ + {"appRef missing", func(c *seiv1alpha1.NodeConfig) { c.AppRef = seiv1alpha1.ConfigFileRef{} }}, + {"configRef missing", func(c *seiv1alpha1.NodeConfig) { c.ConfigRef = seiv1alpha1.ConfigFileRef{} }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + + node := nodeConfigNode(ns, "nc-partial") + tc.clear(node.Spec.NodeConfig) + + g.Expect(testCli.Create(testCtx, node)).To(HaveOccurred(), + "nodeConfig must name both config.toml and app.toml") + }) + } +} + +// Each of these reached config.toml or app.toml through a task a node with +// spec.nodeConfig does not run, so accepting the pair would report success on +// an edit that never reached seid. peers is the sharpest: status.resolvedPeers +// would keep updating and keep looking correct. +func TestNodeConfig_WithControllerManagedConfig_Rejected(t *testing.T) { + cases := []struct { + name string + mutate func(*seiv1alpha1.SeiNode) + wantMsg string + }{ + {"with configValues", func(n *seiv1alpha1.SeiNode) { + n.Spec.ConfigValues = []seiv1alpha1.ConfigValue{{ + FileName: "config.toml", + Key: "p2p.persistent-peers", + Value: apiextensionsv1.JSON{Raw: []byte(`"peer@host:26656"`)}, + }} + }, "configValues"}, + {"with overrides", func(n *seiv1alpha1.SeiNode) { + n.Spec.Overrides = map[string]string{"logging.level": "debug"} + }, "overrides"}, + {"with peers", func(n *seiv1alpha1.SeiNode) { + n.Spec.Peers = []seiv1alpha1.PeerSource{{ + Static: &seiv1alpha1.StaticPeerSource{Addresses: []string{"peer@host:26656"}}, + }} + }, "peers"}, + {"with externalAddress", func(n *seiv1alpha1.SeiNode) { + n.Spec.ExternalAddress = "node.example:26656" + }, "externalAddress"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + + node := nodeConfigNode(ns, "nc-conflict") + tc.mutate(node) + + err := testCli.Create(testCtx, node) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tc.wantMsg)) + }) + } +} + +// The field is deliberately mutable, unlike the create-only pod-template +// fields beside it: adopting a ConfigMap on a running node is the point. +func TestNodeConfig_Mutable(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + + // Created without it, then adopted. + node := nodeConfigNode(ns, "nc-mutable") + node.Spec.NodeConfig = nil + g.Expect(testCli.Create(testCtx, node)).To(Succeed()) + key := client.ObjectKeyFromObject(node) + + g.Expect(updateNodeWithRetry(t, key, func(cur *seiv1alpha1.SeiNode) { + cur.Spec.NodeConfig = &seiv1alpha1.NodeConfig{ + ConfigRef: seiv1alpha1.ConfigFileRef{Name: "rpc-config-v1"}, + AppRef: seiv1alpha1.ConfigFileRef{Name: "rpc-app-v1"}, + } + })).To(Succeed(), "a running node must be able to adopt a ConfigMap") + + g.Expect(updateNodeWithRetry(t, key, func(cur *seiv1alpha1.SeiNode) { + cur.Spec.NodeConfig.ConfigRef.Name = "rpc-config-v2" + })).To(Succeed(), "republishing under a new name must be accepted") + + g.Expect(updateNodeWithRetry(t, key, func(cur *seiv1alpha1.SeiNode) { + cur.Spec.NodeConfig = nil + })).To(Succeed(), "reverting to controller-managed config must be accepted") +} diff --git a/internal/controller/node/workflow.go b/internal/controller/node/workflow.go index 3def4f6a..9930d82f 100644 --- a/internal/controller/node/workflow.go +++ b/internal/controller/node/workflow.go @@ -262,6 +262,17 @@ func (r *SeiNodeReconciler) maybeAdoptWorkflow( return false, ctrl.Result{}, false, nil } + // Every recipe writes config.toml, and the target mounts it read-only from + // the operator's ConfigMap. Kept in lockstep with the planner-side refusal + // in stateSyncWorkflowPlanner.Validate. + if planner.MountsNodeConfig(node) { + for i := range candidates { + r.failWorkflow(ctx, &candidates[i], seiv1alpha1.ReasonWorkflowTargetRejected, + "target sets spec.nodeConfig; workflows rewrite config.toml, which the node mounts read-only") + } + return false, ctrl.Result{}, false, nil + } + winner := &candidates[0] // Seed queued status on the losers regardless of whether the node is idle, // so a workflow created mid-image-roll shows Pending + conditions. diff --git a/internal/noderesource/node_config_test.go b/internal/noderesource/node_config_test.go new file mode 100644 index 00000000..1b3190d1 --- /dev/null +++ b/internal/noderesource/node_config_test.go @@ -0,0 +1,138 @@ +package noderesource + +import ( + "testing" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + + seiv1alpha1 "github.com/sei-protocol/sei-k8s-controller/api/v1alpha1" + "github.com/sei-protocol/sei-k8s-controller/internal/platform/platformtest" +) + +const ( + testConfigMapName = "rpc-config-v1" + testAppConfigMapName = "rpc-app-v1" +) + +func nodeConfigNode() *seiv1alpha1.SeiNode { + node := newSnapshotNode("snap-0", "default") + node.Spec.NodeConfig = &seiv1alpha1.NodeConfig{ + ConfigRef: seiv1alpha1.ConfigFileRef{Name: testConfigMapName}, + AppRef: seiv1alpha1.ConfigFileRef{Name: testAppConfigMapName}, + } + return node +} + +// containerByName searches both container lists: the sidecar and seid-init +// are init containers, and seid is not. +func containerByName(spec corev1.PodSpec, name string) *corev1.Container { + if c := findContainer(spec.Containers, name); c != nil { + return c + } + return findContainer(spec.InitContainers, name) +} + +func mountNames(c *corev1.Container) []string { + names := make([]string, 0, len(c.VolumeMounts)) + for _, m := range c.VolumeMounts { + names = append(names, m.Name) + } + return names +} + +func TestNodeConfigRendersOneVolumePerFile(t *testing.T) { + g := NewWithT(t) + + spec, err := buildNodePodSpec(nodeConfigNode(), platformtest.Config()) + g.Expect(err).NotTo(HaveOccurred()) + + cases := []struct { + volumeName string + configMapName string + dataKey string + }{ + {nodeConfigConfigVolumeName, testConfigMapName, configTomlDataKey}, + {nodeConfigAppVolumeName, testAppConfigMapName, appTomlDataKey}, + } + for _, tc := range cases { + v := findVolume(spec.Volumes, tc.volumeName) + g.Expect(v).NotTo(BeNil(), "volume %s", tc.volumeName) + g.Expect(v.ConfigMap).NotTo(BeNil()) + g.Expect(v.ConfigMap.Name).To(Equal(tc.configMapName)) + g.Expect(*v.ConfigMap.DefaultMode).To(Equal(int32(0o444))) + g.Expect(v.ConfigMap.Items).To(Equal( + []corev1.KeyToPath{{Key: tc.dataKey, Path: tc.dataKey}})) + } + + want := []corev1.VolumeMount{ + { + Name: nodeConfigConfigVolumeName, + MountPath: dataDir + "/config/config.toml", + SubPath: configTomlDataKey, + ReadOnly: true, + }, + { + Name: nodeConfigAppVolumeName, + MountPath: dataDir + "/config/app.toml", + SubPath: appTomlDataKey, + ReadOnly: true, + }, + } + seid := containerByName(spec, containerNameSeid) + g.Expect(seid).NotTo(BeNil()) + g.Expect(seid.VolumeMounts).To(ContainElements(want)) +} + +// TestNodeConfigMountsOnSidecarIsASafetyProperty guards a cleanup that reads +// as harmless. The sidecar does not read config.toml, so a future change could +// drop this mount — and that is exactly what must not happen. A rename onto a +// mounted path from a container WITHOUT the mount succeeds and silently +// detaches it; from a container WITH the mount the same rename returns EBUSY +// and fails the task. This mount is what makes a stray writer loud. +func TestNodeConfigMountsOnSidecarIsASafetyProperty(t *testing.T) { + g := NewWithT(t) + + spec, err := buildNodePodSpec(nodeConfigNode(), platformtest.Config()) + g.Expect(err).NotTo(HaveOccurred()) + + sidecar := containerByName(spec, containerNameSidecar) + g.Expect(sidecar).NotTo(BeNil()) + g.Expect(mountNames(sidecar)).To(ContainElements(nodeConfigConfigVolumeName, nodeConfigAppVolumeName)) +} + +// TestNodeConfigNotMountedOnWritingContainers keeps the mount off every +// container that must write the config directory. seid-init runs +// `seid init --overwrite` on a fresh volume; the other two never touch it. +func TestNodeConfigNotMountedOnWritingContainers(t *testing.T) { + g := NewWithT(t) + + spec, err := buildNodePodSpec(nodeConfigNode(), platformtest.Config()) + g.Expect(err).NotTo(HaveOccurred()) + + for _, name := range []string{"seid-init", containerNameRBACProxy, containerNameCosmosExporter} { + c := containerByName(spec, name) + if c == nil { + continue + } + g.Expect(mountNames(c)).NotTo(ContainElements(nodeConfigConfigVolumeName, nodeConfigAppVolumeName), "container %s", name) + } +} + +func TestNodeConfigUnsetRendersNothing(t *testing.T) { + g := NewWithT(t) + + spec, err := buildNodePodSpec(newSnapshotNode("snap-0", "default"), platformtest.Config()) + g.Expect(err).NotTo(HaveOccurred()) + + rendered := []string{nodeConfigConfigVolumeName, nodeConfigAppVolumeName} + for _, v := range spec.Volumes { + g.Expect(rendered).NotTo(ContainElement(v.Name)) + } + for i := range spec.Containers { + g.Expect(mountNames(&spec.Containers[i])).NotTo(ContainElements(rendered)) + } + for i := range spec.InitContainers { + g.Expect(mountNames(&spec.InitContainers[i])).NotTo(ContainElements(rendered)) + } +} diff --git a/internal/noderesource/noderesource.go b/internal/noderesource/noderesource.go index 6f26f79f..dc3bc563 100644 --- a/internal/noderesource/noderesource.go +++ b/internal/noderesource/noderesource.go @@ -134,6 +134,11 @@ const ( nodeKeyVolumeName = "node-key" nodeKeyDataKey = "node_key.json" + nodeConfigConfigVolumeName = "node-config-config" + nodeConfigAppVolumeName = "node-config-app" + configTomlDataKey = "config.toml" + appTomlDataKey = "app.toml" + operatorKeyringVolumeName = "operator-keyring" // keyring.New(BackendFile, rootDir) opens rootDir/keyring-file/. // Used as the mount path for the projected .secret Secret. @@ -882,11 +887,13 @@ func buildNodePodSpec(node *seiv1alpha1.SeiNode, p PlatformConfig) (corev1.PodSp Name: homeVolumeName, VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, } - volumes := make([]corev1.Volume, 0, 4+len(signingVolumes)+len(nodeVolumes)+len(keyringVolumes)) + configVolumes := nodeConfigVolumes(node) + volumes := make([]corev1.Volume, 0, 4+len(signingVolumes)+len(nodeVolumes)+len(keyringVolumes)+len(configVolumes)) volumes = append(volumes, dataVolume, sidecarTmpVolume, homeVolume, proxyConfigVolume) volumes = append(volumes, signingVolumes...) volumes = append(volumes, nodeVolumes...) volumes = append(volumes, keyringVolumes...) + volumes = append(volumes, configVolumes...) dedicated := IsDedicatedNode(node) pool := NodepoolForMode(NodeMode(node), p, dedicated) @@ -1008,7 +1015,8 @@ func buildSidecarContainer(node *seiv1alpha1.SeiNode, p PlatformConfig) corev1.C env = append(env, keyringEnv...) keyringMounts := operatorKeyringMounts(node) - mounts := make([]corev1.VolumeMount, 0, 3+len(keyringMounts)) + configMounts := nodeConfigMounts(node) + mounts := make([]corev1.VolumeMount, 0, 3+len(keyringMounts)+len(configMounts)) mounts = append(mounts, // The `home` emptyDir backs homeMountPath so the nested data-PVC mount // has a writable-volume parent, rather than depending on the RO rootfs @@ -1019,6 +1027,7 @@ func buildSidecarContainer(node *seiv1alpha1.SeiNode, p PlatformConfig) corev1.C corev1.VolumeMount{Name: sidecarTmpVolumeName, MountPath: sidecarTmpMountPath}, ) mounts = append(mounts, keyringMounts...) + mounts = append(mounts, configMounts...) // No Command — the sidecar image's ENTRYPOINT is the command. c := corev1.Container{ @@ -1318,12 +1327,14 @@ func sidecarWaitCommand(node *seiv1alpha1.SeiNode) (command []string, args []str func buildNodeMainContainer(node *seiv1alpha1.SeiNode) corev1.Container { signingMounts := signingKeyMounts(node) nodeMounts := nodeKeyMounts(node) + configMounts := nodeConfigMounts(node) seidMountEnabled := operatorKeyringSeidMountEnabled(node) - mounts := make([]corev1.VolumeMount, 0, 3+len(signingMounts)+len(nodeMounts)) + mounts := make([]corev1.VolumeMount, 0, 3+len(signingMounts)+len(nodeMounts)+len(configMounts)) mounts = append(mounts, corev1.VolumeMount{Name: "data", MountPath: dataDir}) mounts = append(mounts, corev1.VolumeMount{Name: homeVolumeName, MountPath: homeMountPath}) mounts = append(mounts, signingMounts...) mounts = append(mounts, nodeMounts...) + mounts = append(mounts, configMounts...) if seidMountEnabled { mounts = append(mounts, operatorKeyringMounts(node)...) } @@ -1544,6 +1555,65 @@ func nodeKeyMounts(node *seiv1alpha1.SeiNode) []corev1.VolumeMount { }} } +// nodeConfigVolumes projects the operator's seid config files, one volume per +// file so the two references may name different ConfigMaps. Items names the +// single key, so a ConfigMap missing it fails the kubelet mount and +// `kubectl describe pod` says which key is absent. +func nodeConfigVolumes(node *seiv1alpha1.SeiNode) []corev1.Volume { + cfg := node.Spec.NodeConfig + if cfg == nil { + return nil + } + return []corev1.Volume{ + nodeConfigVolume(nodeConfigConfigVolumeName, cfg.ConfigRef.Name, configTomlDataKey), + nodeConfigVolume(nodeConfigAppVolumeName, cfg.AppRef.Name, appTomlDataKey), + } +} + +func nodeConfigVolume(volumeName, configMapName, dataKey string) corev1.Volume { + return corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: configMapName}, + DefaultMode: ptr.To[int32](0o444), + Items: []corev1.KeyToPath{{Key: dataKey, Path: dataKey}}, + }, + }, + } +} + +// nodeConfigMounts places the operator's files over the data volume's copies. +// subPath matches signingKeyMounts and nodeKeyMounts: kubelet pins the content +// at pod start, and seid re-reads config.toml only on a restart, so a +// hot-refreshing directory mount would buy nothing. +// +// Both the seid container and the sidecar carry these mounts, and the +// sidecar's is a safety property rather than a convenience. A rename onto a +// mounted path from a container that does NOT hold the mount succeeds and +// detaches it, after which seid reads the writer's file with nothing reporting +// the swap. From a container that DOES hold the mount the same rename returns +// EBUSY and fails the task. +// TestNodeConfigMountsOnSidecarIsASafetyProperty guards the sidecar mount. +func nodeConfigMounts(node *seiv1alpha1.SeiNode) []corev1.VolumeMount { + if node.Spec.NodeConfig == nil { + return nil + } + return []corev1.VolumeMount{ + nodeConfigMount(nodeConfigConfigVolumeName, configTomlDataKey), + nodeConfigMount(nodeConfigAppVolumeName, appTomlDataKey), + } +} + +func nodeConfigMount(volumeName, dataKey string) corev1.VolumeMount { + return corev1.VolumeMount{ + Name: volumeName, + MountPath: dataDir + "/config/" + dataKey, + SubPath: dataKey, + ReadOnly: true, + } +} + // nodeKeySecretSource returns the Secret holding this node's P2P identity, from // whichever mode sub-spec carries one (validator or seed). Mode-blind by // delegation, so the node-key volume and mount below serve both. diff --git a/internal/planner/doc.go b/internal/planner/doc.go index 78a0cfc7..bec08105 100644 --- a/internal/planner/doc.go +++ b/internal/planner/doc.go @@ -31,7 +31,8 @@ // // TaskPlan: the structure this package owns. Constructed by the plan builders // -// (buildBasePlan, buildNodeUpdatePlan, buildMarkReadyPlan, the group +// (buildBasePlan, buildBootstrapPlan, buildGenesisPlan, assembleUpdatePlan, +// assembleStaticUpdatePlan, buildMarkReadyPlan, the group // builders); persisted by the controller into .status.plan; read by the // planner on subsequent reconciles; mutated in-memory by Executor. Carries // Phase, the ordered Tasks, TargetPhase, FailedPhase, and on failure @@ -111,6 +112,26 @@ // witnesses (spec-declared rpcServers or the canonical-syncer registry). // Genesis (snap == nil) carries no such task. // Guarded by TestStateSyncGate_S3Restore_OneSyncer_FailsClosed. +// - No config writer reaches a node that mounts its config: spec.nodeConfig +// mounts config.toml and app.toml read-only over the seid config +// directory, and a rename onto a mounted path from another mount +// namespace detaches the mount, leaving seid on the writer's file with +// nothing reporting the swap. MountsNodeConfig is the predicate — the +// union of the spec and the observed stamp, because a node mid-revert +// still has the mount. withoutManagedConfigTasks strips those tasks where +// the progression is assembled, staticConfigPlanner owns the Running +// arms, and mountedConfigWriterInPlan refuses any plan that still carries +// one, whichever builder produced it. There is no per-pod exemption: a +// bootstrap Job pod holds the same PVC, so staticConfigPlanner.Validate +// refuses that combination outright. config-validate is stripped too — it +// reports on a file the operator owns, and sei-config's legacy reader +// defaults a missing [sei] mode to full, so on a validator the verdict +// can be confidently wrong. +// Guarded by TestMountedConfigWriterInPlan, +// TestStaticInitPlanCarriesNoConfigWriter, +// TestStaticNodeConfigRefusesBootstrap, +// TestNodeConfigRevertRollsBeforeAnyConfigWrite and +// TestBuildBootstrapPodSpec_NeverMountsNodeConfig. // // # Zero-Value & Sentinel Semantics // @@ -160,6 +181,15 @@ // empty — failures retry on the next reconcile rather than transitioning to // Failed. // +// Static-config update plans roll a Running node whose spec.nodeConfig names +// the ConfigMaps supplying config.toml and app.toml. staticConfigPlanner +// builds them in place of the mode's own update plan: apply-statefulset, +// apply-service, replace-pod, observe-image, mark-ready. They carry no config +// task at all, so they set no ConfigValuesHash. Kubelet pins a subPath mount +// at pod start, which is why pod replacement is the only way new config +// reaches seid, and replace-pod parses both ConfigMaps before it deletes +// anything. +// // When no drift is detected for a Running node, no plan is built. The node // sits in steady state with no active plan. // diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 4f959e67..fc69563a 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -14,6 +14,7 @@ import ( seiconfig "github.com/sei-protocol/sei-config" "go.opentelemetry.io/otel/metric" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -183,7 +184,7 @@ func (p *NodeResolver) ResolvePlan(ctx context.Context, node *seiv1alpha1.SeiNod return nil } - mode, err := p.plannerForMode(node) + mode, err := p.plannerFor(node) if err != nil { return err } @@ -195,6 +196,15 @@ func (p *NodeResolver) ResolvePlan(ctx context.Context, node *seiv1alpha1.SeiNod if err != nil { return err } + if writer := mountedConfigWriterInPlan(node, plan); writer != "" { + err := fmt.Errorf("plan carries %s on a node that mounts config.toml and app.toml from ConfigMaps: "+ + "the task renames over the mount, which detaches it and leaves seid reading the task's file", writer) + // BuildPlan may already have stamped UpdateStarted. The plan is refused + // and never persisted, so clear the claim rather than leave a node + // reporting an update it does not have. + setNodeUpdateCondition(node, metav1.ConditionFalse, reasonUpdatePlanBuildFailed, err.Error()) + return err + } if plan == nil { if shouldExplainUnobservedConfig(node) { setNodeUpdateCondition(node, metav1.ConditionFalse, "ConfigBaselineUnobserved", @@ -339,6 +349,21 @@ func planFailureMessage(plan *seiv1alpha1.TaskPlan) string { return unknownValue } +// plannerFor returns the NodePlanner for a SeiNode. A node whose config.toml +// and app.toml come from operator ConfigMaps gets its mode planner wrapped in +// staticConfigPlanner, which keeps the mode's own Validate and init plans and +// replaces only the Running arm. +func (r *NodeResolver) plannerFor(node *seiv1alpha1.SeiNode) (NodePlanner, error) { + mode, err := r.plannerForMode(node) + if err != nil { + return nil, err + } + if !MountsNodeConfig(node) { + return mode, nil + } + return &staticConfigPlanner{base: mode, platform: r.Platform}, nil +} + // plannerForMode returns the appropriate NodePlanner for the SeiNode's // mode sub-spec, threaded with the resolver's Platform config so each // planner can resolve the effective sidecar image for drift detection. @@ -594,6 +619,7 @@ func buildBasePlan( if err != nil { return nil, err } + sidecarProg = withoutManagedConfigTasks(node, sidecarProg) // Infrastructure tasks run before sidecar tasks. prog := make([]string, 0, 4+len(sidecarProg)) @@ -842,12 +868,36 @@ func nodeIsolationDrifted(node *seiv1alpha1.SeiNode) bool { return noderesource.EffectiveNodeIsolation(node) != node.Status.CurrentNodeIsolation } +// nodeConfigDrifted reports whether the operator's ConfigMap references +// diverge from the ones the running pod mounts. +// +// There is no unobserved short-circuit here, unlike the three predicates +// above: unset is the correct observation for a node that mounts nothing, and +// no node carries spec.nodeConfig before the controller that reads it, so an +// unset stamp cannot fleet-roll. +// +// Only the references are compared. Kubelet pins a subPath mount at pod start, +// so editing a ConfigMap in place never reaches a running seid. +func nodeConfigDrifted(node *seiv1alpha1.SeiNode) bool { + return !apiequality.Semantic.DeepEqual(node.Spec.NodeConfig, node.Status.CurrentNodeConfig) +} + +// formatNodeConfig renders the ConfigMap references for the +// NodeUpdateInProgress message an operator reads on a roll. +func formatNodeConfig(cfg *seiv1alpha1.NodeConfig) string { + if cfg == nil { + return "none" + } + return fmt.Sprintf("config=%q app=%q", cfg.ConfigRef.Name, cfg.AppRef.Name) +} + // podTemplateDrifted reports whether an observed pod-template input has -// drifted: seid image, sidecar image, or node isolation. The rendered nodepool -// is not observed, so an app-config scheduling.dedicated.* change alone does -// not roll. +// drifted: seid image, sidecar image, node isolation, or the operator's config +// ConfigMap. The rendered nodepool is not observed, so an app-config +// scheduling.dedicated.* change alone does not roll. func podTemplateDrifted(node *seiv1alpha1.SeiNode, p platform.Config) bool { - return imageDrifted(node) || sidecarImageDrifted(node, p) || nodeIsolationDrifted(node) + return imageDrifted(node) || sidecarImageDrifted(node, p) || + nodeIsolationDrifted(node) || nodeConfigDrifted(node) } // podTemplateDriftMessage formats the NodeUpdateInProgress message every mode @@ -858,6 +908,7 @@ func podTemplateDriftMessage(node *seiv1alpha1.SeiNode, p platform.Config) strin seid := imageDrifted(node) sc := sidecarImageDrifted(node, p) iso := nodeIsolationDrifted(node) + cfg := nodeConfigDrifted(node) var parts []string if seid { parts = append(parts, fmt.Sprintf("seid spec=%s current=%s", node.Spec.Image, node.Status.CurrentImage)) @@ -870,8 +921,16 @@ func podTemplateDriftMessage(node *seiv1alpha1.SeiNode, p platform.Config) strin parts = append(parts, fmt.Sprintf("nodeIsolation spec=%s current=%s", noderesource.EffectiveNodeIsolation(node), node.Status.CurrentNodeIsolation)) } + if cfg { + parts = append(parts, fmt.Sprintf("nodeConfig spec=%s current=%s", + formatNodeConfig(node.Spec.NodeConfig), formatNodeConfig(node.Status.CurrentNodeConfig))) + } detail := strings.Join(parts, "; ") switch { + case cfg && !seid && !sc && !iso: + return "node config drift detected: " + detail + case cfg: + return "node config and image drift detected: " + detail case iso && !seid && !sc: return "node isolation drift detected: " + detail case iso: diff --git a/internal/planner/static_config.go b/internal/planner/static_config.go new file mode 100644 index 00000000..0e4f7bd1 --- /dev/null +++ b/internal/planner/static_config.go @@ -0,0 +1,217 @@ +package planner + +import ( + "fmt" + "slices" + + "github.com/google/uuid" + 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/platform" + "github.com/sei-protocol/sei-k8s-controller/internal/task" + "github.com/sei-protocol/sei-k8s-controller/sidecarapi/client" +) + +// mountedConfigWriters are the sidecar tasks that write config.toml or +// app.toml. None of them may run on a pod that mounts those files from the +// operator's ConfigMaps. +// +// config-apply, config-reload, config-patch, configure-state-sync and +// set-genesis-peers all commit with os.Rename. A rename onto a mounted path +// from another container succeeds and detaches the mount, after which seid +// reads the writer's file instead of the operator's. generate-identity +// truncates rather than renames, which leaves the mount in place but fails +// against a read-only one. +// +// The list is maintained by hand against the sidecar's task handlers; nothing +// checks it automatically, because the sidecar is a separate module. +var mountedConfigWriters = []string{ + TaskConfigApply, + client.TaskTypeConfigReload, + TaskConfigPatch, + TaskConfigureStateSync, + TaskSetGenesisPeers, + TaskGenerateIdentity, +} + +// MountsNodeConfig reports whether this node's pod carries the ConfigMap +// mounts, or is about to. The spec describes the pod the controller wants and +// the stamp describes the pod that exists; a task that renames a mounted file +// is a hazard under either, so every refusal keys on the union. +// +// Reverting a node to controller-managed config is the case that needs the +// stamp: the spec no longer names a ConfigMap while the live pod still mounts +// one, and the mode planner's own update plan submits config-patch before it +// replaces the pod. +func MountsNodeConfig(node *seiv1alpha1.SeiNode) bool { + return node.Spec.NodeConfig != nil || node.Status.CurrentNodeConfig != nil +} + +// withoutManagedConfigTasks removes the config tasks from a progression built +// for a node whose config.toml and app.toml the controller does not own. It +// runs where the progression is assembled; mountedConfigWriterInPlan checks +// the finished plan whichever builder produced it. +// +// The writers go because they detach the mount. config-validate goes because +// it reports on a file the operator wrote, through sei-config's legacy reader, +// which falls back to mode "full" when app.toml carries no [sei] mode — so on +// a validator it passes a config seid will refuse. A verdict that can be +// confidently wrong is worse than no verdict. replace-pod parses both files +// before it deletes anything, and that check is the one that matters. +func withoutManagedConfigTasks(node *seiv1alpha1.SeiNode, prog []string) []string { + if !MountsNodeConfig(node) { + return prog + } + return slices.DeleteFunc(slices.Clone(prog), func(taskType string) bool { + return taskType == TaskConfigValidate || slices.Contains(mountedConfigWriters, taskType) + }) +} + +// mountedConfigWriterInPlan returns the first task in the plan that would +// write config.toml or app.toml on a pod that mounts them, or "" when the plan +// is safe. ResolvePlan refuses a plan it names, so the invariant is guarded +// once no matter which builder produced the plan. +// +// There is no per-pod exemption. A bootstrap Job pod carries no mount, but it +// holds the same PVC as the production pod, and a rename from there detaches +// the production pod's mount just as silently. staticConfigPlanner.Validate +// refuses the bootstrap combination for that reason, and this is the backstop. +func mountedConfigWriterInPlan(node *seiv1alpha1.SeiNode, plan *seiv1alpha1.TaskPlan) string { + if !MountsNodeConfig(node) || plan == nil { + return "" + } + for _, t := range plan.Tasks { + if slices.Contains(mountedConfigWriters, t.Type) { + return t.Type + } + } + return "" +} + +// staticConfigPlanner plans a node whose config.toml and app.toml come from +// operator-supplied ConfigMaps. +// +// It wraps the node's mode planner rather than replacing it, so the mode keeps +// its own Validate and its own init and bootstrap plans. Those plans are +// already safe: both production-pod progressions run through +// withoutMountedConfigWriters. What the wrapper owns is the Running arm, which +// the mode planners route through assembleUpdatePlan — an assembler that +// force-inserts config-apply whenever the configValues baseline is unobserved, +// which on one of these nodes is always. +type staticConfigPlanner struct { + base NodePlanner + platform platform.Config +} + +func (p *staticConfigPlanner) Mode() string { return p.base.Mode() } + +// Validate runs the mode's own checks first, then refuses the node shapes +// whose configuration seid can only learn at run time. Each of them reaches +// config.toml through a task this planner removes, so the ConfigMap would +// silently win and the node would start on configuration nobody intended. +func (p *staticConfigPlanner) Validate(node *seiv1alpha1.SeiNode) error { + if err := p.base.Validate(node); err != nil { + return err + } + if isGenesisCeremonyNode(node) { + return fmt.Errorf("nodeConfig is not supported on a genesis-ceremony validator: " + + "the founding validator set is assembled during the ceremony and written by set-genesis-peers, " + + "so it cannot be in a ConfigMap written beforehand") + } + if NeedsBootstrap(node) { + return fmt.Errorf("nodeConfig is not supported with a bootstrap Job: " + + "the Job pod holds the same data volume as the production pod and rewrites config.toml there, " + + "which detaches the production pod's mount and leaves seid reading the Job's file") + } + if snap := node.Spec.SnapshotSource(); snap != nil && snap.StateSync != nil { + return fmt.Errorf("nodeConfig is not supported with a state-sync snapshot source: " + + "configure-state-sync discovers the trust height and hash from live witnesses at run time, " + + "so they cannot be in a ConfigMap written beforehand") + } + if node.Spec.Consensus.IsAutobahn() { + return fmt.Errorf("nodeConfig is not supported under consensus engine Autobahn: " + + "the engine's config.toml keys are controller-derived and reach the node through the overlay this planner removes") + } + return nil +} + +// BuildPlan delegates every arm but Running to the mode planner. +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) +} + +// buildRunningPlan returns the update plan for a Running node, or nil if no +// drift. There is no configValues arm: the CRD rejects configValues alongside +// nodeConfig, so pod replacement is the only config-delivery mechanism here. +func (p *staticConfigPlanner) buildRunningPlan(node *seiv1alpha1.SeiNode) (*seiv1alpha1.TaskPlan, error) { + if podTemplateDrifted(node, p.platform) { + plan, err := p.buildUpdatePlan(node) + if err != nil { + return nil, err + } + setNodeUpdateCondition(node, metav1.ConditionTrue, "UpdateStarted", podTemplateDriftMessage(node, p.platform)) + return plan, nil + } + if sidecarNeedsReapproval(node) { + return buildMarkReadyPlan(node) + } + return nil, nil +} + +// buildUpdatePlan rolls the pod and nothing else. Kubelet pins a subPath mount +// at pod start, so replacing the pod is what delivers new config. The +// key-validation gates lead, as they do in every mode's update plan, so a +// missing Secret fails controller-side rather than as a kubelet mount error on +// the recreated pod. +func (p *staticConfigPlanner) buildUpdatePlan(node *seiv1alpha1.SeiNode) (*seiv1alpha1.TaskPlan, error) { + prog := make([]string, 0, 9) + if needsValidateSigningKey(node) { + prog = append(prog, task.TaskTypeValidateSigningKey) + } + if needsValidateNodeKey(node) { + prog = append(prog, task.TaskTypeValidateNodeKey) + } + if needsValidateOperatorKeyring(node) { + prog = append(prog, task.TaskTypeValidateOperatorKeyring) + } + prog = append(prog, + task.TaskTypeApplyStatefulSet, + task.TaskTypeApplyService, + task.TaskTypeReplacePod, + task.TaskTypeObserveImage, + TaskMarkReady, + ) + return assembleStaticUpdatePlan(node, prog) +} + +// assembleStaticUpdatePlan composes the progression into a TaskPlan. It is the +// sibling of assembleUpdatePlan for nodes that carry no configValues: no +// config-apply insertion, no overlay splice, and so no ConfigValuesHash to +// stamp. FailedPhase stays empty so a failure retries on the next reconcile. +func assembleStaticUpdatePlan(node *seiv1alpha1.SeiNode, prog []string) (_ *seiv1alpha1.TaskPlan, retErr error) { + defer func() { + if retErr != nil { + setNodeUpdateCondition(node, metav1.ConditionFalse, reasonUpdatePlanBuildFailed, retErr.Error()) + } + }() + + planID := uuid.New().String() + tasks := make([]seiv1alpha1.PlannedTask, len(prog)) + for i, taskType := range prog { + t, err := buildPlannedTask(planID, taskType, i, paramsForUpdateTask(node, taskType, nil)) + if err != nil { + return nil, err + } + tasks[i] = t + } + return &seiv1alpha1.TaskPlan{ + ID: planID, + Phase: seiv1alpha1.TaskPlanActive, + Tasks: tasks, + TargetPhase: seiv1alpha1.PhaseRunning, + }, nil +} diff --git a/internal/planner/static_config_test.go b/internal/planner/static_config_test.go new file mode 100644 index 00000000..3c607a8c --- /dev/null +++ b/internal/planner/static_config_test.go @@ -0,0 +1,341 @@ +package planner + +import ( + "context" + "testing" + + . "github.com/onsi/gomega" + "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/task" +) + +const staticConfigMapName = "rpc-config-v1" + +func withNodeConfig(node *seiv1alpha1.SeiNode, name string) *seiv1alpha1.SeiNode { + node.Spec.NodeConfig = &seiv1alpha1.NodeConfig{ + ConfigRef: seiv1alpha1.ConfigFileRef{Name: name}, + AppRef: seiv1alpha1.ConfigFileRef{Name: name}, + } + return node +} + +// pendingNode is an un-provisioned node in the given mode, ready for an init plan. +func pendingNode(configure func(*seiv1alpha1.SeiNode)) *seiv1alpha1.SeiNode { + node := &seiv1alpha1.SeiNode{ + ObjectMeta: metav1.ObjectMeta{Name: "node-0", Namespace: "default", Generation: 1}, + Spec: seiv1alpha1.SeiNodeSpec{ChainID: "atlantic-2", Image: "sei:v1.0.0"}, + Status: seiv1alpha1.SeiNodeStatus{Phase: seiv1alpha1.PhasePending}, + } + configure(node) + return node +} + +var staticModes = []struct { + name string + configure func(*seiv1alpha1.SeiNode) +}{ + {"full", func(n *seiv1alpha1.SeiNode) { n.Spec.FullNode = &seiv1alpha1.FullNodeSpec{} }}, + {"archive", func(n *seiv1alpha1.SeiNode) { n.Spec.Archive = &seiv1alpha1.ArchiveSpec{} }}, + {"validator", func(n *seiv1alpha1.SeiNode) { n.Spec.Validator = &seiv1alpha1.ValidatorSpec{} }}, +} + +// TestStaticInitPlanCarriesNoConfigWriter is the assertion the whole feature +// rests on. A task that writes config.toml on the production pod renames it, +// and a rename onto a mounted path from another container detaches the mount. +func TestStaticInitPlanCarriesNoConfigWriter(t *testing.T) { + for _, mode := range staticModes { + t.Run(mode.name, func(t *testing.T) { + g := NewWithT(t) + node := withNodeConfig(pendingNode(mode.configure), staticConfigMapName) + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).NotTo(BeNil()) + + types := planTaskTypes(node.Status.Plan) + for _, writer := range mountedConfigWriters { + g.Expect(types).NotTo(ContainElement(writer)) + } + g.Expect(types).NotTo(ContainElement(TaskConfigValidate), + "the controller does not validate a file the operator owns") + g.Expect(types).To(ContainElement(TaskConfigureGenesis)) + g.Expect(types[len(types)-1]).To(Equal(TaskMarkReady)) + }) + } +} + +// TestInitPlanKeepsConfigWriterWithoutConfigSource pins the other half: the +// filter is inert for a node the controller configures. +func TestInitPlanKeepsConfigWriterWithoutConfigSource(t *testing.T) { + for _, mode := range staticModes { + t.Run(mode.name, func(t *testing.T) { + g := NewWithT(t) + node := pendingNode(mode.configure) + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(planTaskTypes(node.Status.Plan)).To(ContainElement(TaskConfigApply)) + }) + } +} + +// TestStaticNodeConfigRefusesBootstrap closes the one silent path. The +// bootstrap Job pod carries no mount, but it holds the same data volume as the +// production pod, which reconcileStatefulSet creates unconditionally. A rename +// from the Job pod detaches the production pod's mount and seid then boots on +// the Job's generated config, with the plan reporting success. +func TestStaticNodeConfigRefusesBootstrap(t *testing.T) { + g := NewWithT(t) + node := withNodeConfig(pendingNode(func(n *seiv1alpha1.SeiNode) { + n.Spec.FullNode = &seiv1alpha1.FullNodeSpec{ + Snapshot: &seiv1alpha1.SnapshotSource{ + BootstrapImage: "sei:v1.0.0", + S3: &seiv1alpha1.S3SnapshotSource{TargetHeight: 100}, + }, + } + }), staticConfigMapName) + g.Expect(NeedsBootstrap(node)).To(BeTrue(), "fixture must need a bootstrap Job") + + err := (&NodeResolver{}).ResolvePlan(context.Background(), node) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("bootstrap Job")) + g.Expect(node.Status.Plan).To(BeNil()) +} + +// TestStaticRunningPlanRollsThePod covers every transition of the reference. +// Pod replacement is the only config-delivery mechanism: kubelet pins a subPath +// mount at pod start. +func TestStaticRunningPlanRollsThePod(t *testing.T) { + cases := []struct { + name string + spec string + observed string + wantRoll bool + wantInMsg string + }{ + {"adopted by a running node", staticConfigMapName, "", true, staticConfigMapName}, + {"republished under a new name", "rpc-config-v2", staticConfigMapName, true, "rpc-config-v2"}, + {"cleared", "", staticConfigMapName, true, staticConfigMapName}, + {"unchanged", staticConfigMapName, staticConfigMapName, false, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + node := runningFullNode() + if tc.spec != "" { + withNodeConfig(node, tc.spec) + } + if tc.observed != "" { + node.Status.CurrentNodeConfig = &seiv1alpha1.NodeConfig{ + ConfigRef: seiv1alpha1.ConfigFileRef{Name: tc.observed}, + AppRef: seiv1alpha1.ConfigFileRef{Name: tc.observed}, + } + } + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + + if !tc.wantRoll { + g.Expect(node.Status.Plan).To(BeNil()) + return + } + g.Expect(node.Status.Plan).NotTo(BeNil()) + types := planTaskTypes(node.Status.Plan) + g.Expect(types).To(ContainElement(task.TaskTypeReplacePod)) + g.Expect(types).To(ContainElement(task.TaskTypeObserveImage)) + g.Expect(types).To(ContainElement(TaskMarkReady)) + + // The roll carries no config task at all: the writers detach the + // mount, and config-validate reports on a file the operator owns. + for _, writer := range mountedConfigWriters { + g.Expect(types).NotTo(ContainElement(writer)) + } + g.Expect(types).NotTo(ContainElement(TaskConfigValidate)) + + cond := meta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionNodeUpdateInProgress) + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Message).To(ContainSubstring(tc.wantInMsg)) + }) + } +} + +// TestStaticRunningPlanDoesNotLoop pins the stamp that ends the roll. +func TestStaticRunningPlanDoesNotLoop(t *testing.T) { + g := NewWithT(t) + node := withNodeConfig(runningFullNode(), staticConfigMapName) + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).NotTo(BeNil()) + + node.Status.CurrentNodeConfig = node.Spec.NodeConfig.DeepCopy() + node.Status.Plan = nil + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).To(BeNil()) +} + +// TestNodeConfigUnsetDoesNotRollOnControllerUpgrade is the invariant that +// replaces the unobserved short-circuit the other pod-template predicates +// carry. Every node in the fleet looks like this on the first reconcile after +// the controller ships. +func TestNodeConfigUnsetDoesNotRollOnControllerUpgrade(t *testing.T) { + g := NewWithT(t) + node := runningFullNode() + + g.Expect(nodeConfigDrifted(node)).To(BeFalse()) + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).To(BeNil()) +} + +// TestStaticValidateRunsTheModesOwnChecks pins the wrapping. Replacing the mode +// planner instead of wrapping it would drop every per-mode Validate. +func TestStaticValidateRunsTheModesOwnChecks(t *testing.T) { + g := NewWithT(t) + // A seed without a node-key Secret is refused by seedPlanner.Validate. + node := withNodeConfig(pendingNode(func(n *seiv1alpha1.SeiNode) { + n.Spec.Seed = &seiv1alpha1.SeedSpec{} + }), staticConfigMapName) + + err := (&NodeResolver{}).ResolvePlan(context.Background(), node) + g.Expect(err).To(HaveOccurred()) + g.Expect(node.Status.Plan).To(BeNil()) +} + +// TestStaticValidateRefusesRuntimeDiscoveredConfig covers the node shapes whose +// configuration seid can only learn while running. A ConfigMap written +// beforehand cannot hold those values, and the task that would write them is +// the one this planner removes. +func TestStaticValidateRefusesRuntimeDiscoveredConfig(t *testing.T) { + cases := []struct { + name string + configure func(*seiv1alpha1.SeiNode) + wantErr string + }{ + {"genesis ceremony", func(n *seiv1alpha1.SeiNode) { + n.Spec.Validator = &seiv1alpha1.ValidatorSpec{ + GenesisCeremony: &seiv1alpha1.GenesisCeremonyNodeConfig{ + ChainID: "atlantic-2", + StakingAmount: "1000000usei", + AccountBalance: "2000000usei", + }, + } + }, "genesis-ceremony"}, + {"state-sync snapshot source", func(n *seiv1alpha1.SeiNode) { + n.Spec.FullNode = &seiv1alpha1.FullNodeSpec{ + Snapshot: &seiv1alpha1.SnapshotSource{StateSync: &seiv1alpha1.StateSyncSource{}}, + } + }, "state-sync"}, + {"autobahn consensus", func(n *seiv1alpha1.SeiNode) { + n.Spec.FullNode = &seiv1alpha1.FullNodeSpec{} + n.Spec.Consensus = &seiv1alpha1.ConsensusSpec{Engine: seiv1alpha1.ConsensusEngineAutobahn} + }, "Autobahn"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + node := withNodeConfig(pendingNode(tc.configure), staticConfigMapName) + + err := (&NodeResolver{}).ResolvePlan(context.Background(), node) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tc.wantErr)) + g.Expect(node.Status.Plan).To(BeNil()) + }) + } +} + +// TestStaticWorkflowRefused pins the planner half of the lockstep pair with +// SeiNodeReconciler's adoption-time refusal. Every recipe writes config.toml. +func TestStaticWorkflowRefused(t *testing.T) { + g := NewWithT(t) + node := withNodeConfig(runningFullNode(), staticConfigMapName) + wf := &seiv1alpha1.SeiNodeTaskWorkflow{ + Spec: seiv1alpha1.SeiNodeTaskWorkflowSpec{ + StateSync: &seiv1alpha1.StateSyncWorkflow{}, + }, + } + + err := (&stateSyncWorkflowPlanner{}).Validate(node, wf) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("static-config")) +} + +// TestMountedConfigWriterInPlan covers the single gate that holds the +// invariant. The progression filter removes the writers and +// staticConfigPlanner owns the Running arms; this checks the finished plan +// regardless of which builder produced it. +func TestMountedConfigWriterInPlan(t *testing.T) { + plan := func(types ...string) *seiv1alpha1.TaskPlan { + tasks := make([]seiv1alpha1.PlannedTask, len(types)) + for i, tt := range types { + tasks[i] = seiv1alpha1.PlannedTask{Type: tt} + } + return &seiv1alpha1.TaskPlan{Tasks: tasks} + } + + cases := []struct { + name string + plan *seiv1alpha1.TaskPlan + want string + }{ + {"clean plan", plan(TaskConfigureGenesis, TaskConfigValidate, TaskMarkReady), ""}, + {"config-apply", plan(TaskConfigureGenesis, TaskConfigApply, TaskMarkReady), TaskConfigApply}, + {"config-patch", plan(task.TaskTypeApplyStatefulSet, TaskConfigPatch), TaskConfigPatch}, + {"inside a bootstrap window is NOT exempt", plan( + task.TaskTypeDeployBootstrapJob, TaskConfigApply, + task.TaskTypeTeardownBootstrap, TaskMarkReady), TaskConfigApply}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + node := withNodeConfig(runningFullNode(), staticConfigMapName) + g.Expect(mountedConfigWriterInPlan(node, tc.plan)).To(Equal(tc.want)) + }) + } + + t.Run("inert for a node that mounts nothing", func(t *testing.T) { + g := NewWithT(t) + g.Expect(mountedConfigWriterInPlan(runningFullNode(), plan(TaskConfigApply))).To(BeEmpty()) + }) + + // A node mid-revert still has the mount, so the stamp gates it too. + t.Run("keys on the stamp as well as the spec", func(t *testing.T) { + g := NewWithT(t) + node := runningFullNode() + node.Status.CurrentNodeConfig = &seiv1alpha1.NodeConfig{ + ConfigRef: seiv1alpha1.ConfigFileRef{Name: staticConfigMapName}, + AppRef: seiv1alpha1.ConfigFileRef{Name: staticConfigMapName}, + } + g.Expect(MountsNodeConfig(node)).To(BeTrue()) + g.Expect(mountedConfigWriterInPlan(node, plan(TaskConfigPatch))).To(Equal(TaskConfigPatch)) + }) +} + +// TestNodeConfigRevertRollsBeforeAnyConfigWrite covers the revert. A node +// whose spec no longer names ConfigMaps still has a pod that mounts them, and +// the mode planner's own update plan submits config-patch before it replaces +// the pod — a rename in the sidecar's own mount namespace, which fails EBUSY +// and rebuilds the same plan every reconcile. The stamp keeps the node on the +// static planner until the roll has actually dropped the mount. +func TestNodeConfigRevertRollsBeforeAnyConfigWrite(t *testing.T) { + g := NewWithT(t) + node := runningFullNode() + node.Status.CurrentNodeConfig = &seiv1alpha1.NodeConfig{ + ConfigRef: seiv1alpha1.ConfigFileRef{Name: staticConfigMapName}, + AppRef: seiv1alpha1.ConfigFileRef{Name: staticConfigMapName}, + } + + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).NotTo(BeNil()) + + types := planTaskTypes(node.Status.Plan) + for _, writer := range mountedConfigWriters { + g.Expect(types).NotTo(ContainElement(writer)) + } + g.Expect(types).To(ContainElement(task.TaskTypeReplacePod)) + + // Once the roll drops the mount, the node returns to the mode planner and + // settles: no drift, no plan. + node.Status.CurrentNodeConfig = nil + node.Status.Plan = nil + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(node.Status.Plan).To(BeNil()) +} diff --git a/internal/planner/workflow.go b/internal/planner/workflow.go index 319591ed..b382521e 100644 --- a/internal/planner/workflow.go +++ b/internal/planner/workflow.go @@ -63,6 +63,12 @@ func (p *stateSyncWorkflowPlanner) Validate(node *seiv1alpha1.SeiNode, wf *seiv1 if node.Spec.FullNode == nil { return fmt.Errorf("stateSync workflow refuses non-full/RPC target %s/%s", node.Namespace, node.Name) } + // The recipe's config-patch and configure-state-sync both write config.toml, + // which a node with nodeConfig mounts read-only. The stamp matters as much + // as the spec: a node mid-revert still has the mount. + if MountsNodeConfig(node) { + return fmt.Errorf("stateSync workflow refuses static-config target %s/%s", node.Namespace, node.Name) + } return nil } diff --git a/internal/task/bootstrap_resources_test.go b/internal/task/bootstrap_resources_test.go index 19351b54..758455aa 100644 --- a/internal/task/bootstrap_resources_test.go +++ b/internal/task/bootstrap_resources_test.go @@ -398,3 +398,31 @@ func TestBuildBootstrapPodSpec_DedicatedFollowsSingleTenantPool(t *testing.T) { g.Expect(terms[0].MatchExpressions[0].Values).To(ConsistOf("sei-validator-dedicated")) g.Expect(spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution).To(HaveLen(2)) } + +// TestBuildBootstrapPodSpec_NeverMountsNodeConfig is the other half of the +// static-config split. The bootstrap Job pod keeps config-apply and +// configure-state-sync in its progression, so it must never carry the mount +// those tasks would rename over. +func TestBuildBootstrapPodSpec_NeverMountsNodeConfig(t *testing.T) { + g := NewWithT(t) + snap := &seiv1alpha1.SnapshotSource{S3: &seiv1alpha1.S3SnapshotSource{TargetHeight: 100}} + node := validatorNodeWithSecrets("", "", "") + node.Spec.NodeConfig = &seiv1alpha1.NodeConfig{ + ConfigRef: seiv1alpha1.ConfigFileRef{Name: "rpc-config-v1"}, + AppRef: seiv1alpha1.ConfigFileRef{Name: "rpc-config-v1"}, + } + + spec := buildBootstrapPodSpec(node, snap, platformtest.Config()) + + for _, v := range spec.Volumes { + g.Expect(v.ConfigMap == nil || v.ConfigMap.Name != "rpc-config-v1").To(BeTrue(), + "bootstrap pod must not mount the operator config ConfigMap") + } + containers := append(append([]corev1.Container{}, spec.Containers...), spec.InitContainers...) + for _, c := range containers { + for _, m := range c.VolumeMounts { + g.Expect(m.MountPath).NotTo(ContainSubstring("/config/config.toml"), "container %s", c.Name) + g.Expect(m.MountPath).NotTo(ContainSubstring("/config/app.toml"), "container %s", c.Name) + } + } +} diff --git a/internal/task/observe_image.go b/internal/task/observe_image.go index 6da67364..c07084bb 100644 --- a/internal/task/observe_image.go +++ b/internal/task/observe_image.go @@ -45,9 +45,9 @@ func deserializeObserveImage(id string, params json.RawMessage, cfg ExecutionCon } // Execute polls the StatefulSet rollout. If the rollout is complete, stamps -// status.currentImage, status.currentSidecarImage and -// status.currentNodeIsolation on the owning SeiNode -// and marks the task complete. If the rollout is still in progress, returns +// status.currentImage, status.currentSidecarImage, +// status.currentNodeIsolation and status.currentNodeConfig on the owning +// SeiNode and marks the task complete. If the rollout is still in progress, returns // nil — the executor will re-invoke on the next reconcile since the task // remains Pending. func (e *observeImageExecution) Execute(ctx context.Context) error { @@ -79,6 +79,7 @@ func (e *observeImageExecution) Execute(ctx context.Context) error { node.Status.CurrentImage = node.Spec.Image node.Status.CurrentSidecarImage = EffectiveSidecarImage(node, e.cfg.Platform) node.Status.CurrentNodeIsolation = noderesource.EffectiveNodeIsolation(node) + node.Status.CurrentNodeConfig = node.Spec.NodeConfig.DeepCopy() e.complete() return nil } diff --git a/internal/task/replace_pod.go b/internal/task/replace_pod.go index 51d2e638..cfaa27ad 100644 --- a/internal/task/replace_pod.go +++ b/internal/task/replace_pod.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "strings" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -12,6 +13,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" seiv1alpha1 "github.com/sei-protocol/sei-k8s-controller/api/v1alpha1" + "github.com/sei-protocol/sei-k8s-controller/sidecarapi/tomlpatch" ) const TaskTypeReplacePod = "replace-pod" @@ -76,6 +78,10 @@ func (e *replacePodExecution) Execute(ctx context.Context) error { return err } + if err := e.guardNodeConfig(ctx, node); err != nil { + return err + } + pods, err := e.ownedPods(ctx, node, sts) if err != nil { return err @@ -103,6 +109,66 @@ func (e *replacePodExecution) Execute(ctx context.Context) error { return nil } +// guardNodeConfig re-reads the ConfigMaps that supply this node's seid config +// files, and refuses to delete the pod when one of them cannot be mounted or +// cannot be loaded. +// Plan build already checked the spec, but reconciles pass between then and +// now, and a ConfigMap pruned in that window would leave the replacement pod +// in ContainerCreating with nothing to roll it back — StatefulSets are +// OnDelete. Failing the plan leaves the running pod running. +// +// It parses the content too. Nothing else reads these files before seid does, +// and by then the previous pod is gone, so unparseable TOML checked any later +// costs the node its only working pod. +func (e *replacePodExecution) guardNodeConfig(ctx context.Context, node *seiv1alpha1.SeiNode) error { + cfg := node.Spec.NodeConfig + if cfg == nil { + return nil + } + files := []struct { + ref seiv1alpha1.ConfigFileRef + file string + }{ + {cfg.ConfigRef, "config.toml"}, + {cfg.AppRef, "app.toml"}, + } + for _, f := range files { + cm := &corev1.ConfigMap{} + key := types.NamespacedName{Name: f.ref.Name, Namespace: node.Namespace} + if err := e.cfg.APIReader.Get(ctx, key, cm); err != nil { + if apierrors.IsNotFound(err) { + return Terminal(fmt.Errorf("configmap %q not found: the replacement pod could not mount %s", f.ref.Name, f.file)) + } + return fmt.Errorf("getting configmap %q: %w", f.ref.Name, err) + } + content, ok := cm.Data[f.file] + if !ok { + raw, binary := cm.BinaryData[f.file] + if !binary { + return Terminal(fmt.Errorf("configmap %q has no %q key: the replacement pod could not mount it", f.ref.Name, f.file)) + } + content = string(raw) + } + if err := validateTOML(content); err != nil { + return Terminal(fmt.Errorf("configmap %q key %q: %w", f.ref.Name, f.file, err)) + } + } + return nil +} + +// validateTOML rejects content seid cannot load. The replacement pod mounts +// these bytes and nothing else reads them first, so an unparseable file would +// otherwise reach a pod whose predecessor has already been deleted. +func validateTOML(content string) error { + if strings.TrimSpace(content) == "" { + return fmt.Errorf("is empty") + } + if _, err := tomlpatch.UnmarshalTOML([]byte(content)); err != nil { + return fmt.Errorf("is not valid TOML: %w", err) + } + return nil +} + // Status returns the cached execution status. replace-pod completes // synchronously within Execute (no readiness wait). func (e *replacePodExecution) Status(_ context.Context) ExecutionStatus { diff --git a/internal/task/replace_pod_test.go b/internal/task/replace_pod_test.go index 58d202ae..0f7a85ba 100644 --- a/internal/task/replace_pod_test.go +++ b/internal/task/replace_pod_test.go @@ -372,3 +372,65 @@ func TestReplacePod_MultiReplica_TerminalError(t *testing.T) { g.Expect(err).To(BeAssignableToTypeOf(termErr)) g.Expect(err.Error()).To(ContainSubstring("multi-replica")) } + +// TestReplacePod_GuardNodeConfig covers the one-way action's precondition. +// Plan build checked the spec, but reconciles pass before replace-pod runs, +// and nothing reads these files before seid does — by then the previous pod is +// gone. +func TestReplacePod_GuardNodeConfig(t *testing.T) { + const validTOML = "moniker = \"node-1\"\n" + + configMap := func(data map[string]string) *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "rpc-config-v1", Namespace: testReplaceNs}, + Data: data, + } + } + + cases := []struct { + name string + objs []client.Object + wantErr string + wantTerm bool + }{ + {"both keys valid", []client.Object{configMap(map[string]string{ + "config.toml": validTOML, "app.toml": validTOML})}, "", false}, + {"configmap absent", nil, "not found", true}, + {"app.toml missing", []client.Object{configMap(map[string]string{ + "config.toml": validTOML})}, `no "app.toml" key`, true}, + {"config.toml missing", []client.Object{configMap(map[string]string{ + "app.toml": validTOML})}, `no "config.toml" key`, true}, + {"app.toml empty", []client.Object{configMap(map[string]string{ + "config.toml": validTOML, "app.toml": " \n"})}, "is empty", true}, + {"config.toml malformed", []client.Object{configMap(map[string]string{ + "config.toml": "moniker = \n[[[", "app.toml": validTOML})}, "not valid TOML", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + node := replacePodNode() + node.Spec.NodeConfig = &seiv1alpha1.NodeConfig{ + ConfigRef: seiv1alpha1.ConfigFileRef{Name: "rpc-config-v1"}, + AppRef: seiv1alpha1.ConfigFileRef{Name: "rpc-config-v1"}, + } + cfg := replacePodCfg(t, node, tc.objs...) + + err := newReplacePodExecRaw(t, cfg).guardNodeConfig(context.Background(), node) + + if !tc.wantTerm { + g.Expect(err).NotTo(HaveOccurred()) + return + } + var termErr *TerminalError + g.Expect(err).To(BeAssignableToTypeOf(termErr)) + g.Expect(err.Error()).To(ContainSubstring(tc.wantErr)) + }) + } + + t.Run("no nodeConfig is a no-op", func(t *testing.T) { + g := NewWithT(t) + node := replacePodNode() + cfg := replacePodCfg(t, node) + g.Expect(newReplacePodExecRaw(t, cfg).guardNodeConfig(context.Background(), node)).To(Succeed()) + }) +} diff --git a/manifests/sei.io_seinodes.yaml b/manifests/sei.io_seinodes.yaml index 3c02be84..707d08a2 100644 --- a/manifests/sei.io_seinodes.yaml +++ b/manifests/sei.io_seinodes.yaml @@ -86,6 +86,11 @@ spec: limits is not compared here: the equality rule already pins limits.memory to requests.memory, and the controller derives the limit from the request, so the footprint is frozen by freezing requests. + A node with spec.nodeConfig runs no task that writes config.toml or + app.toml, so every field whose only route to seid was one of those tasks is + rejected beside it. Accepting one would report success on an edit that never + reached the node — and for peers, status.resolvedPeers would keep updating + and keep looking correct while config.toml stayed as the operator wrote it. properties: archive: description: Archive configures an archive node with full history @@ -557,6 +562,75 @@ spec: maxLength: 512 minLength: 1 type: string + nodeConfig: + description: |- + NodeConfig supplies this node's seid config files from existing + ConfigMaps. The files mount read-only over the seid config directory, so + they replace whatever the data volume already holds. + + A node with this field set takes the static-config plan: no task in its + plan writes either file on the production pod. That is what keeps the + mount attached. A rename onto a mounted path from another container + detaches the mount, and seid then reads the writer's file. + + The operator owns both files verbatim. The controller supplies nothing: + not the mode's base configuration, not persistent-peers, not + external-address, not the freeze height, not the snapshot-generation + keys. It does not validate them either — replace-pod parses both files + before it deletes a pod, and that is the only check. + + The fields whose only route to seid was a config task are rejected + beside this one: configValues, overrides, peers, externalAddress. + + The references are the unit of change. Kubelet pins a subPath mount at + pod start, so editing a ConfigMap in place does not reach a running pod. + Publish under a new name and the node rolls. + + Not supported with a bootstrap Job, a state-sync snapshot source, a + genesis ceremony, or consensus engine Autobahn. Each of those writes + config.toml at run time, and the plan is refused. + + The StatefulSet carries the references as soon as they are set, before + any plan runs. A reference that does not resolve leaves the template + unmountable: the controller will not replace the pod itself, but a + drain, an eviction, or a manual delete recreates it into + ContainerCreating, and StatefulSets are OnDelete so nothing rolls it + back. Create the ConfigMaps first. + properties: + appRef: + description: AppRef holds app.toml. + properties: + name: + description: |- + Name of an existing ConfigMap in the SeiNode's namespace. The file is + read from the key matching its own name, config.toml or app.toml. + Kubelet refuses the mount when that key is absent, so the pod stays in + ContainerCreating and `kubectl describe pod` names the missing key. + maxLength: 253 + minLength: 1 + type: string + required: + - name + type: object + configRef: + description: ConfigRef holds config.toml. + properties: + name: + description: |- + Name of an existing ConfigMap in the SeiNode's namespace. The file is + read from the key matching its own name, config.toml or app.toml. + Kubelet refuses the mount when that key is absent, so the pod stays in + ContainerCreating and `kubectl describe pod` names the missing key. + maxLength: 253 + minLength: 1 + type: string + required: + - name + type: object + required: + - appRef + - configRef + type: object overrides: additionalProperties: type: string @@ -1377,6 +1451,22 @@ spec: == 0) && (!('memory' in self.resources.requests) || !('memory' in oldSelf.resources.requests) || quantity(string(self.resources.requests['memory'])).compareTo(quantity(string(oldSelf.resources.requests['memory']))) == 0)))) + - message: 'spec.nodeConfig and spec.configValues cannot both be set: + a node reading its config from ConfigMaps runs no config-patch task, + so the values would never reach seid; put them in the ConfigMap' + rule: '!has(self.nodeConfig) || !has(self.configValues)' + - message: 'spec.nodeConfig and spec.overrides cannot both be set: a node + reading its config from ConfigMaps runs no config-apply task, so the + overrides would never reach seid; put them in the ConfigMap' + rule: '!has(self.nodeConfig) || !has(self.overrides)' + - message: 'spec.nodeConfig and spec.peers cannot both be set: nothing + carries a resolved peer set into config.toml on a node reading its + config from ConfigMaps; write p2p.persistent-peers in the ConfigMap' + rule: '!has(self.nodeConfig) || !has(self.peers)' + - message: 'spec.nodeConfig and spec.externalAddress cannot both be set: + nothing carries it into config.toml on a node reading its config from + ConfigMaps; write p2p.external-address in the ConfigMap' + rule: '!has(self.nodeConfig) || !has(self.externalAddress)' status: description: SeiNodeStatus defines the observed state of a SeiNode. properties: @@ -1495,6 +1585,50 @@ spec: Parent controllers compare this against spec.image to determine whether a spec change has been fully actuated. type: string + currentNodeConfig: + description: |- + CurrentNodeConfig is the spec.nodeConfig the owned StatefulSet's pod was + last rolled with, stamped jointly with CurrentImage on rollout + completion. Unset means the pod mounts no operator-supplied config. + + Unset means the pod mounts no operator-supplied config. Unlike the fields + above, unset is a real observation and not "not yet observed". It records + the references, never the ConfigMaps' contents. + properties: + appRef: + description: AppRef holds app.toml. + properties: + name: + description: |- + Name of an existing ConfigMap in the SeiNode's namespace. The file is + read from the key matching its own name, config.toml or app.toml. + Kubelet refuses the mount when that key is absent, so the pod stays in + ContainerCreating and `kubectl describe pod` names the missing key. + maxLength: 253 + minLength: 1 + type: string + required: + - name + type: object + configRef: + description: ConfigRef holds config.toml. + properties: + name: + description: |- + Name of an existing ConfigMap in the SeiNode's namespace. The file is + read from the key matching its own name, config.toml or app.toml. + Kubelet refuses the mount when that key is absent, so the pod stays in + ContainerCreating and `kubectl describe pod` names the missing key. + maxLength: 253 + minLength: 1 + type: string + required: + - name + type: object + required: + - appRef + - configRef + type: object currentNodeIsolation: description: |- CurrentNodeIsolation is the effective node isolation the owned From 3bb2054712b3f79c6ab16e6e1c15057f6c56b80f Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 17 Sep 2026 09:47:22 +0200 Subject: [PATCH 2/4] fix(lint): satisfy goconst; noderesource owns the ConfigMap key names golangci-lint flagged eleven repeated string literals across the new files. The planner tests reuse the package's existing constants; the task tests gain one for the fixture ConfigMap name. config.toml and app.toml were restated in replace_pod.go, which checks a ConfigMap the renderer mounts. The renderer now exports ConfigTomlKey and AppTomlKey and the guard consumes them, so the mount contract has one owner. Co-Authored-By: Claude Opus 5 (1M context) --- internal/noderesource/noderesource.go | 12 ++++++++++-- internal/planner/static_config_test.go | 23 ++++++++++++++--------- internal/task/bootstrap_resources_test.go | 6 +++--- internal/task/replace_pod.go | 5 +++-- internal/task/replace_pod_test.go | 21 +++++++++++++-------- 5 files changed, 43 insertions(+), 24 deletions(-) diff --git a/internal/noderesource/noderesource.go b/internal/noderesource/noderesource.go index dc3bc563..407bf9f9 100644 --- a/internal/noderesource/noderesource.go +++ b/internal/noderesource/noderesource.go @@ -136,8 +136,8 @@ const ( nodeConfigConfigVolumeName = "node-config-config" nodeConfigAppVolumeName = "node-config-app" - configTomlDataKey = "config.toml" - appTomlDataKey = "app.toml" + configTomlDataKey = ConfigTomlKey + appTomlDataKey = AppTomlKey operatorKeyringVolumeName = "operator-keyring" // keyring.New(BackendFile, rootDir) opens rootDir/keyring-file/. @@ -1555,6 +1555,14 @@ func nodeKeyMounts(node *seiv1alpha1.SeiNode) []corev1.VolumeMount { }} } +// ConfigTomlKey and AppTomlKey are the ConfigMap keys a spec.nodeConfig +// reference must carry. They are the mount contract, so anything that checks a +// referenced ConfigMap reads them from here rather than restating them. +const ( + ConfigTomlKey = "config.toml" + AppTomlKey = "app.toml" +) + // nodeConfigVolumes projects the operator's seid config files, one volume per // file so the two references may name different ConfigMaps. Items names the // single key, so a ConfigMap missing it fails the kubelet mount and diff --git a/internal/planner/static_config_test.go b/internal/planner/static_config_test.go index 3c607a8c..3af22708 100644 --- a/internal/planner/static_config_test.go +++ b/internal/planner/static_config_test.go @@ -12,7 +12,12 @@ import ( "github.com/sei-protocol/sei-k8s-controller/internal/task" ) -const staticConfigMapName = "rpc-config-v1" +const ( + staticConfigMapName = "rpc-config-v1" + staticTestNamespace = "default" + staticTestChainID = "atlantic-2" + staticTestImage = "sei:v1.0.0" +) func withNodeConfig(node *seiv1alpha1.SeiNode, name string) *seiv1alpha1.SeiNode { node.Spec.NodeConfig = &seiv1alpha1.NodeConfig{ @@ -25,8 +30,8 @@ func withNodeConfig(node *seiv1alpha1.SeiNode, name string) *seiv1alpha1.SeiNode // pendingNode is an un-provisioned node in the given mode, ready for an init plan. func pendingNode(configure func(*seiv1alpha1.SeiNode)) *seiv1alpha1.SeiNode { node := &seiv1alpha1.SeiNode{ - ObjectMeta: metav1.ObjectMeta{Name: "node-0", Namespace: "default", Generation: 1}, - Spec: seiv1alpha1.SeiNodeSpec{ChainID: "atlantic-2", Image: "sei:v1.0.0"}, + ObjectMeta: metav1.ObjectMeta{Name: testNodeName, Namespace: staticTestNamespace, Generation: 1}, + Spec: seiv1alpha1.SeiNodeSpec{ChainID: staticTestChainID, Image: staticTestImage}, Status: seiv1alpha1.SeiNodeStatus{Phase: seiv1alpha1.PhasePending}, } configure(node) @@ -38,8 +43,8 @@ var staticModes = []struct { configure func(*seiv1alpha1.SeiNode) }{ {"full", func(n *seiv1alpha1.SeiNode) { n.Spec.FullNode = &seiv1alpha1.FullNodeSpec{} }}, - {"archive", func(n *seiv1alpha1.SeiNode) { n.Spec.Archive = &seiv1alpha1.ArchiveSpec{} }}, - {"validator", func(n *seiv1alpha1.SeiNode) { n.Spec.Validator = &seiv1alpha1.ValidatorSpec{} }}, + {overlayTestArchive, func(n *seiv1alpha1.SeiNode) { n.Spec.Archive = &seiv1alpha1.ArchiveSpec{} }}, + {overlayTestValidator, func(n *seiv1alpha1.SeiNode) { n.Spec.Validator = &seiv1alpha1.ValidatorSpec{} }}, } // TestStaticInitPlanCarriesNoConfigWriter is the assertion the whole feature @@ -90,7 +95,7 @@ func TestStaticNodeConfigRefusesBootstrap(t *testing.T) { node := withNodeConfig(pendingNode(func(n *seiv1alpha1.SeiNode) { n.Spec.FullNode = &seiv1alpha1.FullNodeSpec{ Snapshot: &seiv1alpha1.SnapshotSource{ - BootstrapImage: "sei:v1.0.0", + BootstrapImage: staticTestImage, S3: &seiv1alpha1.S3SnapshotSource{TargetHeight: 100}, }, } @@ -213,8 +218,8 @@ func TestStaticValidateRefusesRuntimeDiscoveredConfig(t *testing.T) { {"genesis ceremony", func(n *seiv1alpha1.SeiNode) { n.Spec.Validator = &seiv1alpha1.ValidatorSpec{ GenesisCeremony: &seiv1alpha1.GenesisCeremonyNodeConfig{ - ChainID: "atlantic-2", - StakingAmount: "1000000usei", + ChainID: staticTestChainID, + StakingAmount: testAccountBalance, AccountBalance: "2000000usei", }, } @@ -223,7 +228,7 @@ func TestStaticValidateRefusesRuntimeDiscoveredConfig(t *testing.T) { n.Spec.FullNode = &seiv1alpha1.FullNodeSpec{ Snapshot: &seiv1alpha1.SnapshotSource{StateSync: &seiv1alpha1.StateSyncSource{}}, } - }, "state-sync"}, + }, overlayTestStateSync}, {"autobahn consensus", func(n *seiv1alpha1.SeiNode) { n.Spec.FullNode = &seiv1alpha1.FullNodeSpec{} n.Spec.Consensus = &seiv1alpha1.ConsensusSpec{Engine: seiv1alpha1.ConsensusEngineAutobahn} diff --git a/internal/task/bootstrap_resources_test.go b/internal/task/bootstrap_resources_test.go index 758455aa..af78d023 100644 --- a/internal/task/bootstrap_resources_test.go +++ b/internal/task/bootstrap_resources_test.go @@ -408,14 +408,14 @@ func TestBuildBootstrapPodSpec_NeverMountsNodeConfig(t *testing.T) { snap := &seiv1alpha1.SnapshotSource{S3: &seiv1alpha1.S3SnapshotSource{TargetHeight: 100}} node := validatorNodeWithSecrets("", "", "") node.Spec.NodeConfig = &seiv1alpha1.NodeConfig{ - ConfigRef: seiv1alpha1.ConfigFileRef{Name: "rpc-config-v1"}, - AppRef: seiv1alpha1.ConfigFileRef{Name: "rpc-config-v1"}, + ConfigRef: seiv1alpha1.ConfigFileRef{Name: testNodeConfigMap}, + AppRef: seiv1alpha1.ConfigFileRef{Name: testNodeConfigMap}, } spec := buildBootstrapPodSpec(node, snap, platformtest.Config()) for _, v := range spec.Volumes { - g.Expect(v.ConfigMap == nil || v.ConfigMap.Name != "rpc-config-v1").To(BeTrue(), + g.Expect(v.ConfigMap == nil || v.ConfigMap.Name != testNodeConfigMap).To(BeTrue(), "bootstrap pod must not mount the operator config ConfigMap") } containers := append(append([]corev1.Container{}, spec.Containers...), spec.InitContainers...) diff --git a/internal/task/replace_pod.go b/internal/task/replace_pod.go index cfaa27ad..ef76982e 100644 --- a/internal/task/replace_pod.go +++ b/internal/task/replace_pod.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" 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/sidecarapi/tomlpatch" ) @@ -129,8 +130,8 @@ func (e *replacePodExecution) guardNodeConfig(ctx context.Context, node *seiv1al ref seiv1alpha1.ConfigFileRef file string }{ - {cfg.ConfigRef, "config.toml"}, - {cfg.AppRef, "app.toml"}, + {cfg.ConfigRef, noderesource.ConfigTomlKey}, + {cfg.AppRef, noderesource.AppTomlKey}, } for _, f := range files { cm := &corev1.ConfigMap{} diff --git a/internal/task/replace_pod_test.go b/internal/task/replace_pod_test.go index 0f7a85ba..7cf8e377 100644 --- a/internal/task/replace_pod_test.go +++ b/internal/task/replace_pod_test.go @@ -3,6 +3,7 @@ package task import ( "context" "encoding/json" + "fmt" "testing" . "github.com/onsi/gomega" @@ -17,9 +18,13 @@ import ( "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/noderesource" ) const ( + // testNodeConfigMap is the ConfigMap a spec.nodeConfig fixture references. + testNodeConfigMap = "rpc-config-v1" + stsUID = types.UID("sts-uid-1") testReplaceNs = "default" testReplaceSTS = "node-1" @@ -382,7 +387,7 @@ func TestReplacePod_GuardNodeConfig(t *testing.T) { configMap := func(data map[string]string) *corev1.ConfigMap { return &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: "rpc-config-v1", Namespace: testReplaceNs}, + ObjectMeta: metav1.ObjectMeta{Name: testNodeConfigMap, Namespace: testReplaceNs}, Data: data, } } @@ -394,24 +399,24 @@ func TestReplacePod_GuardNodeConfig(t *testing.T) { wantTerm bool }{ {"both keys valid", []client.Object{configMap(map[string]string{ - "config.toml": validTOML, "app.toml": validTOML})}, "", false}, + noderesource.ConfigTomlKey: validTOML, noderesource.AppTomlKey: validTOML})}, "", false}, {"configmap absent", nil, "not found", true}, {"app.toml missing", []client.Object{configMap(map[string]string{ - "config.toml": validTOML})}, `no "app.toml" key`, true}, + noderesource.ConfigTomlKey: validTOML})}, fmt.Sprintf("no %q key", noderesource.AppTomlKey), true}, {"config.toml missing", []client.Object{configMap(map[string]string{ - "app.toml": validTOML})}, `no "config.toml" key`, true}, + noderesource.AppTomlKey: validTOML})}, fmt.Sprintf("no %q key", noderesource.ConfigTomlKey), true}, {"app.toml empty", []client.Object{configMap(map[string]string{ - "config.toml": validTOML, "app.toml": " \n"})}, "is empty", true}, + noderesource.ConfigTomlKey: validTOML, "app.toml": " \n"})}, "is empty", true}, {"config.toml malformed", []client.Object{configMap(map[string]string{ - "config.toml": "moniker = \n[[[", "app.toml": validTOML})}, "not valid TOML", true}, + "config.toml": "moniker = \n[[[", noderesource.AppTomlKey: validTOML})}, "not valid TOML", true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) node := replacePodNode() node.Spec.NodeConfig = &seiv1alpha1.NodeConfig{ - ConfigRef: seiv1alpha1.ConfigFileRef{Name: "rpc-config-v1"}, - AppRef: seiv1alpha1.ConfigFileRef{Name: "rpc-config-v1"}, + ConfigRef: seiv1alpha1.ConfigFileRef{Name: testNodeConfigMap}, + AppRef: seiv1alpha1.ConfigFileRef{Name: testNodeConfigMap}, } cfg := replacePodCfg(t, node, tc.objs...) From 53aa87884d381d90911b9f9e8c44c7cfc27b0950 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 17 Sep 2026 09:53:26 +0200 Subject: [PATCH 3/4] fix(planner): a revert restores the controller-managed base config Clearing spec.nodeConfig rolled the pod to drop the mounts and then settled with no follow-up plan. A node created with nodeConfig never ran config-apply, so the volume held only what `seid init` wrote: no mode base, no freeze height, no snapshot-generation keys. After the revert seid read those defaults and the node reported success. The revert plan now appends config-apply and config-validate after observe-image. The replacement pod carries no mount by then, so the rename lands on a plain file, and the node leaves with the configuration its mode expects. mountedConfigWriterInPlan gains the matching exemption: on a revert, tasks after replace-pod run against a pod the template no longer gives the mounts. Nothing else is exempt. Reported by Cursor Bugbot on #565. Co-Authored-By: Claude Opus 5 (1M context) --- internal/planner/doc.go | 10 ++++-- internal/planner/static_config.go | 42 +++++++++++++++++++++----- internal/planner/static_config_test.go | 31 ++++++++++++++----- 3 files changed, 66 insertions(+), 17 deletions(-) diff --git a/internal/planner/doc.go b/internal/planner/doc.go index bec08105..64fd0f76 100644 --- a/internal/planner/doc.go +++ b/internal/planner/doc.go @@ -123,7 +123,11 @@ // arms, and mountedConfigWriterInPlan refuses any plan that still carries // one, whichever builder produced it. There is no per-pod exemption: a // bootstrap Job pod holds the same PVC, so staticConfigPlanner.Validate -// refuses that combination outright. config-validate is stripped too — it +// refuses that combination outright. The one exemption is a revert plan, +// which replaces the pod with one the template no longer gives the +// mounts and then writes the controller-managed base that was never +// written while the ConfigMaps were in place. config-validate is +// stripped too — it // reports on a file the operator owns, and sei-config's legacy reader // defaults a missing [sei] mode to full, so on a validator the verdict // can be confidently wrong. @@ -188,7 +192,9 @@ // task at all, so they set no ConfigValuesHash. Kubelet pins a subPath mount // at pod start, which is why pod replacement is the only way new config // reaches seid, and replace-pod parses both ConfigMaps before it deletes -// anything. +// anything. Clearing spec.nodeConfig builds the same roll with config-apply +// and config-validate appended after observe-image, so the node leaves with +// the base configuration its mode expects rather than `seid init` defaults. // // When no drift is detected for a Running node, no plan is built. The node // sits in steady state with no active plan. diff --git a/internal/planner/static_config.go b/internal/planner/static_config.go index 0e4f7bd1..d26d65e1 100644 --- a/internal/planner/static_config.go +++ b/internal/planner/static_config.go @@ -73,15 +73,26 @@ func withoutManagedConfigTasks(node *seiv1alpha1.SeiNode, prog []string) []strin // is safe. ResolvePlan refuses a plan it names, so the invariant is guarded // once no matter which builder produced the plan. // -// There is no per-pod exemption. A bootstrap Job pod carries no mount, but it -// holds the same PVC as the production pod, and a rename from there detaches -// the production pod's mount just as silently. staticConfigPlanner.Validate -// refuses the bootstrap combination for that reason, and this is the backstop. +// A revert plan is the one case where a writer belongs: it replaces the pod +// with one the template no longer gives the mounts, so everything after that +// replace-pod runs against a plain file. Nothing else is exempt. A bootstrap +// Job pod carries no mount of its own, but it holds the same PVC as the +// production pod, and a rename from there detaches the production pod's mount +// just as silently; staticConfigPlanner.Validate refuses that combination. func mountedConfigWriterInPlan(node *seiv1alpha1.SeiNode, plan *seiv1alpha1.TaskPlan) string { if !MountsNodeConfig(node) || plan == nil { return "" } - for _, t := range plan.Tasks { + lastMountedTask := len(plan.Tasks) + if revertingNodeConfig(node) { + for i, t := range plan.Tasks { + if t.Type == task.TaskTypeReplacePod { + lastMountedTask = i + break + } + } + } + for _, t := range plan.Tasks[:lastMountedTask] { if slices.Contains(mountedConfigWriters, t.Type) { return t.Type } @@ -89,6 +100,12 @@ func mountedConfigWriterInPlan(node *seiv1alpha1.SeiNode, plan *seiv1alpha1.Task return "" } +// revertingNodeConfig reports whether the operator has taken the ConfigMaps +// away from a node whose pod still mounts them. +func revertingNodeConfig(node *seiv1alpha1.SeiNode) bool { + return node.Spec.NodeConfig == nil && node.Status.CurrentNodeConfig != nil +} + // staticConfigPlanner plans a node whose config.toml and app.toml come from // operator-supplied ConfigMaps. // @@ -162,8 +179,9 @@ func (p *staticConfigPlanner) buildRunningPlan(node *seiv1alpha1.SeiNode) (*seiv return nil, nil } -// buildUpdatePlan rolls the pod and nothing else. Kubelet pins a subPath mount -// at pod start, so replacing the pod is what delivers new config. The +// buildUpdatePlan rolls the pod. Kubelet pins a subPath mount at pod start, so +// replacing the pod is what delivers new config. A revert also restores the +// controller-managed base afterwards; see below. The // key-validation gates lead, as they do in every mode's update plan, so a // missing Secret fails controller-side rather than as a kubelet mount error on // the recreated pod. @@ -183,8 +201,16 @@ func (p *staticConfigPlanner) buildUpdatePlan(node *seiv1alpha1.SeiNode) (*seiv1 task.TaskTypeApplyService, task.TaskTypeReplacePod, task.TaskTypeObserveImage, - TaskMarkReady, ) + if revertingNodeConfig(node) { + // The replacement pod has no mount, so the controller writes the base + // configuration it never wrote while the ConfigMaps were in place. A + // node created with nodeConfig has only what `seid init` left on the + // volume: no mode base, no freeze height, no snapshot-generation keys. + // Without this the node keeps those defaults and reports success. + prog = append(prog, TaskConfigApply, TaskConfigValidate) + } + prog = append(prog, TaskMarkReady) return assembleStaticUpdatePlan(node, prog) } diff --git a/internal/planner/static_config_test.go b/internal/planner/static_config_test.go index 3af22708..012e1d03 100644 --- a/internal/planner/static_config_test.go +++ b/internal/planner/static_config_test.go @@ -2,6 +2,7 @@ package planner import ( "context" + "slices" "testing" . "github.com/onsi/gomega" @@ -150,12 +151,20 @@ func TestStaticRunningPlanRollsThePod(t *testing.T) { g.Expect(types).To(ContainElement(task.TaskTypeObserveImage)) g.Expect(types).To(ContainElement(TaskMarkReady)) - // The roll carries no config task at all: the writers detach the - // mount, and config-validate reports on a file the operator owns. - for _, writer := range mountedConfigWriters { - g.Expect(types).NotTo(ContainElement(writer)) + if tc.spec == "" { + // A revert restores the controller-managed base, after the + // roll has replaced the pod with one that has no mount. + g.Expect(slices.Index(types, TaskConfigApply)).To( + BeNumerically(">", slices.Index(types, task.TaskTypeReplacePod))) + } else { + // Adopting or republishing carries no config task at all: the + // writers detach the mount, and config-validate reports on a + // file the operator owns. + for _, writer := range mountedConfigWriters { + g.Expect(types).NotTo(ContainElement(writer)) + } + g.Expect(types).NotTo(ContainElement(TaskConfigValidate)) } - g.Expect(types).NotTo(ContainElement(TaskConfigValidate)) cond := meta.FindStatusCondition(node.Status.Conditions, seiv1alpha1.ConditionNodeUpdateInProgress) g.Expect(cond).NotTo(BeNil()) @@ -332,10 +341,18 @@ func TestNodeConfigRevertRollsBeforeAnyConfigWrite(t *testing.T) { g.Expect(node.Status.Plan).NotTo(BeNil()) types := planTaskTypes(node.Status.Plan) + replace := slices.Index(types, task.TaskTypeReplacePod) + g.Expect(replace).To(BeNumerically(">=", 0)) + + // Nothing writes config while the outgoing pod still has the mount. for _, writer := range mountedConfigWriters { - g.Expect(types).NotTo(ContainElement(writer)) + g.Expect(types[:replace]).NotTo(ContainElement(writer)) } - g.Expect(types).To(ContainElement(task.TaskTypeReplacePod)) + // The replacement pod has none, so the base the controller never wrote is + // written there. A node created with nodeConfig otherwise keeps the files + // `seid init` left on the volume. + g.Expect(slices.Index(types, TaskConfigApply)).To(BeNumerically(">", replace)) + g.Expect(types[len(types)-1]).To(Equal(TaskMarkReady)) // Once the roll drops the mount, the node returns to the mode planner and // settles: no drift, no plan. From 707e8d19f59eb636f0759c15b49339c714dba75e Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 17 Sep 2026 10:10:21 +0200 Subject: [PATCH 4/4] fix(planner): the revert stamp waits for the restore, and carries the overlay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings on the revert path. observe-image cleared status.currentNodeConfig before config-apply ran. A restore that failed after that point dropped the node off staticConfigPlanner with no drift left to rebuild the plan, settling it on `seid init` defaults — the outcome the revert exists to prevent. observe-image now stamps only a gained mount; the plan carries ClearsNodeConfig and the executor applies it on completion, mirroring how ConfigValuesHash is stamped. The revert plan assembled through assembleStaticUpdatePlan, which never calls withConfigValues, so a revert that also set configValues neither applied them nor established the observed baseline. It now finishes through withConfigValues, whose config-validate anchor the progression carries. ConfigFileRef.Name carried only length bounds, so a value that is not a DNS subdomain passed admission and then rendered a StatefulSet the API server rejects. It now pins the pattern its sibling object-name fields use. Also corrects a doc comment that named withoutMountedConfigWriters, which no longer exists, and claimed coverage of progressions the filter never sees. Reported by seidroid and Cursor Bugbot on #565. Co-Authored-By: Claude Opus 5 (1M context) --- api/v1alpha1/seinode_types.go | 9 +++++ config/crd/sei.io_seinetworks.yaml | 8 ++++ config/crd/sei.io_seinodes.yaml | 12 ++++++ config/crd/sei.io_seinodetaskworkflows.yaml | 8 ++++ internal/planner/executor.go | 9 ++++- internal/planner/static_config.go | 41 ++++++++++++++------- internal/planner/static_config_test.go | 23 ++++++++++++ internal/task/observe_image.go | 7 +++- manifests/sei.io_seinetworks.yaml | 8 ++++ manifests/sei.io_seinodes.yaml | 12 ++++++ manifests/sei.io_seinodetaskworkflows.yaml | 8 ++++ 11 files changed, 129 insertions(+), 16 deletions(-) diff --git a/api/v1alpha1/seinode_types.go b/api/v1alpha1/seinode_types.go index 41d8e384..07f587b6 100644 --- a/api/v1alpha1/seinode_types.go +++ b/api/v1alpha1/seinode_types.go @@ -413,6 +413,7 @@ type ConfigFileRef struct { // ContainerCreating and `kubectl describe pod` names the missing key. // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 + // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$` Name string `json:"name"` } @@ -555,6 +556,14 @@ type TaskPlan struct { // +optional ConfigValuesHash string `json:"configValuesHash,omitempty"` + // ClearsNodeConfig marks a plan that takes the operator's ConfigMaps away + // from a node. On successful completion Status.CurrentNodeConfig is + // cleared. It is not cleared earlier: the plan writes the + // controller-managed base after the pod is replaced, and until that write + // lands the stamp is what keeps the node on the planner that will retry. + // +optional + ClearsNodeConfig bool `json:"clearsNodeConfig,omitempty"` + // FailedPhase is the SeiNodePhase the executor sets on the owning // resource when the plan fails terminally. When empty, the executor // does not perform a phase transition on failure. diff --git a/config/crd/sei.io_seinetworks.yaml b/config/crd/sei.io_seinetworks.yaml index c4782b29..6d9fbb48 100644 --- a/config/crd/sei.io_seinetworks.yaml +++ b/config/crd/sei.io_seinetworks.yaml @@ -1134,6 +1134,14 @@ spec: Plan tracks the active network-level task plan (genesis assembly, deployment, etc.). Nil when no plan is in progress. properties: + clearsNodeConfig: + description: |- + ClearsNodeConfig marks a plan that takes the operator's ConfigMaps away + from a node. On successful completion Status.CurrentNodeConfig is + cleared. It is not cleared earlier: the plan writes the + controller-managed base after the pod is replaced, and until that write + lands the stamp is what keeps the node on the planner that will retry. + type: boolean configValuesHash: description: |- ConfigValuesHash identifies the configValues captured by this materialization diff --git a/config/crd/sei.io_seinodes.yaml b/config/crd/sei.io_seinodes.yaml index 707d08a2..9fdd8eba 100644 --- a/config/crd/sei.io_seinodes.yaml +++ b/config/crd/sei.io_seinodes.yaml @@ -608,6 +608,7 @@ spec: ContainerCreating and `kubectl describe pod` names the missing key. maxLength: 253 minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string required: - name @@ -623,6 +624,7 @@ spec: ContainerCreating and `kubectl describe pod` names the missing key. maxLength: 253 minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string required: - name @@ -1606,6 +1608,7 @@ spec: ContainerCreating and `kubectl describe pod` names the missing key. maxLength: 253 minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string required: - name @@ -1621,6 +1624,7 @@ spec: ContainerCreating and `kubectl describe pod` names the missing key. maxLength: 253 minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string required: - name @@ -1704,6 +1708,14 @@ spec: Plan tracks the active task sequence for this node. A planner generates the plan based on the node's current state and conditions. properties: + clearsNodeConfig: + description: |- + ClearsNodeConfig marks a plan that takes the operator's ConfigMaps away + from a node. On successful completion Status.CurrentNodeConfig is + cleared. It is not cleared earlier: the plan writes the + controller-managed base after the pod is replaced, and until that write + lands the stamp is what keeps the node on the planner that will retry. + type: boolean configValuesHash: description: |- ConfigValuesHash identifies the configValues captured by this materialization diff --git a/config/crd/sei.io_seinodetaskworkflows.yaml b/config/crd/sei.io_seinodetaskworkflows.yaml index 97a90790..5ced823f 100644 --- a/config/crd/sei.io_seinodetaskworkflows.yaml +++ b/config/crd/sei.io_seinodetaskworkflows.yaml @@ -274,6 +274,14 @@ spec: controllers persist, driven by the generic plan executor. TargetPhase and FailedPhase are always empty: a workflow never drives a node phase. properties: + clearsNodeConfig: + description: |- + ClearsNodeConfig marks a plan that takes the operator's ConfigMaps away + from a node. On successful completion Status.CurrentNodeConfig is + cleared. It is not cleared earlier: the plan writes the + controller-managed base after the pod is replaced, and until that write + lands the stamp is what keeps the node on the planner that will retry. + type: boolean configValuesHash: description: |- ConfigValuesHash identifies the configValues captured by this materialization diff --git a/internal/planner/executor.go b/internal/planner/executor.go index fafaf5a0..96922779 100644 --- a/internal/planner/executor.go +++ b/internal/planner/executor.go @@ -112,8 +112,13 @@ func executePlan( // phase. The planner handles cleanup (nilling the plan, clearing // conditions) when it observes the terminal plan on the next reconcile. plan.Phase = seiv1alpha1.TaskPlanComplete - if node, ok := obj.(*seiv1alpha1.SeiNode); ok && plan.ConfigValuesHash != "" { - node.Status.CurrentConfigValuesHash = plan.ConfigValuesHash + if node, ok := obj.(*seiv1alpha1.SeiNode); ok { + if plan.ConfigValuesHash != "" { + node.Status.CurrentConfigValuesHash = plan.ConfigValuesHash + } + if plan.ClearsNodeConfig { + node.Status.CurrentNodeConfig = nil + } } setTargetPhase(obj, plan.TargetPhase) planActiveCount.Add(ctx, -1, diff --git a/internal/planner/static_config.go b/internal/planner/static_config.go index d26d65e1..62219d66 100644 --- a/internal/planner/static_config.go +++ b/internal/planner/static_config.go @@ -110,9 +110,10 @@ func revertingNodeConfig(node *seiv1alpha1.SeiNode) bool { // operator-supplied ConfigMaps. // // It wraps the node's mode planner rather than replacing it, so the mode keeps -// its own Validate and its own init and bootstrap plans. Those plans are -// already safe: both production-pod progressions run through -// withoutMountedConfigWriters. What the wrapper owns is the Running arm, which +// its own Validate and its own init plan, which withoutManagedConfigTasks +// strips for it. The bootstrap and genesis-ceremony progressions are not +// filtered at all — Validate refuses both shapes, and mountedConfigWriterInPlan +// refuses any plan that slips through. What the wrapper owns is the Running arm, which // the mode planners route through assembleUpdatePlan — an assembler that // force-inserts config-apply whenever the configValues baseline is unobserved, // which on one of these nodes is always. @@ -202,16 +203,30 @@ func (p *staticConfigPlanner) buildUpdatePlan(node *seiv1alpha1.SeiNode) (*seiv1 task.TaskTypeReplacePod, task.TaskTypeObserveImage, ) - if revertingNodeConfig(node) { - // The replacement pod has no mount, so the controller writes the base - // configuration it never wrote while the ConfigMaps were in place. A - // node created with nodeConfig has only what `seid init` left on the - // volume: no mode base, no freeze height, no snapshot-generation keys. - // Without this the node keeps those defaults and reports success. - prog = append(prog, TaskConfigApply, TaskConfigValidate) - } - prog = append(prog, TaskMarkReady) - return assembleStaticUpdatePlan(node, prog) + if !revertingNodeConfig(node) { + prog = append(prog, TaskMarkReady) + return assembleStaticUpdatePlan(node, prog) + } + + // The replacement pod has no mount, so the controller writes the base + // configuration it never wrote while the ConfigMaps were in place. A node + // created with nodeConfig has only what `seid init` left on the volume: no + // mode base, no freeze height, no snapshot-generation keys. Without this + // the node keeps those defaults and reports success. + // + // seid has not started yet. Its container blocks on the sidecar's + // /v0/healthz, which reports ready only after mark-ready, so the write + // lands before seid reads the file and no restart is needed. + prog = append(prog, TaskConfigApply, TaskConfigValidate, TaskMarkReady) + plan, err := assembleStaticUpdatePlan(node, prog) + if err != nil { + return nil, err + } + plan.ClearsNodeConfig = true + // The node is back on the controller-managed path, so it takes the overlay + // and the observed baseline with it. withConfigValues splices the patch + // before config-validate, which this progression carries. + return withConfigValues(plan, node) } // assembleStaticUpdatePlan composes the progression into a TaskPlan. It is the diff --git a/internal/planner/static_config_test.go b/internal/planner/static_config_test.go index 012e1d03..e17fe960 100644 --- a/internal/planner/static_config_test.go +++ b/internal/planner/static_config_test.go @@ -352,8 +352,31 @@ func TestNodeConfigRevertRollsBeforeAnyConfigWrite(t *testing.T) { // written there. A node created with nodeConfig otherwise keeps the files // `seid init` left on the volume. g.Expect(slices.Index(types, TaskConfigApply)).To(BeNumerically(">", replace)) + + // mark-ready is last, and seid's container blocks on the sidecar's + // /v0/healthz until it runs, so the write lands before seid reads the file. + // That is why the plan needs no restart-seid. g.Expect(types[len(types)-1]).To(Equal(TaskMarkReady)) + // The stamp survives the plan. observe-image runs before config-apply, so + // clearing it there would drop a node whose restore failed off this planner + // with no drift left to rebuild the plan. + g.Expect(slices.Index(types, task.TaskTypeObserveImage)).To( + BeNumerically("<", slices.Index(types, TaskConfigApply))) + g.Expect(node.Status.Plan.ClearsNodeConfig).To(BeTrue()) + g.Expect(node.Status.CurrentNodeConfig).NotTo(BeNil()) + + // The node rejoins the controller-managed path, so it takes the overlay and + // the observed baseline with it. + g.Expect(node.Status.Plan.ConfigValuesHash).NotTo(BeEmpty()) + + // A restore that fails leaves the stamp in place, so the next reconcile + // resolves this planner again and rebuilds the revert. + node.Status.Plan = nil + g.Expect(MountsNodeConfig(node)).To(BeTrue()) + g.Expect((&NodeResolver{}).ResolvePlan(context.Background(), node)).To(Succeed()) + g.Expect(planTaskTypes(node.Status.Plan)).To(ContainElement(TaskConfigApply)) + // Once the roll drops the mount, the node returns to the mode planner and // settles: no drift, no plan. node.Status.CurrentNodeConfig = nil diff --git a/internal/task/observe_image.go b/internal/task/observe_image.go index c07084bb..ccca5f84 100644 --- a/internal/task/observe_image.go +++ b/internal/task/observe_image.go @@ -79,7 +79,12 @@ func (e *observeImageExecution) Execute(ctx context.Context) error { node.Status.CurrentImage = node.Spec.Image node.Status.CurrentSidecarImage = EffectiveSidecarImage(node, e.cfg.Platform) node.Status.CurrentNodeIsolation = noderesource.EffectiveNodeIsolation(node) - node.Status.CurrentNodeConfig = node.Spec.NodeConfig.DeepCopy() + // Only a gained mount is stamped here. A revert clears the stamp on plan + // completion instead, because the plan still has the base config to write + // and the stamp is what keeps the node on the planner that writes it. + if node.Spec.NodeConfig != nil { + node.Status.CurrentNodeConfig = node.Spec.NodeConfig.DeepCopy() + } e.complete() return nil } diff --git a/manifests/sei.io_seinetworks.yaml b/manifests/sei.io_seinetworks.yaml index c4782b29..6d9fbb48 100644 --- a/manifests/sei.io_seinetworks.yaml +++ b/manifests/sei.io_seinetworks.yaml @@ -1134,6 +1134,14 @@ spec: Plan tracks the active network-level task plan (genesis assembly, deployment, etc.). Nil when no plan is in progress. properties: + clearsNodeConfig: + description: |- + ClearsNodeConfig marks a plan that takes the operator's ConfigMaps away + from a node. On successful completion Status.CurrentNodeConfig is + cleared. It is not cleared earlier: the plan writes the + controller-managed base after the pod is replaced, and until that write + lands the stamp is what keeps the node on the planner that will retry. + type: boolean configValuesHash: description: |- ConfigValuesHash identifies the configValues captured by this materialization diff --git a/manifests/sei.io_seinodes.yaml b/manifests/sei.io_seinodes.yaml index 707d08a2..9fdd8eba 100644 --- a/manifests/sei.io_seinodes.yaml +++ b/manifests/sei.io_seinodes.yaml @@ -608,6 +608,7 @@ spec: ContainerCreating and `kubectl describe pod` names the missing key. maxLength: 253 minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string required: - name @@ -623,6 +624,7 @@ spec: ContainerCreating and `kubectl describe pod` names the missing key. maxLength: 253 minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string required: - name @@ -1606,6 +1608,7 @@ spec: ContainerCreating and `kubectl describe pod` names the missing key. maxLength: 253 minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string required: - name @@ -1621,6 +1624,7 @@ spec: ContainerCreating and `kubectl describe pod` names the missing key. maxLength: 253 minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string required: - name @@ -1704,6 +1708,14 @@ spec: Plan tracks the active task sequence for this node. A planner generates the plan based on the node's current state and conditions. properties: + clearsNodeConfig: + description: |- + ClearsNodeConfig marks a plan that takes the operator's ConfigMaps away + from a node. On successful completion Status.CurrentNodeConfig is + cleared. It is not cleared earlier: the plan writes the + controller-managed base after the pod is replaced, and until that write + lands the stamp is what keeps the node on the planner that will retry. + type: boolean configValuesHash: description: |- ConfigValuesHash identifies the configValues captured by this materialization diff --git a/manifests/sei.io_seinodetaskworkflows.yaml b/manifests/sei.io_seinodetaskworkflows.yaml index 97a90790..5ced823f 100644 --- a/manifests/sei.io_seinodetaskworkflows.yaml +++ b/manifests/sei.io_seinodetaskworkflows.yaml @@ -274,6 +274,14 @@ spec: controllers persist, driven by the generic plan executor. TargetPhase and FailedPhase are always empty: a workflow never drives a node phase. properties: + clearsNodeConfig: + description: |- + ClearsNodeConfig marks a plan that takes the operator's ConfigMaps away + from a node. On successful completion Status.CurrentNodeConfig is + cleared. It is not cleared earlier: the plan writes the + controller-managed base after the pod is replaced, and until that write + lands the stamp is what keeps the node on the planner that will retry. + type: boolean configValuesHash: description: |- ConfigValuesHash identifies the configValues captured by this materialization