-
Notifications
You must be signed in to change notification settings - Fork 897
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
Create initial tower objects when we start the worker #14283
Changes from all commits
3484703
8322c66
f9ae577
5d2d5df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
module EmbeddedAnsibleWorker::ObjectManagement | ||
extend ActiveSupport::Concern | ||
|
||
def ensure_initial_objects(provider, connection) | ||
ensure_organization(provider, connection) | ||
ensure_credential(provider, connection) | ||
ensure_inventory(provider, connection) | ||
ensure_host(provider, connection) | ||
end | ||
|
||
def remove_demo_data(connection) | ||
connection.api.credentials.all(:name => "Demo Credential").each(&:destroy!) | ||
connection.api.inventories.all(:name => "Demo Inventory").each(&:destroy!) | ||
connection.api.job_templates.all(:name => "Demo Job Template").each(&:destroy!) | ||
connection.api.projects.all(:name => "Demo Project").each(&:destroy!) | ||
connection.api.organizations.all(:name => "Default").each(&:destroy!) | ||
end | ||
|
||
def ensure_organization(provider, connection) | ||
return if provider.default_organization | ||
|
||
provider.default_organization = connection.api.organizations.create!( | ||
:name => I18n.t("product.name"), | ||
:description => "#{I18n.t("product.name")} Default Organization" | ||
).id | ||
end | ||
|
||
def ensure_credential(provider, connection) | ||
return if provider.default_credential | ||
|
||
provider.default_credential = connection.api.credentials.create!( | ||
:name => "#{I18n.t("product.name")} Default Credential", | ||
:kind => "ssh", | ||
:organization => provider.default_organization | ||
).id | ||
end | ||
|
||
def ensure_inventory(provider, connection) | ||
return if provider.default_inventory | ||
|
||
provider.default_inventory = connection.api.inventories.create!( | ||
:name => "#{I18n.t("product.name")} Default Inventory", | ||
:organization => provider.default_organization | ||
).id | ||
end | ||
|
||
def ensure_host(provider, connection) | ||
return if provider.default_host | ||
|
||
provider.default_host = connection.api.hosts.create!( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @carbonin make sure the created There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bzwei Is that in the variables yaml string? I'm guessing that would be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be addressed now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
:name => "localhost", | ||
:inventory => provider.default_inventory, | ||
:variables => {'ansible_connection' => "local"}.to_yaml | ||
).id | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ class EmbeddedAnsible | |
START_EXCLUDE_TAGS = "packages,migrations,firewall".freeze | ||
HTTP_PORT = 54_321 | ||
HTTPS_PORT = 54_322 | ||
WAIT_FOR_ANSIBLE_SLEEP = 1.second | ||
|
||
def self.available? | ||
path = ENV["APPLIANCE_ANSIBLE_DIRECTORY"] || APPLIANCE_ANSIBLE_DIRECTORY | ||
|
@@ -50,6 +51,15 @@ def self.configure | |
def self.start | ||
configure_secret_key | ||
run_setup_script(START_EXCLUDE_TAGS) | ||
|
||
5.times do | ||
return if alive? | ||
|
||
_log.info("Waiting for EmbeddedAnsible to respond") | ||
sleep WAIT_FOR_ANSIBLE_SLEEP | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this better. Now the caller can be guaranteed to be alive or get an exception. 👍 |
||
|
||
raise "EmbeddedAnsible service is not responding after setup" | ||
end | ||
|
||
def self.stop | ||
|
@@ -64,6 +74,15 @@ def self.services | |
AwesomeSpawn.run!("source /etc/sysconfig/ansible-tower; echo $TOWER_SERVICES").output.split | ||
end | ||
|
||
def self.api_connection | ||
admin_auth = miq_database.ansible_admin_authentication | ||
AnsibleTowerClient::Connection.new( | ||
:base_url => URI::HTTP.build(:host => "localhost", :path => "/api/v1", :port => HTTP_PORT).to_s, | ||
:username => admin_auth.userid, | ||
:password => admin_auth.password | ||
) | ||
end | ||
|
||
def self.run_setup_script(exclude_tags) | ||
json_extra_vars = { | ||
:minimum_var_space => 0, | ||
|
@@ -171,14 +190,4 @@ def self.database_connection | |
ActiveRecord::Base.connection | ||
end | ||
private_class_method :database_connection | ||
|
||
def self.api_connection | ||
admin_auth = miq_database.ansible_admin_authentication | ||
AnsibleTowerClient::Connection.new( | ||
:base_url => URI::HTTP.build(:host => "localhost", :path => "/api/v1", :port => HTTP_PORT).to_s, | ||
:username => admin_auth.userid, | ||
:password => admin_auth.password | ||
) | ||
end | ||
private_class_method :api_connection | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,9 @@ | |
let(:runner) { | ||
worker | ||
allow_any_instance_of(described_class).to receive(:worker_initialization) | ||
described_class.new(:guid => worker_guid) | ||
r = described_class.new(:guid => worker_guid) | ||
allow(r).to receive(:worker).and_return(worker) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only necessary because of the stub above. Instead can we just tap the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 I'll look into this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😢 I have to finish fixing this. So much code now depends on all of this junk running when you call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So I tried to do a bit to get rid of this and it turns out we have to jump through hoops because Even then there is no way to deal with how the runner looks up the worker, I would still need to stub the So then the last option would be to get rid of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the answer here will be to just remove the zone/role stuff from settings. Then we don't have to deal with multiple sources of information for this type of thing. Also, I don't really want to do that in this PR. I'll work on that as a follow up and fix this up there. |
||
r | ||
} | ||
|
||
it "#do_before_work_loop exits on exceptions" do | ||
|
@@ -21,13 +23,20 @@ | |
end | ||
|
||
context "#update_embedded_ansible_provider" do | ||
let(:api_connection) { double("AnsibleAPIConnection") } | ||
before do | ||
EvmSpecHelper.local_guid_miq_server_zone | ||
MiqDatabase.seed | ||
MiqDatabase.first.set_ansible_admin_authentication(:password => "secret") | ||
|
||
allow(EmbeddedAnsible).to receive(:api_connection).and_return(api_connection) | ||
end | ||
|
||
it "creates initial" do | ||
expect(worker).to receive(:remove_demo_data).with(api_connection) | ||
expect(worker).to receive(:ensure_initial_objects) | ||
.with(instance_of(ManageIQ::Providers::EmbeddedAnsible::Provider), api_connection) | ||
|
||
runner.update_embedded_ansible_provider | ||
|
||
provider = ManageIQ::Providers::EmbeddedAnsible::Provider.first | ||
|
@@ -39,6 +48,10 @@ | |
end | ||
|
||
it "updates existing" do | ||
expect(worker).to receive(:remove_demo_data).twice.with(api_connection) | ||
expect(worker).to receive(:ensure_initial_objects).twice | ||
.with(instance_of(ManageIQ::Providers::EmbeddedAnsible::Provider), api_connection) | ||
|
||
runner.update_embedded_ansible_provider | ||
new_zone = FactoryGirl.create(:zone) | ||
miq_server.update(:hostname => "boringserver", :zone => new_zone) | ||
|
@@ -51,6 +64,25 @@ | |
expect(provider.default_endpoint.url).to eq("https://boringserver/ansibleapi/v1") | ||
end | ||
end | ||
|
||
context "#setup_ansible" do | ||
it "configures EmbeddedAnsible if it is not configured" do | ||
expect(EmbeddedAnsible).to receive(:start) | ||
|
||
expect(EmbeddedAnsible).to receive(:configured?).and_return(false) | ||
expect(EmbeddedAnsible).to receive(:configure) | ||
|
||
runner.setup_ansible | ||
end | ||
|
||
it "doesn't call configure if EmbeddedAnsible is already configured" do | ||
expect(EmbeddedAnsible).to receive(:start) | ||
|
||
expect(EmbeddedAnsible).to receive(:configured?).and_return(true) | ||
expect(EmbeddedAnsible).not_to receive(:configure) | ||
|
||
runner.setup_ansible | ||
end | ||
end | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,138 @@ | ||
describe EmbeddedAnsibleWorker do | ||
subject { FactoryGirl.create(:embedded_ansible_worker) } | ||
|
||
context "ObjectManagement concern" do | ||
let(:provider) { FactoryGirl.create(:provider_embedded_ansible) } | ||
let(:api_connection) { double("AnsibleAPIConnection", :api => tower_api) } | ||
let(:tower_api) do | ||
methods = { | ||
:organizations => org_collection, | ||
:credentials => cred_collection, | ||
:inventories => inv_collection, | ||
:hosts => host_collection, | ||
:job_templates => job_templ_collection, | ||
:projects => proj_collection | ||
|
||
} | ||
double("TowerAPI", methods) | ||
end | ||
|
||
let(:org_collection) { double("AnsibleOrgCollection", :all => [org_resource]) } | ||
let(:cred_collection) { double("AnsibleCredCollection", :all => [cred_resource]) } | ||
let(:inv_collection) { double("AnsibleInvCollection", :all => [inv_resource]) } | ||
let(:host_collection) { double("AnsibleHostCollection", :all => [host_resource]) } | ||
let(:job_templ_collection) { double("AnsibleJobTemplCollection", :all => [job_templ_resource]) } | ||
let(:proj_collection) { double("AnsibleProjCollection", :all => [proj_resource]) } | ||
|
||
let(:org_resource) { double("AnsibleOrgResource", :id => 12) } | ||
let(:cred_resource) { double("AnsibleCredResource", :id => 13) } | ||
let(:inv_resource) { double("AnsibleInvResource", :id => 14) } | ||
let(:host_resource) { double("AnsibleHostResource", :id => 15) } | ||
let(:job_templ_resource) { double("AnsibleJobTemplResource", :id => 16) } | ||
let(:proj_resource) { double("AnsibleProjResource", :id => 17) } | ||
|
||
describe "#ensure_initial_objects" do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, I agree for contexts, but I always use a |
||
it "creates the expected objects" do | ||
expect(org_collection).to receive(:create!).and_return(org_resource) | ||
expect(cred_collection).to receive(:create!).and_return(cred_resource) | ||
expect(inv_collection).to receive(:create!).and_return(inv_resource) | ||
expect(host_collection).to receive(:create!).and_return(host_resource) | ||
|
||
subject.ensure_initial_objects(provider, api_connection) | ||
end | ||
end | ||
|
||
describe "#remove_demo_data" do | ||
it "removes the existing data" do | ||
expect(org_resource).to receive(:destroy!) | ||
expect(cred_resource).to receive(:destroy!) | ||
expect(inv_resource).to receive(:destroy!) | ||
expect(job_templ_resource).to receive(:destroy!) | ||
expect(proj_resource).to receive(:destroy!) | ||
|
||
subject.remove_demo_data(api_connection) | ||
end | ||
end | ||
|
||
describe "#ensure_organization" do | ||
it "sets the provider default organization" do | ||
expect(org_collection).to receive(:create!).with( | ||
:name => "ManageIQ", | ||
:description => "ManageIQ Default Organization" | ||
).and_return(org_resource) | ||
|
||
subject.ensure_organization(provider, api_connection) | ||
expect(provider.default_organization).to eq(12) | ||
end | ||
|
||
it "doesn't recreate the organization if one is already set" do | ||
provider.default_organization = 1 | ||
expect(org_collection).not_to receive(:create!) | ||
|
||
subject.ensure_organization(provider, api_connection) | ||
end | ||
end | ||
|
||
describe "#ensure_credential" do | ||
it "sets the provider default credential" do | ||
provider.default_organization = 123 | ||
expect(cred_collection).to receive(:create!).with( | ||
:name => "ManageIQ Default Credential", | ||
:kind => "ssh", | ||
:organization => 123 | ||
).and_return(cred_resource) | ||
|
||
subject.ensure_credential(provider, api_connection) | ||
expect(provider.default_credential).to eq(13) | ||
end | ||
|
||
it "doesn't recreate the credential if one is already set" do | ||
provider.default_credential = 2 | ||
expect(cred_collection).not_to receive(:create!) | ||
|
||
subject.ensure_credential(provider, api_connection) | ||
end | ||
end | ||
|
||
describe "#ensure_inventory" do | ||
it "sets the provider default inventory" do | ||
provider.default_organization = 123 | ||
expect(inv_collection).to receive(:create!).with( | ||
:name => "ManageIQ Default Inventory", | ||
:organization => 123 | ||
).and_return(inv_resource) | ||
|
||
subject.ensure_inventory(provider, api_connection) | ||
expect(provider.default_inventory).to eq(14) | ||
end | ||
|
||
it "doesn't recreate the inventory if one is already set" do | ||
provider.default_inventory = 3 | ||
expect(inv_collection).not_to receive(:create!) | ||
|
||
subject.ensure_inventory(provider, api_connection) | ||
end | ||
end | ||
|
||
describe "#ensure_host" do | ||
it "sets the provider default host" do | ||
provider.default_inventory = 234 | ||
expect(host_collection).to receive(:create!).with( | ||
:name => "localhost", | ||
:inventory => 234, | ||
:variables => "---\nansible_connection: local\n" | ||
).and_return(host_resource) | ||
|
||
subject.ensure_host(provider, api_connection) | ||
expect(provider.default_host).to eq(15) | ||
end | ||
|
||
it "doesn't recreate the host if one is already set" do | ||
provider.default_host = 1 | ||
expect(host_collection).not_to receive(:create!) | ||
|
||
subject.ensure_host(provider, api_connection) | ||
end | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this even work?
default_orgainzation=
creates aCustomAttribute
on the Provider right? But you're passing in aAnsibleTowerClient::Organization
instance.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm passing an id. See https://github.com/ManageIQ/manageiq/pull/14283/files/b67becaf26a4faffbc9d6b6c199837cdbe804a15#diff-45e99ec23caedc019723f05f7817652eR25