diff --git a/AGENTS.md b/AGENTS.md index 5fd997a2a946..a90fd6588574 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,6 +19,7 @@ Project documentation is published via MkDocs. The site structure and navigation | **Control plane components (CPOv2)** | [support/controlplane-component/AGENTS.md](support/controlplane-component/AGENTS.md) and [support/controlplane-component/README.md](support/controlplane-component/README.md) | | **Control plane operator** | [control-plane-operator/AGENTS.md](control-plane-operator/AGENTS.md) | | **E2E v2 test framework** | [test/e2e/v2/AGENTS.md](test/e2e/v2/AGENTS.md) | +| **E2E async assertions** | [test/e2e/util/AGENTS.md](test/e2e/util/AGENTS.md) — `EventuallyObject`/`EventuallyObjects` required for all Kubernetes object polling | | **Envtest (CEL validation tests)** | [test/envtest/README.md](test/envtest/README.md) — YAML-driven, runs across k8s 1.30–1.35, supports feature gate filtering | | **CEL over webhooks** | [.claude/rules/webhook-validation.md](.claude/rules/webhook-validation.md) | | **Code formatting** | [DEVELOPMENT.md](DEVELOPMENT.md) — code quality commands and conventions | diff --git a/test/e2e/util/AGENTS.md b/test/e2e/util/AGENTS.md new file mode 100644 index 000000000000..e9d4cfed3866 --- /dev/null +++ b/test/e2e/util/AGENTS.md @@ -0,0 +1,31 @@ +# E2E Utility Package (`test/e2e/util`) + +## Async Assertions + +All asynchronous assertions that poll Kubernetes objects **must** use `EventuallyObject` or `EventuallyObjects` from `eventually.go`. Do **not** use raw `wait.PollUntilContextTimeout` with `t.Logf` inside the poll body. + +### Why + +Raw polling loops log on every iteration. A 20-minute timeout with a 10-second interval produces up to 120 identical log lines per object, drowning useful output. The `EventuallyObject` framework only logs when state **changes**, keeps output compact, and provides a structured failure summary at the end. + +### Usage + +```go +EventuallyObject(t, ctx, "DaemonSet kube-system/my-ds to be ready", + func(ctx context.Context) (*appsv1.DaemonSet, error) { + ds := &appsv1.DaemonSet{} + err := client.Get(ctx, crclient.ObjectKey{Name: name, Namespace: ns}, ds) + return ds, err + }, + []Predicate[*appsv1.DaemonSet]{ + myPredicate(), + }, + WithTimeout(20*time.Minute), +) +``` + +A `Predicate[T]` has signature `func(T) (done bool, reason string, err error)`. The `reason` is logged only when it differs from the previous poll cycle. + +### When to use Gomega `Eventually` + +Gomega's `Eventually` (from Ginkgo/Gomega) is acceptable for short-lived, non-object checks (e.g., verifying a function returns true within 30 seconds). For anything polling a Kubernetes object with a timeout longer than ~1 minute, prefer `EventuallyObject`/`EventuallyObjects`. diff --git a/test/e2e/util/cilium.go b/test/e2e/util/cilium.go index 0c668324f351..ddf63267e3e3 100644 --- a/test/e2e/util/cilium.go +++ b/test/e2e/util/cilium.go @@ -324,8 +324,7 @@ func InstallCilium(t *testing.T, ctx context.Context, guestClient crclient.Clien // Now wait for DaemonSet pods to be ready t.Log("Waiting for Cilium agent pods from DaemonSet to be ready") - err = waitForDaemonSetReady(t, ctx, guestClient, ciliumDaemonSet.Name, ciliumDaemonSet.Namespace, nodePoolReplicas) - g.Expect(err).NotTo(HaveOccurred(), "failed to wait for Cilium DaemonSet to be ready") + waitForDaemonSetReady(t, ctx, guestClient, ciliumDaemonSet.Name, ciliumDaemonSet.Namespace, nodePoolReplicas) t.Log("Cilium installation completed successfully") }) diff --git a/test/e2e/util/globalps.go b/test/e2e/util/globalps.go index 8f1e14177a36..c488c672147e 100644 --- a/test/e2e/util/globalps.go +++ b/test/e2e/util/globalps.go @@ -15,7 +15,6 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/wait" "k8s.io/utils/ptr" crclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -178,19 +177,18 @@ func VerifyKubeletConfigWithDaemonSet(t *testing.T, ctx context.Context, guestCl } }) - t.Log("Waiting for syncer DS rollout to complete before deploying verifier") - g.Expect(waitForDaemonSetRollout(t, ctx, guestClient, hccomanifests.GlobalPullSecretDSName, hccomanifests.GlobalPullSecretNamespace, expectedNodeCount)).To(Succeed()) + waitForDaemonSetRollout(t, ctx, guestClient, hccomanifests.GlobalPullSecretDSName, hccomanifests.GlobalPullSecretNamespace, expectedNodeCount) t.Log("Creating kubelet config verifier DaemonSet") err := CreateKubeletConfigVerifierDaemonSet(ctx, guestClient, dsImage) g.Expect(err).NotTo(HaveOccurred(), "failed to create kubelet config verifier DaemonSet") t.Log("Waiting for OVN, GlobalPullSecret, Konnectivity and kubelet config verifier DaemonSets to be ready") - g.Expect(waitForDaemonSetReady(t, ctx, guestClient, "ovnkube-node", "openshift-ovn-kubernetes", expectedNodeCount)).To(Succeed()) - g.Expect(waitForDaemonSetReady(t, ctx, guestClient, hccomanifests.GlobalPullSecretDSName, hccomanifests.GlobalPullSecretNamespace, expectedNodeCount)).To(Succeed()) + waitForDaemonSetReady(t, ctx, guestClient, "ovnkube-node", "openshift-ovn-kubernetes", expectedNodeCount) + waitForDaemonSetReady(t, ctx, guestClient, hccomanifests.GlobalPullSecretDSName, hccomanifests.GlobalPullSecretNamespace, expectedNodeCount) konnectivityDS := hccomanifests.KonnectivityAgentDaemonSet() - g.Expect(waitForDaemonSetReady(t, ctx, guestClient, konnectivityDS.Name, konnectivityDS.Namespace, expectedNodeCount)).To(Succeed()) - g.Expect(waitForDaemonSetReady(t, ctx, guestClient, KubeletConfigVerifierDaemonSetName, KubeletConfigVerifierNamespace, expectedNodeCount)).To(Succeed()) + waitForDaemonSetReady(t, ctx, guestClient, konnectivityDS.Name, konnectivityDS.Namespace, expectedNodeCount) + waitForDaemonSetReady(t, ctx, guestClient, KubeletConfigVerifierDaemonSetName, KubeletConfigVerifierNamespace, expectedNodeCount) t.Log("All verifier pods are Ready — on-disk kubelet config.json matches cluster pull secret on all nodes") } @@ -198,36 +196,18 @@ func VerifyKubeletConfigWithDaemonSet(t *testing.T, ctx context.Context, guestCl // must be updated with the latest template AND ready. This catches the case where // HCCO updates the pod template (e.g. new configSeed) and the DS controller is // still replacing pods. -func waitForDaemonSetRollout(t *testing.T, ctx context.Context, client crclient.Client, name, namespace string, minExpected int32) error { - t.Logf("Waiting for %s DaemonSet rollout to complete (min expected: %d)", name, minExpected) - - return wait.PollUntilContextTimeout(ctx, 10*time.Second, 5*time.Minute, true, func(ctx context.Context) (done bool, err error) { - ds := &appsv1.DaemonSet{} - if err := client.Get(ctx, crclient.ObjectKey{Name: name, Namespace: namespace}, ds); err != nil { - t.Logf("Failed to get DaemonSet %s: %v", name, err) - return false, nil - } - - if ds.Status.ObservedGeneration < ds.Generation { - t.Logf("DaemonSet %s generation %d not yet observed (current %d)", name, ds.Generation, ds.Status.ObservedGeneration) - return false, nil - } - - desired := ds.Status.DesiredNumberScheduled - if desired < minExpected { - t.Logf("DaemonSet %s: desired=%d < minExpected=%d", name, desired, minExpected) - return false, nil - } - - updated := ds.Status.UpdatedNumberScheduled - ready := ds.Status.NumberReady - - if updated != desired || ready != desired { - t.Logf("DaemonSet %s rollout: %d/%d updated, %d/%d ready", name, updated, desired, ready, desired) - return false, nil - } - - t.Logf("DaemonSet %s rollout complete: %d/%d pods updated and ready", name, ready, desired) - return true, nil - }) +func waitForDaemonSetRollout(t *testing.T, ctx context.Context, c crclient.Client, name, namespace string, minExpected int32) { + EventuallyObject(t, ctx, fmt.Sprintf("DaemonSet %s/%s rollout to complete", namespace, name), + func(ctx context.Context) (*appsv1.DaemonSet, error) { + ds := &appsv1.DaemonSet{} + err := c.Get(ctx, crclient.ObjectKey{Name: name, Namespace: namespace}, ds) + return ds, err + }, + []Predicate[*appsv1.DaemonSet]{ + daemonSetGenerationObserved(), + daemonSetDesiredScheduled(minExpected), + daemonSetAllUpdatedAndReady(), + }, + WithTimeout(5*time.Minute), + ) } diff --git a/test/e2e/util/util.go b/test/e2e/util/util.go index 8fc5e576d4a3..af431a1a892c 100644 --- a/test/e2e/util/util.go +++ b/test/e2e/util/util.go @@ -2169,10 +2169,10 @@ func EnsureGlobalPullSecret(t *testing.T, ctx context.Context, mgmtClient crclie }) t.Run("Wait for critical DaemonSets to be ready - first check", func(t *testing.T) { - g.Expect(waitForDaemonSetReady(t, ctx, guestClient, "ovnkube-node", "openshift-ovn-kubernetes", nodeCount)).To(Succeed()) - g.Expect(waitForDaemonSetReady(t, ctx, guestClient, hccomanifests.GlobalPullSecretDSName, hccomanifests.GlobalPullSecretNamespace, nodeCount)).To(Succeed()) + waitForDaemonSetReady(t, ctx, guestClient, "ovnkube-node", "openshift-ovn-kubernetes", nodeCount) + waitForDaemonSetReady(t, ctx, guestClient, hccomanifests.GlobalPullSecretDSName, hccomanifests.GlobalPullSecretNamespace, nodeCount) konnectivityDS := hccomanifests.KonnectivityAgentDaemonSet() - g.Expect(waitForDaemonSetReady(t, ctx, guestClient, konnectivityDS.Name, konnectivityDS.Namespace, nodeCount)).To(Succeed()) + waitForDaemonSetReady(t, ctx, guestClient, konnectivityDS.Name, konnectivityDS.Namespace, nodeCount) }) // Create a pod which uses the restricted image, should fail @@ -2211,10 +2211,10 @@ func EnsureGlobalPullSecret(t *testing.T, ctx context.Context, mgmtClient crclie }) t.Run("Wait for critical DaemonSets to be ready - second check", func(t *testing.T) { - g.Expect(waitForDaemonSetReady(t, ctx, guestClient, "ovnkube-node", "openshift-ovn-kubernetes", nodeCount)).To(Succeed()) - g.Expect(waitForDaemonSetReady(t, ctx, guestClient, hccomanifests.GlobalPullSecretDSName, hccomanifests.GlobalPullSecretNamespace, nodeCount)).To(Succeed()) + waitForDaemonSetReady(t, ctx, guestClient, "ovnkube-node", "openshift-ovn-kubernetes", nodeCount) + waitForDaemonSetReady(t, ctx, guestClient, hccomanifests.GlobalPullSecretDSName, hccomanifests.GlobalPullSecretNamespace, nodeCount) konnectivityDS := hccomanifests.KonnectivityAgentDaemonSet() - g.Expect(waitForDaemonSetReady(t, ctx, guestClient, konnectivityDS.Name, konnectivityDS.Namespace, nodeCount)).To(Succeed()) + waitForDaemonSetReady(t, ctx, guestClient, konnectivityDS.Name, konnectivityDS.Namespace, nodeCount) }) // Check if we can run a pod with the restricted image @@ -2249,7 +2249,7 @@ func EnsureGlobalPullSecret(t *testing.T, ctx context.Context, mgmtClient crclie // Wait for all DaemonSets to be ready after nodes stabilize t.Run("Wait for pull secret synchronization to stabilize across all nodes", func(t *testing.T) { t.Log("Waiting for GlobalPullSecretDaemonSet to process the deletion and stabilize all nodes") - g.Expect(waitForDaemonSetReady(t, ctx, guestClient, hccomanifests.GlobalPullSecretDSName, hccomanifests.GlobalPullSecretNamespace, nodeCount)).To(Succeed()) + waitForDaemonSetReady(t, ctx, guestClient, hccomanifests.GlobalPullSecretDSName, hccomanifests.GlobalPullSecretNamespace, nodeCount) }) // Check if the config.json is updated in all of the nodes @@ -2280,54 +2280,54 @@ func createAdditionalPullSecret(ctx context.Context, guestClient crclient.Client // waitForDaemonSetReady waits for a DaemonSet to have all pods ready. // minExpected is a sanity check to ensure we don't succeed when DesiredNumberScheduled is temporarily 0. -func waitForDaemonSetReady(t *testing.T, ctx context.Context, client crclient.Client, name, namespace string, minExpected int32) error { - t.Logf("Waiting for %s DaemonSet to be ready (min expected: %d)", name, minExpected) - - err := wait.PollUntilContextTimeout(ctx, 10*time.Second, 20*time.Minute, true, func(ctx context.Context) (done bool, err error) { - daemonSet := &appsv1.DaemonSet{} - err = client.Get(ctx, crclient.ObjectKey{Name: name, Namespace: namespace}, daemonSet) - if err != nil { - t.Logf("Failed to get DaemonSet %s: %v", name, err) - return false, nil - } - - if daemonSet.Status.ObservedGeneration < daemonSet.Generation { - t.Logf("DaemonSet %s status has not observed generation %d yet (current %d)", name, daemonSet.Generation, daemonSet.Status.ObservedGeneration) - return false, nil - } - - desired := daemonSet.Status.DesiredNumberScheduled - ready := daemonSet.Status.NumberReady +func waitForDaemonSetReady(t *testing.T, ctx context.Context, c crclient.Client, name, namespace string, minExpected int32) { + t.Helper() + EventuallyObject(t, ctx, fmt.Sprintf("DaemonSet %s/%s to be ready", namespace, name), + func(ctx context.Context) (*appsv1.DaemonSet, error) { + ds := &appsv1.DaemonSet{} + err := c.Get(ctx, crclient.ObjectKey{Name: name, Namespace: namespace}, ds) + return ds, err + }, + []Predicate[*appsv1.DaemonSet]{ + daemonSetGenerationObserved(), + daemonSetDesiredScheduled(minExpected), + daemonSetAllUpdatedAndReady(), + }, + WithTimeout(20*time.Minute), + ) +} - // Ensure we have at least the minimum expected nodes - if desired < minExpected { - t.Logf("DaemonSet %s: desired=%d < minExpected=%d, waiting for nodes", name, desired, minExpected) - return false, nil +// daemonSetGenerationObserved checks that the DaemonSet status has observed the current generation. +func daemonSetGenerationObserved() Predicate[*appsv1.DaemonSet] { + return func(ds *appsv1.DaemonSet) (bool, string, error) { + if ds.Status.ObservedGeneration < ds.Generation { + return false, fmt.Sprintf("generation %d not yet observed (current %d)", ds.Generation, ds.Status.ObservedGeneration), nil } + return true, "", nil + } +} - // Wait for rollout to complete (all pods updated with latest template) - updated := daemonSet.Status.UpdatedNumberScheduled - if updated != desired { - t.Logf("DaemonSet %s rollout in progress: %d/%d pods updated", name, updated, desired) - return false, nil +// daemonSetDesiredScheduled checks that at least minExpected pods are desired. +func daemonSetDesiredScheduled(minExpected int32) Predicate[*appsv1.DaemonSet] { + return func(ds *appsv1.DaemonSet) (bool, string, error) { + if ds.Status.DesiredNumberScheduled < minExpected { + return false, fmt.Sprintf("desired=%d < minExpected=%d", ds.Status.DesiredNumberScheduled, minExpected), nil } + return true, "", nil + } +} - // Wait for all desired pods to be ready - if ready != desired { - t.Logf("DaemonSet %s not ready: %d/%d pods ready", name, ready, desired) - return false, nil +// daemonSetAllUpdatedAndReady checks that all pods are updated and ready. +func daemonSetAllUpdatedAndReady() Predicate[*appsv1.DaemonSet] { + return func(ds *appsv1.DaemonSet) (bool, string, error) { + desired := ds.Status.DesiredNumberScheduled + updated := ds.Status.UpdatedNumberScheduled + ready := ds.Status.NumberReady + if updated != desired || ready != desired { + return false, fmt.Sprintf("%d/%d updated, %d/%d ready", updated, desired, ready, desired), nil } - - t.Logf("DaemonSet %s ready: %d/%d pods (all updated)", name, ready, desired) - return true, nil - }) - - if err != nil { - return fmt.Errorf("failed to wait for DaemonSet %s to be ready: %w", name, err) + return true, "", nil } - - t.Logf("✓ %s DaemonSet is ready", name) - return nil } func EnsureKubeAPIDNSNameCustomCert(t *testing.T, ctx context.Context, mgmtClient crclient.Client, entryHostedCluster *hyperv1.HostedCluster, clusterOpts PlatformAgnosticOptions) {