MGMT-22245: eval-test update for the SSH key fix

[zszabo-rh]

SSH key fix requires updating the eval test, as cluster creation and applying the optional SSH key are now performed through two separate tool calls.

Related to MGMT-22245

Summary by CodeRabbit

• New Features

  • Adds a Pre-ISO Configuration flow that always checks and optionally offers SSH key and static network setup together before Discovery ISO download, validates network input, generates a reviewable network configuration, and requires user confirmation; ISO offer proceeds if none chosen.
  • Adds explicit mandatory pre‑flight checks guidance for cluster creation.

• Tests

  • Clarified user-facing messaging: cluster creation responses now offer configuration options before ISO and report when a provided SSH key is invalid but the cluster was created.

[openshift-ci-robot]

@zszabo-rh: This pull request references MGMT-22245 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.21.0" version, but no target version was set.

In response to this:

SSH key fix requires updating the eval test, as cluster creation and applying the optional SSH key are now performed through two separate tool calls.

Related to MGMT-22245

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.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zszabo-rh
Once this PR has been reviewed and has the lgtm label, please assign jhernand for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Walkthrough

Changed eval expectations to use intent-based evaluation for multinode creation and updated invalid SSH key wording; replaced the static-network reminder in the template with a Pre‑ISO Configuration flow that always checks and optionally offers SSH key and static network configuration (with validation, NMState YAML generation, and explicit user confirmation) before offering the Discovery ISO.

Changes

Cohort / File(s)	Summary
Test eval adjustments test/evals/eval_data.yaml	Switched multinode eval_types to use response_eval:intent (replacing accuracy-based actionEval), updated expected outcome to an expected_intent describing cluster creation success with offers to configure SSH key and static network before Discovery ISO, and changed invalid-SSH-key case to an expected_intent that the cluster was created but the provided SSH key is invalid and was not set.
Pre-ISO configuration flow (template) template.yaml	Replaced prior static-network reminder with a Pre-ISO Configuration (Always Check and Offer) flow that: always checks and optionally offers SSH Key Configuration and Static Network Configuration (each skipped if already addressed); presents both options together before ISO download; adds Static Network Configuration Details (validate existing config, generate NMState YAML per host with default interface naming, allow user tweaks, require user confirmation before applying); and introduces a separate Mandatory Pre‑Flight Checks section.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Assistant
    participant PreISO as Pre-ISO Config
    participant NMGen as NMState Generator
    participant ISO as ISO Offer

    User->>Assistant: Request Discovery ISO / start cluster creation
    Assistant->>PreISO: Check for unanswered pre-ISO options (SSH key, static network)
    PreISO-->>Assistant: Determine which options to offer (none / SSH / Static / both)
    alt Options present
        Assistant->>User: Offer SSH Key and Static Network configuration together (optional)
        alt User provides SSH key
            Assistant->>Assistant: Validate SSH public key format
            Assistant-->>User: Confirm SSH key set or report invalid key
        end
        alt User requests static network config
            Assistant->>NMGen: Generate NMState YAML per host (default interface names)
            NMGen-->>Assistant: Proposed NMState YAML
            Assistant->>User: Show YAML, allow tweaks, ask for confirmation
            alt User confirms
                Assistant->>PreISO: Mark static network configured (do not apply without user action)
            else User declines or no response
                Assistant->>PreISO: Skip static network configuration
            end
        end
        Assistant->>ISO: Offer ISO download (after optional flows complete or skipped)
    else No options
        Assistant->>ISO: Offer ISO directly
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas needing attention:
  • template.yaml — verify branching, gating logic for optional steps, NMState YAML generation wording, and explicit requirement that YAML must be confirmed before applying.
  • test/evals/eval_data.yaml — ensure eval_types and expected_intent entries align with the new template wording and cover edge cases (invalid SSH key vs cluster creation success).

Possibly related PRs

• MGMT-21887: personality change refusal #248 — Adjusts eval_data expectations to intent-based evaluations and includes invalid SSH key handling changes similar to this PR.
• changing the prompt so the host_booted_but_not_discovered test would be more consistent #223 — Modifies the static network / pre-ISO configuration flow and related eval tests; overlaps template and eval changes.
• MGMT-21240: improve eval test coverage #182 — Overlaps multinode cluster evaluation flow changes including SSH key and ISO download steps.

Suggested labels

approved, lgtm

Suggested reviewers

• keitwb
• maorfr

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly references the SSH key fix and evaluation test updates, which align with the PR's main objective of updating eval tests to reflect separated cluster creation and SSH key application operations.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci-robot]

@zszabo-rh: This pull request references MGMT-22245 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.21.0" version, but no target version was set.

In response to this:

SSH key fix requires updating the eval test, as cluster creation and applying the optional SSH key are now performed through two separate tool calls.

Related to MGMT-22245

Summary by CodeRabbit

• Tests
• Updated SSH key validation test expectations to reflect improved error messaging. Users will now receive clearer notification when cluster creation succeeds but SSH key configuration fails due to invalid format, rather than a generic invalid key error.

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.

[eranco74]

/lgtm
Consider updating the system prompt so that the chat offer to add the ssh key after the cluster is created (prior to iso download)

[openshift-ci-robot]

@zszabo-rh: This pull request references MGMT-22245 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.21.0" version, but no target version was set.

In response to this:

SSH key fix requires updating the eval test, as cluster creation and applying the optional SSH key are now performed through two separate tool calls.

Related to MGMT-22245

Summary by CodeRabbit

• New Features
• Introduces a Pre-ISO Configuration flow that proactively offers optional SSH key and static network setup before ISO download, validates network input, generates configuration, and requires user confirmation; unanswered options are aggregated and the ISO offer proceeds if no response.
• Tests
• Updated SSH key validation tests and messaging so users are informed when a cluster is created but SSH key configuration fails rather than receiving a generic invalid-key error.

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.

[zszabo-rh]

@eranco74

Consider updating the system prompt so that the chat offer to add the ssh key after the cluster is created (prior to iso download)

Done.. partially
Now the chatbot offers both options after a simple cluster creation, e.g.:

...
Before downloading the Discovery ISO, would you like to configure an SSH public key for the cluster nodes? This will enable SSH access to the nodes during and after installation.
Also, by default the cluster will use Dynamic Host Configuration Protocol (DHCP) for host networking. Would you like to configure static network configuration?

However I couldn't make it to offer static networking when SSH key was already specified together with the cluster creation request, no matter how strict rules I created. I've tried with several variations, but the outcome was always something like this:

... have also set the provided SSH key.
Next, I can generate the Discovery ISO, which you can use to boot your host. After the host boots, it will be automatically assigned the `master` role. Would you like me to generate the Discovery ISO now?

[openshift-ci-robot]

@zszabo-rh: This pull request references MGMT-22245 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.21.0" version, but no target version was set.

In response to this:

SSH key fix requires updating the eval test, as cluster creation and applying the optional SSH key are now performed through two separate tool calls.

Related to MGMT-22245

Summary by CodeRabbit

• New Features

• Adds a Pre-ISO Configuration flow that always checks and optionally offers SSH key and static network setup before ISO download, validates network input, generates a network configuration for review, and requires user confirmation; unanswered options are shown together and the ISO offer proceeds if none are chosen.

• Adds explicit mandatory pre‑flight checks guidance for cluster creation.

• Tests

• Updated SSH key validation messaging so users are informed if the cluster is created but the provided SSH key could not be set.

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.

[openshift-ci-robot]

@zszabo-rh: This pull request references MGMT-22245 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.21.0" version, but no target version was set.

In response to this:

SSH key fix requires updating the eval test, as cluster creation and applying the optional SSH key are now performed through two separate tool calls.

Related to MGMT-22245

Summary by CodeRabbit

• New Features

• Adds a Pre-ISO Configuration flow that always checks and optionally offers SSH key and static network setup together before Discovery ISO download, validates network input, generates a reviewable network configuration, and requires user confirmation; ISO offer proceeds if none chosen.

• Adds explicit mandatory pre‑flight checks guidance for cluster creation.

• Tests

• Clarified user-facing messaging: cluster creation responses now offer configuration options before ISO and report when a provided SSH key is invalid but the cluster was created.

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]

Actionable comments posted: 0

🧹 Nitpick comments (2)

template.yaml (2)

256-262: Header wording should reflect the conditional nature of the offers.

The new Pre-ISO Configuration section correctly implements the SSH key fix by separating SSH key application from cluster creation and offering both as optional post-creation steps. However, the section header "Pre-ISO Configuration (Always Check and Offer)" is slightly misleading—these configurations are offered conditionally only if not already addressed by the user. Consider revising the header to something like "Pre-ISO Configuration (Check and Offer Optional Steps)" or "Pre-ISO Configuration (Conditionally Offer Remaining Steps)".

The instructions in lines 257–262 are otherwise clear and well-aligned with the PR objective: the workflow correctly distinguishes SSH key setup from cluster creation and treats both as independent optional configurations.

Consider revising the section header to better reflect the conditional logic:

- **Pre-ISO Configuration (Always Check and Offer)**
- * After cluster creation and before offering the Discovery ISO, proactively check which of the following optional configuration steps have not yet been addressed with the user, and offer those that remain:
+ **Pre-ISO Configuration (Check and Offer Remaining Optional Steps)**
+ * After cluster creation and before offering the Discovery ISO, proactively check which of the following optional configuration steps have not yet been addressed with the user, and offer only those that remain:

---

275-276: Section header mismatch: content discusses static networking distinction rather than pre-flight checks for cluster creation.

The "Mandatory Pre-Flight Checks for Cluster Creation" header (line 275) does not accurately describe the content in lines 275–276, which clarifies the distinction between static networking and user-managed networking (API/Ingress VIP configuration). This note is valuable for preventing confusion between two different concepts, but it appears somewhat disconnected from the preceding Pre-ISO Configuration section and its placement/framing is unclear.

Consider reorganizing this section or renaming it to better reflect its actual purpose (e.g., "Important Distinction: Static Networking vs. User-Managed Networking" or moving it to a clearer location in the workflow where VIP configuration is discussed).

- **Mandatory Pre-Flight Checks for Cluster Creation**
- * **Important Distinction:** Do not confuse static networking and user-managed networking. API and Ingress VIPs are set when user-managed networking is disabled in multi-node clusters. Static networking is specific to individual hosts and must be configured before downloading the Discovery ISO.
+ **Important Distinction: Static Networking vs. User-Managed Networking**
+ * Do not confuse static networking and user-managed networking. API and Ingress VIPs are set when user-managed networking is disabled in multi-node clusters. Static networking is specific to individual hosts and must be configured before downloading the Discovery ISO.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60263b8 and 063bee0.

📒 Files selected for processing (2)

• template.yaml (1 hunks)
• test/evals/eval_data.yaml (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• test/evals/eval_data.yaml

⏰ 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). (2)

• GitHub Check: Red Hat Konflux / assisted-chat-saas-main-on-pull-request
• GitHub Check: Red Hat Konflux / assisted-chat-test-image-saas-main-on-pull-request

[openshift-ci]

@zszabo-rh: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/eval-test	063bee0	link	false	/test eval-test

Full PR test history. Your PR dashboard.

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

[zszabo-rh]

/hold checking a different approach first