Adding many models for EmbeddedAnsible provider #13879

blomquisg · 2017-02-13T05:01:30Z

WOW! So many changes here.

This is the first attempt to create a way to differentiate the AnsibleTower models from the EmbeddedAnsible models.

The point behind this differentiation is to allow querying done via the MIQ API and UI to filter based on the Embedded vs. External nature of each.

Specifically, one should be able to ask for ManageIQ::Providers::EmbeddedAutomationManager::ConfigurationScript.all
and not get any configuration script from any AnsibleTower providers, because AnsibleTower providers should be strictly ExternalAutomationManagers.

How was this accomplished?

Any model-level logic in AnsibleTower models has been extracted and moved to a new ManageIQ::Providers::AnsibleTower::Shared namespace
All models in AnsibleTower and EmbeddedAnsible now pull all logic from models in ManageIQ::Providers::AnsibleTower::Shared models.
a. Note that only a few models are actually in Shared. This is because only a few actually had any implementation.
EmbeddedAutomationManager and ExternalAutomationManager have been built out more. Maybe not completely. This PR might have missed some gaps.
AutomationManager has more models for completeness. We can decide whether this is really necessary, but it felt better to do it this way.

Authentication / Credential modeling for Ansible Tower and Embedded Ansible

`ConfigurationScript` modeling for Ansible Tower and Embedded Ansible

Fryguy · 2017-02-13T14:09:46Z

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

      find_by(:manager_id => manager.id, :manager_ref => job_template.id)
    end

    def create_in_provider_queue(manager_id, params)
+      manager = ExtManagementSystem.find(manager_id)
+
+      manager = ExtManagementSystem.find(manager_id)


Duplicated lines.

We need to do it twice because of the non-deterministic nature of computers. LOL

Fryguy · 2017-02-13T14:10:53Z

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

@@ -0,0 +1,90 @@
+require 'ansible_tower_client'
+module ManageIQ::Providers::AnsibleTower::Shared::AutomationManager::Job
+  include Status


Might want to use include_concern here.

Ah, I could not figure that out last night! Thanks, I'll give this a try.

Fryguy · 2017-02-13T14:11:53Z

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

+                  :ext_management_system => template.manager,
+                  :job_template          => template)
+      stack.send(:update_with_provider_object, raw_create_stack(template, options))
+      stack


I'm sure this copy paste, but a preferred pattern than

x = blah x.foo = bar return x

is

blah.tap do |x| x.foo = bar end

Yep, 100% copy/paste. I can update this as part of the PR, though.

Fryguy · 2017-02-13T14:13:34Z

app/models/manageiq/providers/embedded_ansible/automation_manager/azure_credential.rb

@@ -0,0 +1,3 @@
+# This corresponds to Ansible Tower's Azure Resource Manager (azure_rm) type credential.  We are not modeling the deprecated Azzure classic


Typo Azzure -> Azure

Haha, also copy/paste. I'll add it to the list.

hehe, my fault

I azzure you, it's ok 😄

@Fryguy 😑

Fryguy · 2017-02-13T14:16:23Z

@blomquisg LGTM, with a few minor comments

There are also a few minor rubocop issues, but if they were there previously, then no need to do them in this PR.

bdunne

@blomquisg looks like specs failed because you missed a few automate service models. See this for an example

blomquisg · 2017-02-14T20:08:17Z

looks like specs failed because you missed a few automate service models.

Thanks @bdunne .... on it

blomquisg · 2017-02-15T02:38:06Z

OMG it's 🍏 💚 📗 🐸 🐍 🐢 🍀 🌵 🎾 ✳️ ❇️ ✅

Though, any chance we can get #13776 reviewed and merged first? Then, I'll rebase mine on top of that and incorporate the new models introduced there.

miq-bot · 2017-02-15T19:16:46Z

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

bdunne · 2017-02-16T15:11:15Z

@blomquisg #13776 is merged. I think we can merge after rebase.

blomquisg · 2017-02-16T23:34:05Z

tag_collection_spec failures?

...

Those tests pass locally for me, so it must be something very recently added since my last rebase.

jameswnl · 2017-02-17T01:38:30Z

It's broken in master

miq-bot · 2017-02-21T14:51:48Z

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

blomquisg · 2017-02-21T21:24:09Z

it's the classic
master is broken ↩️
close/open PR to restart tests ↩️
PR is unmergeable b/c master changed ↩️
rebase ↩️
hope master isn't broken again 🔁

miq-bot · 2017-02-22T14:31:43Z

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

WOW! So many changes here. This is the first attempt to create a way to differentiate the AnsibleTower models from the EmbeddedAnsible models. The point behind this differentiation is to allow querying done via the MIQ API and UI to filter based on the Embedded vs. External nature of each. Specifically, one should be able to ask for `ManageIQ::Providers::EmbeddedAutomationManager::ConfigurationScript.all` and not get any configuration script from any `AnsibleTower` providers, because `AnsibleTower` providers should be strictly `ExternalAutomationManagers`. This definitely feels like a lot of convoluted hierarchical organization that's going to hurt in the future. TODO: OMG, draw a picture of this!

miq-bot · 2017-02-22T15:52:06Z

Fryguy · 2017-02-23T18:18:36Z

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

@@ -8,22 +11,19 @@ def create_in_provider(manager_id, params)

      # 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, true) if manager.authentication_status_ok?
-
+      EmsRefresh.queue_refresh(manager, nil, true) if !manager.missing_credentials? && manager.authentication_status_ok?


@blomquisg This changed?

It did ... or, it did when I first copied all this stuff around

Fryguy · 2017-02-23T18:19:33Z

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

+
+  def provider_object(connection = nil)
+    (connection || connection_source.connect).api.job_templates.find(manager_ref)
+  end


Also why was this added?

Not added:

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

Line 16 in 8773af5

def provider_object(connection = nil)

blomquisg · 2017-02-24T16:58:31Z

Adding a comment regarding filtering by models. This has come up a few times, and I wanna make sure it's captured somewhere.

Preface

There are two different AutomationManagers: AnsibleTower::AutomationManager and EmbeddedAnsible::AutomationManager. MIQ wants to show different details of the model objects under those different AutomationManagers.

However, propagating specific provider names back into the UI breaks future-looking pluggability of trying to keep specific provider names out of the UI.

So, an intermediate layer has been added to allow model-level filtering between "Embedded" and "External" types of AutomationManagers. Specifically, there are:

ManageIQ::Providers::EmbeddedAutomationManager
- For all EmbeddedAnsible things (and any future embedded automation manager things)
ManageIQ::Providers::ExternalAutomationManager
- For all AnsibleTower things (and any future external automation manager things)

Filtering

To see all of the ConfigurationScriptSource objects for EmbeddedAnsible:

ManageIQ::Providers::EmbeddedAutomationManager.ConfigurationScriptSource.all

To see all of the ConfigurationScript (Job Template) objects for AnsibleTower:

ManageIQ::Providers::ExternalAutomationManager.ConfigurationScript.all

blomquisg assigned Fryguy, bdunne and jameswnl Feb 13, 2017

blomquisg requested review from Fryguy, bdunne and jameswnl February 13, 2017 05:02

blomquisg unassigned bdunne and jameswnl Feb 13, 2017

blomquisg added enhancement providers/ansible_tower labels Feb 13, 2017

Fryguy reviewed Feb 13, 2017

View reviewed changes

Fryguy approved these changes Feb 13, 2017

View reviewed changes

blomquisg force-pushed the embedded_automation_manager branch 2 times, most recently from 38520bf to c90a59f Compare February 13, 2017 23:28

bdunne requested changes Feb 14, 2017

View reviewed changes

blomquisg force-pushed the embedded_automation_manager branch from c90a59f to 640f9d3 Compare February 15, 2017 01:49

This was referenced Feb 15, 2017

Migration and model changes for ConfigurationScriptSource #13683

Merged

Model change for Ansible Tower Credential #13773

Merged

miq-bot added the unmergeable label Feb 15, 2017

blomquisg force-pushed the embedded_automation_manager branch from 640f9d3 to 51d607a Compare February 16, 2017 22:10

miq-bot removed the unmergeable label Feb 16, 2017

bdunne approved these changes Feb 17, 2017

View reviewed changes

blomquisg closed this Feb 21, 2017

blomquisg reopened this Feb 21, 2017

miq-bot added the unmergeable label Feb 21, 2017

blomquisg force-pushed the embedded_automation_manager branch from 51d607a to 57a1399 Compare February 21, 2017 21:18

miq-bot removed the unmergeable label Feb 21, 2017

miq-bot added the unmergeable label Feb 22, 2017

blomquisg added 2 commits February 22, 2017 10:46

Updating automate models for AutomationManagers

f72a278

blomquisg force-pushed the embedded_automation_manager branch from 57a1399 to f72a278 Compare February 22, 2017 15:47

miq-bot removed the unmergeable label Feb 22, 2017

blomquisg closed this Feb 23, 2017

blomquisg reopened this Feb 23, 2017

blomquisg closed this Feb 23, 2017

blomquisg reopened this Feb 23, 2017

Fryguy reviewed Feb 23, 2017

View reviewed changes

Fryguy merged commit 13acb63 into ManageIQ:master Feb 23, 2017

Fryguy added this to the Sprint 55 Ending Feb 27, 2017 milestone Feb 23, 2017

blomquisg mentioned this pull request Feb 24, 2017

Add missing ConfigurationScriptSource hierarchy and Automate models #14069

Merged

blomquisg deleted the embedded_automation_manager branch November 28, 2017 18:31

Fryguy added the providers/automation label Jan 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding many models for EmbeddedAnsible provider #13879

Adding many models for EmbeddedAnsible provider #13879

blomquisg commented Feb 13, 2017 •

edited

Loading

Fryguy Feb 13, 2017

blomquisg Feb 13, 2017

Fryguy Feb 13, 2017

blomquisg Feb 13, 2017

Fryguy Feb 13, 2017

blomquisg Feb 13, 2017

Fryguy Feb 13, 2017

blomquisg Feb 13, 2017

jameswnl Feb 13, 2017

Fryguy Feb 14, 2017

jrafanie Feb 14, 2017

Fryguy commented Feb 13, 2017

bdunne left a comment

blomquisg commented Feb 14, 2017

blomquisg commented Feb 15, 2017 •

edited

Loading

miq-bot commented Feb 15, 2017

bdunne commented Feb 16, 2017

blomquisg commented Feb 16, 2017 •

edited

Loading

jameswnl commented Feb 17, 2017

miq-bot commented Feb 21, 2017

blomquisg commented Feb 21, 2017

miq-bot commented Feb 22, 2017

miq-bot commented Feb 22, 2017

Fryguy Feb 23, 2017

blomquisg Feb 23, 2017

Fryguy Feb 23, 2017

blomquisg Feb 23, 2017

blomquisg commented Feb 24, 2017 •

edited

Loading

		@@ -0,0 +1,3 @@
		# This corresponds to Ansible Tower's Azure Resource Manager (azure_rm) type credential. We are not modeling the deprecated Azzure classic

Adding many models for EmbeddedAnsible provider #13879

Adding many models for EmbeddedAnsible provider #13879

Conversation

blomquisg commented Feb 13, 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

Fryguy commented Feb 13, 2017

bdunne left a comment

Choose a reason for hiding this comment

blomquisg commented Feb 14, 2017

blomquisg commented Feb 15, 2017 • edited Loading

miq-bot commented Feb 15, 2017

bdunne commented Feb 16, 2017

blomquisg commented Feb 16, 2017 • edited Loading

jameswnl commented Feb 17, 2017

miq-bot commented Feb 21, 2017

blomquisg commented Feb 21, 2017

miq-bot commented Feb 22, 2017

miq-bot commented Feb 22, 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

blomquisg commented Feb 24, 2017 • edited Loading

Preface

Filtering

blomquisg commented Feb 13, 2017 •

edited

Loading

blomquisg commented Feb 15, 2017 •

edited

Loading

blomquisg commented Feb 16, 2017 •

edited

Loading

blomquisg commented Feb 24, 2017 •

edited

Loading