Add target spoke deploy for ibu lane#80770
Conversation
WalkthroughA new CI step ( ChangesIBU Target SNO Spoke Deployment
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibu-4.20-cnf-ran-ztp-tests |
|
@shaior: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-deploy-spoke-sno/telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno-commands.sh (1)
17-21: ⚡ Quick winQuote variable expansions in
cppaths.These lines are currently vulnerable to word-splitting/globbing edge cases (ShellCheck SC2086).
Suggested patch
-cp ${SHARED_DIR}/target-all ${INVENTORY_PATH}/group_vars/all -cp ${SHARED_DIR}/target-bastions ${INVENTORY_PATH}/group_vars/bastions -cp ${SHARED_DIR}/target-hypervisors ${INVENTORY_PATH}/group_vars/hypervisors -cp ${SHARED_DIR}/target-nodes ${INVENTORY_PATH}/group_vars/nodes -cp ${SHARED_DIR}/target-masters ${INVENTORY_PATH}/group_vars/masters +cp "${SHARED_DIR}/target-all" "${INVENTORY_PATH}/group_vars/all" +cp "${SHARED_DIR}/target-bastions" "${INVENTORY_PATH}/group_vars/bastions" +cp "${SHARED_DIR}/target-hypervisors" "${INVENTORY_PATH}/group_vars/hypervisors" +cp "${SHARED_DIR}/target-nodes" "${INVENTORY_PATH}/group_vars/nodes" +cp "${SHARED_DIR}/target-masters" "${INVENTORY_PATH}/group_vars/masters" @@ -cp ${SHARED_DIR}/target-bastion ${INVENTORY_PATH}/host_vars/bastion -cp ${SHARED_DIR}/target-hypervisor ${INVENTORY_PATH}/host_vars/hypervisor -cp ${SHARED_DIR}/target-master0 ${INVENTORY_PATH}/host_vars/master0 +cp "${SHARED_DIR}/target-bastion" "${INVENTORY_PATH}/host_vars/bastion" +cp "${SHARED_DIR}/target-hypervisor" "${INVENTORY_PATH}/host_vars/hypervisor" +cp "${SHARED_DIR}/target-master0" "${INVENTORY_PATH}/host_vars/master0"Also applies to: 27-29
🤖 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-target-deploy-spoke-sno/telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno-commands.sh` around lines 17 - 21, The cp command paths are vulnerable to word-splitting and globbing issues because variable expansions are not quoted. Fix this by wrapping all variable expansions in double quotes in the cp commands on lines 17-21 and also on lines 27-29. Specifically, wrap ${SHARED_DIR} and ${INVENTORY_PATH} variables in double quotes so that each path is treated as a single argument to the cp command, preventing unintended word-splitting or globbing expansion.Source: Linters/SAST tools
🤖 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/config/openshift-kni/eco-ci-cd/openshift-kni-eco-ci-cd-main__cnf-ran-ibu-4.20.yaml`:
- Line 36: The SPOKE_CLUSTER and TARGET_SPOKE_CLUSTER environment variables use
Python-style list syntax with escaped quotes that produces invalid JSON when
parsed by the shell. Replace the current Python-style syntax (with doubled
single quotes for escaping) with proper JSON array syntax using double quotes
inside single quotes. Change the format from '[''value'']' to '["value"]' so
that after shell parsing strips the outer quotes, a valid JSON array remains for
Ansible consumption.
In
`@ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-deploy-spoke-sno/telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno-commands.sh`:
- Around line 2-3: The shell script is missing the -u flag in its error handling
setup. Replace the two separate set commands (set -e and set -o pipefail) with a
single combined command that includes the -u flag (set -euo pipefail) at the
beginning of the script to ensure unset environment variables are caught and
cause the script to exit with a clear error rather than silently expanding to
empty strings.
In
`@ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-mirror-ocp/telcov10n-functional-cnf-ran-ibu-target-mirror-ocp-commands.sh`:
- Around line 20-25: The CLUSTER_NAME variable is only conditionally assigned
when the file at ${SHARED_DIR}/cluster_name exists, but it is then used
unconditionally in the KUBECONFIG_PATH construction. If the file does not exist
and CLUSTER_NAME is not pre-set, the path will be malformed. Add validation
after the conditional block to check that CLUSTER_NAME is not empty, and either
exit with an error message or set a default value if appropriate for your use
case. This ensures KUBECONFIG_PATH always has a valid cluster name.
---
Nitpick comments:
In
`@ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-deploy-spoke-sno/telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno-commands.sh`:
- Around line 17-21: The cp command paths are vulnerable to word-splitting and
globbing issues because variable expansions are not quoted. Fix this by wrapping
all variable expansions in double quotes in the cp commands on lines 17-21 and
also on lines 27-29. Specifically, wrap ${SHARED_DIR} and ${INVENTORY_PATH}
variables in double quotes so that each path is treated as a single argument to
the cp command, preventing unintended word-splitting or globbing expansion.
🪄 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: 90648372-5e18-48b2-a448-f38291860164
📒 Files selected for processing (8)
ci-operator/config/openshift-kni/eco-ci-cd/openshift-kni-eco-ci-cd-main__cnf-ran-ibu-4.20.yamlci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-deploy-spoke-sno/OWNERSci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-deploy-spoke-sno/telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno-commands.shci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-deploy-spoke-sno/telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno-ref.metadata.jsonci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-deploy-spoke-sno/telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno-ref.yamlci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-hub-deploy/telcov10n-functional-cnf-ran-ibu-target-hub-deploy-commands.shci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-mirror-ocp/telcov10n-functional-cnf-ran-ibu-target-mirror-ocp-commands.shci-operator/step-registry/telcov10n/functional/cnf-ran/ibu/telcov10n-functional-cnf-ran-ibu-workflow.yaml
| {"name":"topology-aware-lifecycle-manager","catalog":"topology-aware-lifecycle-manager-fbc","og_name":"global-operators","nsname":"openshift-operators","fbc_iib_repo":"latest","channel":"stable","deploy_default_config":false,"ocp_operator_mirror_fbc_image_base":"quay.io/redhat-user-workloads/telco-5g-tenant/topology-aware-lifecycle-manager-fbc-${VERSION_TAG}"} | ||
| ] | ||
| SPOKE_CLUSTER: ibu-seed | ||
| SPOKE_CLUSTER: '[''ibu-seed'']' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show where these vars are injected into ansible extra-vars.
rg -n "spoke_clusters='\\$\\{(SPOKE_CLUSTER|TARGET_SPOKE_CLUSTER)\\}'" ci-operator/step-registry/telcov10n/functional/cnf-ran
# Quick parser sanity check for problematic quoting shape.
python - <<'PY'
import shlex
cases = [
"spoke_clusters='['ibu-seed']'",
"spoke_clusters='[\"ibu-seed\"]'",
]
for c in cases:
print(f"\nCASE: {c}")
try:
print("shlex:", shlex.split(c))
except Exception as e:
print("ERROR:", e)
PYRepository: openshift/release
Length of output: 594
🏁 Script executed:
cat -n ci-operator/config/openshift-kni/eco-ci-cd/openshift-kni-eco-ci-cd-main__cnf-ran-ibu-4.20.yaml | head -60Repository: openshift/release
Length of output: 6643
🏁 Script executed:
cat -n ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-deploy-spoke-sno/telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno-commands.shRepository: openshift/release
Length of output: 2188
🏁 Script executed:
cat -n ci-operator/step-registry/telcov10n/functional/cnf-ran/deploy-spoke-sno/telcov10n-functional-cnf-ran-deploy-spoke-sno-commands.shRepository: openshift/release
Length of output: 1935
🏁 Script executed:
find ci-operator/step-registry/telcov10n -name "deploy-spoke-sno.yaml" -o -name "*.yaml" | xargs rg -l "deploy-spoke-sno" | head -5Repository: openshift/release
Length of output: 594
🏁 Script executed:
# Find the Ansible playbook
fd -t f "deploy-spoke-sno.yaml" /eco-ci-cd 2>/dev/null || echo "Playbook not in repo; searching step-registry references"
rg -A 5 "def spoke_clusters" ci-operator/step-registry/Repository: openshift/release
Length of output: 118
🏁 Script executed:
# Check if there's any documentation on how these vars should be formatted
rg -B 3 -A 3 "spoke_clusters" ci-operator/step-registry/telcov10n/functional/cnf-ran/ --type sh --type yamlRepository: openshift/release
Length of output: 2805
🏁 Script executed:
# Verify actual impact: test both formats with Python's JSON parser
python3 <<'PY'
import json
# Test what Ansible would see after shell parsing
test_cases = [
("[ibu-seed]", "Python-style after shell parsing (quotes lost)"),
('["ibu-seed"]', "JSON-style after shell parsing (quotes preserved)"),
]
for val, desc in test_cases:
print(f"\n{desc}")
print(f" Value: {val}")
try:
parsed = json.loads(val)
print(f" JSON parse OK: {parsed} (type: {type(parsed).__name__})")
except json.JSONDecodeError as e:
print(f" JSON parse FAILED: {e}")
PYRepository: openshift/release
Length of output: 308
Use JSON array syntax for SPOKE_CLUSTER and TARGET_SPOKE_CLUSTER environment variables.
The current Python-style list syntax with escaped quotes ('[''ibu-seed'']') produces invalid JSON when passed to Ansible. After shell parsing, the outer quotes are stripped, leaving [ibu-seed], which fails JSON validation. Ansible expects valid JSON arrays for these variables; use '["ibu-seed"]' instead, which correctly produces ["ibu-seed"] after shell parsing.
Suggested patch
- SPOKE_CLUSTER: '[''ibu-seed'']'
+ SPOKE_CLUSTER: '["ibu-seed"]'
@@
- TARGET_SPOKE_CLUSTER: '[''ibu-target'']'
+ TARGET_SPOKE_CLUSTER: '["ibu-target"]'🤖 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/config/openshift-kni/eco-ci-cd/openshift-kni-eco-ci-cd-main__cnf-ran-ibu-4.20.yaml`
at line 36, The SPOKE_CLUSTER and TARGET_SPOKE_CLUSTER environment variables use
Python-style list syntax with escaped quotes that produces invalid JSON when
parsed by the shell. Replace the current Python-style syntax (with doubled
single quotes for escaping) with proper JSON array syntax using double quotes
inside single quotes. Change the format from '[''value'']' to '["value"]' so
that after shell parsing strips the outer quotes, a valid JSON array remains for
Ansible consumption.
| set -e | ||
| set -o pipefail |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use set -euo pipefail in this step script.
set -u is missing, so unset env vars can silently expand to empty strings and fail later with less actionable errors.
Suggested patch
-set -e
-set -o pipefail
+set -euo pipefailAs per coding guidelines, step registry command scripts should use the default set -euo pipefail (without -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/telcov10n/functional/cnf-ran/ibu-target-deploy-spoke-sno/telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno-commands.sh`
around lines 2 - 3, The shell script is missing the -u flag in its error
handling setup. Replace the two separate set commands (set -e and set -o
pipefail) with a single combined command that includes the -u flag (set -euo
pipefail) at the beginning of the script to ensure unset environment variables
are caught and cause the script to exit with a clear error rather than silently
expanding to empty strings.
Source: Coding guidelines
| if [[ -f "${SHARED_DIR}/cluster_name" ]]; then | ||
| CLUSTER_NAME=$(cat "${SHARED_DIR}/cluster_name") | ||
| fi | ||
|
|
||
| KUBECONFIG_PATH="/home/telcov10n/project/generated/${CLUSTER_NAME}/auth/kubeconfig" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether the step ref guarantees CLUSTER_NAME in env.
fd -i 'telcov10n-functional-cnf-ran-ibu-target-mirror-ocp-ref.yaml' ci-operator/step-registry --exec sed -n '1,220p' {}Repository: openshift/release
Length of output: 675
🏁 Script executed:
cat -n ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-target-mirror-ocp/telcov10n-functional-cnf-ran-ibu-target-mirror-ocp-commands.shRepository: openshift/release
Length of output: 1397
🏁 Script executed:
# Search for workflows/chains that use this step
fd -i 'ibu-target-mirror-ocp' ci-operator/step-registry --exec grep -l {} \; 2>/dev/null | head -20Repository: openshift/release
Length of output: 43
🏁 Script executed:
# Check if CLUSTER_NAME is used elsewhere in similar telcov10n steps
rg 'CLUSTER_NAME' ci-operator/step-registry/telcov10n --type sh -B 2 -A 2 | head -50Repository: openshift/release
Length of output: 9914
Validate CLUSTER_NAME before deriving kubeconfig path.
CLUSTER_NAME is only conditionally assigned from ${SHARED_DIR}/cluster_name, but used unconditionally on line 24 to construct KUBECONFIG_PATH. If the file doesn't exist and CLUSTER_NAME is not pre-set, the path becomes invalid (/home/telcov10n/project/generated//auth/kubeconfig). The step's ref file does not declare CLUSTER_NAME in env, so it must be guaranteed externally.
Suggested patch
if [[ -f "${SHARED_DIR}/cluster_name" ]]; then
CLUSTER_NAME=$(cat "${SHARED_DIR}/cluster_name")
fi
+if [[ -z "${CLUSTER_NAME:-}" ]]; then
+ echo "ERROR: CLUSTER_NAME is empty and ${SHARED_DIR}/cluster_name was not found" >&2
+ exit 1
+fi
+
KUBECONFIG_PATH="/home/telcov10n/project/generated/${CLUSTER_NAME}/auth/kubeconfig"🤖 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-target-mirror-ocp/telcov10n-functional-cnf-ran-ibu-target-mirror-ocp-commands.sh`
around lines 20 - 25, The CLUSTER_NAME variable is only conditionally assigned
when the file at ${SHARED_DIR}/cluster_name exists, but it is then used
unconditionally in the KUBECONFIG_PATH construction. If the file does not exist
and CLUSTER_NAME is not pre-set, the path will be malformed. Add validation
after the conditional block to check that CLUSTER_NAME is not empty, and either
exit with an error message or set a default value if appropriate for your use
case. This ensures KUBECONFIG_PATH always has a valid cluster name.
|
@shaior: The following test failed, say
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. |
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibu-4.20-cnf-ran-ztp-tests |
|
@shaior: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
1 participant
Summary by CodeRabbit
This PR adds support for target spoke cluster deployment to the IBU (Image-Based Upgrade) lane testing infrastructure for CNF RAN (Cloud Native Function RAN).
Key Changes:
New CI Step Registration: Introduces a new workflow step
telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-snothat deploys a target SNO (Single Node OpenShift) spoke cluster via ZTP (Zero Touch Provisioning) on the IBU target hub. The step includes comprehensive environment variable configuration for target spoke version, cluster naming, and Git repository references.Workflow Integration: Updates the main IBU CNF RAN workflow to wire in the new target spoke deployment step, which executes after the target hub mirror/preparation phase and uses inventory files preserved from the target hub deployment step.
Target Hub Setup: Modifies the target hub deployment step to preserve inventory files with a
target-prefix in shared storage, ensuring they're available for downstream steps including the new spoke deployment.Mirror Configuration Update: Updates the target mirror/OCP preparation step to dynamically derive the kubeconfig path from the cluster name and pass it to the Ansible playbook.
Config Updates: Modifies the eco-ci-cd configuration to properly format the
SPOKE_CLUSTERandTARGET_SPOKE_CLUSTERvariables as list representations.The changes enable the IBU testing lane to deploy and validate a complete target hub and spoke topology for image-based upgrade scenarios, extending the existing seed hub-spoke testing infrastructure.