Merge pull request #10223 from chrisroberts/e-rhel-nmsysd

Update restart logic in redhat change_host_name cap

lib/vagrant/util/guest_inspection.rb

@@ -12,44 +12,57 @@ 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")
+          comm.test("command -v netplan")
         end
 
         ## nmcli helpers
 
         # nmcli is installed
         #
+        # @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
         #
-        # @param comm [Communicator]
+        # @param [Vagrant::Plugin::V2::Communicator] comm Guest communicator
         # @param device_name [String]
         # @return [Boolean]
         def nm_controlled?(comm, device_name)

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,27 +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
-              service network restart
             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

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("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("systemctl -q is-active systemd-networkd.service", anything).and_return(false)
-      allow(comm).to receive(:test).with("netplan -h").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)
+        expect(comm).to receive(:test).with("ps -o comm= 1 | grep systemd").and_return(true)
-        allow(comm).to receive(:test).with("netplan -h").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
@@ -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" }

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

@@ -20,24 +20,92 @@ 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/)
     end
 
-    it "does not change the hostname if already set" do
+    context "when hostnamectl is in use" do
-      comm.stub_command("hostname -f | grep '^#{name}$'", exit_code: 0)
+      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
+      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
+
+      context "when NetworkManager is in use" do
+        let(:networkd) { false }
+        let(:network_manager) { true }
+
+        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
 end