From f8bef005f4366b556abea56ad9f8244631bc6c0a Mon Sep 17 00:00:00 2001 From: Andreas Fritzler Date: Thu, 15 Aug 2024 15:23:11 +0200 Subject: [PATCH] Address sporadic in ServerClaim test --- internal/controller/serverclaim_controller.go | 37 ++++++++++--------- .../controller/serverclaim_controller_test.go | 25 +++++++++---- internal/controller/suite_test.go | 2 +- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/internal/controller/serverclaim_controller.go b/internal/controller/serverclaim_controller.go index 034bfbc..8c024bd 100644 --- a/internal/controller/serverclaim_controller.go +++ b/internal/controller/serverclaim_controller.go @@ -153,7 +153,7 @@ func (r *ServerClaimReconciler) reconcile(ctx context.Context, log logr.Logger, return ctrl.Result{}, nil } - if modified, err := r.patchServerRef(ctx, claim, server.Name); err != nil || modified { + if modified, err := r.patchServerRef(ctx, claim, server); err != nil || modified { return ctrl.Result{}, err } log.V(1).Info("Patched ServerRef in Claim") @@ -161,22 +161,25 @@ func (r *ServerClaimReconciler) reconcile(ctx context.Context, log logr.Logger, if err := r.applyBootConfiguration(ctx, log, server, claim); err != nil { return ctrl.Result{}, fmt.Errorf("failed to apply boot configuration: %w", err) } + log.V(1).Info("Applied BootConfiguration for ServerClaim") - serverBase := server.DeepCopy() - server.Spec.ServerClaimRef = &v1.ObjectReference{ - APIVersion: "metal.ironcore.dev/v1alpha1", - Kind: "ServerClaim", - Namespace: claim.Namespace, - Name: claim.Name, - UID: claim.UID, - } - server.Spec.Power = claim.Spec.Power - if err := r.Patch(ctx, server, client.MergeFrom(serverBase)); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch claim ref for server: %w", err) + if server.Spec.ServerClaimRef == nil { + serverBase := server.DeepCopy() + server.Spec.ServerClaimRef = &v1.ObjectReference{ + APIVersion: "metal.ironcore.dev/v1alpha1", + Kind: "ServerClaim", + Namespace: claim.Namespace, + Name: claim.Name, + UID: claim.UID, + } + server.Spec.Power = claim.Spec.Power + if err := r.Patch(ctx, server, client.MergeFrom(serverBase)); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch claim ref for server: %w", err) + } + log.V(1).Info("Patched ServerClaim reference on Server", "Server", server.Name, "ServerClaimRef", claim.Name) } - log.V(1).Info("Patched ServerClaim reference on Server", "Server", server.Name, "ServerClaimRef", claim.Name) - if modified, err := r.patchServerClaimPhase(ctx, claim, metalv1alpha1.PhaseBound); err != nil || modified { + if _, err := r.patchServerClaimPhase(ctx, claim, metalv1alpha1.PhaseBound); err != nil { return ctrl.Result{}, err } @@ -196,17 +199,17 @@ func (r *ServerClaimReconciler) patchServerClaimPhase(ctx context.Context, claim return true, nil } -func (r *ServerClaimReconciler) patchServerRef(ctx context.Context, claim *metalv1alpha1.ServerClaim, server string) (bool, error) { +func (r *ServerClaimReconciler) patchServerRef(ctx context.Context, claim *metalv1alpha1.ServerClaim, server *metalv1alpha1.Server) (bool, error) { if claim.Spec.ServerRef == nil { claimBase := claim.DeepCopy() - claim.Spec.ServerRef = &v1.LocalObjectReference{Name: server} + claim.Spec.ServerRef = &v1.LocalObjectReference{Name: server.Name} if err := r.Patch(ctx, claim, client.MergeFrom(claimBase)); err != nil { return false, err } return true, nil } - if claim.Spec.ServerRef != nil && claim.Spec.ServerRef.Name == server { + if claim.Spec.ServerRef != nil && claim.Spec.ServerRef.Name == server.Name { return false, nil } diff --git a/internal/controller/serverclaim_controller_test.go b/internal/controller/serverclaim_controller_test.go index dd96946..64577ad 100644 --- a/internal/controller/serverclaim_controller_test.go +++ b/internal/controller/serverclaim_controller_test.go @@ -46,6 +46,15 @@ var _ = Describe("ServerClaim Controller", func() { Eventually(Get(bmc)).Should(Succeed()) DeferCleanup(k8sClient.Delete, bmc) + By("Ensuring that the BMCSecret will be removed") + bmcSecret := &metalv1alpha1.BMCSecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bmc.Name, + }, + } + Eventually(Get(bmcSecret)).Should(Succeed()) + DeferCleanup(k8sClient.Delete, bmcSecret) + By("Ensuring that the Server resource has been created") server = &metalv1alpha1.Server{ ObjectMeta: metav1.ObjectMeta{ @@ -247,6 +256,13 @@ var _ = Describe("ServerClaim Controller", func() { server.Status.State = metalv1alpha1.ServerStateAvailable })).Should(Succeed()) + By("Ensuring that the ServerClaim is bound") + Eventually(Object(claim)).Should(SatisfyAll( + HaveField("Finalizers", ContainElement(ServerClaimFinalizer)), + HaveField("Spec.ServerRef", Equal(&v1.LocalObjectReference{Name: server.Name})), + HaveField("Status.Phase", metalv1alpha1.PhaseBound), + )) + By("Ensuring that the Server has the correct claim ref") Eventually(Object(server)).Should(SatisfyAll( HaveField("Spec.ServerClaimRef", &v1.ObjectReference{ @@ -258,14 +274,7 @@ var _ = Describe("ServerClaim Controller", func() { }), HaveField("Spec.Power", metalv1alpha1.PowerOff), HaveField("Status.State", metalv1alpha1.ServerStateReserved), - )) - - By("Ensuring that the ServerClaim is bound") - Eventually(Object(claim)).Should(SatisfyAll( - HaveField("Finalizers", ContainElement(ServerClaimFinalizer)), - HaveField("Status.Phase", metalv1alpha1.PhaseBound), - HaveField("Spec.ServerRef", Not(BeNil())), - HaveField("Spec.ServerRef.Name", server.Name), + HaveField("Status.PowerState", metalv1alpha1.ServerOffPowerState), )) By("Deleting the ServerClaim") diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index da35aca..c736801 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -33,7 +33,7 @@ import ( const ( pollingInterval = 50 * time.Millisecond - eventuallyTimeout = 3 * time.Second + eventuallyTimeout = 5 * time.Second consistentlyDuration = 1 * time.Second )