From e0ec679838264996ce46afcb314673969eb14f42 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 5 Aug 2012 13:45:24 -0700 Subject: [PATCH] `vagrant ssh` with full console works with new provider. This works by now calling the `:ssh` action on the provider. This action is allowed to do whatever it pleases, but should at some point probably call the `SSHExec` built-in middleware. The `SSHExec` built-in middleware was added. This uses the information returned by `Machine#ssh_info` and uses the `Vagrant::Util::SSH` helper to exec into the remote machine. The provider should do any work upfront in verifying that the machine is ready to be SSHed into. --- lib/vagrant/action.rb | 1 + lib/vagrant/action/builtin/ssh_exec.rb | 37 +++++++++++++++++++ lib/vagrant/machine.rb | 3 ++ lib/vagrant/util/safe_exec.rb | 4 +- lib/vagrant/util/ssh.rb | 10 ++--- plugins/commands/ssh/command.rb | 8 ++-- plugins/providers/virtualbox/action.rb | 13 +++++++ .../virtualbox/action/check_created.rb | 21 +++++++++++ .../virtualbox/action/check_running.rb | 21 +++++++++++ plugins/providers/virtualbox/provider.rb | 31 ++++++++++++---- .../vagrant/action/builtin/ssh_exec_test.rb | 22 +++++++++++ 11 files changed, 151 insertions(+), 20 deletions(-) create mode 100644 lib/vagrant/action/builtin/ssh_exec.rb create mode 100644 plugins/providers/virtualbox/action/check_created.rb create mode 100644 plugins/providers/virtualbox/action/check_running.rb create mode 100644 test/unit/vagrant/action/builtin/ssh_exec_test.rb diff --git a/lib/vagrant/action.rb b/lib/vagrant/action.rb index 9cc2641c4..91a2091cf 100644 --- a/lib/vagrant/action.rb +++ b/lib/vagrant/action.rb @@ -17,6 +17,7 @@ module Vagrant module Builtin autoload :Call, "vagrant/action/builtin/call" autoload :Confirm, "vagrant/action/builtin/confirm" + autoload :SSHExec, "vagrant/action/builtin/ssh_exec" end module Env diff --git a/lib/vagrant/action/builtin/ssh_exec.rb b/lib/vagrant/action/builtin/ssh_exec.rb new file mode 100644 index 000000000..3c54a6614 --- /dev/null +++ b/lib/vagrant/action/builtin/ssh_exec.rb @@ -0,0 +1,37 @@ +require "vagrant/util/ssh" + +module Vagrant + module Action + module Builtin + # This class will exec into a full fledged SSH console into the + # remote machine. This middleware assumes that the VM is running and + # ready for SSH, and uses the {Machine#ssh_info} method to retrieve + # SSH information necessary to connect. + # + # Note: If there are any middleware after `SSHExec`, they will **not** + # run, since exec replaces the currently running process. + class SSHExec + # For quick access to the `SSH` class. + include Vagrant::Util + + def initialize(app, env) + @app = app + end + + def call(env) + # Grab the SSH info from the machine + info = env[:machine].ssh_info + # XXX: Raise an exception if info is nil, since that means that + # SSH is not ready. + + # Check the SSH key permissions + SSH.check_key_permissions(info[:private_key_path]) + + # Exec! + # XXX: Options given in env + SSH.exec(info) + end + end + end + end +end diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb index bdab72f3d..b8b0438ed 100644 --- a/lib/vagrant/machine.rb +++ b/lib/vagrant/machine.rb @@ -70,6 +70,9 @@ module Vagrant # actually implement the action. # # @param [Symbol] name Name of the action to run. + # @param [Hash] extra_env This data will be passed into the action runner + # as extra data set on the environment hash for the middleware + # runner. def action(name, extra_env=nil) @logger.debug("Calling action: #{name} on provider #{@provider}") diff --git a/lib/vagrant/util/safe_exec.rb b/lib/vagrant/util/safe_exec.rb index ccc214de6..d49a58a56 100644 --- a/lib/vagrant/util/safe_exec.rb +++ b/lib/vagrant/util/safe_exec.rb @@ -6,8 +6,8 @@ module Vagrant # This issue causes `exec` to fail if there is more than one system # thread. In that case, `safe_exec` automatically falls back to # forking. - module SafeExec - def safe_exec(command, *args) + class SafeExec + def self.exec(command, *args) # Create a list of things to rescue from. Since this is OS # specific, we need to do some defined? checks here to make # sure they exist. diff --git a/lib/vagrant/util/ssh.rb b/lib/vagrant/util/ssh.rb index 0bac2a4a7..f7c3a859e 100644 --- a/lib/vagrant/util/ssh.rb +++ b/lib/vagrant/util/ssh.rb @@ -10,8 +10,6 @@ module Vagrant # helpers don't depend on any part of Vagrant except what is given # via the parameters. class SSH - include SafeExec - LOGGER = Log4r::Logger.new("vagrant::util::ssh") # Checks that the permissions for a private key are valid, and fixes @@ -53,7 +51,7 @@ module Vagrant # required please see the documentation of {Machine#ssh_info}. # @param [Hash] opts These are additional options that are supported # by exec. - def exec(ssh_info, opts={}) + def self.exec(ssh_info, opts={}) # If we're running Windows, raise an exception since we currently # still don't support exec-ing into SSH. In the future this should # certainly be possible if we can detect we're in an environment that @@ -98,7 +96,7 @@ module Vagrant end # If we're not in plain mode, attach the private key path. - command_options += ["-i", options[:private_key_path]] if !plain_mode + command_options += ["-i", options[:private_key_path].to_s] if !plain_mode if ssh_info[:forward_x11] # Both are required so that no warnings are shown regarding X11 @@ -113,8 +111,8 @@ module Vagrant command_options << host_string # Invoke SSH with all our options - @logger.info("Invoking SSH: #{command_options.inspect}") - safe_exec("ssh", *command_options) + LOGGER.info("Invoking SSH: #{command_options.inspect}") + SafeExec.exec("ssh", *command_options) end end end diff --git a/plugins/commands/ssh/command.rb b/plugins/commands/ssh/command.rb index 6f5aa7b3f..9adf92bef 100644 --- a/plugins/commands/ssh/command.rb +++ b/plugins/commands/ssh/command.rb @@ -38,9 +38,9 @@ module VagrantPlugins # Execute the actual SSH with_target_vms(argv, :single_target => true) do |vm| # Basic checks that are required for proper SSH - raise Vagrant::Errors::VMNotCreatedError if !vm.created? - raise Vagrant::Errors::VMInaccessible if !vm.state == :inaccessible - raise Vagrant::Errors::VMNotRunningError if vm.state != :running + # raise Vagrant::Errors::VMNotCreatedError if !vm.created? + # raise Vagrant::Errors::VMInaccessible if !vm.state == :inaccessible + # raise Vagrant::Errors::VMNotRunningError if vm.state != :running if options[:command] ssh_execute(vm, options[:command]) @@ -50,7 +50,7 @@ module VagrantPlugins :extra_args => options[:ssh_args] } - ssh_connect(vm, opts) + vm.action(:ssh, :ssh_opts => opts) end end diff --git a/plugins/providers/virtualbox/action.rb b/plugins/providers/virtualbox/action.rb index dcba609a6..44961e2a3 100644 --- a/plugins/providers/virtualbox/action.rb +++ b/plugins/providers/virtualbox/action.rb @@ -4,6 +4,8 @@ module VagrantPlugins module ProviderVirtualBox module Action autoload :CheckAccessible, File.expand_path("../action/check_accessible", __FILE__) + autoload :CheckCreated, File.expand_path("../action/check_created", __FILE__) + autoload :CheckRunning, File.expand_path("../action/check_running", __FILE__) autoload :CheckVirtualbox, File.expand_path("../action/check_virtualbox", __FILE__) autoload :Created, File.expand_path("../action/created", __FILE__) autoload :DestroyConfirm, File.expand_path("../action/destroy_confirm", __FILE__) @@ -55,6 +57,17 @@ module VagrantPlugins end end end + + # This is the action that will exec into an SSH shell. + def self.action_ssh + Vagrant::Action::Builder.new.tap do |b| + b.use CheckVirtualbox + b.use CheckCreated + b.use CheckAccessible + b.use CheckRunning + b.use SSHExec + end + end end end end diff --git a/plugins/providers/virtualbox/action/check_created.rb b/plugins/providers/virtualbox/action/check_created.rb new file mode 100644 index 000000000..307cc1ed4 --- /dev/null +++ b/plugins/providers/virtualbox/action/check_created.rb @@ -0,0 +1,21 @@ +module VagrantPlugins + module ProviderVirtualBox + module Action + # This middleware checks that the VM is created, and raises an exception + # if it is not, notifying the user that creation is required. + class CheckCreated + def initialize(app, env) + @app = app + end + + def call(env) + if env[:machine].state == :not_created + raise Vagrant::Errors::VMNotCreatedError + end + + @app.call(env) + end + end + end + end +end diff --git a/plugins/providers/virtualbox/action/check_running.rb b/plugins/providers/virtualbox/action/check_running.rb new file mode 100644 index 000000000..483493602 --- /dev/null +++ b/plugins/providers/virtualbox/action/check_running.rb @@ -0,0 +1,21 @@ +module VagrantPlugins + module ProviderVirtualBox + module Action + # This middleware checks that the VM is running, and raises an exception + # if it is not, notifying the user that the VM must be running. + class CheckRunning + def initialize(app, env) + @app = app + end + + def call(env) + if env[:machine].state != :running + raise Vagrant::Errors::VMNotRunningError + end + + @app.call(env) + end + end + end + end +end diff --git a/plugins/providers/virtualbox/provider.rb b/plugins/providers/virtualbox/provider.rb index a3180b7d4..ba41289e5 100644 --- a/plugins/providers/virtualbox/provider.rb +++ b/plugins/providers/virtualbox/provider.rb @@ -18,14 +18,19 @@ module VagrantPlugins nil end - # Returns a human-friendly string version of this provider which - # includes the machine's ID that this provider represents, if it - # has one. - # - # @return [String] - def to_s - id = @machine.id ? @machine.id : "new VM" - "VirtualBox (#{id})" + # Returns the SSH info for accessing the VirtualBox VM. + def ssh_info + # If the VM is not created then we cannot possibly SSH into it, so + # we return nil. + return nil if state == :not_created + + # Return what we know. The host is always "127.0.0.1" because + # VirtualBox VMs are always local. The port we try to discover + # by reading the forwarded ports. + return { + :host => "127.0.0.1", + :port => @driver.ssh_port(@machine.config.ssh.guest_port) + } end # Return the state of VirtualBox virtual machine by actually @@ -38,6 +43,16 @@ module VagrantPlugins return :unknown if !state state end + + # Returns a human-friendly string version of this provider which + # includes the machine's ID that this provider represents, if it + # has one. + # + # @return [String] + def to_s + id = @machine.id ? @machine.id : "new VM" + "VirtualBox (#{id})" + end end end end diff --git a/test/unit/vagrant/action/builtin/ssh_exec_test.rb b/test/unit/vagrant/action/builtin/ssh_exec_test.rb new file mode 100644 index 000000000..867848e3e --- /dev/null +++ b/test/unit/vagrant/action/builtin/ssh_exec_test.rb @@ -0,0 +1,22 @@ +require File.expand_path("../../../../base", __FILE__) + +require "vagrant/util/ssh" + +describe Vagrant::Action::Builtin::SSHExec do + let(:app) { lambda { |env| } } + let(:env) { { :machine => machine } } + let(:machine) do + result = double("machine") + result.stub(:ssh_info).and_return(machine_ssh_info) + result + end + let(:machine_ssh_info) { {} } + + it "should check key permissions then exec" do + pending + end + + it "should exec with the options given in `ssh_opts`" do + pending + end +end