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

🤖 Notify of master failures via slack #1616

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

Itxaka
Copy link
Member

@Itxaka Itxaka commented Jul 18, 2023

What this PR does / why we need it:

This introduces a way for the main branch to notify in the slack channel
when failures happen on the main branch. Currently we have no visibility
over what happens on the main branhc unless we check manually. And as we
are moving more jobs to be run only in the main branch we need to keep
up to date on those failures.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

@Itxaka Itxaka force-pushed the notify_slack_main_failures branch 5 times, most recently from 38046aa to ba29ebc Compare July 19, 2023 07:25
@Itxaka Itxaka changed the title Test 🤖 Notify of main failures via slack Jul 19, 2023
@Itxaka Itxaka requested a review from a team July 19, 2023 07:26
@Itxaka Itxaka marked this pull request as ready for review July 19, 2023 07:26
@Itxaka Itxaka force-pushed the notify_slack_main_failures branch from ba29ebc to 1f1586f Compare July 19, 2023 07:44
Copy link
Member

@mauromorales mauromorales left a comment

Choose a reason for hiding this comment

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

I think the best would be to add a final job that depends on all of these jobs. I haven't tried it but I think you could ensure it always runs by using if: always() and then in the part that does the actual notification you can validate if any of the previous jobs failed by doing if: failure()

https://docs.github.com/en/actions/learn-github-actions/expressions#always

https://docs.github.com/en/actions/learn-github-actions/expressions#failure

@Itxaka
Copy link
Member Author

Itxaka commented Jul 19, 2023

I think the best would be to add a final job that depends on all of these jobs. I haven't tried it but I think you could ensure it always runs by using if: always() and then in the part that does the actual notification you can validate if any of the previous jobs failed by doing if: failure()

docs.github.com/en/actions/learn-github-actions/expressions#always

docs.github.com/en/actions/learn-github-actions/expressions#failure

But im wondering, as if you depend on a previous job, and that job fails, your dependant job would not even trigger no? Even if you have a failure() check, that is only checked one the job is started, and it wont start.

Let me create a test repo with this to see if it actually works like that.

@Itxaka
Copy link
Member Author

Itxaka commented Jul 19, 2023

I think the best would be to add a final job that depends on all of these jobs. I haven't tried it but I think you could ensure it always runs by using if: always() and then in the part that does the actual notification you can validate if any of the previous jobs failed by doing if: failure()
docs.github.com/en/actions/learn-github-actions/expressions#always
docs.github.com/en/actions/learn-github-actions/expressions#failure

But im wondering, as if you depend on a previous job, and that job fails, your dependant job would not even trigger no? Even if you have a failure() check, that is only checked one the job is started, and it wont start.

Let me create a test repo with this to see if it actually works like that.

shiiiiet, it seems to actually work!

https://github.com/Itxaka/test-ci-depends/actions/runs/5597106674

@mauromorales
Copy link
Member

fantastic!

@Itxaka
Copy link
Member Author

Itxaka commented Jul 19, 2023

fantastic!

ah yes, the problem I guess its how to depend on everything

Like if i depend on core and qemu-acceptance test and core fails, will it trigger? Then that means we still can not notify when building is broken as it wont trigger the notify job as the dependent jobs havent triggered

Gonna test that as well.

@Itxaka
Copy link
Member Author

Itxaka commented Jul 19, 2023

fantastic!

ah yes, the problem I guess its how to depend on everything

Like if i depend on core and qemu-acceptance test and core fails, will it trigger? Then that means we still can not notify when building is broken as it wont trigger the notify job as the dependent jobs havent triggered

Gonna test that as well.

wow, this actually works: https://github.com/Itxaka/test-ci-depends/actions/runs/5597190257

Im sure it has changed as I tried the same thing a couple of years ago and it was a no-go. NICE

This introduces a way for the main branch to notify in the slack channel
when failures happen on the main branch. Currently we have no visibility
over what happens on the main branhc unless we check manually. And as we
are moving more jobs to be run only in the main branch we need to keep
up to date on those failures.

Signed-off-by: Itxaka <itxaka.garcia@spectrocloud.com>
@Itxaka Itxaka force-pushed the notify_slack_main_failures branch from 1f1586f to cef8c08 Compare July 19, 2023 09:09
@Itxaka Itxaka requested review from mauromorales and a team July 19, 2023 09:09
@Itxaka Itxaka changed the title 🤖 Notify of main failures via slack 🤖 Notify of master failures via slack Jul 19, 2023
mauromorales
mauromorales previously approved these changes Jul 19, 2023
Copy link
Member

@mauromorales mauromorales left a comment

Choose a reason for hiding this comment

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

Very cool! It's only missing to alwasy put a random gif 🙊

@Itxaka
Copy link
Member Author

Itxaka commented Jul 19, 2023

Very cool! It's only missing to alwasy put a random gif speak_no_evil

yeah... not too sure about "random" gifs. I dont want it to post gore or something racist into the chat....internet is evil!

@Itxaka Itxaka requested review from mauromorales and a team July 19, 2023 11:19
mauromorales
mauromorales previously approved these changes Jul 19, 2023
Copy link
Member

@mauromorales mauromorales left a comment

Choose a reason for hiding this comment

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

lgtm

@Itxaka
Copy link
Member Author

Itxaka commented Jul 19, 2023

Fixed the lint and pushing as it wont affect PR tests!

Signed-off-by: Itxaka <itxaka.garcia@spectrocloud.com>
@Itxaka Itxaka merged commit c84e074 into kairos-io:master Jul 19, 2023
6 of 17 checks passed
@Itxaka Itxaka deleted the notify_slack_main_failures branch July 19, 2023 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants