Skip to content

e2e: add mid-rollout image change test - #129

Open
ptalgulk01 wants to merge 1 commit into
bootc-dev:mainfrom
ptalgulk01:e2e-mid-rollout-image-change
Open

e2e: add mid-rollout image change test#129
ptalgulk01 wants to merge 1 commit into
bootc-dev:mainfrom
ptalgulk01:e2e-mid-rollout-image-change

Conversation

@ptalgulk01

Copy link
Copy Markdown
Collaborator

Summary

  • Add TestMidRolloutImageChange e2e test that verifies changing the target image mid-rollout does not cause unnecessary reboots
  • Build a second update image (node:update2) for testing distinct image transitions
  • Add NodeImageUpdate2DigestedPullSpec() and NodeImageUpdate2Digest() helpers to the e2e test environment

The test provisions two worker nodes, starts a rollout to one image, the switches the target to a different image while one node is rebooting. It verifies both nodes converge to the final image and uses journalctl --list-boots to confirm the non-rebooting node did not wastefully reboot into the first image.

Partial progress on #69

Test plan

  • make e2e V=1 RUN=TestMidRolloutImageChange passes (183s)
  • Boot count verified: non-rebooting node rebooted exactly once
  • make -n build-update-image and make -n e2e dry-runs confirm
    Makefile correctness
  • All three images (latest, update, update2) produce distinct
    digests

Comment thread test/e2e/e2eutil/env.go Outdated
Comment on lines +266 to +269
if e.nodeImageRegistry == "" || e.nodeImageUpdate2Digest == "" {
return ""
}
return e.nodeImageRegistry + "@" + e.nodeImageUpdate2Digest

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a duplication of the function NodeImageUpdateDigestedPullSpec can you define a local common function which takes the image as input

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Extracted a common digestedPullSpec(digest string) private method that all three *DigestedPullSpec() methods now delegate to.

Comment thread test/e2e/bootcnode_test.go Outdated
Comment on lines +484 to +510
var daemonPod corev1.Pod
g.Eventually(func(g Gomega) {
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())
var matched []corev1.Pod
for _, p := range pods.Items {
if p.Spec.NodeName == nodeName && p.Status.Phase == corev1.PodRunning {
matched = append(matched, p)
}
}
g.Expect(matched).To(HaveLen(1))
daemonPod = matched[0]
}).WithTimeout(1 * time.Minute).Should(Succeed())

kubeconfigPath := os.Getenv("KUBECONFIG")
cmd := exec.CommandContext(ctx, "kubectl", "--kubeconfig", kubeconfigPath,
"-n", "bootc-operator", "exec", daemonPod.Name, "--",
"nsenter", "-m/proc/1/ns/mnt", "--", "journalctl", "--list-boots")
out, err := cmd.CombinedOutput()
g.Expect(err).NotTo(HaveOccurred(),
fmt.Sprintf("journalctl --list-boots failed: %s", string(out)))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very similar to what we do already here, would you mind to refactor and create a common function to reduce duplication

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Extracted execOnNode() that handles daemon pod lookup and kubectl exec. Both TestUpdateReboot (update-marker check) and getBootCount() now use it.

Comment thread test/e2e/bootcnode_test.go Outdated
HaveField("Status", metav1.ConditionTrue),
HaveField("Reason", bootcv1alpha1.NodeReasonIdle),
)))
}).WithTimeout(3 * time.Minute).Should(Succeed())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the REVIEW_GOLANG.md for the test assertions

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Converted the func(g Gomega) + Should(Succeed()) pattern to return (T, error) with SatisfyAll/HaveField matchers per the review guidelines.

Comment thread test/e2e/bootcnode_test.go Outdated
Comment on lines +405 to +419
g.Eventually(func(g Gomega) {
for _, nodeName := range []string{nodeA, nodeB} {
var bn bootcv1alpha1.BootcNode
g.Expect(env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)).To(Succeed())
for _, c := range bn.Status.Conditions {
if c.Type == bootcv1alpha1.NodeIdle &&
c.Status == metav1.ConditionFalse &&
c.Reason == bootcv1alpha1.NodeReasonRebooting {
rebootingNode = nodeName
return
}
}
}
g.Expect(rebootingNode).NotTo(BeEmpty(), "expected at least one node to be Rebooting")
}).WithTimeout(5 * time.Minute).Should(Succeed())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
g.Eventually(func(g Gomega) {
for _, nodeName := range []string{nodeA, nodeB} {
var bn bootcv1alpha1.BootcNode
g.Expect(env.Client.Get(ctx, client.ObjectKey{Name: nodeName}, &bn)).To(Succeed())
for _, c := range bn.Status.Conditions {
if c.Type == bootcv1alpha1.NodeIdle &&
c.Status == metav1.ConditionFalse &&
c.Reason == bootcv1alpha1.NodeReasonRebooting {
rebootingNode = nodeName
return
}
}
}
g.Expect(rebootingNode).NotTo(BeEmpty(), "expected at least one node to be Rebooting")
}).WithTimeout(5 * time.Minute).Should(Succeed())
rebootingNode := 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 {
return name
}
}
return ""
}).WithTimeout(5 * time.Minute).ShouldNot(BeEmpty())

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Using meta.FindStatusCondition and ShouldNot(BeEmpty()) as suggested.

Comment on lines +439 to +454
// Phase 6: Wait for both nodes to be Idle with the second update image.
for _, nodeName := range []string{nodeA, nodeB} {
g.Eventually(func(g Gomega) {
var bn bootcv1alpha1.BootcNode
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.NodeImageUpdate2Digest()),
"expected booted digest to match second update image")
g.Expect(bn.Status.Conditions).To(ContainElement(And(
HaveField("Type", bootcv1alpha1.NodeIdle),
HaveField("Status", metav1.ConditionTrue),
HaveField("Reason", bootcv1alpha1.NodeReasonIdle),
)))
}).WithTimeout(8 * time.Minute).Should(Succeed(),
"expected node %s to reach Idle with second update image", nodeName)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Phase 6: Wait for both nodes to be Idle with the second update image.
for _, nodeName := range []string{nodeA, nodeB} {
g.Eventually(func(g Gomega) {
var bn bootcv1alpha1.BootcNode
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.NodeImageUpdate2Digest()),
"expected booted digest to match second update image")
g.Expect(bn.Status.Conditions).To(ContainElement(And(
HaveField("Type", bootcv1alpha1.NodeIdle),
HaveField("Status", metav1.ConditionTrue),
HaveField("Reason", bootcv1alpha1.NodeReasonIdle),
)))
}).WithTimeout(8 * time.Minute).Should(Succeed(),
"expected node %s to reach Idle with second update image", nodeName)
}
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)
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Using return (BootcNode, error) with SatisfyAll + HaveField matchers.

@alicefr

alicefr commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

@ptalgulk01 the ci is timing out, you need to increase the overall timeout. I hit the same in #128 , the PR already increases it. So, either way, we wait for mine to be merged or try to increase it here, and I will rebase if this land first

@ptalgulk01
ptalgulk01 force-pushed the e2e-mid-rollout-image-change branch 2 times, most recently from e9a8bc0 to a9c0fb1 Compare August 10, 2026 14:40
Signed-off-by: Prachiti Talgulkar <ptalgulk01@users.noreply.github.com>
@ptalgulk01
ptalgulk01 force-pushed the e2e-mid-rollout-image-change branch from a9c0fb1 to 0c7018f Compare August 10, 2026 14:49
@ptalgulk01

Copy link
Copy Markdown
Collaborator Author

Hi @alicefr , I checked the CI is failing because ghcr.io/bootc-dev/bink/node:v1.36-fedora-44-disk references a bootc image digest (sha256:83956d...) that doesn't exist in the registry. This looks like a broken image publish

@alicefr

alicefr commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

It seems the recent bootc-dev/bink#109 broke the node image. I will drop soon, but I will take a look tomorrow. Probably, we will need to pin the disk image by digest instead of using the tag, this could avoid sudden regression like this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants