Merge pull request #10399 from chrisroberts/e-comm-reset

Add communicator reset

lib/vagrant/plugin/v2/communicator.rb

@@ -117,6 +117,13 @@ module Vagrant
         # @see #execute
         def test(command, opts=nil)
         end
+
+        # Reset the communicator. For communicators which establish
+        # a persistent connection to the remote machine, this connection
+        # should be terminated and re-established. The communicator
+        # instance should be in a "fresh" state after calling this method.
+        def reset!
+        end
       end
     end
   end

plugins/communicators/ssh/communicator.rb

@@ -69,11 +69,14 @@ module VagrantPlugins
           end
 
           # Got it! Let the user know what we're connecting to.
-          @machine.ui.detail("SSH address: #{ssh_info[:host]}:#{ssh_info[:port]}")
-          @machine.ui.detail("SSH username: #{ssh_info[:username]}")
-          ssh_auth_type = "private key"
-          ssh_auth_type = "password" if ssh_info[:password]
-          @machine.ui.detail("SSH auth method: #{ssh_auth_type}")
+          if !@ssh_info_notification
+            @machine.ui.detail("SSH address: #{ssh_info[:host]}:#{ssh_info[:port]}")
+            @machine.ui.detail("SSH username: #{ssh_info[:username]}")
+            ssh_auth_type = "private key"
+            ssh_auth_type = "password" if ssh_info[:password]
+            @machine.ui.detail("SSH auth method: #{ssh_auth_type}")
+            @ssh_info_notification = true
+          end
 
           previous_messages = {}
           while true
@@ -309,6 +312,15 @@ module VagrantPlugins
           to: to.to_s
       end
 
+      def reset!
+        if @connection
+          @connection.close
+          @connection = nil
+        end
+        @ssh_info_notification = true # suppress ssh info output
+        wait_for_ready(5)
+      end
+
       protected
 
       # Opens an SSH connection and yields it to a block.

plugins/communicators/winrm/communicator.rb

@@ -121,6 +121,10 @@ module VagrantPlugins
         return false
       end
 
+      def reset!
+        shell(true)
+      end
+
       def shell(reload=false)
         @shell = nil if reload
         @shell ||= create_shell

plugins/provisioners/docker/cap/linux/docker_configure_vagrant_user.rb

@@ -9,6 +9,7 @@ module VagrantPlugins
             machine.communicate.tap do |comm|
               if comm.test("getent group docker") && !comm.test("id -Gn | grep docker")
                 comm.sudo("usermod -a -G docker #{ssh_info[:username]}")
+                comm.reset!
               end
             end
           end

plugins/provisioners/shell/config.rb

@@ -17,6 +17,7 @@ module VagrantPlugins
       attr_accessor :sensitive
       attr_accessor :powershell_args
       attr_accessor :powershell_elevated_interactive
+      attr_accessor :reset
 
       def initialize
         @args                  = UNSET_VALUE
@@ -31,6 +32,7 @@ module VagrantPlugins
         @keep_color            = UNSET_VALUE
         @name                  = UNSET_VALUE
         @sensitive             = UNSET_VALUE
+        @reset                 = UNSET_VALUE
         @powershell_args       = UNSET_VALUE
         @powershell_elevated_interactive  = UNSET_VALUE
       end
@@ -48,6 +50,7 @@ module VagrantPlugins
         @keep_color           = false if @keep_color == UNSET_VALUE
         @name                 = nil if @name == UNSET_VALUE
         @sensitive            = false if @sensitive == UNSET_VALUE
+        @reset                = false if @reset == UNSET_VALUE
         @powershell_args      = "-ExecutionPolicy Bypass" if @powershell_args == UNSET_VALUE
         @powershell_elevated_interactive = false if @powershell_elevated_interactive == UNSET_VALUE
 
@@ -68,7 +71,7 @@ module VagrantPlugins
         # Validate that the parameters are properly set
         if path && inline
           errors << I18n.t("vagrant.provisioners.shell.path_and_inline_set")
-        elsif !path && !inline
+        elsif !path && !inline && !reset
           errors << I18n.t("vagrant.provisioners.shell.no_path_or_inline")
         end
 

plugins/provisioners/shell/provisioner.rb

@@ -18,6 +18,10 @@ module VagrantPlugins
           args = " #{args.join(" ")}"
         end
 
+        # In cases where the connection is just being reset
+        # bail out before attempting to do any actual provisioning
+        return if !config.path && !config.inline
+
         case @machine.config.vm.communicator
         when :winrm
           provision_winrm(args)
@@ -26,6 +30,8 @@ module VagrantPlugins
         else
           provision_ssh(args)
         end
+      ensure
+        @machine.communicate.reset! if config.reset
       end
 
       protected

test/unit/plugins/communicators/ssh/communicator_test.rb

@@ -162,6 +162,26 @@ describe VagrantPlugins::CommunicatorSSH::Communicator do
     end
   end
 
+  describe "reset!" do
+    let(:connection) { double("connection") }
+
+    before do
+      allow(communicator).to receive(:wait_for_ready)
+      allow(connection).to receive(:close)
+      communicator.send(:instance_variable_set, :@connection, connection)
+    end
+
+    it "should close existing connection" do
+      expect(connection).to receive(:close)
+      communicator.reset!
+    end
+
+    it "should call wait_for_ready to re-enable the connection" do
+      expect(communicator).to receive(:wait_for_ready)
+      communicator.reset!
+    end
+  end
+
   describe ".ready?" do
     before(&connection_setup)
     it "returns true if shell test is successful" do

test/unit/plugins/communicators/winrm/communicator_test.rb

@@ -57,6 +57,13 @@ describe VagrantPlugins::CommunicatorWinRM::Communicator do
     end
   end
 
+  describe ".reset!" do
+    it "should create a new shell" do
+      expect(subject).to receive(:shell).with(true)
+      subject.reset!
+    end
+  end
+
   describe ".ready?" do
     it "returns true if hostname command executes without error" do
       expect(shell).to receive(:cmd).with("hostname").and_return({ exitcode: 0 })

test/unit/plugins/provisioners/shell/config_test.rb

@@ -109,6 +109,31 @@ describe "VagrantPlugins::Shell::Config" do
         I18n.t("vagrant.provisioners.shell.env_must_be_a_hash")
       )
     end
+
+    it "returns an error if file and script are unset" do
+      subject.finalize!
+      result = subject.validate(machine)
+      expect(result["shell provisioner"]).to include(
+        I18n.t("vagrant.provisioners.shell.no_path_or_inline")
+      )
+    end
+
+    it "returns an error if inline and path are both set" do
+      subject.inline = "script"
+      subject.path = "script"
+      result = subject.validate(machine)
+      expect(result["shell provisioner"]).to include(
+        I18n.t("vagrant.provisioners.shell.path_and_inline_set")
+      )
+    end
+
+    it "returns no error when inline and path are unset but reset is true" do
+      subject.reset = true
+      subject.finalize!
+
+      result = subject.validate(machine)
+      expect(result["shell provisioner"]).to be_empty
+    end
   end
 
   describe 'finalize!' do

test/unit/plugins/provisioners/shell/provisioner_test.rb

@@ -16,6 +16,52 @@ describe "Vagrant::Shell::Provisioner" do
     allow(env).to receive(:tmp_path).and_return(Pathname.new("/dev/null"))
   end
 
+  context "when reset is enabled" do
+    let(:path) { nil }
+    let(:inline) { "" }
+    let(:communicator) { double("communicator") }
+
+    let(:config) {
+      double(
+        :config,
+        :args        => "doesn't matter",
+        :env         => {},
+        :upload_path => "arbitrary",
+        :remote?     => false,
+        :path        => path,
+        :inline      => inline,
+        :binary      => false,
+        :reset       => true
+      )
+    }
+
+    let(:vsp) {
+      VagrantPlugins::Shell::Provisioner.new(machine, config)
+    }
+
+    before {
+      allow(machine).to receive(:communicate).and_return(communicator)
+      allow(vsp).to receive(:provision_ssh)
+    }
+
+    it "should provision and then reset the connection" do
+      expect(vsp).to receive(:provision_ssh)
+      expect(communicator).to receive(:reset!)
+      vsp.provision
+    end
+
+    context "when path and inline are not set" do
+      let(:path) { nil }
+      let(:inline) { nil }
+
+      it "should reset the connection and not provision" do
+        expect(vsp).not_to receive(:provision_ssh)
+        expect(communicator).to receive(:reset!)
+        vsp.provision
+      end
+    end
+  end
+
   context "with a script that contains invalid us-ascii byte sequences" do
     let(:config) {
       double(
@@ -27,6 +73,7 @@ describe "Vagrant::Shell::Provisioner" do
         :path        => nil,
         :inline      => script_that_is_incorrectly_us_ascii_encoded,
         :binary      => false,
+        :reset       => false
       )
     }
 
@@ -59,6 +106,7 @@ describe "Vagrant::Shell::Provisioner" do
         :path        => nil,
         :inline      => script,
         :binary      => false,
+        :reset       => false
       )
     }
 
@@ -87,7 +135,8 @@ describe "Vagrant::Shell::Provisioner" do
           :path        => "http://example.com/script.sh",
           :binary      => false,
           :md5         => nil,
-          :sha1        => 'EXPECTED_VALUE'
+          :sha1        => 'EXPECTED_VALUE',
+          :reset       => false
         )
       }
 
@@ -117,7 +166,8 @@ describe "Vagrant::Shell::Provisioner" do
           :path        => "http://example.com/script.sh",
           :binary      => false,
           :md5         => 'EXPECTED_VALUE',
-          :sha1        => nil
+          :sha1        => nil,
+          :reset       => false
         )
       }
 

website/source/docs/provisioning/shell.html.md

@@ -43,31 +43,22 @@ The remainder of the available options are optional:
   etc. as needed. You may also pass the arguments in using an array. In this
   case, Vagrant will handle quoting for you.
 
-* `env` (hash) - List of key-value pairs to pass in as environment variables to
-  the script. Vagrant will handle quoting for environment variable values, but
-  the keys remain untouched.
-
 * `binary` (boolean) - Vagrant automatically replaces Windows line endings with
   Unix line endings. If this is false, then Vagrant will not do this. By default
   this is "false". If the shell provisioner is communicating over WinRM, this
   defaults to "true".
 
-* `privileged` (boolean) - Specifies whether to execute the shell script
-  as a privileged user or not (`sudo`). By default this is "true". Windows
-  guests use a scheduled task to run as a true administrator without the
-  WinRM limitations.
-
-* `upload_path` (string) - Is the remote path where the shell script will
-  be uploaded to. The script is uploaded as the SSH user over SCP, so this
-  location must be writable to that user. By default this is
-  "/tmp/vagrant-shell". On Windows, this will default to
-  "C:\tmp\vagrant-shell".
+* `env` (hash) - List of key-value pairs to pass in as environment variables to
+  the script. Vagrant will handle quoting for environment variable values, but
+  the keys remain untouched.
 
 * `keep_color` (boolean) - Vagrant automatically colors output in green and
   red depending on whether the output is from stdout or stderr. If this is
   true, Vagrant will not do this, allowing the native colors from the script
   to be outputted.
 
+* `md5` (string) - MD5 checksum used to validate remotely downloaded shell files.
+
 * `name` (string) - This value will be displayed in the output so that
   identification by the user is easier when many shell provisioners are present.
 
@@ -79,13 +70,25 @@ The remainder of the available options are optional:
   enable auto-login for Windows as the user must be logged in for interactive
   mode to work.
 
-* `md5` (string) - MD5 checksum used to validate remotely downloaded shell files.
+* `privileged` (boolean) - Specifies whether to execute the shell script
+  as a privileged user or not (`sudo`). By default this is "true". Windows
+  guests use a scheduled task to run as a true administrator without the
+  WinRM limitations.
+
+* `reset` (boolean) - Reset the communicator to the machine after completion. This
+  is useful when a shell may need to be reloaded.
 
 * `sha1` (string) - SHA1 checksum used to validate remotely downloaded shell files.
 
 * `sensitive` (boolean) - Marks the Hash values used in the `env` option as sensitive
   and hides them from output. By default this is "false".
 
+* `upload_path` (string) - Is the remote path where the shell script will
+  be uploaded to. The script is uploaded as the SSH user over SCP, so this
+  location must be writable to that user. By default this is
+  "/tmp/vagrant-shell". On Windows, this will default to
+  "C:\tmp\vagrant-shell".
+
 <a name="inline-scripts"></a>
 ## Inline Scripts