From f164b11422a2fe2157d137a5bc4d1b0048331bf0 Mon Sep 17 00:00:00 2001 From: gnolong <46259301+gnolong@users.noreply.github.com> Date: Mon, 6 May 2024 19:36:57 +0800 Subject: [PATCH] fix: set backuppolicy.spec.backofflimit to default value (#7192) Set backofflimit to default value when creating backuppolicy (cherry picked from commit 4b299f244efe534f2214930054bfcf422ec8b7a4) --- apis/apps/v1alpha1/backuppolicytemplate_types.go | 1 + apis/dataprotection/v1alpha1/backuppolicy_types.go | 1 + .../apps.kubeblocks.io_backuppolicytemplates.yaml | 1 + ...dataprotection.kubeblocks.io_backuppolicies.yaml | 1 + .../apps/transformer_cluster_backup_policy.go | 1 - .../dataprotection/backuppolicy_controller.go | 1 + .../dataprotection/backuppolicy_controller_test.go | 13 +++++++++++++ .../apps.kubeblocks.io_backuppolicytemplates.yaml | 1 + ...dataprotection.kubeblocks.io_backuppolicies.yaml | 1 + 9 files changed, 20 insertions(+), 1 deletion(-) diff --git a/apis/apps/v1alpha1/backuppolicytemplate_types.go b/apis/apps/v1alpha1/backuppolicytemplate_types.go index 93a7bd82e3f..d571752f610 100644 --- a/apis/apps/v1alpha1/backuppolicytemplate_types.go +++ b/apis/apps/v1alpha1/backuppolicytemplate_types.go @@ -95,6 +95,7 @@ type BackupPolicy struct { // Specifies the maximum number of retry attempts for a backup before it is considered a failure. // // +optional + // +kubebuilder:default=2 // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=10 BackoffLimit *int32 `json:"backoffLimit,omitempty"` diff --git a/apis/dataprotection/v1alpha1/backuppolicy_types.go b/apis/dataprotection/v1alpha1/backuppolicy_types.go index 130b9aed41f..92527c5c17e 100644 --- a/apis/dataprotection/v1alpha1/backuppolicy_types.go +++ b/apis/dataprotection/v1alpha1/backuppolicy_types.go @@ -40,6 +40,7 @@ type BackupPolicySpec struct { // Specifies the number of retries before marking the backup as failed. // // +optional + // +kubebuilder:default=2 // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=10 BackoffLimit *int32 `json:"backoffLimit,omitempty"` diff --git a/config/crd/bases/apps.kubeblocks.io_backuppolicytemplates.yaml b/config/crd/bases/apps.kubeblocks.io_backuppolicytemplates.yaml index 8ecb903635f..cb89fd2f317 100644 --- a/config/crd/bases/apps.kubeblocks.io_backuppolicytemplates.yaml +++ b/config/crd/bases/apps.kubeblocks.io_backuppolicytemplates.yaml @@ -63,6 +63,7 @@ spec: are different versions of definitions of the same component. properties: backoffLimit: + default: 2 description: Specifies the maximum number of retry attempts for a backup before it is considered a failure. format: int32 diff --git a/config/crd/bases/dataprotection.kubeblocks.io_backuppolicies.yaml b/config/crd/bases/dataprotection.kubeblocks.io_backuppolicies.yaml index 94238b78ae2..b107e98b163 100644 --- a/config/crd/bases/dataprotection.kubeblocks.io_backuppolicies.yaml +++ b/config/crd/bases/dataprotection.kubeblocks.io_backuppolicies.yaml @@ -50,6 +50,7 @@ spec: description: BackupPolicySpec defines the desired state of BackupPolicy properties: backoffLimit: + default: 2 description: Specifies the number of retries before marking the backup as failed. format: int32 diff --git a/controllers/apps/transformer_cluster_backup_policy.go b/controllers/apps/transformer_cluster_backup_policy.go index fb4243caeea..f1d9172d0b8 100644 --- a/controllers/apps/transformer_cluster_backup_policy.go +++ b/controllers/apps/transformer_cluster_backup_policy.go @@ -349,7 +349,6 @@ func (r *clusterBackupPolicyTransformer) syncBackupPolicy(comp *appsv1alpha1.Clu backupPolicy.Spec.BackupRepoName = &r.Cluster.Spec.Backup.RepoName } backupPolicy.Spec.BackoffLimit = r.backupPolicy.BackoffLimit - r.syncBackupMethods(backupPolicy, comp) r.syncBackupPolicyTargetSpec(backupPolicy, comp) } diff --git a/controllers/dataprotection/backuppolicy_controller.go b/controllers/dataprotection/backuppolicy_controller.go index 5e78b30c571..bd0bcff3f02 100644 --- a/controllers/dataprotection/backuppolicy_controller.go +++ b/controllers/dataprotection/backuppolicy_controller.go @@ -89,6 +89,7 @@ func (r *BackupPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request } return intctrlutil.Reconciled() } + if err = patchStatus(dpv1alpha1.AvailablePhase, ""); err != nil { return intctrlutil.CheckedRequeueWithError(err, reqCtx.Log, "") } diff --git a/controllers/dataprotection/backuppolicy_controller_test.go b/controllers/dataprotection/backuppolicy_controller_test.go index ba14de7185f..d2625fa1c65 100644 --- a/controllers/dataprotection/backuppolicy_controller_test.go +++ b/controllers/dataprotection/backuppolicy_controller_test.go @@ -28,6 +28,7 @@ import ( dpv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1" "github.com/apecloud/kubeblocks/pkg/constant" + dptypes "github.com/apecloud/kubeblocks/pkg/dataprotection/types" "github.com/apecloud/kubeblocks/pkg/generics" testapps "github.com/apecloud/kubeblocks/pkg/testutil/apps" testdp "github.com/apecloud/kubeblocks/pkg/testutil/dataprotection" @@ -60,6 +61,18 @@ var _ = Describe("BackupPolicy Controller test", func() { }) Context("create a backup policy", func() { + It("test backup policy without setting backoffLimit", func() { + By("creating backupPolicy without setting backoffLimit") + bp := testdp.NewFakeBackupPolicy(&testCtx, func(backupPolicy *dpv1alpha1.BackupPolicy) { + backupPolicy.Spec.BackoffLimit = nil + }) + By("expect its backoffLimit should be set to the value of DefaultBackOffLimit") + Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(bp), + func(g Gomega, bp *dpv1alpha1.BackupPolicy) { + g.Expect(*bp.Spec.BackoffLimit).Should(Equal(dptypes.DefaultBackOffLimit)) + })).Should(Succeed()) + }) + It("backup policy should be available for target", func() { By("creating actionSet used by backup policy") as := testdp.NewFakeActionSet(&testCtx) diff --git a/deploy/helm/crds/apps.kubeblocks.io_backuppolicytemplates.yaml b/deploy/helm/crds/apps.kubeblocks.io_backuppolicytemplates.yaml index 8ecb903635f..cb89fd2f317 100644 --- a/deploy/helm/crds/apps.kubeblocks.io_backuppolicytemplates.yaml +++ b/deploy/helm/crds/apps.kubeblocks.io_backuppolicytemplates.yaml @@ -63,6 +63,7 @@ spec: are different versions of definitions of the same component. properties: backoffLimit: + default: 2 description: Specifies the maximum number of retry attempts for a backup before it is considered a failure. format: int32 diff --git a/deploy/helm/crds/dataprotection.kubeblocks.io_backuppolicies.yaml b/deploy/helm/crds/dataprotection.kubeblocks.io_backuppolicies.yaml index 94238b78ae2..b107e98b163 100644 --- a/deploy/helm/crds/dataprotection.kubeblocks.io_backuppolicies.yaml +++ b/deploy/helm/crds/dataprotection.kubeblocks.io_backuppolicies.yaml @@ -50,6 +50,7 @@ spec: description: BackupPolicySpec defines the desired state of BackupPolicy properties: backoffLimit: + default: 2 description: Specifies the number of retries before marking the backup as failed. format: int32