From 15773337d19376f502df6bb51cbb430e5f894a38 Mon Sep 17 00:00:00 2001 From: Sachin Singla Date: Thu, 19 Sep 2024 21:57:12 +0530 Subject: [PATCH] OADP-3050: Remove unwanted BSL/VSLs on create/edit DPA --- controllers/bsl.go | 31 +++++++++++++++++++++++++ controllers/vsl.go | 32 ++++++++++++++++++++++++++ tests/e2e/dpa_deployment_suite_test.go | 25 +++++++++----------- 3 files changed, 74 insertions(+), 14 deletions(-) diff --git a/controllers/bsl.go b/controllers/bsl.go index 226fe99ff9..e181a120a4 100644 --- a/controllers/bsl.go +++ b/controllers/bsl.go @@ -3,6 +3,7 @@ package controllers import ( "errors" "fmt" + "slices" "github.com/go-logr/logr" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" @@ -100,6 +101,8 @@ func (r *DPAReconciler) ReconcileBackupStorageLocations(log logr.Logger) (bool, if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil { return false, err } + + dpaBSLNames := []string{} // Loop through all configured BSLs for i, bslSpec := range dpa.Spec.BackupLocations { // Create BSL as is, we can safely assume they are valid from @@ -110,6 +113,7 @@ func (r *DPAReconciler) ReconcileBackupStorageLocations(log logr.Logger) (bool, if bslSpec.Name != "" { bslName = bslSpec.Name } + dpaBSLNames = append(dpaBSLNames, bslName) bsl := velerov1.BackupStorageLocation{ ObjectMeta: metav1.ObjectMeta{ @@ -196,6 +200,33 @@ func (r *DPAReconciler) ReconcileBackupStorageLocations(log logr.Logger) (bool, ) } } + + dpaBSLs := velerov1.BackupStorageLocationList{} + dpaBSLLabels := map[string]string{ + "app.kubernetes.io/name": common.OADPOperatorVelero, + "app.kubernetes.io/managed-by": common.OADPOperator, + "app.kubernetes.io/component": "bsl", + } + err := r.List(r.Context, &dpaBSLs, client.InNamespace(r.NamespacedName.Namespace), client.MatchingLabels(dpaBSLLabels)) + if err != nil { + return false, err + } + // If current BSLs do not match the spec, delete extra BSLs + if len(dpaBSLNames) != len(dpaBSLs.Items) { + for _, bsl := range dpaBSLs.Items { + if !slices.Contains(dpaBSLNames, bsl.Name) { + if err := r.Delete(r.Context, &bsl); err != nil { + return false, err + } + // Record event for BSL deletion + r.EventRecorder.Event(&bsl, + corev1.EventTypeNormal, + "BackupStorageLocationDeleted", + fmt.Sprintf("BackupStorageLocation %s created by OADP in namespace %s was deleted as it was not in DPA spec.", bsl.Name, bsl.Namespace)) + } + } + } + return true, nil } diff --git a/controllers/vsl.go b/controllers/vsl.go index e65f9d8633..567a4ba6e7 100644 --- a/controllers/vsl.go +++ b/controllers/vsl.go @@ -3,12 +3,14 @@ package controllers import ( "errors" "fmt" + "slices" "strings" "github.com/go-logr/logr" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1" @@ -216,6 +218,7 @@ func (r *DPAReconciler) ReconcileVolumeSnapshotLocations(log logr.Logger) (bool, return false, err } + dpaVSLNames := []string{} // Loop through all configured VSLs for i, vslSpec := range dpa.Spec.SnapshotLocations { // Create VSL as is, we can safely assume they are valid from @@ -226,6 +229,7 @@ func (r *DPAReconciler) ReconcileVolumeSnapshotLocations(log logr.Logger) (bool, if vslSpec.Name != "" { vslName = vslSpec.Name } + dpaVSLNames = append(dpaVSLNames, vslName) vsl := velerov1.VolumeSnapshotLocation{ ObjectMeta: metav1.ObjectMeta{ @@ -272,6 +276,34 @@ func (r *DPAReconciler) ReconcileVolumeSnapshotLocations(log logr.Logger) (bool, } } + + dpaVSLs := velerov1.VolumeSnapshotLocationList{} + dpaVslLabels := map[string]string{ + "app.kubernetes.io/name": common.OADPOperatorVelero, + "app.kubernetes.io/managed-by": common.OADPOperator, + "app.kubernetes.io/component": "vsl", + } + err := r.List(r.Context, &dpaVSLs, client.InNamespace(r.NamespacedName.Namespace), client.MatchingLabels(dpaVslLabels)) + if err != nil { + return false, err + } + + // If current VSLs do not match the spec, delete extra VSLs + if len(dpaVSLNames) != len(dpaVSLs.Items) { + for _, vsl := range dpaVSLs.Items { + if !slices.Contains(dpaVSLNames, vsl.Name) { + if err := r.Delete(r.Context, &vsl); err != nil { + return false, err + } + // Record event for VSL deletion + r.EventRecorder.Event(&vsl, + corev1.EventTypeNormal, + "VolumeSnapshotLocationDeleted", + fmt.Sprintf("VolumeSnapshotLocation %s created by OADP in namespace %s was deleted as it was not in DPA spec.", vsl.Name, vsl.Namespace)) + } + } + } + return true, nil } diff --git a/tests/e2e/dpa_deployment_suite_test.go b/tests/e2e/dpa_deployment_suite_test.go index cf7b2e0eea..c42761115e 100644 --- a/tests/e2e/dpa_deployment_suite_test.go +++ b/tests/e2e/dpa_deployment_suite_test.go @@ -1,6 +1,7 @@ package e2e_test import ( + "fmt" "log" "strings" "time" @@ -190,14 +191,12 @@ var _ = ginkgo.Describe("Configuration testing for DPA Custom Resource", func() log.Printf("Checking for BSL spec") gomega.Expect(dpaCR.DoesBSLSpecMatchesDpa(namespace, *bsl.Velero)).To(gomega.BeTrue()) } + } else { + log.Println("Checking no BSLs are deployed") + _, err = dpaCR.ListBSLs() + gomega.Expect(err).To(gomega.HaveOccurred()) + gomega.Expect(err.Error()).To(gomega.Equal(fmt.Sprintf("no BSL in %s namespace", namespace))) } - // TODO bug - // else { - // log.Println("Checking no BSLs are deployed") - // _, err = dpaCR.ListBSLs() - // Expect(err).To(HaveOccurred()) - // Expect(err.Error()).To(Equal(fmt.Sprintf("no BSL in %s namespace", namespace))) - // } if len(installCase.DpaSpec.SnapshotLocations) > 0 { // TODO Check if VSLs are available creating new backup/restore test with VSL @@ -205,14 +204,12 @@ var _ = ginkgo.Describe("Configuration testing for DPA Custom Resource", func() log.Printf("Checking for VSL spec") gomega.Expect(dpaCR.DoesVSLSpecMatchesDpa(namespace, *vsl.Velero)).To(gomega.BeTrue()) } + } else { + log.Println("Checking no VSLs are deployed") + _, err = dpaCR.ListVSLs() + gomega.Expect(err).To(gomega.HaveOccurred()) + gomega.Expect(err.Error()).To(gomega.Equal(fmt.Sprintf("no VSL in %s namespace", namespace))) } - // TODO bug - // else { - // log.Println("Checking no VSLs are deployed") - // _, err = dpaCR.ListVSLs() - // Expect(err).To(HaveOccurred()) - // Expect(err.Error()).To(Equal(fmt.Sprintf("no VSL in %s namespace", namespace))) - // } if len(installCase.DpaSpec.Configuration.Velero.PodConfig.Tolerations) > 0 { log.Printf("Checking for velero tolerances")