guests/freebsd: Configure predictable networks in a single command

This commit refactors the freebsd networking to:

1. Use predictable network naming
2. Properly handle DHCP vs static networks on up and reload [GH-5852]
3. Perform all networking configuration in a single command to prevent
   partial configuration.

plugins/guests/freebsd/cap/configure_networks.rb

@@ -9,40 +9,57 @@ module VagrantPlugins
         include Vagrant::Util
 
         def self.configure_networks(machine, networks)
-          # Remove any previous network additions to the configuration file.
-          machine.communicate.sudo("sed -i '' -e '/^#VAGRANT-BEGIN/,/^#VAGRANT-END/ d' /etc/rc.conf", {shell: "sh"})
+          options = { shell: "sh" }
+          comm = machine.communicate
 
-          networks.each do |network|
-            # Determine the interface prefix...
-            command = "ifconfig -a | grep -o ^[0-9a-z]*"
-            result = ""
-            ifname = ""
-            machine.communicate.execute(command) do |type, data|
-              result << data if type == :stdout
-              if result.split(/\n/).any?{|i| i.match(/vtnet*/)}
-                ifname = "vtnet#{network[:interface]}"
-              else
-                ifname = "em#{network[:interface]}"
-              end
+          commands   = []
+          interfaces = []
+
+          # Remove any previous network additions to the configuration file.
+          commands << "sed -i'' -e '/^#VAGRANT-BEGIN/,/^#VAGRANT-END/ d' /etc/rc.conf"
+
+          comm.sudo("ifconfig -a | grep -o ^[0-9a-z]* | grep -v '^lo'", options) do |_, stdout|
+            interfaces = stdout.split("\n")
           end
 
+          networks.each.with_index do |network, i|
+            network[:device] = interfaces[network[:interface]]
+
             entry = TemplateRenderer.render("guests/freebsd/network_#{network[:type]}",
-                                            options: network, ifname: ifname)
+              options: network,
+            )
+
+            remote_path = "/tmp/vagrant-network-#{network[:device]}-#{Time.now.to_i}-#{i}"
 
             Tempfile.open("vagrant-freebsd-configure-networks") do |f|
               f.binmode
               f.write(entry)
               f.fsync
               f.close
-              machine.communicate.upload(f.path, "/tmp/vagrant-network-entry")
+              comm.upload(f.path, remote_path)
             end
 
-            machine.communicate.sudo("su -m root -c 'cat /tmp/vagrant-network-entry >> /etc/rc.conf'", {shell: "sh"})
-            machine.communicate.sudo("rm -f /tmp/vagrant-network-entry", {shell: "sh"})
+            commands << <<-EOH.gsub(/^ {14}/, '')
+              cat '#{remote_path}' >> /etc/rc.conf
+              rm -f '#{remote_path}'
+            EOH
 
-            # Restart interface so it loads configuration stored in /etc/rc.conf
-            machine.communicate.sudo("service netif restart #{ifname}", {shell: "sh"})
-          end
+            # If the network is DHCP, then we have to start the dhclient, unless
+            # it is already running. See GH-5852 for more information
+            if network[:type].to_sym == :dhcp
+              file = "/var/run/dhclient.#{network[:device]}.pid"
+              commands << <<-EOH.gsub(/^ {16}/, '')
+                if ! test -f '#{file}' || ! kill -0 $(cat '#{file}'); then
+                  dhclient '#{network[:device]}'
+                fi
+              EOH
+            end
+
+            # For some reason, this returns status 1... every time
+            commands << "/etc/rc.d/netif restart '#{network[:device]}' || true"
+          end
+
+          comm.sudo(commands.join("\n"), options)
         end
       end
     end

templates/guests/freebsd/network_dhcp.erb

@@ -1,3 +1,4 @@
 #VAGRANT-BEGIN
-ifconfig_<%= ifname %>="DHCP"
+ifconfig_<%= options[:device] %>="DHCP"
+synchronous_dhclient="YES"
 #VAGRANT-END

templates/guests/freebsd/network_static.erb

@@ -1,5 +1,5 @@
 #VAGRANT-BEGIN
-ifconfig_<%= ifname %>="inet <%= options[:ip] %> netmask <%= options[:netmask] %>"
+ifconfig_<%= options[:device] %>="inet <%= options[:ip] %> netmask <%= options[:netmask] %>"
 <% if options[:gateway] %>
 default_router="<%= options[:gateway] %>"
 <% end %>

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

@@ -0,0 +1,51 @@
+require_relative "../../../../base"
+
+describe "VagrantPlugins::GuestFreeBSD::Cap::ConfigureNetworks" do
+  let(:described_class) do
+    VagrantPlugins::GuestFreeBSD::Plugin
+      .components
+      .guest_capabilities[:freebsd]
+      .get(:configure_networks)
+  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: "em1\nem2")
+  end
+
+  after do
+    comm.verify_expectations!
+  end
+
+  describe ".configure_networks" do
+    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
+
+    it "creates and starts the networks" do
+      described_class.configure_networks(machine, [network_1, network_2])
+      expect(comm.received_commands[1]).to match(/dhclient 'em1'/)
+      expect(comm.received_commands[1]).to match(/\/etc\/rc.d\/netif restart 'em1'/)
+
+      expect(comm.received_commands[1]).to_not match(/dhclient 'em2'/)
+      expect(comm.received_commands[1]).to match(/\/etc\/rc.d\/netif restart 'em2'/)
+    end
+  end
+end