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
Fixes #36166 - Revamp job hooks #791
Conversation
954b183
to
c94b00c
Compare
Wanted to try this on my local foreman-3.1, so I backported the fix onto 5.0.6 and built it.
|
Considering I'm not touching any frontend bits here, I'd say it is unrelated, but who knows |
end | ||
|
||
def emit_event(execution_plan, hook) | ||
raise "HALT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, that was just to see the hooks are actually getting called, fixed.
c94b00c
to
3717b5f
Compare
3717b5f
to
6750648
Compare
My current hypothesis is that simply the number of events is somehow to much for the |
OK, so apparently it also happens on forklift with latest foreman_webhooks-Plugin. I should probably open an issue for foreman_webhooks, even though the problem seems to lay deeper 🤔
|
theforeman/foreman_webhooks#56 should fix that. |
def emit_event(execution_plan, hook) | ||
return unless root_action? | ||
|
||
if input["job_features"]&.any? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tested it again on my forklift and had the problem, that the Event was not triggered, because there were no job_features present:
{
"host": {
"id": 1,
"name": "centos8-katello-devel-stable.example.com"
},
"job_category": "Commands",
"description": "Run uname -a",
"job_invocation_id": 1,
"job_features": [],
"proxy_id": 1,
"delegated_action_id": 2,
"with_event_logging": true,
"locale": "de",
"current_request_id": "8c551231-c6d4-49fa-9864-845ad758a0ba",
"current_timezone": "Europe/Berlin",
"current_user_id": 4,
"current_organization_id": 1,
"current_location_id": 2
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did you trigger the job?
Edit: Wait, what are we even doing here
Edit 2: Oh, this, I'll take a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have to trigger the event "#{self.class.event_name_base}_#{self.class.event_name_suffix(hook)}"
if there are no features defined, which will then mean triggering the 'generic' actions.remote_execution.run_hosts_job_(failed|success|running)
event.
|
||
if input["job_features"]&.any? | ||
payload = event_payload(execution_plan) | ||
input['job_features'].each do |feature| | ||
name = "#{self.class.event_name_base}_#{feature}_#{self.class.event_name_suffix(hook)}" | ||
trigger_hook name, payload: payload | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following change sends 'Host(s) Job' event, if no features are specified.
We could also always trigger that event since 🤔
if input["job_features"]&.any? | |
payload = event_payload(execution_plan) | |
input['job_features'].each do |feature| | |
name = "#{self.class.event_name_base}_#{feature}_#{self.class.event_name_suffix(hook)}" | |
trigger_hook name, payload: payload | |
end | |
payload = event_payload(execution_plan) | |
if input["job_features"]&.any? | |
input['job_features'].each do |feature| | |
name = "#{self.class.event_name_base}_#{feature}_#{self.class.event_name_suffix(hook)}" | |
trigger_hook name, payload: payload | |
end | |
else | |
name = "#{self.class.event_name_base}_#{self.class.event_name_suffix(hook)}" | |
trigger_hook name, payload: payload | |
end |
6750648
to
a3869bb
Compare
@m-bucher Hi, could you please check this out once more? |
@adamruzicka sorry for the delay 😇 The latest patch works for me 👍 |
Per host in job now have per feature events when the action succeeds or fails. Per job now have general running/stopped/failed as well as per feature running/stopped/failed events.
a3869bb
to
dd72122
Compare
Sorry that I keep changing this all the time, but I managed to remove some of the code by reusing things which were added in tasks recently. I mapped multiple features to a single template and then run it. Excerpts from logs:
On failure
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still works as far as I can say so 👍
But you might want to merge it eventually 😉
Thank you @m-bucher ! |
Per host in job now have per feature events when the action succeeds or fails.
Per job now have general running/stopped/failed as well as per feature running/stopped/failed events. Note, both the generic and per-feature are emitted.