From 1761e65f2682b2aa0e459565c35ef5cbca672442 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 23 Oct 2018 10:17:39 -0700 Subject: [PATCH 1/3] Fixes #9763 #10300: Fall back on ifdown/ifup tools for network restart This commit adds some additional logic that falls back to using the ifdown/ifup tools to restart networking. On Ubuntu 14.04, the init script was designed to always fail to restart newtorking, so it needs to use the ifdown/up tools instead. This commit will use the networking init script as a last resort to restart networking, assuming other commands haven't broken networking already. https://bugs.launchpad.net/ubuntu/+source/ifupdown/+bug/1301015 --- plugins/guests/debian/cap/change_host_name.rb | 21 ++++++++++++++++-- .../debian/cap/change_host_name_test.rb | 22 ++++++++++++++++--- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/plugins/guests/debian/cap/change_host_name.rb b/plugins/guests/debian/cap/change_host_name.rb index d8a127100..2323734bd 100644 --- a/plugins/guests/debian/cap/change_host_name.rb +++ b/plugins/guests/debian/cap/change_host_name.rb @@ -1,3 +1,5 @@ +require "log4r" + module VagrantPlugins module GuestDebian module Cap @@ -6,6 +8,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,16 +47,30 @@ module VagrantPlugins EOH end - restart_command = "/etc/init.d/networking restart" + init_restart_command = "/etc/init.d/networking restart" + ifdownup_restart_command = "ifdown --exclude=lo -a && ifup --exclude=lo -a" + systemctl_restart_command = "systemctl stop ifup@eth0.service && systemctl start ifup@eth0.service" + restart_command = ifdownup_restart_command 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" + else + @logger.debug("Attempting to restart networking with ifup@.service") + restart_command = systemctl_restart_command end + else + @logger.debug("Attempting to restart networking with ifdown/ifup net tools") + end + + if !comm.test(restart_command, sudo: true) + @logger.debug("Attempting to restart networking with init networking service") + comm.sudo(init_restart_command) end - comm.sudo(restart_command) end end end diff --git a/test/unit/plugins/guests/debian/cap/change_host_name_test.rb b/test/unit/plugins/guests/debian/cap/change_host_name_test.rb index 73a70564c..b5a319185 100644 --- a/test/unit/plugins/guests/debian/cap/change_host_name_test.rb +++ b/test/unit/plugins/guests/debian/cap/change_host_name_test.rb @@ -80,10 +80,17 @@ 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 cap.change_host_name(machine, name) - expect(comm.received_commands[3]).to match(/networking restart/) + expect(comm.received_commands[3]).to match(/systemctl stop ifup/) + end + + it "restarts networking with networking init script" do + comm.stub_command("systemctl stop ifup@eth0.service && systemctl start ifup@eth0.service", exit_code: 1) + cap.change_host_name(machine, name) + expect(comm.received_commands[4]).to match(/networking restart/) end end @@ -91,8 +98,17 @@ describe "VagrantPlugins::GuestDebian::Cap::ChangeHostName" do let(:systemd) { false } it "restarts the network using service" do + comm.stub_command("systemctl stop ifup@eth0.service && systemctl start ifup@eth0.service", exit_code: 1) cap.change_host_name(machine, name) - expect(comm.received_commands[3]).to match(/networking restart/) + expect(comm.received_commands[4]).to match(/networking restart/) + end + + it "restarts the network using ifdown/ifup" do + comm.stub_command("systemctl stop ifup@eth0.service && systemctl start ifup@eth0.service", exit_code: 1) + comm.stub_command("/etc/init.d/networking restart", exit_code: 1) + cap.change_host_name(machine, name) + expect(comm.received_commands[4]).to match(/ifdown/) + expect(comm.received_commands[4]).to match(/ifup/) end end end From e8c6916ebcbc699b44eb976264117ce8451fd1a6 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 24 Oct 2018 11:29:59 -0700 Subject: [PATCH 2/3] Restart each interface if systemd-networkd or networkmanager is not used This commit is a workaround due to how older debian and ubuntu systems fail to properly restart networking. Instead of relying on the init scripts or ifup/down tools to restart each interface, this commit instead restarts each interface individually --- plugins/guests/debian/cap/change_host_name.rb | 52 ++++++++++++++----- .../debian/cap/change_host_name_test.rb | 52 +++++++++++++++---- 2 files changed, 81 insertions(+), 23 deletions(-) diff --git a/plugins/guests/debian/cap/change_host_name.rb b/plugins/guests/debian/cap/change_host_name.rb index 2323734bd..2db5355cd 100644 --- a/plugins/guests/debian/cap/change_host_name.rb +++ b/plugins/guests/debian/cap/change_host_name.rb @@ -1,4 +1,5 @@ require "log4r" +require_relative "../../linux/cap/network_interfaces" module VagrantPlugins module GuestDebian @@ -47,11 +48,7 @@ module VagrantPlugins EOH end - init_restart_command = "/etc/init.d/networking restart" - ifdownup_restart_command = "ifdown --exclude=lo -a && ifup --exclude=lo -a" - systemctl_restart_command = "systemctl stop ifup@eth0.service && systemctl start ifup@eth0.service" - - restart_command = ifdownup_restart_command + restart_command = nil if systemd?(comm) if systemd_networkd?(comm) @logger.debug("Attempting to restart networking with systemd-networkd") @@ -59,20 +56,49 @@ module VagrantPlugins elsif systemd_controlled?(comm, "NetworkManager.service") @logger.debug("Attempting to restart networking with NetworkManager") restart_command = "systemctl restart NetworkManager.service" - else - @logger.debug("Attempting to restart networking with ifup@.service") - restart_command = systemctl_restart_command end - else - @logger.debug("Attempting to restart networking with ifdown/ifup net tools") end - if !comm.test(restart_command, sudo: true) - @logger.debug("Attempting to restart networking with init networking service") - comm.sudo(init_restart_command) + 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 end end end diff --git a/test/unit/plugins/guests/debian/cap/change_host_name_test.rb b/test/unit/plugins/guests/debian/cap/change_host_name_test.rb index b5a319185..6beff6451 100644 --- a/test/unit/plugins/guests/debian/cap/change_host_name_test.rb +++ b/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 @@ -83,14 +84,15 @@ describe "VagrantPlugins::GuestDebian::Cap::ChangeHostName" do let(:systemd) { true } it "restarts the network using systemctl" do + expect(cap).to receive(:restart_each_interface). + with(machine, anything) cap.change_host_name(machine, name) - expect(comm.received_commands[3]).to match(/systemctl stop ifup/) end it "restarts networking with networking init script" do - comm.stub_command("systemctl stop ifup@eth0.service && systemctl start ifup@eth0.service", exit_code: 1) + expect(cap).to receive(:restart_each_interface). + with(machine, anything) cap.change_host_name(machine, name) - expect(comm.received_commands[4]).to match(/networking restart/) end end @@ -98,17 +100,15 @@ describe "VagrantPlugins::GuestDebian::Cap::ChangeHostName" do let(:systemd) { false } it "restarts the network using service" do - comm.stub_command("systemctl stop ifup@eth0.service && systemctl start ifup@eth0.service", exit_code: 1) + expect(cap).to receive(:restart_each_interface). + with(machine, anything) cap.change_host_name(machine, name) - expect(comm.received_commands[4]).to match(/networking restart/) end it "restarts the network using ifdown/ifup" do - comm.stub_command("systemctl stop ifup@eth0.service && systemctl start ifup@eth0.service", exit_code: 1) - comm.stub_command("/etc/init.d/networking restart", exit_code: 1) + expect(cap).to receive(:restart_each_interface). + with(machine, anything) cap.change_host_name(machine, name) - expect(comm.received_commands[4]).to match(/ifdown/) - expect(comm.received_commands[4]).to match(/ifup/) end end end @@ -119,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 From a1bb7b837a1adfdcd8f1be94c76d4a00bbbd80d4 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 24 Oct 2018 15:27:33 -0700 Subject: [PATCH 3/3] Use semicolon over ampersand to separate commands --- plugins/guests/debian/cap/change_host_name.rb | 4 ++-- .../guests/debian/cap/change_host_name_test.rb | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/guests/debian/cap/change_host_name.rb b/plugins/guests/debian/cap/change_host_name.rb index 2db5355cd..b64c236f8 100644 --- a/plugins/guests/debian/cap/change_host_name.rb +++ b/plugins/guests/debian/cap/change_host_name.rb @@ -92,9 +92,9 @@ module VagrantPlugins interfaces.each do |iface| logger.debug("Restarting interface #{iface} on guest #{machine.name}") if nettools - restart_command = "ifdown #{iface} && ifup #{iface}" + restart_command = "ifdown #{iface};ifup #{iface}" else - restart_command = "systemctl stop ifup@#{iface}.service && systemctl start ifup@#{iface}.service" + restart_command = "systemctl stop ifup@#{iface}.service;systemctl start ifup@#{iface}.service" end comm.sudo(restart_command) end diff --git a/test/unit/plugins/guests/debian/cap/change_host_name_test.rb b/test/unit/plugins/guests/debian/cap/change_host_name_test.rb index 6beff6451..1c05625d5 100644 --- a/test/unit/plugins/guests/debian/cap/change_host_name_test.rb +++ b/test/unit/plugins/guests/debian/cap/change_host_name_test.rb @@ -136,18 +136,18 @@ describe "VagrantPlugins::GuestDebian::Cap::ChangeHostName" do 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/) + 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/) + 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