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]>

.golangci.yml

@@ -63,6 +63,8 @@ linters-settings:
         disabled: true
       - name: function-length
         disabled: true
+      - name: max-public-structs
+        disabled: true
       # TODO remove
       - name: cyclomatic
         disabled: true

api/v1alpha1/nonadminbackup_types.go

@@ -21,16 +21,37 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+// NonAdminBackupPhase is a simple one high-level summary of the lifecycle of an NonAdminBackup.
+// +kubebuilder:validation:Enum=New;BackingOff;Created
+type NonAdminBackupPhase string
+
+const (
+	// NonAdminBackupPhaseNew - NonAdminBackup resource was accepted by the OpenShift cluster, but it has not yet been processed by the NonAdminController
+	NonAdminBackupPhaseNew NonAdminBackupPhase = "New"
+	// NonAdminBackupPhaseBackingOff - Velero Backup object was not created due to NonAdminBackup error (configuration or similar)
+	NonAdminBackupPhaseBackingOff NonAdminBackupPhase = "BackingOff"
+	// NonAdminBackupPhaseCreated - Velero Backup was created. The Phase will not have additional informations about the Backup.
+	NonAdminBackupPhaseCreated NonAdminBackupPhase = "Created"
+)
+
+// NonAdminCondition are used for more detailed information supporing NonAdminBackupPhase state.
+// +kubebuilder:validation:Enum=BackupAccepted;BackupQueued
+type NonAdminCondition string
+
+// Predefined conditions for NonAdminBackup.
+// One NonAdminBackup object may have multiple conditions.
+// It is more granular knowledge of the NonAdminBackup object and represents the
+// array of the conditions through which the NonAdminBackup has or has not passed
+const (
+	NonAdminBackupConditionAccepted NonAdminCondition = "BackupAccepted"
+	NonAdminBackupConditionQueued   NonAdminCondition = "BackupQueued"
+)
+
 // NonAdminBackupSpec defines the desired state of NonAdminBackup
 type NonAdminBackupSpec struct {
-	// https://github.com/vmware-tanzu/velero/blob/main/pkg/apis/velero/v1/backup_types.go
-
 	// 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"`
-
 	// NonAdminBackup log level (use debug for the most logging, leave unset for default)
 	// +optional
 	// +kubebuilder:validation:Enum=trace;debug;info;warning;error;fatal;panic
@@ -39,19 +60,28 @@ type NonAdminBackupSpec struct {
 
 // NonAdminBackupStatus defines the observed state of NonAdminBackup
 type NonAdminBackupStatus struct {
-	Conditions []metav1.Condition `json:"conditions,omitempty"`
+	// OadpVeleroBackup references the VeleroBackup object. Format: <name>.<namespace>
+	// +optional
+	OadpVeleroBackup string `json:"oadpVeleroBackup,omitempty"`
+
+	// BackupStatus captures the current status of a Velero backup.
+	// +optional
+	BackupStatus *velerov1api.BackupStatus `json:"backupStatus,omitempty"`
+
+	Phase      NonAdminBackupPhase `json:"phase,omitempty"`
+	Conditions []metav1.Condition  `json:"conditions,omitempty"`
 }
 
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 
 // NonAdminBackup is the Schema for the nonadminbackups API
 type NonAdminBackup struct {
-	metav1.TypeMeta   `json:",inline"`
-	metav1.ObjectMeta `json:"metadata,omitempty"`
-
 	Spec   NonAdminBackupSpec   `json:"spec,omitempty"`
 	Status NonAdminBackupStatus `json:"status,omitempty"`
+
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
 }
 
 // +kubebuilder:object:root=true

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,18 @@ spec:
                   - type
                   type: object
                 type: array
+              oadpVeleroBackup:
+                description: 'OadpVeleroBackup references the VeleroBackup object.
+                  Format: <name>.<namespace>'
+                type: string
+              phase:
+                description: NonAdminBackupPhase is a simple one high-level summary
+                  of the lifecycle of an NonAdminBackup.
+                enum:
+                - New
+                - BackingOff
+                - Created
+                type: string
             type: object
         type: object
     served: true

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.56

internal/common/constant/constant.go

@@ -29,3 +29,16 @@ const (
 	NabOriginNamespaceAnnotation = "openshift.io/oadp-nab-origin-namespace"
 	NabOriginUUIDAnnotation      = "openshift.io/oadp-nab-origin-uuid"
 )
+
+// EmptyString defines a constant for the empty string
+const EmptyString = ""
+
+// NameSpaceString k8s Namespace string
+const NameSpaceString = "Namespace"
+
+// MaxKubernetesNameLength represents maximum length of the name in k8s
+const MaxKubernetesNameLength = 253
+
+// VeleroBackupNamePrefix represents the prefix for the object name generated
+// by the NonAdminController
+const VeleroBackupNamePrefix = "nab"

internal/common/function/function.go

@@ -21,11 +21,14 @@ import (
 	"context"
 	"crypto/sha1" //nolint:gosec // TODO remove
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"reflect"
 
 	"github.com/go-logr/logr"
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	apimeta "k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -117,28 +120,135 @@ func GenerateVeleroBackupName(namespace, nabName string) string {
 	return veleroBackupName
 }
 
-// UpdateNonAdminBackupFromVeleroBackup update, if necessary, NonAdminBackup object fields related to referenced Velero Backup object, if no error occurs
-func UpdateNonAdminBackupFromVeleroBackup(ctx context.Context, r client.Client, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup, veleroBackup *velerov1api.Backup) error {
-	// Make a copy of the current status for comparison
-	oldStatus := nab.Spec.BackupStatus.DeepCopy()
-	oldSpec := nab.Spec.BackupSpec.DeepCopy()
+// UpdateNonAdminPhase updates the phase of a NonAdminBackup object with the provided phase.
+func UpdateNonAdminPhase(ctx context.Context, r client.Client, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup, phase nacv1alpha1.NonAdminBackupPhase) (bool, error) {
+	if nab == nil {
+		return false, errors.New("NonAdminBackup object is nil")
+	}
+
+	// Ensure phase is valid
+	if phase == constant.EmptyString {
+		return false, errors.New("NonAdminBackupPhase cannot be empty")
+	}
+
+	oldPhase := nab.Status.Phase
+	nab.Status.Phase = phase
+
+	logger.V(1).Info(fmt.Sprintf("Setting NonAdminBackup Phase to: %s", phase))
+
+	if oldPhase == nab.Status.Phase {
+		// No change, no need to update
+		logger.V(1).Info("NonAdminBackup Phase is already up to date")
+		return false, nil
+	}
+
+	// Update NAB status
+	if err := r.Status().Update(ctx, nab); err != nil {
+		logger.Error(err, "Failed to update NonAdminBackup Phase")
+		return false, err
+	}
+
+	logger.V(1).Info(fmt.Sprintf("NonAdminBackup Phase set to: %s", phase))
+
+	return true, nil
+}
+
+// UpdateNonAdminBackupCondition updates the condition of a NonAdminBackup object
+// based on the provided parameters. It validates the input parameters and ensures
+// that the condition is set to the desired status only if it differs from the current status.
+// If the condition is already set to the desired status, no update is performed.
+func UpdateNonAdminBackupCondition(ctx context.Context, r client.Client, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup, condition nacv1alpha1.NonAdminCondition, conditionStatus metav1.ConditionStatus, reason string, message string) (bool, error) {
+	if nab == nil {
+		return false, errors.New("NonAdminBackup object is nil")
+	}
+
+	// Ensure phase and condition are valid
+	if condition == constant.EmptyString {
+		return false, errors.New("NonAdminBackup Condition cannot be empty")
+	}
 
-	// Update the status & spec
-	nab.Spec.BackupStatus = &veleroBackup.Status
-	nab.Spec.BackupSpec = &veleroBackup.Spec
+	if conditionStatus == constant.EmptyString {
+		return false, errors.New("NonAdminBackup Condition Status cannot be empty")
+	} else if conditionStatus != metav1.ConditionTrue && conditionStatus != metav1.ConditionFalse && conditionStatus != metav1.ConditionUnknown {
+		return false, errors.New("NonAdminBackup Condition Status must be valid metav1.ConditionStatus")
+	}
+
+	if reason == constant.EmptyString {
+		return false, errors.New("NonAdminBackup Condition Reason cannot be empty")
+	}
+
+	if message == constant.EmptyString {
+		return false, errors.New("NonAdminBackup Condition Message cannot be empty")
+	}
+
+	// Check if the condition is already set to the desired status
+	currentCondition := apimeta.FindStatusCondition(nab.Status.Conditions, string(condition))
+	if currentCondition != nil && currentCondition.Status == conditionStatus {
+		// Condition is already set to the desired status, no need to update
+		logger.V(1).Info(fmt.Sprintf("NonAdminBackup Condition is already set to: %s", condition))
+		return false, nil
+	}
+
+	// Update NAB status condition
+	apimeta.SetStatusCondition(&nab.Status.Conditions,
+		metav1.Condition{
+			Type:    string(condition),
+			Status:  conditionStatus,
+			Reason:  reason,
+			Message: message,
+		},
+	)
+
+	logger.V(1).Info(fmt.Sprintf("NonAdminBackup Condition to: %s", condition))
+	logger.V(1).Info(fmt.Sprintf("NonAdminBackup Condition Reason to: %s", reason))
+	logger.V(1).Info(fmt.Sprintf("NonAdminBackup Condition Message to: %s", message))
+
+	// Update NAB status
+	if err := r.Status().Update(ctx, nab); err != nil {
+		logger.Error(err, "NonAdminBackup Condition - Failed to update")
+		return false, err
+	}
 
-	if reflect.DeepEqual(oldStatus, nab.Spec.BackupStatus) && reflect.DeepEqual(oldSpec, nab.Spec.BackupSpec) {
+	return true, nil
+}
+
+// UpdateNonAdminBackupFromVeleroBackup update, if necessary, NonAdminBackup object fields related to referenced Velero Backup object, if no error occurs
+func UpdateNonAdminBackupFromVeleroBackup(ctx context.Context, r client.Client, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup, veleroBackup *velerov1api.Backup) (bool, error) {
+	logger.V(1).Info("NonAdminBackup BackupSpec and BackupStatus - request to update")
+
+	if reflect.DeepEqual(nab.Status.BackupStatus, &veleroBackup.Status) && reflect.DeepEqual(nab.Spec.BackupSpec, &veleroBackup.Spec) {
 		// No change, no need to update
-		logger.V(1).Info("NonAdminBackup status and spec is already up to date")
-		return nil
+		logger.V(1).Info("NonAdminBackup BackupSpec and BackupStatus - nothing to update")
+		return false, nil
 	}
 
-	if err := r.Update(ctx, nab); err != nil {
-		logger.Error(err, "Failed to update NonAdminBackup")
-		return err
+	// Check if BackupStatus needs to be updated
+	if !reflect.DeepEqual(nab.Status.BackupStatus, &veleroBackup.Status) || nab.Status.OadpVeleroBackup != veleroBackup.Name {
+		nab.Status.BackupStatus = veleroBackup.Status.DeepCopy()
+		nab.Status.OadpVeleroBackup = veleroBackup.Name
+		if err := r.Status().Update(ctx, nab); err != nil {
+			logger.Error(err, "NonAdminBackup BackupStatus - Failed to update")
+			return false, err
+		}
+		logger.V(1).Info("NonAdminBackup BackupStatus - updated")
+	} else {
+		logger.V(1).Info("NonAdminBackup BackupStatus - up to date")
 	}
 
-	return nil
+	// Check if BackupSpec needs to be updated
+	if !reflect.DeepEqual(nab.Spec.BackupSpec, &veleroBackup.Spec) {
+		nab.Spec.BackupSpec = veleroBackup.Spec.DeepCopy()
+		if err := r.Update(ctx, nab); err != nil {
+			logger.Error(err, "NonAdminBackup BackupSpec - Failed to update")
+			return false, err
+		}
+		logger.V(1).Info("NonAdminBackup BackupSpec - updated")
+	} else {
+		logger.V(1).Info("NonAdminBackup BackupSpec - up to date")
+	}
+
+	// If either BackupStatus or BackupSpec was updated, return true
+	return true, nil
 }
 
 // CheckVeleroBackupLabels return true if Velero Backup object has required Non Admin labels, false otherwise
@@ -149,8 +259,6 @@ func CheckVeleroBackupLabels(backup *velerov1api.Backup) bool {
 	return exists && value == constant.ManagedByLabelValue
 }
 
-// TODO not used
-
 // GetNonAdminBackupFromVeleroBackup return referenced NonAdminBackup object from Velero Backup object, if no error occurs
 func GetNonAdminBackupFromVeleroBackup(ctx context.Context, clientInstance client.Client, backup *velerov1api.Backup) (*nacv1alpha1.NonAdminBackup, error) {
 	// Check if the backup has the required annotations to identify the associated NonAdminBackup object