From 45cb26348ecafa409b35f023a422b7d626d63425 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 4 Nov 2021 14:52:52 -0700 Subject: [PATCH 1/4] Update atomic detection to prevent overlap with other guests --- plugins/guests/atomic/guest.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/guests/atomic/guest.rb b/plugins/guests/atomic/guest.rb index 3fbb439db..433a08274 100644 --- a/plugins/guests/atomic/guest.rb +++ b/plugins/guests/atomic/guest.rb @@ -4,7 +4,7 @@ module VagrantPlugins module GuestAtomic class Guest < Vagrant.plugin("2", :guest) def detect?(machine) - machine.communicate.test("grep 'ostree=' /proc/cmdline") + machine.communicate.test("grep 'ostree=.*atomic' /proc/cmdline") end end end From 657b2a39d925af2248e32b8176fcc75ea8f1e085 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 4 Nov 2021 14:53:22 -0700 Subject: [PATCH 2/4] Fix network configuration for coreos guests --- .../guests/coreos/cap/configure_networks.rb | 62 +++ .../coreos/cap/configure_networks_test.rb | 402 ++++++++++++------ 2 files changed, 335 insertions(+), 129 deletions(-) diff --git a/plugins/guests/coreos/cap/configure_networks.rb b/plugins/guests/coreos/cap/configure_networks.rb index a954b45bd..e4f3e1d79 100644 --- a/plugins/guests/coreos/cap/configure_networks.rb +++ b/plugins/guests/coreos/cap/configure_networks.rb @@ -1,4 +1,5 @@ require "tempfile" +require "yaml" require_relative "../../../../lib/vagrant/util/template_renderer" @@ -8,9 +9,70 @@ module VagrantPlugins class ConfigureNetworks extend Vagrant::Util::GuestInspection::Linux + NETWORK_MANAGER_CONN_DIR = "/etc/NetworkManager/system-connections".freeze DEFAULT_ENVIRONMENT_IP = "127.0.0.1".freeze def self.configure_networks(machine, networks) + comm = machine.communicate + return configure_networks_cloud_init(machine, networks) if comm.test("command -v cloud-init") + + interfaces = machine.guest.capability(:network_interfaces) + nm_dev = {} + comm.execute("nmcli -t c show") do |type, data| + if type == :stdout + _, id, _, dev = data.strip.split(":") + nm_dev[dev] = id + end + end + comm.sudo("rm #{File.join(NETWORK_MANAGER_CONN_DIR, 'vagrant-*.conf')}") + + networks.each_with_index do |network, i| + network[:device] = interfaces[network[:interface]] + addr = IPAddr.new(network[:ip]) + mask = addr.mask(network[:netmask]) + if !network[:mac_address] + comm.execute("cat /sys/class/net/#{network[:device]}/address") do |type, data| + if type == :stdout + network[:mac_address] = data + end + end + end + + f = Tempfile.new("vagrant-coreos-network") + { + connection: { + type: "ethernet", + id: network[:device], + "interface-name": network[:device] + }, + ethernet: { + "mac-address": network[:mac_address] + }, + ipv4: { + method: "manual", + addresses: "#{network[:ip]}/#{mask.prefix}", + gateway: network.fetch(:gateway, mask.to_range.first.succ), + }, + }.each_pair do |section, content| + f.puts "[#{section}]" + content.each_pair do |key, value| + f.puts "#{key}=#{value}" + end + end + f.close + comm.sudo("nmcli c delete '#{nm_dev[network[:device]]}'") + dst = File.join("/var/tmp", "vagrant-#{network[:device]}.conf") + final = File.join(NETWORK_MANAGER_CONN_DIR, "vagrant-#{network[:device]}.conf") + comm.upload(f.path, dst) + comm.sudo("chown root:root '#{dst}'") + comm.sudo("chmod 0600 '#{dst}'") + comm.sudo("mv '#{dst}' '#{final}'") + f.delete + end + comm.sudo("nmcli c reload") + end + + def self.configure_networks_cloud_init(machine, networks) cloud_config = {} # Locate configured IP addresses to drop in /etc/environment # for export. If no addresses found, fall back to default diff --git a/test/unit/plugins/guests/coreos/cap/configure_networks_test.rb b/test/unit/plugins/guests/coreos/cap/configure_networks_test.rb index d1efad64f..9204a97c8 100644 --- a/test/unit/plugins/guests/coreos/cap/configure_networks_test.rb +++ b/test/unit/plugins/guests/coreos/cap/configure_networks_test.rb @@ -12,12 +12,10 @@ describe "VagrantPlugins::GuestCoreOS::Cap::ConfigureNetworks" do let(:guest) { double("guest") } let(:config) { double("config", vm: vm) } let(:vm) { double("vm") } - # let(:comm) { VagrantTests::DummyCommunicator::Communicator.new(machine) } let(:comm) { double("comm") } let(:env) do double("env", machine: machine, active_machines: [machine]) end - let(:interfaces) { ["eth0", "eth1", "lo"] } before do allow(machine).to receive(:communicate).and_return(comm) @@ -25,127 +23,203 @@ describe "VagrantPlugins::GuestCoreOS::Cap::ConfigureNetworks" do end describe ".configure_networks" do - let(:network_1) do - { - interface: 0, - type: "dhcp", - } - end - let(:netconfig_1) do - [:public_interface, {}] - 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(:netconfig_2) do - [:public_network, {ip: "33.33.33.10", netmask: 16}] - end - let(:network_3) do - { - interface: 2, - type: "static", - ip: "192.168.120.22", - netmask: "255.255.255.0", - gateway: "192.168.120.1" - } - end - let(:netconfig_3) do - [:private_network, {ip: "192.168.120.22", netmask: 24}] - end - let(:networks) { [network_1, network_2, network_3] } - let(:network_configs) { [netconfig_1, netconfig_2, netconfig_3] } - let(:vm) { double("vm") } - let(:default_env_ip) { described_class.const_get(:DEFAULT_ENVIRONMENT_IP) } - - before do - allow(guest).to receive(:capability).with(:network_interfaces). - and_return(interfaces) - allow(vm).to receive(:networks).and_return(network_configs) - allow(comm).to receive(:upload) - allow(comm).to receive(:sudo) - end - - it "should upload network configuration file" do - expect(comm).to receive(:upload) - described_class.configure_networks(machine, networks) - end - - it "should configure public ipv4 address" do - expect(comm).to receive(:upload) do |src, dst| - content = File.read(src) - expect(content).to include("COREOS_PUBLIC_IPV4=#{netconfig_2.last[:ip]}") - end - described_class.configure_networks(machine, networks) - end - - it "should configure the private ipv4 address" do - expect(comm).to receive(:upload) do |src, dst| - content = File.read(src) - expect(content).to include("COREOS_PRIVATE_IPV4=#{netconfig_3.last[:ip]}") - end - described_class.configure_networks(machine, networks) - end - - it "should configure network interfaces" do - expect(comm).to receive(:upload) do |src, dst| - content = File.read(src) - interfaces.each { |i| expect(content).to include("Name=#{i}") } - end - described_class.configure_networks(machine, networks) - end - - it "should configure DHCP interface" do - expect(comm).to receive(:upload) do |src, dst| - content = File.read(src) - expect(content).to include("DHCP=yes") - end - described_class.configure_networks(machine, networks) - end - - it "should configure static IP addresses" do - expect(comm).to receive(:upload) do |src, dst| - content = File.read(src) - network_configs.map(&:last).find_all { |c| c[:ip] }.each { |c| - expect(content).to include("Address=#{c[:ip]}") + context "with network manager" do + let(:network_1) do + { + interface: 1, + type: "static", + ip: "10.0.0.3", + netmask: "255.255.255.0", + mac_address: "00:00:00:00:00:00", + gateway: "10.0.0.2", } end - described_class.configure_networks(machine, networks) - end - - context "when no public network is defined" do - let(:networks) { [network_1, network_3] } - let(:network_configs) { [netconfig_1, netconfig_3] } - - - it "should set public IP to the default environment IP" do - expect(comm).to receive(:upload) do |src, dst| - content = File.read(src) - expect(content).to include("COREOS_PUBLIC_IPV4=#{default_env_ip}") + let(:network_2) do + { + interface: 2, + type: "static", + ip: "192.168.3.3", + netmask: "255.255.0.0", + } + end + let(:nm_list) do + [ + "Wired connection 1:UUID_for_eth1:ethernet:eth1\n", + "Wired connection 2:UUID_for_eth2:ethernet:eth2\n" + ] + end + let(:interfaces) { ["eth0", "eth1", "eth2"] } + let(:networks) do + [ + network_1, + network_2, + ] + end + let(:tempfile) do + double("tempfile", + close: nil, + delete: nil, + path: temp_path, + ).tap do |f| + allow(f).to receive(:puts) end + end + let(:temp_path) { "/dev/null" } + + before do + allow(guest).to receive(:capability). + with(:network_interfaces). + and_return(interfaces) + allow(comm).to receive(:upload) + allow(comm).to receive(:sudo) + allow(comm).to receive(:execute) + allow(Tempfile).to receive(:new).and_return(tempfile) + + expect(comm).to receive(:execute). + with("nmcli -t c show") { |&block| + nm_list.each { |line| + block.call(:stdout, line) + } + } + + allow(comm).to receive(:test). + with("command -v cloud-init"). + and_return(false) + end + + it "should test for cloud-init" do + expect(comm).to receive(:test). + with("command -v cloud-init"). + and_return(false) described_class.configure_networks(machine, networks) end - it "should set the private IP to the private network" do - expect(comm).to receive(:upload) do |src, dst| - content = File.read(src) - expect(content).to include("COREOS_PRIVATE_IPV4=#{netconfig_3.last[:ip]}") - end + it "should remove any previous vagrant configuration" do + expect(comm).to receive(:sudo). + with(/rm .*vagrant-.*conf/) + described_class.configure_networks(machine, networks) + end + + it "should get MAC address from guest if not provided" do + expect(comm).to receive(:execute). + with(/cat .*eth2\/address/) + described_class.configure_networks(machine, networks) + end + + it "should not get MAC address from guest when provided" do + expect(comm).not_to receive(:execute). + with(/cat .*eth1\/address/) + described_class.configure_networks(machine, networks) + end + + it "should provide a default gateway when one is not provided" do + expect(tempfile).to receive(:puts). + with("gateway=192.168.0.1") + described_class.configure_networks(machine, networks) + end + + it "should use gateway value when provided" do + expect(tempfile).to receive(:puts). + with("gateway=10.0.0.2") + described_class.configure_networks(machine, networks) + end + + it "should delete device from network manager" do + expect(comm).to receive(:sudo). + with("nmcli c delete 'UUID_for_eth1'") + expect(comm).to receive(:sudo). + with("nmcli c delete 'UUID_for_eth2'") + described_class.configure_networks(machine, networks) + end + + it "should upload configuration files" do + expect(comm).to receive(:upload).twice + described_class.configure_networks(machine, networks) + end + + it "should change file ownership to root" do + expect(comm).to receive(:sudo). + with(/chown root:root .*/) + described_class.configure_networks(machine, networks) + end + + it "should modify file permissions to remove read access" do + expect(comm).to receive(:sudo). + with(/chmod 0600 .*/) + described_class.configure_networks(machine, networks) + end + + it "should delete local temporary files" do + expect(tempfile).to receive(:delete) + described_class.configure_networks(machine, networks) + end + + it "should reload network manager" do + expect(comm).to receive(:sudo). + with("nmcli c reload") described_class.configure_networks(machine, networks) end end - context "when no private network is defined" do - let(:networks) { [network_1, network_2] } - let(:network_configs) { [netconfig_1, netconfig_2] } + context "with cloud-init" do + let(:interfaces) { ["eth0", "eth1", "lo"] } + let(:network_1) do + { + interface: 0, + type: "dhcp", + } + end + let(:netconfig_1) do + [:public_interface, {}] + 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(:netconfig_2) do + [:public_network, {ip: "33.33.33.10", netmask: 16}] + end + let(:network_3) do + { + interface: 2, + type: "static", + ip: "192.168.120.22", + netmask: "255.255.255.0", + gateway: "192.168.120.1" + } + end + let(:netconfig_3) do + [:private_network, {ip: "192.168.120.22", netmask: 24}] + end + let(:networks) { [network_1, network_2, network_3] } + let(:network_configs) { [netconfig_1, netconfig_2, netconfig_3] } + let(:vm) { double("vm") } + let(:default_env_ip) { described_class.const_get(:DEFAULT_ENVIRONMENT_IP) } - it "should set public IP to the public network" do + before do + allow(guest).to receive(:capability).with(:network_interfaces). + and_return(interfaces) + allow(vm).to receive(:networks).and_return(network_configs) + allow(comm).to receive(:upload) + allow(comm).to receive(:sudo) + + allow(comm).to receive(:test). + with("command -v cloud-init"). + and_return(true) + end + + it "should upload network configuration file" do + expect(comm).to receive(:upload) + described_class.configure_networks(machine, networks) + end + + it "should configure public ipv4 address" do expect(comm).to receive(:upload) do |src, dst| content = File.read(src) expect(content).to include("COREOS_PUBLIC_IPV4=#{netconfig_2.last[:ip]}") @@ -153,35 +227,105 @@ describe "VagrantPlugins::GuestCoreOS::Cap::ConfigureNetworks" do described_class.configure_networks(machine, networks) end - it "should set the private IP to the public IP" do + it "should configure the private ipv4 address" do expect(comm).to receive(:upload) do |src, dst| content = File.read(src) - expect(content).to include("COREOS_PRIVATE_IPV4=#{netconfig_2.last[:ip]}") - end - described_class.configure_networks(machine, networks) - end - end - - context "when no public or private network is defined" do - let(:networks) { [network_1] } - let(:network_configs) { [netconfig_1] } - - - it "should set public IP to the default environment IP" do - expect(comm).to receive(:upload) do |src, dst| - content = File.read(src) - expect(content).to include("COREOS_PUBLIC_IPV4=#{default_env_ip}") + expect(content).to include("COREOS_PRIVATE_IPV4=#{netconfig_3.last[:ip]}") end described_class.configure_networks(machine, networks) end - it "should set the private IP to the default environment IP" do + it "should configure network interfaces" do expect(comm).to receive(:upload) do |src, dst| content = File.read(src) - expect(content).to include("COREOS_PRIVATE_IPV4=#{default_env_ip}") + interfaces.each { |i| expect(content).to include("Name=#{i}") } end described_class.configure_networks(machine, networks) end + + it "should configure DHCP interface" do + expect(comm).to receive(:upload) do |src, dst| + content = File.read(src) + expect(content).to include("DHCP=yes") + end + described_class.configure_networks(machine, networks) + end + + it "should configure static IP addresses" do + expect(comm).to receive(:upload) do |src, dst| + content = File.read(src) + network_configs.map(&:last).find_all { |c| c[:ip] }.each { |c| + expect(content).to include("Address=#{c[:ip]}") + } + end + described_class.configure_networks(machine, networks) + end + + context "when no public network is defined" do + let(:networks) { [network_1, network_3] } + let(:network_configs) { [netconfig_1, netconfig_3] } + + + it "should set public IP to the default environment IP" do + expect(comm).to receive(:upload) do |src, dst| + content = File.read(src) + expect(content).to include("COREOS_PUBLIC_IPV4=#{default_env_ip}") + end + described_class.configure_networks(machine, networks) + end + + it "should set the private IP to the private network" do + expect(comm).to receive(:upload) do |src, dst| + content = File.read(src) + expect(content).to include("COREOS_PRIVATE_IPV4=#{netconfig_3.last[:ip]}") + end + described_class.configure_networks(machine, networks) + end + end + + context "when no private network is defined" do + let(:networks) { [network_1, network_2] } + let(:network_configs) { [netconfig_1, netconfig_2] } + + + it "should set public IP to the public network" do + expect(comm).to receive(:upload) do |src, dst| + content = File.read(src) + expect(content).to include("COREOS_PUBLIC_IPV4=#{netconfig_2.last[:ip]}") + end + described_class.configure_networks(machine, networks) + end + + it "should set the private IP to the public IP" do + expect(comm).to receive(:upload) do |src, dst| + content = File.read(src) + expect(content).to include("COREOS_PRIVATE_IPV4=#{netconfig_2.last[:ip]}") + end + described_class.configure_networks(machine, networks) + end + end + + context "when no public or private network is defined" do + let(:networks) { [network_1] } + let(:network_configs) { [netconfig_1] } + + + it "should set public IP to the default environment IP" do + expect(comm).to receive(:upload) do |src, dst| + content = File.read(src) + expect(content).to include("COREOS_PUBLIC_IPV4=#{default_env_ip}") + end + described_class.configure_networks(machine, networks) + end + + it "should set the private IP to the default environment IP" do + expect(comm).to receive(:upload) do |src, dst| + content = File.read(src) + expect(content).to include("COREOS_PRIVATE_IPV4=#{default_env_ip}") + end + described_class.configure_networks(machine, networks) + end + end end end end From 8062242f7f57559651eef3a1844f8a401ce66bfa Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 4 Nov 2021 15:26:34 -0700 Subject: [PATCH 3/4] Ignore errors when removing existing configurations --- plugins/guests/coreos/cap/configure_networks.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/guests/coreos/cap/configure_networks.rb b/plugins/guests/coreos/cap/configure_networks.rb index e4f3e1d79..74993b0c9 100644 --- a/plugins/guests/coreos/cap/configure_networks.rb +++ b/plugins/guests/coreos/cap/configure_networks.rb @@ -24,7 +24,8 @@ module VagrantPlugins nm_dev[dev] = id end end - comm.sudo("rm #{File.join(NETWORK_MANAGER_CONN_DIR, 'vagrant-*.conf')}") + comm.sudo("rm #{File.join(NETWORK_MANAGER_CONN_DIR, 'vagrant-*.conf')}", + error_check: false) networks.each_with_index do |network, i| network[:device] = interfaces[network[:interface]] From cae807fcf023202e71c255d34665614c9723b4b0 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 4 Nov 2021 15:51:08 -0700 Subject: [PATCH 4/4] Configure each device individually to prevent orphan connections --- .../guests/coreos/cap/configure_networks.rb | 6 ++-- .../coreos/cap/configure_networks_test.rb | 28 +++++++++++++++---- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/plugins/guests/coreos/cap/configure_networks.rb b/plugins/guests/coreos/cap/configure_networks.rb index 74993b0c9..5ccd23e80 100644 --- a/plugins/guests/coreos/cap/configure_networks.rb +++ b/plugins/guests/coreos/cap/configure_networks.rb @@ -61,16 +61,18 @@ module VagrantPlugins end end f.close - comm.sudo("nmcli c delete '#{nm_dev[network[:device]]}'") + comm.sudo("nmcli d disconnect '#{network[:device]}'", error_check: false) + comm.sudo("nmcli c delete '#{nm_dev[network[:device]]}'", error_check: false) dst = File.join("/var/tmp", "vagrant-#{network[:device]}.conf") final = File.join(NETWORK_MANAGER_CONN_DIR, "vagrant-#{network[:device]}.conf") comm.upload(f.path, dst) comm.sudo("chown root:root '#{dst}'") comm.sudo("chmod 0600 '#{dst}'") comm.sudo("mv '#{dst}' '#{final}'") + comm.sudo("nmcli c load '#{final}'") + comm.sudo("nmcli d connect '#{network[:device]}'") f.delete end - comm.sudo("nmcli c reload") end def self.configure_networks_cloud_init(machine, networks) diff --git a/test/unit/plugins/guests/coreos/cap/configure_networks_test.rb b/test/unit/plugins/guests/coreos/cap/configure_networks_test.rb index 9204a97c8..f423bf055 100644 --- a/test/unit/plugins/guests/coreos/cap/configure_networks_test.rb +++ b/test/unit/plugins/guests/coreos/cap/configure_networks_test.rb @@ -96,7 +96,7 @@ describe "VagrantPlugins::GuestCoreOS::Cap::ConfigureNetworks" do it "should remove any previous vagrant configuration" do expect(comm).to receive(:sudo). - with(/rm .*vagrant-.*conf/) + with(/rm .*vagrant-.*conf/, error_check: false) described_class.configure_networks(machine, networks) end @@ -124,11 +124,19 @@ describe "VagrantPlugins::GuestCoreOS::Cap::ConfigureNetworks" do described_class.configure_networks(machine, networks) end - it "should delete device from network manager" do + it "should disconnect device in network manager" do expect(comm).to receive(:sudo). - with("nmcli c delete 'UUID_for_eth1'") + with("nmcli d disconnect 'eth1'", error_check: false) expect(comm).to receive(:sudo). - with("nmcli c delete 'UUID_for_eth2'") + with("nmcli d disconnect 'eth2'", error_check: false) + described_class.configure_networks(machine, networks) + end + + it "should delete connection from network manager" do + expect(comm).to receive(:sudo). + with("nmcli c delete 'UUID_for_eth1'", error_check: false) + expect(comm).to receive(:sudo). + with("nmcli c delete 'UUID_for_eth2'", error_check: false) described_class.configure_networks(machine, networks) end @@ -154,9 +162,17 @@ describe "VagrantPlugins::GuestCoreOS::Cap::ConfigureNetworks" do described_class.configure_networks(machine, networks) end - it "should reload network manager" do + it "should load the configuration files into network manager" do expect(comm).to receive(:sudo). - with("nmcli c reload") + with(/nmcli c load .*conf/).twice + described_class.configure_networks(machine, networks) + end + + it "should connect the devices in network manager" do + expect(comm).to receive(:sudo). + with("nmcli d connect 'eth1'") + expect(comm).to receive(:sudo). + with("nmcli d connect 'eth2'") described_class.configure_networks(machine, networks) end end