From 5da3372bb02f8bbfce9410619e39c4d99767fdab Mon Sep 17 00:00:00 2001 From: Dipanshu Gupta Date: Tue, 10 Sep 2024 18:45:47 +0530 Subject: [PATCH] RHOAIENG-1109 Select Storage Classes in add cluster storage sections --- frontend/src/api/k8s/pvcs.ts | 5 +- .../detail/storage/ManageStorageModal.tsx | 11 +-- .../screens/spawner/SpawnerFooter.tsx | 6 +- .../projects/screens/spawner/SpawnerPage.tsx | 10 ++- .../pages/projects/screens/spawner/service.ts | 5 +- .../projects/screens/spawner/spawnerUtils.ts | 2 + .../storage/CreateNewStorageSection.tsx | 75 +++++++++++++------ .../spawner/storage/useDefaultStorageClass.ts | 29 +++++++ .../projects/screens/spawner/storage/utils.ts | 15 ++++ frontend/src/pages/projects/types.ts | 1 + frontend/src/pages/storageClasses/utils.ts | 24 +++++- 11 files changed, 137 insertions(+), 46 deletions(-) create mode 100644 frontend/src/pages/projects/screens/spawner/storage/useDefaultStorageClass.ts diff --git a/frontend/src/api/k8s/pvcs.ts b/frontend/src/api/k8s/pvcs.ts index b1249ced3a..792bf6e67e 100644 --- a/frontend/src/api/k8s/pvcs.ts +++ b/frontend/src/api/k8s/pvcs.ts @@ -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); @@ -68,10 +68,9 @@ export const getDashboardPvcs = (projectName: string): Promise => { - const pvc = assemblePvc(data, namespace, undefined, storageClassName); + const pvc = assemblePvc(data, namespace, undefined); return k8sCreateResource( applyK8sAPIOptions({ model: PVCModel, resource: pvc }, opts), diff --git a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx index f281fef202..cc3c586bd8 100644 --- a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx @@ -12,7 +12,6 @@ import useRelatedNotebooks, { 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'; @@ -40,8 +39,6 @@ const ManageStorageModal: React.FC = ({ existingData, isOp createData.forNotebook.name, ]); - const storageClass = usePreferredStorageClass(); - const onBeforeClose = (submitted: boolean) => { onClose(submitted); setActionInProgress(false); @@ -66,7 +63,8 @@ const ManageStorageModal: React.FC = ({ existingData, isOp 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 ) { pvcPromises.push(updatePvc(createData, existingData, namespace, { dryRun })); } @@ -87,9 +85,7 @@ const ManageStorageModal: React.FC = ({ existingData, isOp } 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, @@ -145,6 +141,7 @@ const ManageStorageModal: React.FC = ({ existingData, isOp setData={(key, value) => setCreateData(key, value)} currentSize={existingData?.status?.capacity?.storage} autoFocusName + isEdit={!!existingData} /> {createData.hasExistingNotebookConnections && ( diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx index 22978b9fd3..78d8953f75 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx @@ -242,11 +242,7 @@ const SpawnerFooter: React.FC = ({ ? [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, diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx index 790da0f874..5c9fa9a3ae 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx @@ -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; @@ -68,7 +69,14 @@ const SpawnerPage: React.FC = ({ 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, diff --git a/frontend/src/pages/projects/screens/spawner/service.ts b/frontend/src/pages/projects/screens/spawner/service.ts index 3d84738503..1a9e90e573 100644 --- a/frontend/src/pages/projects/screens/spawner/service.ts +++ b/frontend/src/pages/projects/screens/spawner/service.ts @@ -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 }); @@ -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 }; diff --git a/frontend/src/pages/projects/screens/spawner/spawnerUtils.ts b/frontend/src/pages/projects/screens/spawner/spawnerUtils.ts index de6ab0719e..23dd8ca484 100644 --- a/frontend/src/pages/projects/screens/spawner/spawnerUtils.ts +++ b/frontend/src/pages/projects/screens/spawner/spawnerUtils.ts @@ -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); @@ -48,6 +49,7 @@ export const useMergeDefaultPVCName = ( ...storageData.creating.nameDesc, name: storageData.creating.nameDesc.name || defaultPVCName, }, + storageClassName: defaultStorageClassName ?? '', }, }; }; diff --git a/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx b/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx index de9c3fc5ec..b63a6aeede 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx @@ -1,8 +1,11 @@ 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; @@ -10,6 +13,7 @@ type CreateNewStorageSectionProps = { currentSize?: string; autoFocusName?: boolean; menuAppendTo?: HTMLElement; + isEdit?: boolean; }; const CreateNewStorageSection: React.FC = ({ @@ -18,27 +22,52 @@ const CreateNewStorageSection: React.FC = ({ currentSize, menuAppendTo, autoFocusName, -}) => ( - - - setData('nameDesc', newData)} - autoFocusName={autoFocusName} - /> - - - setData('size', size)} - /> - - -); + isEdit, +}) => { + const [storageClasses, storageClassesLoaded] = useStorageClasses(); + + return ( + + + setData('nameDesc', newData)} + autoFocusName={autoFocusName} + /> + + + + { + const { name } = sc.metadata; + const desc = getStorageClassConfig(sc)?.description; + return { key: name, label: name, description: desc }; + })} + onChange={(selection) => { + setData('storageClassName', selection); + }} + isDisabled={isEdit || !storageClassesLoaded} + placeholder={isEdit && !storageClassesLoaded ? data.storageClassName : 'Select one'} + popperProps={{ appendTo: menuAppendTo }} + /> + + + + setData('size', size)} + /> + + + ); +}; export default CreateNewStorageSection; diff --git a/frontend/src/pages/projects/screens/spawner/storage/useDefaultStorageClass.ts b/frontend/src/pages/projects/screens/spawner/storage/useDefaultStorageClass.ts new file mode 100644 index 0000000000..b784c22949 --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/useDefaultStorageClass.ts @@ -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; diff --git a/frontend/src/pages/projects/screens/spawner/storage/utils.ts b/frontend/src/pages/projects/screens/spawner/storage/utils.ts index 54767a2011..6650e27ed4 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/utils.ts +++ b/frontend/src/pages/projects/screens/spawner/storage/utils.ts @@ -14,6 +14,7 @@ import useGenericObjectState from '~/utilities/useGenericObjectState'; 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, @@ -23,6 +24,8 @@ export const useCreateStorageObjectForNotebook = ( resetDefaults: () => void, ] => { const size = useDefaultPvcSize(); + const defaultStorageClass = useDefaultStorageClass(); + const createDataState = useGenericObjectState({ nameDesc: { name: '', @@ -30,6 +33,7 @@ export const useCreateStorageObjectForNotebook = ( description: '', }, size, + storageClassName: defaultStorageClass?.metadata.name ?? '', forNotebook: { name: '', mountPath: { @@ -42,9 +46,16 @@ export const useCreateStorageObjectForNotebook = ( const [, setCreateData] = createDataState; + React.useEffect(() => { + if (defaultStorageClass) { + setCreateData('storageClassName', defaultStorageClass.metadata.name); + } + }, [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, @@ -61,6 +72,8 @@ export const useCreateStorageObjectForNotebook = ( setCreateData('hasExistingNotebookConnections', hasExistingNotebookConnections); setCreateData('size', existingSize); + + setCreateData('storageClassName', existingStorageClass); } }, [ existingName, @@ -68,6 +81,7 @@ export const useCreateStorageObjectForNotebook = ( setCreateData, hasExistingNotebookConnections, existingSize, + existingStorageClass, ]); return createDataState; @@ -102,6 +116,7 @@ export const useStorageDataObject = ( description: '', }, size, + storageClassName: '', }, existing: { storage: getRootVolumeName(notebook), diff --git a/frontend/src/pages/projects/types.ts b/frontend/src/pages/projects/types.ts index e8e6be0de3..4c91727ffe 100644 --- a/frontend/src/pages/projects/types.ts +++ b/frontend/src/pages/projects/types.ts @@ -26,6 +26,7 @@ export type NameDescType = { export type CreatingStorageObject = { nameDesc: NameDescType; size: string; + storageClassName?: string; }; export type MountPath = { diff --git a/frontend/src/pages/storageClasses/utils.ts b/frontend/src/pages/storageClasses/utils.ts index cf12dbd5bd..06c9e395e7 100644 --- a/frontend/src/pages/storageClasses/utils.ts +++ b/frontend/src/pages/storageClasses/utils.ts @@ -3,12 +3,28 @@ import { MetadataAnnotation, StorageClassConfig, StorageClassKind } from '~/k8sT export const getStorageClassConfig = ( storageClass: StorageClassKind, ): StorageClassConfig | undefined => { - const storageClassConfig: StorageClassConfig | undefined = JSON.parse( - storageClass.metadata.annotations?.[MetadataAnnotation.OdhStorageClassConfig] || '', - ); + const annotation = storageClass.metadata.annotations?.[MetadataAnnotation.OdhStorageClassConfig]; + + if (!annotation) { + return undefined; + } - return storageClassConfig; + try { + const storageClassConfig: StorageClassConfig = JSON.parse(annotation); + return storageClassConfig; + } catch (error) { + return undefined; + } }; export const isOpenshiftDefaultStorageClass = (storageClass: StorageClassKind): boolean => storageClass.metadata.annotations?.[MetadataAnnotation.StorageClassIsDefault] === 'true'; + +export const getDefaultStorageClass = ( + storageClasses: StorageClassKind[], +): StorageClassKind | undefined => + storageClasses.find( + (storageClass) => + isOpenshiftDefaultStorageClass(storageClass) || + getStorageClassConfig(storageClass)?.isDefault, + );