-
Notifications
You must be signed in to change notification settings - Fork 168
K8SPSMDB-1520: fix restore from minio backupSource #2207
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
Conversation
Signed-off-by: Mayank Shah <mayank.shah@percona.com>
Signed-off-by: Mayank Shah <mayank.shah@percona.com>
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 fixes a validation issue where restoring a backup using MinIO as a backup source incorrectly rejected the operation. The fix adds MinIO to the list of valid backup source types in the validation logic and includes an E2E test to verify the functionality.
Changes:
- Updated
CheckFields()validation to include MinIO as a valid backup source option - Added E2E test for restore from MinIO backup source
- Updated error message to include MinIO in the list of valid options
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/apis/psmdb/v1/perconaservermongodbrestore_types.go | Added Minio nil check to validation and updated error message to include minio option |
| e2e-tests/demand-backup-physical-minio-native/run | Added test case for restore using backupSource with MinIO |
| e2e-tests/demand-backup-physical-minio-native/conf/restore-backupsource.yml | Added restore configuration file for backupSource testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e2e-tests/demand-backup-physical-minio-native/conf/restore-backupsource.yml
Outdated
Show resolved
Hide resolved
Signed-off-by: Mayank Shah <mayank.shah@percona.com>
|
@mayankshah1607 |
It is already fixed in a previous commit, that comment is outdated - c1bc4fd |
Signed-off-by: Mayank Shah <mayank.shah@percona.com>
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 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Oh, good catch, the e2e test was passing because it also specified storageName.. |
Signed-off-by: Mayank Shah <mayank.shah@percona.com>
commit: d97a62a |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Due to the high volume of requests, we're unable to provide free service for this account. To continue using the service, please upgarde to a paid plan.
CHANGE DESCRIPTION
Problem:
Restore a backup using
miniobackup source gives the following error:Cause:
CheckFields()does not consider minio settingsSolution:
Update
CheckFields()validation to take Minio configuration into accountCHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability