Data Migration: Ansible Tower Configuration Manager type to Automation Manager

[jameswnl]

Existing AnsibleTower Configruration Manager in the ext_management_systems table will be modified to have the new automation manager type.

[jameswnl]

@miq-bot add_labels providers/ansible_tower, enhancement

[bdunne]

Don't forget about all of the other STI subclasses that were renamed by #13630

[bdunne]

Don't use .each in this context, prefer .update_all

ExtManagementSystem.where(
  :type => 'ManageIQ::Providers::AnsibleTower::ConfigurationManager'
).update_all(
  :type => 'ManageIQ::Providers::AnsibleTower::AutomationManager'
)

[jameswnl]

nice, thanks, done!

[bdunne]

Same .update_all

[Fryguy]

Are there any polymorphic tables that also need to be updated? We have a helper for that rename_class_references. Should that be included here as well?

[bdunne]

There is an Authentication for a Provider but it shouldn't need to change

[jameswnl]

The authentication is related to provider. As shown in the following,

vmdb_dev_2=# select id, name, resource_id, resource_type, type from authentications where resource_type = 'Provider';
 id  |                         name                         | resource_id | resource_type |        type
-----+------------------------------------------------------+-------------+---------------+--------------------
 191 | ManageIQ::Providers::AnsibleTower::Provider Ansible  |           1 | Provider      | AuthUseridPassword
 192 | ManageIQ::Providers::AnsibleTower::Provider Ansible2 |           2 | Provider      | AuthUseridPassword
(2 rows)

We don't need to change those, right?

[Fryguy]

@jameswnl Correct, those are base class so those are fine.

[Fryguy]

This should not be here.

[jameswnl]

If I take it out, spec will fail as follows. How to fix it?

  1) MigrateAnsibleTowerConfigurationManagerStiTypeToAutomationManager#down migrates Ansible Tower Automation Manager to Configuration Manager type
     Failure/Error: configuration_manager = ExtManagementSystem.find(automation_manager.id)

     ActiveRecord::SubclassNotFound:
       Invalid single-table inheritance type: ManageIQ::Providers::AnsibleTower::ConfigurationManager is not a subclass of ExtManagementSystem

[jameswnl]

So once I changed to the configuration_manager.reload.type, the above error won't happen even without the definition of ExtManagementSystem here now.
Removed now. Thanks

[Fryguy]

You should not need this...just do a .reload on configuration_manager:

expect(configuration_manager.reload.type).to eq('ManageIQ::Providers::AnsibleTower::AutomationManager')

[jameswnl]

Great. Thanks

[Fryguy]

You may also want a test that shows that other existing providers are not affected by this change.

[jameswnl]

You may also want a test that shows that other existing providers are not affected by this change.

@Fryguy thanks for reminding. Had that in mind but then it slipped. Now added.

[bdunne]

Don't forget about Job and InventoryGroup

[jameswnl]

@bdunne thanks! Done.

[bdunne]

👍 I feel like the tests could be consolidated to only call migrate once per migration_context, but I'm not going to reject it because of that.

[bdunne]

Rails/SkipsModelValidations - Avoid using update_all because it skips validations.

This is okay in this context because it's a migration, so:

• There are no validations on the stub models
• It is potentially updating many records and is more efficient than updating each record individually

[jameswnl]

@bdunne I've consolidated the specs.

[Fryguy]

LGTM

@bdunne if I don't get to it first, please merge when green.

[miq-bot]

Checked commits jameswnl/manageiq@23ccd69~...ed1bb5f with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 10 offenses detected

db/migrate/20170131160216_migrate_ansible_tower_configuration_manager_sti_type_to_automation_manager.rb

• ❗ - Line 24, Col 101 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.
• ❗ - Line 30, Col 122 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.
• ❗ - Line 36, Col 116 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.
• ❗ - Line 42, Col 90 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.
• ❗ - Line 48, Col 107 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.
• ❗ - Line 56, Col 98 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.
• ❗ - Line 62, Col 119 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.
• ❗ - Line 68, Col 113 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.
• ❗ - Line 74, Col 87 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.
• ❗ - Line 80, Col 104 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.