Skip to content

Commit

Permalink
SKS-1243: Save MachineStaticIPFinalizer first and then allocate IP (#26)
Browse files Browse the repository at this point in the history
  • Loading branch information
@haijianyang
haijianyang authored Apr 26, 2023
1 parent 339642c commit 8a3c2ed
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
26 changes: 22 additions & 4 deletions controllers/elfmachine_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,16 @@ func (r *ElfMachineReconciler) Reconcile(ctx goctx.Context, req ctrl.Request) (_

func (r *ElfMachineReconciler) reconcileDelete(ctx *context.MachineContext) (reconcile.Result, error) {
if !ipamutil.HasStaticIPDevice(ctx.ElfMachine.Spec.Network.Devices) {
ctrlutil.RemoveFinalizer(ctx.ElfMachine, MachineStaticIPFinalizer)
if ctrlutil.ContainsFinalizer(ctx.ElfMachine, MachineStaticIPFinalizer) {
ctx.Logger.V(1).Info("No static IP network device found, but MachineStaticIPFinalizer is set and remove it")

ctrlutil.RemoveFinalizer(ctx.ElfMachine, MachineStaticIPFinalizer)
}

return ctrl.Result{}, nil
}

ctx.Logger.V(1).Info("Reconciling ElfMachine IP delete")
ctx.Logger.V(1).Info("Reconciling ElfMachine IP delete", "finalizers", ctx.ElfMachine.Finalizers)

if ctrlutil.ContainsFinalizer(ctx.ElfMachine, capev1.MachineFinalizer) {
ctx.Logger.V(1).Info("Waiting for MachineFinalizer to be removed")
Expand All @@ -191,6 +196,8 @@ func (r *ElfMachineReconciler) reconcileDelete(ctx *context.MachineContext) (rec
if ipPool == nil {
ctrlutil.RemoveFinalizer(ctx.ElfMachine, MachineStaticIPFinalizer)

r.Logger.V(1).Info("IPPool is not found, remove MachineStaticIPFinalizer")

return ctrl.Result{}, nil
}

Expand All @@ -210,6 +217,7 @@ func (r *ElfMachineReconciler) reconcileDelete(ctx *context.MachineContext) (rec
}

ctrlutil.RemoveFinalizer(ctx.ElfMachine, MachineStaticIPFinalizer)
r.Logger.V(1).Info("The IPs used by Machine has been released, remove MachineStaticIPFinalizer")

return reconcile.Result{}, nil
}
Expand All @@ -234,8 +242,17 @@ func (r *ElfMachineReconciler) reconcileIPAddress(ctx *context.MachineContext) (

ctx.Logger.Info("Reconcile IP address")

// If the ElfMachine doesn't have our finalizer, add it.
ctrlutil.AddFinalizer(ctx.ElfMachine, MachineStaticIPFinalizer)
// Save MachineStaticIPFinalizer first and then allocate IP.
// If the IP has been allocated but the MachineStaticIPFinalizer has not been saved,
// deleting the Machine at this time may not release the IP.
//
// 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) {
ctrlutil.AddFinalizer(ctx.ElfMachine, MachineStaticIPFinalizer)

return ctrl.Result{RequeueAfter: 3 * time.Second}, nil
}

ipPool, err := r.getIPPool(ctx)
if err != nil {
Expand Down Expand Up @@ -322,6 +339,7 @@ func (r *ElfMachineReconciler) getIPPool(ctx *context.MachineContext) (ipam.IPPo
return nil, err
}
if ipPool == nil {
ctx.Logger.Info("IPPool is not found", "ipPoolNamespace", poolMatchLabels[ipam.ClusterIPPoolNamespaceKey], "ipPoolName", poolMatchLabels[ipam.ClusterIPPoolNameKey], "ipPoolGroupKey", poolMatchLabels[ipam.ClusterIPPoolGroupKey])
return nil, nil
}

Expand Down
21 changes: 21 additions & 0 deletions controllers/elfmachine_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ var _ = Describe("ElfMachineReconciler", func() {
})

It("should not reconcile without devices", func() {
ctrlutil.RemoveFinalizer(elfMachine, MachineStaticIPFinalizer)
elfMachine.Spec.Network.Devices = []capev1.NetworkDeviceSpec{}
ctrlContext := newCtrlContexts(elfCluster, cluster, elfMachine, machine, elfMachineTemplate)
fake.InitOwnerReferences(ctrlContext, elfCluster, cluster, elfMachine, machine)
Expand Down Expand Up @@ -149,7 +150,20 @@ var _ = Describe("ElfMachineReconciler", func() {
Expect(ctrlutil.ContainsFinalizer(elfMachine, MachineStaticIPFinalizer)).To(BeFalse())
})

It("should set MachineStaticIPFinalizer first", func() {
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())
})

It("should not reconcile when no cloned-from-name annotation", func() {
ctrlutil.AddFinalizer(elfMachine, MachineStaticIPFinalizer)
elfMachine.Annotations[capiv1.TemplateClonedFromNameAnnotation] = ""
ctrlContext := newCtrlContexts(elfCluster, cluster, elfMachine, machine, elfMachineTemplate)
fake.InitOwnerReferences(ctrlContext, elfCluster, cluster, elfMachine, machine)
Expand All @@ -164,6 +178,7 @@ var _ = Describe("ElfMachineReconciler", func() {
})

It("should not reconcile when no IPPool", func() {
ctrlutil.AddFinalizer(elfMachine, MachineStaticIPFinalizer)
ctrlContext := newCtrlContexts(elfCluster, cluster, elfMachine, machine, elfMachineTemplate)
fake.InitOwnerReferences(ctrlContext, elfCluster, cluster, elfMachine, machine)

Expand All @@ -177,6 +192,7 @@ var _ = Describe("ElfMachineReconciler", func() {
})

It("should create IPClaim and wait when no IPClaim", func() {
ctrlutil.AddFinalizer(elfMachine, MachineStaticIPFinalizer)
elfMachineTemplate.Labels[ipam.ClusterIPPoolNamespaceKey] = metal3IPPool.Namespace
elfMachineTemplate.Labels[ipam.ClusterIPPoolNameKey] = metal3IPPool.Name
ctrlContext := newCtrlContexts(elfCluster, cluster, elfMachine, machine, elfMachineTemplate, metal3IPPool)
Expand All @@ -198,6 +214,7 @@ var _ = Describe("ElfMachineReconciler", func() {
})

It("should wait for IP when IPClaim without IP", func() {
ctrlutil.AddFinalizer(elfMachine, MachineStaticIPFinalizer)
metal3IPPool.Labels = map[string]string{
ipam.ClusterIPPoolGroupKey: "ip-pool-group",
ipam.ClusterNetworkNameKey: "ip-pool-vm-network",
Expand All @@ -218,6 +235,7 @@ var _ = Describe("ElfMachineReconciler", func() {
})

It("should set IP for devices when IP ready", func() {
ctrlutil.AddFinalizer(elfMachine, MachineStaticIPFinalizer)
metal3IPPool.Namespace = ipam.DefaultIPPoolNamespace
metal3IPPool.Labels = map[string]string{ipam.DefaultIPPoolKey: "true"}
metal3IPPool.Spec.DNSServers = append(metal3IPPool.Spec.DNSServers, ipamv1.IPAddressStr("1.1.1.1"), ipamv1.IPAddressStr("4.4.4.4"))
Expand Down Expand Up @@ -247,6 +265,7 @@ var _ = Describe("ElfMachineReconciler", func() {
})

It("should remove MachineStaticIPFinalizer without IPV4 devices", func() {
ctrlutil.AddFinalizer(elfMachine, MachineStaticIPFinalizer)
elfMachine.Spec.Network.Devices = []capev1.NetworkDeviceSpec{{NetworkType: capev1.NetworkTypeIPV4DHCP}}
ctrlContext := newCtrlContexts(elfCluster, cluster, elfMachine, machine, elfMachineTemplate)
fake.InitOwnerReferences(ctrlContext, elfCluster, cluster, elfMachine, machine)
Expand All @@ -255,6 +274,7 @@ 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 static IP network device found, but MachineStaticIPFinalizer is set and remove it"))
Expect(ctrlContext.Client.Get(ctrlContext, capiutil.ObjectKey(elfMachine), elfMachine)).To(Succeed())
Expect(ctrlutil.ContainsFinalizer(elfMachine, MachineStaticIPFinalizer)).To(BeFalse())
})
Expand All @@ -268,6 +288,7 @@ 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("IPPool is not found, remove MachineStaticIPFinalizer"))
Expect(apierrors.IsNotFound(ctrlContext.Client.Get(ctrlContext, capiutil.ObjectKey(elfMachine), elfMachine))).To(BeTrue())
})

Expand Down

0 comments on commit 8a3c2ed

Please sign in to comment.