From 2b1a60c1279d95138cff6c4ec0bcfe3e748ef1a3 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Tue, 2 Apr 2019 08:28:09 -0400 Subject: [PATCH 01/13] Run the playbook on the appliance with the conversion host in inventory --- app/models/conversion_host.rb | 46 +++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 0d2a53a74d6..c9a6806cf83 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -170,8 +170,9 @@ def get_conversion_log(path) end def check_conversion_host_role - playbook = "/usr/share/ovirt-ansible-v2v-conversion-host/playbooks/conversion_host_check.yml" - ansible_playbook(playbook) + playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_check.yml" + extra_vars = { :v2v_host_type => resource.ext_management_system.emstype } + ansible_playbook(playbook, extra_vars) tag_resource_as('enabled') rescue tag_resource_as('disabled') @@ -180,8 +181,9 @@ def check_conversion_host_role def enable_conversion_host_role(vmware_vddk_package_url = nil, vmware_ssh_private_key = nil) raise "vddk_package_url is mandatory if transformation method is vddk" if vddk_transport_supported && vmware_vddk_package_url.nil? raise "ssh_private_key is mandatory if transformation_method is ssh" if ssh_transport_supported && vmware_ssh_private_key.nil? - playbook = "/usr/share/ovirt-ansible-v2v-conversion-host/playbooks/conversion_host_enable.yml" + playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_enable.yml" extra_vars = { + :v2v_host_type => resource.ext_management_system.emstype, :v2v_transport_method => vddk_transport_supported ? 'vddk' : 'ssh', :v2v_vddk_package_url => vmware_vddk_package_url, :v2v_ssh_private_key => vmware_ssh_private_key, @@ -193,8 +195,9 @@ def enable_conversion_host_role(vmware_vddk_package_url = nil, vmware_ssh_privat end def disable_conversion_host_role - playbook = "/usr/share/ovirt-ansible-v2v-conversion-host/playbooks/conversion_host_disable.yml" - ansible_playbook(playbook) + playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_disable.yml" + extra_vars = { :v2v_host_type => resource.ext_management_system.emstype } + ansible_playbook(playbook, extra_vars) ensure check_conversion_host_role end @@ -274,16 +277,39 @@ def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm # +extra_vars+ option should be a hash of key/value pairs which, if present, # will be passed to the '-e' flag. # - def ansible_playbook(playbook, extra_vars = {}) - command = "ansible-playbook #{playbook} -i #{ipaddress}" + def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) + host = hostname || ipaddress + + command = "ansible-playbook #{playbook} --inventory #{host}, --become -vvv" + + auth = authentication_type(auth_type) || authentications.first + command += " --user #{auth.userid}" + + case auth + when AuthUseridPassword + extra_vars[:ansible_ssh_pass] = auth.password + when AuthPrivateKey + ssh_private_key_file = Tempfile.new('ansible_key') + ssh_private_key_file.write(auth.auth_key) + command += " --private-key #{ssh_private_key_file.path}" + else + raise MiqException::MiqInvalidCredentialsError, _("Unknown auth type: #{auth.authtype}") + end + - extra_vars[:v2v_host_type] = resource.ext_management_system.emstype - extra_vars.each { |k, v| command += " -e '#{k}=#{v}'" } + extra_vars.each { |k, v| command += " --extra-vars '#{k}=#{v}'" } - connect_ssh { |ssu| ssu.shell_exec(command) } + _log.info("FDUPONT - Calling Ansible playbook: #{command}") + result = AwesomeSpawn.run(command) + _log.info("FDUPONT - Result of playbook (#{result.exit_status}):\nOUTPUT: #{result.output}\nERROR: #{result.error}") rescue => e _log.error("Ansible playbook '#{playbook}' failed for '#{resource.name}' with [#{e.class}: #{e}]") raise e + ensure + if !ssh_private_key_file.nil? && File.exist?(ssh_private_key_file) + ssh_private_key_file.close unless ssh_private_key_file.closed? + File.delete(ssh_private_key_file) + end end # Wrapper method for the various tag_resource_as_xxx methods. From ed39704a94514a62c58203fe37eaeeebbbd1c57a Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Tue, 2 Apr 2019 09:40:13 -0400 Subject: [PATCH 02/13] Prefer << over += --- app/models/conversion_host.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index c9a6806cf83..6607bea1d62 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -283,7 +283,7 @@ def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) command = "ansible-playbook #{playbook} --inventory #{host}, --become -vvv" auth = authentication_type(auth_type) || authentications.first - command += " --user #{auth.userid}" + command << " --user #{auth.userid}" case auth when AuthUseridPassword @@ -291,13 +291,12 @@ def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) when AuthPrivateKey ssh_private_key_file = Tempfile.new('ansible_key') ssh_private_key_file.write(auth.auth_key) - command += " --private-key #{ssh_private_key_file.path}" + command << " --private-key #{ssh_private_key_file.path}" else raise MiqException::MiqInvalidCredentialsError, _("Unknown auth type: #{auth.authtype}") end - - extra_vars.each { |k, v| command += " --extra-vars '#{k}=#{v}'" } + extra_vars.each { |k, v| command << " --extra-vars '#{k}=#{v}'" } _log.info("FDUPONT - Calling Ansible playbook: #{command}") result = AwesomeSpawn.run(command) From 4a0adcaabbd71fefa0f214acc232419244363f1f Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Fri, 5 Apr 2019 10:41:56 -0400 Subject: [PATCH 03/13] Use ansible-runner instead of ansible-playbook --- app/models/conversion_host.rb | 49 ++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 6607bea1d62..6a3cf76110d 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -171,7 +171,10 @@ def get_conversion_log(path) def check_conversion_host_role playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_check.yml" - extra_vars = { :v2v_host_type => resource.ext_management_system.emstype } + extra_vars = { + :v2v_host_type => resource.ext_management_system.emstype, + :v2v_transport_method => vddk_transport_supported ? 'vddk' : 'ssh' + } ansible_playbook(playbook, extra_vars) tag_resource_as('enabled') rescue @@ -196,7 +199,10 @@ def enable_conversion_host_role(vmware_vddk_package_url = nil, vmware_ssh_privat def disable_conversion_host_role playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_disable.yml" - extra_vars = { :v2v_host_type => resource.ext_management_system.emstype } + extra_vars = { + :v2v_host_type => resource.ext_management_system.emstype, + :v2v_transport_method => vddk_transport_supported ? 'vddk' : 'ssh' + } ansible_playbook(playbook, extra_vars) ensure check_conversion_host_role @@ -278,37 +284,44 @@ def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm # will be passed to the '-e' flag. # def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) - host = hostname || ipaddress + task = MiqTask.all.select { |t| t.context_data.present? && t.context_data[:conversion_host_id] == id }.sort_by(&:created_on).last + runner_basedir = Dir.mktmpdir("ansible-runner") + %w(env inventory).each { |d| Dir.mkdir(File.join(runner_basedir, d)) } - command = "ansible-playbook #{playbook} --inventory #{host}, --become -vvv" + host = hostname || ipaddress + File.open(File.join(runner_basedir, 'inventory', 'hosts'), 'w') { |f| f.write(host) } auth = authentication_type(auth_type) || authentications.first - command << " --user #{auth.userid}" - + extra_vars[:ansible_user] = auth.userid + extra_vars[:ansible_become] = true unless auth.userid == 'root' case auth when AuthUseridPassword - extra_vars[:ansible_ssh_pass] = auth.password + runner_password_file = File.join(runner_basedir, 'env', 'password') + File.open(runner_password_file, 'w') { |f| f.write("---\n\"Password:\\s*?$\": \"#{auth.password}\"") } when AuthPrivateKey - ssh_private_key_file = Tempfile.new('ansible_key') - ssh_private_key_file.write(auth.auth_key) - command << " --private-key #{ssh_private_key_file.path}" + runner_ssh_key_file = File.join(runner_basedir, 'env', 'ssh_key') + File.open(runner_ssh_key_file, 'w') { |f| f.write(auth.auth_key) } else raise MiqException::MiqInvalidCredentialsError, _("Unknown auth type: #{auth.authtype}") end - extra_vars.each { |k, v| command << " --extra-vars '#{k}=#{v}'" } + File.open(File.join(runner_basedir, 'env', 'extravars'), 'w') { |f| f.write(extra_vars.to_json) } - _log.info("FDUPONT - Calling Ansible playbook: #{command}") - result = AwesomeSpawn.run(command) - _log.info("FDUPONT - Result of playbook (#{result.exit_status}):\nOUTPUT: #{result.output}\nERROR: #{result.error}") + runner_args = {} + runner_args[:playbook] = playbook + runner_args[:ident] = "result" + runner_params = ['run', runner_basedir, :json, runner_args] + + result = AwesomeSpawn.run("ansible-runner", :params => runner_params) + raise result.error unless result.exit_status.zero? rescue => e _log.error("Ansible playbook '#{playbook}' failed for '#{resource.name}' with [#{e.class}: #{e}]") + task.status = 'Error' unless task.nil? raise e ensure - if !ssh_private_key_file.nil? && File.exist?(ssh_private_key_file) - ssh_private_key_file.close unless ssh_private_key_file.closed? - File.delete(ssh_private_key_file) - end + task.context_data[:ansible_output] = result.output unless task.nil? || result.nil? + File.delete(runner_password_file) if !runner_password_file.nil? && File.exist?(runner_password_file) + File.delete(runner_ssh_key_file) if !runner_ssh_key_file.nil? && File.exist?(runner_ssh_key_file) end # Wrapper method for the various tag_resource_as_xxx methods. From 1c8d12bb04601548cdf92a8a03c7838a716cd79f Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Fri, 5 Apr 2019 13:30:40 -0400 Subject: [PATCH 04/13] Fix rubocop --- app/models/conversion_host.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 6a3cf76110d..4ab29b378d3 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -286,7 +286,7 @@ def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) task = MiqTask.all.select { |t| t.context_data.present? && t.context_data[:conversion_host_id] == id }.sort_by(&:created_on).last runner_basedir = Dir.mktmpdir("ansible-runner") - %w(env inventory).each { |d| Dir.mkdir(File.join(runner_basedir, d)) } + %w[env inventory].each { |d| Dir.mkdir(File.join(runner_basedir, d)) } host = hostname || ipaddress File.open(File.join(runner_basedir, 'inventory', 'hosts'), 'w') { |f| f.write(host) } @@ -319,9 +319,9 @@ def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) task.status = 'Error' unless task.nil? raise e ensure - task.context_data[:ansible_output] = result.output unless task.nil? || result.nil? - File.delete(runner_password_file) if !runner_password_file.nil? && File.exist?(runner_password_file) - File.delete(runner_ssh_key_file) if !runner_ssh_key_file.nil? && File.exist?(runner_ssh_key_file) + task.context_data[:ansible_output] = result.output unless task.nil? || result.nil? + File.delete(runner_password_file) if !runner_password_file.nil? && File.exist?(runner_password_file) + File.delete(runner_ssh_key_file) if !runner_ssh_key_file.nil? && File.exist?(runner_ssh_key_file) end # Wrapper method for the various tag_resource_as_xxx methods. From ec54ccc75fd65cca3e0a42070e32153c8b57c632 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Fri, 5 Apr 2019 16:01:07 -0400 Subject: [PATCH 05/13] Update task context_data only for enable/disable --- app/models/conversion_host.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 4ab29b378d3..4f73df18209 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -175,7 +175,7 @@ def check_conversion_host_role :v2v_host_type => resource.ext_management_system.emstype, :v2v_transport_method => vddk_transport_supported ? 'vddk' : 'ssh' } - ansible_playbook(playbook, extra_vars) + ansible_playbook(playbook, extra_vars, false) tag_resource_as('enabled') rescue tag_resource_as('disabled') @@ -283,8 +283,8 @@ def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm # +extra_vars+ option should be a hash of key/value pairs which, if present, # will be passed to the '-e' flag. # - def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) - task = MiqTask.all.select { |t| t.context_data.present? && t.context_data[:conversion_host_id] == id }.sort_by(&:created_on).last + def ansible_playbook(playbook, extra_vars = {}, update_task = true, auth_type = nil) + task = MiqTask.all.select { |t| t.context_data.present? && t.context_data[:conversion_host_id] == id }.sort_by(&:created_on).last if update_task runner_basedir = Dir.mktmpdir("ansible-runner") %w[env inventory].each { |d| Dir.mkdir(File.join(runner_basedir, d)) } @@ -313,13 +313,15 @@ def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) runner_params = ['run', runner_basedir, :json, runner_args] result = AwesomeSpawn.run("ansible-runner", :params => runner_params) - raise result.error unless result.exit_status.zero? + raise unless result.exit_status.zero? rescue => e - _log.error("Ansible playbook '#{playbook}' failed for '#{resource.name}' with [#{e.class}: #{e}]") - task.status = 'Error' unless task.nil? + errormsg = "Ansible playbook '#{playbook}' failed for '#{resource.name}' with [#{e.class}: #{e}]" + _log.error(errormsg) + task.error(errormsg) unless task.nil? raise e ensure - task.context_data[:ansible_output] = result.output unless task.nil? || result.nil? + context = task.context_data + task.update_context(task.context_data.merge!(:ansible_output => result.output)) unless task.nil? || result.nil? File.delete(runner_password_file) if !runner_password_file.nil? && File.exist?(runner_password_file) File.delete(runner_ssh_key_file) if !runner_ssh_key_file.nil? && File.exist?(runner_ssh_key_file) end From 3d6e5ff765bcb3326c341b709357843244f60862 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Fri, 5 Apr 2019 16:19:33 -0400 Subject: [PATCH 06/13] Fix rubocop --- app/models/conversion_host.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 4f73df18209..97640b3702f 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -317,11 +317,10 @@ def ansible_playbook(playbook, extra_vars = {}, update_task = true, auth_type = rescue => e errormsg = "Ansible playbook '#{playbook}' failed for '#{resource.name}' with [#{e.class}: #{e}]" _log.error(errormsg) - task.error(errormsg) unless task.nil? + task&.error(errormsg) raise e ensure - context = task.context_data - task.update_context(task.context_data.merge!(:ansible_output => result.output)) unless task.nil? || result.nil? + task&.update_context(task.context_data.merge!(:ansible_output => result.output)) unless result.nil? File.delete(runner_password_file) if !runner_password_file.nil? && File.exist?(runner_password_file) File.delete(runner_ssh_key_file) if !runner_ssh_key_file.nil? && File.exist?(runner_ssh_key_file) end From 707d7f556b1b2cdce69501b073eec58d304cc2d3 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Mon, 15 Apr 2019 10:23:44 -0400 Subject: [PATCH 07/13] Use source_transport_method --- app/models/conversion_host.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 97640b3702f..efcc18c65e0 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -173,7 +173,7 @@ def check_conversion_host_role playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_check.yml" extra_vars = { :v2v_host_type => resource.ext_management_system.emstype, - :v2v_transport_method => vddk_transport_supported ? 'vddk' : 'ssh' + :v2v_transport_method => source_transport_method } ansible_playbook(playbook, extra_vars, false) tag_resource_as('enabled') @@ -187,7 +187,7 @@ def enable_conversion_host_role(vmware_vddk_package_url = nil, vmware_ssh_privat playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_enable.yml" extra_vars = { :v2v_host_type => resource.ext_management_system.emstype, - :v2v_transport_method => vddk_transport_supported ? 'vddk' : 'ssh', + :v2v_transport_method => source_transport_method, :v2v_vddk_package_url => vmware_vddk_package_url, :v2v_ssh_private_key => vmware_ssh_private_key, :v2v_ca_bundle => resource.ext_management_system.connection_configurations['default'].certificate_authority @@ -201,7 +201,7 @@ def disable_conversion_host_role playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_disable.yml" extra_vars = { :v2v_host_type => resource.ext_management_system.emstype, - :v2v_transport_method => vddk_transport_supported ? 'vddk' : 'ssh' + :v2v_transport_method => source_transport_method } ansible_playbook(playbook, extra_vars) ensure From 2565bc553dff638dc15fed416fb093d3e5294485 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Mon, 15 Apr 2019 17:01:20 -0400 Subject: [PATCH 08/13] Add specs --- app/models/conversion_host.rb | 4 +- spec/models/conversion_host_spec.rb | 94 ++++++++++++++++++++++++++++- 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index efcc18c65e0..5a0152f28f2 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -182,8 +182,8 @@ def check_conversion_host_role end def enable_conversion_host_role(vmware_vddk_package_url = nil, vmware_ssh_private_key = nil) - raise "vddk_package_url is mandatory if transformation method is vddk" if vddk_transport_supported && vmware_vddk_package_url.nil? - raise "ssh_private_key is mandatory if transformation_method is ssh" if ssh_transport_supported && vmware_ssh_private_key.nil? + raise "vmware_vddk_package_url is mandatory if transformation method is vddk" if vddk_transport_supported && vmware_vddk_package_url.nil? + raise "vmware_ssh_private_key is mandatory if transformation_method is ssh" if ssh_transport_supported && vmware_ssh_private_key.nil? playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_enable.yml" extra_vars = { :v2v_host_type => resource.ext_management_system.emstype, diff --git a/spec/models/conversion_host_spec.rb b/spec/models/conversion_host_spec.rb index 157a6b35de7..d21a97ddadf 100644 --- a/spec/models/conversion_host_spec.rb +++ b/spec/models/conversion_host_spec.rb @@ -163,12 +163,58 @@ it_behaves_like "#check_ssh_connection" end end + + context "#ansible_playbook" do + it "check_conversion_host_role calls ansible_playbook with extra_vars" do + check_playbook = '/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_check.yml' + check_extra_vars = { + :v2v_host_type => 'rhevm', + :v2v_transport_method => 'vddk' + } + expect(conversion_host).to receive(:ansible_playbook).with(check_playbook, check_extra_vars, false) + conversion_host.check_conversion_host_role + end + + it "disable_conversion_host_role calls ansible_playbook with extra_vars" do + disable_playbook = '/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_disable.yml' + check_playbook = '/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_check.yml' + disable_extra_vars = { + :v2v_host_type => 'rhevm', + :v2v_transport_method => 'vddk' + } + check_extra_vars = disable_extra_vars + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(disable_playbook, disable_extra_vars) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, false) + conversion_host.disable_conversion_host_role + end + + it "enable_conversion_host_role raises if vmware_vddk_package_url is nil" do + expect { conversion_host.enable_conversion_host_role }.to raise_error("vmware_vddk_package_url is mandatory if transformation method is vddk") + end + + it "enable_conversion_host_role calls ansible_playbook with extra_vars" do + enable_playbook = '/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_enable.yml' + check_playbook = '/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_check.yml' + enable_extra_vars = { + :v2v_host_type => 'rhevm', + :v2v_transport_method => 'vddk', + :v2v_vddk_package_url => 'http://file.example.com/vddk-stable.tar.gz' + } + check_extra_vars = { + :v2v_host_type => 'rhevm', + :v2v_transport_method => 'vddk' + } + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(enable_playbook, enable_extra_vars) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, false) + conversion_host.enable_conversion_host_role('http://file.example.com/vddk-stable.tar.gz', nil) + end + end end context "resource provider is openstack" do let(:ems) { FactoryBot.create(:ems_openstack, :zone => FactoryBot.create(:zone)) } let(:vm) { FactoryBot.create(:vm_openstack, :ext_management_system => ems) } - let(:conversion_host) { FactoryBot.create(:conversion_host, :resource => vm, :vddk_transport_supported => true) } + let(:conversion_host) { FactoryBot.create(:conversion_host, :resource => vm, :ssh_transport_supported => true) } context "ems authentications is empty" do it { expect(conversion_host.check_ssh_connection).to be(false) } @@ -186,6 +232,52 @@ it_behaves_like "#check_ssh_connection" end + + context "#ansible_playbook" do + it "check_conversion_host_role calls ansible_playbook with extra_vars" do + check_playbook = '/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_check.yml' + check_extra_vars = { + :v2v_host_type => 'openstack', + :v2v_transport_method => 'ssh' + } + expect(conversion_host).to receive(:ansible_playbook).with(check_playbook, check_extra_vars, false) + conversion_host.check_conversion_host_role + end + + it "disable_conversion_host_role calls ansible_playbook with extra_vars" do + disable_playbook = '/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_disable.yml' + check_playbook = '/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_check.yml' + disable_extra_vars = { + :v2v_host_type => 'openstack', + :v2v_transport_method => 'ssh' + } + check_extra_vars = disable_extra_vars + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(disable_playbook, disable_extra_vars) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, false) + conversion_host.disable_conversion_host_role + end + + it "enable_conversion_host_role raises if vmware_ssh_private_key is nil" do + expect { conversion_host.enable_conversion_host_role }.to raise_error("vmware_ssh_private_key is mandatory if transformation_method is ssh") + end + + it "enable_conversion_host_role calls ansible_playbook with extra_vars" do + enable_playbook = '/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_enable.yml' + check_playbook = '/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_check.yml' + enable_extra_vars = { + :v2v_host_type => 'openstack', + :v2v_transport_method => 'ssh', + :v2v_ssh_private_key => 'fake ssh private key' + } + check_extra_vars = { + :v2v_host_type => 'openstack', + :v2v_transport_method => 'ssh' + } + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(enable_playbook, enable_extra_vars) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, false) + conversion_host.enable_conversion_host_role(nil, 'fake ssh private key') + end + end end context "address validation" do From b41be35c2e1dcf3df2e926086e4e101114e550e6 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Wed, 17 Apr 2019 16:42:38 -0400 Subject: [PATCH 09/13] Reverting to using ansible-playbook --- app/models/conversion_host.rb | 46 ++++++++++++++--------------- spec/models/conversion_host_spec.rb | 12 ++++---- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 5a0152f28f2..cf8116cbc86 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -175,7 +175,7 @@ def check_conversion_host_role :v2v_host_type => resource.ext_management_system.emstype, :v2v_transport_method => source_transport_method } - ansible_playbook(playbook, extra_vars, false) + ansible_playbook(playbook, extra_vars) tag_resource_as('enabled') rescue tag_resource_as('disabled') @@ -283,46 +283,46 @@ def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm # +extra_vars+ option should be a hash of key/value pairs which, if present, # will be passed to the '-e' flag. # - def ansible_playbook(playbook, extra_vars = {}, update_task = true, auth_type = nil) - task = MiqTask.all.select { |t| t.context_data.present? && t.context_data[:conversion_host_id] == id }.sort_by(&:created_on).last if update_task - runner_basedir = Dir.mktmpdir("ansible-runner") - %w[env inventory].each { |d| Dir.mkdir(File.join(runner_basedir, d)) } + def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) + task = MiqTask.all.select { |t| t.context_data.present? && t.context_data[:conversion_host_id] == id }.sort_by(&:created_on).last host = hostname || ipaddress - File.open(File.join(runner_basedir, 'inventory', 'hosts'), 'w') { |f| f.write(host) } + + command = "ansible-playbook #{playbook} --inventory #{host}, --become --extra-vars=\"ansible_ssh_common_args='-o StrictHostKeyChecking=no'\"" auth = authentication_type(auth_type) || authentications.first - extra_vars[:ansible_user] = auth.userid - extra_vars[:ansible_become] = true unless auth.userid == 'root' + command += " --user #{auth.userid}" + case auth when AuthUseridPassword - runner_password_file = File.join(runner_basedir, 'env', 'password') - File.open(runner_password_file, 'w') { |f| f.write("---\n\"Password:\\s*?$\": \"#{auth.password}\"") } + extra_vars[:ansible_ssh_pass] = auth.password when AuthPrivateKey - runner_ssh_key_file = File.join(runner_basedir, 'env', 'ssh_key') - File.open(runner_ssh_key_file, 'w') { |f| f.write(auth.auth_key) } + ssh_private_key_file = Tempfile.new('ansible_key') + ssh_private_key_file.write(auth.auth_key) + ssh_private_key_file.close + command += " --private-key #{ssh_private_key_file.path}" else raise MiqException::MiqInvalidCredentialsError, _("Unknown auth type: #{auth.authtype}") end - File.open(File.join(runner_basedir, 'env', 'extravars'), 'w') { |f| f.write(extra_vars.to_json) } + extra_vars.each { |k, v| command += " --extra-vars '#{k}=#{v}'" } - runner_args = {} - runner_args[:playbook] = playbook - runner_args[:ident] = "result" - runner_params = ['run', runner_basedir, :json, runner_args] - - result = AwesomeSpawn.run("ansible-runner", :params => runner_params) - raise unless result.exit_status.zero? + result = AwesomeSpawn.run(command) + raise unless result.exit_status.zero? rescue => e errormsg = "Ansible playbook '#{playbook}' failed for '#{resource.name}' with [#{e.class}: #{e}]" _log.error(errormsg) task&.error(errormsg) raise e ensure - task&.update_context(task.context_data.merge!(:ansible_output => result.output)) unless result.nil? - File.delete(runner_password_file) if !runner_password_file.nil? && File.exist?(runner_password_file) - File.delete(runner_ssh_key_file) if !runner_ssh_key_file.nil? && File.exist?(runner_ssh_key_file) + unless result.nil? + ansible_output_name = playbook.split('/').last.split('.').first + task&.update_context(task.context_data.merge!(ansible_output_name => result.output)) + end + if !ssh_private_key_file.nil? && File.exist?(ssh_private_key_file.path) + ssh_private_key_file.close unless ssh_private_key_file.closed? + File.delete(ssh_private_key_file) + end end # Wrapper method for the various tag_resource_as_xxx methods. diff --git a/spec/models/conversion_host_spec.rb b/spec/models/conversion_host_spec.rb index d21a97ddadf..dc216568c04 100644 --- a/spec/models/conversion_host_spec.rb +++ b/spec/models/conversion_host_spec.rb @@ -171,7 +171,7 @@ :v2v_host_type => 'rhevm', :v2v_transport_method => 'vddk' } - expect(conversion_host).to receive(:ansible_playbook).with(check_playbook, check_extra_vars, false) + expect(conversion_host).to receive(:ansible_playbook).with(check_playbook, check_extra_vars) conversion_host.check_conversion_host_role end @@ -184,7 +184,7 @@ } check_extra_vars = disable_extra_vars expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(disable_playbook, disable_extra_vars) - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, false) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars) conversion_host.disable_conversion_host_role end @@ -205,7 +205,7 @@ :v2v_transport_method => 'vddk' } expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(enable_playbook, enable_extra_vars) - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, false) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars) conversion_host.enable_conversion_host_role('http://file.example.com/vddk-stable.tar.gz', nil) end end @@ -240,7 +240,7 @@ :v2v_host_type => 'openstack', :v2v_transport_method => 'ssh' } - expect(conversion_host).to receive(:ansible_playbook).with(check_playbook, check_extra_vars, false) + expect(conversion_host).to receive(:ansible_playbook).with(check_playbook, check_extra_vars) conversion_host.check_conversion_host_role end @@ -253,7 +253,7 @@ } check_extra_vars = disable_extra_vars expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(disable_playbook, disable_extra_vars) - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, false) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars) conversion_host.disable_conversion_host_role end @@ -274,7 +274,7 @@ :v2v_transport_method => 'ssh' } expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(enable_playbook, enable_extra_vars) - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, false) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars) conversion_host.enable_conversion_host_role(nil, 'fake ssh private key') end end From 11372188e54ad2f2c298030f7f9399c984eb6872 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Wed, 17 Apr 2019 16:47:32 -0400 Subject: [PATCH 10/13] Use << instead of += --- app/models/conversion_host.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index cf8116cbc86..f9e2f4d2de0 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -291,7 +291,7 @@ def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) command = "ansible-playbook #{playbook} --inventory #{host}, --become --extra-vars=\"ansible_ssh_common_args='-o StrictHostKeyChecking=no'\"" auth = authentication_type(auth_type) || authentications.first - command += " --user #{auth.userid}" + command << " --user #{auth.userid}" case auth when AuthUseridPassword @@ -300,12 +300,12 @@ def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) ssh_private_key_file = Tempfile.new('ansible_key') ssh_private_key_file.write(auth.auth_key) ssh_private_key_file.close - command += " --private-key #{ssh_private_key_file.path}" + command << " --private-key #{ssh_private_key_file.path}" else raise MiqException::MiqInvalidCredentialsError, _("Unknown auth type: #{auth.authtype}") end - extra_vars.each { |k, v| command += " --extra-vars '#{k}=#{v}'" } + extra_vars.each { |k, v| command << " --extra-vars '#{k}=#{v}'" } result = AwesomeSpawn.run(command) raise unless result.exit_status.zero? From ea531cd2b9da011f02767137e9744f6ca9435fca Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Wed, 17 Apr 2019 16:51:19 -0400 Subject: [PATCH 11/13] Fix rubocop --- app/models/conversion_host.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index f9e2f4d2de0..e66d71de9ff 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -308,7 +308,7 @@ def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) extra_vars.each { |k, v| command << " --extra-vars '#{k}=#{v}'" } result = AwesomeSpawn.run(command) - raise unless result.exit_status.zero? + raise unless result.exit_status.zero? rescue => e errormsg = "Ansible playbook '#{playbook}' failed for '#{resource.name}' with [#{e.class}: #{e}]" _log.error(errormsg) From 0ed69825268725bffe19fd05284218f562e83734 Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Thu, 18 Apr 2019 11:44:12 -0400 Subject: [PATCH 12/13] Pass the task id to ansible_playbook. --- app/models/conversion_host.rb | 37 +++++++++----------- app/models/conversion_host/configurations.rb | 6 ++-- spec/models/conversion_host_spec.rb | 28 +++++++-------- 3 files changed, 33 insertions(+), 38 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index e66d71de9ff..280cd6b33ee 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -169,19 +169,19 @@ def get_conversion_log(path) raise "Could not get conversion log '#{path}' from '#{resource.name}' with [#{e.class}: #{e}" end - def check_conversion_host_role + def check_conversion_host_role(miq_task_id = nil) playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_check.yml" extra_vars = { :v2v_host_type => resource.ext_management_system.emstype, :v2v_transport_method => source_transport_method } - ansible_playbook(playbook, extra_vars) + ansible_playbook(playbook, extra_vars, miq_task_id) tag_resource_as('enabled') rescue tag_resource_as('disabled') end - def enable_conversion_host_role(vmware_vddk_package_url = nil, vmware_ssh_private_key = nil) + def enable_conversion_host_role(vmware_vddk_package_url = nil, vmware_ssh_private_key = nil, miq_task_id = nil) raise "vmware_vddk_package_url is mandatory if transformation method is vddk" if vddk_transport_supported && vmware_vddk_package_url.nil? raise "vmware_ssh_private_key is mandatory if transformation_method is ssh" if ssh_transport_supported && vmware_ssh_private_key.nil? playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_enable.yml" @@ -192,20 +192,20 @@ def enable_conversion_host_role(vmware_vddk_package_url = nil, vmware_ssh_privat :v2v_ssh_private_key => vmware_ssh_private_key, :v2v_ca_bundle => resource.ext_management_system.connection_configurations['default'].certificate_authority }.compact - ansible_playbook(playbook, extra_vars) + ansible_playbook(playbook, extra_vars, miq_task_id) ensure - check_conversion_host_role + check_conversion_host_role(miq_task_id) end - def disable_conversion_host_role + def disable_conversion_host_role(miq_task_id = nil) playbook = "/usr/share/v2v-conversion-host-ansible/playbooks/conversion_host_disable.yml" extra_vars = { :v2v_host_type => resource.ext_management_system.emstype, :v2v_transport_method => source_transport_method } - ansible_playbook(playbook, extra_vars) + ansible_playbook(playbook, extra_vars, miq_task_id) ensure - check_conversion_host_role + check_conversion_host_role(miq_task_id) end private @@ -283,8 +283,8 @@ def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm # +extra_vars+ option should be a hash of key/value pairs which, if present, # will be passed to the '-e' flag. # - def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) - task = MiqTask.all.select { |t| t.context_data.present? && t.context_data[:conversion_host_id] == id }.sort_by(&:created_on).last + def ansible_playbook(playbook, extra_vars = {}, miq_task_id = nil, auth_type = nil) + task = MiqTask.find(miq_task_id) if miq_task_id.present? host = hostname || ipaddress @@ -298,8 +298,11 @@ def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) extra_vars[:ansible_ssh_pass] = auth.password when AuthPrivateKey ssh_private_key_file = Tempfile.new('ansible_key') - ssh_private_key_file.write(auth.auth_key) - ssh_private_key_file.close + begin + ssh_private_key_file.write(auth.auth_key) + ensure + ssh_private_key_file.close + end command << " --private-key #{ssh_private_key_file.path}" else raise MiqException::MiqInvalidCredentialsError, _("Unknown auth type: #{auth.authtype}") @@ -309,20 +312,12 @@ def ansible_playbook(playbook, extra_vars = {}, auth_type = nil) result = AwesomeSpawn.run(command) raise unless result.exit_status.zero? - rescue => e - errormsg = "Ansible playbook '#{playbook}' failed for '#{resource.name}' with [#{e.class}: #{e}]" - _log.error(errormsg) - task&.error(errormsg) - raise e ensure unless result.nil? ansible_output_name = playbook.split('/').last.split('.').first task&.update_context(task.context_data.merge!(ansible_output_name => result.output)) end - if !ssh_private_key_file.nil? && File.exist?(ssh_private_key_file.path) - ssh_private_key_file.close unless ssh_private_key_file.closed? - File.delete(ssh_private_key_file) - end + File.delete(ssh_private_key_file) if ssh_private_key_file.present? && File.exist?(ssh_private_key_file.path) end # Wrapper method for the various tag_resource_as_xxx methods. diff --git a/app/models/conversion_host/configurations.rb b/app/models/conversion_host/configurations.rb index 348c380f36c..a8b2b6246b4 100644 --- a/app/models/conversion_host/configurations.rb +++ b/app/models/conversion_host/configurations.rb @@ -55,7 +55,7 @@ def enable_queue(params, auth_user = nil) def enable(params, auth_user = nil) params = params.symbolize_keys - _log.info("Enabling a conversion_host with parameters: #{params}") + _log.debug("Enabling a conversion_host with parameters: #{params}") params.delete(:task_id) # In case this is being called through *_queue which will stick in a :task_id miq_task_id = params.delete(:miq_task_id) # The miq_queue.activate_miq_task will stick in a :miq_task_id @@ -78,7 +78,7 @@ def enable(params, auth_user = nil) ) end - conversion_host.enable_conversion_host_role(vmware_vddk_package_url, vmware_ssh_private_key) + conversion_host.enable_conversion_host_role(vmware_vddk_package_url, vmware_ssh_private_key, miq_task_id) conversion_host.save! if miq_task_id @@ -102,7 +102,7 @@ def disable_queue(auth_user = nil) def disable resource_info = "type=#{resource.class.name} id=#{resource.id}" - _log.info("Disabling a conversion_host #{resource_info}") + _log.debug("Disabling a conversion_host #{resource_info}") disable_conversion_host_role destroy! diff --git a/spec/models/conversion_host_spec.rb b/spec/models/conversion_host_spec.rb index dc216568c04..8078ecbbe54 100644 --- a/spec/models/conversion_host_spec.rb +++ b/spec/models/conversion_host_spec.rb @@ -171,8 +171,8 @@ :v2v_host_type => 'rhevm', :v2v_transport_method => 'vddk' } - expect(conversion_host).to receive(:ansible_playbook).with(check_playbook, check_extra_vars) - conversion_host.check_conversion_host_role + expect(conversion_host).to receive(:ansible_playbook).with(check_playbook, check_extra_vars, 1) + conversion_host.check_conversion_host_role(1) end it "disable_conversion_host_role calls ansible_playbook with extra_vars" do @@ -183,9 +183,9 @@ :v2v_transport_method => 'vddk' } check_extra_vars = disable_extra_vars - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(disable_playbook, disable_extra_vars) - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars) - conversion_host.disable_conversion_host_role + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(disable_playbook, disable_extra_vars, 1) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, 1) + conversion_host.disable_conversion_host_role(1) end it "enable_conversion_host_role raises if vmware_vddk_package_url is nil" do @@ -204,8 +204,8 @@ :v2v_host_type => 'rhevm', :v2v_transport_method => 'vddk' } - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(enable_playbook, enable_extra_vars) - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(enable_playbook, enable_extra_vars, nil) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, nil) conversion_host.enable_conversion_host_role('http://file.example.com/vddk-stable.tar.gz', nil) end end @@ -240,8 +240,8 @@ :v2v_host_type => 'openstack', :v2v_transport_method => 'ssh' } - expect(conversion_host).to receive(:ansible_playbook).with(check_playbook, check_extra_vars) - conversion_host.check_conversion_host_role + expect(conversion_host).to receive(:ansible_playbook).with(check_playbook, check_extra_vars, 1) + conversion_host.check_conversion_host_role(1) end it "disable_conversion_host_role calls ansible_playbook with extra_vars" do @@ -252,9 +252,9 @@ :v2v_transport_method => 'ssh' } check_extra_vars = disable_extra_vars - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(disable_playbook, disable_extra_vars) - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars) - conversion_host.disable_conversion_host_role + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(disable_playbook, disable_extra_vars, 1) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, 1) + conversion_host.disable_conversion_host_role(1) end it "enable_conversion_host_role raises if vmware_ssh_private_key is nil" do @@ -273,8 +273,8 @@ :v2v_host_type => 'openstack', :v2v_transport_method => 'ssh' } - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(enable_playbook, enable_extra_vars) - expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(enable_playbook, enable_extra_vars, nil) + expect(conversion_host).to receive(:ansible_playbook).once.ordered.with(check_playbook, check_extra_vars, nil) conversion_host.enable_conversion_host_role(nil, 'fake ssh private key') end end From 4d18a99cfe4171f27fdd979253063c05881935db Mon Sep 17 00:00:00 2001 From: Fabien Dupont Date: Thu, 18 Apr 2019 12:33:25 -0400 Subject: [PATCH 13/13] Simplify code --- app/models/conversion_host.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/app/models/conversion_host.rb b/app/models/conversion_host.rb index 280cd6b33ee..7426e466f72 100644 --- a/app/models/conversion_host.rb +++ b/app/models/conversion_host.rb @@ -313,11 +313,8 @@ def ansible_playbook(playbook, extra_vars = {}, miq_task_id = nil, auth_type = n result = AwesomeSpawn.run(command) raise unless result.exit_status.zero? ensure - unless result.nil? - ansible_output_name = playbook.split('/').last.split('.').first - task&.update_context(task.context_data.merge!(ansible_output_name => result.output)) - end - File.delete(ssh_private_key_file) if ssh_private_key_file.present? && File.exist?(ssh_private_key_file.path) + task&.update_context(task.context_data.merge!(File.basename(playbook, '.yml') => result.output)) unless result.nil? + ssh_private_key_file&.unlink end # Wrapper method for the various tag_resource_as_xxx methods.