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(artifacts): Stop copying expectedArtifactIds to child pipelines #4404

Merged
merged 1 commit into from
Mar 2, 2023

Conversation

jervi
Copy link
Contributor

@jervi jervi commented Mar 2, 2023

This commit partially reverts #4397. It broke some pipelines for a team that had a CI stage that created more artifacts with the same name as the expected artifact from the original trigger, and then had a pipeline stage. The result was that the child pipeline was triggered with multiple artifacts with the same name (but different version numbers), and then failed to start because the artifact resolver matched multiple artifacts instead of exactly one. Turns out the changes in DependentPipelineStarter wasn't really needed to fix the issue that #4397 tried to solve, so I'm reverting them.

This commit partially reverts spinnaker#4397. It broke some pipelines for a team that had a CI stage that created more artifacts with the same name as the expected artifact from the original trigger, and then had a pipeline stage. The result was that the child pipeline was triggered with multiple artifacts with the same name (but different version numbers), and then failed to start because the artifact resolver matched multiple artifacts instead of exactly one.
Turns out the changes in DependentPipelineStarter wasn't really needed to fix the issue that spinnaker#4397 tried to solve, so I'm reverting them.
@jervi
Copy link
Contributor Author

jervi commented Mar 2, 2023

@dbyron-sf This is getting ridiculous, but I have another fix I need you to look at! Sorry 😅

image

jervi added a commit to jervi/spinnaker.io that referenced this pull request Mar 2, 2023
The feature that copied expected artifact from parent to child pipeline was reverted in https://spinnaker/orca/pull/4404. Removing it from the release notes as well.
jervi added a commit to jervi/spinnaker.io that referenced this pull request Mar 2, 2023
The feature that copied expected artifact from parent to child pipeline was reverted in spinnaker/orca#4404. Removing it from the release notes as well.
@dbyron-sf
Copy link
Contributor

This does make me wonder if there's a different kind of test that we could use to verify this behavior. I haven't looked to see if anything like this exists...do you know? Somewhere to define a pipeline that executes another pipeline, or something?

@dbyron-sf dbyron-sf added the ready to merge Approved and ready for merge label Mar 2, 2023
@mergify mergify bot added the auto merged Merged automatically by a bot label Mar 2, 2023
@mergify mergify bot merged commit df6d916 into spinnaker:master Mar 2, 2023
dbyron-sf pushed a commit to spinnaker/spinnaker.io that referenced this pull request Mar 2, 2023
The feature that copied expected artifact from parent to child pipeline was reverted in spinnaker/orca#4404. Removing it from the release notes as well.
@jervi
Copy link
Contributor Author

jervi commented Mar 3, 2023

This does make me wonder if there's a different kind of test that we could use to verify this behavior. I haven't looked to see if anything like this exists...do you know? Somewhere to define a pipeline that executes another pipeline, or something?

DependentPipelineStarterSpec more or less tests this. The issue was more a use case I didn't imagine. When I changed the test here to add a stage that produced an artifact, I was able to reproduce it.
But you are maybe thinking of some integration tests or something that tests this?

@jervi jervi deleted the another_artifact_fix branch March 3, 2023 09:13
@dbyron-sf
Copy link
Contributor

Yeah, I was thinking of more like an integration test, but the test here is good for me.

yugaa22 pushed a commit to OpsMx/orca-oes that referenced this pull request Jun 26, 2023
…pinnaker#4404)

This commit partially reverts spinnaker#4397. It broke some pipelines for a team that had a CI stage that created more artifacts with the same name as the expected artifact from the original trigger, and then had a pipeline stage. The result was that the child pipeline was triggered with multiple artifacts with the same name (but different version numbers), and then failed to start because the artifact resolver matched multiple artifacts instead of exactly one.
Turns out the changes in DependentPipelineStarter wasn't really needed to fix the issue that spinnaker#4397 tried to solve, so I'm reverting them.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants