Skip to content

Conversation

@mgahagan73
Copy link
Collaborator

@mgahagan73 mgahagan73 commented Dec 17, 2025

Test to confirm HCP cluster can accept and use user provided pull secrets

ARO-20543

What

create cluster then use kubernetes API to update the global pull secret, verify pull secret was updated. Confirm pull secret is usable by verifying it can download a Red Hat Operator.
replaces: #2502

Why

Test confirms functionality of key milestone deliverable

@openshift-ci openshift-ci bot requested review from bennerv and deads2k December 17, 2025 15:22
@openshift-ci
Copy link

openshift-ci bot commented Dec 17, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mgahagan73
Once this PR has been reviewed and has the lgtm label, please assign bennerv for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mgahagan73
Copy link
Collaborator Author

/test e2e-parallel

1 similar comment
@mgahagan73
Copy link
Collaborator Author

/test e2e-parallel

@mgahagan73
Copy link
Collaborator Author

/pj-rehearse ack

Copy link
Member

@bennerv bennerv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Steve's comments about testing functionality that's not local to our team. Testing an operator from operatorhub and testing the HyperShift global pull secret functionality when it doesn't flow through our service (frontend, backend, cs, etc.) seems like duplicated effort in which the HyperShift team (or any other OCP team) may already be performing this test.

I think it would make sense to eventually convert this into something such that we hand the hypershift e2e suite a kubeconfig of the data plane and ensure it passes from their perspective.

That being said, I think a few cleanup / simplification things can be done here and we can have this as a stop-gap and work toward potentially integrating other team's test suites against our data plane (such as hypershift) should they exist to ensure we're compliant.


By("waiting for HCCO to merge the additional pull secret with the global pull secret")
Eventually(func() error {
return verifiers.VerifyPullSecretMergedIntoGlobal(testPullSecretHost).Verify(ctx, adminRESTConfig)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be good to track status changes between iteration of the verifier.

Is there also any other use case of this verifier outside of this test? Why not inline it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update as i'm reviewing this:
I can see the reuse within the test itself. I'm fine leaving it in verifiers, but most likely no one else will use it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is any future test that needs to add pull secrets can use the verifier to make things easier.

Comment on lines +270 to +275
"spec": map[string]interface{}{
"channel": "stable",
"name": "nfd",
"source": "redhat-operators",
"sourceNamespace": "openshift-marketplace",
"installPlanApproval": "Automatic",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're relying on an external source and that it never gets removed. Is there a better image to just create a pod and ensure the image pulled successfully rather than leveraging the marketplace images?

return verifiers.VerifyOperatorInstalled(nfdNamespace, "nfd").Verify(ctx, adminRESTConfig)
}, 300*time.Second, 15*time.Second).Should(Succeed(), "NFD operator should be installed successfully")

By("creating NodeFeatureDiscovery CR to deploy NFD worker")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like we're testing NFD, which doesn't seem relevant to the "global pull secret" test itself.

@mgahagan73
Copy link
Collaborator Author

/test e2e-parallel

@mgahagan73
Copy link
Collaborator Author

/test e2e-parallel

1 similar comment
@mgahagan73
Copy link
Collaborator Author

/test e2e-parallel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants