MGMT-23131:Third-Party CNI Support#3457
Merged
openshift-merge-bot[bot] merged 2 commits intoopenshift-assisted:masterfrom Mar 16, 2026
Merged
MGMT-23131:Third-Party CNI Support#3457openshift-merge-bot[bot] merged 2 commits intoopenshift-assisted:masterfrom
openshift-merge-bot[bot] merged 2 commits intoopenshift-assisted:masterfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@LiorSoffer: This pull request references MGMT-23131 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 story to target the "4.22.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. |
openshift-ci
bot
added
the
size/XL
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR refactors network type selection from separate SDN/OVN fields to a unified dropdown component, simplifies custom manifests handling by removing wizard-level toggles and integrating configuration into a dedicated page, expands type definitions with support for disconnected clusters, incompatibility reporting, operators, and third-party CNIs, and updates related tests and utilities accordingly. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Form as NetworkTypeDropdown
participant Formik as Formik Field
participant FeatureCheck as useNewFeatureSupportLevel
participant UpdateUI as Update Cluster
User->>Form: Open network type dropdown
Form->>FeatureCheck: Check SDN disable conditions<br/>(SNO, IPv6, feature support)
FeatureCheck-->>Form: SDN disabled status
alt SDN disabled
Form->>Form: Show SDN option with disabled tooltip
else SDN enabled
Form->>Form: Enable SDN option
end
User->>Form: Select network type
Form->>Formik: Update field value
alt Selected SDN but disabled
Formik-->>Form: Redirect to OVN
else Valid selection
Formik-->>Form: Confirm value
end
Form->>UpdateUI: Trigger form state change
UpdateUI-->>User: Network type updated
sequenceDiagram
actor User
participant Page as CustomManifestsPage
participant Platform as isExternalPlatform
participant CNI as isThirdPartyCNI
participant Wizard as useClusterWizardContext
participant API as ClustersAPI/Service
participant Modal as DeleteCustomManifestModal
Page->>Platform: Check if external platform
Page->>CNI: Check if third-party CNI
Platform-->>Page: External platform status
CNI-->>Page: Third-party CNI status
Page->>Page: Render switch (disabled if required)
alt External platform OR third-party CNI
Page->>Page: Show contextual alert
end
User->>Page: Toggle custom manifests switch
alt Switch OFF and manifests exist
Page->>Modal: Open delete confirmation
User->>Modal: Confirm deletion
Modal->>API: Fetch current manifests
API-->>Modal: Manifest list
Modal->>API: Delete manifests
API-->>Modal: Deletion complete
Modal->>Wizard: Update context
Wizard-->>Page: Refresh state
else Switch ON
Page->>Page: Show manifest upload form
end
User->>Page: Update manifests
Page->>UpdateUI: Notify form state change
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use your project's `biome` configuration to improve the quality of JS/TS/CSS/JSON code reviews.Add a configuration file to your project to customize how CodeRabbit runs |
|
@LiorSoffer: This pull request references MGMT-23131 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 story to target the "4.22.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: 4
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/clusterWizard/ClusterDetails.tsx (1)
101-115:⚠️ Potential issue | 🟠 MajorPersist the assisted-migration UI settings before navigating.
The route change happens before the
uiPatchwrites complete. That lets the next screen mount against stale UI settings, and any failure in either settings call falls into the "Failed to create new cluster" path even though the cluster was already created.💡 Suggested sequencing
const cluster = await ClustersService.create( params, isAssistedMigration, isSingleClusterFeatureEnabled, ); - navigate(`../${cluster.id}`, { state: ClusterWizardFlowStateNew }); if (isAssistedMigration) { const uiPatch: UISettingsValues = { bundlesSelected: ['virtualization'], isAssistedMigration: true, }; await UISettingService.update(cluster.id, uiPatch); await clusterWizardContext.updateUISettings(uiPatch); } + navigate(`../${cluster.id}`, { state: ClusterWizardFlowStateNew });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetails.tsx` around lines 101 - 115, The navigate call happens before persisting UI settings so the next screen may see stale settings and any settings failure incorrectly treats cluster creation as failed; move the navigate(`../${cluster.id}`, ...) to after awaiting both UISettingService.update(cluster.id, uiPatch) and clusterWizardContext.updateUISettings(uiPatch) when isAssistedMigration is true, and ensure errors from the settings calls are handled separately (so failures in UISettingService.update or clusterWizardContext.updateUISettings don’t trigger the "Failed to create new cluster" error path for ClustersService.create); update the code paths involving ClustersService.create, UISettingService.update, clusterWizardContext.updateUISettings, and navigate accordingly.
🧹 Nitpick comments (1)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx (1)
154-155: Drop the temporary logs before merge.These fire on every UI-settings refresh and will spam the browser console in normal wizard usage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx` around lines 154 - 155, The two temporary console.log calls inside ClusterWizardContextProvider (logging customManifestsStepNeedsToBeFilled and requiredStepId) should be removed to avoid spamming the browser console; locate the console.log('customManifestsStepNeedsToBeFilled', customManifestsStepNeedsToBeFilled) and console.log('requiredStepId', requiredStepId) statements and delete them (or replace them with a conditional/debug logger behind an explicit debug flag if you need runtime diagnostics).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/types/assisted-installer-service.d.ts`:
- Around line 219-220: ClusterStatus.tsx's ClusterStatusIcon component is
missing an explicit case for the new 'unmonitored' status and falls through to
the default empty fragment; add a case for 'unmonitored' inside the switch in
ClusterStatusIcon (the same switch that handles other statuses between lines
~25-70) and return the appropriate icon component (matching the style used by
other statuses) so that 'unmonitored' renders an icon consistent with
clusterStatusLabels; ensure the case uses the exact string 'unmonitored' and
follows the same props/size/color conventions as the other status icon cases.
In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx`:
- Around line 63-69: In cleanCustomManifests() the call to
ClustersService.removeClusterManifests(manifests, cluster.id) is
fire-and-forget; await it and handle failures before mutating UI state—wrap the
await ClustersService.removeClusterManifests(...) in a try/catch, only call
setUseCustomManifests(false), setDeleteModalOpen(false), and await
clusterWizardContext.updateUISettings({ customManifestsAdded: false }) after the
await succeeds, and in the catch log or surface the error (and avoid flipping
the wizard flag on failure) so stale manifests aren't left while the UI reports
they were removed.
In
`@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx`:
- Line 118: The provider allocates and manipulates the customManifestsStep state
(customManifestsStep, setCustomManifestsStep) but ClusterWizardContextType no
longer includes those fields, causing a type mismatch when building
contextValue; remove the state allocation and all uses (initialization, reset,
updater calls) and stop returning these properties from the context object in
ClusterWizardContextProvider (also remove the debug artifacts related to
customManifestsStep), or alternatively add customManifestsStep and
setCustomManifestsStep back into ClusterWizardContextType if they are
required—update the contextValue construction (variable contextValue) and any
places referencing customManifestsStep or setCustomManifestsStep accordingly so
the returned object matches ClusterWizardContextType exactly.
In `@libs/ui-lib/lib/ocm/components/utils.ts`:
- Around line 14-15: Remove the local duplicate isThirdPartyCNI implementation
in the CustomManifestsPage component and instead import the shared
isThirdPartyCNI utility exported by the utils module; update imports to
reference isThirdPartyCNI and use that function wherever the local version was
used (search for isThirdPartyCNI in CustomManifestsPage) so the file relies on
the centralized implementation rather than a duplicated local one.
---
Outside diff comments:
In `@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetails.tsx`:
- Around line 101-115: The navigate call happens before persisting UI settings
so the next screen may see stale settings and any settings failure incorrectly
treats cluster creation as failed; move the navigate(`../${cluster.id}`, ...) to
after awaiting both UISettingService.update(cluster.id, uiPatch) and
clusterWizardContext.updateUISettings(uiPatch) when isAssistedMigration is true,
and ensure errors from the settings calls are handled separately (so failures in
UISettingService.update or clusterWizardContext.updateUISettings don’t trigger
the "Failed to create new cluster" error path for ClustersService.create);
update the code paths involving ClustersService.create, UISettingService.update,
clusterWizardContext.updateUISettings, and navigate accordingly.
---
Nitpick comments:
In
`@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx`:
- Around line 154-155: The two temporary console.log calls inside
ClusterWizardContextProvider (logging customManifestsStepNeedsToBeFilled and
requiredStepId) should be removed to avoid spamming the browser console; locate
the console.log('customManifestsStepNeedsToBeFilled',
customManifestsStepNeedsToBeFilled) and console.log('requiredStepId',
requiredStepId) statements and delete them (or replace them with a
conditional/debug logger behind an explicit debug flag if you need runtime
diagnostics).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9bcf45ab-4c15-41c3-ad22-d77c9bfe32a5
📒 Files selected for processing (28)
libs/locales/lib/en/translation.jsonlibs/types/assisted-installer-service.d.tslibs/ui-lib-tests/cypress/integration/create-multinode/2-networking.cy.tslibs/ui-lib-tests/cypress/integration/create-sno/2-networking.cy.tslibs/ui-lib-tests/cypress/integration/custom-manifests/1-create-cluster-with-custom-manifests.cy.tslibs/ui-lib-tests/cypress/integration/custom-manifests/2-modifying-existing-custom-manifest.cy.tslibs/ui-lib-tests/cypress/support/interceptors.tslibs/ui-lib-tests/cypress/support/variables/networking.tslibs/ui-lib-tests/cypress/views/forms/CustomManifests/CustomManifestsForm.tslibs/ui-lib-tests/cypress/views/networkingPage.tslibs/ui-lib/lib/common/config/constants.tslibs/ui-lib/lib/common/types/ui-settings.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/CustomManifestCheckbox.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/OcmClusterDetailsFormFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropDown.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetails.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetailsForm.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/constants.tslibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.tslibs/ui-lib/lib/ocm/components/utils.tslibs/ui-lib/lib/ocm/hooks/useUISettings.tslibs/ui-lib/lib/ocm/services/ClusterDetailsService.ts
💤 Files with no reviewable changes (5)
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/CustomManifestCheckbox.tsx
- libs/ui-lib/lib/common/types/ui-settings.ts
- libs/ui-lib-tests/cypress/integration/custom-manifests/1-create-cluster-with-custom-manifests.cy.ts
- libs/ui-lib/lib/ocm/hooks/useUISettings.ts
...i-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx
Outdated
Show resolved
Hide resolved
|
@LiorSoffer: This pull request references MGMT-23131 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 story to target the "4.22.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: 4
♻️ Duplicate comments (3)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx (1)
296-297:⚠️ Potential issue | 🔴 CriticalContext value contains fields not defined in
ClusterWizardContextType.The provider returns
customManifestsStepandsetCustomManifestsStepin the context value, butClusterWizardContextType(defined inClusterWizardContext.tsx) doesn't include these fields. Either add these fields to the type definition or remove them from the context value if they're no longer needed externally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx` around lines 296 - 297, The context value returned by ClusterWizardContextProvider includes customManifestsStep and setCustomManifestsStep (set via onSetAddCustomManifestsStep) but ClusterWizardContextType doesn't declare them; update the type definition in ClusterWizardContext.tsx to include customManifestsStep (correct type) and setCustomManifestsStep (function type) so the provider's value matches the interface, or if these fields are internal remove them from ClusterWizardContextProvider instead—locate the symbols ClusterWizardContextProvider, customManifestsStep, setCustomManifestsStep, onSetAddCustomManifestsStep, and ClusterWizardContextType and make the type/signature change accordingly.libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx (2)
24-25: 🛠️ Refactor suggestion | 🟠 MajorConsolidate duplicate
isThirdPartyCNIimplementation by importing from utils.This function is already exported from
libs/ui-lib/lib/ocm/components/utils.ts. Replace the local definition with an import to maintain a single source of truth, consistent with other files in this PR (e.g.,NetworkTypeDropDown.tsx,ClusterWizardContextProvider.tsx).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx` around lines 24 - 25, Remove the local isThirdPartyCNI definition in CustomManifestsPage.tsx and import the existing exported isThirdPartyCNI helper from the shared utils module (the same function used by NetworkTypeDropDown and ClusterWizardContextProvider); update any references to use the imported isThirdPartyCNI and ensure the import is added to the file's imports list so there is a single source of truth for that logic.
63-70:⚠️ Potential issue | 🟠 MajorAwait manifest deletion before clearing
customManifestsAdded.
removeClusterManifests()is fire-and-forget here. If deletion fails, the UI will already show manifests as removed while they still exist on the cluster, leaving the UI and backend out of sync.💡 Suggested fix
const cleanCustomManifests = React.useCallback(async () => { if (!clusterWizardContext.uiSettings?.customManifestsAdded || !cluster.id) return; - const { data: manifests } = await ClustersAPI.getManifests(cluster.id); - void ClustersService.removeClusterManifests(manifests, cluster.id); - setUseCustomManifests(false); - setDeleteModalOpen(false); - await clusterWizardContext.updateUISettings({ customManifestsAdded: false }); + try { + const { data: manifests } = await ClustersAPI.getManifests(cluster.id); + await ClustersService.removeClusterManifests(manifests, cluster.id); + setUseCustomManifests(false); + setDeleteModalOpen(false); + await clusterWizardContext.updateUISettings({ customManifestsAdded: false }); + } catch (e) { + // Handle error - keep modal open and show error to user + console.error('Failed to remove manifests:', e); + } }, [cluster.id, clusterWizardContext]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx` around lines 63 - 70, cleanCustomManifests is calling ClustersService.removeClusterManifests fire-and-forget, so the UI and clusterWizardContext.updateUISettings({ customManifestsAdded: false }) are applied before deletion completes; modify cleanCustomManifests to await the removal (await ClustersService.removeClusterManifests(manifests, cluster.id)) and wrap it in try/catch so you only call setUseCustomManifests(false), setDeleteModalOpen(false) and clusterWizardContext.updateUISettings(...) after successful removal, and handle/log the error (e.g., show user notification) if removal fails so the UI stays in sync with the backend.
🧹 Nitpick comments (5)
libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts (1)
20-28: Consider derivingdefaultWizardStepsfrom the shared step order.This step now has to stay in sync with the separate ordering in
wizardTransition.ts. Centralizing the order and filtering per flow would make future insertions less error-prone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts` around lines 20 - 28, defaultWizardSteps is hard-coded and duplicates the canonical order defined elsewhere (wizardTransition.ts); change it to derive from the shared ordering source (e.g., the exported stepOrder/stepSequence in wizardTransition.ts) and then filter or map that sequence to ClusterWizardStepsType as needed; update the constant (defaultWizardSteps) to import the shared order, apply any flow-specific filters (remove steps not applicable to this flow) and ensure the resulting array preserves the canonical order so future insertions only need to change the single source of truth.libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetails.tsx (1)
109-114: Potential inconsistency between direct service call and context update.
UISettingService.updateperforms a full replace (not merge), whileclusterWizardContext.updateUISettingsperforms a merge. Calling both in sequence creates redundancy and potential inconsistency:
- Line 113: Directly replaces all UI settings with
{ bundlesSelected, isAssistedMigration }- Line 114: Merges the same values into local state
For a newly created cluster this is likely safe since no prior settings exist, but consider using only
clusterWizardContext.updateUISettingswhich handles both persistence and state update with proper merging.♻️ Suggested fix
if (isAssistedMigration) { const uiPatch: UISettingsValues = { bundlesSelected: ['virtualization'], isAssistedMigration: true, }; - await UISettingService.update(cluster.id, uiPatch); await clusterWizardContext.updateUISettings(uiPatch); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetails.tsx` around lines 109 - 114, The code calls UISettingService.update (which performs a full replace) then clusterWizardContext.updateUISettings (which merges) causing redundancy and potential inconsistency; change the flow to use only clusterWizardContext.updateUISettings with the uiPatch to ensure proper merge and state update, removing the direct UISettingService.update(cluster.id, uiPatch) call (or, if you must call the service directly, change clusterWizardContext.updateUISettings to accept the persisted result) so that persistence and local state are handled consistently via clusterWizardContext.updateUISettings and references to uiPatch and cluster.id remain accurate.libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx (2)
51-56: Minor: Remove stray blank line from function signature.Line 54 appears to be leftover formatting from removing the
customManifestsStepparameter.♻️ Suggested fix
const getWizardStepIds = ( wizardStepIds: ClusterWizardStepsType[] | undefined, staticIpView?: StaticIpView | 'dhcp-selected', - isSingleClusterFeatureEnabled?: boolean, ): ClusterWizardStepsType[] => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx` around lines 51 - 56, The function signature for getWizardStepIds contains a stray blank line (leftover from removing customManifestsStep); edit the getWizardStepIds declaration to remove the blank line so all parameters (wizardStepIds: ClusterWizardStepsType[] | undefined, staticIpView?: StaticIpView | 'dhcp-selected', isSingleClusterFeatureEnabled?: boolean) appear consecutively on the same signature block without an extra blank line or stray formatting.
171-171: Redundant state update —customManifestsStepis already initialized totrue.The state is initialized as
trueat line 118, and this unconditionalsetCustomManifestsStep(true)call in the effect appears to serve no purpose since it always sets to the same value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx` at line 171, The effect in ClusterWizardContextProvider unconditionally calls setCustomManifestsStep(true) even though customManifestsStep is initialized to true; remove the redundant setCustomManifestsStep(true) call from the effect (or guard it with if (!customManifestsStep) setCustomManifestsStep(true)) to avoid unnecessary state updates and re-renders; look for the effect that contains setCustomManifestsStep and update it accordingly.libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx (1)
39-43: Consider narrowing the useEffect dependency to avoid unnecessary re-renders.Including the entire
clusterWizardContextobject causes this effect to re-run whenever any context value changes. Consider depending only onclusterWizardContext.uiSettings?.customManifestsAdded.♻️ Suggested fix
+ const customManifestsAdded = clusterWizardContext.uiSettings?.customManifestsAdded; + React.useEffect(() => { - if (isRequired || clusterWizardContext.uiSettings?.customManifestsAdded) { + if (isRequired || customManifestsAdded) { setUseCustomManifests(true); } - }, [isRequired, clusterWizardContext]); + }, [isRequired, customManifestsAdded]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx` around lines 39 - 43, The effect in CustomManifestsPage re-runs on any change to the whole clusterWizardContext object; narrow the dependency to only the needed value by replacing clusterWizardContext in the dependency array with clusterWizardContext.uiSettings?.customManifestsAdded (or destructure const { uiSettings } = clusterWizardContext and depend on uiSettings?.customManifestsAdded) so the React.useEffect that setsUseCustomManifests only runs when isRequired or customManifestsAdded actually change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/types/assisted-installer-service.d.ts`:
- Around line 3039-3044: The cpuArchitecture union in the
V2SupportLevelsFeaturesDetailed interface is missing 'aarch64', causing valid
ARM queries to be rejected; update the cpuArchitecture field in
V2SupportLevelsFeaturesDetailed to include 'aarch64' alongside 'arm64' (matching
the sibling types V2SupportLevelsFeatures and V2OperatorsBundles) so the type
accepts that architecture.
- Around line 1380-1383: Remove the optional fencingCredentials field from the
Host read model types (Host and HostRegistrationResponse) to prevent BMC secrets
leaking into read paths; keep fencingCredentials only in the write/update
contract (HostUpdateParams) and replace the removed field in read models with a
boolean flag like hasFencingCredentials or a redacted metadata object to
indicate presence without exposing secrets; update any serialization, mapping,
and API response construction functions that currently copy fencingCredentials
into Host or HostRegistrationResponse to instead set the presence flag or
redacted metadata, and ensure tests and type references are updated to reflect
the removed field.
In `@libs/ui-lib-tests/cypress/views/networkingPage.ts`:
- Around line 280-282: The tests still call removed getters
getOvnNetworkingField() and getSdnNetworkingField() while the page object now
only exposes getNetworkTypeSelect(); to fix, either add temporary compatibility
wrapper methods named getOvnNetworkingField and getSdnNetworkingField in
networkingPage that delegate to the new selectors (or call
getNetworkTypeSelect() and then narrow to the correct option/control), or update
the spec file libs/ui-lib-tests/cypress/integration/dualstack/2-networking.cy.ts
to use getNetworkTypeSelect() directly where those methods are referenced (lines
around the earlier 42–43 and 74–75 references); implement one of these so the
tests don’t break.
In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx`:
- Around line 72-74: tooltipWhenDisabled currently uses overly specific wording
mentioning "Oracle Cloud Infrastructure" while isRequired is derived from
isExternalPlatform(cluster) which applies to any external platform; update the
tooltipWhenDisabled string to use a more general phrase (e.g., "external
platforms" or "third-party/cloud platforms") so it correctly reflects
isExternalPlatform(cluster). Locate the tooltipWhenDisabled constant and the
isRequired usage in CustomManifestsPage (and any related UI copy that refers to
Oracle Cloud Infrastructure) and replace the specific OCI text with the
generalized wording.
---
Duplicate comments:
In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx`:
- Around line 24-25: Remove the local isThirdPartyCNI definition in
CustomManifestsPage.tsx and import the existing exported isThirdPartyCNI helper
from the shared utils module (the same function used by NetworkTypeDropDown and
ClusterWizardContextProvider); update any references to use the imported
isThirdPartyCNI and ensure the import is added to the file's imports list so
there is a single source of truth for that logic.
- Around line 63-70: cleanCustomManifests is calling
ClustersService.removeClusterManifests fire-and-forget, so the UI and
clusterWizardContext.updateUISettings({ customManifestsAdded: false }) are
applied before deletion completes; modify cleanCustomManifests to await the
removal (await ClustersService.removeClusterManifests(manifests, cluster.id))
and wrap it in try/catch so you only call setUseCustomManifests(false),
setDeleteModalOpen(false) and clusterWizardContext.updateUISettings(...) after
successful removal, and handle/log the error (e.g., show user notification) if
removal fails so the UI stays in sync with the backend.
In
`@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx`:
- Around line 296-297: The context value returned by
ClusterWizardContextProvider includes customManifestsStep and
setCustomManifestsStep (set via onSetAddCustomManifestsStep) but
ClusterWizardContextType doesn't declare them; update the type definition in
ClusterWizardContext.tsx to include customManifestsStep (correct type) and
setCustomManifestsStep (function type) so the provider's value matches the
interface, or if these fields are internal remove them from
ClusterWizardContextProvider instead—locate the symbols
ClusterWizardContextProvider, customManifestsStep, setCustomManifestsStep,
onSetAddCustomManifestsStep, and ClusterWizardContextType and make the
type/signature change accordingly.
---
Nitpick comments:
In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx`:
- Around line 39-43: The effect in CustomManifestsPage re-runs on any change to
the whole clusterWizardContext object; narrow the dependency to only the needed
value by replacing clusterWizardContext in the dependency array with
clusterWizardContext.uiSettings?.customManifestsAdded (or destructure const {
uiSettings } = clusterWizardContext and depend on
uiSettings?.customManifestsAdded) so the React.useEffect that
setsUseCustomManifests only runs when isRequired or customManifestsAdded
actually change.
In `@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetails.tsx`:
- Around line 109-114: The code calls UISettingService.update (which performs a
full replace) then clusterWizardContext.updateUISettings (which merges) causing
redundancy and potential inconsistency; change the flow to use only
clusterWizardContext.updateUISettings with the uiPatch to ensure proper merge
and state update, removing the direct UISettingService.update(cluster.id,
uiPatch) call (or, if you must call the service directly, change
clusterWizardContext.updateUISettings to accept the persisted result) so that
persistence and local state are handled consistently via
clusterWizardContext.updateUISettings and references to uiPatch and cluster.id
remain accurate.
In
`@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx`:
- Around line 51-56: The function signature for getWizardStepIds contains a
stray blank line (leftover from removing customManifestsStep); edit the
getWizardStepIds declaration to remove the blank line so all parameters
(wizardStepIds: ClusterWizardStepsType[] | undefined, staticIpView?:
StaticIpView | 'dhcp-selected', isSingleClusterFeatureEnabled?: boolean) appear
consecutively on the same signature block without an extra blank line or stray
formatting.
- Line 171: The effect in ClusterWizardContextProvider unconditionally calls
setCustomManifestsStep(true) even though customManifestsStep is initialized to
true; remove the redundant setCustomManifestsStep(true) call from the effect (or
guard it with if (!customManifestsStep) setCustomManifestsStep(true)) to avoid
unnecessary state updates and re-renders; look for the effect that contains
setCustomManifestsStep and update it accordingly.
In `@libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts`:
- Around line 20-28: defaultWizardSteps is hard-coded and duplicates the
canonical order defined elsewhere (wizardTransition.ts); change it to derive
from the shared ordering source (e.g., the exported stepOrder/stepSequence in
wizardTransition.ts) and then filter or map that sequence to
ClusterWizardStepsType as needed; update the constant (defaultWizardSteps) to
import the shared order, apply any flow-specific filters (remove steps not
applicable to this flow) and ensure the resulting array preserves the canonical
order so future insertions only need to change the single source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c66aa16-9fc4-4493-bcae-9fa78858ad69
📒 Files selected for processing (28)
libs/locales/lib/en/translation.jsonlibs/types/assisted-installer-service.d.tslibs/ui-lib-tests/cypress/integration/create-multinode/2-networking.cy.tslibs/ui-lib-tests/cypress/integration/create-sno/2-networking.cy.tslibs/ui-lib-tests/cypress/integration/custom-manifests/1-create-cluster-with-custom-manifests.cy.tslibs/ui-lib-tests/cypress/integration/custom-manifests/2-modifying-existing-custom-manifest.cy.tslibs/ui-lib-tests/cypress/support/interceptors.tslibs/ui-lib-tests/cypress/support/variables/networking.tslibs/ui-lib-tests/cypress/views/forms/CustomManifests/CustomManifestsForm.tslibs/ui-lib-tests/cypress/views/networkingPage.tslibs/ui-lib/lib/common/config/constants.tslibs/ui-lib/lib/common/types/ui-settings.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/CustomManifestCheckbox.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/OcmClusterDetailsFormFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropDown.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetails.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetailsForm.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/constants.tslibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.tslibs/ui-lib/lib/ocm/components/utils.tslibs/ui-lib/lib/ocm/hooks/useUISettings.tslibs/ui-lib/lib/ocm/services/ClusterDetailsService.ts
💤 Files with no reviewable changes (5)
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/CustomManifestCheckbox.tsx
- libs/ui-lib/lib/common/types/ui-settings.ts
- libs/ui-lib-tests/cypress/integration/custom-manifests/1-create-cluster-with-custom-manifests.cy.ts
- libs/ui-lib/lib/ocm/hooks/useUISettings.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- libs/locales/lib/en/translation.json
- libs/ui-lib-tests/cypress/views/forms/CustomManifests/CustomManifestsForm.ts
- libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmClusterDetailsFormFields.tsx
- libs/ui-lib/lib/common/config/constants.ts
- libs/ui-lib-tests/cypress/support/variables/networking.ts
- libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropDown.tsx
- libs/ui-lib/lib/ocm/services/ClusterDetailsService.ts
- libs/ui-lib-tests/cypress/support/interceptors.ts
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx
...i-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx
Show resolved
Hide resolved
|
@LiorSoffer: This pull request references MGMT-23131 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 story to target the "4.22.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.
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-tests/cypress/fixtures/custom-manifests/1-cluster-created.ts (1)
6-15:⚠️ Potential issue | 🟡 MinorUpdate
featureUsageto reflect Cilium network type instead of SDN.The
featureUsageobject references'SDN network type'with id'SDN_NETWORK_TYPE', but the fixture'snetwork_typeis set to'Cilium'. This should be'Cilium network type'with id'CILIUM_NETWORK_TYPE'to match the actual network type being used and align with the pattern in other fixtures (e.g.,create-snouses'OVN network type'withOVN_NETWORK_TYPEfor OVNKubernetes).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib-tests/cypress/fixtures/custom-manifests/1-cluster-created.ts` around lines 6 - 15, The featureUsage entry currently named 'SDN network type' with id 'SDN_NETWORK_TYPE' should be updated to represent Cilium; rename the object key to 'Cilium network type', change its id to 'CILIUM_NETWORK_TYPE', and update the name field to 'Cilium network type' in the featureUsage constant so it matches the fixture's network_type and follows the same naming/id pattern used by other fixtures (locate the featureUsage object in the fixture and update the key and inner id/name accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@libs/ui-lib-tests/cypress/fixtures/custom-manifests/1-cluster-created.ts`:
- Around line 6-15: The featureUsage entry currently named 'SDN network type'
with id 'SDN_NETWORK_TYPE' should be updated to represent Cilium; rename the
object key to 'Cilium network type', change its id to 'CILIUM_NETWORK_TYPE', and
update the name field to 'Cilium network type' in the featureUsage constant so
it matches the fixture's network_type and follows the same naming/id pattern
used by other fixtures (locate the featureUsage object in the fixture and update
the key and inner id/name accordingly).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f353fc97-cf80-47a7-a636-e77249468edf
📒 Files selected for processing (2)
libs/ui-lib-tests/cypress/fixtures/custom-manifests/1-cluster-created.tslibs/ui-lib-tests/cypress/integration/dualstack/2-networking.cy.ts
|
@LiorSoffer: This pull request references MGMT-23131 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 story to target the "4.22.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. |
|
@LiorSoffer: This pull request references MGMT-23131 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 story to target the "4.22.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. |
jgyselov
requested changes
Mar 13, 2026
Contributor
There was a problem hiding this comment.
"insufficient" state because of the missing custom manifests, the first step we show when the users open the wizard is Custom manifests. (It's Host discovery and the clicking on Next gets tiring 😅)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetails.tsx
Outdated
Show resolved
Hide resolved
.../ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropDown.tsx
Outdated
Show resolved
Hide resolved
.../ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropDown.tsx
Outdated
Show resolved
Hide resolved
.../ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropDown.tsx
Outdated
Show resolved
Hide resolved
.../ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropDown.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib-tests/cypress/fixtures/custom-manifests/1-cluster-created.ts
Show resolved
Hide resolved
Contributor
Author
|
@jgyselov thanks for the detailed code review, the pictures and examples help a lot :) |
|
@LiorSoffer: An error was encountered searching for bug MGMT-23131 on the Jira server at https://issues.redhat.com. No known errors were detected, please see the full error message for details. Full error message.
No response returned: Get "https://issues.redhat.com/rest/api/2/issue/MGMT-23131": GET https://issues.redhat.com/rest/api/2/issue/MGMT-23131 giving up after 5 attempt(s)
Please contact an administrator to resolve this issue, then request a bug refresh with 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. |
openshift-ci-robot
removed
the
jira/valid-reference
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/types/assisted-installer-service.d.ts (1)
2274-2283:⚠️ Potential issue | 🟡 MinorBreaking type change:
macAddressis now required inMacInterfaceMap.The field changed from optional to required. All existing usages in
formDataToInfraEnvField.ts,dummyData.ts,MacIpMapping.tsx, andemptyData.tsalready providemacAddressin every construction, and the validation schema enforces this requirement. No breaking changes needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/types/assisted-installer-service.d.ts` around lines 2274 - 2283, The MacInterfaceMap type now requires macAddress (changed from optional), so update any code or types that assumed optionality: ensure all constructions and declarations of MacInterfaceMap include macAddress and match the validation schema; review and adjust usages in formDataToInfraEnvField.ts, dummyData.ts, MacIpMapping.tsx, and emptyData.ts to supply a valid macAddress string where missing, and update any related tests/types to reflect the required field on MacInterfaceMap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx`:
- Around line 33-40: The component initializes useCustomManifests from
isRequired only, which causes an initial "off" render that pushes an empty form
before the effect flips it on when
clusterWizardContext.uiSettings?.customManifestsAdded is true; update the state
initialization for useCustomManifests to use isRequired OR
clusterWizardContext.uiSettings?.customManifestsAdded (e.g. initialize with
isRequired || clusterWizardContext.uiSettings?.customManifestsAdded) so the
first render reflects persisted manifests, and then remove or simplify the
React.useEffect that redundantly sets setUseCustomManifests to avoid duplicate
state transitions (adjust or remove the effect that references isRequired and
clusterWizardContext).
In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx`:
- Around line 262-268: The new NetworkTypeDropDown is only being disabled by
isViewerMode; update NetworkConfiguration to also compute the support-gate
(NETWORK_TYPE_SELECTION) state and pass it into NetworkTypeDropDown (e.g.,
isDisabled={isViewerMode || !isFeatureSupported(NETWORK_TYPE_SELECTION,
featureSupportLevelData)}) and provide a disabledReason prop sourced from
featureSupportLevelData for NETWORK_TYPE_SELECTION so the control can show why
it’s locked; also update NetworkTypeDropDown to accept and render the
disabledReason when present.
In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropdown.tsx`:
- Around line 71-75: The effect in NetworkTypeDropdown is silently normalizing
field.value from NETWORK_TYPE_SDN to NETWORK_TYPE_OVN whenever sdnDisabledReason
becomes truthy, which can override existing OpenShiftSDN values on load; remove
this passive normalization from the generic useEffect and instead trigger
normalization only on the explicit transition that indicates a user-driven
incompatibility (e.g., in the stack-type or support-level onChange handler or a
dedicated watcher for the specific prop change). Concretely: stop calling
setValue inside the current React.useEffect that watches
sdnDisabledReason/field.value; implement normalization logic in the handler that
changes the stack type (or use a prevProp comparison like usePrevious of the
specific prop that indicates incompatibility) and only call
setValue(NETWORK_TYPE_OVN) there when the change originates from that transition
and the current field.value is NETWORK_TYPE_SDN. Reference symbols:
NetworkTypeDropdown, field.value, setValue, sdnDisabledReason, NETWORK_TYPE_SDN,
NETWORK_TYPE_OVN.
- Around line 80-103: The dropdown selection is wired to event.currentTarget.id
but PatternFly 6 requires wrapping items in a DropdownList and using the second
onSelect argument as the selected value; update the onSelect function signature
in NetworkTypeDropdown to (event?: React.MouseEvent<Element, MouseEvent>,
nextValue?: string) and use nextValue to call setValue(nextValue) and then
setOpen(false), and render dropdownItems inside a <DropdownList> so the onSelect
receives the value (also adjust any other identical handler at lines 123–131 to
the same pattern); keep references to DropdownItem, dropdownItems, onSelect,
setValue and setOpen when making the change.
In `@libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx`:
- Around line 27-29: The getNetworkType function can return undefined for
unexpected or empty Cluster['networkType']; update getNetworkType to safely
resolve NETWORK_TYPE_LABELS by looking up
NETWORK_TYPE_LABELS[clusterNetworkType] and returning a sensible default (e.g.,
"Unknown" or "N/A") when the lookup yields undefined or when clusterNetworkType
is falsy so the "Networking Type" field is never blank.
In `@libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetails.tsx`:
- Around line 107-113: The post-create UI settings update
(UISettingService.update and clusterWizardContext.updateUISettings) is currently
inside the outer catch so UI-setting failures are reported as “Failed to create
new cluster”; move these two calls into their own try/catch (or treat them as
best-effort) so cluster creation errors are distinct from UI-setting errors —
catch errors from UISettingService.update and
clusterWizardContext.updateUISettings, log or surface a specific UI-settings
error message (without rethrowing or changing the overall success flow), and
ensure the outer catch still only reports creation failures.
In
`@libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx`:
- Around line 85-87: The truthy check on infraEnv.staticNetworkConfig
incorrectly treats empty arrays as STATIC; update the condition used when
setting hostsNetworkConfigurationType (where
HostsNetworkConfigurationType.STATIC or .DHCP is chosen) to check array length
instead (e.g., infraEnv.staticNetworkConfig?.length > 0) so only non-empty
staticNetworkConfig arrays map to STATIC and empty arrays map to DHCP.
---
Outside diff comments:
In `@libs/types/assisted-installer-service.d.ts`:
- Around line 2274-2283: The MacInterfaceMap type now requires macAddress
(changed from optional), so update any code or types that assumed optionality:
ensure all constructions and declarations of MacInterfaceMap include macAddress
and match the validation schema; review and adjust usages in
formDataToInfraEnvField.ts, dummyData.ts, MacIpMapping.tsx, and emptyData.ts to
supply a valid macAddress string where missing, and update any related
tests/types to reflect the required field on MacInterfaceMap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8477f49e-171f-467a-ad68-048b3a5d518a
📒 Files selected for processing (32)
libs/locales/lib/en/translation.jsonlibs/types/assisted-installer-service.d.tslibs/ui-lib-tests/cypress/fixtures/custom-manifests/1-cluster-created.tslibs/ui-lib-tests/cypress/integration/create-multinode/2-networking.cy.tslibs/ui-lib-tests/cypress/integration/create-sno/2-networking.cy.tslibs/ui-lib-tests/cypress/integration/custom-manifests/1-create-cluster-with-custom-manifests.cy.tslibs/ui-lib-tests/cypress/integration/custom-manifests/2-modifying-existing-custom-manifest.cy.tslibs/ui-lib-tests/cypress/integration/dualstack/2-networking.cy.tslibs/ui-lib-tests/cypress/integration/use-cases/create-cluster/with-external-partner-integrations.cy.tslibs/ui-lib-tests/cypress/support/interceptors.tslibs/ui-lib-tests/cypress/support/variables/networking.tslibs/ui-lib-tests/cypress/views/forms/CustomManifests/CustomManifestsForm.tslibs/ui-lib-tests/cypress/views/networkingPage.tslibs/ui-lib/lib/common/config/constants.tslibs/ui-lib/lib/common/types/ui-settings.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/CustomManifestCheckbox.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/OcmClusterDetailsFormFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropdown.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetails.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetailsForm.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/constants.tslibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.tslibs/ui-lib/lib/ocm/components/utils.tslibs/ui-lib/lib/ocm/hooks/useUISettings.tslibs/ui-lib/lib/ocm/services/ClusterDetailsService.tslibs/ui-lib/lib/ocm/services/types.ts
💤 Files with no reviewable changes (8)
- libs/ui-lib/lib/ocm/services/types.ts
- libs/ui-lib-tests/cypress/integration/use-cases/create-cluster/with-external-partner-integrations.cy.ts
- libs/ui-lib-tests/cypress/integration/custom-manifests/1-create-cluster-with-custom-manifests.cy.ts
- libs/ui-lib/lib/ocm/components/clusterConfiguration/CustomManifestCheckbox.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx
- libs/ui-lib/lib/ocm/services/ClusterDetailsService.ts
- libs/ui-lib/lib/ocm/hooks/useUISettings.ts
- libs/ui-lib/lib/common/types/ui-settings.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- libs/ui-lib-tests/cypress/integration/create-sno/2-networking.cy.ts
- libs/ui-lib-tests/cypress/fixtures/custom-manifests/1-cluster-created.ts
- libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmClusterDetailsFormFields.tsx
- libs/ui-lib/lib/ocm/components/utils.ts
- libs/ui-lib-tests/cypress/integration/custom-manifests/2-modifying-existing-custom-manifest.cy.ts
- libs/ui-lib/lib/common/config/constants.ts
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx
- libs/ui-lib-tests/cypress/support/variables/networking.ts
- libs/locales/lib/en/translation.json
- libs/ui-lib-tests/cypress/integration/dualstack/2-networking.cy.ts
- libs/ui-lib-tests/cypress/support/interceptors.ts
...i-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx
Show resolved
Hide resolved
...ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx
Show resolved
Hide resolved
.../ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropdown.tsx
Show resolved
Hide resolved
.../ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropdown.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetails.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Lior Soffer <liorsoffer1@gmail.com>
jgyselov
requested changes
Mar 16, 2026
.../ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropdown.tsx
Outdated
Show resolved
Hide resolved
.../ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropdown.tsx
Show resolved
Hide resolved
.../ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropDown.tsx
Outdated
Show resolved
Hide resolved
.../ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkTypeDropdown.tsx
Outdated
Show resolved
Hide resolved
...i-lib/lib/ocm/components/clusterConfiguration/manifestsConfiguration/CustomManifestsPage.tsx
Show resolved
Hide resolved
jgyselov
approved these changes
Mar 16, 2026
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jgyselov, LiorSoffer 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-ci
bot
added
the
approved
openshift-merge-bot
bot
merged commit c0d3a39
into
openshift-assisted:master
14 checks passed
Contributor
Author
|
/cherry-pick releases/v2.51 |
Contributor
|
@LiorSoffer: #3457 failed to apply on top of branch "releases/v2.51": 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 kubernetes-sigs/prow repository. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://issues.redhat.com/browse/MGMT-23131
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor