matschaffer#450 rsync command doesn't work in windows with cygwin due…

… to broken ControlMaster support

lib/knife-solo/ssh_command.rb

@@ -1,9 +1,9 @@
-
 module KnifeSolo
   module SshCommand
 
     def self.load_deps
       require 'knife-solo/ssh_connection'
+      require 'knife-solo/tools'
       require 'net/ssh'
       require 'net/ssh/gateway'
     end
@@ -49,8 +49,7 @@ def self.included(other)
 
         option :ssh_control_master,
           :long => '--ssh-control-master SETTING',
-          :description => 'Control master setting to use when running rsync (use "no" to disable)',
-          :default => 'auto'
+          :description => 'Control master setting to use when running rsync (use "no" to disable)'
 
         option :identity_file,
           :short       => '-i IDENTITY_FILE',
@@ -115,6 +114,10 @@ def validate_ssh_options!
         host_descriptor[:user] ||= config[:ssh_user]
       end
 
+      if !config[:ssh_control_master]
+         config[:ssh_control_master] = KnifeSolo::Tools.cygwin_client? ? 'no' : 'auto'
+      end
+
       # NOTE: can't rely on default since it won't get called when invoked via knife bootstrap --solo
       if config[:ssh_keepalive_interval] && config[:ssh_keepalive_interval] <= 0
         ui.fatal '`--ssh-keepalive-interval` must be a positive number'

lib/knife-solo/tools.rb

@@ -1,33 +1,37 @@
-module KnifeSolo
-  module Tools
-    def system!(*command)
-      raise "Failed to launch command #{command}" unless system(*command)
-    end
-
-    def windows_client?
-      RbConfig::CONFIG['host_os'] =~ /mswin|mingw/
-    end
-
-    def config_value(key, default = nil)
-      Tools.config_value(config, key, default)
-    end
-
-    # Chef 10 compatible way of getting correct precedence for command line
-    # and configuration file options. Adds correct handling of `false` values
-    # to the original example in
-    # http://docs.opscode.com/breaking_changes_chef_11.html#knife-configuration-parameter-changes
-    def self.config_value(config, key, default = nil)
-      key = key.to_sym
-      if !config[key].nil?
-        config[key]
-      elsif !Chef::Config[:knife][key].nil?
-        # when Chef 10 support is dropped, this branch can be removed
-        # as Chef 11 automatically merges the values to the `config` hash
-        Chef::Config[:knife][key]
-      else
-        default
-      end
-    end
-
-  end
-end
+module KnifeSolo
+  module Tools
+    def system!(*command)
+      raise "Failed to launch command #{command}" unless system(*command)
+    end
+
+    def windows_client?
+      RbConfig::CONFIG['host_os'] =~ /mswin|mingw/
+    end
+
+    def self.cygwin_client?
+      RbConfig::CONFIG['host_os'] =~ /cygwin/
+    end
+
+    def config_value(key, default = nil)
+      Tools.config_value(config, key, default)
+    end
+
+    # Chef 10 compatible way of getting correct precedence for command line
+    # and configuration file options. Adds correct handling of `false` values
+    # to the original example in
+    # http://docs.opscode.com/breaking_changes_chef_11.html#knife-configuration-parameter-changes
+    def self.config_value(config, key, default = nil)
+      key = key.to_sym
+      if !config[key].nil?
+        config[key]
+      elsif !Chef::Config[:knife][key].nil?
+        # when Chef 10 support is dropped, this branch can be removed
+        # as Chef 11 automatically merges the values to the `config` hash
+        Chef::Config[:knife][key]
+      else
+        default
+      end
+    end
+
+  end
+end

test/ssh_command_test.rb

@@ -1,6 +1,7 @@
 require 'test_helper'
 
 require 'knife-solo/ssh_command'
+require 'knife-solo/tools'
 require 'chef/knife'
 require 'net/ssh'
 
@@ -9,6 +10,11 @@ class DummySshCommand < Chef::Knife
 end
 
 class SshCommandTest < TestCase
+
+  def setup
+    KnifeSolo::Tools.stubs(:cygwin_client?).returns(false)
+  end
+
   def test_separates_user_and_host
     assert_equal "ubuntu", command("[email protected]").user
     assert_equal "10.0.0.1", command("[email protected]").host
@@ -69,7 +75,7 @@ def test_try_connection_with_gateway_connects_using_ssh_gateway
     cmd = command("10.0.0.1", "--ssh-gateway=user@gateway")
     ssh_mock = mock 'ssh_mock'
     Net::SSH::Gateway.expects(:new).with('gateway', 'user').returns(ssh_mock)
-    ssh_mock.expects(:ssh).with(cmd.host, cmd.user, cmd.connection_options.except(:gateway))
+    ssh_mock.expects(:ssh).with(cmd.host, cmd.user, cmd.connection_options.select{|o| o != :gateway})
 
     Net::SSH.expects(:start).never
 
@@ -164,6 +170,26 @@ def test_builds_cli_ssh_args
     assert_equal "[email protected] -o ControlMaster=auto -o ControlPath=#{cmd.ssh_control_path} -o ControlPersist=3600", cmd.ssh_args
   end
 
+  def test_no_control_master_by_default_in_cygwin
+    DummySshCommand.any_instance.stubs(:try_connection)
+    DummySshCommand.any_instance.stubs(:ssh_control_path).returns('/path')
+    KnifeSolo::Tools.stubs(:cygwin_client?).returns(false)
+    cmd = command_internal('[email protected]')
+    cmd.validate_ssh_options!
+    assert_equal '[email protected] -o ControlMaster=auto -o ControlPath=/path -o ControlPersist=3600', cmd.ssh_args
+
+    # if knife-solo runs in cygwin, disable ssh ControlMaster by default
+    KnifeSolo::Tools.stubs(:cygwin_client?).returns(true)
+    cmd = command_internal('[email protected]')
+    cmd.validate_ssh_options!
+    assert_equal '[email protected]', cmd.ssh_args
+
+    KnifeSolo::Tools.stubs(:cygwin_client?).returns(true)
+    cmd = command_internal('[email protected]', '--ssh-control-master=auto')
+    cmd.validate_ssh_options!
+    assert_equal '[email protected] -o ControlMaster=auto -o ControlPath=/path -o ControlPersist=3600', cmd.ssh_args
+  end
+
   def test_barks_without_atleast_a_hostname
     cmd = command
     cmd.ui.expects(:fatal).with(regexp_matches(/hostname.*argument/))