fix(aws): Fixing bugs related to clone CX when instance types are inc…

…ompatible with image/region (#9901) * fix(aws): Fixing bugs related to clone UX when instance types are incompatible with image/region Displaying warnings when incompatible instance types are removed spinnaker/spinnaker#6734 * fix(aws): PR feedback Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit d7290c4)
spinnaker · Nov 8, 2022 · 57a73f4 · 57a73f4
1 parent c3ea5bc
commit 57a73f4
Show file tree

Hide file tree

Showing 10 changed files with 177 additions and 48 deletions.
diff --git a/packages/amazon/src/serverGroup/configure/serverGroupConfiguration.service.ts b/packages/amazon/src/serverGroup/configure/serverGroupConfiguration.service.ts
@@ -3,6 +3,7 @@ import {
   chain,
   clone,
   cloneDeep,
+  difference,
   extend,
   find,
   flatten,
@@ -61,6 +62,7 @@ export type IBlockDeviceMappingSource = 'source' | 'ami' | 'default';
 
 export interface IAmazonServerGroupCommandDirty extends IServerGroupCommandDirty {
   targetGroups?: string[];
+  launchTemplateOverridesForInstanceType?: IAmazonInstanceTypeOverride[];
 }
 
 export interface IAmazonServerGroupCommandResult extends IServerGroupCommandResult {
@@ -376,11 +378,13 @@ export class AwsServerGroupConfigurationService {
 
   public configureInstanceTypes(command: IAmazonServerGroupCommand): IServerGroupCommandResult {
     const result: IAmazonServerGroupCommandResult = { dirty: {} };
+
     if (command.region && (command.virtualizationType || command.viewState.disableImageSelection)) {
       let filteredTypesInfo: IAmazonInstanceType[] = this.awsInstanceTypeService.getAvailableTypesForRegions(
         command.backingData.instanceTypesInfo,
         [command.region],
       );
+
       if (command.virtualizationType || command.amiArchitecture) {
         filteredTypesInfo = this.awsInstanceTypeService.filterInstanceTypes(
           filteredTypesInfo,
@@ -389,18 +393,35 @@ export class AwsServerGroupConfigurationService {
           command.amiArchitecture,
         );
       }
-
       const filteredTypes: string[] = map(filteredTypesInfo, 'name');
+
+      // handle incompatibility for single instance type case
       if (command.instanceType && !filteredTypes.includes(command.instanceType)) {
         result.dirty.instanceType = command.instanceType;
         command.instanceType = null;
       }
+
+      // handle incompatibility for multiple instance types case
+      const multipleInstanceTypes: string[] = map(command.launchTemplateOverridesForInstanceType, 'instanceType');
+      const validInstanceTypes: string[] = intersection(multipleInstanceTypes, filteredTypes);
+      const invalidInstanceTypes: string[] = difference(multipleInstanceTypes, validInstanceTypes);
+
+      if (command.launchTemplateOverridesForInstanceType && invalidInstanceTypes.length > 0) {
+        result.dirty.launchTemplateOverridesForInstanceType = command.launchTemplateOverridesForInstanceType.filter(
+          (it) => invalidInstanceTypes.includes(it.instanceType),
+        );
+        command.launchTemplateOverridesForInstanceType = command.launchTemplateOverridesForInstanceType.filter((it) =>
+          validInstanceTypes.includes(it.instanceType),
+        );
+      }
+
       command.backingData.filtered.instanceTypes = filteredTypes;
       command.backingData.filtered.instanceTypesInfo = filteredTypesInfo;
     } else {
       command.backingData.filtered.instanceTypes = [];
       command.backingData.filtered.instanceTypesInfo = [];
     }
+
     extend(command.viewState.dirty, result.dirty);
     return result;
   }
@@ -613,6 +634,7 @@ export class AwsServerGroupConfigurationService {
       });
       command.vpcId = subnet ? subnet.vpcId : null;
     }
+
     extend(result.dirty, this.configureInstanceTypes(command).dirty);
     return result;
   }

diff --git a/packages/amazon/src/serverGroup/configure/wizard/instanceType/InstanceTypeSelector.tsx b/packages/amazon/src/serverGroup/configure/wizard/instanceType/InstanceTypeSelector.tsx
@@ -16,51 +16,83 @@ export interface IInstanceTypeSelectorProps {
 
 export function InstanceTypeSelector(props: IInstanceTypeSelectorProps) {
   const { instanceTypeDetails } = props;
-  const { values: command, setFieldValue } = props.formik;
+  const { values, setFieldValue } = props.formik;
   const isLaunchTemplatesEnabled = AWSProviderSettings.serverGroups?.enableLaunchTemplates;
 
-  const useSimpleMode = command.viewState.useSimpleInstanceTypeSelector;
-  const [unlimitedCpuCredits, setUnlimitedCpuCredits] = useState(command.unlimitedCpuCredits);
+  const useSimpleMode = values.viewState.useSimpleInstanceTypeSelector;
+  const [unlimitedCpuCredits, setUnlimitedCpuCredits] = useState(values.unlimitedCpuCredits);
 
   useEffect(() => {
-    if (command.unlimitedCpuCredits !== unlimitedCpuCredits) {
+    if (values.unlimitedCpuCredits !== unlimitedCpuCredits) {
       setFieldValue('unlimitedCpuCredits', unlimitedCpuCredits);
     }
   }, [unlimitedCpuCredits]);
 
+  const clearWarnings = () => {
+    const { formik } = props;
+
+    // clear for both keys to support consistency between simple and advanced modes
+    formik.values.viewState.dirty['instanceType'] = null;
+    formik.values.viewState.dirty['launchTemplateOverridesForInstanceType'] = null;
+
+    formik.validateForm();
+  };
+
   const handleModeChange = (useSimpleModeNew: boolean) => {
     if (useSimpleMode !== useSimpleModeNew) {
       setFieldValue('viewState', {
-        ...command.viewState,
+        ...values.viewState,
         useSimpleInstanceTypeSelector: useSimpleModeNew,
       });
 
       // update selected instance type(s) if mode changed.
       // Simple mode uses command.instanceType to track selected type. Advanced mode uses command.launchTemplateOverridesForInstanceType to track selected types.
-      const multipleInstanceTypesInProps = command.launchTemplateOverridesForInstanceType;
-      const singleInstanceTypeInProps = command.instanceType;
-
-      const toSimple = useSimpleModeNew && multipleInstanceTypesInProps?.length;
-      const toAdvanced = !useSimpleModeNew && singleInstanceTypeInProps;
-      if (toSimple) {
-        const highestPriorityNum = Math.min(...multipleInstanceTypesInProps.map((it) => it.priority));
-        const instanceTypeWithHighestPriority = multipleInstanceTypesInProps.find(
-          (it) => it.priority === highestPriorityNum,
-        ).instanceType;
-
-        setFieldValue('instanceType', instanceTypeWithHighestPriority);
-        setFieldValue('launchTemplateOverridesForInstanceType', []);
-        command.instanceTypeChanged(command);
-      } else if (toAdvanced) {
-        const instanceTypes: IAmazonInstanceTypeOverride[] = [
-          {
-            instanceType: singleInstanceTypeInProps,
-            priority: 1,
-          },
-        ];
-        setFieldValue('instanceType', undefined);
-        setFieldValue('launchTemplateOverridesForInstanceType', instanceTypes);
-        command.launchTemplateOverridesChanged(command);
+      if (useSimpleModeNew) {
+        const multipleInstanceTypesInProps = values.launchTemplateOverridesForInstanceType;
+        const dirtyMultipleInstanceTypesInProps = values.viewState.dirty.launchTemplateOverridesForInstanceType;
+
+        if (multipleInstanceTypesInProps?.length) {
+          const instanceTypeWithHighestPriority = multipleInstanceTypesInProps.reduce((prev, current) => {
+            return prev.priority < current.priority ? prev : current;
+          }).instanceType;
+          setFieldValue('instanceType', instanceTypeWithHighestPriority);
+          setFieldValue('launchTemplateOverridesForInstanceType', []);
+          values.instanceTypeChanged(values);
+        }
+
+        if (dirtyMultipleInstanceTypesInProps?.length) {
+          const instanceTypeWithHighestPriorityDirty = dirtyMultipleInstanceTypesInProps.reduce((prev, current) => {
+            return prev.priority < current.priority ? prev : current;
+          }).instanceType;
+          setFieldValue('viewState.dirty.instanceType', instanceTypeWithHighestPriorityDirty);
+          setFieldValue('viewState.dirty.launchTemplateOverridesForInstanceType', []);
+        }
+      } else if (!useSimpleModeNew) {
+        const singleInstanceTypeInProps = values.instanceType;
+        const dirtySingleInstanceTypeInProps = values.viewState.dirty.instanceType;
+
+        if (singleInstanceTypeInProps) {
+          const instanceTypes: IAmazonInstanceTypeOverride[] = [
+            {
+              instanceType: singleInstanceTypeInProps,
+              priority: 1,
+            },
+          ];
+          setFieldValue('instanceType', undefined);
+          setFieldValue('launchTemplateOverridesForInstanceType', instanceTypes);
+          values.launchTemplateOverridesChanged(values);
+        }
+
+        if (dirtySingleInstanceTypeInProps) {
+          const dirtyInstanceTypes: IAmazonInstanceTypeOverride[] = [
+            {
+              instanceType: dirtySingleInstanceTypeInProps,
+              priority: 1,
+            },
+          ];
+          setFieldValue('viewState.dirty.instanceType', undefined);
+          setFieldValue('viewState.dirty.launchTemplateOverridesForInstanceType', dirtyInstanceTypes);
+        }
       }
     }
   };
@@ -86,6 +118,7 @@ export function InstanceTypeSelector(props: IInstanceTypeSelectorProps) {
           formik={props.formik}
           instanceTypeDetails={instanceTypeDetails}
           setUnlimitedCpuCredits={setUnlimitedCpuCredits}
+          clearWarnings={clearWarnings}
         />
       </div>
     );
@@ -124,9 +157,10 @@ export function InstanceTypeSelector(props: IInstanceTypeSelectorProps) {
         )}
       </div>
       <SimpleModeSelector
-        command={command}
+        command={values}
         setUnlimitedCpuCredits={setUnlimitedCpuCredits}
         setFieldValue={setFieldValue}
+        clearWarnings={clearWarnings}
       />
     </div>
   );

diff --git a/packages/amazon/src/serverGroup/configure/wizard/instanceType/InstanceTypeWarning.tsx b/packages/amazon/src/serverGroup/configure/wizard/instanceType/InstanceTypeWarning.tsx
@@ -0,0 +1,44 @@
+import React from 'react';
+
+import type { IAmazonServerGroupCommandDirty } from '../../serverGroupConfiguration.service';
+
+export function InstanceTypeWarning(props: { dirty: IAmazonServerGroupCommandDirty; clearWarnings: () => void }) {
+  if (
+    props.dirty.instanceType ||
+    (props.dirty.launchTemplateOverridesForInstanceType &&
+      props.dirty.launchTemplateOverridesForInstanceType.length > 0)
+  ) {
+    return (
+      <div className="col-md-12">
+        <div className="alert alert-warning">
+          <p>
+            <i className="fa fa-exclamation-triangle" />
+            {props.dirty.instanceType &&
+              'The following instance type was found incompatible with the selected image/region and was removed:'}
+            {props.dirty.launchTemplateOverridesForInstanceType &&
+              props.dirty.launchTemplateOverridesForInstanceType.length > 0 &&
+              'The following instance type(s) were found incompatible with the selected image/region and were removed:'}
+          </p>
+          <ul>
+            {props.dirty.instanceType && <li key={props.dirty.instanceType}>{props.dirty.instanceType}</li>}
+            {props.dirty.launchTemplateOverridesForInstanceType &&
+              props.dirty.launchTemplateOverridesForInstanceType.length > 0 &&
+              props.dirty.launchTemplateOverridesForInstanceType.map((it) => (
+                <li key={it.instanceType}>
+                  {it.instanceType}
+                  {it.weightedCapacity ? ' with weight ' + it.weightedCapacity : ''}
+                </li>
+              ))}
+          </ul>
+          <p className="text-right">
+            <a className="btn btn-sm btn-default dirty-flag-dismiss clickable" onClick={() => props.clearWarnings()}>
+              Okay
+            </a>
+          </p>
+        </div>
+      </div>
+    );
+  } else {
+    return null;
+  }
+}
diff --git a/...mazon/src/serverGroup/configure/wizard/instanceType/advancedMode/AdvancedModeSelector.tsx b/...mazon/src/serverGroup/configure/wizard/instanceType/advancedMode/AdvancedModeSelector.tsx
@@ -15,6 +15,7 @@ export interface IAdvancedModeSelectorProps {
   formik: FormikProps<IAmazonServerGroupCommand>;
   instanceTypeDetails: IAmazonInstanceTypeCategory[];
   setUnlimitedCpuCredits: (unlimitedCpuCredits: boolean | undefined) => void;
+  clearWarnings: () => void;
 }
 
 /**
@@ -93,6 +94,8 @@ export function AdvancedModeSelector(props: IAdvancedModeSelectorProps) {
         }
         handleInstanceTypesChange={handleInstanceTypesChange}
         setUnlimitedCpuCredits={setUnlimitedCpuCredits}
+        viewState={command.viewState}
+        clearWarnings={props.clearWarnings}
       />
     </div>
   );

diff --git a/...serverGroup/configure/wizard/instanceType/advancedMode/AmazonInstanceTypeInfoRenderer.tsx b/...serverGroup/configure/wizard/instanceType/advancedMode/AmazonInstanceTypeInfoRenderer.tsx
@@ -4,50 +4,50 @@ import React from 'react';
 import type { IAmazonInstanceType } from '../../../../../instance/awsInstanceType.service';
 
 export function AmazonInstanceTypeInfoRenderer(props: { instanceType: IAmazonInstanceType }) {
-  const spotSupport = props.instanceType.supportedUsageClasses.includes('spot') ? '| SPOT supported' : '';
-  const CpuMem = `${props.instanceType.defaultVCpus} vCPU | ${props.instanceType.memoryInGiB} Gib Memory ${spotSupport}`;
-  const instanceStorageInfo = props.instanceType.instanceStorageSupported && (
+  const spotSupport = props.instanceType?.supportedUsageClasses?.includes('spot') ? '| SPOT supported' : '';
+  const CpuMem = `${props.instanceType?.defaultVCpus} vCPU | ${props.instanceType?.memoryInGiB} Gib Memory ${spotSupport}`;
+  const instanceStorageInfo = props.instanceType?.instanceStorageSupported && (
     <span>
       <br />
       <span className={`select-option-label-attributes`}>
-        {`Instance Storage: ${_.toUpper(props.instanceType.instanceStorageInfo.storageTypes)} | ${
-          props.instanceType.instanceStorageInfo.totalSizeInGB
+        {`Instance Storage: ${_.toUpper(props.instanceType?.instanceStorageInfo?.storageTypes)} | ${
+          props.instanceType?.instanceStorageInfo?.totalSizeInGB
         } Gib total size`}
       </span>
     </span>
   );
-  const ebsInfo = props.instanceType.ebsInfo && (
+  const ebsInfo = props.instanceType?.ebsInfo && (
     <span>
       <br />
       <span className={`select-option-label-attributes`}>
-        {`EBS: optimization ${props.instanceType.ebsInfo.ebsOptimizedSupport} | NVMe ${props.instanceType.ebsInfo.nvmeSupport} | Encryption ${props.instanceType.ebsInfo.encryptionSupport}`}
+        {`EBS: optimization ${props.instanceType?.ebsInfo?.ebsOptimizedSupport} | NVMe ${props.instanceType?.ebsInfo?.nvmeSupport} | Encryption ${props.instanceType?.ebsInfo?.encryptionSupport}`}
       </span>
     </span>
   );
-  const gpuInfo = props.instanceType.gpuInfo && (
+  const gpuInfo = props.instanceType?.gpuInfo && (
     <span>
       <br />
       <span className={`select-option-label-attributes`}>
-        {`GPU: ${props.instanceType.gpuInfo.totalGpuMemoryInMiB} MiB total memory`}
-        {props.instanceType.gpuInfo.gpus.map(
+        {`GPU: ${props.instanceType?.gpuInfo?.totalGpuMemoryInMiB} MiB total memory`}
+        {props.instanceType?.gpuInfo?.gpus.map(
           (g) => ` | ${g.count} ${g.manufacturer} ${g.name} GPUs, size: ${g.gpuSizeInMiB} MiB`,
         )}
       </span>
     </span>
   );
-  const generationInfo = typeof props.instanceType.currentGeneration !== 'undefined' &&
-    props.instanceType.currentGeneration !== null && (
+  const generationInfo = typeof props.instanceType?.currentGeneration !== 'undefined' &&
+    props.instanceType?.currentGeneration !== null && (
       <span>
         <br />
         <span className={`select-option-label-attributes`}>
-          {props.instanceType.currentGeneration ? 'Current Generation' : 'Previous Generation'}
+          {props.instanceType?.currentGeneration ? 'Current Generation' : 'Previous Generation'}
         </span>
       </span>
     );
 
   return (
     <span>
-      <span className={`select-option-label`}>{props.instanceType.name}</span>
+      <span className={`select-option-label`}>{props.instanceType?.name}</span>
       <br />
       <span className={`select-option-label-attributes`}>{CpuMem}</span>
       {instanceStorageInfo}

diff --git a/...s/amazon/src/serverGroup/configure/wizard/instanceType/advancedMode/InstanceTypeTable.tsx b/...s/amazon/src/serverGroup/configure/wizard/instanceType/advancedMode/InstanceTypeTable.tsx
@@ -6,10 +6,12 @@ import { CpuCreditsToggle } from '../CpuCreditsToggle';
 import { InstanceTypeTableBody } from './InstanceTypeTableBody';
 import { Header, Heading } from './InstanceTypeTableParts';
 import { Footer } from './InstanceTypeTableParts';
+import { InstanceTypeWarning } from '../InstanceTypeWarning';
 import { AWSProviderSettings } from '../../../../../aws.settings';
 import type { IAmazonInstanceTypeCategory } from '../../../../../instance/awsInstanceType.service';
 import type { IAmazonInstanceType } from '../../../../../instance/awsInstanceType.service';
 import type { IAmazonInstanceTypeOverride } from '../../../serverGroupConfiguration.service';
+import type { IAmazonServerGroupCommandViewState } from '../../../serverGroupConfiguration.service';
 
 import './advancedMode.less';
 
@@ -21,6 +23,8 @@ export interface IInstanceTypeTableProps {
   availableInstanceTypesList: IAmazonInstanceType[];
   handleInstanceTypesChange: (instanceTypes: IAmazonInstanceTypeOverride[]) => void;
   setUnlimitedCpuCredits: (unlimitedCpuCredits: boolean | undefined) => void;
+  viewState: IAmazonServerGroupCommandViewState;
+  clearWarnings: () => void;
 }
 
 export function InstanceTypeTable(props: IInstanceTypeTableProps) {
@@ -96,6 +100,7 @@ export function InstanceTypeTable(props: IInstanceTypeTableProps) {
           profileLabel={label}
           profileDescriptionArr={descriptionListOverride ? descriptionListOverride : families.map((f) => f.description)}
         />
+        <InstanceTypeWarning dirty={props.viewState.dirty} clearWarnings={props.clearWarnings} />
         {isCpuCreditsEnabled && cpuCreditsToggle}
         <table className="table table-hover">
           <Header isCustom={isCustom} showCpuCredits={showCpuCredits} />
@@ -115,6 +120,7 @@ export function InstanceTypeTable(props: IInstanceTypeTableProps) {
     return (
       <div className={'row sub-section'}>
         <Heading isCustom={isCustom} />
+        <InstanceTypeWarning dirty={props.viewState.dirty} clearWarnings={props.clearWarnings} />
         {isCpuCreditsEnabled && cpuCreditsToggle}
         <table className="table table-hover">
           <Header isCustom={isCustom} />

diff --git a/...es/amazon/src/serverGroup/configure/wizard/instanceType/simpleMode/SimpleModeSelector.tsx b/...es/amazon/src/serverGroup/configure/wizard/instanceType/simpleMode/SimpleModeSelector.tsx
@@ -3,13 +3,15 @@ import React from 'react';
 import { NgReact } from '@spinnaker/core';
 
 import { CpuCreditsToggle } from '../CpuCreditsToggle';
+import { InstanceTypeWarning } from '../InstanceTypeWarning';
 import { AWSProviderSettings } from '../../../../../aws.settings';
 import type { IAmazonServerGroupCommand } from '../../../serverGroupConfiguration.service';
 
 export interface ISimpleModeSelectorProps {
   command: IAmazonServerGroupCommand;
   setUnlimitedCpuCredits: (unlimitedCpuCredits: boolean | undefined) => void;
   setFieldValue: (field: keyof IAmazonServerGroupCommand, value: any, shouldValidate?: boolean) => void;
+  clearWarnings: () => void;
 }
 
 export function SimpleModeSelector(props: ISimpleModeSelectorProps) {
@@ -41,6 +43,7 @@ export function SimpleModeSelector(props: ISimpleModeSelectorProps) {
           onTypeChanged={instanceTypeChanged}
           onProfileChanged={instanceProfileChanged}
         />
+        <InstanceTypeWarning dirty={command.viewState.dirty} clearWarnings={props.clearWarnings} />
         <div style={{ padding: '0 15px' }}>
           {command.viewState.instanceProfile && command.viewState.instanceProfile !== 'custom' && (
             <InstanceTypeSelector command={command} onTypeChanged={instanceTypeChanged} />