Skip to content

Support specifying backupVersion in Restore Spec #2317

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

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

millasml
Copy link
Contributor

@millasml millasml commented Jul 7, 2025

Description

Support specifying backupVersion in Restore Spec. Passed into the -v option in fdbrestore

Type of change

  • New feature (non-breaking change which adds functionality)

Discussion

Are there any design details that you would like to discuss further?

Testing

Please describe the tests that you ran to verify your changes. Unit tests?
Manual testing?

Do we need to perform additional testing once this is merged, or perform in a larger testing environment?

Documentation

updated docs/restore_spec.md

Follow-up

Are there any follow-up issues that we should pursue in the future?

Does this introduce new defaults that we should re-evaluate in the future?

@johscheuer johscheuer self-requested a review July 7, 2025 15:47
@millasml millasml requested a review from johscheuer July 8, 2025 02:08
Copy link
Member

@johscheuer johscheuer left a comment

Choose a reason for hiding this comment

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

We have one backup e2e test case (https://github.com/FoundationDB/fdb-kubernetes-operator/blob/main/e2e/test_operator_backups/operator_backup_test.go), could you add a new test case for the new feature?

@millasml millasml force-pushed the add-restore-versionstamp branch from 8bdac29 to 9f0e18e Compare July 11, 2025 02:06
@millasml millasml requested a review from johscheuer July 15, 2025 01:39
@millasml millasml requested a review from johscheuer July 17, 2025 09:03
@millasml
Copy link
Contributor Author

@johscheuer bump on this

Copy link
Member

@johscheuer johscheuer left a comment

Choose a reason for hiding this comment

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

Were you able to run the e2e test cases?

AfterEach(func() {
backup.Stop()
log.Println("deleting backup")
factory.Delete(backup.GetBackupPod())
Copy link
Member

Choose a reason for hiding this comment

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

Your current setup deletes a backup agent pod not the actual backup.

@@ -916,8 +916,7 @@ func (client *cliAdminClient) GetBackupStatus() (*fdbv1beta2.FoundationDBLiveBac
// StartRestore starts a new restore.
func (client *cliAdminClient) StartRestore(
url string,
keyRanges []fdbv1beta2.FoundationDBKeyRange,
encryptionKeyPath string,
fdbRestoreSpec fdbv1beta2.FoundationDBRestoreSpec,
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to pass down the spec and not the *fdbv1beta2.FoundationDBRestore? I would prefer to pass down the *fdbv1beta2.FoundationDBRestore.

@foundationdb-ci
Copy link
Contributor

Result of fdb-kubernetes-operator-pr on Linux RHEL 9

  • Commit ID: 84185ad
  • Duration 4:02:05
  • Result: ❌ FAILED
  • Error: Error while executing command: if $fail_test; then exit 1; fi. Reason: exit status 1
  • Build Log terminal output (available for 30 days)
  • Build Workspace zip file of the working directory (available for 30 days)

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.

3 participants