-
Notifications
You must be signed in to change notification settings - Fork 1
BREAKING CHANGE: rename ReadyToUse to SnapshotCaptured #195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 performs a comprehensive rename of the ReadyToUse terminology to SnapshotCaptured across the entire Mantle backup and restore system. This change affects API types, controller logic, tests, and documentation to better reflect that the condition indicates when a snapshot has been successfully captured, rather than using the generic "ready to use" terminology.
Key Changes:
- Renamed API condition types from
ReadyToUsetoSnapshotCapturedfor both MantleBackup and MantleRestore resources - Updated all method names from
IsReady()toIsSnapshotCaptured()for both resource types - Updated condition reason constants and helper function names throughout the codebase
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| api/v1/mantlebackup_types.go | Renamed BackupConditionReadyToUse to BackupConditionSnapshotCaptured, updated reason constants, and renamed IsReady() method to IsSnapshotCaptured() |
| api/v1/mantlerestore_types.go | Renamed RestoreConditionReadyToUse to RestoreConditionSnapshotCaptured, updated comments, and renamed IsReady() method to IsSnapshotCaptured() |
| internal/controller/mantlebackup_controller.go | Updated all references to use IsSnapshotCaptured(), renamed struct field isSecondaryMantleBackupReadyToUse to isSecondaryMantleBackupSnapshotCaptured, and updated condition types and reasons |
| internal/controller/mantlebackup_controller_test.go | Updated all test assertions to use IsSnapshotCaptured(), renamed helper function parameters and updated condition types throughout tests |
| internal/controller/mantlerestore_controller.go | Updated condition type from RestoreConditionReadyToUse to RestoreConditionSnapshotCaptured and changed method calls to IsSnapshotCaptured() |
| internal/controller/mantlerestore_controller_test.go | Updated test setup to use renamed WaitForBackupSnapshotCaptured() helper |
| internal/controller/replication.go | Updated condition check to use IsSnapshotCaptured() and updated error message text |
| internal/testutil/resources.go | Renamed WaitForBackupReady() to WaitForBackupSnapshotCaptured() and updated internal method call |
| test/e2e/singlek8s/util.go | Renamed isMantleBackupReady() to isMantleBackupSnapshotCaptured() and updated isMantleRestoreReady() to call IsSnapshotCaptured() |
| test/e2e/singlek8s/backup_test.go | Updated test descriptions and function calls to use new SnapshotCaptured terminology |
| test/e2e/singlek8s/restore_test.go | Updated test description and assertion to use IsSnapshotCaptured() |
| test/e2e/multik8s/testutil/util.go | Renamed WaitMantleBackupReadyToUse() to WaitMantleBackupSnapshotCaptured() and updated comments and error messages |
| test/e2e/multik8s/misc_test.go | Updated test assertions to use IsSnapshotCaptured() |
| test/e2e/multik8s/full_backup_test.go | Updated condition check and error message to use SnapshotCaptured terminology |
| test/e2e/multik8s/change_to_secondary_test.go | Updated function call to use renamed WaitMantleBackupSnapshotCaptured() |
| test/e2e/multik8s/change_to_primary_test.go | Updated function call to use renamed WaitMantleBackupSnapshotCaptured() |
| docs/design.md | Updated documentation examples to show SnapshotCaptured condition type instead of ReadyToUse |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ebeca0a to
7a68790
Compare
d7f339d to
2ad8bc3
Compare
9135ec2 to
1b4f7df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ed808ec to
ffcf527
Compare
|
@llamerada-jp I've fixed my code in the commit 80ab2f3. Could you review this again? I'll merge this PR next year, so we don't have to rush. |
llamerada-jp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
80ab2f3 to
056a3cf
Compare
This commit renames MantleBackup's ReadyToUse condition to SnapshotCaptured. This is a breaking change; before deploying it, users should stop all of the mantle-controller Pods in the primary and secondary clusters, patch the conditions by hand, and then start the new controller. Signed-off-by: Ryotaro Banno <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
056a3cf to
a4ddae2
Compare
Don't merge this PR until the release process is completed.This PR has a breaking change, so please release and deploy it carefully.In addition, this PR should be merged after #197 is merged.done.This PR should be merged after #220.Done.This commit renames MantleBackup's ReadyToUse condition to
SnapshotCaptured. This is a breaking change; before deploying it, users
should stop all of the mantle-controller Pods in the primary and
secondary clusters, patch the conditions by hand, and then start the new
controller.