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

[WIP] RHOAIENG-1109 Select Storage Classes in add cluster storage sections #3184

Closed
wants to merge 1 commit into from
Closed
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
5 changes: 2 additions & 3 deletions frontend/src/api/k8s/pvcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ export const assemblePvc = (
data: CreatingStorageObject,
namespace: string,
editName?: string,
storageClassName?: string,
): PersistentVolumeClaimKind => {
const {
nameDesc: { name: pvcName, description },
size,
storageClassName,
} = data;

const name = editName || translateDisplayNameForK8s(pvcName);
Expand Down Expand Up @@ -68,10 +68,9 @@ export const getDashboardPvcs = (projectName: string): Promise<PersistentVolumeC
export const createPvc = (
data: CreatingStorageObject,
namespace: string,
storageClassName?: string,
opts?: K8sAPIOptions,
): Promise<PersistentVolumeClaimKind> => {
const pvc = assemblePvc(data, namespace, undefined, storageClassName);
const pvc = assemblePvc(data, namespace, undefined);

return k8sCreateResource<PersistentVolumeClaimKind>(
applyK8sAPIOptions({ model: PVCModel, resource: pvc }, opts),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import NotebookRestartAlert from '~/pages/projects/components/NotebookRestartAlert';
import useWillNotebooksRestart from '~/pages/projects/notebook/useWillNotebooksRestart';
import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter';
import usePreferredStorageClass from '~/pages/projects/screens/spawner/storage/usePreferredStorageClass';
import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils';
import ExistingConnectedNotebooks from './ExistingConnectedNotebooks';

Expand Down Expand Up @@ -40,8 +39,6 @@
createData.forNotebook.name,
]);

const storageClass = usePreferredStorageClass();

const onBeforeClose = (submitted: boolean) => {
onClose(submitted);
setActionInProgress(false);
Expand All @@ -66,7 +63,8 @@
if (
getDisplayNameFromK8sResource(existingData) !== createData.nameDesc.name ||
getDescriptionFromK8sResource(existingData) !== createData.nameDesc.description ||
existingData.spec.resources.requests.storage !== createData.size
existingData.spec.resources.requests.storage !== createData.size ||
existingData.spec.storageClassName !== createData.storageClassName

Check warning on line 67 in frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx#L66-L67

Added lines #L66 - L67 were not covered by tests
) {
pvcPromises.push(updatePvc(createData, existingData, namespace, { dryRun }));
}
Expand All @@ -87,9 +85,7 @@
}
return;
}
const createdPvc = await createPvc(createData, namespace, storageClass?.metadata.name, {
dryRun,
});
const createdPvc = await createPvc(createData, namespace, { dryRun });
if (notebookName) {
await attachNotebookPVC(notebookName, namespace, createdPvc.metadata.name, mountPath.value, {
dryRun,
Expand Down Expand Up @@ -145,6 +141,7 @@
setData={(key, value) => setCreateData(key, value)}
currentSize={existingData?.status?.capacity?.storage}
autoFocusName
isEdit={!!existingData}
/>
</StackItem>
{createData.hasExistingNotebookConnections && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,11 +242,7 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
? [dataConnection.existing]
: [];

const pvcDetails = await createPvcDataForNotebook(
projectName,
storageData,
storageClass?.metadata.name,
).catch(handleError);
const pvcDetails = await createPvcDataForNotebook(projectName, storageData).catch(handleError);
const envFrom = await createConfigMapsAndSecretsForNotebook(projectName, [
...envVariables,
...newDataConnection,
Expand Down
10 changes: 9 additions & 1 deletion frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { useNotebookEnvVariables } from './environmentVariables/useNotebookEnvVa
import DataConnectionField from './dataConnection/DataConnectionField';
import { useNotebookDataConnection } from './dataConnection/useNotebookDataConnection';
import { useNotebookSizeState } from './useNotebookSizeState';
import useDefaultStorageClass from './storage/useDefaultStorageClass';

type SpawnerPageProps = {
existingNotebook?: NotebookKind;
Expand All @@ -68,7 +69,14 @@ const SpawnerPage: React.FC<SpawnerPageProps> = ({ existingNotebook }) => {
string[] | undefined
>();
const [storageDataWithoutDefault, setStorageData] = useStorageDataObject(existingNotebook);
const storageData = useMergeDefaultPVCName(storageDataWithoutDefault, nameDesc.name);

const defaultStorageClass = useDefaultStorageClass();
const storageData = useMergeDefaultPVCName(
storageDataWithoutDefault,
nameDesc.name,
defaultStorageClass?.metadata.name,
);

const [envVariables, setEnvVariables] = useNotebookEnvVariables(existingNotebook);
const [dataConnectionData, setDataConnectionData] = useNotebookDataConnection(
dataConnections.data,
Expand Down
5 changes: 2 additions & 3 deletions frontend/src/pages/projects/screens/spawner/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,13 @@ import { fetchNotebookEnvVariables } from './environmentVariables/useNotebookEnv
export const createPvcDataForNotebook = async (
projectName: string,
storageData: StorageData,
storageClassName?: string,
): Promise<{ volumes: Volume[]; volumeMounts: VolumeMount[] }> => {
const { storageType } = storageData;

const { volumes, volumeMounts } = getVolumesByStorageData(storageData);

if (storageType === StorageType.NEW_PVC) {
const pvc = await createPvc(storageData.creating, projectName, storageClassName);
const pvc = await createPvc(storageData.creating, projectName);
const newPvcName = pvc.metadata.name;
volumes.push({ name: newPvcName, persistentVolumeClaim: { claimName: newPvcName } });
volumeMounts.push({ mountPath: ROOT_MOUNT_PATH, name: newPvcName });
Expand Down Expand Up @@ -70,7 +69,7 @@ export const replaceRootVolumesForNotebook = async (
};
replacedVolumeMount = { name: existingName, mountPath: ROOT_MOUNT_PATH };
} else {
const pvc = await createPvc(storageData.creating, projectName, storageClassName, { dryRun });
const pvc = await createPvc(storageData.creating, projectName, { dryRun });
const newPvcName = pvc.metadata.name;
replacedVolume = { name: newPvcName, persistentVolumeClaim: { claimName: newPvcName } };
replacedVolumeMount = { mountPath: ROOT_MOUNT_PATH, name: newPvcName };
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/pages/projects/screens/spawner/spawnerUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { FAILED_PHASES, PENDING_PHASES, IMAGE_ANNOTATIONS } from './const';
export const useMergeDefaultPVCName = (
storageData: StorageData,
defaultPVCName: string,
defaultStorageClassName?: string,
): StorageData => {
const modifiedRef = React.useRef(false);

Expand All @@ -48,6 +49,7 @@ export const useMergeDefaultPVCName = (
...storageData.creating.nameDesc,
name: storageData.creating.nameDesc.name || defaultPVCName,
},
storageClassName: defaultStorageClassName ?? '',
},
};
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
import * as React from 'react';
import { Stack, StackItem } from '@patternfly/react-core';
import { FormGroup, Stack, StackItem } from '@patternfly/react-core';
import { CreatingStorageObject, UpdateObjectAtPropAndValue } from '~/pages/projects/types';
import PVSizeField from '~/pages/projects/components/PVSizeField';
import NameDescriptionField from '~/concepts/k8s/NameDescriptionField';
import SimpleSelect from '~/components/SimpleSelect';
import useStorageClasses from '~/concepts/k8s/useStorageClasses';
import { getStorageClassConfig } from '~/pages/storageClasses/utils';

type CreateNewStorageSectionProps = {
data: CreatingStorageObject;
setData: UpdateObjectAtPropAndValue<CreatingStorageObject>;
currentSize?: string;
autoFocusName?: boolean;
menuAppendTo?: HTMLElement;
isEdit?: boolean;
};

const CreateNewStorageSection: React.FC<CreateNewStorageSectionProps> = ({
Expand All @@ -18,27 +22,52 @@
currentSize,
menuAppendTo,
autoFocusName,
}) => (
<Stack hasGutter>
<StackItem>
<NameDescriptionField
nameFieldId="create-new-storage-name"
descriptionFieldId="create-new-storage-description"
data={data.nameDesc}
setData={(newData) => setData('nameDesc', newData)}
autoFocusName={autoFocusName}
/>
</StackItem>
<StackItem>
<PVSizeField
menuAppendTo={menuAppendTo}
fieldID="create-new-storage-size"
currentSize={currentSize}
size={data.size}
setSize={(size) => setData('size', size)}
/>
</StackItem>
</Stack>
);
isEdit,
}) => {
const [storageClasses, storageClassesLoaded] = useStorageClasses();

return (
<Stack hasGutter>
<StackItem>
<NameDescriptionField
nameFieldId="create-new-storage-name"
descriptionFieldId="create-new-storage-description"
data={data.nameDesc}
setData={(newData) => setData('nameDesc', newData)}
autoFocusName={autoFocusName}
/>
</StackItem>
<StackItem>
<FormGroup label="Storage class" fieldId="storage-class" isRequired>
<SimpleSelect
id="storage-classes-selector"
isFullWidth
value={data.storageClassName}
options={storageClasses.map((sc) => {
const { name } = sc.metadata;
const desc = getStorageClassConfig(sc)?.description;
return { key: name, label: name, description: desc };

Check warning on line 49 in frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx#L46-L49

Added lines #L46 - L49 were not covered by tests
})}
onChange={(selection) => {
setData('storageClassName', selection);

Check warning on line 52 in frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx#L51-L52

Added lines #L51 - L52 were not covered by tests
}}
isDisabled={isEdit || !storageClassesLoaded}
placeholder={isEdit && !storageClassesLoaded ? data.storageClassName : 'Select one'}
popperProps={{ appendTo: menuAppendTo }}
/>
</FormGroup>
</StackItem>
<StackItem>
<PVSizeField
menuAppendTo={menuAppendTo}
fieldID="create-new-storage-size"
currentSize={currentSize}
size={data.size}
setSize={(size) => setData('size', size)}
/>
</StackItem>
</Stack>
);
};

export default CreateNewStorageSection;
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import * as React from 'react';
import useStorageClasses from '~/concepts/k8s/useStorageClasses';
import { StorageClassKind } from '~/k8sTypes';
import {
getStorageClassConfig,
isOpenshiftDefaultStorageClass,
} from '~/pages/storageClasses/utils';

const useDefaultStorageClass = (): StorageClassKind | undefined => {
const [storageClasses, storageClassesLoaded] = useStorageClasses();
const [defaultStorageClass, setDefaultStorageClass] = React.useState<
StorageClassKind | undefined
>();

React.useEffect(() => {
if (!storageClassesLoaded) {
return;
}

const defaultSc = storageClasses.find(
(sc) => isOpenshiftDefaultStorageClass(sc) || getStorageClassConfig(sc)?.isDefault,
);
setDefaultStorageClass(defaultSc);
}, [storageClasses, storageClassesLoaded]);

return defaultStorageClass;
};

export default useDefaultStorageClass;
15 changes: 15 additions & 0 deletions frontend/src/pages/projects/screens/spawner/storage/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import { getRootVolumeName } from '~/pages/projects/screens/spawner/spawnerUtils';
import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils';
import useDefaultPvcSize from './useDefaultPvcSize';
import useDefaultStorageClass from './useDefaultStorageClass';

export const useCreateStorageObjectForNotebook = (
existingData?: PersistentVolumeClaimKind,
Expand All @@ -23,13 +24,16 @@
resetDefaults: () => void,
] => {
const size = useDefaultPvcSize();
const defaultStorageClass = useDefaultStorageClass();

const createDataState = useGenericObjectState<CreatingStorageObjectForNotebook>({
nameDesc: {
name: '',
k8sName: undefined,
description: '',
},
size,
storageClassName: defaultStorageClass?.metadata.name ?? '',
forNotebook: {
name: '',
mountPath: {
Expand All @@ -42,9 +46,16 @@

const [, setCreateData] = createDataState;

React.useEffect(() => {
if (defaultStorageClass) {
setCreateData('storageClassName', defaultStorageClass.metadata.name);

Check warning on line 51 in frontend/src/pages/projects/screens/spawner/storage/utils.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/spawner/storage/utils.ts#L51

Added line #L51 was not covered by tests
}
}, [defaultStorageClass, setCreateData]);

const existingName = existingData ? getDisplayNameFromK8sResource(existingData) : '';
const existingDescription = existingData ? getDescriptionFromK8sResource(existingData) : '';
const existingSize = existingData ? existingData.spec.resources.requests.storage : size;
const existingStorageClass = existingData ? existingData.spec.storageClassName : '';
const { notebooks: relatedNotebooks } = useRelatedNotebooks(
ConnectedNotebookContext.REMOVABLE_PVC,
existingData ? existingData.metadata.name : undefined,
Expand All @@ -61,13 +72,16 @@
setCreateData('hasExistingNotebookConnections', hasExistingNotebookConnections);

setCreateData('size', existingSize);

setCreateData('storageClassName', existingStorageClass);
}
}, [
existingName,
existingDescription,
setCreateData,
hasExistingNotebookConnections,
existingSize,
existingStorageClass,
]);

return createDataState;
Expand Down Expand Up @@ -102,6 +116,7 @@
description: '',
},
size,
storageClassName: '',
},
existing: {
storage: getRootVolumeName(notebook),
Expand Down
1 change: 1 addition & 0 deletions frontend/src/pages/projects/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export type NameDescType = {
export type CreatingStorageObject = {
nameDesc: NameDescType;
size: string;
storageClassName?: string;
};

export type MountPath = {
Expand Down
9 changes: 9 additions & 0 deletions frontend/src/pages/storageClasses/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,12 @@

export const isOpenshiftDefaultStorageClass = (storageClass: StorageClassKind): boolean =>
storageClass.metadata.annotations?.[MetadataAnnotation.StorageClassIsDefault] === 'true';

export const getDefaultStorageClass = (
storageClasses: StorageClassKind[],

Check warning on line 21 in frontend/src/pages/storageClasses/utils.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/storageClasses/utils.ts#L21

Added line #L21 was not covered by tests
): StorageClassKind | undefined =>
storageClasses.find(
(storageClass) =>
isOpenshiftDefaultStorageClass(storageClass) ||
getStorageClassConfig(storageClass)?.isDefault,

Check warning on line 26 in frontend/src/pages/storageClasses/utils.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/storageClasses/utils.ts#L23-L26

Added lines #L23 - L26 were not covered by tests
);
Loading