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

Update push-chart workflow concurrency group #25431

Merged
merged 1 commit into from May 23, 2023

Conversation

chancez
Copy link
Contributor

@chancez chancez commented May 12, 2023

I think we're seeing unintended cancellations of this workflow because it's a workflow_run event in most cases and github.event.after might not be working as we expect.

First, add the event name into the concurrency group and then use a different ref based on the event type based on the git SHA the image chart will use for the image tags. This ensures we'll get one build for each image build but if we re-run the same job with the same parameters it'll cancel any previous invocations with for the same ref and event type.

I think we're seeing unintended cancellations of this workflow because
it's a workflow_run event in most cases and github.event.after might not
be working as we expect.

First, add the event name into the concurrency group and then use a
different ref based on the event type based on the git SHA the image
chart will use for the image tags. This ensures we'll get one build for
each image build but if we re-run the same job with the same parameters
it'll cancel any previous invocations with for the same ref and event
type.

Signed-off-by: Chance Zibolski <chance.zibolski@gmail.com>
@chancez chancez added area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. labels May 12, 2023
@chancez chancez requested a review from christarazi May 12, 2023 20:08
@chancez chancez requested a review from a team as a code owner May 12, 2023 20:08
@chancez chancez self-assigned this May 12, 2023
@chancez chancez requested a review from a team as a code owner May 12, 2023 20:08
@chancez chancez requested a review from brlbil May 12, 2023 20:08
Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

Logic seems reasonable to me at a high level, thanks for the fix!

Copy link
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

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

😇

@chancez chancez added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 22, 2023
@squeed squeed merged commit e49b16a into main May 23, 2023
46 checks passed
@squeed squeed deleted the pr/chancez/push_chart_concurrency_update branch May 23, 2023 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants