Skip to content

Commit

Permalink
[RHOAIENG-1101] Cannot remove additional storage from the workbench c…
Browse files Browse the repository at this point in the history 
…onfiguration
  • Loading branch information
@jpuzz0
jpuzz0 committed Nov 13, 2024
1 parent 4ce7e5b commit c3ad890
Show file tree
Hide file tree
Showing 37 changed files with 935 additions and 908 deletions.
15 changes: 6 additions & 9 deletions frontend/src/__mocks__/mockStartNotebookData.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
8 changes: 4 additions & 4 deletions frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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) {
Expand All @@ -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() {
Expand All @@ -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() {
Expand Down
50 changes: 35 additions & 15 deletions frontend/src/__tests__/cypress/cypress/pages/workbench.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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() {
Expand Down Expand Up @@ -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;
Expand Down
92 changes: 6 additions & 86 deletions frontend/src/__tests__/cypress/cypress/tests/mocked/projects/clusterStorage.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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');
Expand Down
62 changes: 36 additions & 26 deletions frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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');

Expand All @@ -702,9 +686,35 @@ describe('Workbench page', () => {
editSpawnerPage.findAcceleratorProfileSelect().findSelectOption('None').click();
editSpawnerPage.findAcceleratorProfileSelect().should('contain', 'None');

cy.interceptK8s('PUT', PVCModel, mockPVCK8sResource({ name: 'test-notebook' })).as(
'editClusterStorage',
);
cy.interceptK8s('PUT', NotebookModel, mockNotebookK8sResource({})).as('editWorkbenchDryRun');
cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})).as('editWorkbench');

editSpawnerPage.findSubmitButton().click();

cy.wait('@editClusterStorage').then((interception) => {
expect(interception.request.url).to.include('?dryRun=All');
expect(interception.request.body).to.containSubset({
metadata: {
annotations: {
'openshift.io/description': '',
'openshift.io/display-name': 'Test Storage',
},
name: 'test-notebook',
namespace: 'test-project',
labels: {
'opendatahub.io/dashboard': 'true',
},
},
spec: {
resources: { requests: { storage: '5Gi' } },
},
status: { phase: 'Pending', accessModes: ['ReadWriteOnce'], capacity: { storage: '5Gi' } },
});
});

cy.wait('@editWorkbenchDryRun').then((interception) => {
expect(interception.request.url).to.include('?dryRun=All');
expect(interception.request.body).to.containSubset({
Expand Down
10 changes: 4 additions & 6 deletions frontend/src/api/k8s/__tests__/pvcs.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { mockPVCK8sResource } from '~/__mocks__/mockPVCK8sResource';
import { assemblePvc, createPvc, deletePvc, getDashboardPvcs, updatePvc } 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(),
Expand All @@ -26,11 +26,9 @@ const k8sCreateResourceMock = jest.mocked(k8sCreateResource<PersistentVolumeClai
const k8sUpdateResourceMock = jest.mocked(k8sUpdateResource<PersistentVolumeClaimKind>);
const k8sDeleteResourceMock = jest.mocked(k8sDeleteResource<PersistentVolumeClaimKind, K8sStatus>);

const data: CreatingStorageObject = {
nameDesc: {
name: 'pvc',
description: 'Test Storage',
},
const data: StorageData = {
name: 'pvc',
description: 'Test Storage',
size: '5Gi',
};

Expand Down
Loading

0 comments on commit c3ad890

Please sign in to comment.