Lookup non existant service in decom (#351)

Decommission flag.
Default to true for most decommissions.
Remove Docker image feature now stable.
No need to scale if already scaled.

src/api/decommission-service/controller.js

@@ -3,9 +3,10 @@ import { removeStatus } from '~/src/api/status/helpers/remove-status.js'
 import { portalBackEndDecommissionService } from '~/src/api/decommission-service/helpers/decommission-portal-backend.js'
 import { triggerRemoveWorkflows } from '~/src/api/decommission-service/helpers/trigger-remove-workflows.js'
 import { getRepositoryInfo } from '~/src/helpers/portal-backend/get-repository-info.js'
-import { undeployServiceFromAllEnvironments } from '~/src/api/undeploy/helpers/undeploy-service-from-all-environments.js'
 import { deleteDockerImages } from '~/src/api/decommission-service/helpers/delete-docker-images.js'
 import { triggerArchiveGithubWorkflow } from '~/src/api/decommission-service/helpers/archive-github-workflow.js'
+import { featureToggles } from '~/src/helpers/feature-toggle/feature-toggles.js'
+import { isFeatureEnabled } from '~/src/helpers/feature-toggle/is-feature-enabled.js'
 
 const decommissionServiceController = {
   options: {
@@ -25,9 +26,18 @@ const decommissionServiceController = {
     const serviceName = request.params.serviceName
     const logger = request.logger
     const user = request.auth.credentials
+
+    if (!isFeatureEnabled(featureToggles.decommissionService)) {
+      logger.info('Decommission service feature is disabled')
+      return h
+        .response({
+          message: 'Decommission disabled'
+        })
+        .code(409)
+    }
+
     const response = await getRepositoryInfo(serviceName)
 
-    await undeployServiceFromAllEnvironments({ serviceName, user, logger })
     await deleteDockerImages(serviceName, user, logger)
     await triggerRemoveWorkflows(serviceName, response.repository, logger)
     await portalBackEndDecommissionService(serviceName)

src/api/decommission-service/helpers/delete-docker-images.js

@@ -1,7 +1,5 @@
 import { deleteDockerHubImages } from '~/src/helpers/remove/workflows/delete-dockerhub-images.js'
 import { deleteEcrImages } from '~/src/helpers/remove/workflows/delete-ecr-images.js'
-import { isFeatureEnabled } from '~/src/helpers/feature-toggle/is-feature-enabled.js'
-import { featureToggles } from '~/src/helpers/feature-toggle/feature-toggles.js'
 
 /**
  * Delete docker images
@@ -13,15 +11,7 @@ import { featureToggles } from '~/src/helpers/feature-toggle/feature-toggles.js'
 export async function deleteDockerImages(serviceName, user, logger) {
   logger.info(`Deleting docker images for service: ${serviceName}`)
 
-  if (isFeatureEnabled(featureToggles.dockerImages.deleteEcr)) {
-    await deleteEcrImages(serviceName, logger)
-  } else {
-    logger.info('Deleting ECR images feature is disabled')
-  }
+  await deleteEcrImages(serviceName, logger)
 
-  if (isFeatureEnabled(featureToggles.dockerImages.deleteDockerHub)) {
-    await deleteDockerHubImages(serviceName, logger)
-  } else {
-    logger.info('Deleting DockerHub images feature is disabled')
-  }
+  await deleteDockerHubImages(serviceName, logger)
 }

src/api/decommission-service/helpers/delete-docker-images.test.js

@@ -1,61 +1,23 @@
 import { deleteDockerHubImages } from '~/src/helpers/remove/workflows/delete-dockerhub-images.js'
 import { deleteEcrImages } from '~/src/helpers/remove/workflows/delete-ecr-images.js'
-import { isFeatureEnabled } from '~/src/helpers/feature-toggle/is-feature-enabled.js'
 import { deleteDockerImages } from '~/src/api/decommission-service/helpers/delete-docker-images.js'
 
-jest.mock('~/src/helpers/feature-toggle/is-feature-enabled', () => {
-  return {
-    isFeatureEnabled: jest.fn()
-  }
-})
-
-jest.mock('~/src/helpers/remove/workflows/delete-dockerhub-images', () => {
-  return {
-    deleteDockerHubImages: jest.fn()
-  }
-})
-
-jest.mock('~/src/helpers/remove/workflows/delete-ecr-images', () => {
-  return {
-    deleteEcrImages: jest.fn()
-  }
-})
+jest.mock('~/src/helpers/remove/workflows/delete-dockerhub-images')
+jest.mock('~/src/helpers/remove/workflows/delete-ecr-images')
 
-const logger = {
-  info: jest.fn()
-}
+const logger = { info: jest.fn() }
 const serviceName = 'some-service'
 const user = { id: 'some-user-id', displayName: 'some-name' }
 
 describe('#deleteDockerImages', () => {
-  test('if not enabled should not call delete ECR images', async () => {
-    isFeatureEnabled.mockReturnValue(false)
-
-    await deleteDockerImages(serviceName, user, logger)
-
-    expect(deleteEcrImages).toHaveBeenCalledTimes(0)
-  })
-
-  test('if enabled should call delete ECR images', async () => {
-    isFeatureEnabled.mockReturnValueOnce(true).mockReturnValue(false)
-
+  test('should call delete ECR images', async () => {
     await deleteDockerImages(serviceName, user, logger)
 
     expect(deleteEcrImages).toHaveBeenCalledTimes(1)
     expect(deleteEcrImages).toHaveBeenCalledWith(serviceName, logger)
   })
 
-  test('if not enabled should not call delete DockerHub images', async () => {
-    isFeatureEnabled.mockReturnValue(false)
-
-    await deleteDockerImages(serviceName, user, logger)
-
-    expect(deleteDockerHubImages).toHaveBeenCalledTimes(0)
-  })
-
-  test('if enabled should call delete DockerHub images', async () => {
-    isFeatureEnabled.mockReturnValueOnce(false).mockReturnValue(true)
-
+  test('should call delete DockerHub images', async () => {
     await deleteDockerImages(serviceName, user, logger)
 
     expect(deleteDockerHubImages).toHaveBeenCalledTimes(1)

src/api/deploy-terminal/controllers/deploy-terminal.js

@@ -6,7 +6,7 @@ import { sendSnsMessage } from '~/src/helpers/sns/send-sns-message.js'
 import { canRunTerminalInEnvironment } from '~/src/api/deploy-terminal/helpers/can-run-terminal-in-environment.js'
 import { generateTerminalToken } from '~/src/api/deploy-terminal/helpers/generate-terminal-token.js'
 import { deployTerminalPayload } from '~/src/api/deploy-terminal/helpers/deploy-terminal-payload.js'
-import { lookupTenantService } from '~/src/api/deploy/helpers/lookup-tenant-service.js'
+import { lookupTenantService } from '~/src/helpers/portal-backend/lookup-tenant-service.js'
 
 const deployTerminalController = {
   options: {

src/api/deploy/controllers/deploy-service.js

@@ -7,7 +7,7 @@ import { getRepoTeams } from '~/src/api/deploy/helpers/get-repo-teams.js'
 import { sendSnsDeploymentMessage } from '~/src/api/deploy/helpers/send-sns-deployment-message.js'
 import { generateDeployment } from '~/src/helpers/deployments/generate-deployment.js'
 import { commitDeploymentFile } from '~/src/helpers/deployments/commit-deployment-file.js'
-import { lookupTenantService } from '~/src/api/deploy/helpers/lookup-tenant-service.js'
+import { lookupTenantService } from '~/src/helpers/portal-backend/lookup-tenant-service.js'
 import { getLatestAppConfigCommitSha } from '~/src/helpers/portal-backend/get-latest-app-config-commit-sha.js'
 
 const deployFromFileEnvironments = config.get('deployFromFileEnvironments')

src/api/deploy/controllers/existing-service-info.js

@@ -1,5 +1,5 @@
 import { config } from '~/src/config/index.js'
-import { lookupTenantService } from '~/src/api/deploy/helpers/lookup-tenant-service.js'
+import { lookupTenantService } from '~/src/helpers/portal-backend/lookup-tenant-service.js'
 import { getExistingDeployment } from '~/src/api/deploy/helpers/get-existing-deployment.js'
 
 const deploymentRepo = config.get('github.repos.appDeployments')

src/api/undeploy/helpers/delete-deployment-file.js

@@ -12,7 +12,10 @@ export async function deleteDeploymentFiles({
 }) {
   logger.info(`Deleting deployment files for ${serviceName}`)
 
-  if (!isFeatureEnabled(featureToggles.undeploy.deleteDeploymentFiles)) {
+  if (
+    !isFeatureEnabled(featureToggles.decommissionService) ||
+    !isFeatureEnabled(featureToggles.undeploy.deleteDeploymentFiles)
+  ) {
     logger.info('Deleting deployment file feature is disabled')
     return
   }

src/api/undeploy/helpers/delete-deployment-file.test.js

@@ -21,8 +21,16 @@ describe('#deleteDeploymentFiles', () => {
     expect(removeDeployment).toHaveBeenCalledWith(serviceName, logger)
   })
 
+  test('should not delete deployment file if decommission is disabled', async () => {
+    isFeatureEnabled.mockReturnValueOnce(false).mockReturnValue(true)
+
+    await deleteDeploymentFiles({ serviceName, logger })
+
+    expect(removeDeployment).toHaveBeenCalledTimes(0)
+  })
+
   test('should not delete deployment file if feature disabled', async () => {
-    isFeatureEnabled.mockReturnValue(false)
+    isFeatureEnabled.mockReturnValueOnce(true).mockReturnValue(false)
 
     await deleteDeploymentFiles({ serviceName, logger })
 

src/api/undeploy/helpers/delete-ecs-service.js

@@ -1,6 +1,6 @@
 import { createLogger } from '~/src/helpers/logging/logger.js'
 import { orderedEnvironments } from '~/src/config/environments.js'
-import { lookupTenantService } from '~/src/api/deploy/helpers/lookup-tenant-service.js'
+import { lookupTenantService } from '~/src/helpers/portal-backend/lookup-tenant-service.js'
 import { featureToggles } from '~/src/helpers/feature-toggle/feature-toggles.js'
 import { isFeatureEnabled } from '~/src/helpers/feature-toggle/is-feature-enabled.js'
 import { removeEcsService } from '~/src/helpers/remove/workflows/remove-ecs-service.js'
@@ -15,7 +15,10 @@ async function deleteEcsService({
 }) {
   logger.info(`Deleting ECS service for ${serviceName} in env ${environment}`)
 
-  if (!isFeatureEnabled(featureToggles.undeploy.deleteEcsService)) {
+  if (
+    !isFeatureEnabled(featureToggles.decommissionService) ||
+    !isFeatureEnabled(featureToggles.undeploy.deleteEcsService)
+  ) {
     logger.info('Deleting ECS service feature is disabled')
     return
   }

src/api/undeploy/helpers/delete-ecs-service.test.js

@@ -2,12 +2,12 @@ import {
   deleteEcsService,
   deleteAllEcsServices
 } from '~/src/api/undeploy/helpers/delete-ecs-service.js'
-import { lookupTenantService } from '~/src/api/deploy/helpers/lookup-tenant-service.js'
+import { lookupTenantService } from '~/src/helpers/portal-backend/lookup-tenant-service.js'
 import { isFeatureEnabled } from '~/src/helpers/feature-toggle/is-feature-enabled.js'
 import { removeEcsService } from '~/src/helpers/remove/workflows/remove-ecs-service.js'
 import { featureToggles } from '~/src/helpers/feature-toggle/feature-toggles.js'
 
-jest.mock('~/src/api/deploy/helpers/lookup-tenant-service')
+jest.mock('~/src/helpers/portal-backend/lookup-tenant-service')
 jest.mock('~/src/helpers/feature-toggle/is-feature-enabled')
 jest.mock('~/src/helpers/remove/workflows/remove-ecs-service')
 
@@ -37,7 +37,37 @@ describe('#deleteEcsService', () => {
     )
   })
 
-  test('should not remove ECS service if feature disabled', async () => {
+  test('should not remove ECS service if decommission is enabled but feature is disabled', async () => {
+    isFeatureEnabled.mockReturnValueOnce(true).mockReturnValue(false)
+
+    await deleteEcsService({ serviceName, environment, logger })
+
+    expect(isFeatureEnabled).toHaveBeenCalledTimes(2)
+    expect(isFeatureEnabled).toHaveBeenNthCalledWith(
+      1,
+      featureToggles.decommissionService
+    )
+    expect(isFeatureEnabled).toHaveBeenLastCalledWith(
+      featureToggles.undeploy.deleteEcsService
+    )
+    expect(lookupTenantService).toHaveBeenCalledTimes(0)
+    expect(removeEcsService).toHaveBeenCalledTimes(0)
+  })
+
+  test('should not remove ECS service if decommission feature is disabled', async () => {
+    isFeatureEnabled.mockReturnValueOnce(false).mockReturnValue(true)
+
+    await deleteEcsService({ serviceName, environment, logger })
+
+    expect(isFeatureEnabled).toHaveBeenCalledTimes(1)
+    expect(isFeatureEnabled).toHaveBeenLastCalledWith(
+      featureToggles.decommissionService
+    )
+    expect(lookupTenantService).toHaveBeenCalledTimes(0)
+    expect(removeEcsService).toHaveBeenCalledTimes(0)
+  })
+
+  test('should not remove ECS service if both features are disabled', async () => {
     isFeatureEnabled.mockReturnValue(false)
 
     await deleteEcsService({ serviceName, environment, logger })

src/api/undeploy/helpers/scale-ecs-to-zero.js

@@ -27,6 +27,11 @@ export async function scaleEcsToZero({
     return
   }
 
+  if (runningDetails.instanceCount === 0) {
+    logger.info('ECS Service already scaled to 0')
+    return
+  }
+
   const deployment = transformRunningDetailsToDeployment(runningDetails, zone)
   const undeployment = scaleDeploymentToZeroInstances({
     deployment,

src/api/undeploy/helpers/scale-ecs-to-zero.test.js

@@ -51,6 +51,18 @@ describe('#scaleEcsToZero', () => {
     expect(commitDeploymentFile).toHaveBeenCalledTimes(0)
   })
 
+  test('If existing deployment is already scaled to 0 do nothing', async () => {
+    findRunningDetails.mockReturnValue({
+      ...runningDetails,
+      instanceCount: 0
+    })
+
+    await scaleEcsToZero(scaleDetails)
+
+    expect(findRunningDetails).toHaveBeenCalledTimes(1)
+    expect(commitDeploymentFile).toHaveBeenCalledTimes(0)
+  })
+
   test('With existing deployment should proceed with scale to 0', async () => {
     findRunningDetails.mockReturnValue(runningDetails)
 

src/api/undeploy/helpers/undeploy-service-from-environment.js

@@ -1,7 +1,7 @@
 import { config } from '~/src/config/index.js'
 import { registerUndeployment } from '~/src/api/undeploy/helpers/register-undeployment.js'
 import { scaleEcsToZero } from '~/src/api/undeploy/helpers/scale-ecs-to-zero.js'
-import { lookupTenantService } from '~/src/api/deploy/helpers/lookup-tenant-service.js'
+import { lookupServiceInEnvironment } from '~/src/helpers/portal-backend/lookup-tenant-service.js'
 import { isFeatureEnabled } from '~/src/helpers/feature-toggle/is-feature-enabled.js'
 import { featureToggles } from '~/src/helpers/feature-toggle/feature-toggles.js'
 import { createLogger } from '~/src/helpers/logging/logger.js'
@@ -30,23 +30,18 @@ export async function undeployServiceFromEnvironment({
     return undeploymentId
   }
 
-  const service = await lookupTenantService(serviceName, environment, logger)
+  const zone = await lookupZone(serviceName, environment)
 
-  if (!service) {
+  if (!zone) {
     logger.warn(
       `Unable to find deployment zone for [${serviceName}] in environment [${environment}].`
     )
     return
   }
 
-  if (isFeatureEnabled(featureToggles.undeploy.register)) {
-    await registerUndeployment(serviceName, environment, user, undeploymentId)
-    logger.info('Undeployment registered')
-  } else {
-    logger.info('Undeployment registration feature is disabled')
-  }
+  await registerUndeployment(serviceName, environment, user, undeploymentId)
 
-  if (isFeatureEnabled(featureToggles.undeploy.scaleEcsToZero)) {
+  if (isFeatureEnabled(featureToggles.scaleEcsToZero)) {
     const shouldDeployByFile = deployFromFileEnvironments.includes(environment)
     if (!shouldDeployByFile) {
       logger.warn(
@@ -56,7 +51,7 @@ export async function undeployServiceFromEnvironment({
       await scaleEcsToZero({
         serviceName,
         environment,
-        zone: service.zone,
+        zone,
         user,
         undeploymentId,
         logger
@@ -67,3 +62,11 @@ export async function undeployServiceFromEnvironment({
   }
   return undeploymentId
 }
+
+async function lookupZone(serviceName, environment) {
+  const { serviceJson } = await lookupServiceInEnvironment(
+    serviceName,
+    environment
+  )
+  return serviceJson?.zone
+}