e2e: add daemon-recovery, reboot-timeout, and operator-upgrade tests - #174
ptalgulk01 wants to merge 4 commits into
Conversation
|
|
||
| container := "k8s-" + e.clusterName + "-" + nodeName | ||
| t.Logf("Powering off node %q (podman stop %s)", nodeName, container) | ||
| cmd := exec.Command("podman", "stop", container) |
There was a problem hiding this comment.
You can also use bink node remove
| // CentOS Stream) and verifies the node upgrades across the distro boundary. | ||
| // Covers scenario 8 of #69. Skipped when the different-OS image is not seeded | ||
| // (run `make seed-different-os-image`). | ||
| func TestDifferentOSUpgrade(t *testing.T) { |
There was a problem hiding this comment.
Can we remove this test and enable when we have the bink support
|
|
||
| // expectNodeIdleOnDigest waits until the named node is Idle (no active update | ||
| // cycle) with the given booted image digest. | ||
| func expectNodeIdleOnDigest( |
There was a problem hiding this comment.
I think this function can be reused also in other existing tests. Can you please move the refactoring into a separate commit?
| patched := pool.DeepCopy() | ||
| patched.Spec.Image.Ref = updateRef | ||
| if patched.Spec.Rollout == nil { | ||
| patched.Spec.Rollout = &bootcv1alpha1.RolloutSpec{} | ||
| } | ||
| patched.Spec.Rollout.Paused = true | ||
| g.Expect(env.Client.Patch(ctx, patched, client.MergeFrom(pool))).To(Succeed()) | ||
| *pool = *patched | ||
|
|
||
| t.Logf("Patched pool to update image %s (paused)", updateRef) | ||
|
|
||
| // Phase 3: Wait until the node has staged the update but has not rebooted. | ||
| 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("Staged", And( | ||
| Not(BeNil()), | ||
| HaveField("ImageDigest", Equal(env.NodeImageUpdateDigest())), | ||
| )), | ||
| HaveField("Conditions", ContainElement(And( | ||
| HaveField("Type", bootcv1alpha1.NodeIdle), | ||
| HaveField("Status", metav1.ConditionFalse), | ||
| HaveField("Reason", bootcv1alpha1.NodeReasonStaged), | ||
| ))), | ||
| ), "expected node to stage the update before killing the daemon") |
There was a problem hiding this comment.
This can be also refactored in a function and unified with the previous test you implemented in #152
| // (both the controller Deployment and the DaemonSet). Kubernetes recreates | ||
| // them as it would after an image bump. It waits for a fresh controller pod and | ||
| // a fresh daemon pod on the given node to be Running before returning. | ||
| func restartOperator(t *testing.T, env *e2eutil.Env, ctx context.Context, nodeName string) { |
There was a problem hiding this comment.
we are start having a lot of helper functions, maybe it will be better to move them under test/util/
| # Base image for the cross-distro upgrade test. Deliberately a different OS | ||
| # lineage than the Fedora-based node image so the test exercises switching | ||
| # across distributions (CentOS Stream vs Fedora). | ||
| DIFFERENT_OS_IMAGE ?= quay.io/centos-bootc/centos-bootc:stream10 |
There was a problem hiding this comment.
How would we make bink working with this image? do we need to change the firmware?
There was a problem hiding this comment.
As mentioned for the test, I would keep this as separate PR once bink is able to work with both
|
@ptalgulk01 thanks for the work. Could you please try to split the PR into multiple commits and add a short description as commit body? Thanks |
|
The test suite is also becoming quite large and time consuming, we should prioritizing also #77 over new tests in the future. Do you have some cycles to take a look? |
|
Probably, you will also need to increase the overall timeout for the test suite https://github.com/bootc-dev/bootc-operator/blob/main/Makefile#L106 |
90406ae to
a32ecd0
Compare
|
/cc @alicefr can ptal? |
|
There is one missing comment to address https://github.com/bootc-dev/bootc-operator/pull/174/changes#r4036403874, but the rest looks good |
0137e84 to
95d7992
Compare
|
This requires a rebase to include the testing for k8s 1.37 |
…ence tests Covers scenarios 4, 5 (daemon half), and 9 of bootc-dev#69: a node that never comes back after a reboot must be reported degraded at the pool level, a rollout must survive the daemon pod being killed mid-flight, and the operator itself must survive a pod restart without disrupting managed nodes. Drops the cross-distro upgrade test added in an earlier revision: a genuinely different-OS-lineage bootc image does not currently boot under bink (stages and switches fine, but fails to boot on bink's disk/firmware layout and rolls back), so the test can't pass in CI. Revisit once bink supports it. Renames the operator-restart test to TestOperatorRestartResilience to avoid colliding with the now-merged TestOperatorUpgrade, which covers an actual version upgrade via a released manifest rather than a same-version pod restart. Signed-off-by: Prachiti Talgulkar <ptalgulk01@users.noreply.github.com> Assisted-by: AI
The new daemon-recovery and reboot-timeout tests duplicated the local expectNodeIdleOnDigest helper that testutil.WaitForNodeIdle already covers, and TestDaemonRecovery duplicated the "patch a paused update and wait for it to park at Staged pre-reboot" setup already used by TestControllerRecovery. Reuse the shared helper and extract the duplicated setup into stagePausedUpdate so both tests share it. Signed-off-by: Prachiti Talgulkar <ptalgulk01@users.noreply.github.com> Assisted-by: AI
Moves stagePausedUpdate, daemonPodsOnNode, and restartOperator into test/util alongside the existing WaitForNodeIdle, taking a client.Client instead of *e2eutil.Env so they don't depend on the e2e package and can be reused by other test packages later. Signed-off-by: Prachiti Talgulkar <ptalgulk01@users.noreply.github.com> Assisted-by: AI
The full e2e suite now includes 3 additional tests (TestDaemonRecovery, TestRebootTimeoutDegraded, TestOperatorRestartResilience) on top of an already near-full 40m budget. On the slower CI runners (observed on kube-minor 1.34 and 1.36), the suite now exceeds `go test -timeout 40m`, killing the run mid-test with "panic: test timed out after 40m0s" rather than a real test failure. Raise the go test timeout to 60m and the surrounding job timeout-minutes to 75 to give enough headroom for runner speed variance. Signed-off-by: Prachiti Talgulkar <ptalgulk01@users.noreply.github.com> Assisted-by: AI
dd58448 to
b6c3ff1
Compare
I actually tried swapping this to bink node remove --force and ran it through CI to check — it breaks the test. bink node remove drains and deletes the Kubernetes Node object before removing the container, and BootcNodePoolReconciler.syncMembership garbage-collects the owned BootcNode the moment its backing Node is deleted. So the node just cleanly disappears from the pool instead of getting stuck — DegradedCount never reaches 1 and the test times out: podman stop is intentional: it leaves the Node object in place (NotReady), which correctly simulates a node that went down mid-reboot and never came back, as opposed to one that was cleanly decommissioned. I've kept podman stop as-is. |
This branch has not been deployed
2 participants
e2e: add daemon-recovery, reboot-timeout, and operator-upgrade tests
Automates scenarios 4, 5, and 9 from #69, plus scaffolding for the
cross-distro upgrade (#8), which is skipped because a different-OS
image does not boot under bink (see in-code TODO).
Adds env.PowerOffNode and different-OS image plumbing, and a
seed-different-os-image Makefile target.