Skip to content

feat: decouple snapshot generation from publishing#108

Merged
bdchatham merged 2 commits intomainfrom
feat/snapshot-publish-config
Apr 21, 2026
Merged

feat: decouple snapshot generation from publishing#108
bdchatham merged 2 commits intomainfrom
feat/snapshot-publish-config

Conversation

@bdchatham
Copy link
Copy Markdown
Collaborator

Summary

Implements the merged design in docs/design-snapshot-publish-config.md (#107). Operators can now generate local Tendermint snapshots without also uploading them to S3, and the snapshot-config surface is shaped so future snapshot flavors slot in without renaming fields.

  • Refactored SnapshotGenerationConfig to nest a TendermintSnapshotGenerationConfig sub-struct; added optional Publish struct whose presence enables S3 upload. Matches the existing StateSyncSource struct{} / SnapshotSource presence-as-toggle pattern already used in common_types.go.
  • Re-wired the existing seictl TaskTypeSnapshotUpload sidecar task as a fire-and-forget plan task. Inserted into the init progression immediately before mark-ready only when publish is set. Restores controller-side plumbing that was deleted in feat: remove monitor task system #89 with the monitor-task subsystem, now as a plan task.
  • Cross-field validation (empty snapshotGeneration, publish + keepRecent < 2) lives in each planner's Validate(node) hook with its own mode-prefixed error (fullNode: / archive:). No admission webhook introduced; no shared helper takes a mode parameter.
  • Samples (pacific-1-snapshotter.yaml, pacific-1-state-syncer.yaml) updated to the new shape, preserving their prior publishing behavior.
  • CRDs regenerated via make manifests generate; DeepCopy regenerated. No hand edits to generated files.

Shape of the new surface

# Generate 5 local snapshots + upload to S3 (previous default behavior)
snapshotGeneration:
  tendermint:
    keepRecent: 5
    publish: {}

# Generate 3 local snapshots, do NOT upload (new capability)
snapshotGeneration:
  tendermint:
    keepRecent: 3

# No snapshot generation
# (field omitted)

Verification

  • make build — succeeds
  • make lint — 0 issues
  • make test — all packages pass
  • Independent cross-review of the diff vs. the design doc: all 14 checks COMPATIBLE (type shape, planner overrides, validation shape & rules, plan-task wiring on both non-bootstrap and post-bootstrap paths, exclusion from the bootstrap Job's own sidecar, samples, tests, generated files)

Test plan

  • Apply an updated sample SeiNode with publish: {} and observe snapshot-upload appearing in .status.plan between config-validate and mark-ready
  • Apply the same sample with publish omitted and confirm snapshot-upload is NOT in the plan; confirm seid still writes snapshots to the data volume at the configured interval
  • Apply a spec with publish: {} and keepRecent: 1 — expect the planner's Validate() to reject with a clear error mentioning the upload algorithm
  • Apply snapshotGeneration: {} (empty) — expect the planner's Validate() to reject as a typo-shape
  • Spot-check one existing snapshotter node: delete + recreate under the new shape; confirm the sidecar begins uploading after mark-ready
  • Review generated CRD YAML in manifests/sei.io_seinodes.yaml and manifests/sei.io_seinodedeployments.yaml for the new nested shape

🤖 Generated with Claude Code

Refactors SnapshotGenerationConfig to nest a TendermintSnapshotGenerationConfig
sub-struct (extensible for future snapshot modes) with an optional Publish
struct whose presence toggles S3 upload. Absence of Publish means snapshots
are kept on disk only.

Re-wires the existing seictl TaskTypeSnapshotUpload sidecar task as a
fire-and-forget plan task, inserted into the init progression immediately
before mark-ready only when Publish is set. Restores controller-side
plumbing that was removed in #89 with the monitor-task subsystem, now as
a plan task rather than a monitor task.

Cross-field validation lives in each per-mode planner's Validate(node) hook,
wrapping errors with its own mode prefix (fullNode: / archive:) — no shared
helper takes a mode parameter. Rules: empty snapshotGeneration is rejected;
keepRecent must be >= 2 when Publish is set (upload algorithm requires the
second-to-latest snapshot).

Implements the design in docs/design-snapshot-publish-config.md (merged
via #107). No backward-compat concerns: product has not launched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread internal/planner/bootstrap.go Outdated
Comment thread internal/planner/full.go
Comment thread internal/planner/planner.go Outdated
- Inline the snapshot-upload gate into buildBasePlan and
  buildPostBootstrapProgression; remove the maybeInsertSnapshotUpload
  helper (called in only two places, "maybe" prefix was unclear).
- Trim validateSnapshotGeneration doc comment to a single line — the
  rules are self-evident from the code; only the caller-wrap convention
  is worth calling out.

Tracking issue for the sei-config rename suggestion:
sei-protocol/sei-config#6

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bdchatham bdchatham merged commit 75fb627 into main Apr 21, 2026
2 checks passed
bdchatham added a commit that referenced this pull request Apr 21, 2026
Refactors the flat ResultExportConfig into a wrapper around use-case
sub-structs, following the pattern merged in #108. First (and currently
only) use case is ShadowResultConfig, which carries the CanonicalRPC
endpoint for app-hash divergence detection.

Removes stale "monitor mode" / "scheduled mode" godoc that described
a scheduled-export behavior that was never reachable (canonicalRpc is
CRD-required via MinLength=1, so "without canonicalRpc" was never a
valid input).

Adds cross-field validation to replayerPlanner.Validate rejecting an
empty resultExport wrapper, mirroring validateSnapshotGeneration at
planner.go:331. Context-free helper, caller wraps with replayer:
prefix — no shared helper takes a mode parameter.

No sidecar code currently consumes ResultExport (was removed with the
monitor-task subsystem in #89); this is a schema-only refactor to
prepare for future use-case sub-structs without needing to rename
existing fields.

Co-authored-by: Claude Opus 4.7 (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