Merge pull request #551 from electrofelix/respect-destroy-on-error

Respect destroy on error

Gemfile

@@ -13,6 +13,10 @@ group :development do
   else
     gem 'vagrant', :git => 'https://github.com/mitchellh/vagrant.git'
   end
+
+  gem 'vagrant-spec', :github => 'mitchellh/vagrant-spec',
+    tag: ENV['VAGRANT_SPEC_VERSION'] || "9bba7e1228379c0a249a06ce76ba8ea7d276afbe"
+
   gem 'pry'
 end
 

lib/vagrant-libvirt/action/wait_till_up.rb

@@ -1,4 +1,5 @@
 require 'log4r'
+require 'vagrant-libvirt/errors'
 require 'vagrant-libvirt/util/timer'
 require 'vagrant/util/retryable'
 
@@ -21,8 +22,11 @@ module VagrantPlugins
           env[:metrics] ||= {}
 
           # Get domain object
-          domain = env[:machine].provider.driver.connection.servers.get(env[:machine].id.to_s)
-          raise NoDomainError if domain == nil
+          domain = env[:machine].provider.driver.get_domain(env[:machine].id.to_s)
+          if domain == nil
+            raise Errors::NoDomainError,
+              :error_message => "Domain #{env[:machine].id} not found"
+          end
 
           # Wait for domain to obtain an ip address. Ip address is searched
           # from arp table, either localy or remotely via ssh, if libvirt
@@ -32,7 +36,7 @@ module VagrantPlugins
             env[:ui].info(I18n.t("vagrant_libvirt.waiting_for_ip"))
             retryable(:on => Fog::Errors::TimeoutError, :tries => 300) do
               # If we're interrupted don't worry about waiting
-              next if env[:interrupted]
+              return terminate(env) if env[:interrupted]
 
               # Wait for domain to obtain an ip address
               domain.wait_for(2) {
@@ -43,7 +47,6 @@ module VagrantPlugins
               }
             end
           end
-          terminate(env) if env[:interrupted]
           @logger.info("Got IP address #{env[:ip_address]}")
           @logger.info("Time for getting IP: #{env[:metrics]["instance_ip_time"]}")
           
@@ -64,7 +67,8 @@ module VagrantPlugins
               end            
             end
           end
-          terminate(env) if env[:interrupted]
+          # if interrupted above, just terminate immediately
+          return terminate(env) if env[:interrupted]
           @logger.info("Time for SSH ready: #{env[:metrics]["instance_ssh_time"]}")
 
           # Booted and ready for use.
@@ -76,18 +80,21 @@ module VagrantPlugins
         def recover(env)
           return if env["vagrant.error"].is_a?(Vagrant::Errors::VagrantError)
 
-          if env[:machine].provider.state.id != :not_created
-            # Undo the import
-            terminate(env)
-          end
+          # Undo the import
+          terminate(env)
         end
 
         def terminate(env)
-          destroy_env = env.dup
-          destroy_env.delete(:interrupted)
-          destroy_env[:config_validate] = false
-          destroy_env[:force_confirm_destroy] = true
-          env[:action_runner].run(Action.action_destroy, destroy_env)        
+          if env[:machine].provider.state.id != :not_created
+            # If we're not supposed to destroy on error then just return
+            return if !env[:destroy_on_error]
+
+            destroy_env = env.dup
+            destroy_env.delete(:interrupted)
+            destroy_env[:config_validate] = false
+            destroy_env[:force_confirm_destroy] = true
+            env[:action_runner].run(Action.action_destroy, destroy_env)
+          end
         end
       end
     end

lib/vagrant-libvirt/plugin.rb

@@ -10,6 +10,9 @@ if Vagrant::VERSION < '1.5.0'
   raise 'The Vagrant Libvirt plugin is only compatible with Vagrant 1.5+'
 end
 
+# compatibility fix to define constant not available vagrant <1.6
+::Vagrant::MachineState::NOT_CREATED_ID ||= :not_created
+
 module VagrantPlugins
   module ProviderLibvirt
     class Plugin < Vagrant.plugin('2')

spec/spec_helper.rb

@@ -1,5 +1,6 @@
 require 'vagrant-libvirt'
 require 'support/environment_helper'
+require 'vagrant-spec/unit'
 
 RSpec.configure do |spec|
 end

spec/support/libvirt_context.rb

@@ -0,0 +1,24 @@
+require 'fog/libvirt'
+
+shared_context "libvirt" do
+  include_context "unit"
+
+  let(:libvirt_context) { true                      }
+  let(:id)              { "dummy-vagrant_dummy"     }
+  let(:connection)      { double("::Fog::Compute")  }
+
+  def connection_result(options={})
+    result = options.fetch(:result, nil)
+    double("connection_result" => result)
+  end
+
+  before do
+    # we don't want unit tests to ever run commands on the system; so we wire
+    # in a double to ensure any unexpected messages raise exceptions
+    stub_const("::Fog::Compute", connection)
+
+    # drivers also call vm_exists? during init;
+    allow(connection).to receive(:servers).with(kind_of(String)).
+      and_return(connection_result(result: nil))
+  end
+end

spec/support/sharedcontext.rb

@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+shared_context "unit" do
+  include_context 'vagrant-unit'
+
+  let(:test_env) do
+    vagrantfile ||= <<-EOF
+    Vagrant.configure('2') do |config|
+      config.vm.define :test
+    end
+    EOF
+    test_env = isolated_environment
+    test_env.vagrantfile vagrantfile
+    test_env
+  end
+  let(:env)              { { env: iso_env, machine: machine, ui: ui, root_path: '/rootpath' } }
+  let(:conf)             { Vagrant::Config::V2::DummyConfig.new() }
+  let(:ui)               { Vagrant::UI::Basic.new() }
+  let(:iso_env)          { test_env.create_vagrant_env ui_class: Vagrant::UI::Basic }
+  let(:machine)          { iso_env.machine(:test, :libvirt) }
+  # Mock the communicator to prevent SSH commands for being executed.
+  let(:communicator)     { double('communicator') }
+  # Mock the guest operating system.
+  let(:guest)            { double('guest') }
+  let(:app)              { lambda { |env| } }
+  let(:plugin)           { register_plugin() }
+
+  before (:each) do
+    machine.stub(:guest => guest)
+    machine.stub(:communicator => communicator)
+    machine.stub(:id => id)
+  end
+
+end

spec/vagrant-libvirt/action/wait_till_up_spec.rb

@@ -0,0 +1,111 @@
+require "vagrant-libvirt/action/wait_till_up"
+require "vagrant-libvirt/errors"
+
+require "spec_helper"
+require "support/sharedcontext"
+require "support/libvirt_context"
+
+describe VagrantPlugins::ProviderLibvirt::Action::WaitTillUp do
+
+  subject { described_class.new(app, env) }
+
+  include_context "vagrant-unit"
+  include_context "libvirt"
+  include_context "unit"
+
+  describe "#call" do
+    context "when machine does not exist" do
+      before do
+        allow_any_instance_of(VagrantPlugins::ProviderLibvirt::Driver).to receive(:get_domain).and_return(nil)
+        allow_any_instance_of(VagrantPlugins::ProviderLibvirt::Driver).to receive(:state).
+          and_return(:not_created)
+      end
+
+      it "raises exception" do
+        expect(app).to_not receive(:call)
+        expect{subject.call(env)}.to raise_error(::VagrantPlugins::ProviderLibvirt::Errors::NoDomainError,
+                                                 /No domain found. Domain dummy-vagrant_dummy not found/)
+      end
+    end
+
+    context "when machine is booting" do
+      before do
+        allow_any_instance_of(VagrantPlugins::ProviderLibvirt::Driver).to receive(:get_domain).and_return(machine)
+        allow_any_instance_of(VagrantPlugins::ProviderLibvirt::Driver).to receive(:state).
+          and_return(:running)
+      end
+
+      context "if interrupted looking for IP" do
+        before do
+          env[:interrupted] = true
+        end
+        it "should exit" do
+          expect(app).to_not receive(:call)
+          expect(ui).to receive(:info).with("Waiting for domain to get an IP address...")
+          expect(subject.call(env)).to be_nil
+        end
+      end
+
+      context "if interrupted waiting for SSH" do
+        before do
+          allow(machine).to receive(:wait_for).and_return(true)
+          allow(env).to receive(:[]).and_call_original
+          allow(env).to receive(:[]).with(:interrupted).and_return(false, true, true)
+          allow(env).to receive(:[]).with(:ip_address).and_return("192.168.121.2")
+        end
+        it "should exit after getting IP" do
+          expect(app).to_not receive(:call)
+          expect(ui).to receive(:info).with("Waiting for domain to get an IP address...")
+          expect(ui).to receive(:info).with("Waiting for SSH to become available...")
+          logger = subject.instance_variable_get(:@logger)
+          expect(logger).to receive(:info).with("Got IP address 192.168.121.2")
+          expect(logger).to receive(:info).with(/Time for getting IP: .*/)
+          expect(env[:machine].communicate).to_not receive(:ready?)
+          expect(subject.call(env)).to be_nil
+        end
+      end
+    end
+  end
+
+  describe "#recover" do
+    before do
+      allow_any_instance_of(VagrantPlugins::ProviderLibvirt::Driver).to receive(:get_domain).and_return(machine)
+      allow_any_instance_of(VagrantPlugins::ProviderLibvirt::Driver).to receive(:state).
+        and_return(:not_created)
+      allow(env).to receive(:[]).and_call_original
+    end
+
+    it "should do nothing by default" do
+      expect(env).to_not receive(:[]).with(:action_runner) # cleanup
+      expect(subject.recover(env)).to be_nil
+    end
+
+    context "with machine coming up" do
+      before do
+        allow_any_instance_of(VagrantPlugins::ProviderLibvirt::Driver).to receive(:state).
+          and_return(:running)
+        env[:destroy_on_error] = true
+      end
+
+      context "and user has disabled destroy on failure" do
+        before do
+          env[:destroy_on_error] = false
+        end
+
+        it "skips terminate on failure" do
+          expect(env).to_not receive(:[]).with(:action_runner) # cleanup
+          expect(subject.recover(env)).to be_nil
+        end
+      end
+
+      context "and using default settings" do
+        let(:runner) { double('runner') }
+        it "deletes VM on failure" do
+          expect(env).to receive(:[]).with(:action_runner).and_return(runner) # cleanup
+          expect(runner).to receive(:run)
+          expect(subject.recover(env)).to be_nil
+        end
+      end
+    end
+  end
+end

vagrant-libvirt.gemspec

@@ -16,9 +16,9 @@ Gem::Specification.new do |gem|
   gem.require_paths = ['lib']
   gem.version       = VagrantPlugins::ProviderLibvirt::VERSION
 
-  gem.add_development_dependency "rspec-core", "~> 2.12.2"
-  gem.add_development_dependency "rspec-expectations", "~> 2.12.1"
-  gem.add_development_dependency "rspec-mocks", "~> 2.12.1"
+  gem.add_development_dependency "rspec-core", "~> 2.14.0"
+  gem.add_development_dependency "rspec-expectations", "~> 2.14.0"
+  gem.add_development_dependency "rspec-mocks", "~> 2.14.0"
 
   gem.add_runtime_dependency 'fog-libvirt', '~> 0.0.1'
   gem.add_runtime_dependency 'nokogiri', '~> 1.6.0'