Skip to content
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

Bug 1988905: External mode deployments fails on parsing json in ODF wizard #9966

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -265,11 +265,12 @@
"Password": "Password",
"Hide password": "Hide password",
"Reveal password": "Reveal password",
"The uploaded file is not a valid JSON file": "The uploaded file is not a valid JSON file",
"External storage metadata": "External storage metadata",
"Download <1>{{SCRIPT_NAME}}</1> script and run on the RHCS cluster, then upload the results (JSON).": "Download <1>{{SCRIPT_NAME}}</1> script and run on the RHCS cluster, then upload the results (JSON).",
"Download script": "Download script",
"The uploaded file is not a valid JSON file": "The uploaded file is not a valid JSON file",
"Browse": "Browse",
"Clear": "Clear",
"Upload JSON file": "Upload JSON file",
"An error has occurred": "An error has occurred",
"Create StorageSystem": "Create StorageSystem",
Expand Down
Expand Up @@ -57,7 +57,7 @@ export const createSteps = (
},
reviewAndCreate: {
name: StepsName(t)[Steps.ReviewAndCreate],
component: <ReviewAndCreate state={state} />,
component: <ReviewAndCreate state={state} hasOCS={hasOCS} />,
},
};

Expand Down
Expand Up @@ -5,7 +5,7 @@ import { Form, FormSelect, FormSelectOption, FormSelectProps, Radio } from '@pat
import { useFlag } from '@console/shared/src';
import { StorageClassDropdown } from '@console/internal/components/utils/storage-class-dropdown';
import { ListKind, StorageClassResourceKind } from '@console/internal/module/k8s';
import { StorageClassModel } from '@console/internal/models';
import { InfrastructureModel, StorageClassModel } from '@console/internal/models';
import { useK8sGet } from '@console/internal/components/utils/k8s-get-hook';
import {
ClusterServiceVersionKind,
Expand All @@ -31,6 +31,8 @@ import { CEPH_STORAGE_NAMESPACE, NO_PROVISIONER } from '../../../../constants';
import './backing-storage-step.scss';
import { GUARDED_FEATURES } from '../../../../features';

const RHCS_SUPPORTED_INFRA = ['BareMetal', 'None', 'VSphere', 'OpenStack', 'oVirt', 'IBMCloud'];
Copy link
Contributor

Choose a reason for hiding this comment

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

We can add this to constants file.

Copy link
Contributor Author

@afreen23 afreen23 Sep 2, 2021

Choose a reason for hiding this comment

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

Its just used here. lets keep it.


const ExternalSystemSelection: React.FC<ExternalSystemSelectionProps> = ({
dispatch,
stepIdReached,
Expand Down Expand Up @@ -121,6 +123,7 @@ export const BackingStorage: React.FC<BackingStorageProps> = ({
const [sc, scLoaded, scLoadError] = useK8sGet<ListKind<StorageClassResourceKind>>(
StorageClassModel,
);
const [infra, infraLoaded, infraLoadError] = useK8sGet<any>(InfrastructureModel, 'cluster');
const isMCGStandalone = useFlag(GUARDED_FEATURES.ODF_MCG_STANDALONE);
const [csvList, csvListLoaded, csvListLoadError] = useK8sGet<ListKind<ClusterServiceVersionKind>>(
ClusterServiceVersionModel,
Expand All @@ -134,12 +137,16 @@ export const BackingStorage: React.FC<BackingStorageProps> = ({
const odfCsv = getODFCsv(csvList?.items);
const supportedODFVendors = getSupportedVendors(odfCsv);

const allowedExternalStorage: ExternalStorage[] = SUPPORTED_EXTERNAL_STORAGE.filter(
({ model }) => {
const kind = getStorageSystemKind(model);
return supportedODFVendors.includes(kind) && !formattedSS.has(kind);
},
);
const infraType = infra?.spec?.platformSpec?.type;
const enableRhcs = RHCS_SUPPORTED_INFRA.includes(infraType);

const allowedExternalStorage: ExternalStorage[] =
!enableRhcs || hasOCS
? SUPPORTED_EXTERNAL_STORAGE.filter(({ model }) => {
const kind = getStorageSystemKind(model);
return supportedODFVendors.includes(kind) && kind !== STORAGE_CLUSTER_SYSTEM_KIND;
})
: SUPPORTED_EXTERNAL_STORAGE;

const { type, externalStorage, deployment, isAdvancedOpen } = state;

Expand All @@ -150,6 +157,13 @@ export const BackingStorage: React.FC<BackingStorageProps> = ({
*/
if (hasOCS && allowedExternalStorage.length) {
dispatch({ type: 'backingStorage/setType', payload: BackingStorageType.EXTERNAL });
dispatch({
type: 'wizard/setStorageClass',
payload: {
name: '',
provisioner: '',
},
});
}
}, [dispatch, allowedExternalStorage.length, hasOCS]);

Expand All @@ -158,20 +172,21 @@ export const BackingStorage: React.FC<BackingStorageProps> = ({
* Allow pre selecting the "create new storage class" option instead of the "existing" option
* if no storage classes present. This is true for a baremetal platform.
*/
if (sc?.items?.length === 0 && deployment === DeploymentType.FULL) {
if (
sc?.items?.length === 0 &&
deployment === DeploymentType.FULL &&
type !== BackingStorageType.EXTERNAL
) {
dispatch({ type: 'backingStorage/setType', payload: BackingStorageType.LOCAL_DEVICES });
}
}, [deployment, dispatch, sc]);

React.useEffect(() => {
/* Update storage class state when existing storage class is not selected. */
if (type === BackingStorageType.LOCAL_DEVICES || type === BackingStorageType.EXTERNAL) {
dispatch({
type: 'wizard/setStorageClass',
payload: { name: '', provisioner: NO_PROVISIONER },
payload: {
name: '',
provisioner: NO_PROVISIONER,
},
});
}
}, [dispatch, type]);
}, [deployment, dispatch, sc, type]);

const showExternalStorageSelection =
type === BackingStorageType.EXTERNAL && allowedExternalStorage.length;
Expand All @@ -180,20 +195,31 @@ export const BackingStorage: React.FC<BackingStorageProps> = ({
const RADIO_GROUP_NAME = 'backing-storage-radio-group';

const onRadioSelect = (_, event) => {
dispatch({ type: 'backingStorage/setType', payload: event.target.value });
const newType = event.target.value;
if (stepIdReached !== 1) {
/*
* Reset the wizard when user has selected a new deployment flow
* and has not visited any step other than first step.
*/
dispatch({ type: 'wizard/setInitialState' });
}
/* Update storage class state when existing storage class is not selected. */
if (newType === BackingStorageType.LOCAL_DEVICES || newType === BackingStorageType.EXTERNAL) {
dispatch({
type: 'wizard/setStorageClass',
payload: {
name: '',
provisioner: type === BackingStorageType.EXTERNAL ? '' : NO_PROVISIONER,
},
});
}
dispatch({ type: 'backingStorage/setType', payload: newType });
};

return (
<ErrorHandler
error={error || scLoadError || csvListLoadError}
loaded={loaded && scLoaded && csvListLoaded}
error={error || scLoadError || infraLoadError || csvListLoadError}
loaded={loaded && scLoaded && infraLoaded && csvListLoaded}
>
<Form>
<Radio
Expand Down
@@ -1,4 +1,5 @@
import * as React from 'react';
import * as _ from 'lodash';
import { useTranslation } from 'react-i18next';
import { TextContent, Text, TextVariants, List, ListItem } from '@patternfly/react-core';
import { useFlag } from '@console/shared/src';
Expand All @@ -25,7 +26,7 @@ export const ReviewItem = ({ children, title }) => (
</div>
);

export const ReviewAndCreate: React.FC<ReviewAndCreateProps> = ({ state }) => {
export const ReviewAndCreate: React.FC<ReviewAndCreateProps> = ({ state, hasOCS }) => {
const { t } = useTranslation();
const isMultusSupported = useFlag(GUARDED_FEATURES.OCS_MULTUS);

Expand All @@ -35,13 +36,17 @@ export const ReviewAndCreate: React.FC<ReviewAndCreateProps> = ({ state }) => {
securityAndNetwork,
createLocalVolumeSet,
backingStorage,
connectionDetails,
createStorageClass,
nodes,
} = state;
const { capacity, arbiterLocation, enableArbiter } = capacityAndNodes;
const { encryption, kms, networkType } = securityAndNetwork;
const { deployment, externalStorage, type } = backingStorage;

const isMCG = deployment === DeploymentType.MCG;
const isRhcs = !_.isEmpty(connectionDetails);
const isStandaloneExternal = hasOCS && !_.isEmpty(createStorageClass);
Copy link
Contributor

Choose a reason for hiding this comment

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

One doubt... we also have already set flags to check whether it is external cluster or not, will that not work here ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If independent mode is true then we can say its rhcs but if its false we cannot say -

  1. a non rhcs cluster created or
  2. a non rhcs cluster not present

Also in future we will have many storage systems, so keeping this way, would not complicate


const isNoProvisioner = storageClass.provisioner === NO_PROVISIONER;
const formattedCapacity = !isNoProvisioner
Expand All @@ -65,7 +70,7 @@ export const ReviewAndCreate: React.FC<ReviewAndCreateProps> = ({ state }) => {
return (
<>
<ReviewItem title={t('ceph-storage-plugin~Backing storage')}>
{!isMCG && (
{!isMCG && !isRhcs && (
<ListItem>
{t('ceph-storage-plugin~StorageClass: {{name}}', {
name: storageClass.name || createLocalVolumeSet.volumeSetName,
Expand All @@ -85,7 +90,7 @@ export const ReviewAndCreate: React.FC<ReviewAndCreateProps> = ({ state }) => {
</ListItem>
)}
</ReviewItem>
{!isMCG && (
{!isMCG && !isRhcs && !isStandaloneExternal && (
<ReviewItem title={t('ceph-storage-plugin~Capacity and nodes')}>
<ListItem>
{t('ceph-storage-plugin~Cluster capacity: {{capacity}}', {
Expand Down Expand Up @@ -119,39 +124,42 @@ export const ReviewAndCreate: React.FC<ReviewAndCreateProps> = ({ state }) => {
)}
</ReviewItem>
)}
{isMCG ? (
<ReviewItem title={t('ceph-storage-plugin~Security')}>
<ListItem>{t('ceph-storage-plugin~Encryption: Enabled')}</ListItem>
<ListItem>
{t('ceph-storage-plugin~External key management service: {{kmsStatus}}', {
kmsStatus,
})}
</ListItem>
</ReviewItem>
) : (
<ReviewItem title={t('ceph-storage-plugin~Security and network')}>
<ListItem>
{t('ceph-storage-plugin~Encryption: {{encryptionStatus}}', { encryptionStatus })}
</ListItem>
{hasEncryption && (
{!isRhcs &&
!isStandaloneExternal &&
(isMCG ? (
<ReviewItem title={t('ceph-storage-plugin~Security')}>
<ListItem>{t('ceph-storage-plugin~Encryption: Enabled')}</ListItem>
<ListItem>
{t('ceph-storage-plugin~External key management service: {{kmsStatus}}', {
kmsStatus,
})}
</ListItem>
)}
{isMultusSupported && (
</ReviewItem>
) : (
<ReviewItem title={t('ceph-storage-plugin~Security and network')}>
<ListItem>
{t('ceph-storage-plugin~Network: {{networkType}}', {
networkType: NetworkTypeLabels[networkType],
})}
{t('ceph-storage-plugin~Encryption: {{encryptionStatus}}', { encryptionStatus })}
</ListItem>
)}
</ReviewItem>
)}
{hasEncryption && (
<ListItem>
{t('ceph-storage-plugin~External key management service: {{kmsStatus}}', {
kmsStatus,
})}
</ListItem>
)}
{isMultusSupported && (
<ListItem>
{t('ceph-storage-plugin~Network: {{networkType}}', {
networkType: NetworkTypeLabels[networkType],
})}
</ListItem>
)}
</ReviewItem>
))}
</>
);
};
type ReviewAndCreateProps = {
state: WizardState;
hasOCS: boolean;
};
Expand Up @@ -7,5 +7,5 @@
}

.odf-connection-details__helper-text {
padding-top: 0.2rem;
padding-top: var(--pf-global--spacer--xs);
}