fix ABI folder naming issue and Hub cluster network MTU setting at deployment time#595
fix ABI folder naming issue and Hub cluster network MTU setting at deployment time#595elhadjici wants to merge 6 commits intoopenshift-kni:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: elhadjici 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 |
openshift-ci
Bot
added
the
needs-ok-to-test
|
Hi @elhadjici. Thanks for your PR. I'm waiting for a openshift-kni member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
/lgtm |
openshift documentation reference and recommendation Co-authored-by: Ian Miller <34246471+imiller0@users.noreply.github.com>
|
New changes are detected. LGTM label has been removed. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 19 minutes and 22 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDocumentation and configuration updates for OpenShift cluster installation. Main changes include updating working directory references from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
telco-hub/install/install_directory/README.md (1)
35-35:⚠️ Potential issue | 🟡 MinorFix typo in Line 35 (“OpenShit” → “OpenShift”).
This is user-facing documentation text and should be corrected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@telco-hub/install/install_directory/README.md` at line 35, Replace the typo "OpenShit" with "OpenShift" in the README.md link text (the markdown link that currently reads "Installing an OpenShit cluster with the Agent-based Installer"); update the link label to "Installing an OpenShift cluster with the Agent-based Installer" while leaving the URL unchanged so the displayed text is corrected for users.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@telco-hub/install/install_directory/openshift/README.md`:
- Line 2: Update the unclear sentence "This is extra-manifest folder would
contain examples that can be added during the cluster installation." in the
README (the sentence starting "This is extra-manifest folder...") to a clear,
grammatical form such as: "This extra-manifest folder contains examples that can
be added during cluster installation." Replace the existing broken sentence with
this corrected version to improve readability.
- Line 3: The README's guidance and the presence of openshift/Network.yaml are
misleading because files under the openshift/ directory are additive-only and
cannot override install-time cluster resources like
Network.operator.openshift.io; remove or relocate openshift/Network.yaml and
update the README to instruct users to set network parameters in
install-config.yaml before generating manifests or to edit the
installer-generated manifests after running the installer’s "create manifests"
step (i.e., customize the Cluster Network Operator manifest in the install
assets), and clarify that openshift/ is for additive customizations (e.g.,
MachineConfigs) only.
---
Outside diff comments:
In `@telco-hub/install/install_directory/README.md`:
- Line 35: Replace the typo "OpenShit" with "OpenShift" in the README.md link
text (the markdown link that currently reads "Installing an OpenShit cluster
with the Agent-based Installer"); update the link label to "Installing an
OpenShift cluster with the Agent-based Installer" while leaving the URL
unchanged so the displayed text is corrected for users.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcb9a563-ea15-401e-95de-be898ab8eca4
📒 Files selected for processing (5)
telco-hub/install/install_directory/README.mdtelco-hub/install/install_directory/agent-config.yamltelco-hub/install/install_directory/install-config.yamltelco-hub/install/install_directory/openshift/Network.yamltelco-hub/install/install_directory/openshift/README.md
| @@ -0,0 +1,3 @@ | |||
| # Openshift folder | |||
| This is extra-manifest folder would contain examples that can be added during the cluster installation. | |||
There was a problem hiding this comment.
Fix unclear sentence structure in Line 2.
The current sentence is grammatically broken and can confuse users following install steps. Please rewrite for clarity (for example: “This extra-manifest folder contains examples that can be added during cluster installation.”).
As per coding guidelines "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@telco-hub/install/install_directory/openshift/README.md` at line 2, Update
the unclear sentence "This is extra-manifest folder would contain examples that
can be added during the cluster installation." in the README (the sentence
starting "This is extra-manifest folder...") to a clear, grammatical form such
as: "This extra-manifest folder contains examples that can be added during
cluster installation." Replace the existing broken sentence with this corrected
version to improve readability.
| @@ -0,0 +1,3 @@ | |||
| # Openshift folder | |||
| This is extra-manifest folder would contain examples that can be added during the cluster installation. | |||
| Using the openshift folder imposes a restriction: its contents cannot override default cluster manifests. | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In OpenShift agent-based installs, for manifests placed under <install_dir>/openshift/, can user-provided manifests modify install-time cluster configuration resources (e.g., Network.operator.openshift.io named "cluster"), or are they strictly non-overriding/additive only?
💡 Result:
In agent-based installs, files you place under <install_dir>/openshift/ are treated as additional manifests (most commonly MachineConfig) that get applied during bootstrap, and Red Hat explicitly warns these customizations are not validated and can even yield a nonfunctional cluster—they’re not presented as a supported “override” mechanism for installer-owned cluster config objects. [1]
For install-time cluster configuration resources (for example, cluster-wide network/operator configuration), the documented/supported workflow is instead:
- Set what you can in
install-config.yamlbefore generating manifests; and - If further network customization is needed, do it by creating/editing the installer-generated manifests (for example, a customized Cluster Network Operator manifest after
create manifests)—with the constraint that you cannot override phase-1 (install-config.yaml) values in that later step. [2]
So, practically: don’t rely on <install_dir>/openshift/ to “modify/override” existing install-time singleton resources like Network.operator.openshift.io (or related cluster-scoped “cluster” objects). Treat it as additive customization, and make supported changes via install-config.yaml and by editing the generated manifests in the install assets instead. [1][2]
Sources:
[1] OCP 4.16 Agent-based installer guide – “Creating additional manifest files” (notes + openshift/ directory guidance) (access.redhat.com)
[2] OCP 4.12 install docs – Network customization “Phase 2” + “cannot override” phase-1 values (docs.redhat.com)
Citations:
- 1: https://access.redhat.com/documentation/en-us/openshift_container_platform/4.16/pdf/installing_an_on-premise_cluster_with_the_agent-based_installer/OpenShift_Container_Platform-4.16-Installing_an_on-premise_cluster_with_the_Agent-based_Installer-en-US.pdf
- 2: https://docs.redhat.com/en/documentation/openshift_container_platform/4.12/pdf/installing_on_aws/OpenShift_Container_Platform-4.12-Installing_on_AWS-en-US.pdf
The README statement is accurate, but the Network.yaml approach is incorrect for install-time cluster configuration.
Per OpenShift agent-based installer documentation, manifests under <install_dir>/openshift/ are treated as additive only and cannot override install-time cluster resources like Network.operator.openshift.io. If openshift/Network.yaml is intended to customize cluster network settings (e.g., MTU) at install time, this approach will not work.
To modify install-time cluster configuration, use the supported workflow:
- Set network parameters in
install-config.yamlbefore generating manifests, or - Edit the installer-generated manifests directly after
create manifests(e.g., customizing the Cluster Network Operator manifest in the install assets).
The openshift/ directory should be used only for additive customizations like MachineConfigs, not for overriding cluster-scoped configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@telco-hub/install/install_directory/openshift/README.md` at line 3, The
README's guidance and the presence of openshift/Network.yaml are misleading
because files under the openshift/ directory are additive-only and cannot
override install-time cluster resources like Network.operator.openshift.io;
remove or relocate openshift/Network.yaml and update the README to instruct
users to set network parameters in install-config.yaml before generating
manifests or to edit the installer-generated manifests after running the
installer’s "create manifests" step (i.e., customize the Cluster Network
Operator manifest in the install assets), and clarify that openshift/ is for
additive customizations (e.g., MachineConfigs) only.
3 participants
The Goal of the PR are :