Skip to content
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

Facilitate deletion of VeleroBackup object via NAB object #58

Open
shubham-pampattiwar opened this issue May 6, 2024 · 11 comments · May be fixed by #67
Open

Facilitate deletion of VeleroBackup object via NAB object #58

shubham-pampattiwar opened this issue May 6, 2024 · 11 comments · May be fixed by #67
Assignees

Comments

@shubham-pampattiwar
Copy link
Member

No description provided.

@mateusoliveira43
Copy link
Contributor

We also need similar logic for Restores ❓

@shubham-pampattiwar
Copy link
Member Author

yes definitely

@mpryc
Copy link
Collaborator

mpryc commented May 6, 2024

I was not sure about that. If we delete VeleroBackup won't we have problem with restore? I thought that VeleroBackup should not get deleted so it can be restored when namespace which includes NAB gets deleted as an example?

@shubham-pampattiwar
Copy link
Member Author

If the NAB object that created the VeleroBackup object gets deleted then the corresponding VeleroBackup should also get deleted, right ?

@shubham-pampattiwar
Copy link
Member Author

Non-Admin restores will be referring non-admin backups and not VeleroBackup objects

@mpryc
Copy link
Collaborator

mpryc commented May 7, 2024

Non-Admin restores will be referring non-admin backups and not VeleroBackup objects

Ok, but isn't VeleroBackup required to be on cluster to perform Restore operation? If not and we only need to know the name of the original VeleroBackup object stored in S3 or other storage then we are safe to delete.

@shubham-pampattiwar
Copy link
Member Author

yes VeleroBackup is needed for restore but we are not talking about performing a restore operation here, We are saying if a non-admin backup is deleted by the user then let's delete its corresponding VeleroBackup. There might be a scenario where we do not want to do this when the namespace deletion would trigger the NAB deletion and subsequently VeleroBackup deletion, we might need to somehow different between these 2 scenarios.

@mateusoliveira43
Copy link
Contributor

If we do not delete Velero Backup when deleting NAB, sync controller will recreate it, right ❓

this would mean, in the end, that non admin users can not delete their backups, right ❓

@shubham-pampattiwar
Copy link
Member Author

shubham-pampattiwar commented May 8, 2024

There are most likely 2 cases when NAB CR gets deleted:

  • User deletes the NAB CR
  • User deletes the NS which causes a cascade delete of NAB CR
    For case 1 we want to delete the corresponding VeleroBackup CR and for case 2 we are unsure whether we should delete the corresponding VeleroBackup CR.

One way of solving this for both cases would a user input opt-in based deletion. By default the NAB CR has an annotation applied by our NAB controller like oadp.io/preserve: true and when the user wants to delete the NAB CR they could just mark this annotation as false and we could process the update event and carry out the corresponding VeleroBackup delete. Discussed this approach today with @mpryc

@shawn-hurley
Copy link

I would prefer that the mechanism be an actual spec field. This will allow us to version this field, which can become important in the future.

@kaovilai
Copy link
Member

kaovilai commented May 8, 2024

If we add finalizer to NAB then we will have time to check if namespace was deleted as well, or just the NAB. Then we can automate from there either velero backup deletion or honor annotation/spec to preserve velero backup for NAB recreation on namespace recreation.

mpryc added a commit to mpryc/oadp-non-admin that referenced this issue May 14, 2024
Enhancement which will delete original VeleroBackup when
the user sets the deleteVeleroBackup witin NonAdminBackup Spec.

Fixes Issue migtools#58

Signed-off-by: Michal Pryc <[email protected]>
mpryc added a commit to mpryc/oadp-non-admin that referenced this issue May 14, 2024
Enhancement which will delete original VeleroBackup when
the user sets the deleteVeleroBackup witin NonAdminBackup Spec.

Fixes Issue migtools#58

Signed-off-by: Michal Pryc <[email protected]>
mpryc added a commit to mpryc/oadp-non-admin that referenced this issue May 14, 2024
Enhancement which will delete original VeleroBackup when
the user sets the deleteVeleroBackup witin NonAdminBackup Spec.

Fixes Issue migtools#58

Signed-off-by: Michal Pryc <[email protected]>
mpryc added a commit to mpryc/oadp-non-admin that referenced this issue May 14, 2024
Enhancement which will delete original VeleroBackup when
the user sets the deleteVeleroBackup witin NonAdminBackup Spec.

Fixes Issue migtools#58

Signed-off-by: Michal Pryc <[email protected]>
@mpryc mpryc self-assigned this May 14, 2024
@shubham-pampattiwar shubham-pampattiwar changed the title Delete VeleroBackup if NAB object gets deleted Facilitate deletion of VeleroBackup object via NAB object May 14, 2024
mpryc added a commit to mpryc/oadp-non-admin that referenced this issue May 20, 2024
Enhancement which will delete original VeleroBackup when
the user sets the deleteVeleroBackup witin NonAdminBackup Spec.

Fixes Issue migtools#58

Signed-off-by: Michal Pryc <[email protected]>
mpryc added a commit to mpryc/oadp-non-admin that referenced this issue May 21, 2024
Enhancement which will delete original VeleroBackup when
the user sets the deleteVeleroBackup witin NonAdminBackup Spec.

Fixes Issue migtools#58

Signed-off-by: Michal Pryc <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

5 participants