Replace ugly driver builder with a more intelligent approach

References #118

lib/vagrant-lxc/action.rb

@@ -8,6 +8,8 @@ require 'vagrant-lxc/action/destroy'
 require 'vagrant-lxc/action/destroy_confirm'
 require 'vagrant-lxc/action/disconnect'
 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/forced_halt'
 require 'vagrant-lxc/action/forward_ports'
 require 'vagrant-lxc/action/handle_box_metadata'
@@ -169,6 +171,16 @@ module Vagrant
         end
       end
 
+      # This action is called to read the IP of the container. The IP found
+      # is expected to be put into the `:machine_ip` key.
+      def self.action_fetch_ip
+        Vagrant::Action::Builder.new.tap do |b|
+          b.use Vagrant::Action::Builtin::ConfigValidate
+          b.use FetchIpWithLxcAttach
+          b.use FetchIpFromDnsmasqLeases
+        end
+      end
+
       # This is the action that will exec into an SSH shell.
       def self.action_ssh
         Vagrant::Action::Builder.new.tap do |b|

lib/vagrant-lxc/action/fetch_ip_from_dnsmasq_leases.rb

@@ -1,11 +1,21 @@
 module Vagrant
   module LXC
-    class Driver
+    module Action
-      module FetchIpFromDsnmasq
+      class FetchIpFromDnsmasqLeases
-        def assigned_ip
+        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)
           @logger.debug 'Loading ip from dnsmasq leases'
+          mac_address = env[:machine].provider.driver.mac_address
           ip = nil
-          # TODO: Use Vagrant::Util::Retryable
           10.times do
             if dnsmasq_leases =~ /#{Regexp.escape mac_address}\s+([0-9.]+)\s+/
               ip = $1.to_s
@@ -15,15 +25,9 @@ module Vagrant
               sleep 2
             end
           end
-          # TODO: Raise an user friendly error
-          raise 'Unable to identify container IP!' unless ip
           ip
         end
 
-        def mac_address
-          @mac_address ||= base_path.join('config').read.match(/^lxc\.network\.hwaddr\s+=\s+(.+)$/)[1]
-        end
-
         LEASES_PATHS = %w(
           /var/lib/misc/dnsmasq.leases
           /var/lib/dnsmasq/dnsmasq.leases

lib/vagrant-lxc/action/fetch_ip_with_lxc_attach.rb

@@ -0,0 +1,46 @@
+module Vagrant
+  module LXC
+    module Action
+        class FetchIpWithLxcAttach
+          # 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")
+          end
+
+          def call(env)
+            env[:machine_ip] ||= assigned_ip(env)
+            @app.call(env)
+          end
+
+          def assigned_ip(env)
+            driver  = env[:machine].provider.driver
+            version = driver.version.match(/^(\d+\.\d+)\./)[1].to_f
+            unless version >= 0.8
+              @logger.debug "lxc version does not support the --namespaces argument to lxc-attach"
+              return nil
+            end
+
+            ip = ''
+            retryable(:on => LXC::Errors::ExecuteError, :tries => 10, :sleep => 3) do
+              unless ip = get_container_ip_from_ip_addr(driver)
+                # retry
+                raise LXC::Errors::ExecuteError, :command => "lxc-attach"
+              end
+            end
+            ip
+          end
+
+          # 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'
+            if output =~ /^\s+inet ([0-9.]+)\/[0-9]+\s+/
+              return $1.to_s
+            end
+          end
+        end
+      end
+    end
+  end

lib/vagrant-lxc/action/forward_ports.rb

@@ -50,7 +50,7 @@ module Vagrant
             redir_pid = redirect_port(
               fp[:host_ip],
               fp[:host],
-              fp[:guest_ip] || @env[:machine].provider.driver.assigned_ip,
+              fp[:guest_ip] || @env[:machine].provider.ssh_info[:host],
               fp[:guest]
             )
             store_redir_pid(fp[:host], redir_pid)

lib/vagrant-lxc/driver.rb

@@ -34,6 +34,10 @@ module Vagrant
         Pathname.new(base_path.join('config').read.match(/^lxc\.rootfs\s+=\s+(.+)$/)[1])
       end
 
+      def mac_address
+        @mac_address ||= base_path.join('config').read.match(/^lxc\.network\.hwaddr\s+=\s+(.+)$/)[1]
+      end
+
       def create(name, template_path, config_file, template_options = {})
         @cli.name = @container_name = name
 
@@ -81,6 +85,14 @@ module Vagrant
         @cli.destroy
       end
 
+      def attach(*command)
+        @cli.attach(*command)
+      end
+
+      def version
+        @cli.version
+      end
+
       # TODO: This needs to be reviewed and specs needs to be written
       def compress_rootfs
         rootfs_dirname = File.dirname rootfs_path
@@ -106,9 +118,6 @@ module Vagrant
         end
       end
 
-      def assigned_ip
-      end
-
       protected
 
       # Root folder where container configs are stored

lib/vagrant-lxc/driver/builder.rb

@@ -1,21 +0,0 @@
-require_relative 'fetch_ip_with_attach'
-require_relative 'fetch_ip_from_dnsmasq'
-
-module Vagrant
-  module LXC
-    class Driver
-      class Builder
-        def self.build(id, shell)
-          version = CLI.new(shell).version.match(/^(\d+\.\d+)\./)[1].to_f
-          Driver.new(id, shell).tap do |driver|
-            mod = version >= 0.8 ?
-              Driver::FetchIpWithAttach :
-              Driver::FetchIpFromDsnmasq
-
-            driver.extend(mod)
-          end
-        end
-      end
-    end
-  end
-end

lib/vagrant-lxc/driver/fetch_ip_with_attach.rb

@@ -1,29 +0,0 @@
-module Vagrant
-  module LXC
-    class Driver
-      module FetchIpWithAttach
-        # Include this so we can use `Subprocess` more easily.
-        include Vagrant::Util::Retryable
-
-        def assigned_ip
-          ip = ''
-          retryable(:on => LXC::Errors::ExecuteError, :tries => 10, :sleep => 3) do
-            unless ip = get_container_ip_from_ip_addr
-              # retry
-              raise LXC::Errors::ExecuteError, :command => "lxc-attach"
-            end
-          end
-          ip
-        end
-
-        # From: https://github.com/lxc/lxc/blob/staging/src/python-lxc/lxc/__init__.py#L371-L385
-        def get_container_ip_from_ip_addr
-          output = @cli.attach '/sbin/ip', '-4', 'addr', 'show', 'scope', 'global', 'eth0', namespaces: 'network'
-          if output =~ /^\s+inet ([0-9.]+)\/[0-9]+\s+/
-            return $1.to_s
-          end
-        end
-      end
-    end
-  end
-end

lib/vagrant-lxc/provider.rb

@@ -2,7 +2,6 @@ require "log4r"
 
 require "vagrant-lxc/action"
 require "vagrant-lxc/driver"
-require "vagrant-lxc/driver/builder"
 require "vagrant-lxc/sudo_wrapper"
 
 module Vagrant
@@ -39,7 +38,7 @@ module Vagrant
 
         begin
           @logger.debug("Instantiating the container for: #{id.inspect}")
-          @driver = Driver::Builder.build(id, self.sudo_wrapper)
+          @driver = Driver.new(id, self.sudo_wrapper)
           @driver.validate!
         rescue Driver::ContainerNotFound
           # The container doesn't exist, so we probably have a stale
@@ -66,8 +65,16 @@ module Vagrant
         # we return nil.
         return nil if state == :not_created
 
+        # Run a custom action called "fetch_ip" which does what it says and puts
+        # the IP found into the `:machine_ip` key in the environment.
+        env = @machine.action("fetch_ip")
+
+        # If we were not able to identify the container's IP, we return nil
+        # here and we let Vagrant core deal with it ;)
+        return nil unless env[:machine_ip]
+
         {
-          :host => @driver.assigned_ip,
+          :host => env[:machine_ip],
           :port => @machine.config.ssh.guest_port
         }
       end

spec/unit/action/forward_ports_spec.rb

@@ -9,8 +9,7 @@ describe Vagrant::LXC::Action::ForwardPorts do
   let(:env)          { {machine: machine, ui: double(info: true)} }
   let(:machine)      { double(:machine) }
   let!(:data_dir)    { Pathname.new(Dir.mktmpdir) }
-  let(:provider)     { instance_double('Vagrant::LXC::Provider', driver: driver) }
+  let(:provider)     { instance_double('Vagrant::LXC::Provider', ssh_info: {host: container_ip}) }
-  let(:driver)       { instance_double('Vagrant::LXC::Driver', assigned_ip: container_ip) }
   let(:host_ip)      { '127.0.0.1' }
   let(:host_port)    { 8080 }
   let(:guest_port)   { 80 }