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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Garbage collector input #1623

Merged
merged 3 commits into from
Feb 5, 2025
+71 −34
Merged

feat: Garbage collector input #1623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d82c2b3
feat: Garbage collector input
mateusoliveira43 Jan 31, 2025
599210d
fixup! feat: Garbage collector input
mateusoliveira43 Feb 3, 2025
bd7d3cf
fixup! feat: Garbage collector input
mateusoliveira43 Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions api/v1alpha1/dataprotectionapplication_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,11 @@ type NonAdmin struct {
// which restore spec field values to enforce
// +optional
EnforceRestoreSpec *velero.RestoreSpec `json:"enforceRestoreSpec,omitempty"`

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/non admin objects/non admin related objects
We are cleaning up residual Velero Objects and secrets, right ? (Not the actual Non-Admin prefixed objects)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, objects NAC controller created in OADP namespace and for some reason they were not properly deleted, GC will do this job

will make the change

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revise PR title/description of the same detail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not see other places that needs changing. Can you point it to me?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feat: Garbage collector input

do not convey that this GC is limited to NAC created resources in oadp-operator 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"`
}

// DataMover defines the various config for DPA data mover
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2123,6 +2123,11 @@ spec:
type: boolean
type: object
type: object
garbageCollectionPeriod:
description: |-
GarbageCollectionPeriod defines how frequently to look for possible leftover non admin related objects in OADP namespace.
By default 24h
type: string
type: object
podAnnotations:
additionalProperties:
Expand Down
5 changes: 5 additions & 0 deletions config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2123,6 +2123,11 @@ spec:
type: boolean
type: object
type: object
garbageCollectionPeriod:
description: |-
GarbageCollectionPeriod defines how frequently to look for possible leftover non admin related objects in OADP namespace.
By default 24h
type: string
type: object
podAnnotations:
additionalProperties:
Expand Down
25 changes: 17 additions & 8 deletions internal/controller/nonadmin_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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) {
Expand Down Expand Up @@ -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))
Expand All @@ -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)
}

Expand Down
60 changes: 34 additions & 26 deletions internal/controller/validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
mateusoliveira43 marked this conversation as resolved.
Show resolved Hide resolved
return false, fmt.Errorf("DPA spec.nonAdmin.garbageCollectionPeriod can not be negative")
}
}
}

return true, nil
Expand Down