From c3ad89094d0b735b160b1ce85a47c76e204de28f Mon Sep 17 00:00:00 2001 From: Jeff Puzzo Date: Wed, 13 Nov 2024 10:39:48 -0500 Subject: [PATCH] [RHOAIENG-1101] Cannot remove additional storage from the workbench configuration --- .../src/__mocks__/mockStartNotebookData.ts | 15 +- .../cypress/cypress/pages/clusterStorage.ts | 8 +- .../cypress/cypress/pages/workbench.ts | 50 ++-- .../mocked/projects/clusterStorage.cy.ts | 92 +------- .../tests/mocked/projects/workbench.cy.ts | 62 ++--- frontend/src/api/k8s/__tests__/pvcs.spec.ts | 10 +- frontend/src/api/k8s/pvcs.ts | 19 +- .../src/concepts/k8s/NameDescriptionField.tsx | 3 + .../NIMServiceModal/ManageNIMServingModal.tsx | 14 +- .../screens/projects/__tests__/utils.spec.ts | 12 +- .../modelServing/screens/projects/utils.ts | 6 +- .../components/NotebookRestartAlert.tsx | 5 +- .../pages/projects/pvc/useNotebookPVCItems.ts | 6 +- .../detail/storage/BaseStorageModal.tsx | 32 ++- .../detail/storage/ClusterStorageModal.tsx | 70 ++---- .../detail/storage/ManageStorageModal.tsx | 213 ------------------ .../screens/detail/storage/StorageTable.tsx | 2 +- .../projects/screens/detail/storage/utils.ts | 13 ++ .../screens/spawner/SpawnerFooter.tsx | 81 +++++-- .../projects/screens/spawner/SpawnerPage.tsx | 174 ++++++++++++-- .../pages/projects/screens/spawner/service.ts | 72 ++---- .../projects/screens/spawner/spawnerUtils.ts | 102 +++------ .../storage/AddExistingStorageField.tsx | 38 +++- .../storage/AttachExistingStorageModal.tsx | 27 ++- .../storage/ClusterStorageDetachModal.tsx | 31 +++ .../storage/ClusterStorageEmptyState.tsx | 22 ++ .../spawner/storage/ClusterStorageTable.tsx | 172 ++++++++++++++ .../storage/CreateNewStorageSection.tsx | 25 +- .../spawner/storage/SpawnerMountPathField.tsx | 99 ++++---- .../spawner/storage/StorageClassSelect.tsx | 5 +- .../screens/spawner/storage/StorageField.tsx | 67 ------ .../spawner/storage/WorkbenchStorageModal.tsx | 65 ++++-- .../spawner/storage/__tests__/utils.spec.ts | 60 ++--- .../screens/spawner/storage/constants.ts | 36 +++ .../screens/spawner/storage/useRelatedPvcs.ts | 0 .../projects/screens/spawner/storage/utils.ts | 109 ++++----- frontend/src/pages/projects/types.ts | 26 ++- 37 files changed, 935 insertions(+), 908 deletions(-) delete mode 100644 frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/storage/ClusterStorageDetachModal.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/storage/ClusterStorageEmptyState.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/storage/ClusterStorageTable.tsx delete mode 100644 frontend/src/pages/projects/screens/spawner/storage/StorageField.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/storage/constants.ts create mode 100644 frontend/src/pages/projects/screens/spawner/storage/useRelatedPvcs.ts 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/clusterStorage.ts b/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts index 9e8139e7ab..f90694e973 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts @@ -46,7 +46,7 @@ class ClusterStorageModal extends Modal { findWorkbenchConnectionSelect() { return this.find() .findByTestId('connect-existing-workbench-group') - .findByRole('button', { name: 'Typeahead menu toggle' }); + .findByRole('button', { name: 'Typeahead menu toggle', hidden: true }); } findMountField() { @@ -79,7 +79,7 @@ class ClusterStorageModal extends Modal { selectPVSize(name: string) { this.findPVSizeSelectButton().click(); - cy.findByRole('menuitem', { name }).click(); + cy.findByRole('menuitem', { name, hidden: true }).click(); } shouldHavePVSizeSelectValue(name: string) { @@ -92,7 +92,7 @@ class ClusterStorageModal extends Modal { } findPVSizeMinusButton() { - return this.findPVSizeField().findByRole('button', { name: 'Minus' }); + return this.findPVSizeField().findByRole('button', { name: 'Minus', hidden: true }); } findPersistentStorageWarningCanNotEdit() { @@ -108,7 +108,7 @@ class ClusterStorageModal extends Modal { } findPVSizePlusButton() { - return this.findPVSizeField().findByRole('button', { name: 'Plus' }); + return this.findPVSizeField().findByRole('button', { name: 'Plus', hidden: true }); } findStorageClassSelect() { 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/clusterStorage.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/clusterStorage.cy.ts index df0b2e7031..697ade7029 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/clusterStorage.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/clusterStorage.cy.ts @@ -158,7 +158,8 @@ describe('ClusterStorage', () => { addClusterStorageModal.find().findByText('openshift-default-sc').should('exist'); // select storage class - addClusterStorageModal.findStorageClassSelect().findSelectOption('Test SC 1').click(); + addClusterStorageModal.findStorageClassSelect().click(); + addClusterStorageModal.find().findByText('Test SC 1').click(); addClusterStorageModal.findSubmitButton().should('be.enabled'); addClusterStorageModal.findDescriptionInput().fill('description'); addClusterStorageModal.findPVSizeMinusButton().click(); @@ -168,10 +169,8 @@ describe('ClusterStorage', () => { addClusterStorageModal.selectPVSize('MiB'); //connect workbench - addClusterStorageModal - .findWorkbenchConnectionSelect() - .findSelectOption('Test Notebook') - .click(); + addClusterStorageModal.findWorkbenchConnectionSelect().click(); + addClusterStorageModal.find().findByText('Test Notebook').click(); // don't allow duplicate path addClusterStorageModal.findMountField().clear(); @@ -274,88 +273,9 @@ describe('ClusterStorage', () => { clusterStorageRow.findKebabAction('Edit storage').click(); // Connect to 'Another Notebook' - updateClusterStorageModal - .findWorkbenchConnectionSelect() - .findSelectOption('Another Notebook') - .click(); - updateClusterStorageModal.findMountField().fill('new-data'); - - cy.interceptK8s('PATCH', NotebookModel, anotherNotebook).as('updateClusterStorage'); - - updateClusterStorageModal.findSubmitButton().click(); - cy.wait('@updateClusterStorage').then((interception) => { - expect(interception.request.url).to.include('?dryRun=All'); - expect(interception.request.body).to.eql([ - { - op: 'add', - path: '/spec/template/spec/volumes/-', - value: { name: 'existing-pvc', persistentVolumeClaim: { claimName: 'existing-pvc' } }, - }, - { - op: 'add', - path: '/spec/template/spec/containers/0/volumeMounts/-', - value: { name: 'existing-pvc', mountPath: '/opt/app-root/src/new-data' }, - }, - ]); - }); - - cy.wait('@updateClusterStorage').then((interception) => { - expect(interception.request.url).not.to.include('?dryRun=All'); - }); - - cy.get('@updateClusterStorage.all').then((interceptions) => { - expect(interceptions).to.have.length(2); - }); - }); - - it('Add cluster storage with multiple workbench connections', () => { - // one notebook already connected to a PVC - const testNotebook = mockNotebookK8sResource({ - displayName: 'Test Notebook', - name: 'test-notebook', - opts: { - spec: { - template: { - spec: { - volumes: [ - { name: 'existing-pvc', persistentVolumeClaim: { claimName: 'existing-pvc' } }, - ], - containers: [ - { - volumeMounts: [{ name: 'existing-pvc', mountPath: '/' }], - }, - ], - }, - }, - }, - }, - }); + updateClusterStorageModal.findWorkbenchConnectionSelect().click(); + updateClusterStorageModal.find().findByText('Another Notebook').click(); - // another notebook not connected to PVC - const anotherNotebook = mockNotebookK8sResource({ - displayName: 'Another Notebook', - name: 'another-notebook', - }); - - initInterceptors({}); - cy.interceptK8sList(NotebookModel, mockK8sResourceList([testNotebook, anotherNotebook])); - - cy.interceptK8sList( - { model: PVCModel, ns: 'test-project' }, - mockK8sResourceList([ - mockPVCK8sResource({ name: 'existing-pvc', displayName: 'Existing PVC' }), - ]), - ); - - clusterStorage.visit('test-project'); - const clusterStorageRow = clusterStorage.getClusterStorageRow('Existing PVC'); - clusterStorageRow.findKebabAction('Edit storage').click(); - - // Connect to 'Another Notebook' - updateClusterStorageModal - .findWorkbenchConnectionSelect() - .findSelectOption('Another Notebook') - .click(); updateClusterStorageModal.findMountField().fill('new-data'); cy.interceptK8s('PATCH', NotebookModel, anotherNotebook).as('updateClusterStorage'); 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 void; }; @@ -50,6 +51,7 @@ const NameDescriptionField: React.FC = ({ maxLengthName, maxLengthDesc, nameHelperText, + hasNameError, onNameChange, }) => { const k8sName = React.useMemo(() => { @@ -81,6 +83,7 @@ const NameDescriptionField: React.FC = ({ : undefined } maxLength={maxLengthName} + validated={hasNameError ? 'error' : 'default'} /> {maxLengthName && } diff --git a/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/ManageNIMServingModal.tsx b/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/ManageNIMServingModal.tsx index b8c52fb215..ac5d634e74 100644 --- a/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/ManageNIMServingModal.tsx +++ b/frontend/src/pages/modelServing/screens/projects/NIMServiceModal/ManageNIMServingModal.tsx @@ -33,11 +33,7 @@ import ServingRuntimeSizeSection from '~/pages/modelServing/screens/projects/Ser import NIMModelListSection from '~/pages/modelServing/screens/projects/NIMServiceModal/NIMModelListSection'; import NIMModelDeploymentNameSection from '~/pages/modelServing/screens/projects/NIMServiceModal/NIMModelDeploymentNameSection'; import ProjectSection from '~/pages/modelServing/screens/projects/InferenceServiceModal/ProjectSection'; -import { - CreatingStorageObject, - DataConnection, - NamespaceApplicationCase, -} from '~/pages/projects/types'; +import { DataConnection, NamespaceApplicationCase } from '~/pages/projects/types'; import { getDisplayNameFromK8sResource, translateDisplayNameForK8s, @@ -246,12 +242,10 @@ const ManageNIMServingModal: React.FC = ({ 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/components/NotebookRestartAlert.tsx b/frontend/src/pages/projects/components/NotebookRestartAlert.tsx index 0ca1af11b8..85119f88df 100644 --- a/frontend/src/pages/projects/components/NotebookRestartAlert.tsx +++ b/frontend/src/pages/projects/components/NotebookRestartAlert.tsx @@ -9,7 +9,10 @@ type NotebookRestartAlertProps = { }; const NotebookRestartAlert: React.FC = ({ notebooks, isCurrent }) => { - const runningNotebooks = notebooks.filter((notebookState) => notebookState.isRunning); + const runningNotebooks = notebooks.filter( + (notebookState) => notebookState.isRunning || notebookState.isStarting, + ); + return ( { 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..120ffd1c27 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,12 +20,16 @@ export type BaseStorageModalProps = { description?: string; children: React.ReactNode; isValid: boolean; - onSubmit: (data: CreatingStorageObject) => Promise; - existingData?: PersistentVolumeClaimKind; + onSubmit: (data: CreateStorageObjectData) => Promise; + hasDuplicateName?: boolean; + onNameChange?: (value: string) => void; + existingData?: CreateStorageObjectData; + existingPvc?: PersistentVolumeClaimKind; onClose: (submitted: boolean) => void; }; const BaseStorageModal: React.FC = ({ + existingPvc, existingData, onSubmit, submitLabel = 'Add storage', @@ -28,16 +37,19 @@ const BaseStorageModal: React.FC = ({ description = 'Add storage and optionally connect it with an existing workbench.', children, isValid, + hasDuplicateName, onClose, + onNameChange, }) => { - const [createData, setCreateData, resetData] = useCreateStorageObject(existingData); + const [createData, setCreateData, resetData] = useCreateStorageObject(existingPvc, existingData); const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status; const preferredStorageClass = usePreferredStorageClass(); const [defaultStorageClass] = useDefaultStorageClass(); const [error, setError] = React.useState(); const [actionInProgress, setActionInProgress] = React.useState(false); + React.useEffect(() => { - if (!existingData) { + if (!existingPvc) { if (isStorageClassesAvailable) { setCreateData('storageClassName', defaultStorageClass?.metadata.name); } else { @@ -48,7 +60,7 @@ const BaseStorageModal: React.FC = ({ isStorageClassesAvailable, defaultStorageClass, preferredStorageClass, - existingData, + existingPvc, setCreateData, ]); @@ -59,7 +71,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); @@ -103,9 +115,11 @@ const BaseStorageModal: React.FC = ({ {children} diff --git a/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx index cae3f2ff28..151ad9d35a 100644 --- a/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx @@ -1,26 +1,26 @@ 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 { ConnectedNotebookContext, useRelatedNotebooks, } from '~/pages/projects/notebook/useRelatedNotebooks'; -import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; import NotebookRestartAlert from '~/pages/projects/components/NotebookRestartAlert'; import StorageNotebookConnections from '~/pages/projects/notebook/StorageNotebookConnections'; import useWillNotebooksRestart from '~/pages/projects/notebook/useWillNotebooksRestart'; import BaseStorageModal from './BaseStorageModal'; import ExistingConnectedNotebooks from './ExistingConnectedNotebooks'; +import { isPvcUpdateRequired } from './utils'; type ClusterStorageModalProps = { - existingData?: PersistentVolumeClaimKind; + existingPvc?: PersistentVolumeClaimKind; onClose: (submit: boolean) => void; }; -const ClusterStorageModal: React.FC = (props) => { +const ClusterStorageModal: React.FC = ({ existingPvc, onClose }) => { const { currentProject } = React.useContext(ProjectDetailsContext); const namespace = currentProject.metadata.name; const [removedNotebooks, setRemovedNotebooks] = React.useState([]); @@ -30,51 +30,40 @@ const ClusterStorageModal: React.FC = (props) => { }); const { notebooks: connectedNotebooks } = useRelatedNotebooks( ConnectedNotebookContext.EXISTING_PVC, - props.existingData?.metadata.name, + existingPvc?.metadata.name, ); - const { notebooks: relatedNotebooks } = useRelatedNotebooks( - ConnectedNotebookContext.REMOVABLE_PVC, - props.existingData?.metadata.name, - ); - - const hasExistingNotebookConnections = relatedNotebooks.length > 0; const { notebooks: removableNotebooks, loaded: removableNotebookLoaded, error: removableNotebookError, - } = useRelatedNotebooks( - ConnectedNotebookContext.REMOVABLE_PVC, - props.existingData?.metadata.name, - ); + } = useRelatedNotebooks(ConnectedNotebookContext.REMOVABLE_PVC, existingPvc?.metadata.name); + + const hasExistingNotebookConnections = removableNotebooks.length > 0; const restartNotebooks = useWillNotebooksRestart([...removedNotebooks, notebookData.name]); const handleSubmit = async ( - storageData: StorageData['creating'], + storageData: StorageData, attachNotebookData?: ForNotebookSelection, dryRun?: boolean, ) => { const promises: Promise[] = []; - const { existingData } = props; - if (existingData) { - const pvcName = existingData.metadata.name; + if (existingPvc) { + const pvcName = existingPvc.metadata.name; // Check if PVC needs to be updated (name, description, size, storageClass) - if ( - getDisplayNameFromK8sResource(existingData) !== storageData.nameDesc.name || - getDescriptionFromK8sResource(existingData) !== storageData.nameDesc.description || - existingData.spec.resources.requests.storage !== storageData.size || - existingData.spec.storageClassName !== storageData.storageClassName - ) { - promises.push(updatePvc(storageData, existingData, namespace, { dryRun })); + if (isPvcUpdateRequired(existingPvc, storageData)) { + promises.push(updatePvc(storageData, existingPvc, namespace, { dryRun })); } // Restart notebooks if the PVC size has changed - if (existingData.spec.resources.requests.storage !== storageData.size) { - connectedNotebooks.map((connectedNotebook) => - promises.push(restartNotebook(connectedNotebook.metadata.name, namespace, { dryRun })), + if (existingPvc.spec.resources.requests.storage !== storageData.size) { + restartNotebooks.map((connectedNotebook) => + promises.push( + restartNotebook(connectedNotebook.notebook.metadata.name, namespace, { dryRun }), + ), ); } @@ -115,17 +104,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 @@ -135,17 +115,17 @@ const ClusterStorageModal: React.FC = (props) => { return ( submit(data)} - title={props.existingData ? 'Update cluster storage' : 'Add cluster storage'} + title={existingPvc ? 'Update cluster storage' : 'Add cluster storage'} description={ - props.existingData + existingPvc ? 'Make changes to cluster storage, or connect it to additional workspaces.' : 'Add storage and optionally connect it with an existing workbench.' } - submitLabel={props.existingData ? 'Update' : 'Add storage'} + submitLabel={existingPvc ? 'Update' : 'Add storage'} isValid={isValid} - onClose={(submitted) => props.onClose(submitted)} + onClose={(submitted) => onClose(submitted)} + existingPvc={existingPvc} > <> {hasExistingNotebookConnections && ( 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/detail/storage/StorageTable.tsx b/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx index c1e888ced0..8f087e1678 100644 --- a/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx @@ -87,7 +87,7 @@ const StorageTable: React.FC = ({ pvcs, refresh, onAddPVC }) /> {editPVC ? ( { if (updated) { refresh(); diff --git a/frontend/src/pages/projects/screens/detail/storage/utils.ts b/frontend/src/pages/projects/screens/detail/storage/utils.ts index a29e9ea1be..3af3ceaaf2 100644 --- a/frontend/src/pages/projects/screens/detail/storage/utils.ts +++ b/frontend/src/pages/projects/screens/detail/storage/utils.ts @@ -1,3 +1,7 @@ +import { getDisplayNameFromK8sResource, getDescriptionFromK8sResource } from '~/concepts/k8s/utils'; +import { PersistentVolumeClaimKind } from '~/k8sTypes'; +import { StorageData } from '~/pages/projects/types'; + type Status = 'error' | 'warning' | 'info' | null; export const getFullStatusFromPercentage = (percentageFull: number): Status => { if (percentageFull === 100) { @@ -11,3 +15,12 @@ export const getFullStatusFromPercentage = (percentageFull: number): Status => { } return null; }; + +export const isPvcUpdateRequired = ( + existingPvc: PersistentVolumeClaimKind, + storageData: StorageData, +): boolean => + getDisplayNameFromK8sResource(existingPvc) !== storageData.name || + getDescriptionFromK8sResource(existingPvc) !== storageData.description || + existingPvc.spec.resources.requests.storage !== storageData.size || + existingPvc.spec.storageClassName !== storageData.storageClassName; diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx index 5222f17383..9924c278e0 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx @@ -9,7 +9,13 @@ import { Stack, StackItem, } from '@patternfly/react-core'; -import { createNotebook, K8sStatusError, mergePatchUpdateNotebook, updateNotebook } from '~/api'; +import { + createNotebook, + K8sStatusError, + mergePatchUpdateNotebook, + restartNotebook, + updateNotebook, +} from '~/api'; import { DataConnectionData, EnvVariable, @@ -26,19 +32,21 @@ import { FormTrackingEventProperties, TrackingOutcome, } from '~/concepts/analyticsTracking/trackingProperties'; +import { NotebookKind } from '~/k8sTypes'; +import { getNotebookPVCNames } from '~/pages/projects/pvc/utils'; 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; @@ -78,17 +86,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 = { @@ -109,8 +113,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, @@ -140,17 +142,47 @@ const SpawnerFooter: React.FC = ({ setCreateInProgress(true); }; + const getPvcRequests = (dryRun: boolean) => { + const restartConnectedNotebooksPromises: Promise[] = []; + + const pvcRequests = storageData.map((pvcData) => { + if (pvcData.existingPvc) { + // Restart connected notebooks if the PVC size has changed + if (pvcData.existingPvc.spec.resources.requests.storage !== pvcData.size) { + data + .filter( + (nbs) => + (nbs.isRunning || nbs.isStarting) && + getNotebookPVCNames(nbs.notebook).includes( + pvcData.existingPvc?.metadata.name || '', + ), + ) + .map((connectedNotebook) => + restartConnectedNotebooksPromises.push( + restartNotebook(connectedNotebook.notebook.metadata.name, projectName, { dryRun }), + ), + ); + } + return updatePvcDataForNotebook(projectName, pvcData, pvcData.existingPvc, dryRun); + } + + return createPvcDataForNotebook(projectName, pvcData, dryRun); + }); + + return { pvcRequests, restartConnectedNotebooksPromises }; + }; + const updateNotebookPromise = async (dryRun: boolean) => { if (!editNotebook) { return; } - const pvcDetails = await replaceRootVolumesForNotebook( - projectName, - editNotebook, - storageData, - dryRun, - ); + const { pvcRequests, restartConnectedNotebooksPromises } = getPvcRequests(dryRun); + + const pvcResponses = await Promise.all(pvcRequests); + const pvcVolumeDetails = getPvcVolumeDetails(pvcResponses); + + await Promise.all(restartConnectedNotebooksPromises); let envFrom = await updateConfigMapsAndSecretsForNotebook( projectName, @@ -171,7 +203,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, @@ -209,14 +241,21 @@ const SpawnerFooter: React.FC = ({ dataConnection.enabled && dataConnection.type === 'existing' && dataConnection.existing ? [dataConnection.existing] : []; - const pvcDetails = await createPvcDataForNotebook(projectName, storageData, dryRun); + + const { pvcRequests, restartConnectedNotebooksPromises } = getPvcRequests(dryRun); + + const pvcResponses = await Promise.all(pvcRequests); + const pvcVolumeDetails = getPvcVolumeDetails(pvcResponses); + + await Promise.all(restartConnectedNotebooksPromises); + 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..b4b41dd940 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,17 @@ 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 { getNotebookPVCNames } from '~/pages/projects/pvc/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 +52,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; @@ -58,6 +68,7 @@ const SpawnerPage: React.FC = ({ existingNotebook }) => { currentProject, dataConnections, connections: { data: projectConnections, refresh: refreshProjectConnections }, + notebooks: { data: notebooks }, } = React.useContext(ProjectDetailsContext); const displayName = getDisplayNameFromK8sResource(currentProject); @@ -66,6 +77,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,19 +87,46 @@ 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 existingStorageNames = storageData.map((storageDataEntry) => storageDataEntry.name); const [dataConnectionData, setDataConnectionData] = useNotebookDataConnection( dataConnections.data, @@ -106,7 +146,24 @@ const SpawnerPage: React.FC = ({ existingNotebook }) => { dataConnectionData.existing?.secretRef.name || '', ]); - const restartNotebooks = useWillNotebooksRestart([existingNotebook?.metadata.name || '']); + const notebooksUsingPVCsWithSizeChanges = React.useMemo(() => { + const attachedPVCs = storageData.filter((storage) => storage.existingPvc !== undefined); + + return attachedPVCs.flatMap((storage) => + notebooks + .filter( + ({ notebook }) => + getNotebookPVCNames(notebook).includes(storage.existingPvc?.metadata.name || '') && + storage.existingPvc?.spec.resources.requests.storage !== storage.size, + ) + .map(({ notebook }) => notebook.metadata.name), + ); + }, [storageData, notebooks]); + + const restartNotebooks = useWillNotebooksRestart([ + existingNotebook?.metadata.name || '', + ...notebooksUsingPVCsWithSizeChanges, + ]); const [data, loaded, loadError] = useNotebookImageData(existingNotebook); React.useEffect(() => { @@ -225,19 +282,46 @@ const SpawnerPage: React.FC = ({ existingNotebook }) => { + + {SpawnerPageSectionTitles[SpawnerPageSectionID.CLUSTER_STORAGE]} + + + + + + + } id={SpawnerPageSectionID.CLUSTER_STORAGE} aria-label={SpawnerPageSectionTitles[SpawnerPageSectionID.CLUSTER_STORAGE]} > - - + {storageData.length ? ( + ({ ...formData, id: index }))} + existingStorageNames={existingStorageNames} + existingMountPaths={existingMountPaths} + setStorageData={setStorageData} + workbenchName={k8sNameDescriptionData.data.k8sName.value} + /> + ) : ( + + )} {isConnectionTypesEnabled ? ( = ({ existingNotebook }) => { {restartNotebooks.length !== 0 && ( - + )} @@ -299,6 +383,52 @@ const SpawnerPage: React.FC = ({ 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, + }, + ]), + ) + } + existingStorageNames={existingStorageNames} + 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..3d4fb92385 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,9 @@ 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 { isPvcUpdateRequired } from '~/pages/projects/screens/detail/storage/utils'; import { fetchNotebookEnvVariables } from './environmentVariables/useNotebookEnvVariables'; import { getDeletedConfigMapOrSecretVariables } from './environmentVariables/utils'; @@ -34,68 +35,39 @@ 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) { + if (isPvcUpdateRequired(existingPvc, storageData)) { + await updatePvc(storageData, existingPvc, projectName, { dryRun }); + } + const { name } = existingPvc.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..9c38777f50 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/AddExistingStorageField.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/AddExistingStorageField.tsx @@ -3,7 +3,7 @@ import { Alert, FormGroup } from '@patternfly/react-core'; import { ExistingStorageObject } from '~/pages/projects/types'; import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; -import TypeaheadSelect from '~/components/TypeaheadSelect'; +import TypeaheadSelect, { TypeaheadSelectOption } from '~/components/TypeaheadSelect'; import useProjectPvcs from '~/pages/projects/screens/detail/storage/useProjectPvcs'; type AddExistingStorageFieldProps = { @@ -11,6 +11,7 @@ type AddExistingStorageFieldProps = { setData: (data: ExistingStorageObject) => void; selectDirection?: 'up' | 'down'; menuAppendTo?: HTMLElement; + existingStorageNames?: string[]; }; const AddExistingStorageField: React.FC = ({ @@ -18,6 +19,7 @@ const AddExistingStorageField: React.FC = ({ setData, selectDirection, menuAppendTo, + existingStorageNames, }) => { const { currentProject } = React.useContext(ProjectDetailsContext); const [storages, loaded, loadError] = useProjectPvcs(currentProject.metadata.name); @@ -32,16 +34,22 @@ const AddExistingStorageField: React.FC = ({ const selectOptions = React.useMemo( () => loaded - ? storages.map((pvc) => ({ - value: pvc.metadata.name, - content: getDisplayNameFromK8sResource(pvc), - description: selectDescription( - pvc.spec.resources.requests.storage, - pvc.metadata.annotations?.['openshift.io/description'], - ), - })) + ? storages.reduce((acc: TypeaheadSelectOption[], pvc) => { + if (!existingStorageNames?.includes(pvc.metadata.name)) { + acc.push({ + value: pvc.metadata.name, + content: getDisplayNameFromK8sResource(pvc), + description: selectDescription( + pvc.spec.resources.requests.storage, + pvc.metadata.annotations?.['openshift.io/description'], + ), + }); + } + + return acc; + }, []) : [], - [loaded, storages], + [existingStorageNames, loaded, storages], ); if (loadError) { @@ -72,7 +80,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..5725116c20 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/AttachExistingStorageModal.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/AttachExistingStorageModal.tsx @@ -4,21 +4,29 @@ import { ExistingStorageObject, MountPath } from '~/pages/projects/types'; import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter'; import SpawnerMountPathField from './SpawnerMountPathField'; import AddExistingStorageField from './AddExistingStorageField'; +import { MOUNT_PATH_PREFIX } from './const'; type AttachExistingStorageModalData = ExistingStorageObject & { mountPath: MountPath; }; type AttachExistingStorageModalProps = { + existingMountPaths: string[]; + existingStorageNames: string[]; onClose: (submit: boolean, storageData?: AttachExistingStorageModalData) => void; }; const initialState: AttachExistingStorageModalData = { storage: '', - mountPath: { value: '', error: '' }, + pvc: undefined, + mountPath: { value: MOUNT_PATH_PREFIX, error: '' }, }; -const AttachExistingStorageModal: React.FC = ({ onClose }) => { +const AttachExistingStorageModal: React.FC = ({ + existingMountPaths, + existingStorageNames, + onClose, +}) => { const [data, setData] = React.useState(initialState); const onBeforeClose = (submitted: boolean, storageData?: AttachExistingStorageModalData) => { @@ -26,12 +34,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 +61,16 @@ const AttachExistingStorageModal: React.FC = ({ setData({ ...data, storage: storageName.storage })} + setData={(existingStorage) => + setData({ ...data, storage: existingStorage.storage, pvc: existingStorage.pvc }) + } + existingStorageNames={existingStorageNames} /> 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..6e91919019 --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/storage/ClusterStorageTable.tsx @@ -0,0 +1,172 @@ +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; + existingStorageNames: string[]; + existingMountPaths: string[]; + setStorageData: React.Dispatch>; +} + +export const ClusterStorageTable: React.FC = ({ + storageData, + workbenchName, + existingStorageNames, + 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); + }} + existingStorageNames={existingStorageNames.filter( + (storageName) => storageName !== selectedStorage.name, + )} + existingMountPaths={existingMountPaths.filter( + (mountPath) => mountPath !== selectedStorage.mountPath, + )} + /> + )} + + {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..79a461354d 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/CreateNewStorageSection.tsx @@ -1,28 +1,33 @@ 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 { DuplicateNameHelperText } from '~/concepts/pipelines/content/DuplicateNameHelperText'; import StorageClassSelect from './StorageClassSelect'; -type CreateNewStorageSectionProps = { +type CreateNewStorageSectionProps = { data: D; setData: UpdateObjectAtPropAndValue; currentStatus?: PersistentVolumeClaimKind['status']; autoFocusName?: boolean; menuAppendTo?: HTMLElement; disableStorageClassSelect?: boolean; + onNameChange?: (value: string) => void; + hasDuplicateName?: boolean; }; -const CreateNewStorageSection = ({ +const CreateNewStorageSection = ({ data, setData, currentStatus, menuAppendTo, autoFocusName, disableStorageClassSelect, + onNameChange, + hasDuplicateName, }: CreateNewStorageSectionProps): React.ReactNode => { const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status; @@ -32,8 +37,16 @@ const CreateNewStorageSection = ({ setData('nameDesc', newData)} + data={{ name: data.name, description: data.description || '' }} + setData={(newData) => { + setData('name', newData.name); + setData('description', newData.description); + }} + onNameChange={onNameChange} + hasNameError={hasDuplicateName} + nameHelperText={ + hasDuplicateName ? : undefined + } autoFocusName={autoFocusName} /> @@ -52,7 +65,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..32bbe62982 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, { useCallback } from 'react'; + import { Flex, FlexItem, @@ -12,75 +14,59 @@ import { StackItem, TextInput, } from '@patternfly/react-core'; -import React from 'react'; + import FieldGroupHelpLabelIcon from '~/components/FieldGroupHelpLabelIcon'; -import { useMountPathFormat, validateMountPath } from './utils'; +import { MountPath } from '~/pages/projects/types'; +import { validateMountPath } from './utils'; import { MountPathFormat } from './types'; import { MOUNT_PATH_PREFIX } from './const'; -interface MountPath { - value: string; - error: string; -} - interface SpawnerMountPathFieldProps { - isCreate: boolean; + isDisabled?: boolean; mountPath: MountPath; inUseMountPaths?: string[]; onChange: (path: MountPath) => void; } const SpawnerMountPathField: React.FC = ({ - isCreate, + isDisabled = false, mountPath, - inUseMountPaths, + inUseMountPaths = [], onChange, }) => { - const [format, setFormat] = useMountPathFormat(isCreate, mountPath.value); - const [shouldShowValidation, setShouldShowValidation] = React.useState(false); - - const pathSuffix = React.useMemo(() => { - const prefix = format === MountPathFormat.STANDARD ? MOUNT_PATH_PREFIX : '/'; - return mountPath.value.startsWith(prefix) - ? mountPath.value.slice(prefix.length) - : mountPath.value; - }, [mountPath.value, format]); + const initialValidationRef = React.useRef(false); - React.useEffect(() => { - const initialValue = format === MountPathFormat.STANDARD ? MOUNT_PATH_PREFIX : '/'; - onChange({ value: initialValue, error: '' }); - // Only run on initial mount - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + const suffix = mountPath.value.startsWith(MOUNT_PATH_PREFIX) + ? mountPath.value.slice(MOUNT_PATH_PREFIX.length) + : mountPath.value.slice(1); - const validateAndUpdate = React.useCallback( - (suffix: string, newFormat: MountPathFormat = format) => { - const prefix = newFormat === MountPathFormat.STANDARD ? MOUNT_PATH_PREFIX : '/'; - const newValue = `${prefix}${suffix}`; + const format = mountPath.value.startsWith(MOUNT_PATH_PREFIX) + ? MountPathFormat.STANDARD + : MountPathFormat.CUSTOM; - // Only validate after the field has been touched - if (!shouldShowValidation && !suffix.trim()) { - onChange({ value: newValue, error: '' }); - return; - } + const validateAndUpdate = useCallback( + (value: string, newFormat?: MountPathFormat) => { + const prefix = (newFormat ?? format) === MountPathFormat.STANDARD ? MOUNT_PATH_PREFIX : '/'; + const newValue = `${prefix}${value}`; - const error = validateMountPath(suffix, inUseMountPaths || [], newFormat); - onChange({ value: newValue, error }); + onChange({ + value: newValue, + error: validateMountPath(newValue, inUseMountPaths) || '', + }); }, - [format, inUseMountPaths, onChange, shouldShowValidation], + [inUseMountPaths, format, onChange], ); const handleFormatChange = (newFormat: MountPathFormat) => { - setFormat(newFormat); - validateAndUpdate(pathSuffix, newFormat); + validateAndUpdate(suffix, newFormat); }; - const handleSuffixChange = (suffix: string) => { - if (!shouldShowValidation) { - setShouldShowValidation(true); + React.useEffect(() => { + if (!initialValidationRef.current) { + initialValidationRef.current = true; + validateAndUpdate(suffix); } - validateAndUpdate(suffix); - }; + }, [suffix, validateAndUpdate]); return ( = ({ } > - {isCreate ? ( + {!isDisabled ? ( <> @@ -137,23 +123,26 @@ const SpawnerMountPathField: React.FC = ({ data-testid="mount-path-folder-value" aria-label="Mount path suffix" type="text" - value={pathSuffix} - onChange={(_, value) => handleSuffixChange(value)} + value={suffix} + onChange={(_, value) => validateAndUpdate(value)} isRequired validated={ - mountPath.error ? 'error' : pathSuffix.length > 0 ? 'success' : 'default' + mountPath.error ? 'error' : suffix.length > 0 ? 'success' : 'default' } + onBlur={() => validateAndUpdate(suffix)} /> - - {mountPath.error || - 'Must only consist of lowercase letters, dashes, and slashes.'} - + {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..41fae36a55 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/WorkbenchStorageModal.tsx @@ -1,47 +1,70 @@ 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'; +import { MOUNT_PATH_PREFIX } from './const'; type WorkbenchStorageModalProps = { - existingData?: PersistentVolumeClaimKind; + onSubmit: (storageData: StorageData) => void; onClose: (submit: boolean) => void; + existingStorageNames: string[]; + existingMountPaths: string[]; + formData?: StorageData; }; -const WorkbenchStorageModal: React.FC = (props) => { +const WorkbenchStorageModal: React.FC = ({ + formData, + existingStorageNames, + existingMountPaths, + onSubmit, + onClose, +}) => { + const existingMountPath = formData?.mountPath; const [mountPath, setMountPath] = React.useState({ - value: '', + value: existingMountPath ?? MOUNT_PATH_PREFIX, error: '', }); const [actionInProgress, setActionInProgress] = React.useState(false); + const [hasDuplicateName, setHasDuplicateName] = 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} + existingData={formData} + existingPvc={formData?.existingPvc} + hasDuplicateName={hasDuplicateName} + onSubmit={handleSubmit} + onNameChange={(newName) => { + if (existingStorageNames.includes(newName)) { + setHasDuplicateName(true); + } else { + setHasDuplicateName(false); + } + }} + title={formData ? 'Edit storage' : 'Create storage'} + submitLabel={formData ? 'Save' : 'Create'} + isValid={!actionInProgress && !mountPath.error && !hasDuplicateName} + 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..5b86f3b132 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(); }); @@ -83,61 +84,56 @@ describe('validateMountPath', () => { const inUseMountPaths = ['/existing-folder', '/another-folder']; it('should return error message for empty value in CUSTOM format', () => { - const result = validateMountPath('', inUseMountPaths, MountPathFormat.CUSTOM); + const result = validateMountPath('', inUseMountPaths); expect(result).toBe( 'Enter a path to a model or folder. This path cannot point to a root folder.', ); }); - it('should not return an error for empty value in STANDARD format', () => { - const result = validateMountPath('', inUseMountPaths, MountPathFormat.STANDARD); - expect(result).toBe(''); + it('should return an error for empty value in STANDARD format', () => { + const result = validateMountPath('', inUseMountPaths); + expect(result).toBe( + 'Enter a path to a model or folder. This path cannot point to a root folder.', + ); }); it('should return error message for invalid characters in the path', () => { - const result = validateMountPath('Invalid/Path', inUseMountPaths, MountPathFormat.STANDARD); + const result = validateMountPath('Invalid/Path', inUseMountPaths); expect(result).toBe('Must only consist of lowercase letters, dashes, and slashes.'); }); 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); expect(result).toBe('Mount folder is already in use for this workbench.'); }); it('should return an empty string for valid and unused mount path', () => { - const result = validateMountPath('new-folder', inUseMountPaths, MountPathFormat.STANDARD); + const result = validateMountPath('new-folder', inUseMountPaths); expect(result).toBe(''); }); it('should allow valid folder name with a trailing slash', () => { - const result = validateMountPath('valid-folder/', inUseMountPaths, MountPathFormat.STANDARD); + const result = validateMountPath('valid-folder/', inUseMountPaths); expect(result).toBe(''); }); it('should return error for an invalid folder name with numbers or uppercase letters', () => { - const result = validateMountPath('Invalid123', inUseMountPaths, MountPathFormat.STANDARD); + const result = validateMountPath('Invalid123', inUseMountPaths); expect(result).toBe('Must only consist of lowercase letters, dashes, and slashes.'); }); it('should return an empty string for valid mount path in CUSTOM format', () => { - const result = validateMountPath('custom-folder', inUseMountPaths, MountPathFormat.CUSTOM); + const result = validateMountPath('custom-folder', inUseMountPaths); expect(result).toBe(''); }); it('should return error for an invalid folder name with uppercase letters in CUSTOM format', () => { - const result = validateMountPath('InvalidFolder', inUseMountPaths, MountPathFormat.CUSTOM); + const result = validateMountPath('InvalidFolder', inUseMountPaths); expect(result).toBe('Must only consist of lowercase letters, dashes, and slashes.'); }); }); 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 +167,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..75a5380397 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,16 @@ 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, -): string => { - if (value.length === 0 && format === MountPathFormat.CUSTOM) { +export const validateMountPath = (value: string, inUseMountPaths: string[]): string => { + const format = value.startsWith(MOUNT_PATH_PREFIX) + ? MountPathFormat.STANDARD + : MountPathFormat.CUSTOM; + + 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 +172,14 @@ export const validateMountPath = ( return 'Must only consist of lowercase letters, dashes, and slashes.'; } - if (inUseMountPaths.includes(`/${value}`)) { + if ( + inUseMountPaths.includes(value) || + (format === MountPathFormat.STANDARD && + inUseMountPaths.includes(`${MOUNT_PATH_PREFIX}${value}`)) || + (format === MountPathFormat.CUSTOM && 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..3021cc8b92 100644 --- a/frontend/src/pages/projects/types.ts +++ b/frontend/src/pages/projects/types.ts @@ -9,7 +9,7 @@ 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'; @@ -26,12 +26,6 @@ export type NameDescType = { description: string; }; -export type CreatingStorageObject = { - nameDesc: NameDescType; - size: string; - storageClassName?: string; -}; - export type MountPath = { /** Suffix to the root path */ value: string; @@ -44,15 +38,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 +59,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); + }, + }, + ]} + /> +