From 36ea0ea24173ad57a6f900e6cd65de940ae4d7d0 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Wed, 21 Jun 2017 10:25:52 -0400 Subject: [PATCH 1/6] Use the ansible service name for the provider URL in containers When we know we are running in a container (read: OpenShift) we also know that embedded ansible will be provided as a separate service. So rather than pointing at one of our servers as the provider URL, use the service name. --- app/models/embedded_ansible_worker/runner.rb | 16 ++++++++++- .../embedded_ansible_worker/runner_spec.rb | 28 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/app/models/embedded_ansible_worker/runner.rb b/app/models/embedded_ansible_worker/runner.rb index 32dfb63a33f..d596c86bb60 100644 --- a/app/models/embedded_ansible_worker/runner.rb +++ b/app/models/embedded_ansible_worker/runner.rb @@ -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! @@ -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, diff --git a/spec/models/embedded_ansible_worker/runner_spec.rb b/spec/models/embedded_ansible_worker/runner_spec.rb index 34765da4b92..f9489a32827 100644 --- a/spec/models/embedded_ansible_worker/runner_spec.rb +++ b/spec/models/embedded_ansible_worker/runner_spec.rb @@ -63,6 +63,34 @@ 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| + old_env = ENV["ANSIBLE_SERVICE_NAME"] + ENV["ANSIBLE_SERVICE_NAME"] = "ansible-service" + example.run + ENV["ANSIBLE_SERVICE_NAME"] = old_env + 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 From 73992a472b6160ef661ce073acd0b707f3f523ad Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Wed, 21 Jun 2017 14:34:03 -0400 Subject: [PATCH 2/6] Stub out some service-centric EmbeddedAnsible methods in a container We don't need to concern ourselves with trying to manage the ansible services when we are in a container because we will be running them as a separate container and contacting the API through a service rather than locally. --- lib/embedded_ansible.rb | 5 ++++ spec/lib/embedded_ansible_spec.rb | 38 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/lib/embedded_ansible.rb b/lib/embedded_ansible.rb index ea5a9613704..0323d112ff4 100644 --- a/lib/embedded_ansible.rb +++ b/lib/embedded_ansible.rb @@ -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"] @@ -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) @@ -62,10 +65,12 @@ def self.start 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 diff --git a/spec/lib/embedded_ansible_spec.rb b/spec/lib/embedded_ansible_spec.rb index 88121025ec9..a606ee7c17a 100644 --- a/spec/lib/embedded_ansible_spec.rb +++ b/spec/lib/embedded_ansible_spec.rb @@ -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") } @@ -238,6 +270,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 From 0e9aed42ec463c0b4833ed7426f34e46432d9650 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Wed, 21 Jun 2017 14:56:40 -0400 Subject: [PATCH 3/6] Make EmbeddedAnsible.api_connection use the service when in a container This also sets verify_ssl to 0 for this connection. This is okay because in the appliance case we are communicating locally and in the container case we are communicating within the openshift project. --- lib/embedded_ansible.rb | 15 ++++++++++++--- spec/lib/embedded_ansible_spec.rb | 30 +++++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/lib/embedded_ansible.rb b/lib/embedded_ansible.rb index 0323d112ff4..b0c7a21a9d5 100644 --- a/lib/embedded_ansible.rb +++ b/lib/embedded_ansible.rb @@ -79,11 +79,20 @@ 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, + :username => admin_auth.userid, + :password => admin_auth.password, + :verify_ssl => 0 ) end diff --git a/spec/lib/embedded_ansible_spec.rb b/spec/lib/embedded_ansible_spec.rb index a606ee7c17a..56f7c987cf9 100644 --- a/spec/lib/embedded_ansible_spec.rb +++ b/spec/lib/embedded_ansible_spec.rb @@ -165,6 +165,29 @@ EvmSpecHelper.create_guid_miq_server_zone end + describe ".api_connection" do + around do |example| + old_env = ENV["ANSIBLE_SERVICE_NAME"] + ENV["ANSIBLE_SERVICE_NAME"] = "ansible-service" + example.run + ENV["ANSIBLE_SERVICE_NAME"] = old_env + 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 @@ -188,9 +211,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 From 2115b1b15f21e59de9d02893708c3f6ad5a52568 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Wed, 21 Jun 2017 15:11:41 -0400 Subject: [PATCH 4/6] Make EmbeddedAnsible.start container aware If we are in a container we don't attempt to start any of the services locally, but we set the password appropriatly and wait for the service to be available --- lib/embedded_ansible.rb | 47 ++++++++++++++++++++++--------- spec/lib/embedded_ansible_spec.rb | 17 +++++++++++ 2 files changed, 51 insertions(+), 13 deletions(-) diff --git a/lib/embedded_ansible.rb b/lib/embedded_ansible.rb index b0c7a21a9d5..9b5cc70886e 100644 --- a/lib/embedded_ansible.rb +++ b/lib/embedded_ansible.rb @@ -47,21 +47,11 @@ 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) + appliance_start 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 def self.stop @@ -96,6 +86,37 @@ def self.api_connection ) 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 + return 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, diff --git a/spec/lib/embedded_ansible_spec.rb b/spec/lib/embedded_ansible_spec.rb index 56f7c987cf9..904b0079720 100644 --- a/spec/lib/embedded_ansible_spec.rb +++ b/spec/lib/embedded_ansible_spec.rb @@ -386,6 +386,23 @@ end end + describe ".start when in a container" do + around do |example| + old_env = ENV["ANSIBLE_ADMIN_PASSWORD"] + ENV["ANSIBLE_ADMIN_PASSWORD"] = "thepassword" + example.run + ENV["ANSIBLE_ADMIN_PASSWORD"] = old_env + 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) } From 1ad05ba4fd55ba4371ccc6a3867f5482fccada8c Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Mon, 26 Jun 2017 10:28:12 -0400 Subject: [PATCH 5/6] Delete the ANSIBLE_SERVICE_NAME environment variable in specs This shouldn't really be set anywhere we are running the specs --- spec/lib/embedded_ansible_spec.rb | 6 ++---- spec/models/embedded_ansible_worker/runner_spec.rb | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/spec/lib/embedded_ansible_spec.rb b/spec/lib/embedded_ansible_spec.rb index 904b0079720..c4bcdfdedbb 100644 --- a/spec/lib/embedded_ansible_spec.rb +++ b/spec/lib/embedded_ansible_spec.rb @@ -167,10 +167,9 @@ describe ".api_connection" do around do |example| - old_env = ENV["ANSIBLE_SERVICE_NAME"] ENV["ANSIBLE_SERVICE_NAME"] = "ansible-service" example.run - ENV["ANSIBLE_SERVICE_NAME"] = old_env + ENV.delete("ANSIBLE_SERVICE_NAME") end it "connects to the ansible service when running in a container" do @@ -388,10 +387,9 @@ describe ".start when in a container" do around do |example| - old_env = ENV["ANSIBLE_ADMIN_PASSWORD"] ENV["ANSIBLE_ADMIN_PASSWORD"] = "thepassword" example.run - ENV["ANSIBLE_ADMIN_PASSWORD"] = old_env + ENV.delete("ANSIBLE_ADMIN_PASSWORD") end it "sets the admin password using the environment variable and waits for the service to respond" do diff --git a/spec/models/embedded_ansible_worker/runner_spec.rb b/spec/models/embedded_ansible_worker/runner_spec.rb index f9489a32827..f482f81c5c8 100644 --- a/spec/models/embedded_ansible_worker/runner_spec.rb +++ b/spec/models/embedded_ansible_worker/runner_spec.rb @@ -70,10 +70,9 @@ end around do |example| - old_env = ENV["ANSIBLE_SERVICE_NAME"] ENV["ANSIBLE_SERVICE_NAME"] = "ansible-service" example.run - ENV["ANSIBLE_SERVICE_NAME"] = old_env + ENV.delete("ANSIBLE_SERVICE_NAME") end it "creates the provider with the service name for the URL" do From dee942b863e5e77c5bfbae776e53b5f5f0b5c6bc Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Mon, 26 Jun 2017 10:30:50 -0400 Subject: [PATCH 6/6] Use break rather than return for loops --- lib/embedded_ansible.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/embedded_ansible.rb b/lib/embedded_ansible.rb index 9b5cc70886e..423602a1447 100644 --- a/lib/embedded_ansible.rb +++ b/lib/embedded_ansible.rb @@ -109,7 +109,7 @@ def self.container_start miq_database.set_ansible_admin_authentication(:password => ENV["ANSIBLE_ADMIN_PASSWORD"]) loop do - return if alive? + break if alive? _log.info("Waiting for Ansible container to respond") sleep WAIT_FOR_ANSIBLE_SLEEP