From 7bfc7afa32bf690b2503e2d52d1be5c0143d473d Mon Sep 17 00:00:00 2001 From: Alessandro Menduni Date: Fri, 10 Feb 2023 16:00:16 +0100 Subject: [PATCH 1/4] Extract wizard state init into a function --- frontend/src/old-pages/Configure/Cluster.tsx | 58 +++++++++++--------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/frontend/src/old-pages/Configure/Cluster.tsx b/frontend/src/old-pages/Configure/Cluster.tsx index c8dc0ff3..47d51f14 100644 --- a/frontend/src/old-pages/Configure/Cluster.tsx +++ b/frontend/src/old-pages/Configure/Cluster.tsx @@ -23,7 +23,6 @@ import { CheckboxProps, Container, FormField, - Header, Select, SpaceBetween, } from '@cloudscape-design/components' @@ -47,6 +46,7 @@ import {SelectProps} from '@cloudscape-design/components/select/interfaces' // Constants const errorsPath = ['app', 'wizard', 'errors', 'cluster'] +const configPath = ['app', 'wizard', 'config'] const selectQueues = (state: any) => getState(state, ['app', 'wizard', 'config', 'Scheduling', 'SlurmQueues']) @@ -181,6 +181,35 @@ function RegionSelect() { ) } +function initWizardState( + config: Record, + region: string, + isMultipleInstanceTypesActive: boolean, +) { + const customAMIEnabled = getIn(config, ['Image', 'CustomAmi']) ? true : false + setState(['app', 'wizard', 'customAMI', 'enabled'], customAMIEnabled) + setState([...configPath, 'HeadNode', 'InstanceType'], 't2.micro') + setState([...configPath, 'Scheduling', 'Scheduler'], 'slurm') + setState([...configPath, 'Region'], region) + setState([...configPath, 'Image', 'Os'], 'alinux2') + setState( + [...configPath, 'Scheduling', 'SlurmQueues'], + [ + { + Name: 'queue0', + AllocationStrategy: isMultipleInstanceTypesActive + ? 'lowest-price' + : undefined, + ComputeResources: [ + isMultipleInstanceTypesActive + ? multiCreate(0, 0) + : singleCreate(0, 0), + ], + }, + ], + ) +} + function OsSelect() { const {t} = useTranslation() const oses: [string, string][] = [ @@ -341,7 +370,6 @@ function VpcSelect() { function Cluster() { const {t} = useTranslation() const editing = useState(['app', 'wizard', 'editing']) - const configPath = ['app', 'wizard', 'config'] let config = useState(configPath) let clusterConfig = useState(['app', 'wizard', 'clusterConfigYaml']) || '' let wizardLoaded = useState(['app', 'wizard', 'loaded']) @@ -357,7 +385,6 @@ function Cluster() { useHelpPanel() React.useEffect(() => { - const configPath = ['app', 'wizard', 'config'] // Don't overwrite the config if we go back, still gets overwritten // after going forward so need to consider better way of handling this if (clusterConfig) return @@ -366,30 +393,7 @@ function Cluster() { if (!wizardLoaded) { setState(['app', 'wizard', 'loaded'], true) if (!config) { - const customAMIEnabled = getIn(config, ['Image', 'CustomAmi']) - ? true - : false - setState(['app', 'wizard', 'customAMI', 'enabled'], customAMIEnabled) - setState([...configPath, 'HeadNode', 'InstanceType'], 't2.micro') - setState([...configPath, 'Scheduling', 'Scheduler'], 'slurm') - setState([...configPath, 'Region'], region) - setState([...configPath, 'Image', 'Os'], 'alinux2') - setState( - [...configPath, 'Scheduling', 'SlurmQueues'], - [ - { - Name: 'queue0', - AllocationStrategy: isMultipleInstanceTypesActive - ? 'lowest-price' - : undefined, - ComputeResources: [ - isMultipleInstanceTypesActive - ? multiCreate(0, 0) - : singleCreate(0, 0), - ], - }, - ], - ) + initWizardState(config, region, isMultipleInstanceTypesActive) } } From fa362a84a0e6cdb741735a4d0dffb106e2e7c58b Mon Sep 17 00:00:00 2001 From: Alessandro Menduni Date: Fri, 10 Feb 2023 16:01:26 +0100 Subject: [PATCH 2/4] Init wizard state when changing region in wizard --- frontend/src/old-pages/Configure/Cluster.tsx | 46 +++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/frontend/src/old-pages/Configure/Cluster.tsx b/frontend/src/old-pages/Configure/Cluster.tsx index 47d51f14..791edf43 100644 --- a/frontend/src/old-pages/Configure/Cluster.tsx +++ b/frontend/src/old-pages/Configure/Cluster.tsx @@ -37,7 +37,10 @@ import {useFeatureFlag} from '../../feature-flags/useFeatureFlag' import {createComputeResource as singleCreate} from './Queues/SingleInstanceComputeResource' import {createComputeResource as multiCreate} from './Queues/MultiInstanceComputeResource' import {MultiUser, multiUserValidate} from './MultiUser' -import {NonCancelableEventHandler} from '@cloudscape-design/components/internal/events' +import { + NonCancelableCustomEvent, + NonCancelableEventHandler, +} from '@cloudscape-design/components/internal/events' import TitleDescriptionHelpPanel from '../../components/help-panel/TitleDescriptionHelpPanel' import {useHelpPanel} from '../../components/help-panel/HelpPanel' import {useCallback, useMemo} from 'react' @@ -112,23 +115,34 @@ function RegionSelect() { const {t} = useTranslation() const region = useState(['app', 'wizard', 'config', 'Region']) || 'Please select a region.' - const queues = useSelector(selectQueues) const editing = useState(['app', 'wizard', 'editing']) + const config = useState(configPath) + const isMultipleInstanceTypesActive = useFeatureFlag( + 'queues_multiple_instance_types', + ) - const handleChange = ({detail}: any) => { - const chosenRegion = - detail.selectedOption.value === 'Default' - ? null - : detail.selectedOption.value - LoadAwsConfig(chosenRegion) - setState(['app', 'wizard', 'vpc'], null) - setState(['app', 'wizard', 'headNode', 'subnet'], null) - if (queues) - queues.forEach((_queue: any, i: any) => { - clearState(['app', 'wizard', 'queues', i, 'subnet']) - }) - setState(['app', 'wizard', 'config', 'Region'], chosenRegion) - } + const handleChange = useCallback( + ({detail}: NonCancelableCustomEvent) => { + const chosenRegion = detail.selectedOption.value + + if (!chosenRegion) return + + /** + * Clear wizard state + * + * We keep the part of the state that is necessary + * to continue with the experience + */ + const {page, source, clusterName, errors} = + getState(['app', 'wizard']) || {} + setState(['app', 'wizard'], {page, source, clusterName, errors}) + + initWizardState(config, chosenRegion, isMultipleInstanceTypesActive) + + LoadAwsConfig(chosenRegion) + }, + [config, isMultipleInstanceTypesActive], + ) const supportedRegions = [ 'af-south-1', From b8c03796c3f277035ec41b008da5cc23ac762a7b Mon Sep 17 00:00:00 2001 From: Alessandro Menduni Date: Fri, 10 Feb 2023 16:02:22 +0100 Subject: [PATCH 3/4] Remove keypair initialization from Cluster section --- frontend/src/old-pages/Configure/Cluster.tsx | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/frontend/src/old-pages/Configure/Cluster.tsx b/frontend/src/old-pages/Configure/Cluster.tsx index 791edf43..ec92a990 100644 --- a/frontend/src/old-pages/Configure/Cluster.tsx +++ b/frontend/src/old-pages/Configure/Cluster.tsx @@ -410,20 +410,9 @@ function Cluster() { initWizardState(config, region, isMultipleInstanceTypesActive) } } - - // Load these values when we get a new config as well (e.g. changing region) - if (awsConfig && awsConfig.keypairs && awsConfig.keypairs.length > 0) { - const keypairs = getState(['aws', 'keypairs']) || [] - const keypairNames = new Set(keypairs.map((kp: any) => kp.KeyName)) - const headNodeKPPath = [...configPath, 'HeadNode', 'Ssh', 'KeyName'] - if (keypairs.length > 0 && !keypairNames.has(getState(headNodeKPPath))) { - setState(headNodeKPPath, awsConfig.keypairs[0].KeyName) - } - } }, [ region, config, - awsConfig, clusterConfig, wizardLoaded, isMultipleInstanceTypesActive, From a31f65fc4bffa5cc22680d5211a88251318d916d Mon Sep 17 00:00:00 2001 From: Alessandro Menduni Date: Fri, 10 Feb 2023 16:03:08 +0100 Subject: [PATCH 4/4] Initialize keypair in HeadNode section --- frontend/src/old-pages/Configure/HeadNode.tsx | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/frontend/src/old-pages/Configure/HeadNode.tsx b/frontend/src/old-pages/Configure/HeadNode.tsx index 71401a66..1935becd 100644 --- a/frontend/src/old-pages/Configure/HeadNode.tsx +++ b/frontend/src/old-pages/Configure/HeadNode.tsx @@ -15,6 +15,7 @@ import i18next from 'i18next' import {Trans, useTranslation} from 'react-i18next' import {useSelector} from 'react-redux' import {findFirst, getIn} from '../../util' +import safeGet from 'lodash/get' // UI Elements import { @@ -61,6 +62,7 @@ import {useHelpPanel} from '../../components/help-panel/HelpPanel' // Constants const headNodePath = ['app', 'wizard', 'config', 'HeadNode'] const errorsPath = ['app', 'wizard', 'errors', 'headNode'] +const keypairPath = [...headNodePath, 'Ssh', 'KeyName'] function headNodeValidate() { const subnetPath = [...headNodePath, 'Networking', 'SubnetId'] @@ -210,12 +212,18 @@ function enableSsm(enable: any) { } } } +const setKeyPair = (kpValue?: string) => { + if (kpValue) setState(keypairPath, kpValue) + else { + clearState([...headNodePath, 'Ssh']) + enableSsm(true) + } +} function KeypairSelect() { const {t} = useTranslation() - const keypairs = useState(['aws', 'keypairs']) || [] - const keypairPath = [...headNodePath, 'Ssh', 'KeyName'] - const keypair = useState(keypairPath) || '' + const keypairsInAWSConfig = useState(['aws', 'keypairs']) + const selectedKeypairName = useState(keypairPath) const editing = useState(['app', 'wizard', 'editing']) const keypairToOption = (kp: any) => { if (kp === 'None' || kp === null || kp === undefined) @@ -223,15 +231,15 @@ function KeypairSelect() { else return {label: kp.KeyName, value: kp.KeyName} } - const keypairsWithNone = ['None', ...keypairs] + const keypairsWithNone = ['None', ...keypairsInAWSConfig] - const setKeyPair = (kpValue: any) => { - if (kpValue) setState(keypairPath, kpValue) - else { - clearState([...headNodePath, 'Ssh']) - enableSsm(true) + React.useEffect(() => { + const firstAvailableKeypair = safeGet(keypairsInAWSConfig, ['0', 'KeyName']) + if (!selectedKeypairName && firstAvailableKeypair) { + setKeyPair(firstAvailableKeypair) } - } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [keypairsInAWSConfig]) return ( { - return x.KeyName === keypair + findFirst(keypairsInAWSConfig, (x: any) => { + return x.KeyName === selectedKeypairName }), )} onChange={({detail}) => {