Merge pull request #10330 from briancain/fall-back-on-ifdown-ifup-tools-hostname

Reliably restart Debian and Ubuntu guests networking on setting hostname

plugins/guests/debian/cap/change_host_name.rb

@@ -1,3 +1,6 @@
+require "log4r"
+require_relative "../../linux/cap/network_interfaces"
+
 module VagrantPlugins
   module GuestDebian
     module Cap
@@ -6,6 +9,7 @@ module VagrantPlugins
         extend Vagrant::Util::GuestInspection::Linux
 
         def self.change_host_name(machine, name)
+          @logger = Log4r::Logger.new("vagrant::guest::debian::changehostname")
           comm = machine.communicate
 
           if !comm.test("hostname -f | grep '^#{name}$'", sudo: false)
@@ -44,15 +48,54 @@ module VagrantPlugins
               EOH
             end
 
-            restart_command = "/etc/init.d/networking restart"
-
+            restart_command = nil
             if systemd?(comm)
               if systemd_networkd?(comm)
+                @logger.debug("Attempting to restart networking with systemd-networkd")
                 restart_command = "systemctl restart systemd-networkd.service"
               elsif systemd_controlled?(comm, "NetworkManager.service")
+                @logger.debug("Attempting to restart networking with NetworkManager")
                 restart_command = "systemctl restart NetworkManager.service"
               end
             end
+
+            if restart_command
+              comm.sudo(restart_command)
+            else
+              restart_each_interface(machine, @logger)
+            end
+          end
+        end
+
+        protected
+
+        # Due to how most Debian systems and older Ubuntu systems handle restarting
+        # networking, we cannot simply run the networking init script or use the ifup/down
+        # tools to restart all interfaces to renew the machines DHCP lease when setting
+        # its hostname. This method is a workaround for those older systems that
+        # cannoy reliably restart networking. It restarts each individual interface
+        # on its own instead.
+        #
+        # @param [Vagrant::Machine] machine
+        # @param [Log4r::Logger] logger
+        def self.restart_each_interface(machine, logger)
+          comm = machine.communicate
+          interfaces = VagrantPlugins::GuestLinux::Cap::NetworkInterfaces.network_interfaces(machine)
+          nettools = true
+          if systemd?(comm)
+            @logger.debug("Attempting to restart networking with systemctl")
+            nettools = false
+          else
+            @logger.debug("Attempting to restart networking with ifup/down nettools")
+          end
+
+          interfaces.each do |iface|
+            logger.debug("Restarting interface #{iface} on guest #{machine.name}")
+            if nettools
+             restart_command = "ifdown #{iface};ifup #{iface}"
+            else
+             restart_command = "systemctl stop ifup@#{iface}.service;systemctl start ifup@#{iface}.service"
+            end
             comm.sudo(restart_command)
           end
         end

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

@@ -7,7 +7,8 @@ describe "VagrantPlugins::GuestDebian::Cap::ChangeHostName" do
       .guest_capabilities[:debian]
   end
 
-  let(:machine) { double("machine") }
+  let(:machine) { double("machine", name: "guestname") }
+  let(:logger) { double("logger", debug: true) }
   let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) }
 
   before do
@@ -80,10 +81,18 @@ describe "VagrantPlugins::GuestDebian::Cap::ChangeHostName" do
       context "when networkd and NetworkManager are not in use" do
         let(:networkd) { false }
         let(:network_manager) { false }
+        let(:systemd) { true }
 
-        it "restarts the network using service" do
+        it "restarts the network using systemctl" do
+          expect(cap).to receive(:restart_each_interface).
+            with(machine, anything)
+          cap.change_host_name(machine, name)
+        end
+
+        it "restarts networking with networking init script" do
+          expect(cap).to receive(:restart_each_interface).
+            with(machine, anything)
           cap.change_host_name(machine, name)
-          expect(comm.received_commands[3]).to match(/networking restart/)
         end
       end
 
@@ -91,8 +100,15 @@ describe "VagrantPlugins::GuestDebian::Cap::ChangeHostName" do
         let(:systemd) { false }
 
         it "restarts the network using service" do
+          expect(cap).to receive(:restart_each_interface).
+            with(machine, anything)
+          cap.change_host_name(machine, name)
+        end
+
+        it "restarts the network using ifdown/ifup" do
+          expect(cap).to receive(:restart_each_interface).
+            with(machine, anything)
           cap.change_host_name(machine, name)
-          expect(comm.received_commands[3]).to match(/networking restart/)
         end
       end
     end
@@ -103,4 +119,36 @@ describe "VagrantPlugins::GuestDebian::Cap::ChangeHostName" do
       expect(comm.received_commands.size).to eq(1)
     end
   end
+
+  describe ".restart_each_interface" do
+    let(:cap) { caps.get(:change_host_name) }
+    let(:systemd) { true }
+    let(:interfaces) { ["eth0", "eth1", "eth2"] }
+
+    before do
+      allow(cap).to receive(:systemd?).and_return(systemd)
+      allow(VagrantPlugins::GuestLinux::Cap::NetworkInterfaces).to receive(:network_interfaces).
+        and_return(interfaces)
+    end
+
+    context "with nettools" do
+      let(:systemd) { false }
+
+      it "restarts every interface" do
+        cap.send(:restart_each_interface, machine, logger)
+        expect(comm.received_commands[0]).to match(/ifdown eth0;ifup eth0/)
+        expect(comm.received_commands[1]).to match(/ifdown eth1;ifup eth1/)
+        expect(comm.received_commands[2]).to match(/ifdown eth2;ifup eth2/)
+      end
+    end
+
+    context "with systemctl" do
+      it "restarts every interface" do
+        cap.send(:restart_each_interface, machine, logger)
+        expect(comm.received_commands[0]).to match(/systemctl stop ifup@eth0.service;systemctl start ifup@eth0.service/)
+        expect(comm.received_commands[1]).to match(/systemctl stop ifup@eth1.service;systemctl start ifup@eth1.service/)
+        expect(comm.received_commands[2]).to match(/systemctl stop ifup@eth2.service;systemctl start ifup@eth2.service/)
+      end
+    end
+  end
 end