Invert forward ssh port behaviour (#1289)

Multi machines environments will result in collisions until auto
correction is implemented. Therefore invert to keep the forwarding of
the default ssh port disabled by default initially until can handle the
port collisions detection and resolving.

Relates: #1012

README.md

@@ -1419,22 +1419,25 @@ Default is `eth0`.
 
 ### Forwarding the ssh-port
 
-By default vagrant-libvirt now allows the standard ssh-port to be forwarded to
-the localhost to allow for consistent provisioning steps/ports used when
+Vagrant-libvirt now supports forwarding the standard ssh-port on port 2222 from
+the localhost to allow for consistent provisioning steps/ports to be used when
 defining across multiple providers.
 
-Previously by default libvirt skipped the forwarding of the ssh-port because
-you can access the machine directly. To return to this behaviour set the
-option `forward_ssh_port` to `false` in the Vagrantfile.
-
+To enable, set the following:
 ```ruby
 Vagrant.configure("2") do |config|
   config.vm.provider :libvirt do |libvirt|
-    # Disable forwarding of forwarded_port with id 'ssh'.
-    libvirt.forward_ssh_port = false
+    # Enable forwarding of forwarded_port with id 'ssh'.
+    libvirt.forward_ssh_port = true
   end
+end
 ```
 
+Previously by default libvirt skipped the forwarding of the ssh-port because
+you can access the machine directly. In the future it is expected that this
+will be enabled by default once autocorrect support is added to handle port
+collisions for multi machine environments gracefully.
+
 ## Synced Folders
 
 Vagrant automatically syncs the project folder on the host to `/vagrant` in

lib/vagrant-libvirt/config.rb

@@ -789,7 +789,7 @@ module VagrantPlugins
         finalize_proxy_command
 
         # forward port with id 'ssh'
-        @forward_ssh_port = true if @forward_ssh_port == UNSET_VALUE
+        @forward_ssh_port = false if @forward_ssh_port == UNSET_VALUE
 
         @storage_pool_name = 'default' if @storage_pool_name == UNSET_VALUE
         @snapshot_pool_name = @storage_pool_name if @snapshot_pool_name == UNSET_VALUE

spec/unit/action/forward_ports_spec.rb

@@ -19,7 +19,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::ForwardPorts do
     allow(machine).to receive(:provider_config).and_return(provider_config)
     allow(machine_config).to receive(:vm).and_return(vm_config)
     allow(vm_config).to receive(:networks).and_return([])
-    allow(provider_config).to receive(:forward_ssh_port).and_return(true)
+    allow(provider_config).to receive(:forward_ssh_port).and_return(false)
   end
 
   describe '#call' do
@@ -30,7 +30,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::ForwardPorts do
       end
     end
 
-    context 'with network includes a forwarded port' do
+    context 'with network including one forwarded port' do
       let(:networks) { [
         [:private_network, {:ip=>"10.20.30.40", :protocol=>"tcp", :id=>"6b8175ed-3220-4b63-abaf-0bb8d7cdd723"}],
         [:forwarded_port, port_options],
@@ -38,12 +38,14 @@ describe VagrantPlugins::ProviderLibvirt::Action::ForwardPorts do
 
       let(:port_options){ {guest: 80, host: 8080} }
 
-      it 'should be called only once' do
+      it 'should compile a single port forward to set up' do
         expect(vm_config).to receive(:networks).and_return(networks)
         expect(ui).to_not receive(:warn)
         expect(subject).to receive(:forward_ports).and_return(nil)
 
         expect(subject.call(env)).to be_nil
+
+        expect(env[:forwarded_ports]).to eq([networks[1][1]])
       end
 
       context 'when host port in protected range' do
@@ -79,6 +81,21 @@ describe VagrantPlugins::ProviderLibvirt::Action::ForwardPorts do
       ]}
 
       context 'with default config' do
+        it 'should not forward the ssh port' do
+          expect(vm_config).to receive(:networks).and_return(networks)
+          expect(subject).to receive(:forward_ports)
+
+          expect(subject.call(env)).to be_nil
+
+          expect(env[:forwarded_ports]).to eq([networks[1][1]])
+        end
+      end
+
+      context 'with forward_ssh_port enabled' do
+        before do
+          allow(provider_config).to receive(:forward_ssh_port).and_return(true)
+        end
+
         it 'should forward the port' do
           expect(vm_config).to receive(:networks).and_return(networks)
           expect(subject).to receive(:forward_ports)
@@ -88,21 +105,6 @@ describe VagrantPlugins::ProviderLibvirt::Action::ForwardPorts do
           expect(env[:forwarded_ports]).to eq(networks.drop(1).map { |_, opts| opts })
         end
       end
-
-      context 'with forward_ssh_port disabled' do
-        before do
-          allow(provider_config).to receive(:forward_ssh_port).and_return(false)
-        end
-
-        it 'should not forward the port' do
-          expect(vm_config).to receive(:networks).and_return(networks)
-          expect(subject).to receive(:forward_ports)
-
-          expect(subject.call(env)).to be_nil
-
-          expect(env[:forwarded_ports]).to eq([networks[1][1]])
-        end
-      end
     end
   end