Update cluster storage modal

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<NotebookKind>;
   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,
             ],
           },
         },

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() {

frontend/src/__tests__/cypress/cypress/tests/mocked/projects/clusterStorage.cy.ts

@@ -69,6 +69,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 +182,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 +212,7 @@ describe('ClusterStorage', () => {
         {
           op: 'add',
           path: '/spec/template/spec/containers/0/volumeMounts/-',
-          value: { mountPath: '/opt/app-root/src/data' },
+          value: { mountPath: 'data' },
         },
       ]);
     });
@@ -248,7 +240,7 @@ describe('ClusterStorage', () => {
               ],
               containers: [
                 {
-                  volumeMounts: [{ name: 'existing-pvc', mountPath: '/' }],
+                  volumeMounts: [{ name: 'existing-pvc', mountPath: '/opt/app-root/src' }],
                 },
               ],
             },
@@ -276,12 +268,9 @@ describe('ClusterStorage', () => {
     clusterStorage.visit('test-project');
     const clusterStorageRow = clusterStorage.getClusterStorageRow('Existing PVC');
     clusterStorageRow.findKebabAction('Edit storage').click();
-
-    // Connect to 'Another Notebook'
-    updateClusterStorageModal.findWorkbenchConnectionSelect().click();
-    updateClusterStorageModal.find().findByText('Another Notebook').click();
-
-    updateClusterStorageModal.findMountField().fill('new-data');
+    updateClusterStorageModal.findAddWorkbenchButton().click();
+    updateClusterStorageModal.selectWorkbenchName(1, 'another-notebook');
+    updateClusterStorageModal.findMountPathField(1).fill('new-data');
 
     cy.interceptK8s('PATCH', NotebookModel, anotherNotebook).as('updateClusterStorage');
 

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(

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<NotebookKind> => {
   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<NotebookKind> =>
+  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<NotebookKind>(
+          applyK8sAPIOptions(
+            {
+              model: NotebookModel,
+              queryOptions: { name: notebookName, ns: namespace },
+              patches,
+            },
+            opts,
+          ),
+        )
+          .then(resolve)
+          .catch(reject);
+      })
+      .catch(reject);
+  });
+
 export const removeNotebookPVC = (
   notebookName: string,
   namespace: string,

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,

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<void>;
   hasDuplicateName?: boolean;
@@ -122,7 +122,7 @@ const BaseStorageModal: React.FC<BaseStorageModalProps> = ({
               disableStorageClassSelect={!!existingPvc}
             />
           </StackItem>
-          {children}
+          {typeof children === 'function' ? children(createData.name) : children}
         </Stack>
       </Form>
     </Modal>