MGMT-20898: In disconnected Standalone cluster installation ClusterImageSet is not getting populated in AgentClusterInstall#3019
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@jgyselov: This pull request references MGMT-20898 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.20.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
WalkthroughA new English translation string was added, and the OpenShiftVersionDropdown component was updated to improve dropdown rendering and event handling. The dropdown now conditionally displays groups, uses translation for labels, prevents default selection on a specific item, and refines logic for enabling the toggle and handling selections. Additionally, ClusterDetailsFormFields initializes OpenShift version fields with a default fallback on mount if no versions are available. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDetailsFormFields.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 "/libs/ui-lib/.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/ui/OpenShiftVersionDropdown.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 "/libs/ui-lib/.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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
@jgyselov: This pull request references MGMT-20898 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.20.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. |
|
/cherry-pick releases/v2.14-cim |
|
@jgyselov: once the present PR merges, I will cherry-pick it on top of 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. |
|
@jgyselov: This pull request references MGMT-20898 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.20.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. |
|
/cherry-pick releases/v2.15-cim |
|
@jgyselov: once the present PR merges, I will cherry-pick it on top of 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. |
jgyselov
force-pushed
the
openshift_versions_always_enabled
branch
from
5287967 to
b7aed1b
Compare
openshift-ci
bot
added
size/M
|
@jgyselov: This pull request references MGMT-20898 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.20.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)
libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDetailsFormFields.tsx (1)
71-78: Consider improving dependency management for the fallback logic.The fallback logic correctly addresses the disconnected installation issue, but the empty dependency array with disabled lint rule could lead to stale closures.
Consider this approach to properly handle dependencies:
React.useEffect(() => { if (!versions.length && !values.openshiftVersion) { const fallbackOpenShiftVersion = allVersions.find((version) => version.default); setFieldValue('customOpenshiftSelect', fallbackOpenShiftVersion); setFieldValue('openshiftVersion', fallbackOpenShiftVersion?.value); } - // eslint-disable-next-line react-hooks/exhaustive-deps -}, []); +}, [versions.length, values.openshiftVersion, allVersions, setFieldValue]);Alternatively, if the "run once on mount" behavior is specifically required, consider using a ref to track if the effect has run:
+const hasSetFallback = React.useRef(false); + React.useEffect(() => { - if (!versions.length && !values.openshiftVersion) { + if (!hasSetFallback.current && !versions.length && !values.openshiftVersion) { const fallbackOpenShiftVersion = allVersions.find((version) => version.default); setFieldValue('customOpenshiftSelect', fallbackOpenShiftVersion); setFieldValue('openshiftVersion', fallbackOpenShiftVersion?.value); + hasSetFallback.current = true; } - // eslint-disable-next-line react-hooks/exhaustive-deps -}, []); +}, [versions.length, values.openshiftVersion, allVersions, setFieldValue]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
libs/locales/lib/en/translation.json(1 hunks)libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDetailsFormFields.tsx(2 hunks)libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/locales/lib/en/translation.json
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/ui-lib/lib/common/components/ui/OpenShiftVersionDropdown.tsx
🧰 Additional context used
🧠 Learnings (1)
libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDetailsFormFields.tsx (1)
Learnt from: rawagner
PR: openshift-assisted/assisted-installer-ui#2942
File: libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDeploymentManifestsStep.tsx:93-97
Timestamp: 2025-05-22T09:12:50.632Z
Learning: When working with Formik's FieldArray, use the provided array helpers (like `remove()`, `push()`, etc.) instead of directly mutating the values array to ensure proper state tracking and re-rendering.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: tests
- GitHub Check: circular-deps
- GitHub Check: lint
- GitHub Check: translation-files
- GitHub Check: unit-tests
- GitHub Check: format
🔇 Additional comments (1)
libs/ui-lib/lib/cim/components/ClusterDeployment/ClusterDetailsFormFields.tsx (1)
58-58: LGTM! Proper Formik context usage.Adding
setFieldValueto the destructuring follows Formik best practices and is needed for the new fallback logic.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jgyselov, 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 |
|
/lgtm |
openshift-merge-bot
bot
merged commit c08bc2d
into
openshift-assisted:master
|
@jgyselov: #3019 failed to apply on top of branch "releases/v2.14-cim": 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. |
|
@jgyselov: new pull request created: #3023 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. |
4 participants
https://issues.redhat.com/browse/MGMT-20898
Let's always leave the dropdown enabled so that users can access the All versions modal even if the versions list is empty.
Summary by CodeRabbit
New Features
Bug Fixes
Enhancements