From 85f4a4d5ee84a99067fbee97c57de2161b49eef6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 23 Apr 2014 06:13:16 -0700 Subject: [PATCH] commands/box/remove: if box is in use, warn user, ask for confirmation --- lib/vagrant/action/builtin/box_remove.rb | 43 ++++++++++++ templates/locales/en.yml | 9 +++ .../vagrant/action/builtin/box_remove_test.rb | 69 +++++++++++++++++++ 3 files changed, 121 insertions(+) diff --git a/lib/vagrant/action/builtin/box_remove.rb b/lib/vagrant/action/builtin/box_remove.rb index 51c600297..8ee9cd7f6 100644 --- a/lib/vagrant/action/builtin/box_remove.rb +++ b/lib/vagrant/action/builtin/box_remove.rb @@ -71,6 +71,49 @@ module Vagrant box = env[:box_collection].find( box_name, box_provider, box_version) + # Verify that this box is not in use by an active machine, + # otherwise warn the user. + users = [] + env[:machine_index].each do |entry| + box_data = entry.extra_data["box"] + next if !box_data + + # If all the data matches AND the entry is a seemingly + # valid entry, then track it. + if box_data["name"] == box.name && + box_data["provider"] == box.provider.to_s && + box_data["version"] == box.version.to_s && + entry.valid?(env[:home_path]) + users << entry + end + end + + if !users.empty? + # Build up the output to show the user. + users = users.map do |entry| + "#{entry.name} (ID: #{entry.id})" + end.join("\n") + + force_key = :force_confirm_box_remove + message = I18n.t( + "vagrant.commands.box.remove_in_use_query", + name: box.name, + provider: box.provider, + version: box.version, + users: users) + " " + + # Ask the user if we should do this + stack = Builder.new.tap do |b| + b.use Confirm, message, force_key + end + + result = env[:action_runner].run(stack, env) + if !result[:result] + # They said "no", so just return + return @app.call(env) + end + end + env[:ui].info(I18n.t("vagrant.commands.box.removing", :name => box.name, :provider => box.provider, diff --git a/templates/locales/en.yml b/templates/locales/en.yml index a1a521794..f10e84390 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1251,6 +1251,15 @@ en: vm_not_running: "VM is not currently running. Please, first bring it up with `vagrant up` then run this command." box: no_installed_boxes: "There are no installed boxes! Use `vagrant box add` to add some." + remove_in_use_query: |- + Box '%{name}' (v%{version}) with provider '%{provider}' appears + to still be in use by at least one Vagrant environment. Removing + the box could corrupt the environment. We recommend destroying + these environments first: + + %{users} + + Are you sure you want to remove this box? [y/N] removing: |- Removing box '%{name}' (v%{version}) with provider '%{provider}'... destroy: diff --git a/test/unit/vagrant/action/builtin/box_remove_test.rb b/test/unit/vagrant/action/builtin/box_remove_test.rb index 3a21cb820..9de3e467b 100644 --- a/test/unit/vagrant/action/builtin/box_remove_test.rb +++ b/test/unit/vagrant/action/builtin/box_remove_test.rb @@ -6,12 +6,16 @@ describe Vagrant::Action::Builtin::BoxRemove do let(:app) { lambda { |env| } } let(:env) { { box_collection: box_collection, + home_path: home_path, + machine_index: machine_index, ui: Vagrant::UI::Silent.new, } } subject { described_class.new(app, env) } let(:box_collection) { double("box_collection") } + let(:home_path) { "foo" } + let(:machine_index) { [] } let(:iso_env) { isolated_environment } let(:box) do @@ -74,6 +78,71 @@ describe Vagrant::Action::Builtin::BoxRemove do expect(env[:box_removed]).to equal(box) end + context "checking if a box is in use" do + def new_entry(name, provider, version, valid=true) + Vagrant::MachineIndex::Entry.new.tap do |entry| + entry.extra_data["box"] = { + "name" => "foo", + "provider" => "virtualbox", + "version" => "1.0", + } + + entry.stub(valid?: valid) + end + end + + let(:action_runner) { double("action_runner") } + + before do + env[:action_runner] = action_runner + + box_collection.stub( + all: [ + ["foo", "1.0", :virtualbox], + ["foo", "1.1", :virtualbox], + ]) + + env[:box_name] = "foo" + env[:box_version] = "1.0" + end + + it "does delete if the box is not in use" do + expect(box_collection).to receive(:find).with( + "foo", :virtualbox, "1.0").and_return(box) + expect(box).to receive(:destroy!).once + + subject.call(env) + end + + it "does delete if the box is in use and user confirms" do + machine_index << new_entry("foo", "virtualbox", "1.0") + + result = { result: true } + expect(action_runner).to receive(:run). + with(anything, env).and_return(result) + + expect(box_collection).to receive(:find).with( + "foo", :virtualbox, "1.0").and_return(box) + expect(box).to receive(:destroy!).once + + subject.call(env) + end + + it "doesn't delete if the box is in use" do + machine_index << new_entry("foo", "virtualbox", "1.0") + + result = { result: false } + expect(action_runner).to receive(:run). + with(anything, env).and_return(result) + + expect(box_collection).to receive(:find).with( + "foo", :virtualbox, "1.0").and_return(box) + expect(box).to receive(:destroy!).never + + subject.call(env) + end + end + it "errors if the box doesn't exist" do box_collection.stub(all: [])