fix(pipeline_template): Correctly handle multiple stage injects

spinnaker · Sep 15, 2017 · 6556540 · 6556540
1 parent 6745adb
commit 6556540
Show file tree

Hide file tree

Showing 5 changed files with 135 additions and 8 deletions.
diff --git a/...innaker/orca/pipelinetemplate/v1schema/graph/transform/ConfigStageInjectionTransform.java b/...innaker/orca/pipelinetemplate/v1schema/graph/transform/ConfigStageInjectionTransform.java
@@ -193,7 +193,7 @@ private static List<StageDefinition> createGraph(List<StageDefinition> stages) {
 
   private static void injectStages(List<StageDefinition> stages, List<StageDefinition> templateStages) {
     // Using a stream here can cause a ConcurrentModificationException.
-    for (StageDefinition s : stages) {
+    for (StageDefinition s : new ArrayList<>(stages)) {
       if (s.getInject() == null) {
         continue;
       }
@@ -205,22 +205,22 @@ private static void injectStages(List<StageDefinition> stages, List<StageDefinit
 
       if (rule.getFirst() != null && rule.getFirst()) {
         injectFirst(s, templateStages);
-        return;
+        continue;
       }
 
       if (rule.getLast() != null && rule.getLast()) {
         injectLast(s, templateStages);
-        return;
+        continue;
       }
 
       if (rule.getBefore() != null) {
         injectBefore(s, rule.getBefore(), templateStages);
-        return;
+        continue;
       }
 
       if (rule.getAfter() != null) {
         injectAfter(s, rule.getAfter(), templateStages);
-        return;
+        continue;
       }
 
       throw new IllegalTemplateConfigurationException(String.format("stage did not have any valid injections defined (id: %s)", s.getId()));
@@ -273,7 +273,7 @@ private static void injectBefore(StageDefinition stage, List<String> targetIds,
     }
 
     stage.getRequisiteStageRefIds().addAll(targetEdges);
-    allStages.add(targetIndexes.last() + 1, stage);
+    allStages.add(targetIndexes.last(), stage);
 
     Map<String, StageDefinition> graph = allStages
       .stream()

diff --git a/...roovy/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/V1SchemaIntegrationSpec.groovy b/...roovy/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/V1SchemaIntegrationSpec.groovy
@@ -65,8 +65,14 @@ class V1SchemaIntegrationSpec extends Specification {
 
   @Unroll
   def 'test "#integration.name"'() {
-    expect:
-    assertReflectionEquals(integration.expected, subject.process(integration.toRequest()), ReflectionComparatorMode.IGNORE_DEFAULTS)
+    given:
+    def expected = integration.expected
+
+    when:
+    def result = subject.process(integration.toRequest())
+
+    then:
+    assertReflectionEquals(expected, result, ReflectionComparatorMode.IGNORE_DEFAULTS)
 
     where:
     integration << new IntegrationTestDataProvider().provide()

diff --git a/orca-pipelinetemplate/src/test/resources/integration/v1schema/stageInjection-config.yml b/orca-pipelinetemplate/src/test/resources/integration/v1schema/stageInjection-config.yml
@@ -0,0 +1,31 @@
+schema: "1"
+pipeline:
+  application: orca
+  name: Stage Injection Test
+  variables:
+    waitTime: 5
+stages:
+- id: first
+  type: wait
+  inject:
+    first: true
+  config:
+    waitTime: "{{ waitTime }}"
+- id: last
+  type: wait
+  inject:
+    last: true
+  config:
+    waitTime: "{{ waitTime }}"
+- id: afterWait2
+  type: wait
+  inject:
+    after: [wait2]
+  config:
+    waitTime: "{{ waitTime }}"
+- id: beforeWait2
+  type: wait
+  inject:
+    before: [wait2]
+  config:
+    waitTime: "{{ waitTime }}"
diff --git a/orca-pipelinetemplate/src/test/resources/integration/v1schema/stageInjection-expected.json b/orca-pipelinetemplate/src/test/resources/integration/v1schema/stageInjection-expected.json
@@ -0,0 +1,66 @@
+{
+  "keepWaitingPipelines": false,
+  "limitConcurrent": true,
+  "application": "orca",
+  "name": "Stage Injection Test",
+  "stages": [
+    {
+      "requisiteStageRefIds": [],
+      "name": "first",
+      "id": null,
+      "refId": "first",
+      "type": "wait",
+      "waitTime": 5
+    },
+    {
+      "requisiteStageRefIds": ["first"],
+      "name": "wait",
+      "id": null,
+      "refId": "wait",
+      "type": "wait",
+      "waitTime": 5
+    },
+    {
+      "requisiteStageRefIds": ["wait"],
+      "name": "wait3",
+      "id": null,
+      "refId": "wait3",
+      "type": "wait",
+      "waitTime": 5
+    },
+    {
+      "requisiteStageRefIds": ["wait"],
+      "name": "beforeWait2",
+      "id": null,
+      "refId": "beforeWait2",
+      "type": "wait",
+      "waitTime": 5
+    },
+    {
+      "requisiteStageRefIds": ["beforeWait2"],
+      "name": "wait2",
+      "id": null,
+      "refId": "wait2",
+      "type": "wait",
+      "waitTime": 5
+    },
+    {
+      "requisiteStageRefIds": ["wait2"],
+      "name": "afterWait2",
+      "id": null,
+      "refId": "afterWait2",
+      "type": "wait",
+      "waitTime": 5
+    },
+    {
+      "requisiteStageRefIds": ["wait3", "afterWait2"],
+      "name": "last",
+      "id": null,
+      "refId": "last",
+      "type": "wait",
+      "waitTime": 5
+    }
+  ],
+  "id": "unknown",
+  "notifications": []
+}
diff --git a/orca-pipelinetemplate/src/test/resources/integration/v1schema/stageInjection-template.yml b/orca-pipelinetemplate/src/test/resources/integration/v1schema/stageInjection-template.yml
@@ -0,0 +1,24 @@
+schema: "1"
+id: stageInjection
+metadata:
+  name: Stage injection
+  description: Stage injection test
+variables:
+- name: waitTime
+  description: The time a wait stage should pause
+  type: int
+stages:
+- id: wait
+  type: wait
+  config:
+    waitTime: "{{ waitTime }}"
+- id: wait2
+  type: wait
+  dependsOn: [wait]
+  config:
+    waitTime: "{{ waitTime }}"
+- id: wait3
+  type: wait
+  dependsOn: [wait]
+  config:
+    waitTime: "{{ waitTime }}"