Skip to content

Commit

Permalink
fix(google): fix deploying with red/black into an empty cluster (#3173)
Browse files Browse the repository at this point in the history 
* fix(google): fix deploying with red/black into an empty cluster

* fix(google): replace catch with null check, handle clone case
  • Loading branch information
@maggieneterval
maggieneterval committed Sep 18, 2019
1 parent f2a239c commit 9817373
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 63 deletions.
58 changes: 35 additions & 23 deletions ...netflix/spinnaker/orca/clouddriver/pipeline/providers/gce/GceDeployStagePreProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@
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.HashMap;
import java.util.Map;
import java.util.Optional;
import javax.annotation.Nullable;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
Expand Down Expand Up @@ -66,7 +67,13 @@ public ImmutableList<StageDefinition> beforeStageDefinitions(Stage stage) {
return ImmutableList.of();
}

Map<String, Object> resizeContext = getResizeContext(stageData);
Optional<Map<String, Object>> optionalResizeContext = getResizeContext(stageData);
if (!optionalResizeContext.isPresent()) {
// no source server group, no need to resize
return ImmutableList.of();
}

Map<String, Object> resizeContext = optionalResizeContext.get();
resizeContext.put("pinMinimumCapacity", true);

return ImmutableList.of(
Expand Down Expand Up @@ -107,26 +114,25 @@ private static boolean shouldPinSourceServerGroup(String strategy) {
return Strategy.fromStrategyKey(strategy) == Strategy.RED_BLACK;
}

private Map<String, Object> getResizeContext(StageData stageData) {
AbstractDeployStrategyStage.CleanupConfig cleanupConfig =
AbstractDeployStrategyStage.CleanupConfig.fromStage(stageData);
Map<String, Object> resizeContext = new HashMap<>();

resizeContext.put(
cleanupConfig.getLocation().singularType(), cleanupConfig.getLocation().getValue());
resizeContext.put("cluster", cleanupConfig.getCluster());
resizeContext.put("moniker", cleanupConfig.getMoniker());
resizeContext.put("credentials", cleanupConfig.getAccount());
resizeContext.put("cloudProvider", cleanupConfig.getCloudProvider());

ResizeStrategy.Source source = getSource(targetServerGroupResolver, stageData, resizeContext);
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 resizeContext;
private Optional<Map<String, Object>> getResizeContext(StageData stageData) {
Map<String, Object> 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
Expand All @@ -140,7 +146,13 @@ private StageDefinition buildUnpinServerGroupStage(StageData stageData) {
return null;
}

Map<String, Object> resizeContext = getResizeContext(stageData);
Optional<Map<String, Object>> optionalResizeContext = getResizeContext(stageData);
if (!optionalResizeContext.isPresent()) {
// no source server group, no need to unpin
return null;
}

Map<String, Object> resizeContext = optionalResizeContext.get();
resizeContext.put("unpinMinimumCapacity", true);

return new StageDefinition(
Expand Down
11 changes: 11 additions & 0 deletions ...naker/orca/clouddriver/pipeline/servergroup/strategies/AbstractDeployStrategyStage.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,5 +200,16 @@ abstract class AbstractDeployStrategyStage extends AbstractCloudProviderAwareSta
location: loc
)
}

static Map<String, Object> toContext(StageData stageData) {
CleanupConfig cleanupConfig = fromStage(stageData)
Map<String, Object> context = new HashMap<>()
context.put(cleanupConfig.getLocation().singularType(), cleanupConfig.getLocation().getValue())
context.put("cloudProvider", cleanupConfig.getCloudProvider())
context.put("cluster", cleanupConfig.getCluster())
context.put("credentials", cleanupConfig.getAccount())
context.put("moniker", cleanupConfig.getMoniker())
return context
}
}
}
37 changes: 26 additions & 11 deletions ...etflix/spinnaker/orca/clouddriver/pipeline/servergroup/strategies/RedBlackStrategy.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@

package com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.strategies

import com.netflix.spinnaker.orca.ExecutionStatus
import com.netflix.spinnaker.orca.clouddriver.pipeline.cluster.DisableClusterStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.cluster.ScaleDownClusterStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.cluster.ShrinkClusterStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.CloneServerGroupStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.CreateServerGroupStage
import com.netflix.spinnaker.orca.kato.pipeline.support.StageData
import com.netflix.spinnaker.orca.pipeline.WaitStage
import com.netflix.spinnaker.orca.pipeline.model.Stage
Expand Down Expand Up @@ -50,17 +53,29 @@ class RedBlackStrategy implements Strategy, ApplicationContextAware {

@Override
List<Stage> composeFlow(Stage stage) {
def stages = []
def stageData = stage.mapTo(StageData)
def cleanupConfig = AbstractDeployStrategyStage.CleanupConfig.fromStage(stage)

Map baseContext = [
(cleanupConfig.location.singularType()): cleanupConfig.location.value,
cluster : cleanupConfig.cluster,
credentials : cleanupConfig.account,
moniker : cleanupConfig.moniker,
cloudProvider : cleanupConfig.cloudProvider,
]
List<Stage> stages = []
StageData stageData = stage.mapTo(StageData)
Map<String, Object> baseContext = AbstractDeployStrategyStage.CleanupConfig.toContext(stageData)

Stage parentCreateServerGroupStage = stage.directAncestors().find() { it.type == CreateServerGroupStage.PIPELINE_CONFIG_TYPE || it.type == CloneServerGroupStage.PIPELINE_CONFIG_TYPE }
if (parentCreateServerGroupStage == null) {
throw new IllegalStateException("Failed to determine source server group from parent stage while planning red/black flow")
}
if (parentCreateServerGroupStage.status == ExecutionStatus.NOT_STARTED) {
// No point in composing the flow if we are called to plan "beforeStages" since we don't have any STAGE_BEFOREs.
// Also, we rely on the the source server group task to have run already.
// In the near future we will move composeFlow into beforeStages and afterStages instead of the
// deprecated aroundStages
return []
}

StageData parentStageData = parentCreateServerGroupStage.mapTo(StageData)
StageData.Source sourceServerGroup = parentStageData.source

// Short-circuit if there is no source server group
if (sourceServerGroup == null || sourceServerGroup.serverGroupName == null) {
return []
}

// We don't want the key propagated if interestingHealthProviderNames isn't defined, since this prevents
// health providers from the stage's 'determineHealthProviders' task to be added to the context.
Expand Down
92 changes: 63 additions & 29 deletions ...ix/spinnaker/orca/clouddriver/pipeline/servergroup/strategies/RedBlackStrategySpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,19 @@ package com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.strategies
import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
import com.netflix.spinnaker.kork.dynamicconfig.SpringDynamicConfigService
import com.netflix.spinnaker.moniker.Moniker
import com.netflix.spinnaker.orca.ExecutionStatus
import com.netflix.spinnaker.orca.clouddriver.pipeline.cluster.DisableClusterStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.cluster.ScaleDownClusterStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.cluster.ShrinkClusterStage
import com.netflix.spinnaker.orca.clouddriver.utils.TrafficGuard
import com.netflix.spinnaker.orca.pipeline.WaitStage
import com.netflix.spinnaker.orca.pipeline.model.Execution
import com.netflix.spinnaker.orca.pipeline.model.Stage
import com.netflix.spinnaker.orca.pipeline.model.SyntheticStageOwner
import org.springframework.mock.env.MockEnvironment
import spock.lang.Specification

import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.pipeline
import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.stage

class RedBlackStrategySpec extends Specification {

def trafficGuard = Stub(TrafficGuard)
Expand All @@ -44,28 +46,64 @@ class RedBlackStrategySpec extends Specification {

def "should compose flow"() {
given:
Moniker moniker = new Moniker(app: "unit", stack: "tests");
def ctx = [
account : "testAccount",
application : "unit",
stack : "tests",
moniker : moniker,
cloudProvider : "aws",
region : "north",
availabilityZones : [
north: ["pole-1a"]
]
]
def stage = new Stage(Execution.newPipeline("orca"), "whatever", ctx)
Moniker moniker = new Moniker(app: "unit", stack: "tests")

def pipeline = pipeline {
application = "orca"
stage {
refId = "1-create"
type = "createServerGroup"
context = [
refId: "stage_createASG"
]

stage {
refId = "2-deploy1"
parent
context = [
refId : "stage",
account : "testAccount",
application : "unit",
stack : "tests",
moniker : moniker,
cloudProvider : "aws",
region : "north",
availabilityZones : [
north: ["pole-1a"]
]
]
}
}
}

def strat = new RedBlackStrategy(
shrinkClusterStage: shrinkClusterStage,
scaleDownClusterStage: scaleDownClusterStage,
disableClusterStage: disableClusterStage,
waitStage: waitStage
)
def stage = pipeline.stageByRef("2-deploy1")

when:
when: 'planning without having run createServerGroup'
def syntheticStages = strat.composeFlow(stage)

then:
syntheticStages.size() == 0

when: 'deploying into a new cluster (no source server group)'
pipeline.stageByRef("1-create").status = ExecutionStatus.RUNNING

then:
syntheticStages.size() == 0

when: 'deploying into an existing cluster'
pipeline.stageByRef("1-create").context.source = [
account: "testAccount",
region: "north",
serverGroupName: "unit-tests-v000",
asgName: "unit-tests-v000"
]
syntheticStages = strat.composeFlow(stage)
def beforeStages = syntheticStages.findAll { it.syntheticStageOwner == SyntheticStageOwner.STAGE_BEFORE }
def afterStages = syntheticStages.findAll { it.syntheticStageOwner == SyntheticStageOwner.STAGE_AFTER }

Expand All @@ -83,9 +121,8 @@ class RedBlackStrategySpec extends Specification {
preferLargerOverNewer : false,
]

when:
ctx.maxRemainingAsgs = 10
stage = new Stage(Execution.newPipeline("orca"), "whatever", ctx)
when: 'maxRemainingAsgs is specified'
pipeline.stageByRef("2-deploy1").context.maxRemainingAsgs = 10
syntheticStages = strat.composeFlow(stage)
beforeStages = syntheticStages.findAll { it.syntheticStageOwner == SyntheticStageOwner.STAGE_BEFORE }
afterStages = syntheticStages.findAll { it.syntheticStageOwner == SyntheticStageOwner.STAGE_AFTER }
Expand All @@ -97,9 +134,8 @@ class RedBlackStrategySpec extends Specification {
afterStages.first().context.shrinkToSize == 10
afterStages.last().type == disableClusterStage.type

when:
ctx.scaleDown = true
stage = new Stage(Execution.newPipeline("orca"), "whatever", ctx)
when: 'scaleDown is true'
pipeline.stageByRef("2-deploy1").context.scaleDown = true
syntheticStages = strat.composeFlow(stage)
beforeStages = syntheticStages.findAll { it.syntheticStageOwner == SyntheticStageOwner.STAGE_BEFORE }
afterStages = syntheticStages.findAll { it.syntheticStageOwner == SyntheticStageOwner.STAGE_AFTER }
Expand All @@ -112,9 +148,8 @@ class RedBlackStrategySpec extends Specification {
afterStages[2].type == scaleDownClusterStage.type
afterStages[2].context.allowScaleDownActive == false

when:
ctx.interestingHealthProviderNames = ["Google"]
stage = new Stage(Execution.newPipeline("orca"), "whatever", ctx)
when: 'interestingHealthProviderNames is specified'
pipeline.stageByRef("2-deploy1").context.interestingHealthProviderNames = ["Google"]
syntheticStages = strat.composeFlow(stage)
beforeStages = syntheticStages.findAll { it.syntheticStageOwner == SyntheticStageOwner.STAGE_BEFORE }
afterStages = syntheticStages.findAll { it.syntheticStageOwner == SyntheticStageOwner.STAGE_AFTER }
Expand All @@ -125,10 +160,9 @@ class RedBlackStrategySpec extends Specification {
afterStages.first().type == shrinkClusterStage.type
afterStages.first().context.interestingHealthProviderNames == ["Google"]

when:
ctx.delayBeforeDisableSec = 5
ctx.delayBeforeScaleDownSec = 10
stage = new Stage(Execution.newPipeline("orca"), "resize", ctx)
when: 'delayBeforeDisableSec and delayBeforeScaleDownSec are specified'
pipeline.stageByRef("2-deploy1").context.delayBeforeDisableSec = 5
pipeline.stageByRef("2-deploy1").context.delayBeforeScaleDownSec = 10
syntheticStages = strat.composeFlow(stage)
beforeStages = syntheticStages.findAll { it.syntheticStageOwner == SyntheticStageOwner.STAGE_BEFORE }
afterStages = syntheticStages.findAll { it.syntheticStageOwner == SyntheticStageOwner.STAGE_AFTER }
Expand Down
30 changes: 30 additions & 0 deletions ...lix/spinnaker/orca/clouddriver/pipeline/providers/gce/GceDeployStagePreProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,36 @@ void redBlackStrategyTest() {
.containsExactly(resizeServerGroupStage);
}

@Test
void redBlackStrategyNoExistingServerGroupTest() {
when(targetServerGroupResolver.resolve(any()))
.thenThrow(TargetServerGroup.NotFoundException.class);
Stage stage = new Stage();
Map<String, Object> context = createDefaultContext();
context.put("strategy", "redblack");
stage.setContext(context);

// Before Stages
List<StageDefinition> beforeStages = preProcessor.beforeStageDefinitions(stage);
assertThat(beforeStages).isEmpty();

// Additional Steps
List<StepDefinition> additionalSteps = preProcessor.additionalSteps(stage);
assertThat(additionalSteps)
.extracting(StepDefinition::getTaskClass)
.containsExactly(CaptureSourceServerGroupCapacityTask.class);

// After Stages
List<StageDefinition> afterStages = preProcessor.afterStageDefinitions(stage);
assertThat(afterStages)
.extracting(stageDefinition -> stageDefinition.stageDefinitionBuilder)
.containsExactly(applySourceServerGroupCapacityStage);

// On Failure Stages
List<StageDefinition> failureStages = preProcessor.onFailureStageDefinitions(stage);
assertThat(failureStages).isEmpty();
}

@Test
void noneStrategyTest() {
Stage stage = new Stage();
Expand Down

0 comments on commit 9817373

Please sign in to comment.