Adjust the Status of NonAdminBackup

Moves Status outside of Spec and adjusts this to reflect Velero Backup status as well additional Status when the Spec within NonAdminBackup is not defined. Signed-off-by: Michal Pryc <[email protected]>
migtools · Mar 7, 2024 · 068dbc6 · 068dbc6
1 parent 5bdd750
commit 068dbc6
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 42 deletions.
diff --git a/api/v1alpha1/nonadminbackup_types.go b/api/v1alpha1/nonadminbackup_types.go
@@ -21,6 +21,21 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+// +kubebuilder:validation:Enum=Error;FailedValidation;New;Completed
+type NonAdminBackupPhase string
+
+const (
+	NonAdminBackupPhaseError NonAdminBackupPhase = "Error"
+
+	NonAdminBackupPhaseFailedValidation NonAdminBackupPhase = "FailedValidation"
+
+	// Used as a New object, that needs to be processed
+	// By NonAdminController
+	NonAdminBackupPhaseNew NonAdminBackupPhase = "New"
+
+	NonAdminBackupPhaseCompleted NonAdminBackupPhase = "Completed"
+)
+
 // NonAdminBackupSpec defines the desired state of NonAdminBackup
 type NonAdminBackupSpec struct {
 	// NonAdminBackup log level (use debug for the most logging, leave unset for default)
@@ -32,14 +47,21 @@ type NonAdminBackupSpec struct {
 
 	// BackupSpec defines the specification for a Velero backup.
 	BackupSpec *velerov1api.BackupSpec `json:"backupSpec,omitempty"`
-
-	// BackupStatus captures the current status of a Velero backup.
-	BackupStatus *velerov1api.BackupStatus `json:"backupStatus,omitempty"`
 }
 
 // NonAdminBackupStatus defines the observed state of NonAdminBackup
 type NonAdminBackupStatus struct {
 	Conditions []metav1.Condition `json:"conditions,omitempty"`
+
+	// +optional
+	Phase NonAdminBackupPhase `json:"phase,omitempty"`
+
+	// +optional
+	FailureReason string `json:"failureReason,omitempty"`
+
+	// BackupStatus captures the current status of a Velero backup.
+	// +optional
+	BackupStatus *velerov1api.BackupStatus `json:"backupStatus,omitempty"`
 }
 
 //+kubebuilder:object:root=true

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml b/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml
@@ -503,6 +503,22 @@ spec:
                       type: string
                     type: array
                 type: object
+              logLevel:
+                description: NonAdminBackup log level (use debug for the most logging,
+                  leave unset for default)
+                enum:
+                - trace
+                - debug
+                - info
+                - warning
+                - error
+                - fatal
+                - panic
+                type: string
+            type: object
+          status:
+            description: NonAdminBackupStatus defines the observed state of NonAdminBackup
+            properties:
               backupStatus:
                 description: BackupStatus captures the current status of a Velero
                   backup.
@@ -634,22 +650,6 @@ spec:
                       file in object storage.
                     type: integer
                 type: object
-              logLevel:
-                description: NonAdminBackup log level (use debug for the most logging,
-                  leave unset for default)
-                enum:
-                - trace
-                - debug
-                - info
-                - warning
-                - error
-                - fatal
-                - panic
-                type: string
-            type: object
-          status:
-            description: NonAdminBackupStatus defines the observed state of NonAdminBackup
-            properties:
               conditions:
                 items:
                   description: "Condition contains details for one aspect of the current
@@ -719,6 +719,15 @@ spec:
                   - type
                   type: object
                 type: array
+              failureReason:
+                type: string
+              phase:
+                enum:
+                - Error
+                - FailedValidation
+                - New
+                - Completed
+                type: string
             type: object
         type: object
     served: true

diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml
@@ -5,4 +5,4 @@ kind: Kustomization
 images:
 - name: controller
   newName: quay.io/migi/oadp-nac-operator
-  newTag: v0.0.37
+  newTag: v0.0.41
diff --git a/internal/controller/common_nab.go b/internal/controller/common_nab.go
@@ -55,22 +55,35 @@ func GenerateVeleroBackupName(namespace, nabName string) string {
 
 func UpdateNonAdminBackupFromVeleroBackup(ctx context.Context, r client.Client, log logr.Logger, nab *nacv1alpha1.NonAdminBackup, veleroBackup *velerov1api.Backup) error {
 	// Make a copy of the current status for comparison
-	oldStatus := nab.Spec.BackupStatus.DeepCopy()
+	oldStatus := nab.Status.BackupStatus.DeepCopy()
 	oldSpec := nab.Spec.BackupSpec.DeepCopy()
 
 	// Update the status & spec
-	nab.Spec.BackupStatus = &veleroBackup.Status
-	nab.Spec.BackupSpec = &veleroBackup.Spec
+	// Copy the status from veleroBackup.Status to nab.Status
+	nab.Status.BackupStatus = veleroBackup.Status.DeepCopy()
 
-	if reflect.DeepEqual(oldStatus, nab.Spec.BackupStatus) && reflect.DeepEqual(oldSpec, nab.Spec.BackupSpec) {
-		// No change, no need to update
-		log.V(1).Info("NonAdminBackup status and spec is already up to date")
-		return nil
+	nab.Spec.BackupSpec = veleroBackup.Spec.DeepCopy()
+
+	// Check if the spec has been updated
+	if !reflect.DeepEqual(oldSpec, nab.Spec.BackupSpec) {
+		if err := r.Update(ctx, nab); err != nil {
+			log.Error(err, "Failed to update NonAdminBackup Spec")
+			return err
+		}
+		log.V(1).Info("NonAdminBackup spec was updated")
+	} else {
+		log.V(1).Info("NonAdminBackup spec is already up to date")
 	}
 
-	if err := r.Update(ctx, nab); err != nil {
-		log.Error(err, "Failed to update NonAdminBackup")
-		return err
+	// Check if the status has been updated
+	if !reflect.DeepEqual(oldStatus, nab.Status.BackupStatus) {
+		if err := r.Status().Update(ctx, nab); err != nil {
+			log.Error(err, "Failed to update NonAdminBackup Status")
+			return err
+		}
+		log.V(1).Info("NonAdminBackup status was updated")
+	} else {
+		log.V(1).Info("NonAdminBackup status is already up to date")
 	}
 
 	return nil

diff --git a/internal/controller/nonadminbackup_controller.go b/internal/controller/nonadminbackup_controller.go
@@ -66,6 +66,8 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	r.NamespacedName = req.NamespacedName
 
 	nab := nacv1alpha1.NonAdminBackup{}
+	nab.Status.Phase = nacv1alpha1.NonAdminBackupPhaseNew
+
 	err := r.Get(ctx, req.NamespacedName, &nab)
 
 	if err != nil && errors.IsNotFound(err) {
@@ -82,6 +84,12 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 
 	if veleroBackupSpec == nil {
 		log.Error(err, "NonAdminBackup CR does not contain valid VeleroBackupSpec")
+		nab.Status.Phase = nacv1alpha1.NonAdminBackupPhaseFailedValidation
+		nab.Status.FailureReason = "NonAdminBackup CR does not contain valid VeleroBackupSpec"
+		if err := r.Status().Update(ctx, &nab); err != nil {
+			log.Error(err, "Failed to update NonAdminBackup Status")
+			return ctrl.Result{}, err
+		}
 		return ctrl.Result{}, nil
 	}