Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[IA-4137] Add types to desired and existing runtime config #5079

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
+996 −179
Open

[IA-4137] Add types to desired and existing runtime config #5079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
147e687
add typing for existing runtime
Sep 11, 2024
85418a6
changes in place to support desired and existing runtime typing
Sep 11, 2024
7fd7634
lint
Sep 17, 2024
84b539e
lint
Sep 17, 2024
192644e
tests
Sep 17, 2024
9fd4766
fix tests
Sep 18, 2024
676b01a
pr feedback
Sep 19, 2024
96cf679
cleanup
Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 0 additions & 99 deletions src/analysis/modal-utils.ts
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion src/analysis/modals/ComputeModal/AzureComputeModal/AzurePersistentDiskSizeSelectInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useUniqueId } from '@terra-ui-packages/components';
import { ReactNode } from 'react';
import { div, h, label } from 'react-hyperscript-helpers';
import { IComputeConfig } from 'src/analysis/modal-utils';
import { IComputeConfig } from 'src/analysis/modals/modal-utils';
import { computeStyles } from 'src/analysis/modals/modalStyles';
import { Select } from 'src/components/common';
import { defaultAzureDiskSize } from 'src/libs/azure-utils';
Expand Down
92 changes: 21 additions & 71 deletions src/analysis/modals/ComputeModal/GcpComputeModal/GcpComputeModal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,20 @@ import { formatDate } from '@terra-ui-packages/core-utils';
import _ from 'lodash/fp';
import { Fragment, useEffect, useState } from 'react';
import { b, div, fieldset, h, label, legend, p, span, strong } from 'react-hyperscript-helpers';
import { buildExistingEnvironmentConfig } from 'src/analysis/modal-utils';
import { AboutPersistentDiskView } from 'src/analysis/modals/ComputeModal/AboutPersistentDiskView';
import { GcpComputeImageSection } from 'src/analysis/modals/ComputeModal/GcpComputeModal/GcpComputeImageSection';
import { GcpPersistentDiskSection } from 'src/analysis/modals/ComputeModal/GcpComputeModal/GcpPersistentDiskSection';
import { DeleteDiskChoices } from 'src/analysis/modals/DeleteDiskChoices';
import { DeleteEnvironment } from 'src/analysis/modals/DeleteEnvironment';
import {
buildDesiredEnvironmentConfig,
buildExistingEnvironmentConfig,
isDataproc,
isDataprocCluster,
isGce,
runtimeTypes,
shouldUsePersistentDisk,
} from 'src/analysis/modals/modal-utils';
import { WarningTitle } from 'src/analysis/modals/WarningTitle';
import { SaveFilesHelp, SaveFilesHelpRStudio } from 'src/analysis/runtime-common-text';
import { getPersistentDiskCostMonthly, runtimeConfigBaseCost, runtimeConfigCost } from 'src/analysis/utils/cost-utils';
Expand Down Expand Up @@ -40,7 +48,6 @@ import {
getValidGpuOptions,
getValidGpuTypesForZone,
isAutopauseEnabled,
runtimeTypes,
} from 'src/analysis/utils/runtime-utils';
import { getToolLabelFromCloudEnv, runtimeToolLabels } from 'src/analysis/utils/tool-utils';
import { ClipboardButton } from 'src/components/ClipboardButton';
Expand Down Expand Up @@ -198,15 +205,6 @@ const SparkInterface = ({ sparkInterface, namespace, name, onDismiss }) => {
};
// Auxiliary components -- end

// Auxiliary functions -- begin
const isGce = (runtimeType) => runtimeType === runtimeTypes.gceVm;

const isDataproc = (runtimeType) => runtimeType === runtimeTypes.dataprocSingleNode || runtimeType === runtimeTypes.dataprocCluster;

const isDataprocCluster = (runtimeType) => runtimeType === runtimeTypes.dataprocCluster;

const shouldUsePersistentDisk = (runtimeType, runtimeDetails, upgradeDiskSelected) =>
isGce(runtimeType) && (!runtimeDetails?.runtimeConfig?.diskSize || upgradeDiskSelected);
// Auxiliary functions -- end

export const GcpComputeModalBase = ({
Expand Down Expand Up @@ -471,67 +469,19 @@ export const GcpComputeModalBase = ({
const getExistingEnvironmentConfig = () =>
buildExistingEnvironmentConfig(computeConfig, currentRuntimeDetails, currentPersistentDiskDetails, isDataproc(runtimeType));

const getDesiredEnvironmentConfig = () => {
const { persistentDisk: existingPersistentDisk, runtime: existingRuntime } = getExistingEnvironmentConfig();
const cloudService = isDataproc(runtimeType) ? cloudServices.DATAPROC : cloudServices.GCE;
const desiredNumberOfWorkers = isDataprocCluster(runtimeType) ? computeConfig.numberOfWorkers : 0;
const getDesiredEnvironmentParams = () => ({
desiredRuntimeType: runtimeType,
timeoutInMinutes,
deleteDiskSelected,
upgradeDiskSelected,
jupyterUserScriptUri,
isCustomSelectedImage,
customImageUrl,
selectedImage,
});

return {
hasGpu: computeConfig.hasGpu,
autopauseThreshold: computeConfig.autopauseThreshold,
runtime: Utils.cond(
[
viewMode !== 'deleteEnvironment',
() => {
return {
cloudService,
toolDockerImage: isCustomSelectedImage ? customImageUrl : selectedImage?.url,
tool: selectedImage?.toolLabel ?? getToolLabelFromCloudEnv(existingRuntime),
...(jupyterUserScriptUri ? { jupyterUserScriptUri } : {}),
...(timeoutInMinutes ? { timeoutInMinutes } : {}),
...(cloudService === cloudServices.GCE
? {
zone: computeConfig.computeZone,
region: computeConfig.computeRegion,
machineType: computeConfig.masterMachineType || getDefaultMachineType(false, getToolLabelFromCloudEnv(existingRuntime)),
...(computeConfig.gpuEnabled ? { gpuConfig: { gpuType: computeConfig.gpuType, numOfGpus: computeConfig.numGpus } } : {}),
bootDiskSize: computeConfig.bootDiskSize,
...(shouldUsePersistentDisk(runtimeType, currentRuntimeDetails, upgradeDiskSelected)
? {
persistentDiskAttached: true,
}
: {
diskSize: computeConfig.masterDiskSize,
}),
}
: {
region: computeConfig.computeRegion,
masterMachineType: computeConfig.masterMachineType || defaultDataprocMachineType,
masterDiskSize: computeConfig.masterDiskSize,
numberOfWorkers: desiredNumberOfWorkers,
componentGatewayEnabled: computeConfig.componentGatewayEnabled,
...(desiredNumberOfWorkers && {
numberOfPreemptibleWorkers: computeConfig.numberOfPreemptibleWorkers,
workerMachineType: computeConfig.workerMachineType || defaultDataprocMachineType,
workerDiskSize: computeConfig.workerDiskSize,
}),
}),
};
},
],
[!deleteDiskSelected || existingRuntime?.persistentDiskAttached, () => undefined],
() => existingRuntime
),
persistentDisk: Utils.cond(
[deleteDiskSelected, () => undefined],
[
viewMode !== 'deleteEnvironment' && shouldUsePersistentDisk(runtimeType, currentRuntimeDetails, upgradeDiskSelected),
() => ({ size: computeConfig.diskSize, diskType: computeConfig.diskType }),
],
() => existingPersistentDisk
),
};
};
// TODO: converge type with getExistingEnvironmentConfig
const getDesiredEnvironmentConfig = () => buildDesiredEnvironmentConfig(getExistingEnvironmentConfig(), viewMode, getDesiredEnvironmentParams());

/**
* Transforms the new environment config into the shape of a disk returned
Expand Down
9 changes: 8 additions & 1 deletion src/analysis/modals/ComputeModal/GcpComputeModal/GcpComputeModal.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ describe('GcpComputeModal', () => {
screen.getByDisplayValue(disk.size);

verifyEnabled(screen.getByText('Delete Environment'));
verifyEnabled(screen.getByText('Update'));
verifyDisabled(screen.getByText('Update'));
}
);

Expand All @@ -377,6 +377,7 @@ describe('GcpComputeModal', () => {
const disk = getDisk();
const runtimeProps = { status: status.leoLabel, runtimeConfig: getJupyterRuntimeConfig({ diskId: disk.id }) };
const runtime = getGoogleRuntime(runtimeProps);
const user = userEvent.setup();

const runtimeFunc = jest.fn(() => ({
details: () => runtime,
Expand All @@ -403,6 +404,12 @@ describe('GcpComputeModal', () => {
selectRuntimeImage(GcpComputeImageSection, runtime);
});

await user.click(screen.getByLabelText('CPUs'));
const selectOption = await screen.findByText('2');
await user.click(selectOption);
const nextButton = await screen.findByText('Next');
await user.click(nextButton);
Copy link
Contributor Author

@jdcanas jdcanas Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was actually a bug that these tests relied on to pass that was fixed.

Essentially, the canUpdateRuntime function compares existing and desired configs and if it finds a difference say 'yes you can update'. The toolLabel in the desired config was incorrectly computed, and was undefined, which signaled to the modal an update was allowed (hence update button Enabled). The test on line 369 now correctly reads update should be disabled when no changes have been made, and this subsequent test actually requires a field to be changed (CPUs) for Update to be enabled.

before (note the undefined):
Pasted Graphic

now (note two objects are now identical):
image


// Assert
if (status.canChangeCompute) {
verifyEnabled(screen.getByText('Update'));
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/modals/ComputeModal/PersistentDiskTypeInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Select, SelectProps, useUniqueId } from '@terra-ui-packages/components'
import { DiskType } from '@terra-ui-packages/leonardo-data-client';
import { ReactNode } from 'react';
import { div, h, label } from 'react-hyperscript-helpers';
import { IComputeConfig } from 'src/analysis/modal-utils';
import { IComputeConfig } from 'src/analysis/modals/modal-utils';
import { computeStyles } from 'src/analysis/modals/modalStyles';

type PersistentDiskTypeSelectProps<
Expand Down
Loading
Loading