diff --git a/frontend/src/__mocks__/mockNotebookK8sResource.ts b/frontend/src/__mocks__/mockNotebookK8sResource.ts index 3235e20141..966519944e 100644 --- a/frontend/src/__mocks__/mockNotebookK8sResource.ts +++ b/frontend/src/__mocks__/mockNotebookK8sResource.ts @@ -1,7 +1,13 @@ import _ from 'lodash-es'; import { KnownLabels, NotebookKind } from '~/k8sTypes'; import { DEFAULT_NOTEBOOK_SIZES } from '~/pages/projects/screens/spawner/const'; -import { ContainerResources, TolerationEffect, TolerationOperator, VolumeMount } from '~/types'; +import { + ContainerResources, + TolerationEffect, + TolerationOperator, + Volume, + VolumeMount, +} from '~/types'; import { genUID } from '~/__mocks__/mockUtils'; import { RecursivePartial } from '~/typeHelpers'; import { EnvironmentFromVariable } from '~/pages/projects/types'; @@ -19,6 +25,7 @@ type MockResourceConfigType = { opts?: RecursivePartial; uid?: string; additionalVolumeMounts?: VolumeMount[]; + additionalVolumes?: Volume[]; }; export const mockNotebookK8sResource = ({ @@ -41,6 +48,7 @@ export const mockNotebookK8sResource = ({ opts = {}, uid = genUID('notebook'), additionalVolumeMounts = [], + additionalVolumes = [], }: MockResourceConfigType): NotebookKind => _.merge( { @@ -257,6 +265,7 @@ export const mockNotebookK8sResource = ({ secretName: 'workbench-tls', }, }, + ...additionalVolumes, ], }, }, diff --git a/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts b/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts index d87255a01e..d6df9eedbc 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts @@ -8,7 +8,7 @@ class ClusterStorageRow extends TableRow { } findConnectedWorkbenches() { - return this.find().find('[data-label="Connected workbenches"]'); + return this.find().find('[data-label="Workbench connections"]'); } toggleExpandableContent() { @@ -55,14 +55,35 @@ class ClusterStorageModal extends Modal { super(edit ? 'Update cluster storage' : 'Add cluster storage'); } - findWorkbenchConnectionSelect() { - return this.find() - .findByTestId('connect-existing-workbench-group') - .findByRole('button', { name: 'Typeahead menu toggle', hidden: true }); + findAddWorkbenchButton() { + return cy.findByTestId('add-workbench-button'); } - findMountField() { - return this.find().findByTestId('mount-path-folder-value'); + findWorkbenchTable() { + return cy.findByTestId('workbench-connection-table'); + } + + findWorkbenchRow(row: number) { + return this.findWorkbenchTable().find('tbody').eq(row).find('tr'); + } + + selectWorkbenchName(row: number, name: string) { + this.findWorkbenchRow(row).find(`[data-label=Name]`).find('button').click(); + this.findWorkbenchTable().findByRole('option', { name, hidden: true }).click(); + } + + selectCustomPathFormat(row: number) { + this.findWorkbenchRow(row).find(`[data-label="Path format"]`).find('button').click(); + this.findWorkbenchTable() + .findByRole('option', { + name: 'Custom Custom paths that do not begin with /opt/app-root/src/ are not visible in the JupyterLab file browser.', + hidden: true, + }) + .click(); + } + + findMountPathField(row: number) { + return this.findWorkbenchRow(row).find(`[data-label="Mount path"]`).find('input'); } findMountFieldHelperText() { 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 33dddf5e95..668225a7be 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 @@ -43,6 +43,7 @@ const initInterceptors = ({ isEmpty = false, storageClassName }: HandlersProps) code: 200, response: mockPrometheusQueryResponse({}), }); + cy.interceptK8s('GET', NotebookModel, mockNotebookK8sResource({})); cy.interceptK8sList( { model: PVCModel, ns: 'test-project' }, mockK8sResourceList( @@ -69,6 +70,14 @@ const initInterceptors = ({ isEmpty = false, storageClassName }: HandlersProps) name: 'test-dupe-pvc-path', }, ], + additionalVolumes: [ + { + name: 'test-dupe-pvc-path', + persistentVolumeClaim: { + claimName: 'test-dupe-pvc-path', + }, + }, + ], }), ]), ); @@ -174,38 +183,22 @@ describe('ClusterStorage', () => { addClusterStorageModal.selectPVSize('MiB'); //connect workbench - addClusterStorageModal.findWorkbenchConnectionSelect().click(); - addClusterStorageModal.find().findByText('Test Notebook').click(); - - // don't allow duplicate path - addClusterStorageModal.findMountField().clear(); - addClusterStorageModal.findMountField().fill('test-dupe'); - addClusterStorageModal - .findMountFieldHelperText() - .should('have.text', 'Mount folder is already in use for this workbench.'); - - // don't allow number in the path - addClusterStorageModal.findMountField().clear(); - addClusterStorageModal.findMountField().fill('test2'); - addClusterStorageModal - .findMountFieldHelperText() - .should('have.text', 'Must only consist of lowercase letters and dashes.'); + addClusterStorageModal.findAddWorkbenchButton().click(); + addClusterStorageModal.findWorkbenchTable().should('exist'); + addClusterStorageModal.selectWorkbenchName(0, 'test-notebook'); - // Allow trailing slash - addClusterStorageModal.findMountField().clear(); - addClusterStorageModal.findMountField().fill('test/'); + //don't allow duplicate path + addClusterStorageModal.findMountPathField(0).fill('test-dupe'); addClusterStorageModal .findMountFieldHelperText() - .should('have.text', 'Must consist of lowercase letters and dashes.'); + .should('contain.text', 'This path is already connected to this workbench'); - addClusterStorageModal.findMountField().clear(); + addClusterStorageModal.findMountPathField(0).clear(); + addClusterStorageModal.selectCustomPathFormat(0); addClusterStorageModal .findMountFieldHelperText() - .should( - 'have.text', - 'Enter a path to a model or folder. This path cannot point to a root folder.', - ); - addClusterStorageModal.findMountField().fill('data'); + .should('contain.text', 'Enter a path to a model or folder.'); + addClusterStorageModal.findMountPathField(0).fill('data'); addClusterStorageModal.findWorkbenchRestartAlert().should('exist'); cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})).as('addClusterStorage'); @@ -220,7 +213,7 @@ describe('ClusterStorage', () => { { op: 'add', path: '/spec/template/spec/containers/0/volumeMounts/-', - value: { mountPath: '/opt/app-root/src/data' }, + value: { mountPath: 'data' }, }, ]); }); @@ -248,7 +241,7 @@ describe('ClusterStorage', () => { ], containers: [ { - volumeMounts: [{ name: 'existing-pvc', mountPath: '/' }], + volumeMounts: [{ name: 'existing-pvc', mountPath: '/opt/app-root/src' }], }, ], }, @@ -276,13 +269,15 @@ describe('ClusterStorage', () => { clusterStorage.visit('test-project'); const clusterStorageRow = clusterStorage.getClusterStorageRow('Existing PVC'); clusterStorageRow.findKebabAction('Edit storage').click(); - + updateClusterStorageModal.findAddWorkbenchButton().click(); + updateClusterStorageModal.selectWorkbenchName(1, 'another-notebook'); + updateClusterStorageModal.findMountPathField(1).fill('new-data'); // Connect to 'Another Notebook' - updateClusterStorageModal.findWorkbenchConnectionSelect().click(); - updateClusterStorageModal.find().findByText('Another Notebook').click(); - - updateClusterStorageModal.findMountField().fill('new-data'); + // updateClusterStorageModal.findWorkbenchConnectionSelect().click(); + // updateClusterStorageModal.find().findByText('Another Notebook').click(); + // updateClusterStorageModal.findMountField().fill('new-data'); + cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})); cy.interceptK8s('PATCH', NotebookModel, anotherNotebook).as('updateClusterStorage'); updateClusterStorageModal.findSubmitButton().click(); diff --git a/frontend/src/api/k8s/__tests__/notebooks.spec.ts b/frontend/src/api/k8s/__tests__/notebooks.spec.ts index cb51f1a3c6..6441cd6972 100644 --- a/frontend/src/api/k8s/__tests__/notebooks.spec.ts +++ b/frontend/src/api/k8s/__tests__/notebooks.spec.ts @@ -696,7 +696,7 @@ describe('attachNotebookPVC', () => { const namespace = 'test-project'; const notebookName = 'test-notebook'; const pvcName = 'test-pvc'; - const mountSuffix = 'data'; + const mountSuffix = '/opt/app-root/src/data'; const mockNotebook = mockNotebookK8sResource({ uid }); k8sPatchResourceMock.mockResolvedValue(mockNotebook); @@ -728,7 +728,7 @@ describe('attachNotebookPVC', () => { const namespace = 'test-project'; const notebookName = 'test-notebook'; const pvcName = 'test-pvc'; - const mountSuffix = 'data'; + const mountSuffix = '/opt/app-root/src/data'; k8sPatchResourceMock.mockRejectedValue(new Error('error1')); await expect(attachNotebookPVC(notebookName, namespace, pvcName, mountSuffix)).rejects.toThrow( diff --git a/frontend/src/api/k8s/notebooks.ts b/frontend/src/api/k8s/notebooks.ts index 0e5e5a608c..523911f340 100644 --- a/frontend/src/api/k8s/notebooks.ts +++ b/frontend/src/api/k8s/notebooks.ts @@ -409,7 +409,7 @@ export const attachNotebookPVC = ( notebookName: string, namespace: string, pvcName: string, - mountSuffix: string, + mountPath: string, opts?: K8sAPIOptions, ): Promise => { const patches: Patch[] = [ @@ -422,7 +422,7 @@ export const attachNotebookPVC = ( op: 'add', // TODO: can we assume first container? path: '/spec/template/spec/containers/0/volumeMounts/-', - value: { mountPath: `${ROOT_MOUNT_PATH}/${mountSuffix}`, name: pvcName }, + value: { mountPath, name: pvcName }, }, ]; @@ -438,6 +438,56 @@ export const attachNotebookPVC = ( ); }; +export const UpdateNotebookPVC = ( + notebookName: string, + namespace: string, + mountPath: string, + pvcName: string, + opts?: K8sAPIOptions, +): Promise => + new Promise((resolve, reject) => { + getNotebook(notebookName, namespace) + .then((notebook) => { + const volumes = notebook.spec.template.spec.volumes || []; + const volumeMounts = notebook.spec.template.spec.containers[0].volumeMounts || []; + + const filteredVolumeMounts = volumeMounts.map((volumeMount) => { + if (volumeMount.name === pvcName) { + return { ...volumeMount, mountPath }; + } + return volumeMount; + }); + + const patches: Patch[] = [ + { + op: 'replace', + path: '/spec/template/spec/volumes', + value: volumes, + }, + { + op: 'replace', + // TODO: can we assume first container? + path: '/spec/template/spec/containers/0/volumeMounts', + value: filteredVolumeMounts, + }, + ]; + + k8sPatchResource( + applyK8sAPIOptions( + { + model: NotebookModel, + queryOptions: { name: notebookName, ns: namespace }, + patches, + }, + opts, + ), + ) + .then(resolve) + .catch(reject); + }) + .catch(reject); + }); + export const removeNotebookPVC = ( notebookName: string, namespace: string, diff --git a/frontend/src/pages/projects/notebook/StorageNotebookConnections.tsx b/frontend/src/pages/projects/notebook/StorageNotebookConnections.tsx deleted file mode 100644 index 88b3d75cb3..0000000000 --- a/frontend/src/pages/projects/notebook/StorageNotebookConnections.tsx +++ /dev/null @@ -1,75 +0,0 @@ -import * as React from 'react'; -import { Alert } from '@patternfly/react-core'; -import { ForNotebookSelection } from '~/pages/projects/types'; -import MountPathField from '~/pages/projects/pvc/MountPathField'; -import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; -import { NotebookKind } from '~/k8sTypes'; -import { getNotebookMountPaths } from './utils'; -import ConnectedNotebookField from './ConnectedNotebookField'; - -type StorageNotebookConnectionsProps = { - forNotebookData: ForNotebookSelection; - setForNotebookData: (value: ForNotebookSelection) => void; - connectedNotebooks: NotebookKind[]; -}; - -const StorageNotebookConnections: React.FC = ({ - forNotebookData, - setForNotebookData, - connectedNotebooks, -}) => { - const { - notebooks: { data, loaded, error }, - } = React.useContext(ProjectDetailsContext); - const notebooks = data.map(({ notebook }) => notebook); - - if (error) { - return ( - - {error.message} - - ); - } - - const inUseMountPaths = getNotebookMountPaths( - notebooks.find((notebook) => notebook.metadata.name === forNotebookData.name), - ); - - const connectedNotebookNames = connectedNotebooks.map((notebook) => notebook.metadata.name); - const availableNotebooks = notebooks.filter( - (notebook) => !connectedNotebookNames.includes(notebook.metadata.name), - ); - - return ( - <> - { - const selection = selectionItems[0]; - if (selection) { - setForNotebookData({ - name: selection, - mountPath: { value: '', error: '' }, - }); - } else { - setForNotebookData({ name: '', mountPath: { value: '', error: '' } }); - } - }} - /> - {forNotebookData.name && ( - { - setForNotebookData({ ...forNotebookData, mountPath }); - }} - /> - )} - - ); -}; - -export default StorageNotebookConnections; diff --git a/frontend/src/pages/projects/notebook/utils.ts b/frontend/src/pages/projects/notebook/utils.ts index 022b6fb45a..2ebc2cc5a5 100644 --- a/frontend/src/pages/projects/notebook/utils.ts +++ b/frontend/src/pages/projects/notebook/utils.ts @@ -1,6 +1,5 @@ import { EventKind, NotebookKind } from '~/k8sTypes'; import { EventStatus, NotebookSize, NotebookStatus } from '~/types'; -import { ROOT_MOUNT_PATH } from '~/pages/projects/pvc/const'; import { fireFormTrackingEvent } from '~/concepts/analyticsTracking/segmentIOUtils'; import { TrackingOutcome } from '~/concepts/analyticsTracking/trackingProperties'; import { AcceleratorProfileState } from '~/utilities/useReadAcceleratorState'; @@ -24,8 +23,7 @@ export const getNotebookPVCVolumeNames = (notebook: NotebookKind): { [name: stri }; }, {}); -const relativeMountPath = (mountPath: string): string => - mountPath.slice(ROOT_MOUNT_PATH.length) || '/'; +const relativeMountPath = (mountPath: string): string => mountPath || '/'; export const getNotebookPVCMountPathMap = ( notebook?: NotebookKind, diff --git a/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx index 120ffd1c27..d97253266f 100644 --- a/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/BaseStorageModal.tsx @@ -18,7 +18,7 @@ export type BaseStorageModalProps = { submitLabel?: string; title?: string; description?: string; - children: React.ReactNode; + children: ((opts: string) => React.ReactNode) | React.ReactNode; isValid: boolean; onSubmit: (data: CreateStorageObjectData) => Promise; hasDuplicateName?: boolean; @@ -122,7 +122,7 @@ const BaseStorageModal: React.FC = ({ disableStorageClassSelect={!!existingPvc} /> - {children} + {typeof children === 'function' ? children(createData.name) : children} diff --git a/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx index cce0496cfc..e7d48a4967 100644 --- a/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageModal.tsx @@ -1,20 +1,31 @@ import * as React from 'react'; -import { StackItem } from '@patternfly/react-core'; +import { Button, FormGroup, Stack, StackItem } from '@patternfly/react-core'; +import { PlusCircleIcon } from '@patternfly/react-icons'; import { NotebookKind, PersistentVolumeClaimKind } from '~/k8sTypes'; -import { ForNotebookSelection, StorageData } from '~/pages/projects/types'; +import { ClusterStorageNotebookSelection, MountPath, StorageData } from '~/pages/projects/types'; import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; -import { attachNotebookPVC, createPvc, removeNotebookPVC, restartNotebook, updatePvc } from '~/api'; +import { + attachNotebookPVC, + createPvc, + removeNotebookPVC, + restartNotebook, + UpdateNotebookPVC, + updatePvc, +} from '~/api'; import { ConnectedNotebookContext, useRelatedNotebooks, } from '~/pages/projects/notebook/useRelatedNotebooks'; -import NotebookRestartAlert from '~/pages/projects/components/NotebookRestartAlert'; -import StorageNotebookConnections from '~/pages/projects/notebook/StorageNotebookConnections'; import useWillNotebooksRestart from '~/pages/projects/notebook/useWillNotebooksRestart'; import { useIsAreaAvailable, SupportedArea } from '~/concepts/areas'; +import { Table } from '~/components/table'; +import { getNotebookPVCMountPathMap } from '~/pages/projects/notebook/utils'; +import { MOUNT_PATH_PREFIX } from '~/pages/projects/screens/spawner/storage/const'; +import NotebookRestartAlert from '~/pages/projects/components/NotebookRestartAlert'; import BaseStorageModal from './BaseStorageModal'; -import ExistingConnectedNotebooks from './ExistingConnectedNotebooks'; -import { isPvcUpdateRequired } from './utils'; +import { handleConnectedNotebooks, isPvcUpdateRequired } from './utils'; +import { storageColumns } from './clusterTableColumns'; +import ClusterStorageTableRow from './ClusterStorageTableRow'; type ClusterStorageModalProps = { existingPvc?: PersistentVolumeClaimKind; @@ -22,37 +33,82 @@ type ClusterStorageModalProps = { }; const ClusterStorageModal: React.FC = ({ existingPvc, onClose }) => { - const { currentProject } = React.useContext(ProjectDetailsContext); + const { + currentProject, + notebooks: { data }, + } = React.useContext(ProjectDetailsContext); const namespace = currentProject.metadata.name; - const [removedNotebooks, setRemovedNotebooks] = React.useState([]); - const [notebookData, setNotebookData] = React.useState({ - name: '', - mountPath: { value: '', error: '' }, - }); + const { notebooks: connectedNotebooks } = useRelatedNotebooks( - ConnectedNotebookContext.EXISTING_PVC, + ConnectedNotebookContext.REMOVABLE_PVC, existingPvc?.metadata.name, ); const workbenchEnabled = useIsAreaAvailable(SupportedArea.WORKBENCHES).status; - const { - notebooks: removableNotebooks, - loaded: removableNotebookLoaded, - error: removableNotebookError, - } = useRelatedNotebooks(ConnectedNotebookContext.REMOVABLE_PVC, existingPvc?.metadata.name); + const hasExistingNotebookConnections = connectedNotebooks.length > 0; + const [notebookData, setNotebookData] = React.useState([]); - const hasExistingNotebookConnections = removableNotebooks.length > 0; + React.useEffect(() => { + if (hasExistingNotebookConnections) { + setNotebookData( + connectedNotebooks.map((connectedNotebook) => ({ + name: connectedNotebook.metadata.name, + mountPath: { + value: existingPvc + ? getNotebookPVCMountPathMap(connectedNotebook)[existingPvc.metadata.name] + : '', + error: '', + }, + existingValue: true, + })), + ); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [existingPvc, hasExistingNotebookConnections]); - const restartNotebooks = useWillNotebooksRestart([...removedNotebooks, notebookData.name]); + const availableNotebooks = React.useMemo( + () => + data + .filter( + (currentNotebookData) => + !notebookData.some( + (currentNotebook) => + currentNotebook.name === currentNotebookData.notebook.metadata.name, + ), + ) + .map((currentData) => currentData.notebook), + [data, notebookData], + ); - const handleSubmit = async ( - storageData: StorageData, - attachNotebookData?: ForNotebookSelection, - dryRun?: boolean, - ) => { - const promises: Promise[] = []; + const addEmptyRow = React.useCallback(() => { + setNotebookData(() => [ + ...notebookData, + { + name: '', + mountPath: { + value: MOUNT_PATH_PREFIX, + error: '', + }, + existingValue: false, + }, + ]); + }, [notebookData]); + + const { updatedNotebooks, removedNotebooks, newNotebooks } = handleConnectedNotebooks( + notebookData, + connectedNotebooks, + existingPvc, + ); + const restartNotebooks = useWillNotebooksRestart([ + ...updatedNotebooks.map((updatedNotebook) => updatedNotebook.name), + ...removedNotebooks, + ...newNotebooks.map((newNotebook) => newNotebook.name), + ]); + + const handleSubmit = async (storageData: StorageData, dryRun?: boolean) => { + const promises: Promise[] = []; if (existingPvc) { const pvcName = existingPvc.metadata.name; @@ -70,55 +126,62 @@ const ClusterStorageModal: React.FC = ({ existingPvc, ); } - // Remove connections to notebooks that were removed + if (updatedNotebooks.length > 0) { + promises.push( + ...updatedNotebooks.map((nM) => + UpdateNotebookPVC(nM.name, namespace, nM.mountPath.value, pvcName, { dryRun }), + ), + ); + } + if (removedNotebooks.length > 0) { promises.push( ...removedNotebooks.map((nM) => removeNotebookPVC(nM, namespace, pvcName, { dryRun })), ); } - - await Promise.all(promises); - - if (attachNotebookData?.name) { - await attachNotebookPVC( - attachNotebookData.name, - namespace, - pvcName, - attachNotebookData.mountPath.value, - { - dryRun, - }, + if (newNotebooks.length > 0) { + promises.push( + ...newNotebooks.map((nM) => + attachNotebookPVC(nM.name, namespace, pvcName, nM.mountPath.value, { dryRun }), + ), ); } + await Promise.all(promises); return; } + // Create new PVC if it doesn't exist const createdPvc = await createPvc(storageData, namespace, { dryRun }); + const newCreatedPVCPromises: Promise[] = []; // Attach the new PVC to a notebook if specified - if (attachNotebookData?.name) { - await attachNotebookPVC( - attachNotebookData.name, - namespace, - createdPvc.metadata.name, - attachNotebookData.mountPath.value, - { dryRun }, + + if (newNotebooks.length > 0) { + newCreatedPVCPromises.push( + ...newNotebooks.map((nM) => + attachNotebookPVC(nM.name, namespace, createdPvc.metadata.name, nM.mountPath.value, { + dryRun, + }), + ), ); } + await Promise.all(newCreatedPVCPromises); }; - const submit = async (data: StorageData) => - handleSubmit(data, notebookData, true).then(() => handleSubmit(data, notebookData, false)); + const submit = async (dataSubmit: StorageData) => + handleSubmit(dataSubmit, true).then(() => handleSubmit(dataSubmit, false)); - const hasValidNotebookRelationship = notebookData.name - ? !!notebookData.mountPath.value && !notebookData.mountPath.error - : true; + const hasAllValidNotebookRelationship = notebookData.every((currentNotebookData) => + currentNotebookData.name + ? !!currentNotebookData.mountPath.value && !currentNotebookData.mountPath.error + : false, + ); - const isValid = hasValidNotebookRelationship; + const isValid = hasAllValidNotebookRelationship; return ( submit(data)} + onSubmit={(dataSubmit) => submit(dataSubmit)} title={existingPvc ? 'Update cluster storage' : 'Add cluster storage'} description={ existingPvc @@ -130,33 +193,72 @@ const ClusterStorageModal: React.FC = ({ existingPvc, onClose={(submitted) => onClose(submitted)} existingPvc={existingPvc} > - {workbenchEnabled && ( + {(name) => ( <> - {hasExistingNotebookConnections && ( - - - setRemovedNotebooks([...removedNotebooks, notebook.metadata.name]) - } - loaded={removableNotebookLoaded} - error={removableNotebookError} - /> - - )} - - { - setNotebookData(forNotebookData); - }} - forNotebookData={notebookData} - connectedNotebooks={connectedNotebooks} - /> - - {restartNotebooks.length !== 0 && ( - - - + {workbenchEnabled && ( + + + + ( + { + const copiedArray = [...notebookData]; + if (notebookName) { + copiedArray[rowIndex].name = notebookName; + } + if (existingValue) { + copiedArray[rowIndex].existingValue = existingValue; + } + if (mountPath) { + copiedArray[rowIndex].mountPath = mountPath; + } + + setNotebookData(copiedArray); + }} + onDelete={() => { + const copiedArray = [...notebookData]; + copiedArray.splice(rowIndex, 1); + setNotebookData(copiedArray); + }} + /> + )} + /> + + + + + + {restartNotebooks.length !== 0 && ( + + )} + + + )} )} diff --git a/frontend/src/pages/projects/screens/detail/storage/ClusterStorageTableRow.tsx b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageTableRow.tsx new file mode 100644 index 0000000000..c0b8d76cb2 --- /dev/null +++ b/frontend/src/pages/projects/screens/detail/storage/ClusterStorageTableRow.tsx @@ -0,0 +1,268 @@ +import { + ActionList, + ActionListItem, + Button, + HelperText, + HelperTextItem, + InputGroup, + InputGroupItem, + InputGroupText, + List, + ListItem, + Popover, + Stack, + StackItem, + TextInput, + Text, + Bullseye, + Spinner, +} from '@patternfly/react-core'; +import { MinusCircleIcon } from '@patternfly/react-icons'; +import { Tbody, Td, Tr } from '@patternfly/react-table'; +import React from 'react'; +import TypeaheadSelect from '~/components/TypeaheadSelect'; +import { NotebookKind } from '~/k8sTypes'; +import { ClusterStorageNotebookSelection, MountPath } from '~/pages/projects/types'; +import { MOUNT_PATH_PREFIX } from '~/pages/projects/screens/spawner/storage/const'; +import SimpleSelect from '~/components/SimpleSelect'; +import { MountPathFormat } from '~/pages/projects/screens/spawner/storage/types'; +import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; +import { getNotebookPVCMountPathMap } from '~/pages/projects/notebook/utils'; +import { + isMountPathFormat, + mountPathFormat, + mountPathSuffix, + validateClusterMountPath, +} from './utils'; + +type ClusterStorageTableRowProps = { + obj: ClusterStorageNotebookSelection; + existingPVC?: string; + rowIndex: number; + clusterStorageName: string; + + addNotebookData: (notebookName?: string, mountPath?: MountPath, existingValue?: boolean) => void; + availableNotebooks: NotebookKind[]; + onDelete: () => void; +}; + +const ClusterStorageTableRow: React.FC = ({ + obj, + clusterStorageName, + rowIndex, + existingPVC, + addNotebookData, + availableNotebooks, + onDelete, +}) => { + const { + notebooks: { data }, + } = React.useContext(ProjectDetailsContext); + const notebooks = data.map(({ notebook }) => notebook); + + React.useEffect(() => { + if (!obj.existingValue) { + addNotebookData( + undefined, + clusterStorageName + ? { + value: clusterStorageName + ? `${MOUNT_PATH_PREFIX}${clusterStorageName.toLowerCase().replace(/\s+/g, '-')}-${ + rowIndex + 1 + }` + : '', + error: '', + } + : undefined, + ); + } + }, [ + addNotebookData, + clusterStorageName, + existingPVC, + obj.existingValue, + obj.mountPath.value, + rowIndex, + ]); + + const suffix = mountPathSuffix(obj.mountPath.value); + const format = mountPathFormat(obj.mountPath.value); + + const allInUseMountPaths = getNotebookPVCMountPathMap( + notebooks.find((notebook) => notebook.metadata.name === obj.name), + ); + + const inUseMountPaths = Object.keys(allInUseMountPaths) + .filter((key) => key !== existingPVC) + .map((key) => allInUseMountPaths[key]); + + const pathFormatOptions = [ + { + type: MountPathFormat.STANDARD, + description: `Standard paths that begins with ${MOUNT_PATH_PREFIX} are visible in JupyterLab file browser.`, + }, + { + type: MountPathFormat.CUSTOM, + description: `Custom paths that do not begin with ${MOUNT_PATH_PREFIX} are not visible in the JupyterLab file browser.`, + }, + ]; + + // Define options for notebook selection + const selectOptions = React.useMemo(() => { + const options = availableNotebooks.map((notebook) => ({ + value: notebook.metadata.name, + content: notebook.metadata.name, + })); + + if (obj.name) { + options.push({ + value: obj.name, + content: obj.name, + }); + } + + return options; + }, [availableNotebooks, obj.name]); + + const validateAndUpdate = React.useCallback( + (value: string, newFormat?: 'Standard' | 'Custom') => { + const prefix = (newFormat ?? format) === MountPathFormat.STANDARD ? MOUNT_PATH_PREFIX : ''; + const newValue = `${prefix}${value}`; + addNotebookData( + obj.name, + { + value: newValue, + error: validateClusterMountPath(newValue, inUseMountPaths) || '', + }, + true, + ); + }, + [addNotebookData, format, inUseMountPaths, obj.name], + ); + + const handleFormatChange = (newFormat: 'Standard' | 'Custom') => { + validateAndUpdate(suffix, newFormat); + }; + + return ( + + + + + + + + + ); +}; + +export default ClusterStorageTableRow; diff --git a/frontend/src/pages/projects/screens/detail/storage/ExistingConnectedNotebooks.tsx b/frontend/src/pages/projects/screens/detail/storage/ExistingConnectedNotebooks.tsx deleted file mode 100644 index 2f813aaaf9..0000000000 --- a/frontend/src/pages/projects/screens/detail/storage/ExistingConnectedNotebooks.tsx +++ /dev/null @@ -1,59 +0,0 @@ -import * as React from 'react'; -import { Alert, Chip, ChipGroup, FormGroup, Spinner, Text } from '@patternfly/react-core'; -import { NotebookKind } from '~/k8sTypes'; -import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; - -type ExistingConnectedNotebooksProps = { - connectedNotebooks: NotebookKind[]; - onNotebookRemove: (notebook: NotebookKind) => void; - loaded: boolean; - error?: Error; -}; - -const ExistingConnectedNotebooks: React.FC = ({ - connectedNotebooks, - onNotebookRemove, - loaded, - error, -}) => { - const [notebooksToShow, setNotebooksToShow] = React.useState(connectedNotebooks); - let content: React.ReactNode; - if (error) { - content = ( - - {error.message} - - ); - } else if (!loaded) { - content = ; - } else if (notebooksToShow.length === 0) { - content = All existing connections have been removed.; - } else { - content = ( - - {notebooksToShow.map((notebook) => { - const notebookDisplayName = getDisplayNameFromK8sResource(notebook); - return ( - { - setNotebooksToShow( - notebooksToShow.filter( - (notebookToShow) => notebookToShow.metadata.name !== notebook.metadata.name, - ), - ); - onNotebookRemove(notebook); - }} - > - {notebookDisplayName} - - ); - })} - - ); - } - - return {content}; -}; - -export default ExistingConnectedNotebooks; diff --git a/frontend/src/pages/projects/screens/detail/storage/StorageTableRow.tsx b/frontend/src/pages/projects/screens/detail/storage/StorageTableRow.tsx index 253f3e4492..ce427ee6d7 100644 --- a/frontend/src/pages/projects/screens/detail/storage/StorageTableRow.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/StorageTableRow.tsx @@ -167,7 +167,7 @@ const StorageTableRow: React.FC = ({ {workbenchEnabled && ( -
+ addNotebookData('')} + onSelect={(_ev, selectedValue) => { + if (typeof selectedValue === 'string') { + addNotebookData(selectedValue); + } + }} + /> + + ({ + ...option, + label: option.type, + description: option.description, + key: option.type, + }))} + value={format} + onChange={(newSelection) => { + if (isMountPathFormat(newSelection)) { + handleFormatChange(newSelection); + } + }} + previewDescription={false} + /> + + + + + {format === MountPathFormat.STANDARD && ( + + {MOUNT_PATH_PREFIX} + + )} + + validateAndUpdate(value)} + isRequired + validated={ + obj.mountPath.error ? 'error' : suffix.length > 0 ? 'success' : 'default' + } + onBlur={() => validateAndUpdate(suffix)} + /> + + + + + + {obj.mountPath.error && ( + + {obj.mountPath.error.includes(`This path is already connected`) ? ( +
+ {obj.mountPath.error}.{' '} + 0 ? ( + + {inUseMountPaths.map((mountPath, i) => ( + {mountPath} + ))} + + ) : ( + + + Loading + + ) + } + > + View connection paths + +
+ ) : ( + obj.mountPath.error + )} + + )} + + + + 		+ + + + + +
+ [] = [ + { + label: 'ID', + field: 'id', + sortable: false, + className: 'pf-v5-u-hidden', + }, + { + label: 'Name', + field: 'name', + sortable: false, + width: 25, + }, + { + label: 'Path format', + field: 'pathFormat', + sortable: false, + width: 30, + }, + { + label: 'Mount path', + field: 'mountPath', + sortable: false, + info: { + popover: + 'The directory within a container where a volume is mounted and accessible. Must consist of lowercase letters, numbers and hyphens (-). Use slashes (/) to indicate subdirectories', + }, + width: 45, + }, +]; diff --git a/frontend/src/pages/projects/screens/detail/storage/data.ts b/frontend/src/pages/projects/screens/detail/storage/data.ts index 047fc40631..3731d6234e 100644 --- a/frontend/src/pages/projects/screens/detail/storage/data.ts +++ b/frontend/src/pages/projects/screens/detail/storage/data.ts @@ -38,7 +38,7 @@ export const columns: SortableData[] = [ }, { field: 'connected', - label: 'Connected workbenches', + label: 'Workbench connections', width: 20, sortable: false, }, diff --git a/frontend/src/pages/projects/screens/detail/storage/utils.ts b/frontend/src/pages/projects/screens/detail/storage/utils.ts index 3af3ceaaf2..b29a15024e 100644 --- a/frontend/src/pages/projects/screens/detail/storage/utils.ts +++ b/frontend/src/pages/projects/screens/detail/storage/utils.ts @@ -1,6 +1,9 @@ import { getDisplayNameFromK8sResource, getDescriptionFromK8sResource } from '~/concepts/k8s/utils'; -import { PersistentVolumeClaimKind } from '~/k8sTypes'; -import { StorageData } from '~/pages/projects/types'; +import { NotebookKind, PersistentVolumeClaimKind } from '~/k8sTypes'; +import { ClusterStorageNotebookSelection, StorageData } from '~/pages/projects/types'; +import { MOUNT_PATH_PREFIX } from '~/pages/projects/screens/spawner/storage/const'; +import { MountPathFormat } from '~/pages/projects/screens/spawner/storage/types'; +import { getNotebookPVCMountPathMap } from '~/pages/projects/notebook/utils'; type Status = 'error' | 'warning' | 'info' | null; export const getFullStatusFromPercentage = (percentageFull: number): Status => { @@ -24,3 +27,87 @@ export const isPvcUpdateRequired = ( getDescriptionFromK8sResource(existingPvc) !== storageData.description || existingPvc.spec.resources.requests.storage !== storageData.size || existingPvc.spec.storageClassName !== storageData.storageClassName; + +export const handleConnectedNotebooks = ( + tempData: ClusterStorageNotebookSelection[], + notebooks: NotebookKind[], + existingPvc?: PersistentVolumeClaimKind, +): { + updatedNotebooks: ClusterStorageNotebookSelection[]; + removedNotebooks: string[]; + newNotebooks: ClusterStorageNotebookSelection[]; +} => { + const updatedNotebooks: ClusterStorageNotebookSelection[] = []; + const removedNotebooks: string[] = []; + const unChangedNotebooks: ClusterStorageNotebookSelection[] = []; + + notebooks.forEach((notebook) => { + const tempEntry = tempData.find((item) => item.name === notebook.metadata.name); + if (!tempEntry) { + removedNotebooks.push(notebook.metadata.name); + } else { + const mountPath = existingPvc + ? getNotebookPVCMountPathMap(notebook)[existingPvc.metadata.name] + : ''; + if (mountPath !== tempEntry.mountPath.value) { + updatedNotebooks.push(tempEntry); + } + if (mountPath === tempEntry.mountPath.value) { + unChangedNotebooks.push(tempEntry); + } + } + }); + + const newNotebooks = tempData.filter( + (item) => + !updatedNotebooks.includes(item) && + !removedNotebooks.includes(item.name) && + !unChangedNotebooks.includes(item), + ); + + return { updatedNotebooks, removedNotebooks, newNotebooks }; +}; + +export const mountPathSuffix = (mountPath: string): string => + mountPath.startsWith(MOUNT_PATH_PREFIX) ? mountPath.slice(MOUNT_PATH_PREFIX.length) : mountPath; + +export const mountPathFormat = (mountPath: string): MountPathFormat => + mountPath.startsWith(MOUNT_PATH_PREFIX) ? MountPathFormat.STANDARD : MountPathFormat.CUSTOM; + +export const isMountPathFormat = (value: string): value is MountPathFormat => + value === MountPathFormat.STANDARD || value === MountPathFormat.CUSTOM; + +export const validateClusterMountPath = (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.'; + } + + if (value === '/' && format === MountPathFormat.CUSTOM) { + return 'Enter a path to a model or folder. This path cannot point to a root folder.'; + } + + // Regex to allow empty string for Standard format + const regex = + format === MountPathFormat.STANDARD + ? /^(\/?[a-z0-9-]+(\/[a-z0-9-]+)*\/?|)$/ + : /^(\/?[a-z0-9-]+(\/[a-z0-9-]+)*\/?)?$/; + + if (!regex.test(value)) { + return 'Must consist of lowercase letters, numbers and hyphens (-). Use slashes (/) to indicate the subdirectories'; + } + + if ( + inUseMountPaths.includes(value) || + (format === MountPathFormat.STANDARD && + inUseMountPaths.includes(`${MOUNT_PATH_PREFIX}${value}`)) || + (format === MountPathFormat.CUSTOM && inUseMountPaths.includes(`/${value}`)) + ) { + return `This path is already connected to this workbench, Try a different folder name`; + } + + return ''; +}; diff --git a/frontend/src/pages/projects/screens/spawner/storage/types.ts b/frontend/src/pages/projects/screens/spawner/storage/types.ts index aed06196d8..dc86350a31 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/types.ts +++ b/frontend/src/pages/projects/screens/spawner/storage/types.ts @@ -1,4 +1,4 @@ export enum MountPathFormat { - STANDARD = 'standard', - CUSTOM = 'custom', + STANDARD = 'Standard', + CUSTOM = 'Custom', } diff --git a/frontend/src/pages/projects/types.ts b/frontend/src/pages/projects/types.ts index 3021cc8b92..61535dff6e 100644 --- a/frontend/src/pages/projects/types.ts +++ b/frontend/src/pages/projects/types.ts @@ -38,6 +38,10 @@ export type ForNotebookSelection = { mountPath: MountPath; }; +export type ClusterStorageNotebookSelection = ForNotebookSelection & { + existingValue: boolean; +}; + export type CreatingStorageObjectForNotebook = NameDescType & { size: string; forNotebook: ForNotebookSelection;