fix(bake): Fix the fix for inconsistent names in parallel bakes (#3236)

* test(bake): Add test to reproduce bug with image name mismatch check with unrelated bake stages * fix(bake): Fix logic to fail bake on inconsistent image names to look at the applicable stages * fix(bake): Address review feedback * fix(bake): Address review feedback * fix(bake): Address review feedback
spinnaker · Oct 18, 2019 · a31cde3 · a31cde3
1 parent f95f6b1
commit a31cde3
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 3 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
@@ -141,6 +141,9 @@ class BakeStage implements StageDefinitionBuilder {
         it.parentStageId == stage.parentStageId && it.status == ExecutionStatus.RUNNING
       }
 
+      // FIXME? (lpollo): I don't know why we do this, but we include in relatedStages all parallel bake stages in the
+      //  same pipeline, not just the ones related to the stage passed to this task, which means this list actually
+      //  contains *unrelated* bake stages...
       def relatedBakeStages = stage.execution.stages.findAll {
         it.type == PIPELINE_CONFIG_TYPE && bakeInitializationStages*.id.contains(it.parentStageId)
       }
@@ -159,8 +162,11 @@ class BakeStage implements StageDefinitionBuilder {
       ]
 
       if (failOnImageNameMismatchEnabled()) {
-         List<String> distinctImageNames = globalContext.deploymentDetails.stream()
-          .map({details -> details['imageName']})
+        // find distinct image names in bake stages that are actually related to the stage passed into the task
+        List<String> distinctImageNames = relatedBakeStages
+          .findAll { childStage -> childStage.parentStageId == stage.id && childStage.context.imageName }
+          .stream()
+          .map { childStage -> childStage.context.imageName }
           .distinct()
           .collect(Collectors.toList())
 

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,7 +120,7 @@ class BakeStageSpec extends Specification {
     }
   }
 
-  def "should fail if image names don't match across regions (unless user opts out)"() {
+  def "should fail if image names don't match across regions"() {
     given:
     def pipeline = pipeline {
       stage {
@@ -154,6 +154,51 @@ class BakeStageSpec extends Specification {
     thrown(ConstraintViolationException)
   }
 
+  def "should NOT fail if image names from unrelated bake stages don't match"() {
+    given:
+    def pipeline = pipeline {
+      stage {
+        id = "1"
+        type = "bake"
+        context = [
+          "region": "us-east-1",
+          "regions": ["us-east-1", "us-west-2"]
+        ]
+        status = ExecutionStatus.RUNNING
+      }
+      // this is a sibling bake stage whose child bake contexts should not be included in stage 1's outputs, but are
+      stage {
+        id = "2"
+        type = "bake"
+        context = [
+          "region": "us-east-1",
+          "regions": ["us-east-1"]
+        ]
+        status = ExecutionStatus.RUNNING
+      }
+    }
+
+    for (stageId in ["1", "2"]) {
+      def bakeStage = pipeline.stageById(stageId)
+      def graph = StageGraphBuilder.beforeStages(bakeStage)
+      new BakeStage(regionCollector: new RegionCollector()).beforeStages(bakeStage, graph)
+      def childBakeStages = graph.build()
+      childBakeStages.eachWithIndex { it, idx ->
+        it.context.ami = "${idx}"
+        it.context.imageName = "image-from-bake-stage-${stageId}"
+      }
+      pipeline.stages.addAll(childBakeStages)
+    }
+
+    dynamicConfigService.isEnabled("stages.bake.failOnImageNameMismatch", false) >> { true }
+
+    when:
+    new BakeStage.CompleteParallelBakeTask(dynamicConfigService).execute(pipeline.stageById("1"))
+
+    then:
+    notThrown(ConstraintViolationException)
+  }
+
   private
   static List<Map> deployAz(String cloudProvider, String prefix, String... regions) {
     if (prefix == "clusters") {