From aa777653f4d99d514a02389754057d182163e64e Mon Sep 17 00:00:00 2001 From: Virgil Dupras Date: Sun, 10 Dec 2017 13:11:22 -0500 Subject: [PATCH] Use lxc-info instead of lxc-attach to retrieve container IP `lxc-info -iH` to retrieve IP address was not available in early LXC development but was there at LXC 1.0. Because we've bumped our minimum LXC requirement to v1.0 recently, we can simplify the IP retrieval process and also get rid of the `dnsmasq` fallback. --- lib/vagrant-lxc/action.rb | 6 +-- .../action/fetch_ip_from_dnsmasq_leases.rb | 49 ------------------- ...xc_attach.rb => fetch_ip_with_lxc_info.rb} | 12 ++--- lib/vagrant-lxc/driver.rb | 4 ++ lib/vagrant-lxc/driver/cli.rb | 18 ++----- lib/vagrant-lxc/errors.rb | 3 -- spec/unit/driver/cli_spec.rb | 7 --- templates/sudoers.rb.erb | 1 + 8 files changed, 17 insertions(+), 83 deletions(-) delete mode 100644 lib/vagrant-lxc/action/fetch_ip_from_dnsmasq_leases.rb rename lib/vagrant-lxc/action/{fetch_ip_with_lxc_attach.rb => fetch_ip_with_lxc_info.rb} (71%) diff --git a/lib/vagrant-lxc/action.rb b/lib/vagrant-lxc/action.rb index 0b2a82a..38d08ac 100644 --- a/lib/vagrant-lxc/action.rb +++ b/lib/vagrant-lxc/action.rb @@ -4,8 +4,7 @@ require 'vagrant-lxc/action/create' require 'vagrant-lxc/action/destroy' require 'vagrant-lxc/action/destroy_confirm' require 'vagrant-lxc/action/compress_rootfs' -require 'vagrant-lxc/action/fetch_ip_with_lxc_attach' -require 'vagrant-lxc/action/fetch_ip_from_dnsmasq_leases' +require 'vagrant-lxc/action/fetch_ip_with_lxc_info' require 'vagrant-lxc/action/forced_halt' require 'vagrant-lxc/action/forward_ports' require 'vagrant-lxc/action/gc_private_network_bridges' @@ -182,8 +181,7 @@ module Vagrant def self.action_ssh_ip Builder.new.tap do |b| b.use Builtin::Call, Builtin::ConfigValidate do |env, b2| - b2.use FetchIpWithLxcAttach if env[:machine].provider.driver.supports_attach? - b2.use FetchIpFromDnsmasqLeases + b2.use FetchIpWithLxcInfo end end end diff --git a/lib/vagrant-lxc/action/fetch_ip_from_dnsmasq_leases.rb b/lib/vagrant-lxc/action/fetch_ip_from_dnsmasq_leases.rb deleted file mode 100644 index 86067b8..0000000 --- a/lib/vagrant-lxc/action/fetch_ip_from_dnsmasq_leases.rb +++ /dev/null @@ -1,49 +0,0 @@ -module Vagrant - module LXC - module Action - class FetchIpFromDnsmasqLeases - def initialize(app, env) - @app = app - @logger = Log4r::Logger.new("vagrant::lxc::action::fetch_ip_from_dnsmasq_leases") - end - - def call(env) - env[:machine_ip] ||= assigned_ip(env) - @app.call(env) - end - - def assigned_ip(env) - mac_address = env[:machine].provider.driver.mac_address - ip = nil - 10.times do - dnsmasq_leases = read_dnsmasq_leases - @logger.debug "Attempting to load ip from dnsmasq leases (mac: #{mac_address})" - @logger.debug dnsmasq_leases - if dnsmasq_leases =~ /#{Regexp.escape mac_address.to_s}\s+([0-9.]+)\s+/i - ip = $1.to_s - break - else - @logger.debug 'Ip could not be parsed from dnsmasq leases file' - sleep 2 - end - end - ip - end - - LEASES_PATHS = %w( - /var/lib/misc/dnsmasq.*.leases - /var/lib/misc/dnsmasq.leases - /var/lib/dnsmasq/dnsmasq.leases - /var/db/dnsmasq.leases - /var/lib/libvirt/dnsmasq/*.leases - ) - - def read_dnsmasq_leases - Dir["{#{LEASES_PATHS.join(',')}}"].map do |file| - File.read(file) - end.join("\n") - end - end - end - end -end diff --git a/lib/vagrant-lxc/action/fetch_ip_with_lxc_attach.rb b/lib/vagrant-lxc/action/fetch_ip_with_lxc_info.rb similarity index 71% rename from lib/vagrant-lxc/action/fetch_ip_with_lxc_attach.rb rename to lib/vagrant-lxc/action/fetch_ip_with_lxc_info.rb index 0e5d28f..38882ac 100644 --- a/lib/vagrant-lxc/action/fetch_ip_with_lxc_attach.rb +++ b/lib/vagrant-lxc/action/fetch_ip_with_lxc_info.rb @@ -1,19 +1,17 @@ module Vagrant module LXC module Action - class FetchIpWithLxcAttach + class FetchIpWithLxcInfo # Include this so we can use `Subprocess` more easily. include Vagrant::Util::Retryable def initialize(app, env) @app = app - @logger = Log4r::Logger.new("vagrant::lxc::action::fetch_ip_with_lxc_attach") + @logger = Log4r::Logger.new("vagrant::lxc::action::fetch_ip_with_lxc_info") end def call(env) env[:machine_ip] ||= assigned_ip(env) - rescue LXC::Errors::NamespacesNotSupported - @logger.info 'The `lxc-attach` command available does not support the --namespaces parameter, falling back to dnsmasq leases to fetch container ip' ensure @app.call(env) end @@ -26,7 +24,7 @@ module Vagrant retryable(:on => LXC::Errors::ExecuteError, :tries => fetch_ip_tries, :sleep => 3) do unless ip = get_container_ip_from_ip_addr(driver) # retry - raise LXC::Errors::ExecuteError, :command => "lxc-attach" + raise LXC::Errors::ExecuteError, :command => "lxc-info" end end ip @@ -34,8 +32,8 @@ module Vagrant # From: https://github.com/lxc/lxc/blob/staging/src/python-lxc/lxc/__init__.py#L371-L385 def get_container_ip_from_ip_addr(driver) - output = driver.attach '/sbin/ip', '-4', 'addr', 'show', 'scope', 'global', 'eth0', namespaces: ['network', 'mount'] - if output =~ /^\s+inet ([0-9.]+)\/[0-9]+\s+/ + output = driver.info '-iH' + if output =~ /^([0-9.]+)/ return $1.to_s end end diff --git a/lib/vagrant-lxc/driver.rb b/lib/vagrant-lxc/driver.rb index b374493..ce5191d 100644 --- a/lib/vagrant-lxc/driver.rb +++ b/lib/vagrant-lxc/driver.rb @@ -127,6 +127,10 @@ module Vagrant @cli.attach(*command) end + def info(*command) + @cli.info(*command) + end + def configure_private_network(bridge_name, bridge_ip, container_name, address_type, ip) @logger.info "Configuring network interface for #{container_name} using #{ip} and bridge #{bridge_name}" if ip diff --git a/lib/vagrant-lxc/driver/cli.rb b/lib/vagrant-lxc/driver/cli.rb index b055a09..373c458 100644 --- a/lib/vagrant-lxc/driver/cli.rb +++ b/lib/vagrant-lxc/driver/cli.rb @@ -114,17 +114,17 @@ module Vagrant end if namespaces - if supports_attach_with_namespaces? - extra = ['--namespaces', namespaces] - else - raise LXC::Errors::NamespacesNotSupported - end + extra = ['--namespaces', namespaces] end end run :attach, '--name', @name, *((extra || []) + cmd) end + def info(*cmd) + run(:info, '--name', @name, *cmd) + end + def transition_to(target_state, tries = 30, timeout = 1, &block) raise TransitionBlockNotProvided unless block_given? @@ -170,14 +170,6 @@ module Vagrant def run(command, *args) @sudo_wrapper.run("lxc-#{command}", *args) end - - def supports_attach_with_namespaces? - unless defined?(@supports_attach_with_namespaces) - @supports_attach_with_namespaces = run(:attach, '-h', :show_stderr => true).values.join.include?('--namespaces') - end - - return @supports_attach_with_namespaces - end end end end diff --git a/lib/vagrant-lxc/errors.rb b/lib/vagrant-lxc/errors.rb index 5190a20..65f05be 100644 --- a/lib/vagrant-lxc/errors.rb +++ b/lib/vagrant-lxc/errors.rb @@ -25,9 +25,6 @@ module Vagrant end - class NamespacesNotSupported < Vagrant::Errors::VagrantError - end - class LxcLinuxRequired < Vagrant::Errors::VagrantError error_key(:lxc_linux_required) end diff --git a/spec/unit/driver/cli_spec.rb b/spec/unit/driver/cli_spec.rb index e52533b..ab0c2aa 100644 --- a/spec/unit/driver/cli_spec.rb +++ b/spec/unit/driver/cli_spec.rb @@ -232,13 +232,6 @@ describe Vagrant::LXC::Driver::CLI do subject.attach *(command + [{namespaces: ['network', 'mount']}]) expect(subject).to have_received(:run).with(:attach, '--name', name, '--namespaces', 'NETWORK|MOUNT', '--', *command) end - - it 'raises a NamespacesNotSupported error if not supported' do - allow(subject).to receive(:run).with(:attach, '-h', :show_stderr => true).and_return({:stdout => '', :stderr => 'not supported'}) - expect { - subject.attach *(command + [{namespaces: ['network', 'mount']}]) - }.to raise_error(Vagrant::LXC::Errors::NamespacesNotSupported) - end end describe 'transition block' do diff --git a/templates/sudoers.rb.erb b/templates/sudoers.rb.erb index 6f91123..a38fabc 100644 --- a/templates/sudoers.rb.erb +++ b/templates/sudoers.rb.erb @@ -109,6 +109,7 @@ Whitelist.add_regex %r{<%= pipework_regex %>}, '**' Whitelist.add '<%= cmd_paths['lxc_bin'] %>/lxc-version' Whitelist.add '<%= cmd_paths['lxc_bin'] %>/lxc-ls' Whitelist.add '<%= cmd_paths['lxc_bin'] %>/lxc-info', '--name', /.*/ +Whitelist.add '<%= cmd_paths['lxc_bin'] %>/lxc-info', '--name', /.*/, '-iH' Whitelist.add '<%= cmd_paths['lxc_bin'] %>/lxc-create', '-B', /.*/, '--template', /.*/, '--name', /.*/, '**' Whitelist.add '<%= cmd_paths['lxc_bin'] %>/lxc-create', '--version' Whitelist.add '<%= cmd_paths['lxc_bin'] %>/lxc-destroy', '--name', /.*/