update

api/v1alpha1/nonadmindownloadrequest_types.go

@@ -42,13 +42,6 @@ type VeleroDownloadRequest struct {
 	// VeleroDownloadRequestStatus represents VeleroDownloadRequestStatus
 	// +optional
 	Status *velerov1.DownloadRequestStatus `json:"status,omitempty"`
-	// name references the Velero delete backup request object by it's name.
-	// +optional
-	Name string `json:"name,omitempty"`
-
-	// namespace references the Namespace in which Velero delete backup request exists.
-	// +optional
-	Namespace string `json:"namespace,omitempty"`
 }
 
 // NonAdminDownloadRequestStatus defines the observed state of NonAdminDownloadRequest.

internal/common/function/function.go

@@ -28,7 +28,6 @@ import (
 	velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/validation"
@@ -39,6 +38,11 @@ import (
 	"github.com/migtools/oadp-non-admin/internal/common/constant"
 )
 
+// Common labels for objects manipulated by the Non Admin Controller
+// Labels should be used to identify the NAC object
+// Annotations on the other hand should be used to define ownership
+// of the specific Object, such as Backup/Restore.
+
 // GetNonAdminLabels return the required Non Admin labels
 func GetNonAdminLabels() map[string]string {
 	return map[string]string{
@@ -55,29 +59,37 @@ func GetNonAdminRestoreLabels(uniqueIdentifier string) map[string]string {
 }
 
 // GetNonAdminBackupAnnotations return the required Non Admin annotations
-func GetNonAdminBackupAnnotations(objectMeta metav1.ObjectMeta) map[string]string {
+func GetNonAdminBackupAnnotations(objectMeta nacv1alpha1.NonAdminBackup) map[string]string {
 	return map[string]string{
 		constant.NabOriginNamespaceAnnotation: objectMeta.Namespace,
 		constant.NabOriginNameAnnotation:      objectMeta.Name,
 	}
 }
 
 // GetNonAdminRestoreAnnotations return the required Non Admin restore annotations
-func GetNonAdminRestoreAnnotations(objectMeta metav1.ObjectMeta) map[string]string {
+func GetNonAdminRestoreAnnotations(objectMeta nacv1alpha1.NonAdminRestore) map[string]string {
 	return map[string]string{
 		constant.NarOriginNamespaceAnnotation: objectMeta.Namespace,
 		constant.NarOriginNameAnnotation:      objectMeta.Name,
 	}
 }
 
 // GetNonAdminBackupStorageLocationAnnotations return the required Non Admin annotations
-func GetNonAdminBackupStorageLocationAnnotations(objectMeta metav1.ObjectMeta) map[string]string {
+func GetNonAdminBackupStorageLocationAnnotations(objectMeta nacv1alpha1.NonAdminBackupStorageLocation) map[string]string {
 	return map[string]string{
 		constant.NabslOriginNamespaceAnnotation: objectMeta.Namespace,
 		constant.NabslOriginNameAnnotation:      objectMeta.Name,
 	}
 }
 
+// GetNonAdminDownloadRequestAnnotations return the required Non Admin annotations
+func GetNonAdminDownloadRequestAnnotations(objectMeta nacv1alpha1.NonAdminDownloadRequest) map[string]string {
+	return map[string]string{
+		constant.NadrOriginNamespaceAnnotation: objectMeta.Namespace,
+		constant.NadrOriginNameAnnotation:      objectMeta.Name,
+	}
+}
+
 // containsOnlyNamespace checks if the given namespaces slice contains only the specified namespace
 func containsOnlyNamespace(namespaces []string, namespace string) bool {
 	for _, ns := range namespaces {

internal/controller/nonadminbackup_controller.go

@@ -307,7 +307,7 @@ func (r *NonAdminBackupReconciler) createVeleroDeleteBackupRequest(ctx context.C
 					constant.NabOriginNACUUIDLabel, veleroBackupNACUUID,
 				),
 				builder.WithLabelsMap(function.GetNonAdminLabels()),
-				builder.WithAnnotationsMap(function.GetNonAdminBackupAnnotations(nab.ObjectMeta)),
+				builder.WithAnnotationsMap(function.GetNonAdminBackupAnnotations(*nab)),
 				builder.WithGenerateName(veleroBackup.Name+"-"),
 			).Result()
 
@@ -697,7 +697,7 @@ func (r *NonAdminBackupReconciler) createVeleroBackupAndSyncWithNonAdminBackup(c
 				Name:        veleroBackupNACUUID,
 				Namespace:   r.OADPNamespace,
 				Labels:      function.GetNonAdminLabels(),
-				Annotations: function.GetNonAdminBackupAnnotations(nab.ObjectMeta),
+				Annotations: function.GetNonAdminBackupAnnotations(*nab),
 			},
 			Spec: *backupSpec,
 		}

internal/controller/nonadmindownloadrequest_controller.go

@@ -81,7 +81,11 @@ func (r *NonAdminDownloadRequestReconciler) Reconcile(ctx context.Context, req *
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      req.VeleroDownloadRequestName(),
 			Namespace: r.OADPNamespace,
-			Labels:    function.GetNonAdminLabels(),
+			Labels: func() map[string]string {
+				nal := function.GetNonAdminLabels()
+				nal[constant.NadrOriginNACUUIDLabel] = string(req.GetUID())
+				return nal
+			}(),
 			// Annotations: ,
 		},
 		Spec: velerov1.DownloadRequestSpec{
@@ -90,29 +94,55 @@ func (r *NonAdminDownloadRequestReconciler) Reconcile(ctx context.Context, req *
 			},
 		},
 	}
+	// try get veleroDR if exists, then update status
+	if err := r.Get(ctx, types.NamespacedName{Namespace: veleroDR.Namespace, Name: veleroDR.Name}, &veleroDR); err == nil {
+		// wait for next reconcile?
+		// if url is available and not expired, then set status to completed
+		if veleroDR.Status.DownloadURL != constant.EmptyString &&
+			veleroDR.Status.Phase == velerov1.DownloadRequestPhaseProcessed &&
+			!veleroDR.Status.Expiration.Before(&metav1.Time{Time: time.Now()}) {
+			// copy req, prepare to patch
+			prePatch := req.DeepCopy()
+			// copy status to NADR from VDR
+			req.Status.VeleroDownloadRequest.Status = &veleroDR.Status
+			// veleroDR is processed, update NADR status
+			req.Status.Phase = nacv1alpha1.NonAdminPhaseCompleted
+			// clear conditions
+			req.Status.Conditions = []metav1.Condition{}
+			if patchErr := r.Status().Patch(ctx, req, client.MergeFrom(prePatch)); patchErr != nil {
+				logger.Error(patchErr, "unable to patch status", req.Kind, req.Name)
+				return ctrl.Result{}, patchErr
+			}
+
+		}
+	} else if !apierrors.IsNotFound(err) {
+		// some other errors, requeue to retry get
+		return reconcile.Result{}, err
+	}
 	// request is expired, so delete NADR after deleting velero DR
-	if req.Status.VeleroDownloadRequest.Status != nil  && req.Status.VeleroDownloadRequest.Status.Expiration.Before(&metav1.Time{Time: time.Now()}) {
+	if req.Status.VeleroDownloadRequest.Status != nil && req.Status.VeleroDownloadRequest.Status.Expiration != nil && req.Status.VeleroDownloadRequest.Status.Expiration.Before(&metav1.Time{Time: time.Now()}) {
 		// find associated velero downloadrequest and delete that first
 		logger.V(1).Info("Deleting expired NonAdminDownloadRequest associated velero download request", req.VeleroDownloadRequestName(), req.Namespace)
 		if err := r.Delete(ctx, &veleroDR); err != nil {
 			if !apierrors.IsNotFound(err) {
 				logger.V(1).Info("Failed to delete expired NonAdminDownloadRequest associated velero download request", req.VeleroDownloadRequestName(), err)
 				// other errors, requeue to retry delete
-				return reconcile.Result{Requeue: true}, nil
+				return reconcile.Result{}, err
 			}
 		}
 		logger.V(1).Info("Deleting expired NonAdminDownloadRequest", req.Name, req.Namespace)
 		if err := r.Delete(ctx, req); err != nil {
 			if !apierrors.IsNotFound(err) {
 				logger.V(1).Info("Failed to delete expired NonAdminDownloadRequest", req.Name, err)
 				// other errors, requeue to retry delete
-				return reconcile.Result{Requeue: true}, nil
+				return reconcile.Result{}, err
 			}
+			// not found, stop requeue
 			return reconcile.Result{Requeue: false}, nil
 		}
 	}
-	var nab nacv1alpha1.NonAdminBackup
-	var nabName string
+	var nab nacv1alpha1.NonAdminBackup // holds nonadminbackup
+	var nabName string                 // holds nonadminbackup name
 	switch req.Spec.Target.Kind {
 	case velerov1.DownloadTargetKindBackupLog,
 		velerov1.DownloadTargetKindBackupContents,
@@ -123,7 +153,6 @@ func (r *NonAdminDownloadRequestReconciler) Reconcile(ctx context.Context, req *
 		velerov1.DownloadTargetKindCSIBackupVolumeSnapshots,
 		velerov1.DownloadTargetKindCSIBackupVolumeSnapshotContents,
 		velerov1.DownloadTargetKindBackupVolumeInfos:
-		// get velero backup name from nab
 		nabName = req.Spec.Target.Name
 	case velerov1.DownloadTargetKindRestoreLog,
 		velerov1.DownloadTargetKindRestoreResults,
@@ -149,15 +178,15 @@ func (r *NonAdminDownloadRequestReconciler) Reconcile(ctx context.Context, req *
 		logger.Error(err, "unable to get nab", nab.Name, nab.Namespace)
 		return ctrl.Result{}, err
 	}
-	// check nab for nabsl
+	// error if nab does not use nabsl
 	if !nab.UsesNaBSL() {
 		patchErr := r.patchAddStatusConditionTypeFalseBackoff(ctx, req, "NonAdminBackupStorageLocationUsed")
 		logger.Error(patchErr, statusPatchErr, req.Kind, req.Name)
 		// patch status to completed to stop processing this NADR
 		// because it is not using NonAdminBackupStorageLocation, user is expected to recreate NADR
 		// after they have a NAB using NABSL
 		prePatch := req.DeepCopy()
-		req.Status.Phase = nacv1alpha1.NonAdminPhaseCompleted
+		req.Status.Phase = nacv1alpha1.NonAdminPhaseCompleted // not using nabsl is terminal
 		if patchErr := r.Status().Patch(ctx, req, client.MergeFrom(prePatch)); patchErr != nil {
 			logger.Error(patchErr, statusPatchErr, req.Kind, req.Name)
 			return ctrl.Result{}, patchErr
@@ -171,22 +200,11 @@ func (r *NonAdminDownloadRequestReconciler) Reconcile(ctx context.Context, req *
 	if veleroDR.Spec.Target.Name != constant.EmptyString {
 		veleroDR.Spec.Target.Name = nab.VeleroBackupName()
 	}
-
-	// wait for next reconcile?
-	prePatch := req.DeepCopy()
-	// copy status to NADR from VDR
-	req.Status.VeleroDownloadRequest.Status = &veleroDR.Status
-	// if url is available and not expired, then set status to completed
-	if veleroDR.Status.DownloadURL != constant.EmptyString && !veleroDR.Status.Expiration.Before(&metav1.Time{Time: time.Now()}) {
-		req.Status.Phase = nacv1alpha1.NonAdminPhaseCompleted
-		// clear conditions
-		req.Status.Conditions = []metav1.Condition{}
-		if patchErr := r.Status().Patch(ctx, req, client.MergeFrom(prePatch)); patchErr != nil {
-			logger.Error(patchErr, "unable to patch status", req.Kind, req.Name)
-			return ctrl.Result{}, patchErr
-		}
+	// veleroDR is now ready to be created
+	if err := r.Create(ctx, &veleroDR); err != nil {
+		// requeue so Get can update nadr status (if exists) or recreate
+		return ctrl.Result{}, err
 	}
-
 	return ctrl.Result{}, nil
 }