Skip to content

Commit

Permalink
feat(cfn): request to delete CFN changeset if empty (#3234)
Browse files Browse the repository at this point in the history
* feat(cfn): request to delete CFN changeset if empty

This patch requests the deletion of the CFN change set if it's empty.
The rationale behind it is that whenever a change set does not contain
changes to the CFN template, the AWS API marks the changeset as FAILED.
However, in a CD world, an empty change should not be considered a
failure, and instead continue the normal execution flow.

In this patch we detect that the change set is empty by the status
message and request it's deletion accordingly. This is implemented as
a separate task so it can be reused in different scenarios planned for
the future (e.g. configuration option to delete all changesets that
have been executed).

* Address comments
  • Loading branch information
xavileon authored and mergify[bot] committed Oct 28, 2019
1 parent 578c736 commit 871358f
Show file tree
Hide file tree
Showing 6 changed files with 210 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@

import com.netflix.spinnaker.orca.clouddriver.tasks.MonitorKatoTask;
import com.netflix.spinnaker.orca.clouddriver.tasks.providers.aws.cloudformation.CloudFormationForceCacheRefreshTask;
import com.netflix.spinnaker.orca.clouddriver.tasks.providers.aws.cloudformation.DeleteCloudFormationChangeSetTask;
import com.netflix.spinnaker.orca.clouddriver.tasks.providers.aws.cloudformation.DeployCloudFormationTask;
import com.netflix.spinnaker.orca.clouddriver.tasks.providers.aws.cloudformation.WaitForCloudFormationCompletionTask;
import com.netflix.spinnaker.orca.pipeline.StageDefinitionBuilder;
import com.netflix.spinnaker.orca.pipeline.TaskNode;
import com.netflix.spinnaker.orca.pipeline.model.Stage;
import java.util.Optional;
import javax.annotation.Nonnull;
import org.springframework.stereotype.Component;

Expand All @@ -36,5 +38,9 @@ public void taskGraph(@Nonnull Stage stage, @Nonnull TaskNode.Builder builder) {
.withTask("monitorCloudFormation", MonitorKatoTask.class)
.withTask("forceRefreshCache", CloudFormationForceCacheRefreshTask.class)
.withTask("waitForCloudFormationCompletion", WaitForCloudFormationCompletionTask.class);
if ((boolean) Optional.ofNullable(stage.getContext().get("isChangeSet")).orElse(false)) {
builder.withTask("deleteCloudFormationChangeSet", DeleteCloudFormationChangeSetTask.class);
builder.withTask("monitorDeleteCloudFormationChangeSet", MonitorKatoTask.class);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* Copyright (c) 2019 Adevinta
*
* Licensed under the Apache License, Version 2.0 (the "License")
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.netflix.spinnaker.orca.clouddriver.tasks.providers.aws.cloudformation;

import com.google.common.collect.ImmutableMap;
import com.netflix.spinnaker.orca.ExecutionStatus;
import com.netflix.spinnaker.orca.Task;
import com.netflix.spinnaker.orca.TaskResult;
import com.netflix.spinnaker.orca.clouddriver.KatoService;
import com.netflix.spinnaker.orca.clouddriver.model.TaskId;
import com.netflix.spinnaker.orca.clouddriver.tasks.AbstractCloudProviderAwareTask;
import com.netflix.spinnaker.orca.pipeline.model.Stage;
import java.util.*;
import javax.annotation.Nonnull;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

@Slf4j
@Component
public class DeleteCloudFormationChangeSetTask extends AbstractCloudProviderAwareTask
implements Task {

@Autowired KatoService katoService;

public static final String TASK_NAME = "deleteCloudFormationChangeSet";

@Nonnull
@Override
public TaskResult execute(@Nonnull Stage stage) {
String cloudProvider = getCloudProvider(stage);

Map<String, Object> stageContext = stage.getContext();

String stackName = (String) stageContext.get("stackName");
String changeSetName = (String) stageContext.get("changeSetName");

if ((boolean) Optional.ofNullable(stageContext.get("deleteChangeSet")).orElse(false)) {
log.debug(
"Deleting CloudFormation changeset {} from stack {} as requested.",
changeSetName,
stackName);
List<String> regions = (List<String>) stageContext.get("regions");
String region =
regions.stream()
.findFirst()
.orElseThrow(
() ->
new IllegalArgumentException(
"No regions selected. At least one region must be chosen."));

Map<String, Object> task = new HashMap<>();
task.put("stackName", stackName);
task.put("changeSetName", changeSetName);
task.put("region", region);

Map<String, Map> operation =
new ImmutableMap.Builder<String, Map>().put(TASK_NAME, task).build();

TaskId taskId =
katoService
.requestOperations(cloudProvider, Collections.singletonList(operation))
.toBlocking()
.first();

Map<String, Object> context =
new ImmutableMap.Builder<String, Object>()
.put("kato.result.expected", false)
.put("kato.last.task.id", taskId)
.build();

return TaskResult.builder(ExecutionStatus.SUCCEEDED).context(context).build();
} else {
log.debug(
"Not deleting CloudFormation ChangeSet {} from stack {}.", changeSetName, stackName);
return TaskResult.builder(ExecutionStatus.SUCCEEDED).build();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,15 @@ public TaskResult execute(@Nonnull Stage stage) {
isChangeSet
? getChangeSetInfo(stack, stage.getContext(), "status")
: getStackInfo(stack, "stackStatus");
if (isComplete(status) || isEmptyChangeSet(stage, stack)) {
if (isComplete(status)) {
return TaskResult.builder(ExecutionStatus.SUCCEEDED).outputs(stack).build();
} else if (isEmptyChangeSet(stage, stack)) {
String changeSetName = (String) result.get("changeSetName");
log.info("CloudFormation ChangeSet {} empty. Requesting to be deleted.", changeSetName);
return TaskResult.builder(ExecutionStatus.SUCCEEDED)
.context("deleteChangeSet", true)
.outputs(stack)
.build();
} else if (isInProgress(status)) {
return TaskResult.RUNNING;
} else if (isFailed(status)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,18 @@ class DeployCloudFormationStageTest extends Specification {
def stage = new Stage(new Execution(Execution.ExecutionType.PIPELINE, "testApp"), "cf", [:])

when:
if (isChangeSet) {
stage.context.put("isChangeSet", true)
}
cloudFormationStage.taskGraph(stage, builder)

then:
builder.graph.size == 4
builder.graph.size == graphSize

where:
isChangeSet || graphSize
false || 4
true || 6
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copyright (c) 2019 Schibsted Media Group.
*
* Licensed under the Apache License, Version 2.0 (the "License")
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.netflix.spinnaker.orca.clouddriver.tasks.providers.aws.cloudformation

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.kork.artifacts.model.Artifact
import com.netflix.spinnaker.orca.ExecutionStatus
import com.netflix.spinnaker.orca.clouddriver.KatoService
import com.netflix.spinnaker.orca.clouddriver.OortService
import com.netflix.spinnaker.orca.clouddriver.model.TaskId
import com.netflix.spinnaker.orca.pipeline.model.Execution
import com.netflix.spinnaker.orca.pipeline.model.Stage
import retrofit.client.Response
import retrofit.mime.TypedString
import rx.Observable
import spock.lang.Specification
import spock.lang.Subject
import spock.lang.Unroll

class DeleteCloudFormationChangeSetTaskSpec extends Specification {

def katoService = Mock(KatoService)
def oortService = Mock(OortService)
def objectMapper = new ObjectMapper()

@Subject
def deleteCloudFormationChangeSetTask = new DeleteCloudFormationChangeSetTask(katoService: katoService)

def "should delete change set if requested by the context"() {
given:
def taskId = new TaskId(id: 'id')
def pipeline = Execution.newPipeline('orca')
def context = [
credentials: 'creds',
cloudProvider: 'aws',
stackName: 'stackName',
changeSetName: 'changeSetName',
deleteChangeSet: true,
regions: ['eu-west-1']
]
def stage = new Stage(pipeline, 'test', 'test', context)

when:
def result = deleteCloudFormationChangeSetTask.execute(stage)

then:
1 * katoService.requestOperations("aws", {
it.get(0).get("deleteCloudFormationChangeSet") != null
}) >> Observable.just(taskId)
result.getStatus() == ExecutionStatus.SUCCEEDED

}

def "should succeed deleting change set if not requested by the context"() {
given:
def pipeline = Execution.newPipeline('orca')
def context = [
credentials: 'creds',
cloudProvider: 'aws',
stackName: 'stackName',
changeSetName: 'changeSetName',
deleteChangeSet: false,
regions: ['eu-west-1']
]
def stage = new Stage(pipeline, 'test', 'test', context)

when:
def result = deleteCloudFormationChangeSetTask.execute(stage)

then:
result.getStatus() == ExecutionStatus.SUCCEEDED

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,15 @@ class WaitForCloudFormationCompletionTaskSpec extends Specification {
1 * oortService.getCloudFormationStack('stackId') >> stack
result.status == expectedResult
result.outputs == stack
def deleteChangeSet = result.context.get('deleteChangeSet')
(deleteChangeSet != null && deleteChangeSet) == shouldDeleteChangeset

where:
isChangeSet | status | statusReason || expectedResult
false | 'CREATE_COMPLETE' | 'ignored' || ExecutionStatus.SUCCEEDED
false | 'UPDATE_COMPLETE' | 'ignored' || ExecutionStatus.SUCCEEDED
true | 'FAILED' | 'The submitted information didn\'t contain changes' || ExecutionStatus.SUCCEEDED
isChangeSet | status | statusReason | shouldDeleteChangeset || expectedResult
false | 'CREATE_COMPLETE' | 'ignored' | false || ExecutionStatus.SUCCEEDED
false | 'UPDATE_COMPLETE' | 'ignored' | false || ExecutionStatus.SUCCEEDED
true | 'FAILED' | 'The submitted information didn\'t contain changes' | true || ExecutionStatus.SUCCEEDED
true | 'CREATE_COMPLETE' | 'ignored' | false || ExecutionStatus.SUCCEEDED
}

@Unroll
Expand Down

0 comments on commit 871358f

Please sign in to comment.