diff --git a/python/packages/jumpstarter-driver-cuttlefish/README.md b/python/packages/jumpstarter-driver-cuttlefish/README.md index 5f4050108..e71ed87d4 100644 --- a/python/packages/jumpstarter-driver-cuttlefish/README.md +++ b/python/packages/jumpstarter-driver-cuttlefish/README.md @@ -112,6 +112,37 @@ Or restart the container - ephemeral `/var/tmp/cvd` means a restart is equivalent to a full reset. Fetched images in the `cvd-images` volume are preserved. +Prefer `cvd reset -y --clean-runtime-dir` over the manual `rm -rf` above when +you have shell access - it's the supported cleanup path (stops instances, +resets lock files, clears `/var/tmp/cvd//`). + +**Operational hazard: `cvd reset` must run as the `httpcvd` user, not root.** +`cvd` tracks running instance groups in a per-user instance database, and HO +launches CVDs as `httpcvd`. Running the command as root (e.g. plain +`podman exec`) reports "Found 0 untracked running instance groups" and +silently does nothing, even with a CVD actively running - this is dangerous +precisely because it *looks* like a clean result rather than a permissions +failure: + +```bash +podman exec -u httpcvd cuttlefish-orchestrator cvd reset -y --clean-runtime-dir +``` + +The driver's `reset_host()` (`j cuttlefish reset`) doesn't hit this problem - +it calls HO's own `POST /reset`, which HO executes as itself (`httpcvd`). + +**Operational hazard: `auto_reset` is host-wide, not per-CVD.** Setting +`auto_reset: true` makes `power.on()` call `reset_host()` automatically and +retry once when it detects stale state (orphaned CVDs that won't delete, or +creation failing with "in use"/"already running"). Off by default because +`POST /reset` stops every CVD on the host, so it is only safe when this +exporter is the sole tenant of the host orchestrator - a 1:1 CVD-to-exporter +mapping does **not** imply that, since several exporters can still share one +HO on a packed host, and `auto_reset` would silently kill their CVDs too. An +ADB port mismatch after creation is excluded from `auto_reset` for the same +reason: the occupied slot may be a different tenant's legitimately running +CVD, so that case always raises for manual investigation instead of retrying. + ### Teardown Delete CVDs and snapshots when done to avoid accumulation: @@ -169,6 +200,7 @@ export: | instance_num | CVD instance number (determines ADB/netsim/HCI ports). Must match HO's assigned slot. Pinning avoids drift (see `env_config` example). | int | no | 1 | | adb_server_port | ADB server port on the exporter | int | no | 15037 | | boot_timeout | Seconds to wait for boot on power on| int | no | 300 | +| auto_reset | On stale HO state (orphaned CVDs, port drift), call the HO-wide reset and retry `power.on()` once. See "Resetting stale state" below - only safe when this exporter is the sole tenant of the host. | bool | no | false | | env_config | Default env_config for CVD creation | dict | no | {} | This is a **composite driver** with three children: diff --git a/python/packages/jumpstarter-driver-cuttlefish/jumpstarter_driver_cuttlefish/driver.py b/python/packages/jumpstarter-driver-cuttlefish/jumpstarter_driver_cuttlefish/driver.py index 42ec00deb..02cd87138 100644 --- a/python/packages/jumpstarter-driver-cuttlefish/jumpstarter_driver_cuttlefish/driver.py +++ b/python/packages/jumpstarter-driver-cuttlefish/jumpstarter_driver_cuttlefish/driver.py @@ -20,6 +20,10 @@ class CuttlefishTimeout(CuttlefishError): """Raised when an operation doesn't complete in time.""" +class _StaleHostState(CuttlefishError): + """Internal signal: HO state looks stale/inconsistent, eligible for one auto_reset retry.""" + + @dataclass(kw_only=True) class Cuttlefish(Driver): """Cuttlefish Host Orchestrator driver for managing Android virtual devices. @@ -37,6 +41,13 @@ class Cuttlefish(Driver): instance_num: int = 1 adb_server_port: int = 15037 boot_timeout: int = 300 + # Auto-recover from stale HO state (orphaned CVDs, port drift) by calling + # reset_host() and retrying power.on() once. Off by default: HO's /reset is + # host-wide and stops every CVD on the host, so it is only safe when this + # exporter is the sole tenant of the host orchestrator. A 1:1 CVD-to-exporter + # mapping does NOT imply that - several exporters can still share one HO on + # a packed host, and enabling this would silently kill their CVDs too. + auto_reset: bool = False env_config: dict = field(default_factory=dict) webrtc_url: str = "" _cvd_group: str | None = field(default=None, init=False, repr=False) @@ -348,6 +359,13 @@ class CvdPower(VirtualPowerInterface, Driver): on() creates a CVD if none exists, or starts an existing one. If multiple CVDs exist in the configured group, all are deleted before creating a fresh one (assumes single-tenant host orchestrator). + If HO state looks stale (orphaned CVDs won't delete, creation fails with + "in use"/"already running") and `auto_reset` is enabled, on() calls + reset_host() once and retries - see `auto_reset` for the host-wide caveat. + An ADB port mismatch after creation is deliberately NOT auto-reset-eligible: + the occupied slot may belong to a different tenant's legitimately running + CVD on a shared host, and reset_host() would destroy it too - that case + always raises for manual investigation instead of retrying. off() stops the CVD; off(destroy=True) deletes it entirely. """ @@ -358,7 +376,28 @@ def client(cls) -> str: return "jumpstarter_driver_cuttlefish.client.CvdPowerClient" @export - def on(self) -> None: # noqa: C901 + def on(self) -> None: + auto_reset_attempted = False + while True: + try: + self._create_or_start() + break + except _StaleHostState as e: + if not self.parent.auto_reset or auto_reset_attempted: + raise + auto_reset_attempted = True + self.logger.warning( + "auto_reset: %s -- resetting Host Orchestrator and retrying once. " + "This is HOST-WIDE: it stops every CVD on this host, not just this driver's.", + e, + ) + self.parent.reset_host() + + self.parent._auto_connect_adb() + if self.parent.boot_timeout: + self.parent._wait_boot(self.parent.boot_timeout) + + def _create_or_start(self) -> None: # noqa: C901 existing = self.parent._get_existing_cvds() if len(existing) > 1: @@ -375,7 +414,7 @@ def on(self) -> None: # noqa: C901 self.logger.warning("Failed to delete stale CVD %s/%s", group, name) failed.append(f"{group}/{name}") if failed: - raise CuttlefishError( + raise _StaleHostState( f"cannot create CVD - failed to delete stale CVDs: {', '.join(failed)}. " f"Run 'j cuttlefish reset' then retry." ) @@ -402,7 +441,7 @@ def on(self) -> None: # noqa: C901 except CuttlefishError as e: msg = str(e) if "in use" in msg or "already running" in msg or "ValidateTapDevices" in msg: - raise CuttlefishError( + raise _StaleHostState( f"CVD creation failed - orphaned processes from a previous session. " f"Run 'j cuttlefish reset' then retry. Original error: {msg}" ) from e @@ -419,6 +458,9 @@ def on(self) -> None: # noqa: C901 self.logger.warning("Failed to clean up CVD after port mismatch") self.parent._cvd_group = None self.parent._cvd_name = None + # Not auto_reset-eligible: the slot may be occupied by a different + # tenant's legitimately running CVD on a shared host, and reset_host() + # would destroy it too. Requires manual investigation. raise CuttlefishError( f"HO assigned adb_port {actual_port} but expected " f"{self.parent._expected_adb_port} — stale state may have leaked. " diff --git a/python/packages/jumpstarter-driver-cuttlefish/jumpstarter_driver_cuttlefish/driver_test.py b/python/packages/jumpstarter-driver-cuttlefish/jumpstarter_driver_cuttlefish/driver_test.py index 50207d0dd..d07d37968 100644 --- a/python/packages/jumpstarter-driver-cuttlefish/jumpstarter_driver_cuttlefish/driver_test.py +++ b/python/packages/jumpstarter-driver-cuttlefish/jumpstarter_driver_cuttlefish/driver_test.py @@ -464,6 +464,52 @@ def test_cvd_power_on_stale_cleanup_failure(requests_mock, drv): power.on() +def test_cvd_power_on_auto_reset_retries_once(requests_mock, drv): + """auto_reset calls reset_host() and retries after a stale-state failure, then succeeds.""" + drv.children["adb"] = MagicMock() + drv.boot_timeout = 0 + drv.auto_reset = True + power = drv.children["power"] + requests_mock.get( + f"{BASE}/cvds", + [ + {"json": {"cvds": [{"name": "d1", "group": "cvd_1"}, {"name": "d2", "group": "cvd_1"}]}}, + {"json": {"cvds": []}}, + ], + ) + requests_mock.delete(f"{BASE}/cvds/cvd_1/d1", status_code=500, json={"error": "busy"}) + requests_mock.delete(f"{BASE}/cvds/cvd_1/d2", json={"name": "op-d2", "done": False}) + requests_mock.post(f"{BASE}/operations/op-d2/:wait", json={"name": "op-d2", "done": True}) + requests_mock.post(f"{BASE}/reset", json={"name": "op-reset", "done": False}) + requests_mock.post(f"{BASE}/operations/op-reset/:wait", json={"name": "op-reset", "done": True}) + requests_mock.post(f"{BASE}/cvds", json={"name": "op-c", "done": False}) + requests_mock.post( + f"{BASE}/operations/op-c/:wait", + json={"name": "op-c", "done": True, "cvds": [{"group": "cvd_1", "name": "dev1", "adb_port": 6520}]}, + ) + power.on() + assert any(r.method == "POST" and r.path == "/reset" for r in requests_mock.request_history) + assert drv._cvd_group == "cvd_1" + assert drv._cvd_name == "dev1" + + +def test_cvd_power_on_auto_reset_disabled_raises_immediately(requests_mock, drv): + """Without auto_reset, a stale-state failure raises without touching /reset.""" + drv.children["adb"] = MagicMock() + drv.boot_timeout = 0 + power = drv.children["power"] + requests_mock.get( + f"{BASE}/cvds", + json={"cvds": [{"name": "d1", "group": "cvd_1"}, {"name": "d2", "group": "cvd_1"}]}, + ) + requests_mock.delete(f"{BASE}/cvds/cvd_1/d1", status_code=500, json={"error": "busy"}) + requests_mock.delete(f"{BASE}/cvds/cvd_1/d2", json={"name": "op-d2", "done": False}) + requests_mock.post(f"{BASE}/operations/op-d2/:wait", json={"name": "op-d2", "done": True}) + with pytest.raises(CuttlefishError, match="failed to delete stale CVDs"): + power.on() + assert not any(r.method == "POST" and r.path == "/reset" for r in requests_mock.request_history) + + def test_cvd_power_on_port_mismatch(requests_mock, drv): drv.children["adb"] = MagicMock() drv.boot_timeout = 0