WIP: Adds doc aimed at converting Jetlag OCP cluster to ACM hub/spoke - #854
WIP: Adds doc aimed at converting Jetlag OCP cluster to ACM hub/spoke#854ahasson-rh wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
📝 WalkthroughWalkthroughAdds an ACM Hub and spoke deployment guide, expands hypervisor quick-start documentation, and links the new documentation pages from ChangesDocumentation workflows
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/acm.md`:
- Around line 100-101: Replace the hard-coded inventory filename in docs/acm.md
lines 100-101 with the consistent <CLOUDNAME>.local placeholder and instruct
readers to substitute their generated inventory; make the same replacement in
the quick-start commands in docs/hypervisors.md lines 23-26, preserving command
accuracy across environments.
- Around line 116-126: Complete the “Manual Spoke Deployment” and “Automated
Spoke Deployment via GOGS” sections by replacing each <TBD> with actionable
commands, required variables, and relevant repository references, or links to
authoritative procedures. Ensure both workflows explain how to generate, access,
and apply spoke manifests through the oc CLI or GOGS without leaving placeholder
content.
- Around line 31-42: Update the ACM configuration example near setup_lso and
setup_odf to set hv_inventory to true, ensuring the inventory template generates
the required [hv] and [hv_vm] groups for the later spoke-manifest workflow.
- Line 65: Update the ansible-playbook command in the ACM deployment
documentation to use the acm-deploy-load inventory path with the -i option
instead of the <hub-kubeconfig-path> placeholder. Keep the rhacm-deploy.yml
playbook invocation unchanged and align the example with the later
ansible/inventory/cloud49.local pattern.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 333633d4-23b4-4f82-a051-be5c67975890
📒 Files selected for processing (4)
CLAUDE.mdREADME.mddocs/acm.mddocs/hypervisors.md
| ```yaml | ||
| # Multi Node OpenShift with zero worker nodes (compact cluster) | ||
| cluster_type: mno | ||
| worker_node_count: 0 | ||
|
|
||
| # Disable LSO and ODF in Jetlag - will be managed by acm-deploy-load instead | ||
| setup_lso: false | ||
| setup_odf: false | ||
|
|
||
| # Optionally deploy MinIO if needed for test workloads if S3 object storage is needed | ||
| # setup_bastion_minio: true | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Enable hv_inventory in the Hub configuration.
The later spoke-manifest workflow depends on [hv] and [hv_vm] groups, but the example never sets hv_inventory: true. The inventory template only renders those groups when that flag is enabled, so users following this guide will have no hypervisor entries to copy.
Proposed fix
# Multi Node OpenShift with zero worker nodes (compact cluster)
cluster_type: mno
worker_node_count: 0
+hv_inventory: trueAs per path instructions, documentation configuration examples must accurately reflect the required deployment configuration.
📝 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.
| ```yaml | |
| # Multi Node OpenShift with zero worker nodes (compact cluster) | |
| cluster_type: mno | |
| worker_node_count: 0 | |
| # Disable LSO and ODF in Jetlag - will be managed by acm-deploy-load instead | |
| setup_lso: false | |
| setup_odf: false | |
| # Optionally deploy MinIO if needed for test workloads if S3 object storage is needed | |
| # setup_bastion_minio: true | |
| ``` |
🤖 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 `@docs/acm.md` around lines 31 - 42, Update the ACM configuration example near
setup_lso and setup_odf to set hv_inventory to true, ensuring the inventory
template generates the required [hv] and [hv_vm] groups for the later
spoke-manifest workflow.
Source: Path instructions
| [root@<bastion> ~]# git clone https://github.com/redhat-performance/acm-deploy-load.git | ||
| [root@<bastion> ~]# cd acm-deploy-load | ||
| [root@<bastion> acm-deploy-load]# source bootstrap.sh | ||
| [root@<bastion> acm-deploy-load]# ansible-playbook -i <hub-kubeconfig-path> rhacm-deploy.yml |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- docs/acm.md around line 65 ---\n'
if [ -f docs/acm.md ]; then
sed -n '1,140p' docs/acm.md | cat -n
else
echo 'docs/acm.md not found'
fi
printf '\n--- search for acm-deploy-load / rhacm-deploy.yml / kubeconfig ---\n'
rg -n "acm-deploy-load|rhacm-deploy\.yml|hub-kubeconfig|kubeconfig" . --glob '!**/.git/**'Repository: redhat-performance/jetlag
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files docs/acm.md
wc -l docs/acm.md
sed -n '1,120p' docs/acm.md | cat -nRepository: redhat-performance/jetlag
Length of output: 6546
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- docs/acm.md mentions of inventory / kubeconfig / rhacm-deploy ---'
rg -n "inventory|kubeconfig|rhacm-deploy|bootstrap\.sh|telco-core-manifests" docs/acm.md
printf '\n%s\n' '--- any acm-deploy-load references in repo docs ---'
rg -n "acm-deploy-load|rhacm-deploy\.yml|hub-kubeconfig-path|kubeconfig" docs . --glob '!**/.git/**'Repository: redhat-performance/jetlag
Length of output: 25899
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "acm-deploy-load|rhacm-deploy\.yml|hub-kubeconfig-path|kubeconfig|bootstrap\.sh|telco-core-manifests" docs . --glob '!**/.git/**'Repository: redhat-performance/jetlag
Length of output: 28450
Use the acm-deploy-load inventory here, not a kubeconfig path. This section says the playbooks derive configuration from the inventory file, and the later example already uses -i ansible/inventory/cloud49.local, so this command should follow the same pattern.
🤖 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 `@docs/acm.md` at line 65, Update the ansible-playbook command in the ACM
deployment documentation to use the acm-deploy-load inventory path with the -i
option instead of the <hub-kubeconfig-path> placeholder. Keep the
rhacm-deploy.yml playbook invocation unchanged and align the example with the
later ansible/inventory/cloud49.local pattern.
| ```console | ||
| [root@<bastion> acm-deploy-load]# sed -n '/\[hv/,$p' ../jetlag/ansible/inventory/cloud49.local >> ansible/inventory/cloud49.local |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a placeholder or variable for the inventory filename.
The added commands hard-code cloud49.local and cloud42.local, so they fail when copied into another lab or when the generated inventory has a different cloud name. Use a consistent placeholder such as <CLOUDNAME>.local and explicitly instruct readers to substitute their generated inventory.
docs/acm.md#L100-L101: replacecloud49.localwith the generated inventory placeholder.docs/hypervisors.md#L23-L26: replacecloud42.localwith the same placeholder in the quick-start commands.
As per path instructions, documentation examples must remain accurate across supported environments.
📍 Affects 2 files
docs/acm.md#L100-L101(this comment)docs/hypervisors.md#L23-L26
🤖 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 `@docs/acm.md` around lines 100 - 101, Replace the hard-coded inventory
filename in docs/acm.md lines 100-101 with the consistent <CLOUDNAME>.local
placeholder and instruct readers to substitute their generated inventory; make
the same replacement in the quick-start commands in docs/hypervisors.md lines
23-26, preserving command accuracy across environments.
Source: Path instructions
| ### Manual Spoke Deployment | ||
|
|
||
| Once manifests are generated, apply them from the bastion using the `oc` CLI to create the spoke cluster resources in the Hub: | ||
|
|
||
| <TBD> | ||
|
|
||
| ### Automated Spoke Deployment via GOGS | ||
|
|
||
| For larger deployments or continuous provisioning, integrate with GOGS (Git server) to automatically manage spoke cluster manifests: | ||
|
|
||
| <TBD> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Complete the spoke deployment procedures before publishing this guide.
Both manual and GOGS deployment paths end in <TBD>, so the document does not provide an actionable way to deploy spoke clusters despite listing those workflows. Add the required commands, variables, and repository references, or link directly to complete authoritative procedures.
As per path instructions, documentation must provide clear, usable configuration and deployment instructions.
🤖 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 `@docs/acm.md` around lines 116 - 126, Complete the “Manual Spoke Deployment”
and “Automated Spoke Deployment via GOGS” sections by replacing each <TBD> with
actionable commands, required variables, and relevant repository references, or
links to authoritative procedures. Ensure both workflows explain how to
generate, access, and apply spoke manifests through the oc CLI or GOGS without
leaving placeholder content.
Source: Path instructions
ahasson-rh
changed the title
| @@ -0,0 +1,134 @@ | |||
| # Advanced Cluster Management (ACM) Hub Setup | |||
There was a problem hiding this comment.
imo this is better suited for the acm-deploy-load repo since it covers additional configuration required for ACM hub.
cc @akrzos wdyt?
There was a problem hiding this comment.
@mcornea sure, I don't have a strong opinion on where it goes but as a new user of jetlag, I did not know there's a specific path to follow that crosses both repos to get ACM on top :)
There was a problem hiding this comment.
I had originally suggested the idea of a jetlag doc to outline how to create an ocp cluster that is "geared" toward becoming an ACM Hub cluster. This means we need a few things mostly storage related to be completed by jetlag and which are not explicitedly documented in either jetlag or acm docs. Now that I am putting a bit more thought to it I would suggest we improve the existing deploy docs and include a section outlining what vars someone might tune as an example to configure and install the operators and storage to eventually become an ACM cluster. Let me know your thoughts for this idea?
There was a problem hiding this comment.
Sounds good to me as well! @akrzos do you want to continue this PR/thread or will you be submitting a new PR for such doc outline?
There was a problem hiding this comment.
Maybe you could put together a section for how you have configured your hub to the existing guides where you feel it fits best either a section to one of the quickstarts or maybe to the tips and vars document, WDYT?
|
|
||
| By default, Jetlag can deploy Local Storage Operator (LSO) and OpenShift Data Foundation (ODF) on the cluster. For ACM Hub clusters, these should be managed by acm-deploy-load instead to ensure consistent configuration across your Hub and spoke clusters. | ||
|
|
||
| To disable LSO and ODF during Jetlag deployment, add these variables to the `Extra vars` section of `ansible/vars/all.yml`: |
There was a problem hiding this comment.
There was a problem hiding this comment.
You're right @mcornea, I had actually enabled LSO in Jetlag for my specific use case and meant for this comment to remind the user who takes this ACM path that they'd need to consider turning it off in the acm-deploy-load side or disable it in Jetlag and manage it in acm-deploy-load. I can reword it like this:
By default, Jetlag can deploy Local Storage Operator (LSO) and OpenShift Data Foundation (ODF) on the cluster. For ACM Hub clusters, these should be managed by acm-deploy-load instead to ensure consistent configuration across your Hub and spoke clusters.
LSO and ODF are disabled by default in Jetlag.
wdyt?
3 participants
This doc is focused on taking the next steps towards converting a Jetlag powered OCP cluster to an ACM Hub and some quick guides to help as an overview with pointers to the acm-deploy-load repo.
Summary by CodeRabbit