Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle the ssh port collisions on a vagrant file with multiple hosts #63

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

karcaw
Copy link

@karcaw karcaw commented Nov 14, 2024

In using vargrant-qemu under an ansible molecule configuration, I needed to be able not to have port collisions when there were multiple hosts in the VagrantFile, much like I have enjoyed with the vagrant-virtualbox implementation.

This patch adds the needed code to detect collisions using the sys/protectable gem and parse out the forwarded ports.

This patch also adds code to be sure that the vagrant ssh-config calls return the proper ports.

This patch uses a modified version of the handle_forwarded_port_collisions.rb action copied from the vagrant source code to modify the ssh_port appropriately.

I am not in ruby every day, and this is my first time hacking on vagrant plugins, so if there are better ways to do some of these things, please suggest them and I will re-write

@ppggff
Copy link
Owner

ppggff commented Nov 15, 2024

Thanks for the pr, I will check it later.
Can you provide a example for the 'port collisions' case?

@karcaw
Copy link
Author

karcaw commented Nov 15, 2024

Here is a sample Vagrant file that should boot 3 different systems on a M2/3/4-based mac with qemu.
Vagrantfile.txt

@ppggff
Copy link
Owner

ppggff commented Nov 19, 2024

May I ask why not just specify the ssh port for each machine?

@karcaw
Copy link
Author

karcaw commented Nov 19, 2024

Two reasons, this is normally generated automatically by molecule, and while one could specify it for 3 machines in the config, as soon as you start sharing the config across scenarios, and you try to start two scenarios or CI runs at the same time, we are back in conflict mode, and need this anyway.

@ppggff
Copy link
Owner

ppggff commented Dec 1, 2024

Thanks, I check your PR carefully, there are some areas for improvement:

  • don't copy the handle_forwarded_port_collisions.rb from vagrant, the vagrant core already have the capability to handle the port collisions, we just need to set the ssh port in the network configuration
  • is there another way (without new dependency) to check the cmd line for running process? may be we don't need it in this problem
  • for the get_ssh_port function, consider creating a new function to call with control port. then use it in stop function too
  • don't change the version info

I did a little hack in the prepare_forwarded_port_collision_params.rb and start_instance.rb as an example to use vagrant's build-in 'port collisions' function:

diff --git a/lib/vagrant-qemu/action/prepare_forwarded_port_collision_params.rb b/lib/vagrant-qemu/action/prepare_forwarded_port_collision_params.rb
index 159a785..b7c93a3 100644
--- a/lib/vagrant-qemu/action/prepare_forwarded_port_collision_params.rb
+++ b/lib/vagrant-qemu/action/prepare_forwarded_port_collision_params.rb
@@ -16,16 +16,23 @@ module VagrantPlugins
       # Build the remap for any existing collision detections
       remap = {}
       env[:port_collision_remap] = remap
+
+      has_ssh_forward = false
       machine.config.vm.networks.each do |type, options|
         next if type != :forwarded_port

         # remap ssh.host to ssh_port
         if options[:id] == "ssh"
           remap[options[:host]] = machine.provider_config.ssh_port
+          has_ssh_forward = true
           break
         end
       end

+      if !has_ssh_forward
+        machine.config.vm.networks.forward_port(22, machine.provider_config.ssh_port, [id: "ssh", auto_correct: true])
+      end
+
       @app.call(env)
       end
     end
diff --git a/lib/vagrant-qemu/action/start_instance.rb b/lib/vagrant-qemu/action/start_instance.rb
index 13c1d9d..b434bb3 100644
--- a/lib/vagrant-qemu/action/start_instance.rb
+++ b/lib/vagrant-qemu/action/start_instance.rb
@@ -11,6 +11,7 @@ module VagrantPlugins
         end

         def call(env)
+          fwPorts = forwarded_ports(env)
           options = {
             :ssh_host => env[:machine].provider_config.ssh_host,
             :ssh_port => env[:machine].provider_config.ssh_port,
@@ -23,7 +24,7 @@ module VagrantPlugins
             :drive_interface => env[:machine].provider_config.drive_interface,
             :extra_qemu_args => env[:machine].provider_config.extra_qemu_args,
             :extra_netdev_args => env[:machine].provider_config.extra_netdev_args,
-            :ports => forwarded_ports(env),
+            :ports => fwPorts,
             :control_port => env[:machine].provider_config.control_port,
             :debug_port => env[:machine].provider_config.debug_port,
             :no_daemonize => env[:machine].provider_config.no_daemonize,
@@ -43,7 +44,12 @@ module VagrantPlugins
             next if type != :forwarded_port

             # Don't include SSH
-            next if options[:id] == "ssh"
+            if options[:id] == "ssh"
+              if options[:host] != env[:machine].provider_config.ssh_port
+                  env[:machine].provider_config.ssh_port = options[:host]
+              end
+              next
+            end

             # Skip port if it is disabled
             next if options[:disabled]

@karcaw
Copy link
Author

karcaw commented Dec 19, 2024

Thanks, I check your PR carefully, there are some areas for improvement:

  • don't copy the handle_forwarded_port_collisions.rb from vagrant, the vagrant core already have the capability to handle the port collisions, we just need to set the ssh port in the network configuration

Removed this file, and changed to use your patch

  • is there another way (without new dependency) to check the cmd line for running process? may be we don't need it in this problem

Yes there is a way, just less library use. direct calls to ps do fine on linux and osx. If we need other platforms this may need more work.

  • for the get_ssh_port function, consider creating a new function to call with control port. then use it in stop function too

I'm not sure what you meant here, so I have not changed this part.

  • don't change the version info

Sorry about that, I had changed it to make gem files, and it got included

I did a little hack in the prepare_forwarded_port_collision_params.rb and start_instance.rb as an example to use vagrant's build-in 'port collisions' function:

Thanks, I included it in my repairs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants