fix: Optimize OpenShift cluster creation workflow#276
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eranco74 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 |
WalkthroughUpdates the cluster creation/control-flow in Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 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}
| * Identify and extract the **cluster name**, **OpenShift version**, **base domain**, and whether it's a **single-node cluster** from the user's input or conversation history. These details must be specified before the cluster is created. | ||
| * Only prompt the user for these specific parameters if they are missing. If all required details are provided in a single message, proceed to create the cluster immediately without asking for confirmation or repeating the parameters back to the user. |
There was a problem hiding this comment.
Add confirmation or parameter echo-back before immediate cluster creation.
While the optimization improves efficiency by reducing friction, proceeding immediately to cluster creation without any confirmation or parameter verification creates risk. If the AI misinterprets user input—such as ambiguous phrasing, hypothetical questions, or incorrect parameter extraction—it could result in unintended cluster creation with wrong configurations.
Consider one of these safeguards:
- Echo-back extracted parameters: Before creating the cluster, briefly confirm: "I'll create a cluster with: name=X, version=Y, domain=Z, single-node=true/false. Proceeding now..."
- Quick confirmation for auto-extracted params: "I found all required parameters. Create cluster now? (yes/no)"
- Confidence threshold: Only skip confirmation if parameters are explicitly stated (not inferred from vague context)
This preserves the streamlined experience while preventing costly mistakes from misinterpretation.
🔎 Suggested refinement to line 288:
- * Only prompt the user for these specific parameters if they are missing. If all required details are provided in a single message, proceed to create the cluster immediately without asking for confirmation or repeating the parameters back to the user.
+ * Only prompt the user for these specific parameters if they are missing. If all required details are provided in a single message, briefly confirm the extracted parameters (cluster name, version, domain, topology) before proceeding with cluster creation.🤖 Prompt for AI Agents
In template.yaml around lines 287 to 288, the flow currently proceeds to create
a cluster immediately when all required parameters are detected; add a safeguard
so the system echoes back the extracted parameters and requests explicit user
confirmation before creating the cluster (or alternatively present a yes/no
prompt when parameters were auto-extracted), and implement a simple confidence
check that only skips confirmation for parameters explicitly stated (not
inferred) — update the prompt logic to show "I will create a cluster with:
name=..., version=..., domain=..., single-node=... Proceed? (yes/no)" and
require affirmative confirmation before proceeding.
|
/retest |
Updated the installation workflow to automatically identify cluster details (name, version, domain, topology) from the user's input or history. - Only prompts for missing parameters. - Reduces friction by removing redundant confirmation prompts. - Triggers immediate creation if all required details are present. Signed-off-by: Eran Cohen <eranco@redhat.com>
- Update OCI and multinode cluster creation tests to reflect the flow asking for SSH keys immediately. - Correct the scope definition in the Azure refusal test to explicitly include OCI support. - Relax expectations for mixed-intent tests to accept full refusals. Signed-off-by: Eran Cohen <eranco@redhat.com>
eranco74
force-pushed
the
optimize_cluster_creation
branch
from
b124654 to
bc988fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
template.yamltest/evals/eval_data.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- template.yaml
🔇 Additional comments (7)
test/evals/eval_data.yaml (7)
130-131: LGTM! Test expectations align with the streamlined workflow.The updated expected response correctly reflects the new SSH-key-centric flow, removing the prior Discovery ISO prompt and instead prompting conditionally for SSH key after cluster creation.
162-162: LGTM! Consistent with the updated cluster creation flow.The response now correctly prompts for SSH key configuration after cluster creation instead of referencing the Discovery ISO.
172-172: LGTM! Consistent with OCI test variant.The expected response matches the OCI cluster creation flow on line 162, correctly testing both the "OCI" abbreviation and the full "Oracle Cloud Infrastructure" term.
260-260: LGTM! More concise description.The shortened description is clearer and more direct.
321-321: LGTM! Formatting consistency improvement.Removing the trailing question mark improves consistency across test queries.
363-363: LGTM! Consistent formatting.Removing the trailing punctuation aligns with the formatting pattern applied elsewhere in the file.
365-365: LGTM! Clearer and more consistent refusal messaging.The updated expected_intent values provide more explicit refusal language and consistent punctuation, improving test clarity.
Also applies to: 370-370
| eval_types: [response_eval:intent] | ||
| expected_intent: An explanation that indicates clusters cannot be created on public clouds and a reminder that the scope of this installer is on-premise installations | ||
| description: Refuse out-of-scope request to create clusters on Azure; clarify on-prem scope and Assisted Installer | ||
| expected_intent: An explanation that indicates clusters cannot be created on unsupported public clouds (like Azure) and a clarification that the scope of this installer is limited to Bare Metal, vSphere, Nutanix abd OCI. |
There was a problem hiding this comment.
Fix typo in expected_intent.
There's a typographical error: "Nutanix abd OCI" should be "Nutanix and OCI".
🔎 Proposed fix
- expected_intent: An explanation that indicates clusters cannot be created on unsupported public clouds (like Azure) and a clarification that the scope of this installer is limited to Bare Metal, vSphere, Nutanix abd OCI.
+ expected_intent: An explanation that indicates clusters cannot be created on unsupported public clouds (like Azure) and a clarification that the scope of this installer is limited to Bare Metal, vSphere, Nutanix and OCI.📝 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.
| expected_intent: An explanation that indicates clusters cannot be created on unsupported public clouds (like Azure) and a clarification that the scope of this installer is limited to Bare Metal, vSphere, Nutanix abd OCI. | |
| expected_intent: An explanation that indicates clusters cannot be created on unsupported public clouds (like Azure) and a clarification that the scope of this installer is limited to Bare Metal, vSphere, Nutanix and OCI. |
🤖 Prompt for AI Agents
In test/evals/eval_data.yaml around line 259, the expected_intent string
contains a typo "Nutanix abd OCI"; update it to "Nutanix and OCI" so the
sentence reads "...Bare Metal, vSphere, Nutanix and OCI." Ensure spacing and
punctuation remain consistent with the surrounding YAML formatting.
|
@eranco74: 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. |
|
/lgtm |
openshift-merge-bot
bot
merged commit e8b8b20
into
rh-ecosystem-edge:main
2 participants
Updated the installation workflow to automatically identify cluster details (name, version, domain, topology) from the user's input or history.
Summary by CodeRabbit
Refactor
Documentation / Messaging
✏️ Tip: You can customize this high-level summary in your review settings.