diff --git a/.golangci.yml b/.golangci.yml index a786073..61a193e 100644 --- a/.golangci.yml +++ b/.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 diff --git a/api/v1alpha1/nonadminbackup_types.go b/api/v1alpha1/nonadminbackup_types.go index 10fb806..34621de 100644 --- a/api/v1alpha1/nonadminbackup_types.go +++ b/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,7 +60,16 @@ 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: . + // +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 @@ -47,11 +77,11 @@ type NonAdminBackupStatus struct { // 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 diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 80b7114..c755b14 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -29,10 +29,10 @@ import ( // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NonAdminBackup) DeepCopyInto(out *NonAdminBackup) { *out = *in - out.TypeMeta = in.TypeMeta - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NonAdminBackup. @@ -93,11 +93,6 @@ func (in *NonAdminBackupSpec) DeepCopyInto(out *NonAdminBackupSpec) { *out = new(v1.BackupSpec) (*in).DeepCopyInto(*out) } - if in.BackupStatus != nil { - in, out := &in.BackupStatus, &out.BackupStatus - *out = new(v1.BackupStatus) - (*in).DeepCopyInto(*out) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NonAdminBackupSpec. @@ -113,6 +108,11 @@ func (in *NonAdminBackupSpec) DeepCopy() *NonAdminBackupSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NonAdminBackupStatus) DeepCopyInto(out *NonAdminBackupStatus) { *out = *in + if in.BackupStatus != nil { + in, out := &in.BackupStatus, &out.BackupStatus + *out = new(v1.BackupStatus) + (*in).DeepCopyInto(*out) + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]metav1.Condition, len(*in)) diff --git a/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml b/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml index 389c25d..80a3e26 100644 --- a/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml +++ b/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml @@ -503,6 +503,22 @@ spec: type: string type: array type: object + logLevel: + description: NonAdminBackup log level (use debug for the most logging, + leave unset for default) + enum: + - trace + - debug + - info + - warning + - error + - fatal + - panic + type: string + type: object + status: + description: NonAdminBackupStatus defines the observed state of NonAdminBackup + properties: backupStatus: description: BackupStatus captures the current status of a Velero backup. @@ -634,22 +650,6 @@ spec: file in object storage. type: integer type: object - logLevel: - description: NonAdminBackup log level (use debug for the most logging, - leave unset for default) - enum: - - trace - - debug - - info - - warning - - error - - fatal - - panic - type: string - type: object - status: - description: NonAdminBackupStatus defines the observed state of NonAdminBackup - properties: conditions: items: description: "Condition contains details for one aspect of the current @@ -719,6 +719,18 @@ spec: - type type: object type: array + oadpVeleroBackup: + description: 'OadpVeleroBackup references the VeleroBackup object. + Format: .' + 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 diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 8ff4f65..071fb6e 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -5,4 +5,4 @@ kind: Kustomization images: - name: controller newName: quay.io/migi/oadp-nac-operator - newTag: v0.0.37 + newTag: v0.0.56 diff --git a/internal/common/constant/constant.go b/internal/common/constant/constant.go index 6d44ee8..bdab198 100644 --- a/internal/common/constant/constant.go +++ b/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" diff --git a/internal/common/function/function.go b/internal/common/function/function.go index f6a9c0e..05d929b 100644 --- a/internal/common/function/function.go +++ b/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 diff --git a/internal/controller/common_nab.go b/internal/controller/common_nab.go new file mode 100644 index 0000000..2110902 --- /dev/null +++ b/internal/controller/common_nab.go @@ -0,0 +1,118 @@ +/* +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" + "crypto/sha256" + "encoding/hex" + "fmt" + "reflect" + + "github.com/go-logr/logr" + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" + "github.com/migtools/oadp-non-admin/internal/common/constant" +) + +// GetVeleroBackupSpecFromNonAdminBackup gets the BackupSpec from the NonAdminBackup object +func GetVeleroBackupSpecFromNonAdminBackup(nonAdminBackup *nacv1alpha1.NonAdminBackup) (*velerov1api.BackupSpec, error) { + if nonAdminBackup == nil { + return nil, fmt.Errorf("nonAdminBackup is nil") + } + + if nonAdminBackup.Spec.BackupSpec == nil { + return nil, fmt.Errorf("BackupSpec is nil") + } + + // TODO: Additional validations, before continuing + veleroBackup := nonAdminBackup.Spec.BackupSpec.DeepCopy() + + return veleroBackup, nil +} + +// GenerateVeleroBackupName generates a Velero backup name based on the provided namespace and NonAdminBackup name. +// It calculates a hash of the NonAdminBackup name and combines it with the namespace and a prefix to create the Velero backup name. +// If the resulting name exceeds the maximum Kubernetes name length, it truncates the namespace to fit within the limit. +func GenerateVeleroBackupName(namespace, nabName string) string { + // Calculate a hash of the name + const hashLength = 14 + prefixLength := len(constant.VeleroBackupNamePrefix) + len("--") // Account for two "-" + + hasher := sha256.New() + _, err := hasher.Write([]byte(nabName)) + if err != nil { + return "" + } + + nameHash := hex.EncodeToString(hasher.Sum(nil))[:hashLength] // Take first 14 chars + + // Generate the Velero backup name created from NAB + veleroBackupName := fmt.Sprintf("%s-%s-%s", constant.VeleroBackupNamePrefix, namespace, nameHash) + + // Ensure the name is within the character limit + if len(veleroBackupName) > constant.MaxKubernetesNameLength { + // Truncate the namespace if necessary + maxNamespaceLength := constant.MaxKubernetesNameLength - len(nameHash) - prefixLength + if len(namespace) > maxNamespaceLength { + namespace = namespace[:maxNamespaceLength] + } + veleroBackupName = fmt.Sprintf("%s-%s-%s", constant.VeleroBackupNamePrefix, namespace, nameHash) + } + + return veleroBackupName +} + +// UpdateNonAdminBackupFromVeleroBackup takes the Velero Backup and Velero Status and updates the corresponding +// NonAdminBackup with exact same copies of that information. +func UpdateNonAdminBackupFromVeleroBackup(ctx context.Context, r client.Client, log logr.Logger, nab *nacv1alpha1.NonAdminBackup, veleroBackup *velerov1api.Backup) error { + // Make a copy of the current status for comparison + oldStatus := nab.Status.BackupStatus.DeepCopy() + oldSpec := nab.Spec.BackupSpec.DeepCopy() + + // Update the status & spec + // Copy the status from veleroBackup.Status to nab.Status + nab.Status.BackupStatus = veleroBackup.Status.DeepCopy() + + nab.Spec.BackupSpec = veleroBackup.Spec.DeepCopy() + + // Check if the spec has been updated + if !reflect.DeepEqual(oldSpec, nab.Spec.BackupSpec) { + if err := r.Update(ctx, nab); err != nil { + log.Error(err, "Failed to update NonAdminBackup Spec") + return err + } + log.V(1).Info("NonAdminBackup spec was updated") + } else { + log.V(1).Info("NonAdminBackup spec is already up to date") + } + + // Check if the status has been updated + if !reflect.DeepEqual(oldStatus, nab.Status.BackupStatus) { + if err := r.Status().Update(ctx, nab); err != nil { + log.Error(err, "Failed to update NonAdminBackup Status") + return err + } + log.V(1).Info("NonAdminBackup status was updated") + } else { + log.V(1).Info("NonAdminBackup status is already up to date") + } + + return nil +} diff --git a/internal/controller/nonadminbackup_controller.go b/internal/controller/nonadminbackup_controller.go index 5f4281d..683394f 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -19,10 +19,12 @@ package controller import ( "context" + "errors" + "time" "github.com/go-logr/logr" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - "k8s.io/apimachinery/pkg/api/errors" + 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" @@ -32,6 +34,7 @@ import ( "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" @@ -47,8 +50,9 @@ type NonAdminBackupReconciler struct { } const ( - nameField = "Name" - oadpNamespace = "openshift-adp" // TODO user input + nameField = "Name" + oadpNamespace = "openshift-adp" // TODO user input + requeueTimeSeconds = 10 ) // +kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups,verbs=get;list;watch;create;update;patch;delete @@ -69,44 +73,98 @@ const ( 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) + 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 nab := nacv1alpha1.NonAdminBackup{} err := r.Get(ctx, req.NamespacedName, &nab) - if err != nil && errors.IsNotFound(err) { - logger.V(1).Info("Deleted NonAdminBackup CR", nameField, req.Name, "Namespace", req.Namespace) + // 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) return ctrl.Result{}, nil } if err != nil { - logger.Error(err, "Unable to fetch NonAdminBackup CR", nameField, req.Name, "Namespace", req.Namespace) + logger.Error(err, "Unable to fetch NonAdminBackup CR", nameField, req.Name, constant.NameSpaceString, req.Namespace) return ctrl.Result{}, err } - backupSpec, err := function.GetBackupSpecFromNonAdminBackup(&nab) - - if backupSpec == nil { - logger.Error(err, "NonAdminBackup CR does not contain valid BackupSpec") - return ctrl.Result{}, nil + 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 { - logger.Error(err, "Error while performing NonAdminBackup reconcile") + 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.NonAdminBackupConditionAccepted, metav1.ConditionFalse, "invalid_backupspec", 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 } - logger.Info("NonAdminBackup Reconcile loop") + // Phase: New # already set + // BackupAccepted: True + // BackupQueued: False # already set + updatedStatus, errUpdate := function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminBackupConditionAccepted, metav1.ConditionTrue, "backup_accepted", "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: oadpNamespace, Name: veleroBackupName}, &veleroBackup) - if err != nil && errors.IsNotFound(err) { + 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{ @@ -115,14 +173,25 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque }, Spec: *backupSpec, } - } else if err != nil && !errors.IsNotFound(err) { + } 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) - err = function.UpdateNonAdminBackupFromVeleroBackup(ctx, r.Client, logger, &nab, &veleroBackup) - if err != nil { - return ctrl.Result{}, err + 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 } @@ -143,10 +212,28 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque 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.NonAdminBackupConditionAccepted, 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.NonAdminBackupConditionQueued, metav1.ConditionTrue, "backup_scheduled", "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 + } - logger.Info("Backup successfully created", nameField, veleroBackupName) - - logger.Info("NonAdminBackup Reconcile loop end") + logger.V(1).Info(">>> Reconcile NonAdminBackup - loop end") return ctrl.Result{}, nil } diff --git a/internal/predicate/composite_predicate.go b/internal/predicate/composite_predicate.go index 751bebc..d6f9aef 100644 --- a/internal/predicate/composite_predicate.go +++ b/internal/predicate/composite_predicate.go @@ -20,7 +20,10 @@ package predicate import ( "context" + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "sigs.k8s.io/controller-runtime/pkg/event" + + nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" ) // CompositePredicate is a combination of all project event filters @@ -32,40 +35,51 @@ type CompositePredicate struct { // Create event filter func (p CompositePredicate) Create(evt event.CreateEvent) bool { - // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate - if p.NonAdminBackupPredicate.Create(p.Context, evt) { - return true + switch evt.Object.(type) { + case *nacv1alpha1.NonAdminBackup: + // Apply NonAdminBackupPredicate + return p.NonAdminBackupPredicate.Create(p.Context, evt) + case *velerov1api.Backup: + // Apply VeleroBackupPredicate + return p.VeleroBackupPredicate.Create(p.Context, evt) + default: + // Unknown object type, return false + return false } - // Otherwise, apply VeleroBackupPredicate - return p.VeleroBackupPredicate.Create(p.Context, evt) } // Update event filter func (p CompositePredicate) Update(evt event.UpdateEvent) bool { - // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate - if p.NonAdminBackupPredicate.Update(p.Context, evt) { - return true + switch evt.ObjectNew.(type) { + case *nacv1alpha1.NonAdminBackup: + return p.NonAdminBackupPredicate.Update(p.Context, evt) + case *velerov1api.Backup: + return p.VeleroBackupPredicate.Update(p.Context, evt) + default: + return false } - // Otherwise, apply VeleroBackupPredicate - return p.VeleroBackupPredicate.Update(p.Context, evt) } // Delete event filter func (p CompositePredicate) Delete(evt event.DeleteEvent) bool { - // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate - if p.NonAdminBackupPredicate.Delete(p.Context, evt) { - return true + switch evt.Object.(type) { + case *nacv1alpha1.NonAdminBackup: + return p.NonAdminBackupPredicate.Delete(p.Context, evt) + case *velerov1api.Backup: + return p.VeleroBackupPredicate.Delete(p.Context, evt) + default: + return false } - // Otherwise, apply VeleroBackupPredicate - return p.VeleroBackupPredicate.Delete(p.Context, evt) } // Generic event filter func (p CompositePredicate) Generic(evt event.GenericEvent) bool { - // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate - if p.NonAdminBackupPredicate.Generic(p.Context, evt) { - return true + switch evt.Object.(type) { + case *nacv1alpha1.NonAdminBackup: + return p.NonAdminBackupPredicate.Generic(p.Context, evt) + case *velerov1api.Backup: + return p.VeleroBackupPredicate.Generic(p.Context, evt) + default: + return false } - // Otherwise, apply VeleroBackupPredicate - return p.VeleroBackupPredicate.Generic(p.Context, evt) } diff --git a/internal/predicate/nonadminbackup_predicate.go b/internal/predicate/nonadminbackup_predicate.go index a117061..d3309a3 100644 --- a/internal/predicate/nonadminbackup_predicate.go +++ b/internal/predicate/nonadminbackup_predicate.go @@ -25,6 +25,7 @@ import ( "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" ) // NonAdminBackupPredicate contains event filters for Non Admin Backup objects @@ -38,49 +39,67 @@ func getNonAdminBackupPredicateLogger(ctx context.Context, name, namespace strin // Create event filter func (NonAdminBackupPredicate) Create(ctx context.Context, evt event.CreateEvent) bool { - if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { - nameSpace := evt.Object.GetNamespace() - name := evt.Object.GetName() - logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) - logger.V(1).Info("Received Create NonAdminBackupPredicate") - return true + nameSpace := evt.Object.GetNamespace() + name := evt.Object.GetName() + logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + logger.V(1).Info("NonAdminBackupPredicate: Received Create event") + if nonAdminBackup, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { + if nonAdminBackup.Status.Phase == constant.EmptyString || nonAdminBackup.Status.Phase == nacv1alpha1.NonAdminBackupPhaseNew { + logger.V(1).Info("NonAdminBackupPredicate: Accepted Create event") + return true + } } - + logger.V(1).Info("NonAdminBackupPredicate: Rejecting Create event") return false } // Update event filter func (NonAdminBackupPredicate) Update(ctx context.Context, evt event.UpdateEvent) bool { - if _, ok := evt.ObjectNew.(*nacv1alpha1.NonAdminBackup); ok { - nameSpace := evt.ObjectNew.GetNamespace() - name := evt.ObjectNew.GetName() - logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) - logger.V(1).Info("Received Update NonAdminBackupPredicate") + // Do not reconcile on Status update + nameSpace := evt.ObjectNew.GetNamespace() + name := evt.ObjectNew.GetName() + logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + logger.V(1).Info("NonAdminBackupPredicate: Received Update event") + + if evt.ObjectNew.GetGeneration() != evt.ObjectOld.GetGeneration() { + logger.V(1).Info("NonAdminBackupPredicate: Accepted Update event - generation change") return true } + + if nonAdminBackupOld, ok := evt.ObjectOld.(*nacv1alpha1.NonAdminBackup); ok { + if nonAdminBackupNew, ok := evt.ObjectNew.(*nacv1alpha1.NonAdminBackup); ok { + oldPhase := nonAdminBackupOld.Status.Phase + newPhase := nonAdminBackupNew.Status.Phase + + // New phase set, reconcile + if oldPhase == constant.EmptyString && newPhase != constant.EmptyString { + logger.V(1).Info("NonAdminBsackupPredicate: Accepted Update event - phase change") + return true + } else if oldPhase == nacv1alpha1.NonAdminBackupPhaseNew && newPhase == nacv1alpha1.NonAdminBackupPhaseCreated { + logger.V(1).Info("NonAdminBackupPredicate: Accepted Update event - phase created") + return true + } + } + } + logger.V(1).Info("NonAdminBackupPredicate: Rejecting Update event") + return false } // Delete event filter func (NonAdminBackupPredicate) Delete(ctx context.Context, evt event.DeleteEvent) bool { - if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { - nameSpace := evt.Object.GetNamespace() - name := evt.Object.GetName() - logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) - logger.V(1).Info("Received Delete NonAdminBackupPredicate") - return true - } - return false + nameSpace := evt.Object.GetNamespace() + name := evt.Object.GetName() + logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + logger.V(1).Info("NonAdminBackupPredicate: Accepted Delete event") + return true } // Generic event filter func (NonAdminBackupPredicate) Generic(ctx context.Context, evt event.GenericEvent) bool { - if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { - nameSpace := evt.Object.GetNamespace() - name := evt.Object.GetName() - logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) - logger.V(1).Info("Received Generic NonAdminBackupPredicate") - return true - } - return false + nameSpace := evt.Object.GetNamespace() + name := evt.Object.GetName() + logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + logger.V(1).Info("NonAdminBackupPredicate: Accepted Generic event") + return true } diff --git a/internal/predicate/velerobackup_predicate.go b/internal/predicate/velerobackup_predicate.go index 08c9fe5..e2e78f1 100644 --- a/internal/predicate/velerobackup_predicate.go +++ b/internal/predicate/velerobackup_predicate.go @@ -36,30 +36,23 @@ type VeleroBackupPredicate struct { Logger logr.Logger } -// TODO try to remove calls to get logger functions, try to initialize it - func getBackupPredicateLogger(ctx context.Context, name, namespace string) logr.Logger { return log.FromContext(ctx).WithValues("VeleroBackupPredicate", types.NamespacedName{Name: name, Namespace: namespace}) } // Create event filter func (veleroBackupPredicate VeleroBackupPredicate) Create(ctx context.Context, evt event.CreateEvent) bool { - nameSpace := evt.Object.GetNamespace() - if nameSpace != veleroBackupPredicate.OadpVeleroNamespace { - return false - } + if backup, ok := evt.Object.(*velerov1api.Backup); ok { + nameSpace := evt.Object.GetNamespace() + if nameSpace != veleroBackupPredicate.OadpVeleroNamespace { + return false + } - name := evt.Object.GetName() - logger := getBackupPredicateLogger(ctx, name, nameSpace) - logger.V(1).Info("Received Create VeleroBackupPredicate") + name := evt.Object.GetName() + logger := getBackupPredicateLogger(ctx, name, nameSpace) + logger.V(1).Info("VeleroBackupPredicate: Received Create event") - backup, ok := evt.Object.(*velerov1api.Backup) - if !ok { - // The event object is not a Backup, ignore it - return false - } - if function.CheckVeleroBackupLabels(backup) { - return true + return function.CheckVeleroBackupLabels(backup) } return false } @@ -69,8 +62,7 @@ func (veleroBackupPredicate VeleroBackupPredicate) Update(ctx context.Context, e nameSpace := evt.ObjectNew.GetNamespace() name := evt.ObjectNew.GetName() logger := getBackupPredicateLogger(ctx, name, nameSpace) - logger.V(1).Info("Received Update VeleroBackupPredicate") - + logger.V(1).Info("VeleroBackupPredicate: Received Update event") return nameSpace == veleroBackupPredicate.OadpVeleroNamespace }