From b696ff23f5c28b7396a765e7e807c92ed6a6845c Mon Sep 17 00:00:00 2001 From: "Weber.Yang" Date: Mon, 5 Jun 2023 10:11:31 +0800 Subject: [PATCH] Keep MachineStaticIPFinalizer to prevent MachineStaticIPFinalizer from being able to release IP after being removed by CAPE (#28) --- controllers/elfmachine_controller.go | 35 ++++++++++++++--------- controllers/elfmachine_controller_test.go | 21 ++++++++++++-- 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/controllers/elfmachine_controller.go b/controllers/elfmachine_controller.go index f4bf3ab..884b725 100644 --- a/controllers/elfmachine_controller.go +++ b/controllers/elfmachine_controller.go @@ -223,20 +223,9 @@ func (r *ElfMachineReconciler) reconcileDelete(ctx *context.MachineContext) (rec } func (r *ElfMachineReconciler) reconcileIPAddress(ctx *context.MachineContext) (reconcile.Result, error) { - // If the ElfMachine is in an error state, return early. - if ctx.ElfMachine.IsFailed() { - r.Logger.V(2).Info("Error state detected, skipping reconciliation") - return reconcile.Result{}, nil - } - devices := ctx.ElfMachine.Spec.Network.Devices - if len(devices) == 0 { - ctx.Logger.V(2).Info("No network device found") - return ctrl.Result{}, nil - } - - if !ipamutil.NeedsAllocateIP(devices) { - ctx.Logger.V(6).Info("No need to allocate static IP") + if !ipamutil.HasStaticIPDevice(devices) { + ctx.Logger.V(6).Info("No static IP network device found") return ctrl.Result{}, nil } @@ -249,11 +238,31 @@ func (r *ElfMachineReconciler) reconcileIPAddress(ctx *context.MachineContext) ( // If the ElfMachine doesn't have MachineStaticIPFinalizer, add it and return with requeue. // In next reconcile, the static IP will be allocated. if !ctrlutil.ContainsFinalizer(ctx.ElfMachine, MachineStaticIPFinalizer) { + // Add MachineStaticIPFinalizer after setting MachineFinalizer, + // otherwise MachineStaticIPFinalizer may be overwritten by CAPE + // when setting MachineFinalizer. + if !ctrlutil.ContainsFinalizer(ctx.ElfMachine, capev1.MachineFinalizer) { + r.Logger.V(2).Info("Waiting for CAPE to set MachineFinalizer on ElfMachine") + + return reconcile.Result{RequeueAfter: config.DefaultRequeue}, nil + } + ctrlutil.AddFinalizer(ctx.ElfMachine, MachineStaticIPFinalizer) return ctrl.Result{RequeueAfter: 3 * time.Second}, nil } + // If the ElfMachine is in an error state, return early. + if ctx.ElfMachine.IsFailed() { + r.Logger.V(2).Info("Error state detected, skipping reconciliation") + return reconcile.Result{}, nil + } + + if !ipamutil.NeedsAllocateIP(devices) { + ctx.Logger.V(6).Info("No need to allocate static IP") + return ctrl.Result{}, nil + } + ipPool, err := r.getIPPool(ctx) if err != nil { return ctrl.Result{}, err diff --git a/controllers/elfmachine_controller_test.go b/controllers/elfmachine_controller_test.go index f7b577e..d9b8862 100644 --- a/controllers/elfmachine_controller_test.go +++ b/controllers/elfmachine_controller_test.go @@ -84,6 +84,8 @@ var _ = Describe("ElfMachineReconciler", func() { }) It("should not reconcile when ElfMachine in an error state", func() { + ctrlutil.AddFinalizer(elfMachine, capev1.MachineFinalizer) + ctrlutil.AddFinalizer(elfMachine, MachineStaticIPFinalizer) elfMachine.Status.FailureMessage = pointer.String("some error") ctrlContext := newCtrlContexts(elfCluster, cluster, elfMachine, machine, elfMachineTemplate) fake.InitOwnerReferences(ctrlContext, elfCluster, cluster, elfMachine, machine) @@ -117,7 +119,7 @@ var _ = Describe("ElfMachineReconciler", func() { Expect(logBuffer.String()).To(ContainSubstring("ElfMachine linked to a cluster that is paused")) }) - It("should not reconcile without devices", func() { + It("should not reconcile without static IP network devices", func() { ctrlutil.RemoveFinalizer(elfMachine, MachineStaticIPFinalizer) elfMachine.Spec.Network.Devices = []capev1.NetworkDeviceSpec{} ctrlContext := newCtrlContexts(elfCluster, cluster, elfMachine, machine, elfMachineTemplate) @@ -127,12 +129,14 @@ var _ = Describe("ElfMachineReconciler", func() { result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: capiutil.ObjectKey(elfMachine)}) Expect(result).To(BeZero()) Expect(err).ToNot(HaveOccurred()) - Expect(logBuffer.String()).To(ContainSubstring("No network device found")) + Expect(logBuffer.String()).To(ContainSubstring("No static IP network device found")) Expect(ctrlContext.Client.Get(ctrlContext, capiutil.ObjectKey(elfMachine), elfMachine)).To(Succeed()) Expect(ctrlutil.ContainsFinalizer(elfMachine, MachineStaticIPFinalizer)).To(BeFalse()) }) It("should not reconcile when no need to allocate static IP", func() { + ctrlutil.AddFinalizer(elfMachine, capev1.MachineFinalizer) + ctrlutil.AddFinalizer(elfMachine, MachineStaticIPFinalizer) elfMachine.Spec.Network.Devices = []capev1.NetworkDeviceSpec{ {NetworkType: capev1.NetworkTypeIPV4, IPAddrs: []string{fake.IP()}}, {NetworkType: capev1.NetworkTypeIPV4DHCP}, @@ -147,7 +151,6 @@ var _ = Describe("ElfMachineReconciler", func() { Expect(err).ToNot(HaveOccurred()) Expect(logBuffer.String()).To(ContainSubstring("No need to allocate static IP")) Expect(ctrlContext.Client.Get(ctrlContext, capiutil.ObjectKey(elfMachine), elfMachine)).To(Succeed()) - Expect(ctrlutil.ContainsFinalizer(elfMachine, MachineStaticIPFinalizer)).To(BeFalse()) }) It("should set MachineStaticIPFinalizer first", func() { @@ -159,6 +162,18 @@ var _ = Describe("ElfMachineReconciler", func() { Expect(result).NotTo(BeZero()) Expect(err).NotTo(HaveOccurred()) Expect(ctrlContext.Client.Get(ctrlContext, capiutil.ObjectKey(elfMachine), elfMachine)).To(Succeed()) + Expect(ctrlutil.ContainsFinalizer(elfMachine, MachineStaticIPFinalizer)).To(BeFalse()) + Expect(logBuffer.String()).To(ContainSubstring("Waiting for CAPE to set MachineFinalizer on ElfMachine")) + + ctrlutil.AddFinalizer(elfMachine, capev1.MachineFinalizer) + ctrlContext = newCtrlContexts(elfCluster, cluster, elfMachine, machine, elfMachineTemplate) + fake.InitOwnerReferences(ctrlContext, elfCluster, cluster, elfMachine, machine) + + reconciler = &ElfMachineReconciler{ControllerContext: ctrlContext} + result, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: capiutil.ObjectKey(elfMachine)}) + Expect(result).NotTo(BeZero()) + Expect(err).NotTo(HaveOccurred()) + Expect(ctrlContext.Client.Get(ctrlContext, capiutil.ObjectKey(elfMachine), elfMachine)).To(Succeed()) Expect(ctrlutil.ContainsFinalizer(elfMachine, MachineStaticIPFinalizer)).To(BeTrue()) })