From 0587a491bc44faa4960959663082f54c4ebdc935 Mon Sep 17 00:00:00 2001 From: Georgi Chulkov Date: Wed, 17 Apr 2024 18:19:30 +0200 Subject: [PATCH] Improve MachineClaim controller (#38) --- .../controller/machineclaim_controller.go | 346 +++++++++++++----- .../machineclaim_controller_test.go | 145 ++++---- internal/controller/suite_test.go | 4 - internal/util/utils.go | 41 +++ 4 files changed, 375 insertions(+), 161 deletions(-) diff --git a/internal/controller/machineclaim_controller.go b/internal/controller/machineclaim_controller.go index fbb9d602..b2e15780 100644 --- a/internal/controller/machineclaim_controller.go +++ b/internal/controller/machineclaim_controller.go @@ -31,9 +31,9 @@ import ( // +kubebuilder:rbac:groups=metal.ironcore.dev,resources=machines/finalizers,verbs=update const ( - MachineClaimFieldOwner string = "metal.ironcore.dev/machineclaim" - MachineClaimFinalizer string = "metal.ironcore.dev/machineclaim" - MachineClaimSpecMachineRef string = ".spec.machineRef.Name" + MachineClaimFieldManager = "metal.ironcore.dev/machineclaim" + MachineClaimFinalizer = "metal.ironcore.dev/machineclaim" + MachineClaimSpecMachineRef = ".spec.machineRef.Name" ) func NewMachineClaimReconciler() (*MachineClaimReconciler, error) { @@ -66,130 +66,292 @@ func (r *MachineClaimReconciler) finalize(ctx context.Context, claim *metalv1alp } log.Debug(ctx, "Finalizing") - switch { - default: - if claim.Spec.MachineRef == nil { - break + err := r.finalizeMachine(ctx, claim) + if err != nil { + return err + } + + log.Debug(ctx, "Removing finalizer") + var apply *metalv1alpha1apply.MachineClaimApplyConfiguration + apply, err = metalv1alpha1apply.ExtractMachineClaim(claim, MachineClaimFieldManager) + if err != nil { + return err + } + apply.Finalizers = util.Clear(apply.Finalizers, MachineClaimFinalizer) + err = r.Patch(ctx, claim, ssa.Apply(apply), client.FieldOwner(MachineClaimFieldManager), client.ForceOwnership) + if err != nil { + return fmt.Errorf("cannot apply MachineClaim: %w", err) + } + + log.Debug(ctx, "Finalized successfully") + return nil +} + +func (r *MachineClaimReconciler) finalizeMachine(ctx context.Context, claim *metalv1alpha1.MachineClaim) error { + if claim.Spec.MachineRef == nil { + return nil + } + ctx = log.WithValues(ctx, "machine", claim.Spec.MachineRef.Name) + + var machine metalv1alpha1.Machine + err := r.Get(ctx, client.ObjectKey{ + Name: claim.Spec.MachineRef.Name, + }, &machine) + if err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("cannot get Machine: %w", err) + } + if errors.IsNotFound(err) { + return nil + } + + if machine.Spec.MachineClaimRef == nil { + return nil + } + if machine.Spec.MachineClaimRef.UID != claim.UID { + return fmt.Errorf("MachineClaimRef in Machine does not match MachineClaim UID") + } + + log.Debug(ctx, "Removing finalizer from Machine and clearing MachineClaimRef and Power") + var machineApply *metalv1alpha1apply.MachineApplyConfiguration + machineApply, err = metalv1alpha1apply.ExtractMachine(&machine, MachineClaimFieldManager) + if err != nil { + return err + } + machineApply.Finalizers = util.Clear(machineApply.Finalizers, MachineClaimFinalizer) + machineApply.Spec = nil + err = r.Patch(ctx, &machine, ssa.Apply(machineApply), client.FieldOwner(MachineClaimFieldManager), client.ForceOwnership) + if err != nil { + return fmt.Errorf("cannot apply Machine: %w", err) + } + + return nil +} + +func (r *MachineClaimReconciler) reconcile(ctx context.Context, claim *metalv1alpha1.MachineClaim) (ctrl.Result, error) { + log.Debug(ctx, "Reconciling") + + var ok bool + var err error + + ctx, ok, err = r.applyOrContinue(log.WithValues(ctx, "phase", "InitialState"), claim, r.processInitialState) + if !ok { + if err == nil { + log.Debug(ctx, "Reconciled successfully") } - ctx = log.WithValues(ctx, "machine", claim.Spec.MachineRef.Name) + return ctrl.Result{}, err + } - log.Debug(ctx, "Getting Machine") - var machine metalv1alpha1.Machine - err := r.Get(ctx, client.ObjectKey{Name: claim.Spec.MachineRef.Name}, &machine) - if err != nil { - if errors.IsNotFound(err) { - break - } - return fmt.Errorf("cannot get Machine: %w", err) + ctx, ok, err = r.applyOrContinue(log.WithValues(ctx, "phase", "Machine"), claim, r.processMachine) + if !ok { + if err == nil { + log.Debug(ctx, "Reconciled successfully") } - if machine.Spec.MachineClaimRef == nil { - break + return ctrl.Result{}, err + } + + ctx = log.WithValues(ctx, "phase", "all") + log.Debug(ctx, "Reconciled successfully") + return ctrl.Result{}, nil +} + +type nachineClaimProcessFunc func(context.Context, *metalv1alpha1.MachineClaim) (context.Context, *metalv1alpha1apply.MachineClaimApplyConfiguration, *metalv1alpha1apply.MachineClaimStatusApplyConfiguration, error) + +func (r *MachineClaimReconciler) applyOrContinue(ctx context.Context, claim *metalv1alpha1.MachineClaim, pfunc nachineClaimProcessFunc) (context.Context, bool, error) { + var apply *metalv1alpha1apply.MachineClaimApplyConfiguration + var status *metalv1alpha1apply.MachineClaimStatusApplyConfiguration + var err error + + ctx, apply, status, err = pfunc(ctx, claim) + if err != nil { + return ctx, false, err + } + + if apply != nil { + log.Debug(ctx, "Applying") + err = r.Patch(ctx, claim, ssa.Apply(apply), client.FieldOwner(MachineClaimFieldManager), client.ForceOwnership) + if err != nil { + return ctx, false, fmt.Errorf("cannot apply MachineClaim: %w", err) } - if machine.Spec.MachineClaimRef.UID != claim.UID { - return fmt.Errorf("MachineClaimRef in Machine does not match MachineClaim UID") + } + + if status != nil { + apply = metalv1alpha1apply.MachineClaim(claim.Name, claim.Namespace).WithStatus(status) + + log.Debug(ctx, "Applying status") + err = r.Status().Patch(ctx, claim, ssa.Apply(apply), client.FieldOwner(MachineClaimFieldManager), client.ForceOwnership) + if err != nil { + return ctx, false, fmt.Errorf("cannot apply MachineClaim status: %w", err) } + } + + return ctx, apply == nil, err +} - log.Debug(ctx, "Updating Machine") - machineApply := metalv1alpha1apply.Machine(machine.Name, machine.Namespace).WithFinalizers().WithSpec(metalv1alpha1apply.MachineSpec()) - err = r.Patch(ctx, &machine, ssa.Apply(machineApply), client.FieldOwner(MachineClaimFieldOwner), client.ForceOwnership) +func (r *MachineClaimReconciler) processInitialState(ctx context.Context, claim *metalv1alpha1.MachineClaim) (context.Context, *metalv1alpha1apply.MachineClaimApplyConfiguration, *metalv1alpha1apply.MachineClaimStatusApplyConfiguration, error) { + var apply *metalv1alpha1apply.MachineClaimApplyConfiguration + var status *metalv1alpha1apply.MachineClaimStatusApplyConfiguration + var err error + + if !controllerutil.ContainsFinalizer(claim, MachineClaimFinalizer) { + apply, err = metalv1alpha1apply.ExtractMachineClaim(claim, MachineClaimFieldManager) if err != nil { - return fmt.Errorf("cannot patch Machine: %w", err) + return ctx, nil, nil, err } + apply.Finalizers = util.Set(apply.Finalizers, MachineClaimFinalizer) } - log.Debug(ctx, "Removing finalizer") - apply := metalv1alpha1apply.MachineClaim(claim.Name, claim.Namespace).WithFinalizers() - err := r.Patch(ctx, claim, ssa.Apply(apply), client.FieldOwner(MachineClaimFieldOwner), client.ForceOwnership) - if err != nil { - return fmt.Errorf("cannot remove finalizer: %w", err) + if claim.Status.Phase == "" { + var applyst *metalv1alpha1apply.MachineClaimApplyConfiguration + applyst, err = metalv1alpha1apply.ExtractMachineClaimStatus(claim, MachineClaimFieldManager) + if err != nil { + return ctx, nil, nil, err + } + status = util.Ensure(applyst.Status). + WithPhase(metalv1alpha1.MachineClaimPhaseUnbound) } - log.Debug(ctx, "Finalized successfully") - return nil + return ctx, apply, status, nil } -func (r *MachineClaimReconciler) reconcile(ctx context.Context, claim *metalv1alpha1.MachineClaim) (ctrl.Result, error) { - log.Debug(ctx, "Reconciling") +func (r *MachineClaimReconciler) processMachine(ctx context.Context, claim *metalv1alpha1.MachineClaim) (context.Context, *metalv1alpha1apply.MachineClaimApplyConfiguration, *metalv1alpha1apply.MachineClaimStatusApplyConfiguration, error) { + var apply *metalv1alpha1apply.MachineClaimApplyConfiguration + var status *metalv1alpha1apply.MachineClaimStatusApplyConfiguration + var err error - applySpec := metalv1alpha1apply.MachineClaimSpec() - applyStatus := metalv1alpha1apply.MachineClaimStatus().WithPhase(metalv1alpha1.MachineClaimPhaseUnbound) - - var machines []metalv1alpha1.Machine + var machine metalv1alpha1.Machine if claim.Spec.MachineRef != nil { - log.Debug(ctx, "Getting referenced Machine") - var machine metalv1alpha1.Machine - err := r.Get(ctx, client.ObjectKey{Name: claim.Spec.MachineRef.Name}, &machine) + err = r.Get(ctx, client.ObjectKey{ + Name: claim.Spec.MachineRef.Name, + }, &machine) if err != nil && !errors.IsNotFound(err) { - return ctrl.Result{}, fmt.Errorf("cannot get Machine: %w", err) + return ctx, nil, nil, fmt.Errorf("cannot get Machine: %w", err) } - if !errors.IsNotFound(err) { - machines = append(machines, machine) + + if errors.IsNotFound(err) { + claim.Spec.MachineRef = nil + + apply, err = metalv1alpha1apply.ExtractMachineClaim(claim, MachineClaimFieldManager) + if err != nil { + return ctx, nil, nil, err + } + apply = apply.WithSpec(util.Ensure(apply.Spec)) + apply.Spec.MachineRef = nil } - } else if claim.Spec.MachineSelector != nil { - log.Debug(ctx, "Listing Machines with matching labels") + } + if claim.Spec.MachineRef == nil { var machineList metalv1alpha1.MachineList - err := r.List(ctx, &machineList, client.MatchingLabels(claim.Spec.MachineSelector.MatchLabels)) - if err != nil { - return ctrl.Result{}, fmt.Errorf("cannot list Machines: %w", err) + if claim.Spec.MachineSelector != nil { + err = r.List(ctx, &machineList, client.MatchingLabels(claim.Spec.MachineSelector.MatchLabels)) + if err != nil { + return ctx, nil, nil, fmt.Errorf("cannot list Machines: %w", err) + } } - machines = machineList.Items - } - for _, m := range machines { - if m.Status.State != metalv1alpha1.MachineStateReady { - continue + found := false + for _, m := range machineList.Items { + if m.DeletionTimestamp != nil || m.Status.State != metalv1alpha1.MachineStateReady || (m.Spec.MachineClaimRef != nil && m.Spec.MachineClaimRef.UID != claim.UID) { + continue + } + machine = m + found = true + ctx = log.WithValues(ctx, "machine", machine.Name) + + claim.Spec.MachineRef = &v1.LocalObjectReference{ + Name: machine.Name, + } + + if apply == nil { + apply, err = metalv1alpha1apply.ExtractMachineClaim(claim, MachineClaimFieldManager) + if err != nil { + return ctx, nil, nil, err + } + } + apply = apply.WithSpec(util.Ensure(apply.Spec). + WithMachineRef(*claim.Spec.MachineRef)) + + break } - if m.Spec.MachineClaimRef != nil && m.Spec.MachineClaimRef.UID != claim.UID { - continue + if !found { + phase := metalv1alpha1.MachineClaimPhaseUnbound + if claim.Status.Phase != phase { + var applyst *metalv1alpha1apply.MachineClaimApplyConfiguration + applyst, err = metalv1alpha1apply.ExtractMachineClaimStatus(claim, MachineClaimFieldManager) + if err != nil { + return ctx, nil, nil, err + } + status = util.Ensure(applyst.Status). + WithPhase(phase) + } + + return ctx, apply, status, nil } + } - machine := m - ctx = log.WithValues(ctx, "machine", machine.Name) + claimRef := v1.ObjectReference{ + Namespace: claim.Namespace, + Name: claim.Name, + UID: claim.UID, + } - machineApply := metalv1alpha1apply.Machine(machine.Name, machine.Namespace).WithFinalizers(MachineClaimFinalizer).WithSpec(metalv1alpha1apply.MachineSpec(). - WithMachineClaimRef(v1.ObjectReference{ - Namespace: claim.Namespace, - Name: claim.Name, - UID: claim.UID, - }). - WithPower(claim.Spec.Power)) - if !controllerutil.ContainsFinalizer(&machine, MachineClaimFinalizer) || - !util.NilOrEqual(machine.Spec.MachineClaimRef, machineApply.Spec.MachineClaimRef) || - machine.Spec.Power != *machineApply.Spec.Power { - log.Debug(ctx, "Updating Machine") - err := r.Patch(ctx, &machine, ssa.Apply(machineApply), client.FieldOwner(MachineClaimFieldOwner), client.ForceOwnership) + if machine.Status.State != metalv1alpha1.MachineStateReady { + log.Debug(ctx, "Removing finalizer from Machine and clearing MachineClaimRef and Power") + var machineApply *metalv1alpha1apply.MachineApplyConfiguration + machineApply, err = metalv1alpha1apply.ExtractMachine(&machine, MachineClaimFieldManager) + if err != nil { + return ctx, nil, nil, err + } + machineApply.Finalizers = util.Clear(machineApply.Finalizers, MachineClaimFinalizer) + machineApply = nil + err = r.Patch(ctx, &machine, ssa.Apply(machineApply), client.FieldOwner(MachineClaimFieldManager), client.ForceOwnership) + if err != nil { + return ctx, nil, nil, fmt.Errorf("cannot apply Machine: %w", err) + } + + phase := metalv1alpha1.MachineClaimPhaseUnbound + if claim.Status.Phase != phase { + var applyst *metalv1alpha1apply.MachineClaimApplyConfiguration + applyst, err = metalv1alpha1apply.ExtractMachineClaimStatus(claim, MachineClaimFieldManager) if err != nil { - return ctrl.Result{}, fmt.Errorf("cannot patch Machine: %w", err) + return ctx, nil, nil, err } + status = util.Ensure(applyst.Status). + WithPhase(phase) } - applySpec = applySpec.WithMachineRef(v1.LocalObjectReference{Name: machine.Name}) - applyStatus = applyStatus.WithPhase(metalv1alpha1.MachineClaimPhaseBound) - - break + return ctx, apply, status, nil } - apply := metalv1alpha1apply.MachineClaim(claim.Name, claim.Namespace).WithFinalizers(MachineClaimFinalizer).WithSpec(applySpec) - if !controllerutil.ContainsFinalizer(claim, MachineClaimFinalizer) || - !util.NilOrEqual(claim.Spec.MachineRef, apply.Spec.MachineRef) { - log.Debug(ctx, "Updating") - err := r.Patch(ctx, claim, ssa.Apply(apply), client.FieldOwner(MachineClaimFieldOwner), client.ForceOwnership) + if !controllerutil.ContainsFinalizer(&machine, MachineClaimFinalizer) || + !util.NilOrEqual(machine.Spec.MachineClaimRef, &claimRef) || + machine.Spec.Power != claim.Spec.Power { + log.Debug(ctx, "Adding finalizer to Machine and setting MachineClaimRef and Power") + var machineApply *metalv1alpha1apply.MachineApplyConfiguration + machineApply, err = metalv1alpha1apply.ExtractMachine(&machine, MachineClaimFieldManager) + if err != nil { + return ctx, nil, nil, err + } + machineApply.Finalizers = util.Set(machineApply.Finalizers, MachineClaimFinalizer) + machineApply = machineApply.WithSpec(util.Ensure(machineApply.Spec). + WithMachineClaimRef(claimRef). + WithPower(claim.Spec.Power)) + err = r.Patch(ctx, &machine, ssa.Apply(machineApply), client.FieldOwner(MachineClaimFieldManager), client.ForceOwnership) if err != nil { - return ctrl.Result{}, fmt.Errorf("cannot patch MachineClaim: %w", err) + return ctx, nil, nil, fmt.Errorf("cannot apply Machine: %w", err) } } - apply = metalv1alpha1apply.MachineClaim(claim.Name, claim.Namespace).WithStatus(applyStatus) - if claim.Status.Phase != *apply.Status.Phase { - log.Debug(ctx, "Updating status") - err := r.Status().Patch(ctx, claim, ssa.Apply(apply), client.FieldOwner(MachineClaimFieldOwner), client.ForceOwnership) + phase := metalv1alpha1.MachineClaimPhaseBound + if claim.Status.Phase != phase { + var applyst *metalv1alpha1apply.MachineClaimApplyConfiguration + applyst, err = metalv1alpha1apply.ExtractMachineClaimStatus(claim, MachineClaimFieldManager) if err != nil { - return ctrl.Result{}, fmt.Errorf("cannot patch MachineClaim status: %w", err) + return ctx, nil, nil, err } + status = util.Ensure(applyst.Status). + WithPhase(phase) } - log.Debug(ctx, "Reconciled successfully") - return ctrl.Result{}, nil + return ctx, apply, status, nil } // SetupWithManager sets up the controller with the Manager. @@ -206,25 +368,25 @@ func (r *MachineClaimReconciler) enqueueMachineClaimsFromMachine() handler.Event return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []reconcile.Request { machine := obj.(*metalv1alpha1.Machine) - claimList := &metalv1alpha1.MachineClaimList{} - err := r.List(ctx, claimList, client.MatchingFields{MachineClaimSpecMachineRef: machine.Name}) + claimList := metalv1alpha1.MachineClaimList{} + err := r.List(ctx, &claimList, client.MatchingFields{MachineClaimSpecMachineRef: machine.Name}) if err != nil { log.Error(ctx, fmt.Errorf("cannot list MachineClaims: %w", err)) return nil } - var req []reconcile.Request + var reqs []reconcile.Request for _, c := range claimList.Items { if c.DeletionTimestamp != nil { continue } // TODO: Also watch for machines matching the label selector. - req = append(req, reconcile.Request{NamespacedName: types.NamespacedName{ + reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{ Namespace: c.Namespace, Name: c.Name, }}) } - return req + return reqs }) } diff --git a/internal/controller/machineclaim_controller_test.go b/internal/controller/machineclaim_controller_test.go index e8f09810..ae8a187b 100644 --- a/internal/controller/machineclaim_controller_test.go +++ b/internal/controller/machineclaim_controller_test.go @@ -30,7 +30,7 @@ var _ = Describe("MachineClaim Controller", func() { It("should claim a Machine by ref", func(ctx SpecContext) { By("Creating a Machine") - machine := metalv1alpha1.Machine{ + machine := &metalv1alpha1.Machine{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-", }, @@ -41,16 +41,19 @@ var _ = Describe("MachineClaim Controller", func() { }, }, } - Expect(k8sClient.Create(ctx, &machine)).To(Succeed()) - DeferCleanup(k8sClient.Delete, &machine) + Expect(k8sClient.Create(ctx, machine)).To(Succeed()) + DeferCleanup(func(ctx SpecContext) { + Expect(k8sClient.Delete(ctx, machine)).To(Succeed()) + Eventually(Get(machine)).Should(Satisfy(errors.IsNotFound)) + }) By("Patching Machine state to Ready") - Eventually(UpdateStatus(&machine, func() { + Eventually(UpdateStatus(machine, func() { machine.Status.State = metalv1alpha1.MachineStateReady })).Should(Succeed()) By("Creating a MachineClaim referencing the Machine") - claim := metalv1alpha1.MachineClaim{ + claim := &metalv1alpha1.MachineClaim{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-", Namespace: ns.Name, @@ -63,38 +66,38 @@ var _ = Describe("MachineClaim Controller", func() { Power: metalv1alpha1.PowerOn, }, } - Expect(k8sClient.Create(ctx, &claim)).To(Succeed()) + Expect(k8sClient.Create(ctx, claim)).To(Succeed()) By("Expecting finalizer and phase to be correct on the MachineClaim") - Eventually(Object(&claim)).Should(SatisfyAll( + Eventually(Object(claim)).Should(SatisfyAll( HaveField("Finalizers", ContainElement(MachineClaimFinalizer)), - HaveField("Status.Phase", Equal(metalv1alpha1.MachineClaimPhaseBound)), + HaveField("Status.Phase", metalv1alpha1.MachineClaimPhaseBound), )) By("Expecting finalizer and machineclaimref to be correct on the Machine") - Eventually(Object(&machine)).Should(SatisfyAll( + Eventually(Object(machine)).Should(SatisfyAll( HaveField("Finalizers", ContainElement(MachineClaimFinalizer)), - HaveField("Spec.MachineClaimRef.Namespace", Equal(claim.Namespace)), - HaveField("Spec.MachineClaimRef.Name", Equal(claim.Name)), - HaveField("Spec.MachineClaimRef.UID", Equal(claim.UID)), + HaveField("Spec.MachineClaimRef.Namespace", claim.Namespace), + HaveField("Spec.MachineClaimRef.Name", claim.Name), + HaveField("Spec.MachineClaimRef.UID", claim.UID), )) By("Deleting the MachineClaim") - Expect(k8sClient.Delete(ctx, &claim)).To(Succeed()) + Expect(k8sClient.Delete(ctx, claim)).To(Succeed()) By("Expecting machineclaimref and finalizer to be removed from the Machine") - Eventually(Object(&machine)).Should(SatisfyAll( + Eventually(Object(machine)).Should(SatisfyAll( HaveField("Finalizers", Not(ContainElement(MachineClaimFinalizer))), HaveField("Spec.MachineClaimRef", BeNil()), )) By("Expecting MachineClaim to be removed") - Eventually(Get(&claim)).Should(Satisfy(errors.IsNotFound)) + Eventually(Get(claim)).Should(Satisfy(errors.IsNotFound)) }) It("should claim a Machine by selector", func(ctx SpecContext) { By("Creating a Machine") - machine := metalv1alpha1.Machine{ + machine := &metalv1alpha1.Machine{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-", Labels: map[string]string{ @@ -108,16 +111,19 @@ var _ = Describe("MachineClaim Controller", func() { }, }, } - Expect(k8sClient.Create(ctx, &machine)).To(Succeed()) - DeferCleanup(k8sClient.Delete, &machine) + Expect(k8sClient.Create(ctx, machine)).To(Succeed()) + DeferCleanup(func(ctx SpecContext) { + Expect(k8sClient.Delete(ctx, machine)).To(Succeed()) + Eventually(Get(machine)).Should(Satisfy(errors.IsNotFound)) + }) By("Patching Machine state to Ready") - Eventually(UpdateStatus(&machine, func() { + Eventually(UpdateStatus(machine, func() { machine.Status.State = metalv1alpha1.MachineStateReady })).Should(Succeed()) By("Creating a MachineClaim with a matching selector") - claim := metalv1alpha1.MachineClaim{ + claim := &metalv1alpha1.MachineClaim{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-", Namespace: ns.Name, @@ -132,39 +138,39 @@ var _ = Describe("MachineClaim Controller", func() { Power: metalv1alpha1.PowerOn, }, } - Expect(k8sClient.Create(ctx, &claim)).To(Succeed()) + Expect(k8sClient.Create(ctx, claim)).To(Succeed()) By("Expecting finalizer, machineref, and phase to be correct on the MachineClaim") - Eventually(Object(&claim)).Should(SatisfyAll( + Eventually(Object(claim)).Should(SatisfyAll( HaveField("Finalizers", ContainElement(MachineClaimFinalizer)), - HaveField("Spec.MachineRef.Name", Equal(machine.Name)), - HaveField("Status.Phase", Equal(metalv1alpha1.MachineClaimPhaseBound)), + HaveField("Spec.MachineRef.Name", machine.Name), + HaveField("Status.Phase", metalv1alpha1.MachineClaimPhaseBound), )) By("Expecting finalizer and machineclaimref to be correct on the Machine") - Eventually(Object(&machine)).Should(SatisfyAll( + Eventually(Object(machine)).Should(SatisfyAll( HaveField("Finalizers", ContainElement(MachineClaimFinalizer)), - HaveField("Spec.MachineClaimRef.Namespace", Equal(claim.Namespace)), - HaveField("Spec.MachineClaimRef.Name", Equal(claim.Name)), - HaveField("Spec.MachineClaimRef.UID", Equal(claim.UID)), + HaveField("Spec.MachineClaimRef.Namespace", claim.Namespace), + HaveField("Spec.MachineClaimRef.Name", claim.Name), + HaveField("Spec.MachineClaimRef.UID", claim.UID), )) By("Deleting the MachineClaim") - Expect(k8sClient.Delete(ctx, &claim)).To(Succeed()) + Expect(k8sClient.Delete(ctx, claim)).To(Succeed()) By("Expecting machineclaimref and finalizer to be removed from the Machine") - Eventually(Object(&machine)).Should(SatisfyAll( + Eventually(Object(machine)).Should(SatisfyAll( HaveField("Finalizers", Not(ContainElement(MachineClaimFinalizer))), HaveField("Spec.MachineClaimRef", BeNil()), )) By("Expecting MachineClaim to be removed") - Eventually(Get(&claim)).Should(Satisfy(errors.IsNotFound)) + Eventually(Get(claim)).Should(Satisfy(errors.IsNotFound)) }) It("should not claim a Machine with a wrong ref", func(ctx SpecContext) { By("Creating a MachineClaim referencing the Machine") - claim := metalv1alpha1.MachineClaim{ + claim := &metalv1alpha1.MachineClaim{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-", Namespace: ns.Name, @@ -177,24 +183,24 @@ var _ = Describe("MachineClaim Controller", func() { Power: metalv1alpha1.PowerOn, }, } - Expect(k8sClient.Create(ctx, &claim)).To(Succeed()) + Expect(k8sClient.Create(ctx, claim)).To(Succeed()) By("Expecting finalizer and phase to be correct on the MachineClaim") - Eventually(Object(&claim)).Should(SatisfyAll( + Eventually(Object(claim)).Should(SatisfyAll( HaveField("Finalizers", ContainElement(MachineClaimFinalizer)), - HaveField("Status.Phase", Equal(metalv1alpha1.MachineClaimPhaseUnbound)), + HaveField("Status.Phase", metalv1alpha1.MachineClaimPhaseUnbound), )) By("Deleting the MachineClaim") - Expect(k8sClient.Delete(ctx, &claim)).To(Succeed()) + Expect(k8sClient.Delete(ctx, claim)).To(Succeed()) By("Expecting MachineClaim to be removed") - Eventually(Get(&claim)).Should(Satisfy(errors.IsNotFound)) + Eventually(Get(claim)).Should(Satisfy(errors.IsNotFound)) }) It("should not claim a Machine with no matching selector", func(ctx SpecContext) { By("Creating a Machine") - machine := metalv1alpha1.Machine{ + machine := &metalv1alpha1.Machine{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-", Labels: map[string]string{ @@ -208,16 +214,19 @@ var _ = Describe("MachineClaim Controller", func() { }, }, } - Expect(k8sClient.Create(ctx, &machine)).To(Succeed()) - DeferCleanup(k8sClient.Delete, &machine) + Expect(k8sClient.Create(ctx, machine)).To(Succeed()) + DeferCleanup(func(ctx SpecContext) { + Expect(k8sClient.Delete(ctx, machine)).To(Succeed()) + Eventually(Get(machine)).Should(Satisfy(errors.IsNotFound)) + }) By("Patching Machine state to Ready") - Eventually(UpdateStatus(&machine, func() { + Eventually(UpdateStatus(machine, func() { machine.Status.State = metalv1alpha1.MachineStateReady })).Should(Succeed()) By("Creating a MachineClaim referencing the Machine") - claim := metalv1alpha1.MachineClaim{ + claim := &metalv1alpha1.MachineClaim{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-", Namespace: ns.Name, @@ -232,30 +241,30 @@ var _ = Describe("MachineClaim Controller", func() { Power: metalv1alpha1.PowerOn, }, } - Expect(k8sClient.Create(ctx, &claim)).To(Succeed()) + Expect(k8sClient.Create(ctx, claim)).To(Succeed()) By("Expecting finalizer and phase to be correct on the MachineClaim") - Eventually(Object(&claim)).Should(SatisfyAll( + Eventually(Object(claim)).Should(SatisfyAll( HaveField("Finalizers", ContainElement(MachineClaimFinalizer)), - HaveField("Status.Phase", Equal(metalv1alpha1.MachineClaimPhaseUnbound)), + HaveField("Status.Phase", metalv1alpha1.MachineClaimPhaseUnbound), )) By("Expecting no finalizer or claimref on the Machine") - Eventually(Object(&machine)).Should(SatisfyAll( + Eventually(Object(machine)).Should(SatisfyAll( HaveField("Finalizers", Not(ContainElement(MachineClaimFinalizer))), HaveField("Spec.MachineClaimRef", BeNil()), )) By("Deleting the MachineClaim") - Expect(k8sClient.Delete(ctx, &claim)).To(Succeed()) + Expect(k8sClient.Delete(ctx, claim)).To(Succeed()) By("Expecting MachineClaim to be removed") - Eventually(Get(&claim)).Should(Satisfy(errors.IsNotFound)) + Eventually(Get(claim)).Should(Satisfy(errors.IsNotFound)) }) It("should claim a Machine by ref once the Machine becomes Ready", func(ctx SpecContext) { By("Creating a Machine") - machine := metalv1alpha1.Machine{ + machine := &metalv1alpha1.Machine{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-", }, @@ -266,16 +275,19 @@ var _ = Describe("MachineClaim Controller", func() { }, }, } - Expect(k8sClient.Create(ctx, &machine)).To(Succeed()) - DeferCleanup(k8sClient.Delete, &machine) + Expect(k8sClient.Create(ctx, machine)).To(Succeed()) + DeferCleanup(func(ctx SpecContext) { + Expect(k8sClient.Delete(ctx, machine)).To(Succeed()) + Eventually(Get(machine)).Should(Satisfy(errors.IsNotFound)) + }) By("Patching Machine state to Error") - Eventually(UpdateStatus(&machine, func() { + Eventually(UpdateStatus(machine, func() { machine.Status.State = metalv1alpha1.MachineStateError })).Should(Succeed()) By("Creating a MachineClaim referencing the Machine") - claim := metalv1alpha1.MachineClaim{ + claim := &metalv1alpha1.MachineClaim{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "test-", Namespace: ns.Name, @@ -288,38 +300,41 @@ var _ = Describe("MachineClaim Controller", func() { Power: metalv1alpha1.PowerOn, }, } - Expect(k8sClient.Create(ctx, &claim)).To(Succeed()) - DeferCleanup(k8sClient.Delete, &claim) + Expect(k8sClient.Create(ctx, claim)).To(Succeed()) + DeferCleanup(func(ctx SpecContext) { + Expect(k8sClient.Delete(ctx, claim)).To(Succeed()) + Eventually(Get(claim)).Should(Satisfy(errors.IsNotFound)) + }) By("Expecting finalizer and phase to be correct on the MachineClaim") - Eventually(Object(&claim)).Should(SatisfyAll( + Eventually(Object(claim)).Should(SatisfyAll( HaveField("Finalizers", ContainElement(MachineClaimFinalizer)), - HaveField("Status.Phase", Equal(metalv1alpha1.MachineClaimPhaseUnbound)), + HaveField("Status.Phase", metalv1alpha1.MachineClaimPhaseUnbound), )) By("Expecting no finalizer or claimref on the Machine") - Eventually(Object(&machine)).Should(SatisfyAll( + Eventually(Object(machine)).Should(SatisfyAll( HaveField("Finalizers", Not(ContainElement(MachineClaimFinalizer))), HaveField("Spec.MachineClaimRef", BeNil()), )) By("Patching Machine state to Ready") - Eventually(UpdateStatus(&machine, func() { + Eventually(UpdateStatus(machine, func() { machine.Status.State = metalv1alpha1.MachineStateReady })).Should(Succeed()) By("Expecting finalizer and phase to be correct on the MachineClaim") - Eventually(Object(&claim)).Should(SatisfyAll( + Eventually(Object(claim)).Should(SatisfyAll( HaveField("Finalizers", ContainElement(MachineClaimFinalizer)), - HaveField("Status.Phase", Equal(metalv1alpha1.MachineClaimPhaseBound)), + HaveField("Status.Phase", metalv1alpha1.MachineClaimPhaseBound), )) By("Expecting finalizer and machineclaimref to be correct on the Machine") - Eventually(Object(&machine)).Should(SatisfyAll( + Eventually(Object(machine)).Should(SatisfyAll( HaveField("Finalizers", ContainElement(MachineClaimFinalizer)), - HaveField("Spec.MachineClaimRef.Namespace", Equal(claim.Namespace)), - HaveField("Spec.MachineClaimRef.Name", Equal(claim.Name)), - HaveField("Spec.MachineClaimRef.UID", Equal(claim.UID)), + HaveField("Spec.MachineClaimRef.Namespace", claim.Namespace), + HaveField("Spec.MachineClaimRef.Name", claim.Name), + HaveField("Spec.MachineClaimRef.UID", claim.UID), )) }) }) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 09eb80a3..c5a72345 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -56,10 +56,6 @@ var _ = BeforeSuite(func() { ctrl.SetLogger(l) scheme := runtime.NewScheme() - Expect(kscheme.AddToScheme(scheme)).To(Succeed()) - Expect(metalv1alpha1.AddToScheme(scheme)).To(Succeed()) - //+kubebuilder:scaffold:scheme - Expect(kscheme.AddToScheme(scheme)).To(Succeed()) Expect(metalv1alpha1.AddToScheme(scheme)).To(Succeed()) Expect(ipamv1alpha1.AddToScheme(scheme)).To(Succeed()) diff --git a/internal/util/utils.go b/internal/util/utils.go index b07e9d44..4722db55 100644 --- a/internal/util/utils.go +++ b/internal/util/utils.go @@ -3,6 +3,47 @@ package util +import ( + "slices" +) + func NilOrEqual[T comparable](x, y *T) bool { return (x == nil && y == nil) || (x != nil && y != nil && *x == *y) } + +func Ensure[T any](x *T) *T { + if x == nil { + return new(T) + } + return x +} + +func Set[T comparable](s []T, x T) []T { + for _, e := range s { + if e == x { + return s + } + } + return append(s, x) +} + +func Clear[T comparable](s []T, x T) []T { + for i, e := range s { + if e == x { + return slices.Concat(s[:i], s[i+1:]) + } + } + return s +} + +type PrefixMap[T any] map[string]T + +func (m PrefixMap[T]) Get(p string) (T, bool) { + for i := len(p); i > 0; i-- { + l, ok := m[p[:i]] + if ok { + return l, true + } + } + return *new(T), false +}