Skip to content

feat(nodetask): ConfigPatch kind + CON-368 executor-mixed nightly test#491

Open
bdchatham wants to merge 2 commits into
mainfrom
feat/con368-executor-mixed-nightly
Open

feat(nodetask): ConfigPatch kind + CON-368 executor-mixed nightly test#491
bdchatham wants to merge 2 commits into
mainfrom
feat/con368-executor-mixed-nightly

Conversation

@bdchatham

Copy link
Copy Markdown
Collaborator

What

Two things, one dependent on the other:

  1. ConfigPatch SeiNodeTask kind — a standalone, operator-facing day-2 config patch. 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 dotted→file render table. A fail-closed allowlist (giga_executor.*, chain.occ_enabled, mempool.*) is enforced purely as safety policy at three points (CRD CEL family gate, controller, SDK client-check); it excludes keys the controller re-asserts on its own plans (persistent_peers, external-address, genesis/state-sync-managed).

  2. TestNightlyGigaExecutorMixed — the CON-368 regression gate. Provisions 4 giga-executor validators (shipped default) + one RPC follower flipped to the v2 executor via ConfigPatch + a standard StateSync re-bootstrap, runs the external release-test suite against it, and asserts the follower stays caught up and advancing throughout.

Why

A v2-executor node must stay in consensus with a giga-executor validator set. A regression that reintroduced the CON-368 executor divergence would make the v2 follower compute a different result than the validators committed, halt it at the diverging block, and fail the post-suite advance assertion. Storage layout is deliberately out of scope (all nodes run the shipped default) so the executor is the only variable under test.

One-way door (CRD API)

New served v1alpha1 surface, additive: enum value ConfigPatch, union field spec.configPatch, payload spec.configPatch.overrides (map[string]string, MinProperties=1). Retained-not-removed thereafter. The allowlist is a value contract worth a look.

Verification

go build ./... + -tags integration, go vet -tags integration, unit tests (SDK / internal/task / nodetask envtest CEL), gofmt, and golangci-lint --new-from-rev=origin/main (how CI gates) — all clean, 0 new issues. The integration test requires a live cluster (nightly harness); it is not run in unit CI.

🤖 Generated with Claude Code

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) <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 15, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Additive v1alpha1 API with a hand-maintained three-way allowlist that must stay in sync; runtime node config changes need composed restart/re-bootstrap. Large new nightly integration test depends on live cluster and external release image.

Overview
Adds ConfigPatch as a new SeiNodeTask kind for day-2, allowlisted dotted sei-config overrides written on disk via the sidecar config-apply incremental path (not the StateSync file-tree ConfigPatch task). Admission uses updated union/CEL rules; overrides are limited to chain.occ_enabled, giga_executor.*, and mempool.*, with the same policy re-checked in the controller param builder and SDK.

Wires the kind through nodetask (2m default timeout), internal/taskseiconfig.ConfigIntent, SDK TaskConfigPatch + K8s render, CRD/manifests/deepcopy, and parity tests for the three-way allowlist contract.

Adds TestNightlyGigaExecutorMixed: four giga-executor validators, one RPC follower flipped with ConfigPatch + StateSync re-bootstrap, release-test suite, on-disk v2 verification, and post-suite caught-up/advancing checks with stall attribution. Promotes BurntSushi/toml for integration config reads.

Reviewed by Cursor Bugbot for commit 6518a7f. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 089a4df. Configure here.

Comment thread test/integration/giga_executor_mixed_test.go
Comment thread test/integration/giga_executor_mixed_test.go
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) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant