[RHOAIENG-1101] Cannot remove additional storage from the workbench configuration

frontend/src/__mocks__/mockStartNotebookData.ts

@@ -94,18 +94,20 @@ 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,
+    creating: {
+      nameDesc: {
+        name: 'test-pvc',
+        description: '',
+      },
+      size: '20Gi',
+      storageClassName: 'gp2-csi',
     },
-    size: '20Gi',
-    storageClassName: 'gp2-csi',
+    existing: { storage: '' },
   },
-  existing: { storage: '' },
-};
+];
 
 export const mockDataConnectionData: DataConnectionData = {
   type: 'creating',

frontend/src/__tests__/cypress/cypress/pages/workbench.ts

@@ -174,6 +174,39 @@ class NotebookRow extends TableRow {
   }
 }
 
+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<HTMLTableRowElement>
+        >,
+    );
+  }
+}
+
 class CreateSpawnerPage {
   k8sNameDescription = new K8sNameDescriptionField('workbench');
 
@@ -182,16 +215,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() {
@@ -322,11 +347,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;

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

@@ -273,22 +273,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('Mi');
-
-    //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/apt-root/src/');
 
     //add data connection
     createSpawnerPage.findDataConnectionCheckbox().check();
@@ -325,15 +314,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');
@@ -565,7 +545,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-notebook-Test Notebook');
     editSpawnerPage.findSubmitButton().should('be.enabled');
     editSpawnerPage.k8sNameDescription.findDisplayNameInput().fill('Updated Notebook');
 

frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx

@@ -33,10 +33,11 @@
 } from './service';
 import { checkRequiredFieldsForNotebookStart } from './spawnerUtils';
 import { getNotebookDataConnection } from './dataConnection/useNotebookDataConnection';
+import { defaultClusterStorage } from './storage/constants';
 
 type SpawnerFooterProps = {
   startNotebookData: StartNotebookData;
-  storageData: StorageData;
+  storageData: StorageData[];
   envVariables: EnvVariable[];
   dataConnection: DataConnectionData;
   canEnablePipelines: boolean;
@@ -83,6 +84,11 @@
     editNotebook,
     existingDataConnections,
   );
+  const rootPathStorageData =
+    storageData.find(
+      (formData) => formData.creating.mountPath === defaultClusterStorage.mountPath,
+    ) || storageData[0];
+
   const afterStart = (name: string, type: 'created' | 'updated') => {
     const { selectedAcceleratorProfile, notebookSize, image } = startNotebookData;
     const tep: FormTrackingEventProperties = {
@@ -103,8 +109,8 @@
       imageName: image.imageStream?.metadata.name,
       projectName,
       notebookName: name,
-      storageType: storageData.storageType,
-      storageDataSize: storageData.creating.size,
+      storageType: rootPathStorageData.storageType,
+      storageDataSize: rootPathStorageData.creating.size,
       dataConnectionType: dataConnection.creating?.type?.toString(),
       dataConnectionCategory: dataConnection.creating?.values?.category?.toString(),
       dataConnectionEnabled: dataConnection.enabled,
@@ -137,7 +143,7 @@
     const pvcDetails = await replaceRootVolumesForNotebook(
       projectName,
       editNotebook,
-      storageData,
+      rootPathStorageData,
       dryRun,
     );
 
@@ -193,7 +199,34 @@
       dataConnection.enabled && dataConnection.type === 'existing' && dataConnection.existing
         ? [dataConnection.existing]
         : [];
-    const pvcDetails = await createPvcDataForNotebook(projectName, storageData, dryRun);
+
+    const createPvcRequests = storageData.map((pvcData) =>
+      createPvcDataForNotebook(projectName, pvcData, dryRun),
+    );
+
+    const pvcResponses = await Promise.all(createPvcRequests);
+    const pvcDetails = pvcResponses.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: [],
+      },
+    );
+
     const envFrom = await createConfigMapsAndSecretsForNotebook(
       projectName,
       [...envVariables, ...newDataConnection],

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,
@@ -31,21 +33,23 @@ import K8sNameDescriptionField, {
   useK8sNameDescriptionFieldData,
 } from '~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField';
 import { LimitNameResourceType } from '~/concepts/k8s/K8sNameDescriptionField/utils';
+import { StorageData, StorageType } from '~/pages/projects/types';
 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, getRootVolumeName } from './spawnerUtils';
 import { useNotebookEnvVariables } from './environmentVariables/useNotebookEnvVariables';
 import DataConnectionField from './dataConnection/DataConnectionField';
 import { useNotebookDataConnection } from './dataConnection/useNotebookDataConnection';
 import { useNotebookSizeState } from './useNotebookSizeState';
 import useDefaultStorageClass from './storage/useDefaultStorageClass';
 import usePreferredStorageClass from './storage/usePreferredStorageClass';
+import { ClusterStorageTable } from './storage/ClusterStorageTable';
+import useDefaultPvcSize from './storage/useDefaultPvcSize';
+import { defaultClusterStorage } from './storage/constants';
 
 type SpawnerPageProps = {
   existingNotebook?: NotebookKind;
@@ -68,19 +72,30 @@ const SpawnerPage: React.FC<SpawnerPageProps> = ({ 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 [storageData, setStorageData] = React.useState<StorageData[]>([
+    {
+      storageType: existingNotebook ? StorageType.EXISTING_PVC : StorageType.NEW_PVC,
+      creating: {
+        nameDesc: {
+          name: k8sNameDescriptionData.data.name || defaultClusterStorage.name,
+          description: defaultClusterStorage.description,
+        },
+        size: defaultClusterStorage.size || defaultNotebookSize,
+        storageClassName: defaultStorageClassName || defaultClusterStorage.storageClassName,
+        mountPath: defaultClusterStorage.mountPath,
+      },
+      existing: {
+        storage: getRootVolumeName(existingNotebook),
+      },
+    },
+  ]);
 
   const [envVariables, setEnvVariables] = useNotebookEnvVariables(existingNotebook);
   const [dataConnectionData, setDataConnectionData] = useNotebookDataConnection(
@@ -204,19 +219,32 @@ const SpawnerPage: React.FC<SpawnerPageProps> = ({ existingNotebook }) => {
               <EnvironmentVariables envVariables={envVariables} setEnvVariables={setEnvVariables} />
             </FormSection>
             <FormSection
-              title={SpawnerPageSectionTitles[SpawnerPageSectionID.CLUSTER_STORAGE]}
+              title={
+                <Flex
+                  spaceItems={{ default: 'spaceItemsMd' }}
+                  alignItems={{ default: 'alignItemsCenter' }}
+                >
+                  <FlexItem spacer={{ default: 'spacerLg' }}>
+                    {SpawnerPageSectionTitles[SpawnerPageSectionID.CLUSTER_STORAGE]}
+                  </FlexItem>
+
+                  <Button variant="secondary" data-testid="existing-storage-button">
+                    Attach existing storage
+                  </Button>
+
+                  <Button variant="secondary" data-testid="create-storage-button">
+                    Create storage
+                  </Button>
+                </Flex>
+              }
               id={SpawnerPageSectionID.CLUSTER_STORAGE}
               aria-label={SpawnerPageSectionTitles[SpawnerPageSectionID.CLUSTER_STORAGE]}
             >
-              <Alert
-                data-testid="cluster-storage-alert"
-                component="h2"
-                variant="info"
-                isPlain
-                isInline
-                title="Cluster storage will mount to /"
+              <ClusterStorageTable
+                storageData={storageData.map((formData, index) => ({ ...formData, id: index }))}
+                setStorageData={setStorageData}
+                workbenchName={k8sNameDescriptionData.data.k8sName.value}
               />
-              <StorageField storageData={storageData} setStorageData={setStorageData} />
             </FormSection>
             <FormSection
               title={SpawnerPageSectionTitles[SpawnerPageSectionID.DATA_CONNECTIONS]}