From 5bdd750d0cbd2cd65e6174412cdb76d94690f372 Mon Sep 17 00:00:00 2001 From: Michal Pryc Date: Wed, 6 Mar 2024 14:04:38 +0100 Subject: [PATCH] Add Watch of Velero Backup and remove Backup controller (#12) Signed-off-by: Michal Pryc Co-authored-by: Tiger Kaovilai --- PROJECT | 5 - cmd/main.go | 8 -- config/rbac/role.yaml | 1 - internal/controller/common_nab.go | 27 +++++ internal/controller/composite_predicate.go | 51 +++++++++ .../controller/nonadminbackup_controller.go | 20 +++- .../controller/nonadminbackup_predicate.go | 67 ++++++++++++ .../controller/velerobackup_controller.go | 100 ------------------ .../velerobackup_controller_test.go | 32 ------ internal/controller/velerobackup_handler.go | 70 ++++++++++++ internal/controller/velerobackup_predicate.go | 69 ++++++++++++ 11 files changed, 301 insertions(+), 149 deletions(-) create mode 100644 internal/controller/composite_predicate.go create mode 100644 internal/controller/nonadminbackup_predicate.go delete mode 100644 internal/controller/velerobackup_controller.go delete mode 100644 internal/controller/velerobackup_controller_test.go create mode 100644 internal/controller/velerobackup_handler.go create mode 100644 internal/controller/velerobackup_predicate.go diff --git a/PROJECT b/PROJECT index ae1ea20..b10e3ba 100644 --- a/PROJECT +++ b/PROJECT @@ -17,9 +17,4 @@ resources: kind: NonAdminBackup path: github.com/migtools/oadp-non-admin/api/v1alpha1 version: v1alpha1 -- controller: true - domain: oadp.openshift.io - group: nac.oadp.openshift.io - kind: VeleroBackup - version: v1alpha1 version: "3" diff --git a/cmd/main.go b/cmd/main.go index a06f685..bd76f4e 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -135,14 +135,6 @@ func main() { } //+kubebuilder:scaffold:builder - if err = (&controller.VeleroBackupReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - }).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "VeleroBackup") - os.Exit(1) - } - if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up health check") os.Exit(1) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index d763ad8..24d8c44 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -36,7 +36,6 @@ rules: - backups verbs: - create - - delete - get - list - patch diff --git a/internal/controller/common_nab.go b/internal/controller/common_nab.go index 2a6d23a..d28b23c 100644 --- a/internal/controller/common_nab.go +++ b/internal/controller/common_nab.go @@ -1,11 +1,15 @@ package controller import ( + "context" "crypto/sha1" "encoding/hex" "fmt" + "reflect" + "github.com/go-logr/logr" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "sigs.k8s.io/controller-runtime/pkg/client" nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" ) @@ -48,3 +52,26 @@ func GenerateVeleroBackupName(namespace, nabName string) string { return veleroBackupName } + +func UpdateNonAdminBackupFromVeleroBackup(ctx context.Context, r client.Client, log logr.Logger, nab *nacv1alpha1.NonAdminBackup, veleroBackup *velerov1api.Backup) error { + // Make a copy of the current status for comparison + oldStatus := nab.Spec.BackupStatus.DeepCopy() + oldSpec := nab.Spec.BackupSpec.DeepCopy() + + // Update the status & spec + nab.Spec.BackupStatus = &veleroBackup.Status + nab.Spec.BackupSpec = &veleroBackup.Spec + + if reflect.DeepEqual(oldStatus, nab.Spec.BackupStatus) && reflect.DeepEqual(oldSpec, nab.Spec.BackupSpec) { + // No change, no need to update + log.V(1).Info("NonAdminBackup status and spec is already up to date") + return nil + } + + if err := r.Update(ctx, nab); err != nil { + log.Error(err, "Failed to update NonAdminBackup") + return err + } + + return nil +} diff --git a/internal/controller/composite_predicate.go b/internal/controller/composite_predicate.go new file mode 100644 index 0000000..40000cc --- /dev/null +++ b/internal/controller/composite_predicate.go @@ -0,0 +1,51 @@ +// composite_predicate.go + +package controller + +import ( + "context" + + "sigs.k8s.io/controller-runtime/pkg/event" +) + +type CompositePredicate struct { + NonAdminBackupPredicate NonAdminBackupPredicate + VeleroBackupPredicate VeleroBackupPredicate + Context context.Context +} + +func (p CompositePredicate) Create(evt event.CreateEvent) bool { + // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate + if p.NonAdminBackupPredicate.Create(p.Context, evt) { + return true + } + // Otherwise, apply VeleroBackupPredicate + return p.VeleroBackupPredicate.Create(p.Context, evt) +} + +func (p CompositePredicate) Update(evt event.UpdateEvent) bool { + // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate + if p.NonAdminBackupPredicate.Update(p.Context, evt) { + return true + } + // Otherwise, apply VeleroBackupPredicate + return p.VeleroBackupPredicate.Update(p.Context, evt) +} + +func (p CompositePredicate) Delete(evt event.DeleteEvent) bool { + // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate + if p.NonAdminBackupPredicate.Delete(p.Context, evt) { + return true + } + // Otherwise, apply VeleroBackupPredicate + return p.VeleroBackupPredicate.Delete(p.Context, evt) +} + +func (p CompositePredicate) Generic(evt event.GenericEvent) bool { + // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate + if p.NonAdminBackupPredicate.Generic(p.Context, evt) { + return true + } + // Otherwise, apply VeleroBackupPredicate + return p.VeleroBackupPredicate.Generic(p.Context, evt) +} diff --git a/internal/controller/nonadminbackup_controller.go b/internal/controller/nonadminbackup_controller.go index bcab4b8..ca159b7 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -47,6 +47,8 @@ type NonAdminBackupReconciler struct { //+kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups/status,verbs=get;update;patch //+kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups/finalizers,verbs=update +//+kubebuilder:rbac:groups=velero.io,resources=backups,verbs=get;list;watch;create;update;patch + // 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 @@ -79,7 +81,7 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque veleroBackupSpec, err := GetVeleroBackupSpecFromNonAdminBackup(&nab) if veleroBackupSpec == nil { - log.Error(err, "unable to fetch VeleroBackupSpec from NonAdminBackup") + log.Error(err, "NonAdminBackup CR does not contain valid VeleroBackupSpec") return ctrl.Result{}, nil } @@ -106,10 +108,14 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque Spec: *veleroBackupSpec, } } else if err != nil && !errors.IsNotFound(err) { - log.Error(err, "unable to fetch VeleroBackup") + log.Error(err, "Unable to fetch VeleroBackup") return ctrl.Result{}, err } else { - log.Info("Backup already exists", "Name", veleroBackupName) + log.Info("Backup already exists, updating NonAdminBackup status", "Name", veleroBackupName) + err := UpdateNonAdminBackupFromVeleroBackup(ctx, r.Client, log, &nab, &veleroBackup) + if err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } @@ -142,5 +148,13 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque func (r *NonAdminBackupReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&nacv1alpha1.NonAdminBackup{}). + Watches(&velerov1api.Backup{}, &VeleroBackupHandler{}). + WithEventFilter(CompositePredicate{ + NonAdminBackupPredicate: NonAdminBackupPredicate{}, + VeleroBackupPredicate: VeleroBackupPredicate{ + OadpVeleroNamespace: "openshift-adp", + }, + Context: r.Context, + }). Complete(r) } diff --git a/internal/controller/nonadminbackup_predicate.go b/internal/controller/nonadminbackup_predicate.go new file mode 100644 index 0000000..f9274f9 --- /dev/null +++ b/internal/controller/nonadminbackup_predicate.go @@ -0,0 +1,67 @@ +// velerobackup_predicate.go + +package controller + +import ( + "context" + + "github.com/go-logr/logr" + nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +type NonAdminBackupPredicate struct { + Logger logr.Logger +} + +func getNonAdminBackupPredicateLogger(ctx context.Context, name, namespace string) logr.Logger { + return log.FromContext(ctx).WithValues("NonAdminBackupPredicate", types.NamespacedName{Name: name, Namespace: namespace}) +} + +func (predicate NonAdminBackupPredicate) Create(ctx context.Context, evt event.CreateEvent) bool { + + if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { + nameSpace := evt.Object.GetNamespace() + name := evt.Object.GetName() + log := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + log.V(1).Info("Received Create NonAdminBackupPredicate") + return true + } + + return false +} + +func (predicate NonAdminBackupPredicate) Update(ctx context.Context, evt event.UpdateEvent) bool { + if _, ok := evt.ObjectNew.(*nacv1alpha1.NonAdminBackup); ok { + nameSpace := evt.ObjectNew.GetNamespace() + name := evt.ObjectNew.GetName() + log := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + log.V(1).Info("Received Update NonAdminBackupPredicate") + return true + } + return false +} + +func (predicate NonAdminBackupPredicate) Delete(ctx context.Context, evt event.DeleteEvent) bool { + if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { + nameSpace := evt.Object.GetNamespace() + name := evt.Object.GetName() + log := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + log.V(1).Info("Received Delete NonAdminBackupPredicate") + return true + } + return false +} + +func (predicate NonAdminBackupPredicate) Generic(ctx context.Context, evt event.GenericEvent) bool { + if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { + nameSpace := evt.Object.GetNamespace() + name := evt.Object.GetName() + log := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + log.V(1).Info("Received Generic NonAdminBackupPredicate") + return true + } + return false +} diff --git a/internal/controller/velerobackup_controller.go b/internal/controller/velerobackup_controller.go deleted file mode 100644 index 67a77e1..0000000 --- a/internal/controller/velerobackup_controller.go +++ /dev/null @@ -1,100 +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 controller - -import ( - "context" - - "github.com/go-logr/logr" - "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" - - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" -) - -// VeleroBackupReconciler reconciles a VeleroBackup object -type VeleroBackupReconciler struct { - client.Client - Scheme *runtime.Scheme - Log logr.Logger -} - -//+kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups/finalizers,verbs=update - -//+kubebuilder:rbac:groups=velero.io,resources=backups,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 VeleroBackup 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 *VeleroBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - r.Log = log.FromContext(ctx) - log := r.Log.WithValues("VeleroBackup", req.NamespacedName) - - // Check if the reconciliation request is for the oadp namespace - if req.Namespace != OadpNamespace { - log.Info("Ignoring reconciliation request for namespace", "Namespace", req.Namespace) - return ctrl.Result{}, nil - } - - // TODO(user): your logic here - - backup := &velerov1api.Backup{} - err := r.Get(ctx, req.NamespacedName, backup) - if err != nil { - log.Error(err, "Unable to fetch VeleroBackup CR", "Name", req.Name, "Namespace", req.Namespace) - return ctrl.Result{}, err - } - - if !HasRequiredLabel(backup) { - log.Info("Ignoring VeleroBackup without the required label", "Name", backup.Name, "Namespace", backup.Namespace) - return ctrl.Result{}, nil - } - - log.Info("Velero Backup Reconcile loop") - nonAdminBackup, err := GetNonAdminFromBackup(ctx, r.Client, backup) - if err != nil { - log.V(1).Info("Could not find matching Velero Non Admin Backup", "error", err) - // We don't report error to reconcile - } else { - log.V(1).Info("Got Velero Non Admin Backup:", "nab", nonAdminBackup) - log.V(1).Info("Velero Backup status:", "Status", backup.Status) - nonAdminBackup.Spec.BackupStatus = &backup.Status - if err := r.Client.Update(ctx, nonAdminBackup); err != nil { - log.Error(err, "Failed to update NonAdminBackup status") - } else { - log.V(1).Info("Updated NonAdminBackup status:", "Status", backup.Status) - } - } - return ctrl.Result{}, nil -} - -// SetupWithManager sets up the controller with the Manager. -func (r *VeleroBackupReconciler) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). - For(&velerov1api.Backup{}). - Complete(r) -} diff --git a/internal/controller/velerobackup_controller_test.go b/internal/controller/velerobackup_controller_test.go deleted file mode 100644 index eae584f..0000000 --- a/internal/controller/velerobackup_controller_test.go +++ /dev/null @@ -1,32 +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 controller - -import ( - . "github.com/onsi/ginkgo/v2" -) - -var _ = Describe("VeleroBackup Controller", func() { - Context("When reconciling a resource", func() { - - It("should successfully reconcile the resource", func() { - - // 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/controller/velerobackup_handler.go b/internal/controller/velerobackup_handler.go new file mode 100644 index 0000000..3e64a40 --- /dev/null +++ b/internal/controller/velerobackup_handler.go @@ -0,0 +1,70 @@ +// velerobackup_handler.go + +package controller + +import ( + "context" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/event" +) + +// Handler for VeleroBackup events +type VeleroBackupHandler struct { + Logger logr.Logger +} + +func getVeleroBackupHandlerLogger(ctx context.Context, name, namespace string) logr.Logger { + return log.FromContext(ctx).WithValues("VeleroBackupHandler", types.NamespacedName{Name: name, Namespace: namespace}) +} + +func (h *VeleroBackupHandler) Create(ctx context.Context, evt event.CreateEvent, q workqueue.RateLimitingInterface) { + nameSpace := evt.Object.GetNamespace() + name := evt.Object.GetName() + log := getVeleroBackupHandlerLogger(ctx, name, nameSpace) + log.V(1).Info("Received Create VeleroBackupHandler") +} + +func (h *VeleroBackupHandler) Update(ctx context.Context, evt event.UpdateEvent, q workqueue.RateLimitingInterface) { + nameSpace := evt.ObjectNew.GetNamespace() + name := evt.ObjectNew.GetName() + log := getVeleroBackupHandlerLogger(ctx, name, nameSpace) + log.V(1).Info("Received Update VeleroBackupHandler") + + annotations := evt.ObjectNew.GetAnnotations() + + if annotations == nil { + log.V(1).Info("Backup annotations not found") + return + } + + nabOriginNamespace, ok := annotations[NabOriginNamespaceAnnotation] + if !ok { + log.V(1).Info("Backup NonAdminBackup origin namespace annotation not found") + return + } + + nabOriginName, ok := annotations[NabOriginNameAnnotation] + if !ok { + log.V(1).Info("Backup NonAdminBackup origin name annotation not found") + return + } + + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Name: nabOriginName, + Namespace: nabOriginNamespace, + }}) +} + +func (h *VeleroBackupHandler) Delete(ctx context.Context, evt event.DeleteEvent, q workqueue.RateLimitingInterface) { + // Delete event handler for the Backup object. We should ignore it. +} + +func (h *VeleroBackupHandler) Generic(ctx context.Context, evt event.GenericEvent, q workqueue.RateLimitingInterface) { + // Generic event handler for the Backup object. We should ignore it. +} diff --git a/internal/controller/velerobackup_predicate.go b/internal/controller/velerobackup_predicate.go new file mode 100644 index 0000000..c24f135 --- /dev/null +++ b/internal/controller/velerobackup_predicate.go @@ -0,0 +1,69 @@ +// velerobackup_predicate.go + +package controller + +import ( + "context" + + "github.com/go-logr/logr" + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +func VeleroPredicate(scheme *runtime.Scheme) predicate.Predicate { + return nil +} + +type VeleroBackupPredicate struct { + // We are watching only Velero Backup objects within + // namespace where OADP is. + OadpVeleroNamespace string + Logger logr.Logger +} + +func getBackupPredicateLogger(ctx context.Context, name, namespace string) logr.Logger { + return log.FromContext(ctx).WithValues("VeleroBackupPredicate", types.NamespacedName{Name: name, Namespace: namespace}) +} + +func (veleroBackupPredicate VeleroBackupPredicate) Create(ctx context.Context, evt event.CreateEvent) bool { + nameSpace := evt.Object.GetNamespace() + if nameSpace != veleroBackupPredicate.OadpVeleroNamespace { + return false + } + + name := evt.Object.GetName() + log := getBackupPredicateLogger(ctx, name, nameSpace) + log.V(1).Info("Received Create VeleroBackupPredicate") + + backup, ok := evt.Object.(*velerov1api.Backup) + if !ok { + // The event object is not a Backup, ignore it + return false + } + if HasRequiredLabel(backup) { + return true + } + return false +} + +func (veleroBackupPredicate VeleroBackupPredicate) Update(ctx context.Context, evt event.UpdateEvent) bool { + nameSpace := evt.ObjectNew.GetNamespace() + name := evt.ObjectNew.GetName() + log := getBackupPredicateLogger(ctx, name, nameSpace) + log.V(1).Info("Received Update VeleroBackupPredicate") + + return nameSpace == veleroBackupPredicate.OadpVeleroNamespace +} + +func (veleroBackupPredicate VeleroBackupPredicate) Delete(ctx context.Context, evt event.DeleteEvent) bool { + // NonAdminBackup should not care about VeleroBackup delete events + return false +} + +func (veleroBackupPredicate VeleroBackupPredicate) Generic(ctx context.Context, evt event.GenericEvent) bool { + return false +}