chore(core): simpler handling of parallel stages

spinnaker · Sep 18, 2017 · a40d508 · a40d508
1 parent bdc8757
commit a40d508
Show file tree

Hide file tree

Showing 11 changed files with 114 additions and 221 deletions.
diff --git a/orca-bakery/src/main/groovy/com/netflix/spinnaker/orca/bakery/pipeline/BakeStage.groovy b/orca-bakery/src/main/groovy/com/netflix/spinnaker/orca/bakery/pipeline/BakeStage.groovy
@@ -16,46 +16,49 @@
 
 package com.netflix.spinnaker.orca.bakery.pipeline
 
+import javax.annotation.Nonnull
 import com.netflix.spinnaker.orca.ExecutionStatus
 import com.netflix.spinnaker.orca.RestartableStage
 import com.netflix.spinnaker.orca.Task
 import com.netflix.spinnaker.orca.TaskResult
 import com.netflix.spinnaker.orca.bakery.tasks.CompletedBakeTask
 import com.netflix.spinnaker.orca.bakery.tasks.CreateBakeTask
 import com.netflix.spinnaker.orca.bakery.tasks.MonitorBakeTask
-import com.netflix.spinnaker.orca.pipeline.BranchingStageDefinitionBuilder
+import com.netflix.spinnaker.orca.pipeline.StageDefinitionBuilder
 import com.netflix.spinnaker.orca.pipeline.TaskNode
 import com.netflix.spinnaker.orca.pipeline.model.Execution
 import com.netflix.spinnaker.orca.pipeline.model.Stage
 import groovy.transform.CompileDynamic
 import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
 import org.springframework.stereotype.Component
+import static com.netflix.spinnaker.orca.pipeline.model.SyntheticStageOwner.STAGE_BEFORE
 
 @Slf4j
 @Component
 @CompileStatic
-class BakeStage implements BranchingStageDefinitionBuilder, RestartableStage {
+class BakeStage implements StageDefinitionBuilder, RestartableStage {
 
   public static final String PIPELINE_CONFIG_TYPE = "bake"
 
   @Override
   <T extends Execution<T>> void taskGraph(Stage<T> stage, TaskNode.Builder builder) {
     builder
-      .withTask("createBake", CreateBakeTask)
-      .withTask("monitorBake", MonitorBakeTask)
-      .withTask("completedBake", CompletedBakeTask)
+      .withTask("completeParallel", CompleteParallelBakeTask)
   }
 
   @Override
-  void postBranchGraph(Stage<?> stage, TaskNode.Builder builder) {
-    builder
-      .withTask("completeParallel", CompleteParallelBakeTask)
+  @Nonnull
+  <T extends Execution<T>> List<Stage<T>> parallelStages(
+    @Nonnull Stage<T> stage
+  ) {
+    parallelContexts(stage).collect { context ->
+      newStage(stage.execution, "${type}.parallel", "Bake in ${context.region}", context, stage, STAGE_BEFORE)
+    }
   }
 
-  @Override
   @CompileDynamic
-  public <T extends Execution<T>> Collection<Map<String, Object>> parallelContexts(Stage<T> stage) {
+  <T extends Execution<T>> Collection<Map<String, Object>> parallelContexts(Stage<T> stage) {
     Set<String> deployRegions = stage.context.region ? [stage.context.region] as Set<String> : []
     deployRegions.addAll(stage.context.regions as Set<String> ?: [])
 
@@ -100,9 +103,21 @@ class BakeStage implements BranchingStageDefinitionBuilder, RestartableStage {
     }
   }
 
-  @Override
-  String parallelStageName(Stage<?> stage, boolean hasParallelFlows) {
-    return hasParallelFlows ? "Multi-region Bake" : stage.name
+  @Component
+  @CompileStatic
+  static class Parallel implements StageDefinitionBuilder {
+    @Override
+    <T extends Execution<T>> void taskGraph(Stage<T> stage, TaskNode.Builder builder) {
+      builder
+        .withTask("createBake", CreateBakeTask)
+        .withTask("monitorBake", MonitorBakeTask)
+        .withTask("completedBake", CompletedBakeTask)
+    }
+
+    @Override
+    String getType() {
+      return "${PIPELINE_CONFIG_TYPE}.parallel"
+    }
   }
 
   @Component

diff --git a/orca-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/pipeline/BakeStageSpec.groovy b/orca-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/pipeline/BakeStageSpec.groovy
@@ -120,23 +120,6 @@ class BakeStageSpec extends Specification {
     ]
   }
 
-  @Unroll
-  def "should return a different stage name when parallel flows are present"() {
-    given:
-    def stage = stage {
-      type = "type"
-      name = stageName
-    }
-
-    expect:
-    new BakeStage().parallelStageName(stage, hasParallelFlows) == expectedStageName
-
-    where:
-    stageName | hasParallelFlows || expectedStageName
-    "Default" | false            || "Default"
-    "Default" | true             || "Multi-region Bake"
-  }
-
   private
   static List<Map> deployAz(String cloudProvider, String prefix, String... regions) {
     if (prefix == "clusters") {

diff --git a/...river/src/main/groovy/com/netflix/spinnaker/orca/kato/pipeline/ParallelDeployStage.groovy b/...river/src/main/groovy/com/netflix/spinnaker/orca/kato/pipeline/ParallelDeployStage.groovy
@@ -16,12 +16,13 @@
 
 package com.netflix.spinnaker.orca.kato.pipeline
 
+import javax.annotation.Nonnull
 import com.netflix.spinnaker.orca.ExecutionStatus
 import com.netflix.spinnaker.orca.Task
 import com.netflix.spinnaker.orca.TaskResult
 import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.CloneServerGroupStage
 import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.CreateServerGroupStage
-import com.netflix.spinnaker.orca.pipeline.BranchingStageDefinitionBuilder
+import com.netflix.spinnaker.orca.pipeline.StageDefinitionBuilder
 import com.netflix.spinnaker.orca.pipeline.TaskNode
 import com.netflix.spinnaker.orca.pipeline.model.Execution
 import com.netflix.spinnaker.orca.pipeline.model.Pipeline
@@ -30,11 +31,12 @@ import groovy.transform.CompileDynamic
 import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
 import org.springframework.stereotype.Component
+import static com.netflix.spinnaker.orca.pipeline.model.SyntheticStageOwner.STAGE_BEFORE
 
 @Component
 @Slf4j
 @CompileStatic
-class ParallelDeployStage implements BranchingStageDefinitionBuilder {
+class ParallelDeployStage implements StageDefinitionBuilder {
 
   @Deprecated
   public static final String PIPELINE_CONFIG_TYPE = "deploy"
@@ -46,16 +48,15 @@ class ParallelDeployStage implements BranchingStageDefinitionBuilder {
 
   @Override
   <T extends Execution<T>> void taskGraph(Stage<T> stage, TaskNode.Builder builder) {
-  }
-
-  @Override
-  void postBranchGraph(Stage<?> stage, TaskNode.Builder builder) {
     builder.withTask("completeParallelDeploy", CompleteParallelDeployTask)
   }
 
-  @Override
-  String getChildStageType(Stage childStage) {
-    return isClone(childStage) ? CloneServerGroupStage.PIPELINE_CONFIG_TYPE : PIPELINE_CONFIG_TYPE
+  @Nonnull <T extends Execution<T>> List<Stage<T>> parallelStages(
+    @Nonnull Stage<T> stage) {
+    parallelContexts(stage).collect { context ->
+      def type = isClone(stage) ? CloneServerGroupStage.PIPELINE_CONFIG_TYPE : CreateServerGroupStage.PIPELINE_CONFIG_TYPE
+      newStage(stage.execution, type, context.name as String, context, stage, STAGE_BEFORE)
+    }
   }
 
   @CompileDynamic
@@ -77,9 +78,8 @@ class ParallelDeployStage implements BranchingStageDefinitionBuilder {
     ]
   }
 
-  @Override
   @CompileDynamic
-  <T extends Execution<T>> Collection<Map<String, Object>> parallelContexts(Stage<T> stage) {
+  protected <T extends Execution<T>> Collection<Map<String, Object>> parallelContexts(Stage<T> stage) {
     if (stage.execution instanceof Pipeline) {
       Map trigger = ((Pipeline) stage.execution).trigger
       if (trigger.parameters?.strategy == true) {
@@ -145,11 +145,6 @@ class ParallelDeployStage implements BranchingStageDefinitionBuilder {
     }
   }
 
-  @Override
-  String parallelStageName(Stage<?> stage, boolean hasParallelFlows) {
-    return isClone(stage) ? "Clone" : stage.name
-  }
-
   @CompileDynamic
   private <T extends Execution<T>> boolean isClone(Stage<T> stage) {
     if (stage.execution instanceof Pipeline) {
@@ -166,7 +161,7 @@ class ParallelDeployStage implements BranchingStageDefinitionBuilder {
   @Component
   @Slf4j
   @CompileStatic
-  public static class CompleteParallelDeployTask implements Task {
+  static class CompleteParallelDeployTask implements Task {
     TaskResult execute(Stage stage) {
       log.info("Completed Parallel Deploy")
       new TaskResult(ExecutionStatus.SUCCEEDED, [:], [:])

diff --git a/...r/src/test/groovy/com/netflix/spinnaker/orca/kato/pipeline/ParallelDeployStageSpec.groovy b/...r/src/test/groovy/com/netflix/spinnaker/orca/kato/pipeline/ParallelDeployStageSpec.groovy
@@ -45,20 +45,6 @@ class ParallelDeployStageSpec extends Specification {
                                                                              [account: "prod", restrictedExecutionWindow: [:], cluster: [availabilityZones: ["europe-west1-b": []], cloudProvider: "gce"], type: "createServerGroup", name: "Deploy in europe-west1-b"]]
   }
 
-  @Unroll
-  def "should return stage name regardless of whether parallel flows are present"() {
-    given:
-    def stage = new Stage<>(new Pipeline("orca"), "type", stageName, [:])
-
-    expect:
-    new ParallelDeployStage().parallelStageName(stage, hasParallelFlows) == expectedStageName
-
-    where:
-    stageName | hasParallelFlows || expectedStageName
-    "Default" | false            || "Default"
-    "Default" | true             || "Default"
-  }
-
   Map deployStageContext(String account, String cloudProvider, String... availabilityZones) {
     def context = ["account": account, restrictedExecutionWindow: [:]]
     if (availabilityZones.size() == 1) {

diff --git a/.../src/main/groovy/com/netflix/spinnaker/orca/pipeline/BranchingStageDefinitionBuilder.java b/.../src/main/groovy/com/netflix/spinnaker/orca/pipeline/BranchingStageDefinitionBuilder.java
diff --git a/orca-core/src/main/groovy/com/netflix/spinnaker/orca/pipeline/CheckPreconditionsStage.groovy b/orca-core/src/main/groovy/com/netflix/spinnaker/orca/pipeline/CheckPreconditionsStage.groovy
@@ -16,42 +16,30 @@
 
 package com.netflix.spinnaker.orca.pipeline
 
+import javax.annotation.Nonnull
 import com.netflix.spinnaker.orca.Task
 import com.netflix.spinnaker.orca.pipeline.model.Execution
 import com.netflix.spinnaker.orca.pipeline.model.Stage
 import com.netflix.spinnaker.orca.pipeline.tasks.PreconditionTask
+import groovy.transform.CompileStatic
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
+import static com.netflix.spinnaker.orca.pipeline.model.SyntheticStageOwner.STAGE_BEFORE
 
 @Component
-class CheckPreconditionsStage implements BranchingStageDefinitionBuilder {
+@CompileStatic
+class CheckPreconditionsStage implements StageDefinitionBuilder {
 
   static final String PIPELINE_CONFIG_TYPE = "checkPreconditions"
 
-  private final List<? extends PreconditionTask> preconditionTasks
-
-  @Autowired
-  CheckPreconditionsStage(List<? extends PreconditionTask> preconditionTasks) {
-    this.preconditionTasks = preconditionTasks
-  }
-
-  @Override
-  def <T extends Execution<T>> void taskGraph(Stage<T> stage, TaskNode.Builder builder) {
-    String preconditionType = stage.context.preconditionType
-    if (!preconditionType) {
-      throw new IllegalStateException("no preconditionType specified for stage $stage.id")
-    }
-    Task preconditionTask = preconditionTasks.find {
-      it.preconditionType == preconditionType
+  @Nonnull <T extends Execution<T>> List<Stage<T>> parallelStages(
+    @Nonnull Stage<T> stage) {
+    parallelContexts(stage).collect { context ->
+      newStage(stage.execution, "${type}.parallel", "Check precondition (${context.preconditionType})", context, stage, STAGE_BEFORE)
     }
-    if (!preconditionTask) {
-      throw new IllegalStateException("no Precondition implementation for type $preconditionType")
-    }
-    builder.withTask("checkPrecondition", preconditionTask.getClass() as Class<? extends Task>)
   }
 
-  @Override
-  def <T extends Execution<T>> Collection<Map<String, Object>> parallelContexts(Stage<T> stage) {
+  private <T extends Execution<T>> Collection<Map<String, Object>> parallelContexts(Stage<T> stage) {
     stage.resolveStrategyParams()
     def baseContext = new HashMap(stage.context)
     List<Map> preconditions = baseContext.remove('preconditions') as List<Map>
@@ -67,8 +55,37 @@ class CheckPreconditionsStage implements BranchingStageDefinitionBuilder {
         context['context'][it] = context['context'][it] ?: baseContext[it]
       }
 
-      context.name = context.name ?: "Check precondition (${context.preconditionType})".toString()
       return context
     }
   }
+
+  @Component
+  static class Parallel implements StageDefinitionBuilder {
+    private final List<? extends PreconditionTask> preconditionTasks
+
+    @Autowired
+    Parallel(List<? extends PreconditionTask> preconditionTasks) {
+      this.preconditionTasks = preconditionTasks
+    }
+
+    @Override
+    def <T extends Execution<T>> void taskGraph(Stage<T> stage, TaskNode.Builder builder) {
+      String preconditionType = stage.context.preconditionType
+      if (!preconditionType) {
+        throw new IllegalStateException("no preconditionType specified for stage $stage.id")
+      }
+      Task preconditionTask = preconditionTasks.find {
+        it.preconditionType == preconditionType
+      }
+      if (!preconditionTask) {
+        throw new IllegalStateException("no Precondition implementation for type $preconditionType")
+      }
+      builder.withTask("checkPrecondition", preconditionTask.getClass() as Class<? extends Task>)
+    }
+
+    @Override
+    String getType() {
+      return "${PIPELINE_CONFIG_TYPE}.parallel"
+    }
+  }
 }