From 089a4dfdf84b960f3c6b3b4ab1ee945861ab80df Mon Sep 17 00:00:00 2001 From: bdchatham Date: Tue, 14 Jul 2026 23:09:23 -0700 Subject: [PATCH 1/2] feat(nodetask): add ConfigPatch kind + CON-368 executor-mixed nightly Adds a standalone ConfigPatch SeiNodeTask kind and a nightly integration test that uses it to gate CON-368 executor coexistence. ConfigPatch is an operator-facing day-2 config patch: the SDK surface is just ConfigPatch{Overrides map[string]string}. Internally it always maps to a config-apply incremental task, so sei-config's own resolver (ResolveIncrementalIntent -> registry-aware ApplyOverrides -> legacy render) produces the on-disk config -- correct-by-construction for every registered key, no hand-maintained render table. A fail-closed allowlist (giga_executor.*, chain.occ_enabled, mempool.*) is enforced purely as safety policy at three points (CRD CEL, controller, SDK); it excludes keys the controller re-asserts on its own plans (persistent_peers, etc.). TestNightlyGigaExecutorMixed provisions 4 giga-executor validators + one RPC follower flipped to the v2 executor via ConfigPatch + a standard StateSync re-bootstrap, runs the release-test suite against it, and asserts it stays caught up and advancing -- i.e. a v2-executor node stays consensus-compatible with a giga-executor validator set. A regression that reintroduced the executor divergence would halt the follower and fail the advance assertion. Storage layout is out of scope so the executor is the only variable. Co-Authored-By: Claude Opus 4.8 (1M context) --- api/v1alpha1/seinodetask_types.go | 66 +++- api/v1alpha1/zz_generated.deepcopy.go | 27 ++ config/crd/sei.io_seinodetasks.yaml | 27 +- internal/controller/nodetask/controller.go | 6 + .../nodetask/envtest/cel_validation_test.go | 48 +++ internal/task/seinodetask_params.go | 55 +++ internal/task/seinodetask_params_test.go | 87 ++++- manifests/sei.io_seinodetasks.yaml | 27 +- sdk/sei/provider/k8s/render.go | 4 + sdk/sei/task.go | 51 +++ sdk/sei/task_test.go | 35 ++ test/integration/giga_executor_mixed_test.go | 318 ++++++++++++++++++ 12 files changed, 743 insertions(+), 8 deletions(-) create mode 100644 test/integration/giga_executor_mixed_test.go diff --git a/api/v1alpha1/seinodetask_types.go b/api/v1alpha1/seinodetask_types.go index 8e7df226..f7005edf 100644 --- a/api/v1alpha1/seinodetask_types.go +++ b/api/v1alpha1/seinodetask_types.go @@ -7,7 +7,7 @@ import ( // SeiNodeTaskKind discriminates the SeiNodeTask spec union. Exactly one of // the matching payload sub-structs in SeiNodeTaskSpec must be set. -// +kubebuilder:validation:Enum=GovSoftwareUpgrade;GovVote;GovParamChange;AwaitCondition;UpdateNodeImage;AwaitNodesAtHeight;RestartSeid;MarkReady +// +kubebuilder:validation:Enum=GovSoftwareUpgrade;GovVote;GovParamChange;AwaitCondition;UpdateNodeImage;AwaitNodesAtHeight;RestartSeid;MarkReady;ConfigPatch type SeiNodeTaskKind string const ( @@ -68,6 +68,27 @@ const ( // accepts the request, a beat before /v0/healthz serves 200. Gate on the node // actually serving with a following AwaitCondition/AwaitNodesAtHeight step. SeiNodeTaskKindMarkReady SeiNodeTaskKind = "MarkReady" + + // SeiNodeTaskKindConfigPatch applies an incremental config patch to the target + // node's on-disk config via the sidecar's config-apply task. The payload uses + // the SAME operator-facing dotted sei-config key schema as SeiNodeSpec.Overrides + // (e.g. "giga_executor.enabled"); the sidecar's sei-config resolver reads the + // current on-disk config, applies the overrides (registry-aware, covering every + // registered key), and re-renders config.toml / app.toml — so rendering is + // correct-by-construction, not a controller-side render table. + // + // PATCHING ONLY — this kind does NOT restart seid or reload config. seid + // re-reads config.toml/app.toml only on (re)start, so a caller composes a + // following RestartSeid (in-place seid bounce) or StateSync/re-bootstrap step + // to make the change take effect. Completion = the sidecar acks the patch + // written to disk (idempotent: re-running rewrites the same keys). + // + // Scope is a fail-closed ALLOWLIST (see ConfigPatchPayload.Overrides): only + // runtime-safe toggles are permitted, and keys the controller re-asserts on + // its own update plans (p2p peers/external-address, genesis/state-sync-managed + // keys) are rejected — a patch to those would be silently clobbered by the + // next SeiNode update plan. + SeiNodeTaskKindConfigPatch SeiNodeTaskKind = "ConfigPatch" ) // SeiNodeTaskPhase is the high-level lifecycle state of a SeiNodeTask. @@ -112,7 +133,7 @@ const ( // Field names locked at v1alpha1 — see https://github.com/sei-protocol/bdchatham-designs/blob/main/designs/seinode-task/seinode-task-lld.md // (PR sei-protocol/sei-k8s-controller#277). // -// +kubebuilder:validation:XValidation:rule="(has(self.govSoftwareUpgrade) ? 1 : 0) + (has(self.govVote) ? 1 : 0) + (has(self.govParamChange) ? 1 : 0) + (has(self.awaitCondition) ? 1 : 0) + (has(self.updateNodeImage) ? 1 : 0) + (has(self.awaitNodesAtHeight) ? 1 : 0) + (has(self.restartSeid) ? 1 : 0) + (has(self.markReady) ? 1 : 0) == 1",message="exactly one of govSoftwareUpgrade, govVote, govParamChange, awaitCondition, updateNodeImage, awaitNodesAtHeight, restartSeid, or markReady must be set" +// +kubebuilder:validation:XValidation:rule="(has(self.govSoftwareUpgrade) ? 1 : 0) + (has(self.govVote) ? 1 : 0) + (has(self.govParamChange) ? 1 : 0) + (has(self.awaitCondition) ? 1 : 0) + (has(self.updateNodeImage) ? 1 : 0) + (has(self.awaitNodesAtHeight) ? 1 : 0) + (has(self.restartSeid) ? 1 : 0) + (has(self.markReady) ? 1 : 0) + (has(self.configPatch) ? 1 : 0) == 1",message="exactly one of govSoftwareUpgrade, govVote, govParamChange, awaitCondition, updateNodeImage, awaitNodesAtHeight, restartSeid, markReady, or configPatch must be set" // +kubebuilder:validation:XValidation:rule="self.kind != 'GovSoftwareUpgrade' || has(self.govSoftwareUpgrade)",message="spec.govSoftwareUpgrade is required when kind=GovSoftwareUpgrade" // +kubebuilder:validation:XValidation:rule="self.kind != 'GovVote' || has(self.govVote)",message="spec.govVote is required when kind=GovVote" // +kubebuilder:validation:XValidation:rule="self.kind != 'GovParamChange' || has(self.govParamChange)",message="spec.govParamChange is required when kind=GovParamChange" @@ -121,6 +142,8 @@ const ( // +kubebuilder:validation:XValidation:rule="self.kind != 'AwaitNodesAtHeight' || has(self.awaitNodesAtHeight)",message="spec.awaitNodesAtHeight is required when kind=AwaitNodesAtHeight" // +kubebuilder:validation:XValidation:rule="self.kind != 'RestartSeid' || has(self.restartSeid)",message="spec.restartSeid is required when kind=RestartSeid" // +kubebuilder:validation:XValidation:rule="self.kind != 'MarkReady' || has(self.markReady)",message="spec.markReady is required when kind=MarkReady" +// +kubebuilder:validation:XValidation:rule="self.kind != 'ConfigPatch' || has(self.configPatch)",message="spec.configPatch is required when kind=ConfigPatch" +// +kubebuilder:validation:XValidation:rule="!has(self.configPatch) || self.configPatch.overrides.all(k, k == 'chain.occ_enabled' || k.startsWith('giga_executor.') || k.startsWith('mempool.'))",message="spec.configPatch.overrides only admits allowlisted runtime-safe keys (chain.occ_enabled, giga_executor.*, mempool.*)" // +kubebuilder:validation:XValidation:rule="self.kind == oldSelf.kind",message="spec.kind is immutable" type SeiNodeTaskSpec struct { // Kind selects the task implementation. Immutable after creation. @@ -175,6 +198,10 @@ type SeiNodeTaskSpec struct { // MarkReady is the payload for kind=MarkReady. // +optional MarkReady *MarkReadyPayload `json:"markReady,omitempty"` + + // ConfigPatch is the payload for kind=ConfigPatch. + // +optional + ConfigPatch *ConfigPatchPayload `json:"configPatch,omitempty"` } // SeiNodeTaskTarget identifies the single SeiNode this task operates on. @@ -445,6 +472,41 @@ type RestartSeidPayload struct{} // SeiNodeTaskKindMarkReady doc comment for the use case. type MarkReadyPayload struct{} +// ConfigPatchPayload is the payload for kind=ConfigPatch — a set of dotted +// sei-config key overrides to merge-patch into the target node's on-disk config. +// +// VALUE CONTRACT — THE ALLOWLIST (fail-closed; keep in sync across the three +// enforcement points below). Only these runtime-safe toggle families are +// admitted; everything else is rejected: +// +// chain.occ_enabled — OCC concurrency toggle +// giga_executor.* — v2 (giga) executor toggles, incl. giga_executor.enabled +// mempool.* — mempool sizing/TTL toggles +// +// Deliberately EXCLUDED (the controller re-asserts these on its own SeiNode +// update plans, so a patch here would be silently clobbered): p2p peers +// (network.p2p.persistent_peers), p2p.external-address, and genesis / +// state-sync-managed keys. +// +// The allowlist is pure allow/deny POLICY on key prefixes, not a renderer: +// sei-config resolves and renders every registered key on the node, but only +// these families are safe to patch out-of-band. Enforcement is layered: +// (1) a coarse family-level CEL rule on SeiNodeTaskSpec rejects out-of-family +// keys at admission; (2) the controller re-enforces the same allowlist policy +// when it synthesizes the config-apply task, and the sidecar's sei-config +// resolver additionally rejects unknown fields and type-mismatched values; +// (3) the SDK re-checks family membership client-side for a fast clean error. +// Widening this allowlist is a value-contract change — review the three points +// together. +type ConfigPatchPayload struct { + // Overrides maps dotted sei-config keys (the same operator-facing schema as + // SeiNodeSpec.Overrides, e.g. "giga_executor.enabled": "false") to their + // desired string values. Values are coerced to each field's registered type + // (bool/int/…) when rendered to TOML. At least one entry is required. + // +kubebuilder:validation:MinProperties=1 + Overrides map[string]string `json:"overrides"` +} + // --------------------------------------------------------------------------- // Status // --------------------------------------------------------------------------- diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index a8cbb646..f53df77e 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -147,6 +147,28 @@ func (in *ConfigMigration) DeepCopy() *ConfigMigration { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ConfigPatchPayload) DeepCopyInto(out *ConfigPatchPayload) { + *out = *in + if in.Overrides != nil { + in, out := &in.Overrides, &out.Overrides + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigPatchPayload. +func (in *ConfigPatchPayload) DeepCopy() *ConfigPatchPayload { + if in == nil { + return nil + } + out := new(ConfigPatchPayload) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DataVolumeImport) DeepCopyInto(out *DataVolumeImport) { *out = *in @@ -1347,6 +1369,11 @@ func (in *SeiNodeTaskSpec) DeepCopyInto(out *SeiNodeTaskSpec) { *out = new(MarkReadyPayload) **out = **in } + if in.ConfigPatch != nil { + in, out := &in.ConfigPatch, &out.ConfigPatch + *out = new(ConfigPatchPayload) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SeiNodeTaskSpec. diff --git a/config/crd/sei.io_seinodetasks.yaml b/config/crd/sei.io_seinodetasks.yaml index 09f795cb..bbaa3409 100644 --- a/config/crd/sei.io_seinodetasks.yaml +++ b/config/crd/sei.io_seinodetasks.yaml @@ -106,6 +106,22 @@ spec: required: - targetHeight type: object + configPatch: + description: ConfigPatch is the payload for kind=ConfigPatch. + properties: + overrides: + additionalProperties: + type: string + description: |- + Overrides maps dotted sei-config keys (the same operator-facing schema as + SeiNodeSpec.Overrides, e.g. "giga_executor.enabled": "false") to their + desired string values. Values are coerced to each field's registered type + (bool/int/…) when rendered to TOML. At least one entry is required. + minProperties: 1 + type: object + required: + - overrides + type: object govParamChange: description: GovParamChange is the payload for kind=GovParamChange. properties: @@ -338,6 +354,7 @@ spec: - AwaitNodesAtHeight - RestartSeid - MarkReady + - ConfigPatch type: string markReady: description: MarkReady is the payload for kind=MarkReady. @@ -416,12 +433,12 @@ spec: x-kubernetes-validations: - message: exactly one of govSoftwareUpgrade, govVote, govParamChange, awaitCondition, updateNodeImage, awaitNodesAtHeight, restartSeid, - or markReady must be set + markReady, or configPatch must be set rule: '(has(self.govSoftwareUpgrade) ? 1 : 0) + (has(self.govVote) ? 1 : 0) + (has(self.govParamChange) ? 1 : 0) + (has(self.awaitCondition) ? 1 : 0) + (has(self.updateNodeImage) ? 1 : 0) + (has(self.awaitNodesAtHeight) ? 1 : 0) + (has(self.restartSeid) ? 1 : 0) + (has(self.markReady) - ? 1 : 0) == 1' + ? 1 : 0) + (has(self.configPatch) ? 1 : 0) == 1' - message: spec.govSoftwareUpgrade is required when kind=GovSoftwareUpgrade rule: self.kind != 'GovSoftwareUpgrade' || has(self.govSoftwareUpgrade) - message: spec.govVote is required when kind=GovVote @@ -438,6 +455,12 @@ spec: rule: self.kind != 'RestartSeid' || has(self.restartSeid) - message: spec.markReady is required when kind=MarkReady rule: self.kind != 'MarkReady' || has(self.markReady) + - message: spec.configPatch is required when kind=ConfigPatch + rule: self.kind != 'ConfigPatch' || has(self.configPatch) + - message: spec.configPatch.overrides only admits allowlisted runtime-safe + keys (chain.occ_enabled, giga_executor.*, mempool.*) + rule: '!has(self.configPatch) || self.configPatch.overrides.all(k, k + == ''chain.occ_enabled'' || k.startsWith(''giga_executor.'') || k.startsWith(''mempool.''))' - message: spec.kind is immutable rule: self.kind == oldSelf.kind status: diff --git a/internal/controller/nodetask/controller.go b/internal/controller/nodetask/controller.go index ce828fc1..9035ceeb 100644 --- a/internal/controller/nodetask/controller.go +++ b/internal/controller/nodetask/controller.go @@ -64,6 +64,10 @@ const ( // sidecar SIGTERMs seid (up to ~90s graceful), then polls seid's RPC back up. defaultRestartSeidTimeout = 10 * time.Minute defaultMarkReadyTimeout = 2 * time.Minute + // defaultConfigPatchTimeout bounds a config-patch: a quick TOML merge-and- + // write on the sidecar. A generous 2m guards against a wedged sidecar + // pinning the task unbounded, while leaving ample room for the write + poll. + defaultConfigPatchTimeout = 2 * time.Minute ) // resultRequeueImmediate mirrors planner.ResultRequeueImmediate without @@ -376,6 +380,8 @@ func effectiveTimeout(cr *seiv1alpha1.SeiNodeTask) time.Duration { return defaultRestartSeidTimeout case seiv1alpha1.SeiNodeTaskKindMarkReady: return defaultMarkReadyTimeout + case seiv1alpha1.SeiNodeTaskKindConfigPatch: + return defaultConfigPatchTimeout default: return 0 } diff --git a/internal/controller/nodetask/envtest/cel_validation_test.go b/internal/controller/nodetask/envtest/cel_validation_test.go index 8cd8f2c4..0d6b8294 100644 --- a/internal/controller/nodetask/envtest/cel_validation_test.go +++ b/internal/controller/nodetask/envtest/cel_validation_test.go @@ -102,6 +102,54 @@ func TestCEL_MarkReady_MultiplePayloads_Rejected(t *testing.T) { g.Expect(err.Error()).To(ContainSubstring("exactly one")) } +// ConfigPatch with an allowlisted key is accepted. +func TestCEL_ConfigPatch_Accepted(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "configpatch-ok", seiv1alpha1.SeiNodeTaskKindConfigPatch) + snt.Spec.ConfigPatch = &seiv1alpha1.ConfigPatchPayload{ + Overrides: map[string]string{"giga_executor.enabled": "false"}, + } + g.Expect(testCli.Create(testCtx, snt)).To(Succeed()) +} + +// ConfigPatch with an empty overrides map is rejected (MinProperties=1). +func TestCEL_ConfigPatch_EmptyOverrides_Rejected(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "configpatch-empty", seiv1alpha1.SeiNodeTaskKindConfigPatch) + snt.Spec.ConfigPatch = &seiv1alpha1.ConfigPatchPayload{Overrides: map[string]string{}} + err := testCli.Create(testCtx, snt) + g.Expect(err).To(HaveOccurred()) +} + +// ConfigPatch with an out-of-allowlist key is rejected by the family-level CEL +// rule (before the controller's authoritative renderer ever runs). +func TestCEL_ConfigPatch_DisallowedKey_Rejected(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "configpatch-disallowed", seiv1alpha1.SeiNodeTaskKindConfigPatch) + snt.Spec.ConfigPatch = &seiv1alpha1.ConfigPatchPayload{ + Overrides: map[string]string{"network.p2p.persistent_peers": "a@1.2.3.4:26656"}, + } + err := testCli.Create(testCtx, snt) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("runtime-safe")) +} + +// kind=ConfigPatch with NO payload is rejected. +func TestCEL_ConfigPatch_NoPayload_Rejected(t *testing.T) { + g := NewWithT(t) + ns := makeNamespace(t) + snt := baseTask(ns, "configpatch-nopayload", seiv1alpha1.SeiNodeTaskKindConfigPatch) + err := testCli.Create(testCtx, snt) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(Or( + ContainSubstring("exactly one"), + ContainSubstring("configPatch is required"), + )) +} + // kind=RestartSeid with NO payload is rejected. func TestCEL_RestartSeid_NoPayload_Rejected(t *testing.T) { g := NewWithT(t) diff --git a/internal/task/seinodetask_params.go b/internal/task/seinodetask_params.go index 3f2e3d97..7c0799dd 100644 --- a/internal/task/seinodetask_params.go +++ b/internal/task/seinodetask_params.go @@ -4,7 +4,11 @@ import ( "encoding/json" "errors" "fmt" + "maps" + "slices" + "strings" + seiconfig "github.com/sei-protocol/sei-config" sidecar "github.com/sei-protocol/seictl/sidecar/client" seiv1alpha1 "github.com/sei-protocol/sei-k8s-controller/api/v1alpha1" @@ -94,6 +98,8 @@ func SeiNodeTaskParamsFor(cr *seiv1alpha1.SeiNodeTask, target *seiv1alpha1.SeiNo return restartSeidParams(cr) case seiv1alpha1.SeiNodeTaskKindMarkReady: return markReadyParams(cr) + case seiv1alpha1.SeiNodeTaskKindConfigPatch: + return configPatchParams(cr) default: return SeiNodeTaskParams{}, &ErrUnsupportedKind{Kind: cr.Spec.Kind} } @@ -225,6 +231,55 @@ func markReadyParams(cr *seiv1alpha1.SeiNodeTask) (SeiNodeTaskParams, error) { return SeiNodeTaskParams{sidecar.TaskTypeMarkReady, sidecar.MarkReadyTask{}}, nil } +// configPatchParams synthesizes an incremental config-apply sidecar task from +// the payload's dotted sei-config overrides. sei-config's own resolver on the +// node (ResolveIncrementalIntent -> registry-aware ApplyOverrides -> the legacy +// renderer) turns the overrides into on-disk config, so the controller no longer +// hand-renders a files tree — that path is correct-by-construction for every +// registered key. Mode is left empty: incremental resolution reads the node's +// current on-disk config and patches it, so no mode default is needed. +// +// The controller still enforces the runtime-safe ALLOWLIST as pure policy — +// sei-config's registry only checks that a key exists and type-checks its value, +// and would happily apply an unsafe key (e.g. network.p2p.persistent_peers) the +// controller re-asserts on its own SeiNode update plans. A disallowed key +// surfaces as ParamsBuildFailed (not a remote round-trip). Keys are checked in +// sorted order so a multi-key input reports a stable first offender. +// +// This does NOT restart or reload seid; the caller composes RestartSeid / +// StateSync to make the change take effect. +func configPatchParams(cr *seiv1alpha1.SeiNodeTask) (SeiNodeTaskParams, error) { + p := cr.Spec.ConfigPatch + if p == nil { + return SeiNodeTaskParams{}, paramsErr("spec.configPatch is required for kind=ConfigPatch") + } + if len(p.Overrides) == 0 { + return SeiNodeTaskParams{}, paramsErr("config-patch: overrides must not be empty") + } + for _, key := range slices.Sorted(maps.Keys(p.Overrides)) { + if !configPatchKeyAllowed(key) { + return SeiNodeTaskParams{}, paramsErr(fmt.Sprintf( + "config-patch: key %q is not in the runtime-safe allowlist (chain.occ_enabled, giga_executor.*, mempool.*)", key)) + } + } + return SeiNodeTaskParams{sidecar.TaskTypeConfigApply, &seiconfig.ConfigIntent{ + Incremental: true, + Overrides: p.Overrides, + }}, nil +} + +// configPatchKeyAllowed reports whether a dotted sei-config override key is in +// the ConfigPatch runtime-safe allowlist. It is pure allow/deny POLICY on key +// prefixes, not a renderer: sei-config resolves and renders every registered key, +// but the controller admits only families that are safe to patch out-of-band. +// Keep this in sync with the CEL family rule on SeiNodeTaskSpec and +// sdk/sei.configPatchKeyAllowed — it is a value contract across three points. +func configPatchKeyAllowed(key string) bool { + return key == "chain.occ_enabled" || + strings.HasPrefix(key, "giga_executor.") || + strings.HasPrefix(key, "mempool.") +} + // resolveSigningUID returns explicit when set; otherwise derives from target // via ResolveOperatorKeyringUID. With nil target (early-validation path), the // final marshal happens later from driveTask, which has target. diff --git a/internal/task/seinodetask_params_test.go b/internal/task/seinodetask_params_test.go index 73ae7f73..3b027c12 100644 --- a/internal/task/seinodetask_params_test.go +++ b/internal/task/seinodetask_params_test.go @@ -5,6 +5,7 @@ import ( "errors" "testing" + seiconfig "github.com/sei-protocol/sei-config" sidecar "github.com/sei-protocol/seictl/sidecar/client" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -12,8 +13,10 @@ import ( ) const ( - tpKey = "TimeoutParams" - baseapp = "baseapp" + tpKey = "TimeoutParams" + baseapp = "baseapp" + cpFalse = "false" + keyGigaEnabled = "giga_executor.enabled" ) func govParamChangeCR(value string) *seiv1alpha1.SeiNodeTask { @@ -256,6 +259,86 @@ func TestSeiNodeTaskParamsFor_MarkReady_NilPayload_ParamsBuildFailed(t *testing. } } +// kind=ConfigPatch synthesizes an incremental config-apply task carrying the +// dotted overrides verbatim — sei-config's resolver does the rendering on the +// node, so the controller no longer builds a files tree. +func TestSeiNodeTaskParamsFor_ConfigPatch(t *testing.T) { + cr := &seiv1alpha1.SeiNodeTask{ + Spec: seiv1alpha1.SeiNodeTaskSpec{ + Kind: seiv1alpha1.SeiNodeTaskKindConfigPatch, + ConfigPatch: &seiv1alpha1.ConfigPatchPayload{ + Overrides: map[string]string{keyGigaEnabled: cpFalse}, + }, + }, + } + + p, err := SeiNodeTaskParamsFor(cr, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if p.Type != sidecar.TaskTypeConfigApply { + t.Errorf("Type = %q, want %q", p.Type, sidecar.TaskTypeConfigApply) + } + intent, ok := p.Payload.(*seiconfig.ConfigIntent) + if !ok { + t.Fatalf("Payload = %T, want *seiconfig.ConfigIntent", p.Payload) + } + if !intent.Incremental { + t.Error("intent.Incremental = false, want true") + } + if intent.Mode != "" { + t.Errorf("intent.Mode = %q, want empty (incremental patches the on-disk config)", intent.Mode) + } + if got := intent.Overrides[keyGigaEnabled]; got != cpFalse { + t.Errorf("intent.Overrides[%q] = %q, want %q", keyGigaEnabled, got, cpFalse) + } +} + +// kind=ConfigPatch with an out-of-allowlist key is a ParamsBuildFailed (the +// controller's allowlist policy rejects it), not an unsupported kind. +func TestSeiNodeTaskParamsFor_ConfigPatch_DisallowedKey_ParamsBuildFailed(t *testing.T) { + cr := &seiv1alpha1.SeiNodeTask{ + Spec: seiv1alpha1.SeiNodeTaskSpec{ + Kind: seiv1alpha1.SeiNodeTaskKindConfigPatch, + ConfigPatch: &seiv1alpha1.ConfigPatchPayload{ + Overrides: map[string]string{"network.p2p.persistent_peers": "a@1.2.3.4:26656"}, + }, + }, + } + + _, err := SeiNodeTaskParamsFor(cr, nil) + if err == nil { + t.Fatal("expected error for disallowed key, got nil") + } + var unsupported *ErrUnsupportedKind + if errors.As(err, &unsupported) { + t.Error("disallowed-key error must not be *ErrUnsupportedKind") + } + if got := FailureReason(err); got != ReasonParamsBuildFailed { + t.Errorf("FailureReason = %q, want %q", got, ReasonParamsBuildFailed) + } +} + +// kind=ConfigPatch with a nil payload is a param-build failure, not an +// unsupported kind (CEL normally blocks this; the guard is the backstop). +func TestSeiNodeTaskParamsFor_ConfigPatch_NilPayload_ParamsBuildFailed(t *testing.T) { + cr := &seiv1alpha1.SeiNodeTask{ + Spec: seiv1alpha1.SeiNodeTaskSpec{Kind: seiv1alpha1.SeiNodeTaskKindConfigPatch}, + } + + _, err := SeiNodeTaskParamsFor(cr, nil) + if err == nil { + t.Fatal("expected error for missing payload, got nil") + } + var unsupported *ErrUnsupportedKind + if errors.As(err, &unsupported) { + t.Error("missing-payload error must not be *ErrUnsupportedKind") + } + if got := FailureReason(err); got != ReasonParamsBuildFailed { + t.Errorf("FailureReason = %q, want %q", got, ReasonParamsBuildFailed) + } +} + // kind=RestartSeid maps to the sidecar restart-seid task with an empty payload — // no target needed, no source building (mirrors MarkReady). func TestSeiNodeTaskParamsFor_RestartSeid(t *testing.T) { diff --git a/manifests/sei.io_seinodetasks.yaml b/manifests/sei.io_seinodetasks.yaml index 09f795cb..bbaa3409 100644 --- a/manifests/sei.io_seinodetasks.yaml +++ b/manifests/sei.io_seinodetasks.yaml @@ -106,6 +106,22 @@ spec: required: - targetHeight type: object + configPatch: + description: ConfigPatch is the payload for kind=ConfigPatch. + properties: + overrides: + additionalProperties: + type: string + description: |- + Overrides maps dotted sei-config keys (the same operator-facing schema as + SeiNodeSpec.Overrides, e.g. "giga_executor.enabled": "false") to their + desired string values. Values are coerced to each field's registered type + (bool/int/…) when rendered to TOML. At least one entry is required. + minProperties: 1 + type: object + required: + - overrides + type: object govParamChange: description: GovParamChange is the payload for kind=GovParamChange. properties: @@ -338,6 +354,7 @@ spec: - AwaitNodesAtHeight - RestartSeid - MarkReady + - ConfigPatch type: string markReady: description: MarkReady is the payload for kind=MarkReady. @@ -416,12 +433,12 @@ spec: x-kubernetes-validations: - message: exactly one of govSoftwareUpgrade, govVote, govParamChange, awaitCondition, updateNodeImage, awaitNodesAtHeight, restartSeid, - or markReady must be set + markReady, or configPatch must be set rule: '(has(self.govSoftwareUpgrade) ? 1 : 0) + (has(self.govVote) ? 1 : 0) + (has(self.govParamChange) ? 1 : 0) + (has(self.awaitCondition) ? 1 : 0) + (has(self.updateNodeImage) ? 1 : 0) + (has(self.awaitNodesAtHeight) ? 1 : 0) + (has(self.restartSeid) ? 1 : 0) + (has(self.markReady) - ? 1 : 0) == 1' + ? 1 : 0) + (has(self.configPatch) ? 1 : 0) == 1' - message: spec.govSoftwareUpgrade is required when kind=GovSoftwareUpgrade rule: self.kind != 'GovSoftwareUpgrade' || has(self.govSoftwareUpgrade) - message: spec.govVote is required when kind=GovVote @@ -438,6 +455,12 @@ spec: rule: self.kind != 'RestartSeid' || has(self.restartSeid) - message: spec.markReady is required when kind=MarkReady rule: self.kind != 'MarkReady' || has(self.markReady) + - message: spec.configPatch is required when kind=ConfigPatch + rule: self.kind != 'ConfigPatch' || has(self.configPatch) + - message: spec.configPatch.overrides only admits allowlisted runtime-safe + keys (chain.occ_enabled, giga_executor.*, mempool.*) + rule: '!has(self.configPatch) || self.configPatch.overrides.all(k, k + == ''chain.occ_enabled'' || k.startsWith(''giga_executor.'') || k.startsWith(''mempool.''))' - message: spec.kind is immutable rule: self.kind == oldSelf.kind status: diff --git a/sdk/sei/provider/k8s/render.go b/sdk/sei/provider/k8s/render.go index ac55f17d..e34aa6b9 100644 --- a/sdk/sei/provider/k8s/render.go +++ b/sdk/sei/provider/k8s/render.go @@ -189,6 +189,10 @@ func renderTask(spec sei.TaskSpec, namespace string) *seiv1alpha1.SeiNodeTask { task.Spec.UpdateNodeImage = &seiv1alpha1.UpdateNodeImagePayload{ Image: spec.UpdateNodeImage.Image, } + case spec.ConfigPatch != nil: + task.Spec.ConfigPatch = &seiv1alpha1.ConfigPatchPayload{ + Overrides: maps.Clone(spec.ConfigPatch.Overrides), + } } return task } diff --git a/sdk/sei/task.go b/sdk/sei/task.go index a8092934..cb5f0e14 100644 --- a/sdk/sei/task.go +++ b/sdk/sei/task.go @@ -28,6 +28,7 @@ const ( TaskGovVote = "GovVote" TaskAwaitNodesAtHeight = "AwaitNodesAtHeight" TaskUpdateNodeImage = "UpdateNodeImage" + TaskConfigPatch = "ConfigPatch" ) // Vote options for GovVote.Option (gov v1beta1 VoteOption parse rules). @@ -68,6 +69,7 @@ type TaskSpec struct { GovVote *GovVote AwaitNodesAtHeight *AwaitNodesAtHeight UpdateNodeImage *UpdateNodeImage + ConfigPatch *ConfigPatch } // GovSoftwareUpgrade is the payload for TaskGovSoftwareUpgrade — submit an @@ -136,6 +138,27 @@ type UpdateNodeImage struct { Image string // desired seid image (with tag/digest) } +// ConfigPatch is the payload for TaskConfigPatch — apply an incremental config +// patch to the target node's on-disk config via the sidecar's sei-config +// resolver, which reads the current config and patches only the given keys. +// Overrides uses the operator-facing dotted sei-config key schema (the same as +// SeiNodeSpec.Overrides), e.g. {"giga_executor.enabled": "false"}, and is +// limited to an allowlisted set of runtime-safe toggles. At least one entry is +// required. +// +// PATCHING ONLY: this does not restart or reload seid. seid re-reads its config +// only on (re)start, so a caller composes a following RestartSeid (in-place seid +// bounce) or a StateSync/re-bootstrap step to make the change take effect. +// +// Scope is a fail-closed allowlist — only chain.occ_enabled, giga_executor.*, +// and mempool.* keys are admitted (the controller re-asserts p2p/genesis/state- +// sync keys on its own plans, so patching those would be silently clobbered). +// The SDK rejects out-of-allowlist keys client-side; the controller is the +// authoritative gate. +type ConfigPatch struct { + Overrides map[string]string +} + // TaskOutputs carries a completed task's typed results. A nil sub-field means // that kind produced no typed output. type TaskOutputs struct { @@ -251,6 +274,9 @@ func validateTaskSpec(s TaskSpec) error { if s.UpdateNodeImage != nil { set++ } + if s.ConfigPatch != nil { + set++ + } if set != 1 { return usageErr("TaskSpec requires exactly one payload set, got %d", set) } @@ -274,12 +300,37 @@ func validateTaskSpec(s TaskSpec) error { if s.UpdateNodeImage == nil { return usageErr("TaskSpec.Kind %q requires UpdateNodeImage payload", s.Kind) } + case TaskConfigPatch: + if s.ConfigPatch == nil { + return usageErr("TaskSpec.Kind %q requires ConfigPatch payload", s.Kind) + } + if len(s.ConfigPatch.Overrides) == 0 { + return usageErr("ConfigPatch.Overrides must have at least one entry") + } + for k := range s.ConfigPatch.Overrides { + if !configPatchKeyAllowed(k) { + return usageErr("ConfigPatch.Overrides key %q is not in the runtime-safe allowlist (chain.occ_enabled, giga_executor.*, mempool.*)", k) + } + } default: return usageErr("TaskSpec.Kind %q is not supported by the SDK", s.Kind) } return nil } +// configPatchKeyAllowed reports whether a dotted sei-config override key is in +// the ConfigPatch runtime-safe allowlist. This is the coarse client-side gate +// (family membership) mirroring the CRD's CEL rule; the controller re-enforces +// the same allowlist policy, and the sidecar's sei-config resolver additionally +// rejects unknown fields and type-mismatched values. Keep this family list in +// sync with the CEL rule on SeiNodeTaskSpec and +// internal/task.configPatchKeyAllowed — it is a value contract. +func configPatchKeyAllowed(key string) bool { + return key == "chain.occ_enabled" || + strings.HasPrefix(key, "giga_executor.") || + strings.HasPrefix(key, "mempool.") +} + // validVoteOption reports whether opt is an option the CRD's vote enum accepts // (the two no_with_veto spellings the gov v1beta1 parser allows are both valid). func validVoteOption(opt string) bool { diff --git a/sdk/sei/task_test.go b/sdk/sei/task_test.go index 89524992..85eb4758 100644 --- a/sdk/sei/task_test.go +++ b/sdk/sei/task_test.go @@ -76,6 +76,41 @@ func TestValidateTaskSpec(t *testing.T) { }, false, }, + { + "config-patch valid", + func(s *TaskSpec) { + s.Kind = TaskConfigPatch + s.GovSoftwareUpgrade = nil + s.ConfigPatch = &ConfigPatch{Overrides: map[string]string{"giga_executor.enabled": "false"}} + }, + false, + }, + { + "config-patch empty overrides", + func(s *TaskSpec) { + s.Kind = TaskConfigPatch + s.GovSoftwareUpgrade = nil + s.ConfigPatch = &ConfigPatch{Overrides: map[string]string{}} + }, + true, + }, + { + "config-patch disallowed key", + func(s *TaskSpec) { + s.Kind = TaskConfigPatch + s.GovSoftwareUpgrade = nil + s.ConfigPatch = &ConfigPatch{Overrides: map[string]string{"evm.http_port": "8545"}} + }, + true, + }, + { + "config-patch missing payload", + func(s *TaskSpec) { + s.Kind = TaskConfigPatch + s.GovSoftwareUpgrade = nil + }, + true, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/test/integration/giga_executor_mixed_test.go b/test/integration/giga_executor_mixed_test.go new file mode 100644 index 00000000..c2d260d7 --- /dev/null +++ b/test/integration/giga_executor_mixed_test.go @@ -0,0 +1,318 @@ +//go:build integration + +package integration + +import ( + "context" + "fmt" + "net/http" + "os/signal" + "strconv" + "syscall" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + + "github.com/sei-protocol/sei-k8s-controller/internal/keygen" + "github.com/sei-protocol/sei-k8s-controller/sdk/sei" +) + +// rpcLivenessBlocks is the post-suite advance the follower must show to prove it is +// still producing/replaying blocks (not merely caught-up-then-frozen); small so +// the liveness gate resolves quickly on a healthy chain. +const rpcLivenessBlocks = 5 + +// keySnapshotInterval is the sei-config storage key controlling snapshot cadence; +// the follower zeroes it (a witness serves RPC trust points, not snapshots). +const keySnapshotInterval = "storage.snapshot_interval" + +// TestNightlyGigaExecutorMixed is the CON-368 regression gate: it proves the seid +// execution engine is a per-node concern that stays consensus-compatible across the +// full release conformance suite. It provisions one 4-validator, snapshot-producing +// chain whose validators run the giga executor (the shipped default), brings up a +// single RPC follower flipped to the v2 executor — via the ConfigPatch task + a +// standard StateSync re-bootstrap — and runs the external release-test harness +// against that follower. The gate is: the v2 follower stays in consensus with the +// giga-executor validator set (caught up and advancing) throughout the suite. +// +// A regression that reintroduced the CON-368 executor divergence would make the v2 +// follower compute a different result than the validators committed, halting it at +// the diverging block; the post-suite advance assertion would then fail. Storage +// layout is deliberately out of scope (all nodes run the shipped default) so the +// executor is the only variable under test. +// +// Inputs (env): SEI_CHAIN_ID, SEID_IMAGE, RELEASE_TEST_IMAGE [required]; +// SEI_NAMESPACE [optional]. Run with -test.timeout 0. +func TestNightlyGigaExecutorMixed(t *testing.T) { + requireCluster(t) + chainID := runChainID(mustEnv(t, "SEI_CHAIN_ID")) + seid := mustEnv(t, "SEID_IMAGE") + releaseImage := mustEnv(t, "RELEASE_TEST_IMAGE") + ns := envOr("SEI_NAMESPACE", "") + runLabels := map[string]string{runLabelKey: chainID} + + // The re-bootstrap plus the release-test run (up to 60m per releaseJob's own + // deadline) can overlap; size well above both, matching the sibling. + ctx, cancel := context.WithTimeout(context.Background(), 120*time.Minute) + defer cancel() + ctx, stop := signal.NotifyContext(ctx, syscall.SIGTERM, syscall.SIGINT) + defer stop() + + c := openClient(ctx, t) + cs := clientset(t) + + // 1. Provision a snapshot-producing 4-validator network (giga-executor default) + // with the release admin funded. + env := provisionNetwork(ctx, t, c, cs, chainID, ns, seid, runLabels) + + // 2. Bring up one follower flipped to the v2 executor (ConfigPatch + re-bootstrap). + v2Node := bringUpV2Follower(ctx, t, env) + + // 3. Run the release conformance suite against it. + runReleaseSuite(ctx, t, env, releaseImage, v2Node) + + // 4. Assert the v2 follower stayed in consensus with the giga validators. + assertRPCHealthy(ctx, t, env.hc, v2Node) +} + +// executorMixedEnv is the shared bring-up context the helpers draw on: the SDK + +// client-go clients, the live network (for witness hostnames), the run identifiers +// + seid image, the poll client and snapshot cadence the resync witnesses are gated +// against, and the funded admin identity the release suite signs with. +type executorMixedEnv struct { + c *sei.Client + cs *kubernetes.Clientset + ch *chain + net *sei.Network + chainID string + ns string + seid string + hc *http.Client + interval int + runLabels map[string]string + admin keygen.Identity +} + +// provisionNetwork stands up the fixture: a funded release admin in genesis, a +// 4-validator snapshot-producing chain (no followers — the bring-up helper creates +// its own), advanced past one snapshot interval so the follower can state-sync from +// a snapshot that already exists. It registers teardown and returns the shared +// bring-up context. +func provisionNetwork( + ctx context.Context, t *testing.T, c *sei.Client, cs *kubernetes.Clientset, + chainID, ns, seid string, runLabels map[string]string, +) executorMixedEnv { + t.Helper() + + admin, err := keygen.Derive() + if err != nil { + t.Fatalf("derive admin key: %v", err) + } + + ch, err := provision(ctx, t, c, spec{ + chainID: chainID, + runID: chainID, + namespace: ns, + seidImage: seid, + validators: 4, + rpcNodes: 0, + storageConfig: mergeConfig(releaseBaseConfig, snapshotProductionConfig), + accounts: []sei.GenesisAccount{ + {Address: admin.Address, Balance: releaseAdminBalance}, + }, + }) + cleanupChain(t, ch) + if err != nil { + t.Fatalf("provision: %v", err) + } + + // A follower can only state-sync from a snapshot that already exists; advance the + // network one interval + margin so the re-bootstrap below has one to fetch. + // Derive the delta from the config so retuning the interval cannot silently + // under-wait. + hc := &http.Client{Timeout: 10 * time.Second} + interval, err := strconv.Atoi(snapshotProductionConfig[keySnapshotInterval]) + if err != nil { + t.Fatalf("snapshot_interval not an int: %v", err) + } + if err := sei.WaitHeightAdvances(ctx, hc, ch.network.TendermintRPC(), int64(interval)+10); err != nil { + t.Fatalf("network did not advance past the snapshot interval: %v", err) + } + t.Logf("network %s: ready (4 validators, giga-executor default, snapshot-producing, admin funded)", chainID) + + return executorMixedEnv{ + c: c, cs: cs, ch: ch, net: ch.network, + chainID: chainID, ns: ns, seid: seid, + hc: hc, interval: interval, runLabels: runLabels, + admin: admin, + } +} + +// bringUpV2Follower creates a plain RPC follower (which boots with the giga executor +// by default) and flips it to the v2 executor: a ConfigPatch task disables the giga +// executor on disk, then a STANDARD StateSync re-bootstrap (Migration=nil — the +// plain wipe + re-sync) restarts seid so it re-reads the patched config and comes +// back in v2 mode. ConfigPatch only writes config (seid re-reads it on restart), and +// giga_executor.* is not re-asserted on the controller's own plans, so the patch +// survives the re-bootstrap. Returns the reconfigured node. +func bringUpV2Follower(ctx context.Context, t *testing.T, env executorMixedEnv) *sei.Node { + t.Helper() + node := bringUpPlainFollower(ctx, t, env, env.chainID+"-v2") + + // Patch the on-disk executor config to v2 (giga executor off). runTaskComplete + // runs the task, registers its cleanup, and fails the suite if it does not + // complete. This does NOT restart seid — the StateSync below is what applies it. + runTaskComplete(ctx, t, env.c, sei.TaskSpec{ + Name: "v2patch-" + env.chainID, + Namespace: env.ns, + Node: node.Name(), + Kind: sei.TaskConfigPatch, + Labels: env.runLabels, + ConfigPatch: &sei.ConfigPatch{Overrides: map[string]string{ + "giga_executor.enabled": "false", + "giga_executor.occ_enabled": "false", + }}, + }) + t.Logf("v2 follower %s: config-patch applied (giga executor disabled)", node.Name()) + + // Standard re-bootstrap: restarts seid onto the patched config, in v2 mode. + witnesses := stateSyncWitnesses(t, env) + assertWitnessFresh(ctx, t, env.hc, witnesses[0], witnesses[1], env.interval) + wf, err := env.c.CreateWorkflow(ctx, sei.WorkflowSpec{ + Name: "v2resync-" + env.chainID, + Namespace: env.ns, + Node: node.Name(), + Kind: sei.WorkflowStateSync, + Labels: env.runLabels, + StateSync: &sei.StateSyncWorkflow{Migration: nil, RpcServers: witnesses}, + }) + if err != nil { + t.Fatalf("create v2 resync workflow: %v", err) + } + cleanupWorkflow(t, wf) + t.Logf("workflow %s: created against %s (v2 re-bootstrap)", wf.Name(), node.Name()) + + awaitWorkflowComplete(ctx, t, wf, workflowWaitTimeout) + if err := node.WaitReady(ctx); err != nil { + t.Fatalf("v2 follower %s not Running after re-bootstrap: %v", node.Name(), err) + } + if err := sei.WaitCaughtUp(ctx, env.hc, node.TendermintRPC()); err != nil { + t.Fatalf("v2 follower %s not caught up after re-bootstrap: %v", node.Name(), err) + } + t.Logf("v2 follower %s: caught up in v2 executor mode", node.Name()) + return node +} + +// bringUpPlainFollower creates a plain RPC follower on the network with the +// release-suite config and blocks until it is caught up + EVM-serving. It appends +// the node to the shared chain so cleanupChain reaps it, then returns it ready for +// an executor reconfiguration. The config mirrors the sibling's follower exactly +// (release base + rpc knobs, snapshot production off — a follower is a witness, not +// a snapshot source). +func bringUpPlainFollower(ctx context.Context, t *testing.T, env executorMixedEnv, name string) *sei.Node { + t.Helper() + node, err := createRPCNode(ctx, t, env.c, sei.NodeSpec{ + Name: name, + Network: env.chainID, + Namespace: env.ns, + Image: env.seid, + Labels: env.runLabels, + Config: mergeConfig( + mergeConfig(releaseBaseConfig, snapshotProductionConfig), + mergeConfig(releaseRPCConfig, map[string]string{keySnapshotInterval: "0"}), + ), + }) + if node != nil { + env.ch.rpcNodes = append(env.ch.rpcNodes, node) + } + if err != nil { + t.Fatalf("bring up plain follower %q: %v", name, err) + } + if err := gateRPCNode(ctx, t, env.hc, node); err != nil { + t.Fatalf("plain follower %q not serving: %v", name, err) + } + return node +} + +// stateSyncWitnesses returns two DISTINCT genesis-validator RPC endpoints +// (validator-0 and validator-1) as light-client witnesses for the follower resync. +// A plain follower carries no resolved state-syncers, so the StateSync workflow must +// pass explicit witnesses; two live validators are always at head and decoupled from +// the follower being reconfigured. Bare host:port, the CRD SnapshotSource.RpcServers +// shape. +func stateSyncWitnesses(t *testing.T, env executorMixedEnv) []string { + t.Helper() + witnessNS := witnessNamespace(t, env.net) + return []string{ + nodeRPC(fmt.Sprintf("%s-0", env.chainID), witnessNS), // genesis validator-0 + nodeRPC(fmt.Sprintf("%s-1", env.chainID), witnessNS), // genesis validator-1 + } +} + +// runReleaseSuite launches the release-test Job against the v2 follower, signing +// with the funded admin, and blocks until it completes. The Job name and endpoints +// are logged prominently so a failed run is debuggable from the harness log; the +// suite fails if the run does not complete. +func runReleaseSuite(ctx context.Context, t *testing.T, env executorMixedEnv, releaseImage string, node *sei.Node) { + t.Helper() + cs := env.cs + ns := env.net.Namespace() + + rest := node.REST() + if rest == "" { + t.Fatalf("v2 follower %q has no REST endpoint (release-test needs SEI_REST_ENDPOINT)", node.Name()) + } + if err := sei.WaitRESTServing(ctx, env.hc, rest); err != nil { + t.Fatalf("v2 follower %q REST serving: %v", node.Name(), err) + } + t.Logf("v2 follower %s: REST serving at %s", node.Name(), rest) + + secretName := "admin-" + env.chainID + createMnemonicSecret(ctx, t, cs, ns, secretName, env.runLabels, env.admin.Mnemonic) + + job := releaseJob(releaseParams{ + name: "release-test-" + env.chainID, + namespace: ns, + image: releaseImage, + runID: env.chainID, + chainID: env.chainID, + adminAddr: env.admin.Address, + secretName: secretName, + tmRPC: node.TendermintRPC(), + evmRPC: node.EVMRPC(), + rest: rest, + }) + if _, err := cs.BatchV1().Jobs(ns).Create(ctx, job, metav1.CreateOptions{}); err != nil { + t.Fatalf("create release-test job: %v", err) + } + t.Cleanup(func() { + delCtx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + bg := metav1.DeletePropagationBackground + _ = cs.BatchV1().Jobs(ns).Delete(delCtx, job.Name, metav1.DeleteOptions{PropagationPolicy: &bg}) + }) + t.Logf("release-test job launched: %s (image %s)", job.Name, releaseImage) + + waitJob(ctx, t, cs, ns, job.Name) + t.Logf("release-test PASSED against the v2 executor follower") +} + +// assertRPCHealthy re-affirms the follower survived the release run: still caught up +// AND advancing (not caught-up-then-frozen). Errors are collected per-follower +// (t.Errorf, not Fatalf) so every node is always evaluated. +func assertRPCHealthy(ctx context.Context, t *testing.T, hc *http.Client, nodes ...*sei.Node) { + t.Helper() + for _, n := range nodes { + if err := sei.WaitCaughtUp(ctx, hc, n.TendermintRPC()); err != nil { + t.Errorf("post-release follower %s not caught up: %v", n.Name(), err) + continue + } + if err := sei.WaitHeightAdvances(ctx, hc, n.TendermintRPC(), rpcLivenessBlocks); err != nil { + t.Errorf("post-release follower %s height not advancing (frozen?): %v", n.Name(), err) + continue + } + t.Logf("post-release follower %s: caught up and advancing", n.Name()) + } +} From 6518a7f68c9698900075f4b41ec230530baefa19 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Tue, 14 Jul 2026 23:36:21 -0700 Subject: [PATCH 2/2] =?UTF-8?q?test,nodetask:=20xreview=20fixes=20?= =?UTF-8?q?=E2=80=94=20self-verifying=20v2=20gate=20+=20attribution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 xreview (kubernetes-specialist, systems-engineer dissenter, idiomatic-reviewer) ratified all boundaries; two substantive reviewers converged on one correctness-grade finding, fixed here. - Positive v2 assertion: bringUpV2Follower now reads the follower's on-disk app.toml (client-go exec) and asserts [giga_executor] enabled and occ_enabled are false after the flip. Without this the gate could pass while testing nothing (giga is the default and the validators run giga, so a silently-dropped patch would leave the follower on giga, in consensus, green). - Scoped the docstring: the gate proves no v2/giga divergence ON THE RELEASE WORKLOAD, not v2 == giga; states the un-defer trigger. - Stall attribution: assertRPCHealthy now reports seid restartCount + OOMKilled and greps a broad consensus-mismatch pattern in the log tail, so a red nightly separates a consensus halt from an infra crash. - Allowlist parity guard test in internal/task and sdk/sei (the three copies stay required — SDK apimachinery-free boundary — only guarded). - Comment fixes: present-tense config-apply rationale, configApplyTask embedding-only guard, ConfigPatch-kind->config-apply wiring note. Co-Authored-By: Claude Opus 4.8 (1M context) --- api/v1alpha1/seinodetask_types.go | 5 + go.mod | 5 +- go.sum | 8 + internal/task/config.go | 3 + internal/task/configpatch_allowlist_test.go | 43 ++++ internal/task/seinodetask_params.go | 4 +- internal/task/seinodetask_params_test.go | 4 +- sdk/sei/configpatch_allowlist_test.go | 43 ++++ test/integration/giga_executor_mixed_test.go | 209 +++++++++++++++++-- 9 files changed, 302 insertions(+), 22 deletions(-) create mode 100644 internal/task/configpatch_allowlist_test.go create mode 100644 sdk/sei/configpatch_allowlist_test.go diff --git a/api/v1alpha1/seinodetask_types.go b/api/v1alpha1/seinodetask_types.go index f7005edf..48ff7f0e 100644 --- a/api/v1alpha1/seinodetask_types.go +++ b/api/v1alpha1/seinodetask_types.go @@ -88,6 +88,11 @@ const ( // its own update plans (p2p peers/external-address, genesis/state-sync-managed // keys) are rejected — a patch to those would be silently clobbered by the // next SeiNode update plan. + // + // Wiring note: this kind maps to sidecar.TaskTypeConfigApply (incremental dotted + // intent), NOT sidecar.TaskTypeConfigPatch. The latter drives the separate + // ConfigPatchTask{Files} file-tree task used by the StateSync migration path; do + // not rewire this kind to it. SeiNodeTaskKindConfigPatch SeiNodeTaskKind = "ConfigPatch" ) diff --git a/go.mod b/go.mod index 129a1c3b..649e632c 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/sei-protocol/sei-k8s-controller go 1.26.0 require ( + github.com/BurntSushi/toml v1.5.0 github.com/aws/aws-sdk-go-v2 v1.41.6 github.com/aws/aws-sdk-go-v2/config v1.32.12 github.com/aws/aws-sdk-go-v2/service/ec2 v1.293.0 @@ -35,7 +36,6 @@ require ( require ( cel.dev/expr v0.25.1 // indirect - github.com/BurntSushi/toml v1.5.0 // indirect github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.8 // indirect @@ -74,14 +74,17 @@ require ( github.com/google/btree v1.1.3 // indirect github.com/google/cel-go v0.26.0 // indirect github.com/google/gnostic-models v0.7.0 // indirect + github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/mailru/easyjson v0.9.1 // indirect + github.com/moby/spdystream v0.5.0 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect github.com/oapi-codegen/runtime v1.2.0 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.23.2 // indirect diff --git a/go.sum b/go.sum index e73bafd4..1ea3a872 100644 --- a/go.sum +++ b/go.sum @@ -9,6 +9,8 @@ github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8 github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g= github.com/apapsch/go-jsonmerge/v2 v2.0.0 h1:axGnT1gRIfimI7gJifB699GoE/oq+F2MU7Dml6nw9rQ= github.com/apapsch/go-jsonmerge/v2 v2.0.0/go.mod h1:lvDnEdqiQrp0O42VQGgmlKpxL1AP2+08jFMw88y4klk= +github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= +github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/aws/aws-sdk-go-v2 v1.41.6 h1:1AX0AthnBQzMx1vbmir3Y4WsnJgiydmnJjiLu+LvXOg= github.com/aws/aws-sdk-go-v2 v1.41.6/go.mod h1:dy0UzBIfwSeot4grGvY1AqFWN5zgziMmWGzysDnHFcQ= github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.8 h1:eBMB84YGghSocM7PsjmmPffTa+1FBUeNvGvFou6V/4o= @@ -117,6 +119,8 @@ github.com/google/pprof v0.0.0-20260115054156-294ebfa9ad83 h1:z2ogiKUYzX5Is6zr/v github.com/google/pprof v0.0.0-20260115054156-294ebfa9ad83/go.mod h1:MxpfABSjhmINe3F1It9d+8exIHFvUqtLIRCdOGNXqiI= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 h1:JeSE6pjso5THxAzdVpqr6/geYxZytqFMBCOtn/ujyeo= +github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674/go.mod h1:r4w70xmWCQKmi1ONH4KIaBptdivuRPyosB9RmPlGEwA= github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0 h1:HWRh5R2+9EifMyIHV7ZV+MIZqgz+PMpZ14Jynv3O2Zs= github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0/go.mod h1:JfhWUomR1baixubs02l85lZYYOm7LV6om4ceouMv45c= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= @@ -138,6 +142,8 @@ github.com/leanovate/gopter v0.2.11 h1:vRjThO1EKPb/1NsDXuDrzldR28RLkBflWYcU9CvzW github.com/leanovate/gopter v0.2.11/go.mod h1:aK3tzZP/C+p1m3SPRE4SYZFGP7jjkuSI4f7Xvpt0S9c= github.com/mailru/easyjson v0.9.1 h1:LbtsOm5WAswyWbvTEOqhypdPeZzHavpZx96/n553mR8= github.com/mailru/easyjson v0.9.1/go.mod h1:1+xMtQp2MRNVL/V1bOzuP3aP8VNwRW55fQUto+XFtTU= +github.com/moby/spdystream v0.5.0 h1:7r0J1Si3QO/kjRitvSLVVFUjxMEb/YLj6S9FF62JBCU= +github.com/moby/spdystream v0.5.0/go.mod h1:xBAYlnt/ay+11ShkdFKNAG7LsyK/tmNBVvVOwrfMgdI= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -146,6 +152,8 @@ github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee h1:W5t00kpgFd github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= +github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f h1:y5//uYreIhSUg3J1GEMiLbxo1LJaP8RfCpH6pymGZus= +github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw= github.com/oapi-codegen/runtime v1.2.0 h1:RvKc1CVS1QeKSNzO97FBQbSMZyQ8s6rZd+LpmzwHMP4= github.com/oapi-codegen/runtime v1.2.0/go.mod h1:Y7ZhmmlE8ikZOmuHRRndiIm7nf3xcVv+YMweKgG1DT0= github.com/onsi/ginkgo/v2 v2.28.0 h1:Rrf+lVLmtlBIKv6KrIGJCjyY8N36vDVcutbGJkyqjJc= diff --git a/internal/task/config.go b/internal/task/config.go index 64563851..832ae5ab 100644 --- a/internal/task/config.go +++ b/internal/task/config.go @@ -11,6 +11,9 @@ import ( // produce. Wire format and validation delegate to sidecar.ConfigApplyTask — // the seictl wrapper wraps the same fields in a nested Intent struct, // which would otherwise change the on-disk shape. +// +// Keep this embedding-only: a non-embedded field would break the bare +// *ConfigIntent marshal round-trip that keeps Params.Raw on the flat shape. type configApplyTask struct { seiconfig.ConfigIntent } diff --git a/internal/task/configpatch_allowlist_test.go b/internal/task/configpatch_allowlist_test.go new file mode 100644 index 00000000..9cecbfe7 --- /dev/null +++ b/internal/task/configpatch_allowlist_test.go @@ -0,0 +1,43 @@ +package task + +import "testing" + +// configPatchAllowlistCases is the parity contract for the ConfigPatch runtime-safe +// allowlist. The allowlist deliberately lives in THREE hand-maintained copies — this +// package's configPatchKeyAllowed, sdk/sei.configPatchKeyAllowed, and the CRD's CEL +// family rule on SeiNodeTaskSpec — because the SDK's apimachinery-free boundary +// forbids sharing a single Go predicate across them. This table pins both Go copies +// to the same truth so they cannot drift silently; keep it byte-identical to the +// sibling table in sdk/sei (configpatch_allowlist_test.go). The CEL leg is covered by +// the envtest admission cases. +var configPatchAllowlistCases = []struct { + key string + want bool +}{ + // In-family (runtime-safe toggles the controller does not re-assert). + {"chain.occ_enabled", true}, + {"giga_executor.enabled", true}, + {"giga_executor.occ_enabled", true}, + {"mempool.max-txs", true}, + {"mempool.size", true}, + // Out-of-family / near-misses that must be rejected. + {"chain.occ_enabled_extra", false}, // == match must be exact + {"chain", false}, // family root without member + {"giga_executor", false}, // prefix without the dot + {"gigaexecutor.enabled", false}, // missing separator + {"mempoolx.size", false}, // prefix-of-prefix, not the family + {"network.p2p.persistent_peers", false}, // controller re-asserts this on update plans + {"consensus.timeout_commit", false}, + {"", false}, +} + +// TestConfigPatchKeyAllowedParity pins internal/task.configPatchKeyAllowed to the +// shared allowlist contract. sdk/sei has a mirror test against the identical table; +// a divergence between the two Go copies surfaces as one of the two tests failing. +func TestConfigPatchKeyAllowedParity(t *testing.T) { + for _, tc := range configPatchAllowlistCases { + if got := configPatchKeyAllowed(tc.key); got != tc.want { + t.Errorf("configPatchKeyAllowed(%q) = %t, want %t", tc.key, got, tc.want) + } + } +} diff --git a/internal/task/seinodetask_params.go b/internal/task/seinodetask_params.go index 7c0799dd..50b03bed 100644 --- a/internal/task/seinodetask_params.go +++ b/internal/task/seinodetask_params.go @@ -234,8 +234,8 @@ func markReadyParams(cr *seiv1alpha1.SeiNodeTask) (SeiNodeTaskParams, error) { // configPatchParams synthesizes an incremental config-apply sidecar task from // the payload's dotted sei-config overrides. sei-config's own resolver on the // node (ResolveIncrementalIntent -> registry-aware ApplyOverrides -> the legacy -// renderer) turns the overrides into on-disk config, so the controller no longer -// hand-renders a files tree — that path is correct-by-construction for every +// renderer) turns the overrides into on-disk config, so the controller forwards +// only the dotted overrides — that path is correct-by-construction for every // registered key. Mode is left empty: incremental resolution reads the node's // current on-disk config and patches it, so no mode default is needed. // diff --git a/internal/task/seinodetask_params_test.go b/internal/task/seinodetask_params_test.go index 3b027c12..609c5e60 100644 --- a/internal/task/seinodetask_params_test.go +++ b/internal/task/seinodetask_params_test.go @@ -260,8 +260,8 @@ func TestSeiNodeTaskParamsFor_MarkReady_NilPayload_ParamsBuildFailed(t *testing. } // kind=ConfigPatch synthesizes an incremental config-apply task carrying the -// dotted overrides verbatim — sei-config's resolver does the rendering on the -// node, so the controller no longer builds a files tree. +// dotted overrides verbatim — sei-config's resolver renders on the node, so the +// controller forwards only the dotted overrides. func TestSeiNodeTaskParamsFor_ConfigPatch(t *testing.T) { cr := &seiv1alpha1.SeiNodeTask{ Spec: seiv1alpha1.SeiNodeTaskSpec{ diff --git a/sdk/sei/configpatch_allowlist_test.go b/sdk/sei/configpatch_allowlist_test.go new file mode 100644 index 00000000..32dec150 --- /dev/null +++ b/sdk/sei/configpatch_allowlist_test.go @@ -0,0 +1,43 @@ +package sei + +import "testing" + +// configPatchAllowlistCases is the parity contract for the ConfigPatch runtime-safe +// allowlist. The allowlist deliberately lives in THREE hand-maintained copies — this +// package's configPatchKeyAllowed, internal/task.configPatchKeyAllowed, and the CRD's +// CEL family rule on SeiNodeTaskSpec — because this SDK's apimachinery-free boundary +// forbids sharing a single Go predicate across them. This table pins both Go copies +// to the same truth so they cannot drift silently; keep it byte-identical to the +// sibling table in internal/task (configpatch_allowlist_test.go). The CEL leg is +// covered by the envtest admission cases. +var configPatchAllowlistCases = []struct { + key string + want bool +}{ + // In-family (runtime-safe toggles the controller does not re-assert). + {"chain.occ_enabled", true}, + {"giga_executor.enabled", true}, + {"giga_executor.occ_enabled", true}, + {"mempool.max-txs", true}, + {"mempool.size", true}, + // Out-of-family / near-misses that must be rejected. + {"chain.occ_enabled_extra", false}, // == match must be exact + {"chain", false}, // family root without member + {"giga_executor", false}, // prefix without the dot + {"gigaexecutor.enabled", false}, // missing separator + {"mempoolx.size", false}, // prefix-of-prefix, not the family + {"network.p2p.persistent_peers", false}, // controller re-asserts this on update plans + {"consensus.timeout_commit", false}, + {"", false}, +} + +// TestConfigPatchKeyAllowedParity pins sdk/sei.configPatchKeyAllowed to the shared +// allowlist contract. internal/task has a mirror test against the identical table; a +// divergence between the two Go copies surfaces as one of the two tests failing. +func TestConfigPatchKeyAllowedParity(t *testing.T) { + for _, tc := range configPatchAllowlistCases { + if got := configPatchKeyAllowed(tc.key); got != tc.want { + t.Errorf("configPatchKeyAllowed(%q) = %t, want %t", tc.key, got, tc.want) + } + } +} diff --git a/test/integration/giga_executor_mixed_test.go b/test/integration/giga_executor_mixed_test.go index c2d260d7..18986a4e 100644 --- a/test/integration/giga_executor_mixed_test.go +++ b/test/integration/giga_executor_mixed_test.go @@ -3,19 +3,28 @@ package integration import ( + "bytes" "context" "fmt" "net/http" "os/signal" + "regexp" "strconv" + "strings" "syscall" "testing" "time" + "github.com/BurntSushi/toml" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/remotecommand" + ctrl "sigs.k8s.io/controller-runtime" "github.com/sei-protocol/sei-k8s-controller/internal/keygen" + "github.com/sei-protocol/sei-k8s-controller/internal/platform" "github.com/sei-protocol/sei-k8s-controller/sdk/sei" ) @@ -29,19 +38,30 @@ const rpcLivenessBlocks = 5 const keySnapshotInterval = "storage.snapshot_interval" // TestNightlyGigaExecutorMixed is the CON-368 regression gate: it proves the seid -// execution engine is a per-node concern that stays consensus-compatible across the -// full release conformance suite. It provisions one 4-validator, snapshot-producing -// chain whose validators run the giga executor (the shipped default), brings up a -// single RPC follower flipped to the v2 executor — via the ConfigPatch task + a -// standard StateSync re-bootstrap — and runs the external release-test harness -// against that follower. The gate is: the v2 follower stays in consensus with the -// giga-executor validator set (caught up and advancing) throughout the suite. +// execution engine is a per-node concern that stays consensus-compatible with a +// giga-executor validator set ACROSS THE RELEASE CONFORMANCE WORKLOAD. It provisions +// one 4-validator, snapshot-producing chain whose validators run the giga executor +// (the shipped default), brings up a single RPC follower flipped to the v2 executor +// — via the ConfigPatch task + a standard StateSync re-bootstrap, then VERIFIES on +// disk that the flip actually took (bringUpV2Follower asserts [giga_executor] is off, +// so a silently-dropped patch cannot make the gate vacuous) — and runs the external +// release-test harness against that follower. The gate is: the v2 follower stays in +// consensus with the giga-executor validators (caught up and advancing) throughout +// the suite. // -// A regression that reintroduced the CON-368 executor divergence would make the v2 -// follower compute a different result than the validators committed, halting it at -// the diverging block; the post-suite advance assertion would then fail. Storage -// layout is deliberately out of scope (all nodes run the shipped default) so the -// executor is the only variable under test. +// Scope of the claim: this proves "no v2/giga divergence ON THE RELEASE WORKLOAD," +// not "v2 ≡ giga" in general. The gate only fires if the suite's tx mix actually +// exercises a diverging code path; a divergence confined to a tx type the conformance +// suite never emits would slip through. Un-defer trigger: if a v2/giga divergence +// ever recurs in a tx type this suite does not exercise, add targeted diverging-tx +// vectors here rather than trusting the conformance workload's coverage. +// +// A regression that reintroduced a CON-368-class divergence on a tx the suite emits +// would make the v2 follower compute a different result than the validators +// committed, halting it at the diverging block; the post-suite advance assertion +// would then fail (and attributeStall separates that consensus halt from an unrelated +// infra crash). Storage layout is deliberately out of scope (all nodes run the +// shipped default) so the executor is the only variable under test. // // Inputs (env): SEI_CHAIN_ID, SEID_IMAGE, RELEASE_TEST_IMAGE [required]; // SEI_NAMESPACE [optional]. Run with -test.timeout 0. @@ -74,7 +94,7 @@ func TestNightlyGigaExecutorMixed(t *testing.T) { runReleaseSuite(ctx, t, env, releaseImage, v2Node) // 4. Assert the v2 follower stayed in consensus with the giga validators. - assertRPCHealthy(ctx, t, env.hc, v2Node) + assertRPCHealthy(ctx, t, env.cs, env.hc, v2Node) } // executorMixedEnv is the shared bring-up context the helpers draw on: the SDK + @@ -201,10 +221,46 @@ func bringUpV2Follower(ctx context.Context, t *testing.T, env executorMixedEnv) if err := sei.WaitCaughtUp(ctx, env.hc, node.TendermintRPC()); err != nil { t.Fatalf("v2 follower %s not caught up after re-bootstrap: %v", node.Name(), err) } + + // VERIFY the variable-under-test: giga is the shipped default AND the validators + // run giga, so "caught up" alone cannot prove the follower is on v2 — a silently + // dropped ConfigPatch would leave it on giga, in consensus, and the gate would + // pass while testing nothing. Read app.toml off disk and assert giga is off. + assertFollowerV2(ctx, t, env.cs, node) t.Logf("v2 follower %s: caught up in v2 executor mode", node.Name()) return node } +// assertFollowerV2 reads app.toml off the follower's seid container and asserts the +// executor actually flipped to v2 (giga_executor off). This is the positive check +// that closes the vacuous-pass hole: the flip is VERIFIED on disk, not inferred from +// the follower merely staying in consensus with a giga validator set. +func assertFollowerV2(ctx context.Context, t *testing.T, cs *kubernetes.Clientset, node *sei.Node) { + t.Helper() + // A SeiNode's StatefulSet is replicas=1, so its sole pod is -0. + pod := node.Name() + "-0" + appTomlPath := platform.DataDir + "/config/app.toml" + out, err := execInPod(ctx, t, cs, node.Namespace(), pod, containerSeid, "cat", appTomlPath) + if err != nil { + t.Fatalf("read %s on follower %s: %v", appTomlPath, node.Name(), err) + } + var cfg struct { + GigaExecutor struct { + Enabled bool `toml:"enabled"` + OCCEnabled bool `toml:"occ_enabled"` + } `toml:"giga_executor"` + } + if _, err := toml.Decode(out, &cfg); err != nil { + t.Fatalf("parse app.toml from follower %s: %v\n--- app.toml ---\n%s", node.Name(), err, out) + } + if cfg.GigaExecutor.Enabled || cfg.GigaExecutor.OCCEnabled { + t.Fatalf("follower %s still running giga after ConfigPatch+re-bootstrap "+ + "([giga_executor] enabled=%t occ_enabled=%t) — the executor flip did not survive; "+ + "the gate would be vacuous", node.Name(), cfg.GigaExecutor.Enabled, cfg.GigaExecutor.OCCEnabled) + } + t.Logf("follower %s: verified v2 on disk ([giga_executor] enabled=false, occ_enabled=false)", node.Name()) +} + // bringUpPlainFollower creates a plain RPC follower on the network with the // release-suite config and blocks until it is caught up + EVM-serving. It appends // the node to the shared chain so cleanupChain reaps it, then returns it ready for @@ -301,18 +357,137 @@ func runReleaseSuite(ctx context.Context, t *testing.T, env executorMixedEnv, re // assertRPCHealthy re-affirms the follower survived the release run: still caught up // AND advancing (not caught-up-then-frozen). Errors are collected per-follower -// (t.Errorf, not Fatalf) so every node is always evaluated. -func assertRPCHealthy(ctx context.Context, t *testing.T, hc *http.Client, nodes ...*sei.Node) { +// (t.Errorf, not Fatalf) so every node is always evaluated. A stall failure carries +// attributeStall's root-cause read so a red nightly is actionable at a glance — +// divergence (the thing under test) vs an infra crash (a known flake source). +func assertRPCHealthy( + ctx context.Context, t *testing.T, cs *kubernetes.Clientset, hc *http.Client, nodes ...*sei.Node, +) { t.Helper() for _, n := range nodes { if err := sei.WaitCaughtUp(ctx, hc, n.TendermintRPC()); err != nil { - t.Errorf("post-release follower %s not caught up: %v", n.Name(), err) + t.Errorf("post-release follower %s not caught up: %v\n%s", n.Name(), err, attributeStall(cs, n)) continue } if err := sei.WaitHeightAdvances(ctx, hc, n.TendermintRPC(), rpcLivenessBlocks); err != nil { - t.Errorf("post-release follower %s height not advancing (frozen?): %v", n.Name(), err) + t.Errorf("post-release follower %s height not advancing (frozen?): %v\n%s", n.Name(), err, attributeStall(cs, n)) continue } t.Logf("post-release follower %s: caught up and advancing", n.Name()) } } + +// containerSeid is the seid container name in a SeiNode pod +// (noderesource.containerNameSeid); the exec/log reads below target it directly. +const containerSeid = "seid" + +// consensusMismatchRe broadly matches the app-hash / results-hash / consensus-failure +// log lines a genuine v2/giga divergence emits, WITHOUT pinning one exact +// sei-tendermint literal (the phrasing drifts across releases). A match attributes a +// stall to divergence; its absence — with a clean restart count — points at infra. +var consensusMismatchRe = regexp.MustCompile( + `(?i)wrong app hash|app\s*hash mismatch|apphash|lastresultshash|last results hash|consensus failure`) + +// attributeStall turns an opaque "not advancing" into an actionable root cause. These +// nightlies flake on infra races, so a genuine consensus halt (a divergence — the +// variable under test) and an unrelated OOM/crash otherwise produce the IDENTICAL +// "not advancing" red. It reports the seid container's restart count + last +// termination reason (calling out OOMKilled explicitly as infra death, not a +// divergence), then tails the seid log and surfaces any consensus-mismatch lines. +// Best-effort and on a FRESH context (the suite ctx may already be drained by the +// WaitHeightAdvances budget): a failed diagnostic read must never mask the stall. +func attributeStall(cs *kubernetes.Clientset, node *sei.Node) string { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + ns, pod := node.Namespace(), node.Name()+"-0" + + var b strings.Builder + b.WriteString("--- stall attribution ---\n") + + p, err := cs.CoreV1().Pods(ns).Get(ctx, pod, metav1.GetOptions{}) + if err != nil { + fmt.Fprintf(&b, "(pod %s/%s status unavailable: %v)\n", ns, pod, err) + } else { + for _, st := range p.Status.ContainerStatuses { + if st.Name != containerSeid { + continue + } + fmt.Fprintf(&b, "seid restartCount=%d ready=%t\n", st.RestartCount, st.Ready) + if term := st.LastTerminationState.Terminated; term != nil { + fmt.Fprintf(&b, "seid last termination: reason=%q exitCode=%d\n", term.Reason, term.ExitCode) + if term.Reason == "OOMKilled" { + b.WriteString("=> OOMKilled: INFRA death (memory pressure), NOT an executor divergence\n") + } + } + } + } + + tail := podContainerLogTail(ctx, cs, ns, pod, containerSeid, 200) + if hits := matchingLines(tail, consensusMismatchRe); len(hits) > 0 { + b.WriteString("=> consensus-mismatch pattern in seid log (points at v2/giga DIVERGENCE):\n") + for _, line := range hits { + fmt.Fprintf(&b, " %s\n", line) + } + } else { + b.WriteString("(no consensus-mismatch pattern in seid log tail; with a clean restart count " + + "this reads as an infra stall, not a divergence)\n") + fmt.Fprintf(&b, "--- seid log (tail) ---\n%s\n", tail) + } + return b.String() +} + +// matchingLines returns the lines of text that match re, preserving order. +func matchingLines(text string, re *regexp.Regexp) []string { + var out []string + for line := range strings.SplitSeq(text, "\n") { + if re.MatchString(line) { + out = append(out, line) + } + } + return out +} + +// podContainerLogTail returns the tail of a named container's log in a pod, +// best-effort — the diagnostic signal a stall error alone cannot carry. +func podContainerLogTail(ctx context.Context, cs *kubernetes.Clientset, ns, pod, container string, lines int64) string { + raw, err := cs.CoreV1().Pods(ns).GetLogs(pod, &corev1.PodLogOptions{ + Container: container, + TailLines: &lines, + }).DoRaw(ctx) + if err != nil { + return fmt.Sprintf("(read logs %s/%s[%s] failed: %v)", ns, pod, container, err) + } + return string(raw) +} + +// execInPod runs argv (no shell) in a pod container and returns its stdout. This is +// the harness's one reach-into-pod primitive — used only for test-time introspection +// (reading rendered config off disk); the controller's real side-effect channel to a +// node is the seictl sidecar HTTP task API, never exec. +func execInPod( + ctx context.Context, t *testing.T, cs *kubernetes.Clientset, ns, pod, container string, argv ...string, +) (string, error) { + t.Helper() + cfg, err := ctrl.GetConfig() + if err != nil { + return "", fmt.Errorf("load kubeconfig: %w", err) + } + req := cs.CoreV1().RESTClient().Post(). + Resource("pods").Name(pod).Namespace(ns).SubResource("exec"). + VersionedParams(&corev1.PodExecOptions{ + Container: container, + Command: argv, + Stdout: true, + Stderr: true, + }, scheme.ParameterCodec) + exec, err := remotecommand.NewSPDYExecutor(cfg, http.MethodPost, req.URL()) + if err != nil { + return "", fmt.Errorf("new SPDY executor: %w", err) + } + var stdout, stderr bytes.Buffer + if err := exec.StreamWithContext(ctx, remotecommand.StreamOptions{Stdout: &stdout, Stderr: &stderr}); err != nil { + return stdout.String(), fmt.Errorf( + "exec %v in %s/%s[%s]: %w (stderr: %s)", argv, ns, pod, container, err, stderr.String()) + } + return stdout.String(), nil +}