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(rrb): Store RRB before-cleanup pipeline ID in the right context field #3239

Merged
merged 3 commits into from
Oct 21, 2019

Conversation

luispollo
Copy link
Contributor

@luispollo luispollo commented Oct 21, 2019

This is an addendum for #3224, which was causing Rolling Red/Black deployments with a "before cleanup" pipeline to fail as StartPipelineTask was looking for stage.context.pipeline but the pipeline ID was stored in stage.context.pipelineId here.

@marchello2000 and I search for other similar uses of pipelineId in the context and it looks like it's safe to make this change here.

@luispollo
Copy link
Contributor Author

@marchello2000 @jeyrschabu Please review.

@luispollo luispollo changed the title fix(rrb): Store RRB before cleanup pipeline ID in the right context field fix(rrb): Store RRB before-cleanup pipeline ID in the right context field Oct 21, 2019
@marchello2000
Copy link
Contributor

marchello2000 commented Oct 21, 2019

I am not a fan of the test for RRB, but still worthwhile adding this validation to:
https://github.com/spinnaker/orca/blob/master/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/strategies/RollingRedBlackStrategySpec.groovy#L159

otherwise, lgtm!

@jeyrschabu
Copy link
Contributor

LGTM

@marchello2000 marchello2000 added the ready to merge Approved and ready for merge label Oct 21, 2019
@mergify mergify bot merged commit ba249be into spinnaker:master Oct 21, 2019
@marchello2000 marchello2000 added the auto merged Merged automatically by a bot label Oct 23, 2019
@luispollo luispollo deleted the fix-rrb-pipeline-id branch October 25, 2019 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged Merged automatically by a bot ready to merge Approved and ready for merge target-release/1.17
Projects
None yet
4 participants