From 3b307fec3d58b094f7d8473ff99d08197d4671aa Mon Sep 17 00:00:00 2001 From: Robbert-Jan Sperna Weiland Date: Wed, 27 Jun 2018 10:57:59 +0200 Subject: [PATCH 1/2] Support VM password on config + WinRM loop bug --- .../action/read_winrm_info.rb | 14 +++-- lib/vagrant-cloudstack/action/run_instance.rb | 53 ++++++++++++++----- 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/lib/vagrant-cloudstack/action/read_winrm_info.rb b/lib/vagrant-cloudstack/action/read_winrm_info.rb index a643c09d..993238c1 100644 --- a/lib/vagrant-cloudstack/action/read_winrm_info.rb +++ b/lib/vagrant-cloudstack/action/read_winrm_info.rb @@ -43,15 +43,13 @@ def read_winrm_info(cloudstack, machine) # the username via winrm_info ... yet ;-) # Read password from file into domain_config - if domain_config.vm_password.nil? - vmcredentials_file = machine.data_dir.join("vmcredentials") - if vmcredentials_file.file? - vmcredentials_password = nil - File.read(vmcredentials_file).each_line do |line| - vmcredentials_password = line.strip - end - domain_config.vm_password = vmcredentials_password + vmcredentials_file = machine.data_dir.join("vmcredentials") + if vmcredentials_file.file? + vmcredentials_password = nil + File.read(vmcredentials_file).each_line do |line| + vmcredentials_password = line.strip end + domain_config.vm_password = vmcredentials_password end transport_info = transport_info.merge({ diff --git a/lib/vagrant-cloudstack/action/run_instance.rb b/lib/vagrant-cloudstack/action/run_instance.rb index 12917d52..987e961b 100644 --- a/lib/vagrant-cloudstack/action/run_instance.rb +++ b/lib/vagrant-cloudstack/action/run_instance.rb @@ -37,7 +37,9 @@ def call(env) store_volumes - store_password + password = wait_for_password + + store_password(password) configure_networking @@ -381,6 +383,14 @@ def get_communicator_short_name(communicator) communicator_short_names[communicator.class.name] end + def get_communicator_connect_attempts(communicator) + communicator_connect_attempts = { + 'VagrantPlugins::CommunicatorSSH::Communicator' => 40, + 'VagrantPlugins::CommunicatorWinRM::Communicator' => 1 + } + communicator_connect_attempts[communicator.class.name] + end + def evaluate_pf_private_port return unless @domain_config.pf_private_port.nil? @@ -756,13 +766,14 @@ def generate_ssh_keypair(keyname, account = nil, domainid = nil, projectid = nil @domain_config.keypair = sshkeypair['name'] end - def store_password + def wait_for_password password = nil - if @server.password_enabled and @server.respond_to?('job_id') + + if @server.respond_to?('job_id') server_job_result = @env[:cloudstack_compute].query_async_job_result({:jobid => @server.job_id}) if server_job_result.nil? - @env[:ui].warn(' -- Failed to retrieve job_result for retrieving the password') - return + @env[:ui].warn(' -- Failed to retrieve job_result') + raise 'ERROR -- Failed to retrieve job_result' end while true @@ -776,15 +787,28 @@ def store_password end @env[:ui].info("Password of virtualmachine: #{password}") - # Set the password on the current communicator - @domain_config.vm_password = password + end - # Save password to file - vmcredentials_file = @env[:machine].data_dir.join('vmcredentials') - vmcredentials_file.open('w') do |f| - f.write("#{password}") - end + password + end + + def store_password(password) + unless @server.password_enabled + @env[:ui].info("VM is not password-enabled. Using virtualmachine password from config") + if @domain_config.vm_password.nil? + fail "No vm_password configured but VM is not password enabled either!" + end + password = @domain_config.vm_password end + + # Save password to file + vmcredentials_file = @env[:machine].data_dir.join('vmcredentials') + vmcredentials_file.open('w') do |f| + f.write("#{password}") + end + + # Set the password on the current communicator + @domain_config.vm_password = password end def store_volumes @@ -829,15 +853,18 @@ def wait_for_communicator_ready @env[:metrics]['instance_ssh_time'] = Util::Timer.time do # Wait for communicator to be ready. communicator_short_name = get_communicator_short_name(@env[:machine].communicate) + communicator_connect_attempts = get_communicator_connect_attempts(@env[:machine].communicate) @env[:ui].info( I18n.t('vagrant_cloudstack.waiting_for_communicator', communicator: communicator_short_name.to_s.upcase) ) - while true + connection_attempt = 0 + while connection_attempt Date: Fri, 7 Sep 2018 13:51:34 +0200 Subject: [PATCH 2/2] Testing improvements --- Gemfile.lock | 4 ++++ lib/vagrant-cloudstack/action/run_instance.rb | 12 ++++------ .../action/run_instance_spec.rb | 22 ++++++++++--------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 80e99067..2f301c3a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -46,6 +46,7 @@ GEM erubis (2.7.0) excon (0.62.0) ffi (1.9.23) + ffi (1.9.23-x64-mingw32) fission (0.5.0) CFPropertyList (~> 2.2) fog (2.0.0) @@ -230,6 +231,8 @@ GEM net-ssh (4.2.0) nokogiri (1.8.2) mini_portile2 (~> 2.3.0) + nokogiri (1.8.2-x64-mingw32) + mini_portile2 (~> 2.3.0) nori (2.6.0) rake (10.5.0) rb-fsevent (0.10.3) @@ -291,6 +294,7 @@ GEM PLATFORMS ruby + x64-mingw32 DEPENDENCIES coveralls diff --git a/lib/vagrant-cloudstack/action/run_instance.rb b/lib/vagrant-cloudstack/action/run_instance.rb index 987e961b..fb83d498 100644 --- a/lib/vagrant-cloudstack/action/run_instance.rb +++ b/lib/vagrant-cloudstack/action/run_instance.rb @@ -388,7 +388,7 @@ def get_communicator_connect_attempts(communicator) 'VagrantPlugins::CommunicatorSSH::Communicator' => 40, 'VagrantPlugins::CommunicatorWinRM::Communicator' => 1 } - communicator_connect_attempts[communicator.class.name] + return communicator_connect_attempts[communicator.class.name].nil? ? 1 : communicator_connect_attempts[communicator.class.name] end def evaluate_pf_private_port @@ -498,9 +498,8 @@ def apply_port_forwarding_rule(rule) if response['queryasyncjobresultresponse']['jobstatus'] != 0 port_forwarding_rule = response['queryasyncjobresultresponse']['jobresult']['portforwardingrule'] break - else - sleep 2 end + sleep 2 end rescue Fog::Compute::Cloudstack::Error => e raise Errors::FogError, :message => e.message @@ -727,9 +726,8 @@ def apply_firewall_rule(acl_name, options, response_string, type_string) if response['queryasyncjobresultresponse']['jobstatus'] != 0 firewall_rule = response['queryasyncjobresultresponse']['jobresult'][type_string] break - else - sleep 2 end + sleep 2 end rescue Fog::Compute::Cloudstack::Error => e if e.message =~ /The range specified,.*conflicts with rule/ @@ -772,7 +770,6 @@ def wait_for_password if @server.respond_to?('job_id') server_job_result = @env[:cloudstack_compute].query_async_job_result({:jobid => @server.job_id}) if server_job_result.nil? - @env[:ui].warn(' -- Failed to retrieve job_result') raise 'ERROR -- Failed to retrieve job_result' end @@ -781,9 +778,8 @@ def wait_for_password if server_job_result['queryasyncjobresultresponse']['jobstatus'] != 0 password = server_job_result['queryasyncjobresultresponse']['jobresult']['virtualmachine']['password'] break - else - sleep 2 end + sleep 2 end @env[:ui].info("Password of virtualmachine: #{password}") diff --git a/spec/vagrant-cloudstack/action/run_instance_spec.rb b/spec/vagrant-cloudstack/action/run_instance_spec.rb index 8b57ebe7..e7088e51 100644 --- a/spec/vagrant-cloudstack/action/run_instance_spec.rb +++ b/spec/vagrant-cloudstack/action/run_instance_spec.rb @@ -260,7 +260,11 @@ allow(machine).to receive(:provider_config).and_return(provider_config) expect(server).to receive(:wait_for).and_return(ready = true) - allow(server).to receive(:password_enabled).and_return(false) + allow(server).to receive(:password_enabled).and_return(true) + allow(server).to receive(:job_id).and_return(JOB_ID) + + expect(file).to receive(:write).with(GENERATED_PASSWORD) + allow(cloudstack_compute).to receive(:query_async_job_result).with(jobid: JOB_ID).and_return(fake_job_result) expect(cloudstack_compute).to receive(:servers).and_return(servers) allow(cloudstack_compute).to receive(:send).with(:list_zones, available: true).and_return(list_zones_response) allow(cloudstack_compute).to receive(:send).with(:list_service_offerings, listall: true) @@ -289,6 +293,7 @@ cfg.ssh_key = ssh_key cfg.security_groups = security_groups cfg.network_name = network_name + cfg.vm_password = GENERATED_PASSWORD end config.finalize! config.get_domain_config(:cloudstack) @@ -354,8 +359,8 @@ should eq true end end - end - + end + context 'in advanced zone' do let(:pf_ip_address) { nil } let(:pf_trusted_networks) { nil } @@ -377,6 +382,7 @@ cfg.pf_open_firewall = pf_open_firewall cfg.ssh_key = ssh_key cfg.disk_offering_name = disk_offering_name + cfg.vm_password = GENERATED_PASSWORD end config.finalize! config.get_domain_config(:cloudstack) @@ -385,8 +391,6 @@ let(:winrm_config) { double('VagrantPlugins::VagrantWinRM::WinRMConfig') } before(:each) do - allow(cloudstack_compute).to receive(:query_async_job_result).with(jobid: JOB_ID).and_return(fake_job_result) - allow(cloudstack_compute).to receive(:send).with(:list_networks, {}).and_return(list_networks_response) end @@ -427,11 +431,9 @@ end end - context 'with generated password' do + context 'with static password' do before(:each) do - expect(server).to receive(:password_enabled).and_return(true) - allow(server).to receive(:job_id).and_return(JOB_ID) - expect(file).to receive(:write).with(GENERATED_PASSWORD) + expect(server).to receive(:password_enabled).and_return(false) end it 'starts a vm' do @@ -606,4 +608,4 @@ end end end -end +end \ No newline at end of file