CNTRLPLANE-3352: Convert GHA callers to use reusable workflows @main#8387
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@bryan-cox: This pull request references CNTRLPLANE-3352 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 task 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. |
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMultiple GitHub Actions workflows were refactored to delegate jobs to reusable workflows instead of defining jobs inline. Top-level workflow permissions for repository contents were moved to the corresponding job level. Several workflows had push triggers removed (now only pull_request and workflow_dispatch) and change-detection logic updated to prefer pull-request base/head SHAs with fallbacks. One unit test was added to verify annotation-value checking behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller Workflow (this repo)
participant GH as GitHub Actions
participant Reusable as Reusable Workflow
participant Runner as Runner
participant Repo as Repository
Caller->>GH: invoke job using "uses: reusable-workflow@main"
GH->>Reusable: start reusable workflow run (job-level permissions rgba(0,128,255,0.5))
Reusable->>Runner: allocate runner and execute job steps
Runner->>Repo: actions/checkout (read contents rgba(0,200,100,0.5))
Runner->>Runner: run build/test/lint/make commands rgba(200,100,0,0.5)
Runner->>GH: upload artifacts / set outputs / exit status
GH->>Caller: propagate results (success/failure, outputs)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-build.yaml:
- Around line 11-15: The workflow currently uses an external reusable workflow
with "uses:
openshift/hypershift/.github/workflows/docs-build-reusable.yaml@main", which
causes PRs that only edit the reusable workflow not to validate those edits;
update the caller in .github/workflows/docs-build.yaml to reference the reusable
workflow from the PR branch instead of `@main` by using a local relative path
(e.g. "uses: ./.github/workflows/docs-build-reusable.yaml") or otherwise ensure
it consumes the same ref as the PR so edits to docs-build-reusable.yaml trigger
validation.
In @.github/workflows/envtest-kube-reusable.yaml:
- Around line 31-44: The current step runs git diff and treats any non-matching
output as "should_run=false", but it does not detect git diff failures; modify
the script around the git diff invocation so that failures cause the step to
fail instead of falling through: when computing ref (using variables ref,
PUSH_DIFF_REF, PR_DIFF_REF and EVENT_NAME) run git diff --name-only "$ref" and
check its exit status—if git diff exits non-zero, write an error and exit
non-zero (fail the job) rather than proceeding to echo "should_run=false" to
GITHUB_OUTPUT; ensure the logic that writes "should_run=true"/"false" only runs
when git diff succeeds.
In @.github/workflows/test-reusable.yaml:
- Line 19: Add the custom runner label "arc-runner-set" to the actionlint
configuration so actionlint recognizes it across workflows; locate the
actionlint config (e.g., the repository-level actionlint YAML) and add
"arc-runner-set" to the runner labels list (runner-labels /
allowed-runner-labels) so the label used in workflows like the one containing
runs-on: arc-runner-set is registered and no longer flagged as unknown.
🪄 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: 506990b7-a690-4593-9a17-b3d5d06c07fd
📒 Files selected for processing (18)
.github/workflows/codespell-reusable.yaml.github/workflows/codespell.yaml.github/workflows/cpo-container-sync-reusable.yaml.github/workflows/cpo-container-sync.yaml.github/workflows/docs-build-reusable.yaml.github/workflows/docs-build.yaml.github/workflows/envtest-kube-reusable.yaml.github/workflows/envtest-kube.yaml.github/workflows/envtest-ocp-reusable.yaml.github/workflows/envtest-ocp.yaml.github/workflows/gitlint-reusable.yaml.github/workflows/gitlint.yaml.github/workflows/lint-reusable.yaml.github/workflows/lint.yaml.github/workflows/test-reusable.yaml.github/workflows/test.yaml.github/workflows/verify-reusable.yaml.github/workflows/verify.yaml
Replace workflow logic in caller files with thin stubs that reference the reusable workflow definitions via @main. This ensures PRs always run the latest workflow from main, eliminating the need for the verify-workflows Prow job and forced rebases. Caller stubs handle pull_request (and workflow_dispatch where applicable). Push triggers for main/release branches remain on the reusable workflows so direct pushes still trigger runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- envtest-kube/ocp: use explicit SHAs for ref resolution instead of github.event_name/github.base_ref which may be empty in workflow_call contexts - lint: guard base_ref fetch to avoid invalid refspec when called outside a pull_request context - verify: move git-clean validation after make staticcheck/fmt/vet so file changes from those targets are also detected Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ae79858 to
e79f5ed
Compare
The caller uses @main, so changes to the reusable workflow in a PR won't be tested by this trigger — it would run the main version regardless, which is misleading. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/area ci-tooling |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8387 +/- ##
=======================================
Coverage 36.69% 36.70%
=======================================
Files 767 767
Lines 93353 93353
=======================================
+ Hits 34260 34264 +4
+ Misses 56409 56405 -4
Partials 2684 2684 see 1 file 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:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@support/util/annotations_test.go`:
- Around line 150-171: The test case that constructs a HostedCluster with a
slash in metadata.name is invalid; remove the table entry named "When annotation
has multiple slashes..." (the AzurePrivateLinkService test using
HostedClusterAnnotation and an existing hyperv1.HostedCluster with Name:
"name/extra") and replace it with a test asserting that annotations containing
multiple slashes are treated as a format error: create an
AzurePrivateLinkService with HostedClusterAnnotation: "clusters/name/extra" and
assert the code path (call the function under test that parses
HostedClusterAnnotation) returns an error and does not resolve to a
HostedCluster (i.e., no wantHCNS/wantHCN success expectations); reference the
same symbols HostedClusterAnnotation, hyperv1.AzurePrivateLinkService, and
hyperv1.HostedCluster when locating and editing the test table.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 82faf04d-23b5-48f3-9a46-48e782cb2a54
📒 Files selected for processing (1)
support/util/annotations_test.go
Add test coverage for HasAnnotationWithValue which previously had none. Remove test case that used "/" in metadata.name which violates K8s DNS subdomain naming rules (RFC 1123). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1ddc82a to
71c61f1
Compare
|
@bryan-cox: 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. |
|
/verified by GHA tests |
|
@bryan-cox: 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. |
|
/lgtm |
|
Scheduling tests matching the |
Test Resultse2e-aws
e2e-aks
|
|
/override "ci/prow/e2e-kubevirt-aws-ovn-reduced" |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-kubevirt-aws-ovn-reduced 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. |
|
/override "ci/prow/e2e-azure-self-managed" |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-aks, ci/prow/e2e-aws, ci/prow/e2e-aws-upgrade-hypershift-operator, ci/prow/e2e-azure-self-managed 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. |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
…#78674) GHA workflows in openshift/hypershift now use reusable workflow callers that reference definitions via @main, so PRs always run the latest workflow version without needing to rebase. The verify-workflows job that enforced rebasing is no longer needed. See openshift/hypershift#8386 and openshift/hypershift#8387. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
What this PR does / why we need it:
Converts all 9 GHA workflow callers to thin stubs that reference reusable workflow definitions via
@main. This is the second of two PRs — it depends on #8386 which has already been merged.After this PR merges, every PR-triggered GHA run automatically uses the latest workflow definition from main, eliminating the need for:
verify-workflowsProw job in openshift/releaseCallers converted:
Key details:
docs-build.yamlpreservesname: Docs Buildsodocs-deploy.yaml'sworkflow_runtrigger continues workingtest.yamlusessecrets: inheritto passCODECOV_TOKENJobNametocaller-job / JobName— cosmetic only, nothing gates on theseNext step after merge: Remove
verify-workflowsProw job from openshift/release after a few days of bake time.Future release branches (4.23+): Will need push/PR trigger branch lists updated in both caller and reusable files.
Which issue(s) this PR fixes:
Fixes CNTRLPLANE-3352
Special notes for your reviewer:
@mainref to upstream is intentional — fork PRs always get the latest upstream workflow.Checklist:
🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Tests