e2e: add mid-rollout image change test

Makefile

@@ -85,6 +85,7 @@ e2e: ## Run e2e tests (requires: make deploy-bink). V=1 for verbose. RUN=<regex>
 		ARTIFACTS=$(ARTIFACTS) \
 		BINK_NODE_IMAGE_DIGEST=$$(skopeo inspect --tls-verify=false --format '{{.Digest}}' docker://localhost:5000/node:latest) \
 		BINK_NODE_IMAGE_UPDATE_DIGEST=$$(skopeo inspect --tls-verify=false docker://localhost:5000/node:update | jq -r '.Digest') \
+		BINK_NODE_IMAGE_UPDATE2_DIGEST=$$(skopeo inspect --tls-verify=false docker://localhost:5000/node:update2 | jq -r '.Digest') \
 		go test -timeout 30m -count=1 $(if $(V),-v) $(if $(RUN),-run $(RUN)) .
 
 ##@ Build
@@ -105,10 +106,13 @@ buildimg: ## Build container image.
 	$(CONTAINER_TOOL) build -t $(IMG) .
 
 .PHONY: build-update-image
-build-update-image: ## Build a derived node image for update testing and push to bink registry.
+build-update-image: ## Build derived node images for update testing and push to bink registry.
 	@printf 'FROM localhost:5000/node:latest\nRUN touch /usr/share/update-marker\n' | \
 		podman build -t localhost:5000/node:update -f - .
 	podman push --tls-verify=false localhost:5000/node:update
+	@printf 'FROM localhost:5000/node:latest\nRUN touch /usr/share/update-marker-2\n' | \
+		podman build -t localhost:5000/node:update2 -f - .
+	podman push --tls-verify=false localhost:5000/node:update2
 
 ##@ Deployment
 

internal/controller/bootcnodepool_controller.go

@@ -329,7 +329,10 @@ func (r *BootcNodePoolReconciler) Reconcile(
 // It removes the bootc.dev/managed label from all member nodes and deletes
 // the owned BootcNode objects, then removes the cleanup finalizer so
 // Kubernetes can complete the deletion.
-func (r *BootcNodePoolReconciler) handlePoolDeletion(ctx context.Context, pool *bootcv1alpha1.BootcNodePool) (ctrl.Result, error) {
+func (r *BootcNodePoolReconciler) handlePoolDeletion(
+	ctx context.Context,
+	pool *bootcv1alpha1.BootcNodePool,
+) (ctrl.Result, error) {
 	log := logf.FromContext(ctx).WithValues("pool", pool.Name)
 
 	if !controllerutil.ContainsFinalizer(pool, bootcv1alpha1.FinalizerPoolCleanup) {
@@ -351,7 +354,11 @@ func (r *BootcNodePoolReconciler) handlePoolDeletion(ctx context.Context, pool *
 		}
 		log.Info("Removing BootcNode for pool deletion", "node", bn.Name)
 		if err := r.removeBootcNode(ctx, bn); err != nil {
-			return ctrl.Result{}, fmt.Errorf("removing BootcNode %s during pool deletion: %w", bn.Name, err)
+			return ctrl.Result{}, fmt.Errorf(
+				"removing BootcNode %s during pool deletion: %w",
+				bn.Name,
+				err,
+			)
 		}
 	}
 

internal/controller/membership_test.go

@@ -242,7 +242,11 @@ func TestPoolDeletionRemovesManagedLabel(t *testing.T) {
 	}
 
 	// Create a worker pool and a separate control-plane pool.
-	workerPool := testutil.NewPool("del-workers", testImageDigestRefA, testutil.WithWorkerSelector())
+	workerPool := testutil.NewPool(
+		"del-workers",
+		testImageDigestRefA,
+		testutil.WithWorkerSelector(),
+	)
 	g.Expect(k8sClient.Create(ctx, workerPool)).To(Succeed())
 
 	cpPool := testutil.NewPool("del-control-plane", testImageDigestRefA,
@@ -286,7 +290,11 @@ func TestPoolDeletionRemovesManagedLabel(t *testing.T) {
 
 	// The worker pool itself should be fully deleted (finalizer removed).
 	g.Eventually(func() error {
-		return k8sClient.Get(ctx, client.ObjectKeyFromObject(workerPool), &bootcv1alpha1.BootcNodePool{})
+		return k8sClient.Get(
+			ctx,
+			client.ObjectKeyFromObject(workerPool),
+			&bootcv1alpha1.BootcNodePool{},
+		)
 	}).Should(MatchError(apierrors.IsNotFound, "IsNotFound"), "worker pool should be fully deleted")
 
 	// Control-plane nodes must still carry the managed label — their pool was not deleted.
@@ -299,8 +307,9 @@ func TestPoolDeletionRemovesManagedLabel(t *testing.T) {
 
 	// Control-plane BootcNodes must still exist.
 	for _, node := range controlPlaneNodes {
-		g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: node.Name}, &bootcv1alpha1.BootcNode{})).To(Succeed(),
-			"BootcNode %s should still exist", node.Name)
+		g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: node.Name}, &bootcv1alpha1.BootcNode{})).
+			To(Succeed(),
+				"BootcNode %s should still exist", node.Name)
 	}
 }
 

test/e2e/bootcnode_test.go

@@ -7,12 +7,15 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"strconv"
+	"strings"
 	"testing"
 	"time"
 
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/types"
 	corev1 "k8s.io/api/core/v1"
+	meta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -200,54 +203,8 @@ func TestUpdateReboot(t *testing.T) {
 	}).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.
-	g.Eventually(func() ([]corev1.Pod, error) {
-		var pods corev1.PodList
-		err := env.Client.List(ctx, &pods,
-			client.InNamespace("bootc-operator"),
-			client.MatchingLabels{
-				"app.kubernetes.io/name":      "bootc-operator",
-				"app.kubernetes.io/component": "daemon",
-			},
-		)
-		if err != nil {
-			return nil, err
-		}
-		var matched []corev1.Pod
-		for _, p := range pods.Items {
-			if p.Spec.NodeName == nodeName {
-				matched = append(matched, p)
-			}
-		}
-		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,
-		"-n", "bootc-operator", "exec", daemonPod.Name, "--",
+	execOnNode(t, g, env, ctx, nodeName,
 		"stat", "/proc/1/root/usr/share/update-marker")
-	out, err := cmd.CombinedOutput()
-	g.Expect(err).NotTo(HaveOccurred(),
-		fmt.Sprintf("expected update-marker to exist on host, kubectl exec output: %s", string(out)),
-	)
 
 	t.Logf("Verified update-marker exists on host via daemon pod")
 
@@ -374,6 +331,205 @@ func TestTagResolution(t *testing.T) {
 	t.Logf("Node %q is Idle with update image", nodeName)
 }
 
+// TestMidRolloutImageChange provisions two worker nodes, starts a rollout
+// to one update image, then switches the target to a different update image
+// while one node is rebooting. It verifies both nodes converge to the final
+// image and that the non-rebooting node does not wastefully reboot into the
+// first update image.
+func TestMidRolloutImageChange(t *testing.T) {
+	g := NewWithT(t)
+	g.SetDefaultEventuallyTimeout(pollTimeout)
+	g.SetDefaultEventuallyPollingInterval(pollInterval)
+
+	env := e2eutil.New(t)
+	nodeA := env.AddNode(t)
+	nodeB := env.AddNode(t)
+
+	ctx := context.Background()
+
+	// Phase 1: Create pool with original image and wait for both nodes Idle.
+	pool := env.NewPool("mid-rollout", env.NodeImageDigestedPullSpec())
+	g.Expect(env.Client.Create(ctx, pool)).To(Succeed())
+
+	for _, nodeName := range []string{nodeA, nodeB} {
+		g.Eventually(func() (bootcv1alpha1.BootcNode, error) {
+			var bn bootcv1alpha1.BootcNode
+			err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)
+			return bn, err
+		}).WithTimeout(3 * time.Minute).Should(SatisfyAll(
+			HaveField("Status.Booted", Not(BeNil())),
+			HaveField("Status.Conditions", ContainElement(And(
+				HaveField("Type", bootcv1alpha1.NodeIdle),
+				HaveField("Status", metav1.ConditionTrue),
+				HaveField("Reason", bootcv1alpha1.NodeReasonIdle),
+			))),
+		))
+	}
+
+	t.Logf("Both nodes are Idle with original image")
+
+	// Phase 2: Record boot count for both nodes before the rollout.
+	bootCountBefore := make(map[string]string)
+	for _, nodeName := range []string{nodeA, nodeB} {
+		bootCountBefore[nodeName] = getBootCount(t, env, ctx, nodeName)
+		t.Logf("Node %q boot count before: %s", nodeName, bootCountBefore[nodeName])
+	}
+
+	// Phase 3: Patch pool to first update image.
+	updateRef1 := env.NodeImageUpdateDigestedPullSpec()
+
+	modified := pool.DeepCopy()
+	modified.Spec.Image.Ref = updateRef1
+	g.Expect(env.Client.Patch(ctx, modified, client.MergeFrom(pool))).To(Succeed())
+	*pool = *modified
+
+	t.Logf("Patched pool to first update image %s", updateRef1)
+
+	// Phase 4: Wait for any node to reach Rebooting.
+	var rebootingNode string
+	g.Eventually(func(g Gomega) string {
+		for _, name := range []string{nodeA, nodeB} {
+			var bn bootcv1alpha1.BootcNode
+			g.Expect(env.Client.Get(ctx, client.ObjectKey{Name: name}, &bn)).To(Succeed())
+
+			cond := meta.FindStatusCondition(bn.Status.Conditions, bootcv1alpha1.NodeIdle)
+			if cond != nil &&
+				cond.Status == metav1.ConditionFalse &&
+				cond.Reason == bootcv1alpha1.NodeReasonRebooting {
+				rebootingNode = name
+				return name
+			}
+		}
+		return ""
+	}).WithTimeout(5 * time.Minute).ShouldNot(BeEmpty())
+
+	var otherNode string
+	if rebootingNode == nodeA {
+		otherNode = nodeB
+	} else {
+		otherNode = nodeA
+	}
+
+	t.Logf("Node %q is Rebooting, node %q is the other node", rebootingNode, otherNode)
+
+	// Phase 5: Immediately switch target to second update image.
+	updateRef2 := env.NodeImageUpdate2DigestedPullSpec()
+
+	modified = pool.DeepCopy()
+	modified.Spec.Image.Ref = updateRef2
+	g.Expect(env.Client.Patch(ctx, modified, client.MergeFrom(pool))).To(Succeed())
+	*pool = *modified
+
+	t.Logf("Switched pool to second update image %s", updateRef2)
+
+	// Phase 6: Wait for both nodes to be Idle with the second update image.
+	for _, nodeName := range []string{nodeA, nodeB} {
+		g.Eventually(func() (bootcv1alpha1.BootcNode, error) {
+			var bn bootcv1alpha1.BootcNode
+			err := env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)
+			return bn, err
+		}).WithTimeout(8*time.Minute).Should(SatisfyAll(
+			HaveField("Status.Booted", Not(BeNil())),
+			HaveField("Status.Booted.ImageDigest", Equal(env.NodeImageUpdate2Digest())),
+			HaveField("Status.Conditions", ContainElement(And(
+				HaveField("Type", bootcv1alpha1.NodeIdle),
+				HaveField("Status", metav1.ConditionTrue),
+				HaveField("Reason", bootcv1alpha1.NodeReasonIdle),
+			))),
+		), "expected node %s to reach Idle with second update image", nodeName)
+	}
+
+	t.Logf("Both nodes are Idle with second update image")
+
+	// Phase 7: Verify the other node (the one that was NOT rebooting when
+	// we switched images) did not wastefully reboot into the first update
+	// image. It should have rebooted exactly once (into the second image).
+	bootCountAfter := getBootCount(t, env, ctx, otherNode)
+	t.Logf(
+		"Node %q boot count after: %s (before: %s)",
+		otherNode,
+		bootCountAfter,
+		bootCountBefore[otherNode],
+	)
+
+	beforeCount, err := strconv.Atoi(bootCountBefore[otherNode])
+	g.Expect(err).NotTo(HaveOccurred(), "parsing before boot count")
+	afterCount, err := strconv.Atoi(bootCountAfter)
+	g.Expect(err).NotTo(HaveOccurred(), "parsing after boot count")
+
+	g.Expect(afterCount-beforeCount).To(Equal(1),
+		"expected other node %s to reboot exactly once (from %d to %d), "+
+			"an extra reboot means it wastefully booted into the first update image",
+		otherNode, beforeCount, afterCount)
+
+	t.Logf(
+		"Verified node %q rebooted exactly once (no wasteful reboot into first image)",
+		otherNode,
+	)
+}
+
+// execOnNode finds the running daemon pod on the given node and executes
+// a command inside it via kubectl exec. It returns the command output.
+func execOnNode(
+	t *testing.T,
+	g Gomega,
+	env *e2eutil.Env,
+	ctx context.Context,
+	nodeName string,
+	command ...string,
+) string {
+	t.Helper()
+
+	var daemonPod corev1.Pod
+	g.Eventually(func(g Gomega) string {
+		var pods corev1.PodList
+		g.Expect(env.Client.List(ctx, &pods,
+			client.InNamespace("bootc-operator"),
+			client.MatchingLabels{
+				"app.kubernetes.io/name":      "bootc-operator",
+				"app.kubernetes.io/component": "daemon",
+			},
+		)).To(Succeed())
+		for _, p := range pods.Items {
+			if p.Spec.NodeName == nodeName && p.Status.Phase == corev1.PodRunning {
+				daemonPod = p
+				return p.Name
+			}
+		}
+		return ""
+	}).WithTimeout(1*time.Minute).ShouldNot(BeEmpty(),
+		"expected running daemon pod on %s", nodeName)
+
+	kubeconfigPath := os.Getenv("KUBECONFIG")
+	args := append([]string{"--kubeconfig", kubeconfigPath,
+		"-n", "bootc-operator", "exec", daemonPod.Name, "--"}, command...)
+	cmd := exec.CommandContext(ctx, "kubectl", args...)
+	out, err := cmd.CombinedOutput()
+	g.Expect(err).NotTo(HaveOccurred(),
+		fmt.Sprintf("kubectl exec on %s failed: %s", nodeName, string(out)))
+
+	return strings.TrimSpace(string(out))
+}
+
+// getBootCount returns the number of boots on a node by running
+// journalctl --list-boots inside the daemon pod via kubectl exec.
+func getBootCount(t *testing.T, env *e2eutil.Env, ctx context.Context, nodeName string) string {
+	t.Helper()
+
+	g := NewWithT(t)
+
+	out := execOnNode(t, g, env, ctx, nodeName,
+		"nsenter", "-m/proc/1/ns/mnt", "--", "journalctl", "--list-boots")
+
+	count := 0
+	for _, line := range strings.Split(out, "\n") {
+		if strings.TrimSpace(line) != "" {
+			count++
+		}
+	}
+	return fmt.Sprintf("%d", count)
+}
+
 // TestPauseResume provisions a worker node, starts an update with the
 // pool paused, verifies the node stages but does not reboot, then resumes
 // and verifies the update completes.