guests/redhat: Configure networks in one command

This commit also switches to using predictable network names.

plugins/guests/redhat/cap/configure_networks.rb

@@ -1,4 +1,3 @@
-require "set"
 require "tempfile"
 
 require_relative "../../../../lib/vagrant/util/retryable"
@@ -12,7 +11,7 @@ module VagrantPlugins
         include Vagrant::Util
 
         def self.configure_networks(machine, networks)
-          case machine.guest.capability("flavor")
+          case machine.guest.capability(:flavor)
           when :rhel_7
             configure_networks_rhel7(machine, networks)
           else
@@ -21,66 +20,60 @@ module VagrantPlugins
         end
 
         def self.configure_networks_rhel7(machine, networks)
-          # This is kind of jank but the configure networks is the same
+          # This is kind of jank but the configure networks is the same as
-          # as Fedora at this point.
+          # Fedora at this point.
-          require File.expand_path("../../../fedora/cap/configure_networks", __FILE__)
+          require_relative "../../fedora/cap/configure_networks"
-          ::VagrantPlugins::GuestFedora::Cap::ConfigureNetworks.
+          ::VagrantPlugins::GuestFedora::Cap::ConfigureNetworks
-            configure_networks(machine, networks)
+            .configure_networks(machine, networks)
         end
 
         def self.configure_networks_default(machine, networks)
-          network_scripts_dir = machine.guest.capability("network_scripts_dir")
+          comm = machine.communicate
 
-          # Accumulate the configurations to add to the interfaces file as
+          network_scripts_dir = machine.guest.capability(:network_scripts_dir)
-          # well as what interfaces we're actually configuring since we use that
-          # later.
-          interfaces = Set.new
-          networks.each do |network|
-            interfaces.add(network[:interface])
 
-            # Down the interface before munging the config file. This might fail
+          interfaces = []
-            # if the interface is not actually set up yet so ignore errors.
+          commands   = []
-            machine.communicate.sudo(
-              "/sbin/ifdown eth#{network[:interface]} 2> /dev/null", error_check: false)
 
-            # Remove any previous vagrant configuration in this network interface's
+          comm.sudo("ifconfig -a | grep -o ^[0-9a-z]* | grep -v '^lo'") do |_, stdout|
-            # configuration files.
+            interfaces = stdout.split("\n")
-            machine.communicate.sudo("touch #{network_scripts_dir}/ifcfg-eth#{network[:interface]}")
+          end
-            machine.communicate.sudo("sed -e '/^#VAGRANT-BEGIN/,/^#VAGRANT-END/ d' #{network_scripts_dir}/ifcfg-eth#{network[:interface]} > /tmp/vagrant-ifcfg-eth#{network[:interface]}")
-            machine.communicate.sudo("cat /tmp/vagrant-ifcfg-eth#{network[:interface]} > #{network_scripts_dir}/ifcfg-eth#{network[:interface]}")
-            machine.communicate.sudo("rm -f /tmp/vagrant-ifcfg-eth#{network[:interface]}")
 
-            # Render and upload the network entry file to a deterministic
+          networks.each.with_index do |network, i|
-            # temporary location.
+            network[:device] = interfaces[network[:interface]]
+
+            # Render a new configuration
             entry = TemplateRenderer.render("guests/redhat/network_#{network[:type]}",
-                                            options: network)
+              options: network,
+            )
 
-            Tempfile.open("vagrant-red-hat-configure-networks") do |f|
+            # Upload the new configuration
+            remote_path = "/tmp/vagrant-network-entry-#{network[:device]}-#{Time.now.to_i}-#{i}"
+            Tempfile.open("vagrant-redhat-configure-networks") do |f|
               f.binmode
               f.write(entry)
               f.fsync
               f.close
-              machine.communicate.upload(f.path, "/tmp/vagrant-network-entry_#{network[:interface]}")
+              machine.communicate.upload(f.path, remote_path)
-            end
-          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|
-            retryable(on: Vagrant::Errors::VagrantError, tries: 3, sleep: 2) do
-              # The interface should already be down so this probably
-              # won't do anything, so we run it with error_check false.
-              machine.communicate.sudo(
-                "/sbin/ifdown eth#{interface} 2> /dev/null", error_check: false)
-
-              # Add the new interface and bring it up
-              machine.communicate.sudo("cat /tmp/vagrant-network-entry_#{interface} >> #{network_scripts_dir}/ifcfg-eth#{interface}")
-              machine.communicate.sudo("ARPCHECK=no /sbin/ifup eth#{interface} 2> /dev/null")
             end
 
-            machine.communicate.sudo("rm -f /tmp/vagrant-network-entry_#{interface}")
+            # Add the new interface and bring it back up
+            final_path = "#{network_scripts_dir}/ifcfg-#{network[:device]}"
+            commands << <<-EOH.gsub(/^ {14}/, '')
+              # Down the interface before munging the config file. This might
+              # fail if the interface is not actually set up yet so ignore
+              # errors.
+              /sbin/ifdown '#{network[:device]}' || true
+
+              # Move new config into place
+              mv '#{remote_path}' '#{final_path}'
+
+              # Bring the interface up
+              ARPCHECK=no /sbin/ifup '#{network[:device]}'
+            EOH
           end
+
+          comm.sudo(commands.join("\n"))
         end
       end
     end

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

@@ -0,0 +1,80 @@
+require_relative "../../../../base"
+
+describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
+  let(:caps) do
+    VagrantPlugins::GuestRedHat::Plugin
+      .components
+      .guest_capabilities[:redhat]
+  end
+
+  let(:machine) { double("machine") }
+  let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) }
+
+  before do
+    allow(machine).to receive(:communicate).and_return(comm)
+    comm.stub_command("ifconfig -a | grep -o ^[0-9a-z]* | grep -v '^lo'",
+      stdout: "eth1\neth2")
+  end
+
+  after do
+    comm.verify_expectations!
+  end
+
+  describe ".configure_networks" do
+    let(:cap) { caps.get(:configure_networks) }
+
+    let(:network_1) do
+      {
+        interface: 0,
+        type: "dhcp",
+      }
+    end
+
+    let(:network_2) do
+      {
+        interface: 1,
+        type: "static",
+        ip: "33.33.33.10",
+        netmask: "255.255.0.0",
+        gateway: "33.33.0.1",
+      }
+    end
+
+    let(:network_scripts_dir) { "/" }
+
+    let(:capability) { double("capability") }
+
+    before do
+      allow(machine).to receive(:guest).and_return(capability)
+      allow(capability).to receive(:capability)
+        .with(:network_scripts_dir)
+        .and_return(network_scripts_dir)
+    end
+
+    it "uses fedora for rhel7 configuration" do
+      require_relative "../../../../../../plugins/guests/fedora/cap/configure_networks"
+
+      allow(capability).to receive(:capability)
+        .with(:flavor)
+        .and_return(:rhel_7)
+      allow(VagrantPlugins::GuestFedora::Cap::ConfigureNetworks)
+        .to receive(:configure_networks)
+
+      expect(VagrantPlugins::GuestFedora::Cap::ConfigureNetworks)
+        .to receive(:configure_networks).once
+      cap.configure_networks(machine, [network_1, network_2])
+    end
+
+    it "creates and starts the networks" do
+      allow(capability).to receive(:capability)
+        .with(:flavor)
+        .and_return(:rhel)
+
+      cap.configure_networks(machine, [network_1, network_2])
+      expect(comm.received_commands[1]).to match(/\/sbin\/ifdown 'eth1'/)
+      expect(comm.received_commands[1]).to match(/\/sbin\/ifup 'eth1'/)
+      expect(comm.received_commands[1]).to match(/\/sbin\/ifdown 'eth2'/)
+      expect(comm.received_commands[1]).to match(/\/sbin\/ifup 'eth2'/)
+    end
+  end
+end