Address PR comments.

PoeppingT · brtrvn · commit a4be130914c0 · 2022-08-19T15:48:30.000-07:00
This commit allows for FSX ONTAP if Linux/EC2 are selected and
fixes a small bug with the filesystem type selection where the
radio buttons were showing if "Provision a FileSystem" was not
selected. This commit also makes changes to the structuring of
the FILESYSTEM_TYPES constants to allow for different form
validation depending on whether the FSX_ONTAP OS is WINDOWS or LINUX.
diff --git a/client/web/src/settings/ApplicationComponent.js b/client/web/src/settings/ApplicationComponent.js
@@ -26,7 +26,7 @@ import globalConfig from '../config/appConfig'
 import AppSettingsSubform from './AppSettingsSubform'
 import BillingSubform from './BillingSubform'
 import ServicesComponent from './ServicesComponent'
-import { FILESYSTEM_DEFAULTS, FILESYSTEM_TYPES } from './components/filesystem'
+import { FILESYSTEM_DEFAULTS, FILESYSTEM_TYPES, OS_TO_FS_TYPES } from './components/filesystem'
 
 import { dismissConfigError, dismissConfigMessage } from './ducks'
 
@@ -69,7 +69,7 @@ export function ApplicationComponent(props) {
     window.scrollTo(0, 0)
   }
 
-  const generateAppConfigOrDefaultInitialValuesForTier = (tierValues, defaultValues) => {
+  const generateAppConfigOrDefaultInitialValuesForTier = (tierValues, defaultValues, os) => {
     let filesystem = {
       ...FILESYSTEM_DEFAULTS,
       ...defaultValues.filesystem,
@@ -84,11 +84,14 @@ export function ApplicationComponent(props) {
       ...defaults,
       filesystem: filesystem,
     }
+
+    let filesystemType = OS_TO_FS_TYPES[os]?.filter(type => type.configId === tierValues.filesystem?.type)[0]?.id || ''
+
     return {
       ...uncleanedInitialTierValues,
       provisionDb: !!tierValues.database,
       provisionFS: !!tierValues.filesystem,
-      filesystemType: tierValues.filesystem?.type || '',
+      filesystemType: filesystemType,
       filesystem: splitWeeklyMaintenanceTime(uncleanedInitialTierValues.filesystem),
     }
   }
@@ -144,11 +147,11 @@ export function ApplicationComponent(props) {
           }
     const windowsVersion = os === WINDOWS ? thisService.operatingSystem : ''
     let defaultTierName = tiers.filter(t => t.defaultTier)[0].name
-    let defaultTierValues = generateAppConfigOrDefaultInitialValuesForTier(Object.assign({}, thisService?.tiers[defaultTierName]), {})
+    let defaultTierValues = generateAppConfigOrDefaultInitialValuesForTier(Object.assign({}, thisService?.tiers[defaultTierName]), {}, os)
     let initialTierValues = {}
     for (var i = 0; i < tiers.length; i++) {
       var tierName = tiers[i].name
-      initialTierValues[tierName] = generateAppConfigOrDefaultInitialValuesForTier(Object.assign({}, thisService?.tiers[tierName]), defaultTierValues)
+      initialTierValues[tierName] = generateAppConfigOrDefaultInitialValuesForTier(Object.assign({}, thisService?.tiers[tierName]), defaultTierValues, os)
     }
     return {
       ...thisService,
diff --git a/client/web/src/settings/ApplicationContainer.js b/client/web/src/settings/ApplicationContainer.js
@@ -162,11 +162,11 @@ export function ApplicationContainer(props) {
         weeklyMaintenanceTime,
         ...cleanedFs
       } = filesystem
-      cleanedFs.type = filesystemType
+      cleanedFs.type = FILESYSTEM_TYPES[filesystemType].configId
       if (weeklyMaintenanceDay && weeklyMaintenanceTime) {
         cleanedFs.weeklyMaintenanceTime = `${weeklyMaintenanceDay}:${getFormattedTime(weeklyMaintenanceTime)}`
       }
-      let wantedKeys = Object.keys(FILESYSTEM_TYPES[cleanedFs.type].defaults)
+      let wantedKeys = Object.keys(FILESYSTEM_TYPES[filesystemType].defaults)
       Object.keys(cleanedFs).forEach(k => {
         if (!wantedKeys.includes(k) && k !== 'type') {
           delete cleanedFs[k]
diff --git a/client/web/src/settings/TierServiceSettingsSubform.js b/client/web/src/settings/TierServiceSettingsSubform.js
@@ -38,22 +38,24 @@ const TierServiceSettingsSubform = (props) => {
 
   const formikTierPrefix = formikServicePrefix + '.tiers[' + selectedTier + ']'
 
-  // set compute size if default exists and this tier doesn't
-  if (!!!serviceValues?.tiers[selectedTier]?.computeSize && !!serviceValues?.tiers[defaultTier]?.computeSize) {
-    // set instance to default if it doesn't exist already
-    setFieldValue(formikTierPrefix + '.computeSize', serviceValues?.tiers[defaultTier]?.computeSize)
-  }
+  if (!!serviceValues?.tiers) {
+    // set compute size if default exists and this tier doesn't
+    if (!!!serviceValues?.tiers[selectedTier]?.computeSize && !!serviceValues?.tiers[defaultTier]?.computeSize) {
+      // set instance to default if it doesn't exist already
+      setFieldValue(formikTierPrefix + '.computeSize', serviceValues?.tiers[defaultTier]?.computeSize)
+    }
 
-  // set min if default exists and this tier doesn't
-  if (!!!serviceValues?.tiers[selectedTier]?.min && !!serviceValues?.tiers[defaultTier]?.min) {
-    // set instance to default if it doesn't exist already
-    setFieldValue(formikTierPrefix + '.min', serviceValues?.tiers[defaultTier]?.min)
-  }
+    // set min if default exists and this tier doesn't
+    if (!!!serviceValues?.tiers[selectedTier]?.min && !!serviceValues?.tiers[defaultTier]?.min) {
+      // set instance to default if it doesn't exist already
+      setFieldValue(formikTierPrefix + '.min', serviceValues?.tiers[defaultTier]?.min)
+    }
 
-  // set max if default exists and this tier doesn't
-  if (!!!serviceValues?.tiers[selectedTier]?.max && !!serviceValues?.tiers[defaultTier]?.max) {
-    // set instance to default if it doesn't exist already
-    setFieldValue(formikTierPrefix + '.max', serviceValues?.tiers[defaultTier]?.max)
+    // set max if default exists and this tier doesn't
+    if (!!!serviceValues?.tiers[selectedTier]?.max && !!serviceValues?.tiers[defaultTier]?.max) {
+      // set instance to default if it doesn't exist already
+      setFieldValue(formikTierPrefix + '.max', serviceValues?.tiers[defaultTier]?.max)
+    }
   }
 
   return (
@@ -108,6 +110,7 @@ const TierServiceSettingsSubform = (props) => {
                         label="Minimum Instance Count"
                         name={formikTierPrefix + '.min'}
                         type="number"
+                        min="0"
                       />
                     </Col>
                     <Col>
@@ -116,6 +119,7 @@ const TierServiceSettingsSubform = (props) => {
                         label="Maximum Instance Count"
                         name={formikTierPrefix + '.max'}
                         type="number"
+                        min="0"
                       />
                     </Col>
                   </Row>
@@ -131,6 +135,7 @@ const TierServiceSettingsSubform = (props) => {
                       serviceValues?.tiers[selectedTier]?.provisionFS
                     }
                     containerOs={serviceValues?.operatingSystem}
+                    containerLaunchType={serviceValues?.ecsLaunchType}
                     filesystemType={serviceValues?.tiers[selectedTier]?.filesystemType}
                     setFieldValue={props.setFieldValue}
                   ></FileSystemSubform>
diff --git a/client/web/src/settings/components/filesystem/FileSystemSubform.js b/client/web/src/settings/components/filesystem/FileSystemSubform.js
@@ -61,7 +61,7 @@ const FilesystemType = (props) => {
           id={"fs-" + fs.id + "-" + props.formikTierPrefix}
           name={props.formikTierPrefix + ".filesystemType"}
           value={fs.id}
-          disabled={props.isLocked}
+          disabled={!fs.enabled(props.containerOs, props.containerLaunchType)}
         />
         <Label className="form-check-label" check htmlFor={"fs-" + fs.id + "-" + props.formikTierPrefix}>
           <CIcon icon={fs.icon} /> {fs.name}
@@ -80,7 +80,7 @@ class EmptyComponent extends React.Component {
 export default class FileSystemSubform extends React.Component {
   render() {
     var FsComponent = (!!this.props.filesystemType && FILESYSTEM_TYPES[this.props.filesystemType]) 
-        ? FILESYSTEM_TYPES[this.props.filesystemType].component 
+        ? FILESYSTEM_TYPES[this.props.filesystemType].component
         : EmptyComponent
     return (
       <Fragment>
@@ -94,7 +94,9 @@ export default class FileSystemSubform extends React.Component {
                   name={this.props.formikTierPrefix + '.provisionFS'}
                   label="Provision a File System for the application."
                 />
-                <FilesystemType {...this.props}></FilesystemType>
+                {this.props.provisionFs && (
+                  <FilesystemType {...this.props}></FilesystemType>
+                )}
                 {this.props.provisionFs && this.props.filesystemType && (
                   <FsComponent {...this.props}></FsComponent>
                 )}
@@ -111,6 +113,7 @@ FileSystemSubform.propTypes = {
   provisionFs: PropTypes.bool,
   filesystemType: PropTypes.string,
   containerOs: PropTypes.string,
+  containerLaunchType: PropTypes.string,
   isLocked: PropTypes.bool,
   filesystem: PropTypes.object,
   formik: PropTypes.object,
diff --git a/client/web/src/settings/components/filesystem/FsxOntapFilesystemOptions.js b/client/web/src/settings/components/filesystem/FsxOntapFilesystemOptions.js
@@ -230,6 +230,7 @@ export default class FsxOntapFilesystemOptions extends React.Component {
                   </Row>
                   <Row>
                     <Col xs={6}>
+                      {this.props.containerOs === "WINDOWS" && (
                       <SaasBoostSelect
                         id={
                           props.formikTierPrefix + '.filesystem.windowsMountDrive'
@@ -260,6 +261,7 @@ export default class FsxOntapFilesystemOptions extends React.Component {
                         <option value="Y:">Y:</option>
                         <option value="Z:">Z:</option>
                       </SaasBoostSelect>
+                      )}
                     </Col>
                     <Col xs={6}>
                       <SaasBoostInput
diff --git a/client/web/src/settings/components/filesystem/index.js b/client/web/src/settings/components/filesystem/index.js
@@ -26,6 +26,7 @@ export const FILESYSTEM_TYPES = {
         "name": "EFS",
         "icon": cibAmazonAws,
         "component": EfsFilesystemOptions,
+        "enabled": (os, launchType) => os === "LINUX",
         "defaults": {
             mountPoint: '',
             lifecycle: 'NEVER',
@@ -50,6 +51,7 @@ export const FILESYSTEM_TYPES = {
         "name": "FSX Windows",
         "icon": cibWindows,
         "component": FsxWindowsFilesystemOptions,
+        "enabled": (os, launchType) => os === "WINDOWS",
         "defaults": {
             mountPoint: '',
             storageGb: 32,
@@ -87,11 +89,64 @@ export const FILESYSTEM_TYPES = {
                 .required('Windows mount drive is required'),
         })
     },
-    "FSX_ONTAP": {
-        "id": "FSX_ONTAP",
+    "FSX_ONTAP_LINUX": {
+        "configId": "FSX_ONTAP",
+        "id": "FSX_ONTAP_LINUX",
         "name": "FSX Ontap",
         "icon": cilViewQuilt,
         "component": FsxOntapFilesystemOptions,
+        "enabled": (os, launchType) => os === "LINUX" && launchType === "EC2",
+        "defaults": {
+            mountPoint: '',
+            storageGb: 1024,
+            throughputMbs: 128,
+            backupRetentionDays: 7,
+            dailyBackupTime: '01:00',
+            weeklyMaintenanceTime: '07:01:00',
+            weeklyMaintenanceDay: '1',
+            volumeSize: 40
+        },
+        "validationSchema": Yup.object({
+            mountPoint: Yup.string()
+                .matches(/^(\/[a-zA-Z._-]+)*$/, 'Invalid path. Ex: /mnt')
+                .max(100, "The full path can't exceed 100 characters in length")
+                .test(
+                    'subdirectories',
+                    'The path can only include up to four subdirectories',
+                    (val) => (val?.match(/\//g) || []).length <= 4
+                )
+                .required(),
+            storageGb: Yup.number()
+                .required()
+                .min(1024, 'Storage minimum is 1024 GB')
+                .max(196608, 'Storage maximum is 196,608 GB'),
+            throughputMbs: Yup.number()
+                .required()
+                .min(128, 'Throughput minimum is 128 MB/s')
+                .max(2048, 'Throughput maximum is 2048 MB/s'),
+            backupRetentionDays: Yup.number()
+                .required()
+                .min(7, 'Minimum retention time is 7 days')
+                .max(35, 'Maximum retention time is 35 days'),
+            dailyBackupTime: Yup.string()
+                .required('Daily backup time is required'),
+            weeklyMaintenanceTime: Yup.string()
+                .required('Weekly maintenance time is required'),
+            windowsMountDrive: Yup.string()
+                .required('Windows mount drive is required'),
+            volumeSize: Yup.number()
+                .required()
+                .min(0, 'Volume Size must be a positive number')
+                .max(196608, 'Volume size maximum is 196,608 GB')
+        })
+    },
+    "FSX_ONTAP_WINDOWS": {
+        "configId": "FSX_ONTAP",
+        "id": "FSX_ONTAP_WINDOWS",
+        "name": "FSX Ontap",
+        "icon": cilViewQuilt,
+        "component": FsxOntapFilesystemOptions,
+        "enabled": (os, launchType) => os === "WINDOWS",
         "defaults": {
             mountPoint: '',
             storageGb: 1024,
@@ -131,7 +186,7 @@ export const FILESYSTEM_TYPES = {
             volumeSize: Yup.number()
                 .required()
                 .min(0, 'Volume Size must be a positive number')
-                .max(196608, 'Volume size maximum is 196.608 GB')
+                .max(196608, 'Volume size maximum is 196,608 GB')
         })
     }
 }
@@ -144,10 +199,11 @@ export const FILESYSTEM_DEFAULTS = Object.assign(Object.keys(FILESYSTEM_TYPES)
 
 export const OS_TO_FS_TYPES = {
     "LINUX": [
-        FILESYSTEM_TYPES.EFS
+        FILESYSTEM_TYPES.EFS,
+        FILESYSTEM_TYPES.FSX_ONTAP_LINUX,
     ],
     "WINDOWS": [
         FILESYSTEM_TYPES.FSX_WINDOWS,
-        FILESYSTEM_TYPES.FSX_ONTAP,
+        FILESYSTEM_TYPES.FSX_ONTAP_WINDOWS,
     ]
 }
