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

…onfiguration

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

@@ -158,6 +158,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');
 
@@ -166,16 +199,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() {
@@ -306,11 +331,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

@@ -271,22 +271,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', '20Gi');
+    storageTableRow.findMountPathValue().should('have.text', '/opt/apt-root/src/');
 
     //add data connection
     createSpawnerPage.findDataConnectionCheckbox().check();
@@ -323,15 +312,6 @@ describe('Workbench page', () => {
           name: 'test-project',
           namespace: 'test-project',
         },
-        spec: {
-          template: {
-            spec: {
-              volumes: [
-                { name: 'test-storage', persistentVolumeClaim: { claimName: 'test-storage' } },
-              ],
-            },
-          },
-        },
       });
     });
     verifyRelativeURL('/projects/test-project?section=workbenches');
@@ -570,7 +550,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

@@ -40,10 +40,11 @@ import {
 } 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;
@@ -90,6 +91,11 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
     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 = {
@@ -110,8 +116,8 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
       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,
@@ -144,7 +150,7 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
     const pvcDetails = await replaceRootVolumesForNotebook(
       projectName,
       editNotebook,
-      storageData,
+      rootPathStorageData,
       dryRun,
     ).catch(handleError);
 
@@ -247,7 +253,33 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
         ? [dataConnection.existing]
         : [];
 
-    const pvcDetails = await createPvcDataForNotebook(projectName, storageData).catch(handleError);
+    const createPvcRequests = storageData.map((pvcData) =>
+      createPvcDataForNotebook(projectName, pvcData),
+    );
+
+    const pvcResponses = await Promise.all(createPvcRequests).catch(handleError);
+    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]}

frontend/src/pages/projects/screens/spawner/spawnerUtils.ts

@@ -1,4 +1,3 @@
-import * as React from 'react';
 import compareVersions from 'compare-versions';
 import { NotebookSize, Volume, VolumeMount } from '~/types';
 import {
@@ -30,31 +29,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 '';
@@ -395,22 +369,19 @@ export const isEnvVariableDataValid = (envVariables: EnvVariable[]): boolean =>
 
 export const checkRequiredFieldsForNotebookStart = (
   startNotebookData: StartNotebookData,
-  storageData: StorageData,
+  storageData: StorageData[],
   envVariables: EnvVariable[],
   dataConnection: DataConnectionData,
 ): boolean => {
   const { projectName, notebookData, image } = startNotebookData;
-  const { storageType, creating, existing } = storageData;
   const isNotebookDataValid = !!(
     projectName &&
     isK8sNameDescriptionDataValid(notebookData) &&
     image.imageStream &&
     image.imageVersion
   );
 
-  const newStorageFieldInvalid = storageType === StorageType.NEW_PVC && !creating.nameDesc.name;
-  const existingStorageFieldInvalid = storageType === StorageType.EXISTING_PVC && !existing.storage;
-  const isStorageDataValid = !newStorageFieldInvalid && !existingStorageFieldInvalid;
+  const isStorageDataValid = storageData.length > 0;
 
   const newDataConnectionInvalid =
     dataConnection.type === 'creating' &&