Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ tests:
{"name":"multicluster-engine","catalog":"redhat-operators","nsname":"multicluster-engine","channel":"stable-2.10","og_name":"multicluster-engine","subscription_name":"multicluster-engine","deploy_default_config":true,"og_spec":{}},
{"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'']'

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.

⚠️ Potential issue | 🟠 Major

🧩 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)
PY

Repository: 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 -60

Repository: 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.sh

Repository: 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.sh

Repository: 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 -5

Repository: 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 yaml

Repository: 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}")
PY

Repository: 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.

SPOKE_OPERATORS: |
[
{"name":"sriov-fec","catalog":"certified-operators","nsname":"vran-acceleration-operators","channel":"stable"},
Expand All @@ -44,6 +44,7 @@ tests:
{"name":"lifecycle-agent","catalog":"lifecycle-agent-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/lifecycle-agent-fbc-4-20"},
{"name":"redhat-oadp-operator","catalog":"oadp-fbc","fbc_iib_repo":"oadp-1.5__v4.20__oadp-rhel9-operator","art_fbc":true,"ocp_operator_mirror_fbc_image_base":"quay.io/redhat-user-workloads/ocp-art-tenant/art-fbc","nsname":"openshift-adp","channel":"stable-1.5","og_name":"openshift-adp","subscription_name":"redhat-oadp-operator","deploy_default_config":false,"og_spec":{"targetNamespaces":[]}}
]
TARGET_SPOKE_CLUSTER: '[''ibu-target'']'
TARGET_SPOKE_OPERATORS: |
[
{"name":"sriov-fec","catalog":"certified-operators","nsname":"vran-acceleration-operators","channel":"stable"},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
approvers:
- shaior
- kononovn
- eifrach
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#!/bin/bash
set -e
set -o pipefail
Comment on lines +2 to +3

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.

🛠️ 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 pipefail

As 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


echo "Checking if the job should be skipped..."
if [ -f "${SHARED_DIR}/skip.txt" ]; then
echo "Detected skip.txt file — skipping the job"
exit 0
fi

INVENTORY_PATH="/eco-ci-cd/inventories/ocp-deployment"

echo "Create group_vars directory"
mkdir -p ${INVENTORY_PATH}/group_vars

echo "Copy target hub group inventory files from SHARED_DIR (target- prefixed)"
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

echo "Create host_vars directory"
mkdir -p ${INVENTORY_PATH}/host_vars

echo "Copy target hub host inventory files from SHARED_DIR (target- prefixed)"
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

# Kubeconfig lives on the target hub bastion, under the target cluster name
KUBECONFIG_PATH="/home/telcov10n/project/generated/${TARGET_CLUSTER_NAME}/auth/kubeconfig"

cd /eco-ci-cd

echo "Running ZTP deployment for target SNO spoke cluster: ${TARGET_SPOKE_CLUSTER}"
ansible-playbook ./playbooks/ran/deploy-spoke-sno.yaml \
-i ./inventories/ocp-deployment/build-inventory.py \
--extra-vars "kubeconfig=${KUBECONFIG_PATH} \
spoke_clusters='${TARGET_SPOKE_CLUSTER}' \
ztp_git_repo_url=${ZTP_GIT_REPO} \
ztp_clusters_git_path=siteconfig/${TARGET_SPOKE_VERSION} \
ztp_policies_git_path=policygentemplates/${TARGET_SPOKE_VERSION} \
ztp_git_repo_branch=${ZTP_GIT_BRANCH}"
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"path": "telcov10n/functional/cnf-ran/ibu-target-deploy-spoke-sno/telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno-ref.yaml",
"owners": {
"approvers": [
"shaior",
"kononovn",
"eifrach"
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
ref:
as: telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno
from_image:
namespace: telcov10n-ci
name: eco-ci-cd
tag: eco-ci-cd
commands: telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno-commands.sh
timeout: 5h
resources:
requests:
cpu: 100m
env:
- name: TARGET_SPOKE_VERSION
default: "4.18"
documentation: >-
OCP version of the target spoke; must match ztp-site-configs-ci path
siteconfig/<TARGET_SPOKE_VERSION>/<TARGET_SPOKE_CLUSTER>/
- name: TARGET_CLUSTER_NAME
default: "kni-qe-109"
documentation: IBU target hub cluster name (used to resolve kubeconfig path on the bastion)
- name: TARGET_SPOKE_CLUSTER
default: ""
documentation: >-
Target spoke cluster name in JSON array format, e.g. "['ibu-target']".
Must match the ZTP siteconfig directory name.
- name: ZTP_GIT_REPO
default: "https://gitlab.cee.redhat.com/telcov10n/ztp-site-configs-ci.git"
documentation: ZTP site configs repo
- name: ZTP_GIT_BRANCH
default: "main"
documentation: Branch to use in ztp-site-configs-ci repo
documentation: |-
Deploy target SNO spoke cluster via ZTP on the IBU target hub (kni-qe-109).
Reads target hub inventory from target-* prefixed files in SHARED_DIR saved by
ibu-target-hub-deploy. These files are unaffected by ibu-restore-seed-inventory and
are available as soon as ibu-target-hub-deploy completes.
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ find /eco-ci-cd/inventories/ocp-deployment/group_vars -maxdepth 1 -type f | whil
done
echo "${TARGET_CLUSTER_NAME}" > "${SHARED_DIR}/cluster_name"

echo "Preserving target hub inventory with target- prefix (readable after ibu-restore-seed-inventory)"
for key in bastion hypervisor master0 all bastions hypervisors nodes masters; do
[[ -f "${SHARED_DIR}/${key}" ]] && cp "${SHARED_DIR}/${key}" "${SHARED_DIR}/target-${key}"
done

echo "Getting target hub cluster version"
HUB_KUBECONFIG="/home/telcov10n/project/generated/${TARGET_CLUSTER_NAME}/auth/kubeconfig"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,18 @@ mkdir -p /eco-ci-cd/inventories/ocp-deployment/host_vars

cp "${SHARED_DIR}/bastion" /eco-ci-cd/inventories/ocp-deployment/host_vars/bastion

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"

Comment on lines +20 to +25

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.

⚠️ Potential issue | 🟠 Major

🧩 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.sh

Repository: 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 -20

Repository: 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 -50

Repository: 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.

echo "CLUSTER_NAME=${CLUSTER_NAME}"
echo "TARGET_SPOKE_VERSION=${TARGET_SPOKE_VERSION}"

cd /eco-ci-cd

echo "Mirroring OCP ${TARGET_SPOKE_VERSION} to target hub disconnected registry"
ansible-playbook ./playbooks/ran/mirror-ocp-release.yml \
ansible-playbook ./playbooks/ran/prepare-ocp-release.yml \
-i ./inventories/ocp-deployment/build-inventory.py \
--extra-vars "release=${TARGET_SPOKE_VERSION}"
--extra-vars "release=${TARGET_SPOKE_VERSION} kubeconfig=${KUBECONFIG_PATH}"
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ workflow:
Pre: deploy seed hub (kni-qe-108) and target hub (kni-qe-109) at VERSION, install operators on both,
mirrors operators and OCP release images to target bastion's disconnected registry,
deploy MinIO on target bastion, deploy spoke master VMs for both seed and target hubs,
then deploy the SNO spoke cluster on the seed hub via ZTP.
then deploy the SNO spoke on the seed hub and the target spoke on the target hub via ZTP.
Test: run IBU upgrade and post-upgrade eco-gotests, report results.
Post: notify Slack, trigger next job in chain.
steps:
Expand All @@ -15,6 +15,8 @@ workflow:
- ref: telcov10n-functional-cnf-ran-hub-config
- ref: telcov10n-functional-cnf-ran-mirror-spoke-operators
# Target hub setup — kni-qe-109 deployed at VERSION
# Note: ibu-target-hub-deploy saves target-* prefixed inventory files in SHARED_DIR
# alongside the flat copies so they survive ibu-restore-seed-inventory below.
- ref: telcov10n-functional-cnf-ran-ibu-target-hub-deploy
- ref: telcov10n-functional-cnf-ran-ibu-target-hub-config
- ref: telcov10n-functional-cnf-ran-ibu-target-mirror-spoke-operators
Expand All @@ -25,5 +27,8 @@ workflow:
- ref: telcov10n-functional-cnf-ran-ibu-restore-seed-inventory
# Deploy SNO spoke on seed hub (kni-qe-108) via ZTP
- ref: telcov10n-functional-cnf-ran-deploy-spoke-sno
# Deploy SNO spoke on target hub (kni-qe-109) via ZTP
# Reads target-* prefixed inventory files saved by ibu-target-hub-deploy (unaffected by restore above)
- ref: telcov10n-functional-cnf-ran-ibu-target-deploy-spoke-sno
post: []