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

…onfiguration (#3320)

frontend/src/__mocks__/mockStartNotebookData.ts

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

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

@@ -46,7 +46,7 @@ class ClusterStorageModal extends Modal {
   findWorkbenchConnectionSelect() {
     return this.find()
       .findByTestId('connect-existing-workbench-group')
-      .findByRole('button', { name: 'Typeahead menu toggle' });
+      .findByRole('button', { name: 'Typeahead menu toggle', hidden: true });
   }
 
   findMountField() {
@@ -79,7 +79,7 @@ class ClusterStorageModal extends Modal {
 
   selectPVSize(name: string) {
     this.findPVSizeSelectButton().click();
-    cy.findByRole('menuitem', { name }).click();
+    cy.findByRole('menuitem', { name, hidden: true }).click();
   }
 
   shouldHavePVSizeSelectValue(name: string) {
@@ -92,7 +92,7 @@ class ClusterStorageModal extends Modal {
   }
 
   findPVSizeMinusButton() {
-    return this.findPVSizeField().findByRole('button', { name: 'Minus' });
+    return this.findPVSizeField().findByRole('button', { name: 'Minus', hidden: true });
   }
 
   findPersistentStorageWarningCanNotEdit() {
@@ -108,7 +108,7 @@ class ClusterStorageModal extends Modal {
   }
 
   findPVSizePlusButton() {
-    return this.findPVSizeField().findByRole('button', { name: 'Plus' });
+    return this.findPVSizeField().findByRole('button', { name: 'Plus', hidden: true });
   }
 
   findStorageClassSelect() {

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

@@ -202,6 +202,39 @@ class AttachConnectionModal extends Modal {
   }
 }
 
+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');
 
@@ -210,16 +243,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() {
@@ -363,11 +388,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/clusterStorage.cy.ts

@@ -158,7 +158,8 @@ describe('ClusterStorage', () => {
     addClusterStorageModal.find().findByText('openshift-default-sc').should('exist');
 
     // select storage class
-    addClusterStorageModal.findStorageClassSelect().findSelectOption('Test SC 1').click();
+    addClusterStorageModal.findStorageClassSelect().click();
+    addClusterStorageModal.find().findByText('Test SC 1').click();
     addClusterStorageModal.findSubmitButton().should('be.enabled');
     addClusterStorageModal.findDescriptionInput().fill('description');
     addClusterStorageModal.findPVSizeMinusButton().click();
@@ -168,10 +169,8 @@ describe('ClusterStorage', () => {
     addClusterStorageModal.selectPVSize('MiB');
 
     //connect workbench
-    addClusterStorageModal
-      .findWorkbenchConnectionSelect()
-      .findSelectOption('Test Notebook')
-      .click();
+    addClusterStorageModal.findWorkbenchConnectionSelect().click();
+    addClusterStorageModal.find().findByText('Test Notebook').click();
 
     // don't allow duplicate path
     addClusterStorageModal.findMountField().clear();
@@ -274,88 +273,9 @@ describe('ClusterStorage', () => {
     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: '/' }],
-                },
-              ],
-            },
-          },
-        },
-      },
-    });
+    updateClusterStorageModal.findWorkbenchConnectionSelect().click();
+    updateClusterStorageModal.find().findByText('Another Notebook').click();
 
-    // 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');

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

@@ -300,22 +300,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('MiB');
-
-    //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/app-root/src/');
 
     //add data connection
     createSpawnerPage.findDataConnectionCheckbox().check();
@@ -352,15 +341,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');
@@ -693,7 +673,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 Storage');
     editSpawnerPage.findSubmitButton().should('be.enabled');
     editSpawnerPage.k8sNameDescription.findDisplayNameInput().fill('Updated Notebook');
 
@@ -704,7 +688,9 @@ describe('Workbench page', () => {
 
     cy.interceptK8s('PUT', NotebookModel, mockNotebookK8sResource({})).as('editWorkbenchDryRun');
     cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})).as('editWorkbench');
+
     editSpawnerPage.findSubmitButton().click();
+
     cy.wait('@editWorkbenchDryRun').then((interception) => {
       expect(interception.request.url).to.include('?dryRun=All');
       expect(interception.request.body).to.containSubset({

frontend/src/api/k8s/__tests__/pvcs.spec.ts

@@ -18,7 +18,7 @@ import {
 } from '~/api/k8s/pvcs';
 import { PVCModel } from '~/api/models/k8s';
 import { PersistentVolumeClaimKind } from '~/k8sTypes';
-import { CreatingStorageObject } from '~/pages/projects/types';
+import { StorageData } from '~/pages/projects/types';
 
 jest.mock('@openshift/dynamic-plugin-sdk-utils', () => ({
   k8sGetResource: jest.fn(),
@@ -35,11 +35,9 @@ const k8sUpdateResourceMock = jest.mocked(k8sUpdateResource<PersistentVolumeClai
 const k8sDeleteResourceMock = jest.mocked(k8sDeleteResource<PersistentVolumeClaimKind, K8sStatus>);
 const k8sGetResourceMock = jest.mocked(k8sGetResource<PersistentVolumeClaimKind>);
 
-const data: CreatingStorageObject = {
-  nameDesc: {
-    name: 'pvc',
-    description: 'Test Storage',
-  },
+const data: StorageData = {
+  name: 'pvc',
+  description: 'Test Storage',
   size: '5Gi',
 };
 

frontend/src/api/k8s/pvcs.ts

@@ -12,20 +12,15 @@ import { PVCModel } from '~/api/models';
 import { translateDisplayNameForK8s } from '~/concepts/k8s/utils';
 import { LABEL_SELECTOR_DASHBOARD_RESOURCE } from '~/const';
 import { applyK8sAPIOptions } from '~/api/apiMergeUtils';
-import { CreatingStorageObject } from '~/pages/projects/types';
+import { StorageData } from '~/pages/projects/types';
 
 export const assemblePvc = (
-  data: CreatingStorageObject,
+  data: StorageData,
   namespace: string,
   editName?: string,
   hideFromUI?: boolean,
 ): PersistentVolumeClaimKind => {
-  const {
-    nameDesc: { name: pvcName, description },
-    size,
-    storageClassName,
-  } = data;
-
+  const { name: pvcName, description, size, storageClassName } = data;
   const name = editName || translateDisplayNameForK8s(pvcName);
 
   return {
@@ -41,14 +36,14 @@ export const assemblePvc = (
       }),
       annotations: {
         'openshift.io/display-name': pvcName.trim(),
-        'openshift.io/description': description,
+        ...(description && { 'openshift.io/description': description }),
       },
     },
     spec: {
       accessModes: ['ReadWriteOnce'],
       resources: {
         requests: {
-          storage: size,
+          storage: String(size),
         },
       },
       storageClassName,
@@ -70,7 +65,7 @@ export const getDashboardPvcs = (projectName: string): Promise<PersistentVolumeC
   });
 
 export const createPvc = (
-  data: CreatingStorageObject,
+  data: StorageData,
   namespace: string,
   opts?: K8sAPIOptions,
   hideFromUI?: boolean,
@@ -83,7 +78,7 @@ export const createPvc = (
 };
 
 export const updatePvc = (
-  data: CreatingStorageObject,
+  data: StorageData,
   existingData: PersistentVolumeClaimKind,
   namespace: string,
   opts?: K8sAPIOptions,