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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMPROVEMENT] Consider adding owner reference Backup/BackupVolume CR #5896

Closed
ChanYiLin opened this issue May 9, 2023 · 1 comment
Closed
Assignees
Labels
area/backup-store Remote backup store related component/longhorn-manager Longhorn manager (control plane) kind/improvement Request for improvement of existing function priority/0 Must be fixed in this release (managed by PO)
Milestone

Comments

@ChanYiLin
Copy link
Contributor

Is your improvement request related to a feature? Please describe (馃憤 if you like this request)

Currently Backup and BackupVolume don't have owner reference, it manually delete the Backup in the BackupVolume controller. We should consider adding the owner reference for better maintenance.

Describe the solution you'd like

  • Add ownerReference to Backup/BackupVolume
  • review the logic between Backup/BackupVolume controller to prevent redundant checking

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

Additional context

#5506
#1867

@ChanYiLin ChanYiLin added the kind/improvement Request for improvement of existing function label May 9, 2023
@innobead innobead added this to the v1.6.0 milestone May 9, 2023
@innobead innobead added component/longhorn-manager Longhorn manager (control plane) priority/0 Must be fixed in this release (managed by PO) area/backup-store Remote backup store related labels May 9, 2023
@innobead innobead changed the title [IMPROVEMENT] consider adding owner reference Backup/BackupVolume CR [IMPROVEMENT] Consider adding owner reference Backup/BackupVolume CR Aug 14, 2023
@ChanYiLin
Copy link
Contributor Author

We found in some cases we can't add ownerreference
When the Volume creates the backup for the first time, it actually creates the backup CR first and request the engine to create backup to backupstore. It also creates the backupvolume meta in backupstore at this moment.
After that, Backup target controller then syncs the the meta from backupstore back to create backupvolume.

We can't add owner reference to backup with backupvolume, because backupvolume is created after backup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/backup-store Remote backup store related component/longhorn-manager Longhorn manager (control plane) kind/improvement Request for improvement of existing function priority/0 Must be fixed in this release (managed by PO)
Projects
None yet
Development

No branches or pull requests

2 participants