use ssh.username and ssh.port standard options in the provider

Fix #88

README.md

@@ -73,8 +73,10 @@ require 'vagrant-openstack-provider'
 
 Vagrant.configure('2') do |config|
 
-  config.vm.box = 'dummy-openstack'
-  config.vm.box_url = 'https://github.com/ggiamarchi/vagrant-openstack/raw/master/source/dummy.box'
+  config.vm.box       = 'dummy-openstack'
+  config.vm.box_url   = 'https://github.com/ggiamarchi/vagrant-openstack/raw/master/source/dummy.box'
+
+  config.ssh.username = 'stack'
 
   config.vm.provider :openstack do |os|
     os.openstack_auth_url = 'http://keystone-server.net/v2.0/tokens'
@@ -84,7 +86,6 @@ Vagrant.configure('2') do |config|
     os.flavor             = 'm1.small'
     os.image              = 'ubuntu'
     os.floating_ip_pool   = 'publicNetwork'
-    os.ssh_username       = 'stack'
   end
 end
 ```
@@ -168,7 +169,7 @@ a volume name is not unique, thus if there is two volumes with the same name in
 you have to use only ids. Optionally, you can specify the device that will be assigned to the volume.
 
 Here comes an example that show six volumes attached to a server :
- 
+
 ```ruby
 config.vm.provider :openstack do |os|
  ...
@@ -200,7 +201,6 @@ end
 * `keypair_name` - The name of the key pair register in nova to associate with the VM. The public key should
   be the matching pair for the private key configured with `config.ssh.private_key_path` on Vagrant.
 * `public_key_path` - if `keypair_name` is not provided, the path to the public key will be used by vagrant to generate a keypair on the OpenStack cloud. The keypair will be destroyed when the VM is destroyed.
-* `ssh_username` - Username used by Vagrant for ssh login.
 
 If neither `keypair_name` nor `public_key_path` are set, vagrant will generate a new ssh key and automatically import it in Openstack.
 
@@ -220,6 +220,14 @@ the remote machine over SSH.
 This is good enough for all built-in Vagrant provisioners (shell,
 chef, and puppet) to work!
 
+## Vagrant standard configuration
+
+There are some standard configuration options that this provider takes into account when
+creating and connecting to OpenStack machines
+
+* `ssh.username` - Username used by vagrant for SSH login
+* `ssh.port` - Default SSH port is 22. If set, this option will override the default for SSH login
+* `ssh.private_key_path` - If set, vagrant will use this private key path to SSH on the machine. If you set this option, the `public_key_path` option of the provider should be set.
 
 ## Custom commands
 

source/Vagrantfile

@@ -4,6 +4,7 @@ Vagrant.configure('2') do |config|
 
   config.vm.box = 'dummy-openstack'
   config.vm.box_url = 'https://github.com/ggiamarchi/vagrant-openstack/raw/master/source/dummy.box'
+  config.ssh.username = 'stack'
 
   config.vm.provider :openstack do |os|
     os.username = ENV['OS_USERNAME']
@@ -13,6 +14,5 @@ Vagrant.configure('2') do |config|
     os.image = ENV['OS_IMAGE']
     os.openstack_auth_url = ENV['OS_AUTH_URL']
     os.tenant_name = ENV['OS_TENANT_NAME']
-    os.ssh_username = 'stack'
   end
 end

source/lib/vagrant-openstack-provider/action/create_server.rb

@@ -62,7 +62,7 @@ def call(env)
 
             # Wait for SSH to become available
             ssh_timeout = env[:machine].provider_config.ssh_timeout
-            unless port_open?(env, floating_ip, 22, ssh_timeout)
+            unless port_open?(env, floating_ip, resolve_ssh_port(env), ssh_timeout)
               env[:ui].error(I18n.t('vagrant_openstack.timeout'))
               fail Errors::SshUnavailable, host: floating_ip, timeout: ssh_timeout
             end
@@ -76,6 +76,12 @@ def call(env)
 
         private
 
+        def resolve_ssh_port(env)
+          machine_config = env[:machine].config
+          return machine_config.ssh.port if machine_config.ssh.port
+          22
+        end
+
         # 1. if floating_ip is set, use it
         # 2. if floating_ip_pool is set
         #    GET v2/{{tenant_id}}/os-floating-ips

source/lib/vagrant-openstack-provider/action/read_ssh_info.rb

@@ -24,13 +24,27 @@ def read_ssh_info(env)
           config = env[:machine].provider_config
           hash = {
             host: get_ip_address(env),
-            port: 22,
-            username: config.ssh_username
+            port: resolve_ssh_port(env),
+            username: resolve_ssh_username(env)
           }
           hash[:private_key_path] = "#{env[:machine].data_dir}/#{get_keypair_name(env)}" unless config.keypair_name || config.public_key_path
           hash
         end
 
+        def resolve_ssh_port(env)
+          machine_config = env[:machine].config
+          return machine_config.ssh.port if machine_config.ssh.port
+          22
+        end
+
+        def resolve_ssh_username(env)
+          config = env[:machine].provider_config
+          machine_config = env[:machine].config
+          return machine_config.ssh.username if machine_config.ssh.username
+          return config.ssh_username if config.ssh_username
+          fail Errors::NoMatchingSshUsername
+        end
+
         def get_ip_address(env)
           return env[:machine].provider_config.floating_ip unless env[:machine].provider_config.floating_ip.nil?
           details = env[:openstack_client].nova.get_server_details(env, env[:machine].id)

source/lib/vagrant-openstack-provider/config.rb

@@ -211,7 +211,7 @@ def validate(machine)
         errors << I18n.t('vagrant_openstack.config.password_required') unless @password
         errors << I18n.t('vagrant_openstack.config.username_required') unless @username
 
-        validate_ssh_username
+        validate_ssh_username(machine, errors)
 
         if machine.config.ssh.private_key_path
           puts I18n.t('vagrant_openstack.config.keypair_name_required').yellow unless @keypair_name || @public_key_path
@@ -231,8 +231,9 @@ def validate(machine)
         { 'Openstack Provider' => errors }
       end
 
-      def validate_ssh_username
+      def validate_ssh_username(machine, errors)
         puts I18n.t('vagrant_openstack.config.ssh_username_deprecated').yellow if @ssh_username
+        errors << I18n.t('vagrant_openstack.config.ssh_username_required') unless @ssh_username || machine.config.ssh.username
       end
 
       private

source/lib/vagrant-openstack-provider/errors.rb

@@ -95,6 +95,10 @@ class MissingBootOption < VagrantOpenstackError
       class ConflictBootOption < VagrantOpenstackError
         error_key(:conflict_boot_option)
       end
+
+      class NoMatchingSshUsername < VagrantOpenstackError
+        error_key(:ssh_username_missing)
+      end
     end
   end
 end

source/locales/en.yml

@@ -66,6 +66,8 @@ en:
         ssh_username provider config is deprecated for vagrant-openstack provider.
         If you are using it, it will continue to work but we recommend to switch to the
         standard vagrant configuration option `config.ssh.username` instead
+      ssh_username_required: |-
+        vagrant standard configuration option `ssh.username` is required
 
     errors:
       default: |-
@@ -128,6 +130,8 @@ en:
         Either 'image' or 'volume_boot' configuration must be provided
       conflict_boot_option: |-
         Only one of 'image' and 'volume_boot' configuration must be provided
+      ssh_username_missing: |-
+        Vagrant was unable to resolve which ssh username to use to connect to the machine. Please provide config parameter `ssh.username`
 
     states:
       short_active: |-

source/spec/vagrant-openstack-provider/action/read_ssh_info_spec.rb

@@ -14,9 +14,11 @@
       config.stub(:tenant_name) { 'testTenant' }
       config.stub(:username) { 'username' }
       config.stub(:password) { 'password' }
-      config.stub(:ssh_username) { 'sshuser' }
+      config.stub(:ssh_username) { 'test_username' }
       config.stub(:floating_ip) { nil }
       config.stub(:floating_ip_pool) { nil }
+      config.stub(:keypair_name) { nil }
+      config.stub(:public_key_path) { nil }
     end
   end
 
@@ -36,12 +38,26 @@
     end
   end
 
+  let(:ssh_config) do
+    double('ssh_config').tap do |config|
+      config.stub(:username) { 'sshuser' }
+      config.stub(:port) { nil }
+    end
+  end
+
+  let(:machine_config) do
+    double('machine_config').tap do |config|
+      config.stub(:ssh) { ssh_config }
+    end
+  end
+
   let(:env) do
     Hash.new.tap do |env|
       env[:ui] = double('ui')
       env[:ui].stub(:info).with(anything)
       env[:machine] = double('machine')
       env[:machine].stub(:provider_config) { config }
+      env[:machine].stub(:config) { machine_config }
       env[:machine].stub(:id) { '1234' }
       env[:machine].stub(:data_dir) { '/data/dir' }
       env[:openstack_client] = double('openstack_client')
@@ -56,6 +72,36 @@
   end
 
   describe 'read_ssh_info' do
+    context 'with deprecated ssh_username specified' do
+      context 'with ssh.username specified' do
+        it 'returns ssh.username' do
+          ssh_config.stub(:username) { 'sshuser' }
+          config.stub(:ssh_username) { 'test_username' }
+          config.stub(:floating_ip) { '80.80.80.80' }
+          config.stub(:keypair_name) { 'my_keypair' }
+          @action.read_ssh_info(env).should eq(host: '80.80.80.80', port: 22, username: 'sshuser')
+        end
+      end
+      context 'without ssh.username specified' do
+        it 'returns ssh.username' do
+          ssh_config.stub(:username) { nil }
+          config.stub(:ssh_username) { 'test_username' }
+          config.stub(:floating_ip) { '80.80.80.80' }
+          config.stub(:keypair_name) { 'my_keypair' }
+          @action.read_ssh_info(env).should eq(host: '80.80.80.80', port: 22, username: 'test_username')
+        end
+      end
+    end
+
+    context 'with ssh.port overriden' do
+      it 'returns ssh.port' do
+        ssh_config.stub(:port) { 33 }
+        config.stub(:floating_ip) { '80.80.80.80' }
+        config.stub(:keypair_name) { 'my_keypair' }
+        @action.read_ssh_info(env).should eq(host: '80.80.80.80', port: 33, username: 'sshuser')
+      end
+    end
+
     context 'with config.floating_ip specified' do
       context 'with keypair_name specified' do
         it 'returns the specified floating ip' do
@@ -84,9 +130,7 @@
         end
       end
     end
-  end
 
-  describe 'read_ssh_info' do
     context 'without config.floating_ip specified' do
       it 'return the a floating_ip found by querying server details' do
         nova.stub(:get_server_details).with(env, '1234') do
@@ -106,4 +150,5 @@
       end
     end
   end
+
 end

source/spec/vagrant-openstack-provider/config_spec.rb

@@ -74,6 +74,7 @@
       error_message.stub(:yellow) { 'Yellowed Error message ' }
       machine.stub_chain(:env, :root_path).and_return '/'
       ssh.stub(:private_key_path) { 'private key path' }
+      ssh.stub(:username) { 'ssh username' }
       config.stub(:ssh) { ssh }
       machine.stub(:config) { config }
       subject.username = 'foo'
@@ -96,6 +97,15 @@
       end
     end
 
+    context 'with no ssh username provider' do
+      it 'should raise an error' do
+        ssh.stub(:username) { nil }
+        subject.ssh_username = nil
+        I18n.should_receive(:t).with('vagrant_openstack.config.ssh_username_required').and_return error_message
+        validation_errors.first.should == error_message
+      end
+    end
+
     context 'with good values' do
       it 'should validate' do
         validation_errors.should be_empty