MGMT-21349: system prompt update for vips#150
MGMT-21349: system prompt update for vips#150openshift-merge-bot[bot] merged 1 commit intorh-ecosystem-edge:mainfrom
Conversation
|
@zszabo-rh: This pull request references MGMT-21349 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 bug to target the "4.20.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. |
WalkthroughUpdates Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant S as Assisted Service/UI
participant C as Cluster
rect rgba(220,235,245,0.6)
note over U,S: Provide Discovery ISO URL
U->>S: Enter Discovery ISO URL
S->>U: Show boot instructions
end
rect rgba(230,245,230,0.6)
note over C,S: Host discovery
U->>C: Boot hosts from ISO
C-->>S: Hosts discovered
S->>U: Display discovered hosts
end
rect rgba(245,235,220,0.6)
note over U,S: Configuration & VIPs
U->>S: Assign host roles
alt Multi-node AND installer-managed networking (after discovery)
U->>S: Set API/Ingress VIPs
else Single-node OR user-managed networking
note over S,U: VIPs not set/required
end
end
rect rgba(235,230,245,0.6)
note over U,S: Start installation
U->>S: Start installation
S->>C: Begin install workflow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@zszabo-rh: This pull request references MGMT-21349 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 bug to target the "4.20.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: 0
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)
407-407: Parameter substitution syntax is inconsistent and likely broken.OpenShift Templates use ${PARAM}, not ${{PARAM}}. Current mix will leave some values unsubstituted at deploy time.
- replicas: ${{REPLICAS_COUNT}} + replicas: ${REPLICAS_COUNT} ... - containerPort: ${{SERVICE_PORT}} + containerPort: ${SERVICE_PORT} ... - port: ${{SERVICE_PORT}} + port: ${SERVICE_PORT} ... - port: ${{SERVICE_PORT}} + port: ${SERVICE_PORT} ... - port: ${{SERVICE_PORT}} + port: ${SERVICE_PORT} - targetPort: ${{SERVICE_PORT}} + targetPort: ${SERVICE_PORT}Also applies to: 423-423, 487-487, 494-494, 575-576
🧹 Nitpick comments (3)
template.yaml (3)
253-256: Tighten VIP guidance, fix grammar, and avoid URL terminology conflict.
- Good call restricting VIPs to multi-node + IMN and post-discovery. Suggest minor edits for clarity, grammar, and to address dual-stack. Also reword to avoid saying “URL” since Step 2 forbids exposing it.
- * Before installation, the user might need to **set API and Ingress VIPs**. Only offer this for multi-node clusters with installer-managed networking, and only after hosts have been discovered (post-ISO boot) so that subnets are known. + * Before installation, the user might need to **set API and Ingress VIPs**. Offer this only for multi-node clusters with installer-managed networking, and only after hosts have been discovered (post-ISO boot) so that machine network subnets are known. * Single node clusters don't need to **set API and Ingress VIPs**. - * Cluster with user-managed networking enabled don't need to **set API and Ingress VIPs**. - * When providing the Discovery ISO URL, instruct the user: "Once your hosts are discovered, we can proceed with assigning roles. We can set the API and Ingress VIPs after hosts appear in the cluster." + * Clusters with user-managed networking enabled don't need to **set API and Ingress VIPs**. + * When providing the Discovery ISO (without surfacing the URL), instruct the user: "Once your hosts are discovered, we can proceed with assigning roles. We can set the API and Ingress VIPs after hosts appear in the cluster." + * If dual-stack networking is enabled, ensure both IPv4 and IPv6 VIPs are set and belong to the discovered machine network CIDRs; avoid conflicts with host addresses.
260-261: Gate “start installation” on passing validations.Recommend explicitly requiring cluster validations to be green before initiating.
- * Once the cluster is configured, hosts are discovered and assigned roles, and VIPs are set (if applicable), the final step is to **start the cluster installation**. + * Once the cluster is configured, hosts are discovered and assigned roles, VIPs are set (if applicable), and all cluster validations are passing, the final step is to **start the cluster installation**.
131-133: Typo in parameter name: LIGHTSSPEED → LIGHTSPEED.The extra “S” in LIGHTSSPEED is error-prone and inconsistent with other param names. Recommend correcting the param and its references.
If backward compatibility is a concern, consider adding a second param temporarily and deprecating the misspelled one.
-- name: LIGHTSSPEED_STACK_POSTGRES_SSL_MODE +- name: LIGHTSPEED_STACK_POSTGRES_SSL_MODE value: "verify-full" description: "SSL mode for the PostgreSQL database connection used by lightspeed-stack" ... - ssl_mode: ${LIGHTSSPEED_STACK_POSTGRES_SSL_MODE} + ssl_mode: ${LIGHTSPEED_STACK_POSTGRES_SSL_MODE}Also applies to: 192-193
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
template.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Red Hat Konflux / assisted-chat-saas-main-on-pull-request
zszabo-rh
force-pushed
the
system_prompt_for_vips
branch
from
f0a3113 to
d68500a
Compare
|
@zszabo-rh: This pull request references MGMT-21349 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 bug to target the "4.20.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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eranco74, zszabo-rh 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 |
openshift-merge-bot
bot
merged commit dc9bc1d
into
rh-ecosystem-edge:main
3 participants
Summary by CodeRabbit
New Features
Documentation
Bug Fixes