Update catalog item for playbooks #14073

syncrou · 2017-02-24T19:45:34Z

Created the update_catalog_item method
Queues a job to update the existing job template and run a refresh (async)
Calls ServiceTemplate#update_catalog_item to handle updating the resource actions etc.
If a new dialog name is passed in a new dialog is created.
If the id of the existing dialog is passed in, no changes are made to that dialog

https://www.pivotaltracker.com/story/show/139020599

syncrou · 2017-02-24T19:46:38Z

@miq-bot add_label enhancement providers/ansible_tower

@miq-bot assign @bzwei

cc - @jntullo @gmcculloug

miq-bot · 2017-02-24T19:47:14Z

@syncrou Cannot apply the following label because they are not recognized: enhancement providers/ansible_tower

syncrou · 2017-02-24T19:48:18Z

@miq-bot add_label enhancement, providers/ansible_tower

bzwei · 2017-02-27T19:43:57Z

app/models/service_template_ansible_playbook.rb

@@ -1,4 +1,12 @@
 class ServiceTemplateAnsiblePlaybook < ServiceTemplateGeneric
+  def self.job_template_class


should use a private constant

bzwei · 2017-02-27T19:47:52Z

app/models/service_template_ansible_playbook.rb

@@ -74,15 +72,15 @@ def self.create_job_templates(service_name, description, config_info, auth_user)
  def self.create_job_template(name, description, info, auth_user)
    tower, params = build_parameter_list(name, description, info)

-    task_id = ManageIQ::Providers::AnsibleTower::AutomationManager::ConfigurationScript.create_in_provider_queue(tower.id, params, auth_user)
+    task_id = job_template_class.create_in_provider_queue(tower.id, params, auth_user)


I think before the change the readability is better although the line looks longer.

bzwei · 2017-02-27T19:55:51Z

app/models/manageiq/providers/ansible_tower/shared/automation_manager/configuration_script.rb

@@ -33,6 +33,37 @@ def create_in_provider_queue(manager_id, params, auth_user = nil)

      MiqTask.generic_action_with_callback(task_opts, queue_opts)
    end
+
+    def update_in_provider(manager_id, params)


You can definitely combine update and create methods into one common method, same to the queue version.

The public methods you can still have create and update separated, but they redirect to a common private method.

bzwei · 2017-03-01T21:37:06Z

app/models/manageiq/providers/ansible_tower/shared/automation_manager/configuration_script.rb

+
+      # Get the record in our database
+      # TODO: This needs to be targeted refresh so it doesn't take too long
+      EmsRefresh.queue_refresh(manager, nil) if manager.authentication_status_ok?


EmsRefresh.queue_refresh(manager, nil, true)

bzwei · 2017-03-01T21:38:45Z

app/models/manageiq/providers/ansible_tower/shared/automation_manager/configuration_script.rb

+
+    private
+
+    def create_or_update_in_provider_queue(manager_id, params, auth_user)


def create_or_update_in_provider_queue(action, manager_id, params, auth_user)

bzwei · 2017-03-01T21:39:09Z

app/models/manageiq/providers/ansible_tower/shared/automation_manager/configuration_script.rb

+    end
+
+    def update_in_provider_queue(manager_id, params, auth_user = nil)
+      create_or_update_in_provider_queue(manager_id, params, auth_user)


create_or_update_in_provider_queue('update', manager_id, params, auth_user)

bzwei · 2017-03-01T21:39:26Z

app/models/manageiq/providers/ansible_tower/shared/automation_manager/configuration_script.rb

    def create_in_provider_queue(manager_id, params, auth_user = nil)
+      create_or_update_in_provider_queue(manager_id, params, auth_user)


create_or_update_in_provider_queue('create', manager_id, params, auth_user)

bzwei · 2017-03-01T21:40:22Z

app/models/service_template_ansible_playbook.rb

+
+      params[:manager_ref] = job_template(action).manager_ref
+
+      ManageIQ::Providers::AnsibleTower::AutomationManager::ConfigurationScript.update_in_provider_queue(tower.id, params, auth_user)


ManageIQ::Providers::EmbeddedAnsible::AutomationManager::ConfigurationScript, please update the same for creation.

miq-bot · 2017-03-03T19:46:41Z

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

miq-bot · 2017-03-10T19:37:00Z

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

bzwei · 2017-03-13T21:41:22Z

app/models/service_template_ansible_playbook.rb

-      config_info[action][:dialog] =
-        Dialog::AnsiblePlaybookService.new.create_dialog(config_info[:new_dialog_name], job_template)
-    end
+  def self.create_new_dialog(dialog_name, job_template)


we can make this a private instance method

bzwei · 2017-03-13T21:42:17Z

app/models/service_template_ansible_playbook.rb

@@ -44,6 +41,7 @@ def self.create_catalog_item(options, auth_user)
          next unless dialog_name

          job_template = enhanced_config.fetch_path(action, :configuration_template)
+
          new_dialog = Dialog::AnsiblePlaybookServiceDialog.create_dialog(dialog_name, job_template)


switch to use service_template.send(:create_new_dialog)

syncrou · 2017-03-14T20:16:22Z

app/models/service_template_ansible_playbook.rb

+    name = options[:name]
+    description = options[:description]
+    [:provision, :retirement, :reconfigure].each do |action|
+      next unless config_info[action] && config_info[action].keys.include?(:playbook_id)


@bzwei - Would there ever be a case where we would create a new Dialog without updating the JobTemplate?

yes, I think it is possible.

bzwei · 2017-03-14T21:06:01Z

@syncrou please take a look at #14328. It may have impact on how you update a catalog item too. Sorry for keeping you constantly update this PR because other PRs are merged.

syncrou · 2017-03-14T21:20:06Z

@bzwei - Nothing obvious is jumping out at me for #14328

My current changes (looking for a :playbook_id key) should handle the updated validation.

Thanks for the heads up.

bzwei · 2017-03-14T21:37:26Z

app/models/service_template_ansible_playbook.rb

+  end
+
+  def update_catalog_item(options, auth_user = nil)
+    config_info = validate_update_config_info(options)


Base's validate_update_config_info is not sufficient, we need to override it like this

def validate_update_config_info(options)` opts = super self.class.send(:validate_config_info, opts) end

bzwei · 2017-03-14T21:41:42Z

app/models/service_template_ansible_playbook.rb

+    [:provision, :retirement, :reconfigure].each do |action|
+      next unless config_info[action]
+      info = config_info[action]
+      self.class.send(:create_new_dialog, info[:new_dialog_name], job_template(action)) if info[:new_dialog_name]


create_new_dialog is now instance method that you can directly call.
Also just like create_catalog_item, we need to update options[:config_info][action][:dialog_id] to the newly created dialog id.

bzwei · 2017-03-14T21:48:18Z

spec/models/service_template_ansible_playbook_spec.rb

+      )
+      expect(service_template.dialogs.first.name).to eq catalog_item_options_three
+        .fetch_path(:config_info, :provision, :new_dialog_name)
+    end


Need to add expect(service_template.resource_actions.first) to contain newly created/updated job template.

bzwei · 2017-03-14T21:54:04Z

spec/models/service_template_ansible_playbook_spec.rb

+  describe '#update_catalog_item' do
+    it 'updates and returns the modified catalog item' do
+      service_template = prebuild_service_template
+      expect(described_class).to receive(:create_new_dialog)


I am wondering if you don't return a new dialog here how can you assert the dialog name in line 178.

bzwei · 2017-03-14T21:58:06Z

app/models/service_template_ansible_playbook.rb

+      next unless info.keys.include?(:playbook_id)
+      tower, params = self.class.send(:build_parameter_list, name, description, info)
+      params[:manager_ref] = job_template(action).manager_ref
+      ManageIQ::Providers::EmbeddedAnsible::AutomationManager::ConfigurationScript.update_in_provider_queue(tower.id, params, auth_user)


We need to handle the case for example initially there was no playbook assigned to retirement, but through the update now a new playbook is given. We will also need to update the job template in the corresponding resource action,

@bzwei - Per our conversations around this change - I will add the create_in_provider_queue behavior in a followup PR.

miq-bot · 2017-03-15T15:42:21Z

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

1. created the update_catalog_item method 2. Queues a job to update the existing job template and run a refresh (async) 3. Calls ServiceTemplate#update_catalog_item to handle updating the resource actions etc. 4. If a new dialog name is passed in a new dialog is created. 5. If the id of the existing dialog is passed in, no changes are made to that dialog https://www.pivotaltracker.com/story/show/139020599

https://www.pivotaltracker.com/story/show/139020599

create_in_provider_queue now handles both the create and update queuing https://www.pivotaltracker.com/story/show/139020599

https://www.pivotaltracker.com/story/show/139020599

There are cases where the :retirement key of the config will not include a playbook_id In these cases we need to skip creation of a job template as a playbook id is a requirement https://www.pivotaltracker.com/story/show/139020599

miq-bot · 2017-03-16T00:02:56Z

Checked commits syncrou/manageiq@870717e~...078c178 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 0 offenses detected
Everything looks good. ⭐

gmcculloug · 2017-03-16T01:15:18Z

Tested and verified this is working now. Thanks @syncrou.

miq-bot assigned bzwei Feb 24, 2017

gmcculloug added enhancement euwe/no lifecycle/services labels Feb 24, 2017

miq-bot added the dependencies label Feb 24, 2017

bzwei reviewed Feb 27, 2017

View reviewed changes

syncrou force-pushed the update_catalog_item_for_playbooks branch from a00f753 to 26badb7 Compare March 1, 2017 17:14

bzwei reviewed Mar 1, 2017

View reviewed changes

syncrou force-pushed the update_catalog_item_for_playbooks branch from bb88dad to 5798fa3 Compare March 1, 2017 21:48

bzwei approved these changes Mar 2, 2017

View reviewed changes

miq-bot added the unmergeable label Mar 3, 2017

bzwei force-pushed the update_catalog_item_for_playbooks branch 2 times, most recently from 1937cca to 11e5858 Compare March 3, 2017 19:51

miq-bot removed the unmergeable label Mar 3, 2017

syncrou force-pushed the update_catalog_item_for_playbooks branch 2 times, most recently from fb50d6e to d913343 Compare March 10, 2017 14:57

miq-bot added the unmergeable label Mar 10, 2017

syncrou force-pushed the update_catalog_item_for_playbooks branch from d913343 to 4747286 Compare March 10, 2017 20:01

miq-bot removed the unmergeable label Mar 10, 2017

syncrou force-pushed the update_catalog_item_for_playbooks branch from 4747286 to 10ea2dd Compare March 13, 2017 21:20

bzwei reviewed Mar 13, 2017

View reviewed changes

syncrou force-pushed the update_catalog_item_for_playbooks branch from 10ea2dd to 6dd6864 Compare March 14, 2017 20:11

syncrou commented Mar 14, 2017

View reviewed changes

syncrou force-pushed the update_catalog_item_for_playbooks branch from 6dd6864 to 0fbc44d Compare March 14, 2017 20:53

syncrou force-pushed the update_catalog_item_for_playbooks branch from 0fbc44d to ecb56b9 Compare March 14, 2017 20:58

syncrou force-pushed the update_catalog_item_for_playbooks branch from ecb56b9 to eaa0217 Compare March 14, 2017 21:15

bzwei reviewed Mar 14, 2017

View reviewed changes

miq-bot added the unmergeable label Mar 15, 2017

syncrou force-pushed the update_catalog_item_for_playbooks branch from eaa0217 to 629dfdf Compare March 15, 2017 19:22

syncrou added 6 commits March 15, 2017 15:28

Added specs for update_catalog_item

faa194e

https://www.pivotaltracker.com/story/show/139020599

Consolidate update_in_provider_queue

1762ad0

create_in_provider_queue now handles both the create and update queuing https://www.pivotaltracker.com/story/show/139020599

Refactor how job_template is looked up

ae34ebf

https://www.pivotaltracker.com/story/show/139020599

Add private create_or_update_in_provider_queue

2da6b9e

https://www.pivotaltracker.com/story/show/139020599

Use the Embedded Provider

ad3a676

https://www.pivotaltracker.com/story/show/139020599

syncrou force-pushed the update_catalog_item_for_playbooks branch from 629dfdf to 40d37bf Compare March 15, 2017 19:37

miq-bot removed the unmergeable label Mar 15, 2017

syncrou force-pushed the update_catalog_item_for_playbooks branch 2 times, most recently from 93b2fa6 to c7d63b7 Compare March 15, 2017 20:05

Validate an action includes playbook_id

078c178

There are cases where the :retirement key of the config will not include a playbook_id In these cases we need to skip creation of a job template as a playbook id is a requirement https://www.pivotaltracker.com/story/show/139020599

syncrou force-pushed the update_catalog_item_for_playbooks branch from c7d63b7 to 078c178 Compare March 15, 2017 23:58

gmcculloug merged commit 4ba4e43 into ManageIQ:master Mar 16, 2017

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

Fryguy added the providers/automation label Jan 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update catalog item for playbooks #14073

Update catalog item for playbooks #14073

syncrou commented Feb 24, 2017 •

edited

Loading

syncrou commented Feb 24, 2017

miq-bot commented Feb 24, 2017

syncrou commented Feb 24, 2017

bzwei Feb 27, 2017

bzwei Feb 27, 2017

bzwei Feb 27, 2017

bzwei Mar 1, 2017

bzwei Mar 1, 2017

bzwei Mar 1, 2017 •

edited

Loading

bzwei Mar 1, 2017 •

edited

Loading

bzwei Mar 1, 2017

miq-bot commented Mar 3, 2017

miq-bot commented Mar 10, 2017

bzwei Mar 13, 2017

bzwei Mar 13, 2017

syncrou Mar 14, 2017

bzwei Mar 14, 2017

bzwei commented Mar 14, 2017

syncrou commented Mar 14, 2017

bzwei Mar 14, 2017

bzwei Mar 14, 2017

bzwei Mar 14, 2017

bzwei Mar 14, 2017

bzwei Mar 14, 2017

syncrou Mar 15, 2017

miq-bot commented Mar 15, 2017

miq-bot commented Mar 16, 2017

gmcculloug commented Mar 16, 2017

		@@ -1,4 +1,12 @@
		class ServiceTemplateAnsiblePlaybook < ServiceTemplateGeneric
		def self.job_template_class


		private

		def create_or_update_in_provider_queue(manager_id, params, auth_user)

		def create_in_provider_queue(manager_id, params, auth_user = nil)
		create_or_update_in_provider_queue(manager_id, params, auth_user)


		params[:manager_ref] = job_template(action).manager_ref

		ManageIQ::Providers::AnsibleTower::AutomationManager::ConfigurationScript.update_in_provider_queue(tower.id, params, auth_user)

Update catalog item for playbooks #14073

Update catalog item for playbooks #14073

Conversation

syncrou commented Feb 24, 2017 • edited Loading

syncrou commented Feb 24, 2017

miq-bot commented Feb 24, 2017

syncrou commented Feb 24, 2017

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

bzwei Mar 1, 2017 • edited Loading

Choose a reason for hiding this comment

bzwei Mar 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Mar 3, 2017

miq-bot commented Mar 10, 2017

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

bzwei commented Mar 14, 2017

syncrou commented Mar 14, 2017

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

miq-bot commented Mar 15, 2017

miq-bot commented Mar 16, 2017

gmcculloug commented Mar 16, 2017

syncrou commented Feb 24, 2017 •

edited

Loading

bzwei Mar 1, 2017 •

edited

Loading

bzwei Mar 1, 2017 •

edited

Loading