GCP-413: add image registry v2 e2e tests for hosted clusters#8412
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Skipping CI for Draft Pull Request. |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
@cblecker: This pull request references GCP-413 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
do-not-merge/needs-area
area/control-plane-operator
|
[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
📝 WalkthroughWalkthroughRegisters the OpenShift imageregistry v1 API group in the runtime scheme by importing imageregistryv1 and calling AddToScheme. Adds an e2e test suite for HostedCluster ImageRegistry with ordered scenarios for enabled and disabled capability states. Enabled tests wait for the image-registry ClusterOperator conditions, verify installer-cloud-credentials secret, assert the imageregistry Config has storage backends, and check the cluster-image-registry-operator Deployment; on GCP they also validate GCS bucket config and WIF service-account credentials. Disabled tests assert operator/namespace absence and empty default ImagePullSecrets. Sequence Diagram(s)sequenceDiagram
participant Test as ImageRegistry Test
participant HC as HostedCluster API
participant CO as ClusterOperator(image-registry)
participant Secret as Secret(installer-cloud-credentials)
participant Config as imageregistry Config
participant Deploy as Deployment(cluster-image-registry-operator)
participant GCS as GCS (GCP only)
Test->>HC: Read HostedCluster capabilities
alt Capability enabled
Test->>CO: Wait for Available && not Degraded
Test->>Secret: Check exists and has data
Test->>Config: Read imageregistry Config -> verify >=1 storage backend
Test->>Deploy: Check Deployment has >=1 ready replica
alt Platform is GCP
Test->>Config: Verify storage uses GCS with bucket
Test->>Secret: Read service_account.json
Test->>GCS: Validate external_account creds and impersonation URL
end
else Capability disabled
Test->>CO: Expect ClusterOperator not found
Test->>HC: Expect openshift-image-registry namespace not found
Test->>HC: Verify default ServiceAccount ImagePullSecrets empty in existing and new namespaces
end
🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/test e2e-v2-aws e2e-v2-gke |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8412 +/- ##
=======================================
Coverage 37.22% 37.23%
=======================================
Files 750 752 +2
Lines 91789 91830 +41
=======================================
+ Hits 34172 34196 +24
- Misses 54978 54993 +15
- Partials 2639 2641 +2
... and 6 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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e/v2/tests/hosted_cluster_image_registry_test.go (1)
126-133: ⚡ Quick winPrefer eventual readiness check for operator deployment.
A single-read
ReadyReplicas >= 1assertion can be timing-sensitive in e2e runs. Polling reduces transient failures.Suggested refactor
It("should have a ready cluster-image-registry-operator deployment", func() { - deployment := &appsv1.Deployment{} - Expect(tc.MgmtClient.Get(tc.Context, crclient.ObjectKey{ - Namespace: tc.ControlPlaneNamespace, - Name: "cluster-image-registry-operator", - }, deployment)).To(Succeed()) - Expect(deployment.Status.ReadyReplicas).To(BeNumerically(">=", 1), - "cluster-image-registry-operator deployment should have at least one ready replica") + Eventually(func(g Gomega) { + deployment := &appsv1.Deployment{} + g.Expect(tc.MgmtClient.Get(tc.Context, crclient.ObjectKey{ + Namespace: tc.ControlPlaneNamespace, + Name: "cluster-image-registry-operator", + }, deployment)).To(Succeed()) + g.Expect(deployment.Status.ReadyReplicas).To(BeNumerically(">=", 1), + "cluster-image-registry-operator deployment should have at least one ready replica") + }).WithTimeout(5 * time.Minute).WithPolling(15 * time.Second).Should(Succeed()) })🤖 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_image_registry_test.go` around lines 126 - 133, The test currently asserts deployment.Status.ReadyReplicas >= 1 once which is flaky; replace the single check with a polling assertion that retries until the operator is ready. Use Gomega's Eventually to repeatedly call tc.MgmtClient.Get for the deployment (the existing deployment variable and tc.Context/ tc.MgmtClient.Get) and check deployment.Status.ReadyReplicas >= 1 within a reasonable timeout and interval, ensuring the Get errors are handled inside the polled function so transient API delays are retried until ready.
🤖 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_image_registry_test.go`:
- Around line 92-95: The test currently only checks degraded.Status when the
Degraded condition exists, allowing a missing Degraded condition to pass; update
the assertion to first require the Degraded condition to exist (e.g., use
g.Expect(degraded).NotTo(BeNil()) or equivalent) and then assert that
degraded.Status is not configv1.ConditionTrue using the existing
g.Expect(...).NotTo(Equal(...)) check so the test fails if the Degraded
condition is absent or set to True.
---
Nitpick comments:
In `@test/e2e/v2/tests/hosted_cluster_image_registry_test.go`:
- Around line 126-133: The test currently asserts
deployment.Status.ReadyReplicas >= 1 once which is flaky; replace the single
check with a polling assertion that retries until the operator is ready. Use
Gomega's Eventually to repeatedly call tc.MgmtClient.Get for the deployment (the
existing deployment variable and tc.Context/ tc.MgmtClient.Get) and check
deployment.Status.ReadyReplicas >= 1 within a reasonable timeout and interval,
ensuring the Get errors are handled inside the polled function so transient API
delays are retried until ready.
🪄 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: f4ba3d63-4255-4380-95ef-310ce280cbce
📒 Files selected for processing (2)
support/api/scheme.gotest/e2e/v2/tests/hosted_cluster_image_registry_test.go
cblecker
force-pushed
the
worktree-dapper-munching-fiddle
branch
from
503ad9e to
7d20309
Compare
|
/test e2e-v2-aws e2e-v2-gke |
Add platform-generic image registry e2e tests to the v2 framework, validating ClusterOperator health, installer-cloud-credentials, storage configuration, and operator deployment readiness. Includes GCP-specific sub-context for WIF credentials and GCS bucket validation, and ports the disabled-capability test from v1. Register imageregistryv1 scheme in support/api to enable querying imageregistryv1.Config objects via the hosted cluster client. Assisted-by: Claude:claude-sonnet-4-6[1m]
cblecker
force-pushed
the
worktree-dapper-munching-fiddle
branch
from
f86c7ca to
baaf52e
Compare
|
/test e2e-v2-aws e2e-v2-gke |
|
/test e2e-v2-gke |
|
/verified by e2e |
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. |
|
@cblecker: This pull request references GCP-413 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. |
|
/lgtm |
|
Scheduling tests matching the |
|
/retest-required |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
/retest-required |
|
/retest |
1 similar comment
|
/retest |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryAll 5 test failures are unrelated to PR #8412 (which only adds Root CauseFailure Group 1 — AWS API Throttling (3 tests): Failure Group 2 — Karpenter Teardown Timeout (2 tests):
The teardown timeout is likely related to the same AWS API throttling affecting the account — resource deletion API calls were also being rate-limited, causing the context deadline to expire before all 9 resources could be cleaned up. PR #8412 Relevance: The PR changes only Recommendations
Evidence
|
|
/override ci/prow/e2e-aws |
|
@cblecker: Overrode contexts on behalf of cblecker: ci/prow/e2e-aws 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 kubernetes-sigs/prow repository. |
|
@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. |
3 participants
Summary
installer-cloud-credentialssecret, storage configuration, andcluster-image-registry-operatordeployment readinessinstaller-cloud-credentialssecretEnsureImageRegistryCapabilityDisabled) asImageRegistryCapabilityDisabledTestimageregistryv1insupport/api/scheme.goso the hosted cluster client can queryimageregistryv1.ConfigobjectsTest plan
go vet -tags e2ev2 ./test/e2e/v2/...passesmake lintpassesImageRegistrycapability enabled and verify all specs passImageRegistrycapability disabled and verify the disabled-path specs pass and enabled-path specs skipSummary by CodeRabbit
New Features
Tests