Refactoring: make SudoWrapper a bit more self-contained

By looking at the code, it seems that it was a goal to make the sudo
wrapper path configurable through the Vagrantfile, but it wasn't
effective and didn't make much sense (that kind of config is a per-host
config, not a per-guest one).

This caused the cause to be needlessly complex by giving the Provider
the responsibility of instanciating the wrapper. This commit gets rid of
that.

I didn't get rid of `sudo_wrapper` injection in `Driver` and
`Driver::CLI` constructors because they're needed for tests. I'm not
ready to tackle this yet.
This commit is contained in:
Virgil Dupras 2017-12-13 16:35:16 -05:00
parent 6eb7ec1a2e
commit 97b5882262
7 changed files with 16 additions and 31 deletions

View file

@ -1,22 +1,10 @@
require "vagrant-lxc/version" require "vagrant-lxc/version"
require "vagrant-lxc/plugin" require "vagrant-lxc/plugin"
require "vagrant-lxc/sudo_wrapper"
module Vagrant module Vagrant
module LXC module LXC
def self.source_root def self.source_root
@source_root ||= Pathname.new(File.dirname(__FILE__)).join('..').expand_path @source_root ||= Pathname.new(File.dirname(__FILE__)).join('..').expand_path
end end
def self.sudo_wrapper_path
"/usr/local/bin/vagrant-lxc-wrapper"
end
def self.sudo_wrapper
wrapper = Pathname.new(sudo_wrapper_path).exist? &&
sudo_wrapper_path || nil
SudoWrapper.new(wrapper)
end
end end
end end

View file

@ -1,6 +1,7 @@
require 'tempfile' require 'tempfile'
require "vagrant-lxc/driver" require "vagrant-lxc/driver"
require "vagrant-lxc/sudo_wrapper"
module Vagrant module Vagrant
module LXC module LXC
@ -27,7 +28,7 @@ module Vagrant
argv = parse_options(opts) argv = parse_options(opts)
return unless argv return unless argv
wrapper_path = Vagrant::LXC.sudo_wrapper_path wrapper_path = SudoWrapper.bin_path
wrapper = create_wrapper! wrapper = create_wrapper!
sudoers = create_sudoers!(options[:user], wrapper_path) sudoers = create_sudoers!(options[:user], wrapper_path)
@ -45,7 +46,7 @@ module Vagrant
# This requires vagrant 1.5.2+ https://github.com/mitchellh/vagrant/commit/3371c3716278071680af9b526ba19235c79c64cb # This requires vagrant 1.5.2+ https://github.com/mitchellh/vagrant/commit/3371c3716278071680af9b526ba19235c79c64cb
def create_wrapper! def create_wrapper!
lxc_base_path = Driver.new(0, LXC.sudo_wrapper).containers_path lxc_base_path = Driver.new("").containers_path
wrapper = Tempfile.new('lxc-wrapper').tap do |file| wrapper = Tempfile.new('lxc-wrapper').tap do |file|
template = Vagrant::Util::TemplateRenderer.new( template = Vagrant::Util::TemplateRenderer.new(
'sudoers.rb', 'sudoers.rb',

View file

@ -24,7 +24,6 @@ module Vagrant
@customizations = [] @customizations = []
@backingstore = UNSET_VALUE @backingstore = UNSET_VALUE
@backingstore_options = [] @backingstore_options = []
@sudo_wrapper = UNSET_VALUE
@container_name = UNSET_VALUE @container_name = UNSET_VALUE
@fetch_ip_tries = UNSET_VALUE @fetch_ip_tries = UNSET_VALUE
end end
@ -50,7 +49,6 @@ module Vagrant
end end
def finalize! def finalize!
@sudo_wrapper = nil if @sudo_wrapper == UNSET_VALUE
@container_name = nil if @container_name == UNSET_VALUE @container_name = nil if @container_name == UNSET_VALUE
@backingstore = "best" if @backingstore == UNSET_VALUE @backingstore = "best" if @backingstore == UNSET_VALUE
@existing_container_name = nil if @existing_container_name == UNSET_VALUE @existing_container_name = nil if @existing_container_name == UNSET_VALUE

View file

@ -3,6 +3,7 @@ require "vagrant/util/subprocess"
require "vagrant-lxc/errors" require "vagrant-lxc/errors"
require "vagrant-lxc/driver/cli" require "vagrant-lxc/driver/cli"
require "vagrant-lxc/sudo_wrapper"
require "etc" require "etc"
@ -21,10 +22,10 @@ module Vagrant
attr_reader :container_name, attr_reader :container_name,
:customizations :customizations
def initialize(container_name, sudo_wrapper, cli = nil) def initialize(container_name, sudo_wrapper = nil, cli = nil)
@container_name = container_name @container_name = container_name
@sudo_wrapper = sudo_wrapper @sudo_wrapper = sudo_wrapper || SudoWrapper.new()
@cli = cli || CLI.new(sudo_wrapper, container_name) @cli = cli || CLI.new(@sudo_wrapper, container_name)
@logger = Log4r::Logger.new("vagrant::provider::lxc::driver") @logger = Log4r::Logger.new("vagrant::provider::lxc::driver")
@customizations = [] @customizations = []
end end

View file

@ -2,6 +2,7 @@ require "log4r"
require "vagrant-lxc/action" require "vagrant-lxc/action"
require "vagrant-lxc/driver" require "vagrant-lxc/driver"
require "vagrant-lxc/sudo_wrapper"
module Vagrant module Vagrant
module LXC module LXC
@ -24,13 +25,9 @@ module Vagrant
machine_id_changed machine_id_changed
end end
def sudo_wrapper
@shell ||= LXC.sudo_wrapper
end
def ensure_lxc_installed! def ensure_lxc_installed!
begin begin
sudo_wrapper.run("which", "lxc-create") SudoWrapper.new().run("which", "lxc-create")
rescue Vagrant::LXC::Errors::ExecuteError rescue Vagrant::LXC::Errors::ExecuteError
raise Errors::LxcNotInstalled raise Errors::LxcNotInstalled
end end
@ -43,7 +40,7 @@ module Vagrant
begin begin
@logger.debug("Instantiating the container for: #{id.inspect}") @logger.debug("Instantiating the container for: #{id.inspect}")
@driver = Driver.new(id, self.sudo_wrapper) @driver = Driver.new(id)
@driver.validate! @driver.validate!
rescue Driver::ContainerNotFound rescue Driver::ContainerNotFound
# The container doesn't exist, so we probably have a stale # The container doesn't exist, so we probably have a stale

View file

@ -6,8 +6,12 @@ module Vagrant
attr_reader :wrapper_path attr_reader :wrapper_path
def initialize(wrapper_path = nil) def self.dest_path
@wrapper_path = wrapper_path "/usr/local/bin/vagrant-lxc-wrapper"
end
def initialize()
@wrapper_path = Pathname.new(SudoWrapper.dest_path).exist? && SudoWrapper.dest_path || nil
@logger = Log4r::Logger.new("vagrant::lxc::sudo_wrapper") @logger = Log4r::Logger.new("vagrant::lxc::sudo_wrapper")
end end

4
spec/Vagrantfile vendored
View file

@ -15,10 +15,6 @@ Vagrant.configure("2") do |config|
config.cache.auto_detect = true config.cache.auto_detect = true
config.vm.provider :lxc do |lxc|
# lxc.sudo_wrapper = '/usr/bin/lxc-vagrant-wrapper'
end
config.vm.provision :shell, config.vm.provision :shell,
inline: 'mkdir -p /vagrant/tmp && echo -n "Provisioned" > /vagrant/tmp/provisioning' inline: 'mkdir -p /vagrant/tmp && echo -n "Provisioned" > /vagrant/tmp/provisioning'