AGENT-1160: ABI - add finalizing page#2894
AGENT-1160: ABI - add finalizing page#2894openshift-merge-bot[bot] merged 1 commit intoopenshift-assisted:masterfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@rawagner: This pull request references AGENT-1160 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 sub-task to target the "4.19.0" version, but no target version was set. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
approved
|
[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 |
WalkthroughThis pull request introduces a new custom hook, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClusterPage
participant useFinalizerPage
participant API
participant UI
User->>ClusterPage: Request cluster page
ClusterPage->>useFinalizerPage: Invoke hook to manage finalizer logic
useFinalizerPage->>API: Fetch console URL if status is 'installing' or 'installing-pending-user-action'
API-->>useFinalizerPage: Return console URL
alt Check polling error and bootstrap host
useFinalizerPage->>useFinalizerPage: Determine bootstrap progress stage
end
useFinalizerPage-->>ClusterPage: Provide cluster state and consoleUrl
alt Finalizer conditions met
ClusterPage->>UI: Render SingleClusterFinalizerPage
else
ClusterPage->>UI: Render SingleClusterPage
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/assisted-disconnected-ui/src/components/SingleClusterFinalizerPage.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/apps/assisted-disconnected-ui/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. libs/ui-lib/lib/common/components/clusterDetail/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/libs/ui-lib/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. apps/assisted-disconnected-ui/src/components/ClusterPage.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/apps/assisted-disconnected-ui/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@rawagner: This pull request references AGENT-1160 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 sub-task to target the "4.19.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: 0
🧹 Nitpick comments (1)
apps/assisted-disconnected-ui/src/components/ClusterPage.tsx (1)
40-52: Consider refactoring complex bootstrap host state logicThe logic for checking bootstrap host state is quite complex and nested. This could be extracted into a separate utility function for better readability and testability.
- if (!clusterRef.current && cluster && uiState === ResourceUIState.POLLING_ERROR) { - const boostrapHost = cluster.hosts?.find((h) => h.bootstrap); - if (boostrapHost) { - const stages = getHostProgressStages(boostrapHost); - const rebootIdx = stages.findIndex((s) => s === 'Rebooting'); - const currentStage = boostrapHost.progress?.currentStage; - // Show finalizing page as soon as we fail to poll and bootstrap is Rebooting or state before - if (!!currentStage && stages.slice(rebootIdx - 2, stages.length).includes(currentStage)) { - clusterRef.current = cluster; - } - } - } + if (shouldShowFinalizingPage(clusterRef, cluster, uiState)) { + clusterRef.current = cluster; + }Add this function above the hook:
const shouldShowFinalizingPage = ( clusterRef: React.MutableRefObject<Cluster | undefined>, cluster: Cluster | undefined, uiState: ResourceUIState ): boolean => { if (clusterRef.current || !cluster || uiState !== ResourceUIState.POLLING_ERROR) { return false; } const boostrapHost = cluster.hosts?.find((h) => h.bootstrap); if (!boostrapHost) { return false; } const stages = getHostProgressStages(boostrapHost); const rebootIdx = stages.findIndex((s) => s === 'Rebooting'); const currentStage = boostrapHost.progress?.currentStage; // Show finalizing page as soon as we fail to poll and bootstrap is Rebooting or state before return !!currentStage && stages.slice(rebootIdx - 2, stages.length).includes(currentStage); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/assisted-disconnected-ui/src/components/ClusterPage.tsx(1 hunks)apps/assisted-disconnected-ui/src/components/SingleClusterFinalizerPage.tsx(1 hunks)libs/locales/lib/en/translation.json(3 hunks)libs/ui-lib/lib/common/components/clusterDetail/index.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/RebootNodeZeroModal.css(0 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/RebootNodeZeroModal.tsx(0 hunks)libs/ui-lib/lib/ocm/components/clusters/ClusterPage.tsx(0 hunks)
💤 Files with no reviewable changes (3)
- libs/ui-lib/lib/ocm/components/clusterDetail/RebootNodeZeroModal.css
- libs/ui-lib/lib/ocm/components/clusters/ClusterPage.tsx
- libs/ui-lib/lib/ocm/components/clusterDetail/RebootNodeZeroModal.tsx
🧰 Additional context used
🧬 Code Definitions (2)
apps/assisted-disconnected-ui/src/components/SingleClusterFinalizerPage.tsx (3)
libs/types/assisted-installer-service.d.ts (1)
Cluster(110-398)libs/ui-lib/lib/common/hooks/index.ts (1)
useTranslation(3-3)libs/ui-lib/lib/common/components/clusterDetail/index.ts (1)
TroubleshootingOpenshiftConsoleButton(6-6)
apps/assisted-disconnected-ui/src/components/ClusterPage.tsx (4)
libs/types/assisted-installer-service.d.ts (1)
Cluster(110-398)libs/ui-lib/lib/ocm/services/apis/index.ts (1)
ClustersAPI(5-5)libs/ui-lib/lib/common/index.ts (1)
ResourceUIState(13-13)libs/ui-lib/lib/common/components/hosts/utils.ts (1)
getHostProgressStages(110-110)
🔇 Additional comments (14)
libs/ui-lib/lib/common/components/clusterDetail/index.ts (1)
6-6: Appropriate export for new component usageThe export of
TroubleshootingOpenshiftConsoleButtonfrom the ConsoleModal module makes sense since it will be used in the newly addedSingleClusterFinalizerPagecomponent. This enhances the module's public API with a clear purpose.apps/assisted-disconnected-ui/src/components/SingleClusterFinalizerPage.tsx (6)
1-16: Clean import structureImports are properly organized with related modules grouped together. The component makes use of PatternFly UI elements and correctly imports the newly exported
TroubleshootingOpenshiftConsoleButton.
17-23: Well-structured component interfaceThe component has a clear interface with proper TypeScript typing. It accepts a required
clusterprop and an optionalconsoleUrlprop, making its usage requirements explicit.
24-42: Good use of internationalization and clear user instructionsThe component properly uses the translation hook and provides clear, informative messages to guide users during the cluster installation process. The multi-line explanation is well-formatted with line breaks for better readability.
43-54: Appropriate conditional rendering and securityThe component appropriately checks if a console URL exists before rendering the button. The use of
isInlinefor the button enhances the UI layout. Opening links in a new tab with thenoopenerattribute addresses potential security concerns.
53-54: Good integration with existing componentsThe
TroubleshootingOpenshiftConsoleButtonis correctly integrated, passing through the necessary props for proper functionality.
55-61: Clean component exportThe component is properly exported as the default export, allowing for clean imports in parent components.
apps/assisted-disconnected-ui/src/components/ClusterPage.tsx (4)
1-8: Improved import organizationThe imports have been updated to include the necessary components and utilities for the new functionality. Using named imports helps with code readability.
14-15: Clear Redux selector definitionThe selector function is defined at the module level which is a good practice for performance optimization, preventing unnecessary re-creation of the function on each render.
16-39: Well-designed hook for finalization stage detectionThe
useFinalizerPagehook is well-structured with a clear purpose - determining when to show the finalization page and fetching the console URL. The async function is properly defined with error handling, and the effect has appropriate dependencies.
56-65: Elegant conditional rendering using hook dataThe ClusterPage component has been updated to conditionally render either the new finalizer page or the regular cluster page based on the hook results. This separation of concerns makes the component clean and maintainable.
libs/locales/lib/en/translation.json (3)
30-30: Clear instruction for credential file locationThis translation entry provides users with important information about finding the webconsole URL in the credentials file if they close the browser window.
164-164: Actionable guidance for webconsole verificationThis translation properly instructs users to check if the webconsole is running by clicking the URL below.
177-177: Clear status messageThis translation provides a straightforward status message informing users that the installation is still in progress.
openshift-merge-bot
bot
merged commit 9130081
into
openshift-assisted:master
3 participants
Summary by CodeRabbit
New Features
Documentation
Chores