PR#200: Use random SSH and VNC ports - standard-test-roles

standard-test-roles

#200 Use random SSH and VNC ports

Closed 5 years ago by till. Opened 5 years ago by till.

https://github.com/tyll/standard-test-roles randomport into master

Use random vnc port

Till Maas • 5 years ago

8e70d7c

Use random port for hostfwd

Till Maas • 5 years ago

c8d72df

inventory/standard-inventory-qcow2

file modified

+21 -11

		`@@ -1,19 +1,20 @@`
		`#!/usr/bin/env python`

		`import argparse`
		`+ import distutils.util`
		`import errno`
		`import json`
		`+ import multiprocessing`
		`import os`
		`- import shutil`
		`+ import random`
		`import shlex`
		`+ import shutil`
		`import signal`
		`- import multiprocessing`
		`import socket`
		`import subprocess`
		`import sys`
		`import tempfile`
		`import time`
		`- import distutils.util`

		`IDENTITY = """`
		`-----BEGIN RSA PRIVATE KEY-----`
		`@@ -111,7 +112,10 @@`


		`def start_qemu(image, cloudinit, log, portrange=(2222, 5555)):`
		`- for port in range(*portrange):`
		`+`
		`+ tries = 0`
		`+ while tries < (portrange[1] - portrange[0]):`
		`+ port = random.randrange(*portrange)`
		`sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)`
		`sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)`
		`try:`
		`@@ -122,18 +126,24 @@`
		`break`
		`finally:`
		`sock.close()`
		`+ tries += 1`
		`else:`
		`raise RuntimeError("unable to find free local port to map SSH to")`

		`+ vncoffset = portrange[1] - port`
		`# Use -cpu host and -smp by default.`
		`# virtio-rng-pci: https://wiki.qemu.org/Features/VirtIORNG`
		`- return subprocess.Popen(["/usr/bin/qemu-system-x86_64",`
		`- "-cpu", "host", "-smp", "{}".format(multiprocessing.cpu_count()),`
		`- "-m", "1024", image, "-enable-kvm", "-snapshot", "-cdrom", cloudinit,`
		`- "-net", "nic,model=virtio", "-net", "user,hostfwd=tcp:127.0.0.3:{0}-:22".format(port),`
		`- "-device", "virtio-rng-pci", "-rtc", "base=utc",`
		`- "-device", "isa-serial,chardev=pts2", "-chardev", "file,id=pts2,path=" + log,`
		`- "-display", "none", "-vnc", ":1"], stdout=open(log, 'a'), stderr=subprocess.STDOUT), port`
		`+ return subprocess.Popen(`
		`+ ["/usr/bin/qemu-system-x86_64", "-cpu", "host",`
		`+ "-smp", "{}".format(multiprocessing.cpu_count()),`
		`+ "-m", "1024", image, "-enable-kvm", "-snapshot", "-cdrom", cloudinit,`
		`+ "-net", "nic,model=virtio",`
		`+ "-net", "user,hostfwd=tcp:127.0.0.3:{0}-:22".format(port),`
		`+ "-device", "virtio-rng-pci", "-rtc", "base=utc",`
		`+ "-device", "isa-serial,chardev=pts2",`
		`+ "-chardev", "file,id=pts2,path=" + log, "-display", "none",`
		`+ "-vnc", ":" + str(vncoffset)`
		`+ ], stdout=open(log, 'a'), stderr=subprocess.STDOUT), port`


		`def inv_host(image):`

no initial comment

till commented 5 years ago

This addresses #199

astepano commented 5 years ago

Hi,
Thank you for the PR.
I think the more easy solution is better:
Always use:

qemu-kvm -vnc :0,to=10

It will bee very simple diff:

diff --git a/inventory/standard-inventory-qcow2 b/inventory/standard-inventory-qcow2
index 43af9a5..fc53728 100755
--- a/inventory/standard-inventory-qcow2
+++ b/inventory/standard-inventory-qcow2
@@ -133,7 +133,7 @@ def start_qemu(image, cloudinit, log, portrange=(2222, 5555)):
                              "-net", "nic,model=virtio", "-net", "user,hostfwd=tcp:127.0.0.3:{0}-:22".format(port),
                              "-device", "virtio-rng-pci", "-rtc", "base=utc",
                              "-device", "isa-serial,chardev=pts2", "-chardev", "file,id=pts2,path=" + log,
-                             "-display", "none", "-vnc", ":1"], stdout=open(log, 'a'), stderr=subprocess.STDOUT), port
+                             "-display", "none", "-vnc", ":1,to=20"], stdout=open(log, 'a'), stderr=subprocess.STDOUT), port


 def inv_host(image):

https://qemu.weilnetz.de/doc/qemu-doc.html

to=L
With this option, QEMU will try next available VNC displays, until the number L, if the origianlly defined "-vnc display" is not available, e.g. port 5900+display is already used by another application. By default, to=0.

till commented 5 years ago

Using to sounds good, but I would prefer a bigger number than 20, since there might be already 20 other machines running. How about 4096 or so? Then it is roughly the same as the available SSH ports?

walters commented 5 years ago

I think we should drop the VNC thing by default and have people enable it if they want it.

Or perhaps, looking at all of the issues/PRs against what was an initially simple standard-inventory-qcow2 - perhaps we should deprecate it and add a more explicitly configurable convenience wrapper around running qemu-kvm and outputting an Ansible inventory + ssh keys to talk to it.

Particularly we should expose the raw qemu arguments (unlike libvirt!).

till commented 5 years ago

I was also wondering about the purpose of the vnc option. Maybe it would make sense to only set it when TEST_DEBUGis set for now.

astepano commented 5 years ago

@till @walters
for VNC I made next fix:

https://pagure.io/standard-test-roles/c/99cd77f74cffb8ffec45c71e4a2e3d94cb5e492f?branch=master

Please close this PR is the above fix is OK. Thank you!

walters commented 5 years ago

But we still need to solve the issue for the SSH port right?

BTW would you mind submitting PRs so that other people can review them rather than pushing directly to git master? Currently we use this project as part of our commit gating to e.g. the ostree project, so having things function is fairly critical path for us. I don't mind spending a little bit of time on patch reviews here if asked. We have lots of other people who can do reviews as well.

astepano commented 5 years ago

@walters I usually send my changes as PR.
This changes were really small.
I tested my commit/changes before submitting , and it perfectly works for me.
But, yes, you are right I will follow PR workflow.

We still need a fix for random SSH port.
@till could you please rebase your PR on current master? Thank you!

walters commented 5 years ago

My original PR in https://pagure.io/standard-test-roles/pull-request/188 still applies cleanly FWIW, we could just push that? I am not sure how to reopen a PR on pagure.

astepano commented 5 years ago

ok.

astepano commented 5 years ago

PR #203 fixes SSH ports.

This PR can be closed.

Edited 5 years ago by astepano