Skip to content

fix(rollback): bind rollback to requested apply snapshot#119

Open
aparajon wants to merge 1 commit into
mainfrom
armand/rollback-fixes
Open

fix(rollback): bind rollback to requested apply snapshot#119
aparajon wants to merge 1 commit into
mainfrom
armand/rollback-fixes

Conversation

@aparajon
Copy link
Copy Markdown
Collaborator

@aparajon aparajon commented May 18, 2026

Summary

  • Require rollback and rollback-confirm to name the apply ID being rolled back, then validate that apply belongs to the PR and requested environment.
  • Build and re-build rollback plans from the requested apply stored original schema while preserving server-side route metadata selected at plan time.
  • Attach rollback observers before dispatch so fast rollback applies still update stored check state and aggregate checks deterministically.
  • Keep Vitess per-shard progress recoverable when migration context appears after deploy start, including LocalScale reset behavior.

🤖 Generated by Codex.

Copilot AI review requested due to automatic review settings May 18, 2026 19:19
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates rollback planning to be based on the requested apply’s captured pre-apply schema, routes rollback planning through the normal Plan flow so local and gRPC clients share behavior, and extends Tern plan responses to include the captured “original schema” for later rollback planning and remote plan storage.

Changes:

  • Add original_schema to SchemaChange in the Tern proto and propagate it through LocalClient Plan responses and SchemaBot plan storage.
  • Replace the prior client-level RollbackPlan flow with service-level rollback planning (ExecuteRollbackPlanForApply) that calls the standard Plan path and stores the resulting plan.
  • Update webhook/API rollback handlers and tests to use apply-scoped rollback planning.

Reviewed changes

Copilot reviewed 12 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/webhook/rollback.go Rollback plan generation now uses the requested apply; rollback-confirm re-plans via the service.
pkg/tern/local_control.go Removes LocalClient RollbackPlan implementation (no longer needed).
pkg/tern/local_client.go Attaches stored OriginalSchema to plan response schema changes.
pkg/tern/local_client_integration_test.go Asserts OriginalSchema is present in Plan responses.
pkg/tern/grpc_client.go Removes unsupported RollbackPlan method.
pkg/tern/client.go Removes RollbackPlan from the tern.Client interface.
pkg/proto/tern.proto Adds original_schema field to SchemaChange.
pkg/proto/ternv1/tern.pb.go Regenerated protobuf bindings for original_schema.
pkg/proto/ternv1/tern_grpc.pb.go Regenerated gRPC bindings (version bump).
pkg/api/rollback_plan_integration_test.go Integration test ensuring rollback uses the requested apply’s original schema and stores rollback plan original schema.
pkg/api/proto_helpers.go Persists SchemaChange.OriginalSchema into stored plan namespaces.
pkg/api/plan_handlers.go Introduces ExecuteRollbackPlanForApply, factors out storePlanResponse, adds latest-completed-apply lookup.
pkg/api/handlers_test.go Enhances mock Tern client to capture/return Plan requests/responses.
pkg/api/control_handlers.go Rollback plan endpoint now plans from the requested apply (apply-scoped).
Files not reviewed (2)
  • pkg/proto/ternv1/tern.pb.go: Language not supported
  • pkg/proto/ternv1/tern_grpc.pb.go: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/tern/local_client.go Outdated
Comment thread pkg/webhook/rollback.go
@aparajon aparajon force-pushed the armand/rollback-fixes branch from 152eb57 to 8c18ebb Compare May 18, 2026 19:39
@aparajon aparajon changed the title fix(rollback): plan from requested apply state fix(rollback): require explicit apply ID May 18, 2026
@aparajon aparajon force-pushed the armand/rollback-fixes branch from 8c18ebb to ddca597 Compare May 18, 2026 19:55
@aparajon
Copy link
Copy Markdown
Collaborator Author

🤖 Note for whoever picks this up after #137: PR #137 does not obsolete this rollback PR. #137 only changes admin/control operations (cutover, stop, start, volume, revert, skip-revert) to require apply_id + environment and derive database/routing from storage.

This PR still owns rollback-specific behavior: building rollback plans from the requested apply's captured original schema, preventing fallback to the latest completed apply, requiring/validating the same apply ID through rollback-confirm, preserving original schema in Tern plan responses, and keeping the rollback webhook/integration coverage.

Expect a rebase/conflict cleanup with #137 in shared files like pkg/api/control_handlers.go, pkg/api/handlers_test.go, and TEMPLATES.md. A clearer post-rebase title might be fix(rollback): bind rollback to requested apply snapshot.

@aparajon aparajon force-pushed the armand/rollback-fixes branch from ddca597 to 04bb632 Compare May 21, 2026 21:09
@aparajon aparajon changed the title fix(rollback): require explicit apply ID fix(rollback): bind rollback to requested apply snapshot May 21, 2026
@aparajon aparajon marked this pull request as ready for review May 21, 2026 21:16
@aparajon aparajon requested review from Kiran01bm and morgo as code owners May 21, 2026 21:16
@aparajon aparajon force-pushed the armand/rollback-fixes branch 2 times, most recently from fc353e9 to 3310e73 Compare May 26, 2026 17:24
@aparajon aparajon force-pushed the armand/rollback-fixes branch from 3310e73 to 19b123d Compare May 26, 2026 20:30
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.

2 participants