MGMT-21349: tool description update for vips#75
MGMT-21349: tool description update for vips#75zszabo-rh wants to merge 1 commit intoopenshift-assisted:masterfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@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. |
WalkthroughUpdated documentation and docstrings for set_cluster_vips across README, example prompts, and server.py to clarify VIP applicability (multi-node with user-managed networking disabled) and require host discovery before setting VIPs. No code or API signature changes. Changes
Sequence Diagram(s)(omitted — changes are documentation-only and do not alter control flow) Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ 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 (
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zszabo-rh 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
size/S
|
@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: 1
🧹 Nitpick comments (3)
server.py (1)
391-429: Optional: add a runtime preflight guard to prevent no-op/misuse on SNO or UMAN clusters.While the docs now clarify applicability, adding a lightweight check would improve UX by short-circuiting with a clear message instead of sending an update that the backend will ignore/reject.
Example (outside the changed lines, illustrative only—adapt field names to your client model):
# Inside set_cluster_vips before update_cluster(...) client = InventoryClient(get_access_token()) cluster = await client.get_cluster(cluster_id=cluster_id) is_uman = bool(getattr(cluster, "user_managed_networking", False)) ha_mode = getattr(cluster, "high_availability_mode", "") # "Full" or "None" is_sno = ha_mode == "None" or bool(getattr(cluster, "single_node", False)) if is_uman or is_sno: log.info("Skipping VIP update: cluster is %s", "UMN" if is_uman else "SNO") return "VIPs are not applicable for single-node or user-managed networking clusters."If you want, I can open a follow-up PR to wire this in and add a unit test.
README.md (1)
100-103: Align bullet style with repository conventions and markdownlint MD004.Elsewhere in this README bullets use asterisks; the new note uses dashes and triggers MD004. Switch to asterisks for consistency.
- - Note: - - Use only for multi-node clusters with installer-managed networking. Single-node clusters and clusters with user-managed networking do not require VIPs. - - Ensure hosts are discovered first (after booting with the Discovery ISO) so that matching subnets are known; attempting to set VIPs earlier can result in errors such as "No suitable matching CIDR found for VIP". + + Note: + * Use only for multi-node clusters with installer-managed networking. Single-node clusters and clusters with user-managed networking do not require VIPs. + * Ensure hosts are discovered first (after booting with the Discovery ISO) so that matching subnets are known; attempting to set VIPs earlier can result in errors such as "No suitable matching CIDR found for VIP".doc/example_prompts.md (1)
51-53: Make both examples discovery-gated and use documentation-safe IPs (RFC 5737).The second example omits the discovery prerequisite and the IPs are from private ranges. Using doc-only ranges avoids accidental copy/paste into real networks and keeps both prompts consistent with the Notes.
-**Example Prompts:** -- "Once hosts are discovered for cluster my-cluster, set the API VIP to 192.168.1.100 and ingress VIP to 192.168.1.101" -- "Configure cluster my-cluster with API VIP 10.0.0.10 and ingress VIP 10.0.0.11" +**Example Prompts:** +- "After hosts are discovered for cluster my-cluster, set the API VIP to 198.51.100.100 and the ingress VIP to 198.51.100.101" +- "After hosts are discovered for cluster my-cluster, configure API VIP 203.0.113.10 and ingress VIP 203.0.113.11"
📜 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 (3)
README.md(1 hunks)doc/example_prompts.md(1 hunks)server.py(1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/example_prompts.md
[grammar] ~51-~51: There might be a mistake here.
Context: ... to unknown subnets. Example Prompts: - "Once hosts are discovered for cluster m...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ....168.1.100 and ingress VIP to 192.168.1.101" - "Configure cluster my-cluster with API V...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
README.md
102-102: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
103-103: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
⏰ 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-service-mcp-saas-main-on-pull-request
🔇 Additional comments (2)
README.md (1)
100-103: Clarity boost for users configuring VIPs.The added note succinctly communicates applicability and timing; this should reduce “No suitable matching CIDR found for VIP” churn.
doc/example_prompts.md (1)
47-50: Notes look good and align with Assisted Installer behavior.Applicability (multi-node + IMN) and timing (post-discovery) are clear.
zszabo-rh
force-pushed
the
tool_description_for_vips
branch
from
f51925d to
b8c44cd
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. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
100-103: Standardize list markers and enhance VIP note readability
- Rename the lead-in from “Note:” to “Notes:” to reflect multiple points.
- Switch the two dash-prefixed bullets to asterisk-prefixed bullets to satisfy markdownlint MD004 and match the surrounding list style.
- Wrap the error message in backticks for clarity.
Apply this diff in README.md (lines 100–103):
- Note: - - Use only for multi-node clusters with installer-managed networking. Single-node clusters and clusters with user-managed networking do not require VIPs. - - Ensure hosts are discovered first (after booting with the Discovery ISO) so that matching subnets are known; attempting to set VIPs earlier can result in errors such as "No suitable matching CIDR found for VIP". + Notes: + * Use only for multi-node clusters with installer-managed networking. Single-node clusters and clusters with user-managed networking do not require VIPs. + * Ensure hosts are discovered first (after booting with the Discovery ISO) so that matching subnets are known; attempting to set VIPs earlier can result in errors such as `No suitable matching CIDR found for VIP`.Optional: for consistency with MD004, you may also convert the dash-prefixed sub-bullets under cluster_iso_download_url (lines 117–119) to asterisk-prefixed bullets.
📜 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 (3)
README.md(1 hunks)doc/example_prompts.md(1 hunks)server.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server.py
- doc/example_prompts.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
102-102: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
103-103: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
⏰ 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-service-mcp-saas-main-on-pull-request
zszabo-rh
force-pushed
the
tool_description_for_vips
branch
from
b8c44cd to
76fb675
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. |
|
I'm okay with this. @eranco74 any concerns? |
We have it in the system prompt as well, I don't know if we need it here as well |
zszabo-rh
force-pushed
the
tool_description_for_vips
branch
from
76fb675 to
e831a24
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. |
|
system prompt update should do the trick by itself |
4 participants
Summary by CodeRabbit