From e3e24686a5d6984381d161a0335b45b7a0c996a1 Mon Sep 17 00:00:00 2001 From: Mattia Franchetto Date: Wed, 19 Oct 2022 14:01:11 +0200 Subject: [PATCH 01/12] Extract feature flags logic into a function to import in business logic code --- frontend/src/feature-flags/useFeatureFlag.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frontend/src/feature-flags/useFeatureFlag.ts b/frontend/src/feature-flags/useFeatureFlag.ts index af68bfd7..959178f6 100644 --- a/frontend/src/feature-flags/useFeatureFlag.ts +++ b/frontend/src/feature-flags/useFeatureFlag.ts @@ -14,6 +14,13 @@ import {AvailableFeature} from './types' export function useFeatureFlag(feature: AvailableFeature): boolean { const version = useState(['app', 'version', 'full']) + return isFeatureEnabled(version, feature) +} + +export function isFeatureEnabled( + version: string, + feature: AvailableFeature, +): boolean { const features = new Set(featureFlagsProvider(version)) return features.has(feature) From fbdf5295d8dbdee0f8ddc4f25a2f466eff8cbaab Mon Sep 17 00:00:00 2001 From: Mattia Franchetto Date: Wed, 19 Oct 2022 14:01:55 +0200 Subject: [PATCH 02/12] Add FF for multiple instance types --- frontend/src/feature-flags/featureFlagsProvider.ts | 2 +- frontend/src/feature-flags/types.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/src/feature-flags/featureFlagsProvider.ts b/frontend/src/feature-flags/featureFlagsProvider.ts index 50487a95..e41f709e 100644 --- a/frontend/src/feature-flags/featureFlagsProvider.ts +++ b/frontend/src/feature-flags/featureFlagsProvider.ts @@ -20,7 +20,7 @@ const versionToFeaturesMap: Record = { 'multiuser_cluster', 'slurm_queue_update_strategy', ], - '3.3.0': ['slurm_accounting'], + '3.3.0': ['slurm_accounting', 'queues_multiple_instance_types'], } function composeFlagsListByVersion(currentVersion: string): AvailableFeature[] { diff --git a/frontend/src/feature-flags/types.ts b/frontend/src/feature-flags/types.ts index 064626d6..acf5d221 100644 --- a/frontend/src/feature-flags/types.ts +++ b/frontend/src/feature-flags/types.ts @@ -16,3 +16,4 @@ export type AvailableFeature = | 'memory_based_scheduling' | 'slurm_accounting' | 'slurm_queue_update_strategy' + | 'queues_multiple_instance_types' From db4d764fa29be21aab69872935a05159f546a533 Mon Sep 17 00:00:00 2001 From: Mattia Franchetto Date: Wed, 19 Oct 2022 14:02:48 +0200 Subject: [PATCH 03/12] Extract compute resource logic into separate functions --- frontend/src/old-pages/Configure/Queues.tsx | 348 ++++-------------- .../SingleInstanceComputeResource.tsx | 301 +++++++++++++++ .../src/old-pages/Configure/queues.types.ts | 11 + 3 files changed, 376 insertions(+), 284 deletions(-) create mode 100644 frontend/src/old-pages/Configure/SingleInstanceComputeResource.tsx create mode 100644 frontend/src/old-pages/Configure/queues.types.ts diff --git a/frontend/src/old-pages/Configure/Queues.tsx b/frontend/src/old-pages/Configure/Queues.tsx index 1515adc7..bde2a7d5 100644 --- a/frontend/src/old-pages/Configure/Queues.tsx +++ b/frontend/src/old-pages/Configure/Queues.tsx @@ -35,23 +35,25 @@ import {setState, getState, useState, clearState} from '../../store' import { ActionsEditor, CustomAMISettings, - InstanceSelect, LabeledIcon, RootVolume, SubnetSelect, SecurityGroups, IamPoliciesEditor, - HelpTextInput, } from './Components' -import HelpTooltip from '../../components/HelpTooltip' import {Trans, useTranslation} from 'react-i18next' import {SlurmMemorySettings} from './SlurmMemorySettings' -import {useFeatureFlag} from '../../feature-flags/useFeatureFlag' +import { + isFeatureEnabled, + useFeatureFlag, +} from '../../feature-flags/useFeatureFlag' +import * as SingleInstanceCR from './SingleInstanceComputeResource' +import * as MultiInstanceCR from './MultiInstanceComputeResource' +import {ComputeResource} from './queues.types' // Constants const queuesPath = ['app', 'wizard', 'config', 'Scheduling', 'SlurmQueues'] const queuesErrorsPath = ['app', 'wizard', 'errors', 'queues'] -const defaultInstanceType = 'c5n.large' // Helper Functions // @ts-expect-error TS(7031) FIXME: Binding element 'value' implicitly has an 'any' ty... Remove this comment to see the full error message @@ -179,19 +181,30 @@ function queueValidate(queueIndex: any) { setState([...errorsPath, 'subnet'], null) } - let seenInstances = new Set() - for (let i = 0; i < computeResources.length; i++) { - let computeResource = computeResources[i] - if (seenInstances.has(computeResource.InstanceType)) { - setState( - [...errorsPath, 'computeResource', i, 'type'], - i18next.t('wizard.queues.validation.instanceTypeUnique'), - ) - valid = false - } else { - seenInstances.add(computeResource.InstanceType) - setState([...errorsPath, 'computeResource', i, 'type'], null) - } + const version = getState(['app', 'version', 'full']) + const isMultiInstanceTypesActive = isFeatureEnabled( + version, + 'queues_multiple_instance_types', + ) + // TODO: call multiple instance validator + const {validateComputeResources} = !isMultiInstanceTypesActive + ? SingleInstanceCR + : SingleInstanceCR + const [computeResourcesValid, computeResourcesErrors] = + validateComputeResources(computeResources) + if (!computeResourcesValid) { + valid = false + computeResources.forEach((_: ComputeResource, i: number) => { + const error = computeResourcesErrors[i] + if (error) { + setState( + [...errorsPath, 'computeResource', i, 'type'], + i18next.t('wizard.queues.validation.instanceTypeUnique'), + ) + } else { + setState([...errorsPath, 'computeResource', i, 'type'], null) + } + }) } return valid @@ -213,254 +226,12 @@ function queuesValidate() { return valid } -function ComputeResource({index, queueIndex, computeResource}: any) { - const parentPath = [...queuesPath, queueIndex] - const queue = useState(parentPath) - const computeResources = useState([...parentPath, 'ComputeResources']) - const path = [...parentPath, 'ComputeResources', index] - const errorsPath = [...queuesErrorsPath, queueIndex, 'computeResource', index] - const typeError = useState([...errorsPath, 'type']) - - const tInstances = new Set(['t2.micro', 't2.medium']) - const gravitonInstances = new Set([]) - - const instanceTypePath = [...path, 'InstanceType'] - const instanceType: string = useState(instanceTypePath) - const memoryBasedSchedulingEnabledPath = [ - 'app', - 'wizard', - 'config', - 'Scheduling', - 'SlurmSettings', - 'EnableMemoryBasedScheduling', - ] - const enableMemoryBasedScheduling = useState(memoryBasedSchedulingEnabledPath) - - const disableHTPath = [...path, 'DisableSimultaneousMultithreading'] - const disableHT = useState(disableHTPath) - - const efaPath = [...path, 'Efa'] - - const efaInstances = new Set(useState(['aws', 'efa_instance_types'])) - const enableEFAPath = [...path, 'Efa', 'Enabled'] - const enableEFA = useState(enableEFAPath) || false - - const enablePlacementGroupPath = [ - ...parentPath, - 'Networking', - 'PlacementGroup', - 'Enabled', - ] - - const enableGPUDirectPath = [...path, 'Efa', 'GdrSupport'] || false - const enableGPUDirect = useState(enableGPUDirectPath) - - const instanceSupportsGdr = instanceType === 'p4d.24xlarge' - - const minCount = useState([...path, 'MinCount']) - const maxCount = useState([...path, 'MaxCount']) - - const {t} = useTranslation() - - const remove = () => { - setState( - [...parentPath, 'ComputeResources'], - [ - ...computeResources.slice(0, index), - ...computeResources.slice(index + 1), - ], - ) - } - - const setMinCount = (staticCount: any) => { - const dynamicCount = maxCount - minCount - if (staticCount > 0) - setState([...path, 'MinCount'], !isNaN(staticCount) ? staticCount : 0) - else clearState([...path, 'MinCount']) - setState( - [...path, 'MaxCount'], - (!isNaN(staticCount) ? staticCount : 0) + - (!isNaN(dynamicCount) ? dynamicCount : 0), - ) - } - - const setMaxCount = (dynamicCount: any) => { - const staticCount = minCount - setState( - [...path, 'MaxCount'], - (!isNaN(staticCount) ? staticCount : 0) + - (!isNaN(dynamicCount) ? dynamicCount : 0), - ) - } - - const setSchedulableMemory = ( - schedulableMemoryPath: string[], - schedulableMemory: string, - ) => { - let schedulableMemoryNumber = parseInt(schedulableMemory) - if (enableMemoryBasedScheduling && !isNaN(schedulableMemoryNumber)) { - setState(schedulableMemoryPath, schedulableMemoryNumber) - } else { - clearState(schedulableMemoryPath) - } - } - - const setDisableHT = (disable: any) => { - if (disable) setState(disableHTPath, disable) - else clearState(disableHTPath) - } - - const setEnableEFA = (enable: any) => { - if (enable) { - setState(enableEFAPath, enable) - setState(enablePlacementGroupPath, enable) - } else { - clearState(efaPath) - clearState(enablePlacementGroupPath) - } - } - - const setEnableGPUDirect = (enable: any) => { - if (enable) setState(enableGPUDirectPath, enable) - else clearState(enableGPUDirectPath) - } - - const setInstanceType = (instanceType: any) => { - // setting the instance type on the queue happens in the component - // this updates the name which is derived from the instance type - setState( - [...path, 'Name'], - `${queue.Name}-${instanceType.replace('.', '')}`, - ) - - if (!getState(enableEFAPath) && efaInstances.has(instanceType)) - setEnableEFA(true) - else if (getState(enableEFAPath) && !efaInstances.has(instanceType)) - setEnableEFA(false) - } - - React.useEffect(() => { - if (!instanceType) - setState( - [...queuesPath, queueIndex, 'ComputeResources', index, 'InstanceType'], - defaultInstanceType, - ) - }, [queueIndex, index, instanceType]) - - return ( -
-
- - {index > 0 && } - - -
- - setMinCount(parseInt(detail.value))} - /> - - - setMaxCount(parseInt(detail.value))} - /> - -
- - - - {enableMemoryBasedScheduling && ( - - setSchedulableMemory( - [...path, 'SchedulableMemory'], - detail.value, - ) - } - description={t('wizard.queues.schedulableMemory.description')} - placeholder={t('wizard.queues.schedulableMemory.placeholder')} - help={t('wizard.queues.schedulableMemory.help')} - type="number" - /> - )} - -
- { - setDisableHT(!disableHT) - }} - > - - - { - setEnableEFA(!enableEFA) - }} - > - - -
- { - setEnableGPUDirect(!enableGPUDirect) - }} - > - - - - - - - -
-
-
-
- ) -} - function ComputeResources({queue, index}: any) { + const {ViewComponent} = useComputeResourceAdapter() return ( {queue.ComputeResources.map((computeResource: any, i: any) => ( - { @@ -675,17 +443,7 @@ function Queues() { ...(queues || []), { Name: `queue${queues.length}`, - ComputeResources: [ - { - Name: `queue${queues.length}-${defaultInstanceType.replace( - '.', - '', - )}`, - MinCount: 0, - MaxCount: 4, - InstanceType: defaultInstanceType, - }, - ], + ComputeResources: [adapter.createComputeResource(queues.length)], }, ], ) @@ -716,4 +474,26 @@ function Queues() { ) } +const useComputeResourceAdapter = () => { + const isMultiInstanceTypesActive = useFeatureFlag( + 'queues_multiple_instance_types', + ) + return !isMultiInstanceTypesActive + ? { + ViewComponent: SingleInstanceCR.ComputeResource, + updateComputeResourcesNames: + SingleInstanceCR.updateComputeResourcesNames, + createComputeResource: SingleInstanceCR.createComputeResource, + validateComputeResources: SingleInstanceCR.validateComputeResources, + } + : { + // TODO: implement multi instance variant + ViewComponent: SingleInstanceCR.ComputeResource, + updateComputeResourcesNames: + SingleInstanceCR.updateComputeResourcesNames, + createComputeResource: SingleInstanceCR.createComputeResource, + validateComputeResources: SingleInstanceCR.validateComputeResources, + } +} + export {Queues, queuesValidate} diff --git a/frontend/src/old-pages/Configure/SingleInstanceComputeResource.tsx b/frontend/src/old-pages/Configure/SingleInstanceComputeResource.tsx new file mode 100644 index 00000000..7c5327bf --- /dev/null +++ b/frontend/src/old-pages/Configure/SingleInstanceComputeResource.tsx @@ -0,0 +1,301 @@ +import { + Box, + Button, + ColumnLayout, + FormField, + Input, + Toggle, +} from '@awsui/components-react' +import {useEffect} from 'react' +import {Trans, useTranslation} from 'react-i18next' +import HelpTooltip from '../../components/HelpTooltip' +import {clearState, getState, setState, useState} from '../../store' +import {HelpTextInput, InstanceSelect} from './Components' +import { + QueueValidationErrors, + SingleInstanceComputeResource, +} from './queues.types' + +const queuesPath = ['app', 'wizard', 'config', 'Scheduling', 'SlurmQueues'] +const queuesErrorsPath = ['app', 'wizard', 'errors', 'queues'] +const defaultInstanceType = 'c5n.large' + +export function ComputeResource({index, queueIndex, computeResource}: any) { + const parentPath = [...queuesPath, queueIndex] + const queue = useState(parentPath) + const computeResources = useState([...parentPath, 'ComputeResources']) + const path = [...parentPath, 'ComputeResources', index] + const errorsPath = [...queuesErrorsPath, queueIndex, 'computeResource', index] + const typeError = useState([...errorsPath, 'type']) + + const tInstances = new Set(['t2.micro', 't2.medium']) + const gravitonInstances = new Set([]) + + const instanceTypePath = [...path, 'InstanceType'] + const instanceType: string = useState(instanceTypePath) + const memoryBasedSchedulingEnabledPath = [ + 'app', + 'wizard', + 'config', + 'Scheduling', + 'SlurmSettings', + 'EnableMemoryBasedScheduling', + ] + const enableMemoryBasedScheduling = useState(memoryBasedSchedulingEnabledPath) + + const disableHTPath = [...path, 'DisableSimultaneousMultithreading'] + const disableHT = useState(disableHTPath) + + const efaPath = [...path, 'Efa'] + + const efaInstances = new Set(useState(['aws', 'efa_instance_types'])) + const enableEFAPath = [...path, 'Efa', 'Enabled'] + const enableEFA = useState(enableEFAPath) || false + + const enablePlacementGroupPath = [ + ...parentPath, + 'Networking', + 'PlacementGroup', + 'Enabled', + ] + + const enableGPUDirectPath = [...path, 'Efa', 'GdrSupport'] || false + const enableGPUDirect = useState(enableGPUDirectPath) + + const instanceSupportsGdr = instanceType === 'p4d.24xlarge' + + const minCount = useState([...path, 'MinCount']) + const maxCount = useState([...path, 'MaxCount']) + + const {t} = useTranslation() + + const remove = () => { + setState( + [...parentPath, 'ComputeResources'], + [ + ...computeResources.slice(0, index), + ...computeResources.slice(index + 1), + ], + ) + } + + const setMinCount = (staticCount: any) => { + const dynamicCount = maxCount - minCount + if (staticCount > 0) + setState([...path, 'MinCount'], !isNaN(staticCount) ? staticCount : 0) + else clearState([...path, 'MinCount']) + setState( + [...path, 'MaxCount'], + (!isNaN(staticCount) ? staticCount : 0) + + (!isNaN(dynamicCount) ? dynamicCount : 0), + ) + } + + const setMaxCount = (dynamicCount: any) => { + const staticCount = minCount + setState( + [...path, 'MaxCount'], + (!isNaN(staticCount) ? staticCount : 0) + + (!isNaN(dynamicCount) ? dynamicCount : 0), + ) + } + + const setSchedulableMemory = ( + schedulableMemoryPath: string[], + schedulableMemory: string, + ) => { + let schedulableMemoryNumber = parseInt(schedulableMemory) + if (enableMemoryBasedScheduling && !isNaN(schedulableMemoryNumber)) { + setState(schedulableMemoryPath, schedulableMemoryNumber) + } else { + clearState(schedulableMemoryPath) + } + } + + const setDisableHT = (disable: any) => { + if (disable) setState(disableHTPath, disable) + else clearState(disableHTPath) + } + + const setEnableEFA = (enable: any) => { + if (enable) { + setState(enableEFAPath, enable) + setState(enablePlacementGroupPath, enable) + } else { + clearState(efaPath) + clearState(enablePlacementGroupPath) + } + } + + const setEnableGPUDirect = (enable: any) => { + if (enable) setState(enableGPUDirectPath, enable) + else clearState(enableGPUDirectPath) + } + + const setInstanceType = (instanceType: any) => { + // setting the instance type on the queue happens in the component + // this updates the name which is derived from the instance type + setState( + [...path, 'Name'], + `${queue.Name}-${instanceType.replace('.', '')}`, + ) + + if (!getState(enableEFAPath) && efaInstances.has(instanceType)) + setEnableEFA(true) + else if (getState(enableEFAPath) && !efaInstances.has(instanceType)) + setEnableEFA(false) + } + + useEffect(() => { + if (!instanceType) + setState( + [...queuesPath, queueIndex, 'ComputeResources', index, 'InstanceType'], + defaultInstanceType, + ) + }, [queueIndex, index, instanceType]) + + return ( +
+
+ + {index > 0 && } + + +
+ + setMinCount(parseInt(detail.value))} + /> + + + setMaxCount(parseInt(detail.value))} + /> + +
+ + + + {enableMemoryBasedScheduling && ( + + setSchedulableMemory( + [...path, 'SchedulableMemory'], + detail.value, + ) + } + description={t('wizard.queues.schedulableMemory.description')} + placeholder={t('wizard.queues.schedulableMemory.placeholder')} + help={t('wizard.queues.schedulableMemory.help')} + type="number" + /> + )} + +
+ { + setDisableHT(!disableHT) + }} + > + + + { + setEnableEFA(!enableEFA) + }} + > + + +
+ { + setEnableGPUDirect(!enableGPUDirect) + }} + > + + + + + + + +
+
+
+
+ ) +} + +export function createComputeResource(index: number) { + return { + Name: `queue${index}-${defaultInstanceType.replace('.', '')}`, + InstanceType: defaultInstanceType, + MinCount: 0, + MaxCount: 4, + } +} + +export function updateComputeResourcesNames( + computeResources: SingleInstanceComputeResource[], + newQueueName: string, +) { + return computeResources.map(cr => ({ + ...cr, + Name: `${newQueueName}-${cr.InstanceType.replace('.', '')}`, + })) +} + +export function validateComputeResources( + computeResources: SingleInstanceComputeResource[], +): [boolean, QueueValidationErrors] { + let seenInstances = new Set() + let errors: QueueValidationErrors = {} + let valid = true + for (let i = 0; i < computeResources.length; i++) { + let computeResource = computeResources[i] + if (seenInstances.has(computeResource.InstanceType)) { + errors[i] = 'instance_type_unique' + valid = false + } else { + seenInstances.add(computeResource.InstanceType) + } + } + return [valid, errors] +} diff --git a/frontend/src/old-pages/Configure/queues.types.ts b/frontend/src/old-pages/Configure/queues.types.ts new file mode 100644 index 00000000..85027813 --- /dev/null +++ b/frontend/src/old-pages/Configure/queues.types.ts @@ -0,0 +1,11 @@ +export type QueueValidationErrors = Record + +export type ComputeResource = { + Name: string + MinCount: number + MaxCount: number +} + +export type SingleInstanceComputeResource = ComputeResource & { + InstanceType: string +} From 92f2f0db3ccc78ff1c26e704bdf0398aa02f309d Mon Sep 17 00:00:00 2001 From: Mattia Franchetto Date: Thu, 20 Oct 2022 14:50:14 +0200 Subject: [PATCH 04/12] Implement multi instance types compute resources --- frontend/locales/en/strings.json | 1 + frontend/src/old-pages/Configure/Cluster.tsx | 11 +- .../src/old-pages/Configure/Components.tsx | 25 +- .../MultiInstanceComputeResource.tsx | 326 ++++++++++++++++++ frontend/src/old-pages/Configure/Queues.tsx | 25 +- .../src/old-pages/Configure/queues.types.ts | 12 +- 6 files changed, 364 insertions(+), 36 deletions(-) create mode 100644 frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx diff --git a/frontend/locales/en/strings.json b/frontend/locales/en/strings.json index 17d268ec..fa05580a 100644 --- a/frontend/locales/en/strings.json +++ b/frontend/locales/en/strings.json @@ -276,6 +276,7 @@ "queues": { "validation": { "instanceTypeUnique": "Instance types must be unique within a Queue.", + "instanceTypeMissing": "Select at least once instance type.", "selectSubnet": "You must select a Subnet.", "setRootVolumeSize": "You must set a RootVolume size.", "rootVolumeMinimum": "You must use an integer >= 35GB for Root Volume Size.", diff --git a/frontend/src/old-pages/Configure/Cluster.tsx b/frontend/src/old-pages/Configure/Cluster.tsx index 991c5c81..b991fcf4 100644 --- a/frontend/src/old-pages/Configure/Cluster.tsx +++ b/frontend/src/old-pages/Configure/Cluster.tsx @@ -34,6 +34,7 @@ import {LoadAwsConfig} from '../../model' // Components import {LabeledIcon, CustomAMISettings} from './Components' import {useFeatureFlag} from '../../feature-flags/useFeatureFlag' +import {useComputeResourceAdapter} from './Queues' // Constants const errorsPath = ['app', 'wizard', 'errors', 'cluster'] @@ -342,6 +343,7 @@ function Cluster() { let defaultRegion = useState(['aws', 'region']) || '' const region = useState(['app', 'selectedRegion']) || defaultRegion const isMultiuserClusterActive = useFeatureFlag('multiuser_cluster') + const {createComputeResource} = useComputeResourceAdapter() React.useEffect(() => { const configPath = ['app', 'wizard', 'config'] @@ -366,14 +368,7 @@ function Cluster() { [ { Name: 'queue0', - ComputeResources: [ - { - Name: 'queue0-t2-micro', - MinCount: 0, - MaxCount: 4, - InstanceType: 't2.micro', - }, - ], + ComputeResources: [createComputeResource(0)], }, ], ) diff --git a/frontend/src/old-pages/Configure/Components.tsx b/frontend/src/old-pages/Configure/Components.tsx index ef1e83e1..dd1085a4 100644 --- a/frontend/src/old-pages/Configure/Components.tsx +++ b/frontend/src/old-pages/Configure/Components.tsx @@ -176,21 +176,12 @@ function SubnetSelect({value, onChange, disabled}: any) { ) } -function InstanceSelect({path, selectId, callback, disabled}: any) { - const value = useState(path) || '' +// instance type, description, image +type InstanceGroup = [string, string, string][] +export function useInstanceGroups(): Record { const instanceTypes = useState(['aws', 'instanceTypes']) || [] - let groupNames = [ - 'General Purpose', - 'Compute', - 'HPC', - 'High Memory', - 'Graviton', - 'Mixed', - 'GPU', - ] - let groups: {[key: string]: [string, string, string][]} = {} for (let instance of instanceTypes) { @@ -227,8 +218,12 @@ function InstanceSelect({path, selectId, callback, disabled}: any) { groups[group].push([instance.InstanceType, desc, img]) } + return groups +} - groupNames = groupNames.filter(name => name in groups) +function InstanceSelect({path, selectId, callback, disabled}: any) { + const value = useState(path) || '' + const instanceGroups = useInstanceGroups() // @ts-expect-error TS(7031) FIXME: Binding element 'value' implicitly has an 'any' ty... Remove this comment to see the full error message const instanceToOption = ([value, label, icon]) => { @@ -255,10 +250,10 @@ function InstanceSelect({path, selectId, callback, disabled}: any) { ariaLabel="Instance Selector" placeholder="Instance Type" empty="No matches found" - options={groupNames.map(groupName => { + options={Object.keys(instanceGroups).map(groupName => { return { label: groupName, - options: groups[groupName].map(instanceToOption), + options: instanceGroups[groupName].map(instanceToOption), } })} /> diff --git a/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx b/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx new file mode 100644 index 00000000..6b48ec0e --- /dev/null +++ b/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx @@ -0,0 +1,326 @@ +import { + Box, + Button, + ColumnLayout, + FormField, + Input, + Multiselect, + Select, + Toggle, +} from '@awsui/components-react' +import {useCallback, useMemo} from 'react' +import {Trans, useTranslation} from 'react-i18next' +import {clearState, setState, useState} from '../../store' +import {HelpTextInput, useInstanceGroups} from './Components' +import { + CRAllocationStrategy, + MultiInstanceComputeResource, + QueueValidationErrors, +} from './queues.types' + +const queuesPath = ['app', 'wizard', 'config', 'Scheduling', 'SlurmQueues'] +const queuesErrorsPath = ['app', 'wizard', 'errors', 'queues'] +const defaultInstanceType = 'c5n.large' + +const useAllocationStrategyOptions = () => { + const options = useMemo( + () => [ + { + label: 'Lowest price', + value: 'lowest-price', + }, + { + label: 'Capacity optimized', + value: 'capacity-optimized', + }, + ], + [], + ) + return options +} + +function allInstancesSupportEFA( + instanceTypes: string[], + efaInstances: Set, +): boolean { + if (!instanceTypes || !instanceTypes.length) { + return false + } + return instanceTypes.every(instance => efaInstances.has(instance)) +} + +export function ComputeResource({index, queueIndex, computeResource}: any) { + const parentPath = useMemo(() => [...queuesPath, queueIndex], [queueIndex]) + const computeResources: MultiInstanceComputeResource[] = useState([ + ...parentPath, + 'ComputeResources', + ]) + const path = useMemo( + () => [...parentPath, 'ComputeResources', index], + [index, parentPath], + ) + const errorsPath = [...queuesErrorsPath, queueIndex, 'computeResource', index] + const typeError = useState([...errorsPath, 'type']) + const allocationStrategyOptions = useAllocationStrategyOptions() + + const instanceTypePath = useMemo(() => [...path, 'InstanceTypes'], [path]) + const instanceTypes: string[] = useState(instanceTypePath) + const allocationStrategy: CRAllocationStrategy = useState([ + ...path, + 'AllocationStrategy', + ]) + + const memoryBasedSchedulingEnabledPath = [ + 'app', + 'wizard', + 'config', + 'Scheduling', + 'SlurmSettings', + 'EnableMemoryBasedScheduling', + ] + const enableMemoryBasedScheduling = useState(memoryBasedSchedulingEnabledPath) + + const disableHTPath = [...path, 'DisableSimultaneousMultithreading'] + const disableHT = useState(disableHTPath) + + const efaPath = [...path, 'Efa'] + + const efaInstances = new Set(useState(['aws', 'efa_instance_types'])) + const enableEFAPath = [...path, 'Efa', 'Enabled'] + const enableEFA = useState(enableEFAPath) || false + + const enablePlacementGroupPath = [ + ...parentPath, + 'Networking', + 'PlacementGroup', + 'Enabled', + ] + + const minCount = useState([...path, 'MinCount']) + const maxCount = useState([...path, 'MaxCount']) + + const instanceGroups = useInstanceGroups() + const instanceOptions = useMemo( + () => + Object.keys(instanceGroups).map(groupName => { + return { + label: groupName, + options: instanceGroups[groupName].map(([value, label, icon]) => ({ + label: value, + iconUrl: icon, + description: label, + value: value, + })), + } + }), + [instanceGroups], + ) + + const {t} = useTranslation() + + const remove = () => { + setState( + [...parentPath, 'ComputeResources'], + [ + ...computeResources.slice(0, index), + ...computeResources.slice(index + 1), + ], + ) + } + + const setMinCount = (staticCount: any) => { + const dynamicCount = maxCount - minCount + if (staticCount > 0) + setState([...path, 'MinCount'], !isNaN(staticCount) ? staticCount : 0) + else clearState([...path, 'MinCount']) + setState( + [...path, 'MaxCount'], + (!isNaN(staticCount) ? staticCount : 0) + + (!isNaN(dynamicCount) ? dynamicCount : 0), + ) + } + + const setMaxCount = (dynamicCount: any) => { + const staticCount = minCount + setState( + [...path, 'MaxCount'], + (!isNaN(staticCount) ? staticCount : 0) + + (!isNaN(dynamicCount) ? dynamicCount : 0), + ) + } + + const setSchedulableMemory = ( + schedulableMemoryPath: string[], + schedulableMemory: string, + ) => { + let schedulableMemoryNumber = parseInt(schedulableMemory) + if (enableMemoryBasedScheduling && !isNaN(schedulableMemoryNumber)) { + setState(schedulableMemoryPath, schedulableMemoryNumber) + } else { + clearState(schedulableMemoryPath) + } + } + + const setDisableHT = (disable: any) => { + if (disable) setState(disableHTPath, disable) + else clearState(disableHTPath) + } + + const setEnableEFA = (enable: any) => { + if (enable) { + setState(enableEFAPath, enable) + setState(enablePlacementGroupPath, enable) + } else { + clearState(efaPath) + clearState(enablePlacementGroupPath) + } + } + + const setAllocationStrategy = useCallback( + ({detail}) => { + setState([...path, 'AllocationStrategy'], detail.selectedOption.value) + }, + [path], + ) + + const setInstanceTypes = useCallback( + ({detail}) => { + const selectedInstanceTypes = detail.selectedOptions.map( + option => option.value, + ) + setState(instanceTypePath, selectedInstanceTypes) + if (!allInstancesSupportEFA(selectedInstanceTypes, efaInstances)) { + setEnableEFA(false) + } + }, + [efaInstances, instanceTypePath, setEnableEFA], + ) + + return ( +
+
+ + {index > 0 && } + + +
+ + setMinCount(parseInt(detail.value))} + /> + + + setMaxCount(parseInt(detail.value))} + /> + +
+ + ({ + value: instance, + label: instance, + }))} + tokenLimit={3} + onChange={setInstanceTypes} + options={instanceOptions} + /> + + + Date: Thu, 20 Oct 2022 15:05:50 +0200 Subject: [PATCH 06/12] Fix TS errors --- .../MultiInstanceComputeResource.tsx | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx b/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx index 29d2f8ba..ed67158b 100644 --- a/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx +++ b/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx @@ -5,9 +5,11 @@ import { FormField, Input, Multiselect, + MultiselectProps, Select, Toggle, } from '@awsui/components-react' +import {NonCancelableEventHandler} from '@awsui/components-react/internal/events' import {useCallback, useMemo} from 'react' import {Trans, useTranslation} from 'react-i18next' import {clearState, setState, useState} from '../../store' @@ -167,15 +169,18 @@ export function ComputeResource({index, queueIndex, computeResource}: any) { else clearState(disableHTPath) } - const setEnableEFA = (enable: any) => { - if (enable) { - setState(enableEFAPath, enable) - setState(enablePlacementGroupPath, enable) - } else { - clearState(efaPath) - clearState(enablePlacementGroupPath) - } - } + const setEnableEFA = useCallback( + (enable: any) => { + if (enable) { + setState(enableEFAPath, enable) + setState(enablePlacementGroupPath, enable) + } else { + clearState(efaPath) + clearState(enablePlacementGroupPath) + } + }, + [efaPath, enablePlacementGroupPath, enableEFAPath], + ) const setAllocationStrategy = useCallback( ({detail}) => { @@ -184,18 +189,19 @@ export function ComputeResource({index, queueIndex, computeResource}: any) { [path], ) - const setInstanceTypes = useCallback( - ({detail}) => { - const selectedInstanceTypes = detail.selectedOptions.map( - option => option.value, - ) - setState(instanceTypePath, selectedInstanceTypes) - if (!allInstancesSupportEFA(selectedInstanceTypes, efaInstances)) { - setEnableEFA(false) - } - }, - [efaInstances, instanceTypePath, setEnableEFA], - ) + const setInstanceTypes: NonCancelableEventHandler = + useCallback( + ({detail}) => { + const selectedInstanceTypes = (detail.selectedOptions.map( + option => option.value, + ) || []) as string[] + setState(instanceTypePath, selectedInstanceTypes) + if (!allInstancesSupportEFA(selectedInstanceTypes, efaInstances)) { + setEnableEFA(false) + } + }, + [efaInstances, instanceTypePath, setEnableEFA], + ) return (
From 1acd5e0b9945715e21faa01cb5c081c11be61827 Mon Sep 17 00:00:00 2001 From: Mattia Franchetto Date: Thu, 20 Oct 2022 16:07:50 +0200 Subject: [PATCH 07/12] Fix naming when creating CRs --- .../old-pages/Configure/MultiInstanceComputeResource.tsx | 6 +++--- frontend/src/old-pages/Configure/Queues.tsx | 7 ++++--- .../old-pages/Configure/SingleInstanceComputeResource.tsx | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx b/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx index ed67158b..3e4af574 100644 --- a/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx +++ b/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx @@ -22,7 +22,6 @@ import { const queuesPath = ['app', 'wizard', 'config', 'Scheduling', 'SlurmQueues'] const queuesErrorsPath = ['app', 'wizard', 'errors', 'queues'] -const defaultInstanceType = 'c5n.large' const useAllocationStrategyOptions = () => { const {t} = useTranslation() @@ -299,10 +298,11 @@ export function ComputeResource({index, queueIndex, computeResource}: any) { } export function createComputeResource( - index: number, + queueIndex: number, + crIndex: number, ): MultiInstanceComputeResource { return { - Name: `queue${index}-${defaultInstanceType.replace('.', '')}`, + Name: `queue${queueIndex}-compute-resource-${crIndex}`, InstanceTypes: [], AllocationStrategy: 'lowest-price', MinCount: 0, diff --git a/frontend/src/old-pages/Configure/Queues.tsx b/frontend/src/old-pages/Configure/Queues.tsx index 4b7f7695..88f56f9b 100644 --- a/frontend/src/old-pages/Configure/Queues.tsx +++ b/frontend/src/old-pages/Configure/Queues.tsx @@ -280,11 +280,12 @@ function Queue({index}: any) { ) } const addComputeResource = () => { + const existingCRs = queue.ComputeResources || [] setState([...queuesPath, index], { ...queue, ComputeResources: [ - ...(queue.ComputeResources || []), - computeResourceAdapter.createComputeResource(index), + ...existingCRs, + computeResourceAdapter.createComputeResource(index, existingCRs.length), ], }) } @@ -445,7 +446,7 @@ function Queues() { ...(queues || []), { Name: `queue${queues.length}`, - ComputeResources: [adapter.createComputeResource(queues.length)], + ComputeResources: [adapter.createComputeResource(queues.length, 0)], }, ], ) diff --git a/frontend/src/old-pages/Configure/SingleInstanceComputeResource.tsx b/frontend/src/old-pages/Configure/SingleInstanceComputeResource.tsx index 7c5327bf..3eb70c2f 100644 --- a/frontend/src/old-pages/Configure/SingleInstanceComputeResource.tsx +++ b/frontend/src/old-pages/Configure/SingleInstanceComputeResource.tsx @@ -263,9 +263,9 @@ export function ComputeResource({index, queueIndex, computeResource}: any) { ) } -export function createComputeResource(index: number) { +export function createComputeResource(queueIndex: number, crIndex: number) { return { - Name: `queue${index}-${defaultInstanceType.replace('.', '')}`, + Name: `queue${queueIndex}-${defaultInstanceType.replace('.', '')}`, InstanceType: defaultInstanceType, MinCount: 0, MaxCount: 4, From 9dbab43e6f69cbec17fe54fcd8ff58cf094121b0 Mon Sep 17 00:00:00 2001 From: Mattia Franchetto Date: Thu, 20 Oct 2022 16:33:54 +0200 Subject: [PATCH 08/12] Move queues file into separate folder --- frontend/src/old-pages/Configure/Cluster.tsx | 2 +- frontend/src/old-pages/Configure/Configure.tsx | 2 +- .../{ => Queues}/MultiInstanceComputeResource.tsx | 4 ++-- frontend/src/old-pages/Configure/{ => Queues}/Queues.tsx | 8 ++++---- .../{ => Queues}/SingleInstanceComputeResource.tsx | 6 +++--- .../src/old-pages/Configure/{ => Queues}/queues.types.ts | 0 6 files changed, 11 insertions(+), 11 deletions(-) rename frontend/src/old-pages/Configure/{ => Queues}/MultiInstanceComputeResource.tsx (98%) rename frontend/src/old-pages/Configure/{ => Queues}/Queues.tsx (98%) rename frontend/src/old-pages/Configure/{ => Queues}/SingleInstanceComputeResource.tsx (98%) rename frontend/src/old-pages/Configure/{ => Queues}/queues.types.ts (100%) diff --git a/frontend/src/old-pages/Configure/Cluster.tsx b/frontend/src/old-pages/Configure/Cluster.tsx index b991fcf4..b5be0abc 100644 --- a/frontend/src/old-pages/Configure/Cluster.tsx +++ b/frontend/src/old-pages/Configure/Cluster.tsx @@ -34,7 +34,7 @@ import {LoadAwsConfig} from '../../model' // Components import {LabeledIcon, CustomAMISettings} from './Components' import {useFeatureFlag} from '../../feature-flags/useFeatureFlag' -import {useComputeResourceAdapter} from './Queues' +import {useComputeResourceAdapter} from './Queues/Queues' // Constants const errorsPath = ['app', 'wizard', 'errors', 'cluster'] diff --git a/frontend/src/old-pages/Configure/Configure.tsx b/frontend/src/old-pages/Configure/Configure.tsx index 89c10fa0..f74bca6b 100644 --- a/frontend/src/old-pages/Configure/Configure.tsx +++ b/frontend/src/old-pages/Configure/Configure.tsx @@ -38,7 +38,7 @@ import {Cluster, clusterValidate} from './Cluster' import {HeadNode, headNodeValidate} from './HeadNode' import {MultiUser, multiUserValidate} from './MultiUser' import {Storage, storageValidate} from './Storage' -import {Queues, queuesValidate} from './Queues' +import {Queues, queuesValidate} from './Queues/Queues' import { Create, createValidate, diff --git a/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx b/frontend/src/old-pages/Configure/Queues/MultiInstanceComputeResource.tsx similarity index 98% rename from frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx rename to frontend/src/old-pages/Configure/Queues/MultiInstanceComputeResource.tsx index 3e4af574..b62e0609 100644 --- a/frontend/src/old-pages/Configure/MultiInstanceComputeResource.tsx +++ b/frontend/src/old-pages/Configure/Queues/MultiInstanceComputeResource.tsx @@ -12,8 +12,8 @@ import { import {NonCancelableEventHandler} from '@awsui/components-react/internal/events' import {useCallback, useMemo} from 'react' import {Trans, useTranslation} from 'react-i18next' -import {clearState, setState, useState} from '../../store' -import {HelpTextInput, useInstanceGroups} from './Components' +import {clearState, setState, useState} from '../../../store' +import {HelpTextInput, useInstanceGroups} from '../Components' import { CRAllocationStrategy, MultiInstanceComputeResource, diff --git a/frontend/src/old-pages/Configure/Queues.tsx b/frontend/src/old-pages/Configure/Queues/Queues.tsx similarity index 98% rename from frontend/src/old-pages/Configure/Queues.tsx rename to frontend/src/old-pages/Configure/Queues/Queues.tsx index 88f56f9b..c77a0bcd 100644 --- a/frontend/src/old-pages/Configure/Queues.tsx +++ b/frontend/src/old-pages/Configure/Queues/Queues.tsx @@ -11,7 +11,7 @@ // limitations under the License. import * as React from 'react' import i18next from 'i18next' -import {findFirst} from '../../util' +import {findFirst} from '../../../util' // UI Elements import { @@ -29,7 +29,7 @@ import { } from '@awsui/components-react' // State -import {setState, getState, useState, clearState} from '../../store' +import {setState, getState, useState, clearState} from '../../../store' // Components import { @@ -40,13 +40,13 @@ import { SubnetSelect, SecurityGroups, IamPoliciesEditor, -} from './Components' +} from '../Components' import {Trans, useTranslation} from 'react-i18next' import {SlurmMemorySettings} from './SlurmMemorySettings' import { isFeatureEnabled, useFeatureFlag, -} from '../../feature-flags/useFeatureFlag' +} from '../../../feature-flags/useFeatureFlag' import * as SingleInstanceCR from './SingleInstanceComputeResource' import * as MultiInstanceCR from './MultiInstanceComputeResource' import {ComputeResource} from './queues.types' diff --git a/frontend/src/old-pages/Configure/SingleInstanceComputeResource.tsx b/frontend/src/old-pages/Configure/Queues/SingleInstanceComputeResource.tsx similarity index 98% rename from frontend/src/old-pages/Configure/SingleInstanceComputeResource.tsx rename to frontend/src/old-pages/Configure/Queues/SingleInstanceComputeResource.tsx index 3eb70c2f..0d8cca48 100644 --- a/frontend/src/old-pages/Configure/SingleInstanceComputeResource.tsx +++ b/frontend/src/old-pages/Configure/Queues/SingleInstanceComputeResource.tsx @@ -8,9 +8,9 @@ import { } from '@awsui/components-react' import {useEffect} from 'react' import {Trans, useTranslation} from 'react-i18next' -import HelpTooltip from '../../components/HelpTooltip' -import {clearState, getState, setState, useState} from '../../store' -import {HelpTextInput, InstanceSelect} from './Components' +import HelpTooltip from '../../../components/HelpTooltip' +import {clearState, getState, setState, useState} from '../../../store' +import {HelpTextInput, InstanceSelect} from '../Components' import { QueueValidationErrors, SingleInstanceComputeResource, diff --git a/frontend/src/old-pages/Configure/queues.types.ts b/frontend/src/old-pages/Configure/Queues/queues.types.ts similarity index 100% rename from frontend/src/old-pages/Configure/queues.types.ts rename to frontend/src/old-pages/Configure/Queues/queues.types.ts From 9b50d2a3736bc106fade4e6066c6bdd0255d17f5 Mon Sep 17 00:00:00 2001 From: Mattia Franchetto Date: Thu, 20 Oct 2022 16:45:49 +0200 Subject: [PATCH 09/12] Add default instance type --- frontend/src/old-pages/Configure/Cluster.tsx | 2 +- .../Configure/Queues/MultiInstanceComputeResource.tsx | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/src/old-pages/Configure/Cluster.tsx b/frontend/src/old-pages/Configure/Cluster.tsx index b5be0abc..2c4fc31b 100644 --- a/frontend/src/old-pages/Configure/Cluster.tsx +++ b/frontend/src/old-pages/Configure/Cluster.tsx @@ -368,7 +368,7 @@ function Cluster() { [ { Name: 'queue0', - ComputeResources: [createComputeResource(0)], + ComputeResources: [createComputeResource(0, 0)], }, ], ) diff --git a/frontend/src/old-pages/Configure/Queues/MultiInstanceComputeResource.tsx b/frontend/src/old-pages/Configure/Queues/MultiInstanceComputeResource.tsx index b62e0609..8e8e4795 100644 --- a/frontend/src/old-pages/Configure/Queues/MultiInstanceComputeResource.tsx +++ b/frontend/src/old-pages/Configure/Queues/MultiInstanceComputeResource.tsx @@ -22,6 +22,7 @@ import { const queuesPath = ['app', 'wizard', 'config', 'Scheduling', 'SlurmQueues'] const queuesErrorsPath = ['app', 'wizard', 'errors', 'queues'] +const defaultInstanceType = 'c5n.large' const useAllocationStrategyOptions = () => { const {t} = useTranslation() @@ -66,7 +67,7 @@ export function ComputeResource({index, queueIndex, computeResource}: any) { const allocationStrategyOptions = useAllocationStrategyOptions() const instanceTypePath = useMemo(() => [...path, 'InstanceTypes'], [path]) - const instanceTypes: string[] = useState(instanceTypePath) + const instanceTypes: string[] = useState(instanceTypePath) || [] const allocationStrategy: CRAllocationStrategy = useState([ ...path, 'AllocationStrategy', @@ -303,7 +304,7 @@ export function createComputeResource( ): MultiInstanceComputeResource { return { Name: `queue${queueIndex}-compute-resource-${crIndex}`, - InstanceTypes: [], + InstanceTypes: [defaultInstanceType], AllocationStrategy: 'lowest-price', MinCount: 0, MaxCount: 4, From b77b3a827cd7449ff29f3d3ec0378b6ca3e39bc4 Mon Sep 17 00:00:00 2001 From: Mattia Franchetto Date: Thu, 20 Oct 2022 17:41:19 +0200 Subject: [PATCH 10/12] Add unit tests --- .../__tests__/FeatureFlagsProvider.test.ts | 1 + .../Queues/MultiInstanceComputeResource.tsx | 2 +- .../Configure/Queues/Queues.test.tsx | 87 +++++++++++++++++++ 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 frontend/src/old-pages/Configure/Queues/Queues.test.tsx diff --git a/frontend/src/feature-flags/__tests__/FeatureFlagsProvider.test.ts b/frontend/src/feature-flags/__tests__/FeatureFlagsProvider.test.ts index 68d7eb40..dcad3fff 100644 --- a/frontend/src/feature-flags/__tests__/FeatureFlagsProvider.test.ts +++ b/frontend/src/feature-flags/__tests__/FeatureFlagsProvider.test.ts @@ -52,6 +52,7 @@ describe('given a feature flags provider and a list of rules', () => { 'memory_based_scheduling', 'slurm_queue_update_strategy', 'slurm_accounting', + 'queues_multiple_instance_types', ]) }) }) diff --git a/frontend/src/old-pages/Configure/Queues/MultiInstanceComputeResource.tsx b/frontend/src/old-pages/Configure/Queues/MultiInstanceComputeResource.tsx index 8e8e4795..5c068ecf 100644 --- a/frontend/src/old-pages/Configure/Queues/MultiInstanceComputeResource.tsx +++ b/frontend/src/old-pages/Configure/Queues/MultiInstanceComputeResource.tsx @@ -42,7 +42,7 @@ const useAllocationStrategyOptions = () => { return options } -function allInstancesSupportEFA( +export function allInstancesSupportEFA( instanceTypes: string[], efaInstances: Set, ): boolean { diff --git a/frontend/src/old-pages/Configure/Queues/Queues.test.tsx b/frontend/src/old-pages/Configure/Queues/Queues.test.tsx new file mode 100644 index 00000000..aab7032c --- /dev/null +++ b/frontend/src/old-pages/Configure/Queues/Queues.test.tsx @@ -0,0 +1,87 @@ +import { + allInstancesSupportEFA, + createComputeResource, + validateComputeResources, +} from './MultiInstanceComputeResource' + +describe('Given a list of instances', () => { + const subject = allInstancesSupportEFA + const efaInstances = new Set(['t2.micro', 't2.medium']) + + describe("when it's empty", () => { + it('should deactivate EFA', () => { + expect(subject([], efaInstances)).toBe(false) + }) + }) + + describe('when every instance supports EFA', () => { + it('should enable EFA', () => { + expect(subject(['t2.micro', 't2.medium'], efaInstances)).toBe(true) + }) + }) + + describe('when not every instance supports EFA', () => { + it('should deactivate EFA', () => { + expect(subject(['t2.micro', 't2.large'], efaInstances)).toBe(false) + }) + }) +}) + +describe('Given a list of queues', () => { + const subject = createComputeResource + describe('when creating a new compute resource', () => { + it('should create it with a default instance type', () => { + expect(subject(0, 0).InstanceTypes).toHaveLength(1) + }) + }) +}) + +describe('Given a list of compute resources', () => { + const subject = validateComputeResources + describe('when all of them have at least one instance type', () => { + it('should not return an error', () => { + const [valid] = subject([ + { + Name: 'test1', + MinCount: 0, + MaxCount: 2, + InstanceTypes: ['t2.micro', 't2.medium'], + AllocationStrategy: 'lowest-price', + }, + { + Name: 'test2', + MinCount: 0, + MaxCount: 2, + InstanceTypes: ['t2.micro'], + AllocationStrategy: 'lowest-price', + }, + ]) + + expect(valid).toBe(true) + }) + }) + + describe('when one of these compute resources has no instance types', () => { + it('should return a validation error', () => { + const [valid, errors] = subject([ + { + Name: 'test1', + MinCount: 0, + MaxCount: 2, + InstanceTypes: ['t2.micro', 't2.medium'], + AllocationStrategy: 'lowest-price', + }, + { + Name: 'test2', + MinCount: 0, + MaxCount: 2, + InstanceTypes: [], + AllocationStrategy: 'lowest-price', + }, + ]) + + expect(valid).toBe(false) + expect(errors[1]).toBe('instance_types_empty') + }) + }) +}) From 91c61bfac930022a24d62a9782eef30f2b6c2321 Mon Sep 17 00:00:00 2001 From: Mattia Franchetto Date: Thu, 20 Oct 2022 20:12:15 +0200 Subject: [PATCH 11/12] Align queues for new schema --- frontend/src/old-pages/Configure/Cluster.tsx | 15 +- frontend/src/old-pages/Configure/Queues.tsx | 501 ++++++++++++++++++ .../Queues/MultiInstanceComputeResource.tsx | 85 +-- .../Configure/Queues/Queues.test.tsx | 28 +- .../src/old-pages/Configure/Queues/Queues.tsx | 71 ++- .../{ => Queues}/SlurmMemorySettings.tsx | 2 +- .../Configure/Queues/queues.types.ts | 7 +- 7 files changed, 626 insertions(+), 83 deletions(-) create mode 100644 frontend/src/old-pages/Configure/Queues.tsx rename frontend/src/old-pages/Configure/{ => Queues}/SlurmMemorySettings.tsx (98%) diff --git a/frontend/src/old-pages/Configure/Cluster.tsx b/frontend/src/old-pages/Configure/Cluster.tsx index 2c4fc31b..1983bef1 100644 --- a/frontend/src/old-pages/Configure/Cluster.tsx +++ b/frontend/src/old-pages/Configure/Cluster.tsx @@ -35,6 +35,8 @@ import {LoadAwsConfig} from '../../model' import {LabeledIcon, CustomAMISettings} from './Components' import {useFeatureFlag} from '../../feature-flags/useFeatureFlag' import {useComputeResourceAdapter} from './Queues/Queues' +import {createComputeResource as singleCreate} from './Queues/SingleInstanceComputeResource' +import {createComputeResource as multiCreate} from './Queues/MultiInstanceComputeResource' // Constants const errorsPath = ['app', 'wizard', 'errors', 'cluster'] @@ -343,7 +345,9 @@ function Cluster() { let defaultRegion = useState(['aws', 'region']) || '' const region = useState(['app', 'selectedRegion']) || defaultRegion const isMultiuserClusterActive = useFeatureFlag('multiuser_cluster') - const {createComputeResource} = useComputeResourceAdapter() + const isMultipleInstanceTypesActive = useFeatureFlag( + 'queues_multiple_instance_types', + ) React.useEffect(() => { const configPath = ['app', 'wizard', 'config'] @@ -368,7 +372,14 @@ function Cluster() { [ { Name: 'queue0', - ComputeResources: [createComputeResource(0, 0)], + AllocationStrategy: isMultipleInstanceTypesActive + ? 'lowest-price' + : undefined, + ComputeResources: [ + isMultipleInstanceTypesActive + ? multiCreate(0, 0) + : singleCreate(0, 0), + ], }, ], ) diff --git a/frontend/src/old-pages/Configure/Queues.tsx b/frontend/src/old-pages/Configure/Queues.tsx new file mode 100644 index 00000000..b77852b2 --- /dev/null +++ b/frontend/src/old-pages/Configure/Queues.tsx @@ -0,0 +1,501 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance +// with the License. A copy of the License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES +// OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and +// limitations under the License. +import * as React from 'react' +import i18next from 'i18next' +import {findFirst} from '../../util' + +// UI Elements +import { + Button, + Box, + Container, + ColumnLayout, + ExpandableSection, + FormField, + Header, + Input, + Select, + SpaceBetween, + Toggle, +} from '@awsui/components-react' + +// State +import {setState, getState, useState, clearState} from '../../store' + +// Components +import { + ActionsEditor, + CustomAMISettings, + LabeledIcon, + RootVolume, + SubnetSelect, + SecurityGroups, + IamPoliciesEditor, +} from './Components' +import {Trans, useTranslation} from 'react-i18next' +import {SlurmMemorySettings} from './Queues/SlurmMemorySettings' +import { + isFeatureEnabled, + useFeatureFlag, +} from '../../feature-flags/useFeatureFlag' +import * as SingleInstanceCR from './SingleInstanceComputeResource' +import * as MultiInstanceCR from './MultiInstanceComputeResource' +import {ComputeResource} from './queues.types' + +// Constants +const queuesPath = ['app', 'wizard', 'config', 'Scheduling', 'SlurmQueues'] +const queuesErrorsPath = ['app', 'wizard', 'errors', 'queues'] + +// Helper Functions +// @ts-expect-error TS(7031) FIXME: Binding element 'value' implicitly has an 'any' ty... Remove this comment to see the full error message +function itemToIconOption([value, label, icon]) { + return {value: value, label: label, ...(icon ? {iconUrl: icon} : {})} +} + +// @ts-expect-error TS(7031) FIXME: Binding element 'value' implicitly has an 'any' ty... Remove this comment to see the full error message +function itemToDisplayIconOption([value, label, icon]) { + return { + value: value, + label: icon ? : label, + } +} + +function queueValidate(queueIndex: any) { + let valid = true + const queueSubnet = getState([ + ...queuesPath, + queueIndex, + 'Networking', + 'SubnetIds', + 0, + ]) + const computeResources = getState([ + ...queuesPath, + queueIndex, + 'ComputeResources', + ]) + + const errorsPath = [...queuesErrorsPath, queueIndex] + + const actionsPath = [...queuesPath, queueIndex, 'CustomActions'] + + const onStartPath = [...actionsPath, 'OnNodeStart'] + const onStart = getState(onStartPath) + + const onConfiguredPath = [...actionsPath, 'OnNodeConfigured'] + const onConfigured = getState(onConfiguredPath) + + const customAmiEnabled = getState([ + 'app', + 'wizard', + 'queues', + queueIndex, + 'customAMI', + 'enabled', + ]) + const customAmi = getState([...queuesPath, queueIndex, 'Image', 'CustomAmi']) + + const rootVolumeSizePath = [ + ...queuesPath, + queueIndex, + 'ComputeSettings', + 'LocalStorage', + 'RootVolume', + 'Size', + ] + const rootVolumeValue = getState(rootVolumeSizePath) + + if (rootVolumeValue === '') { + setState( + [...errorsPath, 'rootVolume'], + i18next.t('wizard.queues.validation.setRootVolumeSize'), + ) + valid = false + } else if ( + rootVolumeValue && + (!Number.isInteger(rootVolumeValue) || rootVolumeValue < 35) + ) { + setState( + [...errorsPath, 'rootVolume'], + i18next.t('wizard.queues.validation.rootVolumeMinimum'), + ) + valid = false + } else { + clearState([...errorsPath, 'rootVolume']) + } + + if ( + onStart && + getState([...onStartPath, 'Args']) && + !getState([...onStartPath, 'Script']) + ) { + setState( + [...errorsPath, 'onStart'], + i18next.t('wizard.queues.validation.rootVolumeMinimum'), + ) + valid = false + } else { + clearState([...errorsPath, 'onStart']) + } + + if ( + onConfigured && + getState([...onConfiguredPath, 'Args']) && + !getState([...onConfiguredPath, 'Script']) + ) { + setState( + [...errorsPath, 'onConfigured'], + i18next.t('wizard.queues.validation.scriptWithArgs'), + ) + valid = false + } else { + clearState([...errorsPath, 'onConfigured']) + } + + if (customAmiEnabled && !customAmi) { + setState( + [...errorsPath, 'customAmi'], + i18next.t('wizard.queues.validation.customAmiSelect'), + ) + valid = false + } else { + clearState([...errorsPath, 'customAmi']) + } + + if (!queueSubnet) { + setState( + [...errorsPath, 'subnet'], + i18next.t('wizard.queues.validation.selectSubnet'), + ) + valid = false + } else { + setState([...errorsPath, 'subnet'], null) + } + + const version = getState(['app', 'version', 'full']) + const isMultiInstanceTypesActive = isFeatureEnabled( + version, + 'queues_multiple_instance_types', + ) + const {validateComputeResources} = !isMultiInstanceTypesActive + ? SingleInstanceCR + : MultiInstanceCR + const [computeResourcesValid, computeResourcesErrors] = + validateComputeResources(computeResources) + if (!computeResourcesValid) { + valid = false + computeResources.forEach((_: ComputeResource, i: number) => { + const error = computeResourcesErrors[i] + if (error) { + let message: string + if (error === 'instance_type_unique') { + message = i18next.t('wizard.queues.validation.instanceTypeUnique') + } else { + message = i18next.t('wizard.queues.validation.instanceTypeMissing') + } + setState([...errorsPath, 'computeResource', i, 'type'], message) + } else { + setState([...errorsPath, 'computeResource', i, 'type'], null) + } + }) + } + + return valid +} + +function queuesValidate() { + let valid = true + const config = getState(['app', 'wizard', 'config']) + console.log(config) + + setState([...queuesErrorsPath, 'validated'], true) + + const queues = getState([...queuesPath]) + for (let i = 0; i < queues.length; i++) { + let queueValid = queueValidate(i) + valid &&= queueValid + } + + return valid +} + +function ComputeResources({queue, index}: any) { + const {ViewComponent} = useComputeResourceAdapter() + return ( + + {queue.ComputeResources.map((computeResource: any, i: any) => ( + + ))} + + ) +} + +function Queue({index}: any) { + const {t} = useTranslation() + const queues = useState(queuesPath) + const [editingName, setEditingName] = React.useState(false) + const computeResourceAdapter = useComputeResourceAdapter() + const queue = useState([...queuesPath, index]) + const enablePlacementGroupPath = [ + ...queuesPath, + index, + 'Networking', + 'PlacementGroup', + 'Enabled', + ] + const enablePlacementGroup = useState(enablePlacementGroupPath) + + const errorsPath = [...queuesErrorsPath, index] + const subnetError = useState([...errorsPath, 'subnet']) + + const capacityTypes: [string, string, string][] = [ + ['ONDEMAND', 'On-Demand', '/img/od.svg'], + ['SPOT', 'Spot', '/img/spot.svg'], + ] + const capacityTypePath = [...queuesPath, index, 'CapacityType'] + const capacityType: string = useState(capacityTypePath) || 'ONDEMAND' + + const subnetPath = [...queuesPath, index, 'Networking', 'SubnetIds'] + const subnetValue = useState([...subnetPath, 0]) || '' + + const remove = () => { + setState( + [...queuesPath], + [...queues.slice(0, index), ...queues.slice(index + 1)], + ) + } + const addComputeResource = () => { + const existingCRs = queue.ComputeResources || [] + setState([...queuesPath, index], { + ...queue, + ComputeResources: [ + ...existingCRs, + computeResourceAdapter.createComputeResource(index, existingCRs.length), + ], + }) + } + + const setEnablePG = (enable: any) => { + setState(enablePlacementGroupPath, enable) + } + + const renameQueue = (newName: any) => { + const computeResources = getState([ + ...queuesPath, + index, + 'ComputeResources', + ]) + const updatedCRs = computeResourceAdapter.updateComputeResourcesNames( + computeResources, + newName, + ) + setState([...queuesPath, index, 'Name'], newName) + setState([...queuesPath, index, 'ComputeResources'], updatedCRs) + } + + return ( +
+
+ +
+ + {index > 0 && } + + } + > + + {editingName ? ( + { + if (e.detail.key === 'Enter' || e.detail.key === 'Escape') { + setEditingName(false) + e.stopPropagation() + } + }} + onChange={({detail}) => renameQueue(detail.value)} + /> + ) : ( + + Queue: {queue.Name}{' '} + + + )} + +
+ + + + { + setState(subnetPath, [subnetId]) + queueValidate(index) + }} + /> + + + as.value === allocationStrategy, - )! - } - onChange={setAllocationStrategy} - /> - {enableMemoryBasedScheduling && ( { setEnableEFA(!enableEFA) @@ -304,8 +262,11 @@ export function createComputeResource( ): MultiInstanceComputeResource { return { Name: `queue${queueIndex}-compute-resource-${crIndex}`, - InstanceTypes: [defaultInstanceType], - AllocationStrategy: 'lowest-price', + Instances: [ + { + InstanceType: defaultInstanceType, + }, + ], MinCount: 0, MaxCount: 4, } @@ -325,7 +286,7 @@ export function validateComputeResources( computeResources: MultiInstanceComputeResource[], ): [boolean, QueueValidationErrors] { let errors = computeResources.reduce((acc, cr, i) => { - if (!cr.InstanceTypes || !cr.InstanceTypes.length) { + if (!cr.Instances || !cr.Instances.length) { acc[i] = 'instance_types_empty' } return acc diff --git a/frontend/src/old-pages/Configure/Queues/Queues.test.tsx b/frontend/src/old-pages/Configure/Queues/Queues.test.tsx index aab7032c..79a1435b 100644 --- a/frontend/src/old-pages/Configure/Queues/Queues.test.tsx +++ b/frontend/src/old-pages/Configure/Queues/Queues.test.tsx @@ -16,13 +16,23 @@ describe('Given a list of instances', () => { describe('when every instance supports EFA', () => { it('should enable EFA', () => { - expect(subject(['t2.micro', 't2.medium'], efaInstances)).toBe(true) + expect( + subject( + [{InstanceType: 't2.micro'}, {InstanceType: 't2.medium'}], + efaInstances, + ), + ).toBe(true) }) }) describe('when not every instance supports EFA', () => { it('should deactivate EFA', () => { - expect(subject(['t2.micro', 't2.large'], efaInstances)).toBe(false) + expect( + subject( + [{InstanceType: 't2.micro'}, {InstanceType: 't2.large'}], + efaInstances, + ), + ).toBe(false) }) }) }) @@ -31,7 +41,7 @@ describe('Given a list of queues', () => { const subject = createComputeResource describe('when creating a new compute resource', () => { it('should create it with a default instance type', () => { - expect(subject(0, 0).InstanceTypes).toHaveLength(1) + expect(subject(0, 0).Instances).toHaveLength(1) }) }) }) @@ -45,15 +55,13 @@ describe('Given a list of compute resources', () => { Name: 'test1', MinCount: 0, MaxCount: 2, - InstanceTypes: ['t2.micro', 't2.medium'], - AllocationStrategy: 'lowest-price', + Instances: [{InstanceType: 't2.micro'}, {InstanceType: 't2.medium'}], }, { Name: 'test2', MinCount: 0, MaxCount: 2, - InstanceTypes: ['t2.micro'], - AllocationStrategy: 'lowest-price', + Instances: [{InstanceType: 't2.micro'}], }, ]) @@ -68,15 +76,13 @@ describe('Given a list of compute resources', () => { Name: 'test1', MinCount: 0, MaxCount: 2, - InstanceTypes: ['t2.micro', 't2.medium'], - AllocationStrategy: 'lowest-price', + Instances: [{InstanceType: 't2.micro'}, {InstanceType: 't2.medium'}], }, { Name: 'test2', MinCount: 0, MaxCount: 2, - InstanceTypes: [], - AllocationStrategy: 'lowest-price', + Instances: [], }, ]) diff --git a/frontend/src/old-pages/Configure/Queues/Queues.tsx b/frontend/src/old-pages/Configure/Queues/Queues.tsx index c77a0bcd..7137beed 100644 --- a/frontend/src/old-pages/Configure/Queues/Queues.tsx +++ b/frontend/src/old-pages/Configure/Queues/Queues.tsx @@ -49,7 +49,7 @@ import { } from '../../../feature-flags/useFeatureFlag' import * as SingleInstanceCR from './SingleInstanceComputeResource' import * as MultiInstanceCR from './MultiInstanceComputeResource' -import {ComputeResource} from './queues.types' +import {AllocationStrategy, ComputeResource} from './queues.types' // Constants const queuesPath = ['app', 'wizard', 'config', 'Scheduling', 'SlurmQueues'] @@ -245,6 +245,24 @@ function ComputeResources({queue, index}: any) { ) } +const useAllocationStrategyOptions = () => { + const {t} = useTranslation() + const options = React.useMemo( + () => [ + { + label: t('wizard.queues.allocationStrategy.lowestPrice'), + value: 'lowest-price', + }, + { + label: t('wizard.queues.allocationStrategy.capacityOptimized'), + value: 'capacity-optimized', + }, + ], + [t], + ) + return options +} + function Queue({index}: any) { const {t} = useTranslation() const queues = useState(queuesPath) @@ -260,9 +278,17 @@ function Queue({index}: any) { ] const enablePlacementGroup = useState(enablePlacementGroupPath) + const allocationStrategyOptions = useAllocationStrategyOptions() + const errorsPath = [...queuesErrorsPath, index] const subnetError = useState([...errorsPath, 'subnet']) + const allocationStrategy: AllocationStrategy = useState([ + ...queuesPath, + index, + 'AllocationStrategy', + ]) + const capacityTypes: [string, string, string][] = [ ['ONDEMAND', 'On-Demand', '/img/od.svg'], ['SPOT', 'Spot', '/img/spot.svg'], @@ -308,6 +334,16 @@ function Queue({index}: any) { setState([...queuesPath, index, 'ComputeResources'], updatedCRs) } + const setAllocationStrategy = React.useCallback( + ({detail}) => { + setState( + [...queuesPath, index, 'AllocationStrategy'], + detail.selectedOption.value, + ) + }, + [index], + ) + return (
@@ -380,6 +416,21 @@ function Queue({index}: any) { options={capacityTypes.map(itemToIconOption)} /> + {queue.AllocationStrategy ? ( + + { - if (e.detail.key === 'Enter' || e.detail.key === 'Escape') { - setEditingName(false) - e.stopPropagation() - } - }} - onChange={({detail}) => renameQueue(detail.value)} - /> - ) : ( - - Queue: {queue.Name}{' '} - - - )} - - - - - - { - setState(subnetPath, [subnetId]) - queueValidate(index) - }} - /> - - -