From 762111fbaea28786552f1da610e7a86d8de8ecda Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Thu, 14 Nov 2024 09:30:07 -0300 Subject: [PATCH 01/14] feat: Add NAC restore controller Signed-off-by: Mateus Oliveira --- api/v1alpha1/nonadminbackup_types.go | 26 ++--- api/v1alpha1/nonadmincontroller_types.go | 31 ------ .../oadp.openshift.io_nonadminbackups.yaml | 4 +- internal/common/constant/constant.go | 13 +++ .../controller/nonadminbackup_controller.go | 29 +++--- .../nonadminbackup_controller_test.go | 98 +++++++++---------- 6 files changed, 90 insertions(+), 111 deletions(-) delete mode 100644 api/v1alpha1/nonadmincontroller_types.go diff --git a/api/v1alpha1/nonadminbackup_types.go b/api/v1alpha1/nonadminbackup_types.go index 724e3ca..9544dd5 100644 --- a/api/v1alpha1/nonadminbackup_types.go +++ b/api/v1alpha1/nonadminbackup_types.go @@ -21,19 +21,19 @@ 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 an NonAdminBackup. // +kubebuilder:validation:Enum=New;BackingOff;Created;Deleting -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" - // NonAdminBackupPhaseDeleting - Velero Backup is pending deletion. The Phase will not have additional informations about the Backup. - NonAdminBackupPhaseDeleting NonAdminBackupPhase = "Deleting" + // NonAdminPhaseNew - NonAdmin object 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 NonAdmin object error (configuration or similar) + NonAdminPhaseBackingOff NonAdminPhase = "BackingOff" + // NonAdminPhaseCreated - Velero object was created. The Phase will not have additional information about it. + NonAdminPhaseCreated NonAdminPhase = "Created" + // NonAdminPhaseDeleting - Velero object is pending deletion. The Phase will not have additional information about it. + NonAdminPhaseDeleting NonAdminPhase = "Deleting" ) // NonAdminBackupSpec defines the desired state of NonAdminBackup @@ -106,8 +106,10 @@ type NonAdminBackupStatus struct { // +optional QueueInfo *QueueInfo `json:"queueInfo,omitempty"` - Phase NonAdminBackupPhase `json:"phase,omitempty"` - Conditions []metav1.Condition `json:"conditions,omitempty"` + // phase is a simple one high-level summary of the lifecycle of an NonAdminBackup. + Phase NonAdminPhase `json:"phase,omitempty"` + + Conditions []metav1.Condition `json:"conditions,omitempty"` } // QueueInfo holds the queue position for a specific VeleroBackup. diff --git a/api/v1alpha1/nonadmincontroller_types.go b/api/v1alpha1/nonadmincontroller_types.go deleted file mode 100644 index 0c49b64..0000000 --- a/api/v1alpha1/nonadmincontroller_types.go +++ /dev/null @@ -1,31 +0,0 @@ -/* -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 v1alpha1 - -// NonAdminCondition are used for more detailed information supporing NonAdminBackupPhase state. -// +kubebuilder:validation:Enum=Accepted;Queued;Deleting -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" - NonAdminConditionDeleting NonAdminCondition = "Deleting" -) diff --git a/config/crd/bases/oadp.openshift.io_nonadminbackups.yaml b/config/crd/bases/oadp.openshift.io_nonadminbackups.yaml index 4ac9f1d..b76a026 100644 --- a/config/crd/bases/oadp.openshift.io_nonadminbackups.yaml +++ b/config/crd/bases/oadp.openshift.io_nonadminbackups.yaml @@ -611,8 +611,8 @@ spec: type: object type: array phase: - description: NonAdminBackupPhase is a simple one high-level summary - of the lifecycle of an NonAdminBackup. + description: phase is a simple one high-level summary of the lifecycle + of an NonAdminBackup. enum: - New - BackingOff diff --git a/internal/common/constant/constant.go b/internal/common/constant/constant.go index 937d14f..aa408e2 100644 --- a/internal/common/constant/constant.go +++ b/internal/common/constant/constant.go @@ -55,3 +55,16 @@ const NamespaceString = "Namespace" // MaximumNacObjectNameLength represents Generated Non Admin Object Name and // must be below 63 characters, because it's used within object Label Value const MaximumNacObjectNameLength = validation.DNS1123LabelMaxLength + +// NonAdminCondition are used for more detailed information supporting NonAdminPhase state. +type NonAdminCondition string + +// Predefined conditions for NonAdmin object. +// One NonAdmin object may have multiple conditions. +// It is more granular knowledge of the NonAdmin object and represents the +// array of the conditions through which the NonAdmin object has or has not passed +const ( + NonAdminConditionAccepted NonAdminCondition = "Accepted" + NonAdminConditionQueued NonAdminCondition = "Queued" + NonAdminConditionDeleting NonAdminCondition = "Deleting" +) diff --git a/internal/controller/nonadminbackup_controller.go b/internal/controller/nonadminbackup_controller.go index 4b3febe..e3eeae1 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -162,10 +162,10 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque // - error: any error encountered during the process func (r *NonAdminBackupReconciler) setStatusAndConditionForDeletionAndCallDelete(ctx context.Context, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup) (bool, error) { requeueRequired := false - updatedPhase := updateNonAdminPhase(&nab.Status.Phase, nacv1alpha1.NonAdminBackupPhaseDeleting) + updatedPhase := updateNonAdminPhase(&nab.Status.Phase, nacv1alpha1.NonAdminPhaseDeleting) updatedCondition := meta.SetStatusCondition(&nab.Status.Conditions, metav1.Condition{ - Type: string(nacv1alpha1.NonAdminConditionDeleting), + Type: string(constant.NonAdminConditionDeleting), Status: metav1.ConditionTrue, Reason: "DeletionPending", Message: "backup accepted for deletion", @@ -207,10 +207,10 @@ func (r *NonAdminBackupReconciler) setStatusAndConditionForDeletionAndCallDelete func (r *NonAdminBackupReconciler) setStatusForDirectKubernetesAPIDeletion(ctx context.Context, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup) (bool, error) { // We don't need to check here if the finalizer exists as we already checked if !nab.ObjectMeta.DeletionTimestamp.IsZero() // which means that something prevented the NAB object from being deleted - updatedPhase := updateNonAdminPhase(&nab.Status.Phase, nacv1alpha1.NonAdminBackupPhaseDeleting) + updatedPhase := updateNonAdminPhase(&nab.Status.Phase, nacv1alpha1.NonAdminPhaseDeleting) updatedCondition := meta.SetStatusCondition(&nab.Status.Conditions, metav1.Condition{ - Type: string(nacv1alpha1.NonAdminConditionDeleting), + Type: string(constant.NonAdminConditionDeleting), Status: metav1.ConditionTrue, Reason: "DeletionPending", Message: "backup deletion requires setting spec.deleteBackup or spec.forceDeleteBackup to true or finalizer removal", @@ -452,7 +452,7 @@ func (r *NonAdminBackupReconciler) initNabCreate(ctx context.Context, logger log } // Set phase to New - if updated := updateNonAdminPhase(&nab.Status.Phase, nacv1alpha1.NonAdminBackupPhaseNew); updated { + if updated := updateNonAdminPhase(&nab.Status.Phase, nacv1alpha1.NonAdminPhaseNew); updated { if err := r.Status().Update(ctx, nab); err != nil { logger.Error(err, statusUpdateError) return false, err @@ -480,10 +480,10 @@ func (r *NonAdminBackupReconciler) initNabCreate(ctx context.Context, logger log func (r *NonAdminBackupReconciler) validateSpec(ctx context.Context, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup) (bool, error) { err := function.ValidateBackupSpec(nab, r.EnforcedBackupSpec) if err != nil { - updatedPhase := updateNonAdminPhase(&nab.Status.Phase, nacv1alpha1.NonAdminBackupPhaseBackingOff) + updatedPhase := updateNonAdminPhase(&nab.Status.Phase, nacv1alpha1.NonAdminPhaseBackingOff) updatedCondition := meta.SetStatusCondition(&nab.Status.Conditions, metav1.Condition{ - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionFalse, Reason: "InvalidBackupSpec", Message: err.Error(), @@ -504,7 +504,7 @@ func (r *NonAdminBackupReconciler) validateSpec(ctx context.Context, logger logr updated := meta.SetStatusCondition(&nab.Status.Conditions, metav1.Condition{ - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", @@ -655,11 +655,11 @@ func (r *NonAdminBackupReconciler) createVeleroBackupAndSyncWithNonAdminBackup(c updatedQueueInfo = true } - updatedPhase := updateNonAdminPhase(&nab.Status.Phase, nacv1alpha1.NonAdminBackupPhaseCreated) + updatedPhase := updateNonAdminPhase(&nab.Status.Phase, nacv1alpha1.NonAdminPhaseCreated) updatedCondition := meta.SetStatusCondition(&nab.Status.Conditions, metav1.Condition{ - Type: string(nacv1alpha1.NonAdminConditionQueued), + Type: string(constant.NonAdminConditionQueued), Status: metav1.ConditionTrue, Reason: "BackupScheduled", Message: "Created Velero Backup object", @@ -706,14 +706,9 @@ func (r *NonAdminBackupReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -// updateNonAdminPhase sets the phase in NonAdminBackup object status and returns true +// updateNonAdminPhase sets the phase in NonAdmin object status and returns true // if the phase is changed by this call. -func updateNonAdminPhase(phase *nacv1alpha1.NonAdminBackupPhase, newPhase nacv1alpha1.NonAdminBackupPhase) bool { - // Ensure phase is valid - if newPhase == constant.EmptyString { - return false - } - +func updateNonAdminPhase(phase *nacv1alpha1.NonAdminPhase, newPhase nacv1alpha1.NonAdminPhase) bool { if *phase == newPhase { return false } diff --git a/internal/controller/nonadminbackup_controller_test.go b/internal/controller/nonadminbackup_controller_test.go index 79fd867..e9b86ed 100644 --- a/internal/controller/nonadminbackup_controller_test.go +++ b/internal/controller/nonadminbackup_controller_test.go @@ -324,10 +324,10 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func }, ginkgo.Entry("When triggered by NonAdminBackup Create event without BackupSpec, should update NonAdminBackup phase to BackingOff and exit with terminal error", nonAdminBackupSingleReconcileScenario{ nonAdminBackupExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseBackingOff, + Phase: nacv1alpha1.NonAdminPhaseBackingOff, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionFalse, Reason: "InvalidBackupSpec", Message: "BackupSpec is not defined", @@ -341,10 +341,10 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func DeleteBackup: true, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseBackingOff, + Phase: nacv1alpha1.NonAdminPhaseBackingOff, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionFalse, Reason: "InvalidBackupSpec", Message: "BackupSpec is not defined", @@ -361,17 +361,17 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func DeleteBackup: true, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, + Phase: nacv1alpha1.NonAdminPhaseCreated, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", LastTransitionTime: metav1.NewTime(time.Now()), }, { - Type: string(nacv1alpha1.NonAdminConditionQueued), + Type: string(constant.NonAdminConditionQueued), Status: metav1.ConditionTrue, Reason: "BackupScheduled", Message: "Created Velero Backup object", @@ -380,22 +380,22 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func }, }, nonAdminBackupExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseDeleting, + Phase: nacv1alpha1.NonAdminPhaseDeleting, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", }, { - Type: string(nacv1alpha1.NonAdminConditionQueued), + Type: string(constant.NonAdminConditionQueued), Status: metav1.ConditionTrue, Reason: "BackupScheduled", Message: "Created Velero Backup object", }, { - Type: string(nacv1alpha1.NonAdminConditionDeleting), + Type: string(constant.NonAdminConditionDeleting), Status: metav1.ConditionTrue, Reason: "DeletionPending", Message: "backup accepted for deletion", @@ -413,24 +413,24 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func DeleteBackup: true, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseDeleting, + Phase: nacv1alpha1.NonAdminPhaseDeleting, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", LastTransitionTime: metav1.NewTime(time.Now()), }, { - Type: string(nacv1alpha1.NonAdminConditionQueued), + Type: string(constant.NonAdminConditionQueued), Status: metav1.ConditionTrue, Reason: "BackupScheduled", Message: "Created Velero Backup object", LastTransitionTime: metav1.NewTime(time.Now()), }, { - Type: string(nacv1alpha1.NonAdminConditionDeleting), + Type: string(constant.NonAdminConditionDeleting), Status: metav1.ConditionTrue, Reason: "DeletionPending", Message: "backup accepted for deletion", @@ -439,22 +439,22 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func }, }, nonAdminBackupExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseDeleting, + Phase: nacv1alpha1.NonAdminPhaseDeleting, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", }, { - Type: string(nacv1alpha1.NonAdminConditionQueued), + Type: string(constant.NonAdminConditionQueued), Status: metav1.ConditionTrue, Reason: "BackupScheduled", Message: "Created Velero Backup object", }, { - Type: string(nacv1alpha1.NonAdminConditionDeleting), + Type: string(constant.NonAdminConditionDeleting), Status: metav1.ConditionTrue, Reason: "DeletionPending", Message: "backup accepted for deletion", @@ -470,17 +470,17 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func DeleteBackup: true, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, + Phase: nacv1alpha1.NonAdminPhaseCreated, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", LastTransitionTime: metav1.NewTime(time.Now()), }, { - Type: string(nacv1alpha1.NonAdminConditionQueued), + Type: string(constant.NonAdminConditionQueued), Status: metav1.ConditionTrue, Reason: "BackupScheduled", Message: "Created Velero Backup object", @@ -498,17 +498,17 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func ForceDeleteBackup: true, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, + Phase: nacv1alpha1.NonAdminPhaseCreated, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", LastTransitionTime: metav1.NewTime(time.Now()), }, { - Type: string(nacv1alpha1.NonAdminConditionQueued), + Type: string(constant.NonAdminConditionQueued), Status: metav1.ConditionTrue, Reason: "BackupScheduled", Message: "Created Velero Backup object", @@ -517,22 +517,22 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func }, }, nonAdminBackupExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseDeleting, + Phase: nacv1alpha1.NonAdminPhaseDeleting, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", }, { - Type: string(nacv1alpha1.NonAdminConditionQueued), + Type: string(constant.NonAdminConditionQueued), Status: metav1.ConditionTrue, Reason: "BackupScheduled", Message: "Created Velero Backup object", }, { - Type: string(nacv1alpha1.NonAdminConditionDeleting), + Type: string(constant.NonAdminConditionDeleting), Status: metav1.ConditionTrue, Reason: "DeletionPending", Message: "backup accepted for deletion", @@ -550,24 +550,24 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func ForceDeleteBackup: true, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseDeleting, + Phase: nacv1alpha1.NonAdminPhaseDeleting, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", LastTransitionTime: metav1.NewTime(time.Now()), }, { - Type: string(nacv1alpha1.NonAdminConditionQueued), + Type: string(constant.NonAdminConditionQueued), Status: metav1.ConditionTrue, Reason: "BackupScheduled", Message: "Created Velero Backup object", LastTransitionTime: metav1.NewTime(time.Now()), }, { - Type: string(nacv1alpha1.NonAdminConditionDeleting), + Type: string(constant.NonAdminConditionDeleting), Status: metav1.ConditionTrue, Reason: "DeletionPending", Message: "backup accepted for deletion", @@ -585,19 +585,19 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func BackupSpec: &velerov1.BackupSpec{}, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseNew, + Phase: nacv1alpha1.NonAdminPhaseNew, }, nonAdminBackupExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, + Phase: nacv1alpha1.NonAdminPhaseCreated, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", }, { - Type: string(nacv1alpha1.NonAdminConditionQueued), + Type: string(constant.NonAdminConditionQueued), Status: metav1.ConditionTrue, Reason: "BackupScheduled", Message: "Created Velero Backup object", @@ -611,10 +611,10 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func BackupSpec: &velerov1.BackupSpec{}, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseNew, + Phase: nacv1alpha1.NonAdminPhaseNew, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", @@ -623,10 +623,10 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func }, }, nonAdminBackupExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, + Phase: nacv1alpha1.NonAdminPhaseCreated, Conditions: []metav1.Condition{ { - Type: string(nacv1alpha1.NonAdminConditionAccepted), + Type: string(constant.NonAdminConditionAccepted), Status: metav1.ConditionTrue, Reason: "BackupAccepted", Message: "backup accepted", @@ -648,7 +648,7 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func BackupSpec: &velerov1.BackupSpec{}, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseNew, + Phase: nacv1alpha1.NonAdminPhaseNew, Conditions: []metav1.Condition{ { Type: "Accepted", @@ -660,10 +660,10 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func }, }, nonAdminBackupExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, QueueInfo: &nacv1alpha1.QueueInfo{ EstimatedQueuePosition: 0, }, + Phase: nacv1alpha1.NonAdminPhaseCreated, Conditions: []metav1.Condition{ { Type: "Accepted", @@ -689,10 +689,10 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func BackupSpec: &velerov1.BackupSpec{}, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, QueueInfo: &nacv1alpha1.QueueInfo{ EstimatedQueuePosition: 5, }, + Phase: nacv1alpha1.NonAdminPhaseCreated, VeleroBackup: &nacv1alpha1.VeleroBackup{}, Conditions: []metav1.Condition{ { @@ -712,10 +712,10 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func }, }, nonAdminBackupExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, QueueInfo: &nacv1alpha1.QueueInfo{ EstimatedQueuePosition: 1, }, + Phase: nacv1alpha1.NonAdminPhaseCreated, VeleroBackup: &nacv1alpha1.VeleroBackup{ Status: &velerov1.BackupStatus{}, }, @@ -744,10 +744,10 @@ var _ = ginkgo.Describe("Test single reconciles of NonAdminBackup Reconcile func }, }, nonAdminBackupPriorStatus: &nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseNew, + Phase: nacv1alpha1.NonAdminPhaseNew, }, nonAdminBackupExpectedStatus: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseBackingOff, + Phase: nacv1alpha1.NonAdminPhaseBackingOff, Conditions: []metav1.Condition{ { Type: "Accepted", @@ -918,7 +918,7 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminBackup Controller", BackupSpec: &velerov1.BackupSpec{}, }, status: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseCreated, + Phase: nacv1alpha1.NonAdminPhaseCreated, VeleroBackup: &nacv1alpha1.VeleroBackup{ Namespace: oadpNamespace, Status: nil, @@ -953,7 +953,7 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminBackup Controller", }, }, status: nacv1alpha1.NonAdminBackupStatus{ - Phase: nacv1alpha1.NonAdminBackupPhaseBackingOff, + Phase: nacv1alpha1.NonAdminPhaseBackingOff, Conditions: []metav1.Condition{ { Type: "Accepted", From 43fdebf91c0efd707317b8926a942382a072a0ec Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Thu, 14 Nov 2024 09:37:36 -0300 Subject: [PATCH 02/14] fixup! feat: Add NAC restore controller run kubebuilder command Signed-off-by: Mateus Oliveira --- PROJECT | 9 ++ api/v1alpha1/nonadminrestore_types.go | 64 +++++++++++++ api/v1alpha1/zz_generated.deepcopy.go | 89 +++++++++++++++++++ cmd/main.go | 7 ++ .../oadp.openshift.io_nonadminrestores.yaml | 54 +++++++++++ config/crd/kustomization.yaml | 3 + config/rbac/nonadminrestore_editor_role.yaml | 31 +++++++ config/rbac/nonadminrestore_viewer_role.yaml | 27 ++++++ config/rbac/role.yaml | 26 ++++++ config/samples/kustomization.yaml | 1 + .../oadp_v1alpha1_nonadminrestore.yaml | 12 +++ docs/architecture.md | 6 ++ .../controller/nonadminrestore_controller.go | 62 +++++++++++++ .../nonadminrestore_controller_test.go | 84 +++++++++++++++++ 14 files changed, 475 insertions(+) create mode 100644 api/v1alpha1/nonadminrestore_types.go create mode 100644 config/crd/bases/oadp.openshift.io_nonadminrestores.yaml create mode 100644 config/rbac/nonadminrestore_editor_role.yaml create mode 100644 config/rbac/nonadminrestore_viewer_role.yaml create mode 100644 config/samples/oadp_v1alpha1_nonadminrestore.yaml create mode 100644 internal/controller/nonadminrestore_controller.go create mode 100644 internal/controller/nonadminrestore_controller_test.go diff --git a/PROJECT b/PROJECT index c0925ec..6c59d03 100644 --- a/PROJECT +++ b/PROJECT @@ -17,4 +17,13 @@ resources: kind: NonAdminBackup path: github.com/migtools/oadp-non-admin/api/v1alpha1 version: v1alpha1 +- api: + crdVersion: v1 + namespaced: true + controller: true + domain: openshift.io + group: oadp + kind: NonAdminRestore + path: github.com/migtools/oadp-non-admin/api/v1alpha1 + version: v1alpha1 version: "3" diff --git a/api/v1alpha1/nonadminrestore_types.go b/api/v1alpha1/nonadminrestore_types.go new file mode 100644 index 0000000..3542775 --- /dev/null +++ b/api/v1alpha1/nonadminrestore_types.go @@ -0,0 +1,64 @@ +/* +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 v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! +// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + +// NonAdminRestoreSpec defines the desired state of NonAdminRestore +type NonAdminRestoreSpec struct { + // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + // Important: Run "make" to regenerate code after modifying this file + + // Foo is an example field of NonAdminRestore. Edit nonadminrestore_types.go to remove/update + Foo string `json:"foo,omitempty"` +} + +// NonAdminRestoreStatus defines the observed state of NonAdminRestore +type NonAdminRestoreStatus struct { + // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster + // Important: Run "make" to regenerate code after modifying this file +} + +//+kubebuilder:object:root=true +//+kubebuilder:subresource:status + +// NonAdminRestore is the Schema for the nonadminrestores API +type NonAdminRestore struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec NonAdminRestoreSpec `json:"spec,omitempty"` + Status NonAdminRestoreStatus `json:"status,omitempty"` +} + +//+kubebuilder:object:root=true + +// NonAdminRestoreList contains a list of NonAdminRestore +type NonAdminRestoreList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []NonAdminRestore `json:"items"` +} + +func init() { + SchemeBuilder.Register(&NonAdminRestore{}, &NonAdminRestoreList{}) +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 139228d..e2a364c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -142,6 +142,95 @@ func (in *NonAdminBackupStatus) DeepCopy() *NonAdminBackupStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NonAdminRestore) DeepCopyInto(out *NonAdminRestore) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NonAdminRestore. +func (in *NonAdminRestore) DeepCopy() *NonAdminRestore { + if in == nil { + return nil + } + out := new(NonAdminRestore) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *NonAdminRestore) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NonAdminRestoreList) DeepCopyInto(out *NonAdminRestoreList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]NonAdminRestore, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NonAdminRestoreList. +func (in *NonAdminRestoreList) DeepCopy() *NonAdminRestoreList { + if in == nil { + return nil + } + out := new(NonAdminRestoreList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *NonAdminRestoreList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NonAdminRestoreSpec) DeepCopyInto(out *NonAdminRestoreSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NonAdminRestoreSpec. +func (in *NonAdminRestoreSpec) DeepCopy() *NonAdminRestoreSpec { + if in == nil { + return nil + } + out := new(NonAdminRestoreSpec) + in.DeepCopyInto(out) + return out +} + +// 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 +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NonAdminRestoreStatus. +func (in *NonAdminRestoreStatus) DeepCopy() *NonAdminRestoreStatus { + if in == nil { + return nil + } + out := new(NonAdminRestoreStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *QueueInfo) DeepCopyInto(out *QueueInfo) { *out = *in diff --git a/cmd/main.go b/cmd/main.go index 2050685..b065d34 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -156,6 +156,13 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "NonAdminBackup") os.Exit(1) } + if err = (&controller.NonAdminRestoreReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "NonAdminRestore") + os.Exit(1) + } // +kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { diff --git a/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml b/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml new file mode 100644 index 0000000..1704a7e --- /dev/null +++ b/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml @@ -0,0 +1,54 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.14.0 + name: nonadminrestores.oadp.openshift.io +spec: + group: oadp.openshift.io + names: + kind: NonAdminRestore + listKind: NonAdminRestoreList + plural: nonadminrestores + singular: nonadminrestore + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: NonAdminRestore is the Schema for the nonadminrestores API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: NonAdminRestoreSpec defines the desired state of NonAdminRestore + properties: + foo: + description: Foo is an example field of NonAdminRestore. Edit nonadminrestore_types.go + to remove/update + type: string + type: object + status: + description: NonAdminRestoreStatus defines the observed state of NonAdminRestore + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 0430ad8..bc9bc88 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -3,17 +3,20 @@ # It should be run by config/default resources: - bases/oadp.openshift.io_nonadminbackups.yaml +- bases/oadp.openshift.io_nonadminrestores.yaml #+kubebuilder:scaffold:crdkustomizeresource patches: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. # patches here are for enabling the conversion webhook for each CRD #- path: patches/webhook_in_nonadminbackups.yaml +#- path: patches/webhook_in_nonadminrestores.yaml #+kubebuilder:scaffold:crdkustomizewebhookpatch # [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix. # patches here are for enabling the CA injection for each CRD #- path: patches/cainjection_in_nonadminbackups.yaml +#- path: patches/cainjection_in_nonadminrestores.yaml #+kubebuilder:scaffold:crdkustomizecainjectionpatch # [WEBHOOK] To enable webhook, uncomment the following section diff --git a/config/rbac/nonadminrestore_editor_role.yaml b/config/rbac/nonadminrestore_editor_role.yaml new file mode 100644 index 0000000..4515c5b --- /dev/null +++ b/config/rbac/nonadminrestore_editor_role.yaml @@ -0,0 +1,31 @@ +# permissions for end users to edit nonadminrestores. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app.kubernetes.io/name: clusterrole + app.kubernetes.io/instance: nonadminrestore-editor-role + app.kubernetes.io/component: rbac + app.kubernetes.io/created-by: oadp-nac + app.kubernetes.io/part-of: oadp-nac + app.kubernetes.io/managed-by: kustomize + name: nonadminrestore-editor-role +rules: +- apiGroups: + - oadp.openshift.io + resources: + - nonadminrestores + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - oadp.openshift.io + resources: + - nonadminrestores/status + verbs: + - get diff --git a/config/rbac/nonadminrestore_viewer_role.yaml b/config/rbac/nonadminrestore_viewer_role.yaml new file mode 100644 index 0000000..d400c51 --- /dev/null +++ b/config/rbac/nonadminrestore_viewer_role.yaml @@ -0,0 +1,27 @@ +# permissions for end users to view nonadminrestores. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app.kubernetes.io/name: clusterrole + app.kubernetes.io/instance: nonadminrestore-viewer-role + app.kubernetes.io/component: rbac + app.kubernetes.io/created-by: oadp-nac + app.kubernetes.io/part-of: oadp-nac + app.kubernetes.io/managed-by: kustomize + name: nonadminrestore-viewer-role +rules: +- apiGroups: + - oadp.openshift.io + resources: + - nonadminrestores + verbs: + - get + - list + - watch +- apiGroups: + - oadp.openshift.io + resources: + - nonadminrestores/status + verbs: + - get diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 7a9ad1d..42cfeef 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -36,6 +36,32 @@ rules: - get - patch - update +- apiGroups: + - oadp.openshift.io + resources: + - nonadminrestores + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - oadp.openshift.io + resources: + - nonadminrestores/finalizers + verbs: + - update +- apiGroups: + - oadp.openshift.io + resources: + - nonadminrestores/status + verbs: + - get + - patch + - update - apiGroups: - velero.io resources: diff --git a/config/samples/kustomization.yaml b/config/samples/kustomization.yaml index 05ac669..70a255a 100644 --- a/config/samples/kustomization.yaml +++ b/config/samples/kustomization.yaml @@ -1,4 +1,5 @@ ## Append samples of your project ## resources: - oadp_v1alpha1_nonadminbackup.yaml +- oadp_v1alpha1_nonadminrestore.yaml #+kubebuilder:scaffold:manifestskustomizesamples diff --git a/config/samples/oadp_v1alpha1_nonadminrestore.yaml b/config/samples/oadp_v1alpha1_nonadminrestore.yaml new file mode 100644 index 0000000..af5613d --- /dev/null +++ b/config/samples/oadp_v1alpha1_nonadminrestore.yaml @@ -0,0 +1,12 @@ +apiVersion: oadp.openshift.io/v1alpha1 +kind: NonAdminRestore +metadata: + labels: + app.kubernetes.io/name: nonadminrestore + app.kubernetes.io/instance: nonadminrestore-sample + app.kubernetes.io/part-of: oadp-nac + app.kubernetes.io/managed-by: kustomize + app.kubernetes.io/created-by: oadp-nac + name: nonadminrestore-sample +spec: + # TODO(user): Add fields here diff --git a/docs/architecture.md b/docs/architecture.md index ace31e8..d23a9c0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -43,6 +43,12 @@ kubebuilder create api \ --version v1alpha1 \ --kind NonAdminBackup \ --resource --controller +kubebuilder create api \ + --plugins go.kubebuilder.io/v4 \ + --group oadp \ + --version v1alpha1 \ + --kind NonAdminRestore \ + --resource --controller make manifests ``` > **NOTE:** The information about plugin and project version, as well as project name, repo and domain, is stored in [PROJECT](../PROJECT) file diff --git a/internal/controller/nonadminrestore_controller.go b/internal/controller/nonadminrestore_controller.go new file mode 100644 index 0000000..8ee96c9 --- /dev/null +++ b/internal/controller/nonadminrestore_controller.go @@ -0,0 +1,62 @@ +/* +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" + + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + oadpv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" +) + +// NonAdminRestoreReconciler reconciles a NonAdminRestore object +type NonAdminRestoreReconciler struct { + client.Client + Scheme *runtime.Scheme +} + +//+kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores/finalizers,verbs=update + +// Reconcile is part of the main kubernetes reconciliation loop which aims to +// move the current state of the cluster closer to the desired state. +// TODO(user): Modify the Reconcile function to compare the state specified by +// the NonAdminRestore object against the actual cluster state, and then +// perform operations to make the cluster state reflect the state specified by +// the user. +// +// For more details, check Reconcile and its Result here: +// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.17.0/pkg/reconcile +func (r *NonAdminRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + _ = log.FromContext(ctx) + + // TODO(user): your logic here + + return ctrl.Result{}, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *NonAdminRestoreReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&oadpv1alpha1.NonAdminRestore{}). + Complete(r) +} diff --git a/internal/controller/nonadminrestore_controller_test.go b/internal/controller/nonadminrestore_controller_test.go new file mode 100644 index 0000000..e1c78f7 --- /dev/null +++ b/internal/controller/nonadminrestore_controller_test.go @@ -0,0 +1,84 @@ +/* +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" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + oadpv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" +) + +var _ = Describe("NonAdminRestore Controller", func() { + Context("When reconciling a resource", func() { + const resourceName = "test-resource" + + ctx := context.Background() + + typeNamespacedName := types.NamespacedName{ + Name: resourceName, + Namespace: "default", // TODO(user):Modify as needed + } + nonadminrestore := &oadpv1alpha1.NonAdminRestore{} + + BeforeEach(func() { + By("creating the custom resource for the Kind NonAdminRestore") + err := k8sClient.Get(ctx, typeNamespacedName, nonadminrestore) + if err != nil && errors.IsNotFound(err) { + resource := &oadpv1alpha1.NonAdminRestore{ + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName, + Namespace: "default", + }, + // TODO(user): Specify other spec details if needed. + } + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + } + }) + + AfterEach(func() { + // TODO(user): Cleanup logic after each test, like removing the resource instance. + resource := &oadpv1alpha1.NonAdminRestore{} + err := k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + By("Cleanup the specific resource instance NonAdminRestore") + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + }) + It("should successfully reconcile the resource", func() { + By("Reconciling the created resource") + controllerReconciler := &NonAdminRestoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + // TODO(user): Add more specific assertions depending on your controller's reconciliation logic. + // Example: If you expect a certain status condition after reconciliation, verify it here. + }) + }) +}) From e4c4ff330fae597a394140f4a0de404ac7778b72 Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Thu, 14 Nov 2024 09:47:39 -0300 Subject: [PATCH 03/14] fixup! feat: Add NAC restore controller update documentation Signed-off-by: Mateus Oliveira --- README.md | 25 ++++++++++++++++++- .../oadp_v1alpha1_nonadminrestore.yaml | 3 ++- docs/non_admin_user.md | 20 +++++++++++++++ hack/samples/restores/common.yaml | 23 +++++++++++++++++ 4 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 hack/samples/restores/common.yaml diff --git a/README.md b/README.md index 9c66183..30c44d8 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,8 @@ To use NAC functionality: ``` Check the application was successful deployed by accessing its route. + + Create and update items in application UI, to later check if application was successfully restored. - create NonAdminBackup For example, use one of the sample NonAdminBackup available in `hack/samples/backups/` folder, by running @@ -47,7 +49,28 @@ To use NAC functionality: | oc create -f - ``` - - TODO NonAdminRestore + - delete sample application + + For example, delete one of the sample applications available in `hack/samples/apps/` folder, by running + ```sh + oc process -f ./hack/samples/apps/ \ + -p NAMESPACE= \ + | oc delete -f - + ``` + + Check that application was successful deleted by accessing its route. + - create NonAdminRestore + + For example, use one of the sample NonAdminRestore available in `hack/samples/restores/` folder, by running + ```sh + oc process -f ./hack/samples/restores/ \ + -p NAMESPACE= \ + -p NAME= \ + | oc create -f - + ``` + + + After NonAdminRestore completes, check if the application was successful restored by accessing its route and seeing its items in application UI. ## Contributing diff --git a/config/samples/oadp_v1alpha1_nonadminrestore.yaml b/config/samples/oadp_v1alpha1_nonadminrestore.yaml index af5613d..f72499e 100644 --- a/config/samples/oadp_v1alpha1_nonadminrestore.yaml +++ b/config/samples/oadp_v1alpha1_nonadminrestore.yaml @@ -9,4 +9,5 @@ metadata: app.kubernetes.io/created-by: oadp-nac name: nonadminrestore-sample spec: - # TODO(user): Add fields here + restoreSpec: + backupName: nonadminbackup-sample diff --git a/docs/non_admin_user.md b/docs/non_admin_user.md index 2a5a84c..961a6c1 100644 --- a/docs/non_admin_user.md +++ b/docs/non_admin_user.md @@ -40,6 +40,7 @@ Choose one of the authentication method sections to follow. ``` - Ensure non admin user have appropriate permissions in its namespace, i.e., non admin user have editor roles for the following objects - `nonadminbackups.oadp.openshift.io` + - `nonadminrestores.nac.oadp.openshift.io` For example ```yaml @@ -62,6 +63,25 @@ Choose one of the authentication method sections to follow. - nonadminbackups/status verbs: - get + # config/rbac/nonadminrestore_editor_role.yaml + - apiGroups: + - oadp.openshift.io + resources: + - nonadminrestores + verbs: + - create + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - oadp.openshift.io + resources: + - nonadminrestores/status + verbs: + - get ``` For example, make non admin user have `admin` ClusterRole permissions on its namespace ```sh diff --git a/hack/samples/restores/common.yaml b/hack/samples/restores/common.yaml new file mode 100644 index 0000000..168ad5b --- /dev/null +++ b/hack/samples/restores/common.yaml @@ -0,0 +1,23 @@ +apiVersion: template.openshift.io/v1 +kind: Template +metadata: + name: sample-nonadminrestore +objects: + - apiVersion: oadp.openshift.io/v1alpha1 + kind: NonAdminRestore + metadata: + name: nonadminrestore-sample-${SUFFIX} + namespace: ${NAMESPACE} + spec: + restoreSpec: + backupName: ${NAME} +parameters: + - description: NonAdminRestore suffix + from: '[a-z0-9]{8}' + generate: expression + name: SUFFIX + - description: NonAdminRestore namespace + name: NAMESPACE + value: mysql-persistent + - description: NonAdminBackup name + name: NAME From a8f7198874665c8952464abc98ddd90790a77cb7 Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Thu, 14 Nov 2024 16:14:55 -0300 Subject: [PATCH 04/14] fixup! feat: Add NAC restore controller restore logic Signed-off-by: Mateus Oliveira --- api/v1alpha1/nonadminrestore_types.go | 36 +- api/v1alpha1/zz_generated.deepcopy.go | 41 +- .../oadp.openshift.io_nonadminrestores.yaml | 616 +++++++++++++++++- config/rbac/role.yaml | 12 + hack/extra-crds/velero.io_restores.yaml | 557 ++++++++++++++++ internal/common/constant/constant.go | 21 +- internal/common/function/function.go | 59 ++ .../controller/nonadminbackup_controller.go | 14 +- .../controller/nonadminrestore_controller.go | 303 ++++++++- .../nonadminrestore_controller_test.go | 123 +++- internal/handler/velerorestore_handler.go | 63 ++ ...dicate.go => compositebackup_predicate.go} | 12 +- .../predicate/compositerestore_predicate.go | 70 ++ .../predicate/nonadminrestore_predicate.go | 57 ++ internal/predicate/velerorestore_predicate.go | 48 ++ 15 files changed, 1959 insertions(+), 73 deletions(-) create mode 100644 hack/extra-crds/velero.io_restores.yaml create mode 100644 internal/handler/velerorestore_handler.go rename internal/predicate/{composite_predicate.go => compositebackup_predicate.go} (82%) create mode 100644 internal/predicate/compositerestore_predicate.go create mode 100644 internal/predicate/nonadminrestore_predicate.go create mode 100644 internal/predicate/velerorestore_predicate.go diff --git a/api/v1alpha1/nonadminrestore_types.go b/api/v1alpha1/nonadminrestore_types.go index 3542775..425e475 100644 --- a/api/v1alpha1/nonadminrestore_types.go +++ b/api/v1alpha1/nonadminrestore_types.go @@ -17,25 +17,43 @@ limitations under the License. package v1alpha1 import ( + velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! -// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. - // NonAdminRestoreSpec defines the desired state of NonAdminRestore type NonAdminRestoreSpec struct { - // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster - // Important: Run "make" to regenerate code after modifying this file + // restoreSpec defines the specification for a Velero restore. + RestoreSpec *velerov1.RestoreSpec `json:"restoreSpec"` +} + +// VeleroRestore contains information of the related Velero restore object. +type VeleroRestore struct { + // status captures the current status of the Velero restore. + // +optional + Status *velerov1.RestoreStatus `json:"status,omitempty"` + + // references the Velero Restore object by it's name. + // +optional + Name string `json:"name,omitempty"` - // Foo is an example field of NonAdminRestore. Edit nonadminrestore_types.go to remove/update - Foo string `json:"foo,omitempty"` + // namespace references the Namespace in which Velero Restore exists. + // +optional + Namespace string `json:"namespace,omitempty"` } // NonAdminRestoreStatus defines the observed state of NonAdminRestore type NonAdminRestoreStatus struct { - // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster - // Important: Run "make" to regenerate code after modifying this file + // +optional + VeleroRestore *VeleroRestore `json:"veleroRestore,omitempty"` + + // +optional + UUID string `json:"uuid,omitempty"` + + // phase is a simple one high-level summary of the lifecycle of an NonAdminRestore. + Phase NonAdminPhase `json:"phase,omitempty"` + + Conditions []metav1.Condition `json:"conditions,omitempty"` } //+kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index e2a364c..70d86aa 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -147,8 +147,8 @@ func (in *NonAdminRestore) DeepCopyInto(out *NonAdminRestore) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec - out.Status = in.Status + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NonAdminRestore. @@ -204,6 +204,11 @@ func (in *NonAdminRestoreList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NonAdminRestoreSpec) DeepCopyInto(out *NonAdminRestoreSpec) { *out = *in + if in.RestoreSpec != nil { + in, out := &in.RestoreSpec, &out.RestoreSpec + *out = new(v1.RestoreSpec) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NonAdminRestoreSpec. @@ -219,6 +224,18 @@ 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.VeleroRestore != nil { + in, out := &in.VeleroRestore, &out.VeleroRestore + *out = new(VeleroRestore) + (*in).DeepCopyInto(*out) + } + 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]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NonAdminRestoreStatus. @@ -285,3 +302,23 @@ func (in *VeleroDeleteBackupRequest) DeepCopy() *VeleroDeleteBackupRequest { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VeleroRestore) DeepCopyInto(out *VeleroRestore) { + *out = *in + if in.Status != nil { + in, out := &in.Status, &out.Status + *out = new(v1.RestoreStatus) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VeleroRestore. +func (in *VeleroRestore) DeepCopy() *VeleroRestore { + if in == nil { + return nil + } + out := new(VeleroRestore) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml b/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml index 1704a7e..72dbadf 100644 --- a/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml +++ b/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml @@ -39,13 +39,621 @@ spec: spec: description: NonAdminRestoreSpec defines the desired state of NonAdminRestore properties: - foo: - description: Foo is an example field of NonAdminRestore. Edit nonadminrestore_types.go - to remove/update - type: string + restoreSpec: + description: restoreSpec defines the specification for a Velero restore. + properties: + backupName: + description: |- + BackupName is the unique name of the Velero backup to restore + from. + type: string + excludedNamespaces: + description: |- + ExcludedNamespaces contains a list of namespaces that are not + included in the restore. + items: + type: string + nullable: true + type: array + excludedResources: + description: |- + ExcludedResources is a slice of resource names that are not + included in the restore. + items: + type: string + nullable: true + type: array + existingResourcePolicy: + description: ExistingResourcePolicy specifies the restore behavior + for the Kubernetes resource to be restored + nullable: true + type: string + hooks: + description: Hooks represent custom behaviors that should be executed + during or post restore. + properties: + resources: + items: + description: |- + RestoreResourceHookSpec defines one or more RestoreResrouceHooks that should be executed based on + the rules defined for namespaces, resources, and label selector. + properties: + excludedNamespaces: + description: ExcludedNamespaces specifies the namespaces + to which this hook spec does not apply. + items: + type: string + nullable: true + type: array + excludedResources: + description: ExcludedResources specifies the resources + to which this hook spec does not apply. + items: + type: string + nullable: true + type: array + includedNamespaces: + description: |- + IncludedNamespaces specifies the namespaces to which this hook spec applies. If empty, it applies + to all namespaces. + items: + type: string + nullable: true + type: array + includedResources: + description: |- + IncludedResources specifies the resources to which this hook spec applies. If empty, it applies + to all resources. + items: + type: string + nullable: true + type: array + labelSelector: + description: LabelSelector, if specified, filters the + resources to which this hook spec applies. + nullable: true + properties: + matchExpressions: + description: matchExpressions is a list of label + selector requirements. The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the + selector applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + name: + description: Name is the name of this hook. + type: string + postHooks: + description: PostHooks is a list of RestoreResourceHooks + to execute during and after restoring a resource. + items: + description: RestoreResourceHook defines a restore + hook for a resource. + properties: + exec: + description: Exec defines an exec restore hook. + properties: + command: + description: Command is the command and arguments + to execute from within a container after + a pod has been restored. + items: + type: string + minItems: 1 + type: array + container: + description: |- + Container is the container in the pod where the command should be executed. If not specified, + the pod's first container is used. + type: string + execTimeout: + description: |- + ExecTimeout defines the maximum amount of time Velero should wait for the hook to complete before + considering the execution a failure. + type: string + onError: + description: OnError specifies how Velero + should behave if it encounters an error + executing this hook. + enum: + - Continue + - Fail + type: string + waitForReady: + description: WaitForReady ensures command + will be launched when container is Ready + instead of Running. + nullable: true + type: boolean + waitTimeout: + description: |- + WaitTimeout defines the maximum amount of time Velero should wait for the container to be Ready + before attempting to run the command. + type: string + required: + - command + type: object + init: + description: Init defines an init restore hook. + properties: + initContainers: + description: InitContainers is list of init + containers to be added to a pod during its + restore. + items: + type: object + x-kubernetes-preserve-unknown-fields: true + type: array + x-kubernetes-preserve-unknown-fields: true + timeout: + description: Timeout defines the maximum amount + of time Velero should wait for the initContainers + to complete. + type: string + type: object + type: object + type: array + required: + - name + type: object + type: array + type: object + includeClusterResources: + description: |- + IncludeClusterResources specifies whether cluster-scoped resources + should be included for consideration in the restore. If null, defaults + to true. + nullable: true + type: boolean + includedNamespaces: + description: |- + IncludedNamespaces is a slice of namespace names to include objects + from. If empty, all namespaces are included. + items: + type: string + nullable: true + type: array + includedResources: + description: |- + IncludedResources is a slice of resource names to include + in the restore. If empty, all resources in the backup are included. + items: + type: string + nullable: true + type: array + itemOperationTimeout: + description: |- + ItemOperationTimeout specifies the time used to wait for RestoreItemAction operations + The default value is 4 hour. + type: string + labelSelector: + description: |- + LabelSelector is a metav1.LabelSelector to filter with + when restoring individual objects from the backup. If empty + or nil, all objects are included. Optional. + nullable: true + properties: + matchExpressions: + description: matchExpressions is a list of label selector + requirements. The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the selector + applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + namespaceMapping: + additionalProperties: + type: string + description: |- + NamespaceMapping is a map of source namespace names + to target namespace names to restore into. Any source + namespaces not included in the map will be restored into + namespaces of the same name. + type: object + orLabelSelectors: + description: |- + OrLabelSelectors is list of metav1.LabelSelector to filter with + when restoring individual objects from the backup. If multiple provided + they will be joined by the OR operator. LabelSelector as well as + OrLabelSelectors cannot co-exist in restore request, only one of them + can be used + items: + description: |- + A label selector is a label query over a set of resources. The result of matchLabels and + matchExpressions are ANDed. An empty label selector matches all objects. A null + label selector matches no objects. + properties: + matchExpressions: + description: matchExpressions is a list of label selector + requirements. The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the selector + applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + nullable: true + type: array + preserveNodePorts: + description: PreserveNodePorts specifies whether to restore old + nodePorts from backup. + nullable: true + type: boolean + resourceModifier: + description: ResourceModifier specifies the reference to JSON + resource patches that should be applied to resources before + restoration. + nullable: true + properties: + apiGroup: + description: |- + APIGroup is the group for the resource being referenced. + If APIGroup is not specified, the specified Kind must be in the core API group. + For any other third-party types, APIGroup is required. + type: string + kind: + description: Kind is the type of resource being referenced + type: string + name: + description: Name is the name of resource being referenced + type: string + required: + - kind + - name + type: object + x-kubernetes-map-type: atomic + restorePVs: + description: |- + RestorePVs specifies whether to restore all included + PVs from snapshot + nullable: true + type: boolean + restoreStatus: + description: |- + RestoreStatus specifies which resources we should restore the status + field. If nil, no objects are included. Optional. + nullable: true + properties: + excludedResources: + description: ExcludedResources specifies the resources to + which will not restore the status. + items: + type: string + nullable: true + type: array + includedResources: + description: |- + IncludedResources specifies the resources to which will restore the status. + If empty, it applies to all resources. + items: + type: string + nullable: true + type: array + type: object + scheduleName: + description: |- + ScheduleName is the unique name of the Velero schedule to restore + from. If specified, and BackupName is empty, Velero will restore + from the most recent successful backup created from this schedule. + type: string + uploaderConfig: + description: UploaderConfig specifies the configuration for the + restore. + nullable: true + properties: + parallelFilesDownload: + description: ParallelFilesDownload is the concurrency number + setting for restore. + type: integer + writeSparseFiles: + description: WriteSparseFiles is a flag to indicate whether + write files sparsely or not. + nullable: true + type: boolean + type: object + type: object + required: + - restoreSpec type: object status: description: NonAdminRestoreStatus defines the observed state of NonAdminRestore + properties: + 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: phase is a simple one high-level summary of the lifecycle + of an NonAdminRestore. + enum: + - New + - BackingOff + - Created + - Deleting + type: string + uuid: + type: string + veleroRestore: + description: VeleroRestore contains information of the related Velero + restore object. + properties: + name: + description: references the Velero Restore object by it's name. + type: string + namespace: + description: namespace references the Namespace in which Velero + Restore exists. + type: string + status: + description: status captures the current status of the Velero + restore. + 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 + 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 Restore + enum: + - New + - FailedValidation + - InProgress + - WaitingForPluginOperations + - WaitingForPluginOperationsPartiallyFailed + - Completed + - PartiallyFailed + - Failed + - Finalizing + - FinalizingPartiallyFailed + 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 type: object served: true diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 42cfeef..66c478f 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -86,3 +86,15 @@ rules: - patch - update - watch +- apiGroups: + - velero.io + resources: + - restores + verbs: + - create + - delete + - get + - list + - patch + - update + - watch diff --git a/hack/extra-crds/velero.io_restores.yaml b/hack/extra-crds/velero.io_restores.yaml new file mode 100644 index 0000000..6226a9f --- /dev/null +++ b/hack/extra-crds/velero.io_restores.yaml @@ -0,0 +1,557 @@ +# Code generated by make update-velero-manifests. DO NOT EDIT. +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.14.0 + name: restores.velero.io +spec: + group: velero.io + names: + kind: Restore + listKind: RestoreList + plural: restores + singular: restore + scope: Namespaced + versions: + - name: v1 + schema: + openAPIV3Schema: + description: |- + Restore is a Velero resource that represents the application of + resources from a Velero backup to a target Kubernetes cluster. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: RestoreSpec defines the specification for a Velero restore. + properties: + backupName: + description: |- + BackupName is the unique name of the Velero backup to restore + from. + type: string + excludedNamespaces: + description: |- + ExcludedNamespaces contains a list of namespaces that are not + included in the restore. + items: + type: string + nullable: true + type: array + excludedResources: + description: |- + ExcludedResources is a slice of resource names that are not + included in the restore. + items: + type: string + nullable: true + type: array + existingResourcePolicy: + description: ExistingResourcePolicy specifies the restore behavior + for the Kubernetes resource to be restored + nullable: true + type: string + hooks: + description: Hooks represent custom behaviors that should be executed + during or post restore. + properties: + resources: + items: + description: |- + RestoreResourceHookSpec defines one or more RestoreResrouceHooks that should be executed based on + the rules defined for namespaces, resources, and label selector. + properties: + excludedNamespaces: + description: ExcludedNamespaces specifies the namespaces + to which this hook spec does not apply. + items: + type: string + nullable: true + type: array + excludedResources: + description: ExcludedResources specifies the resources to + which this hook spec does not apply. + items: + type: string + nullable: true + type: array + includedNamespaces: + description: |- + IncludedNamespaces specifies the namespaces to which this hook spec applies. If empty, it applies + to all namespaces. + items: + type: string + nullable: true + type: array + includedResources: + description: |- + IncludedResources specifies the resources to which this hook spec applies. If empty, it applies + to all resources. + items: + type: string + nullable: true + type: array + labelSelector: + description: LabelSelector, if specified, filters the resources + to which this hook spec applies. + nullable: true + properties: + matchExpressions: + description: matchExpressions is a list of label selector + requirements. The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the selector + applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + name: + description: Name is the name of this hook. + type: string + postHooks: + description: PostHooks is a list of RestoreResourceHooks + to execute during and after restoring a resource. + items: + description: RestoreResourceHook defines a restore hook + for a resource. + properties: + exec: + description: Exec defines an exec restore hook. + properties: + command: + description: Command is the command and arguments + to execute from within a container after a pod + has been restored. + items: + type: string + minItems: 1 + type: array + container: + description: |- + Container is the container in the pod where the command should be executed. If not specified, + the pod's first container is used. + type: string + execTimeout: + description: |- + ExecTimeout defines the maximum amount of time Velero should wait for the hook to complete before + considering the execution a failure. + type: string + onError: + description: OnError specifies how Velero should + behave if it encounters an error executing this + hook. + enum: + - Continue + - Fail + type: string + waitForReady: + description: WaitForReady ensures command will + be launched when container is Ready instead + of Running. + nullable: true + type: boolean + waitTimeout: + description: |- + WaitTimeout defines the maximum amount of time Velero should wait for the container to be Ready + before attempting to run the command. + type: string + required: + - command + type: object + init: + description: Init defines an init restore hook. + properties: + initContainers: + description: InitContainers is list of init containers + to be added to a pod during its restore. + items: + type: object + x-kubernetes-preserve-unknown-fields: true + type: array + x-kubernetes-preserve-unknown-fields: true + timeout: + description: Timeout defines the maximum amount + of time Velero should wait for the initContainers + to complete. + type: string + type: object + type: object + type: array + required: + - name + type: object + type: array + type: object + includeClusterResources: + description: |- + IncludeClusterResources specifies whether cluster-scoped resources + should be included for consideration in the restore. If null, defaults + to true. + nullable: true + type: boolean + includedNamespaces: + description: |- + IncludedNamespaces is a slice of namespace names to include objects + from. If empty, all namespaces are included. + items: + type: string + nullable: true + type: array + includedResources: + description: |- + IncludedResources is a slice of resource names to include + in the restore. If empty, all resources in the backup are included. + items: + type: string + nullable: true + type: array + itemOperationTimeout: + description: |- + ItemOperationTimeout specifies the time used to wait for RestoreItemAction operations + The default value is 4 hour. + type: string + labelSelector: + description: |- + LabelSelector is a metav1.LabelSelector to filter with + when restoring individual objects from the backup. If empty + or nil, all objects are included. Optional. + nullable: true + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. + The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the selector applies + to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + namespaceMapping: + additionalProperties: + type: string + description: |- + NamespaceMapping is a map of source namespace names + to target namespace names to restore into. Any source + namespaces not included in the map will be restored into + namespaces of the same name. + type: object + orLabelSelectors: + description: |- + OrLabelSelectors is list of metav1.LabelSelector to filter with + when restoring individual objects from the backup. If multiple provided + they will be joined by the OR operator. LabelSelector as well as + OrLabelSelectors cannot co-exist in restore request, only one of them + can be used + items: + description: |- + A label selector is a label query over a set of resources. The result of matchLabels and + matchExpressions are ANDed. An empty label selector matches all objects. A null + label selector matches no objects. + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. + The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the selector applies + to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + nullable: true + type: array + preserveNodePorts: + description: PreserveNodePorts specifies whether to restore old nodePorts + from backup. + nullable: true + type: boolean + resourceModifier: + description: ResourceModifier specifies the reference to JSON resource + patches that should be applied to resources before restoration. + nullable: true + properties: + apiGroup: + description: |- + APIGroup is the group for the resource being referenced. + If APIGroup is not specified, the specified Kind must be in the core API group. + For any other third-party types, APIGroup is required. + type: string + kind: + description: Kind is the type of resource being referenced + type: string + name: + description: Name is the name of resource being referenced + type: string + required: + - kind + - name + type: object + x-kubernetes-map-type: atomic + restorePVs: + description: |- + RestorePVs specifies whether to restore all included + PVs from snapshot + nullable: true + type: boolean + restoreStatus: + description: |- + RestoreStatus specifies which resources we should restore the status + field. If nil, no objects are included. Optional. + nullable: true + properties: + excludedResources: + description: ExcludedResources specifies the resources to which + will not restore the status. + items: + type: string + nullable: true + type: array + includedResources: + description: |- + IncludedResources specifies the resources to which will restore the status. + If empty, it applies to all resources. + items: + type: string + nullable: true + type: array + type: object + scheduleName: + description: |- + ScheduleName is the unique name of the Velero schedule to restore + from. If specified, and BackupName is empty, Velero will restore + from the most recent successful backup created from this schedule. + type: string + uploaderConfig: + description: UploaderConfig specifies the configuration for the restore. + nullable: true + properties: + parallelFilesDownload: + description: ParallelFilesDownload is the concurrency number setting + for restore. + type: integer + writeSparseFiles: + description: WriteSparseFiles is a flag to indicate whether write + files sparsely or not. + nullable: true + type: boolean + type: object + type: object + status: + description: RestoreStatus captures the current status of a Velero restore + 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 + 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 Restore + enum: + - New + - FailedValidation + - InProgress + - WaitingForPluginOperations + - WaitingForPluginOperationsPartiallyFailed + - Completed + - PartiallyFailed + - Failed + - Finalizing + - FinalizingPartiallyFailed + 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 + served: true + storage: true diff --git a/internal/common/constant/constant.go b/internal/common/constant/constant.go index aa408e2..91efef1 100644 --- a/internal/common/constant/constant.go +++ b/internal/common/constant/constant.go @@ -24,15 +24,20 @@ import "k8s.io/apimachinery/pkg/util/validation" // Annotations on the other hand should be used to define ownership // of the specific Object, such as Backup/Restore. const ( - OadpLabel = "openshift.io/oadp" // TODO import? - OadpLabelValue = TrueString - ManagedByLabel = "app.kubernetes.io/managed-by" - ManagedByLabelValue = "oadp-nac-controller" // TODO why not use same project name as in PROJECT file? - NabOriginNameAnnotation = "openshift.io/oadp-nab-origin-name" - NabOriginNamespaceAnnotation = "openshift.io/oadp-nab-origin-namespace" - NabOriginNACUUIDLabel = "openshift.io/oadp-nab-origin-nacuuid" - NarOriginNACUUIDLabel = "openshift.io/oadp-nar-origin-nacuuid" + OadpLabel = "openshift.io/oadp" // TODO import? + OadpLabelValue = TrueString + ManagedByLabel = "app.kubernetes.io/managed-by" + ManagedByLabelValue = "oadp-nac-controller" // TODO why not use same project name as in PROJECT file? + NabOriginNACUUIDLabel = "openshift.io/oadp-nab-origin-nacuuid" + NarOriginNACUUIDLabel = "openshift.io/oadp-nar-origin-nacuuid" + + NabOriginNameAnnotation = "openshift.io/oadp-nab-origin-name" + NabOriginNamespaceAnnotation = "openshift.io/oadp-nab-origin-namespace" + NonAdminRestoreOriginNameAnnotation = "openshift.io/oadp-nar-origin-name" + NonAdminRestoreOriginNamespaceAnnotation = "openshift.io/oadp-nar-origin-namespace" + NabFinalizerName = "nonadminbackup.oadp.openshift.io/finalizer" + NonAdminRestoreFinalizerName = "nonadminrestore.oadp.openshift.io/finalizer" ) // Common environment variables for the Non Admin Controller diff --git a/internal/common/function/function.go b/internal/common/function/function.go index 09228c3..b4b5981 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -26,6 +26,7 @@ import ( "github.com/go-logr/logr" "github.com/google/uuid" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" @@ -45,6 +46,12 @@ func GetNonAdminLabels() map[string]string { } } +func GetNonAdminRestoreLabels(uuid string) map[string]string { + nonAdminLabels := GetNonAdminLabels() + nonAdminLabels[constant.NarOriginNACUUIDLabel] = uuid + return nonAdminLabels +} + // GetNonAdminBackupAnnotations return the required Non Admin annotations func GetNonAdminBackupAnnotations(objectMeta metav1.ObjectMeta) map[string]string { return map[string]string{ @@ -53,6 +60,13 @@ func GetNonAdminBackupAnnotations(objectMeta metav1.ObjectMeta) map[string]strin } } +func GetNonAdminRestoreAnnotations(objectMeta metav1.ObjectMeta) map[string]string { + return map[string]string{ + constant.NonAdminRestoreOriginNamespaceAnnotation: objectMeta.Namespace, + constant.NonAdminRestoreOriginNameAnnotation: objectMeta.Name, + } +} + // containsOnlyNamespace checks if the given namespaces slice contains only the specified namespace func containsOnlyNamespace(namespaces []string, namespace string) bool { for _, ns := range namespaces { @@ -99,6 +113,22 @@ func ValidateBackupSpec(nonAdminBackup *nacv1alpha1.NonAdminBackup, enforcedBack return nil } +func ValidateRestoreSpec(nonAdminRestore *nacv1alpha1.NonAdminRestore) error { + // TODO check NonAdminBackup with nonAdminRestore.Spec.RestoreSpec.BackupName name exist in NonAdminRestore namespace + + // TODO validate that nonAdminRestore.Spec.RestoreSpec.ScheduleName is not used? (we do not plan to have schedules now, right?) + + // TODO validate that nonAdminRestore.Spec.RestoreSpec.IncludedNamespaces does not contain anything other than its own namespace? + + // TODO validate that nonAdminRestore.Spec.RestoreSpec.ExcludedNamespaces is not set? (to avoid empty restores) + + // TODO nonAdminRestore.Spec.RestoreSpec.NamespaceMapping ? + + // TODO enforce Restore Spec + + return nil +} + // GenerateNacObjectUUID generates a unique name based on the provided namespace and object origin name. // It includes a UUID suffix. If the name exceeds the maximum length, it truncates nacName first, then namespace. func GenerateNacObjectUUID(namespace, nacName string) string { @@ -141,6 +171,19 @@ func GenerateNacObjectUUID(namespace, nacName string) string { return nacObjectName } +func GetGenerateNamePrefix(namespace string, name string) string { + remainingLength := constant.MaximumNacObjectNameLength - 5 - len("--") + if len(namespace+name) > remainingLength { + if len(namespace) >= remainingLength { + return fmt.Sprintf("%s-", namespace[:remainingLength]) + } + remainingLength := remainingLength - len(namespace) + return fmt.Sprintf("%s-%s-", name[:remainingLength], namespace) + } + + return fmt.Sprintf("%s-%s-", namespace, name) +} + // ListObjectsByLabel retrieves a list of Kubernetes objects in a specified namespace // that match a given label key-value pair. func ListObjectsByLabel(ctx context.Context, clientInstance client.Client, namespace string, labelKey string, labelValue string, objectList client.ObjectList) error { @@ -277,6 +320,22 @@ func GetVeleroDeleteBackupRequestByLabel(ctx context.Context, clientInstance cli } } +func GetVeleroRestoreByLabel(ctx context.Context, clientInstance client.Client, namespace string, labelValue string) (*velerov1.Restore, error) { + veleroRestoreList := &velerov1.RestoreList{} + if err := ListObjectsByLabel(ctx, clientInstance, namespace, constant.NarOriginNACUUIDLabel, labelValue, veleroRestoreList); err != nil { + return nil, err + } + + switch len(veleroRestoreList.Items) { + case 0: + return nil, apierrors.NewNotFound(velerov1.Resource("restores"), "") + case 1: + return &veleroRestoreList.Items[0], nil + default: + return nil, fmt.Errorf("multiple Velero Restore objects found with label %s=%s in namespace '%s'", constant.NabOriginNACUUIDLabel, labelValue, namespace) + } +} + // CheckVeleroBackupMetadata return true if Velero Backup object has required Non Admin labels and annotations, false otherwise func CheckVeleroBackupMetadata(obj client.Object) bool { objLabels := obj.GetLabels() diff --git a/internal/controller/nonadminbackup_controller.go b/internal/controller/nonadminbackup_controller.go index e3eeae1..ef63ea5 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -53,7 +53,7 @@ type NonAdminBackupReconciler struct { OADPNamespace string } -type reconcileStepFunction func(ctx context.Context, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup) (bool, error) +type nonAdminBackupReconcileStepFunction func(ctx context.Context, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup) (bool, error) const ( veleroReferenceUpdated = "NonAdminBackup - Status Updated with UUID reference" @@ -92,14 +92,14 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque } // Determine which path to take - var reconcileSteps []reconcileStepFunction + var reconcileSteps []nonAdminBackupReconcileStepFunction // First switch statement takes precedence over the next one switch { case nab.Spec.ForceDeleteBackup: // Force delete path - immediately removes both VeleroBackup and DeleteBackupRequest logger.V(1).Info("Executing force delete path") - reconcileSteps = []reconcileStepFunction{ + reconcileSteps = []nonAdminBackupReconcileStepFunction{ r.setStatusAndConditionForDeletionAndCallDelete, r.deleteVeleroBackupAndDeleteBackupRequestObjects, r.removeNabFinalizerUponVeleroBackupDeletion, @@ -108,7 +108,7 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque case nab.Spec.DeleteBackup: // Standard delete path - creates DeleteBackupRequest and waits for VeleroBackup deletion logger.V(1).Info("Executing standard delete path") - reconcileSteps = []reconcileStepFunction{ + reconcileSteps = []nonAdminBackupReconcileStepFunction{ r.setStatusAndConditionForDeletionAndCallDelete, r.createVeleroDeleteBackupRequest, r.removeNabFinalizerUponVeleroBackupDeletion, @@ -119,14 +119,14 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque // Initializes deletion of the NonAdminBackup object without removing // dependent VeleroBackup object logger.V(1).Info("Executing direct deletion path") - reconcileSteps = []reconcileStepFunction{ + reconcileSteps = []nonAdminBackupReconcileStepFunction{ r.setStatusForDirectKubernetesAPIDeletion, } default: // Standard creation/update path logger.V(1).Info("Executing nab creation/update path") - reconcileSteps = []reconcileStepFunction{ + reconcileSteps = []nonAdminBackupReconcileStepFunction{ r.initNabCreate, r.validateSpec, r.setBackupUUIDInStatus, @@ -688,7 +688,7 @@ func (r *NonAdminBackupReconciler) createVeleroBackupAndSyncWithNonAdminBackup(c func (r *NonAdminBackupReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&nacv1alpha1.NonAdminBackup{}). - WithEventFilter(predicate.CompositePredicate{ + WithEventFilter(predicate.CompositeBackupPredicate{ NonAdminBackupPredicate: predicate.NonAdminBackupPredicate{}, VeleroBackupQueuePredicate: predicate.VeleroBackupQueuePredicate{ OADPNamespace: r.OADPNamespace, diff --git a/internal/controller/nonadminrestore_controller.go b/internal/controller/nonadminrestore_controller.go index 8ee96c9..10f7a27 100644 --- a/internal/controller/nonadminrestore_controller.go +++ b/internal/controller/nonadminrestore_controller.go @@ -18,45 +18,322 @@ package controller import ( "context" + "fmt" + "reflect" + "github.com/go-logr/logr" + "github.com/google/uuid" + velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" - oadpv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" + 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" ) // NonAdminRestoreReconciler reconciles a NonAdminRestore object type NonAdminRestoreReconciler struct { client.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + OADPNamespace string } +type nonAdminRestoreReconcileStepFunction func(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) + //+kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores/status,verbs=get;update;patch //+kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores/finalizers,verbs=update +// +kubebuilder:rbac:groups=velero.io,resources=restores,verbs=get;list;watch;create;update;patch;delete + // Reconcile is part of the main kubernetes reconciliation loop which aims to -// move the current state of the cluster closer to the desired state. -// TODO(user): Modify the Reconcile function to compare the state specified by -// the NonAdminRestore object against the actual cluster state, and then -// perform operations to make the cluster state reflect the state specified by -// the user. -// -// For more details, check Reconcile and its Result here: -// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.17.0/pkg/reconcile +// move the current state of the cluster closer to the desired state, +// defined in NonAdminRestore object Spec. func (r *NonAdminRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - _ = log.FromContext(ctx) + logger := log.FromContext(ctx) + logger.V(1).Info("NonAdminRestore Reconcile start") + + nar := &nacv1alpha1.NonAdminRestore{} + err := r.Get(ctx, req.NamespacedName, nar) + if err != nil { + if apierrors.IsNotFound(err) { + logger.V(1).Info(err.Error()) + return ctrl.Result{}, nil + } + logger.Error(err, "Unable to fetch NonAdminRestore") + return ctrl.Result{}, err + } - // TODO(user): your logic here + if !nar.DeletionTimestamp.IsZero() { + updatedPhase := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseDeleting) + updatedCondition := meta.SetStatusCondition(&nar.Status.Conditions, + metav1.Condition{ + Type: string(constant.NonAdminConditionDeleting), + Status: metav1.ConditionTrue, + Reason: "DeletionPending", + Message: "restore accepted for deletion", + }, + ) + if updatedPhase || updatedCondition { + if err := r.Status().Update(ctx, nar); err != nil { + logger.Error(err, statusUpdateError) + return ctrl.Result{}, err + } + logger.V(1).Info("NonAdminRestore status marked for deletion") + } else { + logger.V(1).Info("NonAdminRestore status unchanged during deletion") + } + veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, nar.Status.UUID) + if err != nil { + if !apierrors.IsNotFound(err) { + logger.Error(err, "Unable to fetch Velero Restore") + return ctrl.Result{}, err + } + } else { + // TODO when/why Velero Restore has finalizer? + // TODO how to safely remove Velero Restore with finalizer? + for _, finalizer := range veleroRestore.GetFinalizers() { + controllerutil.RemoveFinalizer(veleroRestore, finalizer) + } + // TODO does this change generation? need to refetch? + if err := r.Update(ctx, veleroRestore); err != nil { + return ctrl.Result{}, err + } + err = r.Delete(ctx, veleroRestore) + if err != nil { + logger.Error(err, "Unable to delete Velero Restore") + return ctrl.Result{}, err + } + } + + controllerutil.RemoveFinalizer(nar, constant.NonAdminRestoreFinalizerName) + // TODO does this change generation? need to refetch? + if err := r.Update(ctx, nar); err != nil { + return ctrl.Result{}, err + } + logger.V(1).Info("NonAdminRestore Reconcile exit") + return ctrl.Result{}, nil + } + + reconcileSteps := []nonAdminRestoreReconcileStepFunction{ + r.init, + r.validateSpec, + r.setUUID, + r.setFinalizer, + r.createVeleroRestore, + } + for _, step := range reconcileSteps { + requeue, err := step(ctx, logger, nar) + if err != nil { + return ctrl.Result{}, err + } else if requeue { + // TODO needed? + return ctrl.Result{Requeue: true}, nil + } + } + logger.V(1).Info("NonAdminRestore Reconcile exit") return ctrl.Result{}, nil } +func (r *NonAdminRestoreReconciler) init(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { + if nar.Status.Phase == constant.EmptyString { + if updated := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseNew); updated { + if err := r.Status().Update(ctx, nar); err != nil { + logger.Error(err, statusUpdateError) + return false, err + } + logger.V(1).Info("NonAdminRestore Phase set to New") + } + } else { + logger.V(1).Info("NonAdminRestore Phase already initialized") + } + return false, nil +} + +func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { + err := function.ValidateRestoreSpec(nar) + if err != nil { + updatedPhase := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseBackingOff) + updatedCondition := meta.SetStatusCondition(&nar.Status.Conditions, + metav1.Condition{ + Type: string(constant.NonAdminConditionAccepted), + Status: metav1.ConditionFalse, + Reason: "InvalidRestoreSpec", + Message: err.Error(), + }, + ) + if updatedPhase || updatedCondition { + if updateErr := r.Status().Update(ctx, nar); updateErr != nil { + logger.Error(updateErr, statusUpdateError) + return false, updateErr + } + } + return false, reconcile.TerminalError(err) + } + logger.V(1).Info("NonAdminRestore Spec validated") + + updated := meta.SetStatusCondition(&nar.Status.Conditions, + metav1.Condition{ + Type: string(constant.NonAdminConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "RestoreAccepted", + Message: "restore accepted", + }, + ) + if updated { + if err := r.Status().Update(ctx, nar); err != nil { + logger.Error(err, statusUpdateError) + return false, err + } + logger.V(1).Info("NonAdminRestore condition set to Accepted") + } + return false, nil +} + +func (r *NonAdminRestoreReconciler) setUUID(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { + if nar.Status.UUID == constant.EmptyString { + // TODO handle panic + nar.Status.UUID = uuid.New().String() + if err := r.Status().Update(ctx, nar); err != nil { + logger.Error(err, statusUpdateError) + return false, err + } + logger.V(1).Info(veleroReferenceUpdated) + } else { + logger.V(1).Info("NonAdminRestore already contains NAC UUID") + } + return false, nil +} + +func (r *NonAdminRestoreReconciler) setFinalizer(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { + added := controllerutil.AddFinalizer(nar, constant.NonAdminRestoreFinalizerName) + if added { + if err := r.Update(ctx, nar); err != nil { + logger.Error(err, "Failed to add finalizer") + return false, err + } + // TODO does this change generation? need to refetch? + logger.V(1).Info("Finalizer added to NonAdminRestore", "finalizer", constant.NonAdminRestoreFinalizerName) + } else { + logger.V(1).Info("Finalizer already exists on NonAdminRestore", "finalizer", constant.NonAdminRestoreFinalizerName) + } + return false, nil +} + +func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { + nacUUID := nar.Status.UUID + veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, nacUUID) + if err != nil { + if !apierrors.IsNotFound(err) { + return false, err + } + + // TODO how to add this to validation without doing GET call for NAB/Velero backup twice? add field in status? + restoreSpec := nar.Spec.RestoreSpec.DeepCopy() + nab := &nacv1alpha1.NonAdminBackup{} + err = r.Get(ctx, types.NamespacedName{Namespace: nar.Namespace, Name: nar.Spec.RestoreSpec.BackupName}, nab) + if err != nil { + return false, err + } + // TODO better way to check readiness? simplify and ask user to pass velero backup name? (user has access to this info in nonAdminBackup status) + if nab.Status.Phase != nacv1alpha1.NonAdminPhaseCreated { + return false, fmt.Errorf("NonAdminBackup is not ready to use for restore") + } + veleroBackup, err := function.GetVeleroBackupByLabel(ctx, r.Client, r.OADPNamespace, nab.Status.VeleroBackup.NACUUID) + if err != nil || veleroBackup == nil { + return false, fmt.Errorf("velero Backup does not exist or multiple exist") + } + // TODO does velero validate if backup is ready to be restored? + restoreSpec.BackupName = veleroBackup.Name + + veleroRestore = &velerov1.Restore{ + ObjectMeta: metav1.ObjectMeta{ + // TODO even though Kubernetes object name can be up to 253 char length + // using generate name, object name will be 63 char length + // it will add a random 5 char length to GenerateName prefix + // if prefix is more than 58 char length, it is truncated + GenerateName: function.GetGenerateNamePrefix(nar.Namespace, nar.Name), + Namespace: r.OADPNamespace, + Labels: function.GetNonAdminRestoreLabels(nar.Status.UUID), + Annotations: function.GetNonAdminRestoreAnnotations(nab.ObjectMeta), + }, + Spec: *restoreSpec, + } + + err = r.Create(ctx, veleroRestore) + if err != nil { + logger.Error(err, "Failed to create Velero Restore") + return false, err + } + logger.Info("Velero Restore successfully created") + } + + updatedPhase := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseCreated) + updatedCondition := meta.SetStatusCondition(&nar.Status.Conditions, + metav1.Condition{ + Type: string(constant.NonAdminConditionQueued), + Status: metav1.ConditionTrue, + Reason: "RestoreScheduled", // TODO can this confuse user? scheduled -> queued? + Message: "Created Velero Restore object", + }, + ) + // TODO need to refetch velero restore because of generate name? + updatedVeleroStatus := updateVeleroRestoreStatus(&nar.Status, veleroRestore) + if updatedPhase || updatedCondition || updatedVeleroStatus { + if err := r.Status().Update(ctx, nar); err != nil { + logger.Error(err, statusUpdateError) + return false, err + } + logger.V(1).Info("NonAdminRestore Status updated successfully") + } else { + logger.V(1).Info("NonAdminRestore Status unchanged during Velero Restore reconciliation") + } + + return false, nil +} + +func updateVeleroRestoreStatus(status *nacv1alpha1.NonAdminRestoreStatus, veleroRestore *velerov1.Restore) bool { + if status.VeleroRestore == nil { + status.VeleroRestore = &nacv1alpha1.VeleroRestore{ + Name: veleroRestore.Name, + Namespace: veleroRestore.Namespace, + Status: &veleroRestore.Status, + } + return true + } else if status.VeleroRestore.Name != veleroRestore.Name || + status.VeleroRestore.Namespace != veleroRestore.Namespace || + !reflect.DeepEqual(status.VeleroRestore.Status, &veleroRestore.Status) { + status.VeleroRestore.Name = veleroRestore.Name + status.VeleroRestore.Namespace = veleroRestore.Namespace + status.VeleroRestore.Status = &veleroRestore.Status + return true + } + return false +} + // SetupWithManager sets up the controller with the Manager. func (r *NonAdminRestoreReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&oadpv1alpha1.NonAdminRestore{}). + For(&nacv1alpha1.NonAdminRestore{}). + WithEventFilter(predicate.CompositeRestorePredicate{ + NonAdminRestorePredicate: predicate.NonAdminRestorePredicate{}, + VeleroRestorePredicate: predicate.VeleroRestorePredicate{ + OADPNamespace: r.OADPNamespace, + }, + }). + // handler runs after predicate + Watches(&velerov1.Restore{}, &handler.VeleroRestoreHandler{}). Complete(r) } diff --git a/internal/controller/nonadminrestore_controller_test.go b/internal/controller/nonadminrestore_controller_test.go index e1c78f7..554b393 100644 --- a/internal/controller/nonadminrestore_controller_test.go +++ b/internal/controller/nonadminrestore_controller_test.go @@ -18,65 +18,140 @@ package controller import ( "context" + "encoding/json" + "fmt" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + velerov1 "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/types" "sigs.k8s.io/controller-runtime/pkg/reconcile" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - oadpv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" + "github.com/migtools/oadp-non-admin/internal/common/constant" ) -var _ = Describe("NonAdminRestore Controller", func() { - Context("When reconciling a resource", func() { - const resourceName = "test-resource" +var _ = ginkgo.Describe("NonAdminRestore Controller", func() { + ginkgo.Context("When reconciling a resource", func() { + const ( + resourceName = "test-restore-resource-name" + resourceNamespace = "test-restore-resource-namespace" + + nonAdminBackupResourceName = "test-non-admin-backup-resource-name" + + TestUUID = "test-123456789" + + oadpNamespace = "test-restore-oadp-namespace" + ) ctx := context.Background() typeNamespacedName := types.NamespacedName{ Name: resourceName, - Namespace: "default", // TODO(user):Modify as needed + Namespace: resourceNamespace, } nonadminrestore := &oadpv1alpha1.NonAdminRestore{} - BeforeEach(func() { - By("creating the custom resource for the Kind NonAdminRestore") - err := k8sClient.Get(ctx, typeNamespacedName, nonadminrestore) + ginkgo.BeforeEach(func() { + ginkgo.By("creating namespaces") + err := createTestNamespaces(ctx, resourceNamespace, oadpNamespace) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + ginkgo.By("creating the custom resource for the Kind NonAdminBackup") + nonAdminBackupResource := &oadpv1alpha1.NonAdminBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: nonAdminBackupResourceName, + Namespace: resourceNamespace, + }, + Spec: oadpv1alpha1.NonAdminBackupSpec{ + BackupSpec: &velerov1.BackupSpec{ + IncludedNamespaces: []string{resourceNamespace}, + }, + }, + } + gomega.Expect(k8sClient.Create(ctx, nonAdminBackupResource)).To(gomega.Succeed()) + nonAdminBackupResource.Status.Phase = oadpv1alpha1.NonAdminPhaseCreated + nonAdminBackupResource.Status.VeleroBackup = &oadpv1alpha1.VeleroBackup{} + nonAdminBackupResource.Status.VeleroBackup.NACUUID = TestUUID + gomega.Expect(k8sClient.Status().Update(ctx, nonAdminBackupResource)).To(gomega.Succeed()) + + ginkgo.By("creating the custom resource for the Kind Velero Backup") + backupResource := &velerov1.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "does-not-matter", + Namespace: oadpNamespace, + Labels: map[string]string{ + constant.NabOriginNACUUIDLabel: TestUUID, + }, + }, + } + gomega.Expect(k8sClient.Create(ctx, backupResource)).To(gomega.Succeed()) + + ginkgo.By("creating the custom resource for the Kind NonAdminRestore") + err = k8sClient.Get(ctx, typeNamespacedName, nonadminrestore) if err != nil && errors.IsNotFound(err) { resource := &oadpv1alpha1.NonAdminRestore{ ObjectMeta: metav1.ObjectMeta{ Name: resourceName, - Namespace: "default", + Namespace: resourceNamespace, + }, + Spec: oadpv1alpha1.NonAdminRestoreSpec{ + RestoreSpec: &velerov1.RestoreSpec{ + BackupName: nonAdminBackupResourceName, + }, }, - // TODO(user): Specify other spec details if needed. } - Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + gomega.Expect(k8sClient.Create(ctx, resource)).To(gomega.Succeed()) + + p, _ := json.MarshalIndent(resource, "", "\t") + fmt.Printf("%s\n", p) } }) - AfterEach(func() { - // TODO(user): Cleanup logic after each test, like removing the resource instance. + ginkgo.AfterEach(func() { resource := &oadpv1alpha1.NonAdminRestore{} err := k8sClient.Get(ctx, typeNamespacedName, resource) - Expect(err).NotTo(HaveOccurred()) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + p, _ := json.MarshalIndent(resource, "", "\t") + fmt.Printf("%s\n", p) + + ginkgo.By("Cleanup the specific resource instance NonAdminRestore") + gomega.Expect(k8sClient.Delete(ctx, resource)).To(gomega.Succeed()) + + ginkgo.By("Finalizer should not allow NonAdminRestore deletion") + err = k8sClient.Get(ctx, typeNamespacedName, resource) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + controllerReconciler := &NonAdminRestoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + OADPNamespace: oadpNamespace, + } + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) - By("Cleanup the specific resource instance NonAdminRestore") - Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + ginkgo.By("After reconcile should allow NonAdminRestore deletion") + err = k8sClient.Get(ctx, typeNamespacedName, resource) + gomega.Expect(apierrors.IsNotFound(err)).To(gomega.BeTrue()) }) - It("should successfully reconcile the resource", func() { - By("Reconciling the created resource") + ginkgo.It("should successfully reconcile the resource", func() { + ginkgo.By("Reconciling the created resource") controllerReconciler := &NonAdminRestoreReconciler{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), + Client: k8sClient, + Scheme: k8sClient.Scheme(), + OADPNamespace: oadpNamespace, } _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ NamespacedName: typeNamespacedName, }) - Expect(err).NotTo(HaveOccurred()) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) // TODO(user): Add more specific assertions depending on your controller's reconciliation logic. // Example: If you expect a certain status condition after reconciliation, verify it here. }) diff --git a/internal/handler/velerorestore_handler.go b/internal/handler/velerorestore_handler.go new file mode 100644 index 0000000..9603bf4 --- /dev/null +++ b/internal/handler/velerorestore_handler.go @@ -0,0 +1,63 @@ +/* +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 handler + +import ( + "context" + + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + // "github.com/migtools/oadp-non-admin/internal/common/constant" + "github.com/migtools/oadp-non-admin/internal/common/function" +) + +// VeleroRestoreHandler contains event handlers for Velero Restore objects +type VeleroRestoreHandler struct{} + +// Create event handler +func (VeleroRestoreHandler) Create(_ context.Context, _ event.CreateEvent, _ workqueue.RateLimitingInterface) { + // Create event handler for the Restore object +} + +// Update event handler adds Velero Restore's NonAdminRestore to controller queue +func (VeleroRestoreHandler) Update(ctx context.Context, evt event.UpdateEvent, q workqueue.RateLimitingInterface) { + logger := function.GetLogger(ctx, evt.ObjectNew, "VeleroRestoreHandler") + + // TODO + // annotations := evt.ObjectNew.GetAnnotations() + // nabOriginNamespace := annotations[constant.NabOriginNamespaceAnnotation] + // nabOriginName := annotations[constant.NabOriginNameAnnotation] + + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Name: "", + Namespace: "", + }}) + logger.V(1).Info("Handled Update event") +} + +// Delete event handler +func (VeleroRestoreHandler) Delete(_ context.Context, _ event.DeleteEvent, _ workqueue.RateLimitingInterface) { + // Delete event handler for the Restore object +} + +// Generic event handler +func (VeleroRestoreHandler) Generic(_ context.Context, _ event.GenericEvent, _ workqueue.RateLimitingInterface) { + // Generic event handler for the Restore object +} diff --git a/internal/predicate/composite_predicate.go b/internal/predicate/compositebackup_predicate.go similarity index 82% rename from internal/predicate/composite_predicate.go rename to internal/predicate/compositebackup_predicate.go index 98fab4f..89470c3 100644 --- a/internal/predicate/composite_predicate.go +++ b/internal/predicate/compositebackup_predicate.go @@ -26,8 +26,8 @@ import ( nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" ) -// CompositePredicate is a combination of NonAdminBackup and Velero Backup event filters -type CompositePredicate struct { +// CompositeBackupPredicate is a combination of NonAdminBackup and Velero Backup event filters +type CompositeBackupPredicate struct { Context context.Context NonAdminBackupPredicate NonAdminBackupPredicate VeleroBackupPredicate VeleroBackupPredicate @@ -35,7 +35,7 @@ type CompositePredicate struct { } // Create event filter only accepts NonAdminBackup create events -func (p CompositePredicate) Create(evt event.CreateEvent) bool { +func (p CompositeBackupPredicate) Create(evt event.CreateEvent) bool { switch evt.Object.(type) { case *nacv1alpha1.NonAdminBackup: return p.NonAdminBackupPredicate.Create(p.Context, evt) @@ -45,7 +45,7 @@ func (p CompositePredicate) Create(evt event.CreateEvent) bool { } // Update event filter accepts both NonAdminBackup and Velero Backup update events -func (p CompositePredicate) Update(evt event.UpdateEvent) bool { +func (p CompositeBackupPredicate) Update(evt event.UpdateEvent) bool { switch evt.ObjectNew.(type) { case *nacv1alpha1.NonAdminBackup: return p.NonAdminBackupPredicate.Update(p.Context, evt) @@ -57,7 +57,7 @@ func (p CompositePredicate) Update(evt event.UpdateEvent) bool { } // Delete event filter only accepts NonAdminBackup delete events -func (p CompositePredicate) Delete(evt event.DeleteEvent) bool { +func (p CompositeBackupPredicate) Delete(evt event.DeleteEvent) bool { switch evt.Object.(type) { case *nacv1alpha1.NonAdminBackup: return p.NonAdminBackupPredicate.Delete(p.Context, evt) @@ -67,6 +67,6 @@ func (p CompositePredicate) Delete(evt event.DeleteEvent) bool { } // Generic event filter does not accept any generic events -func (CompositePredicate) Generic(_ event.GenericEvent) bool { +func (CompositeBackupPredicate) Generic(_ event.GenericEvent) bool { return false } diff --git a/internal/predicate/compositerestore_predicate.go b/internal/predicate/compositerestore_predicate.go new file mode 100644 index 0000000..d344ce7 --- /dev/null +++ b/internal/predicate/compositerestore_predicate.go @@ -0,0 +1,70 @@ +/* +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 predicate + +import ( + "context" + + velerov1 "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" +) + +// CompositeRestorePredicate is a combination of NonAdminRestore and Velero Restore event filters +type CompositeRestorePredicate struct { + Context context.Context + NonAdminRestorePredicate NonAdminRestorePredicate + VeleroRestorePredicate VeleroRestorePredicate +} + +// Create event filter only accepts NonAdminRestore create events +func (p CompositeRestorePredicate) Create(evt event.CreateEvent) bool { + switch evt.Object.(type) { + case *nacv1alpha1.NonAdminBackup: + return p.NonAdminRestorePredicate.Create(p.Context, evt) + default: + return false + } +} + +// Update event filter accepts both NonAdminRestore and Velero Restore update events +func (p CompositeRestorePredicate) Update(evt event.UpdateEvent) bool { + switch evt.ObjectNew.(type) { + case *nacv1alpha1.NonAdminBackup: + return p.NonAdminRestorePredicate.Update(p.Context, evt) + case *velerov1.Backup: + return p.VeleroRestorePredicate.Update(p.Context, evt) + default: + return false + } +} + +// Delete event filter only accepts NonAdminRestore delete events +func (p CompositeRestorePredicate) Delete(evt event.DeleteEvent) bool { + switch evt.Object.(type) { + case *nacv1alpha1.NonAdminBackup: + return p.NonAdminRestorePredicate.Delete(p.Context, evt) + default: + return false + } +} + +// Generic event filter does not accept any generic events +func (CompositeRestorePredicate) Generic(_ event.GenericEvent) bool { + return false +} diff --git a/internal/predicate/nonadminrestore_predicate.go b/internal/predicate/nonadminrestore_predicate.go new file mode 100644 index 0000000..e162216 --- /dev/null +++ b/internal/predicate/nonadminrestore_predicate.go @@ -0,0 +1,57 @@ +/* +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 predicate + +import ( + "context" + + "sigs.k8s.io/controller-runtime/pkg/event" + + "github.com/migtools/oadp-non-admin/internal/common/function" +) + +const nonAdminRestorePredicateKey = "NonAdminRestorePredicate" + +// NonAdminRestorePredicate contains event filters for Non Admin Backup objects +type NonAdminRestorePredicate struct{} + +// Create event filter accepts all NonAdminRestore create events +func (NonAdminRestorePredicate) Create(ctx context.Context, evt event.CreateEvent) bool { + logger := function.GetLogger(ctx, evt.Object, nonAdminBackupPredicateKey) + logger.V(1).Info("Accepted Create event") + return true +} + +// Update event filter only accepts NonAdminRestore update events that include generation change +func (NonAdminRestorePredicate) Update(ctx context.Context, evt event.UpdateEvent) bool { + logger := function.GetLogger(ctx, evt.ObjectNew, nonAdminBackupPredicateKey) + + if evt.ObjectNew.GetGeneration() != evt.ObjectOld.GetGeneration() { + logger.V(1).Info("Accepted Update event") + return true + } + + logger.V(1).Info("Rejected Update event") + return false +} + +// Delete event filter accepts all NonAdminRestore delete events +func (NonAdminRestorePredicate) Delete(ctx context.Context, evt event.DeleteEvent) bool { + logger := function.GetLogger(ctx, evt.Object, nonAdminBackupPredicateKey) + logger.V(1).Info("Accepted Delete event") + return true +} diff --git a/internal/predicate/velerorestore_predicate.go b/internal/predicate/velerorestore_predicate.go new file mode 100644 index 0000000..518350d --- /dev/null +++ b/internal/predicate/velerorestore_predicate.go @@ -0,0 +1,48 @@ +/* +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 predicate + +import ( + "context" + + "sigs.k8s.io/controller-runtime/pkg/event" + + "github.com/migtools/oadp-non-admin/internal/common/function" +) + +// VeleroRestorePredicate contains event filters for Velero Restore objects +type VeleroRestorePredicate struct { + OADPNamespace string +} + +// Update event filter only accepts Velero Restore update events from OADP namespace +// and from Velero Restores that have required metadata +func (p VeleroRestorePredicate) Update(ctx context.Context, evt event.UpdateEvent) bool { + logger := function.GetLogger(ctx, evt.ObjectNew, "VeleroRestorePredicate") + + namespace := evt.ObjectNew.GetNamespace() + if namespace == p.OADPNamespace { + // TODO + if function.CheckVeleroBackupMetadata(evt.ObjectNew) { + logger.V(1).Info("Accepted Update event") + return true + } + } + + logger.V(1).Info("Rejected Update event") + return false +} From e091a042f803d422ad97a01c06a615f293c8999e Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Mon, 18 Nov 2024 17:17:37 -0300 Subject: [PATCH 05/14] fixup! feat: Add NAC restore controller restore logic and tests Signed-off-by: Mateus Oliveira --- api/v1alpha1/nonadminrestore_types.go | 7 +- cmd/main.go | 5 +- .../oadp.openshift.io_nonadminrestores.yaml | 2 + internal/common/function/function.go | 46 +- .../controller/nonadminrestore_controller.go | 119 +++--- .../nonadminrestore_controller_test.go | 397 +++++++++++++----- internal/handler/velerorestore_handler.go | 13 +- .../predicate/compositerestore_predicate.go | 8 +- .../predicate/nonadminrestore_predicate.go | 6 +- internal/predicate/velerorestore_predicate.go | 3 +- 10 files changed, 402 insertions(+), 204 deletions(-) diff --git a/api/v1alpha1/nonadminrestore_types.go b/api/v1alpha1/nonadminrestore_types.go index 425e475..9a0fd7f 100644 --- a/api/v1alpha1/nonadminrestore_types.go +++ b/api/v1alpha1/nonadminrestore_types.go @@ -56,8 +56,9 @@ type NonAdminRestoreStatus struct { Conditions []metav1.Condition `json:"conditions,omitempty"` } -//+kubebuilder:object:root=true -//+kubebuilder:subresource:status +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status +// +kubebuilder:resource:path=nonadminrestores,shortName=nar // NonAdminRestore is the Schema for the nonadminrestores API type NonAdminRestore struct { @@ -68,7 +69,7 @@ type NonAdminRestore struct { Status NonAdminRestoreStatus `json:"status,omitempty"` } -//+kubebuilder:object:root=true +// +kubebuilder:object:root=true // NonAdminRestoreList contains a list of NonAdminRestore type NonAdminRestoreList struct { diff --git a/cmd/main.go b/cmd/main.go index b065d34..e4236a8 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -157,8 +157,9 @@ func main() { os.Exit(1) } if err = (&controller.NonAdminRestoreReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + OADPNamespace: oadpNamespace, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "NonAdminRestore") os.Exit(1) diff --git a/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml b/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml index 72dbadf..8b23692 100644 --- a/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml +++ b/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml @@ -11,6 +11,8 @@ spec: kind: NonAdminRestore listKind: NonAdminRestoreList plural: nonadminrestores + shortNames: + - nar singular: nonadminrestore scope: Namespaced versions: diff --git a/internal/common/function/function.go b/internal/common/function/function.go index b4b5981..5373c1b 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -46,9 +46,9 @@ func GetNonAdminLabels() map[string]string { } } -func GetNonAdminRestoreLabels(uuid string) map[string]string { +func GetNonAdminRestoreLabels(uniqueIdentifier string) map[string]string { nonAdminLabels := GetNonAdminLabels() - nonAdminLabels[constant.NarOriginNACUUIDLabel] = uuid + nonAdminLabels[constant.NarOriginNACUUIDLabel] = uniqueIdentifier return nonAdminLabels } @@ -113,8 +113,20 @@ func ValidateBackupSpec(nonAdminBackup *nacv1alpha1.NonAdminBackup, enforcedBack return nil } -func ValidateRestoreSpec(nonAdminRestore *nacv1alpha1.NonAdminRestore) error { - // TODO check NonAdminBackup with nonAdminRestore.Spec.RestoreSpec.BackupName name exist in NonAdminRestore namespace +func ValidateRestoreSpec(ctx context.Context, clientInstance client.Client, nonAdminRestore *nacv1alpha1.NonAdminRestore) error { + nab := &nacv1alpha1.NonAdminBackup{} + err := clientInstance.Get(ctx, types.NamespacedName{ + Name: nonAdminRestore.Spec.RestoreSpec.BackupName, + Namespace: nonAdminRestore.Namespace, + }, nab) + if err != nil { + return fmt.Errorf("NonAdminRestore spec.restoreSpec.backupName is invalid: %v", err) + } + // TODO better way to check readiness? simplify and ask user to pass velero backup name? (user has access to this info in nonAdminBackup status) + if nab.Status.Phase != nacv1alpha1.NonAdminPhaseCreated { + return fmt.Errorf("NonAdminRestore spec.restoreSpec.backupName is invalid: NonAdminBackup is not ready to be restored") + } + // TODO does velero validate if backup is ready to be restored? // TODO validate that nonAdminRestore.Spec.RestoreSpec.ScheduleName is not used? (we do not plan to have schedules now, right?) @@ -177,7 +189,7 @@ func GetGenerateNamePrefix(namespace string, name string) string { if len(namespace) >= remainingLength { return fmt.Sprintf("%s-", namespace[:remainingLength]) } - remainingLength := remainingLength - len(namespace) + remainingLength = remainingLength - len(namespace) return fmt.Sprintf("%s-%s-", name[:remainingLength], namespace) } @@ -361,6 +373,30 @@ func CheckVeleroBackupMetadata(obj client.Object) bool { return true } +func CheckVeleroRestoreMetadata(obj client.Object) bool { + objLabels := obj.GetLabels() + if !checkLabelValue(objLabels, constant.OadpLabel, constant.OadpLabelValue) { + return false + } + if !checkLabelValue(objLabels, constant.ManagedByLabel, constant.ManagedByLabelValue) { + return false + } + _, err := uuid.Parse(objLabels[constant.NarOriginNACUUIDLabel]) + if err != nil { + return false + } + + annotations := obj.GetAnnotations() + if !checkLabelAnnotationValueIsValid(annotations, constant.NonAdminRestoreOriginNamespaceAnnotation) { + return false + } + if !checkLabelAnnotationValueIsValid(annotations, constant.NonAdminRestoreOriginNameAnnotation) { + return false + } + + return true +} + func checkLabelValue(objLabels map[string]string, key string, value string) bool { got, exists := objLabels[key] if !exists { diff --git a/internal/controller/nonadminrestore_controller.go b/internal/controller/nonadminrestore_controller.go index 10f7a27..d189f4e 100644 --- a/internal/controller/nonadminrestore_controller.go +++ b/internal/controller/nonadminrestore_controller.go @@ -18,8 +18,8 @@ package controller import ( "context" - "fmt" "reflect" + "time" "github.com/go-logr/logr" "github.com/google/uuid" @@ -49,11 +49,11 @@ type NonAdminRestoreReconciler struct { OADPNamespace string } -type nonAdminRestoreReconcileStepFunction func(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) +type nonAdminRestoreReconcileStepFunction func(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error -//+kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores/finalizers,verbs=update +// +kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores/finalizers,verbs=update // +kubebuilder:rbac:groups=velero.io,resources=restores,verbs=get;list;watch;create;update;patch;delete @@ -87,7 +87,7 @@ func (r *NonAdminRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Requ ) if updatedPhase || updatedCondition { if err := r.Status().Update(ctx, nar); err != nil { - logger.Error(err, statusUpdateError) + logger.Error(err, "Failed to update NonAdminRestore Status") return ctrl.Result{}, err } logger.V(1).Info("NonAdminRestore status marked for deletion") @@ -96,31 +96,25 @@ func (r *NonAdminRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Requ } veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, nar.Status.UUID) - if err != nil { - if !apierrors.IsNotFound(err) { - logger.Error(err, "Unable to fetch Velero Restore") - return ctrl.Result{}, err - } - } else { - // TODO when/why Velero Restore has finalizer? - // TODO how to safely remove Velero Restore with finalizer? - for _, finalizer := range veleroRestore.GetFinalizers() { - controllerutil.RemoveFinalizer(veleroRestore, finalizer) - } - // TODO does this change generation? need to refetch? - if err := r.Update(ctx, veleroRestore); err != nil { - return ctrl.Result{}, err - } + if err == nil { + // TODO any problem in calling delete on something being deleted? err = r.Delete(ctx, veleroRestore) if err != nil { logger.Error(err, "Unable to delete Velero Restore") return ctrl.Result{}, err } + // wait Velero delete restore object + return ctrl.Result{RequeueAfter: 5 * time.Second}, nil + } + if !apierrors.IsNotFound(err) { + logger.Error(err, "Unable to fetch Velero Restore") + return ctrl.Result{}, err } controllerutil.RemoveFinalizer(nar, constant.NonAdminRestoreFinalizerName) // TODO does this change generation? need to refetch? if err := r.Update(ctx, nar); err != nil { + logger.Error(err, "Unable to remove NonAdminRestore finalizer") return ctrl.Result{}, err } logger.V(1).Info("NonAdminRestore Reconcile exit") @@ -135,35 +129,32 @@ func (r *NonAdminRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Requ r.createVeleroRestore, } for _, step := range reconcileSteps { - requeue, err := step(ctx, logger, nar) + err := step(ctx, logger, nar) if err != nil { return ctrl.Result{}, err - } else if requeue { - // TODO needed? - return ctrl.Result{Requeue: true}, nil } } logger.V(1).Info("NonAdminRestore Reconcile exit") return ctrl.Result{}, nil } -func (r *NonAdminRestoreReconciler) init(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { +func (r *NonAdminRestoreReconciler) init(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { if nar.Status.Phase == constant.EmptyString { if updated := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseNew); updated { if err := r.Status().Update(ctx, nar); err != nil { - logger.Error(err, statusUpdateError) - return false, err + logger.Error(err, "Failed to update NonAdminRestore Status") + return err } logger.V(1).Info("NonAdminRestore Phase set to New") } } else { logger.V(1).Info("NonAdminRestore Phase already initialized") } - return false, nil + return nil } -func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { - err := function.ValidateRestoreSpec(nar) +func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { + err := function.ValidateRestoreSpec(ctx, r.Client, nar) if err != nil { updatedPhase := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseBackingOff) updatedCondition := meta.SetStatusCondition(&nar.Status.Conditions, @@ -176,11 +167,11 @@ func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger log ) if updatedPhase || updatedCondition { if updateErr := r.Status().Update(ctx, nar); updateErr != nil { - logger.Error(updateErr, statusUpdateError) - return false, updateErr + logger.Error(updateErr, "Failed to update NonAdminRestore Status") + return updateErr } } - return false, reconcile.TerminalError(err) + return reconcile.TerminalError(err) } logger.V(1).Info("NonAdminRestore Spec validated") @@ -194,69 +185,63 @@ func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger log ) if updated { if err := r.Status().Update(ctx, nar); err != nil { - logger.Error(err, statusUpdateError) - return false, err + logger.Error(err, "Failed to update NonAdminRestore Status") + return err } logger.V(1).Info("NonAdminRestore condition set to Accepted") } - return false, nil + return nil } -func (r *NonAdminRestoreReconciler) setUUID(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { +func (r *NonAdminRestoreReconciler) setUUID(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { if nar.Status.UUID == constant.EmptyString { // TODO handle panic nar.Status.UUID = uuid.New().String() if err := r.Status().Update(ctx, nar); err != nil { logger.Error(err, statusUpdateError) - return false, err + return err } - logger.V(1).Info(veleroReferenceUpdated) + logger.V(1).Info("NonAdminRestore UUID created") } else { - logger.V(1).Info("NonAdminRestore already contains NAC UUID") + logger.V(1).Info("NonAdminRestore already contains UUID") } - return false, nil + return nil } -func (r *NonAdminRestoreReconciler) setFinalizer(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { +func (r *NonAdminRestoreReconciler) setFinalizer(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { added := controllerutil.AddFinalizer(nar, constant.NonAdminRestoreFinalizerName) if added { if err := r.Update(ctx, nar); err != nil { logger.Error(err, "Failed to add finalizer") - return false, err + return err } // TODO does this change generation? need to refetch? logger.V(1).Info("Finalizer added to NonAdminRestore", "finalizer", constant.NonAdminRestoreFinalizerName) } else { logger.V(1).Info("Finalizer already exists on NonAdminRestore", "finalizer", constant.NonAdminRestoreFinalizerName) } - return false, nil + return nil } -func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { +func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { nacUUID := nar.Status.UUID veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, nacUUID) if err != nil { if !apierrors.IsNotFound(err) { - return false, err + return err } - // TODO how to add this to validation without doing GET call for NAB/Velero backup twice? add field in status? restoreSpec := nar.Spec.RestoreSpec.DeepCopy() + + // TODO already did this get call in ValidateRestoreSpec! nab := &nacv1alpha1.NonAdminBackup{} - err = r.Get(ctx, types.NamespacedName{Namespace: nar.Namespace, Name: nar.Spec.RestoreSpec.BackupName}, nab) - if err != nil { - return false, err - } - // TODO better way to check readiness? simplify and ask user to pass velero backup name? (user has access to this info in nonAdminBackup status) - if nab.Status.Phase != nacv1alpha1.NonAdminPhaseCreated { - return false, fmt.Errorf("NonAdminBackup is not ready to use for restore") - } - veleroBackup, err := function.GetVeleroBackupByLabel(ctx, r.Client, r.OADPNamespace, nab.Status.VeleroBackup.NACUUID) - if err != nil || veleroBackup == nil { - return false, fmt.Errorf("velero Backup does not exist or multiple exist") - } - // TODO does velero validate if backup is ready to be restored? - restoreSpec.BackupName = veleroBackup.Name + _ = r.Get(ctx, types.NamespacedName{ + Name: nar.Spec.RestoreSpec.BackupName, + Namespace: nar.Namespace, + }, nab) + restoreSpec.BackupName = nab.Status.VeleroBackup.Name + + // TODO enforce Restore Spec veleroRestore = &velerov1.Restore{ ObjectMeta: metav1.ObjectMeta{ @@ -267,7 +252,7 @@ func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, log GenerateName: function.GetGenerateNamePrefix(nar.Namespace, nar.Name), Namespace: r.OADPNamespace, Labels: function.GetNonAdminRestoreLabels(nar.Status.UUID), - Annotations: function.GetNonAdminRestoreAnnotations(nab.ObjectMeta), + Annotations: function.GetNonAdminRestoreAnnotations(nar.ObjectMeta), }, Spec: *restoreSpec, } @@ -275,7 +260,7 @@ func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, log err = r.Create(ctx, veleroRestore) if err != nil { logger.Error(err, "Failed to create Velero Restore") - return false, err + return err } logger.Info("Velero Restore successfully created") } @@ -293,15 +278,15 @@ func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, log updatedVeleroStatus := updateVeleroRestoreStatus(&nar.Status, veleroRestore) if updatedPhase || updatedCondition || updatedVeleroStatus { if err := r.Status().Update(ctx, nar); err != nil { - logger.Error(err, statusUpdateError) - return false, err + logger.Error(err, "Failed to update NonAdminRestore Status") + return err } logger.V(1).Info("NonAdminRestore Status updated successfully") } else { logger.V(1).Info("NonAdminRestore Status unchanged during Velero Restore reconciliation") } - return false, nil + return nil } func updateVeleroRestoreStatus(status *nacv1alpha1.NonAdminRestoreStatus, veleroRestore *velerov1.Restore) bool { diff --git a/internal/controller/nonadminrestore_controller_test.go b/internal/controller/nonadminrestore_controller_test.go index 554b393..df9ca62 100644 --- a/internal/controller/nonadminrestore_controller_test.go +++ b/internal/controller/nonadminrestore_controller_test.go @@ -18,142 +18,317 @@ package controller import ( "context" - "encoding/json" "fmt" + "reflect" + "strings" + "time" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" velerov1 "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/types" - "sigs.k8s.io/controller-runtime/pkg/reconcile" + "k8s.io/apimachinery/pkg/util/wait" + ctrl "sigs.k8s.io/controller-runtime" - oadpv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" + nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" "github.com/migtools/oadp-non-admin/internal/common/constant" ) -var _ = ginkgo.Describe("NonAdminRestore Controller", func() { - ginkgo.Context("When reconciling a resource", func() { - const ( - resourceName = "test-restore-resource-name" - resourceNamespace = "test-restore-resource-namespace" +type nonAdminRestoreFullReconcileScenario struct { + spec nacv1alpha1.NonAdminRestoreSpec + status nacv1alpha1.NonAdminRestoreStatus + backupStatus nacv1alpha1.NonAdminBackupStatus +} - nonAdminBackupResourceName = "test-non-admin-backup-resource-name" +func buildTestNonAdminRestore(nonAdminNamespace string, nonAdminName string, spec nacv1alpha1.NonAdminRestoreSpec) *nacv1alpha1.NonAdminRestore { + return &nacv1alpha1.NonAdminRestore{ + ObjectMeta: metav1.ObjectMeta{ + Name: nonAdminName, + Namespace: nonAdminNamespace, + }, + Spec: spec, + } +} - TestUUID = "test-123456789" +func checkTestNonAdminRestoreStatus(nonAdminRestore *nacv1alpha1.NonAdminRestore, expectedStatus nacv1alpha1.NonAdminRestoreStatus) error { + if nonAdminRestore.Status.Phase != expectedStatus.Phase { + return fmt.Errorf("NonAdminRestore Status Phase %v is not equal to expected %v", nonAdminRestore.Status.Phase, expectedStatus.Phase) + } - oadpNamespace = "test-restore-oadp-namespace" - ) - - ctx := context.Background() - - typeNamespacedName := types.NamespacedName{ - Name: resourceName, - Namespace: resourceNamespace, + if nonAdminRestore.Status.VeleroRestore != nil { + if nonAdminRestore.Status.UUID == constant.EmptyString { + return fmt.Errorf("NonAdminRestore Status UUID not set") } - nonadminrestore := &oadpv1alpha1.NonAdminRestore{} - - ginkgo.BeforeEach(func() { - ginkgo.By("creating namespaces") - err := createTestNamespaces(ctx, resourceNamespace, oadpNamespace) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) - - ginkgo.By("creating the custom resource for the Kind NonAdminBackup") - nonAdminBackupResource := &oadpv1alpha1.NonAdminBackup{ - ObjectMeta: metav1.ObjectMeta{ - Name: nonAdminBackupResourceName, - Namespace: resourceNamespace, - }, - Spec: oadpv1alpha1.NonAdminBackupSpec{ - BackupSpec: &velerov1.BackupSpec{ - IncludedNamespaces: []string{resourceNamespace}, - }, - }, - } - gomega.Expect(k8sClient.Create(ctx, nonAdminBackupResource)).To(gomega.Succeed()) - nonAdminBackupResource.Status.Phase = oadpv1alpha1.NonAdminPhaseCreated - nonAdminBackupResource.Status.VeleroBackup = &oadpv1alpha1.VeleroBackup{} - nonAdminBackupResource.Status.VeleroBackup.NACUUID = TestUUID - gomega.Expect(k8sClient.Status().Update(ctx, nonAdminBackupResource)).To(gomega.Succeed()) - - ginkgo.By("creating the custom resource for the Kind Velero Backup") - backupResource := &velerov1.Backup{ - ObjectMeta: metav1.ObjectMeta{ - Name: "does-not-matter", - Namespace: oadpNamespace, - Labels: map[string]string{ - constant.NabOriginNACUUIDLabel: TestUUID, - }, - }, - } - gomega.Expect(k8sClient.Create(ctx, backupResource)).To(gomega.Succeed()) - - ginkgo.By("creating the custom resource for the Kind NonAdminRestore") - err = k8sClient.Get(ctx, typeNamespacedName, nonadminrestore) - if err != nil && errors.IsNotFound(err) { - resource := &oadpv1alpha1.NonAdminRestore{ - ObjectMeta: metav1.ObjectMeta{ - Name: resourceName, - Namespace: resourceNamespace, - }, - Spec: oadpv1alpha1.NonAdminRestoreSpec{ - RestoreSpec: &velerov1.RestoreSpec{ - BackupName: nonAdminBackupResourceName, - }, - }, + if nonAdminRestore.Status.VeleroRestore.Namespace == constant.EmptyString { + return fmt.Errorf("NonAdminRestore status.veleroRestore.namespace is not set") + } + if nonAdminRestore.Status.VeleroRestore.Name == constant.EmptyString { + return fmt.Errorf("NonAdminRestore status.veleroRestore.name is not set") + } + if expectedStatus.VeleroRestore != nil { + if expectedStatus.VeleroRestore.Status != nil { + if !reflect.DeepEqual(nonAdminRestore.Status.VeleroRestore.Status, expectedStatus.VeleroRestore.Status) { + return fmt.Errorf("NonAdminRestore status.veleroRestore.status %v is not equal to expected %v", nonAdminRestore.Status.VeleroRestore.Status, expectedStatus.VeleroRestore.Status) } - gomega.Expect(k8sClient.Create(ctx, resource)).To(gomega.Succeed()) - - p, _ := json.MarshalIndent(resource, "", "\t") - fmt.Printf("%s\n", p) } - }) + } + } - ginkgo.AfterEach(func() { - resource := &oadpv1alpha1.NonAdminRestore{} - err := k8sClient.Get(ctx, typeNamespacedName, resource) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) + if len(nonAdminRestore.Status.Conditions) != len(expectedStatus.Conditions) { + return fmt.Errorf("NonAdminRestore Status has %v Condition(s), expected to have %v", len(nonAdminRestore.Status.Conditions), len(expectedStatus.Conditions)) + } + for index := range nonAdminRestore.Status.Conditions { + if nonAdminRestore.Status.Conditions[index].Type != expectedStatus.Conditions[index].Type { + return fmt.Errorf("NonAdminRestore Status Conditions [%v] Type %v is not equal to expected %v", index, nonAdminRestore.Status.Conditions[index].Type, expectedStatus.Conditions[index].Type) + } + if nonAdminRestore.Status.Conditions[index].Status != expectedStatus.Conditions[index].Status { + return fmt.Errorf("NonAdminRestore Status Conditions [%v] Status %v is not equal to expected %v", index, nonAdminRestore.Status.Conditions[index].Status, expectedStatus.Conditions[index].Status) + } + if nonAdminRestore.Status.Conditions[index].Reason != expectedStatus.Conditions[index].Reason { + return fmt.Errorf("NonAdminRestore Status Conditions [%v] Reason %v is not equal to expected %v", index, nonAdminRestore.Status.Conditions[index].Reason, expectedStatus.Conditions[index].Reason) + } + if !strings.Contains(nonAdminRestore.Status.Conditions[index].Message, expectedStatus.Conditions[index].Message) { + return fmt.Errorf("NonAdminRestore Status Conditions [%v] Message %v does not contain expected message %v", index, nonAdminRestore.Status.Conditions[index].Message, expectedStatus.Conditions[index].Message) + } + } + return nil +} + +var _ = ginkgo.Describe("Test full reconcile loop of NonAdminRestore Controller", func() { + var ( + ctx context.Context + cancel context.CancelFunc + nonAdminRestoreName string + nonAdminRestoreNamespace string + oadpNamespace string + counter int + ) - p, _ := json.MarshalIndent(resource, "", "\t") - fmt.Printf("%s\n", p) + ginkgo.BeforeEach(func() { + counter++ + nonAdminRestoreName = fmt.Sprintf("non-admin-restore-object-%v", counter) + nonAdminRestoreNamespace = fmt.Sprintf("test-non-admin-restore-reconcile-full-%v", counter) + oadpNamespace = nonAdminRestoreNamespace + "-oadp" + }) - ginkgo.By("Cleanup the specific resource instance NonAdminRestore") - gomega.Expect(k8sClient.Delete(ctx, resource)).To(gomega.Succeed()) + ginkgo.AfterEach(func() { + gomega.Expect(deleteTestNamespaces(ctx, nonAdminRestoreNamespace, oadpNamespace)).To(gomega.Succeed()) - ginkgo.By("Finalizer should not allow NonAdminRestore deletion") - err = k8sClient.Get(ctx, typeNamespacedName, resource) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) + cancel() + // wait cancel + time.Sleep(1 * time.Second) + }) - controllerReconciler := &NonAdminRestoreReconciler{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), - OADPNamespace: oadpNamespace, - } - _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: typeNamespacedName, + ginkgo.DescribeTable("Reconcile triggered by NonAdminRestore Create event", + func(scenario nonAdminRestoreFullReconcileScenario) { + ctx, cancel = context.WithCancel(context.Background()) + + gomega.Expect(createTestNamespaces(ctx, nonAdminRestoreNamespace, oadpNamespace)).To(gomega.Succeed()) + + nonAdminBackup := buildTestNonAdminBackup(nonAdminRestoreNamespace, scenario.spec.RestoreSpec.BackupName, nacv1alpha1.NonAdminBackupSpec{}) + gomega.Expect(k8sClient.Create(ctx, nonAdminBackup)).To(gomega.Succeed()) + nonAdminBackup.Status = scenario.backupStatus + gomega.Expect(k8sClient.Status().Update(ctx, nonAdminBackup)).To(gomega.Succeed()) + + k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: k8sClient.Scheme(), }) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) - - ginkgo.By("After reconcile should allow NonAdminRestore deletion") - err = k8sClient.Get(ctx, typeNamespacedName, resource) - gomega.Expect(apierrors.IsNotFound(err)).To(gomega.BeTrue()) - }) - ginkgo.It("should successfully reconcile the resource", func() { - ginkgo.By("Reconciling the created resource") - controllerReconciler := &NonAdminRestoreReconciler{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + err = (&NonAdminRestoreReconciler{ + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), OADPNamespace: oadpNamespace, - } + }).SetupWithManager(k8sManager) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + go func() { + defer ginkgo.GinkgoRecover() + err = k8sManager.Start(ctx) + gomega.Expect(err).ToNot(gomega.HaveOccurred(), "failed to run manager") + }() + // wait manager start + managerStartTimeout := 10 * time.Second + pollInterval := 100 * time.Millisecond + ctxTimeout, cancel := context.WithTimeout(ctx, managerStartTimeout) + defer cancel() - _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: typeNamespacedName, + err = wait.PollUntilContextTimeout(ctxTimeout, pollInterval, managerStartTimeout, true, func(ctx context.Context) (done bool, err error) { + select { + case <-ctx.Done(): + return false, ctx.Err() + default: + // Check if the manager has started by verifying if the client is initialized + return k8sManager.GetClient() != nil, nil + } }) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) - // TODO(user): Add more specific assertions depending on your controller's reconciliation logic. - // Example: If you expect a certain status condition after reconciliation, verify it here. - }) - }) + // Check if the context timeout or another error occurred + gomega.Expect(err).ToNot(gomega.HaveOccurred(), "Manager failed to start within the timeout period") + + ginkgo.By("Waiting Reconcile of create event") + nonAdminRestore := buildTestNonAdminRestore(nonAdminRestoreNamespace, nonAdminRestoreName, scenario.spec) + gomega.Expect(k8sClient.Create(ctxTimeout, nonAdminRestore)).To(gomega.Succeed()) + // wait NonAdminRestore reconcile + time.Sleep(2 * time.Second) + + ginkgo.By("Fetching NonAdminRestore after Reconcile") + gomega.Expect(k8sClient.Get( + ctxTimeout, + types.NamespacedName{ + Name: nonAdminRestoreName, + Namespace: nonAdminRestoreNamespace, + }, + nonAdminRestore, + )).To(gomega.Succeed()) + + ginkgo.By("Validating NonAdminRestore Status") + + gomega.Expect(checkTestNonAdminRestoreStatus(nonAdminRestore, scenario.status)).To(gomega.Succeed()) + + if scenario.status.VeleroRestore != nil && len(nonAdminRestore.Status.UUID) > 0 { + ginkgo.By("Checking if NonAdminRestore Spec was not changed") + gomega.Expect(reflect.DeepEqual( + nonAdminRestore.Spec, + scenario.spec, + )).To(gomega.BeTrue()) + + ginkgo.By("Simulating Velero Restore update to finished state") + + veleroRestore := &velerov1.Restore{} + gomega.Expect(k8sClient.Get( + ctxTimeout, + types.NamespacedName{ + Name: nonAdminRestore.Status.VeleroRestore.Name, + Namespace: oadpNamespace, + }, + veleroRestore, + )).To(gomega.Succeed()) + + // can not call .Status().Update() for veleroRestore object https://github.com/vmware-tanzu/velero/issues/8285 + veleroRestore.Status = velerov1.RestoreStatus{ + Phase: velerov1.RestorePhaseCompleted, + } + gomega.Expect(k8sClient.Update(ctxTimeout, veleroRestore)).To(gomega.Succeed()) + + ginkgo.By("Velero Restore updated") + + // wait NonAdminRestore reconcile + gomega.Eventually(func() (bool, error) { + err := k8sClient.Get( + ctxTimeout, + types.NamespacedName{ + Name: nonAdminRestoreName, + Namespace: nonAdminRestoreNamespace, + }, + nonAdminRestore, + ) + if err != nil { + return false, err + } + if nonAdminRestore == nil || nonAdminRestore.Status.VeleroRestore == nil || nonAdminRestore.Status.VeleroRestore.Status == nil { + return false, nil + } + return nonAdminRestore.Status.VeleroRestore.Status.Phase == velerov1.RestorePhaseCompleted, nil + }, 5*time.Second, 1*time.Second).Should(gomega.BeTrue()) + } + + ginkgo.By("Waiting NonAdminRestore deletion") + gomega.Expect(k8sClient.Delete(ctxTimeout, nonAdminRestore)).To(gomega.Succeed()) + gomega.Eventually(func() (bool, error) { + err := k8sClient.Get( + ctxTimeout, + types.NamespacedName{ + Name: nonAdminRestoreName, + Namespace: nonAdminRestoreNamespace, + }, + nonAdminRestore, + ) + if apierrors.IsNotFound(err) { + return true, nil + } + return false, err + }, 10*time.Second, 1*time.Second).Should(gomega.BeTrue()) + }, + ginkgo.Entry("Should update NonAdminRestore until Velero Restore completes and then delete it", nonAdminRestoreFullReconcileScenario{ + spec: nacv1alpha1.NonAdminRestoreSpec{ + RestoreSpec: &velerov1.RestoreSpec{ + BackupName: "test", + }, + }, + backupStatus: nacv1alpha1.NonAdminBackupStatus{ + Phase: nacv1alpha1.NonAdminPhaseCreated, + VeleroBackup: &nacv1alpha1.VeleroBackup{ + Status: nil, + }, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + Reason: "BackupAccepted", + Message: "backup accepted", + LastTransitionTime: metav1.NewTime(time.Now()), + }, + { + Type: "Queued", + Status: metav1.ConditionTrue, + Reason: "BackupScheduled", + Message: "Created Velero Backup object", + LastTransitionTime: metav1.NewTime(time.Now()), + }, + }, + }, + status: nacv1alpha1.NonAdminRestoreStatus{ + Phase: nacv1alpha1.NonAdminPhaseCreated, + VeleroRestore: &nacv1alpha1.VeleroRestore{ + Status: nil, + }, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + Reason: "RestoreAccepted", + Message: "restore accepted", + }, + { + Type: "Queued", + Status: metav1.ConditionTrue, + Reason: "RestoreScheduled", + Message: "Created Velero Restore object", + }, + }, + }, + }), + ginkgo.Entry("Should update NonAdminRestore until it invalidates and then delete it", nonAdminRestoreFullReconcileScenario{ + spec: nacv1alpha1.NonAdminRestoreSpec{ + RestoreSpec: &velerov1.RestoreSpec{ + BackupName: "non-admin-backup-with-phase-backing-off", + }, + }, + backupStatus: nacv1alpha1.NonAdminBackupStatus{ + Phase: nacv1alpha1.NonAdminPhaseBackingOff, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionFalse, + Reason: "InvalidBackupSpec", + Message: "spec.backupSpec.IncludedNamespaces can not contain namespaces other than:", + LastTransitionTime: metav1.NewTime(time.Now()), + }, + }, + }, + status: nacv1alpha1.NonAdminRestoreStatus{ + Phase: nacv1alpha1.NonAdminPhaseBackingOff, + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionFalse, + Reason: "InvalidRestoreSpec", + Message: "NonAdminRestore spec.restoreSpec.backupName is invalid: ", + }, + }, + }, + }), + ) }) diff --git a/internal/handler/velerorestore_handler.go b/internal/handler/velerorestore_handler.go index 9603bf4..1e0b131 100644 --- a/internal/handler/velerorestore_handler.go +++ b/internal/handler/velerorestore_handler.go @@ -24,7 +24,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/reconcile" - // "github.com/migtools/oadp-non-admin/internal/common/constant" + "github.com/migtools/oadp-non-admin/internal/common/constant" "github.com/migtools/oadp-non-admin/internal/common/function" ) @@ -40,14 +40,13 @@ func (VeleroRestoreHandler) Create(_ context.Context, _ event.CreateEvent, _ wor func (VeleroRestoreHandler) Update(ctx context.Context, evt event.UpdateEvent, q workqueue.RateLimitingInterface) { logger := function.GetLogger(ctx, evt.ObjectNew, "VeleroRestoreHandler") - // TODO - // annotations := evt.ObjectNew.GetAnnotations() - // nabOriginNamespace := annotations[constant.NabOriginNamespaceAnnotation] - // nabOriginName := annotations[constant.NabOriginNameAnnotation] + annotations := evt.ObjectNew.GetAnnotations() + nonAdminRestoreName := annotations[constant.NonAdminRestoreOriginNameAnnotation] + nonAdminRestoreNamespace := annotations[constant.NonAdminRestoreOriginNamespaceAnnotation] q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ - Name: "", - Namespace: "", + Name: nonAdminRestoreName, + Namespace: nonAdminRestoreNamespace, }}) logger.V(1).Info("Handled Update event") } diff --git a/internal/predicate/compositerestore_predicate.go b/internal/predicate/compositerestore_predicate.go index d344ce7..6df0366 100644 --- a/internal/predicate/compositerestore_predicate.go +++ b/internal/predicate/compositerestore_predicate.go @@ -35,7 +35,7 @@ type CompositeRestorePredicate struct { // Create event filter only accepts NonAdminRestore create events func (p CompositeRestorePredicate) Create(evt event.CreateEvent) bool { switch evt.Object.(type) { - case *nacv1alpha1.NonAdminBackup: + case *nacv1alpha1.NonAdminRestore: return p.NonAdminRestorePredicate.Create(p.Context, evt) default: return false @@ -45,9 +45,9 @@ func (p CompositeRestorePredicate) Create(evt event.CreateEvent) bool { // Update event filter accepts both NonAdminRestore and Velero Restore update events func (p CompositeRestorePredicate) Update(evt event.UpdateEvent) bool { switch evt.ObjectNew.(type) { - case *nacv1alpha1.NonAdminBackup: + case *nacv1alpha1.NonAdminRestore: return p.NonAdminRestorePredicate.Update(p.Context, evt) - case *velerov1.Backup: + case *velerov1.Restore: return p.VeleroRestorePredicate.Update(p.Context, evt) default: return false @@ -57,7 +57,7 @@ func (p CompositeRestorePredicate) Update(evt event.UpdateEvent) bool { // Delete event filter only accepts NonAdminRestore delete events func (p CompositeRestorePredicate) Delete(evt event.DeleteEvent) bool { switch evt.Object.(type) { - case *nacv1alpha1.NonAdminBackup: + case *nacv1alpha1.NonAdminRestore: return p.NonAdminRestorePredicate.Delete(p.Context, evt) default: return false diff --git a/internal/predicate/nonadminrestore_predicate.go b/internal/predicate/nonadminrestore_predicate.go index e162216..361422a 100644 --- a/internal/predicate/nonadminrestore_predicate.go +++ b/internal/predicate/nonadminrestore_predicate.go @@ -31,14 +31,14 @@ type NonAdminRestorePredicate struct{} // Create event filter accepts all NonAdminRestore create events func (NonAdminRestorePredicate) Create(ctx context.Context, evt event.CreateEvent) bool { - logger := function.GetLogger(ctx, evt.Object, nonAdminBackupPredicateKey) + logger := function.GetLogger(ctx, evt.Object, nonAdminRestorePredicateKey) logger.V(1).Info("Accepted Create event") return true } // Update event filter only accepts NonAdminRestore update events that include generation change func (NonAdminRestorePredicate) Update(ctx context.Context, evt event.UpdateEvent) bool { - logger := function.GetLogger(ctx, evt.ObjectNew, nonAdminBackupPredicateKey) + logger := function.GetLogger(ctx, evt.ObjectNew, nonAdminRestorePredicateKey) if evt.ObjectNew.GetGeneration() != evt.ObjectOld.GetGeneration() { logger.V(1).Info("Accepted Update event") @@ -51,7 +51,7 @@ func (NonAdminRestorePredicate) Update(ctx context.Context, evt event.UpdateEven // Delete event filter accepts all NonAdminRestore delete events func (NonAdminRestorePredicate) Delete(ctx context.Context, evt event.DeleteEvent) bool { - logger := function.GetLogger(ctx, evt.Object, nonAdminBackupPredicateKey) + logger := function.GetLogger(ctx, evt.Object, nonAdminRestorePredicateKey) logger.V(1).Info("Accepted Delete event") return true } diff --git a/internal/predicate/velerorestore_predicate.go b/internal/predicate/velerorestore_predicate.go index 518350d..f8bfeab 100644 --- a/internal/predicate/velerorestore_predicate.go +++ b/internal/predicate/velerorestore_predicate.go @@ -36,8 +36,7 @@ func (p VeleroRestorePredicate) Update(ctx context.Context, evt event.UpdateEven namespace := evt.ObjectNew.GetNamespace() if namespace == p.OADPNamespace { - // TODO - if function.CheckVeleroBackupMetadata(evt.ObjectNew) { + if function.CheckVeleroRestoreMetadata(evt.ObjectNew) { logger.V(1).Info("Accepted Update event") return true } From 0603e982e04982fe3d6df2fe2438cea4dc718d15 Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Tue, 19 Nov 2024 11:16:31 -0300 Subject: [PATCH 06/14] fixup! feat: Add NAC restore controller add more tests Signed-off-by: Mateus Oliveira --- internal/common/function/function.go | 4 + internal/common/function/function_test.go | 98 +++++++++++++++++++ .../nonadminrestore_controller_test.go | 49 ++++++++++ 3 files changed, 151 insertions(+) diff --git a/internal/common/function/function.go b/internal/common/function/function.go index 5373c1b..2472534 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -114,6 +114,10 @@ func ValidateBackupSpec(nonAdminBackup *nacv1alpha1.NonAdminBackup, enforcedBack } func ValidateRestoreSpec(ctx context.Context, clientInstance client.Client, nonAdminRestore *nacv1alpha1.NonAdminRestore) error { + if nonAdminRestore.Spec.RestoreSpec.BackupName == constant.EmptyString { + return fmt.Errorf("NonAdminRestore spec.restoreSpec.backupName is not set") + } + nab := &nacv1alpha1.NonAdminBackup{} err := clientInstance.Get(ctx, types.NamespacedName{ Name: nonAdminRestore.Spec.RestoreSpec.BackupName, diff --git a/internal/common/function/function_test.go b/internal/common/function/function_test.go index 1270d8b..d14fe8a 100644 --- a/internal/common/function/function_test.go +++ b/internal/common/function/function_test.go @@ -375,6 +375,104 @@ func TestValidateBackupSpecEnforcedFields(t *testing.T) { }) } +func TestValidateRestoreSpec(t *testing.T) { + tests := []struct { + name string + errorMessage string + nonAdminRestore *nacv1alpha1.NonAdminRestore + objects []client.Object + }{ + { + name: "[invalid] spec.restoreSpec.backupName not set", + nonAdminRestore: &nacv1alpha1.NonAdminRestore{ + Spec: nacv1alpha1.NonAdminRestoreSpec{ + RestoreSpec: &velerov1.RestoreSpec{}, + }, + }, + errorMessage: "NonAdminRestore spec.restoreSpec.backupName is not set", + }, + { + name: "[invalid] spec.restoreSpec.backupName does not exist", + nonAdminRestore: &nacv1alpha1.NonAdminRestore{ + Spec: nacv1alpha1.NonAdminRestoreSpec{ + RestoreSpec: &velerov1.RestoreSpec{ + BackupName: "john", + }, + }, + }, + errorMessage: "NonAdminRestore spec.restoreSpec.backupName is invalid: nonadminbackups.oadp.openshift.io \"john\" not found", + }, + { + name: "[invalid] spec.restoreSpec.backupName not ready", + nonAdminRestore: &nacv1alpha1.NonAdminRestore{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultStr, + }, + Spec: nacv1alpha1.NonAdminRestoreSpec{ + RestoreSpec: &velerov1.RestoreSpec{ + BackupName: "peter", + }, + }, + }, + objects: []client.Object{ + &nacv1alpha1.NonAdminBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "peter", + Namespace: defaultStr, + }, + Status: nacv1alpha1.NonAdminBackupStatus{ + Phase: nacv1alpha1.NonAdminPhaseBackingOff, + }, + }, + }, + errorMessage: "NonAdminRestore spec.restoreSpec.backupName is invalid: NonAdminBackup is not ready to be restored", + }, + { + name: "[valid] spec.restoreSpec.backupName is ready", + nonAdminRestore: &nacv1alpha1.NonAdminRestore{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultStr, + }, + Spec: nacv1alpha1.NonAdminRestoreSpec{ + RestoreSpec: &velerov1.RestoreSpec{ + BackupName: "steve", + }, + }, + }, + objects: []client.Object{ + &nacv1alpha1.NonAdminBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "steve", + Namespace: defaultStr, + }, + Status: nacv1alpha1.NonAdminBackupStatus{ + Phase: nacv1alpha1.NonAdminPhaseCreated, + }, + }, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fakeScheme := runtime.NewScheme() + if err := nacv1alpha1.AddToScheme(fakeScheme); err != nil { + t.Fatalf("Failed to register NAC type: %v", err) + } + fakeClient := fake.NewClientBuilder().WithScheme(fakeScheme).WithObjects(test.objects...).Build() + err := ValidateRestoreSpec(context.Background(), fakeClient, test.nonAdminRestore) + if err != nil { + if test.errorMessage != err.Error() { + t.Errorf("test '%s' failed: error messages differ. Expected %v, got %v", test.name, test.errorMessage, err) + } + return + } + if test.errorMessage != constant.EmptyString { + t.Errorf("test '%s' failed: expected test to error with '%v'", test.name, err) + } + }) + } +} + func TestGenerateNacObjectNameWithUUID(t *testing.T) { tests := []struct { name string diff --git a/internal/controller/nonadminrestore_controller_test.go b/internal/controller/nonadminrestore_controller_test.go index df9ca62..388c13d 100644 --- a/internal/controller/nonadminrestore_controller_test.go +++ b/internal/controller/nonadminrestore_controller_test.go @@ -26,6 +26,7 @@ import ( "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -36,6 +37,10 @@ import ( "github.com/migtools/oadp-non-admin/internal/common/constant" ) +type nonAdminRestoreClusterValidationScenario struct { + spec nacv1alpha1.NonAdminRestoreSpec +} + type nonAdminRestoreFullReconcileScenario struct { spec nacv1alpha1.NonAdminRestoreSpec status nacv1alpha1.NonAdminRestoreStatus @@ -96,6 +101,50 @@ func checkTestNonAdminRestoreStatus(nonAdminRestore *nacv1alpha1.NonAdminRestore return nil } +var _ = ginkgo.Describe("Test NonAdminRestore in cluster validation", func() { + var ( + ctx context.Context + nonAdminRestoreName string + nonAdminRestoreNamespace string + counter int + ) + + ginkgo.BeforeEach(func() { + ctx = context.Background() + counter++ + nonAdminRestoreName = fmt.Sprintf("non-admin-restore-object-%v", counter) + nonAdminRestoreNamespace = fmt.Sprintf("test-non-admin-restore-cluster-validation-%v", counter) + + nonAdminNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: nonAdminRestoreNamespace, + }, + } + gomega.Expect(k8sClient.Create(ctx, nonAdminNamespace)).To(gomega.Succeed()) + }) + + ginkgo.AfterEach(func() { + nonAdminNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: nonAdminRestoreNamespace, + }, + } + gomega.Expect(k8sClient.Delete(ctx, nonAdminNamespace)).To(gomega.Succeed()) + }) + + ginkgo.DescribeTable("Validation is false", + func(scenario nonAdminRestoreClusterValidationScenario) { + nonAdminRestore := buildTestNonAdminRestore(nonAdminRestoreNamespace, nonAdminRestoreName, scenario.spec) + err := k8sClient.Create(ctx, nonAdminRestore) + gomega.Expect(err).To(gomega.HaveOccurred()) + gomega.Expect(err.Error()).To(gomega.ContainSubstring("Required value")) + }, + ginkgo.Entry("Should NOT create NonAdminRestore without spec.restoreSpec", nonAdminRestoreClusterValidationScenario{ + spec: nacv1alpha1.NonAdminRestoreSpec{}, + }), + ) +}) + var _ = ginkgo.Describe("Test full reconcile loop of NonAdminRestore Controller", func() { var ( ctx context.Context From b7d988dfd08099cef2870c67d807172697f14fd7 Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Tue, 19 Nov 2024 12:36:34 -0300 Subject: [PATCH 07/14] fixup! feat: Add NAC restore controller add more tests Signed-off-by: Mateus Oliveira --- .../nonadminrestore_controller_test.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/internal/controller/nonadminrestore_controller_test.go b/internal/controller/nonadminrestore_controller_test.go index 388c13d..df006e1 100644 --- a/internal/controller/nonadminrestore_controller_test.go +++ b/internal/controller/nonadminrestore_controller_test.go @@ -236,6 +236,7 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminRestore Controller" gomega.Expect(checkTestNonAdminRestoreStatus(nonAdminRestore, scenario.status)).To(gomega.Succeed()) + veleroRestore := &velerov1.Restore{} if scenario.status.VeleroRestore != nil && len(nonAdminRestore.Status.UUID) > 0 { ginkgo.By("Checking if NonAdminRestore Spec was not changed") gomega.Expect(reflect.DeepEqual( @@ -245,7 +246,6 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminRestore Controller" ginkgo.By("Simulating Velero Restore update to finished state") - veleroRestore := &velerov1.Restore{} gomega.Expect(k8sClient.Get( ctxTimeout, types.NamespacedName{ @@ -299,6 +299,22 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminRestore Controller" } return false, err }, 10*time.Second, 1*time.Second).Should(gomega.BeTrue()) + if scenario.status.VeleroRestore != nil && len(nonAdminRestore.Status.UUID) > 0 { + gomega.Eventually(func() (bool, error) { + err := k8sClient.Get( + ctxTimeout, + types.NamespacedName{ + Name: nonAdminRestore.Status.VeleroRestore.Name, + Namespace: oadpNamespace, + }, + veleroRestore, + ) + if apierrors.IsNotFound(err) { + return true, nil + } + return false, err + }, 10*time.Second, 1*time.Second).Should(gomega.BeTrue()) + } }, ginkgo.Entry("Should update NonAdminRestore until Velero Restore completes and then delete it", nonAdminRestoreFullReconcileScenario{ spec: nacv1alpha1.NonAdminRestoreSpec{ From a57ed1e76e9d3de2a226a54ea7a65941df39080b Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Tue, 19 Nov 2024 15:32:48 -0300 Subject: [PATCH 08/14] fixup! feat: Add NAC restore controller Signed-off-by: Mateus Oliveira --- internal/common/constant/constant.go | 9 +++---- internal/common/function/function.go | 24 +++++++++---------- .../controller/nonadminrestore_controller.go | 12 ++++------ 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/internal/common/constant/constant.go b/internal/common/constant/constant.go index 91efef1..7325bed 100644 --- a/internal/common/constant/constant.go +++ b/internal/common/constant/constant.go @@ -61,15 +61,12 @@ const NamespaceString = "Namespace" // must be below 63 characters, because it's used within object Label Value const MaximumNacObjectNameLength = validation.DNS1123LabelMaxLength -// NonAdminCondition are used for more detailed information supporting NonAdminPhase state. -type NonAdminCondition string - // Predefined conditions for NonAdmin object. // One NonAdmin object may have multiple conditions. // It is more granular knowledge of the NonAdmin object and represents the // array of the conditions through which the NonAdmin object has or has not passed const ( - NonAdminConditionAccepted NonAdminCondition = "Accepted" - NonAdminConditionQueued NonAdminCondition = "Queued" - NonAdminConditionDeleting NonAdminCondition = "Deleting" + NonAdminConditionAccepted = "Accepted" + NonAdminConditionQueued = "Queued" + NonAdminConditionDeleting = "Deleting" ) diff --git a/internal/common/function/function.go b/internal/common/function/function.go index 2472534..5af6099 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -187,18 +187,18 @@ func GenerateNacObjectUUID(namespace, nacName string) string { return nacObjectName } -func GetGenerateNamePrefix(namespace string, name string) string { - remainingLength := constant.MaximumNacObjectNameLength - 5 - len("--") - if len(namespace+name) > remainingLength { - if len(namespace) >= remainingLength { - return fmt.Sprintf("%s-", namespace[:remainingLength]) - } - remainingLength = remainingLength - len(namespace) - return fmt.Sprintf("%s-%s-", name[:remainingLength], namespace) - } - - return fmt.Sprintf("%s-%s-", namespace, name) -} +// func GetGenerateNamePrefix(namespace string, name string) string { +// remainingLength := constant.MaximumNacObjectNameLength - 5 - len("--") +// if len(namespace+name) > remainingLength { +// if len(namespace) >= remainingLength { +// return fmt.Sprintf("%s-", namespace[:remainingLength]) +// } +// remainingLength = remainingLength - len(namespace) +// return fmt.Sprintf("%s-%s-", name[:remainingLength], namespace) +// } + +// return fmt.Sprintf("%s-%s-", namespace, name) +// } // ListObjectsByLabel retrieves a list of Kubernetes objects in a specified namespace // that match a given label key-value pair. diff --git a/internal/controller/nonadminrestore_controller.go b/internal/controller/nonadminrestore_controller.go index d189f4e..12216ef 100644 --- a/internal/controller/nonadminrestore_controller.go +++ b/internal/controller/nonadminrestore_controller.go @@ -245,14 +245,10 @@ func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, log veleroRestore = &velerov1.Restore{ ObjectMeta: metav1.ObjectMeta{ - // TODO even though Kubernetes object name can be up to 253 char length - // using generate name, object name will be 63 char length - // it will add a random 5 char length to GenerateName prefix - // if prefix is more than 58 char length, it is truncated - GenerateName: function.GetGenerateNamePrefix(nar.Namespace, nar.Name), - Namespace: r.OADPNamespace, - Labels: function.GetNonAdminRestoreLabels(nar.Status.UUID), - Annotations: function.GetNonAdminRestoreAnnotations(nar.ObjectMeta), + Name: function.GenerateNacObjectUUID(nar.Namespace, nar.Name), + Namespace: r.OADPNamespace, + Labels: function.GetNonAdminRestoreLabels(nar.Status.UUID), + Annotations: function.GetNonAdminRestoreAnnotations(nar.ObjectMeta), }, Spec: *restoreSpec, } From aeaa58b30ea1a12da3662a048c9b3ac7410f0b3f Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Tue, 19 Nov 2024 16:40:37 -0300 Subject: [PATCH 09/14] fixup! feat: Add NAC restore controller Signed-off-by: Mateus Oliveira --- .../controller/nonadminrestore_controller.go | 134 ++++++++++-------- internal/handler/velerorestore_handler.go | 14 +- .../predicate/compositerestore_predicate.go | 2 + internal/predicate/velerorestore_predicate.go | 17 +++ 4 files changed, 107 insertions(+), 60 deletions(-) diff --git a/internal/controller/nonadminrestore_controller.go b/internal/controller/nonadminrestore_controller.go index 12216ef..e9808ce 100644 --- a/internal/controller/nonadminrestore_controller.go +++ b/internal/controller/nonadminrestore_controller.go @@ -19,7 +19,6 @@ package controller import ( "context" "reflect" - "time" "github.com/go-logr/logr" "github.com/google/uuid" @@ -51,6 +50,8 @@ type NonAdminRestoreReconciler struct { type nonAdminRestoreReconcileStepFunction func(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error +const nonAdminRestoreStatusUpdateFailureMessage = "Failed to update NonAdminRestore Status" + // +kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores/status,verbs=get;update;patch // +kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores/finalizers,verbs=update @@ -75,58 +76,22 @@ func (r *NonAdminRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, err } - if !nar.DeletionTimestamp.IsZero() { - updatedPhase := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseDeleting) - updatedCondition := meta.SetStatusCondition(&nar.Status.Conditions, - metav1.Condition{ - Type: string(constant.NonAdminConditionDeleting), - Status: metav1.ConditionTrue, - Reason: "DeletionPending", - Message: "restore accepted for deletion", - }, - ) - if updatedPhase || updatedCondition { - if err := r.Status().Update(ctx, nar); err != nil { - logger.Error(err, "Failed to update NonAdminRestore Status") - return ctrl.Result{}, err - } - logger.V(1).Info("NonAdminRestore status marked for deletion") - } else { - logger.V(1).Info("NonAdminRestore status unchanged during deletion") + var reconcileSteps []nonAdminRestoreReconcileStepFunction + switch { + case !nar.DeletionTimestamp.IsZero(): + logger.V(1).Info("Executing delete path") + reconcileSteps = []nonAdminRestoreReconcileStepFunction{ + r.delete, } - - veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, nar.Status.UUID) - if err == nil { - // TODO any problem in calling delete on something being deleted? - err = r.Delete(ctx, veleroRestore) - if err != nil { - logger.Error(err, "Unable to delete Velero Restore") - return ctrl.Result{}, err - } - // wait Velero delete restore object - return ctrl.Result{RequeueAfter: 5 * time.Second}, nil + default: + logger.V(1).Info("Executing creation/update path") + reconcileSteps = []nonAdminRestoreReconcileStepFunction{ + r.init, + r.validateSpec, + r.setUUID, + r.setFinalizer, + r.createVeleroRestore, } - if !apierrors.IsNotFound(err) { - logger.Error(err, "Unable to fetch Velero Restore") - return ctrl.Result{}, err - } - - controllerutil.RemoveFinalizer(nar, constant.NonAdminRestoreFinalizerName) - // TODO does this change generation? need to refetch? - if err := r.Update(ctx, nar); err != nil { - logger.Error(err, "Unable to remove NonAdminRestore finalizer") - return ctrl.Result{}, err - } - logger.V(1).Info("NonAdminRestore Reconcile exit") - return ctrl.Result{}, nil - } - - reconcileSteps := []nonAdminRestoreReconcileStepFunction{ - r.init, - r.validateSpec, - r.setUUID, - r.setFinalizer, - r.createVeleroRestore, } for _, step := range reconcileSteps { err := step(ctx, logger, nar) @@ -138,11 +103,56 @@ func (r *NonAdminRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } +func (r *NonAdminRestoreReconciler) delete(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { + updatedPhase := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseDeleting) + updatedCondition := meta.SetStatusCondition(&nar.Status.Conditions, + metav1.Condition{ + Type: string(constant.NonAdminConditionDeleting), + Status: metav1.ConditionTrue, + Reason: "DeletionPending", + Message: "restore accepted for deletion", + }, + ) + if updatedPhase || updatedCondition { + if err := r.Status().Update(ctx, nar); err != nil { + logger.Error(err, nonAdminRestoreStatusUpdateFailureMessage) + return err + } + logger.V(1).Info("NonAdminRestore status marked for deletion") + } else { + logger.V(1).Info("NonAdminRestore status unchanged during deletion") + } + + veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, nar.Status.UUID) + if err == nil { + // TODO any problem in calling delete on something being deleted? + err = r.Delete(ctx, veleroRestore) + if err != nil { + logger.Error(err, "Unable to delete Velero Restore") + return err + } + logger.V(1).Info("Waiting Velero Restore be deleted") + return nil + } + if !apierrors.IsNotFound(err) { + logger.Error(err, "Unable to fetch Velero Restore") + return err + } + + controllerutil.RemoveFinalizer(nar, constant.NonAdminRestoreFinalizerName) + // TODO does this change generation? need to refetch? + if err := r.Update(ctx, nar); err != nil { + logger.Error(err, "Unable to remove NonAdminRestore finalizer") + return err + } + return nil +} + func (r *NonAdminRestoreReconciler) init(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { if nar.Status.Phase == constant.EmptyString { if updated := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseNew); updated { if err := r.Status().Update(ctx, nar); err != nil { - logger.Error(err, "Failed to update NonAdminRestore Status") + logger.Error(err, nonAdminRestoreStatusUpdateFailureMessage) return err } logger.V(1).Info("NonAdminRestore Phase set to New") @@ -167,7 +177,7 @@ func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger log ) if updatedPhase || updatedCondition { if updateErr := r.Status().Update(ctx, nar); updateErr != nil { - logger.Error(updateErr, "Failed to update NonAdminRestore Status") + logger.Error(updateErr, nonAdminRestoreStatusUpdateFailureMessage) return updateErr } } @@ -185,7 +195,7 @@ func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger log ) if updated { if err := r.Status().Update(ctx, nar); err != nil { - logger.Error(err, "Failed to update NonAdminRestore Status") + logger.Error(err, nonAdminRestoreStatusUpdateFailureMessage) return err } logger.V(1).Info("NonAdminRestore condition set to Accepted") @@ -216,9 +226,9 @@ func (r *NonAdminRestoreReconciler) setFinalizer(ctx context.Context, logger log return err } // TODO does this change generation? need to refetch? - logger.V(1).Info("Finalizer added to NonAdminRestore", "finalizer", constant.NonAdminRestoreFinalizerName) + logger.V(1).Info("Finalizer added to NonAdminRestore") } else { - logger.V(1).Info("Finalizer already exists on NonAdminRestore", "finalizer", constant.NonAdminRestoreFinalizerName) + logger.V(1).Info("Finalizer already exists on NonAdminRestore") } return nil } @@ -230,6 +240,15 @@ func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, log if !apierrors.IsNotFound(err) { return err } + if nar.Status.Phase == nacv1alpha1.NonAdminPhaseCreated { + logger.V(1).Info("Velero Restore was deleted, deleting NonAdminRestore") + err = r.delete(ctx, logger, nar) + if err != nil { + logger.Error(err, "Failed to delete NonAdminRestore") + return err + } + return nil + } restoreSpec := nar.Spec.RestoreSpec.DeepCopy() @@ -270,11 +289,10 @@ func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, log Message: "Created Velero Restore object", }, ) - // TODO need to refetch velero restore because of generate name? updatedVeleroStatus := updateVeleroRestoreStatus(&nar.Status, veleroRestore) if updatedPhase || updatedCondition || updatedVeleroStatus { if err := r.Status().Update(ctx, nar); err != nil { - logger.Error(err, "Failed to update NonAdminRestore Status") + logger.Error(err, nonAdminRestoreStatusUpdateFailureMessage) return err } logger.V(1).Info("NonAdminRestore Status updated successfully") diff --git a/internal/handler/velerorestore_handler.go b/internal/handler/velerorestore_handler.go index 1e0b131..a757cde 100644 --- a/internal/handler/velerorestore_handler.go +++ b/internal/handler/velerorestore_handler.go @@ -52,8 +52,18 @@ func (VeleroRestoreHandler) Update(ctx context.Context, evt event.UpdateEvent, q } // Delete event handler -func (VeleroRestoreHandler) Delete(_ context.Context, _ event.DeleteEvent, _ workqueue.RateLimitingInterface) { - // Delete event handler for the Restore object +func (VeleroRestoreHandler) Delete(ctx context.Context, evt event.DeleteEvent, q workqueue.RateLimitingInterface) { + logger := function.GetLogger(ctx, evt.Object, "VeleroRestoreHandler") + + annotations := evt.Object.GetAnnotations() + nonAdminRestoreName := annotations[constant.NonAdminRestoreOriginNameAnnotation] + nonAdminRestoreNamespace := annotations[constant.NonAdminRestoreOriginNamespaceAnnotation] + + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Name: nonAdminRestoreName, + Namespace: nonAdminRestoreNamespace, + }}) + logger.V(1).Info("Handled Delete event") } // Generic event handler diff --git a/internal/predicate/compositerestore_predicate.go b/internal/predicate/compositerestore_predicate.go index 6df0366..ce02d85 100644 --- a/internal/predicate/compositerestore_predicate.go +++ b/internal/predicate/compositerestore_predicate.go @@ -59,6 +59,8 @@ func (p CompositeRestorePredicate) Delete(evt event.DeleteEvent) bool { switch evt.Object.(type) { case *nacv1alpha1.NonAdminRestore: return p.NonAdminRestorePredicate.Delete(p.Context, evt) + case *velerov1.Restore: + return p.VeleroRestorePredicate.Delete(p.Context, evt) default: return false } diff --git a/internal/predicate/velerorestore_predicate.go b/internal/predicate/velerorestore_predicate.go index f8bfeab..8dfdae8 100644 --- a/internal/predicate/velerorestore_predicate.go +++ b/internal/predicate/velerorestore_predicate.go @@ -45,3 +45,20 @@ func (p VeleroRestorePredicate) Update(ctx context.Context, evt event.UpdateEven logger.V(1).Info("Rejected Update event") return false } + +// Delete event filter only accepts Velero Restore delete events from OADP namespace +// and from Velero Restores that have required metadata +func (p VeleroRestorePredicate) Delete(ctx context.Context, evt event.DeleteEvent) bool { + logger := function.GetLogger(ctx, evt.Object, "VeleroRestorePredicate") + + namespace := evt.Object.GetNamespace() + if namespace == p.OADPNamespace { + if function.CheckVeleroRestoreMetadata(evt.Object) { + logger.V(1).Info("Accepted Delete event") + return true + } + } + + logger.V(1).Info("Rejected Delete event") + return false +} From f4f1757d899e4d8ad82b087903bd427d68b1ffc4 Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Tue, 19 Nov 2024 16:47:38 -0300 Subject: [PATCH 10/14] fixup! feat: Add NAC restore controller Signed-off-by: Mateus Oliveira --- internal/common/function/function.go | 13 ------------- internal/controller/nonadminrestore_controller.go | 1 - 2 files changed, 14 deletions(-) diff --git a/internal/common/function/function.go b/internal/common/function/function.go index 5af6099..17fc6ec 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -187,19 +187,6 @@ func GenerateNacObjectUUID(namespace, nacName string) string { return nacObjectName } -// func GetGenerateNamePrefix(namespace string, name string) string { -// remainingLength := constant.MaximumNacObjectNameLength - 5 - len("--") -// if len(namespace+name) > remainingLength { -// if len(namespace) >= remainingLength { -// return fmt.Sprintf("%s-", namespace[:remainingLength]) -// } -// remainingLength = remainingLength - len(namespace) -// return fmt.Sprintf("%s-%s-", name[:remainingLength], namespace) -// } - -// return fmt.Sprintf("%s-%s-", namespace, name) -// } - // ListObjectsByLabel retrieves a list of Kubernetes objects in a specified namespace // that match a given label key-value pair. func ListObjectsByLabel(ctx context.Context, clientInstance client.Client, namespace string, labelKey string, labelValue string, objectList client.ObjectList) error { diff --git a/internal/controller/nonadminrestore_controller.go b/internal/controller/nonadminrestore_controller.go index e9808ce..100296f 100644 --- a/internal/controller/nonadminrestore_controller.go +++ b/internal/controller/nonadminrestore_controller.go @@ -125,7 +125,6 @@ func (r *NonAdminRestoreReconciler) delete(ctx context.Context, logger logr.Logg veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, nar.Status.UUID) if err == nil { - // TODO any problem in calling delete on something being deleted? err = r.Delete(ctx, veleroRestore) if err != nil { logger.Error(err, "Unable to delete Velero Restore") From 9c598ca437fdbdab27d1f74db96898bbc559dbbb Mon Sep 17 00:00:00 2001 From: Michal Pryc Date: Wed, 27 Nov 2024 21:31:14 +0100 Subject: [PATCH 11/14] Fix linting errors in the restore controller Few lint errors addressed. Signed-off-by: Michal Pryc --- internal/common/function/function.go | 7 +++++++ internal/controller/nonadminrestore_controller.go | 7 +++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/internal/common/function/function.go b/internal/common/function/function.go index 17fc6ec..b4cdc22 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -46,6 +46,7 @@ func GetNonAdminLabels() map[string]string { } } +// GetNonAdminRestoreLabels return the required Non Admin restore labels func GetNonAdminRestoreLabels(uniqueIdentifier string) map[string]string { nonAdminLabels := GetNonAdminLabels() nonAdminLabels[constant.NarOriginNACUUIDLabel] = uniqueIdentifier @@ -60,6 +61,7 @@ func GetNonAdminBackupAnnotations(objectMeta metav1.ObjectMeta) map[string]strin } } +// GetNonAdminRestoreAnnotations return the required Non Admin restore annotations func GetNonAdminRestoreAnnotations(objectMeta metav1.ObjectMeta) map[string]string { return map[string]string{ constant.NonAdminRestoreOriginNamespaceAnnotation: objectMeta.Namespace, @@ -113,6 +115,7 @@ func ValidateBackupSpec(nonAdminBackup *nacv1alpha1.NonAdminBackup, enforcedBack return nil } +// ValidateRestoreSpec return nil, if NonAdminRestore is valid; error otherwise func ValidateRestoreSpec(ctx context.Context, clientInstance client.Client, nonAdminRestore *nacv1alpha1.NonAdminRestore) error { if nonAdminRestore.Spec.RestoreSpec.BackupName == constant.EmptyString { return fmt.Errorf("NonAdminRestore spec.restoreSpec.backupName is not set") @@ -323,6 +326,9 @@ func GetVeleroDeleteBackupRequestByLabel(ctx context.Context, clientInstance cli } } +// GetVeleroRestoreByLabel retrieves a VeleroRestore object based on a specified label within a given namespace. +// It returns the VeleroRestore only when exactly one object is found, throws an error if multiple restores are found, +// or returns nil if no matches are found. func GetVeleroRestoreByLabel(ctx context.Context, clientInstance client.Client, namespace string, labelValue string) (*velerov1.Restore, error) { veleroRestoreList := &velerov1.RestoreList{} if err := ListObjectsByLabel(ctx, clientInstance, namespace, constant.NarOriginNACUUIDLabel, labelValue, veleroRestoreList); err != nil { @@ -364,6 +370,7 @@ func CheckVeleroBackupMetadata(obj client.Object) bool { return true } +// CheckVeleroRestoreMetadata return true if Velero Restore object has required Non Admin labels and annotations, false otherwise func CheckVeleroRestoreMetadata(obj client.Object) bool { objLabels := obj.GetLabels() if !checkLabelValue(objLabels, constant.OadpLabel, constant.OadpLabelValue) { diff --git a/internal/controller/nonadminrestore_controller.go b/internal/controller/nonadminrestore_controller.go index 100296f..703bb12 100644 --- a/internal/controller/nonadminrestore_controller.go +++ b/internal/controller/nonadminrestore_controller.go @@ -253,10 +253,13 @@ func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, log // TODO already did this get call in ValidateRestoreSpec! nab := &nacv1alpha1.NonAdminBackup{} - _ = r.Get(ctx, types.NamespacedName{ + if err = r.Get(ctx, types.NamespacedName{ Name: nar.Spec.RestoreSpec.BackupName, Namespace: nar.Namespace, - }, nab) + }, nab); err != nil { + logger.Error(err, "Failed to get NonAdminBackup") + return err + } restoreSpec.BackupName = nab.Status.VeleroBackup.Name // TODO enforce Restore Spec From 945378d691eb8300c4d8c804740e435d9958db60 Mon Sep 17 00:00:00 2001 From: Michal Pryc Date: Thu, 28 Nov 2024 13:52:56 +0100 Subject: [PATCH 12/14] Make Restore Controller similar to Backup Contoller This change makes the velero restore controller similar to velero backup controller in the way how the events are handled as well conditions and deletion paths with waiting for the restore objects to be removed before removing final nar object. Signed-off-by: Michal Pryc --- api/v1alpha1/nonadminrestore_types.go | 7 +- .../oadp.openshift.io_nonadminrestores.yaml | 6 +- internal/common/function/function.go | 3 +- .../controller/nonadminrestore_controller.go | 270 +++++++++++------- .../nonadminrestore_controller_test.go | 8 +- 5 files changed, 187 insertions(+), 107 deletions(-) diff --git a/api/v1alpha1/nonadminrestore_types.go b/api/v1alpha1/nonadminrestore_types.go index 9a0fd7f..3d53628 100644 --- a/api/v1alpha1/nonadminrestore_types.go +++ b/api/v1alpha1/nonadminrestore_types.go @@ -37,6 +37,10 @@ type VeleroRestore struct { // +optional Name string `json:"name,omitempty"` + // nacuuid references the Velero Restore object by it's label containing same NACUUID. + // +optional + NACUUID string `json:"nacuuid,omitempty"` + // namespace references the Namespace in which Velero Restore exists. // +optional Namespace string `json:"namespace,omitempty"` @@ -47,9 +51,6 @@ type NonAdminRestoreStatus struct { // +optional VeleroRestore *VeleroRestore `json:"veleroRestore,omitempty"` - // +optional - UUID string `json:"uuid,omitempty"` - // phase is a simple one high-level summary of the lifecycle of an NonAdminRestore. Phase NonAdminPhase `json:"phase,omitempty"` diff --git a/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml b/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml index 8b23692..29d48d2 100644 --- a/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml +++ b/config/crd/bases/oadp.openshift.io_nonadminrestores.yaml @@ -537,12 +537,14 @@ spec: - Created - Deleting type: string - uuid: - type: string veleroRestore: description: VeleroRestore contains information of the related Velero restore object. properties: + nacuuid: + description: nacuuid references the Velero Restore object by it's + label containing same NACUUID. + type: string name: description: references the Velero Restore object by it's name. type: string diff --git a/internal/common/function/function.go b/internal/common/function/function.go index b4cdc22..dbf17eb 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -26,7 +26,6 @@ import ( "github.com/go-logr/logr" "github.com/google/uuid" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" @@ -337,7 +336,7 @@ func GetVeleroRestoreByLabel(ctx context.Context, clientInstance client.Client, switch len(veleroRestoreList.Items) { case 0: - return nil, apierrors.NewNotFound(velerov1.Resource("restores"), "") + return nil, nil // No matching VeleroRestores found case 1: return &veleroRestoreList.Items[0], nil default: diff --git a/internal/controller/nonadminrestore_controller.go b/internal/controller/nonadminrestore_controller.go index 703bb12..ac63150 100644 --- a/internal/controller/nonadminrestore_controller.go +++ b/internal/controller/nonadminrestore_controller.go @@ -18,10 +18,10 @@ package controller import ( "context" + "errors" "reflect" "github.com/go-logr/logr" - "github.com/google/uuid" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -48,9 +48,13 @@ type NonAdminRestoreReconciler struct { OADPNamespace string } -type nonAdminRestoreReconcileStepFunction func(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error +type nonAdminRestoreReconcileStepFunction func(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) -const nonAdminRestoreStatusUpdateFailureMessage = "Failed to update NonAdminRestore Status" +const ( + nonAdminRestoreStatusUpdateFailureMessage = "Failed to update NonAdminRestore Status" + veleroRestoreReferenceUpdated = "NonAdminRestore - Status Updated with UUID reference" + findSingleVRError = "Error encountered while retrieving VeleroRestore for NAR" +) // +kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=oadp.openshift.io,resources=nonadminrestores/status,verbs=get;update;patch @@ -77,11 +81,14 @@ func (r *NonAdminRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Requ } var reconcileSteps []nonAdminRestoreReconcileStepFunction + switch { case !nar.DeletionTimestamp.IsZero(): logger.V(1).Info("Executing delete path") reconcileSteps = []nonAdminRestoreReconcileStepFunction{ - r.delete, + r.setStatusAndConditionForDeletionAndCallDelete, + r.deleteVeleroRestore, + r.removeNarFinalizerUponVeleroRestoreDeletion, } default: logger.V(1).Info("Executing creation/update path") @@ -93,17 +100,34 @@ func (r *NonAdminRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Requ r.createVeleroRestore, } } + + // Execute the selected reconciliation steps for _, step := range reconcileSteps { - err := step(ctx, logger, nar) + requeue, err := step(ctx, logger, nar) if err != nil { return ctrl.Result{}, err + } else if requeue { + return ctrl.Result{Requeue: true}, nil } } + logger.V(1).Info("NonAdminRestore Reconcile exit") return ctrl.Result{}, nil } -func (r *NonAdminRestoreReconciler) delete(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { +// setStatusAndConditionForDeletionAndCallDelete updates the NonAdminBackup status and conditions +// to reflect that deletion has been initiated, and triggers the actual deletion if needed. +// +// Parameters: +// - ctx: Context for managing request lifetime. +// - logger: Logger instance for logging messages. +// - nab: Pointer to the NonAdminBackup object being processed. +// +// Returns: +// - bool: true if reconciliation should be requeued, false otherwise +// - error: any error encountered during the process +func (r *NonAdminRestoreReconciler) setStatusAndConditionForDeletionAndCallDelete(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { + requeueRequired := false updatedPhase := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseDeleting) updatedCondition := meta.SetStatusCondition(&nar.Status.Conditions, metav1.Condition{ @@ -115,54 +139,103 @@ func (r *NonAdminRestoreReconciler) delete(ctx context.Context, logger logr.Logg ) if updatedPhase || updatedCondition { if err := r.Status().Update(ctx, nar); err != nil { - logger.Error(err, nonAdminRestoreStatusUpdateFailureMessage) - return err + logger.Error(err, statusUpdateError) + return false, err } logger.V(1).Info("NonAdminRestore status marked for deletion") + requeueRequired = true // Requeue to ensure latest NAR object in the next reconciliation steps } else { logger.V(1).Info("NonAdminRestore status unchanged during deletion") } - - veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, nar.Status.UUID) - if err == nil { - err = r.Delete(ctx, veleroRestore) - if err != nil { - logger.Error(err, "Unable to delete Velero Restore") - return err + if nar.ObjectMeta.DeletionTimestamp.IsZero() { + logger.V(1).Info("Marking NonAdminRestore for deletion", nameString, nar.Name) + if err := r.Delete(ctx, nar); err != nil { + logger.Error(err, "Failed to call Delete on the NonAdminRestore object") + return false, err } - logger.V(1).Info("Waiting Velero Restore be deleted") - return nil + requeueRequired = true // Requeue to allow deletion to proceed } - if !apierrors.IsNotFound(err) { - logger.Error(err, "Unable to fetch Velero Restore") - return err + return requeueRequired, nil +} + +func (r *NonAdminRestoreReconciler) deleteVeleroRestore(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { + if nar.Status.VeleroRestore == nil || nar.Status.VeleroRestore.NACUUID == constant.EmptyString { + return false, nil + } + + veleroRestoreNACUUID := nar.Status.VeleroRestore.NACUUID + + veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, veleroRestoreNACUUID) + + if err != nil { + // Case in which more then one VeleroRestore is found with the same label NACUUID + logger.Error(err, findSingleVRError, uuidString, veleroRestoreNACUUID) + return false, err } - controllerutil.RemoveFinalizer(nar, constant.NonAdminRestoreFinalizerName) - // TODO does this change generation? need to refetch? - if err := r.Update(ctx, nar); err != nil { - logger.Error(err, "Unable to remove NonAdminRestore finalizer") - return err + if veleroRestore != nil { + // All the data within VeleroRestore is stored in object storage, so veleroRestore deletion is not blocking + // and it will get removed by the Velero cleanup process when the restore object gets deleted + // https://github.com/vmware-tanzu/velero/blob/074f26539d3eb06c7b1a6af9b4975254e61b956c/pkg/cmd/cli/restore/delete.go#L122 + if err = r.Delete(ctx, veleroRestore); err != nil { + logger.Error(err, "Failed to delete VeleroRestore", nameString, veleroRestore.Name) + return false, err + } + logger.V(1).Info("VeleroRestore deletion initiated", nameString, veleroRestore.Name) + } else { + logger.V(1).Info("VeleroRestore already deleted") + } + return false, nil // Continue +} + +func (r *NonAdminRestoreReconciler) removeNarFinalizerUponVeleroRestoreDeletion(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { + if !nar.ObjectMeta.DeletionTimestamp.IsZero() { + if nar.Status.VeleroRestore != nil && nar.Status.VeleroRestore.NACUUID != constant.EmptyString { + veleroRestoreNACUUID := nar.Status.VeleroRestore.NACUUID + + veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, veleroRestoreNACUUID) + if err != nil { + // Case in which more then one VeleroRestore is found with the same label UUID + logger.Error(err, findSingleVRError, uuidString, veleroRestoreNACUUID) + return false, err + } + + if veleroRestore != nil { + logger.V(1).Info("Waiting for VeleroRestore to be deleted", nameString, veleroRestoreNACUUID) + return true, nil // Requeue + } + } + // VeleroRestore is deleted, proceed with deleting the NonAdminRestore + logger.V(1).Info("VeleroRestore deleted, removing NonAdminRestore finalizer") + + controllerutil.RemoveFinalizer(nar, constant.NonAdminRestoreFinalizerName) + + if err := r.Update(ctx, nar); err != nil { + logger.Error(err, "Failed to remove finalizer from NonAdminRestore") + return false, err + } + + logger.V(1).Info("NonAdminRestore finalizer removed and object deleted") } - return nil + return false, nil } -func (r *NonAdminRestoreReconciler) init(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { +func (r *NonAdminRestoreReconciler) init(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { if nar.Status.Phase == constant.EmptyString { if updated := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseNew); updated { if err := r.Status().Update(ctx, nar); err != nil { logger.Error(err, nonAdminRestoreStatusUpdateFailureMessage) - return err + return false, err } logger.V(1).Info("NonAdminRestore Phase set to New") } } else { logger.V(1).Info("NonAdminRestore Phase already initialized") } - return nil + return false, nil } -func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { +func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { err := function.ValidateRestoreSpec(ctx, r.Client, nar) if err != nil { updatedPhase := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseBackingOff) @@ -177,10 +250,10 @@ func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger log if updatedPhase || updatedCondition { if updateErr := r.Status().Update(ctx, nar); updateErr != nil { logger.Error(updateErr, nonAdminRestoreStatusUpdateFailureMessage) - return updateErr + return false, updateErr } } - return reconcile.TerminalError(err) + return false, reconcile.TerminalError(err) } logger.V(1).Info("NonAdminRestore Spec validated") @@ -195,94 +268,100 @@ func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger log if updated { if err := r.Status().Update(ctx, nar); err != nil { logger.Error(err, nonAdminRestoreStatusUpdateFailureMessage) - return err + return false, err } logger.V(1).Info("NonAdminRestore condition set to Accepted") } - return nil + return false, nil } -func (r *NonAdminRestoreReconciler) setUUID(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { - if nar.Status.UUID == constant.EmptyString { - // TODO handle panic - nar.Status.UUID = uuid.New().String() +func (r *NonAdminRestoreReconciler) setUUID(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { + // Get the latest version of the NAR object just before checking if the NACUUID is set + // to ensure we do not miss any updates to the NAR object + narOriginal := nar.DeepCopy() + if err := r.Get(ctx, types.NamespacedName{Name: narOriginal.Name, Namespace: narOriginal.Namespace}, nar); err != nil { + logger.Error(err, "Failed to re-fetch NonAdminRestore") + return false, err + } + + if nar.Status.VeleroRestore == nil || nar.Status.VeleroRestore.NACUUID == constant.EmptyString { + veleroRestoreNACUUID := function.GenerateNacObjectUUID(nar.Namespace, nar.Name) + nar.Status.VeleroRestore = &nacv1alpha1.VeleroRestore{ + NACUUID: veleroRestoreNACUUID, + Namespace: r.OADPNamespace, + Name: veleroRestoreNACUUID, + } if err := r.Status().Update(ctx, nar); err != nil { - logger.Error(err, statusUpdateError) - return err + logger.Error(err, nonAdminRestoreStatusUpdateFailureMessage) + return false, err } - logger.V(1).Info("NonAdminRestore UUID created") + logger.V(1).Info(veleroRestoreReferenceUpdated) } else { - logger.V(1).Info("NonAdminRestore already contains UUID") + logger.V(1).Info("NonAdminRestore already contains VeleroRestore UUID reference") } - return nil + return false, nil } -func (r *NonAdminRestoreReconciler) setFinalizer(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { +func (r *NonAdminRestoreReconciler) setFinalizer(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { added := controllerutil.AddFinalizer(nar, constant.NonAdminRestoreFinalizerName) if added { if err := r.Update(ctx, nar); err != nil { logger.Error(err, "Failed to add finalizer") - return err + return false, err } - // TODO does this change generation? need to refetch? logger.V(1).Info("Finalizer added to NonAdminRestore") } else { logger.V(1).Info("Finalizer already exists on NonAdminRestore") } - return nil + return false, nil } -func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) error { - nacUUID := nar.Status.UUID - veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, nacUUID) - if err != nil { - if !apierrors.IsNotFound(err) { - return err - } - if nar.Status.Phase == nacv1alpha1.NonAdminPhaseCreated { - logger.V(1).Info("Velero Restore was deleted, deleting NonAdminRestore") - err = r.delete(ctx, logger, nar) - if err != nil { - logger.Error(err, "Failed to delete NonAdminRestore") - return err - } - return nil - } +func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { + if nar.Status.VeleroRestore == nil || nar.Status.VeleroRestore.NACUUID == constant.EmptyString { + return false, errors.New("unable to get Velero Restore UUID from NonAdminRestore Status") + } - restoreSpec := nar.Spec.RestoreSpec.DeepCopy() + veleroRestoreNACUUID := nar.Status.VeleroRestore.NACUUID - // TODO already did this get call in ValidateRestoreSpec! - nab := &nacv1alpha1.NonAdminBackup{} - if err = r.Get(ctx, types.NamespacedName{ - Name: nar.Spec.RestoreSpec.BackupName, - Namespace: nar.Namespace, - }, nab); err != nil { - logger.Error(err, "Failed to get NonAdminBackup") - return err - } - restoreSpec.BackupName = nab.Status.VeleroBackup.Name + veleroRestore, err := function.GetVeleroRestoreByLabel(ctx, r.Client, r.OADPNamespace, veleroRestoreNACUUID) - // TODO enforce Restore Spec + if err != nil { + // Case in which more then one VeleroBackup is found with the same label UUID + logger.Error(err, findSingleVRError, uuidString, veleroRestoreNACUUID) + return false, err + } - veleroRestore = &velerov1.Restore{ + if veleroRestore == nil { + logger.Info("VeleroRestore with label not found, creating one", uuidString, veleroRestoreNACUUID) + restoreSpec := nar.Spec.RestoreSpec.DeepCopy() + restoreSpec.IncludedNamespaces = []string{nar.Namespace} + veleroRestore := velerov1.Restore{ ObjectMeta: metav1.ObjectMeta{ - Name: function.GenerateNacObjectUUID(nar.Namespace, nar.Name), + Name: veleroRestoreNACUUID, Namespace: r.OADPNamespace, - Labels: function.GetNonAdminRestoreLabels(nar.Status.UUID), + Labels: function.GetNonAdminLabels(), Annotations: function.GetNonAdminRestoreAnnotations(nar.ObjectMeta), }, Spec: *restoreSpec, } + // Add NonAdminRestore's veleroRestoreNACUUID as the label to the VeleroRestore object + veleroRestore.Labels[constant.NarOriginNACUUIDLabel] = veleroRestoreNACUUID + + err = r.Create(ctx, &veleroRestore) - err = r.Create(ctx, veleroRestore) if err != nil { - logger.Error(err, "Failed to create Velero Restore") - return err + // We do not retry here as the veleroRestoreNACUUID + // should be guaranteed to be unique + logger.Error(err, "Failed to create VeleroRestore") + return false, err } - logger.Info("Velero Restore successfully created") + logger.Info("VeleroRestore successfully created") } + // TODO(migi): do we need estimatedQueuePosition in VeleroRestoreStatus? + updatedPhase := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseCreated) + updatedCondition := meta.SetStatusCondition(&nar.Status.Conditions, metav1.Condition{ Type: string(constant.NonAdminConditionQueued), @@ -291,34 +370,33 @@ func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, log Message: "Created Velero Restore object", }, ) + updatedVeleroStatus := updateVeleroRestoreStatus(&nar.Status, veleroRestore) + if updatedPhase || updatedCondition || updatedVeleroStatus { if err := r.Status().Update(ctx, nar); err != nil { logger.Error(err, nonAdminRestoreStatusUpdateFailureMessage) - return err + return false, err } logger.V(1).Info("NonAdminRestore Status updated successfully") } else { logger.V(1).Info("NonAdminRestore Status unchanged during Velero Restore reconciliation") } - return nil + return false, nil } func updateVeleroRestoreStatus(status *nacv1alpha1.NonAdminRestoreStatus, veleroRestore *velerov1.Restore) bool { + if status == nil || veleroRestore == nil { + return false + } + if status.VeleroRestore == nil { - status.VeleroRestore = &nacv1alpha1.VeleroRestore{ - Name: veleroRestore.Name, - Namespace: veleroRestore.Namespace, - Status: &veleroRestore.Status, - } - return true - } else if status.VeleroRestore.Name != veleroRestore.Name || - status.VeleroRestore.Namespace != veleroRestore.Namespace || - !reflect.DeepEqual(status.VeleroRestore.Status, &veleroRestore.Status) { - status.VeleroRestore.Name = veleroRestore.Name - status.VeleroRestore.Namespace = veleroRestore.Namespace - status.VeleroRestore.Status = &veleroRestore.Status + status.VeleroRestore = &nacv1alpha1.VeleroRestore{} + } + + if status.VeleroRestore.Status == nil || !reflect.DeepEqual(status.VeleroRestore.Status, veleroRestore.Status) { + status.VeleroRestore.Status = veleroRestore.Status.DeepCopy() return true } return false diff --git a/internal/controller/nonadminrestore_controller_test.go b/internal/controller/nonadminrestore_controller_test.go index df006e1..6e0bccf 100644 --- a/internal/controller/nonadminrestore_controller_test.go +++ b/internal/controller/nonadminrestore_controller_test.go @@ -63,8 +63,8 @@ func checkTestNonAdminRestoreStatus(nonAdminRestore *nacv1alpha1.NonAdminRestore } if nonAdminRestore.Status.VeleroRestore != nil { - if nonAdminRestore.Status.UUID == constant.EmptyString { - return fmt.Errorf("NonAdminRestore Status UUID not set") + if nonAdminRestore.Status.VeleroRestore.NACUUID == constant.EmptyString { + return fmt.Errorf("NonAdminRestore Status VeleroRestore NACUUID not set") } if nonAdminRestore.Status.VeleroRestore.Namespace == constant.EmptyString { return fmt.Errorf("NonAdminRestore status.veleroRestore.namespace is not set") @@ -237,7 +237,7 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminRestore Controller" gomega.Expect(checkTestNonAdminRestoreStatus(nonAdminRestore, scenario.status)).To(gomega.Succeed()) veleroRestore := &velerov1.Restore{} - if scenario.status.VeleroRestore != nil && len(nonAdminRestore.Status.UUID) > 0 { + if scenario.status.VeleroRestore != nil && len(nonAdminRestore.Status.VeleroRestore.NACUUID) > 0 { ginkgo.By("Checking if NonAdminRestore Spec was not changed") gomega.Expect(reflect.DeepEqual( nonAdminRestore.Spec, @@ -299,7 +299,7 @@ var _ = ginkgo.Describe("Test full reconcile loop of NonAdminRestore Controller" } return false, err }, 10*time.Second, 1*time.Second).Should(gomega.BeTrue()) - if scenario.status.VeleroRestore != nil && len(nonAdminRestore.Status.UUID) > 0 { + if scenario.status.VeleroRestore != nil && len(nonAdminRestore.Status.VeleroRestore.NACUUID) > 0 { gomega.Eventually(func() (bool, error) { err := k8sClient.Get( ctxTimeout, From 488f25642be3f58d2d3896d858815f81079e6bab Mon Sep 17 00:00:00 2001 From: Michal Pryc Date: Fri, 29 Nov 2024 15:50:35 +0100 Subject: [PATCH 13/14] Fix tests and restore name Fix restore name to reflect velero object name from the nonadminbackup Fix label for the NAR, that Mateus spotted. Signed-off-by: Michal Pryc --- internal/common/function/function.go | 7 +++++-- .../controller/nonadminrestore_controller.go | 18 +++++++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/internal/common/function/function.go b/internal/common/function/function.go index dbf17eb..dddd8fb 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -132,6 +132,8 @@ func ValidateRestoreSpec(ctx context.Context, clientInstance client.Client, nonA if nab.Status.Phase != nacv1alpha1.NonAdminPhaseCreated { return fmt.Errorf("NonAdminRestore spec.restoreSpec.backupName is invalid: NonAdminBackup is not ready to be restored") } + // TODO validate that velero backup exists? + // TODO does velero validate if backup is ready to be restored? // TODO validate that nonAdminRestore.Spec.RestoreSpec.ScheduleName is not used? (we do not plan to have schedules now, right?) @@ -378,8 +380,9 @@ func CheckVeleroRestoreMetadata(obj client.Object) bool { if !checkLabelValue(objLabels, constant.ManagedByLabel, constant.ManagedByLabelValue) { return false } - _, err := uuid.Parse(objLabels[constant.NarOriginNACUUIDLabel]) - if err != nil { + + labelValue, exists := objLabels[constant.NarOriginNACUUIDLabel] + if !exists || len(labelValue) == 0 { return false } diff --git a/internal/controller/nonadminrestore_controller.go b/internal/controller/nonadminrestore_controller.go index ac63150..606ba2c 100644 --- a/internal/controller/nonadminrestore_controller.go +++ b/internal/controller/nonadminrestore_controller.go @@ -237,6 +237,9 @@ func (r *NonAdminRestoreReconciler) init(ctx context.Context, logger logr.Logger func (r *NonAdminRestoreReconciler) validateSpec(ctx context.Context, logger logr.Logger, nar *nacv1alpha1.NonAdminRestore) (bool, error) { err := function.ValidateRestoreSpec(ctx, r.Client, nar) + + // TODO We allow only to reference existing NonAdminBackup object within the same namespace + if err != nil { updatedPhase := updateNonAdminPhase(&nar.Status.Phase, nacv1alpha1.NonAdminPhaseBackingOff) updatedCondition := meta.SetStatusCondition(&nar.Status.Conditions, @@ -333,19 +336,28 @@ func (r *NonAdminRestoreReconciler) createVeleroRestore(ctx context.Context, log if veleroRestore == nil { logger.Info("VeleroRestore with label not found, creating one", uuidString, veleroRestoreNACUUID) + nab := &nacv1alpha1.NonAdminBackup{} + err = r.Get(ctx, types.NamespacedName{Name: nar.Spec.RestoreSpec.BackupName, Namespace: nar.Namespace}, nab) + if err != nil { + logger.Error(err, "Failed to get NonAdminBackup referenced by NonAdminRestore") + return false, err + } + restoreSpec := nar.Spec.RestoreSpec.DeepCopy() + + restoreSpec.BackupName = nab.Status.VeleroBackup.Name + // TODO enforce Restore Spec + restoreSpec.IncludedNamespaces = []string{nar.Namespace} veleroRestore := velerov1.Restore{ ObjectMeta: metav1.ObjectMeta{ Name: veleroRestoreNACUUID, Namespace: r.OADPNamespace, - Labels: function.GetNonAdminLabels(), + Labels: function.GetNonAdminRestoreLabels(veleroRestoreNACUUID), Annotations: function.GetNonAdminRestoreAnnotations(nar.ObjectMeta), }, Spec: *restoreSpec, } - // Add NonAdminRestore's veleroRestoreNACUUID as the label to the VeleroRestore object - veleroRestore.Labels[constant.NarOriginNACUUIDLabel] = veleroRestoreNACUUID err = r.Create(ctx, &veleroRestore) From 0c0416293e57f685dfd0234a63986e42251b17d4 Mon Sep 17 00:00:00 2001 From: Michal Pryc Date: Mon, 2 Dec 2024 21:23:15 +0100 Subject: [PATCH 14/14] Rename Restore Annotations s/NonAdminRestoreOriginNameAnnotation/NarOriginNameAnnotation s/NonAdminRestoreOriginNamespaceAnnotation/NarOriginNamespaceAnnotation Signed-off-by: Michal Pryc --- internal/common/constant/constant.go | 8 ++++---- internal/common/function/function.go | 8 ++++---- internal/handler/velerorestore_handler.go | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/common/constant/constant.go b/internal/common/constant/constant.go index 7325bed..f54ccb4 100644 --- a/internal/common/constant/constant.go +++ b/internal/common/constant/constant.go @@ -31,10 +31,10 @@ const ( NabOriginNACUUIDLabel = "openshift.io/oadp-nab-origin-nacuuid" NarOriginNACUUIDLabel = "openshift.io/oadp-nar-origin-nacuuid" - NabOriginNameAnnotation = "openshift.io/oadp-nab-origin-name" - NabOriginNamespaceAnnotation = "openshift.io/oadp-nab-origin-namespace" - NonAdminRestoreOriginNameAnnotation = "openshift.io/oadp-nar-origin-name" - NonAdminRestoreOriginNamespaceAnnotation = "openshift.io/oadp-nar-origin-namespace" + NabOriginNameAnnotation = "openshift.io/oadp-nab-origin-name" + NabOriginNamespaceAnnotation = "openshift.io/oadp-nab-origin-namespace" + NarOriginNameAnnotation = "openshift.io/oadp-nar-origin-name" + NarOriginNamespaceAnnotation = "openshift.io/oadp-nar-origin-namespace" NabFinalizerName = "nonadminbackup.oadp.openshift.io/finalizer" NonAdminRestoreFinalizerName = "nonadminrestore.oadp.openshift.io/finalizer" diff --git a/internal/common/function/function.go b/internal/common/function/function.go index dddd8fb..9e82ddf 100644 --- a/internal/common/function/function.go +++ b/internal/common/function/function.go @@ -63,8 +63,8 @@ func GetNonAdminBackupAnnotations(objectMeta metav1.ObjectMeta) map[string]strin // GetNonAdminRestoreAnnotations return the required Non Admin restore annotations func GetNonAdminRestoreAnnotations(objectMeta metav1.ObjectMeta) map[string]string { return map[string]string{ - constant.NonAdminRestoreOriginNamespaceAnnotation: objectMeta.Namespace, - constant.NonAdminRestoreOriginNameAnnotation: objectMeta.Name, + constant.NarOriginNamespaceAnnotation: objectMeta.Namespace, + constant.NarOriginNameAnnotation: objectMeta.Name, } } @@ -387,10 +387,10 @@ func CheckVeleroRestoreMetadata(obj client.Object) bool { } annotations := obj.GetAnnotations() - if !checkLabelAnnotationValueIsValid(annotations, constant.NonAdminRestoreOriginNamespaceAnnotation) { + if !checkLabelAnnotationValueIsValid(annotations, constant.NarOriginNamespaceAnnotation) { return false } - if !checkLabelAnnotationValueIsValid(annotations, constant.NonAdminRestoreOriginNameAnnotation) { + if !checkLabelAnnotationValueIsValid(annotations, constant.NarOriginNameAnnotation) { return false } diff --git a/internal/handler/velerorestore_handler.go b/internal/handler/velerorestore_handler.go index a757cde..515de66 100644 --- a/internal/handler/velerorestore_handler.go +++ b/internal/handler/velerorestore_handler.go @@ -41,8 +41,8 @@ func (VeleroRestoreHandler) Update(ctx context.Context, evt event.UpdateEvent, q logger := function.GetLogger(ctx, evt.ObjectNew, "VeleroRestoreHandler") annotations := evt.ObjectNew.GetAnnotations() - nonAdminRestoreName := annotations[constant.NonAdminRestoreOriginNameAnnotation] - nonAdminRestoreNamespace := annotations[constant.NonAdminRestoreOriginNamespaceAnnotation] + nonAdminRestoreName := annotations[constant.NarOriginNameAnnotation] + nonAdminRestoreNamespace := annotations[constant.NarOriginNamespaceAnnotation] q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ Name: nonAdminRestoreName, @@ -56,8 +56,8 @@ func (VeleroRestoreHandler) Delete(ctx context.Context, evt event.DeleteEvent, q logger := function.GetLogger(ctx, evt.Object, "VeleroRestoreHandler") annotations := evt.Object.GetAnnotations() - nonAdminRestoreName := annotations[constant.NonAdminRestoreOriginNameAnnotation] - nonAdminRestoreNamespace := annotations[constant.NonAdminRestoreOriginNamespaceAnnotation] + nonAdminRestoreName := annotations[constant.NarOriginNameAnnotation] + nonAdminRestoreNamespace := annotations[constant.NarOriginNamespaceAnnotation] q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ Name: nonAdminRestoreName,