From 2c591c8d076e24da31049ff6f562b875f5f669a3 Mon Sep 17 00:00:00 2001 From: Maggie Neterval Date: Thu, 19 Sep 2019 16:47:54 -0400 Subject: [PATCH] fix(google): do not pin source server group capacity in red/black (#3181) * fix(google): do not pin source server group capacity in red/black * fix(google): remove unnecessary method --- .../gce/GceDeployStagePreProcessor.java | 99 ------------------- .../gce/GceDeployStagePreProcessorTest.java | 29 +----- 2 files changed, 4 insertions(+), 124 deletions(-) diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/gce/GceDeployStagePreProcessor.java b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/gce/GceDeployStagePreProcessor.java index 1dfc7d587c..d2d1a67076 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/gce/GceDeployStagePreProcessor.java +++ b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/gce/GceDeployStagePreProcessor.java @@ -16,40 +16,27 @@ package com.netflix.spinnaker.orca.clouddriver.pipeline.providers.gce; -import static com.netflix.spinnaker.orca.kato.pipeline.support.ResizeStrategySupport.getSource; - import com.google.common.collect.ImmutableList; import com.netflix.spinnaker.orca.clouddriver.pipeline.providers.aws.ApplySourceServerGroupCapacityStage; import com.netflix.spinnaker.orca.clouddriver.pipeline.providers.aws.CaptureSourceServerGroupCapacityTask; -import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.ResizeServerGroupStage; -import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.strategies.AbstractDeployStrategyStage; import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.strategies.DeployStagePreProcessor; -import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support.TargetServerGroup; import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support.TargetServerGroupResolver; -import com.netflix.spinnaker.orca.kato.pipeline.strategy.Strategy; -import com.netflix.spinnaker.orca.kato.pipeline.support.ResizeStrategy; import com.netflix.spinnaker.orca.kato.pipeline.support.StageData; import com.netflix.spinnaker.orca.pipeline.model.Stage; import java.util.Collections; -import java.util.Map; -import java.util.Optional; -import javax.annotation.Nullable; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @Component public class GceDeployStagePreProcessor implements DeployStagePreProcessor { private final ApplySourceServerGroupCapacityStage applySourceServerGroupSnapshotStage; - private final ResizeServerGroupStage resizeServerGroupStage; private final TargetServerGroupResolver targetServerGroupResolver; @Autowired public GceDeployStagePreProcessor( ApplySourceServerGroupCapacityStage applySourceServerGroupCapacityStage, - ResizeServerGroupStage resizeServerGroupStage, TargetServerGroupResolver targetServerGroupResolver) { this.applySourceServerGroupSnapshotStage = applySourceServerGroupCapacityStage; - this.resizeServerGroupStage = resizeServerGroupStage; this.targetServerGroupResolver = targetServerGroupResolver; } @@ -60,29 +47,6 @@ public ImmutableList additionalSteps(Stage stage) { "snapshotSourceServerGroup", CaptureSourceServerGroupCapacityTask.class)); } - @Override - public ImmutableList beforeStageDefinitions(Stage stage) { - StageData stageData = stage.mapTo(StageData.class); - if (!shouldPinSourceServerGroup(stageData.getStrategy())) { - return ImmutableList.of(); - } - - Optional> optionalResizeContext = getResizeContext(stageData); - if (!optionalResizeContext.isPresent()) { - // no source server group, no need to resize - return ImmutableList.of(); - } - - Map resizeContext = optionalResizeContext.get(); - resizeContext.put("pinMinimumCapacity", true); - - return ImmutableList.of( - new StageDefinition( - String.format("Pin %s", resizeContext.get("serverGroupName")), - resizeServerGroupStage, - resizeContext)); - } - @Override public ImmutableList afterStageDefinitions(Stage stage) { return ImmutableList.of( @@ -92,72 +56,9 @@ public ImmutableList afterStageDefinitions(Stage stage) { Collections.emptyMap())); } - @Override - public ImmutableList onFailureStageDefinitions(Stage stage) { - StageData stageData = stage.mapTo(StageData.class); - - StageDefinition unpinServerGroupStage = buildUnpinServerGroupStage(stageData); - if (unpinServerGroupStage == null) { - return ImmutableList.of(); - } - - return ImmutableList.of(unpinServerGroupStage); - } - @Override public boolean supports(Stage stage) { StageData stageData = stage.mapTo(StageData.class); return stageData.getCloudProvider().equals("gce"); } - - private static boolean shouldPinSourceServerGroup(String strategy) { - return Strategy.fromStrategyKey(strategy) == Strategy.RED_BLACK; - } - - private Optional> getResizeContext(StageData stageData) { - Map resizeContext = - AbstractDeployStrategyStage.CleanupConfig.toContext(stageData); - - try { - ResizeStrategy.Source source = getSource(targetServerGroupResolver, stageData, resizeContext); - if (source == null) { - return Optional.empty(); - } - - resizeContext.put("serverGroupName", source.getServerGroupName()); - resizeContext.put("action", ResizeStrategy.ResizeAction.scale_to_server_group); - resizeContext.put("source", source); - resizeContext.put( - "useNameAsLabel", true); // hint to deck that it should _not_ override the name - return Optional.of(resizeContext); - } catch (TargetServerGroup.NotFoundException e) { - return Optional.empty(); - } - } - - @Nullable - private StageDefinition buildUnpinServerGroupStage(StageData stageData) { - if (!shouldPinSourceServerGroup(stageData.getStrategy())) { - return null; - } - - if (stageData.getScaleDown()) { - // source server group has been scaled down, no need to unpin if deploy was successful - return null; - } - - Optional> optionalResizeContext = getResizeContext(stageData); - if (!optionalResizeContext.isPresent()) { - // no source server group, no need to unpin - return null; - } - - Map resizeContext = optionalResizeContext.get(); - resizeContext.put("unpinMinimumCapacity", true); - - return new StageDefinition( - String.format("Unpin %s", resizeContext.get("serverGroupName")), - resizeServerGroupStage, - resizeContext); - } } diff --git a/orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/gce/GceDeployStagePreProcessorTest.java b/orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/gce/GceDeployStagePreProcessorTest.java index 4aabfa21ea..6e045c5ed4 100644 --- a/orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/gce/GceDeployStagePreProcessorTest.java +++ b/orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/pipeline/providers/gce/GceDeployStagePreProcessorTest.java @@ -18,17 +18,11 @@ package com.netflix.spinnaker.orca.clouddriver.pipeline.providers.gce; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.when; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.netflix.spinnaker.orca.clouddriver.pipeline.providers.aws.ApplySourceServerGroupCapacityStage; import com.netflix.spinnaker.orca.clouddriver.pipeline.providers.aws.CaptureSourceServerGroupCapacityTask; -import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.ResizeServerGroupStage; import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.strategies.DeployStagePreProcessor.StageDefinition; import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.strategies.DeployStagePreProcessor.StepDefinition; -import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support.TargetServerGroup; import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support.TargetServerGroupResolver; import com.netflix.spinnaker.orca.pipeline.model.Stage; import java.util.Collections; @@ -38,33 +32,24 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) class GceDeployStagePreProcessorTest { private GceDeployStagePreProcessor preProcessor; private ApplySourceServerGroupCapacityStage applySourceServerGroupCapacityStage; - private ResizeServerGroupStage resizeServerGroupStage; - - @Mock private TargetServerGroupResolver targetServerGroupResolver; @BeforeEach void setUp() { applySourceServerGroupCapacityStage = new ApplySourceServerGroupCapacityStage(); - resizeServerGroupStage = new ResizeServerGroupStage(); + TargetServerGroupResolver targetServerGroupResolver = new TargetServerGroupResolver(); preProcessor = new GceDeployStagePreProcessor( - applySourceServerGroupCapacityStage, resizeServerGroupStage, targetServerGroupResolver); + applySourceServerGroupCapacityStage, targetServerGroupResolver); } @Test void redBlackStrategyTest() { - when(targetServerGroupResolver.resolve(any())) - .thenReturn( - ImmutableList.of( - new TargetServerGroup( - ImmutableMap.of("name", "testapp-v000", "zone", "us-central1-f")))); Stage stage = new Stage(); Map context = createDefaultContext(); context.put("strategy", "redblack"); @@ -72,9 +57,7 @@ void redBlackStrategyTest() { // Before Stages List beforeStages = preProcessor.beforeStageDefinitions(stage); - assertThat(beforeStages) - .extracting(stageDefinition -> stageDefinition.stageDefinitionBuilder) - .containsExactly(resizeServerGroupStage); + assertThat(beforeStages).isEmpty(); // Additional Steps List additionalSteps = preProcessor.additionalSteps(stage); @@ -90,15 +73,11 @@ void redBlackStrategyTest() { // On Failure Stages List failureStages = preProcessor.onFailureStageDefinitions(stage); - assertThat(failureStages) - .extracting(stageDefinition -> stageDefinition.stageDefinitionBuilder) - .containsExactly(resizeServerGroupStage); + assertThat(failureStages).isEmpty(); } @Test void redBlackStrategyNoExistingServerGroupTest() { - when(targetServerGroupResolver.resolve(any())) - .thenThrow(TargetServerGroup.NotFoundException.class); Stage stage = new Stage(); Map context = createDefaultContext(); context.put("strategy", "redblack");