From 80006251f422a8d534ff9bafa0e0c45d9c98143c Mon Sep 17 00:00:00 2001 From: Joe Doss Date: Mon, 17 Sep 2018 13:30:55 -0500 Subject: [PATCH 01/10] Add in logic to restart NetworkManager if it is enabled. --- plugins/guests/redhat/cap/change_host_name.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/plugins/guests/redhat/cap/change_host_name.rb b/plugins/guests/redhat/cap/change_host_name.rb index 55fcdc4b4..8f3265003 100644 --- a/plugins/guests/redhat/cap/change_host_name.rb +++ b/plugins/guests/redhat/cap/change_host_name.rb @@ -29,7 +29,13 @@ module VagrantPlugins } # Restart network - service network restart + if (test -f /etc/init.d/network && /etc/init.d/network status &> /dev/null ); then + service network restart + elif (test -f /usr/lib/systemd/system/NetworkManager.service && systemctl is-enabled NetworkManager.service &> /dev/null ); then + systemctl restart NetworkManager.service + else + printf "Could not restart the network to set the new hostname!\n" + fi EOH end end From 94954739b53ee4c6741a35c366c2fe5c9853e0ed Mon Sep 17 00:00:00 2001 From: Joe Doss Date: Mon, 17 Sep 2018 14:30:57 -0500 Subject: [PATCH 02/10] Simplified if statements. --- plugins/guests/redhat/cap/change_host_name.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/guests/redhat/cap/change_host_name.rb b/plugins/guests/redhat/cap/change_host_name.rb index 8f3265003..e9c0b1d80 100644 --- a/plugins/guests/redhat/cap/change_host_name.rb +++ b/plugins/guests/redhat/cap/change_host_name.rb @@ -29,9 +29,9 @@ module VagrantPlugins } # Restart network - if (test -f /etc/init.d/network && /etc/init.d/network status &> /dev/null ); then + if test -f /etc/init.d/network; then service network restart - elif (test -f /usr/lib/systemd/system/NetworkManager.service && systemctl is-enabled NetworkManager.service &> /dev/null ); then + elif systemctl -q is-enabled NetworkManager.service; then systemctl restart NetworkManager.service else printf "Could not restart the network to set the new hostname!\n" From c14a4a09f723230682c5ef5f9dc53662e2968b92 Mon Sep 17 00:00:00 2001 From: Joe Doss Date: Mon, 17 Sep 2018 14:56:06 -0500 Subject: [PATCH 03/10] Switch if statements, check for systemctl, and switch to is-active. --- plugins/guests/redhat/cap/change_host_name.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/guests/redhat/cap/change_host_name.rb b/plugins/guests/redhat/cap/change_host_name.rb index e9c0b1d80..70bd49694 100644 --- a/plugins/guests/redhat/cap/change_host_name.rb +++ b/plugins/guests/redhat/cap/change_host_name.rb @@ -29,10 +29,10 @@ module VagrantPlugins } # Restart network - if test -f /etc/init.d/network; then - service network restart - elif systemctl -q is-enabled NetworkManager.service; then + if (test -f /usr/bin/systemctl && systemctl -q is-active NetworkManager.service); then systemctl restart NetworkManager.service + elif test -f /etc/init.d/network; then + service network restart else printf "Could not restart the network to set the new hostname!\n" fi From 19aa9578c797c99a8632955a703490d5e6b50a26 Mon Sep 17 00:00:00 2001 From: Joe Doss Date: Tue, 18 Sep 2018 13:15:26 -0500 Subject: [PATCH 04/10] Exit 1 if we cannot set the hostname. --- plugins/guests/redhat/cap/change_host_name.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/guests/redhat/cap/change_host_name.rb b/plugins/guests/redhat/cap/change_host_name.rb index 70bd49694..ae0246015 100644 --- a/plugins/guests/redhat/cap/change_host_name.rb +++ b/plugins/guests/redhat/cap/change_host_name.rb @@ -35,6 +35,7 @@ module VagrantPlugins service network restart else printf "Could not restart the network to set the new hostname!\n" + exit 1 fi EOH end From 86ab4533b180009ed476026374110fc0bd79f522 Mon Sep 17 00:00:00 2001 From: Joe Doss Date: Tue, 18 Sep 2018 13:16:12 -0500 Subject: [PATCH 05/10] Fix the test to check for systemctl restart NetworkManager.service too. --- test/unit/plugins/guests/redhat/cap/change_host_name_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb b/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb index 766293545..10f43a359 100644 --- a/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb +++ b/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb @@ -31,7 +31,7 @@ describe "VagrantPlugins::GuestRedHat::Cap::ChangeHostName" do expect(comm.received_commands[1]).to match(/\/etc\/sysconfig\/network-scripts\/ifcfg/) expect(comm.received_commands[1]).to match(/hostnamectl set-hostname --static '#{name}'/) expect(comm.received_commands[1]).to match(/hostnamectl set-hostname --transient '#{name}'/) - expect(comm.received_commands[1]).to match(/service network restart/) + expect(comm.received_commands[1]).to match(/service network restart|systemctl restart NetworkManager.service/) end it "does not change the hostname if already set" do From a12b09b098ea87eec815e166d0e1395f6f47f937 Mon Sep 17 00:00:00 2001 From: shotop Date: Thu, 20 Sep 2018 14:21:40 -0500 Subject: [PATCH 06/10] add specs around network restart logic --- .../redhat/cap/change_host_name_test.rb | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb b/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb index 10f43a359..b85802e94 100644 --- a/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb +++ b/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb @@ -39,5 +39,32 @@ describe "VagrantPlugins::GuestRedHat::Cap::ChangeHostName" do cap.change_host_name(machine, name) expect(comm.received_commands.size).to eq(1) end + + context "restarts the network" do + it "uses systemctl and NetworkManager.service" do + comm.stub_command("hostname -f | grep '^#{name}$'", exit_code: 1) + comm.stub_command("test -f /usr/bin/systemctl && systemctl -q is-active NetworkManager.service", exit_code: 0) + cap.change_host_name(machine, name) + expect(comm.received_commands[1]).to match(/systemctl restart NetworkManager.service/) + end + + it "uses the service command" do + comm.stub_command("hostname -f | grep '^#{name}$'", exit_code: 1) + comm.stub_command("test -f /usr/bin/systemctl && systemctl -q is-active NetworkManager.service", exit_code: 1) + comm.stub_command("test -f /etc/init.d/network", exit_code: 0) + cap.change_host_name(machine, name) + expect(comm.received_commands[1]).to match(/service network restart/) + end + end + + context "cannot restart the network" do + it "prints cannot restart message" do + comm.stub_command("hostname -f | grep '^#{name}$'", exit_code: 1) + comm.stub_command("test -f /usr/bin/systemctl && systemctl -q is-active NetworkManager.service", exit_code: 1) + comm.stub_command("test -f /etc/init.d/network", exit_code: 1) + cap.change_host_name(machine, name) + expect(comm.received_commands[1]).to match(/printf "Could not restart the network to set the new hostname!/) + end + end end end From fb5fc0e657a10ee1eaf046980827cc1802c4d0f9 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 20 Sep 2018 16:42:39 -0700 Subject: [PATCH 07/10] Update guest inspection utility module Use sudo option for communicator test command instead of inline sudo to properly use configured sudo value. Use command for checking availability of hostnamectl. Use is-active for determining if a service is being actively managed by systemd. --- lib/vagrant/util/guest_inspection.rb | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/vagrant/util/guest_inspection.rb b/lib/vagrant/util/guest_inspection.rb index 86ab1dc69..5a1902d8d 100644 --- a/lib/vagrant/util/guest_inspection.rb +++ b/lib/vagrant/util/guest_inspection.rb @@ -12,27 +12,39 @@ module Vagrant # # @return [Boolean] def systemd?(comm) - comm.test("sudo ps -o comm= 1 | grep systemd") + comm.test("ps -o comm= 1 | grep systemd") end # systemd-networkd.service is in use # + # @param [Vagrant::Plugin::V2::Communicator] comm Guest communicator # @return [Boolean] def systemd_networkd?(comm) - comm.test("sudo systemctl status systemd-networkd.service") + comm.test("systemctl -q is-active systemd-networkd.service", sudo: true) + end + + # Check if given service is controlled by systemd + # + # @param [Vagrant::Plugin::V2::Communicator] comm Guest communicator + # @param [String] service_name Name of the service to check + # @return [Boolean] + def systemd_controlled?(comm, service_name) + comm.test("systemctl -q is-active #{service_name}", sudo: true) end # systemd hostname set is via hostnamectl # + # @param [Vagrant::Plugin::V2::Communicator] comm Guest communicator # @return [Boolean] def hostnamectl?(comm) - comm.test("hostnamectl") + comm.test("command -v hostnamectl") end ## netplan helpers # netplan is installed # + # @param [Vagrant::Plugin::V2::Communicator] comm Guest communicator # @return [Boolean] def netplan?(comm) comm.test("netplan -h") @@ -42,6 +54,7 @@ module Vagrant # nmcli is installed # + # @param [Vagrant::Plugin::V2::Communicator] comm Guest communicator # @return [Boolean] def nmcli?(comm) comm.test("nmcli -t") @@ -49,7 +62,7 @@ module Vagrant # NetworkManager currently controls device # - # @param comm [Communicator] + # @param [Vagrant::Plugin::V2::Communicator] comm Guest communicator # @param device_name [String] # @return [Boolean] def nm_controlled?(comm, device_name) From ff021fcab404c95e52566bfca4207da9c0101e01 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 20 Sep 2018 16:44:08 -0700 Subject: [PATCH 08/10] Update redhat change host name capability to support systemd Update capability to use guest inspection module for determining correct actions to execute. When systemd is in use restart the correct active service, either NetworkManager or networkd. Default to using the original service restart when systemd service is not found. --- plugins/guests/redhat/cap/change_host_name.rb | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/plugins/guests/redhat/cap/change_host_name.rb b/plugins/guests/redhat/cap/change_host_name.rb index ae0246015..5da660df0 100644 --- a/plugins/guests/redhat/cap/change_host_name.rb +++ b/plugins/guests/redhat/cap/change_host_name.rb @@ -2,6 +2,9 @@ module VagrantPlugins module GuestRedHat module Cap class ChangeHostName + + extend Vagrant::Util::GuestInspection + def self.change_host_name(machine, name) comm = machine.communicate @@ -10,34 +13,32 @@ module VagrantPlugins comm.sudo <<-EOH.gsub(/^ {14}/, '') # Update sysconfig sed -i 's/\\(HOSTNAME=\\).*/\\1#{name}/' /etc/sysconfig/network - # Update DNS sed -i 's/\\(DHCP_HOSTNAME=\\).*/\\1\"#{basename}\"/' /etc/sysconfig/network-scripts/ifcfg-* - # Set the hostname - use hostnamectl if available echo '#{name}' > /etc/hostname - if command -v hostnamectl; then - hostnamectl set-hostname --static '#{name}' - hostnamectl set-hostname --transient '#{name}' - else - hostname -F /etc/hostname - fi - - # Prepend ourselves to /etc/hosts grep -w '#{name}' /etc/hosts || { sed -i'' '1i 127.0.0.1\\t#{name}\\t#{basename}' /etc/hosts } - - # Restart network - if (test -f /usr/bin/systemctl && systemctl -q is-active NetworkManager.service); then - systemctl restart NetworkManager.service - elif test -f /etc/init.d/network; then - service network restart - else - printf "Could not restart the network to set the new hostname!\n" - exit 1 - fi EOH + + if hostnamectl?(comm) + comm.sudo("hostnamectl set-hostname --static '#{name}' ; " \ + "hostnamectl set-hostname --transient '#{name}'") + else + comm.sudo("hostname -F /etc/hostname") + end + + restart_command = "service network restart" + + if systemd? + if systemd_networkd?(comm) + restart_command = "systemctl restart systemd-networkd.service" + elsif systemd_controlled?(comm, "NetworkManager.service") + restart_command = "systemctl restart NetworkManager.service" + end + end + comm.sudo(restart_command) end end end From bc217d5e577457df5ac4ecdfffa17fd0a8c85b18 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 20 Sep 2018 16:46:45 -0700 Subject: [PATCH 09/10] Update redhat change host name capability tests for systemd/NetworkManger updates --- .../redhat/cap/change_host_name_test.rb | 101 ++++++++++++------ 1 file changed, 71 insertions(+), 30 deletions(-) diff --git a/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb b/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb index b85802e94..8d0c9ebd4 100644 --- a/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb +++ b/test/unit/plugins/guests/redhat/cap/change_host_name_test.rb @@ -20,50 +20,91 @@ describe "VagrantPlugins::GuestRedHat::Cap::ChangeHostName" do describe ".change_host_name" do let(:cap) { caps.get(:change_host_name) } - let(:name) { "banana-rama.example.com" } + let(:hostname_changed) { true } + let(:systemd) { true } + let(:hostnamectl) { true } + let(:networkd) { true } + let(:network_manager) { false } + + before do + comm.stub_command("hostname -f | grep '^#{name}$'", exit_code: hostname_changed ? 1 : 0) + allow(cap).to receive(:systemd?).and_return(systemd) + allow(cap).to receive(:hostnamectl?).and_return(hostnamectl) + allow(cap).to receive(:systemd_networkd?).and_return(networkd) + allow(cap).to receive(:systemd_controlled?).with(anything, /NetworkManager/).and_return(network_manager) + end it "sets the hostname" do - comm.stub_command("hostname -f | grep '^#{name}$'", exit_code: 1) - cap.change_host_name(machine, name) expect(comm.received_commands[1]).to match(/\/etc\/sysconfig\/network/) expect(comm.received_commands[1]).to match(/\/etc\/sysconfig\/network-scripts\/ifcfg/) - expect(comm.received_commands[1]).to match(/hostnamectl set-hostname --static '#{name}'/) - expect(comm.received_commands[1]).to match(/hostnamectl set-hostname --transient '#{name}'/) - expect(comm.received_commands[1]).to match(/service network restart|systemctl restart NetworkManager.service/) end - it "does not change the hostname if already set" do - comm.stub_command("hostname -f | grep '^#{name}$'", exit_code: 0) - cap.change_host_name(machine, name) - expect(comm.received_commands.size).to eq(1) + context "when hostnamectl is in use" do + let(:hostnamectl) { true } + + it "sets hostname with hostnamectl" do + cap.change_host_name(machine, name) + expect(comm.received_commands[2]).to match(/hostnamectl/) + end + end + + context "when hostnamectl is not in use" do + let(:hostnamectl) { false } + + it "sets hostname with hostname command" do + cap.change_host_name(machine, name) + expect(comm.received_commands[2]).to match(/hostname -F/) + end + end + + context "when host name is already set" do + let(:hostname_changed) { false } + + it "does not change the hostname" do + cap.change_host_name(machine, name) + expect(comm.received_commands.size).to eq(1) + end end context "restarts the network" do - it "uses systemctl and NetworkManager.service" do - comm.stub_command("hostname -f | grep '^#{name}$'", exit_code: 1) - comm.stub_command("test -f /usr/bin/systemctl && systemctl -q is-active NetworkManager.service", exit_code: 0) - cap.change_host_name(machine, name) - expect(comm.received_commands[1]).to match(/systemctl restart NetworkManager.service/) + context "when networkd is in use" do + let(:networkd) { true } + + it "restarts networkd with systemctl" do + cap.change_host_name(machine, name) + expect(comm.received_commands[3]).to match(/systemctl restart systemd-networkd/) + end end - it "uses the service command" do - comm.stub_command("hostname -f | grep '^#{name}$'", exit_code: 1) - comm.stub_command("test -f /usr/bin/systemctl && systemctl -q is-active NetworkManager.service", exit_code: 1) - comm.stub_command("test -f /etc/init.d/network", exit_code: 0) - cap.change_host_name(machine, name) - expect(comm.received_commands[1]).to match(/service network restart/) - end - end + context "when NetworkManager is in use" do + let(:networkd) { false } + let(:network_manager) { true } - context "cannot restart the network" do - it "prints cannot restart message" do - comm.stub_command("hostname -f | grep '^#{name}$'", exit_code: 1) - comm.stub_command("test -f /usr/bin/systemctl && systemctl -q is-active NetworkManager.service", exit_code: 1) - comm.stub_command("test -f /etc/init.d/network", exit_code: 1) - cap.change_host_name(machine, name) - expect(comm.received_commands[1]).to match(/printf "Could not restart the network to set the new hostname!/) + it "restarts NetworkManager with systemctl" do + cap.change_host_name(machine, name) + expect(comm.received_commands[3]).to match(/systemctl restart NetworkManager/) + end + end + + context "when networkd and NetworkManager are not in use" do + let(:networkd) { false } + let(:network_manager) { false } + + it "restarts the network using service" do + cap.change_host_name(machine, name) + expect(comm.received_commands[3]).to match(/service network restart/) + end + end + + context "when systemd is not in use" do + let(:systemd) { false } + + it "restarts the network using service" do + cap.change_host_name(machine, name) + expect(comm.received_commands[3]).to match(/service network restart/) + end end end end From 8fd05fe3c1b773777f08ca50dd651cbaf33838d3 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 21 Sep 2018 09:19:40 -0700 Subject: [PATCH 10/10] Use `command -v` for checks in all inspection helpers. Fix stubs in tests. --- lib/vagrant/util/guest_inspection.rb | 4 ++-- .../guests/debian/cap/configure_networks_test.rb | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/vagrant/util/guest_inspection.rb b/lib/vagrant/util/guest_inspection.rb index 5a1902d8d..cd0a96d3e 100644 --- a/lib/vagrant/util/guest_inspection.rb +++ b/lib/vagrant/util/guest_inspection.rb @@ -47,7 +47,7 @@ module Vagrant # @param [Vagrant::Plugin::V2::Communicator] comm Guest communicator # @return [Boolean] def netplan?(comm) - comm.test("netplan -h") + comm.test("command -v netplan") end ## nmcli helpers @@ -57,7 +57,7 @@ module Vagrant # @param [Vagrant::Plugin::V2::Communicator] comm Guest communicator # @return [Boolean] def nmcli?(comm) - comm.test("nmcli -t") + comm.test("command -v nmcli") end # NetworkManager currently controls device diff --git a/test/unit/plugins/guests/debian/cap/configure_networks_test.rb b/test/unit/plugins/guests/debian/cap/configure_networks_test.rb index b4691d0fd..d3a523c95 100644 --- a/test/unit/plugins/guests/debian/cap/configure_networks_test.rb +++ b/test/unit/plugins/guests/debian/cap/configure_networks_test.rb @@ -67,9 +67,9 @@ describe "VagrantPlugins::GuestDebian::Cap::ConfigureNetworks" do before do allow(comm).to receive(:test).with("nmcli -t d show eth1").and_return(false) allow(comm).to receive(:test).with("nmcli -t d show eth2").and_return(false) - allow(comm).to receive(:test).with("sudo ps -o comm= 1 | grep systemd").and_return(false) - allow(comm).to receive(:test).with("sudo systemctl status systemd-networkd.service").and_return(false) - allow(comm).to receive(:test).with("netplan -h").and_return(false) + allow(comm).to receive(:test).with("ps -o comm= 1 | grep systemd").and_return(false) + allow(comm).to receive(:test).with("systemctl -q is-active systemd-networkd.service", anything).and_return(false) + allow(comm).to receive(:test).with("command -v netplan").and_return(false) end it "creates and starts the networks using net-tools" do @@ -85,8 +85,8 @@ describe "VagrantPlugins::GuestDebian::Cap::ConfigureNetworks" do context "with systemd" do before do - expect(comm).to receive(:test).with("sudo ps -o comm= 1 | grep systemd").and_return(true) - allow(comm).to receive(:test).with("netplan -h").and_return(false) + expect(comm).to receive(:test).with("ps -o comm= 1 | grep systemd").and_return(true) + allow(comm).to receive(:test).with("command -v netplan").and_return(false) end it "creates and starts the networks using net-tools" do @@ -102,7 +102,7 @@ describe "VagrantPlugins::GuestDebian::Cap::ConfigureNetworks" do context "with systemd-networkd" do before do - expect(comm).to receive(:test).with("sudo systemctl status systemd-networkd.service").and_return(true) + expect(comm).to receive(:test).with("systemctl -q is-active systemd-networkd.service", anything).and_return(true) end it "creates and starts the networks using systemd-networkd" do @@ -117,7 +117,7 @@ describe "VagrantPlugins::GuestDebian::Cap::ConfigureNetworks" do context "with netplan" do before do - expect(comm).to receive(:test).with("netplan -h").and_return(true) + expect(comm).to receive(:test).with("command -v netplan").and_return(true) end let(:nm_yml) { "---\nnetwork:\n version: 2\n renderer: NetworkManager\n ethernets:\n eth1:\n dhcp4: true\n eth2:\n addresses:\n - 33.33.33.10/16\n gateway4: 33.33.0.1\n" }