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 Oct 14, 2024
1 parent 6ec1b1b commit a1db6ec
Show file tree
Hide file tree
Showing 14 changed files with 546 additions and 209 deletions.
22 changes: 12 additions & 10 deletions frontend/src/__mocks__/mockStartNotebookData.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
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 @@ -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');

Expand All @@ -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() {
Expand Down Expand Up @@ -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;
Expand Down
36 changes: 10 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 @@ -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();
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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');

Expand Down
43 changes: 38 additions & 5 deletions frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,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;
Expand Down Expand Up @@ -83,6 +84,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 = {
Expand All @@ -103,8 +109,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,
Expand Down Expand Up @@ -137,7 +143,7 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
const pvcDetails = await replaceRootVolumesForNotebook(
projectName,
editNotebook,
storageData,
rootPathStorageData,
dryRun,
);

Expand Down Expand Up @@ -193,7 +199,34 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
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;

Check warning on line 213 in frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx#L212-L213 

Added lines #L212 - L213 were not covered by tests
}

if (response.volumeMounts.length) {
acc.volumeMounts = acc.volumeMounts.concat(response.volumeMounts);
} else {
acc.volumeMounts = response.volumeMounts;

Check warning on line 219 in frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx#L218-L219 

Added lines #L218 - L219 were not covered by tests
}

return acc;
},
{
volumes: [],
volumeMounts: [],
},
);

const envFrom = await createConfigMapsAndSecretsForNotebook(
projectName,
[...envVariables, ...newDataConnection],
Expand Down
68 changes: 48 additions & 20 deletions frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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]}
Expand Down
Loading

0 comments on commit a1db6ec

Please sign in to comment.