-
Notifications
You must be signed in to change notification settings - Fork 31
test(qemu): replace Fedora Cloud image with Alpine Linux tiny image #809
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| from contextlib import contextmanager | ||
|
|
||
| import click | ||
| from fabric import Connection | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: fd jumpstarter-driver-qemu -type dRepository: jumpstarter-dev/jumpstarter Length of output: 242 🏁 Script executed: cat -n python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/client.py | head -100Repository: jumpstarter-dev/jumpstarter Length of output: 4055 🏁 Script executed: cat -n python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/client.py | tail -20Repository: jumpstarter-dev/jumpstarter Length of output: 913 🏁 Script executed: grep -n "Connection\|127.0.0.1\|Hostfwd" python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/client.pyRepository: jumpstarter-dev/jumpstarter Length of output: 172 🏁 Script executed: fd jumpstarter-driver-network -type dRepository: jumpstarter-dev/jumpstarter Length of output: 242 🏁 Script executed: find . -name "*jumpstarter-driver-network*" -type d 2>/dev/null | head -5Repository: jumpstarter-dev/jumpstarter Length of output: 116 🏁 Script executed: find . -name "FabricAdapter*" -o -path "*/jumpstarter_driver_network/*" -name "*.py" 2>/dev/null | head -10Repository: jumpstarter-dev/jumpstarter Length of output: 979 🏁 Script executed: cat -n python/packages/jumpstarter-driver-network/jumpstarter_driver_network/adapters/fabric.py | head -60Repository: jumpstarter-dev/jumpstarter Length of output: 1419 🏁 Script executed: grep -n "Hostfwd\|hostaddr" python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py | head -20Repository: jumpstarter-dev/jumpstarter Length of output: 884 🏁 Script executed: find . -name "*.py" -type f | xargs grep -l "Hostfwd\|hostaddr" 2>/dev/null | head -10Repository: jumpstarter-dev/jumpstarter Length of output: 228 🏁 Script executed: cat -n python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py | grep -A 5 -B 5 "hostfwd\|Hostfwd" | head -50Repository: jumpstarter-dev/jumpstarter Length of output: 2610 🏁 Script executed: grep -n "TcpNetwork\|get_hostfwd_port" python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.pyRepository: jumpstarter-dev/jumpstarter Length of output: 398 🏁 Script executed: cat -n python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py | sed -n '571,590p'Repository: jumpstarter-dev/jumpstarter Length of output: 871 🏁 Script executed: cat -n python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py | sed -n '485,495p'Repository: jumpstarter-dev/jumpstarter Length of output: 443 🏁 Script executed: cat -n python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py | sed -n '455,475p'Repository: jumpstarter-dev/jumpstarter Length of output: 1112 🏁 Script executed: grep -B5 -A10 "def get_hostfwd_port" python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.pyRepository: jumpstarter-dev/jumpstarter Length of output: 604 🏁 Script executed: cat -n python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/client.py | sed -n '78,98p'Repository: jumpstarter-dev/jumpstarter Length of output: 1024 🏁 Script executed: grep -n "export\|get_hostfwd_port\|get_hostaddr" python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.pyRepository: jumpstarter-dev/jumpstarter Length of output: 480 🏁 Script executed: grep -n "def get_\|`@export`" python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py | grep -A1 "def get_"Repository: jumpstarter-dev/jumpstarter Length of output: 305 🏁 Script executed: cat -n python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py | sed -n '569,610p'Repository: jumpstarter-dev/jumpstarter Length of output: 1847 🏁 Script executed: grep -B5 -A10 "def get_hostfwd" python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.pyRepository: jumpstarter-dev/jumpstarter Length of output: 604 🏁 Script executed: cat -n python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.py | sed -n '431,436p'Repository: jumpstarter-dev/jumpstarter Length of output: 327 Use the network child for SSH shell connections instead of hardcoding 127.0.0.1. The 🤖 Prompt for AI Agents
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| from jumpstarter_driver_composite.client import CompositeClient | ||
| from jumpstarter_driver_network.adapters import FabricAdapter, NovncAdapter | ||
|
|
||
|
|
@@ -75,12 +76,25 @@ def novnc(self): | |
|
|
||
| @contextmanager | ||
| def shell(self): | ||
| with FabricAdapter( | ||
| client=self.ssh, | ||
| user=self.username, | ||
| connect_kwargs={"password": self.password}, | ||
| ) as conn: | ||
| yield conn | ||
| # If the driver has an 'ssh' hostfwd entry, fetch the actual host port | ||
| # (resolving any port=0 assignment) and connect directly over TCP. | ||
| # Otherwise fall back to tunnelling through the jumpstarter stream (vsock). | ||
| try: | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we had to implement this because alpine does not setup ssh over vhost, and we didn't want to pick a port that could potentially conflict with whatever is on the testing host. |
||
| port = int(self.call("get_hostfwd_port", "ssh")) | ||
| with Connection( | ||
| host="127.0.0.1", | ||
| port=port, | ||
| user=self.username, | ||
| connect_kwargs={"password": self.password}, | ||
| ) as conn: | ||
| yield conn | ||
| except KeyError: | ||
|
Comment on lines
+86
to
+91
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay this one is a little backwards but hear me out: I think |
||
| with FabricAdapter( | ||
| client=self.ssh, | ||
| user=self.username, | ||
| connect_kwargs={"password": self.password}, | ||
| ) as conn: | ||
| yield conn | ||
|
Comment on lines
+82
to
+97
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's find and examine the file
find . -type f -name "client.py" | grep jumpstarter-driver-qemuRepository: jumpstarter-dev/jumpstarter Length of output: 147 🏁 Script executed: # Read the client.py file to see the full context
cat -n "python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/client.py" | head -150Repository: jumpstarter-dev/jumpstarter Length of output: 4941 🏁 Script executed: # Check the imports to see if contextmanager is imported
head -20 "python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/client.py"Repository: jumpstarter-dev/jumpstarter Length of output: 618 Narrow the In a Proposed fix+ try:
+ port = int(self.call("get_hostfwd_port", "ssh"))
+ except KeyError:
+ with FabricAdapter(
+ client=self.ssh,
+ user=self.username,
+ connect_kwargs={"password": self.password},
+ ) as conn:
+ yield conn
+ else:
with Connection(
host="127.0.0.1",
port=port,
user=self.username,
connect_kwargs={"password": self.password},
) as conn:
yield conn
- except KeyError:
- with FabricAdapter(
- client=self.ssh,
- user=self.username,
- connect_kwargs={"password": self.password},
- ) as conn:
- yield conn🤖 Prompt for AI Agents
Comment on lines
+86
to
+97
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Narrow the try/except to only wrap the |
||
|
|
||
| def cli(self): | ||
| # Get the base group from CompositeClient which includes all child commands | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| import logging | ||
| import os | ||
| import platform | ||
| import shlex | ||
| import shutil | ||
| from collections.abc import AsyncGenerator | ||
| from dataclasses import dataclass, field | ||
|
|
@@ -381,6 +382,26 @@ async def on(self) -> None: # noqa: C901 | |
| Path(self.parent._pty).unlink(missing_ok=True) | ||
| Path(self.parent._pty).symlink_to(pty) | ||
|
|
||
| # Resolve any hostport=0 hostfwd entries to the actual port QEMU chose. | ||
| # Parse 'info usernet': lines look like "TCP[HOST_FORWARD] fd addr port addr port ..." | ||
| # Store resolved ports on the parent so get_hostfwd_port() can return them to clients. | ||
| zero_fwds = {k: v for k, v in self.parent.hostfwd.items() if v.hostport == 0} | ||
| if zero_fwds: | ||
| usernet = await qmp.execute("human-monitor-command", {"command-line": "info usernet"}) | ||
| self.logger.debug("info usernet output:\n%s", usernet) | ||
| for line in usernet.splitlines(): | ||
| parts = line.split() | ||
| if len(parts) >= 6 and "HOST_FORWARD" in parts[0]: | ||
| # parts: Protocol[State] fd hostaddr hostport guestaddr guestport ... | ||
| actual_hostaddr, actual_hostport, actual_guestport = parts[2], int(parts[3]), int(parts[5]) | ||
| for k, v in zero_fwds.items(): | ||
| if v.hostaddr == actual_hostaddr and v.guestport == actual_guestport: | ||
| self.logger.info( | ||
| "hostfwd '%s': resolved port 0 -> %s:%d (guest port %d)", | ||
| k, actual_hostaddr, actual_hostport, actual_guestport, | ||
| ) | ||
| self.parent._resolved_hostports[k] = actual_hostport | ||
|
Comment on lines
+385
to
+403
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find . -type f -name "driver.py" | grep jumpstarter-driver-qemuRepository: jumpstarter-dev/jumpstarter Length of output: 147 🏁 Script executed: wc -l python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.pyRepository: jumpstarter-dev/jumpstarter Length of output: 149 🏁 Script executed: sed -n '380,410p' python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.pyRepository: jumpstarter-dev/jumpstarter Length of output: 1849 🏁 Script executed: sed -n '560,580p' python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.pyRepository: jumpstarter-dev/jumpstarter Length of output: 765 🏁 Script executed: sed -n '430,440p' python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.pyRepository: jumpstarter-dev/jumpstarter Length of output: 382 🏁 Script executed: sed -n '460,470p' python/packages/jumpstarter-driver-qemu/jumpstarter_driver_qemu/driver.pyRepository: jumpstarter-dev/jumpstarter Length of output: 553 🏁 Script executed: rg "_resolved_hostports" python/packages/jumpstarter-driver-qemu/Repository: jumpstarter-dev/jumpstarter Length of output: 623 🏁 Script executed: rg "get_hostfwd_port" python/packages/jumpstarter-driver-qemu/Repository: jumpstarter-dev/jumpstarter Length of output: 498 🏁 Script executed: rg "get_hostfwd_port" --type pyRepository: jumpstarter-dev/jumpstarter Length of output: 498 Fail fast when a dynamic hostfwd port is unresolved. Currently, if The proposed fix is necessary and should be applied:
🤖 Prompt for AI Agents
Comment on lines
+392
to
+403
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
|
|
||
| await qmp.execute("system_reset") | ||
| await qmp.disconnect() | ||
|
|
||
|
Comment on lines
382
to
407
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A |
||
|
|
@@ -410,7 +431,7 @@ def close(self): | |
| class Hostfwd(BaseModel): | ||
| protocol: Literal["tcp"] = "tcp" | ||
| hostaddr: str = "127.0.0.1" | ||
| hostport: int = Field(ge=1, le=65535) | ||
| hostport: int = Field(ge=0, le=65535) # 0 = let QEMU pick a free port | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] Suggested fix: add a Pydantic AI-generated, human reviewed |
||
| guestport: int = Field(ge=1, le=65535) | ||
|
|
||
|
|
||
|
Comment on lines
431
to
437
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If |
||
|
|
@@ -440,6 +461,8 @@ class Qemu(Driver): | |
| flash_timeout: int = field(default=30 * 60) # 30 minutes | ||
|
|
||
| _tmp_dir: TemporaryDirectory = field(init=False, default_factory=TemporaryDirectory) | ||
| # Maps hostfwd key -> actual host port after QEMU resolves port 0 assignments | ||
| _resolved_hostports: dict[str, int] = field(init=False, default_factory=dict) | ||
|
|
||
| @classmethod | ||
| def client(cls) -> str: | ||
|
Comment on lines
461
to
468
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Defer TcpNetwork child creation for hostport=0 entries until after port resolution in |
||
|
|
@@ -512,6 +535,7 @@ def cidata(self) -> TemporaryDirectory: | |
| { | ||
| "instance-id": str(self.uuid), | ||
| "local-hostname": self.hostname, | ||
| "hostname": self.hostname, | ||
| } | ||
| ) | ||
| ) | ||
|
|
@@ -528,12 +552,30 @@ def cidata(self) -> TemporaryDirectory: | |
| "sudo": "ALL=(ALL) NOPASSWD:ALL", | ||
| } | ||
| ], | ||
| # runcmd sets the password explicitly for cloud-init implementations | ||
| # that do not support plain_text_passwd (e.g. Alpine's tiny-cloud). | ||
| # cloud-init ignores runcmd entries it doesn't understand, so this | ||
| # is safe to include unconditionally. | ||
| # shlex.quote ensures special characters in credentials are safe. | ||
| "runcmd": [ | ||
| f"printf %s {shlex.quote(f'{self.username}:{self.password}')} | chpasswd", | ||
| ], | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
| ) | ||
| ) | ||
|
|
||
| return tmp | ||
|
|
||
| @export | ||
| @validate_call(validate_return=True) | ||
| def get_hostfwd_port(self, key: str) -> int: | ||
| """Return the actual host port for a hostfwd entry (resolves port 0 assignments).""" | ||
| if key in self._resolved_hostports: | ||
| return self._resolved_hostports[key] | ||
| if key in self.hostfwd: | ||
| return self.hostfwd[key].hostport | ||
| raise KeyError(f"hostfwd key {key!r} not found") | ||
|
Comment on lines
+571
to
+577
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] AI-generated, human reviewed |
||
|
|
||
| @export | ||
| @validate_call(validate_return=True) | ||
| def get_hostname(self) -> str: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,26 +59,31 @@ def get_native_arch_config(): | |
| def test_driver_qemu(tmp_path, ovmf): | ||
| arch, ovmf_arch = get_native_arch_config() | ||
|
|
||
| # Alpine uses OpenRC (not systemd), so systemd-ssh-generator does not run | ||
| # and sshd never binds to AF_VSOCK. Use a TCP hostfwd with hostport=0 so | ||
| # QEMU picks a free port automatically; the driver resolves the actual port | ||
| # from QMP after startup and updates the ssh child accordingly. | ||
| with serve( | ||
| Qemu( | ||
| arch=arch, | ||
| default_partitions={ | ||
| "OVMF_CODE.fd": ovmf / ovmf_arch / "code.fd", | ||
| "OVMF_VARS.fd": ovmf / ovmf_arch / "vars.fd", | ||
| }, | ||
| hostfwd={"ssh": {"protocol": "tcp", "hostaddr": "127.0.0.1", "hostport": 0, "guestport": 22}}, | ||
| ) | ||
| ) as qemu: | ||
| hostname = qemu.hostname | ||
| username = qemu.username | ||
| password = qemu.password | ||
|
|
||
| cached_image = Path(__file__).parent.parent / "images" / f"Fedora-Cloud-Base-Generic-43-1.6.{arch}.qcow2" | ||
| cached_image = Path(__file__).parent.parent / "images" / f"nocloud_alpine-3.22.4-{arch}-uefi-tiny-r0.qcow2" | ||
|
|
||
| if cached_image.exists(): | ||
| qemu.flasher.flash(cached_image.resolve()) | ||
| else: | ||
| qemu.flasher.flash( | ||
| f"https://download.fedoraproject.org/pub/fedora/linux/releases/43/Cloud/{arch}/images/Fedora-Cloud-Base-Generic-43-1.6.{arch}.qcow2", | ||
| f"https://dl-cdn.alpinelinux.org/alpine/v3.22/releases/cloud/nocloud_alpine-3.22.4-{arch}-uefi-tiny-r0.qcow2", | ||
| ) | ||
|
|
||
| qemu.power.on() | ||
|
|
@@ -88,16 +93,22 @@ def test_driver_qemu(tmp_path, ovmf): | |
|
|
||
| with qemu.console.pexpect() as p: | ||
| p.logfile = sys.stdout.buffer | ||
| p.expect_exact(f"{hostname} login:", timeout=600) | ||
| # Press Enter if GRUB is waiting. Both the countdown and bootstrap_complete | ||
| # can appear before the login prompt, so match whichever comes first. | ||
| idx = p.expect_exact(["automatically in ", "bootstrap_complete: done"], timeout=600) | ||
| if idx == 0: | ||
| # GRUB countdown: skip it, then wait for cloud-init to finish | ||
| p.sendline("") | ||
| p.expect_exact("bootstrap_complete: done", timeout=600) | ||
| # tiny-cloud finished: password is set, sshd is ready | ||
| p.expect_exact(f"{hostname} login:", timeout=60) | ||
| p.sendline(username) | ||
| p.expect_exact("Password:") | ||
| p.sendline(password) | ||
| p.expect_exact(f"[{username}@{hostname} ~]$") | ||
| p.sendline("sudo setenforce 0") | ||
| p.expect_exact(f"[{username}@{hostname} ~]$") | ||
| p.expect_exact(f"{hostname}:~$") | ||
|
|
||
| with qemu.shell() as s: | ||
| assert s.run("uname -r").stdout.strip() == f"6.17.1-300.fc43.{arch}" | ||
| assert s.run("uname -r").stdout.strip() != "" | ||
|
Comment on lines
110
to
+111
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also assert the return code, e.g., |
||
|
|
||
| qemu.power.off() | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the cache was actually slower than the download.