Add support for Slack notifications #166
Conversation
|
Requires: #164 |
jlebon
force-pushed
the
pr/slack
branch
2 times, most recently
from
d899cd0
to
2f28e20
Compare
|
Rebased! |
There was a problem hiding this comment.
This mostly LGTM. My biggest concern here is that I think we should use IRC for this. While I understand that having both might not seem like a concern, I think it will just encourage conversations to happen in two places. I'd prefer for them to all happen in a public channel. (maybe #fcos-builds or #fcos-noc on freenode).
Regarding the code, I've made a few comments/suggestions. I was trying this out and couldn't quite get it to work. Even though the build had log messages indicating it was including the plugin I would get the following error and also notice the plugin wasn't installed when I looked at the Jenkins web interface:
No such DSL method 'slackSend'
| // declare this early so we can use it in Slack | ||
| def newBuildID | ||
|
|
||
| try { |
There was a problem hiding this comment.
let's indent this block for consistency
There was a problem hiding this comment.
This was deliberate. All the Jenkins scopes would make it very easy to end up with a huge indent that would make it harder to read and hack on.
I can do this if you'd really like, but would prefer not to. :)
There was a problem hiding this comment.
Right now we don't indent this block, but we do indent the catch and the finally, which is a bit odd. How about a compromise? options:
- We could add a comment to the closing brace (
# end try {}) - We could put the code within the try inside a function so that the try/catch/finally are all visible on a single screen of text.
Any other ideas?
There was a problem hiding this comment.
I did the first for now, but I like the idea of the second (and I think it's closer to the RHCOS pipeline too). Let's aim to work towards that.
Factoring it out is something we'll want to do anyway as part of unifying the pipelines.
How about: let's get Slack working for now, while simultaneously investigate the fedmsg -> IRC path? Once we get IRC working, I'm good with dropping the Slack notifications. |
jlebon
force-pushed
the
pr/slack
branch
2 times, most recently
from
3862735
to
3b90452
Compare
HACKING.md
Outdated
|
|
||
| You can obtain a token (Slack calls this the "Integration Token | ||
| Credential ID") when creating a new instance of the Jenkins CI app in | ||
| your Slack workspace. |
There was a problem hiding this comment.
is that a "Legacy token" as described here: https://api.slack.com/docs/token-types ?
There was a problem hiding this comment.
Ahh thanks. No it's not. It's not a token associated with a user scope. It's a bot token generated by the Jenkins Slack app.
There was a problem hiding this comment.
Yeah that wasn't obvious to me. I went to the slack web ui and tried to find a place to create a token. I think this extra bit of text helps the "gap". If there is a link to "how to create the app" that would help even more.
|
No objections to this, but I think this type of stuff can also make sense to be structured as a separate service which monitors the pipeline status externally. |
Yeah, I think this is where we're going with the fedmsg integration (see some discussions about this in coreos/fedora-coreos-tracker#198). Not just the pipeline, there's a bunch of services that need better monitoring (that's coreos/fedora-coreos-releng-automation#5). |
This is an easy way to nuke everything and restart from scratch, which I think we should do once in a while (of course, easier/cleaner would be to delete the whole namespace and recreate it, though we don't have that access here).
For easier selecting and deleting.
We need to add more monitoring on the pipeline. Otherwise it's going to be very easy to go red for a few days without noticing. My initial goal was to hook up to IRC, but the reality is that I think we'll need both Slack and IRC because many folks mostly live in Slack nowadays. (That said, I still fully intend to add IRC support via fedmsgs as discussed in coreos#41). Now, how this patch works is that we add two new Jenkins plugins: - configuration-as-code - slack The first one is used to configure the second one. More broadly, it's able to configure almost all of Jenkins and its plugins via YAML instead of dropping to XML, so we'll likely be leveraging it some more in the future. The actual configuration bit here is more or less the same as RHCOS, with some minor variations (we use secrets instead of defining the token at template generation time, and we just set a default channel instead of passing it through an env var) Of course, this is all still optional. The local developer workflow should still work fine.
|
Tested working! |
We need to add more monitoring on the pipeline. Otherwise it's going to
be very easy to go red for a few days without noticing.
My initial goal was to hook up to IRC, but the reality is that I think
we'll need both Slack and IRC because many folks mostly live in Slack
nowadays.
(That said, I still fully intend to add IRC support via fedmsgs as
discussed in #41).
Now, how this patch works is that we add two new Jenkins plugins:
The first one is used to configure the second one. More broadly, it's
able to configure almost all of Jenkins and its plugins via YAML instead
of dropping to XML, so we'll likely be leveraging it some more in the
future.
The actual configuration bit here is more or less the same as RHCOS,
with some minor variations (we use secrets instead of defining the token
at template generation time, and we just set a default channel instead
of passing it through an env var)
Of course, this is all still optional. The local developer workflow
should still work fine.