From 70434b72322593be800023c9bab3e7a5701be547 Mon Sep 17 00:00:00 2001 From: Michal Pryc Date: Wed, 6 Mar 2024 13:50:20 +0100 Subject: [PATCH] Adjust the Status of NonAdminBackup Moves Status outside of Spec and adjusts this to reflect Velero Backup status as well additional Status when the Spec within NonAdminBackup is not defined. Signed-off-by: Michal Pryc --- api/v1alpha1/nonadminbackup_types.go | 6 +- api/v1alpha1/zz_generated.deepcopy.go | 14 +- ...nac.oadp.openshift.io_nonadminbackups.yaml | 322 +++++++----------- config/manager/kustomization.yaml | 2 +- internal/controller/common_nab.go | 35 +- .../controller/nonadminbackup_controller.go | 6 + internal/controller/velerobackup_predicate.go | 8 +- 7 files changed, 163 insertions(+), 230 deletions(-) diff --git a/api/v1alpha1/nonadminbackup_types.go b/api/v1alpha1/nonadminbackup_types.go index 56bb58e..dcfd26e 100644 --- a/api/v1alpha1/nonadminbackup_types.go +++ b/api/v1alpha1/nonadminbackup_types.go @@ -32,14 +32,12 @@ type NonAdminBackupSpec struct { // 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"` } // NonAdminBackupStatus defines the observed state of NonAdminBackup type NonAdminBackupStatus struct { - Conditions []metav1.Condition `json:"conditions,omitempty"` + // BackupStatus captures the current status of a Velero backup. + velerov1api.BackupStatus `json:",inline"` } //+kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 80b7114..3b264d3 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -22,7 +22,6 @@ package v1alpha1 import ( "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -93,11 +92,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,13 +107,7 @@ 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.Conditions != nil { - in, out := &in.Conditions, &out.Conditions - *out = make([]metav1.Condition, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } + in.BackupStatus.DeepCopyInto(&out.BackupStatus) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NonAdminBackupStatus. diff --git a/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml b/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml index 389c25d..24127b4 100644 --- a/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml +++ b/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml @@ -503,137 +503,6 @@ spec: type: string type: array type: object - backupStatus: - description: BackupStatus captures the current status of a Velero - backup. - properties: - backupItemOperationsAttempted: - description: |- - BackupItemOperationsAttempted is the total number of attempted - async BackupItemAction operations for this backup. - type: integer - backupItemOperationsCompleted: - description: |- - BackupItemOperationsCompleted is the total number of successfully completed - async BackupItemAction operations for this backup. - type: integer - backupItemOperationsFailed: - description: |- - BackupItemOperationsFailed is the total number of async - BackupItemAction operations for this backup which ended with an error. - type: integer - completionTimestamp: - description: |- - CompletionTimestamp records the time a backup was completed. - Completion time is recorded even on failed backups. - Completion time is recorded before uploading the backup object. - The server's time is used for CompletionTimestamps - format: date-time - nullable: true - type: string - csiVolumeSnapshotsAttempted: - description: |- - CSIVolumeSnapshotsAttempted is the total number of attempted - CSI VolumeSnapshots for this backup. - type: integer - csiVolumeSnapshotsCompleted: - description: |- - CSIVolumeSnapshotsCompleted is the total number of successfully - completed CSI VolumeSnapshots for this backup. - type: integer - errors: - description: |- - Errors is a count of all error messages that were generated during - execution of the backup. The actual errors are in the backup's log - file in object storage. - type: integer - expiration: - description: Expiration is when this Backup is eligible for garbage-collection. - format: date-time - nullable: true - type: string - failureReason: - description: FailureReason is an error that caused the entire - backup to fail. - type: string - formatVersion: - description: FormatVersion is the backup format version, including - major, minor, and patch version. - type: string - phase: - description: Phase is the current state of the Backup. - enum: - - New - - FailedValidation - - InProgress - - WaitingForPluginOperations - - WaitingForPluginOperationsPartiallyFailed - - Finalizing - - FinalizingPartiallyFailed - - Completed - - PartiallyFailed - - Failed - - Deleting - type: string - progress: - description: |- - Progress contains information about the backup's execution progress. Note - that this information is best-effort only -- if Velero fails to update it - during a backup for any reason, it may be inaccurate/stale. - nullable: true - properties: - itemsBackedUp: - description: |- - ItemsBackedUp is the number of items that have actually been written to the - backup tarball so far. - type: integer - totalItems: - description: |- - TotalItems is the total number of items to be backed up. This number may change - throughout the execution of the backup due to plugins that return additional related - items to back up, the velero.io/exclude-from-backup label, and various other - filters that happen as items are processed. - type: integer - type: object - startTimestamp: - description: |- - StartTimestamp records the time a backup was started. - Separate from CreationTimestamp, since that value changes - on restores. - 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 - version: - description: |- - Version is the backup format major version. - Deprecated: Please see FormatVersion - type: integer - volumeSnapshotsAttempted: - description: |- - VolumeSnapshotsAttempted is the total number of attempted - volume snapshots for this backup. - type: integer - volumeSnapshotsCompleted: - description: |- - VolumeSnapshotsCompleted is the total number of successfully - completed volume snapshots for this backup. - type: integer - warnings: - description: |- - Warnings is a count of all warning messages that were generated during - execution of the backup. The actual warnings are in the backup's log - file in object storage. - type: integer - type: object logLevel: description: NonAdminBackup log level (use debug for the most logging, leave unset for default) @@ -650,75 +519,132 @@ spec: status: description: NonAdminBackupStatus defines the observed state of NonAdminBackup properties: - conditions: + backupItemOperationsAttempted: + description: |- + BackupItemOperationsAttempted is the total number of attempted + async BackupItemAction operations for this backup. + type: integer + backupItemOperationsCompleted: + description: |- + BackupItemOperationsCompleted is the total number of successfully completed + async BackupItemAction operations for this backup. + type: integer + backupItemOperationsFailed: + description: |- + BackupItemOperationsFailed is the total number of async + BackupItemAction operations for this backup which ended with an error. + type: integer + completionTimestamp: + description: |- + CompletionTimestamp records the time a backup was completed. + Completion time is recorded even on failed backups. + Completion time is recorded before uploading the backup object. + The server's time is used for CompletionTimestamps + format: date-time + nullable: true + type: string + csiVolumeSnapshotsAttempted: + description: |- + CSIVolumeSnapshotsAttempted is the total number of attempted + CSI VolumeSnapshots for this backup. + type: integer + csiVolumeSnapshotsCompleted: + description: |- + CSIVolumeSnapshotsCompleted is the total number of successfully + completed CSI VolumeSnapshots for this backup. + type: integer + errors: + description: |- + Errors is a count of all error messages that were generated during + execution of the backup. The actual errors are in the backup's log + file in object storage. + type: integer + expiration: + description: Expiration is when this Backup is eligible for garbage-collection. + format: date-time + nullable: true + type: string + failureReason: + description: FailureReason is an error that caused the entire backup + to fail. + type: string + formatVersion: + description: FormatVersion is the backup format version, including + major, minor, and patch version. + type: string + phase: + description: Phase is the current state of the Backup. + enum: + - New + - FailedValidation + - InProgress + - WaitingForPluginOperations + - WaitingForPluginOperationsPartiallyFailed + - Finalizing + - FinalizingPartiallyFailed + - Completed + - PartiallyFailed + - Failed + - Deleting + type: string + progress: + description: |- + Progress contains information about the backup's execution progress. Note + that this information is best-effort only -- if Velero fails to update it + during a backup for any reason, it may be inaccurate/stale. + nullable: true + properties: + itemsBackedUp: + description: |- + ItemsBackedUp is the number of items that have actually been written to the + backup tarball so far. + type: integer + totalItems: + description: |- + TotalItems is the total number of items to be backed up. This number may change + throughout the execution of the backup due to plugins that return additional related + items to back up, the velero.io/exclude-from-backup label, and various other + filters that happen as items are processed. + type: integer + type: object + startTimestamp: + description: |- + StartTimestamp records the time a backup was started. + Separate from CreationTimestamp, since that value changes + on restores. + 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: - description: "Condition contains details for one aspect of the current - state of this API Resource.\n---\nThis struct is intended for - direct use as an array at the field path .status.conditions. For - example,\n\n\n\ttype FooStatus struct{\n\t // Represents the - observations of a foo's current state.\n\t // Known .status.conditions.type - are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // - +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t - \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" - patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t - \ // other fields\n\t}" - properties: - lastTransitionTime: - description: |- - lastTransitionTime is the last time the condition transitioned from one status to another. - This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. - format: date-time - type: string - message: - description: |- - message is a human readable message indicating details about the transition. - This may be an empty string. - maxLength: 32768 - type: string - observedGeneration: - description: |- - observedGeneration represents the .metadata.generation that the condition was set based upon. - For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date - with respect to the current state of the instance. - format: int64 - minimum: 0 - type: integer - reason: - description: |- - reason contains a programmatic identifier indicating the reason for the condition's last transition. - Producers of specific condition types may define expected values and meanings for this field, - and whether the values are considered a guaranteed API. - The value should be a CamelCase string. - This field may not be empty. - maxLength: 1024 - minLength: 1 - pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ - type: string - status: - description: status of the condition, one of True, False, Unknown. - enum: - - "True" - - "False" - - Unknown - type: string - type: - description: |- - type of condition in CamelCase or in foo.example.com/CamelCase. - --- - Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be - useful (see .node.status.conditions), the ability to deconflict is important. - The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) - maxLength: 316 - pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ - type: string - required: - - lastTransitionTime - - message - - reason - - status - - type - type: object + type: string + nullable: true type: array + version: + description: |- + Version is the backup format major version. + Deprecated: Please see FormatVersion + type: integer + volumeSnapshotsAttempted: + description: |- + VolumeSnapshotsAttempted is the total number of attempted + volume snapshots for this backup. + type: integer + volumeSnapshotsCompleted: + description: |- + VolumeSnapshotsCompleted is the total number of successfully + completed volume snapshots for this backup. + type: integer + warnings: + description: |- + Warnings is a count of all warning messages that were generated during + execution of the backup. The actual warnings are in the backup's log + file in object storage. + type: integer type: object type: object served: true diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 8ff4f65..e65aec4 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.39 diff --git a/internal/controller/common_nab.go b/internal/controller/common_nab.go index d28b23c..36e61c1 100644 --- a/internal/controller/common_nab.go +++ b/internal/controller/common_nab.go @@ -55,22 +55,37 @@ func GenerateVeleroBackupName(namespace, nabName string) string { 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.Spec.BackupStatus.DeepCopy() + oldStatus := nab.Status.BackupStatus.DeepCopy() oldSpec := nab.Spec.BackupSpec.DeepCopy() // Update the status & spec - nab.Spec.BackupStatus = &veleroBackup.Status - nab.Spec.BackupSpec = &veleroBackup.Spec + // Copy the status from veleroBackup.Status to nab.Status + nab.Status = nacv1alpha1.NonAdminBackupStatus{ + BackupStatus: veleroBackup.Status, + } + + nab.Spec.BackupSpec = veleroBackup.Spec.DeepCopy() - if reflect.DeepEqual(oldStatus, nab.Spec.BackupStatus) && reflect.DeepEqual(oldSpec, nab.Spec.BackupSpec) { - // No change, no need to update - log.V(1).Info("NonAdminBackup status and spec is already up to date") - return nil + // 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") } - if err := r.Update(ctx, nab); err != nil { - log.Error(err, "Failed to update NonAdminBackup") - return err + // 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 ca159b7..1ff8a6c 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -82,6 +82,12 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque if veleroBackupSpec == nil { log.Error(err, "NonAdminBackup CR does not contain valid VeleroBackupSpec") + nab.Status.Phase = velerov1api.BackupPhaseFailedValidation + nab.Status.FailureReason = "NonAdminBackup CR does not contain valid VeleroBackupSpec" + if err := r.Status().Update(ctx, &nab); err != nil { + log.Error(err, "Failed to update NonAdminBackup Status") + return ctrl.Result{}, err + } return ctrl.Result{}, nil } diff --git a/internal/controller/velerobackup_predicate.go b/internal/controller/velerobackup_predicate.go index c24f135..b24477c 100644 --- a/internal/controller/velerobackup_predicate.go +++ b/internal/controller/velerobackup_predicate.go @@ -31,14 +31,14 @@ func getBackupPredicateLogger(ctx context.Context, name, namespace string) logr. func (veleroBackupPredicate VeleroBackupPredicate) Create(ctx context.Context, evt event.CreateEvent) bool { nameSpace := evt.Object.GetNamespace() - if nameSpace != veleroBackupPredicate.OadpVeleroNamespace { - return false - } - name := evt.Object.GetName() log := getBackupPredicateLogger(ctx, name, nameSpace) log.V(1).Info("Received Create VeleroBackupPredicate") + if nameSpace != veleroBackupPredicate.OadpVeleroNamespace { + return false + } + backup, ok := evt.Object.(*velerov1api.Backup) if !ok { // The event object is not a Backup, ignore it