From 89047c6b5d61ead5961fe23bb2f049e74c979b40 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 20 Sep 2026 15:16:18 +0300 Subject: [PATCH] fix: correct the nocopy reason and measure the reservation refusals `nocopy` is not something podman cannot express: `-v vol:/data:nocopy` suppresses the copy-up (measured, 4.9.3), and compose2pod already emits that for a short-syntax entry. Only `--mount`, which the long form renders as, has no nocopy in its grammar, so the message now names the escape and the row moves to LIMITATIONS. The reservation refusals get the two table shapes their claims need: ABSENT_FLAGS for `cpus`, where podman run has no such flag, and STUB_FLAGS for `devices`, where `--gpus` exists, is hidden from --help, and accepts `nonsense` as readily as `all`. Closes #116 --- compose2pod/parsing.py | 17 +++-- compose2pod/resources.py | 13 +++- docs/adr/0006-docker-rejection-parity.md | 10 ++- tests/integration/refusals.py | 89 +++++++++++++++++++++-- tests/integration/test_podman_refusals.py | 41 ++++++++++- tests/test_parsing.py | 2 +- tests/test_resources.py | 4 +- 7 files changed, 155 insertions(+), 21 deletions(-) diff --git a/compose2pod/parsing.py b/compose2pod/parsing.py index 7519145..b7b7618 100644 --- a/compose2pod/parsing.py +++ b/compose2pod/parsing.py @@ -332,13 +332,20 @@ def _reject_subpath(name: str, vtype: str, added_in: str) -> None: def _validate_volume_type_options(name: str, options: dict[str, Any]) -> None: """Check a long-form volume entry's `volume:` sub-map (measured, v5.1.2). - Both keys are real Docker keys that the runtime will not honour: podman's - `--mount` cannot express `nocopy` at any version, and `subpath` on a named - volume arrives in podman 5.4. Each is refused with a "not supported" message - rather than folded into the generic unknown-key check the caller already ran. + Both keys are real Docker keys this entry cannot carry: `--mount`, which a + long-form volume renders as, has no `nocopy` in its grammar, and `subpath` on + a named volume arrives in podman 5.4. Each is refused with a "not supported" + message rather than folded into the generic unknown-key check the caller ran. + + `nocopy` is a limitation of the spelling, not of podman: `-v vol:/data:nocopy` + is honoured (measured, 4.9.3 -- it suppresses the copy-up), and compose2pod + emits exactly that for a short-syntax entry, so the message names the escape. """ if "nocopy" in options: - msg = f"service {name!r}: volume 'nocopy' is not supported (podman cannot express it)" + msg = ( + f"service {name!r}: volume 'nocopy' is not supported here: the long form mounts with " + "--mount, whose grammar has no nocopy (use the short syntax, which emits -v and podman honours)" + ) raise UnsupportedComposeError(msg) if "subpath" in options: _reject_subpath(name, "volume", "5.4") diff --git a/compose2pod/resources.py b/compose2pod/resources.py index 8558f73..3797cff 100644 --- a/compose2pod/resources.py +++ b/compose2pod/resources.py @@ -49,6 +49,15 @@ def _validate_limits(name: str, svc: dict[str, Any], limits: Any) -> None: # no raise UnsupportedComposeError(msg) +# Measured on podman 4.9.3: `--memory-reservation` is the only reservation flag +# podman run has, and `--gpus` exists but is hidden and accepts `nonsense` as +# readily as `all`, so emitting it would reserve nothing and say it had. +_RESERVATION_REFUSALS = { + "cpus": "podman run has no reservation flag for it", + "devices": "podman run's --gpus accepts any value and reserves nothing", +} + + def _validate_reservations(name: str, svc: dict[str, Any], reservations: Any) -> None: # noqa: ANN401 - Compose values are untyped # No `reservations is None` escape -- see `_validate_limits`. if not isinstance(reservations, dict): @@ -59,9 +68,9 @@ def _validate_reservations(name: str, svc: dict[str, Any], reservations: Any) -> if unknown: msg = f"service {name!r}: deploy.resources.reservations: unsupported keys {sorted(unknown)}" raise UnsupportedComposeError(msg) - for field in ("cpus", "devices"): + for field, reason in _RESERVATION_REFUSALS.items(): if field in reservations: - msg = f"service {name!r}: deploy.resources.reservations.{field} is not supported (no podman equivalent)" + msg = f"service {name!r}: deploy.resources.reservations.{field} is not supported ({reason})" raise UnsupportedComposeError(msg) if "memory" in reservations: _validate_memory_size(name, "deploy.resources.reservations.memory", reservations["memory"]) diff --git a/docs/adr/0006-docker-rejection-parity.md b/docs/adr/0006-docker-rejection-parity.md index 1ba698b..4c5638b 100644 --- a/docs/adr/0006-docker-rejection-parity.md +++ b/docs/adr/0006-docker-rejection-parity.md @@ -24,9 +24,13 @@ deferred to it. `tests/conformance/` runs both oracles for real over a probe mat from `SERVICE_KEYS | STRUCTURAL_KEYS | IGNORED_SERVICE_KEYS`, so a new key is probed the moment it is added, and `tests/integration/refusals.py` measures the other side, pairing each volume-family refusal above with the `--mount` that expresses what Docker says the document means, so a claim that -podman cannot express something cannot go stale unnoticed either. It carries both verdicts: -`REFUSALS` for the mounts podman will not make, `LIMITATIONS` for the ones it would, which is what -keeps a limitation from quietly reading as rule two. A gate that every rule-two site has a row is +podman cannot express something cannot go stale unnoticed either. It carries four verdicts, because +four kinds of claim need four experiments: `REFUSALS` for the mounts podman will not make, +`LIMITATIONS` for the ones it would (which is what keeps a limitation from quietly reading as rule +two), and, where the claim is about podman's flag surface rather than a mount, `ABSENT_FLAGS` for a +flag podman does not have and `STUB_FLAGS` for one it has that validates nothing -- a flag accepting +`nonsense` is a worse reason to emit it than a flag that fails, since the script would report +success for something it never did. A gate that every rule-two site has a row is [#109](https://github.com/modern-python/compose2pod/issues/109) phase 3, and it has to carry the exemptions first: not every refusal this document names turns out to be one podman makes. Verdicts are per version, and the supported range is stated rather than implied: the rulings here diff --git a/tests/integration/refusals.py b/tests/integration/refusals.py index e5e7a9b..635645d 100644 --- a/tests/integration/refusals.py +++ b/tests/integration/refusals.py @@ -18,10 +18,18 @@ rather than for podman's inability to mount, which would make a row pass for the wrong reason and hide an emit bug behind a parity claim. -Not every refusal site can have a row. `network_mode` is refused under ADR-0003, not -rule two, and podman honours it (#115); `nocopy` and `deploy.resources.reservations.*` -do not fit this shape at all (#116). The gate that every *rule-two* site has a row is -issue #109 phase 3, and it has to know about those exemptions before it can be written. +Two more tables hold the refusals whose claim is about podman's *flag surface* rather +than about a mount, which `Refusal` cannot express: its argv asserts that a flag fails, +and here the claim is that no flag exists to try, or that one exists and checks nothing. + +- `ABSENT_FLAGS` -- no such flag on `podman run`, checked by running it rather than by + reading `--help`, because `--gpus` proves a flag can exist while staying out of it. +- `STUB_FLAGS` -- the flag exists and accepts deliberate nonsense, so emitting it would + exit 0 having done nothing, which is worse than refusing. + +Four claims, four experiments. `network_mode` alone has no row: it is refused under +ADR-0003, not rule two, and podman honours it (#115). The gate that every rule-two site +has a row is issue #109 phase 3, and that is the exemption it has to know about. A `subpath` row measures the floor, not podman as such: podman gained the option above the supported minimum (ADR-0006), so the row goes red on a runner newer than the floor, @@ -53,14 +61,43 @@ class Limitation: `host_dir` is created under the test's `tmp_path` first, because a bind whose source does not exist fails for that reason instead of the one being measured. - `{host}` in `podman_argv` is substituted with its absolute path. + `{host}` in `podman_argv` is substituted with its absolute path. A row whose + counterfactual needs no host path leaves it empty. """ id: str compose: dict[str, Any] refusal_match: str - host_dir: str podman_argv: list[str] + host_dir: str = "" + + +@dataclass(frozen=True) +class AbsentFlag: + """A refusal whose claim is that podman has no flag to emit for the key. + + Each `unknown_argv` must be rejected by `podman run`. The row goes red the day + podman grows one, which is the only thing keeping "no equivalent" from going stale. + """ + + id: str + compose: dict[str, Any] + refusal_match: str + unknown_argv: list[list[str]] + + +@dataclass(frozen=True) +class StubFlag: + """A refusal whose claim is that podman's flag exists and validates nothing. + + `nonsense_argv` is deliberate rubbish podman accepts anyway; the row goes red when + a podman starts rejecting it, which is when the refusal deserves re-examining. + """ + + id: str + compose: dict[str, Any] + refusal_match: str + nonsense_argv: list[str] def _one_volume(entry: "str | dict[str, Any]") -> dict[str, Any]: @@ -156,7 +193,27 @@ def _one_volume(entry: "str | dict[str, Any]") -> dict[str, Any]: ] +def _reservation(field: str, value: object) -> dict[str, Any]: + return {"services": {"app": {"image": "busybox:1.36", "deploy": {"resources": {"reservations": {field: value}}}}}} + + LIMITATIONS: list[Limitation] = [ + Limitation( + # Measured: `-v vol:/etc:nocopy` leaves only podman's own hosts/hostname/resolv.conf + # in the volume, so the image copy-up really is suppressed. + id="volume-nocopy", + compose={ + "services": { + "app": { + "image": "busybox:1.36", + "volumes": [{"type": "volume", "source": "ncvol", "target": "/data", "volume": {"nocopy": True}}], + } + }, + "volumes": {"ncvol": {}}, + }, + refusal_match="use the short syntax, which emits -v", + podman_argv=["-v", "c2p-nocopy-probe:/data:nocopy"], + ), Limitation( id="drive-qualified-bind", compose=_one_volume("C:\\data:/var"), @@ -172,3 +229,23 @@ def _one_volume(entry: "str | dict[str, Any]") -> dict[str, Any]: podman_argv=["--mount", "type=bind,src={host},dst=/var"], ), ] + + +ABSENT_FLAGS: list[AbsentFlag] = [ + AbsentFlag( + id="reservations-cpus", + compose=_reservation("cpus", "0.5"), + refusal_match="podman run has no reservation flag for it", + unknown_argv=[["--cpu-reservation", "1"], ["--cpus-reservation", "1"]], + ), +] + + +STUB_FLAGS: list[StubFlag] = [ + StubFlag( + id="reservations-devices", + compose=_reservation("devices", [{"capabilities": ["gpu"]}]), + refusal_match="--gpus accepts any value and reserves nothing", + nonsense_argv=["--gpus", "nonsense"], + ), +] diff --git a/tests/integration/test_podman_refusals.py b/tests/integration/test_podman_refusals.py index 615a8cb..149f5e0 100644 --- a/tests/integration/test_podman_refusals.py +++ b/tests/integration/test_podman_refusals.py @@ -14,7 +14,16 @@ from compose2pod.exceptions import UnsupportedComposeError from compose2pod.parsing import validate -from tests.integration.refusals import LIMITATIONS, REFUSALS, Limitation, Refusal +from tests.integration.refusals import ( + ABSENT_FLAGS, + LIMITATIONS, + REFUSALS, + STUB_FLAGS, + AbsentFlag, + Limitation, + Refusal, + StubFlag, +) def _resolve(argv: list[str], host: Path) -> list[str]: @@ -46,9 +55,37 @@ def test_a_tracked_limitation_names_a_mount_podman_would_have_made( validate(limitation.compose) source = tmp_path / limitation.host_dir - source.mkdir(parents=True) + source.mkdir(parents=True, exist_ok=True) argv = _resolve(limitation.podman_argv, source) assert probe_podman(f"{limitation.id} [limitation]", argv) == 0, ( "podman refuses this mount too, so it is rule two and belongs in REFUSALS" ) + + +@pytest.mark.parametrize("absent", ABSENT_FLAGS, ids=lambda absent: absent.id) +def test_a_refusal_citing_no_flag_names_flags_podman_does_not_have( + absent: AbsentFlag, probe_podman: Callable[[str, list[str]], int] +) -> None: + with pytest.raises(UnsupportedComposeError, match=absent.refusal_match): + validate(absent.compose) + + assert probe_podman(f"{absent.id} [control]", []) == 0, ( + "the bare control run failed, so every argv below would look absent whatever podman has" + ) + for argv in absent.unknown_argv: + assert probe_podman(f"{absent.id} {argv[0]}", argv) != 0, ( + "podman took this flag, so it has an equivalent and the refusal needs re-examining" + ) + + +@pytest.mark.parametrize("stub", STUB_FLAGS, ids=lambda stub: stub.id) +def test_a_refusal_citing_a_stub_flag_names_one_podman_does_not_validate( + stub: StubFlag, probe_podman: Callable[[str, list[str]], int] +) -> None: + with pytest.raises(UnsupportedComposeError, match=stub.refusal_match): + validate(stub.compose) + + assert probe_podman(stub.id, stub.nonsense_argv) == 0, ( + "podman rejected deliberate nonsense, so the flag validates something after all" + ) diff --git a/tests/test_parsing.py b/tests/test_parsing.py index e181a47..4d63047 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -220,7 +220,7 @@ def test_nested_option_rejects(self) -> None: ), ( {"type": "volume", "source": "v", "target": "/d", "volume": {"nocopy": True}}, - r"'nocopy' is not supported", + r"use the short syntax, which emits -v", {"v": {}}, ), # per-option schema diff --git a/tests/test_resources.py b/tests/test_resources.py index ca57a3d..c84b77f 100644 --- a/tests/test_resources.py +++ b/tests/test_resources.py @@ -63,11 +63,11 @@ def test_unsupported_limit_key_rejected(self) -> None: validate_deploy("app", _svc({"resources": {"limits": {"gpus": 1}}})) def test_reservations_cpus_rejected(self) -> None: - with pytest.raises(UnsupportedComposeError, match=r"reservations.cpus is not supported"): + with pytest.raises(UnsupportedComposeError, match=r"podman run has no reservation flag for it"): validate_deploy("app", _svc({"resources": {"reservations": {"cpus": "0.5"}}})) def test_reservations_devices_rejected(self) -> None: - with pytest.raises(UnsupportedComposeError, match=r"reservations.devices is not supported"): + with pytest.raises(UnsupportedComposeError, match=r"--gpus accepts any value and reserves nothing"): validate_deploy("app", _svc({"resources": {"reservations": {"devices": []}}})) def test_limit_value_bool_rejected(self) -> None: