Skip to content

Add eco-gotests suites ibu#81178

Open
shaior wants to merge 1 commit into
openshift:mainfrom
shaior:ibu_tests
Open

Add eco-gotests suites ibu#81178
shaior wants to merge 1 commit into
openshift:mainfrom
shaior:ibu_tests

Conversation

@shaior

@shaior shaior commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

This PR updates OpenShift CI step-registry configuration for the telcov10n CNF RAN IBU eco-gotests flow by adding a dedicated target-side upgrade path and tightening how seed/target inventories, kubeconfigs, and JUnit artifacts are prepared and collected.

Practically, it:

  • Adds a new CI step telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade (with OWNERS + *.ref.yaml and *.ref.metadata.json) that:
    • Exits early when ${SHARED_DIR}/skip.txt exists
    • Copies target hub inventories from SHARED_DIR into the OCP deployment and CNF inventories using target-* source filenames (including target-master0host_vars/master-0.yaml)
    • Runs ibu-prepare-seed-sno.yml to retrieve the target spoke kubeconfig
    • Runs deploy-run-eco-gotests.yaml with ECO_GOTESTS_FEATURES (default upgrade) and sets ECO_LCA_IBU_CNF_KUBECONFIG_TARGET_SNO to the target hub kubeconfig path on the target bastion
    • SSH-tunnels to the target bastion to execute eco-gotests-run.sh, then copies back junit_*.xml reports into ${ARTIFACT_DIR} (and onward into SHARED_DIR with junit_ibu_upgrade_*)
  • Wires the new upgrade step into the telcov10n-functional-cnf-ran-ibu workflow test section, running it after the seed image is mirrored to the target disconnected registry.
  • Updates the existing seed eco-gotests command script to copy inventory from SHARED_DIR using flat inventory filenames (non-seed-*-prefixed) into:
    • OCP deployment group_vars/ (all, bastions, hypervisors, nodes, masters) and host_vars/ (bastion, hypervisor, master0)
    • CNF inventory group_vars/*.yaml and host_vars/*.yaml
    • Then continues to run eco-gotests via an SSH tunnel to the bastion and persists JUnit XMLs to ${SHARED_DIR} as junit_ibu_seed_*.
  • Extends step documentation/defaults for both seed and upgrade flows (including VERSION defaulting to 4.20, and seed hub CLUSTER_NAME defaulting to kni-qe-108).
  • Adds a VERSION parameter (default 4.20) to telcov10n-functional-cnf-ran-ibu-mirror-seed-image so seed image tagging stays consistent.
  • Updates the openshift-kni-eco-ci-cd-main__cnf-ran-ibu-4.20.yaml configuration to set REPORTER_TEMPLATE_NAME for cnf-ran-ztp-tests.

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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: fa01c196-7bcc-4adf-982f-53a085858a24

📥 Commits

Reviewing files that changed from the base of the PR and between 162be5f and 7d0475e.

📒 Files selected for processing (1)
  • ci-operator/config/openshift-kni/eco-ci-cd/openshift-kni-eco-ci-cd-main__cnf-ran-ibu-4.20.yaml

Walkthrough

Adds a new IBU eco-gotests upgrade step, wires it into the CNF RAN IBU workflow, and updates the seed step inventory, execution flow, and artifact collection.

Changes

IBU eco-gotests flow

Layer / File(s) Summary
Seed inputs and inventory layout
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh, ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-ref.yaml
Updates the seed step documentation, adds CLUSTER_NAME and VERSION, and changes inventory copying to use non-seed-* sources.
Upgrade step registration
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/OWNERS, ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-ref.metadata.json, ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-ref.yaml, ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu/telcov10n-functional-cnf-ran-ibu-workflow.yaml
Adds the new upgrade step definition, metadata, approvers, timeout and env vars, and appends the step to the IBU workflow.
Upgrade command flow
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh
Implements the upgrade script that prepares inventories, runs ibu-prepare-seed-sno.yml, builds eco-gotests environment variables, and executes deploy-run-eco-gotests.yaml.
Bastion execution and artifacts
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh, ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh
Adds bastion SSH setup, remote eco-gotests execution, junit XML collection, and cleanup of the temporary SSH key.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

rehearsals-ack


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-Sensitive-Data-In-Logs ❌ Error The new upgrade script logs TARGET_CLUSTER_NAME, TARGET_SPOKE_CLUSTER, and MIRROR_REGISTRY, exposing internal hostnames/cluster IDs in CI output. Remove or redact these echo statements, or log only non-sensitive status messages instead of raw inventory/registry values.
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: adding IBU eco-gotests suites and related step-registry updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 No Ginkgo It/Describe/Context/When titles were added; the PR only changes CI step-registry YAML/bash scripts, so there are no test names to destabilize.
Test Structure And Quality ✅ Passed No Ginkgo test code was changed; the PR only updates ci-operator step-registry shell scripts and YAML configs.
Microshift Test Compatibility ✅ Passed The PR only adds CI step-registry bash/YAML assets; no new Ginkgo test bodies or MicroShift-unsafe APIs/features were introduced.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo test bodies were added; the PR only wires CI step-registry scripts/YAML around existing eco-gotests suites, and the docs target SNO clusters.
Topology-Aware Scheduling Compatibility ✅ Passed The PR only changes CI step-registry scripts/YAML; no deployment manifests, controllers, or scheduling constraints were added.
Ote Binary Stdout Contract ✅ Passed PR only changes ci-operator step-registry YAML/bash scripts; no Go OTE binaries or process-level stdout paths (main/TestMain/RunSpecs/etc.) were introduced.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The PR only adds CI step-registry shell/YAML; no new Ginkgo tests or public-internet dependencies were introduced.
No-Weak-Crypto ✅ Passed No MD5/SHA1/DES/RC4/3DES/Blowfish/ECB or custom secret comparisons appear in the changed files; changes are CI inventory/SSH flow only.
Container-Privileges ✅ Passed No touched manifest adds privileged:true, hostPID/hostNetwork/hostIPC, SYS_ADMIN, allowPrivilegeEscalation, or root securityContext; the only capabilities field is a non-security CI capability.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 28, 2026
@openshift-ci openshift-ci Bot requested review from dgoodwin and xueqzhan June 28, 2026 10:29
@shaior

shaior commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibu-4.20-cnf-ran-ztp-tests

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@shaior: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@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: 2

🧹 Nitpick comments (1)
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh (1)

2-3: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Enable nounset for this step script.

Several required inputs are expanded later (TARGET_CLUSTER_NAME, TARGET_SPOKE_CLUSTER, MIRROR_REGISTRY, VERSION), so missing env contracts currently degrade into empty strings instead of failing fast. As per coding guidelines, "Default to set -euo pipefail (without -x) in step registry scripts."

Proposed fix
-#!/bin/bash
-set -e
-set -o pipefail
+#!/bin/bash
+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/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh`
around lines 2 - 3, The step script currently enables only errexit and pipefail,
so missing required environment variables can expand to empty strings instead of
failing fast. Update the script to use nounset as well, following the
step-registry shell convention, and make sure the change is applied at the top
of this script alongside the existing set flags so later uses of
TARGET_CLUSTER_NAME, TARGET_SPOKE_CLUSTER, MIRROR_REGISTRY, and VERSION are
caught immediately.

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/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh`:
- Line 58: The upgrade command still references the old playbook name, so update
the `telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh` script to
use `playbooks/ran/prepare-ibu-seed-sno.yml` like the seed step. Fix the
`ansible-playbook` invocation in the upgrade registry entry so both
`telcov10n-functional-cnf-ran-ibu-eco-gotests-seed` and
`telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade` point to the renamed
playbook.

In
`@ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh`:
- Around line 56-63: Update the IBU upgrade command script to call the renamed
prepare playbook instead of the old path. In the step that invokes
ansible-playbook, replace the reference used for the initial SNO preparation so
it matches the sibling seed flow and the renamed playbook path, keeping the same
inventory and extra-vars handling in the existing upgrade step.

---

Nitpick comments:
In
`@ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh`:
- Around line 2-3: The step script currently enables only errexit and pipefail,
so missing required environment variables can expand to empty strings instead of
failing fast. Update the script to use nounset as well, following the
step-registry shell convention, and make sure the change is applied at the top
of this script alongside the existing set flags so later uses of
TARGET_CLUSTER_NAME, TARGET_SPOKE_CLUSTER, MIRROR_REGISTRY, and VERSION are
caught immediately.
🪄 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: 9ff7bd17-c03f-401d-aa9e-4f5902e19ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 3044539 and a0bd9f6.

📒 Files selected for processing (6)
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/OWNERS
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-ref.metadata.json
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-ref.yaml
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu/telcov10n-functional-cnf-ran-ibu-workflow.yaml


cd /eco-ci-cd
ansible-playbook playbooks/ran/ibu-prepare-seed-sno.yml \
ansible-playbook playbooks/ran/prepare-ibu-seed-sno.yml \

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Update the playbook rename in the upgrade step as well.

This step now calls playbooks/ran/prepare-ibu-seed-sno.yml, but ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh still calls playbooks/ran/ibu-prepare-seed-sno.yml. If the playbook was renamed rather than duplicated, the upgrade suite will fail with a missing file.

🤖 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/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh`
at line 58, The upgrade command still references the old playbook name, so
update the `telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh`
script to use `playbooks/ran/prepare-ibu-seed-sno.yml` like the seed step. Fix
the `ansible-playbook` invocation in the upgrade registry entry so both
`telcov10n-functional-cnf-ran-ibu-eco-gotests-seed` and
`telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade` point to the renamed
playbook.

Comment on lines +56 to +63
echo "=== Step 1: Prepare IBU target SNO and retrieve kubeconfig ==="

cd /eco-ci-cd
ansible-playbook playbooks/ran/ibu-prepare-seed-sno.yml \
-i "${OCP_DEPLOYMENT_INVENTORY_PATH}/build-inventory.py" \
--extra-vars "hub_cluster=${TARGET_CLUSTER_NAME}" \
--extra-vars "spoke_cluster=${TARGET_SPOKE_CLUSTER}" \
--extra-vars "seed_vm_name=${TARGET_VM_NAME}"

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use the renamed prepare playbook here.

This new step still calls playbooks/ran/ibu-prepare-seed-sno.yml, but the sibling seed flow and the PR context both reference playbooks/ran/prepare-ibu-seed-sno.yml. Leaving the old path here will make the upgrade step fail once the pre-rename filename is gone.

Proposed fix
-ansible-playbook playbooks/ran/ibu-prepare-seed-sno.yml \
+ansible-playbook playbooks/ran/prepare-ibu-seed-sno.yml \
📝 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
echo "=== Step 1: Prepare IBU target SNO and retrieve kubeconfig ==="
cd /eco-ci-cd
ansible-playbook playbooks/ran/ibu-prepare-seed-sno.yml \
-i "${OCP_DEPLOYMENT_INVENTORY_PATH}/build-inventory.py" \
--extra-vars "hub_cluster=${TARGET_CLUSTER_NAME}" \
--extra-vars "spoke_cluster=${TARGET_SPOKE_CLUSTER}" \
--extra-vars "seed_vm_name=${TARGET_VM_NAME}"
echo "=== Step 1: Prepare IBU target SNO and retrieve kubeconfig ==="
cd /eco-ci-cd
ansible-playbook playbooks/ran/prepare-ibu-seed-sno.yml \
-i "${OCP_DEPLOYMENT_INVENTORY_PATH}/build-inventory.py" \
--extra-vars "hub_cluster=${TARGET_CLUSTER_NAME}" \
--extra-vars "spoke_cluster=${TARGET_SPOKE_CLUSTER}" \
--extra-vars "seed_vm_name=${TARGET_VM_NAME}"
🤖 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/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh`
around lines 56 - 63, Update the IBU upgrade command script to call the renamed
prepare playbook instead of the old path. In the step that invokes
ansible-playbook, replace the reference used for the initial SNO preparation so
it matches the sibling seed flow and the renamed playbook path, keeping the same
inventory and extra-vars handling in the existing upgrade step.

@shaior

shaior commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibu-4.20-cnf-ran-ztp-tests

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@shaior: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@shaior

shaior commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibu-4.20-cnf-ran-ztp-tests

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@shaior: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@shaior

shaior commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibu-4.20-cnf-ran-ztp-tests

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@shaior: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@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: 2

🧹 Nitpick comments (1)
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh (1)

2-3: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Use the step-registry default shell safety flags here.

Missing -u means unset inputs like SHARED_DIR, CLUSTER_NAME, or VERSION silently collapse to empty strings and fail later in harder-to-diagnose ways. As per coding guidelines, "Default to set -euo pipefail (without -x) in step registry scripts."

Suggested change
-set -e
-set -o 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/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh`
around lines 2 - 3, Use the step-registry default shell safety flags in this
script by updating the top-level shell options in
telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh from the current
set in the script header to include unset-variable checking as well. The fix is
to align the script’s safety settings with the standard step-registry pattern so
references like SHARED_DIR, CLUSTER_NAME, and VERSION fail fast when missing,
while keeping the existing location near the set options in the script entry
point.

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/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh`:
- Around line 87-91: Stop parsing the inventory YAML with grep/sed in this block
and switch to a YAML-aware lookup for the values used to build temp_ssh_key,
BASTION_IP, and BASTION_USER. The current commands in the shell script are
brittle because they assume ansible_ssh_private_key, ansible_host, and
ansible_user appear in fixed positions; update the logic around these variable
assignments to read the YAML structure safely instead. Apply the same change in
the matching block in the new upgrade script so both paths use the same robust
approach.
- Around line 99-104: The artifact collection step in the bastion copy block
currently treats missing eco-gotests XML files as a hard failure. Update the scp
flow in the gather-artifacts section to distinguish “no reports were produced”
from real transfer errors by checking for matching files on the remote side
before copying, or by explicitly handling the empty-report case. Keep the
existing artifact directory setup and make the behavior in the bash script
around the bastion copy logic tolerant of expected no-report outcomes.

---

Nitpick comments:
In
`@ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh`:
- Around line 2-3: Use the step-registry default shell safety flags in this
script by updating the top-level shell options in
telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh from the current
set in the script header to include unset-variable checking as well. The fix is
to align the script’s safety settings with the standard step-registry pattern so
references like SHARED_DIR, CLUSTER_NAME, and VERSION fail fast when missing,
while keeping the existing location near the set options in the script entry
point.
🪄 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: 444e1694-b4ba-4c5c-a6bb-3565bdd075fa

📥 Commits

Reviewing files that changed from the base of the PR and between 5c57350 and 162be5f.

📒 Files selected for processing (8)
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-ref.yaml
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/OWNERS
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-commands.sh
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-ref.metadata.json
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-ref.yaml
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-mirror-seed-image/telcov10n-functional-cnf-ran-ibu-mirror-seed-image-ref.yaml
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu/telcov10n-functional-cnf-ran-ibu-workflow.yaml
✅ Files skipped from review due to trivial changes (2)
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/OWNERS
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-ref.metadata.json
🚧 Files skipped from review as they are similar to previous changes (4)
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu/telcov10n-functional-cnf-ran-ibu-workflow.yaml
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-ref.yaml
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-mirror-seed-image/telcov10n-functional-cnf-ran-ibu-mirror-seed-image-ref.yaml
  • ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-upgrade/telcov10n-functional-cnf-ran-ibu-eco-gotests-upgrade-ref.yaml

Comment on lines +87 to +91
grep ansible_ssh_private_key -A 100 "${CNF_INVENTORY_PATH}/group_vars/all.yaml" | sed 's/ansible_ssh_private_key: //g' | sed "s/'//g" > "${PROJECT_DIR}/temp_ssh_key"
chmod 600 "${PROJECT_DIR}/temp_ssh_key"

BASTION_IP=$(grep -oP '(?<=ansible_host: ).*' "${CNF_INVENTORY_PATH}/host_vars/bastion.yaml" | sed "s/'//g")
BASTION_USER=$(grep -oP '(?<=ansible_user: ).*' "${CNF_INVENTORY_PATH}/group_vars/all.yaml" | sed "s/'//g")

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Stop scraping these inventory YAML files with grep/sed.

grep -A 100 only works if ansible_ssh_private_key stays at the end of group_vars/all.yaml; as soon as more top-level keys follow it, temp_ssh_key will contain trailing YAML and SSH will reject it. The ansible_host/ansible_user lookups have the same brittleness, so this block should use a YAML-aware read instead. The identical block in the new upgrade script needs the same fix.

🤖 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/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh`
around lines 87 - 91, Stop parsing the inventory YAML with grep/sed in this
block and switch to a YAML-aware lookup for the values used to build
temp_ssh_key, BASTION_IP, and BASTION_USER. The current commands in the shell
script are brittle because they assume ansible_ssh_private_key, ansible_host,
and ansible_user appear in fixed positions; update the logic around these
variable assignments to read the YAML structure safely instead. Apply the same
change in the matching block in the new upgrade script so both paths use the
same robust approach.

Comment on lines +99 to +104
echo "Gather artifacts from bastion"
mkdir -p "${ARTIFACT_DIR}/junit_eco_gotests"
scp -r -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
-i "${PROJECT_DIR}/temp_ssh_key" \
"${BASTION_USER}@${BASTION_IP}:/tmp/eco_gotests/report/*.xml" \
"${ARTIFACT_DIR}/junit_eco_gotests/"

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Handle “no XML reports” separately from real copy failures.

The remote test run is intentionally non-blocking (|| true), but scp ... /tmp/eco_gotests/report/*.xml still exits non-zero when no reports were produced, which turns an expected eco-gotests failure back into a hard step failure during artifact gathering. Guard the copy with a remote existence check, or explicitly tolerate the empty-report case.

🧰 Tools
🪛 ast-grep (0.44.0)

[warning] 100-103: This ssh/scp/sftp invocation disables SSH host key verification (StrictHostKeyChecking=no and/or UserKnownHostsFile=/dev/null), which lets an active man-in-the-middle impersonate the server and intercept the session without any warning. Remove these options and verify the host key instead: pre-populate known_hosts with the expected key (e.g. via ssh-keyscan over a trusted channel or ssh-keygen -H), or use StrictHostKeyChecking=yes / accept-new so unexpected key changes are rejected.
Context: scp -r -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null
-i "${PROJECT_DIR}/temp_ssh_key"
"${BASTION_USER}@${BASTION_IP}:/tmp/eco_gotests/report/*.xml"
"${ARTIFACT_DIR}/junit_eco_gotests/"
Note: [CWE-295] Improper Certificate Validation.

(ssh-disable-host-key-check-bash)

🤖 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/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh`
around lines 99 - 104, The artifact collection step in the bastion copy block
currently treats missing eco-gotests XML files as a hard failure. Update the scp
flow in the gather-artifacts section to distinguish “no reports were produced”
from real transfer errors by checking for matching files on the remote side
before copying, or by explicitly handling the empty-report case. Keep the
existing artifact directory setup and make the behavior in the bash script
around the bastion copy logic tolerant of expected no-report outcomes.

@openshift-ci

openshift-ci Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

@shaior: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/rehearse/periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibu-4.20-cnf-ran-ztp-tests 162be5f link unknown /pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibu-4.20-cnf-ran-ztp-tests

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.

@shaior

shaior commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibu-4.20-cnf-ran-ztp-tests

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@shaior: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-ci

openshift-ci Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: shaior

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

The pull request process is described 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

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

[REHEARSALNOTIFIER]
@shaior: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibu-4.20-cnf-ran-ztp-tests N/A periodic Ci-operator config changed
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse list to get an up-to-date list of affected jobs
Comment: /pj-rehearse abort to abort all active rehearsals
Comment: /pj-rehearse network-access-allowed to allow rehearsals of tests that have the restrict_network_access field set to false. This must be executed by an openshift org member who is not the PR author

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant