fix: auto-assign default backup method when cluster backup method is empty#25
Conversation
|
zijiren/kubeblocks:backup-method-1.0.2 |
There was a problem hiding this comment.
Pull request overview
This PR updates the data-protection controller to automatically infer and persist a default spec.backup.method on a Cluster when it is empty, using the ComponentDefinition’s serviceKind or ComponentDefinition name (with fallback to known default methods).
Changes:
- Add default backup method inference logic and patch the Cluster when
spec.backup.methodis empty. - Update backup schedule reconciliation to propagate patch errors (instead of ignoring them).
- Add a controller test covering defaulting behavior when backup is enabled and the method is empty.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| controllers/dataprotection/backuppolicydriver_controller.go | Infers and patches a default Cluster backup method when empty; updates merge flow to return errors; expands RBAC verbs. |
| controllers/dataprotection/backuppolicydriver_controller_test.go | Adds a test asserting the Cluster backup method is defaulted and the schedule contains the defaulted method. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
controllers/dataprotection/backuppolicydriver_controller.go:615
mergeClusterBackupnow returns only an error, but it still contains logic to allocate a newBackupSchedulewhen the inputbackupScheduleis nil. Since the newly created schedule is not returned to the caller, any caller passing nil won’t be able to use it. Either remove this nil-creation branch (if impossible by design) or change the function contract (e.g., return the schedule) so the new object can be persisted by the caller.
// there is no backup schedule created by backup policy template, so we need to
// create a new backup schedule for cluster backup.
if backupSchedule == nil {
backupSchedule = &dpv1alpha1.BackupSchedule{
ObjectMeta: metav1.ObjectMeta{
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…empty When `spec.backup.method` is not set, the controller now automatically infers and patches a default backup method based on the component's ServiceKind or ComponentDefinition name, falling back to the first known default method present in the BackupPolicy. Port of fix from v0.9.3 (9b9afca) to v1.0.2
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
e072bf1 to
1365c27
Compare
When
spec.backup.methodis not set, the controller now automatically infers and patches a default backup method based on the component's ServiceKind or ComponentDefinition name, falling back to the first known default method present in the BackupPolicy.Port of fix from v0.9.3 (9b9afca) to v1.0.2