fix(queue): Ensure that refId generation accounts for existing stages (…

…#2004)
spinnaker · Feb 19, 2018 · f5d3390 · f5d3390
1 parent fb2f3d5
commit f5d3390
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 6 deletions.
diff --git a/orca-queue-tck/src/main/kotlin/com/netflix/spinnaker/orca/q/PipelineBuilder.kt b/orca-queue-tck/src/main/kotlin/com/netflix/spinnaker/orca/q/PipelineBuilder.kt
@@ -35,6 +35,13 @@ fun pipeline(init: Execution.() -> Unit = {}): Execution {
   return pipeline
 }
 
+fun stage(init: Stage.() -> Unit = {}): Stage {
+  val stage = Stage()
+  stage.type = "test"
+  stage.init()
+  return stage
+}
+
 /**
  * Build a top-level stage. Use in the context of [#pipeline].
  *

diff --git a/orca-queue/src/main/kotlin/com/netflix/spinnaker/orca/q/StageDefinitionBuilders.kt b/orca-queue/src/main/kotlin/com/netflix/spinnaker/orca/q/StageDefinitionBuilders.kt
@@ -88,11 +88,14 @@ fun StageDefinitionBuilder.buildSyntheticStages(
 fun StageDefinitionBuilder.buildAfterStages(
   stage: Stage, afterStages: List<Stage>, callback: (Stage) -> Unit = {}
 ) {
+  val offset = stage.afterStages().size
+
   afterStages.forEachIndexed { i, it ->
+    val offsetIndex = offset + i
     it.sanitizeContext()
-    it.refId = "${stage.refId}>${i + 1}"
-    if (i > 0) {
-      it.requisiteStageRefIds = setOf("${stage.refId}>$i")
+    it.refId = "${stage.refId}>${offsetIndex + 1}"
+    if (offsetIndex > 0) {
+      it.requisiteStageRefIds = setOf("${stage.refId}>$offsetIndex")
     } else {
       it.requisiteStageRefIds = emptySet()
     }
@@ -126,11 +129,14 @@ private fun SyntheticStages.buildBeforeStages(stage: Stage,
     listOf(executionWindow) + beforeStages
   }
 
+  val offset = stage.beforeStages().size
+
   allBeforeStages.forEachIndexed { i, it ->
+    val offsetIndex = offset + i
     it.sanitizeContext()
-    it.refId = "${stage.refId}<${i + 1}"
-    if (i > 0) {
-      it.requisiteStageRefIds = setOf("${stage.refId}<$i")
+    it.refId = "${stage.refId}<${offsetIndex + 1}"
+    if (offsetIndex > 0) {
+      it.requisiteStageRefIds = setOf("${stage.refId}<$offsetIndex")
     } else {
       it.requisiteStageRefIds = emptySet()
     }

diff --git a/orca-queue/src/test/kotlin/com/netflix/spinnaker/orca/q/StageDefinitionBuildersTest.kt b/orca-queue/src/test/kotlin/com/netflix/spinnaker/orca/q/StageDefinitionBuildersTest.kt
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2018 Netflix, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.spinnaker.orca.q
+
+import org.jetbrains.spek.api.Spek
+import org.jetbrains.spek.api.dsl.describe
+import org.jetbrains.spek.api.dsl.on
+
+import com.netflix.spinnaker.orca.pipeline.StageDefinitionBuilder
+import com.netflix.spinnaker.orca.pipeline.model.Execution
+import com.netflix.spinnaker.orca.pipeline.model.SyntheticStageOwner
+import com.netflix.spinnaker.spek.shouldEqual
+import org.jetbrains.spek.api.dsl.it
+
+object StageDefinitionBuildersTest : Spek({
+  val stageBuilder = MyStageDefinitionBuilder()
+
+  describe("refIds for synthetic stages") {
+    val pipeline = pipeline {
+      stage {
+        id = "1"
+        refId = "1"
+      }
+    }
+
+    on("adding a synthetic stage") {
+      stageBuilder.buildAfterStages(
+        pipeline.stageById("1"),
+        listOf(makeStage("2", "1", pipeline))
+      )
+    }
+
+    it("sets the refId with _no_ offset") {
+      val stage = pipeline.stageById("2")
+      stage.refId shouldEqual "1>1"
+      stage.requisiteStageRefIds shouldEqual emptySet()
+    }
+
+    on("adding another synthetic stage") {
+      stageBuilder.buildAfterStages(
+        pipeline.stageById("1"),
+        listOf(makeStage("3", "1", pipeline))
+      )
+    }
+
+    it("sets the refId with an initial offset") {
+      val stage = pipeline.stageById("3")
+      stage.refId shouldEqual "1>2"
+      stage.requisiteStageRefIds shouldEqual setOf("1>1")
+    }
+  }
+})
+
+private fun makeStage(stageId: String, syntheticParentStageId: String, pipeline: Execution) = stage {
+  id = stageId
+  parentStageId = pipeline.stageById(syntheticParentStageId).id
+  syntheticStageOwner = SyntheticStageOwner.STAGE_AFTER
+}
+
+private class MyStageDefinitionBuilder : StageDefinitionBuilder