Remove ABI check#3118
Conversation
|
Skipping CI for Draft Pull Request. |
WalkthroughNarrows AssistedInstallerFeatureType to only 'ASSISTED_INSTALLER_SINGLE_CLUSTER_FEATURE' and removes use of 'ASSISTED_INSTALLER_ABI' in ClusterDetailsForm. The disconnected/install Switch now renders based solely on not being in single-cluster mode. No other behavior or API signatures are changed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClusterDetailsForm
participant FeatureGate
User->>ClusterDetailsForm: Open Cluster Details
ClusterDetailsForm->>FeatureGate: useFeature('ASSISTED_INSTALLER_SINGLE_CLUSTER_FEATURE')
FeatureGate-->>ClusterDetailsForm: isSingleClusterFeatureEnabled
alt Not single-cluster
ClusterDetailsForm-->>User: Render disconnected/install Switch
else Single-cluster
ClusterDetailsForm-->>User: Hide disconnected/install Switch
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ammont82, 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 |
|
/cherry-pick releases/v2.44 |
|
@rawagner: 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. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetailsForm.tsx (1)
167-177: Optional: consider i18n for the label text.The switch label is a hard-coded string while the rest of the form leverages i18n. Consider wrapping the label in
t(...)for consistency.libs/ui-lib/lib/common/features/featureGate.tsx (1)
3-8: Optional: derive the feature string type from a single source to avoid drift.To keep the string literal list and type in sync as features evolve, you can define a single const and derive the type from it.
Apply this diff near the top:
-// Must conform Unleash constants -export type AssistedInstallerFeatureType = 'ASSISTED_INSTALLER_SINGLE_CLUSTER_FEATURE'; +// Must conform Unleash constants +export const AssistedInstallerFeatures = ['ASSISTED_INSTALLER_SINGLE_CLUSTER_FEATURE'] as const; +export type AssistedInstallerFeatureType = typeof AssistedInstallerFeatures[number];No other changes are required; existing usages of
AssistedInstallerFeatureTypecontinue to work.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
libs/ui-lib/lib/common/features/featureGate.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetailsForm.tsx(1 hunks)
🔇 Additional comments (4)
libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetailsForm.tsx (2)
167-177: Intentional broadening of switch visibility — aligns with “Remove ABI check”.Dropping the ABI gate to render the disconnected/air‑gapped switch whenever not in single‑cluster mode matches the PR objective. No other logic paths are impacted here.
167-177: No ASSISTED_INSTALLER_ABI references found — review remaining ASSISTED_INSTALLER_ feature-flag usage*Quick summary: a repo-wide search found no occurrences of ASSISTED_INSTALLER_ABI, isABIEnabled, or useFeature('ASSISTED_INSTALLER_ABI'). There are still many places using ASSISTED_INSTALLER_SINGLE_CLUSTER_FEATURE and the installDisconnected flag — confirm whether those feature gates should remain now that ABI gating was removed.
Files/locations to review:
- libs/ui-lib/lib/ocm/hooks/use-feature-detection.ts (handles ASSISTED_INSTALLER* overrides)
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterWizardContextProvider.tsx (defines installDisconnected; useFeature('ASSISTED_INSTALLER_SINGLE_CLUSTER_FEATURE'))
- libs/ui-lib/lib/ocm/components/clusterWizard/ClusterDetailsForm.tsx (original snippet; Switch controlled by installDisconnected and gated by !isSingleClusterFeatureEnabled)
- libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/BasicStep.tsx (disconnected step uses installDisconnected)
- libs/ui-lib/lib/ocm/components/clusterConfiguration/OcmClusterDetailsFormFields.tsx (conditional UI based on isSingleClusterFeatureEnabled)
- libs/ui-lib/lib/ocm/components/clusters/AssistedInstallerHeader.tsx (DeveloperPreview gated by single-cluster feature)
- libs/ui-lib/lib/ocm/components/hosts/use-hosts-table.tsx (logic conditional on isSingleClusterFeatureEnabled)
- Note: vendor Go files under apps/assisted-disconnected-ui/proxy contain unrelated "ABI" comments — safe to ignore.
Actionable next step: if the goal was only to remove ASSISTED_INSTALLER_ABI, no further code changes are required. If you intended to remove any ABI-driven behavior entirely, update or remove the ASSISTED_INSTALLER_SINGLE_CLUSTER_FEATURE checks and the installDisconnected flow in the files above.
libs/ui-lib/lib/common/features/featureGate.tsx (2)
4-4: Type narrowing is consistent with removing ABI.Restricting
AssistedInstallerFeatureTypeto only'ASSISTED_INSTALLER_SINGLE_CLUSTER_FEATURE'aligns with the goal to drop ABI. The hardcoded feature maps below are already consistent with this narrowed type.
4-4: Double-check for external/API breakage and lingering references.I ran the provided ripgrep for ASSISTED_INSTALLER_ABI — no matches were returned from the repository.
- libs/ui-lib/lib/common/features/featureGate.tsx (line ~4): export type AssistedInstallerFeatureType = 'ASSISTED_INSTALLER_SINGLE_CLUSTER_FEATURE';
Please confirm any external consumers have been updated (or release this change under a breaking-change version bump).
134a52a
into
openshift-assisted:master
|
@rawagner: new pull request created: #3119 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. |
Summary by CodeRabbit
New Features
Refactor