Merge pull request #10515 from chrisroberts/f-rsync-auto-clean

Prevent rsync-auto crashes

lib/vagrant/errors.rb

@@ -668,6 +668,10 @@ module Vagrant
       error_key(:push_strategy_not_provided)
     end
 
+    class RSyncPostCommandError < VagrantError
+      error_key(:rsync_post_command_error)
+    end
+
     class RSyncError < VagrantError
       error_key(:rsync_error)
     end

plugins/synced_folders/rsync/command/rsync_auto.rb

@@ -217,6 +217,10 @@ module VagrantPlugins
                 # halt is happening. Just notify the user but don't fail out.
                 opts[:machine].ui.error(I18n.t(
                   "vagrant.rsync_communicator_not_ready_callback"))
+              rescue Vagrant::Errors::RSyncPostCommandError => e
+                # Error executing rsync chown command
+                opts[:machine].ui.error(I18n.t(
+                  "vagrant.rsync_auto_post_command_error", message: e.to_s))
               rescue Vagrant::Errors::RSyncError => e
                 # Error executing rsync, so show an error
                 opts[:machine].ui.error(I18n.t(

plugins/synced_folders/rsync/default_unix_cap.rb

@@ -23,6 +23,7 @@ module VagrantPlugins
         if opts.key?(:chown) && !opts[:chown]
           return
         end
+
         machine.communicate.sudo(build_rsync_chown(opts))
       end
 

plugins/synced_folders/rsync/helper.rb

@@ -213,7 +213,14 @@ module VagrantPlugins
 
         # If we have tasks to do after rsyncing, do those.
         if machine.guest.capability?(:rsync_post)
-          machine.guest.capability(:rsync_post, opts)
+          begin
+            machine.guest.capability(:rsync_post, opts)
+          rescue Vagrant::Errors::VagrantError => err
+            raise Vagrant::Errors::RSyncPostCommandError,
+              guestpath: guestpath,
+              hostpath: hostpath,
+              message: err.to_s
+          end
         end
       end
     end

templates/locales/en.yml

@@ -234,6 +234,12 @@ en:
       This may not be critical since rsync sometimes fails, but if this message
       repeats, then please fix the issue:
 
+      %{message}
+    rsync_auto_post_command_error: |-
+      There was an error while executing the rsync post command. This error is
+      shown below. This may not be critical but if this message repeats please
+      fix the issue:
+
       %{message}
     rsync_communicator_not_ready: |-
       The machine is reporting that it is not ready for rsync to
@@ -1271,6 +1277,14 @@ en:
         Guest path: %{guestpath}
         Command: %{command}
         Error: %{stderr}
+      rsync_post_command_error: |-
+        There was an error while attempting to run the post rsync
+        command for a synced folder. Please inspect the error message
+        below for more info.
+
+        Host path: %{hostpath}
+        Guest path: %{guestpath}
+        Error: %{message}
       rsync_guest_install_error: |-
         Installation of rsync into the guest has failed! The stdout
         and stderr are shown below. Please read the error output, resolve

test/unit/plugins/synced_folders/rsync/command/rsync_auto_test.rb

@@ -217,5 +217,50 @@ describe VagrantPlugins::SyncedFolderRSync::Command::RsyncAuto do
       expect { subject.callback(paths, m, a, r) }.
         to_not raise_error
     end
+
+    context "on failure" do
+      let(:machine) { machine_stub("m1") }
+      let(:opts) { double("opts_m1") }
+      let(:paths) { {"/foo" => [machine: machine, opts: opts]} }
+      let(:args) { [paths, ["/foo/bar"], [], []] }
+
+      before do
+        allow_any_instance_of(Vagrant::Errors::VagrantError).
+          to receive(:translate_error)
+        allow(machine.ui).to receive(:error)
+      end
+
+      context "when rsync command fails" do
+        before do
+          expect(helper_class).to receive(:rsync_single).with(machine, machine.ssh_info, opts).
+            and_raise(Vagrant::Errors::RSyncError)
+        end
+
+        it "should notify on error" do
+          expect(machine.ui).to receive(:error)
+          subject.callback(*args)
+        end
+
+        it "should not raise error" do
+          expect { subject.callback(*args) }.not_to raise_error
+        end
+      end
+
+      context "when rsync post command capability fails" do
+        before do
+          expect(helper_class).to receive(:rsync_single).with(machine, machine.ssh_info, opts).
+            and_raise(Vagrant::Errors::RSyncPostCommandError)
+        end
+
+        it "should notify on error" do
+          expect(machine.ui).to receive(:error)
+          subject.callback(*args)
+        end
+
+        it "should not raise error" do
+          expect { subject.callback(*args) }.not_to raise_error
+        end
+      end
+    end
   end
 end

test/unit/plugins/synced_folders/rsync/helper_test.rb

@@ -154,6 +154,21 @@ describe VagrantPlugins::SyncedFolderRSync::RsyncHelper do
       subject.rsync_single(machine, ssh_info, opts)
     end
 
+    context "with rsync_post capability" do
+      before do
+        allow(guest).to receive(:capability?).with(:rsync_post).and_return(true)
+        allow(Vagrant::Util::Subprocess).to receive(:execute).and_return(result)
+      end
+
+      it "should raise custom error when capability errors" do
+        expect(guest).to receive(:capability).with(:rsync_post, opts).
+          and_raise(Vagrant::Errors::VagrantError)
+
+        expect { subject.rsync_single(machine, ssh_info, opts) }.
+          to raise_error(Vagrant::Errors::RSyncPostCommandError)
+      end
+    end
+
     context "excluding files" do
       it "excludes files if given as a string" do
         opts[:exclude] = "foo"