diff --git a/api/v1alpha1/nonadminbackup_types.go b/api/v1alpha1/nonadminbackup_types.go index 8bd9193..a7772f6 100644 --- a/api/v1alpha1/nonadminbackup_types.go +++ b/api/v1alpha1/nonadminbackup_types.go @@ -21,17 +21,17 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// NonAdminBackupPhase is a simple one high-level summary of the lifecycle of an NonAdminBackup. +// NonAdminPhase is a simple one high-level summary of the lifecycle of a non admin object. // +kubebuilder:validation:Enum=New;BackingOff;Created -type NonAdminBackupPhase string +type NonAdminPhase 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" + // NonAdminPhaseNew - non admin resource was accepted by the OpenShift cluster, but it has not yet been processed by the NonAdminController + NonAdminPhaseNew NonAdminPhase = "New" + // NonAdminPhaseBackingOff - Velero object was not created due to error in non admin object (configuration or similar) + NonAdminPhaseBackingOff NonAdminPhase = "BackingOff" + // NonAdminPhaseCreated - Velero object was created. The Phase will not have additional information about the Velero object. + NonAdminPhaseCreated NonAdminPhase = "Created" ) // NonAdminBackupSpec defines the desired state of NonAdminBackup @@ -60,8 +60,8 @@ type NonAdminBackupStatus struct { // +optional VeleroBackupStatus *velerov1api.BackupStatus `json:"veleroBackupStatus,omitempty"` - Phase NonAdminBackupPhase `json:"phase,omitempty"` - Conditions []metav1.Condition `json:"conditions,omitempty"` + Phase NonAdminPhase `json:"phase,omitempty"` + Conditions []metav1.Condition `json:"conditions,omitempty"` } // +kubebuilder:object:root=true diff --git a/api/v1alpha1/nonadminrestore_types.go b/api/v1alpha1/nonadminrestore_types.go index 73662b0..512ad2a 100644 --- a/api/v1alpha1/nonadminrestore_types.go +++ b/api/v1alpha1/nonadminrestore_types.go @@ -38,9 +38,15 @@ type NonAdminRestoreSpec struct { // NonAdminRestoreStatus defines the observed state of NonAdminRestore type NonAdminRestoreStatus struct { - // TODO https://github.com/migtools/oadp-non-admin/pull/23 - // TODO https://github.com/migtools/oadp-non-admin/pull/13 + // Related Velero Restore name. + // +optional + VeleroRestoreName string `json:"veleroRestoreName,omitempty"` + + // Related Velero Restore status. + // +optional + VeleroRestoreStatus *velerov1api.RestoreStatus `json:"veleroRestoreStatus,omitempty"` + Phase NonAdminPhase `json:"phase,omitempty"` Conditions []metav1.Condition `json:"conditions,omitempty"` } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index edaec21..0d2f898 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -214,6 +214,11 @@ func (in *NonAdminRestoreSpec) DeepCopy() *NonAdminRestoreSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NonAdminRestoreStatus) DeepCopyInto(out *NonAdminRestoreStatus) { *out = *in + if in.VeleroRestoreStatus != nil { + in, out := &in.VeleroRestoreStatus, &out.VeleroRestoreStatus + *out = new(v1.RestoreStatus) + (*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 ad4980d..6437968 100644 --- a/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml +++ b/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml @@ -589,8 +589,8 @@ spec: type: object type: array phase: - description: NonAdminBackupPhase is a simple one high-level summary - of the lifecycle of an NonAdminBackup. + description: NonAdminPhase is a simple one high-level summary of the + lifecycle of a non admin object. enum: - New - BackingOff diff --git a/config/crd/bases/nac.oadp.openshift.io_nonadminrestores.yaml b/config/crd/bases/nac.oadp.openshift.io_nonadminrestores.yaml index 434252b..0f6ae9e 100644 --- a/config/crd/bases/nac.oadp.openshift.io_nonadminrestores.yaml +++ b/config/crd/bases/nac.oadp.openshift.io_nonadminrestores.yaml @@ -516,6 +516,103 @@ spec: - type type: object type: array + phase: + description: NonAdminPhase is a simple one high-level summary of the + lifecycle of a non admin object. + enum: + - New + - BackingOff + - Created + type: string + veleroRestoreName: + description: Related Velero Restore name. + type: string + veleroRestoreStatus: + description: Related Velero Restore status. + properties: + completionTimestamp: + description: |- + CompletionTimestamp records the time the restore operation was completed. + Completion time is recorded even on failed restore. + The server's time is used for StartTimestamps + format: date-time + nullable: true + type: string + errors: + description: |- + Errors is a count of all error messages that were generated during + execution of the restore. The actual errors are stored in object storage. + type: integer + failureReason: + description: FailureReason is an error that caused the entire + restore to fail. + type: string + phase: + description: Phase is the current state of the Restore + enum: + - New + - FailedValidation + - InProgress + - WaitingForPluginOperations + - WaitingForPluginOperationsPartiallyFailed + - Completed + - PartiallyFailed + - Failed + type: string + progress: + description: |- + Progress contains information about the restore's execution progress. Note + that this information is best-effort only -- if Velero fails to update it + during a restore for any reason, it may be inaccurate/stale. + nullable: true + properties: + itemsRestored: + description: ItemsRestored is the number of items that have + actually been restored so far + type: integer + totalItems: + description: |- + TotalItems is the total number of items to be restored. This number may change + throughout the execution of the restore due to plugins that return additional related + items to restore + type: integer + type: object + restoreItemOperationsAttempted: + description: |- + RestoreItemOperationsAttempted is the total number of attempted + async RestoreItemAction operations for this restore. + type: integer + restoreItemOperationsCompleted: + description: |- + RestoreItemOperationsCompleted is the total number of successfully completed + async RestoreItemAction operations for this restore. + type: integer + restoreItemOperationsFailed: + description: |- + RestoreItemOperationsFailed is the total number of async + RestoreItemAction operations for this restore which ended with an error. + type: integer + startTimestamp: + description: |- + StartTimestamp records the time the restore operation was started. + The server's time is used for StartTimestamps + format: date-time + nullable: true + type: string + validationErrors: + description: |- + ValidationErrors is a slice of all validation errors (if + applicable) + items: + type: string + nullable: true + type: array + warnings: + description: |- + Warnings is a count of all warning messages that were generated during + execution of the restore. The actual warnings are stored in object storage. + type: integer + type: object type: object type: object served: true diff --git a/internal/common/constant/constant.go b/internal/common/constant/constant.go index c3748d7..72351f8 100644 --- a/internal/common/constant/constant.go +++ b/internal/common/constant/constant.go @@ -17,7 +17,11 @@ limitations under the License. // Package constant contains all common constants used in the project package constant -import "os" +import ( + "os" + + "github.com/migtools/oadp-non-admin/internal/common/types" +) // Common labels for objects manipulated by the Non Admin Controller // Labels should be used to identify the NAC object @@ -37,6 +41,15 @@ const ( NamespaceEnvVar = "WATCH_NAMESPACE" ) +// 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 ( + NonAdminConditionAccepted types.NonAdminCondition = "Accepted" + NonAdminConditionQueued types.NonAdminCondition = "Queued" +) + // OadpNamespace is the namespace OADP operator is installed var OadpNamespace = os.Getenv(NamespaceEnvVar) diff --git a/internal/common/function/function.go b/internal/common/function/function.go index a8bc4f9..b63df63 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -29,12 +29,13 @@ import ( 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" + apitypes "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "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/types" ) const requiredAnnotationError = "backup does not have the required annotation '%s'" @@ -127,7 +128,7 @@ func GenerateVeleroBackupName(namespace, nabName string) string { } // 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) { +func UpdateNonAdminPhase(ctx context.Context, r client.Client, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup, phase nacv1alpha1.NonAdminPhase) (bool, error) { if nab == nil { return false, errors.New("NonAdminBackup object is nil") } @@ -159,7 +160,7 @@ func UpdateNonAdminPhase(ctx context.Context, r client.Client, logger logr.Logge // 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) { +func UpdateNonAdminBackupCondition(ctx context.Context, r client.Client, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup, condition types.NonAdminCondition, conditionStatus metav1.ConditionStatus, reason string, message string) (bool, error) { if nab == nil { return false, errors.New("NonAdminBackup object is nil") } @@ -288,7 +289,7 @@ func GetNonAdminBackupFromVeleroBackup(ctx context.Context, clientInstance clien return nil, fmt.Errorf(requiredAnnotationError, constant.NabOriginNameAnnotation) } - nonAdminBackupKey := types.NamespacedName{ + nonAdminBackupKey := apitypes.NamespacedName{ Namespace: nabOriginNamespace, Name: nabOriginName, } @@ -304,7 +305,7 @@ func GetNonAdminBackupFromVeleroBackup(ctx context.Context, clientInstance clien return nil, fmt.Errorf(requiredAnnotationError, constant.NabOriginUUIDAnnotation) } // Ensure UID matches - if nonAdminBackup.ObjectMeta.UID != types.UID(nabOriginUUID) { + if nonAdminBackup.ObjectMeta.UID != apitypes.UID(nabOriginUUID) { return nil, fmt.Errorf("UID from annotation does not match UID of fetched NonAdminBackup object") } diff --git a/api/v1alpha1/nonadmincontroller_types.go b/internal/common/types/types.go similarity index 56% rename from api/v1alpha1/nonadmincontroller_types.go rename to internal/common/types/types.go index 61c5608..bfdbd43 100644 --- a/api/v1alpha1/nonadmincontroller_types.go +++ b/internal/common/types/types.go @@ -14,17 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1alpha1 +// Package types contains all common types used in the project +package types -// NonAdminCondition are used for more detailed information supporing NonAdminBackupPhase state. -// +kubebuilder:validation:Enum=Accepted;Queued +// NonAdminCondition are used for more detailed information supporting NonAdminBackupPhase state. 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 ( - NonAdminConditionAccepted NonAdminCondition = "Accepted" - NonAdminConditionQueued NonAdminCondition = "Queued" -) diff --git a/internal/controller/nonadminbackup_controller.go b/internal/controller/nonadminbackup_controller.go index 55141ec..6f053ad 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -97,7 +97,7 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque if nab.Status.Phase == constant.EmptyString { // Phase: New - updatedStatus, errUpdate := function.UpdateNonAdminPhase(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminBackupPhaseNew) + updatedStatus, errUpdate := function.UpdateNonAdminPhase(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminPhaseNew) if updatedStatus { logger.V(1).Info("NonAdminBackup CR - Rqueue after Phase Update") return ctrl.Result{Requeue: true, RequeueAfter: requeueTimeSeconds * time.Second}, nil @@ -117,7 +117,7 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque // Phase: BackingOff // BackupAccepted: False // BackupQueued: False # already set - updatedStatus, errUpdate := function.UpdateNonAdminPhase(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminBackupPhaseBackingOff) + updatedStatus, errUpdate := function.UpdateNonAdminPhase(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminPhaseBackingOff) if errUpdate != nil { logger.Error(errUpdate, "Unable to set NonAdminBackup Phase: BackingOff", nameField, req.Name, constant.NameSpaceString, req.Namespace) return ctrl.Result{}, errUpdate @@ -126,7 +126,7 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ctrl.Result{}, nil } - updatedStatus, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionFalse, "InvalidBackupSpec", errMsg) + updatedStatus, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, constant.NonAdminConditionAccepted, metav1.ConditionFalse, "InvalidBackupSpec", errMsg) if updatedStatus { return ctrl.Result{}, nil } @@ -141,7 +141,7 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque // 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") + updatedStatus, errUpdate := function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, constant.NonAdminConditionAccepted, metav1.ConditionTrue, "BackupAccepted", "backup accepted") if updatedStatus { return ctrl.Result{Requeue: true, RequeueAfter: requeueTimeSeconds * time.Second}, nil } @@ -216,17 +216,17 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque // Phase: Created // BackupAccepted: True # do not update // BackupQueued: True - _, errUpdate = function.UpdateNonAdminPhase(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminBackupPhaseCreated) + _, errUpdate = function.UpdateNonAdminPhase(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminPhaseCreated) 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") + _, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, constant.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") + _, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, constant.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 diff --git a/internal/controller/nonadminrestore_controller.go b/internal/controller/nonadminrestore_controller.go index cd21d52..b254c5b 100644 --- a/internal/controller/nonadminrestore_controller.go +++ b/internal/controller/nonadminrestore_controller.go @@ -68,6 +68,8 @@ func (r *NonAdminRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Requ // TODO try to create Velero Restore + // TODO update status of NonAdminRestore as Velero Restore progresses + return ctrl.Result{}, nil } @@ -77,7 +79,7 @@ func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, req ctrl.R return fmt.Errorf("spec.restoreSpec.scheduleName field is not allowed in NonAdminRestore") } - // TODO nonAdminBackup respect restricted fields + // TODO nonAdminRestore respect restricted fields nonAdminBackupName := objectSpec.RestoreSpec.BackupName nonAdminBackup := &nacv1alpha1.NonAdminBackup{} @@ -89,18 +91,22 @@ func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, req ctrl.R } return err } - // TODO nonAdminBackup has necessary labels (NAB controller job :question:) - // TODO nonAdminBackup is in complete state :question:!!!! + // TODO move this following to another function, it does not check spec // TODO create get function in common :question: oadpNamespace := os.Getenv(constant.NamespaceEnvVar) - veleroBackupName := nonAdminBackup.Labels["naoSei"] + veleroBackupName := nonAdminBackup.Status.VeleroBackupName + if len(veleroBackupName) == 0 { + return fmt.Errorf("NonAdminBackup '%s' does not reference Velero Backup name", nonAdminBackupName) + } veleroBackup := &velerov1api.Backup{} err = r.Get(ctx, types.NamespacedName{Namespace: oadpNamespace, Name: veleroBackupName}, veleroBackup) if err != nil { - // TODO test error messages, THEY MUST BE INFORMATIVE - return err + if errors.IsNotFound(err) { + // TODO add this error message to NonAdminRestore status + return fmt.Errorf("related Velero backup '%s' for NonAdminBackup '%s' does not exist in OADP namespace %s", veleroBackupName, nonAdminBackupName, oadpNamespace) + } } return nil diff --git a/internal/controller/nonadminrestore_controller_test.go b/internal/controller/nonadminrestore_controller_test.go index fb0b087..8d0e4ac 100644 --- a/internal/controller/nonadminrestore_controller_test.go +++ b/internal/controller/nonadminrestore_controller_test.go @@ -199,7 +199,6 @@ var _ = ginkgo.Describe("Test NonAdminRestore Reconcile function", func() { BackupName: "do-not-exist", }, }), - // TODO Should NOT accept NonAdminBackup that is not in complete state :question: // TODO Should NOT accept non existing related Velero Backup ) }) diff --git a/internal/predicate/nonadminbackup_predicate.go b/internal/predicate/nonadminbackup_predicate.go index d3309a3..0c5fd57 100644 --- a/internal/predicate/nonadminbackup_predicate.go +++ b/internal/predicate/nonadminbackup_predicate.go @@ -44,7 +44,7 @@ func (NonAdminBackupPredicate) Create(ctx context.Context, evt event.CreateEvent 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 { + if nonAdminBackup.Status.Phase == constant.EmptyString || nonAdminBackup.Status.Phase == nacv1alpha1.NonAdminPhaseNew { logger.V(1).Info("NonAdminBackupPredicate: Accepted Create event") return true } @@ -75,7 +75,7 @@ func (NonAdminBackupPredicate) Update(ctx context.Context, evt event.UpdateEvent 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 { + } else if oldPhase == nacv1alpha1.NonAdminPhaseNew && newPhase == nacv1alpha1.NonAdminPhaseCreated { logger.V(1).Info("NonAdminBackupPredicate: Accepted Update event - phase created") return true }