Add Nebula scheduled backup CRD #416
Conversation
kevinliu24
added
do not review
|
|
pkg/controller/nebulascheduledbackup/nebula_scheduled_backup_control.go
Outdated
Show resolved
Hide resolved
pkg/controller/nebulascheduledbackup/nebula_scheduled_backup_manager.go
Outdated
Show resolved
Hide resolved
pkg/controller/nebulascheduledbackup/nebula_scheduled_backup_manager.go
Outdated
Show resolved
Hide resolved
pkg/controller/nebulascheduledbackup/nebula_scheduled_backup_manager.go
Outdated
Show resolved
Hide resolved
pkg/controller/nebulascheduledbackup/nebula_scheduled_backup_manager.go
Outdated
Show resolved
Hide resolved
pkg/controller/nebulascheduledbackup/nebula_scheduled_backup_manager.go
Outdated
Show resolved
Hide resolved
pkg/controller/nebulascheduledbackup/nebula_scheduled_backup_manager.go
Outdated
Show resolved
Hide resolved
pkg/controller/nebulascheduledbackup/nebula_scheduled_backup_manager.go
Outdated
Show resolved
Hide resolved
pkg/controller/nebulascheduledbackup/nebula_scheduled_backup_manager.go
Outdated
Show resolved
Hide resolved
pkg/controller/nebulascheduledbackup/nebula_scheduled_backup_manager.go
Outdated
Show resolved
Hide resolved
pkg/controller/nebulascheduledbackup/nebula_scheduled_backup_manager.go
Outdated
Show resolved
Hide resolved
| MaxSuccessfulNebulaBackupJobs *int32 `json:"maxSuccessfulNebulaBackupJobs,omitempty"` | ||
| // MaxFailedNebulaBackupJobs specifies the maximum number of failed backup jobs to keep. Default 3 | ||
| MaxFailedNebulaBackupJobs *int32 `json:"maxFailedNebulaBackupJobs,omitempty"` |
There was a problem hiding this comment.
Why are these fields necessary? I believe users aren't particularly concerned with how many NebulaBackups need to be retained. This is precisely the task for NebulaScheduled. It determines retention based on MaxBackups and MaxRetentionTime.
Additionally, I think the lifecycle of NebulaBackup should be responsible for determining when the corresponding storage is deleted. In other words, when NebulaBackup is deleted, the corresponding storage should also be deleted, maintaining a consistent lifecycle.
NebulaScheduled only operates on NebulaBackup, deleting relevant resources when NebulaBackup is removed.
There was a problem hiding this comment.
Yes, MaxBackups and MaxRetentionTime is for determining how many backups to keep in either the S3 bucket or the gcp bucket. These parameters are there to specify how many backup jobs the user wants to keep as the user may want to keep the backup itself but not the actual job that created it. This also allows the user to keep all failed backup logs if needed. @MegaByte875 what do you think?
| // if both MaxBackups and MaxReservedTime are set at the same time, MaxReservedTime will be used | ||
| // and MaxBackups is ignored. | ||
| MaxBackups *int32 `json:"maxBackups,omitempty"` | ||
| // MaxReservedTime is to specify how long backups we want to keep. | ||
| MaxReservedTime *string `json:"maxReservedTime,omitempty"` | ||
| // BackupTemplate is the specification of the backup structure to get scheduled. | ||
| // MaxRetentionTime specifies how long we want the backups in the remote storage bucket to be kept for. |
There was a problem hiding this comment.
Shouldn't it be the case that if either of these two parameters meets the condition, it should be deleted? Instead of setting both parameters and using only one as the basis for judgement.
There was a problem hiding this comment.
The current design is to use MaxReservedTime if both are specified as this is simpler understand for the customer and easier to implement. Otherwise we don't know whether the customer means "delete a backup if there are more then MaxBackups and the backup is older than MaxReservedTime" or "delete a backup if there are more then MaxBackups or if the backup if older than MaxReservedTime".
We can definitely think about which version we want to support and change the design to accommodate both parameters in a future version if there's customer demand for this. What do you think @MegaByte875
There was a problem hiding this comment.
Here's an example for a simple task: a log cleaning function. There are two settings: one for the maximum number of days to be retained, and one for the maximum storage to be kept. If a user configures both, how should we proceed?
I assume that the user wants to keep a certain number of days, but they also don't want to consume too much storage.
Returning to this issue, I think it's similar. By the way, if the user only wants to retain a certain number of days, why would they configure MaxBackups?
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
#404
Description:
Add Nebula Scheduled backup CRD
How do you solve it?
Added a CRD for a Nebula scheduled backup type
Special notes for your reviewer, ex. impact of this fix, design document, etc: