From d82c2b372c7c14bfe3517c99e4b890b620a741db Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Fri, 31 Jan 2025 12:08:24 -0300 Subject: [PATCH 1/3] feat: Garbage collector input Signed-off-by: Mateus Oliveira --- .../dataprotectionapplication_types.go | 5 ++ api/v1alpha1/zz_generated.deepcopy.go | 5 ++ ...enshift.io_dataprotectionapplications.yaml | 5 ++ ...enshift.io_dataprotectionapplications.yaml | 5 ++ internal/controller/validator.go | 60 +++++++++++-------- 5 files changed, 54 insertions(+), 26 deletions(-) diff --git a/api/v1alpha1/dataprotectionapplication_types.go b/api/v1alpha1/dataprotectionapplication_types.go index 6209a49a03..890704ded4 100644 --- a/api/v1alpha1/dataprotectionapplication_types.go +++ b/api/v1alpha1/dataprotectionapplication_types.go @@ -447,6 +447,11 @@ type NonAdmin struct { // which restore spec field values to enforce // +optional EnforceRestoreSpec *velero.RestoreSpec `json:"enforceRestoreSpec,omitempty"` + + // GarbageCollectionPeriod defines how frequently to look for possible leftover non admin objects in OADP namespace. + // By default 5m + // +optional + GarbageCollectionPeriod *metav1.Duration `json:"garbageCollectionPeriod,omitempty"` } // DataMover defines the various config for DPA data mover diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 7094be2853..6c3750811f 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -609,6 +609,11 @@ func (in *NonAdmin) DeepCopyInto(out *NonAdmin) { *out = new(velerov1.RestoreSpec) (*in).DeepCopyInto(*out) } + if in.GarbageCollectionPeriod != nil { + in, out := &in.GarbageCollectionPeriod, &out.GarbageCollectionPeriod + *out = new(metav1.Duration) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NonAdmin. diff --git a/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml b/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml index 4583a755cc..ed21f0d7f7 100644 --- a/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml +++ b/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml @@ -2123,6 +2123,11 @@ spec: type: boolean type: object type: object + garbageCollectionPeriod: + description: |- + GarbageCollectionPeriod defines how frequently to look for possible leftover non admin objects in OADP namespace. + By default 5m + type: string type: object podAnnotations: additionalProperties: diff --git a/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml b/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml index f9ebdd3597..42b444b0df 100644 --- a/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml +++ b/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml @@ -2123,6 +2123,11 @@ spec: type: boolean type: object type: object + garbageCollectionPeriod: + description: |- + GarbageCollectionPeriod defines how frequently to look for possible leftover non admin objects in OADP namespace. + By default 5m + type: string type: object podAnnotations: additionalProperties: diff --git a/internal/controller/validator.go b/internal/controller/validator.go index 86d74231fa..e986ec733e 100644 --- a/internal/controller/validator.go +++ b/internal/controller/validator.go @@ -83,37 +83,45 @@ func (r *DataProtectionApplicationReconciler) ValidateDataProtectionCR(log logr. } // validate non-admin enable and tech-preview-ack - if r.dpa.Spec.NonAdmin != nil && r.dpa.Spec.NonAdmin.Enable != nil && *r.dpa.Spec.NonAdmin.Enable { - if !(r.dpa.Spec.UnsupportedOverrides[oadpv1alpha1.TechPreviewAck] == TrueVal) { - return false, errors.New("in order to enable/disable the non-admin feature please set dpa.spec.unsupportedOverrides[tech-preview-ack]: \"true\"") - } + if r.dpa.Spec.NonAdmin != nil { + if r.dpa.Spec.NonAdmin.Enable != nil && *r.dpa.Spec.NonAdmin.Enable { + if r.dpa.Spec.UnsupportedOverrides[oadpv1alpha1.TechPreviewAck] != TrueVal { + return false, errors.New("in order to enable/disable the non-admin feature please set dpa.spec.unsupportedOverrides[tech-preview-ack]: \"true\"") + } - dpaList := &oadpv1alpha1.DataProtectionApplicationList{} - err = r.ClusterWideClient.List(r.Context, dpaList) - if err != nil { - return false, err - } - for _, dpa := range dpaList.Items { - if dpa.Namespace != r.NamespacedName.Namespace && (&DataProtectionApplicationReconciler{dpa: &dpa}).checkNonAdminEnabled() { - nonAdminDeployment := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: nonAdminObjectName, - Namespace: dpa.Namespace, - }, - } - if err := r.ClusterWideClient.Get( - r.Context, - types.NamespacedName{ - Name: nonAdminDeployment.Name, - Namespace: nonAdminDeployment.Namespace, - }, - nonAdminDeployment, - ); err == nil { - return false, fmt.Errorf("only a single instance of Non-Admin Controller can be installed across the entire cluster. Non-Admin controller is already configured and installed in %s namespace", dpa.Namespace) + dpaList := &oadpv1alpha1.DataProtectionApplicationList{} + err = r.ClusterWideClient.List(r.Context, dpaList) + if err != nil { + return false, err + } + for _, dpa := range dpaList.Items { + if dpa.Namespace != r.NamespacedName.Namespace && (&DataProtectionApplicationReconciler{dpa: &dpa}).checkNonAdminEnabled() { + nonAdminDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: nonAdminObjectName, + Namespace: dpa.Namespace, + }, + } + if err := r.ClusterWideClient.Get( + r.Context, + types.NamespacedName{ + Name: nonAdminDeployment.Name, + Namespace: nonAdminDeployment.Namespace, + }, + nonAdminDeployment, + ); err == nil { + return false, fmt.Errorf("only a single instance of Non-Admin Controller can be installed across the entire cluster. Non-Admin controller is already configured and installed in %s namespace", dpa.Namespace) + } } } } + garbageCollectionPeriod := r.dpa.Spec.NonAdmin.GarbageCollectionPeriod + if garbageCollectionPeriod != nil { + if garbageCollectionPeriod.Duration < 0 { + return false, fmt.Errorf("DPA spec.nonAdmin.garbageCollectionPeriod can not be negative") + } + } } return true, nil From 599210d1f6e6c6f824c53b3855d0d91831dbd119 Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Mon, 3 Feb 2025 18:43:04 -0300 Subject: [PATCH 2/3] fixup! feat: Garbage collector input Signed-off-by: Mateus Oliveira --- .../dataprotectionapplication_types.go | 2 +- ...enshift.io_dataprotectionapplications.yaml | 2 +- ...enshift.io_dataprotectionapplications.yaml | 2 +- internal/controller/nonadmin_controller.go | 25 +++++++++++++------ 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/api/v1alpha1/dataprotectionapplication_types.go b/api/v1alpha1/dataprotectionapplication_types.go index 890704ded4..cb8c0d1806 100644 --- a/api/v1alpha1/dataprotectionapplication_types.go +++ b/api/v1alpha1/dataprotectionapplication_types.go @@ -449,7 +449,7 @@ type NonAdmin struct { EnforceRestoreSpec *velero.RestoreSpec `json:"enforceRestoreSpec,omitempty"` // GarbageCollectionPeriod defines how frequently to look for possible leftover non admin objects in OADP namespace. - // By default 5m + // By default 24h // +optional GarbageCollectionPeriod *metav1.Duration `json:"garbageCollectionPeriod,omitempty"` } diff --git a/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml b/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml index ed21f0d7f7..027f1881f4 100644 --- a/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml +++ b/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml @@ -2126,7 +2126,7 @@ spec: garbageCollectionPeriod: description: |- GarbageCollectionPeriod defines how frequently to look for possible leftover non admin objects in OADP namespace. - By default 5m + By default 24h type: string type: object podAnnotations: diff --git a/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml b/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml index 42b444b0df..9885a3b7a2 100644 --- a/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml +++ b/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml @@ -2126,7 +2126,7 @@ spec: garbageCollectionPeriod: description: |- GarbageCollectionPeriod defines how frequently to look for possible leftover non admin objects in OADP namespace. - By default 5m + By default 24h type: string type: object podAnnotations: diff --git a/internal/controller/nonadmin_controller.go b/internal/controller/nonadmin_controller.go index 9405b86a7b..5d44c88c24 100644 --- a/internal/controller/nonadmin_controller.go +++ b/internal/controller/nonadmin_controller.go @@ -25,8 +25,9 @@ const ( nonAdminObjectName = "non-admin-controller" controlPlaneKey = "control-plane" - enforcedBackupSpecKey = "enforced-backup-spec" - enforcedRestoreSpecKey = "enforced-restore-spec" + enforcedBackupSpecKey = "enforced-backup-spec" + enforcedRestoreSpecKey = "enforced-restore-spec" + garbageCollectionPeriodKey = "garbage-collection-period" ) var ( @@ -42,10 +43,12 @@ var ( "app.kubernetes.io/part-of": common.OADPOperator, } - previousEnforcedBackupSpec *velero.BackupSpec = nil - dpaBackupSpecResourceVersion = "" - previousEnforcedRestoreSpec *velero.RestoreSpec = nil - dpaRestoreSpecResourceVersion = "" + previousEnforcedBackupSpec *velero.BackupSpec = nil + dpaBackupSpecResourceVersion = "" + previousEnforcedRestoreSpec *velero.RestoreSpec = nil + dpaRestoreSpecResourceVersion = "" + previousGarbageCollectionPeriod *metav1.Duration = nil + dpaGarbageCollectionResourceVersion = "" ) func (r *DataProtectionApplicationReconciler) ReconcileNonAdminController(log logr.Logger) (bool, error) { @@ -163,9 +166,14 @@ func ensureRequiredSpecs(deploymentObject *appsv1.Deployment, dpa *oadpv1alpha1. dpaRestoreSpecResourceVersion = dpa.GetResourceVersion() } previousEnforcedRestoreSpec = dpa.Spec.NonAdmin.EnforceRestoreSpec + if len(dpaGarbageCollectionResourceVersion) == 0 || !reflect.DeepEqual(dpa.Spec.NonAdmin.GarbageCollectionPeriod, previousGarbageCollectionPeriod) { + dpaGarbageCollectionResourceVersion = dpa.GetResourceVersion() + } + previousGarbageCollectionPeriod = dpa.Spec.NonAdmin.GarbageCollectionPeriod enforcedSpecAnnotation := map[string]string{ - enforcedBackupSpecKey: dpaBackupSpecResourceVersion, - enforcedRestoreSpecKey: dpaRestoreSpecResourceVersion, + enforcedBackupSpecKey: dpaBackupSpecResourceVersion, + enforcedRestoreSpecKey: dpaRestoreSpecResourceVersion, + garbageCollectionPeriodKey: dpaGarbageCollectionResourceVersion, } deploymentObject.Spec.Replicas = ptr.To(int32(1)) @@ -187,6 +195,7 @@ func ensureRequiredSpecs(deploymentObject *appsv1.Deployment, dpa *oadpv1alpha1. } else { templateObjectAnnotations[enforcedBackupSpecKey] = enforcedSpecAnnotation[enforcedBackupSpecKey] templateObjectAnnotations[enforcedRestoreSpecKey] = enforcedSpecAnnotation[enforcedRestoreSpecKey] + templateObjectAnnotations[garbageCollectionPeriodKey] = enforcedSpecAnnotation[garbageCollectionPeriodKey] deploymentObject.Spec.Template.SetAnnotations(templateObjectAnnotations) } From bd7d3cf67dbba3e7bb351cca0c7b8333e7147a08 Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Tue, 4 Feb 2025 17:27:13 -0300 Subject: [PATCH 3/3] fixup! feat: Garbage collector input Signed-off-by: Mateus Oliveira --- api/v1alpha1/dataprotectionapplication_types.go | 2 +- .../manifests/oadp.openshift.io_dataprotectionapplications.yaml | 2 +- .../crd/bases/oadp.openshift.io_dataprotectionapplications.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/v1alpha1/dataprotectionapplication_types.go b/api/v1alpha1/dataprotectionapplication_types.go index cb8c0d1806..9a77f4a9f3 100644 --- a/api/v1alpha1/dataprotectionapplication_types.go +++ b/api/v1alpha1/dataprotectionapplication_types.go @@ -448,7 +448,7 @@ type NonAdmin struct { // +optional EnforceRestoreSpec *velero.RestoreSpec `json:"enforceRestoreSpec,omitempty"` - // GarbageCollectionPeriod defines how frequently to look for possible leftover non admin objects in OADP namespace. + // GarbageCollectionPeriod defines how frequently to look for possible leftover non admin related objects in OADP namespace. // By default 24h // +optional GarbageCollectionPeriod *metav1.Duration `json:"garbageCollectionPeriod,omitempty"` diff --git a/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml b/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml index 027f1881f4..319ffc392c 100644 --- a/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml +++ b/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml @@ -2125,7 +2125,7 @@ spec: type: object garbageCollectionPeriod: description: |- - GarbageCollectionPeriod defines how frequently to look for possible leftover non admin objects in OADP namespace. + GarbageCollectionPeriod defines how frequently to look for possible leftover non admin related objects in OADP namespace. By default 24h type: string type: object diff --git a/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml b/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml index 9885a3b7a2..0a8ce9d13e 100644 --- a/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml +++ b/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml @@ -2125,7 +2125,7 @@ spec: type: object garbageCollectionPeriod: description: |- - GarbageCollectionPeriod defines how frequently to look for possible leftover non admin objects in OADP namespace. + GarbageCollectionPeriod defines how frequently to look for possible leftover non admin related objects in OADP namespace. By default 24h type: string type: object