From 74e65d98cc1cae652b32e72508caa70b8dc476ae Mon Sep 17 00:00:00 2001 From: Mark Vulfson Date: Tue, 24 Sep 2019 21:24:03 -0700 Subject: [PATCH] fix(monitored deploy): add monitored deploy to same checks as RRB (#3190) * add monitored deploy to checks where we check for RRB (since, semantically, they are same) * make some better summary messages --- .../aws/AwsDeployStagePreProcessor.groovy | 20 +++++++++---- .../titus/TitusDeployStagePreProcessor.groovy | 12 ++++++-- .../servergroup/CreateServerGroupStage.groovy | 2 +- .../servergroup/PinServerGroupStage.groovy | 1 - .../MonitoredDeployBaseTask.java | 28 +++++++++++++++---- 5 files changed, 47 insertions(+), 16 deletions(-) diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/aws/AwsDeployStagePreProcessor.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/aws/AwsDeployStagePreProcessor.groovy index 2be5a9df15..e06d1fb4b6 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/aws/AwsDeployStagePreProcessor.groovy +++ b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/aws/AwsDeployStagePreProcessor.groovy @@ -36,6 +36,8 @@ import static com.netflix.spinnaker.orca.kato.pipeline.support.ResizeStrategySup @Component class AwsDeployStagePreProcessor implements DeployStagePreProcessor { + private static final List rollingStrategies = [Strategy.ROLLING_RED_BLACK, Strategy.MONITORED] + @Autowired ApplySourceServerGroupCapacityStage applySourceServerGroupSnapshotStage @@ -51,7 +53,9 @@ class AwsDeployStagePreProcessor implements DeployStagePreProcessor { @Override List additionalSteps(Stage stage) { def stageData = stage.mapTo(StageData) - if (Strategy.fromStrategyKey(stageData.strategy) == Strategy.ROLLING_RED_BLACK) { + Strategy strategy = Strategy.fromStrategyKey(stageData.strategy) + + if (rollingStrategies.contains(strategy)) { // rolling red/black has no need to snapshot capacities return [] } @@ -114,8 +118,10 @@ class AwsDeployStagePreProcessor implements DeployStagePreProcessor { List afterStageDefinitions(Stage stage) { def stageData = stage.mapTo(StageData) def stageDefinitions = [] - if (Strategy.fromStrategyKey(stageData.strategy) != Strategy.ROLLING_RED_BLACK) { - // rolling red/black has no need to apply a snapshotted capacity (on the newly created server group) + Strategy strategy = Strategy.fromStrategyKey(stageData.strategy) + + if (!rollingStrategies.contains(strategy)) { + // rolling strategies have no need to apply a snapshotted capacity (on the newly created server group) stageDefinitions << new StageDefinition( name: "restoreMinCapacityFromSnapshot", stageDefinitionBuilder: applySourceServerGroupSnapshotStage, @@ -152,12 +158,14 @@ class AwsDeployStagePreProcessor implements DeployStagePreProcessor { private static boolean shouldPinSourceServerGroup(String strategy) { // TODO-AJ consciously only enabling for rolling red/black -- will add support for other strategies after it's working - return (Strategy.fromStrategyKey(strategy) == Strategy.ROLLING_RED_BLACK) + return (rollingStrategies.contains(Strategy.fromStrategyKey(strategy))) } private static boolean shouldCheckServerGroupsPreconditions(StageData stageData) { - // TODO(dreynaud): enabling cautiously for RRB only for testing, but we would ideally roll this out to other strategies - return (Strategy.fromStrategyKey(stageData.strategy) == Strategy.ROLLING_RED_BLACK) && (stageData.maxInitialAsgs != -1) + // TODO(dreynaud): enabling cautiously for RRB/MD only for testing, but we would ideally roll this out to other strategies + Strategy strategy = Strategy.fromStrategyKey(stageData.strategy) + + return (rollingStrategies.contains(strategy) && (stageData.maxInitialAsgs != -1)) } private Optional> getResizeContext(StageData stageData) { diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/titus/TitusDeployStagePreProcessor.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/titus/TitusDeployStagePreProcessor.groovy index 8155cd63a2..9fa2663d5f 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/titus/TitusDeployStagePreProcessor.groovy +++ b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/titus/TitusDeployStagePreProcessor.groovy @@ -28,13 +28,17 @@ import org.springframework.stereotype.Component @Component class TitusDeployStagePreProcessor implements DeployStagePreProcessor { + private static final List rollingStrategies = [Strategy.ROLLING_RED_BLACK, Strategy.MONITORED] + @Autowired ApplySourceServerGroupCapacityStage applySourceServerGroupSnapshotStage @Override List additionalSteps(Stage stage) { def stageData = stage.mapTo(StageData) - if (Strategy.fromStrategyKey(stageData.strategy) == Strategy.ROLLING_RED_BLACK) { + Strategy strategy = Strategy.fromStrategyKey(stageData.strategy) + + if (rollingStrategies.contains(strategy)) { // rolling red/black has no need to snapshot capacities return [] } @@ -57,8 +61,10 @@ class TitusDeployStagePreProcessor implements DeployStagePreProcessor { List afterStageDefinitions(Stage stage) { def stageData = stage.mapTo(StageData) def stageDefinitions = [] - if (Strategy.fromStrategyKey(stageData.strategy) != Strategy.ROLLING_RED_BLACK) { - // rolling red/black has no need to apply a snapshotted capacity (on the newly created server group) + Strategy strategy = Strategy.fromStrategyKey(stageData.strategy) + + if (!rollingStrategies.contains(strategy)) { + // rolling strategies have no need to apply a snapshotted capacity (on the newly created server group) stageDefinitions << new StageDefinition( name: "restoreMinCapacityFromSnapshot", stageDefinitionBuilder: applySourceServerGroupSnapshotStage, diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/CreateServerGroupStage.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/CreateServerGroupStage.groovy index 38f653e346..9029aa91c3 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/CreateServerGroupStage.groovy +++ b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/CreateServerGroupStage.groovy @@ -110,7 +110,7 @@ class CreateServerGroupStage extends AbstractDeployStrategyStage { def additionalRollbackContext = [:] def strategy = Strategy.fromStrategyKey(stageData.strategy) - if (strategy == Strategy.ROLLING_RED_BLACK) { + if ((strategy == Strategy.ROLLING_RED_BLACK) || (strategy == Strategy.MONITORED)) { // rollback is always supported regardless of where the failure occurred strategySupportsRollback = true additionalRollbackContext.enableAndDisableOnly = true diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/PinServerGroupStage.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/PinServerGroupStage.groovy index 3755304876..e33b3c3da7 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/PinServerGroupStage.groovy +++ b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/PinServerGroupStage.groovy @@ -17,7 +17,6 @@ package com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support.TargetServerGroupLinearStageSupport -import com.netflix.spinnaker.orca.clouddriver.tasks.DetermineHealthProvidersTask import com.netflix.spinnaker.orca.clouddriver.tasks.MonitorKatoTask import com.netflix.spinnaker.orca.clouddriver.tasks.servergroup.ResizeServerGroupTask import com.netflix.spinnaker.orca.clouddriver.tasks.servergroup.ServerGroupCacheForceRefreshTask diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/MonitoredDeployBaseTask.java b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/MonitoredDeployBaseTask.java index 0ca167f842..7357c64a22 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/MonitoredDeployBaseTask.java +++ b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/MonitoredDeployBaseTask.java @@ -32,9 +32,7 @@ import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; import java.time.Duration; -import java.util.Collections; -import java.util.List; -import java.util.Optional; +import java.util.*; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import javax.annotation.Nonnull; @@ -50,11 +48,28 @@ public class MonitoredDeployBaseTask implements RetryableTask { protected final Logger log = LoggerFactory.getLogger(getClass()); protected Registry registry; private DeploymentMonitorServiceProvider deploymentMonitorServiceProvider; + private final Map summaryMapping = + new HashMap<>(); MonitoredDeployBaseTask( DeploymentMonitorServiceProvider deploymentMonitorServiceProvider, Registry registry) { this.deploymentMonitorServiceProvider = deploymentMonitorServiceProvider; this.registry = registry; + + // This could be in deck, but for now, I think it's valuable to have this show up in the JSON + // for easier debugging + summaryMapping.put( + EvaluateHealthResponse.NextStepDirective.WAIT, + "Waiting for deployment monitor to evaluate health of deployed instances"); + summaryMapping.put( + EvaluateHealthResponse.NextStepDirective.ABORT, + "Deployment monitor deemed the deployed instances unhealthy, aborting deployment"); + summaryMapping.put( + EvaluateHealthResponse.NextStepDirective.CONTINUE, + "Deployment monitor deemed the instances healthy, proceeding with deploy"); + summaryMapping.put( + EvaluateHealthResponse.NextStepDirective.COMPLETE, + "Deployment monitor deemed the instances healthy and requested to complete the deployment early"); } @Override @@ -206,13 +221,16 @@ TaskResult buildTaskResult( List statusReasons = Optional.ofNullable(response.getStatusReasons()).orElse(Collections.emptyList()); - String summary = "Deployment monitor requested to: " + response.getNextStep().getDirective(); + String summary = + summaryMapping.getOrDefault( + response.getNextStep().getDirective(), "Health evaluation results are unknown"); StatusExplanation explanation = new StatusExplanation(summary, statusReasons); return taskResultBuilder.context("deploymentMonitorReasons", explanation).build(); } - TaskResult buildTaskResult(TaskResult.TaskResultBuilder taskResultBuilder, String summary) { + private TaskResult buildTaskResult( + TaskResult.TaskResultBuilder taskResultBuilder, String summary) { StatusExplanation explanation = new StatusExplanation(summary); return taskResultBuilder.context("deploymentMonitorReasons", explanation).build();