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 be0981068e..d792e68768 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 @@ -162,6 +162,162 @@ describe('ClusterStorage', () => { }); }); + 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: '/' }], + }, + ], + }, + }, + }, + }, + }); + + // 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'); + + 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: '/' }], + }, + ], + }, + }, + }, + }, + }); + + // 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'); + + 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('list cluster storage and Table sorting', () => { cy.interceptOdh( 'GET /api/config', diff --git a/frontend/src/pages/projects/notebook/ConnectedNotebookNames.tsx b/frontend/src/pages/projects/notebook/ConnectedNotebookNames.tsx index 37f424e09e..f84d4dddb4 100644 --- a/frontend/src/pages/projects/notebook/ConnectedNotebookNames.tsx +++ b/frontend/src/pages/projects/notebook/ConnectedNotebookNames.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { Badge, List, ListItem, Spinner } from '@patternfly/react-core'; +import { Label, LabelGroup, Spinner } from '@patternfly/react-core'; import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; import useRelatedNotebooks, { ConnectedNotebookContext } from './useRelatedNotebooks'; @@ -32,13 +32,13 @@ const ConnectedNotebookNames: React.FC = ({ } return ( - + {connectedNotebooks.map((notebook) => ( - - {getDisplayNameFromK8sResource(notebook)} - + ))} - + ); }; diff --git a/frontend/src/pages/projects/notebook/StorageNotebookConnections.tsx b/frontend/src/pages/projects/notebook/StorageNotebookConnections.tsx index 7f404cffef..88b3d75cb3 100644 --- a/frontend/src/pages/projects/notebook/StorageNotebookConnections.tsx +++ b/frontend/src/pages/projects/notebook/StorageNotebookConnections.tsx @@ -3,19 +3,20 @@ 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; - isDisabled: boolean; + connectedNotebooks: NotebookKind[]; }; const StorageNotebookConnections: React.FC = ({ forNotebookData, setForNotebookData, - isDisabled, + connectedNotebooks, }) => { const { notebooks: { data, loaded, error }, @@ -34,12 +35,17 @@ const StorageNotebookConnections: React.FC = ({ 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]; diff --git a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx index a0b145be47..ee5714eb01 100644 --- a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx +++ b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx @@ -34,13 +34,18 @@ const ManageStorageModal: React.FC = ({ existingData, isOp const [error, setError] = React.useState(); const { currentProject } = React.useContext(ProjectDetailsContext); const namespace = currentProject.metadata.name; - const { - notebooks: connectedNotebooks, - loaded: notebookLoaded, - error: notebookError, - } = useRelatedNotebooks(ConnectedNotebookContext.EXISTING_PVC, existingData?.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, @@ -75,12 +80,8 @@ const ManageStorageModal: React.FC = ({ existingData, isOp ? !!createData.forNotebook.mountPath.value && !createData.forNotebook.mountPath.error : true; - const storageClassSelected = isStorageClassesAvailable ? createData.storageClassName : true; const canCreate = - !actionInProgress && - createData.nameDesc.name.trim() && - hasValidNotebookRelationship && - storageClassSelected; + !actionInProgress && createData.nameDesc.name.trim() && hasValidNotebookRelationship; const runPromiseActions = async (dryRun: boolean) => { const { @@ -176,12 +177,12 @@ const ManageStorageModal: React.FC = ({ existingData, isOp {createData.hasExistingNotebookConnections && ( setRemovedNotebooks([...removedNotebooks, notebook.metadata.name]) } - loaded={notebookLoaded} - error={notebookError} + loaded={removableNotebookLoaded} + error={removableNotebookError} /> )} @@ -191,7 +192,7 @@ const ManageStorageModal: React.FC = ({ existingData, isOp setCreateData('forNotebook', forNotebookData); }} forNotebookData={createData.forNotebook} - isDisabled={connectedNotebooks.length !== 0 && removedNotebooks.length === 0} + connectedNotebooks={connectedNotebooks} /> {restartNotebooks.length !== 0 && ( diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx index 507422bdd5..ec74a5b62f 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx @@ -39,11 +39,7 @@ import ContainerSizeSelector from './deploymentSize/ContainerSizeSelector'; import StorageField from './storage/StorageField'; import EnvironmentVariables from './environmentVariables/EnvironmentVariables'; import { useStorageDataObject } from './storage/utils'; -import { - getCompatibleAcceleratorIdentifiers, - getRootVolumeName, - useMergeDefaultPVCName, -} from './spawnerUtils'; +import { getCompatibleAcceleratorIdentifiers, useMergeDefaultPVCName } from './spawnerUtils'; import { useNotebookEnvVariables } from './environmentVariables/useNotebookEnvVariables'; import DataConnectionField from './dataConnection/DataConnectionField'; import { useNotebookDataConnection } from './dataConnection/useNotebookDataConnection'; @@ -220,11 +216,7 @@ const SpawnerPage: React.FC = ({ existingNotebook }) => { isInline title="Cluster storage will mount to /" /> - + void; - editStorage?: string; selectDirection?: 'up' | 'down'; menuAppendTo?: HTMLElement; }; @@ -17,19 +16,11 @@ type AddExistingStorageFieldProps = { const AddExistingStorageField: React.FC = ({ data, setData, - editStorage, selectDirection, menuAppendTo, }) => { - const { - currentProject, - notebooks: { data: allNotebooks }, - } = React.useContext(ProjectDetailsContext); - const [storages, loaded, loadError] = useAvailablePvcs( - currentProject.metadata.name, - allNotebooks, - editStorage, - ); + const { currentProject } = React.useContext(ProjectDetailsContext); + const [storages, loaded, loadError] = useProjectPvcs(currentProject.metadata.name); const selectOptions = React.useMemo( () => diff --git a/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx b/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx index 651370a5a9..e9127c4f12 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx +++ b/frontend/src/pages/projects/screens/spawner/storage/StorageClassSelect.tsx @@ -77,7 +77,7 @@ const StorageClassSelect: React.FC = ({ }); return ( - + ; - editStorage: string; }; -const StorageField: React.FC = ({ storageData, setStorageData, editStorage }) => { +const StorageField: React.FC = ({ storageData, setStorageData }) => { const { storageType, creating, existing } = storageData; return ( @@ -53,7 +52,6 @@ const StorageField: React.FC = ({ storageData, setStorageData, setStorageData('existing', data)} - editStorage={editStorage} selectDirection="up" menuAppendTo={getDashboardMainContainer()} /> diff --git a/frontend/src/pages/projects/screens/spawner/storage/utils.ts b/frontend/src/pages/projects/screens/spawner/storage/utils.ts index 9d78b03d5c..0a2bedf3bd 100644 --- a/frontend/src/pages/projects/screens/spawner/storage/utils.ts +++ b/frontend/src/pages/projects/screens/spawner/storage/utils.ts @@ -51,6 +51,7 @@ export const useCreateStorageObjectForNotebook = ( ConnectedNotebookContext.REMOVABLE_PVC, existingData ? existingData.metadata.name : undefined, ); + const hasExistingNotebookConnections = relatedNotebooks.length > 0; React.useEffect(() => { diff --git a/manifests/core-bases/base/cluster-role.yaml b/manifests/core-bases/base/cluster-role.yaml index 510bab4f33..97f82cfdad 100644 --- a/manifests/core-bases/base/cluster-role.yaml +++ b/manifests/core-bases/base/cluster-role.yaml @@ -3,6 +3,13 @@ apiVersion: rbac.authorization.k8s.io/v1 metadata: name: odh-dashboard rules: + - verbs: + - update + - patch + apiGroups: + - storage.k8s.io + resources: + - storageclasses - verbs: - get - list diff --git a/manifests/core-bases/base/role.yaml b/manifests/core-bases/base/role.yaml index 2c856fd436..f48ec75ad9 100644 --- a/manifests/core-bases/base/role.yaml +++ b/manifests/core-bases/base/role.yaml @@ -3,15 +3,6 @@ apiVersion: rbac.authorization.k8s.io/v1 metadata: name: odh-dashboard rules: - - verbs: - - get - - list - - update - - patch - apiGroups: - - storage.k8s.io - resources: - - storageclasses - verbs: - create - get