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

Use the Ansible service in containers rather than starting it locally #15423

Merged
merged 6 commits into from
Jun 27, 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
16 changes: 15 additions & 1 deletion app/models/embedded_ansible_worker/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def update_embedded_ansible_provider

provider.name = "Embedded Ansible"
provider.zone = server.zone
provider.url = URI::HTTPS.build(:host => server.hostname || server.ipaddress, :path => "/ansibleapi/v1").to_s
provider.url = provider_url
provider.verify_ssl = 0

provider.save!
Expand All @@ -69,6 +69,20 @@ def message_sync_config(*_args); end

private

def provider_url
server = MiqServer.my_server(true)

if MiqEnvironment::Command.is_container?
host = ENV["ANSIBLE_SERVICE_NAME"]
path = "/api/v1"
else
host = server.hostname || server.ipaddress
path = "/ansibleapi/v1"
end

URI::HTTPS.build(:host => host, :path => path).to_s
end

def raise_role_notification(notification_type)
notification_options = {
:role_name => ServerRole.find_by(:name => worker.class.required_roles.first).description,
Expand Down
67 changes: 51 additions & 16 deletions lib/embedded_ansible.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class EmbeddedAnsible
WAIT_FOR_ANSIBLE_SLEEP = 1.second

def self.available?
return true if MiqEnvironment::Command.is_container?
return false unless MiqEnvironment::Command.is_appliance?

required_rpms = Set["ansible-tower-server", "ansible-tower-setup"]
Expand All @@ -24,10 +25,12 @@ def self.enabled?
end

def self.running?
return true if MiqEnvironment::Command.is_container?
services.all? { |service| LinuxAdmin::Service.new(service).running? }
end

def self.configured?
return true if MiqEnvironment::Command.is_container?
return false unless File.exist?(SECRET_KEY_FILE)
key = miq_database.ansible_secret_key
key.present? && key == File.read(SECRET_KEY_FILE)
Expand All @@ -44,28 +47,20 @@ def self.alive?
end

def self.start
if configured?
services.each { |service| LinuxAdmin::Service.new(service).start.enable }
if MiqEnvironment::Command.is_container?
container_start
else
configure_secret_key
run_setup_script(EXCLUDE_TAGS)
end

5.times do
return if alive?

_log.info("Waiting for EmbeddedAnsible to respond")
sleep WAIT_FOR_ANSIBLE_SLEEP
appliance_start
end

raise "EmbeddedAnsible service is not responding after setup"
end

def self.stop
return if MiqEnvironment::Command.is_container?
services.each { |service| LinuxAdmin::Service.new(service).stop }
end

def self.disable
return if MiqEnvironment::Command.is_container?
services.each { |service| LinuxAdmin::Service.new(service).stop.disable }
end

Expand All @@ -74,14 +69,54 @@ def self.services
end

def self.api_connection
if MiqEnvironment::Command.is_container?
host = ENV["ANSIBLE_SERVICE_NAME"]
port = 80
else
host = "localhost"
port = HTTP_PORT
end

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
:base_url => URI::HTTP.build(:host => host, :path => "/api/v1", :port => port).to_s,
Copy link
Member

Choose a reason for hiding this comment

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

I know this isn't changed in this PR but here and the place we do the URI...build isn't checking if the host is nil. I'm not sure what we can do there but I believe this will blow up building the URI if it's nil.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a better choice than blowing up?

:username => admin_auth.userid,
:password => admin_auth.password,
:verify_ssl => 0
Copy link
Member

Choose a reason for hiding this comment

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

🙈

Copy link
Member

Choose a reason for hiding this comment

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

It's internal API that's not exposed externally, so not a problem

)
end

def self.appliance_start
if configured?
services.each { |service| LinuxAdmin::Service.new(service).start.enable }
else
configure_secret_key
run_setup_script(EXCLUDE_TAGS)
end

5.times do
return if alive?

_log.info("Waiting for EmbeddedAnsible to respond")
sleep WAIT_FOR_ANSIBLE_SLEEP
end

raise "EmbeddedAnsible service is not responding after setup"
end
private_class_method :appliance_start

def self.container_start
miq_database.set_ansible_admin_authentication(:password => ENV["ANSIBLE_ADMIN_PASSWORD"])

loop do
Copy link
Member

Choose a reason for hiding this comment

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

ooh, infinite loop?

Copy link
Member

Choose a reason for hiding this comment

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

any reason it's not 5.times... like the other places?

Copy link
Member Author

Choose a reason for hiding this comment

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

break if alive?

_log.info("Waiting for Ansible container to respond")
sleep WAIT_FOR_ANSIBLE_SLEEP
end
end
private_class_method :container_start

def self.run_setup_script(exclude_tags)
json_extra_vars = {
:minimum_var_space => 0,
Expand Down
83 changes: 80 additions & 3 deletions spec/lib/embedded_ansible_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,44 @@
end
end

it "returns true unconditionally in a container" do
allow(MiqEnvironment::Command).to receive(:is_container?).and_return(true)
expect(described_class.available?).to be_truthy
end

it "returns false outside of an appliance" do
allow(MiqEnvironment::Command).to receive(:is_appliance?).and_return(false)
expect(described_class.available?).to be_falsey
end
end

describe ".running?" do
it "always returns true when in a container" do
expect(MiqEnvironment::Command).to receive(:is_container?).and_return(true)
expect(LinuxAdmin::Service).not_to receive(:new)

expect(described_class.running?).to be_truthy
end
end

describe ".stop" do
it "doesn't attempt to stop services if running in a container" do
expect(MiqEnvironment::Command).to receive(:is_container?).and_return(true)
expect(LinuxAdmin::Service).not_to receive(:new)

described_class.stop
end
end

describe ".disable" do
it "doesn't attempt to disable services if running in a container" do
expect(MiqEnvironment::Command).to receive(:is_container?).and_return(true)
expect(LinuxAdmin::Service).not_to receive(:new)

described_class.disable
end
end

context "with services" do
let(:nginx_service) { double("nginx service") }
let(:supervisord_service) { double("supervisord service") }
Expand Down Expand Up @@ -133,6 +165,28 @@
EvmSpecHelper.create_guid_miq_server_zone
end

describe ".api_connection" do
around do |example|
ENV["ANSIBLE_SERVICE_NAME"] = "ansible-service"
example.run
ENV.delete("ANSIBLE_SERVICE_NAME")
end

it "connects to the ansible service when running in a container" do
miq_database.set_ansible_admin_authentication(:password => "adminpassword")
expect(MiqEnvironment::Command).to receive(:is_container?).and_return(true)

expect(AnsibleTowerClient::Connection).to receive(:new).with(
:base_url => "http://ansible-service/api/v1",
:username => "admin",
:password => "adminpassword",
:verify_ssl => 0
)

described_class.api_connection
end
end

describe ".alive?" do
it "returns false if the service is not configured" do
expect(described_class).to receive(:configured?).and_return false
Expand All @@ -156,9 +210,10 @@
miq_database.set_ansible_admin_authentication(:password => "adminpassword")

expect(AnsibleTowerClient::Connection).to receive(:new).with(
:base_url => "http://localhost:54321/api/v1",
:username => "admin",
:password => "adminpassword"
:base_url => "http://localhost:54321/api/v1",
:username => "admin",
:password => "adminpassword",
:verify_ssl => 0
).and_return(api_conn)
expect(api_conn).to receive(:api).and_return(api)
end
Expand Down Expand Up @@ -238,6 +293,12 @@
miq_database.ansible_secret_key = "password"
expect(described_class.configured?).to be false
end

it "returns true when the file doesn't exist and we are running in a container" do
expect(MiqEnvironment::Command).to receive(:is_container?).and_return(true)
key_file.unlink
expect(described_class.configured?).to be true
end
end

describe ".configure_secret_key (private)" do
Expand Down Expand Up @@ -324,6 +385,22 @@
end
end

describe ".start when in a container" do
around do |example|
ENV["ANSIBLE_ADMIN_PASSWORD"] = "thepassword"
example.run
ENV.delete("ANSIBLE_ADMIN_PASSWORD")
end

it "sets the admin password using the environment variable and waits for the service to respond" do
expect(MiqEnvironment::Command).to receive(:is_container?).and_return(true)
expect(described_class).to receive(:alive?).and_return(true)

described_class.start
expect(miq_database.reload.ansible_admin_authentication.password).to eq("thepassword")
end
end

describe ".generate_database_authentication (private)" do
let(:password) { "secretpassword" }
let(:quoted_password) { ActiveRecord::Base.connection.quote(password) }
Expand Down
27 changes: 27 additions & 0 deletions spec/models/embedded_ansible_worker/runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,33 @@
expect(provider.zone).to eq(new_zone)
expect(provider.default_endpoint.url).to eq("https://boringserver/ansibleapi/v1")
end

context "in a container" do
before do
expect(MiqEnvironment::Command).to receive(:is_container?).and_return(true)
end

around do |example|
ENV["ANSIBLE_SERVICE_NAME"] = "ansible-service"
example.run
ENV.delete("ANSIBLE_SERVICE_NAME")
end

it "creates the provider with the service name for the URL" 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
expect(provider.zone).to eq(miq_server.zone)
expect(provider.default_endpoint.url).to eq("https://ansible-service/api/v1")
userid, password = provider.auth_user_pwd
expect(userid).to eq("admin")
expect(password).to eq("secret")
end
end
end

context "#setup_ansible" do
Expand Down