Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pipelines): an execution only starts one dep pipeline once #3070

Merged
merged 3 commits into from
Jul 31, 2019

Conversation

emjburns
Copy link
Contributor

If a pipeline that triggers a downstream pipeline is canceled, and that downstream pipeline doesn't limit concurrent pipelines, you can get two duplicate downstream pipeline executions.

This PR adds the parent execution id as a correlationId so that the downstream pipeline only run once.

Copy link
Contributor

@marchello2000 marchello2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, like @ajordens said, i am a bit confused how the race is avoided when two instances have the same correlation id almost at the same time... but that's a separate problem

@ajordens
Copy link
Contributor

There are a couple edge cases that this PR doesn't quite cover.

I think creating a correlation id consisting of the parentId + childId + timestamp will be unique enough to support (a) multiple child pipelines and (b) restarts.

@emjburns
Copy link
Contributor Author

@ajordens updated to fix this other cases. Thanks for pointing them out!

@@ -85,7 +85,8 @@ class DependentPipelineStarter implements ApplicationContextAware {
parentExecution : parentPipeline,
parentPipelineStageId: parentPipelineStageId,
parameters : [:],
strategy : suppliedParameters.strategy == true
strategy : suppliedParameters.strategy == true,
correlationId : parentPipeline.id + "_" + pipelineConfig.id + "_" + parentPipeline.startTime
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why pipelineConfig.id? would that change from execution to execution?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A parent pipeline might have multiple child pipelines that trigger on completion.
Start time accounts for a restart, since we change the start time if the pipeline restarts.

@emjburns emjburns merged commit ef666c6 into spinnaker:master Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants