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

Fixes #36166 - Revamp job hooks #791

Merged
merged 2 commits into from Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 42 additions & 0 deletions app/lib/actions/remote_execution/event_helpers.rb
@@ -0,0 +1,42 @@
module Actions
module RemoteExecution
module EventHelpers
module ClassEventHelpers
def event_states
[]
end

def event_names
event_states.map do |state|
event_name_base + '_' + event_name_suffix(state).to_s
end
end

def feature_job_event_names(label)
event_states.map do |state|
::Foreman::Observable.event_name_for("#{event_name_base}_#{label}_#{event_name_suffix(state)}")
end
end
end

def self.included(base)
base.extend ClassEventHelpers
end

def emit_event(execution_plan, hook)
return unless root_action?

payload = event_payload(execution_plan)
base = self.class.event_name_base
suffix = self.class.event_name_suffix(hook)
if input["job_features"]&.any?
Copy link
Contributor

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
}

Copy link
Contributor Author

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

Copy link
Contributor

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.

input['job_features'].each do |feature|
name = "#{base}_#{feature}_#{suffix}"
trigger_hook name, payload: payload
end
end
trigger_hook("#{base}_#{suffix}", payload: payload)
end
end
end
end
19 changes: 3 additions & 16 deletions app/lib/actions/remote_execution/run_host_job.rb
Expand Up @@ -5,8 +5,7 @@ class RunHostJob < Actions::EntryAction
include ::Actions::Helpers::WithDelegatedAction
include ::Actions::ObservableAction
include ::Actions::RemoteExecution::TemplateInvocationProgressLogging

execution_plan_hooks.use :emit_feature_event, :on => :success
include ::Actions::RemoteExecution::EventHelpers

middleware.do_not_use Dynflow::Middleware::Common::Transaction
middleware.use Actions::Middleware::HideSecrets
Expand Down Expand Up @@ -79,20 +78,8 @@ def finalize(*args)
end
end

def self.feature_job_event_name(label, suffix = :success)
::Foreman::Observable.event_name_for("#{::Actions::RemoteExecution::RunHostJob.event_name_base}_#{label}_#{::Actions::RemoteExecution::RunHostJob.event_name_suffix(suffix)}")
end

def emit_feature_event(execution_plan, hook = :success)
return unless root_action?

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
end
def self.event_states
[:success, :failure]
end

def secrets(host, job_invocation, provider)
Expand Down
12 changes: 7 additions & 5 deletions app/lib/actions/remote_execution/run_hosts_job.rb
Expand Up @@ -3,14 +3,15 @@ module RemoteExecution
class RunHostsJob < Actions::ActionWithSubPlans
include Actions::RecurringAction
include Actions::ObservableAction
include Actions::RemoteExecution::EventHelpers

middleware.use Actions::Middleware::BindJobInvocation
middleware.use Actions::Middleware::RecurringLogic
middleware.use Actions::Middleware::WatchDelegatedProxySubTasks

execution_plan_hooks.use :notify_on_success, :on => :success
execution_plan_hooks.use :notify_on_failure, :on => :failure
execution_plan_hooks.use :emit_event_running, :on => :running
execution_plan_hooks.use :emit_running_event, :on => :running

class CheckOnProxyActions; end

Expand All @@ -28,7 +29,8 @@ def delay(delay_options, job_invocation)
def plan(job_invocation)
job_invocation.task_group.save! if job_invocation.task_group.try(:new_record?)
task.add_missing_task_groups(job_invocation.task_group) if job_invocation.task_group
action_subject(job_invocation)
features = job_invocation.pattern_templates.flat_map { |t| t.remote_execution_features.pluck(:label) }.uniq
action_subject(job_invocation, job_features: features)
job_invocation.targeting.resolve_hosts! if job_invocation.targeting.dynamic? || !job_invocation.targeting.resolved?
set_up_concurrency_control job_invocation
input.update(:job_category => job_invocation.job_category)
Expand Down Expand Up @@ -158,11 +160,11 @@ def proxy_batch_size
input[:proxy_batch_size]
end

def self.event_names
super + [event_name_base + '_' + event_name_suffix('running')]
def self.event_states
[:success, :failure, :running]
end

def emit_event_running(plan)
def emit_running_event(plan)
emit_event(plan, :running)
end

Expand Down
2 changes: 1 addition & 1 deletion foreman_remote_execution.gemspec
Expand Up @@ -24,7 +24,7 @@ Gem::Specification.new do |s|

s.add_dependency 'deface'
s.add_dependency 'dynflow', '>= 1.0.2', '< 2.0.0'
s.add_dependency 'foreman-tasks', '>= 8.2.0'
s.add_dependency 'foreman-tasks', '>= 8.3.0'

s.add_development_dependency 'factory_bot_rails', '~> 4.8.0'
s.add_development_dependency 'rdoc'
Expand Down
6 changes: 4 additions & 2 deletions lib/foreman_remote_execution/engine.rb
Expand Up @@ -280,8 +280,10 @@ class Engine < ::Rails::Engine
::Dynflow::Action.descendants.select do |klass|
klass <= ::Actions::ObservableAction
end.map(&:namespaced_event_names) +
RemoteExecutionFeature.all.pluck(:label).map do |label|
::Actions::RemoteExecution::RunHostJob.feature_job_event_name(label)
RemoteExecutionFeature.all.pluck(:label).flat_map do |label|
[::Actions::RemoteExecution::RunHostJob, ::Actions::RemoteExecution::RunHostsJob].map do |klass|
klass.feature_job_event_names(label)
end
end
)
end
Expand Down
2 changes: 1 addition & 1 deletion test/unit/actions/run_hosts_job_test.rb
Expand Up @@ -39,7 +39,7 @@ def run_step_id
end
let(:action) do
create_action(Actions::RemoteExecution::RunHostsJob).tap do |action|
action.expects(:action_subject).with(job_invocation)
action.expects(:action_subject).with(job_invocation, job_features: [])
ForemanTasks::Task::DynflowTask.stubs(:where).returns(mock.tap { |m| m.stubs(:first! => task) })
end
end
Expand Down