diff --git a/Makefile b/Makefile index 74b59ae..37fa5af 100644 --- a/Makefile +++ b/Makefile @@ -225,7 +225,7 @@ editorconfig: $(LOCALBIN) ## Download editorconfig locally if necessary. } # TODO increase to 60? -COVERAGE_THRESHOLD=30 +COVERAGE_THRESHOLD=40 .PHONY: ci ci: simulation-test lint docker-build hadolint check-generate check-manifests ec check-images ## Run all project continuous integration (CI) checks locally. diff --git a/cmd/main.go b/cmd/main.go index d865bbc..6c09ddb 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -135,6 +135,8 @@ func main() { if err = (&controller.NonAdminBackupReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), + // TODO context does not need to be set here??? + // add env var here?? so it is only called once still and is easy to test }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "NonAdminBackup") os.Exit(1) diff --git a/internal/common/function/function.go b/internal/common/function/function.go index 192a97d..135fd3c 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -113,7 +113,7 @@ func GetBackupSpecFromNonAdminBackup(nonAdminBackup *nacv1alpha1.NonAdminBackup) veleroBackupSpec.IncludedNamespaces = []string{nonAdminBackup.Namespace} } else { if !containsOnlyNamespace(veleroBackupSpec.IncludedNamespaces, nonAdminBackup.Namespace) { - return nil, fmt.Errorf("spec.backupSpec.IncludedNamespaces can not contain namespaces other then: %s", nonAdminBackup.Namespace) + return nil, fmt.Errorf("spec.backupSpec.IncludedNamespaces can not contain namespaces other than: %s", nonAdminBackup.Namespace) } } @@ -228,10 +228,10 @@ func UpdateNonAdminBackupCondition(ctx context.Context, r client.Client, logger }, ) - // TODO ... Condition *set* to... ? - 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)) + // TODO these logs should be after err check, no? + logger.V(1).Info(fmt.Sprintf("NonAdminBackup Condition set to: %s", condition)) + logger.V(1).Info(fmt.Sprintf("NonAdminBackup Condition Reason set to: %s", reason)) + logger.V(1).Info(fmt.Sprintf("NonAdminBackup Condition Message set to: %s", message)) // Update NAB status if err := r.Status().Update(ctx, nab); err != nil { diff --git a/internal/common/function/function_test.go b/internal/common/function/function_test.go index dcec107..9f1e645 100644 --- a/internal/common/function/function_test.go +++ b/internal/common/function/function_test.go @@ -219,7 +219,7 @@ func TestGetBackupSpecFromNonAdminBackup(t *testing.T) { assert.Error(t, err) assert.Nil(t, backupSpec) - assert.Equal(t, "spec.backupSpec.IncludedNamespaces can not contain namespaces other then: namespace2", err.Error()) + assert.Equal(t, "spec.backupSpec.IncludedNamespaces can not contain namespaces other than: namespace2", err.Error()) backupSpecInput = &velerov1api.BackupSpec{ IncludedNamespaces: []string{"namespace3"}, @@ -237,7 +237,7 @@ func TestGetBackupSpecFromNonAdminBackup(t *testing.T) { assert.Error(t, err) assert.Nil(t, backupSpec) - assert.Equal(t, "spec.backupSpec.IncludedNamespaces can not contain namespaces other then: namespace4", err.Error()) + assert.Equal(t, "spec.backupSpec.IncludedNamespaces can not contain namespaces other than: namespace4", err.Error()) } func TestGenerateVeleroBackupName(t *testing.T) { diff --git a/internal/controller/nonadminbackup_controller.go b/internal/controller/nonadminbackup_controller.go index 6b3164e..dc23a44 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -27,6 +27,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -43,14 +44,13 @@ import ( // NonAdminBackupReconciler reconciles a NonAdminBackup object type NonAdminBackupReconciler struct { client.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + // needed??? Context context.Context } -const ( - nameField = "Name" - requeueTimeSeconds = 10 -) +// TODO TOO MUCH!!!!!!!!!!!!!!! +const requeueTimeSeconds = 10 // +kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups/status,verbs=get;update;patch @@ -63,30 +63,46 @@ const ( func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { rLog := log.FromContext(ctx) logger := rLog.WithValues("NonAdminBackup", req.NamespacedName) - // TODO remove duplication in logs - // remove >>>? - logger.V(1).Info(">>> Reconcile NonAdminBackup - loop start") + logger.V(1).Info("NonAdminBackup Reconcile start") // Get the NonAdminBackup object nab := nacv1alpha1.NonAdminBackup{} err := r.Get(ctx, req.NamespacedName, &nab) - - // Bail out when the Non Admin Backup reconcile was triggered, when the NAB got deleted - // Reconcile loop was triggered when Velero Backup object got updated and NAB isn't there if err != nil { if apierrors.IsNotFound(err) { - // k/v's are noise? - logger.V(1).Info("Non existing NonAdminBackup CR", nameField, req.Name, constant.NameSpaceString, req.Namespace) + // Delete event triggered this reconcile + logger.V(1).Info("Non existing NonAdminBackup") return ctrl.Result{}, nil - // should not error? } - logger.Error(err, "Unable to fetch NonAdminBackup CR", nameField, req.Name, constant.NameSpaceString, req.Namespace) + logger.Error(err, "Unable to fetch NonAdminBackup") return ctrl.Result{}, err } - // TODO why do we need Requeue? is not that anti performance? + // requeue on every change is the correct pattern! document this + // TODO refactor idea: do not enter on sub functions again + // TODO refactor idea: sub functions can not exit clean, that should be main func responsibility. Remove reconcileExit return param + // TODO refactor idea: + // requeue, err := r.Init(ctx, rLog, &nab) + // if err != nil { + // // handle err smart way to retry when wanted? + // return ctrl.Result{}, reconcile.TerminalError(err) + // } + // if requeue { + // return ctrl.Result{Requeue: true}, nil + // } + // SOURCE https://github.com/kubernetes-sigs/controller-runtime/blob/e6c3d139d2b6c286b1dbba6b6a95919159cfe655/pkg/internal/controller/controller.go#L286 + // Alright, after studies, I believe there are only 2 possibilities (DEV eyes): + // - re trigger reconcile + // AddRateLimited ([requeue and nill error] or [normal error]) + // will re trigger reconcile immediately, after 1 second, after 2 seconds, etc + // AddAfter ([RequeueAfter and nill error]) + // will re trigger reconcile after time + // - will not re trigger reconcile + // Forget (finish process) ([empty result and nill error] or [terminal error]) + reconcileExit, reconcileRequeue, reconcileErr := r.Init(ctx, rLog, &nab) if reconcileRequeue { + // TODO EITHER Requeue or RequeueAfter, both together do not make sense!!! return ctrl.Result{Requeue: true, RequeueAfter: requeueTimeSeconds * time.Second}, reconcileErr } else if reconcileExit && reconcileErr != nil { return ctrl.Result{}, reconcile.TerminalError(reconcileErr) @@ -104,37 +120,6 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ctrl.Result{}, nil } - // TODO refactor idea - // veleroBackupName := function.GenerateVeleroBackupName(nab.Namespace, nab.Name) - // if veleroBackupName == constant.EmptyString { - // return ctrl.Result{}, reconcile.TerminalError(errors.New("unable to generate Velero Backup name")) - // } - // oadpNamespace := function.GetOADPNamespace() - // veleroBackup := velerov1api.Backup{} - // err = r.Get(ctx, client.ObjectKey{Namespace: oadpNamespace, Name: veleroBackupName}, &veleroBackup) - // if err != nil { - // if !apierrors.IsNotFound(err) { - // logger.Error(err, "Unable to fetch VeleroBackup") - // return ctrl.Result{}, reconcile.TerminalError(err) - // } - // reconcileExit, reconcileRequeue, reconcileErr = r.CreateVeleroBackup(ctx, rLog, &nab, veleroBackupName, oadpNamespace) - // if reconcileRequeue { - // return ctrl.Result{Requeue: true, RequeueAfter: requeueTimeSeconds * time.Second}, reconcileErr - // } else if reconcileExit && reconcileErr != nil { - // return ctrl.Result{}, reconcile.TerminalError(reconcileErr) - // } else if reconcileExit { - // return ctrl.Result{}, nil - // } - // reconcileExit, reconcileRequeue, reconcileErr = r.UpdateStatusAfterVeleroBackupCreation(ctx, rLog, &nab) - // if reconcileRequeue { - // return ctrl.Result{Requeue: true, RequeueAfter: requeueTimeSeconds * time.Second}, reconcileErr - // } else if reconcileExit && reconcileErr != nil { - // return ctrl.Result{}, reconcile.TerminalError(reconcileErr) - // } else if reconcileExit { - // return ctrl.Result{}, nil - // } - // } - reconcileExit, reconcileRequeue, reconcileErr = r.UpdateSpecStatus(ctx, rLog, &nab) if reconcileRequeue { return ctrl.Result{Requeue: true, RequeueAfter: requeueTimeSeconds * time.Second}, reconcileErr @@ -144,7 +129,6 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ctrl.Result{}, nil } - logger.V(1).Info(">>> Reconcile NonAdminBackup - loop end") return ctrl.Result{}, nil } @@ -161,24 +145,22 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque // It then returns boolean values indicating whether the reconciliation loop should requeue or exit // and error value whether the status was updated successfully. func (r *NonAdminBackupReconciler) Init(ctx context.Context, logrLogger logr.Logger, nab *nacv1alpha1.NonAdminBackup) (exitReconcile bool, requeueReconcile bool, errorReconcile error) { - // TODO - logger := logrLogger.WithValues("Init", nab.Namespace) - // Set initial Phase + logger := logrLogger.WithValues("Init NonAdminBackup", types.NamespacedName{Name: nab.Name, Namespace: nab.Namespace}) + if nab.Status.Phase == constant.EmptyString { - // Phase: New + // // Set initial Phase to New updatedStatus, errUpdate := function.UpdateNonAdminPhase(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminBackupPhaseNew) - if errUpdate != nil { - logger.Error(errUpdate, "Unable to set NonAdminBackup Phase: New", nameField, nab.Name, constant.NameSpaceString, nab.Namespace) + logger.Error(errUpdate, "Unable to set NonAdminBackup Phase: New") return true, false, errUpdate } - if updatedStatus { - logger.V(1).Info("NonAdminBackup CR - Requeue after Phase Update") + logger.V(1).Info("NonAdminBackup - Requeue after Phase Update") return false, true, nil } } + logger.V(1).Info("NonAdminBackup Status.Phase already initialized") return false, false, nil } @@ -196,25 +178,19 @@ func (r *NonAdminBackupReconciler) Init(ctx context.Context, logrLogger logr.Log // If the BackupSpec is invalid, the function sets the NonAdminBackup condition to "InvalidBackupSpec". THIS DOES NOT HAPPEN // If the BackupSpec is valid, the function sets the NonAdminBackup condition to "BackupAccepted". remove? func (r *NonAdminBackupReconciler) ValidateSpec(ctx context.Context, logrLogger logr.Logger, nab *nacv1alpha1.NonAdminBackup) (exitReconcile bool, requeueReconcile bool, errorReconcile error) { - // TODO - logger := logrLogger.WithValues("ValidateSpec", nab.Namespace) + logger := logrLogger.WithValues("ValidateSpec NonAdminBackup", types.NamespacedName{Name: nab.Name, Namespace: nab.Namespace}) // Main Validation point for the VeleroBackup included in NonAdminBackup spec _, err := function.GetBackupSpecFromNonAdminBackup(nab) if err != nil { - // Use errMsg if errMsgFromErr is not available, otherwise use errMsgFromErr - errMsg := "NonAdminBackup CR does not contain valid BackupSpec" - if errMsgFromErr := err.Error(); errMsgFromErr != "" { - errMsg = errMsgFromErr - } - // TODO logs noise to user - // every logger error logs a stacktrace - logger.Error(err, errMsg) + logger.Error(err, "NonAdminBackup Spec is not valid") + // this should be one call: update both phase and condition at THE SAME TIME + // OR do requeue, CONDITION is never set to false updatedStatus, errUpdateStatus := function.UpdateNonAdminPhase(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminBackupPhaseBackingOff) if errUpdateStatus != nil { - logger.Error(errUpdateStatus, "Unable to set NonAdminBackup Phase: BackingOff", nameField, nab.Name, constant.NameSpaceString, nab.Namespace) + logger.Error(errUpdateStatus, "Unable to set NonAdminBackup Phase: BackingOff") return true, false, errUpdateStatus } else if updatedStatus { // We do not requeue - the State was set to BackingOff @@ -222,9 +198,9 @@ func (r *NonAdminBackupReconciler) ValidateSpec(ctx context.Context, logrLogger } // Continue. VeleroBackup looks fine, setting Accepted condition to false - updatedCondition, errUpdateCondition := function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionFalse, "InvalidBackupSpec", errMsg) + updatedCondition, errUpdateCondition := function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionFalse, "InvalidBackupSpec", "NonAdminBackup does not contain valid BackupSpec") if errUpdateCondition != nil { - logger.Error(errUpdateCondition, "Unable to set BackupAccepted Condition: False", nameField, nab.Name, constant.NameSpaceString, nab.Namespace) + logger.Error(errUpdateCondition, "Unable to set BackupAccepted Condition: False") return true, false, errUpdateCondition } else if updatedCondition { return true, false, nil @@ -233,22 +209,21 @@ func (r *NonAdminBackupReconciler) ValidateSpec(ctx context.Context, logrLogger // We do not requeue - this was an error from getting Spec from NAB return true, false, err } - logger.V(1).Info("NonAdminBackup CR Spec validated successfully") // TODO is this needed? from design, does not seem a valid condition - // updatedStatus, errUpdateStatus := function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionTrue, "Validated", "Valid Backup config") - // if errUpdateStatus != nil { - // logger.Error(errUpdateStatus, "Unable to set BackupAccepted Condition: True", nameField, nab.Name, constant.NameSpaceString, nab.Namespace) - // return true, false, errUpdateStatus - // } else if updatedStatus { - // // We do requeue - The VeleroBackup got validated and next reconcile loop will continue - // // with further work on the VeleroBackup such as creating it - // return false, true, nil - // } - - // TODO move VeleroBackup Spec update to here? + // this keeps being called... + // this or UpdateNonAdminBackupCondition(..., "BackupAccepted", "Backup accepted") should be deleted + updatedStatus, errUpdateStatus := function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionTrue, "Validated", "Valid Backup config") + if errUpdateStatus != nil { + logger.Error(errUpdateStatus, "Unable to set BackupAccepted Condition: True") + return true, false, errUpdateStatus + } else if updatedStatus { + // We do requeue - The VeleroBackup got validated and next reconcile loop will continue + // with further work on the VeleroBackup such as creating it + return false, true, nil + } - // TODO change? + logger.V(1).Info("NonAdminBackup Spec already validated") return false, false, nil } @@ -265,7 +240,7 @@ func (r *NonAdminBackupReconciler) ValidateSpec(ctx context.Context, logrLogger // and updates NonAdminBackup Status. Otherwise, updates NonAdminBackup VeleroBackup Status based on Velero Backup object Status. // The function returns boolean values indicating whether the reconciliation loop should exit or requeue func (r *NonAdminBackupReconciler) UpdateSpecStatus(ctx context.Context, logrLogger logr.Logger, nab *nacv1alpha1.NonAdminBackup) (exitReconcile bool, requeueReconcile bool, errorReconcile error) { - logger := logrLogger.WithValues("UpdateSpecStatus", nab.Namespace) + logger := logrLogger.WithValues("UpdateSpecStatus NonAdminBackup", types.NamespacedName{Name: nab.Name, Namespace: nab.Namespace}) veleroBackupName := function.GenerateVeleroBackupName(nab.Namespace, nab.Name) @@ -275,16 +250,17 @@ func (r *NonAdminBackupReconciler) UpdateSpecStatus(ctx context.Context, logrLog oadpNamespace := function.GetOADPNamespace() veleroBackup := velerov1api.Backup{} + veleroBackupLogger := logger.WithValues("VeleroBackup", types.NamespacedName{Name: veleroBackupName, Namespace: oadpNamespace}) err := r.Get(ctx, client.ObjectKey{Namespace: oadpNamespace, Name: veleroBackupName}, &veleroBackup) if err != nil { if !apierrors.IsNotFound(err) { - logger.Error(err, "Unable to fetch VeleroBackup") + veleroBackupLogger.Error(err, "Unable to fetch VeleroBackup") return true, false, err } // Create VeleroBackup // Don't update phase nor conditions yet. // Those will be updated when then Reconcile loop is triggered by the VeleroBackup object - logger.Info("No backup found", nameField, veleroBackupName) + veleroBackupLogger.Info("VeleroBackup not found") // We don't validate error here. // This was already validated in the ValidateVeleroBackupSpec @@ -316,24 +292,24 @@ func (r *NonAdminBackupReconciler) UpdateSpecStatus(ctx context.Context, logrLog _, err = controllerutil.CreateOrPatch(ctx, r.Client, &veleroBackup, nil) if err != nil { - logger.Error(err, "Failed to create backup", nameField, veleroBackupName) + veleroBackupLogger.Error(err, "Failed to create VeleroBackup") return true, false, err } - logger.Info("VeleroBackup successfully created", nameField, veleroBackupName) + veleroBackupLogger.Info("VeleroBackup successfully created") _, errUpdate := function.UpdateNonAdminPhase(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminBackupPhaseCreated) if errUpdate != nil { - logger.Error(errUpdate, "Unable to set NonAdminBackup Phase: Created", nameField, nab.Name, constant.NameSpaceString, nab.Namespace) + logger.Error(errUpdate, "Unable to set NonAdminBackup Phase: Created") return true, false, errUpdate } _, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionTrue, "BackupAccepted", "Backup accepted") if errUpdate != nil { - logger.Error(errUpdate, "Unable to set BackupAccepted Condition: True", nameField, nab.Name, constant.NameSpaceString, nab.Namespace) + logger.Error(errUpdate, "Unable to set BackupAccepted Condition: True") return true, false, errUpdate } _, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionQueued, metav1.ConditionTrue, "BackupScheduled", "Created Velero Backup object") if errUpdate != nil { - logger.Error(errUpdate, "Unable to set BackupQueued Condition: True", nameField, nab.Name, constant.NameSpaceString, nab.Namespace) + logger.Error(errUpdate, "Unable to set BackupQueued Condition: True") return true, false, errUpdate } @@ -343,127 +319,20 @@ func (r *NonAdminBackupReconciler) UpdateSpecStatus(ctx context.Context, logrLog // The VeleroBackup within NonAdminBackup will // be reverted back to the previous state - the state which created VeleroBackup // in a first place, so they will be in sync. - logger.Info("Backup already exists, updating NonAdminBackup status", nameField, veleroBackupName) + veleroBackupLogger.Info("VeleroBackup already exists, updating NonAdminBackup status") updatedNab, errBackupUpdate := function.UpdateNonAdminBackupFromVeleroBackup(ctx, r.Client, logger, nab, &veleroBackup) // Regardless if the status was updated or not, we should not // requeue here as it was only status update. if errBackupUpdate != nil { return true, false, errBackupUpdate } else if updatedNab { - logger.V(1).Info("NonAdminBackup CR - Rqueue after Status Update") + logger.V(1).Info("NonAdminBackup - Requeue after Status Update") return false, true, nil } return true, false, nil } -// TODO refactor idea -// // CreateVeleroBackup -// // -// // TODO -// func (r *NonAdminBackupReconciler) CreateVeleroBackup(ctx context.Context, logrLogger logr.Logger, nab *nacv1alpha1.NonAdminBackup, veleroBackupName string, oadpNamespace string) (exitReconcile bool, requeueReconcile bool, errorReconcile error) { -// logger := logrLogger.WithValues("CreateVeleroBackup", nab.Namespace) - -// // Create VeleroBackup -// // Don't update phase nor conditions yet. -// // Those will be updated when then Reconcile loop is triggered by the VeleroBackup object -// logger.Info("No backup found", nameField, veleroBackupName) - -// // We don't validate error here. -// // This was already validated in the ValidateVeleroBackupSpec -// backupSpec, errBackup := function.GetBackupSpecFromNonAdminBackup(nab) - -// if errBackup != nil { -// // Should never happen as it was already checked -// return true, false, errBackup -// } - -// veleroBackup := velerov1api.Backup{ -// ObjectMeta: metav1.ObjectMeta{ -// Name: veleroBackupName, -// Namespace: oadpNamespace, -// }, -// Spec: *backupSpec, -// } - -// // Ensure labels are set for the Backup object -// existingLabels := veleroBackup.Labels -// naManagedLabels := function.AddNonAdminLabels(existingLabels) -// veleroBackup.Labels = naManagedLabels - -// // Ensure annotations are set for the Backup object -// existingAnnotations := veleroBackup.Annotations -// ownerUUID := string(nab.ObjectMeta.UID) -// nabManagedAnnotations := function.AddNonAdminBackupAnnotations(nab.Namespace, nab.Name, ownerUUID, existingAnnotations) -// veleroBackup.Annotations = nabManagedAnnotations - -// _, err := controllerutil.CreateOrPatch(ctx, r.Client, &veleroBackup, nil) -// if err != nil { -// logger.Error(err, "Failed to create Velero Backup", nameField, veleroBackupName) -// return true, false, err -// } -// logger.Info("Velero Backup successfully created", nameField, veleroBackupName) - -// // TODO -// return false, false, nil -// } - -// // UpdateStatusAfterVeleroBackupCreation -// // -// // TODO -// func (r *NonAdminBackupReconciler) UpdateStatusAfterVeleroBackupCreation(ctx context.Context, logrLogger logr.Logger, nab *nacv1alpha1.NonAdminBackup) (exitReconcile bool, requeueReconcile bool, errorReconcile error) { -// logger := logrLogger.WithValues("UpdateStatusAfterVeleroBackupCreation", nab.Namespace) - -// _, errUpdate := function.UpdateNonAdminPhase(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminBackupPhaseCreated) -// if errUpdate != nil { -// logger.Error(errUpdate, "Unable to set NonAdminBackup Phase: Created", nameField, nab.Name, constant.NameSpaceString, nab.Namespace) -// return true, false, errUpdate -// } -// _, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionTrue, "BackupAccepted", "Backup accepted") -// if errUpdate != nil { -// logger.Error(errUpdate, "Unable to set BackupAccepted Condition: True", nameField, nab.Name, constant.NameSpaceString, nab.Namespace) -// return true, false, errUpdate -// } -// _, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionQueued, metav1.ConditionTrue, "BackupScheduled", "Created Velero Backup object") -// if errUpdate != nil { -// logger.Error(errUpdate, "Unable to set BackupQueued Condition: True", nameField, nab.Name, constant.NameSpaceString, nab.Namespace) -// return true, false, errUpdate -// } - -// // TODO -// return false, false, nil -// } - -// // UpdateSpecStatus updates the Spec and Status from the NonAdminBackup. -// // -// // Parameters: -// // -// // ctx: Context for the request. -// // log: Logger instance for logging messages. -// // nab: Pointer to the NonAdminBackup object. -// // -// // The function generates a name for the Velero Backup object based on the provided namespace and name. -// // It then checks if a Velero Backup object with that name already exists. If it does not exist, it creates a new one -// // and updates NonAdminBackup Status. Otherwise, updates NonAdminBackup VeleroBackup Status based on Velero Backup object Status. -// // The function returns boolean values indicating whether the reconciliation loop should exit or requeue -// func (r *NonAdminBackupReconciler) UpdateSpecStatus(ctx context.Context, logrLogger logr.Logger, nab *nacv1alpha1.NonAdminBackup, veleroBackup velerov1api.Backup) (exitReconcile bool, requeueReconcile bool, errorReconcile error) { -// logger := logrLogger.WithValues("UpdateSpecStatus", nab.Namespace) - -// // We should not update already created VeleroBackup object. -// // The VeleroBackup within NonAdminBackup will -// // be reverted back to the previous state - the state which created VeleroBackup -// // in a first place, so they will be in sync. -// // logger.Info("Backup already exists, updating NonAdminBackup status", nameField, veleroBackup.Name) -// updatedNab, errBackupUpdate := function.UpdateNonAdminBackupFromVeleroBackup(ctx, r.Client, logger, nab, &veleroBackup) -// // Regardless if the status was updated or not, we should not -// // requeue here as it was only status update. -// if errBackupUpdate != nil { -// return true, false, errBackupUpdate -// } else if updatedNab { -// logger.V(1).Info("NonAdminBackup CR - Requeue after Status Update") -// return false, true, nil -// } -// return true, false, nil -// } +// TODO refactor idea: break in smaller functions: CreateVeleroBackup, UpdateStatusAfterVeleroBackupCreation and UpdateSpecStatus // SetupWithManager sets up the controller with the Manager. func (r *NonAdminBackupReconciler) SetupWithManager(mgr ctrl.Manager) error { diff --git a/internal/controller/nonadminbackup_controller_test.go b/internal/controller/nonadminbackup_controller_test.go index 3ca4d74..5b8f2b5 100644 --- a/internal/controller/nonadminbackup_controller_test.go +++ b/internal/controller/nonadminbackup_controller_test.go @@ -19,6 +19,7 @@ package controller import ( "context" "os" + "time" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" @@ -26,46 +27,35 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/reconcile" nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" "github.com/migtools/oadp-non-admin/internal/common/constant" ) +const testNonAdminBackupName = "test-non-admin-backup" + type nonAdminBackupReconcileScenario struct { - namespace string - nonAdminBackup string - oadpNamespace string - spec nacv1alpha1.NonAdminBackupSpec - status nacv1alpha1.NonAdminBackupStatus - doNotCreateNonAdminBackup bool + namespace string + oadpNamespace string + spec nacv1alpha1.NonAdminBackupSpec + priorStatus *nacv1alpha1.NonAdminBackupStatus + status nacv1alpha1.NonAdminBackupStatus + result reconcile.Result } -func createTestNonAdminBackup(name string, namespace string, spec nacv1alpha1.NonAdminBackupSpec) *nacv1alpha1.NonAdminBackup { +func createTestNonAdminBackup(namespace string, spec nacv1alpha1.NonAdminBackupSpec) *nacv1alpha1.NonAdminBackup { return &nacv1alpha1.NonAdminBackup{ ObjectMeta: metav1.ObjectMeta{ - Name: name, + Name: testNonAdminBackupName, Namespace: namespace, }, Spec: spec, } } -func ruNonAdminBackupReconcilerUntilExit(r *NonAdminBackupReconciler, scenario nonAdminBackupReconcileScenario) (reconcile.Result, error) { - result, err := r.Reconcile( - context.Background(), - reconcile.Request{NamespacedName: types.NamespacedName{ - Namespace: scenario.namespace, - Name: scenario.nonAdminBackup, - }}, - ) - if err == nil && result.Requeue { - return ruNonAdminBackupReconcilerUntilExit(r, scenario) - } - return result, err -} - -var _ = ginkgo.Describe("Test NonAdminBackup Reconcile function", func() { +var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile function", func() { var ( ctx = context.Background() currentTestScenario nonAdminBackupReconcileScenario @@ -89,7 +79,7 @@ var _ = ginkgo.Describe("Test NonAdminBackup Reconcile function", func() { if k8sClient.Get( ctx, types.NamespacedName{ - Name: currentTestScenario.nonAdminBackup, + Name: testNonAdminBackupName, Namespace: currentTestScenario.namespace, }, nonAdminBackup, @@ -105,8 +95,38 @@ var _ = ginkgo.Describe("Test NonAdminBackup Reconcile function", func() { gomega.Expect(k8sClient.Delete(ctx, namespace)).To(gomega.Succeed()) }) - // TODO need to test more reconcile cases... - ginkgo.DescribeTable("Reconcile without error", + ginkgo.DescribeTable("Reconcile should NOT return an error on Delete event", + func(scenario nonAdminBackupReconcileScenario) { + updateTestScenario(scenario) + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: scenario.namespace, + }, + } + gomega.Expect(k8sClient.Create(ctx, namespace)).To(gomega.Succeed()) + + result, err := (&NonAdminBackupReconciler{ + Client: k8sClient, + Scheme: testEnv.Scheme, + }).Reconcile( + context.Background(), + reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: scenario.namespace, + Name: testNonAdminBackupName, + }}, + ) + + gomega.Expect(result).To(gomega.Equal(scenario.result)) + gomega.Expect(err).To(gomega.Not(gomega.HaveOccurred())) + }, + ginkgo.Entry("Should accept deletion of NonAdminBackup", nonAdminBackupReconcileScenario{ + namespace: "test-nonadminbackup-reconcile-0", + result: reconcile.Result{}, + }), + ) + + ginkgo.DescribeTable("Reconcile should NOT return an error on Create and Update events", func(scenario nonAdminBackupReconcileScenario) { updateTestScenario(scenario) @@ -117,9 +137,12 @@ var _ = ginkgo.Describe("Test NonAdminBackup Reconcile function", func() { } gomega.Expect(k8sClient.Create(ctx, namespace)).To(gomega.Succeed()) - if !scenario.doNotCreateNonAdminBackup { - nonAdminBackup := createTestNonAdminBackup(scenario.nonAdminBackup, scenario.namespace, scenario.spec) - gomega.Expect(k8sClient.Create(ctx, nonAdminBackup)).To(gomega.Succeed()) + nonAdminBackup := createTestNonAdminBackup(scenario.namespace, scenario.spec) + gomega.Expect(k8sClient.Create(ctx, nonAdminBackup)).To(gomega.Succeed()) + + if scenario.priorStatus != nil { + nonAdminBackup.Status = *scenario.priorStatus + gomega.Expect(k8sClient.Status().Update(ctx, nonAdminBackup)).To(gomega.Succeed()) } if len(scenario.oadpNamespace) > 0 { @@ -132,76 +155,271 @@ var _ = ginkgo.Describe("Test NonAdminBackup Reconcile function", func() { gomega.Expect(k8sClient.Create(ctx, oadpNamespace)).To(gomega.Succeed()) } - r := &NonAdminBackupReconciler{ + result, err := (&NonAdminBackupReconciler{ Client: k8sClient, Scheme: testEnv.Scheme, - } - - result, err := ruNonAdminBackupReconcilerUntilExit(r, scenario) + }).Reconcile( + context.Background(), + reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: scenario.namespace, + Name: testNonAdminBackupName, + }}, + ) // TODO need to collect logs, so they do not appear in test run // also assert them - - gomega.Expect(result).To(gomega.Equal(reconcile.Result{Requeue: false, RequeueAfter: 0})) + gomega.Expect(result).To(gomega.Equal(scenario.result)) gomega.Expect(err).To(gomega.Not(gomega.HaveOccurred())) - if !scenario.doNotCreateNonAdminBackup { - nonAdminBackup := &nacv1alpha1.NonAdminBackup{} - gomega.Expect(k8sClient.Get( - ctx, - types.NamespacedName{ - Name: currentTestScenario.nonAdminBackup, - Namespace: currentTestScenario.namespace, - }, - nonAdminBackup, - )).To(gomega.Succeed()) - gomega.Expect(nonAdminBackup.Status.Phase).To(gomega.Equal(scenario.status.Phase)) - for index := range nonAdminBackup.Status.Conditions { - gomega.Expect(nonAdminBackup.Status.Conditions[index].Type).To(gomega.Equal(scenario.status.Conditions[index].Type)) - gomega.Expect(nonAdminBackup.Status.Conditions[index].Status).To(gomega.Equal(scenario.status.Conditions[index].Status)) - gomega.Expect(nonAdminBackup.Status.Conditions[index].Reason).To(gomega.Equal(scenario.status.Conditions[index].Reason)) - gomega.Expect(nonAdminBackup.Status.Conditions[index].Message).To(gomega.Equal(scenario.status.Conditions[index].Message)) - } + // if !scenario.doNotCreateNonAdminBackup { + // nonAdminBackup := &nacv1alpha1.NonAdminBackup{} + // gomega.Eventually(func() nacv1alpha1.NonAdminBackupPhase { + // k8sClient.Get( + // ctx, + // types.NamespacedName{ + // Name: testNonAdminBackupName, + // Namespace: currentTestScenario.namespace, + // }, + // nonAdminBackup, + // ) + // return nonAdminBackup.Status.Phase + // }, 30*time.Second, 1*time.Second).Should(gomega.Equal(scenario.status.Phase)) + + gomega.Expect(k8sClient.Get( + ctx, + types.NamespacedName{ + Name: testNonAdminBackupName, + Namespace: currentTestScenario.namespace, + }, + nonAdminBackup, + )).To(gomega.Succeed()) + gomega.Expect(nonAdminBackup.Status.Phase).To(gomega.Equal(scenario.status.Phase)) + gomega.Expect(nonAdminBackup.Status.VeleroBackupName).To(gomega.Equal(scenario.status.VeleroBackupName)) + gomega.Expect(nonAdminBackup.Status.VeleroBackupNamespace).To(gomega.Equal(scenario.status.VeleroBackupNamespace)) + gomega.Expect(nonAdminBackup.Status.VeleroBackupStatus).To(gomega.Equal(scenario.status.VeleroBackupStatus)) + + for index := range nonAdminBackup.Status.Conditions { + gomega.Expect(nonAdminBackup.Status.Conditions[index].Type).To(gomega.Equal(scenario.status.Conditions[index].Type)) + gomega.Expect(nonAdminBackup.Status.Conditions[index].Status).To(gomega.Equal(scenario.status.Conditions[index].Status)) + gomega.Expect(nonAdminBackup.Status.Conditions[index].Reason).To(gomega.Equal(scenario.status.Conditions[index].Reason)) + gomega.Expect(nonAdminBackup.Status.Conditions[index].Message).To(gomega.Equal(scenario.status.Conditions[index].Message)) } }, - ginkgo.Entry("Should NOT accept non existing nonAdminBackup", nonAdminBackupReconcileScenario{ - namespace: "test-nonadminbackup-reconcile-1", - nonAdminBackup: "test-nonadminbackup-reconcile-1-cr", - doNotCreateNonAdminBackup: true, - // TODO should have loop end in logs - // TODO unnecessary duplication in logs - // {"NonAdminBackup": {"name":"test-nonadminbackup-reconcile-1-cr","namespace":"test-nonadminbackup-reconcile-1"}, - // "Name": "test-nonadminbackup-reconcile-1-cr", "Namespace": "test-nonadminbackup-reconcile-1"} + ginkgo.Entry("Should accept creation of NonAdminBackup", nonAdminBackupReconcileScenario{ + namespace: "test-nonadminbackup-reconcile-1", + // even without providing spec, this does not fail... + result: reconcile.Result{Requeue: true, RequeueAfter: 10 * time.Second}, + status: nacv1alpha1.NonAdminBackupStatus{ + Phase: nacv1alpha1.NonAdminBackupPhaseNew, + }, + }), + ginkgo.Entry("Should accept update of NonAdminBackup phase", nonAdminBackupReconcileScenario{ + namespace: "test-nonadminbackup-reconcile-2", + spec: nacv1alpha1.NonAdminBackupSpec{ + BackupSpec: &v1.BackupSpec{}, + }, + priorStatus: &nacv1alpha1.NonAdminBackupStatus{ + Phase: nacv1alpha1.NonAdminBackupPhaseNew, + }, + result: reconcile.Result{Requeue: true, RequeueAfter: 10 * time.Second}, + status: nacv1alpha1.NonAdminBackupStatus{ + Phase: nacv1alpha1.NonAdminBackupPhaseNew, + Conditions: []metav1.Condition{ + // Is this a valid Condition??? + { + Type: "Accepted", + Status: metav1.ConditionTrue, + Reason: "Validated", + Message: "Valid Backup config", + }, + }, + }, }), - ginkgo.Entry("Should NOT accept NonAdminBackup with empty backupSpec", nonAdminBackupReconcileScenario{ - namespace: "test-nonadminbackup-reconcile-2", - nonAdminBackup: "test-nonadminbackup-reconcile-2-cr", - spec: nacv1alpha1.NonAdminBackupSpec{}, + ginkgo.Entry("Should accept update of NonAdminBackup Condition", nonAdminBackupReconcileScenario{ + namespace: "test-nonadminbackup-reconcile-3", + oadpNamespace: "test-nonadminbackup-reconcile-3-oadp", + spec: nacv1alpha1.NonAdminBackupSpec{ + BackupSpec: &v1.BackupSpec{}, + }, + priorStatus: &nacv1alpha1.NonAdminBackupStatus{ + Phase: nacv1alpha1.NonAdminBackupPhaseNew, + Conditions: []metav1.Condition{ + // Is this a valid Condition??? + { + Type: "Accepted", + Status: metav1.ConditionTrue, + Reason: "Validated", + Message: "Valid Backup config", + LastTransitionTime: metav1.NewTime(time.Now()), + }, + }, + }, + status: nacv1alpha1.NonAdminBackupStatus{ + // TODO should not have VeleroBackupName and VeleroBackupNamespace? + Phase: nacv1alpha1.NonAdminBackupPhaseCreated, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + Reason: "BackupAccepted", + Message: "Backup accepted", + }, + { + Type: "Queued", + Status: metav1.ConditionTrue, + Reason: "BackupScheduled", + Message: "Created Velero Backup object", + }, + }, + }, + }), + ginkgo.Entry("Should NOT accept update of NonAdminBackup phase because of empty backupSpec", nonAdminBackupReconcileScenario{ + namespace: "test-nonadminbackup-reconcile-4", + spec: nacv1alpha1.NonAdminBackupSpec{}, + priorStatus: &nacv1alpha1.NonAdminBackupStatus{ + Phase: nacv1alpha1.NonAdminBackupPhaseNew, + }, status: nacv1alpha1.NonAdminBackupStatus{ Phase: nacv1alpha1.NonAdminBackupPhaseBackingOff, }, }), - // TODO should not have loop start again in logs - // TODO error message duplication - // TODO should have loop end in logs - ginkgo.Entry("Should NOT accept NonAdminBackup with includedNamespaces pointing to different namespace", nonAdminBackupReconcileScenario{ - namespace: "test-nonadminbackup-reconcile-3", - nonAdminBackup: "test-nonadminbackup-reconcile-3-cr", + ginkgo.Entry("Should NOT accept update of NonAdminBackup phase because of includedNamespaces pointing to different namespace", nonAdminBackupReconcileScenario{ + namespace: "test-nonadminbackup-reconcile-5", spec: nacv1alpha1.NonAdminBackupSpec{ BackupSpec: &v1.BackupSpec{ IncludedNamespaces: []string{"not-valid"}, }, }, + priorStatus: &nacv1alpha1.NonAdminBackupStatus{ + Phase: nacv1alpha1.NonAdminBackupPhaseNew, + }, status: nacv1alpha1.NonAdminBackupStatus{ Phase: nacv1alpha1.NonAdminBackupPhaseBackingOff, }, }), - // TODO should not have loop start again in logs - // TODO error message duplication - // TODO should have loop end in logs - ginkgo.Entry("Should accept NonAdminBackup and create Velero Backup", nonAdminBackupReconcileScenario{ - namespace: "test-nonadminbackup-reconcile-4", - nonAdminBackup: "test-nonadminbackup-reconcile-4-cr", - oadpNamespace: "test-nonadminbackup-reconcile-4-oadp", + ) +}) + +var _ = ginkgo.Describe("Test full reconciles of NonAdminBackup Reconcile function", func() { + var ( + ctx, cancel = context.WithCancel(context.Background()) + currentTestScenario nonAdminBackupReconcileScenario + updateTestScenario = func(scenario nonAdminBackupReconcileScenario) { + currentTestScenario = scenario + } + ) + + ginkgo.AfterEach(func() { + gomega.Expect(os.Unsetenv(constant.NamespaceEnvVar)).To(gomega.Succeed()) + if len(currentTestScenario.oadpNamespace) > 0 { + oadpNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: currentTestScenario.oadpNamespace, + }, + } + gomega.Expect(k8sClient.Delete(ctx, oadpNamespace)).To(gomega.Succeed()) + } + + // nonAdminBackup := &nacv1alpha1.NonAdminBackup{} + // if k8sClient.Get( + // ctx, + // types.NamespacedName{ + // Name: testNonAdminBackupName, + // Namespace: currentTestScenario.namespace, + // }, + // nonAdminBackup, + // ) == nil { + // gomega.Expect(k8sClient.Delete(ctx, nonAdminBackup)).To(gomega.Succeed()) + // } + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: currentTestScenario.namespace, + }, + } + gomega.Expect(k8sClient.Delete(ctx, namespace)).To(gomega.Succeed()) + cancel() + }) + + ginkgo.DescribeTable("Reconcile should NOT return an error", + func(scenario nonAdminBackupReconcileScenario) { + updateTestScenario(scenario) + + gomega.Expect(os.Setenv(constant.NamespaceEnvVar, scenario.oadpNamespace)).To(gomega.Succeed()) + oadpNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: scenario.oadpNamespace, + }, + } + gomega.Expect(k8sClient.Create(ctx, oadpNamespace)).To(gomega.Succeed()) + + k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: k8sClient.Scheme(), + }) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + err = (&NonAdminBackupReconciler{ + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + }).SetupWithManager(k8sManager) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + // TODO Be CAREFUL about FLAKES with this approach? + // study ref https://book.kubebuilder.io/cronjob-tutorial/writing-tests + go func() { + defer ginkgo.GinkgoRecover() + err = k8sManager.Start(ctx) + gomega.Expect(err).ToNot(gomega.HaveOccurred(), "failed to run manager") + }() + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: scenario.namespace, + }, + } + gomega.Expect(k8sClient.Create(ctx, namespace)).To(gomega.Succeed()) + + nonAdminBackup := createTestNonAdminBackup(scenario.namespace, scenario.spec) + gomega.Expect(k8sClient.Create(ctx, nonAdminBackup)).To(gomega.Succeed()) + + gomega.Eventually(func() (nacv1alpha1.NonAdminBackupPhase, error) { + err := k8sClient.Get( + ctx, + types.NamespacedName{ + Name: testNonAdminBackupName, + Namespace: currentTestScenario.namespace, + }, + nonAdminBackup, + ) + if err != nil { + return "", err + } + return nonAdminBackup.Status.Phase, nil + // TOO MUCH TIME!!!! + }, 30*time.Second, 1*time.Second).Should(gomega.Equal(scenario.status.Phase)) + + gomega.Expect(k8sClient.Get( + ctx, + types.NamespacedName{ + Name: testNonAdminBackupName, + Namespace: currentTestScenario.namespace, + }, + nonAdminBackup, + )).To(gomega.Succeed()) + gomega.Expect(nonAdminBackup.Status.Phase).To(gomega.Equal(scenario.status.Phase)) + gomega.Expect(nonAdminBackup.Status.VeleroBackupName).To(gomega.Equal(scenario.status.VeleroBackupName)) + gomega.Expect(nonAdminBackup.Status.VeleroBackupNamespace).To(gomega.Equal(scenario.status.VeleroBackupNamespace)) + gomega.Expect(nonAdminBackup.Status.VeleroBackupStatus).To(gomega.Equal(scenario.status.VeleroBackupStatus)) + + for index := range nonAdminBackup.Status.Conditions { + gomega.Expect(nonAdminBackup.Status.Conditions[index].Type).To(gomega.Equal(scenario.status.Conditions[index].Type)) + gomega.Expect(nonAdminBackup.Status.Conditions[index].Status).To(gomega.Equal(scenario.status.Conditions[index].Status)) + gomega.Expect(nonAdminBackup.Status.Conditions[index].Reason).To(gomega.Equal(scenario.status.Conditions[index].Reason)) + gomega.Expect(nonAdminBackup.Status.Conditions[index].Message).To(gomega.Equal(scenario.status.Conditions[index].Message)) + } + }, + ginkgo.Entry("Should DO FULL happy path", nonAdminBackupReconcileScenario{ + namespace: "test-nonadminbackup-reconcile-full-1", + oadpNamespace: "test-nonadminbackup-reconcile-full-1-oadp", spec: nacv1alpha1.NonAdminBackupSpec{ BackupSpec: &v1.BackupSpec{}, }, @@ -212,8 +430,8 @@ var _ = ginkgo.Describe("Test NonAdminBackup Reconcile function", func() { { Type: "Accepted", Status: metav1.ConditionTrue, - Reason: "BackupAccepted", - Message: "Backup accepted", + Reason: "Validated", + Message: "Valid Backup config", }, { Type: "Queued", @@ -224,9 +442,5 @@ var _ = ginkgo.Describe("Test NonAdminBackup Reconcile function", func() { }, }, }), - // TODO should not have loop start again and again in logs - // TODO 3 condition logs, only 2 in CR status? - - // TODO create tests for single reconciles, so we can test https://github.com/migtools/oadp-non-admin/blob/master/docs/design/nab_status_update.md ) }) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 63d9578..4f99475 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -22,7 +22,7 @@ import ( "runtime" "testing" - ginkgov2 "github.com/onsi/ginkgo/v2" + "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "k8s.io/client-go/kubernetes/scheme" @@ -43,15 +43,15 @@ var k8sClient client.Client var testEnv *envtest.Environment func TestControllers(t *testing.T) { - gomega.RegisterFailHandler(ginkgov2.Fail) + gomega.RegisterFailHandler(ginkgo.Fail) - ginkgov2.RunSpecs(t, "Controller Suite") + ginkgo.RunSpecs(t, "Controller Suite") } -var _ = ginkgov2.BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(ginkgov2.GinkgoWriter), zap.UseDevMode(true))) +var _ = ginkgo.BeforeSuite(func() { + logf.SetLogger(zap.New(zap.WriteTo(ginkgo.GinkgoWriter), zap.UseDevMode(true))) - ginkgov2.By("bootstrapping test environment") + ginkgo.By("bootstrapping test environment") testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{ filepath.Join("..", "..", "config", "crd", "bases"), @@ -86,8 +86,8 @@ var _ = ginkgov2.BeforeSuite(func() { gomega.Expect(k8sClient).NotTo(gomega.BeNil()) }) -var _ = ginkgov2.AfterSuite(func() { - ginkgov2.By("tearing down the test environment") +var _ = ginkgo.AfterSuite(func() { + ginkgo.By("tearing down the test environment") err := testEnv.Stop() gomega.Expect(err).NotTo(gomega.HaveOccurred()) }) diff --git a/internal/predicate/nonadminbackup_predicate.go b/internal/predicate/nonadminbackup_predicate.go index 2163bb5..b15e086 100644 --- a/internal/predicate/nonadminbackup_predicate.go +++ b/internal/predicate/nonadminbackup_predicate.go @@ -76,6 +76,9 @@ func (NonAdminBackupPredicate) Update(ctx context.Context, evt event.UpdateEvent logger.V(1).Info("NonAdminBackupPredicate: Accepted Update event - phase change") return true } else if oldPhase == nacv1alpha1.NonAdminBackupPhaseNew && newPhase == nacv1alpha1.NonAdminBackupPhaseCreated { + // This is HARD to understand and TEST + // even though reconcile will reach Reconcile loop end + // this will trigger a new reconcile logger.V(1).Info("NonAdminBackupPredicate: Accepted Update event - phase created") return true }