fix(queue): ensure correct precedence of FAILED_CONTINUE and STOPPED …

…statuses
spinnaker · May 5, 2017 · ba32d06 · ba32d06
1 parent e4e4f92
commit ba32d06
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 13 deletions.
diff --git a/orca-queue/src/main/kotlin/com/netflix/spinnaker/orca/q/handler/RunTaskHandler.kt b/orca-queue/src/main/kotlin/com/netflix/spinnaker/orca/q/handler/RunTaskHandler.kt
@@ -70,10 +70,10 @@ open class RunTaskHandler
               SUCCEEDED, REDIRECT ->
                 queue.push(CompleteTask(message, result.status))
               TERMINAL ->
-                if (!stage.shouldFailPipeline()) {
-                  queue.push(CompleteTask(message, STOPPED))
-                } else if (stage.shouldContinueOnFailure()) {
+                if (stage.shouldContinueOnFailure()) {
                   queue.push(CompleteTask(message, FAILED_CONTINUE))
+                } else if (!stage.shouldFailPipeline()) {
+                  queue.push(CompleteTask(message, STOPPED))
                 } else {
                   queue.push(CompleteTask(message, result.status))
                 }

diff --git a/orca-queue/src/test/kotlin/com/netflix/spinnaker/orca/q/handler/RunTaskHandlerSpec.kt b/orca-queue/src/test/kotlin/com/netflix/spinnaker/orca/q/handler/RunTaskHandlerSpec.kt
@@ -25,6 +25,7 @@ import com.netflix.spinnaker.orca.pipeline.model.Stage
 import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository
 import com.netflix.spinnaker.orca.q.*
 import com.netflix.spinnaker.orca.time.fixedClock
+import com.netflix.spinnaker.spek.and
 import com.nhaarman.mockito_kotlin.*
 import org.jetbrains.spek.api.Spek
 import org.jetbrains.spek.api.dsl.context
@@ -119,6 +120,7 @@ class RunTaskHandlerSpec : Spek({
     describe("that fails") {
       val pipeline = pipeline {
         stage {
+          refId = "1"
           type = "whatever"
           startTime = clock.instant().toEpochMilli()
           task {
@@ -130,7 +132,7 @@ class RunTaskHandlerSpec : Spek({
       val message = RunTask(Pipeline::class.java, pipeline.id, "foo", pipeline.stages.first().id, "1", DummyTask::class.java)
       val taskResult = TaskResult(TERMINAL)
 
-      context("no overrides are in place") {
+      and("no overrides are in place") {
         beforeGroup {
           whenever(task.execute(any())) doReturn taskResult
           whenever(repository.retrievePipeline(message.executionId)) doReturn pipeline
@@ -142,16 +144,19 @@ class RunTaskHandlerSpec : Spek({
           handler.handle(message)
         }
 
-        it("emits a failure event") {
+        it("marks the task TERMINAL") {
           verify(queue).push(check<CompleteTask> {
             it.status shouldEqual TERMINAL
           })
         }
       }
 
-      context("the task should not fail the whole pipeline, only the branch") {
+      and("the task should not fail the whole pipeline, only the branch") {
         beforeGroup {
-          pipeline.stages.first().context["failPipeline"] = false
+          pipeline.stageByRef("1").apply {
+            context["failPipeline"] = false
+            context["continuePipeline"] = false
+          }
 
           whenever(task.execute(any())) doReturn taskResult
           whenever(repository.retrievePipeline(message.executionId)) doReturn pipeline
@@ -164,16 +169,19 @@ class RunTaskHandlerSpec : Spek({
           handler.handle(message)
         }
 
-        it("emits a failure event") {
+        it("marks the task STOPPED") {
           verify(queue).push(check<CompleteTask> {
             it.status shouldEqual STOPPED
           })
         }
       }
 
-      context("the task should allow the pipeline to proceed") {
+      and("the task should allow the pipeline to proceed") {
         beforeGroup {
-          pipeline.stages.first().context["continuePipeline"] = true
+          pipeline.stageByRef("1").apply {
+            context["failPipeline"] = false
+            context["continuePipeline"] = true
+          }
 
           whenever(task.execute(any())) doReturn taskResult
           whenever(repository.retrievePipeline(message.executionId)) doReturn pipeline
@@ -186,7 +194,7 @@ class RunTaskHandlerSpec : Spek({
           handler.handle(message)
         }
 
-        it("emits a failure event") {
+        it("marks the task FAILED_CONTINUE") {
           verify(queue).push(check<CompleteTask> {
             it.status shouldEqual FAILED_CONTINUE
           })

diff --git a/orca-queue/src/test/kotlin/com/netflix/spinnaker/orca/q/handler/StartStageHandlerSpec.kt b/orca-queue/src/test/kotlin/com/netflix/spinnaker/orca/q/handler/StartStageHandlerSpec.kt
@@ -34,6 +34,7 @@ import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository
 import com.netflix.spinnaker.orca.pipeline.util.ContextParameterProcessor
 import com.netflix.spinnaker.orca.q.*
 import com.netflix.spinnaker.orca.time.fixedClock
+import com.netflix.spinnaker.spek.and
 import com.nhaarman.mockito_kotlin.*
 import org.jetbrains.spek.api.Spek
 import org.jetbrains.spek.api.dsl.context
@@ -122,7 +123,7 @@ class StartStageHandlerSpec : Spek({
     }
 
     context("with no tasks") {
-      context("and no after stages") {
+      and("no after stages") {
         val pipeline = pipeline {
           application = "foo"
           stage {
@@ -163,7 +164,7 @@ class StartStageHandlerSpec : Spek({
         }
       }
 
-      context("and at least one after stage") {
+      and("at least one after stage") {
         val pipeline = pipeline {
           application = "foo"
           stage {

diff --git a/orca-queue/src/test/kotlin/com/netflix/spinnaker/spek/api.kt b/orca-queue/src/test/kotlin/com/netflix/spinnaker/spek/api.kt
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2017 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.spek
+
+import org.jetbrains.spek.api.dsl.SpecBody
+
+/**
+ * Creates a [group][SpecBody.group].
+ */
+fun SpecBody.and(description: String, body: SpecBody.() -> Unit) {
+  group("and $description", body = body)
+}
+
+/**
+ * Creates a [group][SpecBody.group].
+ */
+fun SpecBody.but(description: String, body: SpecBody.() -> Unit) {
+  group("but $description", body = body)
+}