Disable Cost Monitoring in unsupported regions (#240)

* Add a way to handle unsupported regions for features in featureFlagProvider * Update tests and references to reflect changes in featureFlagProvider
aws · Jun 5, 2023 · 0b34e01 · 0b34e01
1 parent 8c14152
commit 0b34e01
Show file tree

Hide file tree

Showing 9 changed files with 110 additions and 23 deletions.
diff --git a/frontend/src/feature-flags/__tests__/FeatureFlagsProvider.test.ts b/frontend/src/feature-flags/__tests__/FeatureFlagsProvider.test.ts
@@ -13,24 +13,25 @@ import {AvailableFeature} from '../types'
 
 describe('given a feature flags provider and a list of rules', () => {
   const subject = featureFlagsProvider
+  let region: string | undefined = 'us-west-1'
 
   describe('when the features list is retrieved', () => {
     it('should return the list', async () => {
-      const features = await subject('0.0.0')
+      const features = await subject('0.0.0', region)
       expect(features).toEqual<AvailableFeature[]>([])
     })
   })
 
   describe('when the version is between 3.1.0 and 3.2.0', () => {
     it('should return the list of available features', async () => {
-      const features = await subject('3.1.5')
+      const features = await subject('3.1.5', region)
       expect(features).toEqual<AvailableFeature[]>(['multiuser_cluster'])
     })
   })
 
   describe('when the version is between 3.2.0 and 3.3.0', () => {
     it('should return the list of available features', async () => {
-      const features = await subject('3.2.5')
+      const features = await subject('3.2.5', region)
       expect(features).toEqual<AvailableFeature[]>([
         'multiuser_cluster',
         'fsx_ontap',
@@ -46,7 +47,7 @@ describe('given a feature flags provider and a list of rules', () => {
 
   describe('when the version is between 3.3.0 and 3.4.0', () => {
     it('should return the list of available features', async () => {
-      const features = await subject('3.3.2')
+      const features = await subject('3.3.2', region)
       expect(features).toEqual<AvailableFeature[]>([
         'multiuser_cluster',
         'fsx_ontap',
@@ -68,7 +69,7 @@ describe('given a feature flags provider and a list of rules', () => {
 
   describe('when the version is between 3.4.0 and 3.6.0', () => {
     it('should return the list of available features', async () => {
-      const features = await subject('3.4.1')
+      const features = await subject('3.4.1', region)
       expect(features).toEqual<AvailableFeature[]>([
         'multiuser_cluster',
         'fsx_ontap',
@@ -92,7 +93,7 @@ describe('given a feature flags provider and a list of rules', () => {
 
   describe('when the version is above and 3.6.0', () => {
     it('should return the list of available features', async () => {
-      const features = await subject('3.6.0')
+      const features = await subject('3.6.0', region)
       expect(features).toEqual<AvailableFeature[]>([
         'multiuser_cluster',
         'fsx_ontap',
@@ -122,7 +123,7 @@ describe('given a feature flags provider and a list of rules', () => {
       window.sessionStorage.setItem('additionalFeatures', '["cost_monitoring"]')
     })
     it('should be included in the list of features', async () => {
-      const features = await subject('3.1.5')
+      const features = await subject('3.1.5', region)
       expect(features).toEqual<AvailableFeature[]>([
         'multiuser_cluster',
         'cost_monitoring',
@@ -135,8 +136,24 @@ describe('given a feature flags provider and a list of rules', () => {
       window.sessionStorage.clear()
     })
     it('should not be included in the list of features', async () => {
-      const features = await subject('3.1.5')
+      const features = await subject('3.1.5', region)
       expect(features).toEqual<AvailableFeature[]>(['multiuser_cluster'])
     })
   })
+
+  describe('when a feature is not supported in a region', () => {
+    it('should return the list of available features without the unsupported feature', async () => {
+      region = 'us-gov-west-1'
+      const features = await subject('3.6.0', region)
+      expect(features).not.toContain<AvailableFeature[]>(['cost_monitoring'])
+    })
+  })
+
+  describe('when a feature is not supported in a region and the region is undefined', () => {
+    it('should return an empty list', async () => {
+      region = undefined
+      const features = await subject('3.6.0', region)
+      expect(features).not.toContain<AvailableFeature[]>(['cost_monitoring'])
+    })
+  })
 })
diff --git a/frontend/src/feature-flags/featureFlagsProvider.ts b/frontend/src/feature-flags/featureFlagsProvider.ts
@@ -34,6 +34,26 @@ const versionToFeaturesMap: Record<string, AvailableFeature[]> = {
   '3.6.0': ['rhel8', 'new_resources_limits'],
 }
 
+const featureToUnsupportedRegionsMap: Partial<
+  Record<AvailableFeature, string[]>
+> = {
+  cost_monitoring: ['us-gov-west-1'],
+}
+
+function isSupportedInRegion(
+  feature: AvailableFeature,
+  region?: string,
+): boolean {
+  if (feature in featureToUnsupportedRegionsMap) {
+    if (!region) {
+      return false
+    } else {
+      return !featureToUnsupportedRegionsMap[feature]!.includes(region)
+    }
+  }
+  return true
+}
+
 function composeFlagsListByVersion(currentVersion: string): AvailableFeature[] {
   let features: Set<AvailableFeature> = new Set([])
 
@@ -46,7 +66,10 @@ function composeFlagsListByVersion(currentVersion: string): AvailableFeature[] {
   return Array.from(features)
 }
 
-export function featureFlagsProvider(version: string): AvailableFeature[] {
+export function featureFlagsProvider(
+  version: string,
+  region?: string,
+): AvailableFeature[] {
   const features: AvailableFeature[] = []
   const additionalFeatures = window.sessionStorage.getItem('additionalFeatures')
   const additionalFeaturesParsed = additionalFeatures
@@ -56,4 +79,5 @@ export function featureFlagsProvider(version: string): AvailableFeature[] {
   return features
     .concat(composeFlagsListByVersion(version))
     .concat(additionalFeaturesParsed)
+    .filter(feature => isSupportedInRegion(feature, region))
 }
diff --git a/frontend/src/feature-flags/useFeatureFlag.ts b/frontend/src/feature-flags/useFeatureFlag.ts
@@ -14,14 +14,16 @@ import {AvailableFeature} from './types'
 
 export function useFeatureFlag(feature: AvailableFeature): boolean {
   const version = useState(['app', 'version', 'full'])
-  return isFeatureEnabled(version, feature)
+  const region = useState(['aws', 'region'])
+  return isFeatureEnabled(version, region, feature)
 }
 
 export function isFeatureEnabled(
   version: string,
+  region: string,
   feature: AvailableFeature,
 ): boolean {
-  const features = new Set(featureFlagsProvider(version))
+  const features = new Set(featureFlagsProvider(version, region))
 
   return features.has(feature)
 }
diff --git a/frontend/src/old-pages/Clusters/Costs/__tests__/EnableCostMonitoringButton.test.tsx b/frontend/src/old-pages/Clusters/Costs/__tests__/EnableCostMonitoringButton.test.tsx
@@ -61,6 +61,9 @@ describe('given a component to activate cost monitoring for the account', () =>
         app: {
           version: {full: '3.2.0'},
         },
+        aws: {
+          region: 'us-west-1',
+        },
       })
 
       screen = render(
@@ -91,6 +94,9 @@ describe('given a component to activate cost monitoring for the account', () =>
         app: {
           version: {full: '3.2.0'},
         },
+        aws: {
+          region: 'us-west-1',
+        },
       })
       mockGetCostMonitoringStatus.mockResolvedValue(true)
 
@@ -117,6 +123,9 @@ describe('given a component to activate cost monitoring for the account', () =>
         app: {
           version: {full: '3.1.5'},
         },
+        aws: {
+          region: 'us-west-1',
+        },
       })
 
       screen = render(

diff --git a/frontend/src/old-pages/Clusters/Costs/__tests__/useCostMonitoringStatus.test.tsx b/frontend/src/old-pages/Clusters/Costs/__tests__/useCostMonitoringStatus.test.tsx
@@ -41,6 +41,7 @@ jest.mock('../../../../model', () => {
 describe('given a hook to get the cost monitoring status', () => {
   beforeEach(() => {
     jest.clearAllMocks()
+    mockQueryClient.resetQueries()
   })
 
   describe('when PC version is at least 3.2.0', () => {
@@ -49,6 +50,9 @@ describe('given a hook to get the cost monitoring status', () => {
         app: {
           version: {full: '3.2.0'},
         },
+        aws: {
+          region: 'us-west-1',
+        },
       })
     })
 
@@ -65,6 +69,27 @@ describe('given a hook to get the cost monitoring status', () => {
         app: {
           version: {full: '3.1.5'},
         },
+        aws: {
+          region: 'us-west-1',
+        },
+      })
+    })
+    it('should not request the cost monitoring status', async () => {
+      renderHook(() => useCostMonitoringStatus(), {wrapper})
+
+      expect(mockGetCostMonitoringStatus).toHaveBeenCalledTimes(0)
+    })
+  })
+
+  describe('when PC version is at least 3.2.0, and the region is us-gov-west-1', () => {
+    beforeEach(() => {
+      mockStore.getState.mockReturnValue({
+        app: {
+          version: {full: '3.2.0'},
+        },
+        aws: {
+          region: 'us-gov-west-1',
+        },
       })
     })
     it('should not request the cost monitoring status', async () => {

diff --git a/frontend/src/old-pages/Configure/Queues/Queues.tsx b/frontend/src/old-pages/Configure/Queues/Queues.tsx
@@ -68,6 +68,7 @@ import InfoLink from '../../../components/InfoLink'
 // Constants
 const queuesPath = ['app', 'wizard', 'config', 'Scheduling', 'SlurmQueues']
 const queuesErrorsPath = ['app', 'wizard', 'errors', 'queues']
+const defaultRegion = getState(['aws', 'region'])
 
 export function useClusterResourcesLimits(): ClusterResourcesLimits {
   const newResourcesLimits = useFeatureFlag('new_resources_limits')
@@ -203,7 +204,7 @@ function queueValidate(queueIndex: any) {
   }
 
   const version = getState(['app', 'version', 'full'])
-  const isMultiAZActive = isFeatureEnabled(version, 'multi_az')
+  const isMultiAZActive = isFeatureEnabled(version, defaultRegion, 'multi_az')
   if (!queueSubnet) {
     let message: string
     if (isMultiAZActive) {
@@ -219,6 +220,7 @@ function queueValidate(queueIndex: any) {
 
   const isMultiInstanceTypesActive = isFeatureEnabled(
     version,
+    defaultRegion,
     'queues_multiple_instance_types',
   )
   const {validateComputeResources} = !isMultiInstanceTypesActive
@@ -246,6 +248,7 @@ function queueValidate(queueIndex: any) {
 
   const isMemoryBasedSchedulingActive = isFeatureEnabled(
     version,
+    defaultRegion,
     'memory_based_scheduling',
   )
   if (isMemoryBasedSchedulingActive) {

diff --git a/frontend/src/old-pages/Configure/Queues/__tests__/mapComputeResources.test.ts b/frontend/src/old-pages/Configure/Queues/__tests__/mapComputeResources.test.ts
@@ -17,6 +17,7 @@ import {
 
 describe('given a mapper to import the ComputeResources section of the Scheduling config', () => {
   let mockVersion: string
+  let mockRegion: string
 
   describe('when the application supports multiple instance types', () => {
     beforeEach(() => {
@@ -49,9 +50,9 @@ describe('given a mapper to import the ComputeResources section of the Schedulin
             ],
           },
         ]
-        expect(mapComputeResources(mockVersion, mockComputeResources)).toEqual(
-          expected,
-        )
+        expect(
+          mapComputeResources(mockVersion, mockRegion, mockComputeResources),
+        ).toEqual(expected)
       })
     })
     describe('when the configuration was created with the flexible instance types format', () => {
@@ -72,9 +73,9 @@ describe('given a mapper to import the ComputeResources section of the Schedulin
         ]
       })
       it('should leave the configuration as is', () => {
-        expect(mapComputeResources(mockVersion, mockComputeResources)).toEqual(
-          mockComputeResources,
-        )
+        expect(
+          mapComputeResources(mockVersion, mockRegion, mockComputeResources),
+        ).toEqual(mockComputeResources)
       })
     })
   })
@@ -95,9 +96,9 @@ describe('given a mapper to import the ComputeResources section of the Schedulin
     })
 
     it('should leave the configuration as is', () => {
-      expect(mapComputeResources(mockVersion, mockComputeResources)).toEqual(
-        mockComputeResources,
-      )
+      expect(
+        mapComputeResources(mockVersion, mockRegion, mockComputeResources),
+      ).toEqual(mockComputeResources)
     })
   })
 })
diff --git a/frontend/src/old-pages/Configure/Queues/queues.mapper.ts b/frontend/src/old-pages/Configure/Queues/queues.mapper.ts
@@ -35,11 +35,12 @@ function mapComputeResource(
 
 export function mapComputeResources(
   version: string,
+  region: string,
   computeResourcesConfig:
     | SingleInstanceComputeResource[]
     | MultiInstanceComputeResource[],
 ) {
-  if (!isFeatureEnabled(version, 'queues_multiple_instance_types')) {
+  if (!isFeatureEnabled(version, region, 'queues_multiple_instance_types')) {
     return computeResourcesConfig
   }
 

diff --git a/frontend/src/old-pages/Configure/util.tsx b/frontend/src/old-pages/Configure/util.tsx
@@ -26,6 +26,7 @@ function loadTemplateLazy(config: any, callback?: () => void) {
   const keypairNames = new Set(keypairs.map((kp: any) => kp.KeyName))
   const keypairPath = ['HeadNode', 'Ssh', 'KeyName']
   const version = getState(['app', 'version', 'full'])
+  const defaultRegion = getState(['aws', 'region'])
   if (getIn(config, ['Image', 'CustomAmi']))
     setState(['app', 'wizard', 'customAMI', 'enabled'], true)
 
@@ -67,7 +68,11 @@ function loadTemplateLazy(config: any, callback?: () => void) {
         i,
         'ComputeResources',
       ])
-      computeResources = mapComputeResources(version, computeResources)
+      computeResources = mapComputeResources(
+        version,
+        defaultRegion,
+        computeResources,
+      )
       config = setIn(
         config,
         ['Scheduling', 'SlurmQueues', i, 'ComputeResources'],