Skip to content

Commit

Permalink
fix: Don't skip deploy if previous equivalent deployment failed (#247)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tbarlow12 authored Aug 20, 2019
1 parent 44185c5 commit b916e25
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 6 deletions.
5 changes: 5 additions & 0 deletions src/models/armTemplates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
26 changes: 23 additions & 3 deletions src/services/armService.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand All @@ -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()
Expand Down Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions src/services/resourceService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/test/mockFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -193,6 +193,7 @@ export class MockFactory {
properties: {
timestamp: new Date(2019, 1, 1, 0, 0, second),
parameters: MockFactory.createTestParameters(),
provisioningState: ArmTemplateProvisioningState.SUCCEEDED
}
}
}
Expand Down

0 comments on commit b916e25

Please sign in to comment.