Skip to content

[TEST ONLY] test(azure): enable Global Pull Secret test in Azure CI - #9105

Closed
mgencur wants to merge 4 commits into
openshift:mainfrom
mgencur:enable_global_pull_secret_tests_oidc
Closed

[TEST ONLY] test(azure): enable Global Pull Secret test in Azure CI #9105
mgencur wants to merge 4 commits into
openshift:mainfrom
mgencur:enable_global_pull_secret_tests_oidc

Conversation

@mgencur

@mgencur mgencur commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Related to #9073 .
Test co-locating the test with other tests.

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • Tests
    • Improved Azure end-to-end coverage for global pull secret lifecycle, including a dedicated cluster variant for global pull secret output.
    • Strengthened validation that in-place pull secret updates don’t trigger node rollouts (consistent condition checks over time).
    • Updated pull secret test setup to use built-in dummy Docker config data and smarter default node pool selection.
    • Refined Azure test matrix reporting for autoscaling and expanded external OIDC labeling to include global pull secret.

mgencur and others added 3 commits July 24, 2026 14:54
Add a dedicated TestGroup for the global-pull-secret label in the
Azure parallel test matrix. The test needs its own cluster because
it mutates kubelet pull-secret configuration on every worker node
via a DaemonSet rollout, which would interfere with other tests
running on the same HostedCluster.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e dummy data

Remove the E2E_ADDITIONAL_PULL_SECRET_FILE env var requirement from the
Global Pull Secret test. The test only verifies that changing the secret
propagates through the pipeline — it never authenticates against a real
registry — so a second hardcoded dummy with different content exercises
the same update path without requiring CI to provision an external file.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…r under test

The NodePool selection loop listed all NodePools in the namespace without
filtering by HostedCluster name. When multiple hosted clusters share the
same namespace (standard in CI), the test could pick a NodePool belonging
to a different cluster, causing the node label selector to match zero
nodes in the guest cluster API.

Also wrap the UpdatingConfig condition check in Consistently so a brief
flip to True is caught rather than missed by a one-shot read.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 2090529f-ab79-4c7e-8834-455f3d72242d

📥 Commits

Reviewing files that changed from the base of the PR and between 8e74dc2 and 695d294.

📒 Files selected for processing (1)
  • test/e2e/v2/lifecycle/azure.go

📝 Walkthrough

Walkthrough

The global pull-secret E2E test now uses inline Docker configuration payloads, selects a suitable NodePool through getDefaultNodePool, and verifies that in-place updates keep UpdatingConfig false. Azure lifecycle configuration adds a global-pull-secret cluster variant, includes it in the external OIDC label filter, and renames the autoscaling JUnit output. The obsolete environment variable registration is removed.

Possibly related PRs

Suggested reviewers: devguyio, csrwng


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
No-Weak-Crypto ❌ Error FAIL: hosted_cluster_pull_secret_test.go uses bytes.Equal to compare dockerconfigjson secret bytes, a non-constant-time comparison of secret material. Replace the direct byte comparison with subtle.ConstantTimeCompare or compare non-secret metadata instead when verifying pull-secret updates.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: enabling the Global Pull Secret test in Azure CI.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed All Ginkgo titles in the changed test file are static strings; no generated values, dates, node/namespace names, or other dynamic identifiers appear.
Test Structure And Quality ✅ Passed The new e2e test uses BeforeEach/DeferCleanup, all cluster waits are time-bounded, and the single It covers one end-to-end global pull-secret flow consistent with repo patterns.
Topology-Aware Scheduling Compatibility ✅ Passed Touched files only adjust e2e test env/matrix and pull-secret test logic; no pod anti-affinity, spread constraints, nodeSelectors, or PDB changes.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The new Global Pull Secret test uses only cluster-internal Secret/DaemonSet operations and no IPv4 literals, IPv4-only parsing, or external network calls.
Container-Privileges ✅ Passed PASS: The only changed file adjusts the Azure test matrix label filter; no privileged/hostNetwork/hostPID/allowPrivilegeEscalation settings were added.
No-Sensitive-Data-In-Logs ✅ Passed No added log statements in the PR diff; existing logs only mention benign test/cluster state, and pull-secret payloads are never emitted.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@mgencur

mgencur commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-azure-v2-self-managed

@openshift-ci
openshift-ci Bot requested review from csrwng and devguyio July 24, 2026 14:15
@openshift-ci openshift-ci Bot added area/platform/azure PR/issue for Azure (AzurePlatform) platform area/testing Indicates the PR includes changes for e2e testing labels Jul 24, 2026
@openshift-ci

openshift-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mgencur
Once this PR has been reviewed and has the lgtm label, please assign enxebre 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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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/lifecycle/azure.go`:
- Around line 362-367: Update the external-oidc lifecycle group’s ClusterFile to
the literal artifact name cluster-name-global-pull-secret so readClusterName can
resolve it, and remove the now-unused external-oidc cluster specification.
🪄 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: 61e3671d-268f-45bb-9710-705bc1a354a3

📥 Commits

Reviewing files that changed from the base of the PR and between 144cca9 and 8e74dc2.

📒 Files selected for processing (3)
  • test/e2e/v2/internal/env_vars.go
  • test/e2e/v2/lifecycle/azure.go
  • test/e2e/v2/tests/hosted_cluster_pull_secret_test.go
💤 Files with no reviewable changes (1)
  • test/e2e/v2/internal/env_vars.go

Comment thread test/e2e/v2/lifecycle/azure.go
…secret tests

Merge the global-pull-secret TestGroup into external-oidc to reduce
the number of HostedClusters in the Azure CI matrix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mgencur
mgencur force-pushed the enable_global_pull_secret_tests_oidc branch from 8e74dc2 to 695d294 Compare July 24, 2026 14:22
@mgencur

mgencur commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-azure-v2-self-managed

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.51%. Comparing base (b5f3906) to head (695d294).
⚠️ Report is 22 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9105   +/-   ##
=======================================
  Coverage   44.50%   44.51%           
=======================================
  Files         774      774           
  Lines       96980    96997   +17     
=======================================
+ Hits        43164    43179   +15     
- Misses      50828    50830    +2     
  Partials     2988     2988           

see 2 files with indirect coverage changes

Flag Coverage Δ
cmd-support 38.39% <ø> (ø)
cpo-hostedcontrolplane 47.22% <ø> (+0.02%) ⬆️
cpo-other 45.25% <ø> (ø)
hypershift-operator 54.45% <ø> (+<0.01%) ⬆️
other 32.64% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mgencur

mgencur commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-azure-v2-self-managed

2 similar comments
@mgencur

mgencur commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-azure-v2-self-managed

@mgencur

mgencur commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-azure-v2-self-managed

@openshift-ci

openshift-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@mgencur: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@mgencur

mgencur commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

This change incorporated in #9073. Closing.

@mgencur mgencur closed this Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/platform/azure PR/issue for Azure (AzurePlatform) platform area/testing Indicates the PR includes changes for e2e testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant