synced_folders/rsync: only chown when necessary [hashicorpGH-3525]

Run remote rsync as root to guarantee that rsync can write to guestpath. This obviates the need to chown the guestpath to the SSH user prior to sync. This brings a substantial speedup (2x on a moderately-sized shared folder) and properly triggers filesystem notifications on only the files changed by a given sync.
gisboxwinner · Apr 25, 2014 · 2df3689 · 2df3689
1 parent 75ee742
commit 2df3689
Show file tree

Hide file tree

Showing 17 changed files with 105 additions and 82 deletions.
diff --git a/plugins/guests/darwin/cap/rsync.rb b/plugins/guests/darwin/cap/rsync.rb
@@ -6,13 +6,14 @@ def self.rsync_installed(machine)
           machine.communicate.test("which rsync")
         end
 
-        def self.rsync_pre(machine, folder_opts)
-          username = machine.ssh_info[:username]
+        def self.rsync_command(machine)
+          "sudo rsync"
+        end
 
-          machine.communicate.tap do |comm|
-            comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'")
-            comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'")
-          end
+        def self.rsync_post(machine, opts)
+          machine.communicate.sudo(
+            "find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " +
+            "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}")
         end
       end
     end

diff --git a/plugins/guests/darwin/plugin.rb b/plugins/guests/darwin/plugin.rb
@@ -41,7 +41,12 @@ class Plugin < Vagrant.plugin("2")
         Cap::RSync
       end
 
-      guest_capability("darwin", "rsync_pre") do
+      guest_capability("darwin", "rsync_command") do
+        require_relative "cap/rsync"
+        Cap::RSync
+      end
+
+      guest_capability("darwin", "rsync_post") do
         require_relative "cap/rsync"
         Cap::RSync
       end

diff --git a/plugins/guests/freebsd/cap/rsync.rb b/plugins/guests/freebsd/cap/rsync.rb
@@ -12,13 +12,14 @@ def self.rsync_installed(machine)
           machine.communicate.test("which rsync")
         end
 
-        def self.rsync_pre(machine, folder_opts)
-          username = machine.ssh_info[:username]
+        def self.rsync_command(machine)
+          "sudo rsync"
+        end
 
-          machine.communicate.tap do |comm|
-            comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'", shell: "sh")
-            comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'", shell: "sh")
-          end
+        def self.rsync_post(machine, opts)
+          machine.communicate.sudo(
+            "find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " +
+            "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}")
         end
       end
     end

diff --git a/plugins/guests/freebsd/plugin.rb b/plugins/guests/freebsd/plugin.rb
@@ -46,7 +46,12 @@ class Plugin < Vagrant.plugin("2")
         Cap::RSync
       end
 
-      guest_capability("freebsd", "rsync_pre") do
+      guest_capability("freebsd", "rsync_command") do
+        require_relative "cap/rsync"
+        Cap::RSync
+      end
+
+      guest_capability("freebsd", "rsync_post") do
         require_relative "cap/rsync"
         Cap::RSync
       end

diff --git a/plugins/guests/linux/cap/rsync.rb b/plugins/guests/linux/cap/rsync.rb
@@ -6,21 +6,14 @@ def self.rsync_installed(machine)
           machine.communicate.test("which rsync")
         end
 
-        def self.rsync_pre(machine, opts)
-          username = machine.ssh_info[:username]
-
-          machine.communicate.tap do |comm|
-            comm.sudo("mkdir -p '#{opts[:guestpath]}'")
-            comm.sudo("find '#{opts[:guestpath]}' ! -user #{username} -print0 | " +
-              "xargs -0 -r chown -v #{username}:")
-          end
+        def self.rsync_command(machine)
+          "sudo rsync"
         end
 
         def self.rsync_post(machine, opts)
-          machine.communicate.tap do |comm|
-            comm.sudo("find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " +
-              "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}")
-          end
+          machine.communicate.sudo(
+            "find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " +
+            "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}")
         end
       end
     end

diff --git a/plugins/guests/linux/plugin.rb b/plugins/guests/linux/plugin.rb
@@ -67,12 +67,12 @@ class Plugin < Vagrant.plugin("2")
         Cap::RSync
       end
 
-      guest_capability("linux", "rsync_post") do
+      guest_capability("linux", "rsync_command") do
         require_relative "cap/rsync"
         Cap::RSync
       end
 
-      guest_capability("linux", "rsync_pre") do
+      guest_capability("linux", "rsync_post") do
         require_relative "cap/rsync"
         Cap::RSync
       end

diff --git a/plugins/guests/netbsd/cap/rsync.rb b/plugins/guests/netbsd/cap/rsync.rb
@@ -13,13 +13,14 @@ def self.rsync_install(machine)
             'pkg_add rsync')
         end
 
-        def self.rsync_pre(machine, folder_opts)
-          username = machine.ssh_info[:username]
+        def self.rsync_command(machine)
+          "sudo rsync"
+        end
 
-          machine.communicate.tap do |comm|
-            comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'")
-            comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'")
-          end
+        def self.rsync_post(machine, opts)
+          machine.communicate.sudo(
+            "find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " +
+            "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}")
         end
       end
     end

diff --git a/plugins/guests/netbsd/plugin.rb b/plugins/guests/netbsd/plugin.rb
@@ -46,7 +46,12 @@ class Plugin < Vagrant.plugin("2")
         Cap::RSync
       end
 
-      guest_capability("netbsd", "rsync_pre") do
+      guest_capability("netbsd", "rsync_command") do
+        require_relative "cap/rsync"
+        Cap::RSync
+      end
+
+      guest_capability("netbsd", "rsync_post") do
         require_relative "cap/rsync"
         Cap::RSync
       end

diff --git a/plugins/guests/openbsd/cap/rsync.rb b/plugins/guests/openbsd/cap/rsync.rb
@@ -13,13 +13,14 @@ def self.rsync_installed(machine)
           machine.communicate.test("which rsync")
         end
 
-        def self.rsync_pre(machine, folder_opts)
-          username = machine.ssh_info[:username]
+        def self.rsync_command(machine)
+          "sudo rsync"
+        end
 
-          machine.communicate.tap do |comm|
-            comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'")
-            comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'")
-          end
+        def self.rsync_post(machine, opts)
+          machine.communicate.sudo(
+            "find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " +
+            "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}")
         end
       end
     end

diff --git a/plugins/guests/openbsd/plugin.rb b/plugins/guests/openbsd/plugin.rb
@@ -46,7 +46,12 @@ class Plugin < Vagrant.plugin("2")
         Cap::RSync
       end
 
-      guest_capability("openbsd", "rsync_pre") do
+      guest_capability("openbsd", "rsync_command") do
+        require_relative "cap/rsync"
+        Cap::RSync
+      end
+
+      guest_capability("openbsd", "rsync_post") do
         require_relative "cap/rsync"
         Cap::RSync
       end

diff --git a/plugins/guests/smartos/cap/rsync.rb b/plugins/guests/smartos/cap/rsync.rb
@@ -6,14 +6,13 @@ def self.rsync_installed(machine)
           machine.communicate.test("which rsync")
         end
 
-        def self.rsync_pre(machine, folder_opts)
-          username = machine.ssh_info[:username]
-          sudo = machine.config.smartos.suexec_cmd
+        def self.rsync_command(machine)
+          "#{machine.config.smartos.suexec_cmd} rsync"
+        end
 
-          machine.communicate.tap do |comm|
-            comm.execute("#{sudo} mkdir -p '#{folder_opts[:guestpath]}'")
-            comm.execute("#{sudo} chown -R #{username} '#{folder_opts[:guestpath]}'")
-          end
+        def self.rsync_post(machine, opts)
+          machine.communicate.sudo("find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " +
+              "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}")
         end
       end
     end

diff --git a/plugins/guests/smartos/plugin.rb b/plugins/guests/smartos/plugin.rb
@@ -41,7 +41,12 @@ class Plugin < Vagrant.plugin("2")
         Cap::RSync
       end
 
-      guest_capability("smartos", "rsync_pre") do
+      guest_capability("smartos", "rsync_command") do
+        require_relative "cap/rsync"
+        Cap::RSync
+      end
+
+      guest_capability("smartos", "rsync_post") do
         require_relative "cap/rsync"
         Cap::RSync
       end

diff --git a/plugins/guests/solaris/cap/rsync.rb b/plugins/guests/solaris/cap/rsync.rb
@@ -6,13 +6,15 @@ def self.rsync_installed(machine)
           machine.communicate.test("which rsync")
         end
 
-        def self.rsync_pre(machine, folder_opts)
-          username = machine.ssh_info[:username]
+        def self.rsync_command(machine)
+          "#{machine.config.solaris.suexec_cmd} rsync"
+        end
 
-          machine.communicate.tap do |comm|
-            comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'")
-            comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'")
-          end
+        def self.rsync_post(machine, opts)
+          su_cmd = machine.config.solaris.su_cmd
+          machine.communicate.execute(
+            "#{su_cmd} find '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " +
+            "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}")
         end
       end
     end

diff --git a/plugins/guests/solaris/plugin.rb b/plugins/guests/solaris/plugin.rb
@@ -41,7 +41,12 @@ class Plugin < Vagrant.plugin("2")
         Cap::RSync
       end
 
-      guest_capability("solaris", "rsync_pre") do
+      guest_capability("solaris", "rsync_command") do
+        require_relative "cap/rsync"
+        Cap::RSync
+      end
+
+      guest_capability("solaris", "rsync_post") do
         require_relative "cap/rsync"
         Cap::RSync
       end

diff --git a/plugins/guests/solaris11/cap/rsync.rb b/plugins/guests/solaris11/cap/rsync.rb
@@ -6,13 +6,15 @@ def self.rsync_installed(machine)
           machine.communicate.test("which rsync")
         end
 
-        def self.rsync_pre(machine, folder_opts)
-          username = machine.ssh_info[:username]
+        def self.rsync_command(machine)
+          "#{machine.config.solaris11.suexec_cmd} rsync"
+        end
 
-          machine.communicate.tap do |comm|
-            comm.sudo("mkdir -p '#{folder_opts[:guestpath]}'")
-            comm.sudo("chown -R #{username} '#{folder_opts[:guestpath]}'")
-          end
+        def self.rsync_post(machine, opts)
+          su_cmd = machine.config.solaris11.su_cmd
+          machine.communicate.execute(
+            "#{su_cmd} '#{opts[:guestpath]}' '(' ! -user #{opts[:owner]} -or ! -group #{opts[:group]} ')' -print0 | " +
+              "xargs -0 -r chown -v #{opts[:owner]}:#{opts[:group]}")
         end
       end
     end

diff --git a/plugins/synced_folders/rsync/helper.rb b/plugins/synced_folders/rsync/helper.rb
@@ -84,6 +84,17 @@ def self.rsync_single(machine, ssh_info, opts)
           args << "--no-perms" if args.include?("--archive") || args.include?("-a")
         end
 
+        # Disable rsync's owner/group preservation (implied by --archive) unless
+        # specifically requested, since we adjust owner/group to match shared
+        # folder setting ourselves.
+        args << "--no-owner" unless args.include?("--owner") || args.include?("-o")
+        args << "--no-group" unless args.include?("--group") || args.include?("-g")
+
+        # Tell local rsync how to invoke remote rsync with sudo
+        if machine.guest.capability?(:rsync_command)
+          args << "--rsync-path"<< machine.guest.capability(:rsync_command)
+        end
+
         # Build up the actual command to execute
         command = [
           "rsync",

diff --git a/test/unit/plugins/guests/smartos/cap/rsync_test.rb b/test/unit/plugins/guests/smartos/cap/rsync_test.rb
@@ -1,7 +1,7 @@
 require File.expand_path("../../../../../base", __FILE__)
 
 describe "VagrantPlugins::VagrantPlugins::Cap::Rsync" do
-  let(:plugin) { VagrantPlugins::GuestSmartos::Plugin.components.guest_capabilities[:smartos].get(:rsync_pre) }
+  let(:plugin) { VagrantPlugins::GuestSmartos::Plugin.components.guest_capabilities[:smartos].get(:rsync_installed) }
   let(:machine) { double("machine") }
   let(:config) { double("config", smartos: VagrantPlugins::GuestSmartos::Config.new) }
   let(:communicator) { VagrantTests::DummyCommunicator::Communicator.new(machine) }
@@ -30,23 +30,5 @@
       end
     end
   end
-
-  describe ".rsync_pre" do
-    let(:username) { "some_user" }
-
-    before do
-      machine.stub(:ssh_info).and_return({username: username})
-    end
-
-    it "creates a local directory" do
-      communicator.expect_command(%Q(pfexec mkdir -p '/mountpoint'))
-      plugin.rsync_pre(machine, {guestpath: '/mountpoint'})
-    end
-
-    it "chowns local directory to ssh user" do
-      communicator.expect_command(%Q(pfexec chown -R #{username} '/mountpoint'))
-      plugin.rsync_pre(machine, {guestpath: '/mountpoint'})
-    end
-  end
 end