MGMT-21787: Update prompt to include platform integration#218
Conversation
|
@CrystalChun: This pull request references MGMT-21787 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. DetailsIn response to this: Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughTemplate guidance updated to be platform-aware: clusters on baremetal, vsphere, or nutanix must set API and Ingress VIPs; clusters on none or oci do not require VIPs. Existing VIP rules for single-node clusters and when user-managed networking is disabled remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant T as Template UI
participant C as Cluster Config
rect rgb(245,248,255)
note over T,C: Platform selection & VIP rules
U->>T: Select platform (baremetal|vsphere|oci|nutanix|none)
T->>C: Save platform
alt Platform requires VIPs (baremetal|vsphere|nutanix)
T->>U: Show API & Ingress VIP prerequisites
else Platform without VIPs (none|oci)
T->>U: Indicate VIPs not required
end
end
rect rgb(243,255,245)
note over T: Initiate Installation
T->>U: Prompt to initiate installation
U->>T: Confirm start
T->>C: Start installation
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (2)
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 |
|
Hi @CrystalChun. Thanks for your PR. I'm waiting for a rh-ecosystem-edge 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. |
|
@CrystalChun: This pull request references MGMT-21787 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
![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)
template.yaml (1)
207-211: Scope contradiction: OCI listed as a platform but “Public clouds … not supported.”If OCI is now supported via Assisted platform integration, update scope statements accordingly to avoid confusing users.
Suggested tweak:
- - Supported: On-premise OpenShift installs via Assisted Installer on baremetal hosts or VMs (e.g., vSphere, KVM, libvirt). - - Not supported: Public clouds (AWS, Azure, GCP) or any non-Assisted platforms/hosted services. + - Supported: Assisted Installer workflows on baremetal and supported platform integrations (e.g., vSphere, Nutanix, OCI) as available for the target OCP version. + - Not supported: Platforms not supported by Assisted Installer for the target OCP version (e.g., AWS/Azure/GCP unless explicitly supported via Assisted integration).
🧹 Nitpick comments (1)
template.yaml (1)
249-251: Microcopy nit: fix duplicated “or”.Minor grammar fix to polish the refusal text.
- * **Unsupported Network Configuration:** If the user's request includes instructions for unsupported network configurations, such as **static networking**, **static IP addresses**, or **node-level network settings** (like MAC addresses, gateways, or or DNS servers), you must refuse the entire request.** Do not proceed with cluster creation. Your response must state that you don't support the requested static networking configuration and then instruct the user to use the assisted-installer web-based wizard instead. + * **Unsupported Network Configuration:** If the user's request includes instructions for unsupported network configurations, such as **static networking**, **static IP addresses**, or **node-level network settings** (like MAC addresses, gateways, or DNS servers), you must refuse the entire request.** Do not proceed with cluster creation. Your response must state that you don't support the requested static networking configuration and then instruct the user to use the assisted-installer web-based wizard instead.
| * Clusters with platform baremetal, vsphere, or nutanix need to **set API and Ingress VIPs**. | ||
| * Single node clusters don't need to **set API and Ingress VIPs**. | ||
| * Clusters with platform none or oci don't need to **set API and Ingress VIPs**. | ||
| * Cluster with user-managed networking enabled don't need to **set API and Ingress VIPs**. | ||
| * Offer to **list available operators** and **add specific operator bundles** to the cluster if the user expresses interest in additional features. |
There was a problem hiding this comment.
🧩 Analysis chain
Clarify VIP requirements by platform; current bullets can mislead.
As written, readers may infer unconditional VIP requirements/exemptions by platform. In Assisted Installer, VIPs are tied primarily to multi‑node + user‑managed‑networking (UMN) being disabled. Please fold platform nuances under that gating to avoid contradictions.
Apply this refinement:
* Before installation, the user might need to **set API and Ingress VIPs**. Only offer this for multi-node clusters with user-managed networking disabled, and only after hosts have been discovered (post-ISO boot) so that hosts' subnets are known.
- * Clusters with platform baremetal, vsphere, or nutanix need to **set API and Ingress VIPs**.
- * Single node clusters don't need to **set API and Ingress VIPs**.
- * Clusters with platform none or oci don't need to **set API and Ingress VIPs**.
- * Cluster with user-managed networking enabled don't need to **set API and Ingress VIPs**.
+ * Platform nuances:
+ * baremetal, vsphere, nutanix: VIPs required when UMN is disabled; when UMN is enabled, do not set VIPs.
+ * none: do not set VIPs (rely on external/manual load balancing).
+ * oci: do not set VIPs (rely on cloud load balancers if applicable).
+ * Single node clusters don't need to **set API and Ingress VIPs**.
+ * Clusters with user-managed networking enabled don't need to **set API and Ingress VIPs**.To confirm the exact OCI/None behaviors in Assisted Installer, please verify with upstream docs/releases:
🌐 Web query:
OpenShift Assisted Installer VIP requirements by platform (baremetal, vSphere, Nutanix, None, OCI) and user-managed networking
💡 Result:
Summary — VIP requirements (Assisted Installer / OpenShift 4.x)
-
General (cluster-managed networking, default): you must provide two VIPs in the machine network: API VIP (api..<base_domain>) and Ingress VIP (*.apps..<base_domain>). VIPs must be IPv4 when using dual‑stack (IPv4 primary). VIPs must belong to the machine CIDR and be reachable. [1][2][3]
-
User‑managed networking: the Assisted API expects you to supply the load‑balancer IP(s) instead of relying on keepalived/VRRP. Set user_managed_networking=true (and load_balancer.type=user‑managed) and specify api_vips and ingress_vips (they may both point to the same external LB IP if your LB uses a single VIP). VIP DHCP allocation is available but limited and not supported for newer SDN/OVN changes. [4][1]
-
Bare metal (Assisted IPI/UPI): requires API and Ingress VIPs for cluster‑managed networking (or the external LB IPs when user‑managed). If using VIP DHCP allocation, ensure SDN constraints apply. Keep ports 6443 (API) and 443 (console) reachable. [2][5]
-
vSphere (Assisted): same VIP rules as bare metal — provide API + Ingress VIPs (or external LB IPs with user‑managed networking). vSphere platform integration is supported by the Assisted Installer. [6][2]
-
Nutanix (Assisted): same VIP rules — supply API + Ingress VIPs (or external LB IPs if user‑managed). Nutanix is a supported platform for the Assisted Installer. [2][6]
-
OCI (Oracle Cloud Infrastructure): Assisted Installer support for OCI is available only with user‑managed networking (use an external load balancer / provide LB VIPs). You must supply the external LB IP(s) via api_vips/ingress_vips and enable user_managed_networking. [7][2]
Notes / links
- API/Ingress VIP JSON fields and examples, dual‑stack rules, and vip_dhcp_allocation details: Red Hat Assisted Installer docs. [1][4]
- Machine‑CIDR / validation rules, cluster vs. user‑managed networking behavior and validations. [3][5]
Sources
[1] Red Hat — Assisted Installer (VIPs, JSON examples & vip_dhcp_allocation).
[2] OpenShift docs — Installing with the Assisted Installer (networking, VIP requirements).
[3] OpenShift installer UPI docs (load balancer ports/requirements).
[4] Assisted Installer guide — configuring user‑managed load balancer and api_vips/ingress_vips.
[5] Assisted Installer validations (api/ingress VIP validations, machine CIDR).
[6] Assisted Installer platform pages (vSphere, Nutanix support).
[7] Notes on OCI support requiring user‑managed networking.
If you want, I can extract the exact JSON snippets and example CLI/API payloads for each platform (and indicate whether API+Ingress can be the same IP for your LB).
Clarify VIP requirements: gate by networking mode and node-count, not just platform
File: template.yaml Lines: 283-287
- VIPs are determined by node-count + networking mode, not only by platform: for multi-node clusters with cluster-managed networking (default) require API + Ingress VIPs in the machine CIDR and reachable; prompt for them only after hosts are discovered (post‑ISO) so subnets are known.
- If user_managed_networking=true the Assisted API expects external load‑balancer IP(s) — set api_vips/ingress_vips (may be the same IP). OCI is supported only with user‑managed networking and requires external LB IPs.
- Platform mapping: baremetal / vSphere / Nutanix follow the above (VIPs for cluster‑managed; external LB IPs when UMN). platform "none" relies on external/manual load balancing (supply external LB IPs). Single‑node clusters do not need VIPs.
🤖 Prompt for AI Agents
template.yaml lines 283-287: update the VIP guidance to gate requirements by
node-count and networking mode (user_managed_networking) rather than by platform
alone; explicitly state that multi-node clusters with cluster-managed networking
require API and Ingress VIPs in the machine CIDR and reachable (prompt for them
after hosts are discovered/post-ISO when subnets are known), that
user_managed_networking=true requires external load-balancer IPs set via
api_vips/ingress_vips (can be same IP) and that OCI supports only user-managed
networking with external LB IPs, that baremetal/vSphere/Nutanix follow the
node-count + networking-mode rules, platform "none" relies on external/manual
load balancing (supply external LB IPs), and single-node clusters do not need
VIPs.
|
@CrystalChun I would hold this until the submodule update for the MCP server tool changes can be done in this same PR so that it doesn't break integration. |
|
Also have you tried it without the prompt updates that are redundant to the MCP tool descriptions? I'm not sure if it gives a higher priority or weight on the prompt vs tool listing, but it would help keep the prompt smaller if we could just rely on the MCP descriptions for most of the guidelines around setting platform. |
CrystalChun
force-pushed
the
platforms
branch
from
220ed29 to
10d224b
Compare
I have and didn't realize it's best not to be redundant! Thanks for letting me know. I've removed the redundant prompts that are already part of the tool descriptions. |
|
/hold |
|
@CrystalChun: This pull request references MGMT-21787 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
Includes: - additional constraints around setting VIPs during Cluster Configuration (step 4) - additional note about setting host roles with OCI during host discovery (step 3)
CrystalChun
force-pushed
the
platforms
branch
from
10d224b to
9f60272
Compare
|
@CrystalChun: This pull request references MGMT-21787 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/unhold |
|
@CrystalChun: This pull request references MGMT-21787 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: carbonin, CrystalChun 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 |
|
/ok-to-test |
|
/retest |
|
@CrystalChun: 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. |
openshift-merge-bot
bot
merged commit 6e2160f
into
rh-ecosystem-edge:main
4 participants
Includes:
Summary by CodeRabbit