Skip to content

[RHSTOR-7189] Implement Multus network isolation acknowledgment flow #2110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions locales/en/plugin__odf-console.json
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@
"Buckets": "Buckets",
"Buckets card represents the number of S3 buckets managed on Multicloud Object Gateway and the number of ObjectBucketClaims and the ObjectBuckets managed on both Multicloud Object Gateway and RGW (if deployed).": "Buckets card represents the number of S3 buckets managed on Multicloud Object Gateway and the number of ObjectBucketClaims and the ObjectBuckets managed on both Multicloud Object Gateway and RGW (if deployed).",
"By {{label}}": "By {{label}}",
"By checking this box, you acknowledge running the validation test": "By checking this box, you acknowledge running the validation test",
"By enabling thick-provisioning, volumes will allocate the requested capacity upon volume creation. Volume creation will be slower when thick-provisioning is enabled.": "By enabling thick-provisioning, volumes will allocate the requested capacity upon volume creation. Volume creation will be slower when thick-provisioning is enabled.",
"By: {{serviceType}}": "By: {{serviceType}}",
"CA certificate": "CA certificate",
Expand Down Expand Up @@ -439,6 +440,7 @@
"Cluster expansion limit": "Cluster expansion limit",
"Cluster has reached its expansion limit.": "Cluster has reached its expansion limit.",
"Cluster health": "Cluster health",
"Cluster health issues if network attachments are misconfigured": "Cluster health issues if network attachments are misconfigured",
"Cluster Metadata": "Cluster Metadata",
"Cluster name": "Cluster name",
"Cluster name (ID)": "Cluster name (ID)",
Expand Down Expand Up @@ -587,6 +589,7 @@
"Data Foundation will create a dedicated StorageClass.": "Data Foundation will create a dedicated StorageClass.",
"Data Foundation will use a StorageClass provided by the Local Storage Operator (LSO) on top of your attached drives. This option is available on any platform with devices attached to nodes.": "Data Foundation will use a StorageClass provided by the Local Storage Operator (LSO) on top of your attached drives. This option is available on any platform with devices attached to nodes.",
"Data Foundation will use an existing StorageClass available on your hosting platform.": "Data Foundation will use an existing StorageClass available on your hosting platform.",
"Data Foundation will use the default pod network": "Data Foundation will use the default pod network",
"Data Foundation's StorageCluster is not available. Try again after the StorageCluster is ready to use.": "Data Foundation's StorageCluster is not available. Try again after the StorageCluster is ready to use.",
"Data is stored on the NamespaceStores without performing de-duplication, compression, or encryption. BucketClasses of namespace type allow connecting to existing data and serving from them. These are best used for existing data or when other applications (and cloud-native services) need to access the data from outside Data Foundation.": "Data is stored on the NamespaceStores without performing de-duplication, compression, or encryption. BucketClasses of namespace type allow connecting to existing data and serving from them. These are best used for existing data or when other applications (and cloud-native services) need to access the data from outside Data Foundation.",
"Data loss may occur, only recommended for small clusters or when backups are available or data loss is acceptable": "Data loss may occur, only recommended for small clusters or when backups are available or data loss is acceptable",
Expand All @@ -595,6 +598,7 @@
"Data resiliency": "Data resiliency",
"Data Resiliency": "Data Resiliency",
"Data Services": "Data Services",
"Data unavailability or loss due to broken internal communication": "Data unavailability or loss due to broken internal communication",
"Data will be consumed by a Multi-cloud object gateway, deduped, compressed, and encrypted. The encrypted chunks would be saved on the selected BackingStores. Best used when the applications would always use the Data Foundation endpoints to access the data.": "Data will be consumed by a Multi-cloud object gateway, deduped, compressed, and encrypted. The encrypted chunks would be saved on the selected BackingStores. Best used when the applications would always use the Data Foundation endpoints to access the data.",
"Database name": "Database name",
"day": "day",
Expand Down Expand Up @@ -931,6 +935,7 @@
"If not labeled, the selected nodes are labeled <2>{{label}}</2> to make them target hosts for Data Foundation's components.": "If not labeled, the selected nodes are labeled <2>{{label}}</2> to make them target hosts for Data Foundation's components.",
"If not provided a generic name will be generated.": "If not provided a generic name will be generated.",
"If the name you write exists, we will be using the existing folder if not we will create a new folder ": "If the name you write exists, we will be using the existing folder if not we will create a new folder ",
"If you require a custom network configuration, you can modify the network settings after deployment.": "If you require a custom network configuration, you can modify the network settings after deployment.",
"If you wish to use the Arbiter stretch cluster, a minimum of 4 nodes (2 different zones, 2 nodes per zone) and 1 additional zone with 1 node is required. All nodes must be pre-labeled with zones in order to be validated on cluster creation.": "If you wish to use the Arbiter stretch cluster, a minimum of 4 nodes (2 different zones, 2 nodes per zone) and 1 additional zone with 1 node is required. All nodes must be pre-labeled with zones in order to be validated on cluster creation.",
"Image States": "Image States",
"image states info": "image states info",
Expand Down Expand Up @@ -962,6 +967,7 @@
"Inventory": "Inventory",
"IOPS": "IOPS",
"IP address": "IP address",
"Isolate Network": "Isolate Network",
"Isolate network using Multus": "Isolate network using Multus",
"Isolate network using NIC Operators": "Isolate network using NIC Operators",
"Just now": "Just now",
Expand Down Expand Up @@ -1077,6 +1083,7 @@
"MultiCloud Object Gateway supports encryption for objects by default.": "MultiCloud Object Gateway supports encryption for objects by default.",
"Multiple types of BackingStores are supported: asws-s3 s3-compatible google-cloud-storage azure-blob obc PVC.": "Multiple types of BackingStores are supported: asws-s3 s3-compatible google-cloud-storage azure-blob obc PVC.",
"Multiple types of BackingStores are supported: AWS S3 S3 Compatible Google Cloud Storage Azure Blob PVC.": "Multiple types of BackingStores are supported: AWS S3 S3 Compatible Google Cloud Storage Azure Blob PVC.",
"Multus allows a network separation between the data operations and the control plane operations.": "Multus allows a network separation between the data operations and the control plane operations.",
"Multus allows a network seperation between the data operations and the control plane operations.": "Multus allows a network seperation between the data operations and the control plane operations.",
"Must be a positive number 0 Byte or higher.": "Must be a positive number 0 Byte or higher.",
"Must be a positive number 1 Byte or higher.": "Must be a positive number 1 Byte or higher.",
Expand Down Expand Up @@ -1355,6 +1362,7 @@
"PVC label selector:": "PVC label selector:",
"PVC label selectors": "PVC label selectors",
"PVC label selectors:": "PVC label selectors:",
"PVC mounting failures for ODF-dependent workloads": "PVC mounting failures for ODF-dependent workloads",
"Rack": "Rack",
"RADOS Object Gateway": "RADOS Object Gateway",
"Raw capacity": "Raw capacity",
Expand Down Expand Up @@ -1552,6 +1560,7 @@
"Set criteria for filtering objects based on their size.": "Set criteria for filtering objects based on their size.",
"Set default StorageClass for virtualization": "Set default StorageClass for virtualization",
"Set up a storage cluster to view the topology": "Set up a storage cluster to view the topology",
"Set up Multus by following relevant steps in KCS. Incorrectly setting up Multus might lead to:": "Set up Multus by following relevant steps in KCS. Incorrectly setting up Multus might lead to:",
"Share link": "Share link",
"Share object with a presigned URL": "Share object with a presigned URL",
"Share with presigned URL": "Share with presigned URL",
Expand Down Expand Up @@ -1757,6 +1766,7 @@
"This URL is used to access and manage secrets, keys, and certificates stored in Azure Key Vault.": "This URL is used to access and manage secrets, keys, and certificates stored in Azure Key Vault.",
"This URL will automatically expire based on your configured time or when your current session expires.": "This URL will automatically expire based on your configured time or when your current session expires.",
"This view is not available for external mode cluster.": "This view is not available for external mode cluster.",
"This will isolate network to attach additional clusters as external clients. Run Validation test before to proceed further.": "This will isolate network to attach additional clusters as external clients. Run Validation test before to proceed further.",
"This will remove the StorageConsumer from the environment. This action cannot be undone.": "This will remove the StorageConsumer from the environment. This action cannot be undone.",
"thousands": "thousands",
"Throughput": "Throughput",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ export const NetworkFormGroup: React.FC<NetworkFormGroupProps> = ({
clusterNetwork,
publicNetwork,
systemNamespace,
isMultusAcknowledged,
setIsMultusAcknowledged,
}) => {
const { t } = useCustomTranslation();
const isFDF = useFlag(FDF_FLAG);
Expand Down Expand Up @@ -79,6 +81,8 @@ export const NetworkFormGroup: React.FC<NetworkFormGroupProps> = ({
systemNamespace={systemNamespace}
setNetworkType={(type: NetworkType) => setNetworkType(type)}
networkType={networkType}
isMultusAcknowledged={isMultusAcknowledged}
setIsMultusAcknowledged={setIsMultusAcknowledged}
/>
)}
</>
Expand All @@ -98,4 +102,6 @@ type NetworkFormGroupProps = {
cephClusterCIDR: string;
cephPublicCIDR: string;
systemNamespace: WizardState['backingStorage']['systemNamespace'];
isMultusAcknowledged: boolean;
setIsMultusAcknowledged: (val: boolean) => void;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.odf-multus-indent {
margin-left: 1rem;
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,16 @@ import {
WatchK8sResults,
} from '@openshift-console/dynamic-plugin-sdk';
import { SelectOption } from '@patternfly/react-core/deprecated';
import { flatMap } from 'lodash-es';
Copy link
Collaborator

@alfonsomthd alfonsomthd Jul 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary as we're already importing * as 'lodash-es' in the following line.

import * as _ from 'lodash-es';
import { Checkbox, FormGroup } from '@patternfly/react-core';
import {
Alert,
AlertVariant,
Checkbox,
FormGroup,
} from '@patternfly/react-core';
import { WizardState } from '../../reducer';
import './multus.scss';

type MultusWatchResourcesObject = {
multus: NetworkAttachmentDefinitionKind[];
Expand Down Expand Up @@ -52,6 +59,8 @@ type MultusNetworkConfigurationComponentProps = {
clusterNetwork: NetworkAttachmentDefinitionKind;
publicNetwork: NetworkAttachmentDefinitionKind;
systemNamespace: WizardState['backingStorage']['systemNamespace'];
isMultusAcknowledged: boolean;
setIsMultusAcknowledged: (val: boolean) => void;
};

const reduceResourceLoadAndErrorStatus = <
Expand Down Expand Up @@ -80,35 +89,110 @@ export const MultusNetworkConfigurationComponent: React.FC<
publicNetwork,
networkType,
systemNamespace,
isMultusAcknowledged,
setIsMultusAcknowledged,
}) => {
const { t } = useCustomTranslation();
const handleMultusToggle = (_event: any, checked: any) => {
setNetworkType(checked ? NetworkType.MULTUS : NetworkType.DEFAULT);
if (!checked) {
setIsMultusAcknowledged(false);
}
};

const handleAcknowledgementChange = (_event: any, checked: boolean) => {
setIsMultusAcknowledged(checked);
};

return (
<>
<Checkbox
isChecked={networkType === NetworkType.MULTUS}
onChange={() =>
setNetworkType(
networkType === NetworkType.DEFAULT
? NetworkType.MULTUS
: NetworkType.DEFAULT
)
}
label={t('Isolate network using Multus')}
description={t(
'Multus allows a network seperation between the data operations and the control plane operations.'
<div className="odf-multus-configuration">
{/* Network Section */}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{/* Network Section */}

<h2 className="odf-section-header">{t('Network')}</h2>
<Alert
data-test="odf-default-network-alert"
className="odf-alert odf-mb-md"
title={t('Data Foundation will use the default pod network')}
variant={AlertVariant.info}
isInline
>
{t(
'If you require a custom network configuration, you can modify the network settings after deployment.'
)}
id="multus-checkbox"
/>
{networkType === NetworkType.MULTUS && (
<MultusDropdown
setNetwork={setNetwork}
clusterNetwork={clusterNetwork}
publicNetwork={publicNetwork}
systemNamespace={systemNamespace}
</Alert>

{/* Isolate Network Section */}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems not necessary as the header text already explains it.

Suggested change
{/* Isolate Network Section */}

<h2 className="odf-section-header odf-mt-lg">{t('Isolate Network')}</h2>
<div className="odf-indented-section">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In clean code, intentional naming is about your domain (your business concepts, like "Multus"), not formatting details like "indentation".

Suggested change
<div className="odf-indented-section">
<div className="odf-multus-section">

<Checkbox
isChecked={networkType === NetworkType.MULTUS}
onChange={handleMultusToggle}
label={t('Isolate network using Multus')}
description={t(
'Multus allows a network separation between the data operations and the control plane operations.'
)}
id="multus-checkbox"
className="odf-mb-md"
/>
)}
</>

{networkType === NetworkType.MULTUS && (
<>
<div className="odf-multus-indent pf-v5-u-mt-md">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<div className="odf-multus-indent pf-v5-u-mt-md">
<div className="odf-multus-section__warning pf-v5-u-mt-md">

<Alert
variant={AlertVariant.warning}
isInline
title={t(
'This will isolate network to attach additional clusters as external clients. Run Validation test before to proceed further.'
)}
>
<p>
{t(
'Set up Multus by following relevant steps in KCS. Incorrectly setting up Multus might lead to:'
)}
</p>
<ul>
<li>
{t(
'Data unavailability or loss due to broken internal communication'
)}
</li>
<li>
{t(
'Cluster health issues if network attachments are misconfigured'
)}
</li>
<li>
{t('PVC mounting failures for ODF-dependent workloads')}
</li>
</ul>
</Alert>
</div>

<div className="odf-multus-indent">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<div className="odf-multus-indent">
<div className="odf-multus__ack-checkbox">

<Checkbox
isChecked={isMultusAcknowledged}
label={t(
'By checking this box, you acknowledge running the validation test'
)}
onChange={handleAcknowledgementChange}
id="acknowledgment-checkbox"
className="odf-mb-md"
/>
</div>

{isMultusAcknowledged && (
<div className="odf-indented-section">
<MultusDropdown
setNetwork={setNetwork}
clusterNetwork={clusterNetwork}
publicNetwork={publicNetwork}
systemNamespace={systemNamespace}
/>
</div>
)}
</>
)}
</div>
</div>
);
};

Expand Down Expand Up @@ -154,7 +238,7 @@ export const MultusDropdown: React.FC<MultusDropdownProps> = ({
});

if (resourcesLoaded && !resourcesLoadError) {
const devices = _.flatMap(
const devices = flatMap(
Object.values(networkResources),
(res) => res.data
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const SecurityAndNetwork: React.FC<SecurityAndNetworkProps> = ({
publicNetwork,
encryption,
kms,
isMultusAcknowledged,
addressRanges: {
cluster: [cephClusterCIDR],
public: [cephPublicCIDR],
Expand All @@ -35,10 +36,11 @@ export const SecurityAndNetwork: React.FC<SecurityAndNetworkProps> = ({
type: 'securityAndNetwork/setNetworkType',
payload: networkType,
});
if (
networkType === NetworkType.DEFAULT ||
networkType === NetworkType.HOST
) {
if (networkType !== NetworkType.MULTUS) {
dispatch({
type: 'securityAndNetwork/setMultusAcknowledged',
payload: false,
});
dispatch({ type: 'securityAndNetwork/setClusterNetwork', payload: null });
dispatch({ type: 'securityAndNetwork/setPublicNetwork', payload: null });
}
Expand Down Expand Up @@ -70,6 +72,16 @@ export const SecurityAndNetwork: React.FC<SecurityAndNetworkProps> = ({
[dispatch]
);

const setIsMultusAcknowledged = React.useCallback(
(val: boolean) => {
dispatch({
type: 'securityAndNetwork/setMultusAcknowledged',
payload: val,
});
},
[dispatch]
);

return (
<Form noValidate={false}>
<Encryption
Expand All @@ -91,6 +103,8 @@ export const SecurityAndNetwork: React.FC<SecurityAndNetworkProps> = ({
clusterNetwork={clusterNetwork}
publicNetwork={publicNetwork}
systemNamespace={systemNamespace}
isMultusAcknowledged={isMultusAcknowledged}
setIsMultusAcknowledged={setIsMultusAcknowledged}
/>
)}
{isExternal && (
Expand All @@ -106,7 +120,9 @@ export const SecurityAndNetwork: React.FC<SecurityAndNetworkProps> = ({
};

type SecurityAndNetworkProps = {
securityAndNetworkState: WizardState['securityAndNetwork'];
securityAndNetworkState: WizardState['securityAndNetwork'] & {
isMultusAcknowledged?: boolean;
};
dispatch: WizardDispatch;
infraType: string;
isExternal?: boolean;
Expand Down
9 changes: 9 additions & 0 deletions packages/odf/components/create-storage-system/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export const initialState: CreateStorageSystemState = {
cluster: [],
},
networkType: NetworkType.DEFAULT,
isMultusAcknowledged: false,
},
};

Expand Down Expand Up @@ -167,6 +168,7 @@ type CreateStorageSystemState = {
publicNetwork: NetworkAttachmentDefinitionKind;
clusterNetwork: NetworkAttachmentDefinitionKind;
addressRanges: AddressRanges;
isMultusAcknowledged: boolean;
networkType: NetworkType;
};
createLocalVolumeSet: LocalVolumeSet;
Expand Down Expand Up @@ -400,6 +402,9 @@ export const reducer: WizardReducer = (prevState, action) => {
case 'capacityAndNodes/setVolumeValidationType':
newState.capacityAndNodes.volumeValidationType = action.payload;
break;
case 'securityAndNetwork/setMultusAcknowledged':
newState.securityAndNetwork.isMultusAcknowledged = action.payload;
break;
case 'securityAndNetwork/setKms':
newState.securityAndNetwork.kms = action.payload;
break;
Expand Down Expand Up @@ -521,6 +526,10 @@ export type CreateStorageSystemAction =
type: 'capacityAndNodes/enableTaint';
payload: WizardState['capacityAndNodes']['enableTaint'];
}
| {
type: 'securityAndNetwork/setMultusAcknowledged';
payload: boolean;
}
| {
type: 'securityAndNetwork/setKms';
payload: WizardState['securityAndNetwork']['kms'];
Expand Down