Skip to content

Commit

Permalink
NonAdminBackup updates to align with updated design
Browse files Browse the repository at this point in the history 
Additional improvements to the NonAdminBackup:
 - Use generic condition names (Accepted, Queued)
 - Introduce VeleroBackupNamespace
 - Change Status: OadpVeleroBackup to VeleroBackupName
 - Use CammelCase for condition reasoan

Signed-off-by: Michal Pryc <[email protected]>
  • Loading branch information
@mpryc
mpryc committed Apr 23, 2024
1 parent b7d1c08 commit 5bb0bb4
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 102 deletions.
19 changes: 12 additions & 7 deletions api/v1alpha1/nonadminbackup_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,16 @@ const (
)

// NonAdminCondition are used for more detailed information supporing NonAdminBackupPhase state.
// +kubebuilder:validation:Enum=BackupAccepted;BackupQueued
// +kubebuilder:validation:Enum=Accepted;Queued
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"
NonAdminConditionAccepted NonAdminCondition = "Accepted"
NonAdminConditionQueued NonAdminCondition = "Queued"
)

// NonAdminBackupSpec defines the desired state of NonAdminBackup
Expand All @@ -60,13 +60,18 @@ type NonAdminBackupSpec struct {

// NonAdminBackupStatus defines the observed state of NonAdminBackup
type NonAdminBackupStatus struct {
// OadpVeleroBackup references the VeleroBackup object. Format: <name>.<namespace>
// VeleroBackupName references the VeleroBackup object by it's name.
// +optional
OadpVeleroBackup string `json:"oadpVeleroBackup,omitempty"`
VeleroBackupName string `json:"veleroBackupName,omitempty"`

// BackupStatus captures the current status of a Velero backup.
// VeleroBackupNamespace references the Namespace
// in which VeleroBackupName object exists.
// +optional
BackupStatus *velerov1api.BackupStatus `json:"backupStatus,omitempty"`
VeleroBackupNamespace string `json:"veleroBackupNamespace,omitempty"`

// VeleroBackupStatus captures the current status of a Velero backup.
// +optional
VeleroBackupStatus *velerov1api.BackupStatus `json:"veleroBackupStatus,omitempty"`

Phase NonAdminBackupPhase `json:"phase,omitempty"`
Conditions []metav1.Condition `json:"conditions,omitempty"`
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

171 changes: 88 additions & 83 deletions config/crd/bases/nac.oadp.openshift.io_nonadminbackups.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,94 @@ spec:
status:
description: NonAdminBackupStatus defines the observed state of NonAdminBackup
properties:
backupStatus:
description: BackupStatus captures the current status of a Velero
conditions:
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: array
phase:
description: NonAdminBackupPhase is a simple one high-level summary
of the lifecycle of an NonAdminBackup.
enum:
- New
- 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.
properties:
backupItemOperationsAttempted:
Expand Down Expand Up @@ -650,87 +736,6 @@ spec:
file in object storage.
type: integer
type: object
conditions:
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: array
oadpVeleroBackup:
description: 'OadpVeleroBackup references the VeleroBackup object.
Format: <name>.<namespace>'
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
Expand Down
2 changes: 1 addition & 1 deletion config/manager/kustomization.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ kind: Kustomization
images:
- name: controller
newName: quay.io/migi/oadp-nac-operator
newTag: v0.0.56
newTag: v0.0.57
11 changes: 6 additions & 5 deletions internal/common/function/function.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,18 +216,19 @@ func UpdateNonAdminBackupCondition(ctx context.Context, r client.Client, logger

// 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")
logger.V(1).Info("NonAdminBackup BackupSpec and VeleroBackupStatus - request to update")

if reflect.DeepEqual(nab.Status.BackupStatus, &veleroBackup.Status) && reflect.DeepEqual(nab.Spec.BackupSpec, &veleroBackup.Spec) {
if reflect.DeepEqual(nab.Status.VeleroBackupStatus, &veleroBackup.Status) && reflect.DeepEqual(nab.Spec.BackupSpec, &veleroBackup.Spec) {
// No change, no need to update
logger.V(1).Info("NonAdminBackup BackupSpec and BackupStatus - nothing to update")
return false, nil
}

// 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 !reflect.DeepEqual(nab.Status.VeleroBackupStatus, &veleroBackup.Status) || nab.Status.VeleroBackupName != veleroBackup.Name || nab.Status.VeleroBackupNamespace != veleroBackup.Namespace {
nab.Status.VeleroBackupStatus = veleroBackup.Status.DeepCopy()
nab.Status.VeleroBackupName = veleroBackup.Name
nab.Status.VeleroBackupNamespace = veleroBackup.Namespace
if err := r.Status().Update(ctx, nab); err != nil {
logger.Error(err, "NonAdminBackup BackupStatus - Failed to update")
return false, err
Expand Down
8 changes: 4 additions & 4 deletions internal/controller/nonadminbackup_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,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.NonAdminBackupConditionAccepted, metav1.ConditionFalse, "invalid_backupspec", errMsg)
updatedStatus, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionFalse, "InvalidBackupSpec", errMsg)
if updatedStatus {
return ctrl.Result{}, nil
}
Expand All @@ -142,7 +142,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.NonAdminBackupConditionAccepted, metav1.ConditionTrue, "backup_accepted", "backup accepted")
updatedStatus, errUpdate := function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionTrue, "BackupAccepted", "backup accepted")
if updatedStatus {
return ctrl.Result{Requeue: true, RequeueAfter: requeueTimeSeconds * time.Second}, nil
}
Expand Down Expand Up @@ -222,12 +222,12 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque
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")
_, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminConditionAccepted, metav1.ConditionTrue, "Validated", "Valid Backup config")
if errUpdate != nil {
logger.Error(errUpdate, "Unable to set BackupAccepted Condition: True", nameField, req.Name, constant.NameSpaceString, req.Namespace)
return ctrl.Result{}, errUpdate
}
_, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminBackupConditionQueued, metav1.ConditionTrue, "backup_scheduled", "Created Velero Backup object")
_, errUpdate = function.UpdateNonAdminBackupCondition(ctx, r.Client, logger, &nab, nacv1alpha1.NonAdminConditionQueued, metav1.ConditionTrue, "BackupScheduled", "Created Velero Backup object")
if errUpdate != nil {
logger.Error(errUpdate, "Unable to set BackupQueued Condition: True", nameField, req.Name, constant.NameSpaceString, req.Namespace)
return ctrl.Result{}, errUpdate
Expand Down

0 comments on commit 5bb0bb4

Please sign in to comment.