From 1068b130a4597924593284ab076deb6f5b2a2c05 Mon Sep 17 00:00:00 2001 From: Bryan Cox Date: Sat, 25 Jul 2026 10:55:08 -0400 Subject: [PATCH 1/2] fix(e2e): make OSImageStream default status test payload-agnostic The NodePoolOSImageStreamDefaultStatusTest hardcoded expectedStream to rhel-9, which broke when the OCP 5.0 nightly payload switched RHCOS from RHEL 9 to RHEL 10 (July 20-22 builds). The original fix used the OCP release version to predict the RHEL generation, but during transitions the payload's RHCOS version may not match the OCP minor version (e.g. 5.0 payload with RHEL 9 RHCOS on AWS). Instead of predicting, assert that status.osImageStream is set to any recognized stream (rhel-9 or rhel-10). For the no-rollout test, read the actual stream from status rather than deriving it from the version. Co-Authored-By: Claude Opus 4.6 --- .../v2/tests/nodepool_osimagestream_test.go | 75 ++++++++++++------- 1 file changed, 48 insertions(+), 27 deletions(-) diff --git a/test/e2e/v2/tests/nodepool_osimagestream_test.go b/test/e2e/v2/tests/nodepool_osimagestream_test.go index 21c348c7035e..57a126e9a732 100644 --- a/test/e2e/v2/tests/nodepool_osimagestream_test.go +++ b/test/e2e/v2/tests/nodepool_osimagestream_test.go @@ -241,19 +241,10 @@ spec: } // NodePoolOSImageStreamDefaultStatusTest verifies that the existing default NodePool -// (no osImageStream set) reports rhel-9 in status.osImageStream. +// (no osImageStream set) reports a recognized RHEL stream in status.osImageStream. // This is a non-lifecycle test: it reads existing state without mutation. -// -// TODO(CNTRLPLANE-3032): The default OS stream is currently hardcoded to rhel-9 for all -// OCP versions. When the hardcoding is removed and OCP >= 5.0 defaults to rhel-10, -// this test must be updated to expect rhel-10 on OCP >= 5.0: -// -// expectedStream := hyperv1.OSImageStreamRHEL9 -// if e2eutil.IsGreaterThanOrEqualTo(e2eutil.Version50) { -// expectedStream = hyperv1.OSImageStreamRHEL10 -// } func NodePoolOSImageStreamDefaultStatusTest(getTestCtx internal.TestContextGetter) { - It("When no osImageStream is set, it should report rhel-9 in status", func() { + It("When no osImageStream is set, it should report a recognized RHEL stream in status", func() { testCtx := getTestCtx() testCtx.ValidateHostedCluster() @@ -290,23 +281,20 @@ func NodePoolOSImageStreamDefaultStatusTest(getTestCtx internal.TestContextGette e2eutil.WithInterval(15*time.Second), ) - // The default OS stream is currently hardcoded to rhel-9 for all OCP versions. - // TODO(CNTRLPLANE-3032): When the hardcoding is removed, change this to expect rhel-10 - // on OCP >= 5.0. - expectedStream := hyperv1.OSImageStreamRHEL9 - - GinkgoWriter.Printf("Waiting for NodePool %s/%s status.osImageStream.name to be %s\n", - defaultNP.Namespace, defaultNP.Name, expectedStream) + // Verify the controller populated status.osImageStream from the actual + // Machine NodeInfo.OSImage. The concrete value (rhel-9 or rhel-10) depends + // on the RHCOS shipped in the payload, so we assert it is set and valid + // rather than predicting the exact stream. e2eutil.EventuallyObject[*hyperv1.NodePool]( GinkgoTB(), ctx, - "default NodePool status to report osImageStream="+expectedStream, + "default NodePool status to report a non-empty osImageStream", func(pollCtx context.Context) (*hyperv1.NodePool, error) { pool := &hyperv1.NodePool{} err := testCtx.MgmtClient.Get(pollCtx, crclient.ObjectKeyFromObject(defaultNP), pool) return pool, err }, []e2eutil.Predicate[*hyperv1.NodePool]{ - e2eutil.OSImageStreamPredicate(expectedStream), + osImageStreamSetPredicate(), }, e2eutil.WithTimeout(10*time.Minute), e2eutil.WithInterval(15*time.Second), @@ -337,6 +325,23 @@ func nodesInfoPopulatedPredicate() e2eutil.Predicate[*hyperv1.NodePool] { } } +// osImageStreamSetPredicate returns a predicate that validates that a NodePool's +// status.osImageStream.name is set to a recognized RHEL stream value. +func osImageStreamSetPredicate() e2eutil.Predicate[*hyperv1.NodePool] { + return func(pool *hyperv1.NodePool) (bool, string, error) { + name := pool.Status.OSImageStream.Name + if name == "" { + return false, "status.osImageStream.name is empty", nil + } + switch name { + case hyperv1.OSImageStreamRHEL9, hyperv1.OSImageStreamRHEL10: + return true, fmt.Sprintf("status.osImageStream.name=%s", name), nil + default: + return false, fmt.Sprintf("status.osImageStream.name=%q is not a recognized RHEL stream", name), nil + } + } +} + // conditionMessageContains returns a predicate that checks whether a NodePool // condition of the given type has a message containing the specified substring. func conditionMessageContains(condType string, substring string) e2eutil.Predicate[*hyperv1.NodePool] { @@ -382,13 +387,29 @@ func NodePoolOSImageStreamExplicitDefaultNoRolloutTest(getTestCtx internal.TestC "default NodePool %s should have a config hash annotation", defaultNP.Name) GinkgoWriter.Printf("Original config hash for NodePool %s: %s\n", defaultNP.Name, originalConfigHash) - // Determine the version-derived default stream. - // On OCP < 5.0: rhel-9; on OCP >= 5.0: rhel-10 (default NodePool has no runc config). - versionDerivedDefault := hyperv1.OSImageStreamRHEL9 - if e2eutil.IsGreaterThanOrEqualTo(e2eutil.Version50) { - versionDerivedDefault = hyperv1.OSImageStreamRHEL10 - } - GinkgoWriter.Printf("Version-derived default stream: %s\n", versionDerivedDefault) + // Poll for status.osImageStream rather than reading a point-in-time + // snapshot — the controller may not have set it yet if Machines are + // still registering NodeInfo. + GinkgoWriter.Printf("Waiting for NodePool %s to have status.osImageStream set\n", defaultNP.Name) + e2eutil.EventuallyObject[*hyperv1.NodePool]( + GinkgoTB(), ctx, + fmt.Sprintf("NodePool %s status.osImageStream to be set", defaultNP.Name), + func(pollCtx context.Context) (*hyperv1.NodePool, error) { + pool := &hyperv1.NodePool{} + err := testCtx.MgmtClient.Get(pollCtx, crclient.ObjectKeyFromObject(defaultNP), pool) + return pool, err + }, + []e2eutil.Predicate[*hyperv1.NodePool]{ + osImageStreamSetPredicate(), + }, + e2eutil.WithTimeout(10*time.Minute), + e2eutil.WithInterval(15*time.Second), + ) + + // Re-read the NodePool to get the populated status for the patch below. + Expect(testCtx.MgmtClient.Get(ctx, crclient.ObjectKeyFromObject(defaultNP), defaultNP)).To(Succeed()) + versionDerivedDefault := defaultNP.Status.OSImageStream.Name + GinkgoWriter.Printf("Observed default stream from status: %s\n", versionDerivedDefault) // Patch the NodePool to set osImageStream to the version-derived default. base := defaultNP.DeepCopy() From cc60bc48e3ed812add095017513e7207c961861f Mon Sep 17 00:00:00 2001 From: Bryan Cox Date: Sat, 25 Jul 2026 14:36:40 -0400 Subject: [PATCH 2/2] fix(make): exclude build-tagged packages from test-changed Co-Authored-By: Claude Opus 4.6 --- Makefile | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index f21f5840420b..6152cc655665 100644 --- a/Makefile +++ b/Makefile @@ -390,14 +390,26 @@ test: generate # Skips entirely if no .go files changed. No generate dependency (verify-quick handles it). .PHONY: test-changed test-changed: - @CHANGED_PKGS=$$(git diff --name-only $(PULL_BASE_SHA)...HEAD -- '*.go' | \ + @CHANGED_DIRS=$$(git diff --name-only $(PULL_BASE_SHA)...HEAD -- '*.go' | \ while IFS= read -r file; do dirname "$$file"; done | \ - sort -u | sed 's|^|./|' | grep -v '^\./vendor/' | grep -v '^\./hack/tools/' | grep -vE '^\./test/e2e(/|$$)'); \ - if [ -z "$$CHANGED_PKGS" ]; then \ + sort -u | sed 's|^|./|' | grep -v '^\./vendor/' | grep -v '^\./hack/tools/'); \ + if [ -z "$$CHANGED_DIRS" ]; then \ echo "No Go files changed relative to $(PULL_BASE_SHA), skipping tests."; \ else \ - echo "Running tests for changed packages: $$CHANGED_PKGS"; \ - $(GO) test -parallel=$(NUM_CORES) -count=1 -timeout=30m $$CHANGED_PKGS; \ + CHANGED_PKGS=""; \ + for pkg in $$CHANGED_DIRS; do \ + list_out=$$($(GO) list "$$pkg" 2>&1) && { CHANGED_PKGS="$$CHANGED_PKGS $$pkg"; continue; }; \ + case "$$list_out" in \ + *"build constraints exclude"*|*"no Go files"*) ;; \ + *) echo "ERROR: go list $$pkg failed: $$list_out" >&2; exit 1 ;; \ + esac; \ + done; \ + if [ -z "$$CHANGED_PKGS" ]; then \ + echo "Changed packages all excluded by build constraints, skipping tests."; \ + else \ + echo "Running tests for changed packages: $$CHANGED_PKGS"; \ + $(GO) test -parallel=$(NUM_CORES) -count=1 -timeout=30m $$CHANGED_PKGS; \ + fi; \ fi # Run a subset of unit tests (used by CI sharding).