MGMT-22503: After selecting Ai-bundle and discovering only amd gpu host the nvdia…

[ElayAharoni]

https://issues.redhat.com/browse/MGMT-22503

Summary by CodeRabbit

• Improvements
  • Operators that belong to bundles are no longer auto-checked; they must be explicitly selected by the user.
  • On initial load, previously monitored operators are automatically populated into the selection so monitored operators appear pre-selected.
  • Disabled-state messaging for bundled operators remains unchanged.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[openshift-ci-robot]

@ElayAharoni: This pull request references MGMT-22503 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.22.0" version, but no target version was set.

Details

In response to this:

https://issues.redhat.com/browse/MGMT-22503

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: ElayAharoni
Once this PR has been reviewed and has the lgtm label, please assign jgyselov for approval. For more information see the Code Review Process.

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

Details

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

OperatorCheckbox no longer counts bundle membership when computing checkbox checked state; it now only considers whether the operator ID is present in values.selectedOperators. OperatorsSelect initializes values.selectedOperators on mount using selectOlmOperators(cluster) and setFieldValue.

Changes

Cohort / File(s)	Change Summary
Operator checkbox checked-state libs/ui-lib/lib/ocm/components/clusterConfiguration/operators/OperatorCheckbox.tsx	Removed isInBundle from the isChecked computation; isChecked now depends solely on whether the operator ID is in values.selectedOperators. isInBundle remains used for disabled/reason messaging.
Operators select form initialization libs/ui-lib/lib/ocm/components/clusterWizard/OperatorsSelect.tsx	Added import of selectOlmOperators; destructured setFieldValue from useFormikContext; added an effect on mount that calls selectOlmOperators(cluster) and, if names are returned, updates values.selectedOperators via setFieldValue once on initial mount. Comments explaining synchronization logic added.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify that removing isInBundle from isChecked aligns with intended UX and doesn't break aggregated bundle behavior.
• Confirm the mount effect in OperatorsSelect runs only once (no dependency leaks) and correctly updates form state for clusters with OLM-monitored operators.
• Check interactions between pre-existing form logic and the new initialization to avoid double-selection or race conditions.

Possibly related PRs

• Filter out hidden operators for select count #2962 — Also modifies OperatorsSelect.tsx and how values.selectedOperators is computed/used.
• MGMT-20354: LSO Operator Visually Unchecked in UI After Toggling Virt Bundle Whil… #2908 — Syncs monitored/OLM-selected operators into form state; related to setFieldValue usage here.

Suggested labels

lgtm, approved

Suggested reviewers

• rawagner
• celdrake
• jgyselov

Poem

🐰 I nudged a box with whiskers twitching light,
I read the cluster names by starlit night,
Now checks reflect only what the form holds true,
A tidy hop — selected ones shine through! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title is incomplete and cuts off mid-sentence ('nvdia…'), making it unclear and not fully descriptive of the actual changes in the pull request.	Complete the title to clearly describe the main change. For example: 'MGMT-22503: Fix NVIDIA GPU operator selection state after AI bundle discovery' would better summarize the bundle-related operator selection fix.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

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

---

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]

@ElayAharoni: This pull request references MGMT-22503 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.22.0" version, but no target version was set.

Details

In response to this:

https://issues.redhat.com/browse/MGMT-22503

Summary by CodeRabbit

• Improvements
• Updated operator selection in cluster configuration to enforce monitoring status requirements. Operators must now be monitored before they can be manually selected.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

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: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

libs/ui-lib/lib/ocm/components/clusterConfiguration/operators/OperatorCheckbox.tsx (1)

193-205: Prevent user interaction with unmonitored operators.

The onChange handler and isDisabled logic don't account for isMonitored, which creates a confusing user experience:

• Users can click an unmonitored operator's checkbox.
• The operator gets added to/removed from selectedOperators.
• But the checkbox immediately appears unchecked again (because isChecked gates on isMonitored).
• This makes the checkbox appear unresponsive or broken.

Recommended fix: Either disable unmonitored operators or filter them in the onChange handler.

Option 1 (preferred): Disable unmonitored operators:

+ const notMonitoredReason = !isMonitored 
+   ? 'This operator is not available for the current hardware configuration.'
+   : undefined;
+
  const disabledReason = isInBundle
    ? 'This operator is part of a bundle and cannot be deselected.'
    : notStandalone
    ? 'This operator cannot be installed as a standalone'
    : parentOperatorName
    ? `This operator is a dependency of ${parentOperatorName}`
-   : getFeatureDisabledReason(featureId);
+   : notMonitoredReason || getFeatureDisabledReason(featureId);

Option 2: Filter in onChange (less recommended as it's less clear to users):

  onChange={(_, checked) => {
+   if (!isMonitored && checked) return;
    setFieldValue(
      'selectedOperators',
      getNewOperators(
        values.selectedOperators,
        operatorId,
        preflightRequirements,
        checked,
        opSpecs,
      ),
    );
  }}

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4010657 and 0b8210c.

📒 Files selected for processing (1)

• libs/ui-lib/lib/ocm/components/clusterConfiguration/operators/OperatorCheckbox.tsx (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

libs/ui-lib/lib/ocm/components/clusterConfiguration/operators/OperatorCheckbox.tsx (1)

libs/ui-lib/lib/common/selectors/clusterSelectors.ts (1)

• selectMonitoredOperators (35-38)

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

• GitHub Check: format
• GitHub Check: lint
• GitHub Check: circular-deps
• GitHub Check: unit-tests
• GitHub Check: tests
• GitHub Check: translation-files

🔇 Additional comments (1)

libs/ui-lib/lib/ocm/components/clusterConfiguration/operators/OperatorCheckbox.tsx (1)

25-30: LGTM!

The import of selectMonitoredOperators is correctly structured and the selector is available from the common module.

[openshift-ci-robot]

@ElayAharoni: This pull request references MGMT-22503 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.22.0" version, but no target version was set.

Details

In response to this:

https://issues.redhat.com/browse/MGMT-22503

Summary by CodeRabbit

• Improvements
• Operator selection in cluster configuration now requires operators to be monitored before they can be manually selected.
• The selection state shown in the UI now reflects monitored status alongside existing selection and bundle membership rules.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

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]

@ElayAharoni: This pull request references MGMT-22503 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.22.0" version, but no target version was set.

Details

In response to this:

https://issues.redhat.com/browse/MGMT-22503

Summary by CodeRabbit

• Improvements
• Operators that belong to bundles are no longer auto-checked; they must be explicitly selected by the user.
• On initial load, previously monitored operators are automatically populated into the selection so monitored operators appear pre-selected.
• Disabled-state messaging for bundled operators remains unchanged.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

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: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ea3257f and e7b5262.

📒 Files selected for processing (2)

• libs/ui-lib/lib/ocm/components/clusterConfiguration/operators/OperatorCheckbox.tsx (1 hunks)
• libs/ui-lib/lib/ocm/components/clusterWizard/OperatorsSelect.tsx (2 hunks)

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

• libs/ui-lib/lib/ocm/components/clusterConfiguration/operators/OperatorCheckbox.tsx

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-21T04:40:36.292Z

Learnt from: linoyaslan
Repo: openshift-assisted/assisted-installer-ui PR: 3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx:55-63
Timestamp: 2025-10-21T04:40:36.292Z
Learning: In libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx, the network reordering logic in the useEffect (swapping clusterNetworks and serviceNetworks based on the primary machine network's IP family) is for UI consistency only. Validation of empty or invalid CIDRs is handled separately by validation schemas, not by the reordering logic.

Applied to files:

• libs/ui-lib/lib/ocm/components/clusterWizard/OperatorsSelect.tsx

🧬 Code graph analysis (1)

libs/ui-lib/lib/ocm/components/clusterWizard/OperatorsSelect.tsx (2)

libs/ui-lib/lib/common/types/clusters.ts (1)

• OperatorsValues (61-64)

libs/ui-lib/lib/common/selectors/clusterSelectors.ts (1)

• selectOlmOperators (40-44)

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

• GitHub Check: lint
• GitHub Check: format
• GitHub Check: tests
• GitHub Check: unit-tests
• GitHub Check: circular-deps
• GitHub Check: translation-files

🔇 Additional comments (3)

libs/ui-lib/lib/ocm/components/clusterWizard/OperatorsSelect.tsx (3)

14-14: LGTM!

The import correctly brings in the selectOlmOperators selector function for filtering monitored OLM operators.

---

40-40: LGTM!

Correctly extends the Formik context destructuring to obtain setFieldValue for form state updates.

---

54-55: The mount-only sync is intentional for preserving user selections.

The empty dependency array correctly ensures the sync runs only once on initial mount to restore previously selected operators (when monitoredOperators.length > 0). This prevents user selections from being overwritten if the cluster prop updates during the wizard step. This is standard wizard UX behavior and the code comments and logic confirm this is by design.