diff --git a/api/v1alpha1/nonadminbackup_types.go b/api/v1alpha1/nonadminbackup_types.go index 4fd14d6..4e29310 100644 --- a/api/v1alpha1/nonadminbackup_types.go +++ b/api/v1alpha1/nonadminbackup_types.go @@ -45,20 +45,25 @@ type NonAdminBackupSpec struct { LogLevel string `json:"logLevel,omitempty"` } -// NonAdminBackupStatus defines the observed state of NonAdminBackup -type NonAdminBackupStatus struct { - // VeleroBackupName references the VeleroBackup object by it's name. +// VeleroBackup contains information of the related Velero backup object. +type VeleroBackup struct { + // status captures the current status of the Velero backup. // +optional - VeleroBackupName string `json:"veleroBackupName,omitempty"` + Status *velerov1.BackupStatus `json:"status,omitempty"` - // VeleroBackupNamespace references the Namespace - // in which VeleroBackupName object exists. + // name references the Velero backup by it's name. // +optional - VeleroBackupNamespace string `json:"veleroBackupNamespace,omitempty"` + Name string `json:"name,omitempty"` - // VeleroBackupStatus captures the current status of a Velero backup. + // namespace references the Namespace in which Velero backup exists. + // +optional + Namespace string `json:"namespace,omitempty"` +} + +// NonAdminBackupStatus defines the observed state of NonAdminBackup +type NonAdminBackupStatus struct { // +optional - VeleroBackupStatus *velerov1.BackupStatus `json:"veleroBackupStatus,omitempty"` + VeleroBackup *VeleroBackup `json:"veleroBackup,omitempty"` Phase NonAdminBackupPhase `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 9ac916b..ed6b68d 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -108,9 +108,9 @@ 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.VeleroBackupStatus != nil { - in, out := &in.VeleroBackupStatus, &out.VeleroBackupStatus - *out = new(v1.BackupStatus) + if in.VeleroBackup != nil { + in, out := &in.VeleroBackup, &out.VeleroBackup + *out = new(VeleroBackup) (*in).DeepCopyInto(*out) } if in.Conditions != nil { @@ -131,3 +131,23 @@ func (in *NonAdminBackupStatus) DeepCopy() *NonAdminBackupStatus { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VeleroBackup) DeepCopyInto(out *VeleroBackup) { + *out = *in + if in.Status != nil { + in, out := &in.Status, &out.Status + *out = new(v1.BackupStatus) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VeleroBackup. +func (in *VeleroBackup) DeepCopy() *VeleroBackup { + if in == nil { + return nil + } + out := new(VeleroBackup) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml b/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml index f6ba069..495d8a3 100644 --- a/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml +++ b/config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml @@ -608,161 +608,165 @@ spec: - BackingOff - Created type: string - veleroBackupName: - description: VeleroBackupName references the VeleroBackup object by - it's name. - type: string - veleroBackupNamespace: - description: |- - VeleroBackupNamespace references the Namespace - in which VeleroBackupName object exists. - type: string - veleroBackupStatus: - description: VeleroBackupStatus captures the current status of a Velero - backup. + veleroBackup: + description: VeleroBackup contains information of the related Velero + backup object. 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 + name: + description: name references the Velero backup by it's name. 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. + namespace: + description: namespace references the Namespace in which Velero + backup exists. type: string - hookStatus: - description: HookStatus contains information about the status - of the hooks. - nullable: true + status: + description: status captures the current status of the Velero + backup. properties: - hooksAttempted: + backupItemOperationsAttempted: description: |- - HooksAttempted is the total number of attempted hooks - Specifically, HooksAttempted represents the number of hooks that failed to execute - and the number of hooks that executed successfully. + BackupItemOperationsAttempted is the total number of attempted + async BackupItemAction operations for this backup. type: integer - hooksFailed: - description: HooksFailed is the total number of hooks which - ended with an error + backupItemOperationsCompleted: + description: |- + BackupItemOperationsCompleted is the total number of successfully completed + async BackupItemAction operations for this backup. type: integer - type: object - 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: + 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 + hookStatus: + description: HookStatus contains information about the status + of the hooks. + nullable: true + properties: + hooksAttempted: + description: |- + HooksAttempted is the total number of attempted hooks + Specifically, HooksAttempted represents the number of hooks that failed to execute + and the number of hooks that executed successfully. + type: integer + hooksFailed: + description: HooksFailed is the total number of hooks + which ended with an error + type: integer + type: object + 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: |- - ItemsBackedUp is the number of items that have actually been written to the - backup tarball so far. + VolumeSnapshotsCompleted is the total number of successfully + completed volume snapshots for this backup. type: integer - totalItems: + warnings: 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. + 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 - 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 type: object type: object diff --git a/docs/design/nab_and_nar_status_update.md b/docs/design/nab_and_nar_status_update.md new file mode 100644 index 0000000..109680d --- /dev/null +++ b/docs/design/nab_and_nar_status_update.md @@ -0,0 +1,193 @@ +# Developer Workflow: NonAdminBackup and NonAdminRestore Status Update + +## Overview + +This document outlines the design around updating NonAdminBackup (NAB) and NonAdminRestore (NAR) objects' Statuses. + +## NonAdminBackup and NonAdminRestore Status + +The `status` field of NAB and NAR objects contains the following fields: +- `phase` +- `conditions` +- `veleroBackup` for NAB and `veleroRestore` for NAR, which contains name, namespace and status of the related Velero object. + +which are updated by NAB and NAR controllers. + +Only one update call should be performed per reconcile of NAB and NAR objects. Controller can requeue or depend on predicates if more updates are needed in that object. + +### Phase + +The `phase` field is a simple one high-level summary of the lifecycle of the objects, that only moves forward. Once a `phase` changes, it can not return to the previous value. + +It is always a one well defined value, that is intended to be a comprehensive state of a NAB or NAR object. + +Those are are the possible values for phase: + +| **Value** | **Description** | +|-----------|-----------------| +| New | *NonAdminBackup/NonAdminRestore* resource was accepted by the NAB/NAR Controller, but it has not yet been validated by the NAB/NAR Controller | +| BackingOff | *NonAdminBackup/NonAdminRestore* resource was invalidated by the NAB/NAR Controller, due to invalid Spec. NAB/NAR Controller will not reconcile the object further, until user updates it | +| Created | *NonAdminBackup/NonAdminRestore* resource was validated by the NAB/NAR Controller and Velero *Backup/restore* was created. The Phase will not have additional information about the *Backup/Restore* run | +| Deleted | TODO | + +### Conditions + +The `conditions` field is a list of conditions. +One NAB/NAR object may have multiple conditions. +It is more granular knowledge of the NAB/NAR object and represents the array of the conditions through which the object has or has not passed. + +Each `condition` data is composed by the following: + +| **Field name** | **Description** | +|----------------|-----------------| +| type | The `NonAdminCondition` of the condition | +| status | represents the state of individual condition. One of `True`, `False` or `Unknown`. | +| lastTransitionTime | Timestamp for when this condition was created/updated. | +| reason | Machine-readable, UpperCamelCase text indicating details about the last status transition. | +| message | Human-readable message indicating details about the last status transition. | + +Those are are the possible values for `NonAdminCondition`: + +| **Value** | **Description** | +|-----------|-----------------| +| Accepted | The NonAdminBackup/NonAdminRestore object was accepted by the controller, but the Velero Backup/Restore may have not yet been created | +| Queued | The Velero Backup/Restore was created successfully. At this stage errors may still occur either from the Velero not accepting object or during backup/restore procedure. | + +### Velero object reference + +NonAdminBackup/NonAdminRestore `status` contains reference to the related Velero Backup/Restore. + +NonAdminBackup `status.veleroBackup` contains `name`, `namespace` and `status`. +`status.veleroBackup.name` represents the name of the `VeleroBackup` object. `status.veleroBackup.namespace` represents the namespace in which the `VeleroBackup` object was created. +`status.veleroBackup.status` field is a copy of the `VeleroBackup` object status. + +The format of those fields allows to interact with that Backup using `oc` or `velero` commands as follows: + +```yaml +status: + veleroBackup: + name: nab-nacproject-c3499c2729730a + namespace: openshift-adp + status: + ... +``` + +```shell +oc describe -n openshift-adp nab-nacproject-c3499c2729730a + +velero backup describe -n openshift-adp nab-nacproject-c3499c2729730a +``` + +Similarly, NonAdminRestore `status.veleroRestore` contains `name`, `namespace` and `status`. +`status.veleroRestore.name` represents the name of the `veleroRestore` object. `status.veleroRestore.namespace` represents the namespace in which the `veleroRestore` object was created. +`status.veleroRestore.status` field is a copy of the `VeleroBackup` object status. + +## Example + +Sample status field of a NonAdminBackup object. +It is similar for a NonAdminRestore. + +Object passed validation and Velero `Backup` object was created, but there was an error while performing backup operation: + +```yaml +status: + veleroBackup: + name: nab-nacproject-83fc04a2fd253d + namespace: openshift-adp + status: + expiration: '2024-05-16T08:12:11Z' + failureReason: >- + unable to get credentials: unable to get key for secret: Secret + "cloud-credentials" not found + formatVersion: 1.1.0 + phase: Failed + startTimestamp: '2024-04-16T08:12:11Z' + version: 1 + conditions: + - lastTransitionTime: '2024-04-15T20:27:35Z' + message: NonAdminBackup object was validated + reason: NonAdminBackupAccepted + status: 'True' + type: Accepted + - lastTransitionTime: '2024-04-15T20:27:45Z' + message: Created Velero Backup object + reason: BackupScheduled + status: 'True' + type: Queued + phase: Created +``` + +## Status Update scenarios + +The following graph shows the lifecycle of a NonAdminBackup. +It is similar for a NonAdminRestore. + +```mermaid +%%{init: {'theme':'neutral'}}%% +graph +event1{{predicate: Create event NAB}} == If event is accepted ==> + +reconcileStart1[/Reconcile start\] ==> + +phaseNew["`status.phase to **New**`"] -. Requeue: false, err .-> reconcileStart1 +phaseNew ==> + +reconcileExit1[\Requeue: true, nil/] +reconcileExit1 ==> question(is NonAdminBackup Spec valid?) == yes ==> reconcileStart2 +question == no ==> reconcileStartInvalid1 + +reconcileStart2[/Reconcile start\] ==> + +conditionsAcceptedTrue["`status.conditions[Accepted] to **True**`"] -. Requeue: false, err .- reconcileStart2 +conditionsAcceptedTrue ==> + +reconcileExit2[\Requeue: true, nil/] ==> + +reconcileStart3[/Reconcile start\] ==> + +createVeleroBackup([Create Velero Backup]) -. Requeue: false, err .- reconcileStart3 +createVeleroBackup ==> + +phaseCreated["` +status.phase: **Created** +status.conditions[Queued] to **True** +status.conditions.veleroBackup.name +status.conditions.veleroBackup.namespace +`"] -. Requeue: false, err .- reconcileStart3 +phaseCreated ==> + +reconcileExit4[\Requeue: false, nil/] + +reconcileStartInvalid1[/Reconcile start\] ==> + +phaseBackingOff["` +status.phase to **BackingOff** +status.conditions[Accepted] to **False** +`"] -. Requeue: false, err .- reconcileStartInvalid1 +phaseBackingOff ==> + +reconcileExitInvalid1[\Requeue: false, TerminalError/] + +reconcileExit4 ~~~ event2 +reconcileExitInvalid1 ~~~ event2 + +event2{{predicate: Update event VeleroBackup}} == If event is accepted ==> +handler1{{handler: Handle VeleroBackup update event}} ==> reconcileStart5 + +reconcileStart5[/Reconcile start\] ==> conditionsVeleroBackupStatus + +conditionsVeleroBackupStatus["`status.conditions.veleroBackup.status`"] -. Requeue: false, err .- reconcileStart5 +conditionsVeleroBackupStatus ==> + +reconcileExit5[\Requeue: false, nil/] + +reconcileExit5 ~~~ event3 + +event3{{predicate: Delete event NAB}} == If event is accepted ==> + +reconcileDelete1[/Reconcile start\] ==> + +reconcileExitDelete1[\Requeue: false, nil/] -. Requeue: false, err .-> reconcileDelete1 + +event4{{predicate: Update event NAB}} == If event is accepted ==> question +``` diff --git a/docs/design/nab_status_update.md b/docs/design/nab_status_update.md deleted file mode 100644 index 55ec6b7..0000000 --- a/docs/design/nab_status_update.md +++ /dev/null @@ -1,111 +0,0 @@ -# Developer Workflow: NonAdminBackup Phase and Conditions within NonAdminBackup Status - -## Overview - -This document outlines the design around updating NonAdminBackup objects Phase and Conditions within Status. - -### Phase - -A NonAdminBackup's `status` field has a `phase` field, which is updated by NAC controller. - -The `phase` is a simple one high-level summary of the lifecycle of an NonAdminBackup. - -It is always a one well defined value, that is intended to be a comprehensive state of a NonAdminBackup. - -Those are are the possible values for phase: - -| **Value** | **Description** | -|-----------|--------------------------------| -| New | *NonAdminBackup resource was accepted by the OpenShift cluster, but it has not yet been processed by the NonAdminController* | -| BackingOff | Velero *Backup* object was not created due to NonAdminBackup error (configuration or similar) | -| Created | Velero *Backup* was created. The Phase will not have additional informations about the *Backup* run | - -### Conditions - -The `conditions` is also a part of the NonAdminBackup's `status` field. One NAB 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. Each `NonAdminCondition` has one of the following `type`: - -| **Condition** | **Description** | -|-----------|--------------------------------| -| Accepted | The Backup object was accepted by the reconcile loop, but the Velero Backup may have not yet been created | -| Queued | The Velero Backup was created succesfully. At this stage errors may still occur either from the Velero not accepting backup or during backup procedure. | - -The `condition` data is also accomapied with the following: - -| **Field name** | **Description** | -|-----------|--------------------------------| -| type | The `Type` of the condition | -| status | represents the state of individual condition. The resulting `phase` value should report `Success` only when all of the conditions are met and the backup succeded. One of `True`, `False` or `Unknown`. | -| lastTransitionTime | Timestamp for when the NonAdminBackup last transitioned from one status to another. | -| reason | Machine-readable, UpperCamelCase text indicating the reason for the condition's last transition. | -| message | Human-readable message indicating details about the last status transition. | - -### VeleroBackupStatus - -The `VeleroBackupStatus` that is part of `NonAdminBackup.Status` is a copy of the `VeleroBackup.Status`. - - -> Sample `VeleroBackupStatus` with `Conditions` of a `NonAdminBackup` object that allowed to create `VeleroBackup` object, but there was an error while performing backup itself: - -```yaml -status: - veleroBackupStatus: - expiration: '2024-05-16T08:12:11Z' - failureReason: >- - unable to get credentials: unable to get key for secret: Secret - "cloud-credentials" not found - formatVersion: 1.1.0 - phase: Failed - startTimestamp: '2024-04-16T08:12:11Z' - version: 1 - conditions: - - lastTransitionTime: '2024-04-15T20:27:35Z' - message: Backup was accepted by the NAC controller - reason: BackupAccepted - status: 'True' - type: Accepted - - lastTransitionTime: '2024-04-15T20:27:45Z' - message: Created Velero Backup object - reason: BackupScheduled - status: 'True' - type: Queued - veleroBackupName: nab-nacproject-83fc04a2fd253d - veleroBackupNamespace: openshift-adp - phase: Created -``` - -### VeleroBackupName and VeleroBackupNamespace -The `VeleroBackupName` is a component of the `NonAdminBackup.Status` object. It represents the name of the `VeleroBackup` object. The `VeleroBackupNamespace` represents the namespace in which the `VeleroBackup` object was created. - -This `VeleroBackupName` and `VeleroBackupNamespace` serves as a reference to the Backup responsible for executing the backup task. - -The format of those fields allows to interact with that Backup using `oc` or `velero` commands as follows: - -```shell -# Example: -# veleroBackupName: nab-nacproject-c3499c2729730a -# veleroBackupNamespace: openshift-adp - -$ oc describe -n openshift-adp nab-nacproject-c3499c2729730a - -$ velero backup describe -n openshift-adp nab-nacproject-c3499c2729730a -``` - - -## Phase Update scenarios - -```mermaid -%%{init: {'theme':'forest'}}%% -graph -START[Phase: New] -- NAC config OK --> ACCEPTED[Non Admin Backup Accepted] -START -- NAC config NOT OK --> ERROR[Phase: BackingOff] -ACCEPTED -- Create Velero Backup --> CREATED[Phase: Created] -ACCEPTED -.-> COND_ACCEPTED{Conditions:\nAccepted: True\n} -CREATED -.-> COND_QUEUED{Conditions:\nAccepted: True\nQueued: True\n} -ERROR -.-> COND_ERROR{Conditions:\nAccepted: False} - -classDef conditions fill:#ccc,stroke:#ccc,stroke-width:2px; -class COND_ACCEPTED,COND_QUEUED,COND_ERROR conditions; -classDef phases fill:#777,stroke:#ccc,stroke-width:2px; -class START,CREATED,ERROR phases; - -``` diff --git a/internal/controller/nonadminbackup_controller.go b/internal/controller/nonadminbackup_controller.go index b64a7cd..d1ac736 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -337,9 +337,15 @@ func updateNonAdminPhase(phase *nacv1alpha1.NonAdminBackupPhase, newPhase nacv1a // updateNonAdminBackupVeleroBackupStatus sets the VeleroBackup reference fields in NonAdminBackup object status and returns true // if the VeleroBackup fields are changed by this call. func updateNonAdminBackupVeleroBackupReference(status *nacv1alpha1.NonAdminBackupStatus, veleroBackup *velerov1.Backup) bool { - if status.VeleroBackupName != veleroBackup.Name || status.VeleroBackupNamespace != veleroBackup.Namespace { - status.VeleroBackupName = veleroBackup.Name - status.VeleroBackupNamespace = veleroBackup.Namespace + if status.VeleroBackup == nil { + status.VeleroBackup = &nacv1alpha1.VeleroBackup{ + Name: veleroBackup.Name, + Namespace: veleroBackup.Namespace, + } + return true + } else if status.VeleroBackup.Name != veleroBackup.Name || status.VeleroBackup.Namespace != veleroBackup.Namespace { + status.VeleroBackup.Name = veleroBackup.Name + status.VeleroBackup.Namespace = veleroBackup.Namespace return true } return false @@ -348,8 +354,8 @@ func updateNonAdminBackupVeleroBackupReference(status *nacv1alpha1.NonAdminBacku // updateNonAdminBackupVeleroBackupStatus sets the VeleroBackup status field in NonAdminBackup object status and returns true // if the VeleroBackup fields are changed by this call. func updateNonAdminBackupVeleroBackupStatus(status *nacv1alpha1.NonAdminBackupStatus, veleroBackup *velerov1.Backup) bool { - if !reflect.DeepEqual(status.VeleroBackupStatus, &veleroBackup.Status) { - status.VeleroBackupStatus = veleroBackup.Status.DeepCopy() + if !reflect.DeepEqual(status.VeleroBackup.Status, &veleroBackup.Status) { + status.VeleroBackup.Status = veleroBackup.Status.DeepCopy() return true } return false diff --git a/internal/controller/nonadminbackup_controller_test.go b/internal/controller/nonadminbackup_controller_test.go index fe06d6b..9addacc 100644 --- a/internal/controller/nonadminbackup_controller_test.go +++ b/internal/controller/nonadminbackup_controller_test.go @@ -69,22 +69,24 @@ func checkTestNonAdminBackupStatus(nonAdminBackup *nacv1alpha1.NonAdminBackup, e if nonAdminBackup.Status.Phase != expectedStatus.Phase { return fmt.Errorf("NonAdminBackup Status Phase %v is not equal to expected %v", nonAdminBackup.Status.Phase, expectedStatus.Phase) } - veleroBackupName := expectedStatus.VeleroBackupName - if expectedStatus.VeleroBackupName == placeholder { - veleroBackupName = function.GenerateVeleroBackupName(nonAdminNamespaceName, testNonAdminBackupName) - } - if nonAdminBackup.Status.VeleroBackupName != veleroBackupName { - return fmt.Errorf("NonAdminBackup Status VeleroBackupName %v is not equal to expected %v", nonAdminBackup.Status.VeleroBackupName, veleroBackupName) - } - veleroBackupNamespace := expectedStatus.VeleroBackupNamespace - if expectedStatus.VeleroBackupNamespace == placeholder { - veleroBackupNamespace = oadpNamespaceName - } - if nonAdminBackup.Status.VeleroBackupNamespace != veleroBackupNamespace { - return fmt.Errorf("NonAdminBackup Status VeleroBackupNamespace %v is not equal to expected %v", nonAdminBackup.Status.VeleroBackupNamespace, veleroBackupNamespace) - } - if !reflect.DeepEqual(nonAdminBackup.Status.VeleroBackupStatus, expectedStatus.VeleroBackupStatus) { - return fmt.Errorf("NonAdminBackup Status VeleroBackupStatus %v is not equal to expected %v", nonAdminBackup.Status.VeleroBackupStatus, expectedStatus.VeleroBackupStatus) + if expectedStatus.VeleroBackup != nil { + veleroBackupName := expectedStatus.VeleroBackup.Name + if expectedStatus.VeleroBackup.Name == placeholder { + veleroBackupName = function.GenerateVeleroBackupName(nonAdminNamespaceName, testNonAdminBackupName) + } + if nonAdminBackup.Status.VeleroBackup.Name != veleroBackupName { + return fmt.Errorf("NonAdminBackup Status VeleroBackupName %v is not equal to expected %v", nonAdminBackup.Status.VeleroBackup.Name, veleroBackupName) + } + veleroBackupNamespace := expectedStatus.VeleroBackup.Namespace + if expectedStatus.VeleroBackup.Namespace == placeholder { + veleroBackupNamespace = oadpNamespaceName + } + if nonAdminBackup.Status.VeleroBackup.Namespace != veleroBackupNamespace { + return fmt.Errorf("NonAdminBackup Status VeleroBackupNamespace %v is not equal to expected %v", nonAdminBackup.Status.VeleroBackup.Namespace, veleroBackupNamespace) + } + if !reflect.DeepEqual(nonAdminBackup.Status.VeleroBackup.Status, expectedStatus.VeleroBackup.Status) { + return fmt.Errorf("NonAdminBackup Status VeleroBackupStatus %v is not equal to expected %v", nonAdminBackup.Status.VeleroBackup.Status, expectedStatus.VeleroBackup.Status) + } } if len(nonAdminBackup.Status.Conditions) != len(expectedStatus.Conditions) { @@ -225,11 +227,13 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func if scenario.priorStatus != nil { nonAdminBackup.Status = *scenario.priorStatus - if nonAdminBackup.Status.VeleroBackupName == placeholder { - nonAdminBackup.Status.VeleroBackupName = function.GenerateVeleroBackupName(nonAdminNamespaceName, testNonAdminBackupName) - } - if nonAdminBackup.Status.VeleroBackupNamespace == placeholder { - nonAdminBackup.Status.VeleroBackupNamespace = oadpNamespaceName + if nonAdminBackup.Status.VeleroBackup != nil { + if nonAdminBackup.Status.VeleroBackup.Name == placeholder { + nonAdminBackup.Status.VeleroBackup.Name = function.GenerateVeleroBackupName(nonAdminNamespaceName, testNonAdminBackupName) + } + if nonAdminBackup.Status.VeleroBackup.Namespace == placeholder { + nonAdminBackup.Status.VeleroBackup.Namespace = oadpNamespaceName + } } gomega.Expect(k8sClient.Status().Update(ctx, nonAdminBackup)).To(gomega.Succeed()) } @@ -315,9 +319,11 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func }, }, ExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, - VeleroBackupName: placeholder, - VeleroBackupNamespace: placeholder, + Phase: nacv1alpha1.NonAdminBackupPhaseCreated, + VeleroBackup: &nacv1alpha1.VeleroBackup{ + Name: placeholder, + Namespace: placeholder, + }, Conditions: []metav1.Condition{ { Type: "Accepted", @@ -341,9 +347,11 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func BackupSpec: &velerov1.BackupSpec{}, }, priorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, - VeleroBackupName: placeholder, - VeleroBackupNamespace: placeholder, + Phase: nacv1alpha1.NonAdminBackupPhaseCreated, + VeleroBackup: &nacv1alpha1.VeleroBackup{ + Name: placeholder, + Namespace: placeholder, + }, Conditions: []metav1.Condition{ { Type: "Accepted", @@ -362,10 +370,12 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func }, }, ExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, - VeleroBackupName: placeholder, - VeleroBackupNamespace: placeholder, - VeleroBackupStatus: &velerov1.BackupStatus{}, + Phase: nacv1alpha1.NonAdminBackupPhaseCreated, + VeleroBackup: &nacv1alpha1.VeleroBackup{ + Name: placeholder, + Namespace: placeholder, + Status: &velerov1.BackupStatus{}, + }, Conditions: []metav1.Condition{ { Type: "Accepted", @@ -476,7 +486,7 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminBackup Controller", ginkgo.By("Validating NonAdminBackup Status") gomega.Expect(checkTestNonAdminBackupStatus(nonAdminBackup, scenario.status, nonAdminNamespaceName, oadpNamespaceName)).To(gomega.Succeed()) - if len(scenario.status.VeleroBackupName) > 0 { + if scenario.status.VeleroBackup != nil && len(scenario.status.VeleroBackup.Name) > 0 { ginkgo.By("Checking if NonAdminBackup Spec was not changed") gomega.Expect(reflect.DeepEqual( nonAdminBackup.Spec, @@ -509,10 +519,10 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminBackup Controller", if err != nil { return false, err } - if nonAdminBackup.Status.VeleroBackupStatus == nil { + if nonAdminBackup.Status.VeleroBackup.Status == nil { return false, nil } - return nonAdminBackup.Status.VeleroBackupStatus.Phase == velerov1.BackupPhaseCompleted, nil + return nonAdminBackup.Status.VeleroBackup.Status.Phase == velerov1.BackupPhaseCompleted, nil }, 5*time.Second, 1*time.Second).Should(gomega.BeTrue()) } @@ -525,10 +535,12 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminBackup Controller", BackupSpec: &velerov1.BackupSpec{}, }, status: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, - VeleroBackupName: placeholder, - VeleroBackupNamespace: placeholder, - VeleroBackupStatus: nil, + Phase: nacv1alpha1.NonAdminBackupPhaseCreated, + VeleroBackup: &nacv1alpha1.VeleroBackup{ + Name: placeholder, + Namespace: placeholder, + Status: nil, + }, Conditions: []metav1.Condition{ { Type: "Accepted",