[PATCH 1/4] tests/docker: improve handling of docker probes

Daniel P. Berrangé posted 4 patches 1 month ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Thomas Huth <thuth@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH 1/4] tests/docker: improve handling of docker probes
Posted by Daniel P. Berrangé 1 month ago
The docker.py script has logic to guess the container command and
detects one of

  * docker
  * sudo -n docker
  * podman

but the "docker.py probe" command then throws away the detected argv
and prints a slightly different argv based soley on the detected
argv[0]. The result is that 'probe' will print

  * docker
  * sudo docker
  * podman

which means that if sudo was detected & the result of 'probe' were
used direclty, it would end up prompting for password interaction
every time.

The 'configure' script, however, runs 'probe' and then throws away
the printed argv again, reporting only 'podman' or 'docker', which
is used to set the $(RUNC) variable for tests/docker/Makefile.include
which is in turn used to pass --engine to docker.py. So the docker.py
command will re-detect the need for 'sudo -n' and use it correctly

The problem with this is that some commands in Makefile.include do
not call docker.py at all, they invoke $(RUNC) directly. Since
configure threw away the 'sudo' command prefix Makefile.in won't
be adding either 'sudo' or 'sudo -n', it'll just run plani 'docker'
which is wrong.

This commit sanitizes things so that the 'docker.py probe' prints
out the exact detected ARGV, and configure fully preserves this
ARGV when setting $(RUNC). Since "$(RUNC)" is no longer just a bare
engine name, however, we must now also set the $(CONTAINER_ENGINE)
variable for Makefile.include so it can pass something sane to
the --engine arg for docker.py

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 configure                     | 19 +++++++------------
 tests/docker/Makefile.include |  5 +++--
 tests/docker/docker.py        |  7 +------
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/configure b/configure
index 55e0bd3425..1adbb357df 100755
--- a/configure
+++ b/configure
@@ -1316,17 +1316,11 @@ fi
 ##########################################
 # functions to probe cross compilers
 
-container="no"
-runc=""
+runc="no"
 if test $use_containers = "yes" && (has "docker" || has "podman"); then
-    case $($python "$source_path"/tests/docker/docker.py --engine "$container_engine" probe) in
-        *docker) container=docker ;;
-        podman) container=podman ;;
-        no) container=no ;;
-    esac
-    if test "$container" != "no"; then
-        docker_py="$python $source_path/tests/docker/docker.py --engine $container"
-        runc=$container
+    runc=$($python "$source_path"/tests/docker/docker.py --engine "$container_engine" probe)
+    if test "$runc" != "no"; then
+        docker_py="$python $source_path/tests/docker/docker.py --engine $container_engine"
     fi
 fi
 
@@ -1446,7 +1440,7 @@ probe_target_compiler() {
   esac
 
   for host in $container_hosts; do
-    test "$container" != no || continue
+    test "$runc" != no || continue
     test "$host" = "$cpu" || continue
     case $target_arch in
       # debian-all-test-cross architectures
@@ -1775,8 +1769,9 @@ echo all: >> $config_host_mak
 echo "SRC_PATH=$source_path" >> $config_host_mak
 echo "TARGET_DIRS=$target_list" >> $config_host_mak
 echo "GDB=$gdb_bin" >> $config_host_mak
-if test "$container" != no; then
+if test "$runc" != no; then
     echo "RUNC=$runc" >> $config_host_mak
+    echo "CONTAINER_ENGINE=$container_engine" >> $config_host_mak
 fi
 echo "SUBDIRS=$subdirs" >> $config_host_mak
 if test "$rust" != disabled; then
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 38467cca61..7d4582b6a8 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -16,8 +16,9 @@ DOCKER_DEFAULT_REGISTRY := registry.gitlab.com/qemu-project/qemu
 endif
 DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),$(DOCKER_DEFAULT_REGISTRY))
 
-RUNC ?= $(if $(shell command -v docker), docker, podman)
-DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py --engine $(RUNC)
+CONTAINER_ENGINE = auto
+DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py --engine $(CONTAINER_ENGINE)
+RUNC ?= $(shell $(DOCKER_SCRIPT) probe)
 
 CUR_TIME := $(shell date +%Y-%m-%d-%H.%M.%S.$$$$)
 DOCKER_SRC_COPY := $(BUILD_DIR)/docker-src.$(CUR_TIME)
diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 3b8a26704d..ff68c7bf6f 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -618,12 +618,7 @@ class ProbeCommand(SubCommand):
     def run(self, args, argv):
         try:
             docker = Docker()
-            if docker._command[0] == "docker":
-                print("docker")
-            elif docker._command[0] == "sudo":
-                print("sudo docker")
-            elif docker._command[0] == "podman":
-                print("podman")
+            print(" ".join(docker._command))
         except Exception:
             print("no")
 
-- 
2.52.0


Re: [PATCH 1/4] tests/docker: improve handling of docker probes
Posted by Thomas Huth 2 days, 7 hours ago
On 07/01/2026 14.01, Daniel P. Berrangé wrote:
> The docker.py script has logic to guess the container command and
> detects one of
> 
>    * docker
>    * sudo -n docker
>    * podman
> 
> but the "docker.py probe" command then throws away the detected argv
> and prints a slightly different argv based soley on the detected

s/soley/solely/ ?

> argv[0]. The result is that 'probe' will print
> 
>    * docker
>    * sudo docker
>    * podman
> 
> which means that if sudo was detected & the result of 'probe' were
> used direclty, it would end up prompting for password interaction

s/direclty/directly/

> every time.
> 
> The 'configure' script, however, runs 'probe' and then throws away
> the printed argv again, reporting only 'podman' or 'docker', which
> is used to set the $(RUNC) variable for tests/docker/Makefile.include
> which is in turn used to pass --engine to docker.py. So the docker.py
> command will re-detect the need for 'sudo -n' and use it correctly
> 
> The problem with this is that some commands in Makefile.include do
> not call docker.py at all, they invoke $(RUNC) directly. Since
> configure threw away the 'sudo' command prefix Makefile.in won't
> be adding either 'sudo' or 'sudo -n', it'll just run plani 'docker'
>

s/plani/plain/

Apart from the nits:
Reviewed-by: Thomas Huth <thuth@redhat.com>