docs(deployments): trim restate-y comments per review

Address review feedback on the openshell backend: reformat the readiness
fail-closed rationale as a bulleted exit-code table, and cut the
_exec_detached docstring's narration down to the non-obvious facts (the
None exit-code semantics and the timeout rationale) instead of restating
what the code does.

Also satisfy ty in the openshell readiness tests: construct Probe via
pydantic aliases (httpGet/tcpSocket), since ty synthesizes __init__ from
aliases and does not honor populate_by_name, and route
_readiness_probe_command unpacks through a helper that narrows its
tuple|None return. Test-only; no behavior change.

Rename test_openshell_backend_mocked.py -> test_backend.py to match the
k8s unit backend test; the unit/ vs integration/ tree already conveys
mocked vs live, so the _mocked suffix was redundant.

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>

Author: maxdubrinsky

plugins/nemo-deployments/src/nemo_deployments_plugin/backends/openshell/backend.py

@@ -660,10 +660,11 @@ async def _try_get_sandbox(self, sandbox_nm: str) -> Any | None:
     async def _exec_detached(
         self, sandbox_id: str, command: list[str], *, timeout: int | None = None
     ) -> tuple[int | None, str]:
-        """Run a command, draining its event stream. Returns (exit_code, combined output).
+        """Run *command* to completion, returning (exit_code, stdout+stderr merged);
+        ``exit_code`` is None when the stream carried no exit event.
 
-        *timeout* bounds both the RPC and the sandbox-side command; it defaults to the
-        executor's control-plane ``request_timeout_seconds``. Readiness probes pass a much
+        *timeout* bounds both the RPC and the sandbox-side command, defaulting to the
+        executor's control-plane ``request_timeout_seconds``; readiness probes pass a much
         shorter bound so a hung probe cannot stall the serial reconcile loop.
         """
         timeout = timeout if timeout is not None else self._executor_config.request_timeout_seconds
@@ -698,14 +699,14 @@ async def _readiness_pending(self, sandbox_id: str, container: Container) -> Bac
             return None
         command, description, exec_timeout = probe_command
         exit_code, _ = await self._exec_detached(sandbox_id, command, timeout=exec_timeout)
-        # Readiness fails closed, unlike liveness. Liveness fails open on an undecidable
-        # probe so a flaky RPC never demotes a healthy deployment, but readiness gates
-        # admission the other way: only a probe that positively proves reachability
-        # (exit 0) may expose the port and flip to sticky READY. A rare no-exit-event
-        # (None) stays STARTING and self-heals -- the port is not exposed while pending,
-        # so the next poll re-probes; a workload that can never be probed never claims
-        # READY, which is the contract this gate exists to keep. Timeouts and RPC errors
-        # surface as RpcError -> UNKNOWN upstream, not as a None exit here.
+        # Readiness fails closed (liveness fails open): the gate admits only positive
+        # proof of reachability, so a flaky probe never exposes an unready workload.
+        #   - exit 0        -> reachable; expose the port and read READY
+        #   - nonzero       -> not reachable yet; stay STARTING
+        #   - no exit event -> undecidable; stay STARTING and re-probe next poll (the port
+        #                      is not exposed while pending, so this self-heals)
+        # A workload that can never be probed never claims READY -- the contract this gate
+        # keeps. Timeouts and RPC errors surface as UNKNOWN upstream, not as a None exit.
         if exit_code == 0:
             return None
         return BackendStatusUpdate(status="STARTING", status_message=f"Awaiting readiness: {description}")

…enshell/test_openshell_backend_mocked.py …/unit/backends/openshell/test_backend.pyplugins/nemo-deployments/tests/unit/backends/openshell/test_openshell_backend_mocked.py renamed to plugins/nemo-deployments/tests/unit/backends/openshell/test_backend.py plugins/nemo-deployments/tests/unit/backends/openshell/test_openshell_backend_mocked.py renamed to plugins/nemo-deployments/tests/unit/backends/openshell/test_backend.py

@@ -507,7 +507,7 @@ async def test_read_status_gates_on_declared_httpget_probe(
 ) -> None:
     # A declared httpGet readinessProbe is honoured against loopback; a failing probe
     # keeps the deployment STARTING and unexposed, and the probe hits the declared path.
-    mock_entities.get.return_value = _config_with_readiness(Probe(http_get=HTTPGetAction(path="/health", port=8000)))
+    mock_entities.get.return_value = _config_with_readiness(Probe(httpGet=HTTPGetAction(path="/health", port=8000)))
     mock_stub.GetSandbox.return_value = _sandbox(pb.SANDBOX_PHASE_READY)
     mock_stub.ExecSandbox.side_effect = [
         _exec_events(0),  # marker present
@@ -677,8 +677,15 @@ async def test_read_status_cleanup_swallows_delete_error(
     mock_stub.DeleteSandbox.assert_called_once()
 
 
+def _require_probe_command(container: Container) -> tuple[list[str], str, int]:
+    """Return the container's readiness probe command, asserting it exists (narrows None)."""
+    probe_command = _readiness_probe_command(container)
+    assert probe_command is not None
+    return probe_command
+
+
 def test_readiness_probe_command_defaults_to_tcp_on_the_first_port() -> None:
-    command, description, timeout = _readiness_probe_command(_config().containers[0])
+    command, description, timeout = _require_probe_command(_config().containers[0])
     assert command[:2] == ["/bin/sh", "-c"]
     assert "127.0.0.1" in command[2]
     assert description == "tcp 127.0.0.1:8000"
@@ -687,16 +694,16 @@ def test_readiness_probe_command_defaults_to_tcp_on_the_first_port() -> None:
 
 
 def test_readiness_probe_command_uses_declared_httpget() -> None:
-    container = _config_with_readiness(Probe(http_get=HTTPGetAction(path="/ready", port=8000))).containers[0]
-    command, description, _timeout = _readiness_probe_command(container)
+    container = _config_with_readiness(Probe(httpGet=HTTPGetAction(path="/ready", port=8000))).containers[0]
+    command, description, _timeout = _require_probe_command(container)
     assert "http://127.0.0.1:8000/ready" in command[2]
     assert description == "httpGet http://127.0.0.1:8000/ready"
 
 
 def test_readiness_probe_command_runs_declared_exec_directly() -> None:
     probe = Probe(exec=ExecAction(command=["/bin/true"]), timeoutSeconds=4)
     container = _config_with_readiness(probe).containers[0]
-    command, description, timeout = _readiness_probe_command(container)
+    command, description, timeout = _require_probe_command(container)
     assert command == ["/bin/true"]
     assert description == "exec readiness probe"
     # An exec probe is bounded by its own timeoutSeconds, not the control-plane deadline,
@@ -705,8 +712,8 @@ def test_readiness_probe_command_runs_declared_exec_directly() -> None:
 
 
 def test_readiness_probe_command_resolves_named_tcp_socket_port() -> None:
-    container = _config_with_readiness(Probe(tcp_socket=TCPSocketAction(port="http"))).containers[0]
-    _command, description, _timeout = _readiness_probe_command(container)
+    container = _config_with_readiness(Probe(tcpSocket=TCPSocketAction(port="http"))).containers[0]
+    _command, description, _timeout = _require_probe_command(container)
     assert description == "tcp 127.0.0.1:8000"
 
 
@@ -716,17 +723,17 @@ def test_readiness_probe_command_is_none_without_probe_or_ports() -> None:
 
 def test_readiness_probe_command_https_uses_unverified_context() -> None:
     container = _config_with_readiness(
-        Probe(http_get=HTTPGetAction(path="/health", port=8000, scheme="HTTPS"))
+        Probe(httpGet=HTTPGetAction(path="/health", port=8000, scheme="HTTPS"))
     ).containers[0]
-    command, _description, _timeout = _readiness_probe_command(container)
+    command, _description, _timeout = _require_probe_command(container)
     assert "https://127.0.0.1:8000/health" in command[2]
     # An https probe against a loopback/self-signed cert must not fail verification.
     assert "_create_unverified_context" in command[2]
 
 
 def test_readiness_probe_command_normalizes_httpget_path_without_leading_slash() -> None:
-    container = _config_with_readiness(Probe(http_get=HTTPGetAction(path="ready", port=8000))).containers[0]
-    _command, description, _timeout = _readiness_probe_command(container)
+    container = _config_with_readiness(Probe(httpGet=HTTPGetAction(path="ready", port=8000))).containers[0]
+    _command, description, _timeout = _require_probe_command(container)
     assert description == "httpGet http://127.0.0.1:8000/ready"
 
 
@@ -743,10 +750,10 @@ def test_readiness_probe_command_skips_udp_only_ports() -> None:
 def test_readiness_probe_command_falls_back_when_declared_port_unresolvable() -> None:
     # A declared probe naming a port absent from the container falls back to the first
     # TCP port rather than skipping the gate (which would re-open the bind race).
-    container = _config_with_readiness(Probe(http_get=HTTPGetAction(path="/health", port="does-not-exist"))).containers[
+    container = _config_with_readiness(Probe(httpGet=HTTPGetAction(path="/health", port="does-not-exist"))).containers[
         0
     ]
-    _command, description, _timeout = _readiness_probe_command(container)
+    _command, description, _timeout = _require_probe_command(container)
     assert description == "httpGet http://127.0.0.1:8000/health"