fix(core): prevent stages evaluating target server groups twice

The problem previously was that we'd end up calling the aroundStages when planning before stages and then again when planning after stages (following recent changes to do that just in time). That meant the target resolution stuff happened twice and the second time it might not be valid (because of what the stage did to its target). I've changed it so that everything happens in before stages. Before it was transforming its own context to deal with a single target then spawning copies to deal with each other target. It also had to do some complex shenanigoats to weave in before and after stages of each target stage. Now the top level stage is a no op and just creates child before stages to deal with each target. Those child stages can worry about their own before and after stages so the whole thing is just a lot simpler.
spinnaker · Mar 20, 2018 · c117f1f · c117f1f
1 parent d58bf79
commit c117f1f
Show file tree

Hide file tree

Showing 13 changed files with 362 additions and 289 deletions.
diff --git a/...naker/orca/clouddriver/pipeline/providers/appengine/StartAppEngineServerGroupStage.groovy b/...naker/orca/clouddriver/pipeline/providers/appengine/StartAppEngineServerGroupStage.groovy
@@ -33,7 +33,7 @@ class StartAppEngineServerGroupStage extends TargetServerGroupLinearStageSupport
   public static final String PIPELINE_CONFIG_TYPE = "startServerGroup"
 
   @Override
-  void taskGraph(Stage stage, TaskNode.Builder builder) {
+  protected void taskGraphInternal(Stage stage, TaskNode.Builder builder) {
     builder
       .withTask("determineHealthProviders", DetermineHealthProvidersTask)
       .withTask("startServerGroup", StartAppEngineServerGroupTask)

diff --git a/...nnaker/orca/clouddriver/pipeline/providers/appengine/StopAppEngineServerGroupStage.groovy b/...nnaker/orca/clouddriver/pipeline/providers/appengine/StopAppEngineServerGroupStage.groovy
@@ -33,7 +33,7 @@ class StopAppEngineServerGroupStage extends TargetServerGroupLinearStageSupport
   public static final String PIPELINE_CONFIG_TYPE = "stopServerGroup"
 
   @Override
-  void taskGraph(Stage stage, TaskNode.Builder builder) {
+  protected void taskGraphInternal(Stage stage, TaskNode.Builder builder) {
     builder
       .withTask("determineHealthProviders", DetermineHealthProvidersTask)
       .withTask("stopServerGroup", StopAppEngineServerGroupTask)

diff --git a/...lix/spinnaker/orca/clouddriver/pipeline/providers/aws/ModifyAwsScalingProcessStage.groovy b/...lix/spinnaker/orca/clouddriver/pipeline/providers/aws/ModifyAwsScalingProcessStage.groovy
@@ -36,12 +36,11 @@ import org.springframework.stereotype.Component
 @Component
 @CompileStatic
 class ModifyAwsScalingProcessStage extends TargetServerGroupLinearStageSupport {
-  ModifyAwsScalingProcessStage() {
-    name = "Modify Scaling Process"
-  }
+
+  public static final String TYPE = getType(ModifyAwsScalingProcessStage)
 
   @Override
-  void taskGraph(Stage stage, TaskNode.Builder builder) {
+  protected void taskGraphInternal(Stage stage, TaskNode.Builder builder) {
     def data = stage.mapTo(StageData)
     switch (data.action) {
       case StageAction.suspend:

diff --git a/...om/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/DestroyServerGroupStage.groovy b/...om/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/DestroyServerGroupStage.groovy
@@ -22,18 +22,14 @@ import com.netflix.spinnaker.orca.clouddriver.tasks.MonitorKatoTask
 import com.netflix.spinnaker.orca.clouddriver.tasks.servergroup.*
 import com.netflix.spinnaker.orca.pipeline.TaskNode
 import com.netflix.spinnaker.orca.pipeline.model.Stage
-import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
 
 @Component
 class DestroyServerGroupStage extends TargetServerGroupLinearStageSupport {
   static final String PIPELINE_CONFIG_TYPE = "destroyServerGroup"
 
-  @Autowired
-  DisableServerGroupStage disableServerGroupStage
-
   @Override
-  void taskGraph(Stage stage, TaskNode.Builder builder) {
+  protected void taskGraphInternal(Stage stage, TaskNode.Builder builder) {
     try {
       builder
         .withTask("disableServerGroup", DisableServerGroupTask)

diff --git a/...om/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/DisableServerGroupStage.groovy b/...om/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/DisableServerGroupStage.groovy
@@ -33,7 +33,7 @@ class DisableServerGroupStage extends TargetServerGroupLinearStageSupport {
   static final String PIPELINE_CONFIG_TYPE = "disableServerGroup"
 
   @Override
-  void taskGraph(Stage stage, TaskNode.Builder builder) {
+  protected void taskGraphInternal(Stage stage, TaskNode.Builder builder) {
     builder
       .withTask("determineHealthProviders", DetermineHealthProvidersTask)
       .withTask("disableServerGroup", DisableServerGroupTask)

diff --git a/...com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/EnableServerGroupStage.groovy b/...com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/EnableServerGroupStage.groovy
@@ -32,7 +32,7 @@ class EnableServerGroupStage extends TargetServerGroupLinearStageSupport {
   public static final String PIPELINE_CONFIG_TYPE = "enableServerGroup"
 
   @Override
-  void taskGraph(Stage stage, TaskNode.Builder builder) {
+  protected void taskGraphInternal(Stage stage, TaskNode.Builder builder) {
     builder
       .withTask("determineHealthProviders", DetermineHealthProvidersTask)
       .withTask("enableServerGroup", EnableServerGroupTask)

diff --git a/...com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/ResizeServerGroupStage.groovy b/...com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/ResizeServerGroupStage.groovy
@@ -24,9 +24,9 @@ import com.netflix.spinnaker.orca.clouddriver.tasks.servergroup.ResizeServerGrou
 import com.netflix.spinnaker.orca.clouddriver.tasks.servergroup.ServerGroupCacheForceRefreshTask
 import com.netflix.spinnaker.orca.clouddriver.tasks.servergroup.WaitForCapacityMatchTask
 import com.netflix.spinnaker.orca.pipeline.TaskNode
+import com.netflix.spinnaker.orca.pipeline.graph.StageGraphBuilder
 import com.netflix.spinnaker.orca.pipeline.model.Stage
 import groovy.util.logging.Slf4j
-import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
 
 /**
@@ -39,11 +39,8 @@ import org.springframework.stereotype.Component
 class ResizeServerGroupStage extends TargetServerGroupLinearStageSupport {
   public static final String TYPE = getType(ResizeServerGroupStage)
 
-  @Autowired
-  ModifyAwsScalingProcessStage modifyAwsScalingProcessStage
-
   @Override
-  void taskGraph(Stage stage, TaskNode.Builder builder) {
+  protected void taskGraphInternal(Stage stage, TaskNode.Builder builder) {
     builder
       .withTask("determineHealthProviders", DetermineHealthProvidersTask)
       .withTask("resizeServerGroup", ResizeServerGroupTask)
@@ -53,70 +50,64 @@ class ResizeServerGroupStage extends TargetServerGroupLinearStageSupport {
   }
 
   @Override
-  protected List<Injectable> preStatic(Map descriptor) {
-    if (descriptor.cloudProvider != 'aws') {
-      return []
+  protected void preStatic(Map<String, Object> descriptor, StageGraphBuilder graph) {
+    if (descriptor.cloudProvider == "aws") {
+      graph.add {
+        it.name = "resumeScalingProcesses"
+        it.type = ModifyAwsScalingProcessStage.TYPE
+        it.context = [
+          serverGroupName: descriptor.asgName,
+          cloudProvider  : descriptor.cloudProvider,
+          credentials    : descriptor.credentials,
+          region         : descriptor.region,
+          action         : "resume",
+          processes      : ["Launch", "Terminate"]
+        ]
+      }
     }
-    [new Injectable(
-      name: "resumeScalingProcesses",
-      stage: modifyAwsScalingProcessStage,
-      context: [
-        serverGroupName: descriptor.asgName,
-        cloudProvider  : descriptor.cloudProvider,
-        credentials    : descriptor.credentials,
-        region         : descriptor.region,
-        action         : "resume",
-        processes      : ["Launch", "Terminate"]
-      ]
-    )]
   }
 
   @Override
-  protected List<Injectable> postStatic(Map descriptor) {
-    if (descriptor.cloudProvider != 'aws') {
-      return []
+  protected void postStatic(Map<String, Object> descriptor, StageGraphBuilder graph) {
+    if (descriptor.cloudProvider == "aws") {
+      graph.add {
+        it.name = "suspendScalingProcesses"
+        it.type = ModifyAwsScalingProcessStage.TYPE
+        it.context = [
+          serverGroupName: descriptor.asgName,
+          cloudProvider  : descriptor.cloudProvider,
+          credentials    : descriptor.credentials,
+          region         : descriptor.region,
+          action         : "suspend"
+        ]
+      }
     }
-    [new Injectable(
-      name: "suspendScalingProcesses",
-      stage: modifyAwsScalingProcessStage,
-      context: [
-        serverGroupName: descriptor.asgName,
-        cloudProvider  : descriptor.cloudProvider,
-        credentials    : descriptor.credentials,
-        region         : descriptor.region,
-        action         : "suspend"
-      ]
-    )]
   }
 
   @Override
-  protected List<Injectable> preDynamic(Map context) {
-    if (context.cloudProvider != 'aws') {
-      return []
+  protected void preDynamic(Map<String, Object> context, StageGraphBuilder graph) {
+    if (context.cloudProvider == "aws") {
+      context.remove("asgName")
+      graph.add {
+        it.name = "resumeScalingProcesses"
+        it.type = ModifyAwsScalingProcessStage.TYPE
+        it.context.putAll(context)
+        it.context["action"] = "resume"
+        it.context["processes"] = ["Launch", "Terminate"]
+      }
     }
-    context.remove("asgName")
-    [new Injectable(
-      name: "resumeScalingProcesses",
-      stage: modifyAwsScalingProcessStage,
-      context: context + [
-        action   : "resume",
-        processes: ["Launch", "Terminate"]
-      ]
-    )]
   }
 
   @Override
-  protected List<Injectable> postDynamic(Map context) {
-    if (context.cloudProvider != 'aws') {
-      return []
+  protected void postDynamic(Map<String, Object> context, StageGraphBuilder graph) {
+    if (context.cloudProvider == "aws") {
+      context.remove("asgName")
+      graph.add {
+        it.name = "suspendScalingProcesses"
+        it.type = ModifyAwsScalingProcessStage.TYPE
+        it.context.putAll(context)
+        it.context["action"] = "suspend"
+      }
     }
-    context.remove("asgName")
-    [new Injectable(
-      name: "suspendScalingProcesses",
-      stage: modifyAwsScalingProcessStage,
-      context: context + [
-        action: "suspend",
-      ],
-    )]
   }
 }
diff --git a/.../netflix/spinnaker/orca/clouddriver/pipeline/servergroup/support/TargetServerGroup.groovy b/.../netflix/spinnaker/orca/clouddriver/pipeline/servergroup/support/TargetServerGroup.groovy
@@ -17,13 +17,13 @@
 package com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support
 
 import com.fasterxml.jackson.databind.ObjectMapper
-import groovy.transform.InheritConstructors
-import groovy.transform.ToString
-import groovy.util.logging.Slf4j
 import com.netflix.frigga.Names
 import com.netflix.spinnaker.moniker.Moniker
 import com.netflix.spinnaker.orca.kato.pipeline.support.StageData
 import com.netflix.spinnaker.orca.pipeline.model.Stage
+import groovy.transform.InheritConstructors
+import groovy.transform.ToString
+import groovy.util.logging.Slf4j
 
 /**
  * A TargetServerGroup is a ServerGroup that is dynamically resolved using a target like "current" or "oldest".