From bdadfe7339fb9e6987c26b0996c77d9f7f4d3db1 Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Wed, 27 Apr 2016 16:27:39 +0100 Subject: [PATCH 1/2] Rudimentary tests for destroy domain Some simple tests for destroy domain Change-Id: I94c85b362f20c69c4ba75d879d20eedb4a934bcf --- spec/support/sharedcontext.rb | 5 +- spec/unit/action/destroy_domain_spec.rb | 75 +++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 spec/unit/action/destroy_domain_spec.rb diff --git a/spec/support/sharedcontext.rb b/spec/support/sharedcontext.rb index 474bdb0..adfa173 100644 --- a/spec/support/sharedcontext.rb +++ b/spec/support/sharedcontext.rb @@ -3,12 +3,13 @@ require 'spec_helper' shared_context "unit" do include_context 'vagrant-unit' - let(:test_env) do - vagrantfile ||= <<-EOF + let(:vagrantfile) do <<-EOF Vagrant.configure('2') do |config| config.vm.define :test end EOF + end + let(:test_env) do test_env = isolated_environment test_env.vagrantfile vagrantfile test_env diff --git a/spec/unit/action/destroy_domain_spec.rb b/spec/unit/action/destroy_domain_spec.rb new file mode 100644 index 0000000..b3b40ee --- /dev/null +++ b/spec/unit/action/destroy_domain_spec.rb @@ -0,0 +1,75 @@ +require "spec_helper" +require "support/sharedcontext" +require "support/libvirt_context" + +require "vagrant-libvirt/action/destroy_domain" + +describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do + + subject { described_class.new(app, env) } + + include_context "unit" + include_context "libvirt" + + let(:libvirt_domain) { double("libvirt_domain") } + let(:libvirt_client) { double("libvirt_client") } + let(:servers) { double("servers") } + + describe "#call" do + before do + allow_any_instance_of(VagrantPlugins::ProviderLibvirt::Driver). + to receive(:connection).and_return(connection) + allow(connection).to receive(:client).and_return(libvirt_client) + allow(libvirt_client).to receive(:lookup_domain_by_uuid). + and_return(libvirt_domain) + allow(connection).to receive(:servers).and_return(servers) + allow(servers).to receive(:get).and_return(domain) + # always see this at the start of #call + expect(ui).to receive(:info).with("Removing domain...") + end + + context "when no snapshots" do + let(:root_disk) { double("libvirt_root_disk") } + + before do + allow(libvirt_domain).to receive(:list_snapshots).and_return([]) + allow(libvirt_domain).to receive(:has_managed_save?).and_return(nil) + root_disk.stub(:name => "test.img") + end + + context "when only has root disk" do + it "calls fog to destroy volumes" do + expect(domain).to receive(:destroy).with(:destroy_volumes => true) + expect(subject.call(env)).to be_nil + end + end + + context "when has additional disks" do + let(:vagrantfile) { <<-EOF + Vagrant.configure('2') do |config| + config.vm.define :test + config.vm.provider :libvirt do |libvirt| + libvirt.storage :file + end + end + EOF + } + + let(:extra_disk) { double("libvirt_extra_disk") } + before do + extra_disk.stub(:name => "test-vdb.qcow2") + end + + it "destroys disks individually" do + allow(libvirt_domain).to receive(:name).and_return("test") + allow(domain).to receive(:volumes).and_return([extra_disk], [root_disk]) + + expect(domain).to receive(:destroy).with(:destroy_volumes => false) + expect(extra_disk).to receive(:destroy) # extra disk remove + expect(root_disk).to receive(:destroy) # root disk remove + expect(subject.call(env)).to be_nil + end + end + end + end +end From 22acaebf610bdfa5fdf5689c68948605b669eeef Mon Sep 17 00:00:00 2001 From: Michael Kerrin Date: Thu, 13 Aug 2015 16:03:59 +0100 Subject: [PATCH 2/2] Use explicit removal of disk volumes if CDROM attached Use the more conservative path if either disks or cdroms present for the domain configuration. Domain destroy including volumes will attempt to delete any attached CDROM images as they are registered as volumes. Resulting in the following error message: fog-libvirt-0.0.3/lib/fog/libvirt/requests/compute/volume_action.rb:6:in `delete': Call to virStorageVol Delete failed: cannot unlink file '': Success (Libvirt::Error) Co-Authored-By: Darragh Bailey Change-Id: Ia497aef0e871de88e65c46afe071b2618fda5588 --- lib/vagrant-libvirt/action/destroy_domain.rb | 6 ++++-- spec/unit/action/destroy_domain_spec.rb | 21 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/lib/vagrant-libvirt/action/destroy_domain.rb b/lib/vagrant-libvirt/action/destroy_domain.rb index 50e7f71..8242b2b 100644 --- a/lib/vagrant-libvirt/action/destroy_domain.rb +++ b/lib/vagrant-libvirt/action/destroy_domain.rb @@ -35,8 +35,10 @@ module VagrantPlugins domain = env[:machine].provider.driver.connection.servers.get(env[:machine].id.to_s) - if env[:machine].provider_config.disks.empty? - # if using default configuration of disks + if env[:machine].provider_config.disks.empty? and + env[:machine].provider_config.cdroms.empty? + # if using default configuration of disks and cdroms + # cdroms are consider volumes, but cannot be destroyed domain.destroy(destroy_volumes: true) else domain.destroy(destroy_volumes: false) diff --git a/spec/unit/action/destroy_domain_spec.rb b/spec/unit/action/destroy_domain_spec.rb index b3b40ee..741d033 100644 --- a/spec/unit/action/destroy_domain_spec.rb +++ b/spec/unit/action/destroy_domain_spec.rb @@ -70,6 +70,27 @@ describe VagrantPlugins::ProviderLibvirt::Action::DestroyDomain do expect(subject.call(env)).to be_nil end end + + context "when has CDROMs attached" do + let(:vagrantfile) { <<-EOF + Vagrant.configure('2') do |config| + config.vm.define :test + config.vm.provider :libvirt do |libvirt| + libvirt.storage :file, :device => :cdrom + end + end + EOF + } + + it "uses explicit removal of disks" do + allow(libvirt_domain).to receive(:name).and_return("test") + allow(domain).to receive(:volumes).and_return([root_disk]) + + expect(domain).to_not receive(:destroy).with(:destroy_volumes => true) + expect(root_disk).to receive(:destroy) # root disk remove + expect(subject.call(env)).to be_nil + end + end end end end