Migrate AnsibleTower ConfigurationManager to AutomationManager #13630

jameswnl · 2017-01-23T21:41:19Z

search and replace, plus some new AutomationManagers.

jameswnl · 2017-01-23T21:41:53Z

@miq-bot add_labels wip, providers/anisble_tower

miq-bot · 2017-01-23T21:42:17Z

@jameswnl Cannot apply the following label because they are not recognized: providers/anisble_tower

jameswnl · 2017-01-23T21:53:27Z

@miq-bot add_label providers/ansible_tower

chrisarcand · 2017-01-24T17:54:10Z

spec/models/manageiq/providers/automation_manager_spec.rb

+describe ManageIQ::Providers::AutomationManager do
+  let(:provider)           { FactoryGirl.build(:provider) }
+  let(:automation_manager) { FactoryGirl.build(:automation_manager, :provider => provider) }
+end


@chrisarcand Thanks, but this 🍖 is still very raw, not ready for consumption yet 😄. Way too many broken cases. Actively updated...

👍

🙏 Please squash your commits when it's ready, this is quite a many rough commits to follow in history later on.

☠️ Don't look further.... dangerous to your eyes...

AnsibleTower::AutomationManager https://www.pivotaltracker.com/story/show/138063313

miq-bot · 2017-01-25T17:01:23Z

Some comments on commits jameswnl/manageiq@2f0ef1a~...af2c269

spec/models/manageiq/providers/ansible_tower/automation_manager/configuration_script_spec.rb

⚠️ - 38 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

spec/models/manageiq/providers/ansible_tower/automation_manager/job_spec.rb

⚠️ - 50 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.
⚠️ - 69 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.
⚠️ - 98 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

spec/models/manageiq/providers/ansible_tower/automation_manager/refresh_parser_spec.rb

⚠️ - 22 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

spec/models/service_ansible_tower_spec.rb

⚠️ - 77 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

spec/vcr_cassettes/manageiq/providers/ansible_tower/automation_manager/refresher.yml

miq-bot · 2017-01-25T17:02:20Z

Checked commits jameswnl/manageiq@2f0ef1a~...af2c269 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
62 files checked, 17 offenses detected

app/models/manageiq/providers/ansible_tower/automation_manager/job.rb

❗ - Line 48, Col 5 - Style/MultilineIfModifier - Favor a normal if-statement over a modifier clause in a multiline statement.

app/models/manageiq/providers/ansible_tower/automation_manager/refresh_parser.rb

❗ - Line 32, Col 11 - Style/AccessorMethodName - Do not prefix reader method names with get_.
❗ - Line 37, Col 11 - Style/AccessorMethodName - Do not prefix reader method names with get_.
❗ - Line 42, Col 11 - Style/AccessorMethodName - Do not prefix reader method names with get_.
❗ - Line 77, Col 17 - Rails/FindBy - Use find_by instead of where.first.

app/models/manageiq/providers/ansible_tower/automation_manager/refresher.rb

❗ - Line 8, Col 13 - Style/CommentAnnotation - Annotation keywords like TODO should be all upper case, followed by a colon, and a space, then a note describing the problem.

spec/factories/ext_management_system.rb

❗ - Line 335, Col 42 - Style/WordArray - Use %w or %W for an array of words.

spec/lib/miq_automation_engine/service_methods/miq_ae_service_manageiq-providers-ansible_tower-automation_manager-playbook_spec.rb

❗ - Line 1, Col 1 - Style/FileName - The name of this source file (miq_ae_service_manageiq-providers-ansible_tower-automation_manager-playbook_spec.rb) should use snake_case.

spec/lib/miq_automation_engine/service_models/miq_ae_service_manageiq-providers-ansible_tower-automation_manager-configuration_script_spec.rb

❗ - Line 1, Col 1 - Style/FileName - The name of this source file (miq_ae_service_manageiq-providers-ansible_tower-automation_manager-configuration_script_spec.rb) should use snake_case.

spec/lib/miq_automation_engine/service_models/miq_ae_service_manageiq-providers-ansible_tower-automation_manager-configured_system_spec.rb

❗ - Line 1, Col 1 - Style/FileName - The name of this source file (miq_ae_service_manageiq-providers-ansible_tower-automation_manager-configured_system_spec.rb) should use snake_case.

spec/lib/miq_automation_engine/service_models/miq_ae_service_manageiq-providers-ansible_tower-automation_manager_spec.rb

❗ - Line 1, Col 1 - Style/FileName - The name of this source file (miq_ae_service_manageiq-providers-ansible_tower-automation_manager_spec.rb) should use snake_case.

spec/lib/miq_automation_engine/service_models/miq_ae_service_manageiq-providers-ansible_tower-provider_spec.rb

❗ - Line 1, Col 1 - Style/FileName - The name of this source file (miq_ae_service_manageiq-providers-ansible_tower-provider_spec.rb) should use snake_case.

spec/lib/miq_automation_engine/service_models/miq_ae_service_manageiq-providers-automation_manager-inventory_group_spec.rb

❗ - Line 1, Col 1 - Style/FileName - The name of this source file (miq_ae_service_manageiq-providers-automation_manager-inventory_group_spec.rb) should use snake_case.

spec/lib/miq_automation_engine/service_models/miq_ae_service_manageiq-providers-automation_manager-inventory_root_group_spec.rb

❗ - Line 1, Col 1 - Style/FileName - The name of this source file (miq_ae_service_manageiq-providers-automation_manager-inventory_root_group_spec.rb) should use snake_case.

spec/models/manageiq/providers/ansible_tower/automation_manager/refresh_parser_spec.rb

❗ - Line 10, Col 26 - Style/MultilineMethodCallBraceLayout - Closing method call brace must be on the same line as the last argument when opening brace is on the same line as the first argument.
❗ - Line 11, Col 11 - Style/MultilineMethodCallBraceLayout - Closing method call brace must be on the same line as the last argument when opening brace is on the same line as the first argument.

spec/models/manageiq/providers/ansible_tower/automation_manager/refresher_spec.rb

❗ - Line 11, Col 23 - Style/MultilineMethodCallBraceLayout - Closing method call brace must be on the same line as the last argument when opening brace is on the same line as the first argument.

Fryguy · 2017-01-25T20:39:43Z

app/models/manageiq/providers/ansible_tower/automation_manager/refresh_worker.rb

+  end
+
+  def self.settings_name
+    :ems_refresh_worker_ansible_tower_automation


This implies a Settings migration is needed, as well as a change to settings.yml

Follow up PR?

Yeah, just to unblock others, but it must be the next PR so we don't forget. (Normally I'd say "heck no" 😉 )

Fryguy · 2017-01-25T20:43:03Z

app/models/manageiq/providers/configuration_manager.rb

  has_many :configured_systems,           :dependent => :destroy, :foreign_key => "manager_id"
  has_many :configuration_profiles,       :dependent => :destroy, :foreign_key => "manager_id"
  has_many :configuration_scripts,        :dependent => :destroy, :foreign_key => "manager_id"
-  has_many :inventory_groups,             :dependent => :destroy, :foreign_key => "ems_id", :inverse_of => :manager


@bdunne Should this stay in ConfigurationManager? That is, would any config manager need it, and if not, why was it here in the base to begin with?

Let's put back in a follow up PR if necessary.

It was introduced for Ansible Tower with the thought that it would be reused in Foreman (but that hasn't been done yet).

Fryguy · 2017-01-25T20:44:01Z

config/dictionary_strings.rb

@@ -1448,13 +1448,13 @@
 _("Provider")
 # TRANSLATORS: en.yml key: dictionary.model.ManageIQ::Providers::BaseManager (plural form)
 _("Providers")
-# TRANSLATORS: en.yml key: dictionary.model.ManageIQ::Providers::AnsibleTower::ConfigurationManager


dictionary strings and .po/.pot files should not be in this PR. @mzazrivec will rebuild them for zanata afterwards.

Fryguy · 2017-01-25T20:44:25Z

config/permissions.tmpl.yml

@@ -14,13 +14,14 @@
 - :sto
 - :svc
 - :vi
- ems-type:ansible_tower_configuration
+- ems-type:ansible_tower_automation


Note to @simaishi for downstream.

Fryguy · 2017-01-25T20:44:46Z

config/settings.yml

@@ -110,7 +110,7 @@
    :purge_window_size: 1000
 :ems_refresh:
  :capture_vm_created_on_date: false
-  :ansible_tower_configuration:
+  :ansible_tower_automation:


yeah, data migration is necessary for both of these changes to settings.yml

Follow up PR as above.

Fryguy · 2017-01-25T20:51:10Z

...ervice_models/miq_ae_service_manageiq-providers-ansible_tower-automation_manager-playbook.rb

@@ -0,0 +1,5 @@
+module MiqAeMethodService
+  class MiqAeServiceManageIQ_Providers_AnsibleTower_AutomationManager_Playbook <
+      MiqAeServiceConfigurationScriptPayload


I'm not a fan of breaking the line...just keep it one line (We turned off the 120 char rule :P )

Also can be a separate PR.

Fryguy · 2017-01-25T20:53:03Z

locale/en.yml

-      ManageIQ::Providers::AnsibleTower::ConfigurationManager:              Configuration Manager (Ansible Tower)
-      ManageIQ::Providers::AnsibleTower::ConfigurationManager::ConfigurationScript: Job Template (Ansible Tower)
+      ManageIQ::Providers::AnsibleTower::AutomationManager:                 Automation Manager (Ansible Tower)
+      ManageIQ::Providers::AnsibleTower::AutomationManager::ConfigurationScript: Job Template (Ansible Tower)


@mzazrivec Should these kinds of changes stay in this PR, or would you prefer they were generated by you separately?

Fryguy · 2017-01-25T20:53:34Z

product/views/ManageIQ_Providers_AnsibleTower_ConfigurationManager.yaml

@@ -15,7 +15,7 @@ title: Ansible Tower Providers
 name: ConfigurationManagerForeman


o_O WAT @bdunne ??

o_O WAT is that??

Not letting it block this PR as this PR is blocking a bunch, but that looks like a bug.

Fryguy · 2017-01-25T20:56:13Z

spec/models/ext_management_system_spec.rb

      "azure"                       => "Azure",
      "azure_network"               => "Azure Network",
      "ec2"                         => "Amazon EC2",
      "ec2_network"                 => "Amazon EC2 Network",
      "ec2_ebs_storage"             => "Amazon EBS",
+      "embedded_ansible_automation" => "Embedded Ansible Automation",


I really don't like the name having Automation at the end. @blomquisg Thoughts? I understand it's because of the multiple manager thing, but it feels gross. Even so, it should not hold up this PR.

Ansible Inside?

I really don't like that name, but that's could just be me 😄

Who would be the consumer of this name?

I guess technically no one on the end, but it might show up as a "provider type" in a summary screen or something.

It's not in this PR, but at some point, we will have to exclude it from the supported_emses list so they don't appear in the "create provider" screens. Perhaps we do that with permissions.yml...haven't quite decided yet.

embedded_ansible seems fine to me. We should change it to that in a follow up PR.

It's a bit like ec2, where we have newer managers that were added later for network and ebs (which really should be "storage"). We could start with a baseline embedded_ansible, then tack on new managers if they ever pop up.

Fryguy · 2017-01-25T21:07:10Z

I unWIPed this PR, because I think it's ready, but I am not comfortable merging until at least these changes are made to the UI: https://github.com/ManageIQ/manageiq-ui-classic/search?utf8=%E2%9C%93&q=AnsibleTower%3A%3AConfigurationManager

Otherwise we insta-break the UI, which is unfair to them as they are busted across the board. I am not sure this will affect the service UI at all, but I don't think it will.

blomquisg · 2017-01-26T15:10:01Z

Fixes ManageIQ/manageiq-ui-classic#248

bdunne · 2017-01-30T21:59:56Z

Where are the associated data migrations? I don't see anything in this PR updating the :type column for all of the STI records. Is that on the to-do list @jameswnl ?

jameswnl · 2017-01-30T23:14:36Z

@bdunne yes, there're 2 migrations pending, 1 for the STI records and 1 for the settings.

bdunne · 2017-01-31T15:30:31Z

@jameswnl Is there a PR for them? I can't use my master database at the moment.

Update wait_for_completion method. See ManageIQ/manageiq#13630

miq-bot added the wip label Jan 23, 2017

miq-bot added the providers/ansible_tower label Jan 23, 2017

chessbyte assigned Fryguy Jan 24, 2017

chrisarcand reviewed Jan 24, 2017

View reviewed changes

Fryguy closed this Jan 24, 2017

Fryguy reopened this Jan 24, 2017

jameswnl force-pushed the automgr2 branch 4 times, most recently from 3779e60 to 5bc38aa Compare January 25, 2017 14:56

AnsibleTower::ConfigurationManager is now

2f0ef1a

AnsibleTower::AutomationManager https://www.pivotaltracker.com/story/show/138063313

jameswnl force-pushed the automgr2 branch from 3cd4320 to 2f0ef1a Compare January 25, 2017 16:18

jameswnl added 2 commits January 25, 2017 11:28

remove ansible_tower_configuration from permissions.yml

b32d77e

remove automation_manager_mixin.rb for now

af2c269

blomquisg mentioned this pull request Jan 25, 2017

ansible refresh parser as graph refresh #13585

Merged

Fryguy reviewed Jan 25, 2017

View reviewed changes

Fryguy changed the title ~~[WIP] Migrate AnsibleTower ConfigurationManager to AutomationManager~~ Migrate AnsibleTower ConfigurationManager to AutomationManager Jan 25, 2017

Fryguy removed the wip label Jan 25, 2017

jameswnl mentioned this pull request Jan 25, 2017

[WIP] Automation managers (for Ansible Tower) #13607

Closed

blomquisg merged commit 4012200 into ManageIQ:master Jan 26, 2017

blomquisg added this to the Sprint 53 Ending Jan 30, 2017 milestone Jan 26, 2017

jameswnl deleted the automgr2 branch January 30, 2017 23:13

billfitzgerald0120 added a commit to billfitzgerald0120/manageiq-content that referenced this pull request Feb 1, 2017

Automate - Rename ConfigurationManagement to AutomationManagement.

de0120d

Update wait_for_completion method. See ManageIQ/manageiq#13630

billfitzgerald0120 mentioned this pull request Feb 1, 2017

[WIP] Automate - Rename ConfigurationManagement to AutomationManagement. ManageIQ/manageiq-content#30

Closed

mkanoor mentioned this pull request Mar 6, 2017

Incorrect source_type in has_many #14206

Merged

Fryguy added the providers/automation label Jan 24, 2020

		@@ -15,7 +15,7 @@ title: Ansible Tower Providers
		name: ConfigurationManagerForeman

Migrate AnsibleTower ConfigurationManager to AutomationManager #13630

Migrate AnsibleTower ConfigurationManager to AutomationManager #13630

Conversation

jameswnl commented Jan 23, 2017

jameswnl commented Jan 23, 2017

miq-bot commented Jan 23, 2017

jameswnl commented Jan 23, 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

miq-bot commented Jan 25, 2017

miq-bot commented Jan 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fryguy Jan 25, 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

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

Fryguy commented Jan 25, 2017

blomquisg commented Jan 26, 2017

bdunne commented Jan 30, 2017

jameswnl commented Jan 30, 2017

bdunne commented Jan 31, 2017

Fryguy Jan 25, 2017 •

edited

Loading