Fix guest network configuration by properly extracting extra options

Extra options are extracted from the machine configuration for the
network being configured to allow for customized network manager
behavior. The network entries must be filtered to remove non-network
entries (like port forwards) before accessing by index.

Fixes #9546
This commit is contained in:
Chris Roberts 2018-12-19 16:08:47 -08:00
parent 7f3e842bf7
commit a4a98d97fc
4 changed files with 120 additions and 22 deletions

View File

@ -19,9 +19,12 @@ module VagrantPlugins
# Check if NetworkManager is installed on the system # Check if NetworkManager is installed on the system
nmcli_installed = nmcli?(comm) nmcli_installed = nmcli?(comm)
net_configs = machine.config.vm.networks.map do |type, opts|
opts if type.to_s.end_with?("_network")
end.compact
networks.each.with_index do |network, i| networks.each.with_index do |network, i|
network[:device] = interfaces[network[:interface]] network[:device] = interfaces[network[:interface]]
extra_opts = machine.config.vm.networks[i].last.dup extra_opts = net_configs[i].dup || {}
if nmcli_installed if nmcli_installed
# Now check if the device is actively being managed by NetworkManager # Now check if the device is actively being managed by NetworkManager
@ -46,7 +49,7 @@ module VagrantPlugins
end end
# Render a new configuration # Render a new configuration
template_options = network.merge(extra_opts) template_options = extra_opts.merge(network)
# ALT expects netmasks to be in the CIDR notation, but users may # ALT expects netmasks to be in the CIDR notation, but users may
# specify IPV4 netmasks like "255.255.255.0". This magic converts # specify IPV4 netmasks like "255.255.255.0". This magic converts

View File

@ -19,9 +19,12 @@ module VagrantPlugins
# Check if NetworkManager is installed on the system # Check if NetworkManager is installed on the system
nmcli_installed = nmcli?(comm) nmcli_installed = nmcli?(comm)
net_configs = machine.config.vm.networks.map do |type, opts|
opts if type.to_s.end_with?("_network")
end.compact
networks.each.with_index do |network, i| networks.each.with_index do |network, i|
network[:device] = interfaces[network[:interface]] network[:device] = interfaces[network[:interface]]
extra_opts = machine.config.vm.networks[i].last.dup extra_opts = net_configs[i].dup || {}
if nmcli_installed if nmcli_installed
# Now check if the device is actively being managed by NetworkManager # Now check if the device is actively being managed by NetworkManager
@ -47,7 +50,7 @@ module VagrantPlugins
# Render a new configuration # Render a new configuration
entry = TemplateRenderer.render("guests/redhat/network_#{network[:type]}", entry = TemplateRenderer.render("guests/redhat/network_#{network[:type]}",
options: network.merge(extra_opts), options: extra_opts.merge(network),
) )
# Upload the new configuration # Upload the new configuration

View File

@ -11,7 +11,7 @@ describe "VagrantPlugins::GuestALT::Cap::ConfigureNetworks" do
let(:config) { double("config", vm: vm) } let(:config) { double("config", vm: vm) }
let(:guest) { double("guest") } let(:guest) { double("guest") }
let(:machine) { double("machine", guest: guest, config: config) } let(:machine) { double("machine", guest: guest, config: config) }
let(:networks){ [[{}], [{}]] } let(:networks){ [[:public_network, network_1], [:private_network, network_2]] }
let(:vm){ double("vm", networks: networks) } let(:vm){ double("vm", networks: networks) }
before do before do
@ -57,6 +57,14 @@ describe "VagrantPlugins::GuestALT::Cap::ConfigureNetworks" do
end end
context "with NetworkManager installed" do context "with NetworkManager installed" do
let(:net1_nm_controlled) { true }
let(:net2_nm_controlled) { true }
let(:networks){ [
[:public_network, network_1.merge(nm_controlled: net1_nm_controlled)],
[:private_network, network_2.merge(nm_controlled: net2_nm_controlled)]
] }
before do before do
allow(cap).to receive(:nmcli?).and_return true allow(cap).to receive(:nmcli?).and_return true
end end
@ -67,6 +75,11 @@ describe "VagrantPlugins::GuestALT::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option omitted" do context "with nm_controlled option omitted" do
let(:networks){ [
[:public_network, network_1],
[:private_network, network_2]
] }
it "downs networks via nmcli, creates ifaces and restart NetworksManager" do it "downs networks via nmcli, creates ifaces and restart NetworksManager" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
expect(comm.received_commands[0]).to match(/nmcli.*disconnect/) expect(comm.received_commands[0]).to match(/nmcli.*disconnect/)
@ -77,8 +90,6 @@ describe "VagrantPlugins::GuestALT::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to true" do context "with nm_controlled option set to true" do
let(:networks){ [[{nm_controlled: true}], [{nm_controlled: true}]] }
it "downs networks via nmcli, creates ifaces and restart NetworksManager" do it "downs networks via nmcli, creates ifaces and restart NetworksManager" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
expect(comm.received_commands[0]).to match(/nmcli.*disconnect/) expect(comm.received_commands[0]).to match(/nmcli.*disconnect/)
@ -89,7 +100,8 @@ describe "VagrantPlugins::GuestALT::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to false" do context "with nm_controlled option set to false" do
let(:networks){ [[{nm_controlled: false}], [{nm_controlled: false}]] } let(:net1_nm_controlled) { false }
let(:net2_nm_controlled) { false }
it "downs networks manually, creates ifaces, starts networks manually and restart NetworksManager" do it "downs networks manually, creates ifaces, starts networks manually and restart NetworksManager" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
@ -102,7 +114,8 @@ describe "VagrantPlugins::GuestALT::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to false on first device" do context "with nm_controlled option set to false on first device" do
let(:networks){ [[{nm_controlled: false}], [{nm_controlled: true}]] } let(:net1_nm_controlled) { false }
let(:net2_nm_controlled) { true }
it "downs networks, creates ifaces and starts the networks with one managed manually and one NetworkManager controlled" do it "downs networks, creates ifaces and starts the networks with one managed manually and one NetworkManager controlled" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
@ -121,6 +134,11 @@ describe "VagrantPlugins::GuestALT::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option omitted" do context "with nm_controlled option omitted" do
let(:networks){ [
[:public_network, network_1],
[:private_network, network_2]
] }
it "creates and starts the networks manually" do it "creates and starts the networks manually" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
expect(comm.received_commands[0]).to match(/ifdown/) expect(comm.received_commands[0]).to match(/ifdown/)
@ -132,7 +150,8 @@ describe "VagrantPlugins::GuestALT::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to true" do context "with nm_controlled option set to true" do
let(:networks){ [[{nm_controlled: true}], [{nm_controlled: true}]] } let(:net1_nm_controlled) { true }
let(:net2_nm_controlled) { true }
it "creates and starts the networks via nmcli" do it "creates and starts the networks via nmcli" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
@ -144,7 +163,8 @@ describe "VagrantPlugins::GuestALT::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to false" do context "with nm_controlled option set to false" do
let(:networks){ [[{nm_controlled: false}], [{nm_controlled: false}]] } let(:net1_nm_controlled) { false }
let(:net2_nm_controlled) { false }
it "creates and starts the networks via ifup " do it "creates and starts the networks via ifup " do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
@ -157,7 +177,8 @@ describe "VagrantPlugins::GuestALT::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to false on first device" do context "with nm_controlled option set to false on first device" do
let(:networks){ [[{nm_controlled: false}], [{nm_controlled: true}]] } let(:net1_nm_controlled) { false }
let(:net2_nm_controlled) { true }
it "creates and starts the networks with one managed manually and one NetworkManager controlled" do it "creates and starts the networks with one managed manually and one NetworkManager controlled" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
@ -202,7 +223,10 @@ describe "VagrantPlugins::GuestALT::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set" do context "with nm_controlled option set" do
let(:networks){ [[{nm_controlled: false}], [{nm_controlled: true}]] } let(:networks){ [
[:public_network, network_1.merge(nm_controlled: true)],
[:private_network, network_2.merge(nm_controlled: true)]
] }
it "raises an error" do it "raises an error" do
expect{ cap.configure_networks(machine, [network_1, network_2]) }.to raise_error(Vagrant::Errors::NetworkManagerNotInstalled) expect{ cap.configure_networks(machine, [network_1, network_2]) }.to raise_error(Vagrant::Errors::NetworkManagerNotInstalled)

View File

@ -11,7 +11,7 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
let(:config) { double("config", vm: vm) } let(:config) { double("config", vm: vm) }
let(:guest) { double("guest") } let(:guest) { double("guest") }
let(:machine) { double("machine", guest: guest, config: config) } let(:machine) { double("machine", guest: guest, config: config) }
let(:networks){ [[{}], [{}]] } let(:networks){ [[:public_network, network_1], [:private_network, network_2]] }
let(:vm){ double("vm", networks: networks) } let(:vm){ double("vm", networks: networks) }
before do before do
@ -56,7 +56,59 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
} }
end end
let(:network_3) do
{
interface: 2,
type: "static",
ip: "33.33.33.11",
netmask: "255.255.0.0",
gateway: "33.33.0.1",
}
end
context "network configuration file" do
let(:networks){ [[:public_network, network_1], [:private_network, network_2], [:private_network, network_3]] }
let(:tempfile) { double("tempfile") }
before do
allow(tempfile).to receive(:binmode)
allow(tempfile).to receive(:write)
allow(tempfile).to receive(:fsync)
allow(tempfile).to receive(:close)
allow(tempfile).to receive(:path)
allow(Tempfile).to receive(:open).and_yield(tempfile)
end
it "should generate two configuration files" do
expect(Tempfile).to receive(:open).twice
cap.configure_networks(machine, [network_1, network_2])
end
it "should generate three configuration files" do
expect(Tempfile).to receive(:open).thrice
cap.configure_networks(machine, [network_1, network_2, network_3])
end
it "should generate configuration with network_2 IP address" do
expect(tempfile).to receive(:write).with(/#{network_2[:ip]}/)
cap.configure_networks(machine, [network_1, network_2, network_3])
end
it "should generate configuration with network_3 IP address" do
expect(tempfile).to receive(:write).with(/#{network_3[:ip]}/)
cap.configure_networks(machine, [network_1, network_2, network_3])
end
end
context "with NetworkManager installed" do context "with NetworkManager installed" do
let(:net1_nm_controlled) { true }
let(:net2_nm_controlled) { true }
let(:networks){ [
[:public_network, network_1.merge(nm_controlled: net1_nm_controlled)],
[:private_network, network_2.merge(nm_controlled: net2_nm_controlled)]
] }
before do before do
allow(cap).to receive(:nmcli?).and_return true allow(cap).to receive(:nmcli?).and_return true
end end
@ -67,6 +119,11 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option omitted" do context "with nm_controlled option omitted" do
let(:networks){ [
[:public_network, network_1],
[:private_network, network_2]
] }
it "creates and starts the networks via nmcli" do it "creates and starts the networks via nmcli" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
expect(comm.received_commands[0]).to match(/nmcli/) expect(comm.received_commands[0]).to match(/nmcli/)
@ -75,8 +132,6 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to true" do context "with nm_controlled option set to true" do
let(:networks){ [[{nm_controlled: true}], [{nm_controlled: true}]] }
it "creates and starts the networks via nmcli" do it "creates and starts the networks via nmcli" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
expect(comm.received_commands[0]).to match(/nmcli/) expect(comm.received_commands[0]).to match(/nmcli/)
@ -85,7 +140,8 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to false" do context "with nm_controlled option set to false" do
let(:networks){ [[{nm_controlled: false}], [{nm_controlled: false}]] } let(:net1_nm_controlled) { false }
let(:net2_nm_controlled) { false }
it "creates and starts the networks via ifup and disables devices in NetworkManager" do it "creates and starts the networks via ifup and disables devices in NetworkManager" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
@ -96,7 +152,8 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to false on first device" do context "with nm_controlled option set to false on first device" do
let(:networks){ [[{nm_controlled: false}], [{nm_controlled: true}]] } let(:net1_nm_controlled) { false }
let(:net2_nm_controlled) { true }
it "creates and starts the networks with one managed manually and one NetworkManager controlled" do it "creates and starts the networks with one managed manually and one NetworkManager controlled" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
@ -113,6 +170,11 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option omitted" do context "with nm_controlled option omitted" do
let(:networks){ [
[:public_network, network_1],
[:private_network, network_2]
] }
it "creates and starts the networks manually" do it "creates and starts the networks manually" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
expect(comm.received_commands[0]).to match(/ifdown/) expect(comm.received_commands[0]).to match(/ifdown/)
@ -123,7 +185,8 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to true" do context "with nm_controlled option set to true" do
let(:networks){ [[{nm_controlled: true}], [{nm_controlled: true}]] } let(:net1_nm_controlled) { true }
let(:net2_nm_controlled) { true }
it "creates and starts the networks via nmcli" do it "creates and starts the networks via nmcli" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
@ -134,7 +197,8 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to false" do context "with nm_controlled option set to false" do
let(:networks){ [[{nm_controlled: false}], [{nm_controlled: false}]] } let(:net1_nm_controlled) { false }
let(:net2_nm_controlled) { false }
it "creates and starts the networks via ifup " do it "creates and starts the networks via ifup " do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
@ -146,7 +210,8 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set to false on first device" do context "with nm_controlled option set to false on first device" do
let(:networks){ [[{nm_controlled: false}], [{nm_controlled: true}]] } let(:net1_nm_controlled) { false }
let(:net2_nm_controlled) { true }
it "creates and starts the networks with one managed manually and one NetworkManager controlled" do it "creates and starts the networks with one managed manually and one NetworkManager controlled" do
cap.configure_networks(machine, [network_1, network_2]) cap.configure_networks(machine, [network_1, network_2])
@ -174,7 +239,10 @@ describe "VagrantPlugins::GuestRedHat::Cap::ConfigureNetworks" do
end end
context "with nm_controlled option set" do context "with nm_controlled option set" do
let(:networks){ [[{nm_controlled: false}], [{nm_controlled: true}]] } let(:networks){ [
[:public_network, network_1.merge(nm_controlled: true)],
[:private_network, network_2.merge(nm_controlled: true)]
] }
it "raises an error" do it "raises an error" do
expect{ cap.configure_networks(machine, [network_1, network_2]) }.to raise_error(Vagrant::Errors::NetworkManagerNotInstalled) expect{ cap.configure_networks(machine, [network_1, network_2]) }.to raise_error(Vagrant::Errors::NetworkManagerNotInstalled)