MGMT-22204: add new fields to above sea level ABI#3274
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@ElayAharoni: This pull request references MGMT-22204 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.21.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-merge-robot
added
the
needs-rebase
WalkthroughAdds disconnected-cluster support: new POST /disconnected API and service method, wizard context state for a disconnected InfraEnv, a new "Optional configurations" wizard step, Basic/Review steps updated to register/deregister clusters and handle ISO download, footer and route adjustments, plus a minor comment-only change. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BasicStep
participant ClustersService
participant Context as ClusterWizardContext
participant OptionalStep
participant InfraEnvsService
participant ReviewStep
participant Router
User->>BasicStep: submit name + openshiftVersion
BasicStep->>ClustersService: registerDisconnected(params)
ClustersService-->>BasicStep: cluster (id)
BasicStep->>Context: setDisconnectedInfraEnv / store cluster id
BasicStep->>User: moveNext
User->>OptionalStep: submit ssh + pullSecret
OptionalStep->>InfraEnvsService: createOrUpdate(infraParams)
InfraEnvsService-->>OptionalStep: infraEnv
OptionalStep->>Context: setDisconnectedInfraEnv(infraEnv)
OptionalStep->>User: moveNext
User->>ReviewStep: finish
alt disconnectedInfraEnv.downloadUrl exists
ReviewStep->>ReviewStep: download ISO
end
alt disconnectedClusterId exists
ReviewStep->>ClustersService: remove(clusterId)
ClustersService-->>ReviewStep: success/fail
end
ReviewStep->>Context: clear disconnectedInfraEnv
ReviewStep->>Router: navigate /cluster-list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
openshift-ci
bot
added
the
size/L
|
@ElayAharoni: This pull request references MGMT-22204 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.21.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. |
|
@ElayAharoni: This pull request references MGMT-22204 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.21.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. |
ElayAharoni
force-pushed
the
add-new-fields-to-above-sea-level-ABI
branch
from
3c12e5f to
b8c7ece
Compare
openshift-merge-robot
removed
the
needs-rebase
|
@ElayAharoni: This pull request references MGMT-22204 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.21.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: 3
🧹 Nitpick comments (3)
libs/ui-lib/lib/ocm/services/ClustersService.ts (1)
174-179: Avoid unnecessary field re-mapping inregisterDisconnectedThe method correctly delegates to
ClustersAPI.registerDisconnected, but you can simplify it by passingparamsthrough directly instead of re-mapping each field:- registerDisconnected(params: { name: string; openshiftVersion: string }) { - return ClustersAPI.registerDisconnected({ - name: params.name, - openshiftVersion: params.openshiftVersion, - }); - }, + registerDisconnected(params: { name: string; openshiftVersion: string }) { + return ClustersAPI.registerDisconnected(params); + },libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx (1)
20-106: Consider consolidating context usage and handling removal failures inhandleCancelThe new cancel flow correctly cleans up a disconnected cluster and its InfraEnv state, but there are a couple of tidy-ups you might consider:
- You call
useClusterWizardContext()twice (once forcurrentStepId, once for disconnected state). Consolidating into a single destructuring call will reduce duplication and keep the hook usage a bit cleaner.ClustersService.remove(disconnectedCluster.id)is awaited but any error is silently ignored; you still clear local state and navigate. If backend cleanup fails, the user won’t see it. Wrapping the removal in atry/catch(and at least logging or surfacing an alert) would improve debuggability and user feedback without changing the happy path.These are non-blocking, but would make this footer logic more robust.
libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx (1)
76-80: Consider handlingpullSecretprop changes toundefined.The effect only updates the Formik field when
pullSecret !== undefined. If the prop changes from a defined value toundefined, the Formik field will retain the old value, which could lead to stale data in the form state.Consider this alternative that handles all prop changes:
useEffect(() => { - if (pullSecret !== undefined) { - setFieldValue('pullSecret', pullSecret); - } + setFieldValue('pullSecret', pullSecret ?? ''); }, [pullSecret, setFieldValue]);Alternatively, if retaining the value when the prop becomes undefined is the intended behavior, add a comment explaining this design choice.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx(1 hunks)libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts(1 hunks)libs/ui-lib/lib/common/components/ui/WizardFooter.tsx(1 hunks)libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts(4 hunks)libs/ui-lib/lib/ocm/services/ClustersService.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: linoyaslan
Repo: openshift-assisted/assisted-installer-ui PR: 3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts:69-69
Timestamp: 2025-10-19T17:22:52.502Z
Learning: CIM UI changes in the repository openshift-assisted/assisted-installer-ui (e.g., files under libs/ui-lib/lib/cim/) are handled separately by the CIM team and should be tracked via separate issues rather than being included in PRs for other UI components.
📚 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/disconnected/OptionalConfigurationsStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsxapps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx
📚 Learning: 2025-08-11T06:07:38.056Z
Learnt from: ammont82
Repo: openshift-assisted/assisted-installer-ui PR: 3101
File: libs/ui-lib/lib/common/config/docs_links.ts:203-221
Timestamp: 2025-08-11T06:07:38.056Z
Learning: The MetalLB, OADP, Cluster Observability, and NUMA Resources operators in the virtualization bundle don't require version fallback logic in their documentation link functions (getMetalLbLink, getOadpLink, getClusterObservabilityLink, getNumaResourcesLink) in libs/ui-lib/lib/common/config/docs_links.ts, unlike some other operators like LSO and NVIDIA GPU.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx
🧬 Code graph analysis (7)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
libs/types/assisted-installer-service.d.ts (2)
Cluster(110-407)InfraEnv(1750-1827)
libs/ui-lib/lib/ocm/services/ClustersService.ts (1)
libs/ui-lib/lib/ocm/services/apis/index.ts (1)
ClustersAPI(5-5)
libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts (2)
libs/ui-lib/lib/common/api/axiosClient.ts (1)
client(49-49)libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx (3)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(32-38)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)libs/ui-lib/lib/common/api/utils.ts (2)
handleApiError(19-51)getApiErrorMessage(53-58)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx (3)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(32-38)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)libs/ui-lib/lib/common/api/utils.ts (2)
handleApiError(19-51)getApiErrorMessage(53-58)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx (1)
libs/types/assisted-installer-service.d.ts (2)
Cluster(110-407)InfraEnv(1750-1827)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx (2)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(32-38)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)
⏰ 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: unit-tests
- GitHub Check: tests
- GitHub Check: translation-files
- GitHub Check: circular-deps
- GitHub Check: format
- GitHub Check: lint
🔇 Additional comments (10)
apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx (1)
45-46: Comment addition is clear and harmlessThe new comment accurately describes the existing
ClustersService.remove(cluster.id)behavior and doesn’t affect runtime logic. No further changes needed.libs/ui-lib/lib/common/components/ui/WizardFooter.tsx (1)
69-76: Disabling Next while submitting is a good safeguardUsing
isNextDisabled || isSubmittingon the Next button cleanly prevents double-submission during async flows and aligns with the existing submitting state handling.libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx (1)
7-18: New disconnected optional-configurations step is wired correctlyThe
OptionalConfigurationsStepimport and the'disconnected-optional-configurations'case integrate cleanly intogetCurrentStepand match the step key defined inwizardStepNames.libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts (1)
15-18: Wizard label for the new disconnected step looks consistentThe
'disconnected-optional-configurations'entry matches the new step id and follows the existing naming pattern for wizard steps.libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
1-6: Re-exportinguseClusterWizardContextis a useful API improvementExposing
useClusterWizardContextfrom the index keeps imports consistent with other wizard exports and doesn’t introduce any behavioral change.libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts (1)
248-253:registerDisconnectedendpoint wiring looks correctThe new
registerDisconnectedmethod follows the established pattern for POST endpoints in this API (URI construction and typings) and cleanly supports the disconnected registration flow.libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
6-6: LGTM!The context type extensions for disconnected cluster and InfraEnv state management are well-structured and properly typed.
Also applies to: 24-27
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx (1)
114-121: Verify prop synchronization behavior is intentional.The effect only updates
disconnectedClusteranddisconnectedInfraEnvwhen they areundefined. This means subsequent changes to theclusterorinfraEnvprops after the state is initially set will be ignored.If the cluster/infraEnv props can change during the wizard flow (e.g., from external updates), this could lead to stale state. However, if the disconnected flow is meant to capture the initial values and manage them independently, this is correct.
Please confirm whether this one-time initialization behavior is intentional for the disconnected cluster workflow, or if the state should track all prop changes.
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx (1)
60-93: LGTM!The async deregistration flow is well-structured with proper error handling. The pattern of clearing context and navigating in all code paths (success, error, and no-cluster cases) ensures the wizard doesn't get stuck in an invalid state.
libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts (1)
35-35: LGTM!The wizard step type, flow, and validation map updates for
disconnected-optional-configurationsare correctly integrated into the existing wizard infrastructure.Also applies to: 53-53, 265-265, 281-281
libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
Show resolved
Hide resolved
ElayAharoni
force-pushed
the
add-new-fields-to-above-sea-level-ABI
branch
from
b8c7ece to
8e60fb3
Compare
|
@ElayAharoni: This pull request references MGMT-22204 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.21.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx (1)
44-46: Non-functional comment looks fine; wording could be a bit more specificThe added comment correctly documents the intent of the
ClustersService.remove(cluster.id)call and doesn’t affect behavior. If you want to future‑proof it against different cluster types, you could consider slightly clearer wording like “Remove the cluster resource” or “Remove this cluster” instead of “regular cluster,” but that’s purely optional.libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx (1)
76-80: Consider simplifying the dependency array.The
setFieldValuefunction from Formik is stable and doesn't change between renders, so it's not strictly necessary in the dependency array. However, including it is a safe practice and doesn't cause issues.Optional refactor:
useEffect(() => { if (pullSecret !== undefined) { setFieldValue('pullSecret', pullSecret); } - }, [pullSecret, setFieldValue]); + }, [pullSecret]);libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx (1)
94-101: Consider adding error handling for cluster removal.The
handleCancelfunction removes the disconnected cluster without error handling. If the removal fails, the user is still navigated away and the context is cleared. While this may be acceptable for a cancel operation (to avoid getting stuck), consider logging errors or showing a non-blocking alert.Apply this diff to add error handling:
const handleCancel = React.useCallback(async () => { if (disconnectedCluster?.id) { - await ClustersService.remove(disconnectedCluster.id); + try { + await ClustersService.remove(disconnectedCluster.id); + } catch (error) { + // Log error but continue with navigation for cancel operation + console.error('Failed to remove disconnected cluster:', error); + } } setDisconnectedCluster(undefined); setDisconnectedInfraEnv(undefined); navigate('/cluster-list'); }, [navigate, disconnectedCluster, setDisconnectedCluster, setDisconnectedInfraEnv]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx(1 hunks)libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts(1 hunks)libs/ui-lib/lib/common/components/ui/WizardFooter.tsx(1 hunks)libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts(4 hunks)libs/ui-lib/lib/ocm/services/ClustersService.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts
- libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx
- libs/ui-lib/lib/ocm/services/ClustersService.ts
- libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts
- libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: linoyaslan
Repo: openshift-assisted/assisted-installer-ui PR: 3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts:69-69
Timestamp: 2025-10-19T17:22:52.502Z
Learning: CIM UI changes in the repository openshift-assisted/assisted-installer-ui (e.g., files under libs/ui-lib/lib/cim/) are handled separately by the CIM team and should be tracked via separate issues rather than being included in PRs for other UI components.
📚 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/ClusterWizardFooter.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx
📚 Learning: 2025-08-11T06:07:38.056Z
Learnt from: ammont82
Repo: openshift-assisted/assisted-installer-ui PR: 3101
File: libs/ui-lib/lib/common/config/docs_links.ts:203-221
Timestamp: 2025-08-11T06:07:38.056Z
Learning: The MetalLB, OADP, Cluster Observability, and NUMA Resources operators in the virtualization bundle don't require version fallback logic in their documentation link functions (getMetalLbLink, getOadpLink, getClusterObservabilityLink, getNumaResourcesLink) in libs/ui-lib/lib/common/config/docs_links.ts, unlike some other operators like LSO and NVIDIA GPU.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx
🧬 Code graph analysis (4)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx (2)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(32-38)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx (3)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(32-38)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)libs/ui-lib/lib/common/api/utils.ts (2)
handleApiError(19-51)getApiErrorMessage(53-58)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx (2)
libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)libs/ui-lib/lib/common/api/utils.ts (2)
handleApiError(19-51)getApiErrorMessage(53-58)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
libs/types/assisted-installer-service.d.ts (2)
Cluster(110-407)InfraEnv(1750-1827)
⏰ 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: tests
- GitHub Check: translation-files
- GitHub Check: circular-deps
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: format
🔇 Additional comments (11)
libs/ui-lib/lib/common/components/ui/WizardFooter.tsx (1)
72-72: LGTM! Proper integration of submission state.The Next button now correctly disables during submission by checking both
isNextDisabledandisSubmitting, preventing double-submission issues.libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (2)
6-6: LGTM! Proper type imports for disconnected cluster state.The import of
ClusterandInfraEnvtypes provides the necessary type safety for the new disconnected cluster workflow.
24-27: LGTM! Well-structured context extension for disconnected cluster workflow.The new context properties provide proper state management for disconnected clusters and their infrastructure environments, with appropriate optional typing and setter functions.
libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
5-5: LGTM! Proper public export of context hook.Exporting
useClusterWizardContextmakes it available for components outside this module to access the wizard context.libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx (2)
29-51: LGTM! Proper async flow control and error handling.The implementation correctly:
- Calls
moveNext()only on successful cluster creation (inside try block)- Handles errors gracefully with alerts
- Resets submission state in finally block regardless of outcome
- Prevents wizard advancement when API calls fail
This addresses the previous review concern about advancing on failure.
62-69: Good integration with footer component.The footer correctly receives the async
onNexthandler andisSubmittingstate, providing proper UI feedback during cluster creation.libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx (4)
57-93: LGTM! Proper async flow control and error handling.The onSubmit implementation correctly:
- Validates cluster existence with a guard clause
- Calls
moveNext()only after successful InfraEnv creation (inside try block)- Handles API errors with appropriate alerts
- Resets submission state in finally block
This addresses the previous review concern about wizard advancement on failure.
39-46: LGTM! Proper validation schema setup.The validation schema correctly composes the SSH key and pull secret validations, and is appropriately memoized.
129-135: LGTM! Good UX pattern for optional pull secret editing.The checkbox-controlled editing of the pull secret provides a clean UX, showing the field only when users want to customize it while maintaining the default value when not editing.
71-78: Verify cpuArchitecture alignment—current implementation shows asymmetry between cluster and InfraEnv creation.The
openshiftVersion: '4.19'is correctly aligned (used in both BasicStep cluster creation and OptionalConfigurationsStep InfraEnv creation). However,cpuArchitecture: 'x86_64'is specified only in the InfraEnv creation, not in the cluster creation call toregisterDisconnected().Confirm whether this asymmetry is intentional based on the API contracts for cluster and InfraEnv creation, or if
cpuArchitectureshould also be added to the cluster creation parameters.libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx (1)
60-93: Critical: Duplicate navigation and state clearing in success path.When cluster deregistration succeeds, the code clears context and navigates at lines 69-72, but then execution continues to lines 82-86 which duplicate the context clearing and navigation. This results in:
- Redundant state updates
- Duplicate navigation calls
- The error handling comment at line 81 is misleading for the success path
Apply this diff to fix the control flow:
onNext={() => { void (async () => { if (disconnectedInfraEnv?.downloadUrl) { saveAs(disconnectedInfraEnv.downloadUrl); } if (disconnectedCluster?.id) { try { await ClustersService.remove(disconnectedCluster.id); // Remove cluster and infraEnv from wizard context after successful deregistration setDisconnectedCluster(undefined); setDisconnectedInfraEnv(undefined); // Navigate to cluster-list only after successful deregistration navigate('/cluster-list'); } catch (error) { handleApiError(error, () => { addAlert({ title: 'Failed to deregister cluster', message: getApiErrorMessage(error), variant: AlertVariant.danger, }); }); // Error handling: continue with navigation even if deregistration fails // Still clear the context to avoid stale data setDisconnectedCluster(undefined); setDisconnectedInfraEnv(undefined); // Navigate even on error to avoid getting stuck navigate('/cluster-list'); } } else { // If there's no cluster to deregister, navigate immediately + setDisconnectedCluster(undefined); + setDisconnectedInfraEnv(undefined); navigate('/cluster-list'); } })(); }}Note: Also consider wrapping
saveAsin a try-catch since file operations can fail.Likely an incorrect or invalid review comment.
libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx
Outdated
Show resolved
Hide resolved
ElayAharoni
force-pushed
the
add-new-fields-to-above-sea-level-ABI
branch
from
8e60fb3 to
03f8a65
Compare
|
@ElayAharoni: This pull request references MGMT-22204 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.21.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: 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/clusterWizard/disconnected/ReviewStep.tsx (1)
31-96: Guard against double Next clicks and always clear disconnected wizard stateTwo behavioral issues in the current
onNext:
- The async IIFE isn’t tied to any
isSubmittingflag, so the Next button remains clickable; rapid clicks can trigger multipleClustersService.removecalls and duplicate “Failed to deregister cluster” alerts.- In the
elsebranch (!disconnectedCluster?.id), you navigate to/cluster-listbut don’t cleardisconnectedCluster/disconnectedInfraEnv, which can leave stale wizard state if the provider outlives this route.You can address both by tracking a local submitting flag, wiring it into
ClusterWizardFooter, and centralizing cleanup infinally:const ReviewStep = () => { - const { - moveBack, - disconnectedInfraEnv, - disconnectedCluster, - setDisconnectedCluster, - setDisconnectedInfraEnv, - } = useClusterWizardContext(); - const { addAlert } = useAlerts(); + const { + moveBack, + disconnectedInfraEnv, + disconnectedCluster, + setDisconnectedCluster, + setDisconnectedInfraEnv, + } = useClusterWizardContext(); + const { addAlert } = useAlerts(); + const [isSubmitting, setIsSubmitting] = React.useState(false); @@ - <ClusterWizardFooter - onNext={() => { - void (async () => { - if (disconnectedInfraEnv?.downloadUrl) { - saveAs(disconnectedInfraEnv.downloadUrl); - } - if (disconnectedCluster?.id) { - try { - await ClustersService.remove(disconnectedCluster.id); - // Remove cluster and infraEnv from wizard context after successful deregistration - setDisconnectedCluster(undefined); - setDisconnectedInfraEnv(undefined); - // Navigate to cluster-list only after successful deregistration - navigate('/cluster-list'); - } catch (error) { - handleApiError(error, () => { - addAlert({ - title: 'Failed to deregister cluster', - message: getApiErrorMessage(error), - variant: AlertVariant.danger, - }); - }); - // Error handling: continue with navigation even if deregistration fails - // Still clear the context to avoid stale data - setDisconnectedCluster(undefined); - setDisconnectedInfraEnv(undefined); - // Navigate even on error to avoid getting stuck - navigate('/cluster-list'); - } - } else { - // If there's no cluster to deregister, navigate immediately - navigate('/cluster-list'); - } - })(); - }} + <ClusterWizardFooter + isSubmitting={isSubmitting} + isNextButtonLoading={isSubmitting} + onNext={() => { + if (isSubmitting) { + return; + } + setIsSubmitting(true); + void (async () => { + if (disconnectedInfraEnv?.downloadUrl) { + saveAs(disconnectedInfraEnv.downloadUrl); + } + try { + if (disconnectedCluster?.id) { + await ClustersService.remove(disconnectedCluster.id); + } + } catch (error) { + handleApiError(error, () => { + addAlert({ + title: 'Failed to deregister cluster', + message: getApiErrorMessage(error), + variant: AlertVariant.danger, + }); + }); + } finally { + // Always clear wizard state and navigate away + setDisconnectedCluster(undefined); + setDisconnectedInfraEnv(undefined); + navigate('/cluster-list'); + setIsSubmitting(false); + } + })(); + }} onBack={moveBack} nextButtonText="Download ISO" />This:
- Prevents double-submissions while the deregistration flow is running.
- Ensures the disconnected wizard context is cleared and navigation happens on all paths (success, error, or “no cluster”).
- Reuses the existing
isSubmitting/isNextButtonLoadingprops onWizardFooterfor proper button disabling and spinner behavior.Optionally, consider localizing the new alert title (
'Failed to deregister cluster') via the existing translation wrapper to keep user-facing strings consistent with the rest of the UI.
🧹 Nitpick comments (3)
libs/ui-lib/lib/ocm/services/ClustersService.ts (1)
174-179:registerDisconnectedwrapper is fine; can be simplifiedThe new service method correctly forwards
nameandopenshiftVersiontoClustersAPI.registerDisconnected.You can simplify slightly by passing the params object through unchanged:
- registerDisconnected(params: { name: string; openshiftVersion: string }) { - return ClustersAPI.registerDisconnected({ - name: params.name, - openshiftVersion: params.openshiftVersion, - }); - }, + registerDisconnected(params: { name: string; openshiftVersion: string }) { + return ClustersAPI.registerDisconnected(params); + },libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx (1)
33-36: Consider parameterizing the hardcoded OpenShift version.The
openshiftVersionis hardcoded to'4.19'here and also displayed statically at Line 88. While this may be intentional for the current disconnected flow, hardcoding limits flexibility and makes future version updates harder to maintain.Consider one of these approaches:
- Extract to a constant at the file or module level:
+const DISCONNECTED_CLUSTER_VERSION = '4.19'; + const BasicStep = () => { ... const onNext = async () => { try { setIsSubmitting(true); const { data: cluster } = await ClustersService.registerDisconnected({ name: 'disconnected-cluster', - openshiftVersion: '4.19', + openshiftVersion: DISCONNECTED_CLUSTER_VERSION, });
- Or derive from the form's displayed version (Line 88) via context/props if it becomes configurable in the future.
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx (1)
96-100: Consider parameterizing hardcoded InfraEnv configuration values.The
openshiftVersion('4.19'),cpuArchitecture('x86_64'), and name template are hardcoded. Similar to the BasicStep concern, this limits flexibility and makes version updates harder.Consider deriving these from the
disconnectedClusterobject or shared constants:+const DISCONNECTED_INFRA_ENV_NAME_TEMPLATE = (clusterName: string) => `${clusterName}_infra-env`; + const createParams = { - name: `disconnected-cluster_infra-env`, + name: DISCONNECTED_INFRA_ENV_NAME_TEMPLATE(disconnectedCluster.name || 'disconnected-cluster'), pullSecret: values.pullSecret || pullSecret, clusterId: disconnectedCluster.id, - openshiftVersion: '4.19', - cpuArchitecture: 'x86_64' as const, + openshiftVersion: disconnectedCluster.openshiftVersion || '4.19', + cpuArchitecture: disconnectedCluster.cpuArchitecture || 'x86_64' as const, ...(values.sshPublicKey && { sshAuthorizedKey: values.sshPublicKey }), };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx(1 hunks)libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts(1 hunks)libs/ui-lib/lib/common/components/ui/WizardFooter.tsx(1 hunks)libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts(4 hunks)libs/ui-lib/lib/ocm/services/ClustersService.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts
- libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: linoyaslan
Repo: openshift-assisted/assisted-installer-ui PR: 3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts:69-69
Timestamp: 2025-10-19T17:22:52.502Z
Learning: CIM UI changes in the repository openshift-assisted/assisted-installer-ui (e.g., files under libs/ui-lib/lib/cim/) are handled separately by the CIM team and should be tracked via separate issues rather than being included in PRs for other UI components.
📚 Learning: 2025-08-11T06:07:38.056Z
Learnt from: ammont82
Repo: openshift-assisted/assisted-installer-ui PR: 3101
File: libs/ui-lib/lib/common/config/docs_links.ts:203-221
Timestamp: 2025-08-11T06:07:38.056Z
Learning: The MetalLB, OADP, Cluster Observability, and NUMA Resources operators in the virtualization bundle don't require version fallback logic in their documentation link functions (getMetalLbLink, getOadpLink, getClusterObservabilityLink, getNumaResourcesLink) in libs/ui-lib/lib/common/config/docs_links.ts, unlike some other operators like LSO and NVIDIA GPU.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx
📚 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/ClusterWizardFooter.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts
🧬 Code graph analysis (4)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx (3)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(32-38)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)libs/ui-lib/lib/common/api/utils.ts (2)
handleApiError(19-51)getApiErrorMessage(53-58)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx (3)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(32-38)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)libs/ui-lib/lib/common/api/utils.ts (2)
handleApiError(19-51)getApiErrorMessage(53-58)
libs/ui-lib/lib/ocm/services/ClustersService.ts (1)
libs/ui-lib/lib/ocm/services/apis/index.ts (1)
ClustersAPI(5-5)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx (7)
libs/ui-lib/lib/ocm/hooks/usePullSecret.ts (1)
usePullSecret(6-33)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(32-38)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
sshPublicKeyValidationSchema(106-116)pullSecretValidationSchema(144-163)libs/ui-lib/lib/common/components/ui/formik/utils.ts (2)
getRichTextValidation(75-113)getFormikErrorFields(34-37)libs/ui-lib/lib/ocm/services/apis/index.ts (1)
InfraEnvsAPI(7-7)libs/ui-lib/lib/common/api/assisted-service/EventsAPI.ts (1)
createParams(21-48)libs/ui-lib/lib/common/api/utils.ts (2)
handleApiError(19-51)getApiErrorMessage(53-58)
⏰ 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: tests
- GitHub Check: translation-files
- GitHub Check: circular-deps
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: format
🔇 Additional comments (9)
libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx (1)
68-80: No-op formatting change; component remains correctThe added whitespace after
useTranslation()does not affect behavior.PullSecretFieldstill wires label, helper, and icon as expected.libs/ui-lib/lib/common/components/ui/WizardFooter.tsx (1)
64-76: Disabling Next while submitting is correctTying
isDisabledtoisNextDisabled || isSubmittingis appropriate and prevents duplicate submissions during async work without changing other footer semantics.libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts (1)
21-55: Disconnected optional-configurations step wiring looks consistentThe new
'disconnected-optional-configurations'step is correctly:
- Added to
ClusterWizardStepsType.- Inserted into
disconnectedStepsbetween basic and review.- Backed by its own (currently empty) validations map and registered in
wizardStepsValidationsMap.This keeps validations utils and step navigation coherent with the existing disconnected flow.
Also applies to: 262-282
apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx (1)
34-47: Comment matches existing reset behaviorThe added comment correctly documents that the subsequent call removes the regular cluster. No behavioral change; reset flow remains intact.
libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
1-6: Re-exportinguseClusterWizardContextis appropriateSurfacing
useClusterWizardContextfrom the index keeps imports consistent with the rest of the cluster wizard API and avoids deep import paths for consumers.libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx (4)
31-41: LGTM!The
PullSecretSynccomponent correctly synchronizes the external pull secret into Formik state, with proper guards to avoid setting undefined values.
70-77: LGTM!Good defensive check to ensure the cluster exists before attempting to create or update the InfraEnv. The early return and clear error message prevent cascading failures.
51-63: LGTM!Form validation setup is well-structured:
- Correctly uses imported validation schemas for SSH key and pull secret
- Properly memoized static schema
- Initial values correctly populated from the pull secret hook
- Error field tracking for UI feedback is properly implemented
Also applies to: 68-68, 121-122
87-103: ****The code is consistent and correct.
InfraEnvsService.create()internally destructures the API responseconst { data: infraEnv } = await InfraEnvsAPI.register(params)and returns the unwrappedinfraEnvobject. Both line 87 (const { data: updatedInfraEnv }) and line 103 (const createdInfraEnv) receive the same unwrappedInfraEnvtype—there is no API response inconsistency to fix.Likely an incorrect or invalid review comment.
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx
Outdated
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
Show resolved
Hide resolved
ElayAharoni
force-pushed
the
add-new-fields-to-above-sea-level-ABI
branch
from
03f8a65 to
7db2208
Compare
|
@ElayAharoni: This pull request references MGMT-22204 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.21.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: 2
♻️ Duplicate comments (2)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx (2)
157-157: Capitalize the checkbox label for consistency.The label should follow sentence case for better UI consistency.
Apply this diff:
<Checkbox - label="edit pull secret" + label="Edit pull secret" isChecked={editPullSecret}
96-96: Hard-coded InfraEnv name may cause conflicts.The name
disconnected-cluster_infra-envis static and could cause conflicts if multiple disconnected clusters are created simultaneously or if the wizard is restarted without proper cleanup.Consider using a unique name based on the cluster:
const createParams = { - name: `disconnected-cluster_infra-env`, + name: `${disconnectedCluster.name || disconnectedCluster.id}_infra-env`, pullSecret: values.pullSecret || pullSecret, clusterId: disconnectedCluster.id,
🧹 Nitpick comments (4)
libs/ui-lib/lib/ocm/services/ClustersService.ts (1)
174-179: ThinregisterDisconnectedwrapper is fine; consider reusing a shared param typeThe new
registerDisconnectedmethod is a straightforward pass-through toClustersAPI.registerDisconnectedand matches how it’s used inBasicStep; this looks correct.If you want to tighten typing and avoid future drift, you could consider reusing a shared type (e.g., a
Pick<ClusterCreateParamsWithStaticNetworking, 'name' | 'openshiftVersion'>or an exported API param type) instead of an inline{ name; openshiftVersion }shape, but it’s optional.libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx (1)
20-22: Cancel flow now robust to deregistration failuresThe updated
handleCancelcorrectly attempts to deregister the disconnected cluster, surfaces API errors viahandleApiError+addAlert, and still clears the disconnected state and navigates to/cluster-listeven if the remove call fails. This addresses the earlier concern about users getting stuck with stale context.Optionally, for consistency with other error alerts (e.g., in the disconnected steps), you might want to pass an explicit
variant: AlertVariant.dangerintoaddAlert, but the core behavior looks good.Also applies to: 87-111
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx (1)
32-36: Review step’s download + deregistration flow is consistent and failure-tolerantThe new
onNextbehavior correctly:
- Triggers the ISO download when
disconnectedInfraEnv.downloadUrlis present.- Attempts to deregister the disconnected cluster when an
idexists.- Surfaces API failures via
handleApiErrorand a danger alert, while still clearing disconnected state and navigating to/cluster-listso the user doesn’t get stuck.The minor duplication of
setDisconnectedCluster(undefined),setDisconnectedInfraEnv(undefined), andnavigate('/cluster-list')between the success and error paths could be factored into a small helper or moved into afinally-style branch, but that’s purely a readability/refactor concern.Also applies to: 38-45, 60-92
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx (1)
18-22: Disconnected-cluster registration flow looks correct; consider tightening types to drop ESLint disablesThe
onNextimplementation behaves well:
- Sets
isSubmitting, callsClustersService.registerDisconnected, stores the resulting cluster in context, and only callsmoveNext()on success.- Uses
handleApiError+ a danger alert to surface failures without advancing the wizard.- Always clears
isSubmittinginfinally, so the Next button won’t remain disabled.The
eslint-disableforno-unsafe-assignment/no-unsafe-calland thecluster as Clustercast suggest the underlying API method is typed asany. If you firm up the return type ofClustersAPI.registerDisconnected(and thusClustersService.registerDisconnected), you should be able to remove both the linter suppression and the cast, keeping this flow fully type-safe.Also applies to: 26-53, 64-71
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx(1 hunks)libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts(1 hunks)libs/ui-lib/lib/common/components/ui/WizardFooter.tsx(1 hunks)libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts(4 hunks)libs/ui-lib/lib/ocm/services/ClustersService.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- libs/ui-lib/lib/ocm/components/clusterWizard/index.ts
- libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx
- libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts
- libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts
- libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx
- apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: linoyaslan
Repo: openshift-assisted/assisted-installer-ui PR: 3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts:69-69
Timestamp: 2025-10-19T17:22:52.502Z
Learning: CIM UI changes in the repository openshift-assisted/assisted-installer-ui (e.g., files under libs/ui-lib/lib/cim/) are handled separately by the CIM team and should be tracked via separate issues rather than being included in PRs for other UI components.
📚 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/NewClusterWizard.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
📚 Learning: 2025-08-11T06:07:38.056Z
Learnt from: ammont82
Repo: openshift-assisted/assisted-installer-ui PR: 3101
File: libs/ui-lib/lib/common/config/docs_links.ts:203-221
Timestamp: 2025-08-11T06:07:38.056Z
Learning: The MetalLB, OADP, Cluster Observability, and NUMA Resources operators in the virtualization bundle don't require version fallback logic in their documentation link functions (getMetalLbLink, getOadpLink, getClusterObservabilityLink, getNumaResourcesLink) in libs/ui-lib/lib/common/config/docs_links.ts, unlike some other operators like LSO and NVIDIA GPU.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx
🧬 Code graph analysis (2)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx (3)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(32-38)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)libs/ui-lib/lib/common/api/utils.ts (2)
handleApiError(19-51)getApiErrorMessage(53-58)
libs/ui-lib/lib/ocm/services/ClustersService.ts (1)
libs/ui-lib/lib/ocm/services/apis/index.ts (1)
ClustersAPI(5-5)
⏰ 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: unit-tests
- GitHub Check: tests
- GitHub Check: translation-files
- GitHub Check: circular-deps
- GitHub Check: lint
- GitHub Check: format
🔇 Additional comments (3)
libs/ui-lib/lib/common/components/ui/WizardFooter.tsx (1)
72-72: LGTM! Prevents double-submission.Disabling the Next button during submission is correct and improves UX by preventing users from triggering multiple submissions.
libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx (1)
7-7: Optional-configurations step correctly wired into wizard switchThe new
disconnected-optional-configurationscase and import look consistent with the existing disconnected steps; no issues from a routing/rendering perspective.Also applies to: 16-17
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx (1)
69-119: Well-structured submit handler with proper error handling.The refactored submission logic correctly moves navigation into success paths (lines 92, 105), fixing the critical issue from the previous review where the wizard would advance even on failure. The dual code paths for creating vs. updating InfraEnv are clear and error handling is comprehensive.
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
Show resolved
Hide resolved
ElayAharoni
force-pushed
the
add-new-fields-to-above-sea-level-ABI
branch
from
7db2208 to
89d7d59
Compare
|
@ElayAharoni: This pull request references MGMT-22204 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.21.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: 2
♻️ Duplicate comments (4)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx (3)
79-119: Update hard-coded OpenShift version to 4.20.Line 99 hard-codes
openshiftVersion: '4.19', which is outdated. OpenShift 4.20 (GA November 2025) is the current version. This should be updated to match the latest release and kept in sync with BasicStep.tsx line 36.Apply this diff:
const createParams = { name: `disconnected-cluster_infra-env`, pullSecret: values.pullSecret || pullSecret, clusterId: disconnectedCluster.id, - openshiftVersion: '4.19', + openshiftVersion: '4.20', cpuArchitecture: 'x86_64' as const, ...(values.sshPublicKey && { sshAuthorizedKey: values.sshPublicKey }), };
156-161: Capitalize checkbox label for UI consistency.Line 157 uses lowercase "edit pull secret", which is inconsistent with UI conventions. Follow sentence case or title case for better UX.
Apply this diff:
<Checkbox - label="edit pull secret" + label="Edit pull secret" isChecked={editPullSecret} onChange={(_, checked) => setEditPullSecret(checked)} id="edit-pull-secret-checkbox" />
60-63: Restore initial values from existing InfraEnv to support edit flow.When the user navigates back to this step after creating the InfraEnv, the form resets
sshPublicKeyto an empty string (line 61) rather than restoring it fromdisconnectedInfraEnv?.sshAuthorizedKey. This forces the user to re-enter the SSH key if they need to make changes, degrading the user experience.Apply this diff:
const initialValues: OptionalConfigurationsFormValues = { - sshPublicKey: '', + sshPublicKey: disconnectedInfraEnv?.sshAuthorizedKey || '', pullSecret: pullSecret, };Similarly, initialize
editPullSecretstate to reflect whether a custom pull secret was previously set:- const [editPullSecret, setEditPullSecret] = React.useState(false); + const [editPullSecret, setEditPullSecret] = React.useState( + disconnectedInfraEnv ? disconnectedInfraEnv.pullSecret !== pullSecret : false + );libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx (1)
95-100: Avoid duplicating props in state; initialize disconnected entities toundefined.Based on learnings, the SaaS wizard already passes the
clusterandinfraEnvfrom the parent, and initializingdisconnectedClusteranddisconnectedInfraEnvwith these prop values creates confusion about the source of truth. The disconnected flow creates new temporary cluster and infraEnv entities via API calls (see BasicStep.tsx line 34 and OptionalConfigurationsStep.tsx line 103), so these state variables should start asundefinedand be populated only during the disconnected registration workflow.Apply this diff:
- const [disconnectedCluster, setDisconnectedCluster] = React.useState<Cluster | undefined>( - cluster, - ); - const [disconnectedInfraEnv, setDisconnectedInfraEnv] = React.useState<InfraEnv | undefined>( - infraEnv, - ); + const [disconnectedCluster, setDisconnectedCluster] = React.useState<Cluster | undefined>(); + const [disconnectedInfraEnv, setDisconnectedInfraEnv] = React.useState<InfraEnv | undefined>();
🧹 Nitpick comments (3)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx (1)
133-133: Verify async handler behavior withonCancelprop.
handleCancelis an async function, but it's passed toonCancelwhich likely expects a synchronous callback (or at least doesn't await it). The button click will return a Promise that isn't awaited, which is fine for fire-and-forget, but any unhandled rejection from the navigation or state setters could surface as an uncaught promise error.Consider wrapping navigation/state updates in a try/catch to be defensive:
const handleCancel = React.useCallback(async () => { if (disconnectedCluster?.id) { try { await ClustersService.remove(disconnectedCluster.id); } catch (e) { handleApiError(e, () => addAlert({ title: 'Failed to remove cluster', message: getApiErrorMessage(e), }), ); } } + try { setDisconnectedCluster(undefined); setDisconnectedInfraEnv(undefined); navigate('/cluster-list'); + } catch { + // State/navigation errors are non-recoverable + } }, [navigate, disconnectedCluster, setDisconnectedCluster, setDisconnectedInfraEnv, addAlert]);libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx (1)
95-95: Button label doesn't reflect deregistration side effect.The button reads "Download ISO" (line 95) but also deregisters the temporary cluster. Users may not expect the cluster to be removed when clicking "Download ISO". Consider renaming to "Download ISO and Finish" or adding a confirmation dialog explaining that the temporary disconnected cluster will be deregistered.
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx (1)
100-100: Consider making CPU architecture user-configurable.Line 100 hard-codes
cpuArchitecture: 'x86_64', which prevents support for ARM-based clusters. Since the InfraEnv interface supports multiple architectures (per the type definition), consider making this user-selectable or deriving it from cluster configuration rather than hard-coding.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx(1 hunks)libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts(1 hunks)libs/ui-lib/lib/common/components/ui/WizardFooter.tsx(1 hunks)libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts(4 hunks)libs/ui-lib/lib/ocm/services/ClustersService.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/index.ts
- libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts
- libs/ui-lib/lib/ocm/services/ClustersService.ts
- libs/ui-lib/lib/common/components/ui/WizardFooter.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: linoyaslan
Repo: openshift-assisted/assisted-installer-ui PR: 3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts:69-69
Timestamp: 2025-10-19T17:22:52.502Z
Learning: CIM UI changes in the repository openshift-assisted/assisted-installer-ui (e.g., files under libs/ui-lib/lib/cim/) are handled separately by the CIM team and should be tracked via separate issues rather than being included in PRs for other UI components.
📚 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/disconnected/ReviewStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx
📚 Learning: 2025-08-11T06:07:38.056Z
Learnt from: ammont82
Repo: openshift-assisted/assisted-installer-ui PR: 3101
File: libs/ui-lib/lib/common/config/docs_links.ts:203-221
Timestamp: 2025-08-11T06:07:38.056Z
Learning: The MetalLB, OADP, Cluster Observability, and NUMA Resources operators in the virtualization bundle don't require version fallback logic in their documentation link functions (getMetalLbLink, getOadpLink, getClusterObservabilityLink, getNumaResourcesLink) in libs/ui-lib/lib/common/config/docs_links.ts, unlike some other operators like LSO and NVIDIA GPU.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx
🧬 Code graph analysis (4)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx (3)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(32-38)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)libs/ui-lib/lib/common/api/utils.ts (2)
handleApiError(19-51)getApiErrorMessage(53-58)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx (5)
libs/ui-lib/lib/common/features/featureGate.tsx (1)
useFeature(56-59)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(32-38)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)libs/ui-lib/lib/ocm/components/hosts/ModalDialogsContext.tsx (1)
useModalDialogsContext(146-146)libs/ui-lib/lib/common/api/utils.ts (2)
handleApiError(19-51)getApiErrorMessage(53-58)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx (1)
libs/types/assisted-installer-service.d.ts (2)
Cluster(110-407)InfraEnv(1750-1827)
libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts (3)
libs/ui-lib/lib/common/api/axiosClient.ts (1)
client(49-49)libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)libs/ui-lib/lib/ocm/services/apis/index.ts (1)
ClustersAPI(5-5)
⏰ 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: circular-deps
- GitHub Check: translation-files
- GitHub Check: tests
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: format
🔇 Additional comments (17)
apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx (1)
44-46: LGTM!The comment clarifies the distinction between "regular" cluster removal and the disconnected cluster flows introduced in this PR.
libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx (2)
7-7: LGTM!Import follows the established pattern for disconnected step components.
16-17: LGTM!The new case integrates cleanly with the existing step routing pattern.
libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts (1)
17-17: LGTM!The new step name entry is consistent with the existing naming conventions for disconnected wizard steps.
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx (2)
95-111: Good error handling implementation.The error handling addresses the previous review feedback. The cleanup and navigation will execute regardless of whether the removal succeeds or fails, since the catch block doesn't re-throw.
113-116: LGTM!Reformatting to multiline improves readability while preserving the existing behavior.
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx (3)
32-35: LGTM!The imports correctly support the deregistration workflow and error handling.
38-45: LGTM!Context usage aligns with the updated ClusterWizardContext API.
60-93: Confirm error handling masks deregistration failures; verify backend TTL cleanup policy with backend team.The error handling in lines 73–87 catches
ClustersService.remove()failures, displays an alert, but proceeds to clear context and navigate to/cluster-listregardless. This means a failed deregistration (e.g., API error) leaves the cluster registered on the backend while the UI discards local state.Verification shows
ClustersService.remove()is a sequential API orchestration without retry or special failure logic—ifClustersAPI.deregister()fails, the error propagates. No TTL or automatic cleanup logic was found in the UI codebase for disconnected clusters.Action: Confirm with the backend team whether disconnected clusters have a TTL/expiration policy that will clean them up automatically if deregistration fails. If not, consider whether failed deregistration should block navigation or require user action.
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx (3)
18-21: LGTM!Imports are consistent with the API-driven cluster registration workflow.
25-27: LGTM!Local
isSubmittingstate is appropriate here since the Formik form'sonSubmitis a no-op and the actual submission logic is in the customonNexthandler.
63-70: LGTM!The footer correctly wires the async
onNexthandler withvoidto suppress unhandled promise warnings, andisSubmittingappropriately disables the button during API calls.libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx (5)
1-24: LGTM!Imports are comprehensive and appropriate for this Formik-based configuration step.
26-29: LGTM!Type definition is clear and matches the form fields.
31-41: LGTM!The
PullSecretSynccomponent correctly synchronizes external pull secret state with the Formik form field.
51-58: LGTM!Validation schema is correctly memoized and uses the appropriate validation utilities.
121-138: LGTM!The render section correctly wires Formik's
isSubmittingto the footer (no duplicate state needed) and properly disables the Next button based on validation state.
ElayAharoni
force-pushed
the
add-new-fields-to-above-sea-level-ABI
branch
from
89d7d59 to
d96b2b2
Compare
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
ElayAharoni
force-pushed
the
add-new-fields-to-above-sea-level-ABI
branch
from
d96b2b2 to
354f851
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ElayAharoni, rawagner 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
|
@ElayAharoni: This pull request references MGMT-22204 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.21.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: 1
♻️ Duplicate comments (5)
libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts (1)
248-253: Convert parameter naming from camelCase to snake_case to match API convention.The backend consistently uses snake_case (e.g.,
openshift_cluster_idat line 199). The request body should sendopenshift_versioninstead ofopenshiftVersion.Apply this diff:
registerDisconnected(params: { name: string; openshiftVersion: string }) { - return client.post<Cluster, AxiosResponse<Cluster>, { name: string; openshiftVersion: string }>( + return client.post<Cluster, AxiosResponse<Cluster>, { name: string; openshift_version: string }>( `${ClustersAPI.makeBaseURI()}disconnected`, - params, + { name: params.name, openshift_version: params.openshiftVersion }, ); },libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx (2)
35-35: Critical: ConvertdisconnectedClusterIdto React state to trigger re-renders.The variable is declared as a plain
letand assigned inside the asynconNextat line 48, but React doesn't track plain variables for reactivity. The footer at line 81 will always receiveundefinedduring render.Apply this diff:
const { addAlert } = useAlerts(); const [isSubmitting, setIsSubmitting] = React.useState(false); + const [disconnectedClusterId, setDisconnectedClusterId] = React.useState<string | undefined>(); const navigate = useNavigate(); const location = useLocation(); const currentPath = location.pathname; - let disconnectedClusterId: string | undefined;Then update line 48:
- disconnectedClusterId = disconnectedCluster.id; + setDisconnectedClusterId(disconnectedCluster.id);
46-46: Update hard-coded OpenShift version from 4.19 to 4.20.OpenShift 4.20 reached GA in November 2025. Consider centralizing the version in a constant to avoid duplication across BasicStep, OptionalConfigurationsStep, and ReviewStep.
OpenShift 4.20 release date and latest versionlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx (2)
124-124: Update hard-coded OpenShift version from 4.19 to 4.20.OpenShift 4.20 is the latest version (GA November 2025). This version is duplicated in BasicStep.tsx line 46 and ReviewStep.tsx line 133. Consider centralizing it in a constant.
85-87: Restore initial values from existing disconnectedInfraEnv when editing.When users navigate back to this step, the form resets to empty values instead of restoring from
disconnectedInfraEnv. This forces users to re-enter their SSH key.Apply this diff:
const initialValues: OptionalConfigurationsFormValues = { - sshPublicKey: '', + sshPublicKey: disconnectedInfraEnv?.sshAuthorizedKey || '', pullSecret: pullSecret, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx(1 hunks)libs/ui-lib/lib/common/api/assisted-service/ClustersAPI.ts(1 hunks)libs/ui-lib/lib/common/components/ui/WizardFooter.tsx(1 hunks)libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx(1 hunks)libs/ui-lib/lib/ocm/components/Routes.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts(4 hunks)libs/ui-lib/lib/ocm/services/ClustersService.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx
- apps/assisted-disconnected-ui/src/components/ResetSingleClusterModal.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/index.ts
- libs/ui-lib/lib/ocm/services/ClustersService.ts
- libs/ui-lib/lib/common/components/ui/formik/PullSecretField.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/wizardTransition.ts
- libs/ui-lib/lib/ocm/components/Routes.tsx
- libs/ui-lib/lib/ocm/components/clusterWizard/constants.ts
- libs/ui-lib/lib/common/components/ui/WizardFooter.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: linoyaslan
Repo: openshift-assisted/assisted-installer-ui PR: 3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts:69-69
Timestamp: 2025-10-19T17:22:52.502Z
Learning: CIM UI changes in the repository openshift-assisted/assisted-installer-ui (e.g., files under libs/ui-lib/lib/cim/) are handled separately by the CIM team and should be tracked via separate issues rather than being included in PRs for other UI components.
📚 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/disconnected/OptionalConfigurationsStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx
🧬 Code graph analysis (2)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx (1)
libs/types/assisted-installer-service.d.ts (1)
InfraEnv(1750-1827)
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/OptionalConfigurationsStep.tsx (9)
libs/ui-lib-tests/cypress/fixtures/data/dummy-pull-secret.ts (1)
pullSecret(10-10)libs/ui-lib/lib/ocm/hooks/usePullSecret.ts (1)
usePullSecret(6-33)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContext.tsx (1)
useClusterWizardContext(30-36)libs/ui-lib/lib/ocm/components/clusterWizard/index.ts (1)
useClusterWizardContext(5-5)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
sshPublicKeyValidationSchema(106-116)pullSecretValidationSchema(144-163)libs/ui-lib/lib/common/components/ui/formik/utils.ts (1)
getRichTextValidation(75-113)libs/ui-lib/lib/ocm/services/apis/index.ts (1)
InfraEnvsAPI(7-7)libs/ui-lib/lib/common/api/assisted-service/EventsAPI.ts (1)
createParams(21-48)libs/ui-lib/lib/common/api/axiosClient.ts (1)
isInOcm(49-49)
⏰ 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: translation-files
- GitHub Check: unit-tests
- GitHub Check: circular-deps
- GitHub Check: tests
- GitHub Check: format
- GitHub Check: lint
🔇 Additional comments (5)
libs/ui-lib/lib/ocm/components/clusterWizard/NewClusterWizard.tsx (2)
7-7: LGTM!Import is correctly placed and matches the usage in the switch case below.
16-17: LGTM!The new disconnected step is properly integrated into the wizard flow.
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx (1)
265-266: LGTM!Context value correctly exposes the disconnectedInfraEnv state and setter.
libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx (1)
56-86: LGTM!The async onNext flow correctly handles ISO download, cluster deregistration with error handling, and ensures cleanup/navigation occurs regardless of outcome to prevent stuck states.
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardFooter.tsx (1)
96-111: LGTM!The async cancel flow correctly handles cluster removal with error handling, and ensures cleanup (clearing disconnectedInfraEnv) and navigation always occur to prevent stuck states.
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx
Show resolved
Hide resolved
openshift-merge-bot
bot
merged commit 41a3210
into
openshift-assisted:master
|
/cherry-pick releases/v2.48 |
|
@ElayAharoni: #3274 failed to apply on top of branch "releases/v2.48": 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. |
* Remove preview badge from bundles card (#3169) * Migration to PF6 (#3168) * pf6 deps and codemods updates Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> get emptystate pf6 issues building Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix chip/label changes Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix icon type Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix a prop type that codemods missed Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> update pf5 classnames Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix tokens, some cleanup Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix tests and remaining v5->v6 Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> yarn lock Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> work on some cy tests Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix tests for pf6 changes Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix some tests Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix build error Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> * Change version of package.json to 2.0.0 for testing * MGMT-20974: Rremove unnecessary hyphens in helper text operators * MGMT-20968: align 'learn more about openshift releases' link in the first page of the cluster creation wizard * MGMT-20965: Put the same min-width in the dropdowns of the first page of the cluster creation wizard * MGMT-20966: improving the readibility of text on Troubleshooter Modal * MGMT-20972: Cluster summary section is not collapsable * MGMT-20964: Detached Warning Message for TMPv2 selection * Changes in PrismCode to use the correct color * MGMT-20969: hostname column header is truncated and unredeable * MGMT-20967: errors beneath text boxes dissapears but icon remains in place * fix (20975): make bundle selected operators show in count Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> * MGMT-21103: change size of Provisioning type droodown in Add hosts modal * MGMT-21105: Networking page-machine network dropdown truncate values * add layout with gutter around cluster progress/buttons Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> * MGMT-21147: R&C page - missing vertical spacing between kubeconfig warning and buttons * MGMT-21151: inconsistend icon and text alignment in preflight checks in R&C page * Add data-test-id to MenuToggle components * Add data-testid to HelperTextItem components * Add data-testid to BreadCrumb component * Add data-testid to Spinner component * Add data-testid to Wizard components * Add data-testid to Host's network configuration group * Add data-testid to Use bond option * Add data-testid to Add hosts modal * Add more data-testids for tests * Solving problem with operators count * Remove duplicated scrollbars * Solving errors in Networking page * Add data-testid to close button in Events modal * Migration of TextContent component * Solving problems with some old components * Solving lint issues * Solving problems with unit tests * Solving format issues * Remove unnecessary test * Solving problems with tests --------- Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> Co-authored-by: gitdallas <5322142+gitdallas@users.noreply.github.com> * Use masthead (#3173) * Button text (#3174) * OCPBUGS-61953: Update dependency sourcing to remote (#3155) * Bump axios from 1.6.8 to 1.12.2 (#3180) Bumps [axios](https://github.com/axios/axios) from 1.6.8 to 1.12.2. - [Release notes](https://github.com/axios/axios/releases) - [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md) - [Commits](axios/axios@v1.6.8...v1.12.2) --- updated-dependencies: - dependency-name: axios dependency-version: 1.12.2 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Updating assisted-installer-ui-container image to be consistent with ART for 4.21 (#3179) Reconciling with https://github.com/openshift/ocp-build-data/tree/4fbe3fab45239dc4be6f5d9d98a0bf36e0274ec9/images/agent-installer-ui.yml Co-authored-by: AOS Automation Release Team <noreply@redhat.com> * OCPBUGS-61787: Change NMstate operator docs link (#3182) * MGMT-21862: Add message about vSphere limitations (#3181) * OCPBUGS-62680: Include assisted disconnected UI image in release payload (#3188) * Fixing errors when creating cluster from Assisted Migration (#3191) * MGMT-21025: installing Two Node OpenShift with Arbiter (TNA) (#3170) * Clean up ClusterDetailsFormFields * Allow TNA arbiter in CIM * Remove unused component (#3193) * Add unique data-testid to dualstack subnet dropdowns (#3199) * Tweak spacing between advanced network fields (#3198) * Tweak host status spacing (#3200) * Improve alert spacing (#3201) * MGMT-21825: Textarea field should show both error and helper text (#3202) * Improve alert spacing * Show both error and helper text under textarea fields * Add border to table headers (#3197) * Remove border from rich input field (#3208) * Bump happy-dom from 15.10.2 to 20.0.0 (#3211) Bumps [happy-dom](https://github.com/capricorn86/happy-dom) from 15.10.2 to 20.0.0. - [Release notes](https://github.com/capricorn86/happy-dom/releases) - [Commits](capricorn86/happy-dom@v15.10.2...v20.0.0) --- updated-dependencies: - dependency-name: happy-dom dependency-version: 20.0.0 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Remove nested 'Content' components (#3196) * Upgrade the PF modal component in /common and /ocm (#3213) * Bump happy-dom from 20.0.0 to 20.0.2 (#3218) Bumps [happy-dom](https://github.com/capricorn86/happy-dom) from 20.0.0 to 20.0.2. - [Release notes](https://github.com/capricorn86/happy-dom/releases) - [Commits](capricorn86/happy-dom@v20.0.0...v20.0.2) --- updated-dependencies: - dependency-name: happy-dom dependency-version: 20.0.2 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Consistent ISO Download behavior (#3221) * Add 'noopener noreferrer' to ISO download button (#3222) * Fix preflight check collapsed styling (#3215) * Remove unnecessary custom manifest field (#3217) * Bump vite from 5.4.20 to 5.4.21 (#3223) Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 5.4.20 to 5.4.21. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/v5.4.21/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v5.4.21/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 5.4.21 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * MGMT-21625: Add Dual-Stack with Primary IPv6 Support (#3190) This PR adds support for IPv6-primary dual-stack clusters, enabling users to create dual-stack clusters where IPv6 is the primary IP stack (version-aware for OpenShift 4.12+) * sort OCP versions in create infra (#3171) Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * Update types/parsing for latest lightspeed API (#3226) * Update release job to use trusted-publishers flow (#3228) * Update setup-node config (#3232) * - MGMT-22057:Primary IPv6 should be Tech Preview (#3231) - MGMT-22045: Change style of Dualstack - IPv4 / IPv6 section headers in subnet dropdown - When we change the primary machine network IP to ipv6 we have to select the seconday machine network IP to ipv4 * MGMT-22080: Allow users to install Openshift AI as standalone operator in SNO clusters (#3234) * Release job: Configure Yarn for npm registry (#3235) * configure yarn before publish (#3239) * Pass OIDC token to yarn config (#3241) * MGMT-22047: Add apiVIP and ingressVIP for dual-stack ipv4 and ipv6 (#3246) * Add apiVIP and ingressVIP for dual-stack ipv4 and ipv6 * Add validations to ensure that users add primary and secondary apiVips and ingressVips when using dual-stack * Updating tests to dual-stack changes * Prevent InfraEnv creation on ABI to support OVE Late-Binding workflow (#3244) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * MGMT-22116: Add info about apiVIPs and ingressVIPs for dual-stack in Review and Create page (#3250) * Add info about apiVIPs and ingressVIPs for dual-stack in Review and Create page * Fixing review comments * MGMT-22119: Solving errors with SNO dual-stack (#3249) * Solving errors with SNO dual-stack * Solving errors when change between dual-stack and single-stack * MGMT-22165: Remove what's new link until the info appear in the chatbot (#3253) * Update logic to extract the tool response (#3255) * MGMT-17220: dual-stack with second machine network not populates (#3256) * MGMT-17220: dual-stack with second machine network not populates * Improving the code * Make the parsing compatible with new & old API (#3260) * MGMT-22047: solving errors with dual-stack (#3263) * MGMT-22047: solving errors with dual-stack 1. Changes in AvailableSubnetsControl.tsx so our corrective setFieldValue calls don’t validate immediately. This avoids heavy, repeated validations the moment you pick the first IPv6 option and should stop the freeze when secondary VIPs are IPv6 and empty. Specifically, set the third arg to false for: - Auto-select initialization of machineNetworks - Duplicate-primary fix that updates machineNetworks.1.cidr 2. Changes in AdvancesNetworkFields.tsx: -I found the infinite loop in AdvancedNetworkFields.tsx: when the primary machine network flips to IPv6, the effect was swapping clusterNetworks/serviceNetworks even when both entries had the same IP family, causing continuous reorders and a freeze. -I added guards so we only swap when both entries exist and have opposite families, and the first one mismatches the primary machine family. Lint is clean. * Solving issues in code * MGMT-21837: in YAML view in Static Network Configuration we add the radio buttons to change the form to another view (#3268) * late binding hosts to cluster in ABI (#3259) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * AGENT-1373: Rename feature gate to NoRegistryClusterInstall (#3267) Update the feature gate name from NoRegistryClusterOperations to NoRegistryClusterInstall. * AGENT-1352: Handle cluster reset with late binding (#3270) * late binding hosts to cluster in ABI Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Handle cluster reset on ABI Signed-off-by: Elay Aharoni <elayaha@gmail.com> --------- Signed-off-by: Elay Aharoni <elayaha@gmail.com> * MGMT-22281: Dual stack seconday vips fields not mandatory (#3275) * Show TechPreview badge only in Primary Machine network when user chooses ipV6 IP (#3276) * add new fields to above sea level ABI (#3274) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Revert "Handle cluster reset on ABI" (#3281) This reverts commit d35b70a. * add loki and logging operators (#3285) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * OCPBUGS-65657: Display OpenShift AI GPU validation message from API (#3279) Fix UI to show the friendly label and detailed message from the API instead of displaying the technical validation ID when OpenShift AI operator is selected without GPU support. * Bug fix: OVE Agent Installer UI: Red Hat OCP logo not displaying correctly (#3289) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Adding TechPreview Budge for Assisted installer and agent (#3293) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * MGMT-20138 : Show 'Add hosts' tab for all cases (#3297) * Edit OWNERS file (#3172) * Bump js-yaml from 4.1.0 to 4.1.1 (#3264) Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 4.1.0 to 4.1.1. - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](nodeca/js-yaml@4.1.0...4.1.1) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 4.1.1 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump zx from 7.2.3 to 8.8.5 (#3273) Bumps [zx](https://github.com/google/zx) from 7.2.3 to 8.8.5. - [Release notes](https://github.com/google/zx/releases) - [Commits](google/zx@7.2.3...8.8.5) --- updated-dependencies: - dependency-name: zx dependency-version: 8.8.5 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump mdast-util-to-hast from 13.2.0 to 13.2.1 (#3287) Bumps [mdast-util-to-hast](https://github.com/syntax-tree/mdast-util-to-hast) from 13.2.0 to 13.2.1. - [Release notes](https://github.com/syntax-tree/mdast-util-to-hast/releases) - [Commits](syntax-tree/mdast-util-to-hast@13.2.0...13.2.1) --- updated-dependencies: - dependency-name: mdast-util-to-hast dependency-version: 13.2.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * provides parameters for the GET /v2/operators/bundles route (#3306) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Bump js-yaml from 4.1.0 to 4.1.1 (#3310) Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 4.1.0 to 4.1.1. - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](nodeca/js-yaml@4.1.0...4.1.1) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 4.1.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix Cluster summary styling (#3216) * remove external platforms field from below sea level UI (#3316) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Openshift AI Bundle on SNO enables ODF and LVM which are uncompatible (#3320) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * change ABI above sea level iso size (#3322) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * MGMT-22465: Merge the contents of releases/v2.17-cim into master (#3319) * Fix patches when there were 0 nodes in hypershift nodepool (#3177) * Make OVN the default network type if the version is invalid (#3183) * MGMT-20076: Support external platform in Assisted-installer Kube API (#3151) * Add External platforms field * Restructure files related to CIM cluster deployment wizard * Create SelectFieldWithSearch component * Update '@openshift-console/dynamic-plugin-sdk' * CIM custom manifest step * CIM custom manifests review * Make 'Baremetal' the default external platform * Translations for 2.16-cim (#3225) * MGMT-21025: installing Two Node OpenShift with Arbiter (TNA) (#3224) * Clean up ClusterDetailsFormFields * Allow TNA arbiter in CIM * Set 'userManagedNetworking' as true with 'external' platform (#3245) * Tweak TNA-related strings in CIM (#3248) * Restrict platform options for SNO clusters (#3262) Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * Fix: Undefined OpenShift version producing an incorrect documentation link (#3271) Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * Fix bug - the option to remove host from the cluster disabled while the host is installing (#3219) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Implement user interface for the multiple SSH keys (#3292) Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * Edit OWNERS file (#3303) Co-authored-by: jgyselov <jgyselov@redhat.com> * Fix arbiter translation (#3304) Co-authored-by: jgyselov <jgyselov@redhat.com> * [releases/v2.17-cim] MGMT-22264: Add 'Labels' and 'GPU' columns to infra env host table (#3307) * Add GPUs column to infra agent table * Add labels column and filtering to infra agent table --------- Co-authored-by: jgyselov <jgyselov@redhat.com> * Prevent mass approve crash when hosts are still discovering (#3305) Co-authored-by: jgyselov <jgyselov@redhat.com> * Fix 'Required' translations (#3308) Co-authored-by: jgyselov <jgyselov@redhat.com> * Do not exclude hosts with SpecSyncError status from host selection during binding (#3309) Co-authored-by: jgyselov <jgyselov@redhat.com> * MGMT-22438: Handle empty labels in infra env host table (#3314) * Handle empty labels in infra env host table Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * format fix Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> --------- Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * MGMT-19743: Icon from agent status available is confusing (#3220) * Fix bug - the option to remove host from the cluster disabled while the host is installing Signed-off-by: Elay Aharoni <elayaha@gmail.com> * icon from agent status Available is confusing Signed-off-by: Elay Aharoni <elayaha@gmail.com> --------- Signed-off-by: Elay Aharoni <elayaha@gmail.com> Co-authored-by: Julie Gyselova <jgyselov@redhat.com> --------- Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com> Co-authored-by: Lior Soffer <liorsoffer1@gmail.com> Co-authored-by: Elay Aharoni <elayaha@gmail.com> Co-authored-by: OpenShift Cherrypick Robot <openshift-cherrypick-robot@redhat.com> --------- Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com> Co-authored-by: Montse Ortega Gallart <ammont82@users.noreply.github.com> Co-authored-by: gitdallas <5322142+gitdallas@users.noreply.github.com> Co-authored-by: Pawan Pinjarkar <ppinjark@redhat.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: OpenShift Bot <openshift-bot@redhat.com> Co-authored-by: AOS Automation Release Team <noreply@redhat.com> Co-authored-by: Linoy Hadad <Linoyaslan@gmail.com> Co-authored-by: Lior Soffer <liorsoffer1@gmail.com> Co-authored-by: Rastislav Wagner <rawagner@redhat.com> Co-authored-by: Elay Aharoni <elayaha@gmail.com> Co-authored-by: Richard Su <rwsu@redhat.com> Co-authored-by: Yoav Schwammenthal <33420608+yoavsc0302@users.noreply.github.com> Co-authored-by: OpenShift Cherrypick Robot <openshift-cherrypick-robot@redhat.com>
This reverts commit 41a3210.
* Revert "Adding TechPreview Budge for Assisted installer and agent (#3328)" This reverts commit e61e69b. * Revert "change ABI above sea level iso size (#3326)" This reverts commit d189582. * Reapply "Handle cluster reset on ABI" (#3281) This reverts commit 84b8ef8. * Revert "add new fields to above sea level ABI (#3274)" This reverts commit 41a3210.
This reverts commit 41a3210.
* Revert "add new fields to above sea level ABI (#3274)" This reverts commit 41a3210. * Revert "API/Ingress IP fields lack character limit (#3342)" This reverts commit cd01017. * Revert "Assisted Installer OVE UI shows transient blank page with Technology Preview spinner (#3341)" This reverts commit d47ab5f. * remove operators and change image url Signed-off-by: Elay Aharoni <elayaha@gmail.com> --------- Signed-off-by: Elay Aharoni <elayaha@gmail.com>
This reverts commit 41a3210.
* Revert "UI freezes when clicking Select button without version (#3401)" This reverts commit 52c39a9. * Revert "OCPBUGS-75072 | OVE UI: Above-the-sea UI QE test (#3383)" This reverts commit f61e1b1. * Revert "OCPBUGS-74421 | OVE UI: Fix rendezvousIP field name in error message (#3376)" This reverts commit 142fdc5. * Revert "Add additional new fields to the above the sea UI (#3337)" This reverts commit 2283bcd. * Revert "add new fields to above sea level ABI (#3274)" This reverts commit 41a3210. * Revert "Assisted Installer OVE UI shows transient blank page with Technology Preview spinner (#3341)" This reverts commit d47ab5f.
5 participants
https://issues.redhat.com/browse/MGMT-22204
added a new step to above sea level ABI flow, with optional configurations where the user can add a public ssh key and edit the pull secret if he wants.
it interacts with @omer-vishlitzky recent pr: openshift/assisted-service#8144
Summary by CodeRabbit
New Features
UI/UX Improvements
Style
✏️ Tip: You can customize this high-level summary in your review settings.