From 9885e58c213565234f3e9d2150fb81019660a087 Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Sun, 5 Jun 2022 17:44:10 +0100 Subject: [PATCH] Update after hook definition for recent vagrant (#1506) Vagrant newer than 2.2.11 reworked how the after hook definition functions requiring it to be called in a different way to ensure it is possible to hook the box remove action to perform the expected local action. Add a compatibility function to ensure that the plugin works with both mechanisms and some simple tests to ensure that with the unit tests validated against the older vagrant versions that it confirms the action will be called as expected. As part of fixing the call of the remove_libvirt_image action, ensure it only executes when the box removed is a libvirt one and ignores all others. Fixes: #1196 --- .../action/remove_libvirt_image.rb | 4 +- lib/vagrant-libvirt/plugin.rb | 5 +-- lib/vagrant-libvirt/util/compat.rb | 23 ++++++++++ spec/unit/action/remove_libvirt_image_spec.rb | 43 +++++++++++++++++++ spec/unit/plugin_spec.rb | 42 ++++++++++++++++++ 5 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 lib/vagrant-libvirt/util/compat.rb create mode 100644 spec/unit/action/remove_libvirt_image_spec.rb create mode 100644 spec/unit/plugin_spec.rb diff --git a/lib/vagrant-libvirt/action/remove_libvirt_image.rb b/lib/vagrant-libvirt/action/remove_libvirt_image.rb index d581841..dd81c97 100644 --- a/lib/vagrant-libvirt/action/remove_libvirt_image.rb +++ b/lib/vagrant-libvirt/action/remove_libvirt_image.rb @@ -12,7 +12,9 @@ module VagrantPlugins end def call(env) - env[:ui].info('Vagrant-libvirt plugin removed box only from your LOCAL ~/.vagrant/boxes directory') + return @app.call(env) unless env[:box_removed].provider == :libvirt + + env[:ui].info("Vagrant-libvirt plugin removed box only from #{env[:env].boxes.directory} directory") env[:ui].info('From Libvirt storage pool you have to delete image manually(virsh, virt-manager or by any other tool)') @app.call(env) end diff --git a/lib/vagrant-libvirt/plugin.rb b/lib/vagrant-libvirt/plugin.rb index 63deb5e..5785053 100644 --- a/lib/vagrant-libvirt/plugin.rb +++ b/lib/vagrant-libvirt/plugin.rb @@ -6,8 +6,7 @@ rescue LoadError raise 'The Vagrant Libvirt plugin must be run within Vagrant.' end -# compatibility fix to define constant not available Vagrant <1.6 -::Vagrant::MachineState::NOT_CREATED_ID ||= :not_created +require 'vagrant-libvirt/util/compat' module VagrantPlugins module ProviderLibvirt @@ -27,7 +26,7 @@ module VagrantPlugins Provider end - action_hook(:remove_libvirt_image) do |hook| + action_hook(*(Util::Compat.action_hook_args(:remove_libvirt_image, :box_remove))) do |hook| require_relative 'action' hook.after Vagrant::Action::Builtin::BoxRemove, Action.remove_libvirt_image end diff --git a/lib/vagrant-libvirt/util/compat.rb b/lib/vagrant-libvirt/util/compat.rb new file mode 100644 index 0000000..3c15d55 --- /dev/null +++ b/lib/vagrant-libvirt/util/compat.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'vagrant' + +# compatibility fix to define constant not available Vagrant <1.6 +::Vagrant::MachineState::NOT_CREATED_ID ||= :not_created + +module VagrantPlugins + module ProviderLibvirt + module Util + module Compat + def self.action_hook_args(name, action) + # handle different number of arguments for action_hook depending on vagrant version + if Gem::Version.new(Vagrant::VERSION) >= Gem::Version.new('2.2.11') + return name, action + end + + return name + end + end + end + end +end diff --git a/spec/unit/action/remove_libvirt_image_spec.rb b/spec/unit/action/remove_libvirt_image_spec.rb new file mode 100644 index 0000000..f28807c --- /dev/null +++ b/spec/unit/action/remove_libvirt_image_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'support/sharedcontext' + +require 'vagrant-libvirt/action/remove_libvirt_image' + +describe VagrantPlugins::ProviderLibvirt::Action::RemoveLibvirtImage do + subject { described_class.new(app, env) } + + include_context 'unit' + + let(:box) { instance_double(::Vagrant::Box) } + + describe '#call' do + before do + env[:box_removed] = box + allow(ui).to receive(:info) + end + + context 'when called with libvirt box removed' do + before do + expect(box).to receive(:provider).and_return(:libvirt) + end + + it 'should notify the user about limited removal' do + expect(ui).to receive(:info).with(/Vagrant-libvirt plugin removed box/) + expect(subject.call(env)).to be_nil + end + end + + context 'when called with any other provider box' do + before do + expect(box).to receive(:provider).and_return(:virtualbox) + end + + it 'call the next middle ware immediately' do + expect(ui).to_not receive(:info).with(/Vagrant-libvirt plugin removed box/) + expect(subject.call(env)).to be_nil + end + end + end +end diff --git a/spec/unit/plugin_spec.rb b/spec/unit/plugin_spec.rb new file mode 100644 index 0000000..12a5395 --- /dev/null +++ b/spec/unit/plugin_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'support/sharedcontext' + +require 'vagrant-libvirt/plugin' + + +describe VagrantPlugins::ProviderLibvirt::Plugin do + subject { described_class.new } + + include_context 'unit' + + describe '#action_hook remove_libvirt_image' do + before do + # set up some dummy boxes + box_path = File.join(env[:env].boxes.directory, 'vagrant-libvirt-VAGRANTSLASH-test', '0.0.1') + ['libvirt', 'virtualbox'].each do |provider| + provider_path = File.join(box_path, provider) + FileUtils.mkdir_p(provider_path) + metadata = {'provider': provider} + File.open(File.join(provider_path, 'metadata.json'), "w") { |f| f.write metadata.to_json } + end + end + + it 'should call the action hook after box remove' do + expect(VagrantPlugins::ProviderLibvirt::Action).to receive(:remove_libvirt_image).and_return(Vagrant::Action::Builder.new) + expect { + env[:env].action_runner.run( + Vagrant::Action.action_box_remove, { + box_name: 'vagrant-libvirt/test', + box_provider: 'libvirt', + box_version: '0.0.1', + force_confirm_box_remove: true, + box_remove_all_versions: false, + ui: ui, + } + ) + }.to_not raise_error + end + end +end