Skip to content

Commit

Permalink
fix(front50): Avoid canceling an already canceled child pipeline (#1779)
Browse files Browse the repository at this point in the history
This PR also propagates some details about the particular stage that
failed such that `deck` can render an appropriate link.
  • Loading branch information
ajordens authored Nov 7, 2017
1 parent 6f09711 commit 9e0d5b4
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,14 @@ class RollingRedBlackStrategy implements Strategy, ApplicationContextAware {
source : source,
targetLocation: cleanupConfig.location,
scalePct : p,
pinCapacity : p < 100 // if p = 100, capacity should be unpinned
pinCapacity : p < 100, // if p = 100, capacity should be unpinned,
useNameAsLabel: true // hint to deck that it should _not_ override the name
]

def resizeStage = newStage(
stage.execution,
resizeServerGroupStage.type,
"Grow to $p% Desired Size",
"Grow to $p% of Desired Size",
resizeContext,
stage,
SyntheticStageOwner.STAGE_AFTER
Expand Down Expand Up @@ -182,6 +183,7 @@ class RollingRedBlackStrategy implements Strategy, ApplicationContextAware {
]

def pipelineContext = [
application : stageData.pipelineBeforeCleanup.application,
pipelineApplication: stageData.pipelineBeforeCleanup.application,
pipelineId : stageData.pipelineBeforeCleanup.pipelineId,
pipelineParameters : [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
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;
import com.netflix.spinnaker.orca.pipeline.model.Stage;
import com.netflix.spinnaker.orca.pipeline.model.Task;
import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository;
Expand Down Expand Up @@ -84,8 +85,11 @@ public CancellableStage.Result cancel(Stage stage) {
if (executionRepository == null) {
log.error(format("Stage %s could not be canceled w/o front50 enabled. Please set 'front50.enabled: true' in your orca config.", readableStageDetails));
} else {
// flag the child pipeline as canceled (actual cancellation will happen asynchronously)
executionRepository.cancel(executionId, "parent pipeline", null);
Pipeline childPipeline = executionRepository.retrievePipeline(executionId);
if (!childPipeline.isCanceled()) {
// flag the child pipeline as canceled (actual cancellation will happen asynchronously)
executionRepository.cancel(executionId, "parent pipeline", null);
}
}
}
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ class MonitorPipelineTask implements OverridableTimeoutRetryableTask {

if (childPipeline.status.halt) {
// indicates a failure of some sort
List<String> errors = childPipeline.stages
.findAll { s -> s.status == ExecutionStatus.TERMINAL }
def terminalStages = childPipeline.stages.findAll { s -> s.status == ExecutionStatus.TERMINAL }
List<String> errors = terminalStages
.findResults { s ->
if (s.context["exception"]?.details) {
return [(s.context["exception"].details.errors ?: s.context["exception"].details.error)]
Expand All @@ -66,11 +66,28 @@ class MonitorPipelineTask implements OverridableTimeoutRetryableTask {
}
}
.flatten()
Map context = [status: childPipeline.status]

def exceptionDetails = [:]
if (errors) {
context.exception = [details: [errors: errors]]
exceptionDetails.details = [
errors: errors
]
}

def haltingStage = terminalStages.find { it.status.halt }
if (haltingStage) {
exceptionDetails.source = [
executionId: childPipeline.id,
stageId : haltingStage.id,
stageName : haltingStage.name,
stageIndex : childPipeline.stages.indexOf(haltingStage)
]
}
return new TaskResult(ExecutionStatus.TERMINAL, context)

return new TaskResult(ExecutionStatus.TERMINAL, [
status : childPipeline.status,
exception: exceptionDetails
])
}

return new TaskResult(ExecutionStatus.RUNNING, [status: childPipeline.status])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,27 @@ class PipelineStageSpec extends Specification {
def pipelineStage = new PipelineStage(executionRepository: executionRepository)

@Unroll
def "should cancel child pipeline (if started)"() {
def "should cancel child pipeline (if started and not already canceled)"() {
given:
def childPipeline = new Pipeline("childPipeline")
childPipeline.canceled = childPipelineIsCanceled
def stage = new Stage<>(new Pipeline("orca"), "pipeline", stageContext)
when:
pipelineStage.cancel(stage)
then:
invocations * executionRepository.cancel(stageContext.executionId, "parent pipeline", null)
(shouldCancel ? 1 : 0) * executionRepository.cancel(stageContext.executionId, "parent pipeline", null)
(stageContext.executionId ? 1 : 0) * executionRepository.retrievePipeline(stageContext.executionId) >> {
return childPipeline
}
0 * _
where:
stageContext || invocations
[:] || 0 // child pipeline has not started
[executionId: "sub-pipeline-id"] || 1 // child pipeline has started and should cancel
stageContext || childPipelineIsCanceled || shouldCancel
[:] || false || false // child pipeline has not started
[executionId: "sub-pipeline-id"] || false || true // child pipeline has started and should cancel
[executionId: "sub-pipeline-id"] || true || false // child pipeline has already been canceled
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,25 @@ class MonitorPipelineTaskSpec extends Specification {

when:
def result = task.execute(stage)
def exception = result.context.exception

then:
result.context.exception == [details: [errors: [
"Exception in child pipeline stage (some child: a pipeline): Some error",
"Exception in child pipeline stage (some child: pipeline): Some other error",
"Exception in child pipeline stage (some child: deploy): task failed, no exception",
"Exception in child pipeline stage (some child: deploy): task had exception"
]]]
exception == [
details: [
errors: [
"Exception in child pipeline stage (some child: a pipeline): Some error",
"Exception in child pipeline stage (some child: pipeline): Some other error",
"Exception in child pipeline stage (some child: deploy): task failed, no exception",
"Exception in child pipeline stage (some child: deploy): task had exception"
]
],
// source should reference the _first_ halted stage in the child pipeline
source: [
executionId: pipeline.id,
stageId: pipeline.stages[1].id,
stageName: "a pipeline",
stageIndex: 1
]
]
}
}

0 comments on commit 9e0d5b4

Please sign in to comment.