validator and type updates

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
openshift · Aug 11, 2023 · 99b86dd · 99b86dd
1 parent 27cccab
commit 99b86dd
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 21 deletions.
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/controllers/bsl.go b/controllers/bsl.go
@@ -14,11 +14,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
-func (r *DPAReconciler) ValidateBackupStorageLocations(log logr.Logger) (bool, error) {
-	dpa := oadpv1alpha1.DataProtectionApplication{}
-	if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil {
-		return false, err
-	}
+func (r *DPAReconciler) ValidateBackupStorageLocations(dpa oadpv1alpha1.DataProtectionApplication) (bool, error) {
 	if dpa.Spec.Configuration == nil || dpa.Spec.Configuration.Velero == nil {
 		return false, errors.New("DPA CR Velero configuration cannot be nil")
 	}

diff --git a/controllers/bsl_test.go b/controllers/bsl_test.go
@@ -1218,7 +1218,7 @@ func TestDPAReconciler_ValidateBackupStorageLocations(t *testing.T) {
 				},
 				EventRecorder: record.NewFakeRecorder(10),
 			}
-			got, err := r.ValidateBackupStorageLocations(r.Log)
+			got, err := r.ValidateBackupStorageLocations(*tt.dpa)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("ValidateBackupStorageLocations() error = %v, wantErr %v", err, tt.wantErr)
 				return

diff --git a/controllers/dpa_controller.go b/controllers/dpa_controller.go
@@ -90,14 +90,12 @@ func (r *DPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
 	_, err := ReconcileBatch(r.Log,
 		r.ValidateDataProtectionCR,
 		r.ReconcileResticRestoreHelperConfig,
-		r.ValidateBackupStorageLocations,
 		r.ReconcileBackupStorageLocations,
 		r.ReconcileRegistrySecrets,
 		r.ReconcileRegistries,
 		r.ReconcileRegistrySVCs,
 		r.ReconcileRegistryRoutes,
 		r.ReconcileRegistryRouteConfigs,
-		r.ValidateVolumeSnapshotLocations,
 		r.LabelVSLSecrets,
 		r.ReconcileVolumeSnapshotLocations,
 		r.ReconcileVeleroDeployment,
@@ -214,6 +212,8 @@ type ReconcileFunc func(logr.Logger) (bool, error)
 // reconcileBatch steps through a list of reconcile functions until one returns
 // false or an error.
 func ReconcileBatch(l logr.Logger, reconcileFuncs ...ReconcileFunc) (bool, error) {
+	// TODO: #1127 DPAReconciler already have a logger, use it instead of passing to each reconcile functions
+	// TODO: #1128 Right now each reconcile functions call get for DPA, we can do it once and pass it to each function
 	for _, f := range reconcileFuncs {
 		if cont, err := f(l); !cont || err != nil {
 			return cont, err

diff --git a/controllers/validator.go b/controllers/validator.go
@@ -14,6 +14,9 @@ import (
 	"github.com/openshift/oadp-operator/pkg/credentials"
 )
 
+// ValidateDataProtectionCR function validates the DPA CR, returns true if valid, false otherwise
+// it calls other validation functions to validate the DPA CR
+// TODO: #1129 Clean up duplicate logic for validating backupstoragelocations and volumesnapshotlocations in dpa
 func (r *DPAReconciler) ValidateDataProtectionCR(log logr.Logger) (bool, error) {
 	dpa := oadpv1alpha1.DataProtectionApplication{}
 	if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil {
@@ -99,7 +102,8 @@ func (r *DPAReconciler) ValidateDataProtectionCR(log logr.Logger) (bool, error)
 	if _, err := getResticResourceReqs(&dpa); err != nil {
 		return false, err
 	}
-
+	r.ValidateBackupStorageLocations(dpa)
+	r.ValidateVolumeSnapshotLocations(dpa)
 	return true, nil
 }
 

diff --git a/controllers/validator_test.go b/controllers/validator_test.go
@@ -995,15 +995,13 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
 					},
 					Configuration: &oadpv1alpha1.ApplicationConfig{
 						Velero: &oadpv1alpha1.VeleroConfig{
-							DefaultPlugins: []oadpv1alpha1.DefaultPlugin{
-							},
+							DefaultPlugins: []oadpv1alpha1.DefaultPlugin{},
 						},
 					},
 					BackupImages: pointer.Bool(true),
 				},
 			},
-			objects: []client.Object{
-			},
+			objects: []client.Object{},
 			wantErr: true,
 			want:    false,
 		},
@@ -1027,8 +1025,7 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) {
 					},
 					Configuration: &oadpv1alpha1.ApplicationConfig{
 						Velero: &oadpv1alpha1.VeleroConfig{
-							DefaultPlugins: []oadpv1alpha1.DefaultPlugin{
-							},
+							DefaultPlugins: []oadpv1alpha1.DefaultPlugin{},
 						},
 					},
 					BackupImages: pointer.Bool(true),

diff --git a/controllers/vsl.go b/controllers/vsl.go
@@ -104,11 +104,7 @@ func (r *DPAReconciler) LabelVSLSecrets(log logr.Logger) (bool, error) {
 	return true, nil
 }
 
-func (r *DPAReconciler) ValidateVolumeSnapshotLocations(log logr.Logger) (bool, error) {
-	dpa := oadpv1alpha1.DataProtectionApplication{}
-	if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil {
-		return false, err
-	}
+func (r *DPAReconciler) ValidateVolumeSnapshotLocations(dpa oadpv1alpha1.DataProtectionApplication) (bool, error) {
 	if dpa.Spec.Configuration == nil {
 		return false, errors.New("application configuration not found")
 	}

diff --git a/controllers/vsl_test.go b/controllers/vsl_test.go
@@ -510,7 +510,7 @@ func TestDPAReconciler_ValidateVolumeSnapshotLocation(t *testing.T) {
 				},
 				EventRecorder: record.NewFakeRecorder(10),
 			}
-			got, err := r.ValidateVolumeSnapshotLocations(r.Log)
+			got, err := r.ValidateVolumeSnapshotLocations(*tt.dpa)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("ValidateVolumeSnapshotLocations() error = %v, wantErr %v", err, tt.wantErr)
 				return