From 52e3c4d3b38c944ff6a860ce9e5b6ba2fe90dcb1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Sep 2010 21:12:11 -0700 Subject: [PATCH] Persisting a VM uuid is now implicitly done by Vagrant::VM --- lib/vagrant/action/builtin.rb | 1 - lib/vagrant/action/vm/destroy.rb | 1 - lib/vagrant/action/vm/persist.rb | 22 ---- lib/vagrant/data_store.rb | 3 + lib/vagrant/environment.rb | 49 +-------- lib/vagrant/vm.rb | 21 +++- test/vagrant/action/vm/destroy_test.rb | 1 - test/vagrant/action/vm/import_test.rb | 12 ++- test/vagrant/action/vm/persist_test.rb | 49 --------- test/vagrant/environment_test.rb | 142 +++---------------------- test/vagrant/vm_test.rb | 26 +++++ 11 files changed, 76 insertions(+), 251 deletions(-) delete mode 100644 lib/vagrant/action/vm/persist.rb delete mode 100644 test/vagrant/action/vm/persist_test.rb diff --git a/lib/vagrant/action/builtin.rb b/lib/vagrant/action/builtin.rb index 765ee0a40..f04c1b170 100644 --- a/lib/vagrant/action/builtin.rb +++ b/lib/vagrant/action/builtin.rb @@ -65,7 +65,6 @@ module Vagrant up = Builder.new do use VM::CheckBox use VM::Import - use VM::Persist use VM::MatchMACAddress use VM::CheckGuestAdditions use Action[:start] diff --git a/lib/vagrant/action/vm/destroy.rb b/lib/vagrant/action/vm/destroy.rb index 6d89fe268..db19f2220 100644 --- a/lib/vagrant/action/vm/destroy.rb +++ b/lib/vagrant/action/vm/destroy.rb @@ -10,7 +10,6 @@ module Vagrant env.ui.info "vagrant.actions.vm.destroy.destroying" env["vm"].vm.destroy(:destroy_medium => :delete) env["vm"].vm = nil - env.env.update_dotfile @app.call(env) end diff --git a/lib/vagrant/action/vm/persist.rb b/lib/vagrant/action/vm/persist.rb deleted file mode 100644 index 1255148b5..000000000 --- a/lib/vagrant/action/vm/persist.rb +++ /dev/null @@ -1,22 +0,0 @@ -module Vagrant - class Action - module VM - class Persist - def initialize(app, env) - @app = app - - # Error the environment if the dotfile is not valid - raise Errors::PersistDotfileExists.new(:dotfile_path => env.env.dotfile_path) if File.exist?(env.env.dotfile_path) && - !File.file?(env.env.dotfile_path) - end - - def call(env) - env.ui.info "vagrant.actions.vm.persist.persisting", :uuid => env["vm"].uuid - env.env.update_dotfile - - @app.call(env) - end - end - end - end -end diff --git a/lib/vagrant/data_store.rb b/lib/vagrant/data_store.rb index 70487678c..c21ef539d 100644 --- a/lib/vagrant/data_store.rb +++ b/lib/vagrant/data_store.rb @@ -10,6 +10,7 @@ module Vagrant def initialize(file_path) @file_path = file_path + return if !file_path File.open(file_path, "r") do |f| merge!(JSON.parse(f.read)) @@ -21,6 +22,8 @@ module Vagrant # Commits any changes to the data to disk. Even if the data # hasn't changed, it will be reserialized and written to disk. def commit + return if !file_path + File.open(file_path, "w") do |f| f.write(to_json) end diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 165172c58..7b02ab2d3 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -150,6 +150,7 @@ module Vagrant # and contains a JSON dump of a hash. See {DataStore} for more # information. def local_data + return parent.local_data if parent @local_data ||= DataStore.new(dotfile_path) end @@ -281,27 +282,10 @@ module Vagrant # First load the defaults (blank, noncreated VMs) load_blank_vms! - # If we have no dotfile, then return - return if !dotfile_path || !File.file?(dotfile_path) - - # Open and parse the dotfile - File.open(dotfile_path) do |f| - data = { DEFAULT_VM => f.read } - - begin - data = JSON.parse(data[DEFAULT_VM]) - rescue JSON::ParserError - # Most likely an older (<= 0.3.x) dotfile. Try to load it - # as the :__vagrant VM. - end - - data.each do |key, value| - key = key.to_sym - vms[key] = Vagrant::VM.find(value, self, key) - end + # Load the VM UUIDs from the local data store + (local_data[:active] || {}).each do |name, uuid| + vms[name.to_sym] = Vagrant::VM.find(uuid, self, name.to_sym) end - rescue Errno::ENOENT - # Just rescue it. end # Loads blank VMs into the `vms` attribute. @@ -324,30 +308,5 @@ module Vagrant def load_actions! @actions = Action.new(self) end - - #--------------------------------------------------------------- - # Methods to manage VM - #--------------------------------------------------------------- - - # Persists this environment's VM to the dotfile so it can be - # re-loaded at a later time. - def update_dotfile - return parent.update_dotfile if parent - - # Generate and save the persisted VM info - data = vms.inject({}) do |acc, values| - key, value = values - acc[key] = value.uuid if value.created? - acc - end - - if data.empty? - File.delete(dotfile_path) if File.exist?(dotfile_path) - else - File.open(dotfile_path, 'w+') do |f| - f.write(data.to_json) - end - end - end end end diff --git a/lib/vagrant/vm.rb b/lib/vagrant/vm.rb index 41c38a62b..f75d2d352 100644 --- a/lib/vagrant/vm.rb +++ b/lib/vagrant/vm.rb @@ -5,7 +5,7 @@ module Vagrant attr_reader :env attr_reader :system attr_reader :name - attr_accessor :vm + attr_reader :vm class << self # Finds a virtual machine by a given UUID and either returns @@ -81,6 +81,25 @@ module Vagrant !vm.nil? end + # Sets the currently active VM for this VM. If the VM is a valid, + # created virtual machine, then it will also update the local data + # to persist the VM. Otherwise, it will remove itself from the + # local data (if it exists). + def vm=(value) + @vm = value + env.local_data[:active] ||= {} + + if value && value.uuid + env.local_data[:active][name.to_sym] = value.uuid + else + env.local_data[:active].delete(name.to_sym) + end + + # Commit the local data so that the next time vagrant is initialized, + # it realizes the VM exists + env.local_data.commit + end + def uuid vm ? vm.uuid : nil end diff --git a/test/vagrant/action/vm/destroy_test.rb b/test/vagrant/action/vm/destroy_test.rb index f97fead0f..9d091e497 100644 --- a/test/vagrant/action/vm/destroy_test.rb +++ b/test/vagrant/action/vm/destroy_test.rb @@ -18,7 +18,6 @@ class DestroyVMActionTest < Test::Unit::TestCase should "destroy VM and attached images" do @internal_vm.expects(:destroy).with(:destroy_medium => :delete).once @env["vm"].expects(:vm=).with(nil).once - @env.env.expects(:update_dotfile).once @app.expects(:call).with(@env).once @instance.call(@env) end diff --git a/test/vagrant/action/vm/import_test.rb b/test/vagrant/action/vm/import_test.rb index a9bebe581..c1844ff21 100644 --- a/test/vagrant/action/vm/import_test.rb +++ b/test/vagrant/action/vm/import_test.rb @@ -12,23 +12,25 @@ class ImportVMActionTest < Test::Unit::TestCase @box.stubs(:ovf_file).returns(ovf_file) @env.env.stubs(:box).returns(@box) - @env.env.vm = Vagrant::VM.new + @env.env.vm = Vagrant::VM.new(:env => @env.env, :vm_name => "foobar") VirtualBox::VM.stubs(:import) + + @vm = mock("vm") + @vm.stubs(:uuid).returns("foobar") end should "call import on VirtualBox with proper base" do - VirtualBox::VM.expects(:import).once.with(@env.env.box.ovf_file).returns("foo") + VirtualBox::VM.expects(:import).once.with(@env.env.box.ovf_file).returns(@vm) @instance.call(@env) end should "call next in chain on success and set VM" do - vm = mock("vm") - VirtualBox::VM.stubs(:import).returns(vm) + VirtualBox::VM.stubs(:import).returns(@vm) @app.expects(:call).with(@env).once @instance.call(@env) - assert_equal vm, @env["vm"].vm + assert_equal @vm, @env["vm"].vm end should "mark environment erroneous and not continue chain on failure" do diff --git a/test/vagrant/action/vm/persist_test.rb b/test/vagrant/action/vm/persist_test.rb deleted file mode 100644 index 203154af7..000000000 --- a/test/vagrant/action/vm/persist_test.rb +++ /dev/null @@ -1,49 +0,0 @@ -require "test_helper" - -class PersistVMActionTest < Test::Unit::TestCase - setup do - @klass = Vagrant::Action::VM::Persist - @app, @env = mock_action_data - - @vm = mock("vm") - @vm.stubs(:uuid).returns("123") - @env["vm"] = @vm - end - - context "initializing" do - setup do - File.stubs(:file?).returns(true) - File.stubs(:exist?).returns(true) - @dotfile_path = "foo" - @env.env.stubs(:dotfile_path).returns(@dotfile_path) - end - - should "error environment if dotfile exists but is not a file" do - File.expects(:file?).with(@env.env.dotfile_path).returns(false) - assert_raises(Vagrant::Errors::PersistDotfileExists) { - @klass.new(@app, @env) - } - end - - should "initialize properly if dotfiles doesn't exist" do - File.expects(:exist?).with(@env.env.dotfile_path).returns(false) - assert_nothing_raised { @klass.new(@app, @env) } - end - end - - context "with an instance" do - setup do - File.stubs(:file?).returns(true) - File.stubs(:exist?).returns(true) - @instance = @klass.new(@app, @env) - end - - should "persist the dotfile then continue chain" do - update_seq = sequence("update_seq") - @env.env.expects(:update_dotfile).in_sequence(update_seq) - @app.expects(:call).with(@env).in_sequence(update_seq) - - @instance.call(@env) - end - end -end diff --git a/test/vagrant/environment_test.rb b/test/vagrant/environment_test.rb index 53c5d35e5..785ffcf53 100644 --- a/test/vagrant/environment_test.rb +++ b/test/vagrant/environment_test.rb @@ -235,6 +235,14 @@ class EnvironmentTest < Test::Unit::TestCase assert_equal result, @env.local_data assert_equal result, @env.local_data end + + should "return the parent's local data if a parent exists" do + @env.stubs(:parent).returns(mock_environment) + result = @env.parent.local_data + + Vagrant::DataStore.expects(:new).never + assert_equal result, @env.local_data + end end context "loading logger" do @@ -510,43 +518,25 @@ class EnvironmentTest < Test::Unit::TestCase context "loading the UUID out from the persisted dotfile" do setup do + @local_data = {} + @env = mock_environment @env.stubs(:root_path).returns("foo") - - File.stubs(:file?).returns(true) + @env.stubs(:local_data).returns(@local_data) end should "blank the VMs" do load_seq = sequence("load_seq") @env.stubs(:root_path).returns("foo") @env.expects(:load_blank_vms!).in_sequence(load_seq) - File.expects(:open).in_sequence(load_seq) @env.load_vm! end - should "load the UUID if the JSON parsing fails" do - vm = mock("vm") - - filemock = mock("filemock") - filemock.expects(:read).returns("foo") - Vagrant::VM.expects(:find).with("foo", @env, @klass::DEFAULT_VM).returns(vm) - File.expects(:open).with(@env.dotfile_path).once.yields(filemock) - File.expects(:file?).with(@env.dotfile_path).once.returns(true) - @env.load_vm! - - assert_equal vm, @env.vms.values.first - end - should "load all the VMs from the dotfile" do - vms = { :foo => "bar", :bar => "baz" } + @local_data[:active] = { :foo => "bar", :bar => "baz" } + results = {} - - filemock = mock("filemock") - filemock.expects(:read).returns(vms.to_json) - File.expects(:open).with(@env.dotfile_path).once.yields(filemock) - File.expects(:file?).with(@env.dotfile_path).once.returns(true) - - vms.each do |key, value| + @local_data[:active].each do |key, value| vm = mock("vm#{key}") Vagrant::VM.expects(:find).with(value, @env, key.to_sym).returns(vm) results[key] = vm @@ -565,23 +555,8 @@ class EnvironmentTest < Test::Unit::TestCase @env.load_vm! end - should "do nothing if the dotfile is nil" do - @env.stubs(:dotfile_path).returns(nil) - File.expects(:open).never - - assert_nothing_raised { - @env.load_vm! - } - end - - should "do nothing if dotfile is not a file" do - File.expects(:file?).returns(false) - File.expects(:open).never - @env.load_vm! - end - - should "uuid should be nil if dotfile didn't exist" do - File.expects(:open).raises(Errno::ENOENT) + should "uuid should be nil if local data contains nothing" do + assert @local_data.empty? # sanity @env.load_vm! assert_nil @env.vm end @@ -633,89 +608,4 @@ class EnvironmentTest < Test::Unit::TestCase end end end - - context "managing VM" do - setup do - @env = mock_environment - - @dotfile_path = "foo" - @env.stubs(:dotfile_path).returns(@dotfile_path) - end - - def mock_vm - @vm = mock("vm") - @vm.stubs(:uuid).returns("foo") - @env.stubs(:vm).returns(@vm) - end - end - - context "updating the dotfile" do - setup do - @env = mock_environment - @env.stubs(:parent).returns(nil) - @env.stubs(:dotfile_path).returns("foo") - File.stubs(:open) - File.stubs(:exist?).returns(true) - end - - def create_vm(created) - vm = mock("vm") - vm.stubs(:created?).returns(created) - vm.stubs(:uuid).returns("foo") - vm - end - - should "call parent if exists" do - parent = mock("parent") - @env.stubs(:parent).returns(parent) - parent.expects(:update_dotfile).once - - @env.update_dotfile - end - - should "remove the dotfile if the data is empty" do - vms = { - :foo => create_vm(false) - } - - @env.stubs(:vms).returns(vms) - File.expects(:delete).with(@env.dotfile_path).once - @env.update_dotfile - end - - should "not remove the dotfile if it doesn't exist" do - vms = { - :foo => create_vm(false) - } - - @env.stubs(:vms).returns(vms) - File.expects(:exist?).with(@env.dotfile_path).returns(false) - File.expects(:delete).never - assert_nothing_raised { @env.update_dotfile } - end - - should "write the proper data to dotfile" do - vms = { - :foo => create_vm(false), - :bar => create_vm(true), - :baz => create_vm(true) - } - - f = mock("f") - @env.stubs(:vms).returns(vms) - File.expects(:open).with(@env.dotfile_path, 'w+').yields(f) - f.expects(:write).with() do |json| - assert_nothing_raised { - data = JSON.parse(json) - assert_equal 2, data.length - assert_equal vms[:bar].uuid, data["bar"] - assert_equal vms[:baz].uuid, data["baz"] - } - - true - end - - @env.update_dotfile - end - end end diff --git a/test/vagrant/vm_test.rb b/test/vagrant/vm_test.rb index 288378b15..690c21809 100644 --- a/test/vagrant/vm_test.rb +++ b/test/vagrant/vm_test.rb @@ -47,6 +47,32 @@ class VMTest < Test::Unit::TestCase end end + context "setting the VM" do + setup do + @raw_vm = mock("vm") + @raw_vm.stubs(:uuid).returns("foobar") + end + + should "set the VM" do + @vm.vm = @raw_vm + assert_equal @raw_vm, @vm.vm + end + + should "add the VM to the active list" do + assert !@env.local_data[:active] + @vm.vm = @raw_vm + assert_equal @raw_vm.uuid, @env.local_data[:active][@vm.name.to_sym] + end + + should "remove the VM from the active list if nil is given" do + @env.local_data[:active] = { @vm.name.to_sym => "foo" } + + assert @env.local_data[:active].has_key?(@vm.name.to_sym) # sanity + @vm.vm = nil + assert !@env.local_data[:active].has_key?(@vm.name.to_sym) + end + end + context "accessing the SSH object" do setup do # Reset this to nil to force the reload