Merge branch 'new-state-api'

This adds a new interface for returning machine state from a provider which
allows commands like `vagrant status` to continue to properly function
in the face of differing providers.

lib/vagrant.rb

@@ -75,6 +75,7 @@ module Vagrant
   autoload :Guest,         'vagrant/guest'
   autoload :Hosts,         'vagrant/hosts'
   autoload :Machine,       'vagrant/machine'
+  autoload :MachineState,  'vagrant/machine_state'
   autoload :Plugin,        'vagrant/plugin'
   autoload :TestHelpers,   'vagrant/test_helpers'
   autoload :UI,            'vagrant/ui'

lib/vagrant/errors.rb

@@ -248,6 +248,10 @@ module Vagrant
       error_key(:machine_not_found)
     end
 
+    class MachineStateInvalid < VagrantError
+      error_key(:machine_state_invalid)
+    end
+
     class MultiVMEnvironmentRequired < VagrantError
       status_code(5)
       error_key(:multi_vm_required)

lib/vagrant/machine.rb

@@ -277,7 +277,9 @@ module Vagrant
     #
     # @return [Symbol]
     def state
-      @provider.state
+      result = @provider.state
+      raise Errors::MachineStateInvalid if !result.is_a?(MachineState)
+      result
     end
 
     protected

lib/vagrant/machine_state.rb

@@ -0,0 +1,45 @@
+module Vagrant
+  # This represents the state of a given machine. This is a very basic
+  # class that simply stores a short and long description of the state
+  # of a machine.
+  #
+  # The state also stores a state "id" which ca be used as a unique
+  # identifier for a state. This should be a symbol. This allows internal
+  # code to compare state such as ":not_created" instead of using
+  # string comparison.
+  #
+  # The short description should be a single word description of the
+  # state of the machine such as "running" or "not created".
+  #
+  # The long description can span multiple lines describing what the
+  # state actually means.
+  class MachineState
+    # Unique ID for this state.
+    #
+    # @return [Symbol]
+    attr_reader :id
+
+    # Short description for this state.
+    #
+    # @return [String]
+    attr_reader :short_description
+
+    # Long description for this state.
+    #
+    # @return [String]
+    attr_reader :long_description
+
+    # Creates a new instance to represent the state of a machine.
+    #
+    # @param [Symbol] id Unique identifier for this state.
+    # @param [String] short Short (preferably one-word) description of
+    #   the state.
+    # @param [String] long Long description (can span multiple lines)
+    #   of the state.
+    def initialize(id, short, long)
+      @id                = id
+      @short_description = short
+      @long_description  = long
+    end
+  end
+end

plugins/commands/status/command.rb

@@ -15,15 +15,15 @@ module VagrantPlugins
         state = nil
         results = []
         with_target_vms(argv) do |machine|
-          state = machine.state.to_s if !state
-          results << "#{machine.name.to_s.ljust(25)}#{machine.state.to_s.gsub("_", " ")}"
+          state = machine.state if !state
+          results << "#{machine.name.to_s.ljust(25)}#{machine.state.short_description}"
         end
 
         state = results.length == 1 ? state : "listing"
 
         @env.ui.info(I18n.t("vagrant.commands.status.output",
                             :states => results.join("\n"),
-                            :message => I18n.t("vagrant.commands.status.#{state}")),
+                            :message => state.long_description),
                      :prefix => false)
 
         # Success, exit status 0

plugins/providers/virtualbox/action/boot.rb

@@ -34,7 +34,7 @@ module VagrantPlugins
 
             # If the VM is not starting or running, something went wrong
             # and we need to show a useful error.
-            state = @env[:machine].provider.state
+            state = @env[:machine].provider.state.id
             raise Errors::VMFailedToRun if state != :starting && state != :running
 
             sleep 2 if !@env["vagrant.test"]

plugins/providers/virtualbox/action/check_accessible.rb

@@ -7,7 +7,7 @@ module VagrantPlugins
         end
 
         def call(env)
-          if env[:machine].state == :inaccessible
+          if env[:machine].state.id == :inaccessible
             # The VM we are attempting to manipulate is inaccessible. This
             # is a very bad situation and can only be fixed by the user. It
             # also prohibits us from actually doing anything with the virtual

plugins/providers/virtualbox/action/check_created.rb

@@ -9,7 +9,7 @@ module VagrantPlugins
         end
 
         def call(env)
-          if env[:machine].state == :not_created
+          if env[:machine].state.id == :not_created
             raise Vagrant::Errors::VMNotCreatedError
           end
 

plugins/providers/virtualbox/action/check_running.rb

@@ -9,7 +9,7 @@ module VagrantPlugins
         end
 
         def call(env)
-          if env[:machine].state != :running
+          if env[:machine].state.id != :running
             raise Vagrant::Errors::VMNotRunningError
           end
 

plugins/providers/virtualbox/action/created.rb

@@ -8,7 +8,7 @@ module VagrantPlugins
 
         def call(env)
           # Set the result to be true if the machine is created.
-          env[:result] = env[:machine].state != :not_created
+          env[:result] = env[:machine].state.id != :not_created
 
           # Call the next if we have one (but we shouldn't, since this
           # middleware is built to run with the Call-type middlewares)

plugins/providers/virtualbox/action/discard_state.rb

@@ -7,7 +7,7 @@ module VagrantPlugins
         end
 
         def call(env)
-          if env[:machine].provider.state == :saved
+          if env[:machine].provider.state.id == :saved
             env[:ui].info I18n.t("vagrant.actions.vm.discard_state.discarding")
             env[:machine].provider.driver.discard_saved_state
           end

plugins/providers/virtualbox/action/export.rb

@@ -13,7 +13,8 @@ module VagrantPlugins
         def call(env)
           @env = env
 
-          raise Vagrant::Errors::VMPowerOffToPackage if @env[:machine].provider.state != :poweroff
+          raise Vagrant::Errors::VMPowerOffToPackage if \
+            @env[:machine].provider.state.id != :poweroff
 
           setup_temp_dir
           export

plugins/providers/virtualbox/action/halt.rb

@@ -7,7 +7,7 @@ module VagrantPlugins
         end
 
         def call(env)
-          current_state = env[:machine].provider.state
+          current_state = env[:machine].provider.state.id
           if current_state == :running || current_state == :gurumeditation
             # If the VM is running and we're not forcing, we can
             # attempt a graceful shutdown
@@ -17,7 +17,7 @@ module VagrantPlugins
             end
 
             # If we're not powered off now, then force it
-            if env[:machine].provider.state != :poweroff
+            if env[:machine].provider.state.id != :poweroff
               env[:ui].info I18n.t("vagrant.actions.vm.halt.force")
               env[:machine].provider.driver.halt
             end

plugins/providers/virtualbox/action/import.rb

@@ -33,7 +33,7 @@ module VagrantPlugins
         end
 
         def recover(env)
-          if env[:machine].provider.state != :not_created
+          if env[:machine].provider.state.id != :not_created
             return if env["vagrant.error"].is_a?(Vagrant::Errors::VagrantError)
 
             # Interrupted, destroy the VM. We note that we don't want to

plugins/providers/virtualbox/action/is_running.rb

@@ -8,7 +8,7 @@ module VagrantPlugins
 
         def call(env)
           # Set the result to be true if the machine is running.
-          env[:result] = env[:machine].state == :running
+          env[:result] = env[:machine].state.id == :running
 
           # Call the next if we have one (but we shouldn't, since this
           # middleware is built to run with the Call-type middlewares)

plugins/providers/virtualbox/action/is_saved.rb

@@ -8,7 +8,7 @@ module VagrantPlugins
 
         def call(env)
           # Set the result to be true if the machine is saved.
-          env[:result] = env[:machine].state == :saved
+          env[:result] = env[:machine].state.id == :saved
 
           # Call the next if we have one (but we shouldn't, since this
           # middleware is built to run with the Call-type middlewares)

plugins/providers/virtualbox/action/resume.rb

@@ -7,7 +7,7 @@ module VagrantPlugins
         end
 
         def call(env)
-          if env[:machine].provider.state == :saved
+          if env[:machine].provider.state.id == :saved
             env[:ui].info I18n.t("vagrant.actions.vm.resume.resuming")
             env[:action_runner].run(Boot, env)
           end

plugins/providers/virtualbox/action/suspend.rb

@@ -7,7 +7,7 @@ module VagrantPlugins
         end
 
         def call(env)
-          if env[:machine].provider.state == :running
+          if env[:machine].provider.state.id == :running
             env[:ui].info I18n.t("vagrant.actions.vm.suspend.suspending")
             env[:machine].provider.driver.suspend
           end

plugins/providers/virtualbox/provider.rb

@@ -63,10 +63,19 @@ module VagrantPlugins
       def state
         # XXX: What happens if we destroy the VM but the UUID is still
         # set here?
-        return :not_created if !@driver.uuid
-        state = @driver.read_state
-        return :unknown if !state
-        state
+
+        # Determine the ID of the state here.
+        state_id = nil
+        state_id = :not_created if !@driver.uuid
+        state_id = @driver.read_state if !state_id
+        state_id = :unknown if !state_id
+
+        # Translate into short/long descriptions
+        short = state_id.to_s.gsub("_", " ")
+        long  = I18n.t("vagrant.commands.status.#{state_id}")
+
+        # Return the state
+        Vagrant::MachineState.new(state_id, short, long)
       end
 
       # Returns a human-friendly string version of this provider which

templates/locales/en.yml

@@ -145,6 +145,12 @@ en:
       machine_not_found: |-
         The machine with the name '%{name}' was not found configured for
         this Vagrant environment.
+      machine_state_invalid: |-
+        An internal error has occurred! The provider of the machine you're
+        trying to work with reported an invalid state. This is a bug with
+        the provider you're using, and not with Vagrant itself or with
+        any configuration you may have done. Please report this bug to
+        the proper location.
       multi_vm_required: |-
         A multi-vm environment is required for name specification to this command.
       multi_vm_target_required: |-

test/unit/vagrant/machine_state_test.rb

@@ -0,0 +1,26 @@
+require "pathname"
+
+require File.expand_path("../../base", __FILE__)
+
+describe Vagrant::MachineState do
+  include_context "unit"
+
+  let(:id) { :some_state }
+  let(:short) { "foo" }
+  let(:long) { "I am a longer foo" }
+
+  it "should give access to the id" do
+    instance = described_class.new(id, short, long)
+    instance.id.should == id
+  end
+
+  it "should give access to the short description" do
+    instance = described_class.new(id, short, long)
+    instance.short_description.should == short
+  end
+
+  it "should give access to the long description" do
+    instance = described_class.new(id, short, long)
+    instance.long_description.should == long
+  end
+end

test/unit/vagrant/machine_test.rb

@@ -385,10 +385,16 @@ describe Vagrant::Machine do
 
   describe "state" do
     it "should query state from the provider" do
-      state = :running
+      state = Vagrant::MachineState.new(:id, "short", "long")
 
       provider.should_receive(:state).and_return(state)
-      instance.state.should == state
+      instance.state.id.should == :id
+    end
+
+    it "should raise an exception if a MachineState is not returned" do
+      provider.should_receive(:state).and_return(:old_school)
+      expect { instance.state }.
+        to raise_error(Vagrant::Errors::MachineStateInvalid)
     end
   end
 end