From b916e25a31a89a6e14701328411a797d016961f4 Mon Sep 17 00:00:00 2001
From: Tanner Barlow <tanner.barlow.dev@gmail.com>
Date: Mon, 19 Aug 2019 17:05:21 -0700
Subject: [PATCH] fix: Don't skip deploy if previous equivalent deployment
 failed (#247)

Currently, if a deployment fails and the next deployment's template & parameters are identical, the deployment will be skipped. This adds the check to see if the deployment was successful.

Resolves AB#852
---
 src/models/armTemplates.ts      |  5 +++++
 src/services/armService.test.ts | 26 +++++++++++++++++++++++---
 src/services/resourceService.ts |  4 ++--
 src/test/mockFactory.ts         |  3 ++-
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/models/armTemplates.ts b/src/models/armTemplates.ts
index a35d050a..97ef9141 100644
--- a/src/models/armTemplates.ts
+++ b/src/models/armTemplates.ts
@@ -8,6 +8,11 @@ export interface ArmResourceTemplateGenerator {
   getParameters(config: ServerlessAzureConfig): any;
 }
 
+export enum ArmTemplateProvisioningState {
+  FAILED = "Failed",
+  SUCCEEDED = "Succeeded",
+}
+
 /**
  * The well-known serverless Azure template types
  */
diff --git a/src/services/armService.test.ts b/src/services/armService.test.ts
index 71dedfe0..1dc2c374 100644
--- a/src/services/armService.test.ts
+++ b/src/services/armService.test.ts
@@ -1,12 +1,12 @@
 import Serverless from "serverless";
 import { MockFactory } from "../test/mockFactory";
 import { ArmService } from "./armService";
-import { ArmResourceTemplate, ArmTemplateType, ArmDeployment } from "../models/armTemplates";
+import { ArmResourceTemplate, ArmTemplateType, ArmDeployment, ArmTemplateProvisioningState } from "../models/armTemplates";
 import { ArmTemplateConfig, ServerlessAzureOptions } from "../models/serverless";
 import mockFs from "mock-fs";
 import jsonpath from "jsonpath";
 import { Deployments } from "@azure/arm-resources";
-import { Deployment } from "@azure/arm-resources/esm/models";
+import { Deployment, DeploymentExtended } from "@azure/arm-resources/esm/models";
 import { ResourceService } from "./resourceService";
 
 describe("Arm Service", () => {
@@ -30,7 +30,7 @@ describe("Arm Service", () => {
     };
 
     service = createService();
-    ResourceService.prototype.getDeployments = jest.fn(() => MockFactory.createTestDeployments()) as any;
+    ResourceService.prototype.getDeployments = jest.fn(() => Promise.resolve(MockFactory.createTestDeployments())) as any;
     ResourceService.prototype.getDeploymentTemplate = jest.fn(() => {
       return {
         template: MockFactory.createTestArmTemplate()
@@ -168,6 +168,26 @@ describe("Arm Service", () => {
       expect(Deployments.prototype.createOrUpdate).not.toBeCalled()
     });
 
+    it("Calls deploy if previous template is the same but failed", async () => {
+      const deployments = MockFactory.createTestDeployments();
+      const failedDeployment: DeploymentExtended = {
+        ...deployments[0],
+        properties: {
+          ...deployments[0].properties,
+          provisioningState: ArmTemplateProvisioningState.FAILED
+        }
+      }
+      deployments[0] = failedDeployment;
+      ResourceService.prototype.getDeployments = jest.fn(() => Promise.resolve(deployments))
+
+      const deployment: ArmDeployment = {
+        parameters: MockFactory.createTestParameters(false),
+        template: MockFactory.createTestArmTemplate()
+      };
+      await service.deployTemplate(deployment);
+      expect(Deployments.prototype.createOrUpdate).toBeCalled();
+    });
+
     it("Calls deploy if parameters have changed from deployed template", async () => {
       const deployment: ArmDeployment = {
         parameters: MockFactory.createTestParameters(false),
diff --git a/src/services/resourceService.ts b/src/services/resourceService.ts
index 51885e63..dac7acf7 100644
--- a/src/services/resourceService.ts
+++ b/src/services/resourceService.ts
@@ -3,7 +3,7 @@ import { ResourceManagementClient } from "@azure/arm-resources";
 import { BaseService } from "./baseService";
 import { Utils } from "../shared/utils";
 import { AzureNamingService } from "./namingService";
-import { ArmDeployment } from "../models/armTemplates";
+import { ArmDeployment, ArmTemplateProvisioningState } from "../models/armTemplates";
 import { DeploymentExtended } from "@azure/arm-resources/esm/models";
 
 export class ResourceService extends BaseService {
@@ -41,7 +41,7 @@ export class ResourceService extends BaseService {
    */
   public async getLastDeploymentTemplate(): Promise<ArmDeployment> {
     const deployment = await this.getLastDeployment();
-    if (!deployment) {
+    if (!deployment || deployment.properties.provisioningState !== ArmTemplateProvisioningState.SUCCEEDED) {
       return;
     }
     const { parameters } = deployment.properties;
diff --git a/src/test/mockFactory.ts b/src/test/mockFactory.ts
index 97899407..64242c9e 100644
--- a/src/test/mockFactory.ts
+++ b/src/test/mockFactory.ts
@@ -12,7 +12,7 @@ import Service from "serverless/classes/Service";
 import Utils from "serverless/classes/Utils";
 import PluginManager from "serverless/lib/classes/PluginManager";
 import { ApiCorsPolicy, ApiManagementConfig } from "../models/apiManagement";
-import { ArmDeployment, ArmResourceTemplate } from "../models/armTemplates";
+import { ArmDeployment, ArmResourceTemplate, ArmTemplateProvisioningState } from "../models/armTemplates";
 import { ServicePrincipalEnvVariables } from "../models/azureProvider";
 import { Logger } from "../models/generic";
 import { ServerlessAzureConfig, ServerlessAzureProvider, ServerlessAzureFunctionConfig } from "../models/serverless";
@@ -193,6 +193,7 @@ export class MockFactory {
       properties: {
         timestamp: new Date(2019, 1, 1, 0, 0, second),
         parameters: MockFactory.createTestParameters(),
+        provisioningState: ArmTemplateProvisioningState.SUCCEEDED
       }
     }
   }