guests/debian: Update guest capabilities

This updates the guest capabilities to run in as few communicator
commands as possible. Additionally, it fixes a number of issues around
hostname and idempotency.

This patch was tested against:

- puphpet/debian75-x64
- debian/jessie64
- debian/wheezy64

with custom networking, custom hostname, and rsync shared folders.

plugins/guests/debian/cap/change_host_name.rb

@@ -2,92 +2,45 @@ module VagrantPlugins
   module GuestDebian
     module Cap
       class ChangeHostName
+        # For more information, please see:
+        #
+        #   https://wiki.debian.org/HowTo/ChangeHostname
+        #
         def self.change_host_name(machine, name)
-          new(machine, name).change!
+          comm = machine.communicate
-        end
 
-        attr_reader :machine, :new_hostname
+          if !comm.test("hostname -f | grep -w '#{name}'")
+            basename = name.split(".", 2)[0]
+            comm.sudo <<-EOH.gsub(/^ {14}/, '')
+              # Set the hostname
+              echo '#{name}' > /etc/hostname
+              hostname -F /etc/hostname
 
-        def initialize(machine, new_hostname)
+              # Remove comments and blank lines from /etc/hosts
-          @machine = machine
+              sed -i'' -e 's/#.*$//' -e '/^$/d' /etc/hosts
-          @new_hostname = new_hostname
-        end
 
-        def change!
+              # Prepend ourselves to /etc/hosts
-          return unless should_change?
+              grep -w '#{name}' /etc/hosts || {
+                sed -i'' '1i 127.0.0.1\\t#{name}\\t#{basename}' /etc/hosts
+              }
 
-          update_etc_hostname
+              # Update mailname
-          update_etc_hosts
+              echo '#{name}' > /etc/mailname
-          refresh_hostname_service
-          update_mailname
-          renew_dhcp
-        end
 
-        def should_change?
+              # Restart networking and force new DHCP
-          new_hostname != current_hostname
+              if [ test -f /etc/init.d/hostname.sh ]; then
-        end
+                invoke-rc.d hostname.sh start
+              fi
 
-        def current_hostname
+              if [ test -f /etc/init.d/networking ]; then
-          @current_hostname ||= get_current_hostname
+                invoke-rc.d networking force-reload
-        end
+              fi
 
-        def get_current_hostname
+              if [ test -f /etc/init.d/network-manager ]; then
-          hostname = ""
+                invoke-rc.d network-manager force-reload
-          sudo "hostname -f" do |type, data|
+              fi
-            hostname = data.chomp if type == :stdout && hostname.empty?
+            EOH
           end
-
-          hostname
-        end
-
-        def update_etc_hostname
-          sudo("echo '#{short_hostname}' > /etc/hostname")
-        end
-
-        # /etc/hosts should resemble:
-        # 127.0.0.1   localhost
-        # 127.0.1.1   host.fqdn.com host.fqdn host
-        def update_etc_hosts
-          if test("grep '#{current_hostname}' /etc/hosts")
-            # Current hostname entry is in /etc/hosts
-            ip_address = '([0-9]{1,3}\.){3}[0-9]{1,3}'
-            search     = "^(#{ip_address})\\s+#{Regexp.escape(current_hostname)}(\\s.*)?$"
-            replace    = "\\1 #{fqdn} #{short_hostname}"
-            expression = ['s', search, replace, 'g'].join('@')
-
-            sudo("sed -ri '#{expression}' /etc/hosts")
-          else
-            # Current hostname entry isn't in /etc/hosts, just append it
-            sudo("echo '127.0.1.1 #{fqdn} #{short_hostname}' >>/etc/hosts")
-          end
-        end
-
-        def refresh_hostname_service
-          sudo("hostname -F /etc/hostname")
-        end
-
-        def update_mailname
-          sudo("hostname --fqdn > /etc/mailname")
-        end
-
-        def renew_dhcp
-          sudo("ifdown -a; ifup -a; ifup eth0")
-        end
-
-        def fqdn
-          new_hostname
-        end
-
-        def short_hostname
-          new_hostname.split('.').first
-        end
-
-        def sudo(cmd, &block)
-          machine.communicate.sudo(cmd, &block)
-        end
-
-        def test(cmd)
-          machine.communicate.test(cmd)
         end
       end
     end

plugins/guests/debian/cap/configure_networks.rb

@@ -1,4 +1,3 @@
-require "set"
 require "tempfile"
 
 require_relative "../../../../lib/vagrant/util/template_renderer"
@@ -10,54 +9,69 @@ module VagrantPlugins
         include Vagrant::Util
 
         def self.configure_networks(machine, networks)
-          machine.communicate.tap do |comm|
+          comm = machine.communicate
-            # First, remove any previous network modifications
-            # from the interface file.
-            comm.sudo("sed -e '/^#VAGRANT-BEGIN/,$ d' /etc/network/interfaces > /tmp/vagrant-network-interfaces.pre")
-            comm.sudo("sed -ne '/^#VAGRANT-END/,$ p' /etc/network/interfaces | tac | sed -e '/^#VAGRANT-END/,$ d' | tac > /tmp/vagrant-network-interfaces.post")
 
-            # Accumulate the configurations to add to the interfaces file as
+          interfaces = {}
-            # well as what interfaces we're actually configuring since we use that
+          entries    = []
-            # later.
-            interfaces = Set.new
-            entries = []
-            networks.each do |network|
-              interfaces.add(network[:interface])
-              entry = TemplateRenderer.render("guests/debian/network_#{network[:type]}",
-                                              options: network)
 
-              entries << entry
+          # Accumulate the configurations to add to the interfaces file as
-            end
+          # well as what interfaces we're actually configuring since we use that
+          # later.
+          networks.each do |network|
+            interfaces[network[:interface]] = true
 
-            # Perform the careful dance necessary to reconfigure the network
+            entry = TemplateRenderer.render("guests/debian/network_#{network[:type]}",
-            # interfaces.
+              options: network,
-            Tempfile.open("vagrant-debian-configure-networks") do |f|
+            )
-              f.binmode
+            entries << entry
-              f.write(entries.join("\n"))
-              f.fsync
-              f.close
-              comm.upload(f.path, "/tmp/vagrant-network-entry")
-            end
-
-            # Bring down all the interfaces we're reconfiguring. By bringing down
-            # each specifically, we avoid reconfiguring eth0 (the NAT interface) so
-            # SSH never dies.
-            interfaces.each do |interface|
-              # Ubuntu 16.04+ returns an error when downing an interface that
-              # does not exist. The `|| true` preserves the behavior that older
-              # Ubuntu versions exhibit and Vagrant expects (GH-7155)
-              comm.sudo("/sbin/ifdown eth#{interface} 2> /dev/null || true")
-              comm.sudo("/sbin/ip addr flush dev eth#{interface} 2> /dev/null")
-            end
-
-            comm.sudo('cat /tmp/vagrant-network-interfaces.pre /tmp/vagrant-network-entry /tmp/vagrant-network-interfaces.post > /etc/network/interfaces')
-            comm.sudo('rm -f /tmp/vagrant-network-interfaces.pre /tmp/vagrant-network-entry /tmp/vagrant-network-interfaces.post')
-
-            # Bring back up each network interface, reconfigured
-            interfaces.each do |interface|
-              comm.sudo("/sbin/ifup eth#{interface}")
-            end
           end
+
+          Tempfile.open("vagrant-debian-configure-networks") do |f|
+            f.binmode
+            f.write(entries.join("\n"))
+            f.fsync
+            f.close
+            comm.upload(f.path, "/tmp/vagrant-network-entry")
+          end
+
+          commands = []
+
+          # Bring down all the interfaces we're reconfiguring. By bringing down
+          # each specifically, we avoid reconfiguring eth0 (the NAT interface)
+          # so SSH never dies.
+          interfaces.each do |interface, _|
+            # Ubuntu 16.04+ returns an error when downing an interface that
+            # does not exist. The `|| true` preserves the behavior that older
+            # Ubuntu versions exhibit and Vagrant expects (GH-7155)
+            commands << "/sbin/ifdown 'eth#{interface}' 2> /dev/null || true"
+            commands << "/sbin/ip addr flush dev 'eth#{interface}' 2> /dev/null"
+          end
+
+          # Reconfigure /etc/network/interfaces.
+          commands << <<-EOH.gsub(/^ {12}/, "")
+            # Remove any previous network modifications from the interfaces file
+            sed -e '/^#VAGRANT-BEGIN/,$ d' /etc/network/interfaces > /tmp/vagrant-network-interfaces.pre
+            sed -ne '/^#VAGRANT-END/,$ p' /etc/network/interfaces | tac | sed -e '/^#VAGRANT-END/,$ d' | tac > /tmp/vagrant-network-interfaces.post
+
+            cat \\
+              /tmp/vagrant-network-interfaces.pre \\
+              /tmp/vagrant-network-entry \\
+              /tmp/vagrant-network-interfaces.post \\
+              > /etc/network/interfaces
+
+            rm -f /tmp/vagrant-network-interfaces.pre
+            rm -f /tmp/vagrant-network-entry
+            rm -f /tmp/vagrant-network-interfaces.post
+          EOH
+
+          # Bring back up each network interface, reconfigured.
+          interfaces.each do |interface, _|
+            commands << "/sbin/ifup 'eth#{interface}'"
+          end
+
+          # Run all the commands in one session to prevent partial configuration
+          # due to a severed network.
+          comm.sudo(commands.join("\n"))
         end
       end
     end

plugins/guests/debian/cap/nfs_client.rb

@@ -3,10 +3,11 @@ module VagrantPlugins
     module Cap
       class NFSClient
         def self.nfs_client_install(machine)
-          machine.communicate.tap do |comm|
+          comm = machine.communicate
-            comm.sudo("apt-get -y update")
+          comm.sudo <<-EOH.gsub(/^ {12}/, '')
-            comm.sudo("apt-get -y install nfs-common portmap")
+            apt-get -yqq update
-          end
+            apt-get -yqq install nfs-common portmap
+          EOH
         end
       end
     end

plugins/guests/debian/cap/rsync.rb

@@ -3,9 +3,12 @@ module VagrantPlugins
     module Cap
       class RSync
         def self.rsync_install(machine)
-          machine.communicate.tap do |comm|
+          comm = machine.communicate
-            comm.sudo("apt-get -y update")
+          if !comm.test("command -v rsync")
-            comm.sudo("apt-get -y install rsync")
+            comm.sudo <<-EOH.gsub(/^ {14}/, '')
+              apt-get -yqq update
+              apt-get -yqq install rsync
+            EOH
           end
         end
       end

plugins/guests/debian/cap/smb.rb

@@ -3,13 +3,12 @@ module VagrantPlugins
     module Cap
       class SMB
         def self.smb_install(machine)
-          # Deb/Ubuntu require mount.cifs which doesn't come by default.
+          comm = machine.communicate
-          machine.communicate.tap do |comm|
+          if !comm.test("test -f /sbin/mount.cifs")
-            if !comm.test("test -f /sbin/mount.cifs")
+            comm.sudo <<-EOH.gsub(/^ {14}/, '')
-              machine.ui.detail(I18n.t("vagrant.guest_deb_installing_smb"))
+              apt-get -yqq update
-              comm.sudo("apt-get -y update")
+              apt-get -yqq install cifs-utils
-              comm.sudo("apt-get -y install cifs-utils")
+            EOH
-            end
           end
         end
       end

plugins/guests/debian/guest.rb

@@ -1,3 +1,5 @@
+require "vagrant"
+
 module VagrantPlugins
   module GuestDebian
     class Guest < Vagrant.plugin("2", :guest)

plugins/guests/debian/plugin.rb

@@ -7,7 +7,7 @@ module VagrantPlugins
       description "Debian guest support."
 
       guest("debian", "linux") do
-        require File.expand_path("../guest", __FILE__)
+        require_relative "guest"
         Guest
       end
 

test/unit/plugins/guests/debian/cap/configure_networks_test.rb

@@ -9,14 +9,14 @@ describe "VagrantPlugins::GuestDebian::Cap::ConfigureNetworks" do
   end
 
   let(:machine) { double("machine") }
-  let(:communicator) { VagrantTests::DummyCommunicator::Communicator.new(machine) }
+  let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) }
 
   before do
-    allow(machine).to receive(:communicate).and_return(communicator)
+    allow(machine).to receive(:communicate).and_return(comm)
   end
 
   after do
-    communicator.verify_expectations!
+    comm.verify_expectations!
   end
 
   describe ".configure_networks" do
@@ -38,13 +38,14 @@ describe "VagrantPlugins::GuestDebian::Cap::ConfigureNetworks" do
     end
 
     it "creates and starts the networks" do
-      communicator.expect_command("/sbin/ifdown eth0 2> /dev/null || true")
-      communicator.expect_command("/sbin/ip addr flush dev eth0 2> /dev/null")
-      communicator.expect_command("/sbin/ifdown eth1 2> /dev/null || true")
-      communicator.expect_command("/sbin/ip addr flush dev eth1 2> /dev/null")
-      communicator.expect_command("/sbin/ifup eth0")
-      communicator.expect_command("/sbin/ifup eth1")
       described_class.configure_networks(machine, [network_0, network_1])
+
+      expect(comm.received_commands[0]).to match("/sbin/ifdown 'eth0' 2> /dev/null || true")
+      expect(comm.received_commands[0]).to match("/sbin/ip addr flush dev 'eth0' 2> /dev/null")
+      expect(comm.received_commands[0]).to match("/sbin/ifdown 'eth1' 2> /dev/null || true")
+      expect(comm.received_commands[0]).to match("/sbin/ip addr flush dev 'eth1' 2> /dev/null")
+      expect(comm.received_commands[0]).to match("/sbin/ifup 'eth0'")
+      expect(comm.received_commands[0]).to match("/sbin/ifup 'eth1'")
     end
   end
 end

test/unit/plugins/guests/debian/cap/nfs_client_test.rb

@@ -0,0 +1,30 @@
+require_relative "../../../../base"
+
+describe "VagrantPlugins::GuestDebian::Cap::NFSClient" do
+  let(:described_class) do
+    VagrantPlugins::GuestDebian::Plugin
+      .components
+      .guest_capabilities[:debian]
+      .get(:nfs_client_install)
+  end
+
+  let(:machine) { double("machine") }
+  let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) }
+
+  before do
+    allow(machine).to receive(:communicate).and_return(comm)
+  end
+
+  after do
+    comm.verify_expectations!
+  end
+
+  describe ".nfs_client_install" do
+    it "installs nfs client utilities" do
+      described_class.nfs_client_install(machine)
+
+      expect(comm.received_commands[0]).to match(/apt-get -yqq update/)
+      expect(comm.received_commands[0]).to match(/apt-get -yqq install nfs-common portmap/)
+    end
+  end
+end

test/unit/plugins/guests/debian/cap/rsync_test.rb

@@ -0,0 +1,38 @@
+require_relative "../../../../base"
+
+describe "VagrantPlugins::GuestDebian::Cap:RSync" do
+  let(:described_class) do
+    VagrantPlugins::GuestDebian::Plugin
+      .components
+      .guest_capabilities[:debian]
+      .get(:rsync_install)
+  end
+
+  let(:machine) { double("machine") }
+  let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) }
+
+  before do
+    allow(machine).to receive(:communicate).and_return(comm)
+  end
+
+  after do
+    comm.verify_expectations!
+  end
+
+  describe ".rsync_install" do
+    it "installs rsync when not installed" do
+      comm.stub_command("command -v rsync", exit_code: 1)
+      described_class.rsync_install(machine)
+
+      expect(comm.received_commands[1]).to match(/apt-get -yqq update/)
+      expect(comm.received_commands[1]).to match(/apt-get -yqq install rsync/)
+    end
+
+    it "does not install rsync when installed" do
+      comm.stub_command("command -v rsync", exit_code: 0)
+      described_class.rsync_install(machine)
+
+      expect(comm.received_commands.join("")).to_not match(/update/)
+    end
+  end
+end

test/unit/plugins/guests/debian/cap/smb_test.rb

@@ -0,0 +1,38 @@
+require_relative "../../../../base"
+
+describe "VagrantPlugins::GuestDebian::Cap::SMB" do
+  let(:described_class) do
+    VagrantPlugins::GuestDebian::Plugin
+      .components
+      .guest_capabilities[:debian]
+      .get(:smb_install)
+  end
+
+  let(:machine) { double("machine") }
+  let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) }
+
+  before do
+    allow(machine).to receive(:communicate).and_return(comm)
+  end
+
+  after do
+    comm.verify_expectations!
+  end
+
+  describe ".smb_install" do
+    it "installs smb when /sbin/mount.cifs does not exist" do
+      comm.stub_command("test -f /sbin/mount.cifs", exit_code: 1)
+      described_class.smb_install(machine)
+
+      expect(comm.received_commands[1]).to match(/apt-get -yqq update/)
+      expect(comm.received_commands[1]).to match(/apt-get -yqq install cifs-utils/)
+    end
+
+    it "does not install smb when /sbin/mount.cifs exists" do
+      comm.stub_command("test -f /sbin/mount.cifs", exit_code: 0)
+      described_class.smb_install(machine)
+
+      expect(comm.received_commands.join("")).to_not match(/update/)
+    end
+  end
+end