From 6c1a9dc58eb3b0e75e0a7e86cccd1eda1bab11a6 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 2 Aug 2018 11:01:36 -0700 Subject: [PATCH 1/4] Store box metadata of active guest When a guest is created, the box metadata information is stored in the machine data directory. This allows modifications to happen to the Vagrantfile definition of the box in use (box name change, box version change, etc) while still allowing the Machine instance of an active guest successfully load the box currently backing it. --- lib/vagrant/environment.rb | 2 +- lib/vagrant/vagrantfile.rb | 22 ++++- .../up/middleware/store_box_metadata.rb | 31 +++++++ plugins/commands/up/plugin.rb | 5 ++ .../up/middleware/store_box_metadata_test.rb | 86 +++++++++++++++++++ test/unit/vagrant/machine_test.rb | 30 +++---- 6 files changed, 158 insertions(+), 18 deletions(-) create mode 100644 plugins/commands/up/middleware/store_box_metadata.rb create mode 100644 test/unit/plugins/commands/up/middleware/store_box_metadata_test.rb diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 5fcc7654d..f1e0ff7df 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -334,7 +334,7 @@ module Vagrant # then look there. root_config = vagrantfile.config if opts[:machine] - machine_info = vagrantfile.machine_config(opts[:machine], nil, nil) + machine_info = vagrantfile.machine_config(opts[:machine], nil, nil, nil) root_config = machine_info[:config] end diff --git a/lib/vagrant/vagrantfile.rb b/lib/vagrant/vagrantfile.rb index bb7367ab7..6cba33b5d 100644 --- a/lib/vagrant/vagrantfile.rb +++ b/lib/vagrant/vagrantfile.rb @@ -42,7 +42,7 @@ module Vagrant # @return [Machine] def machine(name, provider, boxes, data_path, env) # Load the configuration for the machine - results = machine_config(name, provider, boxes) + results = machine_config(name, provider, boxes, data_path) box = results[:box] config = results[:config] config_errors = results[:config_errors] @@ -109,7 +109,7 @@ module Vagrant # box Vagrantfile. # @return [Hash] Various configuration parameters for a # machine. See the main documentation body for more info. - def machine_config(name, provider, boxes) + def machine_config(name, provider, boxes, data_path=nil) keys = @keys.dup sub_machine = @config.vm.defined_vms[name] @@ -170,6 +170,19 @@ module Vagrant original_box = config.vm.box original_version = config.vm.box_version + # Check if this machine has a local box metadata file + # describing the existing guest. If so, load it and + # set the box name and version to allow the actual + # box in use to be discovered. + if data_path + meta_file = data_path.join("box_meta") + if meta_file.file? + box_meta = JSON.parse(meta_file.read) + config.vm.box = box_meta["name"] + config.vm.box_version = box_meta["box_version"] + end + end + # The proc below loads the box and provider overrides. This is # in a proc because it may have to recurse if the provider override # changes the box. @@ -214,6 +227,11 @@ module Vagrant # Load the box and provider overrides load_box_proc.call + # Ensure box attributes are set to original values in + # case they were modified by the local box metadata + config.vm.box = original_box + config.vm.box_version = original_version + return { box: box, provider_cls: provider_cls, diff --git a/plugins/commands/up/middleware/store_box_metadata.rb b/plugins/commands/up/middleware/store_box_metadata.rb new file mode 100644 index 000000000..08e362889 --- /dev/null +++ b/plugins/commands/up/middleware/store_box_metadata.rb @@ -0,0 +1,31 @@ +require "json" + +module VagrantPlugins + module CommandUp + # Stores metadata information about the box used + # for the current guest. This allows Vagrant to + # determine the box currently in use when the + # Vagrantfile is modified with a new box name or + # version while the guest still exists. + class StoreBoxMetadata + def initialize(app, env) + @app = app + end + + def call(env) + box = env[:machine].box + box_meta = { + name: box.name, + version: box.version, + provider: box.provider, + directory: box.directory.sub(Vagrant.user_data_path.to_s + "/", "") + } + meta_file = env[:machine].data_dir.join("box_meta") + File.open(meta_file.to_s, "w+") do |file| + file.write(JSON.dump(box_meta)) + end + @app.call(env) + end + end + end +end diff --git a/plugins/commands/up/plugin.rb b/plugins/commands/up/plugin.rb index a01c054f9..34df73f92 100644 --- a/plugins/commands/up/plugin.rb +++ b/plugins/commands/up/plugin.rb @@ -12,6 +12,11 @@ module VagrantPlugins require File.expand_path("../command", __FILE__) Command end + + action_hook(:store_box_metadata, :machine_action_up) do |hook| + require_relative "middleware/store_box_metadata" + hook.append(StoreBoxMetadata) + end end end end diff --git a/test/unit/plugins/commands/up/middleware/store_box_metadata_test.rb b/test/unit/plugins/commands/up/middleware/store_box_metadata_test.rb new file mode 100644 index 000000000..438494beb --- /dev/null +++ b/test/unit/plugins/commands/up/middleware/store_box_metadata_test.rb @@ -0,0 +1,86 @@ +require File.expand_path("../../../../../base", __FILE__) +require Vagrant.source_root.join("plugins/commands/up/middleware/store_box_metadata") + +describe VagrantPlugins::CommandUp::StoreBoxMetadata do + include_context "unit" + + let(:app) { double("app") } + let(:machine) { double("machine", box: box) } + let(:box) { + double("box", + name: box_name, + version: box_version, + provider: box_provider, + directory: box_directory + ) + } + let(:box_name) { "BOX_NAME" } + let(:box_version) { "1.0.0" } + let(:box_provider) { "dummy" } + let(:box_directory) { File.join(vagrant_user_data_path, box_directory_relative) } + let(:box_directory_relative) { File.join("boxes", "BOX_NAME") } + let(:vagrant_user_data_path) { "/vagrant/user/data" } + let(:meta_path) { "META_PATH" } + let(:env) { {machine: machine} } + + let(:subject) { described_class.new(app, env) } + + describe "#call" do + + let(:meta_file) { double("meta_file") } + + before do + allow(Vagrant).to receive(:user_data_path).and_return(vagrant_user_data_path) + allow(machine).to receive(:data_dir).and_return(meta_path) + allow(meta_path).to receive(:join).with("box_meta").and_return(meta_path) + allow(File).to receive(:open) + expect(app).to receive(:call).with(env) + end + + after { subject.call(env) } + + it "should open a metadata file" do + expect(File).to receive(:open).with(meta_path, anything) + end + + context "contents of metadata file" do + + before { expect(File).to receive(:open).with(meta_path, anything).and_yield(meta_file) } + + it "should be JSON data" do + expect(meta_file).to receive(:write) do |data| + val = JSON.parse(data) + expect(val).to be_a(Hash) + end + end + + it "should include box name" do + expect(meta_file).to receive(:write) do |data| + val = JSON.parse(data) + expect(val["name"]).to eq(box_name) + end + end + + it "should include box version" do + expect(meta_file).to receive(:write) do |data| + val = JSON.parse(data) + expect(val["version"]).to eq(box_version) + end + end + + it "should include box provider" do + expect(meta_file).to receive(:write) do |data| + val = JSON.parse(data) + expect(val["provider"]).to eq(box_provider) + end + end + + it "should include relative box directory" do + expect(meta_file).to receive(:write) do |data| + val = JSON.parse(data) + expect(val["directory"]).to eq(box_directory_relative) + end + end + end + end +end diff --git a/test/unit/vagrant/machine_test.rb b/test/unit/vagrant/machine_test.rb index 9a8ee9d4f..639d14b8d 100644 --- a/test/unit/vagrant/machine_test.rb +++ b/test/unit/vagrant/machine_test.rb @@ -270,50 +270,50 @@ describe Vagrant::Machine do end it "should be able to run an action that exists" do - action_name = :up + action_name = :destroy called = false callable = lambda { |_env| called = true } expect(provider).to receive(:action).with(action_name).and_return(callable) - instance.action(:up) + instance.action(action_name) expect(called).to be end it "should provide the machine in the environment" do - action_name = :up + action_name = :destroy machine = nil callable = lambda { |env| machine = env[:machine] } allow(provider).to receive(:action).with(action_name).and_return(callable) - instance.action(:up) + instance.action(action_name) expect(machine).to eql(instance) end it "should pass any extra options to the environment" do - action_name = :up + action_name = :destroy foo = nil callable = lambda { |env| foo = env[:foo] } allow(provider).to receive(:action).with(action_name).and_return(callable) - instance.action(:up, foo: :bar) + instance.action(action_name, foo: :bar) expect(foo).to eq(:bar) end it "should pass any extra options to the environment as strings" do - action_name = :up + action_name = :destroy foo = nil callable = lambda { |env| foo = env["foo"] } allow(provider).to receive(:action).with(action_name).and_return(callable) - instance.action(:up, "foo" => :bar) + instance.action(action_name, "foo" => :bar) expect(foo).to eq(:bar) end it "should return the environment as a result" do - action_name = :up + action_name = :destroy callable = lambda { |env| env[:result] = "FOO" } allow(provider).to receive(:action).with(action_name).and_return(callable) @@ -323,7 +323,7 @@ describe Vagrant::Machine do end it "should raise an exception if the action is not implemented" do - action_name = :up + action_name = :destroy allow(provider).to receive(:action).with(action_name).and_return(nil) @@ -332,7 +332,7 @@ describe Vagrant::Machine do end it 'should not warn if the machines cwd has not changed' do - initial_action_name = :up + initial_action_name = :destroy second_action_name = :reload callable = lambda { |_env| } original_cwd = env.cwd.to_s @@ -349,7 +349,7 @@ describe Vagrant::Machine do end it 'should warn if the machine was last run under a different directory' do - action_name = :up + action_name = :destroy callable = lambda { |_env| } original_cwd = env.cwd.to_s @@ -374,7 +374,7 @@ describe Vagrant::Machine do let (:data_dir) { env.cwd } it 'should not warn if vagrant is run in subdirectory' do - action_name = :up + action_name = :destroy callable = lambda { |_env| } original_cwd = env.cwd.to_s @@ -394,7 +394,7 @@ describe Vagrant::Machine do context "with the vagrant-triggers community plugin" do it "should not call the internal trigger functions if installed" do - action_name = :up + action_name = :destroy callable = lambda { |_env| } allow(provider).to receive(:action).with(action_name).and_return(callable) @@ -412,7 +412,7 @@ describe Vagrant::Machine do end it "should call the internal trigger functions if not installed" do - action_name = :up + action_name = :destroy callable = lambda { |_env| } allow(provider).to receive(:action).with(action_name).and_return(callable) From 03d8965ef763ff13c9dcd75b4a034efb22dc8376 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 2 Aug 2018 15:19:39 -0700 Subject: [PATCH 2/4] Add param documentation and fix key used for box version --- lib/vagrant/vagrantfile.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/vagrant/vagrantfile.rb b/lib/vagrant/vagrantfile.rb index 6cba33b5d..3f3c8c826 100644 --- a/lib/vagrant/vagrantfile.rb +++ b/lib/vagrant/vagrantfile.rb @@ -107,6 +107,7 @@ module Vagrant # be backed by (required for provider overrides). # @param [BoxCollection] boxes BoxCollection to look up the # box Vagrantfile. + # @param [Pathname] data_path Machine data path # @return [Hash] Various configuration parameters for a # machine. See the main documentation body for more info. def machine_config(name, provider, boxes, data_path=nil) @@ -179,7 +180,7 @@ module Vagrant if meta_file.file? box_meta = JSON.parse(meta_file.read) config.vm.box = box_meta["name"] - config.vm.box_version = box_meta["box_version"] + config.vm.box_version = box_meta["version"] end end From 00b783a6a525bfcd35abe786085fa9d7a67d18b3 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 2 Aug 2018 16:12:10 -0700 Subject: [PATCH 3/4] Lookup latest available installed box if required on update When performing a box update and the box version has been updated to be different than the installed version, perform a lookup for the latest available installed box to allow the update command to continue successfully --- plugins/commands/box/command/update.rb | 12 +++++--- .../commands/box/command/update_test.rb | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/plugins/commands/box/command/update.rb b/plugins/commands/box/command/update.rb index 30da1fcb6..fa89b6e77 100644 --- a/plugins/commands/box/command/update.rb +++ b/plugins/commands/box/command/update.rb @@ -100,10 +100,14 @@ module VagrantPlugins end if !machine.box - machine.ui.output(I18n.t( - "vagrant.errors.box_update_no_box", - name: machine.config.vm.box)) - next + collection = Vagrant::BoxCollection.new(@env.boxes_path) + machine.box = collection.find(machine.config.vm.box, provider || @env.default_provider, "> 0") + if !machine.box + machine.ui.output(I18n.t( + "vagrant.errors.box_update_no_box", + name: machine.config.vm.box)) + next + end end name = machine.box.name diff --git a/test/unit/plugins/commands/box/command/update_test.rb b/test/unit/plugins/commands/box/command/update_test.rb index 5a2f9153c..95be1c1a4 100644 --- a/test/unit/plugins/commands/box/command/update_test.rb +++ b/test/unit/plugins/commands/box/command/update_test.rb @@ -307,6 +307,35 @@ describe VagrantPlugins::CommandBox::Command::Update do subject.execute end + context "when box version is updated but previous box exists" do + + let(:collection) { double("collection") } + + it "updates the box" do + # First call gets nil result to for lookup + expect(machine).to receive(:box).and_return(nil) + expect(Vagrant::BoxCollection).to receive(:new).and_return(collection) + expect(collection).to receive(:find).and_return(box) + + expect(box).to receive(:has_update?). + with(machine.config.vm.box_version, + {download_options: + {ca_cert: nil, ca_path: nil, client_cert: nil, + insecure: false}}). + and_return([md, md.version("1.1"), md.version("1.1").provider("virtualbox")]) + + expect(action_runner).to receive(:run).with(any_args) { |action, opts| + expect(opts[:box_url]).to eq(box.metadata_url) + expect(opts[:box_provider]).to eq("virtualbox") + expect(opts[:box_version]).to eq("1.1") + expect(opts[:ui]).to equal(machine.ui) + true + } + + subject.execute + end + end + context "machine has download options" do before do machine.config.vm.box_download_ca_cert = "oof" From ceb7a0b5ac07874dd00e3e684ac225c044407566 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 2 Aug 2018 16:41:28 -0700 Subject: [PATCH 4/4] When doing box lookup, use explicit provider, machine provider, then default --- plugins/commands/box/command/update.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/commands/box/command/update.rb b/plugins/commands/box/command/update.rb index fa89b6e77..7e01768c1 100644 --- a/plugins/commands/box/command/update.rb +++ b/plugins/commands/box/command/update.rb @@ -101,7 +101,7 @@ module VagrantPlugins if !machine.box collection = Vagrant::BoxCollection.new(@env.boxes_path) - machine.box = collection.find(machine.config.vm.box, provider || @env.default_provider, "> 0") + machine.box = collection.find(machine.config.vm.box, provider || machine.provider_name || @env.default_provider, "> 0") if !machine.box machine.ui.output(I18n.t( "vagrant.errors.box_update_no_box",