4 - #82664
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe CNF-RAN IBI workflow now deploys and configures a target hub, mirrors spoke operators from shared inventory state, and updates repository and operator environment wiring. The workflow also disables the post-step wait. ChangesCNF-RAN IBI workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant TargetHubDeploy
participant TargetHubConfig
participant MirrorSpokeOperators
participant SharedDir
participant Ansible
Workflow->>TargetHubDeploy: deploy target hub
TargetHubDeploy->>Ansible: run deploy-ocp-sno.yml
Ansible->>SharedDir: write inventory and cluster_name
Workflow->>TargetHubConfig: configure target hub
TargetHubConfig->>Ansible: configure operators and platform services
Workflow->>MirrorSpokeOperators: mirror spoke operators
MirrorSpokeOperators->>SharedDir: read target hub inventory
MirrorSpokeOperators->>Ansible: run disconnected operator mirroring
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 inconclusive)
✅ Passed checks (13 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ibi/telcov10n-functional-cnf-ran-ibi-workflow.yaml`:
- Around line 14-19: Update the workflow documentation around the
telcov10n-functional-cnf-ran-ibi workflow to reflect that the seed hub
deploy/configuration phases and post wait are disabled. Remove or clearly
annotate the commented references to telcov10n-functional-cnf-ran-hub-deploy,
telcov10n-functional-cnf-ran-hub-config, and wait so they are not presented as
active steps.
🪄 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: Pro Plus
Run ID: 8b1bc629-f87d-4183-a2fd-3b5f506c3865
📒 Files selected for processing (2)
ci-operator/config/openshift-kni/eco-ci-cd/openshift-kni-eco-ci-cd-main__cnf-ran-ibi-4.20.yamlci-operator/step-registry/telcov10n/functional/cnf-ran/ibi/telcov10n-functional-cnf-ran-ibi-workflow.yaml
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibi-4.20-cnf-ran-ibi-tests |
|
@TimurMP: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Create telcov10n-functional-cnf-ran-ibi-target-hub-deploy step that deploys the IBI target hub (kni-qe-128) at VERSION using the same deploy-ocp-sno.yml ansible playbook as the IBU target hub deploy. The commands.sh is reused as-is since it is fully parameterized via TARGET_CLUSTER_NAME. The ref.yaml mounts kni-qe-128 secrets with helix107 as the hypervisor (shared KVM host with kni-qe-127). Wire the new step into the IBI workflow pre phase. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Create telcov10n-functional-cnf-ran-ibi-target-hub-config step that installs ACM 2.15, MCE 2.10, LSO, GitOps, and TALM on the IBI target hub (kni-qe-128). commands.sh reused verbatim from ibu-target-hub-config — it reads cluster_name from SHARED_DIR (written by ibi-target-hub-deploy) so no cluster-specific credentials are needed. LCA/OADP are omitted from the target hub's HUB_OPERATORS since those are deployed on the seed spoke via ZTP. Wire step into IBI workflow pre phase after ibi-target-hub-deploy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Create telcov10n-functional-cnf-ran-ibi-target-mirror-spoke-operators step that mirrors spoke operators to the IBI target hub (kni-qe-128) disconnected registry. Unlike the IBU equivalent which mirrors two versions (pre- and post-upgrade), IBI only needs a single mirror pass at VERSION since the target spoke installs fresh via IBI ISO rather than upgrading. Reads cluster identity and inventory from SHARED_DIR written by ibi-target-hub-deploy. Wire step into IBI workflow pre phase after ibi-target-hub-config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TimurMP 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 |
|
@TimurMP: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/ibi-target-hub-config/telcov10n-functional-cnf-ran-ibi-target-hub-config-commands.sh`:
- Around line 2-3: Enable nounset by replacing the separate shell options with
set -euo pipefail in both
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-hub-config/telcov10n-functional-cnf-ran-ibi-target-hub-config-commands.sh
lines 2-3 and
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-mirror-spoke-operators/telcov10n-functional-cnf-ran-ibi-target-mirror-spoke-operators-commands.sh
lines 2-3.
In
`@ci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-hub-deploy/telcov10n-functional-cnf-ran-ibi-target-hub-deploy-commands.sh`:
- Around line 25-31: Update the inventory generation loop that writes
"${dest_file}" to detect multiline file contents and emit them as YAML block
scalars using | or |-, preserving each newline for values such as
ansible_ssh_private_key. Keep the existing quoted scalar output for single-line
files and continue skipping filenames containing "secretsync-vault-source-path".
🪄 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: Pro Plus
Run ID: f2ab6de5-97aa-4057-9566-13513c345dc3
📒 Files selected for processing (11)
ci-operator/config/openshift-kni/eco-ci-cd/openshift-kni-eco-ci-cd-main__cnf-ran-ibi-4.20.yamlci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-hub-config/OWNERSci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-hub-config/telcov10n-functional-cnf-ran-ibi-target-hub-config-commands.shci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-hub-config/telcov10n-functional-cnf-ran-ibi-target-hub-config-ref.yamlci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-hub-deploy/OWNERSci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-hub-deploy/telcov10n-functional-cnf-ran-ibi-target-hub-deploy-commands.shci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-hub-deploy/telcov10n-functional-cnf-ran-ibi-target-hub-deploy-ref.yamlci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-mirror-spoke-operators/OWNERSci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-mirror-spoke-operators/telcov10n-functional-cnf-ran-ibi-target-mirror-spoke-operators-commands.shci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-mirror-spoke-operators/telcov10n-functional-cnf-ran-ibi-target-mirror-spoke-operators-ref.yamlci-operator/step-registry/telcov10n/functional/cnf-ran/ibi/telcov10n-functional-cnf-ran-ibi-workflow.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/config/openshift-kni/eco-ci-cd/openshift-kni-eco-ci-cd-main__cnf-ran-ibi-4.20.yaml
| set -e | ||
| set -o pipefail |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Enable nounset in both step scripts.
Unset variables currently expand silently, allowing malformed playbook arguments or paths to proceed.
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-hub-config/telcov10n-functional-cnf-ran-ibi-target-hub-config-commands.sh#L2-L3: replace the separate options withset -euo pipefail.ci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-mirror-spoke-operators/telcov10n-functional-cnf-ran-ibi-target-mirror-spoke-operators-commands.sh#L2-L3: replace the separate options withset -euo pipefail.
📍 Affects 2 files
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-hub-config/telcov10n-functional-cnf-ran-ibi-target-hub-config-commands.sh#L2-L3(this comment)ci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-mirror-spoke-operators/telcov10n-functional-cnf-ran-ibi-target-mirror-spoke-operators-commands.sh#L2-L3
🤖 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/ibi-target-hub-config/telcov10n-functional-cnf-ran-ibi-target-hub-config-commands.sh`
around lines 2 - 3, Enable nounset by replacing the separate shell options with
set -euo pipefail in both
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-hub-config/telcov10n-functional-cnf-ran-ibi-target-hub-config-commands.sh
lines 2-3 and
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibi-target-mirror-spoke-operators/telcov10n-functional-cnf-ran-ibi-target-mirror-spoke-operators-commands.sh
lines 2-3.
Source: Coding guidelines
| find "$directory" -type f | while IFS= read -r filename; do | ||
| if [[ $filename == *"secretsync-vault-source-path"* ]]; then | ||
| continue | ||
| else | ||
| echo "$(basename "${filename}")": \'"$(cat "$filename")"\' | ||
| fi | ||
| done > "${dest_file}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve multiline inventory values as YAML block scalars.
ansible_ssh_private_key is processed from the common all credentials, but this emits it as a quoted multiline scalar. YAML folds those newlines, corrupting the private key used by the deployment inventory. Handle multiline content as |/|-, as the target-hub config step already does.
Proposed fix
- echo "$(basename "${filename}")": \'"$(cat "$filename")"\'
+ content=$(<"$filename")
+ varname=$(basename "${filename}")
+ if [[ "$content" == *$'\n'* ]]; then
+ printf '%s: |-\n' "$varname"
+ printf '%s\n' "$content" | sed 's/^/ /'
+ else
+ printf "%s: '%s'\n" "$varname" "${content//\'/\'\'}"
+ fi📝 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.
| find "$directory" -type f | while IFS= read -r filename; do | |
| if [[ $filename == *"secretsync-vault-source-path"* ]]; then | |
| continue | |
| else | |
| echo "$(basename "${filename}")": \'"$(cat "$filename")"\' | |
| fi | |
| done > "${dest_file}" | |
| find "$directory" -type f | while IFS= read -r filename; do | |
| if [[ $filename == *"secretsync-vault-source-path"* ]]; then | |
| continue | |
| else | |
| content=$(<"$filename") | |
| varname=$(basename "${filename}") | |
| if [[ "$content" == *$'\n'* ]]; then | |
| printf '%s: |-\n' "$varname" | |
| printf '%s\n' "$content" | sed 's/^/ /' | |
| else | |
| printf "%s: '%s'\n" "$varname" "${content//\'/\'\'}" | |
| fi | |
| fi | |
| done > "${dest_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/ibi-target-hub-deploy/telcov10n-functional-cnf-ran-ibi-target-hub-deploy-commands.sh`
around lines 25 - 31, Update the inventory generation loop that writes
"${dest_file}" to detect multiline file contents and emit them as YAML block
scalars using | or |-, preserving each newline for values such as
ansible_ssh_private_key. Keep the existing quoted scalar output for single-line
files and continue skipping filenames containing "secretsync-vault-source-path".
Standardize the ZTP repository URL parameter name to ZTP_GIT_REPO in hub-config and ibi-target-hub-config refs and commands.sh. Update all PTP CI config files (4.14, 4.16, 4.18, 4.19, 4.20, 4.22, 5.0) to use ZTP_GIT_REPO. Remove invalid workflow-level env block and drop ZTP_GIT_BRANCH from IBI CI config until a step that consumes it is created. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibi-4.20-cnf-ran-ibi-tests |
|
@TimurMP: your |
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibi-4.20-cnf-ran-ibi-tests |
|
@TimurMP: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
kni-qe-127/128 follow a different secret naming convention than kni-qe-109 (IBU source): master0 and group_vars secrets include 'hub' in the name. Correct three wrong names modeled after kni-qe-109 pattern: telcov10n-ansible-kni-qe-128-master0 → telcov10n-ansible-hub-kni-qe-128 telcov10n-ansible-group-kni-qe-128-nodes → telcov10n-ansible-group-hub-kni-qe-128-nodes telcov10n-ansible-group-kni-qe-128-masters → telcov10n-ansible-group-hub-kni-qe-128-masters Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibi-4.20-cnf-ran-ibi-tests |
|
@TimurMP: your |
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibi-4.20-cnf-ran-ibi-tests |
|
@TimurMP: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibi-4.20-cnf-ran-ibi-tests |
|
@TimurMP: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 32 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: |
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibi-4.20-cnf-ran-ibi-tests |
|
@TimurMP: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@TimurMP: 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. |
Summary by CodeRabbit
ZTP_GIT_REPOsettings.GITLAB_REPO_URLtoZTP_GIT_REPOin related telcov10n steps.postwait step.