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

ci: add slack notification to GH actions #16218

Merged
merged 1 commit into from May 19, 2021
Merged

Conversation

nebril
Copy link
Member

@nebril nebril commented May 19, 2021

No description provided.

Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
@nebril nebril requested review from a team as code owners May 19, 2021 10:08
@nebril nebril requested a review from nbusseneau May 19, 2021 10:08
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 19, 2021
@nebril nebril requested a review from aanm May 19, 2021 10:08
@nebril nebril added the release-note/ci This PR makes changes to the CI. label May 19, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 19, 2021
@nebril
Copy link
Member Author

nebril commented May 19, 2021

This is how it looked for test PR:
image

#16219

All messages that we are going to get from this PR are going to be failures, the passing build from screenshot above is just a test.

Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

Why does it matter if the test was cancelled?

@nebril
Copy link
Member Author

nebril commented May 19, 2021

@aanm cancellation can be caused by a timeout, which will allow us to investigate whether the timeout should be increased, or something else causing timeouts started happening.

Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

This is great 🚀

Besides the cancelled() issue mentioned below, I'm a bit concerned about the number of notifications we will receive though. Where are the notifications sent? Will we receive notifications for every single check failing, in every single pull request?

.github/workflows/aks.yaml Show resolved Hide resolved
.github/workflows/eks.yaml Show resolved Hide resolved
@nebril nebril added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 19, 2021
@nebril nebril merged commit c132e65 into master May 19, 2021
@nebril nebril deleted the pr/nebril/actions-ci-slack branch May 19, 2021 15:14
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.8 May 19, 2021
This was referenced May 21, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.8 May 21, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.8 May 27, 2021
@@ -230,3 +230,12 @@ jobs:
description: Connectivity test cancelled
state: pending
target_url: ${{ env.check_url }}

- name: Send slack notification
if: ${{ (cancelled() || failure()) && (github.event_name == 'schedule' || github.event_name == 'push') }}
Copy link
Member

Choose a reason for hiding this comment

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

In cilium-cli, this was changed to !success(). Should we also make that consistent here as well? cilium/cilium-cli#237 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
No open projects
1.9.8
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

6 participants