Pass event_target and event_name to Ansible #14481

mkanoor · 2017-03-23T21:13:33Z

Pass event_target and event_name to Ansible playbooks. The event_source is passed in as a slug

https://www.pivotaltracker.com/n/projects/1937537/stories/141706313

lfu · 2017-03-24T13:59:49Z

app/models/miq_action.rb

-    service_template.provision_request(target_user(rec), options)
+    dialog_options = { :dialog_hosts => target_hosts(action, rec) }
+    request_options = { :extra_vars => { 'event_source' => rec.href_slug,
+                                         'event_name'   => inputs[:event].try(:name) },


Can we use event_type instead of event_name to be consistent with what is normally used?

@lfu the event_type doesn't have much information since it comes up as Default for most of the miq_event.

@mkanoor What I meant is the field event_name under extra_vars, not the column in event object.

@lfu The attribute is "name" so why would we call it "event_type"?

bzwei · 2017-03-24T13:55:58Z

app/models/service_template.rb

@@ -400,8 +400,8 @@ def self.create_from_options(options)
  end
  private_class_method :create_from_options

-  def provision_request(user, options = nil)
-    provision_workflow(user, options).submit_request
+  def provision_request(user, options = nil, request_options = nil)


def provision_request(user, dialog_options = {}, request_options = {}

My personal preference is a method name starting with a verb, e.g. def make_provision_request. A method name with noun is like a getter.

@bzwei
#13972 implemented this method to "get" a provision_request from a service template.

I understand. It is not a simple get (read only), it actually makes a request. Since it was introduced not long ago, we can refactor it. It is my recommendation, not a have_to_fix issue.

bzwei · 2017-03-24T13:56:35Z

app/models/service_template.rb

-  def provision_workflow(user, options = nil)
-    options ||= {}
-    ra_options = { :target => self, :initiator => options[:initiator] }
+  def provision_workflow(user, dialog_options = nil, request_options = nil)


Since this is a private method, we don't need default values for arguments.

@bzwei
provision_workflow is not a private method its going to be called by the REST API. This was done intentionally in #13972 so that the REST API can switch overt to using this. From the Rest API they first create a workflow and then submit the request at at later time. In our case when we order the service we want to create the request and immediately submit the request.

currently provision_workflow is private. If you want to promote it public, you need to move up the code.

bzwei · 2017-03-24T14:02:22Z

app/models/resource_action_workflow.rb

-    create_values_hash.tap { |value| value[:src_id] = @target.id }
+    create_values_hash.tap do |value|
+      value[:src_id] = @target.id
+      value[:request_options] = @request_options if @request_options.any?


value[:request_options] = request_options unless request_options.blank?
You don't need to line 14 @request_opitons = {}

@bzwei how would some one set the request_options. I have the attr_accessor method used to set the request _options, which saves it in an instance variable (@request_options)?

since you already added it as attr_accessor, you can reference it (get or set) through request_options without explicitly using @request_options

bzwei · 2017-03-24T14:08:24Z

app/models/service_ansible_playbook.rb

+  end
+
+  def request_options_extra_vars
+    ro = miq_request_task.options[:request_options] || {}


miq_request_task.options.fetch_path(:request_options, :extra_vars) || {}.
:extra_vars needs to be replaced with :manageiq_extra_vars or :extra_vars, 'manageiq' if we agree with the other comment.

bzwei · 2017-03-24T14:11:30Z

app/models/miq_action.rb

-                :initiator    => 'control' }
-    service_template.provision_request(target_user(rec), options)
+    dialog_options = { :dialog_hosts => target_hosts(action, rec) }
+    request_options = { :extra_vars => { 'event_source' => rec.href_slug,


{ :manageiq_extra_vars => { 'event_source' => rec.href_slug, ...
or
{ :extra_vars => { 'manageiq' => {'event_source' => rec.href_slug, ...

gmcculloug · 2017-03-24T17:25:14Z

@Fryguy @blomquisg Wondering if you have any thoughts on using the term "event_source" here.

I usually think of the event source as the provider, and the object the event is raised for as the target. But I can easily see how the term "source" could be valid here as well.

My only thought is that the event_stream uses the polymorphic target association to point to the object the event was raised for, so I tend to lean toward that term for consistency.

miq-bot · 2017-03-25T11:36:18Z

This pull request is not mergeable. Please rebase and repush.

mkanoor · 2017-03-27T15:52:57Z

@gmcculloug @Fryguy
Changed to event_target after discussion with @gmcculloug and @Fryguy

miq-bot · 2017-03-27T15:57:22Z

Checked commits mkanoor/manageiq@e7f0305~...abdc35e with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
7 files checked, 0 offenses detected
Everything looks good. 🏆

mkanoor · 2017-03-27T16:38:26Z

@gmcculloug Please review

gmcculloug · 2017-03-27T17:38:42Z

app/models/service_template.rb

+    provision_workflow(user, options, request_options).submit_request
+  end
+
+  def provision_workflow(user, dialog_options = nil, request_options = nil)


@mkanoor Was there a reason this method was moved out of the private method section?

@gmcculloug It was one of bill's feedback to make the function public since its needed by the REST API

mkanoor added the enhancement label Mar 23, 2017

mkanoor assigned gmcculloug and bzwei Mar 23, 2017

mkanoor requested a review from gmcculloug March 23, 2017 21:13

lfu reviewed Mar 24, 2017

View reviewed changes

bzwei reviewed Mar 24, 2017

View reviewed changes

mkanoor force-pushed the send_control_info branch from 67a0c86 to 8297933 Compare March 24, 2017 16:50

miq-bot added the unmergeable label Mar 25, 2017

Pass event_source and event_name to Ansible

e7f0305

mkanoor force-pushed the send_control_info branch from 8297933 to e7f0305 Compare March 27, 2017 14:37

miq-bot removed the unmergeable label Mar 27, 2017

Changed from event_source to event_target

abdc35e

mkanoor changed the title ~~Pass event_source and event_name to Ansible~~ Pass event_target and event_name to Ansible Mar 27, 2017

abellotti mentioned this pull request Mar 27, 2017

Eliminate Api::ServiceTemplateWorkflow service #14501

Merged

gmcculloug reviewed Mar 27, 2017

View reviewed changes

gmcculloug merged commit 098d59f into ManageIQ:master Mar 27, 2017

gmcculloug added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 27, 2017

gmcculloug added core/policy providers/ansible_tower lifecycle/services labels Mar 27, 2017

Fryguy added the providers/automation label Jan 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass event_target and event_name to Ansible #14481

Pass event_target and event_name to Ansible #14481

mkanoor commented Mar 23, 2017 •

edited

Loading

lfu Mar 24, 2017

mkanoor Mar 24, 2017

lfu Mar 24, 2017

mkanoor Mar 24, 2017

bzwei Mar 24, 2017

mkanoor Mar 24, 2017

bzwei Mar 24, 2017

bzwei Mar 24, 2017

mkanoor Mar 24, 2017

bzwei Mar 24, 2017

bzwei Mar 24, 2017

mkanoor Mar 24, 2017

bzwei Mar 24, 2017

bzwei Mar 24, 2017

bzwei Mar 24, 2017

gmcculloug commented Mar 24, 2017

miq-bot commented Mar 25, 2017

mkanoor commented Mar 27, 2017

miq-bot commented Mar 27, 2017

mkanoor commented Mar 27, 2017

gmcculloug Mar 27, 2017

mkanoor Mar 27, 2017

Pass event_target and event_name to Ansible #14481

Pass event_target and event_name to Ansible #14481

Conversation

mkanoor commented Mar 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmcculloug commented Mar 24, 2017

miq-bot commented Mar 25, 2017

mkanoor commented Mar 27, 2017

miq-bot commented Mar 27, 2017

mkanoor commented Mar 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkanoor commented Mar 23, 2017 •

edited

Loading