diff --git a/frontend/src/__mocks__/mockStartNotebookData.ts b/frontend/src/__mocks__/mockStartNotebookData.ts index b01f705ea3..4b9bade42c 100644 --- a/frontend/src/__mocks__/mockStartNotebookData.ts +++ b/frontend/src/__mocks__/mockStartNotebookData.ts @@ -94,18 +94,15 @@ export const mockStartNotebookData = ({ }, }); -export const mockStorageData: StorageData = { - storageType: StorageType.NEW_PVC, - creating: { - nameDesc: { - name: 'test-pvc', - description: '', - }, +export const mockStorageData: StorageData[] = [ + { + storageType: StorageType.NEW_PVC, + name: 'test-pvc', + description: '', size: '20Gi', storageClassName: 'gp2-csi', }, - existing: { storage: '' }, -}; +]; export const mockDataConnectionData: DataConnectionData = { type: 'creating', diff --git a/frontend/src/__tests__/cypress/cypress/pages/workbench.ts b/frontend/src/__tests__/cypress/cypress/pages/workbench.ts index 9824aea5fe..e6ff13c319 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/workbench.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/workbench.ts @@ -202,6 +202,39 @@ class AttachConnectionModal extends Modal { } } +class StorageTableRow extends TableRow { + private findByDataLabel(label: string) { + return this.find().find(`[data-label="${label}"]`); + } + + findNameValue() { + return this.findByDataLabel('Name'); + } + + findStorageSizeValue() { + return this.findByDataLabel('Storage size'); + } + + findMountPathValue() { + return this.findByDataLabel('Mount path'); + } +} + +class StorageTable { + find() { + return cy.findByTestId('cluster-storage-table'); + } + + getRowById(id: number) { + return new StorageTableRow( + () => + this.find().findByTestId(['cluster-storage-table-row', id]) as unknown as Cypress.Chainable< + JQuery + >, + ); + } +} + class CreateSpawnerPage { k8sNameDescription = new K8sNameDescriptionField('workbench'); @@ -210,16 +243,8 @@ class CreateSpawnerPage { return this; } - findNewStorageRadio() { - return cy.findByTestId('persistent-new-storage-type-radio'); - } - - findExistingStorageRadio() { - return cy.findByTestId('persistent-existing-storage-type-radio'); - } - - findClusterStorageInput() { - return cy.findByTestId('create-new-storage-name'); + getStorageTable() { + return new StorageTable(); } private findPVSizeField() { @@ -363,11 +388,6 @@ class EditSpawnerPage extends CreateSpawnerPage { cy.testA11y(); } - shouldHavePersistentStorage(name: string) { - cy.findByTestId('persistent-storage-group').find('input').should('have.value', name); - return this; - } - shouldHaveNotebookImageSelectInput(name: string) { cy.findByTestId('workbench-image-stream-selection').contains(name).should('exist'); return this; diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts index 93befc4ef0..1e3a6bd7be 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts @@ -300,22 +300,11 @@ describe('Workbench page', () => { environmentVariableField.uploadConfigYaml(configYamlPath); environmentVariableField.findRemoveEnvironmentVariableButton().should('be.enabled'); - //cluster storage - createSpawnerPage.shouldHaveClusterStorageAlert(); - - //add new cluster storage - createSpawnerPage.findNewStorageRadio().click(); - createSpawnerPage.findClusterStorageInput().should('have.value', 'test-project'); - createSpawnerPage.findClusterStorageDescriptionInput().fill('test-description'); - createSpawnerPage.findPVSizeMinusButton().click(); - createSpawnerPage.findPVSizeInput().should('have.value', '19'); - createSpawnerPage.findPVSizePlusButton().click(); - createSpawnerPage.findPVSizeInput().should('have.value', '20'); - createSpawnerPage.selectPVSize('MiB'); - - //add existing cluster storage - createSpawnerPage.findExistingStorageRadio().click(); - createSpawnerPage.selectExistingPersistentStorage('Test Storage'); + // cluster storage + const storageTableRow = createSpawnerPage.getStorageTable().getRowById(0); + storageTableRow.findNameValue().should('have.text', 'test-project-storage'); + storageTableRow.findStorageSizeValue().should('have.text', 'Max 20Gi'); + storageTableRow.findMountPathValue().should('have.text', '/opt/app-root/src/'); //add data connection createSpawnerPage.findDataConnectionCheckbox().check(); @@ -352,15 +341,6 @@ describe('Workbench page', () => { name: 'test-project', namespace: 'test-project', }, - spec: { - template: { - spec: { - volumes: [ - { name: 'test-storage-1', persistentVolumeClaim: { claimName: 'test-storage-1' } }, - ], - }, - }, - }, }); }); verifyRelativeURL('/projects/test-project?section=workbenches'); @@ -693,7 +673,11 @@ describe('Workbench page', () => { editSpawnerPage.k8sNameDescription.findDisplayNameInput().should('have.value', 'Test Notebook'); editSpawnerPage.shouldHaveNotebookImageSelectInput('Test Image'); editSpawnerPage.shouldHaveContainerSizeInput('Small'); - editSpawnerPage.shouldHavePersistentStorage('Test Storage'); + editSpawnerPage + .getStorageTable() + .getRowById(0) + .findNameValue() + .should('have.text', 'Test Storage'); editSpawnerPage.findSubmitButton().should('be.enabled'); editSpawnerPage.k8sNameDescription.findDisplayNameInput().fill('Updated Notebook'); @@ -702,9 +686,35 @@ describe('Workbench page', () => { editSpawnerPage.findAcceleratorProfileSelect().findSelectOption('None').click(); editSpawnerPage.findAcceleratorProfileSelect().should('contain', 'None'); + cy.interceptK8s('PUT', PVCModel, mockPVCK8sResource({ name: 'test-notebook' })).as( + 'editClusterStorage', + ); cy.interceptK8s('PUT', NotebookModel, mockNotebookK8sResource({})).as('editWorkbenchDryRun'); cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})).as('editWorkbench'); + editSpawnerPage.findSubmitButton().click(); + + cy.wait('@editClusterStorage').then((interception) => { + expect(interception.request.url).to.include('?dryRun=All'); + expect(interception.request.body).to.containSubset({ + metadata: { + annotations: { + 'openshift.io/description': '', + 'openshift.io/display-name': 'Test Storage', + }, + name: 'test-notebook', + namespace: 'test-project', + labels: { + 'opendatahub.io/dashboard': 'true', + }, + }, + spec: { + resources: { requests: { storage: '5Gi' } }, + }, + status: { phase: 'Pending', accessModes: ['ReadWriteOnce'], capacity: { storage: '5Gi' } }, + }); + }); + cy.wait('@editWorkbenchDryRun').then((interception) => { expect(interception.request.url).to.include('?dryRun=All'); expect(interception.request.body).to.containSubset({ diff --git a/frontend/src/api/k8s/__tests__/pvcs.spec.ts b/frontend/src/api/k8s/__tests__/pvcs.spec.ts index b645fce99b..91d8b9133c 100644 --- a/frontend/src/api/k8s/__tests__/pvcs.spec.ts +++ b/frontend/src/api/k8s/__tests__/pvcs.spec.ts @@ -10,7 +10,7 @@ import { mockPVCK8sResource } from '~/__mocks__/mockPVCK8sResource'; import { assemblePvc, createPvc, deletePvc, getDashboardPvcs, updatePvc } from '~/api/k8s/pvcs'; import { PVCModel } from '~/api/models/k8s'; import { PersistentVolumeClaimKind } from '~/k8sTypes'; -import { CreatingStorageObject } from '~/pages/projects/types'; +import { StorageData } from '~/pages/projects/types'; jest.mock('@openshift/dynamic-plugin-sdk-utils', () => ({ k8sGetResource: jest.fn(), @@ -26,11 +26,9 @@ const k8sCreateResourceMock = jest.mocked(k8sCreateResource); const k8sDeleteResourceMock = jest.mocked(k8sDeleteResource); -const data: CreatingStorageObject = { - nameDesc: { - name: 'pvc', - description: 'Test Storage', - }, +const data: StorageData = { + name: 'pvc', + description: 'Test Storage', size: '5Gi', }; diff --git a/frontend/src/api/k8s/pvcs.ts b/frontend/src/api/k8s/pvcs.ts index 7a42a7398b..05a6be46e5 100644 --- a/frontend/src/api/k8s/pvcs.ts +++ b/frontend/src/api/k8s/pvcs.ts @@ -12,20 +12,15 @@ import { PVCModel } from '~/api/models'; import { translateDisplayNameForK8s } from '~/concepts/k8s/utils'; import { LABEL_SELECTOR_DASHBOARD_RESOURCE } from '~/const'; import { applyK8sAPIOptions } from '~/api/apiMergeUtils'; -import { CreatingStorageObject } from '~/pages/projects/types'; +import { StorageData } from '~/pages/projects/types'; export const assemblePvc = ( - data: CreatingStorageObject, + data: StorageData, namespace: string, editName?: string, hideFromUI?: boolean, ): PersistentVolumeClaimKind => { - const { - nameDesc: { name: pvcName, description }, - size, - storageClassName, - } = data; - + const { name: pvcName, description, size, storageClassName } = data; const name = editName || translateDisplayNameForK8s(pvcName); return { @@ -41,14 +36,14 @@ export const assemblePvc = ( }), annotations: { 'openshift.io/display-name': pvcName.trim(), - 'openshift.io/description': description, + ...(description && { 'openshift.io/description': description }), }, }, spec: { accessModes: ['ReadWriteOnce'], resources: { requests: { - storage: size, + storage: String(size), }, }, storageClassName, @@ -70,7 +65,7 @@ export const getDashboardPvcs = (projectName: string): Promise = ({ createNIMPVC(namespace, nimPVCName, pvcSize, false).then(() => undefined), ); } else if (pvc && pvc.spec.resources.requests.storage !== pvcSize) { - const createData: CreatingStorageObject = { + const createData = { size: pvcSize, // New size - nameDesc: { - name: pvc.metadata.name, - description: pvc.metadata.annotations?.description || '', - }, + name: pvc.metadata.name, + description: pvc.metadata.annotations?.description || '', storageClassName: pvc.spec.storageClassName, }; promises.push( diff --git a/frontend/src/pages/modelServing/screens/projects/__tests__/utils.spec.ts b/frontend/src/pages/modelServing/screens/projects/__tests__/utils.spec.ts index 1cad3ac2bf..e5fd491693 100644 --- a/frontend/src/pages/modelServing/screens/projects/__tests__/utils.spec.ts +++ b/frontend/src/pages/modelServing/screens/projects/__tests__/utils.spec.ts @@ -432,10 +432,8 @@ describe('createNIMPVC', () => { expect(createPvc).toHaveBeenCalledWith( { - nameDesc: { - name: pvcName, - description: '', - }, + name: pvcName, + description: '', size: pvcSize, }, projectName, @@ -451,10 +449,8 @@ describe('createNIMPVC', () => { expect(createPvc).toHaveBeenCalledWith( { - nameDesc: { - name: pvcName, - description: '', - }, + name: pvcName, + description: '', size: pvcSize, }, projectName, diff --git a/frontend/src/pages/modelServing/screens/projects/utils.ts b/frontend/src/pages/modelServing/screens/projects/utils.ts index ea1e7410b3..ec148a5fda 100644 --- a/frontend/src/pages/modelServing/screens/projects/utils.ts +++ b/frontend/src/pages/modelServing/screens/projects/utils.ts @@ -674,10 +674,8 @@ export const createNIMPVC = ( ): Promise => createPvc( { - nameDesc: { - name: pvcName, - description: '', - }, + name: pvcName, + description: '', size: pvcSize, }, projectName, diff --git a/frontend/src/pages/projects/pvc/useNotebookPVCItems.ts b/frontend/src/pages/projects/pvc/useNotebookPVCItems.ts index 39a34c7d90..085ffd0f9f 100644 --- a/frontend/src/pages/projects/pvc/useNotebookPVCItems.ts +++ b/frontend/src/pages/projects/pvc/useNotebookPVCItems.ts @@ -5,16 +5,16 @@ import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; import { getNotebookPVCNames } from './utils'; const useNotebookPVCItems = ( - notebook: NotebookKind, + notebook: NotebookKind | undefined, ): [pvcs: PersistentVolumeClaimKind[], loaded: boolean, loadError?: Error] => { const { pvcs: { data: allPvcs, loaded, error }, } = React.useContext(ProjectDetailsContext); - const pvcNames = useDeepCompareMemoize(getNotebookPVCNames(notebook)); + const pvcNames = useDeepCompareMemoize(notebook && getNotebookPVCNames(notebook)); const pvcs = React.useMemo( - () => allPvcs.filter((pvc) => pvcNames.includes(pvc.metadata.name)), + () => allPvcs.filter((pvc) => pvcNames?.includes(pvc.metadata.name)), [allPvcs, pvcNames], ); diff --git a/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx index e3fb46026f..614c3939c8 100644 --- a/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx @@ -7,7 +7,12 @@ import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas'; import usePreferredStorageClass from '~/pages/projects/screens/spawner/storage/usePreferredStorageClass'; import useDefaultStorageClass from '~/pages/projects/screens/spawner/storage/useDefaultStorageClass'; import { useCreateStorageObject } from '~/pages/projects/screens/spawner/storage/utils'; -import { CreatingStorageObject } from '~/pages/projects/types'; +import { StorageData } from '~/pages/projects/types'; + +type CreateStorageObjectData = Pick< + StorageData, + 'name' | 'description' | 'size' | 'storageClassName' +>; export type BaseStorageModalProps = { submitLabel?: string; @@ -15,8 +20,9 @@ export type BaseStorageModalProps = { description?: string; children: React.ReactNode; isValid: boolean; - onSubmit: (data: CreatingStorageObject) => Promise; + onSubmit: (data: CreateStorageObjectData) => Promise; existingData?: PersistentVolumeClaimKind; + formData?: CreateStorageObjectData; onClose: (submitted: boolean) => void; }; @@ -27,10 +33,11 @@ const BaseStorageModal: React.FC = ({ title = 'Add cluster storage', description = 'Add storage and optionally connect it with an existing workbench.', children, + formData, isValid, onClose, }) => { - const [createData, setCreateData, resetData] = useCreateStorageObject(existingData); + const [createData, setCreateData, resetData] = useCreateStorageObject(existingData, formData); const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status; const preferredStorageClass = usePreferredStorageClass(); const [defaultStorageClass] = useDefaultStorageClass(); @@ -59,7 +66,7 @@ const BaseStorageModal: React.FC = ({ resetData(); }; - const canCreate = !actionInProgress && createData.nameDesc.name.trim().length > 0 && isValid; + const canCreate = !actionInProgress && createData.name.trim().length > 0 && isValid; const submit = () => { setError(undefined); diff --git a/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx index cae3f2ff28..bd6a327b81 100644 --- a/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import { StackItem } from '@patternfly/react-core'; import { NotebookKind, PersistentVolumeClaimKind } from '~/k8sTypes'; -import { CreatingStorageObject, ForNotebookSelection, StorageData } from '~/pages/projects/types'; +import { ForNotebookSelection, StorageData } from '~/pages/projects/types'; import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; import { attachNotebookPVC, createPvc, removeNotebookPVC, restartNotebook, updatePvc } from '~/api'; import { @@ -51,7 +51,7 @@ const ClusterStorageModal: React.FC = (props) => { const restartNotebooks = useWillNotebooksRestart([...removedNotebooks, notebookData.name]); const handleSubmit = async ( - storageData: StorageData['creating'], + storageData: StorageData, attachNotebookData?: ForNotebookSelection, dryRun?: boolean, ) => { @@ -63,8 +63,8 @@ const ClusterStorageModal: React.FC = (props) => { // Check if PVC needs to be updated (name, description, size, storageClass) if ( - getDisplayNameFromK8sResource(existingData) !== storageData.nameDesc.name || - getDescriptionFromK8sResource(existingData) !== storageData.nameDesc.description || + getDisplayNameFromK8sResource(existingData) !== storageData.name || + getDescriptionFromK8sResource(existingData) !== storageData.description || existingData.spec.resources.requests.storage !== storageData.size || existingData.spec.storageClassName !== storageData.storageClassName ) { @@ -115,17 +115,8 @@ const ClusterStorageModal: React.FC = (props) => { } }; - const submit = async (data: CreatingStorageObject) => { - const storageData: CreatingStorageObject = { - nameDesc: data.nameDesc, - size: data.size, - storageClassName: data.storageClassName, - }; - - return handleSubmit(storageData, notebookData, true).then(() => - handleSubmit(storageData, notebookData, false), - ); - }; + const submit = async (data: StorageData) => + handleSubmit(data, notebookData, true).then(() => handleSubmit(data, notebookData, false)); const hasValidNotebookRelationship = notebookData.name ? !!notebookData.mountPath.value && !notebookData.mountPath.error diff --git a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx deleted file mode 100644 index cd57ea5533..0000000000 --- a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx +++ /dev/null @@ -1,213 +0,0 @@ -import * as React from 'react'; -import { Form, Modal, Stack, StackItem } from '@patternfly/react-core'; -import { attachNotebookPVC, createPvc, removeNotebookPVC, restartNotebook, updatePvc } from '~/api'; -import { NotebookKind, PersistentVolumeClaimKind } from '~/k8sTypes'; -import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; -import { useCreateStorageObjectForNotebook } from '~/pages/projects/screens/spawner/storage/utils'; -import CreateNewStorageSection from '~/pages/projects/screens/spawner/storage/CreateNewStorageSection'; -import StorageNotebookConnections from '~/pages/projects/notebook/StorageNotebookConnections'; -import { - useRelatedNotebooks, - ConnectedNotebookContext, -} from '~/pages/projects/notebook/useRelatedNotebooks'; -import NotebookRestartAlert from '~/pages/projects/components/NotebookRestartAlert'; -import useWillNotebooksRestart from '~/pages/projects/notebook/useWillNotebooksRestart'; -import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter'; -import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; -import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas'; -import usePreferredStorageClass from '~/pages/projects/screens/spawner/storage/usePreferredStorageClass'; -import useDefaultStorageClass from '~/pages/projects/screens/spawner/storage/useDefaultStorageClass'; -import ExistingConnectedNotebooks from './ExistingConnectedNotebooks'; - -type AddStorageModalProps = { - existingData?: PersistentVolumeClaimKind; - onClose: (submit: boolean) => void; -}; - -const ManageStorageModal: React.FC = ({ existingData, onClose }) => { - const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status; - const preferredStorageClass = usePreferredStorageClass(); - const [defaultStorageClass] = useDefaultStorageClass(); - - const [createData, setCreateData, resetData] = useCreateStorageObjectForNotebook(existingData); - const [actionInProgress, setActionInProgress] = React.useState(false); - const [error, setError] = React.useState(); - const { currentProject } = React.useContext(ProjectDetailsContext); - const namespace = currentProject.metadata.name; - const { notebooks: connectedNotebooks } = useRelatedNotebooks( - ConnectedNotebookContext.EXISTING_PVC, - existingData?.metadata.name, - ); - const [removedNotebooks, setRemovedNotebooks] = React.useState([]); - - const { - notebooks: removableNotebooks, - loaded: removableNotebookLoaded, - error: removableNotebookError, - } = useRelatedNotebooks(ConnectedNotebookContext.REMOVABLE_PVC, existingData?.metadata.name); - - const restartNotebooks = useWillNotebooksRestart([ - ...removedNotebooks, - createData.forNotebook.name, - ]); - - React.useEffect(() => { - if (!existingData) { - if (isStorageClassesAvailable) { - setCreateData('storageClassName', defaultStorageClass?.metadata.name); - } else { - setCreateData('storageClassName', preferredStorageClass?.metadata.name); - } - } - }, [ - isStorageClassesAvailable, - defaultStorageClass, - preferredStorageClass, - existingData, - setCreateData, - ]); - - const onBeforeClose = (submitted: boolean) => { - onClose(submitted); - setActionInProgress(false); - setRemovedNotebooks([]); - setError(undefined); - resetData(); - }; - - const hasValidNotebookRelationship = createData.forNotebook.name - ? !!createData.forNotebook.mountPath.value && !createData.forNotebook.mountPath.error - : true; - - const canCreate = - !actionInProgress && createData.nameDesc.name.trim() && hasValidNotebookRelationship; - - const runPromiseActions = async (dryRun: boolean) => { - const { - forNotebook: { name: notebookName, mountPath }, - } = createData; - const pvcPromises: Promise[] = []; - if (existingData) { - const pvcName = existingData.metadata.name; - if ( - getDisplayNameFromK8sResource(existingData) !== createData.nameDesc.name || - getDescriptionFromK8sResource(existingData) !== createData.nameDesc.description || - existingData.spec.resources.requests.storage !== createData.size || - existingData.spec.storageClassName !== createData.storageClassName - ) { - pvcPromises.push(updatePvc(createData, existingData, namespace, { dryRun })); - } - if (existingData.spec.resources.requests.storage !== createData.size) { - connectedNotebooks.map((connectedNotebook) => - pvcPromises.push(restartNotebook(connectedNotebook.metadata.name, namespace, { dryRun })), - ); - } - if (removedNotebooks.length > 0) { - // Remove connected pvcs - pvcPromises.push( - ...removedNotebooks.map((currentNotebookName) => - removeNotebookPVC(currentNotebookName, namespace, pvcName, { dryRun }), - ), - ); - } - - await Promise.all(pvcPromises); - if (notebookName) { - await attachNotebookPVC(notebookName, namespace, pvcName, mountPath.value, { - dryRun, - }); - } - return; - } - const createdPvc = await createPvc(createData, namespace, { dryRun }); - if (notebookName) { - await attachNotebookPVC(notebookName, namespace, createdPvc.metadata.name, mountPath.value, { - dryRun, - }); - } - }; - - const submit = () => { - setError(undefined); - setActionInProgress(true); - - runPromiseActions(true) - .then(() => runPromiseActions(false).then(() => onBeforeClose(true))) - .catch((e) => { - setError(e); - setActionInProgress(false); - }); - }; - - return ( - onBeforeClose(false)} - showClose - footer={ - onBeforeClose(false)} - isSubmitDisabled={!canCreate} - error={error} - alertTitle="Error creating storage" - /> - } - > -
{ - e.preventDefault(); - submit(); - }} - > - - - - - {createData.hasExistingNotebookConnections && ( - - - setRemovedNotebooks([...removedNotebooks, notebook.metadata.name]) - } - loaded={removableNotebookLoaded} - error={removableNotebookError} - /> - - )} - - { - setCreateData('forNotebook', forNotebookData); - }} - forNotebookData={createData.forNotebook} - connectedNotebooks={connectedNotebooks} - /> - - {restartNotebooks.length !== 0 && ( - - - - )} - -
- - ); -}; - -export default ManageStorageModal; diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx index 2c26fe6175..ad23951d0d 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx @@ -29,16 +29,16 @@ import { import { createConfigMapsAndSecretsForNotebook, createPvcDataForNotebook, - replaceRootVolumesForNotebook, updateConfigMapsAndSecretsForNotebook, + updatePvcDataForNotebook, } from './service'; -import { checkRequiredFieldsForNotebookStart } from './spawnerUtils'; +import { checkRequiredFieldsForNotebookStart, getPvcVolumeDetails } from './spawnerUtils'; import { getNotebookDataConnection } from './dataConnection/useNotebookDataConnection'; import { setConnectionsOnEnvFrom } from './connections/utils'; type SpawnerFooterProps = { startNotebookData: StartNotebookData; - storageData: StorageData; + storageData: StorageData[]; envVariables: EnvVariable[]; dataConnection: DataConnectionData; isConnectionTypesEnabled?: boolean; @@ -79,17 +79,13 @@ const SpawnerFooter: React.FC = ({ const [createInProgress, setCreateInProgress] = React.useState(false); const isButtonDisabled = createInProgress || - !checkRequiredFieldsForNotebookStart( - startNotebookData, - storageData, - envVariables, - dataConnection, - ); + !checkRequiredFieldsForNotebookStart(startNotebookData, envVariables, dataConnection); const { username } = useUser(); const existingNotebookDataConnection = getNotebookDataConnection( editNotebook, existingDataConnections, ); + const afterStart = (name: string, type: 'created' | 'updated') => { const { selectedAcceleratorProfile, notebookSize, image } = startNotebookData; const tep: FormTrackingEventProperties = { @@ -110,8 +106,6 @@ const SpawnerFooter: React.FC = ({ imageName: image.imageStream?.metadata.name, projectName, notebookName: name, - storageType: storageData.storageType, - storageDataSize: storageData.creating.size, dataConnectionType: dataConnection.creating?.type?.toString(), dataConnectionCategory: dataConnection.creating?.values?.category?.toString(), dataConnectionEnabled: dataConnection.enabled, @@ -141,12 +135,16 @@ const SpawnerFooter: React.FC = ({ return; } - const pvcDetails = await replaceRootVolumesForNotebook( - projectName, - editNotebook, - storageData, - dryRun, - ); + const pvcRequests = storageData.map((pvcData) => { + if (pvcData.existingPvc) { + return updatePvcDataForNotebook(projectName, pvcData, pvcData.existingPvc, dryRun); + } + + return createPvcDataForNotebook(projectName, pvcData, dryRun); + }); + + const pvcResponses = await Promise.all(pvcRequests); + const pvcVolumeDetails = getPvcVolumeDetails(pvcResponses); let envFrom = await updateConfigMapsAndSecretsForNotebook( projectName, @@ -167,7 +165,7 @@ const SpawnerFooter: React.FC = ({ annotations['notebooks.opendatahub.io/notebook-restart'] = 'true'; } - const { volumes, volumeMounts } = pvcDetails; + const { volumes, volumeMounts } = pvcVolumeDetails; const newStartNotebookData: StartNotebookData = { ...startNotebookData, volumes, @@ -205,14 +203,25 @@ const SpawnerFooter: React.FC = ({ dataConnection.enabled && dataConnection.type === 'existing' && dataConnection.existing ? [dataConnection.existing] : []; - const pvcDetails = await createPvcDataForNotebook(projectName, storageData, dryRun); + + const pvcRequests = storageData.map((pvcData) => { + if (pvcData.existingPvc) { + return updatePvcDataForNotebook(projectName, pvcData, pvcData.existingPvc, dryRun); + } + + return createPvcDataForNotebook(projectName, pvcData, dryRun); + }); + + const pvcResponses = await Promise.all(pvcRequests); + const pvcVolumeDetails = getPvcVolumeDetails(pvcResponses); + let envFrom = await createConfigMapsAndSecretsForNotebook( projectName, [...envVariables, ...newDataConnection], dryRun, ); - const { volumes, volumeMounts } = pvcDetails; + const { volumes, volumeMounts } = pvcVolumeDetails; if (isConnectionTypesEnabled) { envFrom = setConnectionsOnEnvFrom(connections, envFrom, projectConnections); } diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx index a4b847d9fb..105b950a96 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx @@ -1,9 +1,11 @@ import * as React from 'react'; import { Link } from 'react-router-dom'; import { - Alert, Breadcrumb, BreadcrumbItem, + Button, + Flex, + FlexItem, Form, FormSection, PageSection, @@ -30,15 +32,16 @@ import { LimitNameResourceType } from '~/concepts/k8s/K8sNameDescriptionField/ut import useConnectionTypesEnabled from '~/concepts/connectionTypes/useConnectionTypesEnabled'; import { Connection } from '~/concepts/connectionTypes/types'; import useNotebookAcceleratorProfileFormState from '~/pages/projects/screens/detail/notebooks/useNotebookAcceleratorProfileFormState'; +import { StorageData, StorageType } from '~/pages/projects/types'; +import useNotebookPVCItems from '~/pages/projects/pvc/useNotebookPVCItems'; +import { getNotebookPVCMountPathMap } from '~/pages/projects/notebook/utils'; import { SpawnerPageSectionID } from './types'; import { ScrollableSelectorID, SpawnerPageSectionTitles } from './const'; import SpawnerFooter from './SpawnerFooter'; import ImageSelectorField from './imageSelector/ImageSelectorField'; import ContainerSizeSelector from './deploymentSize/ContainerSizeSelector'; -import StorageField from './storage/StorageField'; import EnvironmentVariables from './environmentVariables/EnvironmentVariables'; -import { useStorageDataObject } from './storage/utils'; -import { getCompatibleAcceleratorIdentifiers, useMergeDefaultPVCName } from './spawnerUtils'; +import { getCompatibleAcceleratorIdentifiers } from './spawnerUtils'; import { useNotebookEnvVariables } from './environmentVariables/useNotebookEnvVariables'; import DataConnectionField from './dataConnection/DataConnectionField'; import { useNotebookDataConnection } from './dataConnection/useNotebookDataConnection'; @@ -48,6 +51,12 @@ import usePreferredStorageClass from './storage/usePreferredStorageClass'; import { ConnectionsFormSection } from './connections/ConnectionsFormSection'; import { getConnectionsFromNotebook } from './connections/utils'; import AlertWarningText from './environmentVariables/AlertWarningText'; +import { ClusterStorageTable } from './storage/ClusterStorageTable'; +import useDefaultPvcSize from './storage/useDefaultPvcSize'; +import { defaultClusterStorage } from './storage/constants'; +import { ClusterStorageEmptyState } from './storage/ClusterStorageEmptyState'; +import AttachExistingStorageModal from './storage/AttachExistingStorageModal'; +import WorkbenchStorageModal from './storage/WorkbenchStorageModal'; type SpawnerPageProps = { existingNotebook?: NotebookKind; @@ -66,6 +75,8 @@ const SpawnerPage: React.FC = ({ existingNotebook }) => { limitNameResourceType: LimitNameResourceType.WORKBENCH, safePrefix: 'wb-', }); + const [isAttachStorageModalOpen, setIsAttachStorageModalOpen] = React.useState(false); + const [isCreateStorageModalOpen, setIsCreateStorageModalOpen] = React.useState(false); const [selectedImage, setSelectedImage] = React.useState({ imageStream: undefined, imageVersion: undefined, @@ -74,18 +85,44 @@ const SpawnerPage: React.FC = ({ existingNotebook }) => { const [supportedAcceleratorProfiles, setSupportedAcceleratorProfiles] = React.useState< string[] | undefined >(); - const [storageDataWithoutDefault, setStorageData] = useStorageDataObject(existingNotebook); - const [defaultStorageClass] = useDefaultStorageClass(); const preferredStorageClass = usePreferredStorageClass(); const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status; const defaultStorageClassName = isStorageClassesAvailable ? defaultStorageClass?.metadata.name : preferredStorageClass?.metadata.name; - const storageData = useMergeDefaultPVCName( - storageDataWithoutDefault, - k8sNameDescriptionData.data.name, - defaultStorageClassName, + const defaultNotebookSize = useDefaultPvcSize(); + + const [existingPvcs] = useNotebookPVCItems(existingNotebook); + const [storageData, setStorageData] = React.useState( + existingNotebook + ? existingPvcs.map((existingPvc) => ({ + storageType: StorageType.EXISTING_PVC, + existingPvc, + name: + existingPvc.metadata.annotations?.['openshift.io/display-name'] || + existingPvc.metadata.name, + description: existingPvc.metadata.annotations?.['openshift.io/description'], + size: existingPvc.spec.resources.requests.storage, + storageClassName: existingPvc.spec.storageClassName, + mountPath: getNotebookPVCMountPathMap(existingNotebook)[existingPvc.metadata.name], + })) + : [ + { + storageType: StorageType.NEW_PVC, + name: k8sNameDescriptionData.data.name || defaultClusterStorage.name, + description: defaultClusterStorage.description, + size: defaultClusterStorage.size || defaultNotebookSize, + storageClassName: defaultStorageClassName || defaultClusterStorage.storageClassName, + mountPath: defaultClusterStorage.mountPath, + }, + ], + ); + + const existingMountPaths = storageData.reduce( + (acc: string[], storageDataEntry) => + storageDataEntry.mountPath ? acc.concat(storageDataEntry.mountPath) : acc, + [], ); const [dataConnectionData, setDataConnectionData] = useNotebookDataConnection( @@ -225,19 +262,45 @@ const SpawnerPage: React.FC = ({ existingNotebook }) => { + + {SpawnerPageSectionTitles[SpawnerPageSectionID.CLUSTER_STORAGE]} + + + + + + + } id={SpawnerPageSectionID.CLUSTER_STORAGE} aria-label={SpawnerPageSectionTitles[SpawnerPageSectionID.CLUSTER_STORAGE]} > - - + {storageData.length ? ( + ({ ...formData, id: index }))} + setStorageData={setStorageData} + workbenchName={k8sNameDescriptionData.data.k8sName.value} + /> + ) : ( + + )} {isConnectionTypesEnabled ? ( = ({ existingNotebook }) => { + + {isAttachStorageModalOpen && ( + { + if (submit && attachData?.storage) { + setStorageData((prevData) => + prevData.concat([ + { + storageType: StorageType.EXISTING_PVC, + id: storageData.length + 1, + name: attachData.storage, + existingPvc: attachData.pvc, + mountPath: attachData.mountPath.value, + description: attachData.pvc?.metadata.annotations?.['openshift.io/description'], + size: attachData.pvc?.spec.resources.requests.storage, + storageClassName: attachData.pvc?.spec.storageClassName, + }, + ]), + ); + } + + setIsAttachStorageModalOpen(false); + }} + /> + )} + + {isCreateStorageModalOpen && ( + + setStorageData((prevData) => + prevData.concat([ + { + ...newStorageData, + storageType: StorageType.NEW_PVC, + id: storageData.length + 1, + }, + ]), + ) + } + existingMountPaths={existingMountPaths} + onClose={() => setIsCreateStorageModalOpen(false)} + /> + )} ); }; diff --git a/frontend/src/pages/projects/screens/spawner/service.ts b/frontend/src/pages/projects/screens/spawner/service.ts index f92dbe3f7c..bb91da808d 100644 --- a/frontend/src/pages/projects/screens/spawner/service.ts +++ b/frontend/src/pages/projects/screens/spawner/service.ts @@ -10,6 +10,7 @@ import { deleteSecret, replaceConfigMap, replaceSecret, + updatePvc, } from '~/api'; import { Volume, VolumeMount } from '~/types'; import { @@ -23,9 +24,8 @@ import { StorageType, } from '~/pages/projects/types'; import { ROOT_MOUNT_PATH } from '~/pages/projects/pvc/const'; -import { ConfigMapKind, NotebookKind, SecretKind } from '~/k8sTypes'; import { Connection } from '~/concepts/connectionTypes/types'; -import { getVolumesByStorageData } from './spawnerUtils'; +import { ConfigMapKind, NotebookKind, PersistentVolumeClaimKind, SecretKind } from '~/k8sTypes'; import { fetchNotebookEnvVariables } from './environmentVariables/useNotebookEnvVariables'; import { getDeletedConfigMapOrSecretVariables } from './environmentVariables/utils'; @@ -34,68 +34,37 @@ export const createPvcDataForNotebook = async ( storageData: StorageData, dryRun?: boolean, ): Promise<{ volumes: Volume[]; volumeMounts: VolumeMount[] }> => { - const { storageType } = storageData; + const volumes: Volume[] = []; + const volumeMounts: VolumeMount[] = []; - const { volumes, volumeMounts } = getVolumesByStorageData(storageData); + if (storageData.storageType === StorageType.NEW_PVC) { + const pvc = await createPvc(storageData, projectName, { dryRun }); + const { name } = pvc.metadata; - if (storageType === StorageType.NEW_PVC) { - const pvc = await createPvc(storageData.creating, projectName, { dryRun }); - const newPvcName = pvc.metadata.name; - volumes.push({ name: newPvcName, persistentVolumeClaim: { claimName: newPvcName } }); - volumeMounts.push({ mountPath: ROOT_MOUNT_PATH, name: newPvcName }); + volumes.push({ name, persistentVolumeClaim: { claimName: name } }); + volumeMounts.push({ mountPath: storageData.mountPath || ROOT_MOUNT_PATH, name }); } + return { volumes, volumeMounts }; }; -export const replaceRootVolumesForNotebook = async ( +export const updatePvcDataForNotebook = async ( projectName: string, - notebook: NotebookKind, storageData: StorageData, + existingPvc: PersistentVolumeClaimKind | undefined, dryRun?: boolean, ): Promise<{ volumes: Volume[]; volumeMounts: VolumeMount[] }> => { - const { - storageType, - existing: { storage: existingName }, - } = storageData; - - const oldVolumes = notebook.spec.template.spec.volumes || []; - const oldVolumeMounts = notebook.spec.template.spec.containers[0].volumeMounts || []; - - let replacedVolume: Volume; - let replacedVolumeMount: VolumeMount; - - if (storageType === StorageType.EXISTING_PVC) { - replacedVolume = { - name: existingName, - persistentVolumeClaim: { claimName: existingName }, - }; - replacedVolumeMount = { name: existingName, mountPath: ROOT_MOUNT_PATH }; - } else { - 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 }; - } + const volumes: Volume[] = []; + const volumeMounts: VolumeMount[] = []; - const rootVolumeMount = oldVolumeMounts.find( - (volumeMount) => volumeMount.mountPath === ROOT_MOUNT_PATH, - ); + if (existingPvc && storageData.storageType === StorageType.EXISTING_PVC) { + const pvc = await updatePvc(storageData, existingPvc, projectName, { dryRun }); + const { name } = pvc.metadata; - // if no root, add the replaced one as the root - if (!rootVolumeMount) { - return { - volumes: [...oldVolumes, replacedVolume], - volumeMounts: [...oldVolumeMounts, replacedVolumeMount], - }; + volumes.push({ name, persistentVolumeClaim: { claimName: name } }); + volumeMounts.push({ mountPath: storageData.mountPath || ROOT_MOUNT_PATH, name }); } - const volumes = oldVolumes.map((volume) => - volume.name === rootVolumeMount.name ? replacedVolume : volume, - ); - const volumeMounts = oldVolumeMounts.map((volumeMount) => - volumeMount.name === rootVolumeMount.name ? replacedVolumeMount : volumeMount, - ); - return { volumes, volumeMounts }; }; diff --git a/frontend/src/pages/projects/screens/spawner/spawnerUtils.ts b/frontend/src/pages/projects/screens/spawner/spawnerUtils.ts index 955286d4a8..45b3f61637 100644 --- a/frontend/src/pages/projects/screens/spawner/spawnerUtils.ts +++ b/frontend/src/pages/projects/screens/spawner/spawnerUtils.ts @@ -1,13 +1,6 @@ -import * as React from 'react'; import compareVersions from 'compare-versions'; import { NotebookSize, Volume, VolumeMount } from '~/types'; -import { - BuildKind, - ImageStreamKind, - ImageStreamSpecTagType, - K8sDSGResource, - NotebookKind, -} from '~/k8sTypes'; +import { BuildKind, ImageStreamKind, ImageStreamSpecTagType, K8sDSGResource } from '~/k8sTypes'; import { ConfigMapCategory, DataConnectionData, @@ -15,10 +8,7 @@ import { EnvVariableDataEntry, SecretCategory, StartNotebookData, - StorageData, - StorageType, } from '~/pages/projects/types'; -import { ROOT_MOUNT_PATH } from '~/pages/projects/pvc/const'; import { AWS_FIELDS } from '~/pages/projects/dataConnections/const'; import { FieldOptions } from '~/components/FieldList'; import { isK8sNameDescriptionDataValid } from '~/concepts/k8s/K8sNameDescriptionField/utils'; @@ -31,31 +21,6 @@ import { import { FAILED_PHASES, PENDING_PHASES, IMAGE_ANNOTATIONS } from './const'; /******************* Common utils *******************/ -export const useMergeDefaultPVCName = ( - storageData: StorageData, - defaultPVCName: string, - defaultStorageClassName?: string, -): StorageData => { - const modifiedRef = React.useRef(false); - - if (modifiedRef.current || storageData.creating.nameDesc.name) { - modifiedRef.current = true; - return storageData; - } - - return { - ...storageData, - creating: { - ...storageData.creating, - nameDesc: { - ...storageData.creating.nameDesc, - name: storageData.creating.nameDesc.name || defaultPVCName, - }, - storageClassName: storageData.creating.storageClassName || defaultStorageClassName, - }, - }; -}; - export const getVersion = (version?: string | number, prefix?: string): string => { if (!version) { return ''; @@ -270,30 +235,6 @@ export const getSizeDescription = (size: NotebookSize): string => `Requests: ${size.resources.requests?.cpu || '??'} CPU, ` + `${formatMemory(size.resources.requests?.memory) || '??'} Memory`; -/******************* Storage utils *******************/ -export const getVolumesByStorageData = ( - storageData: StorageData, -): { volumes: Volume[]; volumeMounts: VolumeMount[] } => { - const { storageType, existing } = storageData; - const volumes: Volume[] = []; - const volumeMounts: VolumeMount[] = []; - - if (storageType === StorageType.EXISTING_PVC) { - const { storage } = existing; - if (storage) { - volumes.push({ name: storage, persistentVolumeClaim: { claimName: storage } }); - volumeMounts.push({ mountPath: ROOT_MOUNT_PATH, name: storage }); - } - } - - return { volumes, volumeMounts }; -}; - -export const getRootVolumeName = (notebook?: NotebookKind): string => - notebook?.spec.template.spec.containers[0].volumeMounts?.find( - (volumeMount) => volumeMount.mountPath === ROOT_MOUNT_PATH, - )?.name || ''; - /******************* Checking utils *******************/ /** * Check if there is 1 or more versions available for an image stream @@ -396,12 +337,10 @@ export const isEnvVariableDataValid = (envVariables: EnvVariable[]): boolean => export const checkRequiredFieldsForNotebookStart = ( startNotebookData: StartNotebookData, - storageData: StorageData, envVariables: EnvVariable[], dataConnection: DataConnectionData, ): boolean => { const { projectName, notebookData, image } = startNotebookData; - const { storageType, creating, existing } = storageData; const isNotebookDataValid = !!( projectName && isK8sNameDescriptionDataValid(notebookData) && @@ -409,10 +348,6 @@ export const checkRequiredFieldsForNotebookStart = ( image.imageVersion ); - const newStorageFieldInvalid = storageType === StorageType.NEW_PVC && !creating.nameDesc.name; - const existingStorageFieldInvalid = storageType === StorageType.EXISTING_PVC && !existing.storage; - const isStorageDataValid = !newStorageFieldInvalid && !existingStorageFieldInvalid; - const newDataConnectionInvalid = dataConnection.type === 'creating' && !(dataConnection.creating?.values?.data && isAWSValid(dataConnection.creating.values.data)); @@ -421,12 +356,7 @@ export const checkRequiredFieldsForNotebookStart = ( const isDataConnectionValid = !dataConnection.enabled || (!newDataConnectionInvalid && !existingDataConnectionInvalid); - return ( - isNotebookDataValid && - isStorageDataValid && - isEnvVariableDataValid(envVariables) && - isDataConnectionValid - ); + return isNotebookDataValid && isEnvVariableDataValid(envVariables) && isDataConnectionValid; }; export const isInvalidBYONImageStream = (imageStream: ImageStreamKind): boolean => { @@ -440,3 +370,31 @@ export const isInvalidBYONImageStream = (imageStream: ImageStreamKind): boolean (activeTag === undefined || activeTag.items === null) ); }; + +export const getPvcVolumeDetails = ( + pvcVolumeList: { volumes: Volume[]; volumeMounts: VolumeMount[] }[], +): { + volumes: Volume[]; + volumeMounts: VolumeMount[]; +} => + pvcVolumeList.reduce( + (acc, response) => { + if (response.volumes.length) { + acc.volumes = acc.volumes.concat(response.volumes); + } else { + acc.volumes = response.volumes; + } + + if (response.volumeMounts.length) { + acc.volumeMounts = acc.volumeMounts.concat(response.volumeMounts); + } else { + acc.volumeMounts = response.volumeMounts; + } + + return acc; + }, + { + volumes: [], + volumeMounts: [], + }, + ); diff --git a/frontend/src/pages/projects/screens/spawner/storage/AddExistingStorageField.tsx b/frontend/src/pages/projects/screens/spawner/storage/AddExistingStorageField.tsx index 16235a5697..380949e4d0 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/AddExistingStorageField.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/AddExistingStorageField.tsx @@ -72,7 +72,15 @@ const AddExistingStorageField: React.FC = ({ setData({ ...data, storage: String(storage) || '' })} + onSelect={(_, storage) => { + const pvc = storages.find((pvcData) => pvcData.metadata.name === storage); + + setData({ + ...data, + pvc, + storage: String(storage) || '', + }); + }} placeholder={placeholderText} noOptionsFoundMessage={(filter) => `No persistent storage was found for "${filter}"`} popperProps={{ direction: selectDirection, appendTo: menuAppendTo }} diff --git a/frontend/src/pages/projects/screens/spawner/storage/AttachExistingStorageModal.tsx b/frontend/src/pages/projects/screens/spawner/storage/AttachExistingStorageModal.tsx index 56db9041ed..bf80066ebb 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/AttachExistingStorageModal.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/AttachExistingStorageModal.tsx @@ -4,21 +4,27 @@ import { ExistingStorageObject, MountPath } from '~/pages/projects/types'; import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter'; import SpawnerMountPathField from './SpawnerMountPathField'; import AddExistingStorageField from './AddExistingStorageField'; +import { MountPathFormat } from './types'; type AttachExistingStorageModalData = ExistingStorageObject & { mountPath: MountPath; }; type AttachExistingStorageModalProps = { + existingMountPaths: string[]; onClose: (submit: boolean, storageData?: AttachExistingStorageModalData) => void; }; const initialState: AttachExistingStorageModalData = { storage: '', - mountPath: { value: '', error: '' }, + pvc: undefined, + mountPath: { value: '', error: '', format: MountPathFormat.STANDARD }, }; -const AttachExistingStorageModal: React.FC = ({ onClose }) => { +const AttachExistingStorageModal: React.FC = ({ + existingMountPaths, + onClose, +}) => { const [data, setData] = React.useState(initialState); const onBeforeClose = (submitted: boolean, storageData?: AttachExistingStorageModalData) => { @@ -26,12 +32,6 @@ const AttachExistingStorageModal: React.FC = ({ setData(initialState); }; - const isValid = - data.mountPath.value.length > 0 && - data.mountPath.value !== '/' && - !data.mountPath.error && - data.storage.trim() !== ''; - return ( = ({ submitLabel="Attach storage" onSubmit={() => onBeforeClose(true, data)} onCancel={() => onBeforeClose(false)} - isSubmitDisabled={!isValid} + isSubmitDisabled={!data.mountPath.value || !!data.mountPath.error} alertTitle="Error creating storage" /> } @@ -59,13 +59,16 @@ const AttachExistingStorageModal: React.FC = ({ setData({ ...data, storage: storageName.storage })} + setData={(existingStorage) => + setData({ ...data, storage: existingStorage.storage, pvc: existingStorage.pvc }) + } /> setData({ ...data, mountPath: path })} /> diff --git a/frontend/src/pages/projects/screens/spawner/storage/ClusterStorageDetachModal.tsx b/frontend/src/pages/projects/screens/spawner/storage/ClusterStorageDetachModal.tsx new file mode 100644 index 0000000000..e0644831b7 --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/ClusterStorageDetachModal.tsx @@ -0,0 +1,31 @@ +import React from 'react'; +import { Modal, ModalVariant, Button } from '@patternfly/react-core'; + +interface ClusterStorageDetachModalProps { + storageName: string; + onConfirm: () => void; + onClose: () => void; +} + +export const ClusterStorageDetachModal: React.FC = ({ + storageName, + onConfirm, + onClose, +}) => ( + + Detach + , + , + ]} + > + The {storageName} storage and all of its resources will be detached from the workbench. + +); diff --git a/frontend/src/pages/projects/screens/spawner/storage/ClusterStorageEmptyState.tsx b/frontend/src/pages/projects/screens/spawner/storage/ClusterStorageEmptyState.tsx new file mode 100644 index 0000000000..cb76218f87 --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/ClusterStorageEmptyState.tsx @@ -0,0 +1,22 @@ +import React from 'react'; +import { + EmptyState, + EmptyStateVariant, + EmptyStateIcon, + Title, + EmptyStateBody, +} from '@patternfly/react-core'; +import { PlusCircleIcon } from '@patternfly/react-icons'; + +export const ClusterStorageEmptyState: React.FC = () => ( + + + + No cluster storage + + + To save your project's data, attach cluster storage to your workbench. Your data will not + persist without storage. + + +); diff --git a/frontend/src/pages/projects/screens/spawner/storage/ClusterStorageTable.tsx b/frontend/src/pages/projects/screens/spawner/storage/ClusterStorageTable.tsx new file mode 100644 index 0000000000..b18aba8a1b --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/ClusterStorageTable.tsx @@ -0,0 +1,165 @@ +import React from 'react'; + +import { Flex, FlexItem, Icon, Popover, Truncate } from '@patternfly/react-core'; +import { Tr, Td, ActionsColumn } from '@patternfly/react-table'; +import { InfoCircleIcon } from '@patternfly/react-icons'; + +import { Table } from '~/components/table'; +import { StorageData, StorageType } from '~/pages/projects/types'; +import { clusterStorageTableColumns } from './constants'; +import { ClusterStorageDetachModal } from './ClusterStorageDetachModal'; +import WorkbenchStorageModal from './WorkbenchStorageModal'; + +interface ClusterStorageTableProps { + storageData: StorageData[]; + workbenchName: string; + existingMountPaths: string[]; + setStorageData: React.Dispatch>; +} + +export const ClusterStorageTable: React.FC = ({ + storageData, + workbenchName, + existingMountPaths, + setStorageData, +}) => { + const [isEditModalOpen, setIsEditModalOpen] = React.useState(false); + const [isDetachModalOpen, setIsDetachModalOpen] = React.useState(false); + const [selectedId, setSelectedId] = React.useState(); + const selectedStorage = storageData.find((formData) => formData.id === selectedId); + const hasUpdatedDefaultNameRef = React.useRef(false); + const initialDataRef = React.useRef(storageData); + + const updateStorageData = React.useCallback( + (newStorageData: StorageData) => { + setStorageData(() => + storageData.map((pvcData) => { + if (pvcData.id === selectedId) { + return { ...pvcData, ...newStorageData }; + } + + return pvcData; + }), + ); + + hasUpdatedDefaultNameRef.current = true; + }, + [selectedId, setStorageData, storageData], + ); + + React.useEffect(() => { + if (!hasUpdatedDefaultNameRef.current && initialDataRef.current.length) { + const [initialStorageData] = initialDataRef.current; + const { name: pvcName, storageType } = initialStorageData; + + if (storageType === StorageType.NEW_PVC) { + setStorageData((prevData) => + prevData.map((data) => + initialStorageData.id === 0 + ? { + ...initialStorageData, + name: workbenchName + ? (!pvcName.includes('-') ? `${workbenchName}-${pvcName}` : pvcName).replace( + /.*(?=-)/, + workbenchName, + ) + : pvcName.includes('-') + ? pvcName.split('-')[1] + : pvcName, + } + : data, + ), + ); + } + } + }, [workbenchName, setStorageData]); + + return ( + <> + ( + + + + + + + )} + /> + + {isEditModalOpen && selectedStorage && ( + { + setIsEditModalOpen(false); + setSelectedId(undefined); + }} + existingMountPaths={existingMountPaths} + /> + )} + + {isDetachModalOpen && selectedStorage && ( + { + setStorageData(() => storageData.filter((storage) => storage.id !== selectedId)); + setIsDetachModalOpen(false); + }} + storageName={selectedStorage.name} + onClose={() => { + setIsDetachModalOpen(false); + setSelectedId(undefined); + }} + /> + )} + + ); +}; diff --git a/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx b/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx index fdab0cab31..ae7a1e0e62 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx @@ -1,13 +1,13 @@ import * as React from 'react'; import { Stack, StackItem } from '@patternfly/react-core'; -import { CreatingStorageObject, UpdateObjectAtPropAndValue } from '~/pages/projects/types'; +import { StorageData, UpdateObjectAtPropAndValue } from '~/pages/projects/types'; import PVSizeField from '~/pages/projects/components/PVSizeField'; import NameDescriptionField from '~/concepts/k8s/NameDescriptionField'; import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas'; import { PersistentVolumeClaimKind } from '~/k8sTypes'; import StorageClassSelect from './StorageClassSelect'; -type CreateNewStorageSectionProps = { +type CreateNewStorageSectionProps = { data: D; setData: UpdateObjectAtPropAndValue; currentStatus?: PersistentVolumeClaimKind['status']; @@ -16,7 +16,7 @@ type CreateNewStorageSectionProps = { disableStorageClassSelect?: boolean; }; -const CreateNewStorageSection = ({ +const CreateNewStorageSection = ({ data, setData, currentStatus, @@ -32,8 +32,11 @@ const CreateNewStorageSection = ({ setData('nameDesc', newData)} + data={{ name: data.name, description: data.description || '' }} + setData={(newData) => { + setData('name', newData.name); + setData('description', newData.description); + }} autoFocusName={autoFocusName} /> @@ -52,7 +55,7 @@ const CreateNewStorageSection = ({ menuAppendTo={menuAppendTo} fieldID="create-new-storage-size" currentStatus={currentStatus} - size={data.size} + size={String(data.size)} setSize={(size) => setData('size', size)} /> diff --git a/frontend/src/pages/projects/screens/spawner/storage/SpawnerMountPathField.tsx b/frontend/src/pages/projects/screens/spawner/storage/SpawnerMountPathField.tsx index d3f076ac8a..570bace13c 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/SpawnerMountPathField.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/SpawnerMountPathField.tsx @@ -1,3 +1,5 @@ +import React from 'react'; + import { Flex, FlexItem, @@ -12,17 +14,13 @@ import { StackItem, TextInput, } from '@patternfly/react-core'; -import React from 'react'; + import FieldGroupHelpLabelIcon from '~/components/FieldGroupHelpLabelIcon'; +import { MountPath } from '~/pages/projects/types'; import { useMountPathFormat, validateMountPath } from './utils'; import { MountPathFormat } from './types'; import { MOUNT_PATH_PREFIX } from './const'; -interface MountPath { - value: string; - error: string; -} - interface SpawnerMountPathFieldProps { isCreate: boolean; mountPath: MountPath; @@ -33,10 +31,10 @@ interface SpawnerMountPathFieldProps { const SpawnerMountPathField: React.FC = ({ isCreate, mountPath, - inUseMountPaths, + inUseMountPaths = [], onChange, }) => { - const [format, setFormat] = useMountPathFormat(isCreate, mountPath.value); + const [format, setFormat] = useMountPathFormat(isCreate, mountPath.value, mountPath.format); const [shouldShowValidation, setShouldShowValidation] = React.useState(false); const pathSuffix = React.useMemo(() => { @@ -46,9 +44,25 @@ const SpawnerMountPathField: React.FC = ({ : mountPath.value; }, [mountPath.value, format]); + const formatInitialExistingValue = (value: string) => { + if (value.startsWith(MOUNT_PATH_PREFIX)) { + return value.split(MOUNT_PATH_PREFIX)[1] ?? ''; + } + + return value; + }; + React.useEffect(() => { const initialValue = format === MountPathFormat.STANDARD ? MOUNT_PATH_PREFIX : '/'; - onChange({ value: initialValue, error: '' }); + const value = formatInitialExistingValue(mountPath.value || initialValue); + const error = validateMountPath(value, inUseMountPaths, format) || ''; + + onChange({ + value, + error, + format, + }); + // Only run on initial mount // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -58,16 +72,13 @@ const SpawnerMountPathField: React.FC = ({ const prefix = newFormat === MountPathFormat.STANDARD ? MOUNT_PATH_PREFIX : '/'; const newValue = `${prefix}${suffix}`; - // Only validate after the field has been touched - if (!shouldShowValidation && !suffix.trim()) { - onChange({ value: newValue, error: '' }); - return; - } - - const error = validateMountPath(suffix, inUseMountPaths || [], newFormat); - onChange({ value: newValue, error }); + onChange({ + value: newValue, + format: newFormat, + error: validateMountPath(suffix, inUseMountPaths, newFormat) || '', + }); }, - [format, inUseMountPaths, onChange, shouldShowValidation], + [format, inUseMountPaths, onChange], ); const handleFormatChange = (newFormat: MountPathFormat) => { @@ -79,6 +90,7 @@ const SpawnerMountPathField: React.FC = ({ if (!shouldShowValidation) { setShouldShowValidation(true); } + validateAndUpdate(suffix); }; @@ -141,19 +153,29 @@ const SpawnerMountPathField: React.FC = ({ onChange={(_, value) => handleSuffixChange(value)} isRequired validated={ - mountPath.error ? 'error' : pathSuffix.length > 0 ? 'success' : 'default' + shouldShowValidation && mountPath.error + ? 'error' + : pathSuffix.length > 0 + ? 'success' + : 'default' } + onBlur={() => { + validateAndUpdate(pathSuffix, mountPath.format); + setShouldShowValidation(true); + }} /> - - {mountPath.error || - 'Must only consist of lowercase letters, dashes, and slashes.'} - + {shouldShowValidation && mountPath.error && ( + + {mountPath.error} + + )} + {format === MountPathFormat.CUSTOM && ( Depending on the workbench type, this location may not be visible or accessible. diff --git a/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx b/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx index 17593c1d06..7ee8a66532 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx @@ -7,6 +7,7 @@ import { FormHelperText, HelperText, HelperTextItem, + Skeleton, } from '@patternfly/react-core'; import { ExclamationTriangleIcon } from '@patternfly/react-icons'; import React from 'react'; @@ -117,7 +118,9 @@ const StorageClassSelect: React.FC = ({ )} - ) : null; + ) : ( + + ); }; export default StorageClassSelect; diff --git a/frontend/src/pages/projects/screens/spawner/storage/StorageField.tsx b/frontend/src/pages/projects/screens/spawner/storage/StorageField.tsx deleted file mode 100644 index 0e2ac75155..0000000000 --- a/frontend/src/pages/projects/screens/spawner/storage/StorageField.tsx +++ /dev/null @@ -1,67 +0,0 @@ -import * as React from 'react'; -import { FormGroup, Radio, Stack, StackItem } from '@patternfly/react-core'; -import { StorageData, StorageType, UpdateObjectAtPropAndValue } from '~/pages/projects/types'; -import { getDashboardMainContainer } from '~/utilities/utils'; -import CreateNewStorageSection from './CreateNewStorageSection'; -import AddExistingStorageField from './AddExistingStorageField'; - -type StorageFieldType = { - storageData: StorageData; - setStorageData: UpdateObjectAtPropAndValue; -}; - -const StorageField: React.FC = ({ storageData, setStorageData }) => { - const { storageType, creating, existing } = storageData; - - return ( - - - - setStorageData('storageType', StorageType.NEW_PVC)} - body={ - storageType === StorageType.NEW_PVC && ( - - setStorageData('creating', { ...creating, [key]: value }) - } - /> - ) - } - /> - - - setStorageData('storageType', StorageType.EXISTING_PVC)} - body={ - storageType === StorageType.EXISTING_PVC && ( - setStorageData('existing', data)} - selectDirection="up" - menuAppendTo={getDashboardMainContainer()} - /> - ) - } - /> - - - - ); -}; - -export default StorageField; diff --git a/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx b/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx index ebdf1bed66..f3c3de444c 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx @@ -1,47 +1,59 @@ import * as React from 'react'; import { StackItem } from '@patternfly/react-core'; -import { PersistentVolumeClaimKind } from '~/k8sTypes'; -import { CreatingStorageObject, MountPath } from '~/pages/projects/types'; +import { MountPath, StorageData } from '~/pages/projects/types'; import BaseStorageModal from '~/pages/projects/screens/detail/storage/BaseStorageModal'; import SpawnerMountPathField from './SpawnerMountPathField'; type WorkbenchStorageModalProps = { - existingData?: PersistentVolumeClaimKind; + onSubmit: (storageData: StorageData) => void; onClose: (submit: boolean) => void; + existingMountPaths?: string[]; + formData?: StorageData; }; -const WorkbenchStorageModal: React.FC = (props) => { +const WorkbenchStorageModal: React.FC = ({ + formData, + existingMountPaths, + onSubmit, + onClose, +}) => { + const existingMountPath = formData?.mountPath; const [mountPath, setMountPath] = React.useState({ - value: '', + value: existingMountPath ?? '', error: '', }); const [actionInProgress, setActionInProgress] = React.useState(false); - //TODO: handleSubmit function to be completed in RHOAIENG-1101 - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const handleSubmit = async (createData: CreatingStorageObject) => { + React.useEffect(() => { + if (existingMountPath) { + setMountPath({ + value: existingMountPath, + error: '', + }); + } + }, [existingMountPath]); + + const handleSubmit = async (createData: StorageData) => { + onSubmit({ ...formData, ...createData, mountPath: mountPath.value }); setActionInProgress(true); }; - const isValid = - !actionInProgress && - mountPath.error === '' && - mountPath.value.length > 0 && - mountPath.value !== '/'; - return ( handleSubmit(createData)} - title={props.existingData ? 'Edit storage' : 'Create storage'} - submitLabel={props.existingData ? 'Save' : 'Create'} - isValid={isValid} + formData={formData} + existingData={formData?.existingPvc} + onSubmit={handleSubmit} + title={formData ? 'Edit storage' : 'Create storage'} + submitLabel={formData ? 'Save' : 'Create'} + isValid={!actionInProgress && !mountPath.error} + onClose={onClose} > setMountPath(path)} + inUseMountPaths={existingMountPaths} + onChange={setMountPath} /> diff --git a/frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts b/frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts index f7d319bd26..f1caea43b4 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts +++ b/frontend/src/pages/projects/screens/spawner/storage/__tests__/utils.spec.ts @@ -48,7 +48,8 @@ describe('useCreateStorageObject', () => { const [data] = result.current; expect(data).toEqual({ - nameDesc: { name: '', k8sName: undefined, description: '' }, + name: '', + description: '', size: '1Gi', }); }); @@ -57,8 +58,8 @@ describe('useCreateStorageObject', () => { const { result } = renderHook(() => useCreateStorageObject(existingData)); const [data] = result.current; - expect(data.nameDesc.name).toBe('test-pvc'); - expect(data.nameDesc.description).toBe('Test PVC Description'); + expect(data.name).toBe('test-pvc'); + expect(data.description).toBe('Test PVC Description'); expect(data.size).toBe('2Gi'); expect(data.storageClassName).toBe('test-storage-class'); }); @@ -72,8 +73,8 @@ describe('useCreateStorageObject', () => { }); const [data] = result.current; - expect(data.nameDesc.name).toBe(''); - expect(data.nameDesc.description).toBe(''); + expect(data.name).toBe(''); + expect(data.description).toBe(''); expect(data.size).toBe('1Gi'); // Default size from mock expect(data.storageClassName).toBeUndefined(); }); @@ -100,7 +101,7 @@ describe('validateMountPath', () => { }); it('should return error message for already in-use mount path', () => { - const result = validateMountPath('existing-folder', inUseMountPaths, MountPathFormat.STANDARD); + const result = validateMountPath('/existing-folder', inUseMountPaths, MountPathFormat.STANDARD); expect(result).toBe('Mount folder is already in use for this workbench.'); }); @@ -131,13 +132,6 @@ describe('validateMountPath', () => { }); describe('useMountPathFormat', () => { - it('return MountPathFormat.STANDARD if isCreate is true', () => { - const { result } = renderHook(() => useMountPathFormat(true, 'some-path')); - - const [format] = result.current; - expect(format).toBe(MountPathFormat.STANDARD); - }); - it('return MountPathFormat.STANDARD if mountPath starts with /opt/app-root/src/', () => { const { result } = renderHook(() => useMountPathFormat(false, `${MOUNT_PATH_PREFIX}/some-path`), @@ -171,22 +165,4 @@ describe('useMountPathFormat', () => { // Format should update to STANDARD expect(result.current[0]).toBe(MountPathFormat.STANDARD); }); - - it('should not update format if isCreate is true, regardless of mountPath', () => { - const { result, rerender } = renderHook( - ({ isCreate, mountPath }) => useMountPathFormat(isCreate, mountPath), - { - initialProps: { isCreate: true, mountPath: '/custom-path' }, - }, - ); - - // Initial format - expect(result.current[0]).toBe(MountPathFormat.STANDARD); - - // Change the mountPath but keep isCreate true - rerender({ isCreate: true, mountPath: `${MOUNT_PATH_PREFIX}/new-path` }); - - // Format should remain STANDARD - expect(result.current[0]).toBe(MountPathFormat.STANDARD); - }); }); diff --git a/frontend/src/pages/projects/screens/spawner/storage/constants.ts b/frontend/src/pages/projects/screens/spawner/storage/constants.ts new file mode 100644 index 0000000000..49f208d05e --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/constants.ts @@ -0,0 +1,36 @@ +import { SortableData, kebabTableColumn } from '~/components/table'; +import { StorageData } from '~/pages/projects/types'; +import { MOUNT_PATH_PREFIX } from './const'; + +export const clusterStorageTableColumns: SortableData[] = [ + { + label: 'ID', + field: 'id', + sortable: false, + className: 'pf-v5-u-hidden', + }, + { + label: 'Name', + field: 'name', + sortable: false, + }, + { + label: 'Storage size', + field: 'size', + sortable: false, + }, + { + label: 'Mount path', + field: 'mountPath', + sortable: false, + }, + kebabTableColumn(), +]; + +export const defaultClusterStorage = { + name: 'storage', + description: '', + size: '20Gi', + mountPath: MOUNT_PATH_PREFIX, + storageClassName: '', +}; diff --git a/frontend/src/pages/projects/screens/spawner/storage/useRelatedPvcs.ts b/frontend/src/pages/projects/screens/spawner/storage/useRelatedPvcs.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/pages/projects/screens/spawner/storage/utils.ts b/frontend/src/pages/projects/screens/spawner/storage/utils.ts index 41e461216d..00a18963c7 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/utils.ts +++ b/frontend/src/pages/projects/screens/spawner/storage/utils.ts @@ -1,19 +1,16 @@ import * as React from 'react'; import { - CreatingStorageObject, CreatingStorageObjectForNotebook, ExistingStorageObjectForNotebook, StorageData, - StorageType, UpdateObjectAtPropAndValue, } from '~/pages/projects/types'; -import { NotebookKind, PersistentVolumeClaimKind } from '~/k8sTypes'; +import { PersistentVolumeClaimKind } from '~/k8sTypes'; import { useRelatedNotebooks, ConnectedNotebookContext, } from '~/pages/projects/notebook/useRelatedNotebooks'; 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 { MountPathFormat } from './types'; @@ -21,34 +18,33 @@ import { MOUNT_PATH_PREFIX } from './const'; export const useCreateStorageObject = ( existingData?: PersistentVolumeClaimKind, + formData?: StorageData, ): [ - data: CreatingStorageObject, - setData: UpdateObjectAtPropAndValue, + data: StorageData, + setData: UpdateObjectAtPropAndValue, resetDefaults: () => void, ] => { const size = useDefaultPvcSize(); - const createDataState = useGenericObjectState({ - nameDesc: { - name: '', - k8sName: undefined, - description: '', - }, + const createDataState = useGenericObjectState({ + name: '', + description: '', size, }); - const [, setCreateData] = createDataState; - const existingName = existingData ? getDisplayNameFromK8sResource(existingData) : ''; - const existingDescription = existingData ? getDescriptionFromK8sResource(existingData) : ''; - const existingSize = existingData ? existingData.spec.resources.requests.storage : size; - const existingStorageClassName = existingData?.spec.storageClassName; + const existingName = + formData?.name || (existingData ? getDisplayNameFromK8sResource(existingData) : ''); + const existingDescription = + formData?.description || (existingData ? getDescriptionFromK8sResource(existingData) : ''); + const existingSize = + formData?.size || (existingData ? existingData.spec.resources.requests.storage : size); + const existingStorageClassName = + formData?.storageClassName || existingData?.spec.storageClassName; React.useEffect(() => { if (existingName) { - setCreateData('nameDesc', { - name: existingName, - description: existingDescription, - }); + setCreateData('name', existingName); + setCreateData('description', existingDescription); setCreateData('size', existingSize); setCreateData('storageClassName', existingStorageClassName); } @@ -64,15 +60,13 @@ export const useCreateStorageObjectForNotebook = ( setData: UpdateObjectAtPropAndValue, resetDefaults: () => void, ] => { - const size = useDefaultPvcSize(); + const defaultSize = useDefaultPvcSize(); const createDataState = useGenericObjectState({ - nameDesc: { - name: '', - k8sName: undefined, - description: '', - }, - size, + name: '', + k8sName: undefined, + description: '', + size: defaultSize, forNotebook: { name: '', mountPath: { @@ -87,7 +81,7 @@ export const useCreateStorageObjectForNotebook = ( const existingName = existingData ? getDisplayNameFromK8sResource(existingData) : ''; const existingDescription = existingData ? getDescriptionFromK8sResource(existingData) : ''; - const existingSize = existingData ? existingData.spec.resources.requests.storage : size; + const existingSize = existingData ? existingData.spec.resources.requests.storage : defaultSize; const existingStorageClassName = existingData?.spec.storageClassName; const { notebooks: relatedNotebooks } = useRelatedNotebooks( ConnectedNotebookContext.REMOVABLE_PVC, @@ -98,10 +92,8 @@ export const useCreateStorageObjectForNotebook = ( React.useEffect(() => { if (existingName) { - setCreateData('nameDesc', { - name: existingName, - description: existingDescription, - }); + setCreateData('name', existingName); + setCreateData('description', existingDescription); setCreateData('hasExistingNotebookConnections', hasExistingNotebookConnections); setCreateData('size', existingSize); setCreateData('storageClassName', existingStorageClassName); @@ -131,37 +123,14 @@ export const useExistingStorageDataObjectForNotebook = (): [ }, }); -export const useStorageDataObject = ( - notebook?: NotebookKind, -): [ - data: StorageData, - setData: UpdateObjectAtPropAndValue, - resetDefaults: () => void, -] => { - const size = useDefaultPvcSize(); - return useGenericObjectState({ - storageType: notebook ? StorageType.EXISTING_PVC : StorageType.NEW_PVC, - creating: { - nameDesc: { - name: '', - description: '', - }, - size, - storageClassName: '', - }, - existing: { - storage: getRootVolumeName(notebook), - }, - }); -}; - // Returns the initial mount path format based on the isCreate and mountPath props. export const useMountPathFormat = ( isCreate: boolean, mountPath: string, + initialFormat?: MountPathFormat, ): [MountPathFormat, React.Dispatch>] => { const getInitialFormat = React.useCallback(() => { - if (isCreate) { + if (isCreate && !mountPath) { return MountPathFormat.STANDARD; } return mountPath.startsWith(MOUNT_PATH_PREFIX) @@ -169,7 +138,7 @@ export const useMountPathFormat = ( : MountPathFormat.CUSTOM; }, [isCreate, mountPath]); - const [format, setFormat] = React.useState(getInitialFormat); + const [format, setFormat] = React.useState(initialFormat || getInitialFormat()); React.useEffect(() => { if (!isCreate) { @@ -180,16 +149,19 @@ export const useMountPathFormat = ( } }, [isCreate, mountPath]); - return [format, setFormat] as const; + return [format, setFormat]; }; // Validates the mount path for a storage object. export const validateMountPath = ( value: string, inUseMountPaths: string[], - format: MountPathFormat, + format: MountPathFormat | undefined, ): string => { - if (value.length === 0 && format === MountPathFormat.CUSTOM) { + const standardRootPathExists = + format === MountPathFormat.STANDARD && !value && inUseMountPaths.includes(MOUNT_PATH_PREFIX); + + if (!value.length && format === MountPathFormat.CUSTOM) { return 'Enter a path to a model or folder. This path cannot point to a root folder.'; } @@ -203,8 +175,9 @@ export const validateMountPath = ( return 'Must only consist of lowercase letters, dashes, and slashes.'; } - if (inUseMountPaths.includes(`/${value}`)) { + if (standardRootPathExists || inUseMountPaths.includes(value)) { return 'Mount folder is already in use for this workbench.'; } + return ''; }; diff --git a/frontend/src/pages/projects/types.ts b/frontend/src/pages/projects/types.ts index fce6b5634a..0cc1edc0d9 100644 --- a/frontend/src/pages/projects/types.ts +++ b/frontend/src/pages/projects/types.ts @@ -9,11 +9,12 @@ import { Volume, VolumeMount, } from '~/types'; -import { AWSSecretKind } from '~/k8sTypes'; +import { AWSSecretKind, PersistentVolumeClaimKind } from '~/k8sTypes'; import { AcceleratorProfileState } from '~/utilities/useReadAcceleratorState'; import { K8sNameDescriptionFieldData } from '~/concepts/k8s/K8sNameDescriptionField/types'; import { AcceleratorProfileFormData } from '~/utilities/useAcceleratorProfileFormState'; import { AwsKeys } from './dataConnections/const'; +import { MountPathFormat } from './screens/spawner/storage/types'; export type UpdateObjectAtPropAndValue = ( propKey: K, @@ -26,17 +27,12 @@ export type NameDescType = { description: string; }; -export type CreatingStorageObject = { - nameDesc: NameDescType; - size: string; - storageClassName?: string; -}; - export type MountPath = { /** Suffix to the root path */ value: string; /** Any error with the value */ error: string; + format?: MountPathFormat; }; export type ForNotebookSelection = { @@ -44,15 +40,19 @@ export type ForNotebookSelection = { mountPath: MountPath; }; -export type CreatingStorageObjectForNotebook = CreatingStorageObject & { +export type CreatingStorageObjectForNotebook = NameDescType & { + size: string; forNotebook: ForNotebookSelection; hasExistingNotebookConnections: boolean; + storageClassName?: string; + mountPath?: string; }; export type ExistingStorageObjectForNotebook = ForNotebookSelection; export type ExistingStorageObject = { storage: string; + pvc?: PersistentVolumeClaimKind; }; export enum StorageType { @@ -61,9 +61,15 @@ export enum StorageType { } export type StorageData = { - storageType: StorageType; - creating: CreatingStorageObject; - existing: ExistingStorageObject; + name: string; + size?: string; + storageType?: StorageType; + description?: string; + storageClassName?: string; + mountPath?: string; + existingName?: string; + existingPvc?: PersistentVolumeClaimKind; + id?: number; }; export type StartNotebookData = {
+ + + + + + {!hasUpdatedDefaultNameRef.current && + row.storageType === StorageType.NEW_PVC && + rowIndex === 0 && ( + + + + + + + + )} + + Max {row.size}{row.mountPath} + { + setIsEditModalOpen(true); + setSelectedId(row.id); + }, + }, + { + title: 'Detach', + onClick: () => { + setIsDetachModalOpen(true); + setSelectedId(row.id); + }, + }, + ]} + /> +