diff --git a/docs/IMPLEMENTATION_PLAN.md b/docs/IMPLEMENTATION_PLAN.md index 79c78ef..6ffe67e 100644 --- a/docs/IMPLEMENTATION_PLAN.md +++ b/docs/IMPLEMENTATION_PLAN.md @@ -136,7 +136,7 @@ the full controller+daemon loop can be tested end-to-end. nodes are labeled. This exercises the e2e harness for the first time with a running operator and validates it functions in a real cluster. -### 3b. Digest-only rollout state machine +### 3b. Digest-only rollout state machine ✅ - Set `targetDigest` directly from `spec.image.ref` (digest refs only; tag resolution deferred to Milestone 5) diff --git a/internal/controller/bootcnodepool_controller.go b/internal/controller/bootcnodepool_controller.go index 9f37a7a..a859bd3 100644 --- a/internal/controller/bootcnodepool_controller.go +++ b/internal/controller/bootcnodepool_controller.go @@ -40,6 +40,7 @@ import ( // drainStatus tracks an in-progress drain goroutine for a single node. type drainStatus struct { result chan error // receives nil on success, error on failure; closed after send + ctx context.Context // the drain goroutine's context; checked to distinguish cancellation from real errors cancel context.CancelFunc // to abort on targetDigest change or node removal startTime time.Time // for stall detection isStalled bool //nolint:unused // used by drain stall detection @@ -596,8 +597,18 @@ func (r *BootcNodePoolReconciler) ensureManagedLabel(ctx context.Context, node * // removeBootcNode deletes a BootcNode for a node leaving the pool, // removes the managed label, and restores prior cordon state. func (r *BootcNodePoolReconciler) removeBootcNode(ctx context.Context, bn *bootcv1alpha1.BootcNode) error { - // TODO: if the node has an active drain goroutine, cancel it and - // remove it from the drains map. + // Cancel any active drain goroutine for this node. The goroutine will + // exit on its own and send a result on the channel, but since we've + // removed the entry from the map, collectDrainResults will never see + // it. The spurious re-enqueue from drainCh is harmless. + r.drainsMu.Lock() + if ds, exists := r.drains[bn.Name]; exists { + log := logf.FromContext(ctx) + log.Info("Cancelling drain for departing node", "node", bn.Name) + ds.cancel() + delete(r.drains, bn.Name) + } + r.drainsMu.Unlock() // Try to clean up the node (label + cordon state) before deleting // the BootcNode. The node may have been deleted from the cluster. diff --git a/internal/controller/membership_test.go b/internal/controller/membership_test.go index 46f9142..d97b7f4 100644 --- a/internal/controller/membership_test.go +++ b/internal/controller/membership_test.go @@ -10,7 +10,6 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -102,31 +101,34 @@ func TestMembershipCreatesBootcNodes(t *testing.T) { // Wait for BootcNodes to appear and verify their properties. for _, nodeName := range []string{"mem-worker-1", "mem-worker-2"} { name := nodeName - var bn bootcv1alpha1.BootcNode - g.Eventually(func() error { - return k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bn) - }).Should(Succeed()) // Check ownerReference. - owner := metav1.GetControllerOf(&bn) - g.Expect(owner).NotTo(BeNil(), "BootcNode %s has no controller owner", name) - g.Expect(owner.Name).To(Equal(pool.Name), "BootcNode %s owner mismatch", name) - - // Check desiredImage. - g.Expect(bn.Spec.DesiredImage).To(Equal(testImageDigestRefA), "BootcNode %s desiredImage mismatch", name) - - // Check desiredImageState. - g.Expect(bn.Spec.DesiredImageState).To(Equal(bootcv1alpha1.DesiredImageStateStaged), "BootcNode %s desiredImageState mismatch", name) + g.Eventually(func() (*metav1.OwnerReference, error) { + var bn bootcv1alpha1.BootcNode + err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bn) + return metav1.GetControllerOf(&bn), err + }).Should(And(Not(BeNil()), HaveField("Name", pool.Name)), + "BootcNode %s owner", name) + + // Check desiredImage and desiredImageState. + g.Eventually(func() (bootcv1alpha1.BootcNodeSpec, error) { + var bn bootcv1alpha1.BootcNode + err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bn) + return bn.Spec, err + }).Should(And( + HaveField("DesiredImage", testImageDigestRefA), + HaveField("DesiredImageState", bootcv1alpha1.DesiredImageStateStaged), + ), "BootcNode %s spec", name) } // Verify nodes are labeled bootc.dev/managed. for _, nodeName := range []string{"mem-worker-1", "mem-worker-2"} { name := nodeName - g.Eventually(func(g Gomega) { + g.Eventually(func() (map[string]string, error) { var n corev1.Node - g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: name}, &n)).To(Succeed()) - g.Expect(n.Labels).To(HaveKey(bootcv1alpha1.LabelManaged)) - }).Should(Succeed()) + err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &n) + return n.Labels, err + }).Should(HaveKey(bootcv1alpha1.LabelManaged)) } // Remove the worker label from mem-worker-1 and verify cleanup. @@ -142,11 +144,11 @@ func TestMembershipCreatesBootcNodes(t *testing.T) { }).Should(MatchError(apierrors.IsNotFound, "IsNotFound")) // Verify managed label is removed. - g.Eventually(func(g Gomega) { + g.Eventually(func() (map[string]string, error) { var n corev1.Node - g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: "mem-worker-1"}, &n)).To(Succeed()) - g.Expect(n.Labels).NotTo(HaveKey(bootcv1alpha1.LabelManaged)) - }).Should(Succeed()) + err := k8sClient.Get(ctx, client.ObjectKey{Name: "mem-worker-1"}, &n) + return n.Labels, err + }).ShouldNot(HaveKey(bootcv1alpha1.LabelManaged)) // Delete mem-worker-2 and verify its BootcNode is also deleted. g.Expect(k8sClient.Delete(ctx, node2)).To(Succeed()) @@ -178,11 +180,11 @@ func TestMembershipSyncsDesiredImage(t *testing.T) { }) // Wait for BootcNode to be created and verify image A. - var bn bootcv1alpha1.BootcNode - g.Eventually(func() error { - return k8sClient.Get(ctx, client.ObjectKeyFromObject(node), &bn) - }).Should(Succeed()) - g.Expect(bn.Spec.DesiredImage).To(Equal(testImageDigestRefA)) + g.Eventually(func() (string, error) { + var bn bootcv1alpha1.BootcNode + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(node), &bn) + return bn.Spec.DesiredImage, err + }).Should(Equal(testImageDigestRefA)) // Update pool image to B. var freshPool bootcv1alpha1.BootcNodePool @@ -191,12 +193,14 @@ func TestMembershipSyncsDesiredImage(t *testing.T) { g.Expect(k8sClient.Update(ctx, &freshPool)).To(Succeed()) // Wait for BootcNode to be updated with image B. - g.Eventually(func(g Gomega) { + g.Eventually(func() (bootcv1alpha1.BootcNodeSpec, error) { var bn bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(node), &bn)).To(Succeed()) - g.Expect(bn.Spec.DesiredImage).To(Equal(testImageDigestRefB)) - g.Expect(bn.Spec.DesiredImageState).To(Equal(bootcv1alpha1.DesiredImageStateStaged)) - }).Should(Succeed()) + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(node), &bn) + return bn.Spec, err + }).Should(And( + HaveField("DesiredImage", testImageDigestRefB), + HaveField("DesiredImageState", bootcv1alpha1.DesiredImageStateStaged), + )) } // TestMembershipConflictDetection verifies that when a node matches two @@ -260,11 +264,10 @@ func TestMembershipConflictDetection(t *testing.T) { // Verify pool1 is not degraded. var p1 bootcv1alpha1.BootcNodePool g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pool1), &p1)).To(Succeed()) - cond := apimeta.FindStatusCondition(p1.Status.Conditions, bootcv1alpha1.PoolDegraded) - if cond != nil { - g.Expect(cond.Status).To(Equal(metav1.ConditionFalse), - "pool1 should not be degraded, but got: %s/%s: %s", cond.Reason, cond.Status, cond.Message) - } + g.Expect(p1.Status.Conditions).NotTo(ContainElement(And( + HaveField("Type", bootcv1alpha1.PoolDegraded), + HaveField("Status", metav1.ConditionTrue), + )), "pool1 should not be degraded") // Verify non-contested nodes are still handled: node1 by pool1, // node2 by pool2. @@ -276,13 +279,11 @@ func TestMembershipConflictDetection(t *testing.T) { {node2.Name, pool2.Name}, } { tc := tc - var bn bootcv1alpha1.BootcNode - g.Eventually(func() error { - return k8sClient.Get(ctx, client.ObjectKey{Name: tc.nodeName}, &bn) - }).Should(Succeed()) - owner := metav1.GetControllerOf(&bn) - g.Expect(owner).NotTo(BeNil()) - g.Expect(owner.Name).To(Equal(tc.poolName)) + g.Eventually(func() (*metav1.OwnerReference, error) { + var bn bootcv1alpha1.BootcNode + err := k8sClient.Get(ctx, client.ObjectKey{Name: tc.nodeName}, &bn) + return metav1.GetControllerOf(&bn), err + }).Should(And(Not(BeNil()), HaveField("Name", tc.poolName))) } // Now resolve the conflict: remove pool1=true from node3 so pool1 diff --git a/internal/controller/rollout.go b/internal/controller/rollout.go index 27eea00..0ccbb58 100644 --- a/internal/controller/rollout.go +++ b/internal/controller/rollout.go @@ -268,6 +268,7 @@ func (r *BootcNodePoolReconciler) ensureDrain(ctx context.Context, pool *bootcv1 drainCtx, cancel := context.WithCancel(ctx) status := &drainStatus{ result: make(chan error, 1), + ctx: drainCtx, cancel: cancel, startTime: time.Now(), } @@ -328,8 +329,18 @@ func (r *BootcNodePoolReconciler) collectDrainResults(ctx context.Context, owned delete(r.drains, nodeName) if drainErr != nil { - // TODO: handle drain errors and cancellations. - log.Info("Drain failed", "node", nodeName, "error", drainErr) + if ds.ctx.Err() != nil { + // Context was cancelled (node left pool or controller + // shutdown). The canceller is responsible for cleanup. + log.V(1).Info("Drain cancelled", "node", nodeName) + } else { + // Real drain error (e.g. PDB timeout). The node keeps + // its reboot slot and stays cordoned. It will be + // retried naturally: the entry is deleted from the map, + // selectDrainCandidates picks the still-slotted Staged + // node, and ensureDrain starts a new goroutine. + log.Info("Drain failed, will retry", "node", nodeName, "error", drainErr) + } continue } diff --git a/internal/controller/rollout_envtest_test.go b/internal/controller/rollout_envtest_test.go index a9c3b2b..0a056b7 100644 --- a/internal/controller/rollout_envtest_test.go +++ b/internal/controller/rollout_envtest_test.go @@ -8,6 +8,8 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" @@ -79,21 +81,23 @@ func TestSimpleRollout(t *testing.T) { // Wait for this node to receive its reboot slot: cordoned, // annotated, desiredImageState set to Booted (drain completes // instantly in envtest since there are no pods). - g.Eventually(func(g Gomega) { + g.Eventually(func() (*bootcv1alpha1.BootcNode, error) { var bn bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bn)).To(Succeed()) - g.Expect(bn.Annotations).To(HaveKey(bootcv1alpha1.AnnotationInRebootSlot), - "node %s should have in-reboot-slot annotation", name) - g.Expect(bn.Annotations).To(HaveKey(bootcv1alpha1.AnnotationWasCordoned), - "node %s should have was-cordoned annotation", name) - g.Expect(bn.Spec.DesiredImageState).To(Equal(bootcv1alpha1.DesiredImageStateBooted), - "node %s should have desiredImageState Booted", name) - + err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bn) + return &bn, err + }).Should(And( + HaveField("Annotations", And( + HaveKey(bootcv1alpha1.AnnotationInRebootSlot), + HaveKey(bootcv1alpha1.AnnotationWasCordoned), + )), + HaveField("Spec.DesiredImageState", Equal(bootcv1alpha1.DesiredImageStateBooted)), + ), "node %s reboot slot", name) + + g.Eventually(func() (bool, error) { var node corev1.Node - g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: name}, &node)).To(Succeed()) - g.Expect(node.Spec.Unschedulable).To(BeTrue(), - "node %s should be cordoned", name) - }).Should(Succeed()) + err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &node) + return node.Spec.Unschedulable, err + }).Should(BeTrue(), "node %s should be cordoned", name) // Verify remaining nodes are not yet touched. for _, other := range nodeNames[i+1:] { @@ -115,19 +119,20 @@ func TestSimpleRollout(t *testing.T) { // Verify the reboot slot is freed: annotations removed and // node uncordoned. - g.Eventually(func(g Gomega) { + g.Eventually(func() (map[string]string, error) { var bn bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bn)).To(Succeed()) - g.Expect(bn.Annotations).NotTo(HaveKey(bootcv1alpha1.AnnotationInRebootSlot), - "node %s should have in-reboot-slot annotation removed", name) - g.Expect(bn.Annotations).NotTo(HaveKey(bootcv1alpha1.AnnotationWasCordoned), - "node %s should have was-cordoned annotation removed", name) - + err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bn) + return bn.Annotations, err + }).Should(And( + Not(HaveKey(bootcv1alpha1.AnnotationInRebootSlot)), + Not(HaveKey(bootcv1alpha1.AnnotationWasCordoned)), + ), "node %s reboot slot should be freed", name) + + g.Eventually(func() (bool, error) { var node corev1.Node - g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: name}, &node)).To(Succeed()) - g.Expect(node.Spec.Unschedulable).To(BeFalse(), - "node %s should be uncordoned after reboot", name) - }).Should(Succeed()) + err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &node) + return node.Spec.Unschedulable, err + }).Should(BeFalse(), "node %s should be uncordoned after reboot", name) } } @@ -246,14 +251,14 @@ func TestUnhealthyNodesHaltRollout(t *testing.T) { // reboot slots. Wait for w1, w2, w3 to get slots. for _, name := range nodeNames[:3] { name := name - g.Eventually(func(g Gomega) { + g.Eventually(func() (*bootcv1alpha1.BootcNode, error) { var bn bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bn)).To(Succeed()) - g.Expect(bn.Annotations).To(HaveKey(bootcv1alpha1.AnnotationInRebootSlot), - "node %s should have reboot slot", name) - g.Expect(bn.Spec.DesiredImageState).To(Equal(bootcv1alpha1.DesiredImageStateBooted), - "node %s should have desiredImageState Booted", name) - }).Should(Succeed()) + err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bn) + return &bn, err + }).Should(And( + HaveField("Annotations", HaveKey(bootcv1alpha1.AnnotationInRebootSlot)), + HaveField("Spec.DesiredImageState", Equal(bootcv1alpha1.DesiredImageStateBooted)), + ), "node %s reboot slot", name) } // w4 should not have a slot (all 3 slots are occupied). @@ -407,3 +412,144 @@ func setNodeReady(g Gomega, ctx context.Context, nodeName string) { g.Expect(k8sClient.Status().Patch(ctx, modified, client.MergeFrom(&node))).To(Succeed()) } + +// TestNodeLeavesPoolCancelsDrain verifies that when a node leaves the pool +// while a drain is in progress, the drain is cancelled and the node is +// cleaned up: BootcNode deleted, managed label removed, and node uncordoned. +// The drain is held by a PDB-protected pod so we can observe the cancellation. +func TestNodeLeavesPoolCancelsDrain(t *testing.T) { + g := NewWithT(t) + g.SetDefaultEventuallyTimeout(pollTimeout) + g.SetDefaultEventuallyPollingInterval(pollInterval) + ctx := context.Background() + + const ( + poolName = "drain-cancel-pool" + nodeName = "drain-cancel-w1" + namespace = "default" + ) + + // Create a worker node. + node := testutil.NewK8sNode(nodeName, testutil.WorkerLabels()) + g.Expect(k8sClient.Create(ctx, node)).To(Succeed()) + t.Cleanup(func() { + _ = k8sClient.Delete(ctx, node) + }) + + // Create a pod on the node that will resist eviction via a PDB. + podLabels := map[string]string{"app": "drain-block-" + nodeName} + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "drain-blocker-" + nodeName, + Namespace: namespace, + Labels: podLabels, + }, + Spec: corev1.PodSpec{ + NodeName: nodeName, + Containers: []corev1.Container{{ + Name: "pause", + Image: "registry.k8s.io/pause:3.9", + }}, + }, + } + g.Expect(k8sClient.Create(ctx, pod)).To(Succeed()) + t.Cleanup(func() { + _ = k8sClient.Delete(ctx, pod) + }) + + // Set the pod to Running/Ready so the drain helper considers it. + pod.Status.Phase = corev1.PodRunning + pod.Status.Conditions = []corev1.PodCondition{{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }} + g.Expect(k8sClient.Status().Update(ctx, pod)).To(Succeed()) + + // Create a PDB that prevents eviction of the pod. + minAvail := intstr.FromInt32(1) + pdb := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "drain-block-" + nodeName, + Namespace: namespace, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &minAvail, + Selector: &metav1.LabelSelector{ + MatchLabels: podLabels, + }, + }, + } + g.Expect(k8sClient.Create(ctx, pdb)).To(Succeed()) + t.Cleanup(func() { + _ = k8sClient.Delete(ctx, pdb) + }) + + // Create pool targeting digest B. + pool := testutil.NewPool(poolName, testImageDigestRefB, + testutil.WithWorkerSelector(), + testutil.WithMaxUnavailable(intstr.FromInt32(1)), + ) + g.Expect(k8sClient.Create(ctx, pool)).To(Succeed()) + t.Cleanup(func() { + _ = k8sClient.Delete(ctx, pool) + }) + + // Wait for BootcNode to be created. + g.Eventually(func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: nodeName}, &bootcv1alpha1.BootcNode{}) + }).Should(Succeed()) + + // Simulate daemon: node has staged the new image. + simulateDaemonStatus(g, ctx, nodeName, testDigestA, bootcv1alpha1.NodeReasonStaged) + + // Wait for the node to get its reboot slot and be cordoned. The drain + // will be stuck because of the PDB. + g.Eventually(func() (map[string]string, error) { + var bn bootcv1alpha1.BootcNode + err := k8sClient.Get(ctx, client.ObjectKey{Name: nodeName}, &bn) + return bn.Annotations, err + }).Should(HaveKey(bootcv1alpha1.AnnotationInRebootSlot)) + + g.Eventually(func() (bool, error) { + var n corev1.Node + err := k8sClient.Get(ctx, client.ObjectKey{Name: nodeName}, &n) + return n.Spec.Unschedulable, err + }).Should(BeTrue()) + + // Verify desiredImageState is still Staged (drain hasn't completed). + var bn bootcv1alpha1.BootcNode + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)).To(Succeed()) + g.Expect(bn.Spec.DesiredImageState).To(Equal(bootcv1alpha1.DesiredImageStateStaged), + "desiredImageState should still be Staged while drain is blocked") + + // Remove the worker label so the node no longer matches the pool. + var freshNode corev1.Node + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: nodeName}, &freshNode)).To(Succeed()) + modified := freshNode.DeepCopy() + delete(modified.Labels, "node-role.kubernetes.io/worker") + g.Expect(k8sClient.Patch(ctx, modified, client.MergeFrom(&freshNode))).To(Succeed()) + + // Verify cleanup: BootcNode should be deleted. + g.Eventually(func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: nodeName}, &bootcv1alpha1.BootcNode{}) + }).Should(MatchError(apierrors.IsNotFound, "IsNotFound")) + + // Node should be uncordoned and managed label removed. + g.Eventually(func() (bool, error) { + var n corev1.Node + err := k8sClient.Get(ctx, client.ObjectKey{Name: nodeName}, &n) + return n.Spec.Unschedulable, err + }).Should(BeFalse()) + + g.Eventually(func() (map[string]string, error) { + var n corev1.Node + err := k8sClient.Get(ctx, client.ObjectKey{Name: nodeName}, &n) + return n.Labels, err + }).ShouldNot(HaveKey(bootcv1alpha1.LabelManaged)) + + // Verify the drain entry was removed from the reconciler's map. + testReconciler.drainsMu.Lock() + _, drainExists := testReconciler.drains[nodeName] + testReconciler.drainsMu.Unlock() + g.Expect(drainExists).To(BeFalse(), "drain entry should be removed from map") +} diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 11017ca..688a931 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -21,8 +21,9 @@ import ( ) var ( - testEnv *envtest.Environment - k8sClient client.Client + testEnv *envtest.Environment + k8sClient client.Client + testReconciler *BootcNodePoolReconciler ) // TODO: TestMain starts envtest and the manager unconditionally, which means @@ -73,11 +74,12 @@ func TestMain(m *testing.M) { os.Exit(1) } - if err := (&BootcNodePoolReconciler{ + testReconciler = &BootcNodePoolReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), KubeClient: kubeClient, - }).SetupWithManager(mgr); err != nil { + } + if err := testReconciler.SetupWithManager(mgr); err != nil { fmt.Fprintf(os.Stderr, "Failed to setup reconciler: %v\n", err) os.Exit(1) } diff --git a/internal/daemon/reconciler_test.go b/internal/daemon/reconciler_test.go index 6fabcfb..1827efb 100644 --- a/internal/daemon/reconciler_test.go +++ b/internal/daemon/reconciler_test.go @@ -63,36 +63,42 @@ func TestReconcilePopulatesStatus(t *testing.T) { _ = k8sClient.Delete(ctx, bn) }) - g.Eventually(func(g Gomega) { + g.Eventually(func() (bootcv1alpha1.BootcNodeStatus, error) { var got bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got)).To(Succeed()) - - g.Expect(got.Status.Booted).NotTo(BeNil()) - g.Expect(got.Status.Booted.Image).To(Equal(testutil.ImageTaggedRef)) - g.Expect(got.Status.Booted.ImageDigest).To(Equal(testutil.DigestA)) - g.Expect(got.Status.Booted.Version).To(Equal(v1)) - g.Expect(got.Status.Booted.Architecture).To(Equal("amd64")) - - g.Expect(got.Status.Staged).NotTo(BeNil()) - g.Expect(got.Status.Staged.ImageDigest).To(Equal(testutil.DigestB)) - g.Expect(got.Status.Staged.Version).To(Equal(v2)) - g.Expect(got.Status.Staged.SoftRebootCapable).To(BeTrue()) - - g.Expect(got.Status.Rollback).NotTo(BeNil()) - g.Expect(got.Status.Rollback.ImageDigest).To(Equal(testutil.DigestC)) - g.Expect(got.Status.Rollback.Version).To(Equal(v3)) - - g.Expect(got.Status.Conditions).To(ContainElement(And( - HaveField("Type", bootcv1alpha1.NodeIdle), - HaveField("Status", metav1.ConditionTrue), - HaveField("Reason", bootcv1alpha1.NodeReasonIdle), - ))) - g.Expect(got.Status.Conditions).To(ContainElement(And( - HaveField("Type", bootcv1alpha1.NodeDegraded), - HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", bootcv1alpha1.NodeReasonHealthy), - ))) - }).Should(Succeed()) + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got) + return got.Status, err + }).Should(And( + HaveField("Booted", And( + Not(BeNil()), + HaveField("Image", testutil.ImageTaggedRef), + HaveField("ImageDigest", testutil.DigestA), + HaveField("Version", v1), + HaveField("Architecture", "amd64"), + )), + HaveField("Staged", And( + Not(BeNil()), + HaveField("ImageDigest", testutil.DigestB), + HaveField("Version", v2), + HaveField("SoftRebootCapable", BeTrue()), + )), + HaveField("Rollback", And( + Not(BeNil()), + HaveField("ImageDigest", testutil.DigestC), + HaveField("Version", v3), + )), + HaveField("Conditions", And( + ContainElement(And( + HaveField("Type", bootcv1alpha1.NodeIdle), + HaveField("Status", metav1.ConditionTrue), + HaveField("Reason", bootcv1alpha1.NodeReasonIdle), + )), + ContainElement(And( + HaveField("Type", bootcv1alpha1.NodeDegraded), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", bootcv1alpha1.NodeReasonHealthy), + )), + )), + )) } func TestReconcileBootcStatusError(t *testing.T) { @@ -110,16 +116,16 @@ func TestReconcileBootcStatusError(t *testing.T) { _ = k8sClient.Delete(ctx, bn) }) - g.Eventually(func(g Gomega) { + g.Eventually(func() ([]metav1.Condition, error) { var got bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got)).To(Succeed()) - g.Expect(got.Status.Conditions).To(ContainElement(And( - HaveField("Type", bootcv1alpha1.NodeDegraded), - HaveField("Status", metav1.ConditionTrue), - HaveField("Reason", bootcv1alpha1.NodeReasonError), - HaveField("Message", Equal(fmt.Sprintf("populating bootc fields: getting bootc status: %s", bootcStatusErrMsg))), - ))) - }).Should(Succeed()) + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got) + return got.Status.Conditions, err + }).Should(ContainElement(And( + HaveField("Type", bootcv1alpha1.NodeDegraded), + HaveField("Status", metav1.ConditionTrue), + HaveField("Reason", bootcv1alpha1.NodeReasonError), + HaveField("Message", Equal(fmt.Sprintf("populating bootc fields: getting bootc status: %s", bootcStatusErrMsg))), + ))) } func TestStagingTriggered(t *testing.T) { @@ -137,23 +143,27 @@ func TestStagingTriggered(t *testing.T) { _ = k8sClient.Delete(ctx, bn) }) - g.Eventually(func(g Gomega) { + g.Eventually(func() (bootcv1alpha1.BootcNodeStatus, error) { var got bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got)).To(Succeed()) - - g.Expect(got.Status.Staged).NotTo(BeNil()) - g.Expect(got.Status.Staged.ImageDigest).To(Equal(testutil.DigestB)) - - g.Expect(got.Status.Conditions).To(ContainElement(And( - HaveField("Type", bootcv1alpha1.NodeIdle), - HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", bootcv1alpha1.NodeReasonStaged), - ))) - g.Expect(got.Status.Conditions).To(ContainElement(And( - HaveField("Type", bootcv1alpha1.NodeDegraded), - HaveField("Status", metav1.ConditionFalse), - ))) - }).Should(Succeed()) + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got) + return got.Status, err + }).Should(And( + HaveField("Staged", And( + Not(BeNil()), + HaveField("ImageDigest", testutil.DigestB), + )), + HaveField("Conditions", And( + ContainElement(And( + HaveField("Type", bootcv1alpha1.NodeIdle), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", bootcv1alpha1.NodeReasonStaged), + )), + ContainElement(And( + HaveField("Type", bootcv1alpha1.NodeDegraded), + HaveField("Status", metav1.ConditionFalse), + )), + )), + )) g.Expect(fake.getStageImg()).To(Equal(testutil.ImageDigestRefB)) g.Expect(fake.getRebooted()).To(BeFalse()) @@ -175,21 +185,23 @@ func TestStagingError(t *testing.T) { _ = k8sClient.Delete(ctx, bn) }) - g.Eventually(func(g Gomega) { + g.Eventually(func() ([]metav1.Condition, error) { var got bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got)).To(Succeed()) - g.Expect(got.Status.Conditions).To(ContainElement(And( + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got) + return got.Status.Conditions, err + }).Should(And( + ContainElement(And( HaveField("Type", bootcv1alpha1.NodeIdle), HaveField("Status", metav1.ConditionTrue), HaveField("Reason", bootcv1alpha1.NodeReasonIdle), - ))) - g.Expect(got.Status.Conditions).To(ContainElement(And( + )), + ContainElement(And( HaveField("Type", bootcv1alpha1.NodeDegraded), HaveField("Status", metav1.ConditionTrue), HaveField("Reason", bootcv1alpha1.NodeReasonError), HaveField("Message", Equal(fmt.Sprintf("bootc stage failed: %s", stageErrMsg))), - ))) - }).Should(Succeed()) + )), + )) } func TestAlreadyStaged(t *testing.T) { @@ -208,15 +220,15 @@ func TestAlreadyStaged(t *testing.T) { _ = k8sClient.Delete(ctx, bn) }) - g.Eventually(func(g Gomega) { + g.Eventually(func() ([]metav1.Condition, error) { var got bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got)).To(Succeed()) - g.Expect(got.Status.Conditions).To(ContainElement(And( - HaveField("Type", bootcv1alpha1.NodeIdle), - HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", bootcv1alpha1.NodeReasonStaged), - ))) - }).Should(Succeed()) + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got) + return got.Status.Conditions, err + }).Should(ContainElement(And( + HaveField("Type", bootcv1alpha1.NodeIdle), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", bootcv1alpha1.NodeReasonStaged), + ))) g.Expect(fake.getStageImg()).To(BeEmpty()) } @@ -237,15 +249,15 @@ func TestRebootingSet(t *testing.T) { _ = k8sClient.Delete(ctx, bn) }) - g.Eventually(func(g Gomega) { + g.Eventually(func() ([]metav1.Condition, error) { var got bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got)).To(Succeed()) - g.Expect(got.Status.Conditions).To(ContainElement(And( - HaveField("Type", bootcv1alpha1.NodeIdle), - HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", bootcv1alpha1.NodeReasonRebooting), - ))) - }).Should(Succeed()) + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got) + return got.Status.Conditions, err + }).Should(ContainElement(And( + HaveField("Type", bootcv1alpha1.NodeIdle), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", bootcv1alpha1.NodeReasonRebooting), + ))) g.Expect(fake.getRebooted()).To(BeTrue()) } @@ -266,17 +278,21 @@ func TestRollback(t *testing.T) { _ = k8sClient.Delete(ctx, bn) }) - g.Eventually(func(g Gomega) { + g.Eventually(func() (bootcv1alpha1.BootcNodeStatus, error) { var got bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got)).To(Succeed()) - g.Expect(got.Status.Conditions).To(ContainElement(And( + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &got) + return got.Status, err + }).Should(And( + HaveField("Conditions", ContainElement(And( HaveField("Type", bootcv1alpha1.NodeIdle), HaveField("Status", metav1.ConditionFalse), HaveField("Reason", bootcv1alpha1.NodeReasonStaged), - ))) - g.Expect(got.Status.Staged).NotTo(BeNil()) - g.Expect(got.Status.Staged.ImageDigest).To(Equal(testutil.DigestC)) - }).Should(Succeed()) + ))), + HaveField("Staged", And( + Not(BeNil()), + HaveField("ImageDigest", testutil.DigestC), + )), + )) g.Expect(fake.getRebooted()).To(BeFalse()) } @@ -308,11 +324,13 @@ func TestCancelInflightStage(t *testing.T) { fake.setStageHook(nil) close(firstBlock) - g.Eventually(func(g Gomega) { + g.Eventually(func() error { var latest bootcv1alpha1.BootcNode - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &latest)).To(Succeed()) + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(bn), &latest); err != nil { + return err + } latest.Spec.DesiredImage = testutil.ImageDigestRefC - g.Expect(k8sClient.Update(ctx, &latest)).To(Succeed()) + return k8sClient.Update(ctx, &latest) }).Should(Succeed()) g.Eventually(func() string { diff --git a/test/e2e/bootcnode_test.go b/test/e2e/bootcnode_test.go index 7577755..039e788 100644 --- a/test/e2e/bootcnode_test.go +++ b/test/e2e/bootcnode_test.go @@ -40,19 +40,19 @@ func TestControllerMembership(t *testing.T) { pool := env.NewPool("workers", env.NodeImageDigestedPullSpec()) g.Expect(env.Client.Create(ctx, pool)).To(Succeed()) - // Wait for BootcNode to appear for the worker. - var bn bootcv1alpha1.BootcNode - g.Eventually(func() error { - return env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn) - }).Should(Succeed()) - - // Verify ownerReference. - owner := metav1.GetControllerOf(&bn) - g.Expect(owner).NotTo(BeNil()) - g.Expect(owner.Name).To(Equal(pool.Name)) + // Wait for BootcNode to appear and verify ownerReference. + g.Eventually(func() (*metav1.OwnerReference, error) { + var bn bootcv1alpha1.BootcNode + err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn) + return metav1.GetControllerOf(&bn), err + }).Should(And(Not(BeNil()), HaveField("Name", pool.Name))) // Verify desiredImage. - g.Expect(bn.Spec.DesiredImage).To(Equal(env.NodeImageDigestedPullSpec())) + g.Eventually(func() (string, error) { + var bn bootcv1alpha1.BootcNode + err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn) + return bn.Spec.DesiredImage, err + }).Should(Equal(env.NodeImageDigestedPullSpec())) // Verify the worker has the managed label. var node corev1.Node @@ -76,19 +76,22 @@ func TestControllerMembership(t *testing.T) { HaveField("Status.Phase", corev1.PodRunning), )), "expected exactly one running daemon pod on %s", nodeName) - g.Eventually(func(g Gomega) { - g.Expect(env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)).To(Succeed()) - g.Expect(bn.Status.Booted).NotTo(BeNil(), "expected booted status to be populated") - g.Expect(bn.Status.Booted.Image).To(Equal(env.NodeImageDigestedPullSpec()), - "booted image should match seeded registry image") - g.Expect(bn.Status.Booted.ImageDigest).To(Equal(env.NodeImageDigest()), - "booted image digest should match seeded registry image") - g.Expect(bn.Status.Conditions).To(ContainElement(And( + g.Eventually(func() (bootcv1alpha1.BootcNodeStatus, error) { + var bn bootcv1alpha1.BootcNode + err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn) + return bn.Status, err + }).WithTimeout(3 * time.Minute).Should(And( + HaveField("Booted", And( + Not(BeNil()), + HaveField("Image", env.NodeImageDigestedPullSpec()), + HaveField("ImageDigest", env.NodeImageDigest()), + )), + HaveField("Conditions", ContainElement(And( HaveField("Type", bootcv1alpha1.NodeIdle), HaveField("Status", metav1.ConditionTrue), HaveField("Reason", bootcv1alpha1.NodeReasonIdle), - ))) - }).WithTimeout(3 * time.Minute).Should(Succeed()) + ))), + )) } // TestUpdateReboot provisions a worker node, creates a pool with the @@ -108,16 +111,18 @@ func TestUpdateReboot(t *testing.T) { pool := env.NewPool("workers", env.NodeImageDigestedPullSpec()) g.Expect(env.Client.Create(ctx, pool)).To(Succeed()) - var bn bootcv1alpha1.BootcNode - g.Eventually(func(g Gomega) { - g.Expect(env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)).To(Succeed()) - g.Expect(bn.Status.Booted).NotTo(BeNil()) - g.Expect(bn.Status.Conditions).To(ContainElement(And( + g.Eventually(func() (bootcv1alpha1.BootcNodeStatus, error) { + var bn bootcv1alpha1.BootcNode + err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn) + return bn.Status, err + }).WithTimeout(3 * time.Minute).Should(And( + HaveField("Booted", Not(BeNil())), + HaveField("Conditions", ContainElement(And( HaveField("Type", bootcv1alpha1.NodeIdle), HaveField("Status", metav1.ConditionTrue), HaveField("Reason", bootcv1alpha1.NodeReasonIdle), - ))) - }).WithTimeout(3 * time.Minute).Should(Succeed()) + ))), + )) t.Logf("Node %q is Idle with original image", nodeName) @@ -133,61 +138,85 @@ func TestUpdateReboot(t *testing.T) { // Phase 3: Wait for Rebooting — the daemon skips reconciliation after // issuing a reboot, so this state is durable until the node goes down. - g.Eventually(func(g Gomega) { - g.Expect(env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)).To(Succeed()) - g.Expect(bn.Status.Conditions).To(ContainElement(And( - HaveField("Type", bootcv1alpha1.NodeIdle), - HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", bootcv1alpha1.NodeReasonRebooting), - ))) - }).WithTimeout(5*time.Minute).Should(Succeed(), "expected node to reach Rebooting state") + g.Eventually(func() ([]metav1.Condition, error) { + var bn bootcv1alpha1.BootcNode + err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn) + return bn.Status.Conditions, err + }).WithTimeout(5*time.Minute).Should(ContainElement(And( + HaveField("Type", bootcv1alpha1.NodeIdle), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", bootcv1alpha1.NodeReasonRebooting), + )), "expected node to reach Rebooting state") t.Logf("Node %q is Rebooting", nodeName) // Phase 4: Wait for Idle with the update digest — proves the full // update lifecycle completed (staging, reboot, boot into new image). - g.Eventually(func(g Gomega) { - g.Expect(env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)).To(Succeed()) - g.Expect(bn.Status.Booted).NotTo(BeNil()) - g.Expect(bn.Status.Booted.ImageDigest).To(Equal(env.NodeImageUpdateDigest()), - "expected booted digest to match update image") - g.Expect(bn.Status.Conditions).To(ContainElement(And( + g.Eventually(func() (bootcv1alpha1.BootcNodeStatus, error) { + var bn bootcv1alpha1.BootcNode + err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn) + return bn.Status, err + }).WithTimeout(5*time.Minute).Should(And( + HaveField("Booted", And( + Not(BeNil()), + HaveField("ImageDigest", env.NodeImageUpdateDigest()), + )), + HaveField("Conditions", ContainElement(And( HaveField("Type", bootcv1alpha1.NodeIdle), HaveField("Status", metav1.ConditionTrue), HaveField("Reason", bootcv1alpha1.NodeReasonIdle), - ))) - }).WithTimeout(5*time.Minute).Should(Succeed(), "expected node to reach Idle with update image after reboot") + ))), + ), "expected node to reach Idle with update image after reboot") t.Logf("Node %q is Idle with update image", nodeName) // Phase 5: Verify node is schedulable (uncordoned after reboot). - var node corev1.Node - g.Eventually(func(g Gomega) bool { - g.Expect(env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &node)).To(Succeed()) - return node.Spec.Unschedulable + g.Eventually(func() (bool, error) { + var node corev1.Node + err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &node) + return node.Spec.Unschedulable, err }).WithTimeout(3*time.Minute).Should(BeFalse(), "expected node to be schedulable after update") // Phase 6: Verify update marker exists on the host via daemon pod exec. - var daemonPod corev1.Pod - g.Eventually(func(g Gomega) { + g.Eventually(func() ([]corev1.Pod, error) { var pods corev1.PodList - g.Expect(env.Client.List(ctx, &pods, + err := env.Client.List(ctx, &pods, client.InNamespace("bootc-operator"), client.MatchingLabels{ "app.kubernetes.io/name": "bootc-operator", "app.kubernetes.io/component": "daemon", }, - )).To(Succeed()) + ) + if err != nil { + return nil, err + } var matched []corev1.Pod for _, p := range pods.Items { if p.Spec.NodeName == nodeName { matched = append(matched, p) } } - g.Expect(matched).To(HaveLen(1), "expected exactly one daemon pod on %s", nodeName) - g.Expect(matched[0].Status.Phase).To(Equal(corev1.PodRunning)) - daemonPod = matched[0] - }).WithTimeout(1*time.Minute).Should(Succeed(), "expected running daemon pod on %s", nodeName) + return matched, nil + }).WithTimeout(1*time.Minute).Should(ConsistOf( + HaveField("Status.Phase", corev1.PodRunning), + ), "expected running daemon pod on %s", nodeName) + + // Retrieve the daemon pod for exec. + var daemonPods corev1.PodList + g.Expect(env.Client.List(ctx, &daemonPods, + client.InNamespace("bootc-operator"), + client.MatchingLabels{ + "app.kubernetes.io/name": "bootc-operator", + "app.kubernetes.io/component": "daemon", + }, + )).To(Succeed()) + var daemonPod corev1.Pod + for _, p := range daemonPods.Items { + if p.Spec.NodeName == nodeName { + daemonPod = p + break + } + } kubeconfigPath := os.Getenv("KUBECONFIG") cmd := exec.CommandContext(ctx, "kubectl", "--kubeconfig", kubeconfigPath, @@ -220,19 +249,19 @@ func TestTagResolution(t *testing.T) { g.Expect(env.Client.Create(ctx, pool)).To(Succeed()) // Verify targetDigest is resolved to the original image digest. - g.Eventually(func(g Gomega) string { + g.Eventually(func() (string, error) { var p bootcv1alpha1.BootcNodePool - g.Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(pool), &p)).To(Succeed()) - return p.Status.TargetDigest + err := env.Client.Get(ctx, client.ObjectKeyFromObject(pool), &p) + return p.Status.TargetDigest, err }).WithTimeout(1 * time.Minute).Should(Equal(env.NodeImageDigest())) t.Logf("Tag resolved to original digest %s", env.NodeImageDigest()) // Wait for node to reach Idle with the original image. - g.Eventually(func(g Gomega) bootcv1alpha1.BootcNodeStatus { + g.Eventually(func() (bootcv1alpha1.BootcNodeStatus, error) { var bn bootcv1alpha1.BootcNode - g.Expect(env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)).To(Succeed()) - return bn.Status + err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn) + return bn.Status, err }).WithTimeout(3 * time.Minute).Should(And( HaveField("Booted", And( Not(BeNil()), @@ -255,19 +284,19 @@ func TestTagResolution(t *testing.T) { t.Logf("Retagged node:latest to update digest %s", env.NodeImageUpdateDigest()) // Wait for the controller to re-resolve and pick up the new digest. - g.Eventually(func(g Gomega) string { + g.Eventually(func() (string, error) { var p bootcv1alpha1.BootcNodePool - g.Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(pool), &p)).To(Succeed()) - return p.Status.TargetDigest + err := env.Client.Get(ctx, client.ObjectKeyFromObject(pool), &p) + return p.Status.TargetDigest, err }).WithTimeout(1 * time.Minute).Should(Equal(env.NodeImageUpdateDigest())) t.Logf("Tag re-resolved to update digest %s", env.NodeImageUpdateDigest()) // Wait for node to reach Idle with the update image. - g.Eventually(func(g Gomega) bootcv1alpha1.BootcNodeStatus { + g.Eventually(func() (bootcv1alpha1.BootcNodeStatus, error) { var bn bootcv1alpha1.BootcNode - g.Expect(env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)).To(Succeed()) - return bn.Status + err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn) + return bn.Status, err }).WithTimeout(5 * time.Minute).Should(And( HaveField("Booted", And( Not(BeNil()),