issue_71: Let the container engine be named explicitly - #72
Conversation
Detection always picks podman when it is present, with no way to say otherwise. That broke a consuming project's main build today: on GitHub's ubuntu-24.04 runner `podman run` died with "crun: unknown version specified" while docker on the same runner was demonstrably healthy -- the backend job runs Testcontainers against it. The detection could not have caught this. `podman info` reports that the engine is configured; it says nothing about whether the OCI runtime can start a container, and that is precisely the gap the failure lives in. The probe was green about something that did not work. A faithful probe would start a throwaway container with the real runtime flags. Not done here: it costs an image pull before every run, and two on a host where podman is broken -- a regression in the common case to fix an uncommon one. Naming the engine is cheap, explicit, and puts the choice where a reader of the CI workflow sees it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dieterbaier
left a comment
dieterbaier
left a comment
There was a problem hiding this comment.
Es gibt einen blockierenden Randfall im neuen Override:
find_engine übernimmt jeden nichtleeren Wert aus DOCS_TOOLBOX_ENGINE. run_in_container behandelt aber nur podman und docker und besitzt keinen *)-Zweig. Bei beispielsweise
DOCS_TOOLBOX_ENGINE=dockre ./build.sh validateläuft der case ohne Aktion durch und das Skript endet mit Exit 0. CI wäre damit grün, obwohl weder Validator noch Container gestartet wurden.
Das sollte fail-fast gelöst werden, entweder indem find_engine ausschließlich podman|docker akzeptiert oder indem run_in_container für unbekannte Werte eine klare Fehlermeldung ausgibt und mit Exit 2 abbricht. Dazu sollte ein Negativtest kommen, der für einen unbekannten Engine-Namen einen Non-zero-Exit und keine Container-Inkaufnahme erwartet.
Der eigentliche Fix für #71 ist ansonsten stimmig:
- Der explizite Override gewinnt korrekt vor der Podman-Erkennung.
- Root-Skript und Consumer-Template sind synchron angepasst.
- Die Entscheidung gegen einen zusätzlichen Probe-Container ist nachvollziehbar und vermeidet unnötige Pulls.
- Der positive Override-Fall ist reproduzierbar mit Stubs getestet.
- Der GitHub-Workflow
Validateist für den aktuellen Head grün.
Wegen des stillen erfolgreichen No-op würde ich PR #72 in diesem Stand noch nicht mergen. Eine formale Änderungsanforderung lässt GitHub beim eigenen PR nicht zu; daher markiere ich den Punkt hier ausdrücklich als blockierend.
1 participant
Closes #71.
find_enginealways picked podman when present, with no way to override. Aconsuming project's
mainbuild broke on it today:podman rundied withcrun: unknown version specifiedon GitHub'subuntu-24.04runner, reproducedon a re-run, while docker on that same runner was demonstrably healthy — the
repository's backend job runs Testcontainers against it.
DOCS_TOOLBOX_ENGINEnow short-circuits detection, in bothbuild.shandtemplates/scripts/build.sh.Why not a better probe
The detection could not have caught this, and it is worth saying why in the code
rather than only here:
podman inforeports that the engine is configured. Itsays nothing about whether the OCI runtime can start a container. The probe was
green about something that did not work.
A faithful probe would start a throwaway container with the same runtime flags
the real invocation uses. That is not in this PR: it costs an image pull before
every run, and two pulls on a host where podman is broken — a regression in the
common case to fix an uncommon one. If this turns out to be widespread rather
than a runner quirk, it is worth revisiting, and #71 records that.
Tests
Two new cases in
test/build-sh-template.test.mjs, using stubpodman/dockerexecutables on
PATHso nothing is pulled:DOCS_TOOLBOX_ENGINEwins even though podman is present and itsinfoprobesucceeds — which is the case that matters, since that is exactly the state a
broken OCI runtime leaves the host in
The override test was confirmed to fail against
main(not ok 6)../build.sh testand./build.sh check-adaptersboth exit 0.🤖 Generated with Claude Code