From e0a900480d22c11bd2eb512386b9d5fa9e2d9aec Mon Sep 17 00:00:00 2001 From: Michal Pryc Date: Mon, 29 Apr 2024 15:09:53 +0200 Subject: [PATCH] NonAdminBackup reconcile loop major rework Rework of the reconcile loop to include batch reconcile. Move assignment of the Log or Context outisde of the reconcile loop. Signed-off-by: Michal Pryc --- internal/controller/nab_init.go | 59 ++++++ internal/controller/nab_validator.go | 93 +++++++++ internal/controller/nab_velero_backupspec.go | 132 +++++++++++++ internal/controller/nac_reconcile_utils.go | 81 ++++++++ .../controller/nac_reconcile_utils_test.go | 110 +++++++++++ .../controller/nonadminbackup_controller.go | 182 +++--------------- 6 files changed, 502 insertions(+), 155 deletions(-) create mode 100644 internal/controller/nab_init.go create mode 100644 internal/controller/nab_validator.go create mode 100644 internal/controller/nab_velero_backupspec.go create mode 100644 internal/controller/nac_reconcile_utils.go create mode 100644 internal/controller/nac_reconcile_utils_test.go diff --git a/internal/controller/nab_init.go b/internal/controller/nab_init.go new file mode 100644 index 0000000..493d572 --- /dev/null +++ b/internal/controller/nab_init.go @@ -0,0 +1,59 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + + "github.com/go-logr/logr" + nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" + "github.com/migtools/oadp-non-admin/internal/common/constant" + "github.com/migtools/oadp-non-admin/internal/common/function" +) + +// InitNonAdminBackup sets the New Phase on a NonAdminBackup object if it is not already set. +// +// Parameters: +// +// log: Logger instance for logging messages. +// ctx: Context for the request. +// Name: Name of the NonAdminBackup object. +// NameSpace: Namespace of the NonAdminBackup object. +// nab: Pointer to the NonAdminBackup object. +// +// The function checks if the Phase of the NonAdminBackup object is empty. +// If it is empty, it sets the Phase to "New". +// It then returns boolean values indicating whether the reconciliation loop should requeue +// and whether the status was updated. +func (r *NonAdminBackupReconciler) InitNonAdminBackup(log logr.Logger, ctx context.Context, Name string, NameSpace string, nab *nacv1alpha1.NonAdminBackup) (bool, bool, error) { + logger := log.WithValues("NonAdminBackup", NameSpace) + // Set initial Phase + if nab.Status.Phase == constant.EmptyString { + // Phase: New + updatedStatus, errUpdate := function.UpdateNonAdminPhase(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminBackupPhaseNew) + if updatedStatus { + logger.V(1).Info("NonAdminBackup CR - Rqueue after Phase Update") + return false, true, nil + } + if errUpdate != nil { + logger.Error(errUpdate, "Unable to set NonAdminBackup Phase: New", nameField, Name, constant.NameSpaceString, NameSpace) + return true, false, errUpdate + } + } + + return false, false, nil +} diff --git a/internal/controller/nab_validator.go b/internal/controller/nab_validator.go new file mode 100644 index 0000000..655ec4f --- /dev/null +++ b/internal/controller/nab_validator.go @@ -0,0 +1,93 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + + "github.com/go-logr/logr" + nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" + + "github.com/migtools/oadp-non-admin/internal/common/constant" + "github.com/migtools/oadp-non-admin/internal/common/function" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// ValidateVeleroBackupSpec validates the VeleroBackup Spec from the NonAdminBackup. +// +// Parameters: +// +// log: Logger instance for logging messages. +// ctx: Context for the request. +// Name: Name of the NonAdminBackup object. +// NameSpace: Namespace of the NonAdminBackup object. +// nab: Pointer to the NonAdminBackup object. +// +// The function attempts to get the BackupSpec from the NonAdminBackup object. +// If an error occurs during this process, the function sets the NonAdminBackup status to "BackingOff" +// and updates the corresponding condition accordingly. +// If the BackupSpec is invalid, the function sets the NonAdminBackup condition to "InvalidBackupSpec". +// If the BackupSpec is valid, the function sets the NonAdminBackup condition to "BackupAccepted". +func (r *NonAdminBackupReconciler) ValidateVeleroBackupSpec(log logr.Logger, ctx context.Context, Name string, NameSpace string, nab *nacv1alpha1.NonAdminBackup) (bool, bool, error) { + logger := log.WithValues("NonAdminBackup", NameSpace) + + // Main Validation point for the VeleroBackup included in NonAdminBackup spec + _, err := function.GetBackupSpecFromNonAdminBackup(nab) + + if err != nil { + errMsg := "NonAdminBackup CR does not contain valid BackupSpec" + logger.Error(err, errMsg) + + updatedStatus, errUpdateStatus := function.UpdateNonAdminPhase(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminBackupPhaseBackingOff) + if errUpdateStatus != nil { + logger.Error(errUpdateStatus, "Unable to set NonAdminBackup Phase: BackingOff", nameField, Name, constant.NameSpaceString, NameSpace) + return true, false, errUpdateStatus + } else if updatedStatus { + // We do not requeue - the State was set to BackingOff + return true, false, nil + } + + // Continue. VeleroBackup looks fine, setting Accepted condition + updatedCondition, errUpdateCondition := function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionFalse, "InvalidBackupSpec", errMsg) + if updatedCondition { + // We do not requeue - this was only Condition update + return true, false, nil + } + + if errUpdateCondition != nil { + logger.Error(errUpdateCondition, "Unable to set BackupAccepted Condition: False", nameField, Name, constant.NameSpaceString, NameSpace) + return true, false, errUpdateCondition + } + // We do not requeue - this was error from getting Spec from NAB + return true, false, err + } + + updatedStatus, errUpdateStatus := function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionTrue, "BackupAccepted", "backup accepted") + if updatedStatus { + // We do requeue - The VeleroBackup got accepted and next reconcile loop will continue + // with further work on the VeleroBackup such as creating it + return false, true, nil + } + + if errUpdateStatus != nil { + logger.Error(errUpdateStatus, "Unable to set BackupAccepted Condition: True", nameField, Name, constant.NameSpaceString, NameSpace) + return true, false, errUpdateStatus + } + + return false, false, nil +} diff --git a/internal/controller/nab_velero_backupspec.go b/internal/controller/nab_velero_backupspec.go new file mode 100644 index 0000000..ba58f55 --- /dev/null +++ b/internal/controller/nab_velero_backupspec.go @@ -0,0 +1,132 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "errors" + + "github.com/go-logr/logr" + nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" + "github.com/migtools/oadp-non-admin/internal/common/constant" + "github.com/migtools/oadp-non-admin/internal/common/function" + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +// CreateVeleroBackupSpec creates or updates a Velero Backup object based on the provided NonAdminBackup object. +// +// Parameters: +// +// log: Logger instance for logging messages. +// ctx: Context for the request. +// Name: Name of the NonAdminBackup object. +// NameSpace: Namespace of the NonAdminBackup object. +// 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. +// The function returns boolean values indicating whether the reconciliation loop should exit or requeue +func (r *NonAdminBackupReconciler) CreateVeleroBackupSpec(log logr.Logger, ctx context.Context, Name string, NameSpace string, nab *nacv1alpha1.NonAdminBackup) (bool, bool, error) { + logger := log.WithValues("NonAdminBackup", NameSpace) + + veleroBackupName := function.GenerateVeleroBackupName(NameSpace, Name) + + if veleroBackupName == constant.EmptyString { + return true, false, errors.New("unable to generate Velero Backup name") + } + + veleroBackup := velerov1api.Backup{} + err := r.Get(ctx, client.ObjectKey{Namespace: constant.OadpNamespace, Name: veleroBackupName}, &veleroBackup) + + if err != nil && apierrors.IsNotFound(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) + + // We don't validate error here. + // This was already validated in the ValidateVeleroBackupSpec + backupSpec, _ := function.GetBackupSpecFromNonAdminBackup(nab) + + veleroBackup = velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: veleroBackupName, + Namespace: constant.OadpNamespace, + }, + Spec: *backupSpec, + } + } else if err != nil && !apierrors.IsNotFound(err) { + logger.Error(err, "Unable to fetch VeleroBackup") + return true, false, err + } else { + // 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, veleroBackupName) + 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") + return false, true, nil + } + return true, false, nil + } + + // 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 backup", nameField, veleroBackupName) + return true, false, err + } + logger.Info("VeleroBackup successfully created", nameField, veleroBackupName) + + _, errUpdate := function.UpdateNonAdminPhase(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminBackupPhaseCreated) + if errUpdate != nil { + logger.Error(errUpdate, "Unable to set NonAdminBackup Phase: Created", nameField, Name, constant.NameSpaceString, NameSpace) + return true, false, errUpdate + } + _, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionTrue, "Validated", "Valid Backup config") + if errUpdate != nil { + logger.Error(errUpdate, "Unable to set BackupAccepted Condition: True", nameField, Name, constant.NameSpaceString, 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, Name, constant.NameSpaceString, NameSpace) + return true, false, errUpdate + } + + return false, false, nil +} diff --git a/internal/controller/nac_reconcile_utils.go b/internal/controller/nac_reconcile_utils.go new file mode 100644 index 0000000..42ab0a8 --- /dev/null +++ b/internal/controller/nac_reconcile_utils.go @@ -0,0 +1,81 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import "errors" + +// ReconcileFunc represents a function that performs a reconciliation operation. +// Parameters: +// - ...interface{}: A variadic number of arguments of type interface{}, +// allowing for flexibility in passing parameters to the reconciliation function. +// +// Returns: +// - bool: Indicates whether the reconciliation process should exit. +// - bool: Indicates whether the reconciliation process should requeue. +// - error: An error encountered during reconciliation, if any. +type ReconcileFunc func(...interface{}) (bool, bool, error) + +// ReconcileBatch executes a batch of reconcile functions sequentially, +// allowing for complex reconciliation processes in a controlled manner. +// It iterates through the provided reconcile functions until one of the +// following conditions is met: +// - A function signals the need to exit the reconciliation process. +// - A function signals the need to exit and requeue the reconciliation process. +// - An error occurs during reconciliation. +// +// If any reconcile function indicates a need to exit or requeue, +// the function immediately returns with the respective exit and requeue +// flags set. If an error occurs during any reconciliation function call, +// it is propagated up and returned. If none of the reconcile functions +// indicate a need to exit, requeue, or result in an error, the function +// returns false for both exit and requeue flags, indicating successful reconciliation. +// +// If a reconcile function signals both the need to exit and requeue, indicating +// conflicting signals, the function returns an error with the exit flag set to true +// and the requeue flag set to false, so no . +// +// Parameters: +// - reconcileFuncs: A list of ReconcileFunc functions representing +// the reconciliation steps to be executed sequentially. +// +// Returns: +// - bool: Indicates whether the reconciliation process should exit. +// - bool: Indicates whether the reconciliation process should requeue. +// - error: An error encountered during reconciliation, if any. +func ReconcileBatch(reconcileFuncs ...ReconcileFunc) (bool, bool, error) { + var exit, requeue bool + var err error + var conflictError = errors.New("conflicting exit and requeue signals - can not be both true") + + for _, f := range reconcileFuncs { + exit, requeue, err = f() + + // Check if both exit and requeue are true + // If this happens do not requeue, but exit with error + if exit && requeue { + return true, false, conflictError + } + + // Return if there is a need to exit, requeue, or an error occurred + if exit || requeue || err != nil { + return exit, requeue, err + } + } + + // Do not requeue or exit reconcile. Also no error occured. + return false, false, nil +} diff --git a/internal/controller/nac_reconcile_utils_test.go b/internal/controller/nac_reconcile_utils_test.go new file mode 100644 index 0000000..a8079ea --- /dev/null +++ b/internal/controller/nac_reconcile_utils_test.go @@ -0,0 +1,110 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "errors" + "testing" +) + +func sampleReconcileFunc1(args ...interface{}) (bool, bool, error) { + return false, false, nil +} + +func sampleReconcileFunc2(args ...interface{}) (bool, bool, error) { + return true, false, nil +} + +func sampleReconcileFunc3(args ...interface{}) (bool, bool, error) { + return false, true, nil +} + +func sampleReconcileFuncWithError(args ...interface{}) (bool, bool, error) { + return false, false, errors.New("sample error") +} + +func sampleConflictingReconcileFunc(args ...interface{}) (bool, bool, error) { + return true, true, nil +} + +func TestReconcileBatch(t *testing.T) { + tests := []struct { + name string + reconcileFuncs []ReconcileFunc + expectedExit bool + expectedRequeue bool + expectedError error + }{ + { + name: "No functions", + reconcileFuncs: nil, + expectedExit: false, + expectedRequeue: false, + expectedError: nil, + }, + { + name: "All functions return false", + reconcileFuncs: []ReconcileFunc{sampleReconcileFunc1, sampleReconcileFunc1}, + expectedExit: false, + expectedRequeue: false, + expectedError: nil, + }, + { + name: "First function returns true", + reconcileFuncs: []ReconcileFunc{sampleReconcileFunc2, sampleReconcileFunc1}, + expectedExit: true, + expectedRequeue: false, + expectedError: nil, + }, + { + name: "Second function returns requeue", + reconcileFuncs: []ReconcileFunc{sampleReconcileFunc1, sampleReconcileFunc3}, + expectedExit: false, + expectedRequeue: true, + expectedError: nil, + }, + { + name: "Function returns error", + reconcileFuncs: []ReconcileFunc{sampleReconcileFunc1, sampleReconcileFuncWithError}, + expectedExit: false, + expectedRequeue: false, + expectedError: errors.New("sample error"), + }, + { + name: "Function signals conflicting exit and requeue", + reconcileFuncs: []ReconcileFunc{sampleConflictingReconcileFunc}, + expectedExit: true, + expectedRequeue: false, + expectedError: errors.New("conflicting exit and requeue signals - can not be both true"), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + exit, requeue, err := ReconcileBatch(test.reconcileFuncs...) + if exit != test.expectedExit { + t.Errorf("Expected exit: %v, but got: %v", test.expectedExit, exit) + } + if requeue != test.expectedRequeue { + t.Errorf("Expected requeue: %v, but got: %v", test.expectedRequeue, requeue) + } + if (err == nil && test.expectedError != nil) || (err != nil && test.expectedError == nil) || (err != nil && test.expectedError != nil && err.Error() != test.expectedError.Error()) { + t.Errorf("Expected error: %v, but got: %v", test.expectedError, err) + } + }) + } +} diff --git a/internal/controller/nonadminbackup_controller.go b/internal/controller/nonadminbackup_controller.go index 55141ec..cc40ca1 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -19,23 +19,17 @@ package controller import ( "context" - "errors" "time" - "github.com/go-logr/logr" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" 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" "sigs.k8s.io/controller-runtime/pkg/log" nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" "github.com/migtools/oadp-non-admin/internal/common/constant" - "github.com/migtools/oadp-non-admin/internal/common/function" "github.com/migtools/oadp-non-admin/internal/handler" "github.com/migtools/oadp-non-admin/internal/predicate" ) @@ -43,10 +37,8 @@ import ( // NonAdminBackupReconciler reconciles a NonAdminBackup object type NonAdminBackupReconciler struct { client.Client - Scheme *runtime.Scheme - Log logr.Logger - Context context.Context - NamespacedName types.NamespacedName + Scheme *runtime.Scheme + Context context.Context } const ( @@ -70,23 +62,18 @@ const ( // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.17.0/pkg/reconcile func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - r.Log = log.FromContext(ctx) - logger := r.Log.WithValues("NonAdminBackup", req.NamespacedName) + rLog := log.FromContext(ctx) + logger := rLog.WithValues("NonAdminBackup", req.NamespacedName) logger.V(1).Info(">>> Reconcile NonAdminBackup - loop start") - // Resource version - // Generation version - metadata - only one is updated... - r.Context = ctx - r.NamespacedName = req.NamespacedName - - // Get the Non Admin Backup object + // 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 && apierrors.IsNotFound(err) { - logger.V(1).Info("Deleted NonAdminBackup CR", nameField, req.Name, constant.NameSpaceString, req.Namespace) + logger.V(1).Info("Non existing NonAdminBackup CR", nameField, req.Name, constant.NameSpaceString, req.Namespace) return ctrl.Result{}, nil } @@ -95,142 +82,27 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ctrl.Result{}, err } - if nab.Status.Phase == constant.EmptyString { - // Phase: New - updatedStatus, errUpdate := function.UpdateNonAdminPhase(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminBackupPhaseNew) - if updatedStatus { - logger.V(1).Info("NonAdminBackup CR - Rqueue after Phase Update") - return ctrl.Result{Requeue: true, RequeueAfter: requeueTimeSeconds * time.Second}, nil - } - if errUpdate != nil { - logger.Error(errUpdate, "Unable to set NonAdminBackup Phase: New", nameField, req.Name, constant.NameSpaceString, req.Namespace) - return ctrl.Result{}, errUpdate - } - } - - backupSpec, err := function.GetBackupSpecFromNonAdminBackup(&nab) - - if err != nil { - errMsg := "NonAdminBackup CR does not contain valid BackupSpec" - logger.Error(err, errMsg) - - // Phase: BackingOff - // BackupAccepted: False - // BackupQueued: False # already set - updatedStatus, errUpdate := function.UpdateNonAdminPhase(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminBackupPhaseBackingOff) - if errUpdate != nil { - logger.Error(errUpdate, "Unable to set NonAdminBackup Phase: BackingOff", nameField, req.Name, constant.NameSpaceString, req.Namespace) - return ctrl.Result{}, errUpdate - } else if updatedStatus { - // We do not requeue as the State is BackingOff - return ctrl.Result{}, nil - } - - updatedStatus, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionFalse, "InvalidBackupSpec", errMsg) - if updatedStatus { - return ctrl.Result{}, nil - } - - if errUpdate != nil { - logger.Error(errUpdate, "Unable to set BackupAccepted Condition: False", nameField, req.Name, constant.NameSpaceString, req.Namespace) - return ctrl.Result{}, errUpdate - } - return ctrl.Result{}, err - } - - // Phase: New # already set - // BackupAccepted: True - // BackupQueued: False # already set - updatedStatus, errUpdate := function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionTrue, "BackupAccepted", "backup accepted") - if updatedStatus { - return ctrl.Result{Requeue: true, RequeueAfter: requeueTimeSeconds * time.Second}, nil - } - - if errUpdate != nil { - logger.Error(errUpdate, "Unable to set BackupAccepted Condition: True", nameField, req.Name, constant.NameSpaceString, req.Namespace) - return ctrl.Result{}, errUpdate - } - - veleroBackupName := function.GenerateVeleroBackupName(nab.Namespace, nab.Name) - - if veleroBackupName == constant.EmptyString { - return ctrl.Result{}, errors.New("unable to generate Velero Backup name") - } - - veleroBackup := velerov1api.Backup{} - err = r.Get(ctx, client.ObjectKey{Namespace: constant.OadpNamespace, Name: veleroBackupName}, &veleroBackup) - - if err != nil && apierrors.IsNotFound(err) { - // Create backup - // 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) - veleroBackup = velerov1api.Backup{ - ObjectMeta: metav1.ObjectMeta{ - Name: veleroBackupName, - Namespace: constant.OadpNamespace, - }, - Spec: *backupSpec, - } - } else if err != nil && !apierrors.IsNotFound(err) { - logger.Error(err, "Unable to fetch VeleroBackup") - return ctrl.Result{}, err - } else { - // TODO: Currently when one updates VeleroBackup spec inside the NonAdminBackup - // We do not update already created VeleroBackup object. - // Should we update the previously created VeleroBackup object and reflect what was - // updated? In the current situation the VeleroBackup within NonAdminBackup will - // be reverted back to the previous state - the state which created VeleroBackup - // in a first place. - logger.Info("Backup already exists, updating NonAdminBackup status", nameField, veleroBackupName) - 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 ctrl.Result{}, errBackupUpdate - } else if updatedNab { - logger.V(1).Info("NonAdminBackup CR - Rqueue after Status Update") - return ctrl.Result{Requeue: true, RequeueAfter: requeueTimeSeconds * time.Second}, nil - } - return ctrl.Result{}, nil - } - - // 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 backup", nameField, veleroBackupName) - return ctrl.Result{}, err - } - logger.Info("VeleroBackup successfully created", nameField, veleroBackupName) - - // Phase: Created - // BackupAccepted: True # do not update - // BackupQueued: True - _, errUpdate = function.UpdateNonAdminPhase(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminBackupPhaseCreated) - if errUpdate != nil { - logger.Error(errUpdate, "Unable to set NonAdminBackup Phase: Created", nameField, req.Name, constant.NameSpaceString, req.Namespace) - return ctrl.Result{}, errUpdate - } - _, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionTrue, "Validated", "Valid Backup config") - if errUpdate != nil { - logger.Error(errUpdate, "Unable to set BackupAccepted Condition: True", nameField, req.Name, constant.NameSpaceString, req.Namespace) - return ctrl.Result{}, 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, req.Name, constant.NameSpaceString, req.Namespace) - return ctrl.Result{}, errUpdate - } + // Run Reconcile Batch as we have the NonAdminBackup object + reconcileExit, reconcileRequeue, reconcileErr := ReconcileBatch( + ReconcileFunc(func(...interface{}) (bool, bool, error) { + return r.InitNonAdminBackup(rLog, ctx, req.Name, req.Namespace, &nab) // Phase: New + }), + ReconcileFunc(func(...interface{}) (bool, bool, error) { + return r.ValidateVeleroBackupSpec(rLog, ctx, req.Name, req.Namespace, &nab) // Phase: New + }), + ReconcileFunc(func(...interface{}) (bool, bool, error) { + return r.CreateVeleroBackupSpec(rLog, ctx, req.Name, req.Namespace, &nab) // Phase: New + }), + ) + + // Return vars from the ReconcileBatch + if reconcileExit { + return ctrl.Result{}, reconcileErr + } else if reconcileRequeue { + return ctrl.Result{Requeue: true, RequeueAfter: requeueTimeSeconds * time.Second}, reconcileErr + } else if reconcileErr != nil { + return ctrl.Result{}, reconcileErr + } // No error and both reconcileExit and reconcileRequeue false, continue... logger.V(1).Info(">>> Reconcile NonAdminBackup - loop end")