Modify the Non Admin Backup API deletion to align with sync controller #148
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mpryc
force-pushed
the
bsl-deletion-workflow-update
branch
from
9189998 to
41f2d4d
Compare
Non Admin Backup delete event should remove corresponding Velero Backup This allows Velero Sync controller to recreate Velero Backup object and cascate such recreation back to the Non Admin Backup. The cascade action is a responsibility of an Non Admin Sync controller. Signed-off-by: Michal Pryc <[email protected]>
mpryc
force-pushed
the
bsl-deletion-workflow-update
branch
from
41f2d4d to
9ee0f0f
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Removal of the ForceDelete from the Backup Spec: - The standard API delete will handle now deletion of the associated with Non Admin Backup object Velero Backup and Velero DownloadBackupRequests - If the deletion of DownloadBackupRequest fails, there is still Velero garbage colletion mechanism, which should clean those object from the cluster. User is still reqreuired to use deleteBackup Spec field to clean up all the Backup resources including S3 artefacts. Signed-off-by: Michal Pryc <[email protected]>
Remove unnecessary flag and improve implementation of the tests for the NAB controller. Signed-off-by: Michal Pryc <[email protected]>
|
/retest |
|
All the jobs are passing, is there anything else I should change in this PR to get it merged? |
mateusoliveira43
approved these changes
Jan 27, 2025
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mateusoliveira43, mpryc, shubham-pampattiwar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Non Admin Backup delete event should remove corresponding Velero Backup
This allows Velero Sync controller to recreate Velero Backup object and cascate such recreation back to the Non Admin Backup. The cascade action is a responsibility of an Non Admin Sync controller.
Why the changes were made
Those changes are made to allow sync controller that is implemented as part of the #38 implemented by #138 to recreate Non Admin Backup in unified way, meaning it will be recreated on the new Velero Backup recreation by the Velero Sync controller.
In other words with this behavior when user runs:
This will result in removal of associated Velero Backup (if was existing) and then removal of NAB finalizer.
Such action will cause s3 storage to still have the information about the Velero Backup, because the removal was not done via spec field and no DeleteBackupRequest was created.
With the s3 storage containing information about Velero Backup, the Velero Sync Controller will recreate the Velero Backup object in the
openshift-adpnamespace. That event should trigger Non Admin Backup Sync Controller implemented as part of the #138 to recreate Non Admin Backup in the user's namespace.How to test the changes made
e2e tests were written, run simulation tests