From 9177bd8a48e2813d1c9e80567fd554c8574b40cc Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Tue, 22 Apr 2014 11:29:22 -0700 Subject: [PATCH 1/4] Added WQL support to Vagrant communicator execute - Removed duplication for WQL specific commands --- plugins/communicators/winrm/communicator.rb | 27 +++++++++------------ plugins/communicators/winrm/shell.rb | 19 +++------------ 2 files changed, 16 insertions(+), 30 deletions(-) diff --git a/plugins/communicators/winrm/communicator.rb b/plugins/communicators/winrm/communicator.rb index c04bfbeb3..850b0eba7 100644 --- a/plugins/communicators/winrm/communicator.rb +++ b/plugins/communicators/winrm/communicator.rb @@ -57,11 +57,19 @@ module VagrantPlugins :command => command, :shell => :powershell }.merge(opts || {}) - exit_status = do_execute(command, opts[:shell], &block) - if opts[:error_check] && exit_status != 0 - raise_execution_error(opts, exit_status) + + if opts[:shell] == :powershell + script = File.expand_path("../scripts/command_alias.ps1", __FILE__) + script = File.read(script) + command = script << "\r\n" << command << "\r\nexit $LASTEXITCODE" end - exit_status + + output = shell.send(opts[:shell], command, &block) + + return output if opts[:shell] == :wql + exitcode = output[:exitcode] + raise_execution_error(opts, exitcode) if opts[:error_check] && exitcode != 0 + exitcode end alias_method :sudo, :execute @@ -103,17 +111,6 @@ module VagrantPlugins ) end - def do_execute(command, shell_type, &block) - if shell_type == :cmd - return shell.cmd(command, &block)[:exitcode] - end - - script = File.expand_path("../scripts/command_alias.ps1", __FILE__) - script = File.read(script) - command = script << "\r\n" << command << "\r\nexit $LASTEXITCODE" - shell.powershell(command, &block)[:exitcode] - end - def raise_execution_error(opts, exit_code) # The error classes expect the translation key to be _key, but that makes for an ugly # configuration parameter, so we set it here from `error_key` diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index e1bbb5da0..56fc4b992 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -57,8 +57,8 @@ module VagrantPlugins execute_shell(command, :cmd, &block) end - def wql(query) - execute_wql(query) + def wql(query, &block) + execute_shell(query, :wql, &block) end def upload(from, to) @@ -92,7 +92,7 @@ module VagrantPlugins protected def execute_shell(command, shell=:powershell, &block) - raise Errors::InvalidShell, shell: shell unless shell == :cmd || shell == :powershell + raise Errors::WinRMInvalidShell, :shell => shell unless [:powershell, :cmd, :wql].include?(shell) begin execute_shell_with_retry(command, shell, &block) @@ -108,22 +108,11 @@ module VagrantPlugins block.call(:stdout, out) if block_given? && out block.call(:stderr, err) if block_given? && err end - @logger.debug("Exit status: #{output[:exitcode].inspect}") + @logger.debug("Output: #{output.inspect}") return output end end - def execute_wql(query) - retryable(:tries => @max_tries, :on => @@exceptions_to_retry_on, :sleep => 10) do - @logger.debug("#executing wql: #{query}") - output = session.wql(query) - @logger.debug("wql result: #{output.inspect}") - return output - end - rescue => e - raise_winrm_exception(e, :wql, query) - end - def raise_winrm_exception(winrm_exception, shell, command) # If the error is a 401, we can return a more specific error message if winrm_exception.message.include?("401") From 728ec28f2d0434f446a1f662cd5765afe84ae95d Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Tue, 22 Apr 2014 11:54:34 -0700 Subject: [PATCH 2/4] Windows GuestNetwork directly uses the communicator - Fixed slight Law of Demeter violation - Make guest access through comm consistent --- .../guests/windows/cap/configure_networks.rb | 2 +- plugins/guests/windows/guest_network.rb | 20 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/plugins/guests/windows/cap/configure_networks.rb b/plugins/guests/windows/cap/configure_networks.rb index e8c12e7df..931c67345 100644 --- a/plugins/guests/windows/cap/configure_networks.rb +++ b/plugins/guests/windows/cap/configure_networks.rb @@ -11,7 +11,7 @@ module VagrantPlugins def self.configure_networks(machine, networks) @@logger.debug("Networks: #{networks.inspect}") - guest_network = GuestNetwork.new(machine.communicate.shell) + guest_network = GuestNetwork.new(machine.communicate) if machine.provider_name.to_s.start_with?("vmware") machine.ui.warn("Configuring secondary network adapters through VMware ") machine.ui.warn("on Windows is not yet supported. You will need to manually") diff --git a/plugins/guests/windows/guest_network.rb b/plugins/guests/windows/guest_network.rb index 4fb8a01d0..797bd5f4d 100644 --- a/plugins/guests/windows/guest_network.rb +++ b/plugins/guests/windows/guest_network.rb @@ -7,9 +7,9 @@ module VagrantPlugins PS_GET_WSMAN_VER = '((test-wsman).productversion.split(" ") | select -last 1).split("\.")[0]' WQL_NET_ADAPTERS_V2 = 'SELECT * FROM Win32_NetworkAdapter WHERE MACAddress IS NOT NULL' - def initialize(winrmshell) - @logger = Log4r::Logger.new("vagrant::windows::guestnetwork") - @winrmshell = winrmshell + def initialize(communicator) + @logger = Log4r::Logger.new("vagrant::windows::guestnetwork") + @communicator = communicator end # Returns an array of all NICs on the guest. Each array entry is a @@ -26,7 +26,7 @@ module VagrantPlugins def is_dhcp_enabled(nic_index) has_dhcp_enabled = false cmd = "Get-WmiObject -Class Win32_NetworkAdapterConfiguration -Filter \"Index=#{nic_index} and DHCPEnabled=True\"" - @winrmshell.powershell(cmd) do |type, line| + @communicator.execute(cmd) do |type, line| has_dhcp_enabled = !line.nil? end @logger.debug("NIC #{nic_index} has DHCP enabled: #{has_dhcp_enabled}") @@ -41,7 +41,7 @@ module VagrantPlugins @logger.info("Configuring NIC #{net_connection_id} for DHCP") if !is_dhcp_enabled(nic_index) netsh = "netsh interface ip set address \"#{net_connection_id}\" dhcp" - @winrmshell.powershell(netsh) + @communicator.execute(netsh) end end @@ -55,7 +55,7 @@ module VagrantPlugins @logger.info("Configuring NIC #{net_connection_id} using static ip #{ip}") #netsh interface ip set address "Local Area Connection 2" static 192.168.33.10 255.255.255.0 netsh = "netsh interface ip set address \"#{net_connection_id}\" static #{ip} #{netmask}" - @winrmshell.powershell(netsh) + @communicator.execute(netsh) end # Sets all networks on the guest to 'Work Network' mode. This is @@ -64,7 +64,7 @@ module VagrantPlugins def set_all_networks_to_work @logger.info("Setting all networks to 'Work Network'") command = File.read(File.expand_path("../scripts/set_work_network.ps1", __FILE__)) - @winrmshell.powershell(command) + @communicator.execute(command) end protected @@ -76,7 +76,7 @@ module VagrantPlugins def wsman_version @logger.debug("querying WSMan version") version = '' - @winrmshell.powershell(PS_GET_WSMAN_VER) do |type, line| + @communicator.execute(PS_GET_WSMAN_VER) do |type, line| version = version + "#{line}" if type == :stdout && !line.nil? end @logger.debug("wsman version: #{version}") @@ -93,7 +93,7 @@ module VagrantPlugins # Get all NICs that have a MAC address # http://msdn.microsoft.com/en-us/library/windows/desktop/aa394216(v=vs.85).aspx - adapters = @winrmshell.wql(WQL_NET_ADAPTERS_V2)[:win32_network_adapter] + adapters = @communicator.execute(WQL_NET_ADAPTERS_V2, { :shell => :wql } )[:win32_network_adapter] @logger.debug("#{adapters.inspect}") return adapters end @@ -108,7 +108,7 @@ module VagrantPlugins def network_adapters_v3_winrm command = File.read(File.expand_path("../scripts/winrs_v3_get_adapters.ps1", __FILE__)) output = "" - @winrmshell.powershell(command) do |type, line| + @communicator.execute(command) do |type, line| output = output + "#{line}" if type == :stdout && !line.nil? end From 62f5be49d574c8860602469b94357a5307170747 Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Tue, 22 Apr 2014 13:28:51 -0700 Subject: [PATCH 3/4] Cleaner DHCP detection for Windows guests --- plugins/guests/windows/guest_network.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/guests/windows/guest_network.rb b/plugins/guests/windows/guest_network.rb index 797bd5f4d..78327ed14 100644 --- a/plugins/guests/windows/guest_network.rb +++ b/plugins/guests/windows/guest_network.rb @@ -24,13 +24,13 @@ module VagrantPlugins # # @return [Boolean] def is_dhcp_enabled(nic_index) - has_dhcp_enabled = false - cmd = "Get-WmiObject -Class Win32_NetworkAdapterConfiguration -Filter \"Index=#{nic_index} and DHCPEnabled=True\"" - @communicator.execute(cmd) do |type, line| - has_dhcp_enabled = !line.nil? - end - @logger.debug("NIC #{nic_index} has DHCP enabled: #{has_dhcp_enabled}") - has_dhcp_enabled + cmd = <<-EOH + if (Get-WmiObject -Class Win32_NetworkAdapterConfiguration -Filter "Index=#{nic_index} and DHCPEnabled=True") { + exit 0 + } + exit 1 + EOH + @communicator.test(cmd) end # Configures the specified interface for DHCP From dbe73e842fb3a81313f5ad8f98e176f38d6f1143 Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Tue, 22 Apr 2014 14:03:07 -0700 Subject: [PATCH 4/4] Added guest network tests for Windows guests --- .../guests/windows/guest_network_test.rb | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 test/unit/plugins/guests/windows/guest_network_test.rb diff --git a/test/unit/plugins/guests/windows/guest_network_test.rb b/test/unit/plugins/guests/windows/guest_network_test.rb new file mode 100644 index 000000000..dd0f4f546 --- /dev/null +++ b/test/unit/plugins/guests/windows/guest_network_test.rb @@ -0,0 +1,50 @@ +require File.expand_path("../../../../base", __FILE__) + +require Vagrant.source_root.join("plugins/guests/windows/guest_network") + +describe "VagrantPlugins::GuestWindows::GuestNetwork" do + + let(:communicator) { double("communicator") } + let(:subject) { VagrantPlugins::GuestWindows::GuestNetwork.new(communicator) } + + describe ".is_dhcp_enabled" do + it "should query the NIC by ordinal index" do + expect(communicator).to receive(:test).with( + /.+Get-WmiObject -Class Win32_NetworkAdapterConfiguration -Filter "Index=7 and DHCPEnabled=True"/). + and_return(true) + expect(subject.is_dhcp_enabled(7)).to be_true + end + + it "should return false for non-DHCP NICs" do + expect(communicator).to receive(:test).with( + /.+Get-WmiObject -Class Win32_NetworkAdapterConfiguration -Filter "Index=8 and DHCPEnabled=True"/). + and_return(false) + expect(subject.is_dhcp_enabled(8)).to be_false + end + end + + describe ".configure_static_interface" do + it "should configure IP using netsh" do + expect(communicator).to receive(:execute).with( + "netsh interface ip set address \"Local Area Connection 2\" static 192.168.33.10 255.255.255.0"). + and_return(0) + subject.configure_static_interface(7, "Local Area Connection 2", "192.168.33.10", "255.255.255.0") + end + end + + describe ".configure_dhcp_interface" do + it "should configure DHCP when DHCP is disabled" do + allow(communicator).to receive(:test).and_return(false) # is DHCP enabled? + expect(communicator).to receive(:execute).with( + "netsh interface ip set address \"Local Area Connection 2\" dhcp"). + and_return(0) + subject.configure_dhcp_interface(7, "Local Area Connection 2") + end + + it "should not configure DHCP when DHCP is enabled" do + allow(communicator).to receive(:test).and_return(true) # is DHCP enabled? + expect(communicator).to_not receive(:execute) + subject.configure_dhcp_interface(7, "Local Area Connection 2") + end + end +end