Merge pull request #440 from matschaffer/speedup-attempts

Some small efficiency gains plus controlmaster for rsync

lib/chef/knife/solo_cook.rb

@@ -260,18 +260,21 @@ def write(content, dest)
         file.unlink
       end
 
-      def rsync(source_path, target_path, extra_opts = '--delete-after')
+      def rsync(source_path, target_path, extra_opts = ['--delete-after', '-zt'])
         if config[:ssh_gateway]
           ssh_command = "ssh -TA #{config[:ssh_gateway]} ssh -T -o StrictHostKeyChecking=no #{ssh_args}"
         else
           ssh_command = "ssh #{ssh_args}"
         end
 
-        cmd = ['rsync', '-rL', rsync_debug, rsync_permissions, %Q{--rsh=#{ssh_command}}, extra_opts]
+        cmd = ['rsync', '-rL', rsync_debug, rsync_permissions, %Q{--rsh=#{ssh_command}}]
+        cmd += extra_opts
         cmd += rsync_excludes.map { |ignore| "--exclude=#{ignore}" }
-        cmd << adjust_rsync_path_on_client(source_path)
-        cmd << %Q{:#{adjust_rsync_path_on_node(target_path)}}
-        cmd = cmd.flatten.compact
+        cmd += [ adjust_rsync_path_on_client(source_path),
+                 ':' + adjust_rsync_path_on_node(target_path) ]
+
+        cmd = cmd.compact
+
         Chef::Log.debug cmd.inspect
         system!(*cmd)
       end

lib/knife-solo/ssh_command.rb

@@ -47,6 +47,11 @@ def self.included(other)
           :long        => '--ssh-identity FILE',
           :description => 'Deprecated. Replaced with --identity-file.'
 
+        option :ssh_control_master,
+          :long => '--ssh-control-master SETTING',
+          :description => 'Control master setting to use when running rsync (use "no" to disable)',
+          :default => 'auto'
+
         option :identity_file,
           :short       => '-i IDENTITY_FILE',
           :long        => '--identity-file FILE',
@@ -169,7 +174,7 @@ def connection_options
       if config[:ssh_keepalive]
         options[:keepalive] = config[:ssh_keepalive]
         options[:keepalive_interval] = config[:ssh_keepalive_interval]
-      end      
+      end
       # Respect users' specification of config[:ssh_config]
       # Prevents Net::SSH itself from applying the default ssh_config files.
       options[:config] = false
@@ -194,16 +199,25 @@ def detect_authentication_method
     end
 
     def ssh_args
-      host_arg = [user, host].compact.join('@')
-      config_arg = "-F #{config[:ssh_config]}" if config[:ssh_config]
-      ident_arg = "-i #{config[:identity_file]}" if config[:identity_file]
-      forward_arg = "-o ForwardAgent=yes" if config[:forward_agent]
-      port_arg = "-p #{config[:ssh_port]}" if config[:ssh_port]
-      knownhosts_arg  =  "-o UserKnownHostsFile=#{connection_options[:user_known_hosts_file]}" if config[:host_key_verify] == false
-      stricthosts_arg = "-o StrictHostKeyChecking=no" if config[:host_key_verify] == false
+      args = []
 
+      args << [user, host].compact.join('@')
 
-      [host_arg, config_arg, ident_arg, forward_arg, port_arg, knownhosts_arg, stricthosts_arg].compact.join(' ')
+      args << "-F #{config[:ssh_config]}" if config[:ssh_config]
+      args << "-i #{config[:identity_file]}" if config[:identity_file]
+      args << "-o ForwardAgent=yes" if config[:forward_agent]
+      args << "-p #{config[:ssh_port]}" if config[:ssh_port]
+      args << "-o UserKnownHostsFile=#{connection_options[:user_known_hosts_file]}" if config[:host_key_verify] == false
+      args << "-o StrictHostKeyChecking=no" if config[:host_key_verify] == false
+      args << "-o ControlMaster=auto -o ControlPath=#{ssh_control_path} -o ControlPersist=3600" unless config[:ssh_control_master] == "no"
+
+      args.join(' ')
+    end
+
+    def ssh_control_path
+      dir = File.join(ENV['HOME'], '.chef', 'knife-solo-sockets')
+      FileUtils.mkdir_p(dir)
+      File.join(dir, '%h')
     end
 
     def custom_sudo_command
@@ -233,7 +247,11 @@ def startup_script
     def windows_node?
       return @windows_node unless @windows_node.nil?
       @windows_node = run_command('ver', :process_sudo => false).stdout =~ /Windows/i
-      Chef::Log.debug("Windows node detected") if @windows_node
+      if @windows_node
+        Chef::Log.debug("Windows node detected")
+      else
+        @windows_node = false
+      end
       @windows_node
     end
 

test/solo_cook_test.rb

@@ -51,7 +51,6 @@ def test_rsync_without_gateway_connection_options
       cmd.expects(:system!).with('rsync',
                                   '-rL',
                                   '--rsh=ssh ssh_arguments',
-                                  '--delete-after',
                                   '--exclude=revision-deploys',
                                   '--exclude=tmp',
                                   '--exclude=.git',
@@ -64,7 +63,7 @@ def test_rsync_without_gateway_connection_options
       cmd.stubs(:ssh_args => 'ssh_arguments')
       cmd.stubs(:windows_node? => false)
 
-      cmd.rsync 'source', 'dest'
+      cmd.rsync 'source', 'dest', []
     end
   end
 
@@ -76,7 +75,6 @@ def test_rsync_with_gateway_connection_options
       cmd.expects(:system!).with('rsync',
                                   '-rL',
                                   '--rsh=ssh -TA user@gateway ssh -T -o StrictHostKeyChecking=no ssh_arguments',
-                                  '--delete-after',
                                   '--exclude=revision-deploys',
                                   '--exclude=tmp',
                                   '--exclude=.git',
@@ -89,7 +87,7 @@ def test_rsync_with_gateway_connection_options
       cmd.stubs(:ssh_args => 'ssh_arguments')
       cmd.stubs(:windows_node? => false)
 
-      cmd.rsync 'source', 'dest'
+      cmd.rsync 'source', 'dest', []
     end
   end
 

test/ssh_command_test.rb

@@ -158,6 +158,10 @@ def test_builds_cli_ssh_args
     cmd = command("usertest@10.0.0.1", "--ssh-gateway=test@host.com")
     cmd.validate_ssh_options!
     assert_equal "usertest@10.0.0.1", cmd.ssh_args
+
+    cmd = command_internal("usertest@10.0.0.1", "--ssh-gateway=test@host.com")
+    cmd.validate_ssh_options!
+    assert_equal "usertest@10.0.0.1 -o ControlMaster=auto -o ControlPath=#{cmd.ssh_control_path} -o ControlPersist=3600", cmd.ssh_args
   end
 
   def test_barks_without_atleast_a_hostname
@@ -221,6 +225,11 @@ def result(code, stdout = "")
   end
 
   def command(*args)
+    args << '--ssh-control-master=no'
+    command_internal(*args)
+  end
+
+  def command_internal(*args)
     Net::SSH::Config.stubs(:default_files)
     knife_command(DummySshCommand, *args)
   end