GCP-688: Port v1 E2E validations to envtest, unit tests, and v2 Ginkgo specs#8511
Merged
openshift-merge-bot[bot] merged 11 commits intoMay 26, 2026
Merged
Conversation
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
Skipping CI for Draft Pull Request. |
|
@cblecker: This pull request references GCP-688 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
Caution Review failedFailed to post review comments 📝 Walkthrough<review_stack_artifact_start --> All changes that add/modify the e2e v2 test framework, utilities, hosted-cluster validation, many new e2ev2 suites and tests, refactor existing tests to use TestContext, and add controller unit tests.Repository configuration and documentation: .coderabbit.yaml and AGENTS.md updates, E2E env var registration.range_af026fa44543 range_017fd66930e9 range_25450a5dd113 range_c3b6f39d33b8 range_9807b8843ea5 range_db6884189581Adds hosted-cluster REST-config caching/state, GetHostedClusterRESTConfig, and ValidateHostedCluster / ValidateHostedClusterClient APIs; adjusts SetupTestContextFromEnv.range_4e122292bc9b range_7b01b0b1f051 range_86e4796ab745 range_7b5198047192 range_eb64954702f9Adds RunCommandInPod and GetMetricsFromPod utilities (pods/exec SPDY executor and Prometheus text parsing) under e2ev2 build tag.range_8b82ad148157 range_4e0487b8b32b range_db6884189581Adds new e2ev2 test files and registration entrypoints: health, metrics, security, compliance, DNS; registers suites and Describe blocks.range_80de6dac4c98 range_5b3b0824845c range_dda09bad9fb1 range_30e052c08829 range_81ee4aa2f7e5 range_8aa86b5a3212 range_8f865ff0eb78 range_a22f74e6c996 range_754ac66e87c1 range_35ac9febc718Implements hosted-cluster metrics checks, metrics-forwarder end-to-end verification, and node-tuning-operator metrics endpoint validation.range_f470cd8ed72b range_f46d1610788a range_46b186c43bc9Implements guest webhook auto-deletion, admission policy enforcement tests, AWS egress restriction checks, Route compliance, and DNS test scaffold with E2E_SERVICE_DOMAIN gating.range_602e58fb8c8f range_14518b4680c2 range_219752011659 range_14676c2950e2 range_8778758fd887 range_6556eaa600c2 range_81ee4aa2f7e5Refactors many per-workload tests to use testCtx.Context and Context(workload.Name), introduces NoCrashingPods, CustomLabels, CustomTolerations, and makes registry/infra checks blocking.range_d918fac5b374 range_79ca0f34858c range_2029d3ec6bfd range_1fad44d5f3a0 range_d94420b62822 range_4079f26ea25d range_3b1e3aa41940 range_d55c89de207c range_da36ac26d7bf range_91b40bb5c954 range_00028bb6c4e5 range_405cf1626051 range_3c2e6b954b48 range_7c4a40f69ea6 range_a9e2e7583da4 range_0fc988230104 range_2fb3872341d1 range_f268f0c7fb63 range_f13a266ac605 range_f13a266ac605Updates infrastructure tests to use testCtx.Context, refactors resource requests tests, and removes context.Background imports.range_79a4711bdafc range_d8a8c6f95e8d range_b9ed74a5ec46 range_d8a8c6f95e8d range_b9ed74a5ec46Small targeted updates across various tests to call TestContext validation early and improve cleanup/error handling.range_5cab09a7d8ac range_b1aafa7f49f2 range_fd815df3a2a6 range_cc092ea08114 range_6be9a415c2a3 range_fd815df3a2a6 range_fd815df3a2a6 range_a658c6aa072a range_6be9a415c2a3 range_fd815df3a2a6Standardizes hosted-cluster client validation across NodePool autoscaling and lifecycle tests, reorders imports, and improves cleanup handling.range_25251990a8c0 range_46ab53f336d5 range_4edde6d6528e range_83b59b58406d range_870ae26142f5 range_896a10498024 range_c38d28bb95ba range_b4237c16d3f2 range_8a2813bcf741 range_7016865d5dea range_327acd6f36e6 range_b61aaf9db479 range_46e46ada4632 range_4e6c987e8dd1 range_fc82ee670ba3 range_ed193ba19d24 range_d3173679416c range_4252bc5b3509 range_e11f32d297df range_aa77579005c7Adds HCP status reconciler unit test and guest-admission webhook tests (isAllowedWebhookUrl and ensureGuestAdmissionWebhooksAreValid).range_801277ab4dac range_678d74005b58 range_6005e3dcda92 range_5ccffe5069ec range_ed984c3e79c1 range_9d80df728b1b range_69e1617c807eTweaks controlplane-component test to assert pod tolerations, small Ginkgo wrapper/import/whitespace fixes, and BeforeEach validation swaps.range_4478ea697252 range_4f0f0941f4c0 range_21d53dc202f1 range_30e052c08829 range_a658c6aa072a range_f13a266ac605 range_964a84a103b6 range_79a4711bdafc range_d8a8c6f95e8d range_b9ed74a5ec46 range_964a84a103b6🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cblecker The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
openshift-ci
Bot
added
area/cli
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go`:
- Around line 6824-6830: The test currently bypasses production logic by
directly assigning hcluster.Status.Configuration = hcp.Status.Configuration;
instead invoke the controller's real reconciliation path (call the HostedCluster
reconciler's Reconcile method or the helper used by hostedcluster_controller.go
that performs the "Copy the configuration status from the hostedcontrolplane"
logic) so the same code path is exercised; remove the inline assignment, run the
reconcile/helper, then assert hcluster.Status.Configuration equals
tc.hcpConfiguration.
In `@test/e2e/v2/internal/pod_exec.go`:
- Around line 35-68: The function RunCommandInPod currently returns errors;
change it to panic on failures per framework guidelines: replace the error
return after kubernetes.NewForConfig with a panic that includes a clear
diagnostic message containing namespace and podName and the wrapped error;
likewise panic if remotecommand.NewSPDYExecutor fails with a message including
namespace/podName/containerName; and panic if exec.StreamWithContext returns an
error, including namespace/podName/containerName and stderr output in the panic
text; keep the successful return of stdout.String() as-is.
- Around line 72-80: GetMetricsFromPod currently returns an error on failure;
update it to follow the framework panic-on-failure pattern used by
RunCommandInPod: call RunCommandInPod with the same args, and if err != nil
panic with a diagnostic message including namespace/podName/containerName/port
and the error; otherwise return []byte(output). Ensure the function signature
stays the same but remove the error return path and panic on failures so the
framework behavior is consistent.
In `@test/e2e/v2/internal/test_context.go`:
- Around line 130-134: Replace the silent early return in the
tc.hostedClusterRESTConfigOnce.Do closure with a panic that includes a clear
diagnostic message; specifically, in the closure that calls
tc.GetHostedCluster() and checks hc == nil || hc.Status.KubeConfig == nil, panic
(do not assign or cache nil to tc.hostedClusterRESTConfig) with a message
describing whether the HostedCluster is missing or its kubeconfig is nil so
callers fail loudly and the sync.Once does not cache a nil result.
In `@test/e2e/v2/tests/control_plane_workloads_test.go`:
- Around line 695-696: The inline comment before the test that reads
'Label("Informing"): failures skip (non-blocking) until registry is complete' is
stale because the test starting with It("all pods should belong to predefined
workloads") is now a blocking test; update or remove that comment so it reflects
the current gating behavior—either delete the "Informing/non-blocking" note or
replace it with a brief accurate comment about this being a blocking/required
test for pod workload validation, referencing the It("all pods should belong to
predefined workloads") test to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 5d1857a4-dbd4-49da-87c1-4d11b0b3371e
⛔ Files ignored due to path filters (3)
cmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.capabilities.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.services.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.validation.testsuite.yamlis excluded by!cmd/install/assets/**/*.yaml
📒 Files selected for processing (13)
control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus/hcpstatus_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.gohypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.gosupport/controlplane-component/controlplane-component_test.gotest/e2e/v2/internal/env_vars.gotest/e2e/v2/internal/pod_exec.gotest/e2e/v2/internal/test_context.gotest/e2e/v2/tests/control_plane_workloads_test.gotest/e2e/v2/tests/hosted_cluster_compliance_test.gotest/e2e/v2/tests/hosted_cluster_dns_test.gotest/e2e/v2/tests/hosted_cluster_health_test.gotest/e2e/v2/tests/hosted_cluster_metrics_test.gotest/e2e/v2/tests/hosted_cluster_security_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8511 +/- ##
==========================================
+ Coverage 40.41% 40.53% +0.12%
==========================================
Files 755 755
Lines 93235 93235
==========================================
+ Hits 37679 37794 +115
+ Misses 52854 52740 -114
+ Partials 2702 2701 -1 see 3 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
cblecker
force-pushed
the
worktree-smooth-hatching-thompson
branch
2 times, most recently
from
3748c05 to
64758af
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/e2e/v2/tests/hosted_cluster_compliance_test.go`:
- Line 106: The It block "When routes are created in the control plane
namespace, it should label all routes for the per-HCP router" is missing an
explicit Label; modify that It declaration to include a Label(...) argument
(e.g., Label("Compliance") or the same label used by other It blocks in this
file) so the test can be filtered consistently—update the It(...) signature to
It("When routes...", Label("..."), func() { ... }) preserving the existing
description and test body.
- Around line 123-132: The test currently allows a vacuous pass when no routes
exist; after fetching routeList with
tc.MgmtClient.List(namespace=tc.ControlPlaneNamespace), assert that
routeList.Items is non-empty (e.g.,
Expect(len(routeList.Items)).To(BeNumerically(">", 0))) before iterating, then
continue to iterate each route (route := &routeList.Items[i]) and use
netutil.AddHCPRouteLabel and the existing Expect comparing route.Labels to
original.Labels so the test fails when there are zero routes instead of passing
silently.
- Around line 108-121: The test currently proceeds when tc.GetHostedCluster()
returns nil, which hides a missing precondition; update the code after calling
tc.GetHostedCluster() to fail fast if hostedCluster is nil (e.g., call t.Fatalf
or require.NotNil on the hostedCluster) so the test stops with a clear error
instead of continuing into the Route gating logic that uses
hostedCluster.Spec.Services; keep the existing APIServer/Route check and Skip()
only for the non-nil hostedCluster branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: bf3b2881-c543-4a73-ab34-21aff8e58c03
⛔ Files ignored due to path filters (3)
cmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.capabilities.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.services.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.validation.testsuite.yamlis excluded by!cmd/install/assets/**/*.yaml
📒 Files selected for processing (13)
control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus/hcpstatus_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.gohypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.gosupport/controlplane-component/controlplane-component_test.gotest/e2e/v2/internal/env_vars.gotest/e2e/v2/internal/pod_exec.gotest/e2e/v2/internal/test_context.gotest/e2e/v2/tests/control_plane_workloads_test.gotest/e2e/v2/tests/hosted_cluster_compliance_test.gotest/e2e/v2/tests/hosted_cluster_dns_test.gotest/e2e/v2/tests/hosted_cluster_health_test.gotest/e2e/v2/tests/hosted_cluster_metrics_test.gotest/e2e/v2/tests/hosted_cluster_security_test.go
🚧 Files skipped from review as they are similar to previous changes (9)
- control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus/hcpstatus_test.go
- test/e2e/v2/tests/control_plane_workloads_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go
- support/controlplane-component/controlplane-component_test.go
- test/e2e/v2/tests/hosted_cluster_dns_test.go
- test/e2e/v2/internal/test_context.go
- test/e2e/v2/tests/hosted_cluster_metrics_test.go
- test/e2e/v2/tests/hosted_cluster_security_test.go
- test/e2e/v2/tests/hosted_cluster_health_test.go
cblecker
force-pushed
the
worktree-smooth-hatching-thompson
branch
from
64758af to
d83cea8
Compare
openshift-ci
Bot
added
the
needs-rebase
cblecker
force-pushed
the
worktree-smooth-hatching-thompson
branch
from
d83cea8 to
7d23aac
Compare
openshift-ci
Bot
removed
the
needs-rebase
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/e2e/v2/tests/control_plane_infrastructure_test.go`:
- Line 93: The test declaration uses Label("Informing") which makes the registry
validation non-blocking; remove the Label("Informing") token from the It call so
the test reads It("should not contain any unrecognized pods", func() { ... })
making the check blocking and failing CI when unrecognized infrastructure pods
are detected (edit the It(...) invocation that currently includes
Label("Informing")).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: dd12233c-90fe-48f7-84bc-c921e0d9f9e2
⛔ Files ignored due to path filters (3)
cmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.capabilities.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.services.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.validation.testsuite.yamlis excluded by!cmd/install/assets/**/*.yaml
📒 Files selected for processing (16)
control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus/hcpstatus_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.gosupport/controlplane-component/controlplane-component_test.gotest/e2e/v2/internal/env_vars.gotest/e2e/v2/internal/test_context.gotest/e2e/v2/tests/backup_restore_test.gotest/e2e/v2/tests/control_plane_infrastructure_test.gotest/e2e/v2/tests/control_plane_workloads_test.gotest/e2e/v2/tests/hosted_cluster_ccm_test.gotest/e2e/v2/tests/hosted_cluster_compliance_test.gotest/e2e/v2/tests/hosted_cluster_dns_test.gotest/e2e/v2/tests/hosted_cluster_health_test.gotest/e2e/v2/tests/hosted_cluster_image_registry_test.gotest/e2e/v2/tests/hosted_cluster_metrics_test.gotest/e2e/v2/tests/hosted_cluster_security_test.gotest/e2e/v2/util/pod_exec.go
✅ Files skipped from review due to trivial changes (1)
- test/e2e/v2/internal/env_vars.go
🚧 Files skipped from review as they are similar to previous changes (5)
- test/e2e/v2/tests/hosted_cluster_dns_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go
- test/e2e/v2/tests/hosted_cluster_metrics_test.go
- test/e2e/v2/tests/hosted_cluster_health_test.go
- test/e2e/v2/tests/hosted_cluster_compliance_test.go
cblecker
force-pushed
the
worktree-smooth-hatching-thompson
branch
from
7d23aac to
e69555e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/e2e/v2/tests/hosted_cluster_dns_test.go (2)
37-47: ⚡ Quick winAdd explicit nil assertion before dereferencing
hostedCluster.Spec
hostedClusteris dereferenced immediately; add an explicit nil-check in the spec body with diagnostic context (namespace/name) so failures are actionable instead of panicking.As per coding guidelines, "Always nil-check pointers before dereferencing, with diagnostic messages that include namespace/name context".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/v2/tests/hosted_cluster_dns_test.go` around lines 37 - 47, The test dereferences hostedCluster.Spec without checking hostedCluster for nil; update the test (around the call to tc.GetHostedCluster and where hostedCluster.Spec is used) to explicitly assert hostedCluster != nil and error out with a diagnostic message containing the hostedCluster namespace/name (use hostedCluster.Namespace and hostedCluster.Name or tc.Namespace/Name as available) before accessing hostedCluster.Spec and hostedCluster.Spec.Services so failures produce actionable logs instead of panics.
41-43: ⚡ Quick winMove platform skip to
BeforeEachand standardize the skip messageThe KubeVirt platform skip is inside the spec body and uses a custom message. Move it into a
BeforeEachguard with the required message format ('Test name is only for [Platform] platform') to match v2 test conventions.As per coding guidelines, "Use
BeforeEachwithSkip()for platform-specific tests with the message format: 'Test name is only for [Platform] platform'".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/v2/tests/hosted_cluster_dns_test.go` around lines 41 - 43, Remove the inline platform check inside the spec body and add a BeforeEach guard that checks hostedCluster.Spec.Platform.Type against hyperv1.KubevirtPlatform and calls Skip with the standardized message; e.g. in the test's BeforeEach, if hostedCluster.Spec.Platform.Type == hyperv1.KubevirtPlatform { Skip("custom DNS name test is only for KubeVirt platform") }. This keeps the platform-specific skip centralized and uses the required "Test name is only for [Platform] platform" format and references the existing hostedCluster.Spec.Platform.Type and hyperv1.KubevirtPlatform symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/e2e/v2/tests/hosted_cluster_dns_test.go`:
- Line 35: The test is currently marked pending with PIt and contains
platform/version checks and unsafe dereferences; either implement the DNS
validation and change PIt to It or remove the test registration. Move the
platform/version checks out of the test body into a BeforeEach that uses the
standardized message format "Test name is only for [Platform] platform", and add
explicit nil-checks for hostedCluster.Spec (include the hostedCluster
namespace/name in diagnostic messages) before any dereference in the test
(references: PIt/It, BeforeEach, hostedCluster.Spec). Ensure the final test
contains executable DNS validation logic (or is removed) and that all pointer
dereferences are guarded with clear diagnostics.
---
Nitpick comments:
In `@test/e2e/v2/tests/hosted_cluster_dns_test.go`:
- Around line 37-47: The test dereferences hostedCluster.Spec without checking
hostedCluster for nil; update the test (around the call to tc.GetHostedCluster
and where hostedCluster.Spec is used) to explicitly assert hostedCluster != nil
and error out with a diagnostic message containing the hostedCluster
namespace/name (use hostedCluster.Namespace and hostedCluster.Name or
tc.Namespace/Name as available) before accessing hostedCluster.Spec and
hostedCluster.Spec.Services so failures produce actionable logs instead of
panics.
- Around line 41-43: Remove the inline platform check inside the spec body and
add a BeforeEach guard that checks hostedCluster.Spec.Platform.Type against
hyperv1.KubevirtPlatform and calls Skip with the standardized message; e.g. in
the test's BeforeEach, if hostedCluster.Spec.Platform.Type ==
hyperv1.KubevirtPlatform { Skip("custom DNS name test is only for KubeVirt
platform") }. This keeps the platform-specific skip centralized and uses the
required "Test name is only for [Platform] platform" format and references the
existing hostedCluster.Spec.Platform.Type and hyperv1.KubevirtPlatform symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 62b9e085-348f-4322-a3d0-e236471edebf
⛔ Files ignored due to path filters (3)
cmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.capabilities.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.services.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.validation.testsuite.yamlis excluded by!cmd/install/assets/**/*.yaml
📒 Files selected for processing (16)
control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus/hcpstatus_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.gosupport/controlplane-component/controlplane-component_test.gotest/e2e/v2/internal/env_vars.gotest/e2e/v2/internal/test_context.gotest/e2e/v2/tests/backup_restore_test.gotest/e2e/v2/tests/control_plane_infrastructure_test.gotest/e2e/v2/tests/control_plane_workloads_test.gotest/e2e/v2/tests/hosted_cluster_ccm_test.gotest/e2e/v2/tests/hosted_cluster_compliance_test.gotest/e2e/v2/tests/hosted_cluster_dns_test.gotest/e2e/v2/tests/hosted_cluster_health_test.gotest/e2e/v2/tests/hosted_cluster_image_registry_test.gotest/e2e/v2/tests/hosted_cluster_metrics_test.gotest/e2e/v2/tests/hosted_cluster_security_test.gotest/e2e/v2/util/pod_exec.go
✅ Files skipped from review due to trivial changes (2)
- test/e2e/v2/tests/hosted_cluster_ccm_test.go
- test/e2e/v2/internal/env_vars.go
🚧 Files skipped from review as they are similar to previous changes (10)
- support/controlplane-component/controlplane-component_test.go
- test/e2e/v2/util/pod_exec.go
- test/e2e/v2/tests/control_plane_infrastructure_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go
- test/e2e/v2/tests/hosted_cluster_security_test.go
- test/e2e/v2/tests/control_plane_workloads_test.go
- test/e2e/v2/internal/test_context.go
- test/e2e/v2/tests/hosted_cluster_health_test.go
- test/e2e/v2/tests/hosted_cluster_metrics_test.go
- test/e2e/v2/tests/hosted_cluster_compliance_test.go
Move EnsureNoCrashingPodsTest from hosted_cluster_health_test.go to control_plane_workloads_test.go using the per-workload pattern with individual It blocks per registered workload. Remove hosted cluster mutation from EnsureMetricsForwarderWorkingTest so it follows the non-lifecycle verification pattern: skip if the metrics forwarding annotation is not already set instead of setting it. Assisted-by: Claude:claude-opus-4-6
…attern Move EnsureCustomLabelsTest and EnsureCustomTolerationsTest from hosted_cluster_compliance_test.go to control_plane_workloads_test.go using the per-workload pattern with individual It blocks per registered workload, matching the reviewer-requested convention. Assisted-by: Claude:claude-opus-4-6
- Wrap final Prometheus metrics query in Eventually to prevent CI flakes from scrape timing races - Port certificate rotation and leader election exception paths from v1 NoCrashingPods to prevent false test failures - Add error type discrimination to network policy egress test so it catches exec/binary failures rather than silently passing - Wrap OperatorHub update in Eventually to match the Network update retry pattern in the same test - Add private-router egress check to network policy test for parity with v1 coverage - Replace manual toleration loop with Gomega ContainElement matcher - Fix context.Background() usage in getWorkloadPods helper - Change t.Errorf to t.Fatalf in webhook assertion default branches - Extract expectMetricHasLabel helper to deduplicate metric label checks - Add ClusterVersion condition propagation tests covering CVO mapping, nil Upgradeable defaulting, and empty Reason fallback - Remove redundant what-comments in infrastructure test - Fix E2E_SERVICE_DOMAIN env var description accuracy Assisted-by: Claude:claude-opus-4-6
… helpers Use ValidateHostedCluster/ValidateHostedClusterClient helpers instead of inline GetHostedCluster + Expect(hc).NotTo(BeNil()) and GetHostedClusterClient + Expect(guestClient).NotTo(BeNil()) patterns across all four PR openshift#8527 lifecycle test files. Also convert bare defer cleanup blocks to DeferCleanup with apierrors.IsNotFound guards in nodepool_lifecycle_test.go to match the established cleanup pattern. Assisted-by: Claude:claude-opus-4-6
Capture patterns and anti-patterns identified during PR openshift#8511 review: - §13: Non-lifecycle tests must not mutate the hosted cluster - §14: Per-workload test placement in control_plane_workloads_test.go - §15: IPv6-safe URL construction via net.JoinHostPort - §16: Vacuous pass prevention by asserting non-empty lists - Gotcha: MicroShift guards don't apply to v2 e2e tests - Architecture: document util/, lifecycle/, cmd/ packages - Fix envtest version range from 1.31-1.35 to 1.30-1.35 Assisted-by: Claude:claude-opus-4-6
|
@cblecker: This pull request references GCP-688 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
needs-rebase
cblecker
force-pushed
the
worktree-smooth-hatching-thompson
branch
from
1cd2790 to
8027416
Compare
openshift-ci-robot
removed
the
verified
openshift-ci
Bot
removed
the
needs-rebase
Disable Docstrings, MicroShift, SNO, and OTE pre-merge checks that are inherited from the org but do not apply to HyperShift. Assisted-by: Claude:claude-opus-4-6
- Move workload.Name from It() titles into Context() blocks across all per-workload test loops to eliminate dynamic test names - Split EnsureAdmissionPoliciesTest into 4 separate It blocks with Ordered + BeforeAll for shared setup - Add assertion context to bare Expect(err) in payload arch test - Update AGENTS.md section 14 example to match Context() pattern Assisted-by: Claude:claude-opus-4-6
Member
Author
|
/test e2e-v2-aws e2e-v2-gke |
Member
Author
|
/verified by e2e |
Contributor
|
Scheduling tests matching the |
openshift-ci-robot
added
the
verified
|
@cblecker: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
/lgtm |
Contributor
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
Member
Author
|
/retest |
Contributor
Member
Author
|
/retest-required |
Contributor
|
@cblecker: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
6 tasks
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports remaining v1
TestCreateClustervalidations to the v2 test framework, unit tests, and envtest, enabling retirement of thee2e-gkeCI job per GCP-688.onUpdatetest cases for Services, ControllerAvailabilityPolicy, and Capabilities immutability to guard against accidental CEL marker removalWorkloadRegistryValidationTestfromLabel("Informing")(non-blocking) to blocking so pods missing theapplabel fail CIcontrol_plane_workloads_test.go, remove hosted cluster mutation from non-lifecycle tests, align lifecycle tests with branch helpersworkload.NamefromIt()titles intoContext()blocks to eliminate dynamic test names, splitEnsureAdmissionPoliciesTestinto 4Itblocks withOrdered+BeforeAll, add assertion context to payload arch testTest plan
make test-envtest-ocp— validates onUpdate immutability cases (commit 1)go test ./support/controlplane-component/... -run TestReconcile— toleration propagation (commit 2)go test ./control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus/...— hcpstatus Authentication propagation (commit 2)go test ./control-plane-operator/hostedclusterconfigoperator/controllers/resources/... -run "TestIsAllowedWebhookUrl|TestEnsureGuestAdmissionWebhooksAreValid"— webhook validation (commit 2)make e2ev2— validates v2 specs compile cleanly (commits 3-8, 11)e2e-v2-gkejob must pass with all changesSummary by CodeRabbit
Tests
Chores