[WIP] refactor(ci): migrate HyperShift calico private workflow from cucushift to hypershift step registry#80907
Open
mgencur wants to merge 3 commits into
Open
[WIP] refactor(ci): migrate HyperShift calico private workflow from cucushift to hypershift step registry#80907mgencur wants to merge 3 commits into
mgencur wants to merge 3 commits into
Conversation
Port three calico conformance tests from 4.21 to the 4.22 periodics config: e2e-aws-conformance-calico, e2e-aws-conformance-calico-private, and e2e-kubevirt-metal-conformance-calico. LVM operator bumped to stable-4.22; ODF kept at stable-4.21. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ft to hypershift step registry Create new step registry components under hypershift/ for the private Calico conformance workflow, replacing cucushift-prefixed equivalents. Move TEST_SKIPS and TEST_ARGS from CI configs into the workflow itself. Update all calico ref references across hypershift workflows. Components migrated: - hypershift-calico-install, hypershift-calico-health-check (refs) - hypershift-aws-install-private (ref + chain) - hypershift-aws-metadata, hypershift-enable/disable-guest (refs) - hypershift-enable-qe-catalogsource (ref + chain) - hypershift-aws-private-provision/deprovision (chains) - hypershift-aws-conformance-calico-private (workflow) CI configs updated: 4.19-4.22 periodics now use the new workflow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
WalkthroughThe PR introduces a new ChangesHyperShift Calico Private Conformance
Sequence Diagram(s)sequenceDiagram
participant Prow as Prow Scheduler
participant Provision as hypershift-aws-private-provision chain
participant S3Config as hypershift-aws-install-private-config
participant HCPInstall as hypershift-aws-install-private
participant CalicoInstall as hypershift-calico-install
participant CalicoHC as hypershift-calico-health-check
participant EnableGuest as hypershift-enable-guest
participant QEPullSecret as hypershift-enable-qe-pull-secret
participant Conformance as openshift-conformance-suite
participant Deprovision as hypershift-aws-private-deprovision chain
Prow->>Provision: pre-steps: provision private cluster
Provision->>S3Config: create S3 OIDC bucket
Provision->>HCPInstall: hypershift install (private, OIDC S3, external-DNS)
Provision->>CalicoInstall: apply Tigera operator + CRDs + Installation CR
CalicoInstall-->>Provision: Calico installed
Provision->>CalicoHC: wait nodes/tigerastatus/clusteroperators ready
CalicoHC-->>Provision: cluster healthy
Provision->>EnableGuest: swap kubeconfig to nested_kubeconfig, write console URL
Provision->>QEPullSecret: patch HostedCluster pull secret, wait MachineDeployment rollout, verify nodes
Prow->>Conformance: test: run conformance with TEST_SKIPS
Prow->>Deprovision: post: dump state, destroy AWS resources, ipi-deprovision
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[REHEARSALNOTIFIER]
A total of 716 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
![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: 9
🧹 Nitpick comments (3)
ci-operator/step-registry/hypershift/disable-guest/hypershift-disable-guest-commands.sh (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove default xtrace from this step script.
Line 3 enables
-xby default; repository guidance for step-registry scripts prefersset -euo pipefailand enabling tracing only for active debugging.Suggested fix
-set -xeuo pipefail +set -euo pipefailAs per coding guidelines, "Use default
set -euo pipefail(without-x) in step registry command scripts; only enable-xwhen actively debugging."🤖 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 `@ci-operator/step-registry/hypershift/disable-guest/hypershift-disable-guest-commands.sh` at line 3, The hypershift-disable-guest-commands.sh script contains `set -xeuo pipefail` which enables xtrace (the `-x` flag) by default. According to repository guidelines, remove the `-x` flag from the set command to use only `set -euo pipefail`, reserving xtrace only for active debugging sessions rather than enabling it by default in step registry scripts.Source: Coding guidelines
ci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-commands.sh (1)
3-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove unconditional xtrace from step-registry command scripts.
set -xis enabled by default. Repository guidance forci-operator/step-registry/**/*-commands.shrequires defaulting toset -euo pipefailwithout-x, enabling tracing only when actively debugging.As per coding guidelines, "Use default `set -euo pipefail` (without `-x`) in step registry command scripts; only enable `-x` when actively debugging".Suggested fix
set -e set -u set -o pipefail -set -x🤖 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 `@ci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-commands.sh` around lines 3 - 7, The hypershift-enable-qe-catalogsource-commands.sh script unconditionally enables xtrace with set -x, which violates the step-registry command script guidelines. Remove the set -x line from the script, keeping only set -euo pipefail (which encompasses set -e, set -u, and set -o pipefail). The xtrace debugging should only be enabled when actively debugging, not by default in production scripts.Source: Coding guidelines
ci-operator/step-registry/hypershift/aws/metadata/hypershift-aws-metadata-commands.sh (1)
5-5: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAvoid enabling xtrace by default in step-registry scripts.
Please keep default flags to
set -euo pipefailand enable tracing only when explicitly debugging. As per coding guidelines, “Use defaultset -euo pipefail(without-x) in step registry command scripts; only enable-xwhen actively debugging.”Suggested patch
-set -x🤖 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 `@ci-operator/step-registry/hypershift/aws/metadata/hypershift-aws-metadata-commands.sh` at line 5, Remove the `-x` flag from the `set -x` command in the hypershift-aws-metadata-commands.sh script. Replace it with the standard default flags `set -euo pipefail` to disable xtrace by default, as per the coding guidelines for step-registry scripts. The `-x` flag should only be enabled when actively debugging specific issues, not in production step-registry command scripts.Source: Coding guidelines
🤖 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
`@ci-operator/step-registry/hypershift/aws/install-private/config/hypershift-aws-install-private-config-commands.sh`:
- Around line 3-5: The shell script options need to be updated to follow
fail-fast standards and avoid unconditional debug tracing. In the set commands
at lines 3-5, replace the individual `set -o nounset` and `set -o pipefail`
statements with the combined `set -euo pipefail` which includes error exit
handling (errexit) and prevents masking of AWS command failures. Remove the
unconditional `set -x` (xtrace) from line 5 since debug tracing should be opt-in
for debugging only, not enabled by default. Additionally, on line 15 where
PROW_JOB_ID is referenced, ensure it is properly quoted to prevent
word-splitting and globbing issues.
In
`@ci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-commands.sh`:
- Line 3: The shell script currently only sets `-u` option on line 3, which
fails to enable fail-fast behavior for command failures that occur before line
111. Replace the `set -u` statement with `set -euo pipefail` to properly enable
error handling, undefined variable detection, and pipe failure detection
throughout the script. This aligns with the step-registry coding guidelines that
require fail-fast shell options (without trace mode) at the beginning of command
scripts to catch failures from early CLI operations like `oc` calls.
- Line 112: The eval command used to execute the COMMAND array on line 112
causes shell re-interpretation of arguments, which means spaces and
metacharacters in environment variable values can unintentionally change the
command's behavior. Replace the eval invocation with direct array expansion
using COMMAND[@] syntax, which properly passes array elements as separate
arguments without additional shell parsing. This ensures that values containing
spaces or special characters are treated as literal strings rather than being
re-interpreted by the shell.
In
`@ci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-ref.yaml`:
- Line 38: The URL in the comment on line 38 contains a malformed host format
with a colon instead of a dot. Replace the colon in
`https://issues:redhat.com/browse/NE-1298` with a dot to create the valid format
`https://issues.redhat.com/browse/NE-1298`.
In
`@ci-operator/step-registry/hypershift/aws/metadata/hypershift-aws-metadata-commands.sh`:
- Around line 11-14: Add validation checks immediately after the vpc_id,
infra_id, and public_subnet variable assignments to ensure these metadata
lookups return valid non-empty values rather than empty strings or "None". For
each variable, check if it is empty or equals "None" and if so, exit the script
with a clear error message indicating which lookup failed. This prevents invalid
values from being written to shared files that are consumed downstream by
aws-provision-bastionhost-commands.sh, ensuring failures are caught early with
clear diagnostics rather than causing obscure errors later in the provisioning
process.
In
`@ci-operator/step-registry/hypershift/calico/health-check/hypershift-calico-health-check-commands.sh`:
- Line 14: The timeout command with the nested bash shell at line 14 does not
validate that HYPERSHIFT_NODE_COUNT is set and numeric before executing, which
causes the script to wait the full 30 minute timeout if the variable is missing
or invalid. Add validation checks before the timeout command to ensure
HYPERSHIFT_NODE_COUNT is defined and contains only numeric characters, using
parameter expansion checks or explicit arithmetic validation. If validation
fails, exit immediately with a descriptive error message rather than allowing
the script to enter the timeout loop.
In
`@ci-operator/step-registry/hypershift/calico/install/hypershift-calico-install-commands.sh`:
- Line 3: Remove the `-x` flag from the `set -xeuo pipefail` command at the
beginning of the hypershift-calico-install-commands.sh script. According to repo
guidance for step-registry command scripts, the default should be `set -euo
pipefail` without xtrace enabled, and tracing should only be enabled when
actively debugging. Change the set command to remove the `x` option while
keeping the `e`, `u`, `o pipefail` options intact.
In
`@ci-operator/step-registry/hypershift/enable-guest/hypershift-enable-guest-commands.sh`:
- Line 13: The echo command on line 13 masks failures from the oc command
substitution, potentially writing a malformed URL to hostedcluster_console.url
file. Refactor this to first execute the oc command separately and store the
result in a variable, then validate that the host variable is non-empty to catch
any oc failures, and finally construct the URL string using the validated host
before writing it to the file. This prevents malformed URLs from being silently
written when the oc command fails.
In
`@ci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-commands.sh`:
- Around line 106-109: Remove the conditional check starting at line 106 that
uses SKIP_HYPERSHIFT_PULL_SECRET_UPDATE to gate the entire catalogsource
creation step. Since this flag is documented as pull-secret-specific and not
intended to control catalogsource setup, the catalogsource creation logic should
execute independently regardless of the pull-secret skip flag status. Delete the
if statement checking SKIP_HYPERSHIFT_PULL_SECRET_UPDATE and the exit 0 call so
that the catalogsource setup proceeds unconditionally.
---
Nitpick comments:
In
`@ci-operator/step-registry/hypershift/aws/metadata/hypershift-aws-metadata-commands.sh`:
- Line 5: Remove the `-x` flag from the `set -x` command in the
hypershift-aws-metadata-commands.sh script. Replace it with the standard default
flags `set -euo pipefail` to disable xtrace by default, as per the coding
guidelines for step-registry scripts. The `-x` flag should only be enabled when
actively debugging specific issues, not in production step-registry command
scripts.
In
`@ci-operator/step-registry/hypershift/disable-guest/hypershift-disable-guest-commands.sh`:
- Line 3: The hypershift-disable-guest-commands.sh script contains `set -xeuo
pipefail` which enables xtrace (the `-x` flag) by default. According to
repository guidelines, remove the `-x` flag from the set command to use only
`set -euo pipefail`, reserving xtrace only for active debugging sessions rather
than enabling it by default in step registry scripts.
In
`@ci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-commands.sh`:
- Around line 3-7: The hypershift-enable-qe-catalogsource-commands.sh script
unconditionally enables xtrace with set -x, which violates the step-registry
command script guidelines. Remove the set -x line from the script, keeping only
set -euo pipefail (which encompasses set -e, set -u, and set -o pipefail). The
xtrace debugging should only be enabled when actively debugging, not by default
in production scripts.
🪄 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: 5f4dcf11-a705-4543-9691-c19537af62cc
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/hypershift/openshift-hypershift-release-4.22-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (61)
ci-operator/config/openshift/hypershift/openshift-hypershift-release-4.19__periodics.yamlci-operator/config/openshift/hypershift/openshift-hypershift-release-4.20__periodics.yamlci-operator/config/openshift/hypershift/openshift-hypershift-release-4.21__periodics.yamlci-operator/config/openshift/hypershift/openshift-hypershift-release-4.22__periodics.yamlci-operator/step-registry/cucushift/hypershift-extended/enable-qe/pull-secret/cucushift-hypershift-extended-enable-qe-pull-secret-commands.shci-operator/step-registry/cucushift/installer/rehearse/aws/ipi/ovn/hypershift/private/guest/calico/cucushift-installer-rehearse-aws-ipi-ovn-hypershift-private-guest-calico-workflow.yamlci-operator/step-registry/hypershift/aws/conformance-calico-private/OWNERSci-operator/step-registry/hypershift/aws/conformance-calico-private/hypershift-aws-conformance-calico-private-workflow.metadata.jsonci-operator/step-registry/hypershift/aws/conformance-calico-private/hypershift-aws-conformance-calico-private-workflow.yamlci-operator/step-registry/hypershift/aws/conformance-calico/hypershift-aws-conformance-calico-workflow.yamlci-operator/step-registry/hypershift/aws/create/hypershift-aws-create-chain.yamlci-operator/step-registry/hypershift/aws/install-private/OWNERSci-operator/step-registry/hypershift/aws/install-private/config/OWNERSci-operator/step-registry/hypershift/aws/install-private/config/hypershift-aws-install-private-config-commands.shci-operator/step-registry/hypershift/aws/install-private/config/hypershift-aws-install-private-config-ref.metadata.jsonci-operator/step-registry/hypershift/aws/install-private/config/hypershift-aws-install-private-config-ref.yamlci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-chain.metadata.jsonci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-chain.yamlci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-commands.shci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-ref.metadata.jsonci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-ref.yamlci-operator/step-registry/hypershift/aws/metadata/OWNERSci-operator/step-registry/hypershift/aws/metadata/hypershift-aws-metadata-commands.shci-operator/step-registry/hypershift/aws/metadata/hypershift-aws-metadata-ref.metadata.jsonci-operator/step-registry/hypershift/aws/metadata/hypershift-aws-metadata-ref.yamlci-operator/step-registry/hypershift/aws/private-deprovision/OWNERSci-operator/step-registry/hypershift/aws/private-deprovision/hypershift-aws-private-deprovision-chain.metadata.jsonci-operator/step-registry/hypershift/aws/private-deprovision/hypershift-aws-private-deprovision-chain.yamlci-operator/step-registry/hypershift/aws/private-provision/OWNERSci-operator/step-registry/hypershift/aws/private-provision/hypershift-aws-private-provision-chain.metadata.jsonci-operator/step-registry/hypershift/aws/private-provision/hypershift-aws-private-provision-chain.yamlci-operator/step-registry/hypershift/calico/OWNERSci-operator/step-registry/hypershift/calico/health-check/OWNERSci-operator/step-registry/hypershift/calico/health-check/hypershift-calico-health-check-commands.shci-operator/step-registry/hypershift/calico/health-check/hypershift-calico-health-check-ref.metadata.jsonci-operator/step-registry/hypershift/calico/health-check/hypershift-calico-health-check-ref.yamlci-operator/step-registry/hypershift/calico/install/OWNERSci-operator/step-registry/hypershift/calico/install/hypershift-calico-install-commands.shci-operator/step-registry/hypershift/calico/install/hypershift-calico-install-ref.metadata.jsonci-operator/step-registry/hypershift/calico/install/hypershift-calico-install-ref.yamlci-operator/step-registry/hypershift/disable-guest/OWNERSci-operator/step-registry/hypershift/disable-guest/hypershift-disable-guest-commands.shci-operator/step-registry/hypershift/disable-guest/hypershift-disable-guest-ref.metadata.jsonci-operator/step-registry/hypershift/disable-guest/hypershift-disable-guest-ref.yamlci-operator/step-registry/hypershift/enable-guest/OWNERSci-operator/step-registry/hypershift/enable-guest/hypershift-enable-guest-commands.shci-operator/step-registry/hypershift/enable-guest/hypershift-enable-guest-ref.metadata.jsonci-operator/step-registry/hypershift/enable-guest/hypershift-enable-guest-ref.yamlci-operator/step-registry/hypershift/enable-qe/OWNERSci-operator/step-registry/hypershift/enable-qe/catalogsource/OWNERSci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-chain.metadata.jsonci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-chain.yamlci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-commands.shci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-ref.metadata.jsonci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-ref.yamlci-operator/step-registry/hypershift/enable-qe/pull-secret/OWNERSci-operator/step-registry/hypershift/enable-qe/pull-secret/hypershift-enable-qe-pull-secret-commands.shci-operator/step-registry/hypershift/enable-qe/pull-secret/hypershift-enable-qe-pull-secret-ref.metadata.jsonci-operator/step-registry/hypershift/enable-qe/pull-secret/hypershift-enable-qe-pull-secret-ref.yamlci-operator/step-registry/hypershift/kubevirt/baremetalds/conformance-calico/hypershift-kubevirt-baremetalds-conformance-calico-workflow.yamlci-operator/step-registry/hypershift/mce/agent/metal3/create/calico/hypershift-mce-agent-metal3-create-calico-chain.yaml
| @@ -0,0 +1,90 @@ | |||
| #!/bin/bash | |||
|
|
|||
| set -xeuo pipefail | |||
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Disable xtrace by default in this step script.
set -x is enabled globally, but repo guidance for step-registry command scripts is to default to set -euo pipefail and enable tracing only when actively debugging.
Suggested change
-set -xeuo pipefail
+set -euo pipefailAs per coding guidelines, ci-operator/step-registry/**/*-commands.sh should “Use default set -euo pipefail (without -x) … only enable -x when actively debugging.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| set -xeuo pipefail | |
| set -euo pipefail |
🤖 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
`@ci-operator/step-registry/hypershift/calico/install/hypershift-calico-install-commands.sh`
at line 3, Remove the `-x` flag from the `set -xeuo pipefail` command at the
beginning of the hypershift-calico-install-commands.sh script. According to repo
guidance for step-registry command scripts, the default should be `set -euo
pipefail` without xtrace enabled, and tracing should only be enabled when
actively debugging. Change the set command to remove the `x` option while
keeping the `e`, `u`, `o pipefail` options intact.
Source: Coding guidelines
Comment on lines
+106
to
+109
| if [[ $SKIP_HYPERSHIFT_PULL_SECRET_UPDATE == "true" ]]; then | ||
| echo "SKIP ....." | ||
| exit 0 | ||
| fi |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not gate catalogsource creation on the pull-secret skip flag.
Line 106 uses SKIP_HYPERSHIFT_PULL_SECRET_UPDATE to exit this step entirely, but that flag is documented as pull-secret-specific. This can unintentionally skip QE catalogsource setup and break downstream conformance behavior.
Suggested fix
-if [[ $SKIP_HYPERSHIFT_PULL_SECRET_UPDATE == "true" ]]; then
- echo "SKIP ....."
- exit 0
-fi
+# This step manages QE catalogsource only; do not reuse pull-secret skip gating here.🤖 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
`@ci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-commands.sh`
around lines 106 - 109, Remove the conditional check starting at line 106 that
uses SKIP_HYPERSHIFT_PULL_SECRET_UPDATE to gate the entire catalogsource
creation step. Since this flag is documented as pull-secret-specific and not
intended to control catalogsource setup, the catalogsource creation logic should
execute independently regardless of the pull-secret skip flag status. Delete the
if statement checking SKIP_HYPERSHIFT_PULL_SECRET_UPDATE and the exit 0 call so
that the catalogsource setup proceeds unconditionally.
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mgencur The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
mgencur
force-pushed
the
calico_4.22_migrate_steps
branch
from
1db0e91 to
864717a
Compare
Contributor
|
@mgencur: |
Contributor
Author
|
/pj-rehearse periodic-ci-openshift-hypershift-release-4.22-periodics-e2e-aws-conformance-calico-private |
Contributor
|
@mgencur: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
|
@mgencur: |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-commands.sh (1)
94-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid unconditional xtrace in this step script.
set -xis enabled for normal execution; prefer default non-tracing mode and gate tracing behind an explicit debug flag.As per coding guidelines, step-registry command scripts should default to
set -euo pipefail(without-x) and only enable-xwhen actively debugging.Suggested change
-# Hypershift install -set -x +# Hypershift install +if [[ "${HYPERSHIFT_DEBUG_TRACE:-false}" == "true" ]]; then + set -x +fi "${COMMAND[@]}"🤖 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 `@ci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-commands.sh` around lines 94 - 95, Remove the unconditional `set -x` command from the hypershift-aws-install-private-commands.sh script. Instead, ensure the script uses the default flags `set -euo pipefail` without the `-x` flag for normal execution. If tracing is needed for debugging, gate the `set -x` behind an explicit debug flag or environment variable check so that xtrace is only enabled when actively debugging, not during normal execution.Source: Coding guidelines
🤖 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.
Nitpick comments:
In
`@ci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-commands.sh`:
- Around line 94-95: Remove the unconditional `set -x` command from the
hypershift-aws-install-private-commands.sh script. Instead, ensure the script
uses the default flags `set -euo pipefail` without the `-x` flag for normal
execution. If tracing is needed for debugging, gate the `set -x` behind an
explicit debug flag or environment variable check so that xtrace is only enabled
when actively debugging, not during normal execution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 74b628f6-25c3-42cb-8f47-ed3674b05848
📒 Files selected for processing (8)
ci-operator/step-registry/hypershift/aws/install-private/config/hypershift-aws-install-private-config-commands.shci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-commands.shci-operator/step-registry/hypershift/aws/install-private/hypershift-aws-install-private-ref.yamlci-operator/step-registry/hypershift/aws/metadata/hypershift-aws-metadata-commands.shci-operator/step-registry/hypershift/calico/health-check/hypershift-calico-health-check-commands.shci-operator/step-registry/hypershift/disable-guest/hypershift-disable-guest-commands.shci-operator/step-registry/hypershift/enable-guest/hypershift-enable-guest-commands.shci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-chain.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- ci-operator/step-registry/hypershift/disable-guest/hypershift-disable-guest-commands.sh
- ci-operator/step-registry/hypershift/enable-qe/catalogsource/hypershift-enable-qe-catalogsource-chain.yaml
- ci-operator/step-registry/hypershift/calico/health-check/hypershift-calico-health-check-commands.sh
- ci-operator/step-registry/hypershift/aws/metadata/hypershift-aws-metadata-commands.sh
- ci-operator/step-registry/hypershift/aws/install-private/config/hypershift-aws-install-private-config-commands.sh
Contributor
|
@mgencur: 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. |
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
1 participant
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.
Related slack discussion: https://redhat-internal.slack.com/archives/G01QS0P2F6W/p1782211558060659?thread_ts=1782207388.083069&cid=G01QS0P2F6W
Summary by CodeRabbit
This PR refactors the HyperShift Calico conformance testing infrastructure in OpenShift CI by migrating from cucushift-based step registry components to a new hypershift-native component hierarchy. The migration improves code organization and consolidates duplicate configuration across multiple OpenShift versions (4.19-4.22).
Core Changes
Workflow Migration for Calico Conformance Testing:
e2e-aws-conformance-calico-privateacross OpenShift versions 4.19-4.22 is updated to use the newhypershift-aws-conformance-calico-privateworkflow instead of the previouscucushift-installer-rehearse-aws-ipi-ovn-hypershift-private-guest-calicoworkflowTEST_ARGS,TEST_SKIPS) that were previously specified inline in individual CI job configs are now consolidated into the workflow definitions themselves, eliminating redundant specificationse2e-aws-conformance-calico,e2e-aws-conformance-calico-private, ande2e-kubevirt-metal-conformance-calicoNew HyperShift Step Registry Components:
The PR introduces a complete set of new step registry components organized under the
hypershift/directory:Calico CNI Support (
hypershift/calico/):install: Downloads and applies the Tigera Calico operator, configures AWS credentials when needed, and performs the full Calico installationhealth-check: Validates Calico deployment health by checking tigerastatus components, CRD availability, cluster operators readiness, and node readinessAWS Private Cluster Infrastructure (
hypershift/aws/):install-private: Creates an S3 bucket for OIDC documents and installs the HyperShift operator with platform-specific configurationsmetadata: Retrieves VPC ID and public subnet information from the hosted cluster and persists them for downstream stepsprivate-provision: Orchestrates the complete provisioning chain for private hosted clusters including installer setup, operator installation, cluster creation, metadata discovery, bastion provisioning, and proxy configurationprivate-deprovision: Handles cleanup and deprovisioning of private cluster resources with proper dependency orderingGuest Cluster Management (
hypershift/enable-guest/,hypershift/disable-guest/):enable-guest: Switches from management cluster kubeconfig to guest cluster kubeconfig and extracts the OpenShift console URLdisable-guest: Reverts to management cluster kubeconfig for post-test operationsQE Enablement (
hypershift/enable-qe/):pull-secret: Performs day-2 pull-secret updates with verification that MachineDeployment rollouts complete successfully before testingcatalogsource: Installs QE catalog sources in the OpenShift Marketplace for operator testingTechnical Improvements
ciliumandcalicoas CNI providers (previously cilium-only), enabling the load balancer health probe annotation for bothCNI_PROVIDER: "calico"for consistencyImpact
This refactoring centralizes test configuration management, reduces CI configuration duplication across OpenShift versions, and establishes a cleaner component-based architecture for HyperShift Calico conformance testing that can be more easily maintained and extended in the future.