Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ansible refresh inventory #13807

Merged
merged 2 commits into from
Feb 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,8 @@ def hosts
def job_templates
connection.api.job_templates.all
end

def projects
@connection.api.projects.all
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ def parse
inventory_groups
configured_systems
configuration_scripts
configuration_script_sources
end

def inventory_groups
Expand Down Expand Up @@ -32,4 +33,20 @@ def configuration_scripts
o[:inventory_root_group] = target.inventory_groups.lazy_find(i.inventory_id.to_s)
end
end

def configuration_script_sources
collector.projects.each do |i|
o = target.configuration_script_sources.find_or_build(i.id.to_s)
o[:description] = i.description
o[:name] = i.name

i.playbooks.each do |playbook_name|
# FIXME: its not really nice how I have to build a manager_ref / uuid here
p = target.playbooks.find_or_build("#{i.id}__#{playbook_name}")
Copy link
Contributor

@Ladas Ladas Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the manager_ref is "#{i.id}__#{playbook_name}" ?

it might be better not to change manager_ref, since you will need to parse it when talking to provider? E.g. doing some operation with playbook.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the playbook is just a name. So to make it really uniq, I need to scope it under project_id

GET /api/v1/projects/4/playbooks/
HTTP 200 OK
Allow: GET, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept
X-API-Time: 0.039s

[
    "hello_world.yml"
]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, so it's better to have in unchanged in our db, so e.g. it you want to delete a playbook you know you need playbook.manager_ref and playbook.configuration_script_source.manager_ref

So you don't need to do a playbook.manager_ref.split("__")

I was proposing another key for pure unique check in #13806

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it's better to have in unchanged in our db

what should I set it to then? If I do this

target.playbooks.find_or_build(playbook_name)

the tests fail, because there are duplicate playbook names across projects

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this case would need find_or_build to take in more than 1 attribute. Are there other use cases that need this too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we need to add the find_or_build_by, as we talked in the other PR

In AWS, I have several entities with unique key.size >= 2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great!

# FIXME: how about just adding `o` - configuration_script_source here?
p[:configuration_script_source] = target.configuration_script_sources.lazy_find(i.id.to_s)
Copy link
Contributor

@Ladas Ladas Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's always 'o', you can just put it there :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oooh

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hehe' :-)

p[:name] = playbook_name
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,24 @@ def configuration_scripts
:builder_params => {:manager => @root}
)
end

def configuration_script_sources
collections[:configuration_script_sources] ||= ManagerRefresh::InventoryCollection.new(
:model_class => ConfigurationScriptSource,
:association => :configuration_script_sources,
:manager_ref => [:manager_ref],
:parent => @root,
:builder_params => {:manager => @root}
)
end

def playbooks
collections[:playbooks] ||= ManagerRefresh::InventoryCollection.new(
:model_class => ManageIQ::Providers::AnsibleTower::AutomationManager::Playbook,
:association => :configuration_script_payloads,
:manager_ref => [:manager_ref],
:parent => @root,
:builder_params => {:manager => @root}
)
end
end
1 change: 1 addition & 0 deletions app/models/manageiq/providers/automation_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class ManageIQ::Providers::AutomationManager < ::ExtManagementSystem
has_many :inventory_groups, :dependent => :destroy, :foreign_key => "ems_id", :inverse_of => :manager
has_many :inventory_root_groups, :dependent => :destroy, :foreign_key => "ems_id", :inverse_of => :manager
has_many :configuration_script_sources, :dependent => :destroy, :foreign_key => "manager_id"
has_many :configuration_script_payloads, :through => :configuration_script_sources

virtual_column :total_configuration_profiles, :type => :integer
virtual_column :total_configured_systems, :type => :integer
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,39 @@
describe ManageIQ::Providers::AnsibleTower::AutomationManager::Refresher do
let(:auth) { FactoryGirl.create(:authentication) }
# To re-record cassettes or to add cassettes you can add another inner `VCR.use_cassette` block to the
# 'will perform a full refresh' example. When running specs, new requests are recorded to the innermost cassette and
# can be played back from any level of nesting (it tries the innermost cassette first, then searches up the parent
# chain) - http://stackoverflow.com/a/13425826
#
# To add a new cassette
# * add another block (innermost) with an empty cassette
# * change existing cassettes to use your working credentials
# * run the specs to create a new cassette
# * change new and existing cassettes to use default credentials
#
# To re-record a cassette
# * temporarily make the cassette the innermost one (see above about recording)
# * rm cassette ; run specs
# * change back the order of cassettes
#
# To change credentials in cassettes:
# replace with defaults - before committing
# ruby -pi -e 'gsub /dev-ansible-tower3.cloudforms.lab.eng.rdu2.redhat.com/, "dev-ansible-tower2.example.com"; gsub /admin:smartvm/, "testuser:secret"' spec/vcr_cassettes/manageiq/providers/ansible_tower/automation_manager/*.yml
# replace with your working credentials
# ruby -pi -e 'gsub /dev-ansible-tower2.example.com/, "dev-ansible-tower3.cloudforms.lab.eng.rdu2.redhat.com"; gsub /testuser:secret/, "admin:smartvm"' spec/vcr_cassettes/manageiq/providers/ansible_tower/automation_manager/*.yml

let(:tower_url) { ENV['TOWER_URL'] || "https://dev-ansible-tower2.example.com/api/v1/" }
let(:auth_userid) { ENV['TOWER_USER'] || 'testuser' }
let(:auth_password) { ENV['TOWER_PASSWORD'] || 'secret' }

let(:auth) { FactoryGirl.create(:authentication, :userid => auth_userid, :password => auth_password) }
let(:automation_manager) { provider.automation_manager }
let(:expected_counterpart_vm) { FactoryGirl.create(:vm, :uid_ems => "4233080d-7467-de61-76c9-c8307b6e4830") }
let(:provider) do
_guid, _server, zone = EvmSpecHelper.create_guid_miq_server_zone
FactoryGirl.create(:provider_ansible_tower,
:zone => zone,
:url => "https://dev-ansible-tower2.example.com/api/v1/",
:verify_ssl => false,
).tap { |provider| provider.authentications << auth }
:url => tower_url,
:verify_ssl => false,).tap { |provider| provider.authentications << auth }
end

it ".ems_type" do
Expand All @@ -19,16 +44,21 @@
expected_counterpart_vm

2.times do
# to re-record cassettes see comment at the beginning of this file
VCR.use_cassette(described_class.name.underscore) do
EmsRefresh.refresh(automation_manager)
expect(automation_manager.reload.last_refresh_error).to be_nil
VCR.use_cassette(described_class.name.underscore + '_configuration_script_sources') do
EmsRefresh.refresh(automation_manager)
expect(automation_manager.reload.last_refresh_error).to be_nil
end
end

assert_counts
assert_configured_system
assert_configuration_script_with_nil_survey_spec
assert_configuration_script_with_survey_spec
assert_inventory_root_group
assert_configuration_script_sources
assert_playbooks
end
end

Expand All @@ -38,6 +68,25 @@ def assert_counts
expect(automation_manager.configured_systems.count).to eq(98)
expect(automation_manager.configuration_scripts.count).to eq(13)
expect(automation_manager.inventory_groups.count).to eq(7)
expect(automation_manager.configuration_script_sources.count).to eq(6)
expect(automation_manager.configuration_script_payloads.count).to eq(438)
end

def assert_playbooks
configuration_script_source = automation_manager.configuration_script_sources.find_by(:manager_ref => 29)
expect(configuration_script_source.configuration_script_payloads.first).to be_an_instance_of(ManageIQ::Providers::AnsibleTower::AutomationManager::Playbook)
expect(configuration_script_source.configuration_script_payloads.count).to eq(60)
expect(configuration_script_source.configuration_script_payloads.map(&:name)).to include('jboss-standalone/demo-aws-launch.yml')
end

def assert_configuration_script_sources
expect(automation_manager.configuration_script_sources.count).to eq(6)
configuration_script_source = automation_manager.configuration_script_sources.find_by(:manager_ref => 4)
expect(configuration_script_source).to be_an_instance_of(ConfigurationScriptSource)
expect(configuration_script_source).to have_attributes(
:name => 'Demo Project',
:description => 'A great demo',
)
end

def assert_configured_system
Expand Down
Loading