[PATCH v2] tests/docker: ensure container command is probed at most once

["Daniel P. Berrangé" <berrange@redhat.com>]

The '--engine' arg accepts either 'podman' or 'docker', which is
not sufficiently granular to map directly to a command. This
means that  docker.py still has to then probe the exact command
to use.

Meanwhile the 'probe' command prints out the full command to use
but this cannot be passed back to docker.py to avoid probing
again, so the caching is only useful in the few case where we
run a container directly bypassing docker.py.

Address this by replacing --engine with --command for docker.py.

This in turn requires the --container-engine configure arg to be
replaced with --container-command.

With these changes the container command is probed at most once
during configure and never again, while running in an unconfigured
tree will still probe on demand.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

In v2:

 * Don't set container_command to "no" if 'probe' finds
   not viable binaries.

 configure                     | 25 +++++++------
 tests/docker/Makefile.include | 13 ++++---
 tests/docker/docker.py        | 67 ++++++++++++-----------------------
 3 files changed, 40 insertions(+), 65 deletions(-)

diff --git a/configure b/configure
index d8bc10060e..5a8e2fd278 100755
--- a/configure
+++ b/configure
@@ -172,7 +172,7 @@ fi
 # some defaults, based on the host environment
 
 # default parameters
-container_engine="auto"
+container_command=""
 cpu=""
 cross_compile="no"
 cross_prefix=""
@@ -734,7 +734,7 @@ for opt do
   ;;
   --disable-containers) use_containers="no"
   ;;
-  --container-engine=*) container_engine="$optarg"
+  --container-command=*) container_command="$optarg"
   ;;
   --rust-target-triple=*) rust_target_triple="$optarg"
   ;;
@@ -869,7 +869,7 @@ Advanced options (experts only):
   --enable-debug           enable common debug build options
   --cpu=CPU                Build for host CPU [$cpu]
   --disable-containers     don't use containers for cross-building
-  --container-engine=TYPE  which container engine to use [$container_engine]
+  --container-command=CMD  which container command to use [autodetect]
   --gdb=GDB-path           gdb to use for gdbstub tests [$gdb_bin]
   --wasm64-32bit-address-limit Restrict wasm64 address space to 32-bit (default
                                is to use the whole 64-bit range).
@@ -1291,12 +1291,12 @@ fi
 ##########################################
 # functions to probe cross compilers
 
-runc="no"
-if test $use_containers = "yes" && (has "docker" || has "podman"); then
-    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
+if test "$container_command" = ""; then
+    container_command=$($python "$source_path"/tests/docker/docker.py probe)
+    test "$container_command" == "no" && container_command=""
+fi
+if test $use_containers = "yes" && test "$container_command" != ""; then
+    docker_py="$python $source_path/tests/docker/docker.py --command $container_command"
 fi
 
 # cross compilers defaults, can be overridden with --cross-cc-ARCH
@@ -1415,7 +1415,7 @@ probe_target_compiler() {
   esac
 
   for host in $container_hosts; do
-    test "$runc" != no || continue
+    test "$container_command" != "" || continue
     test "$host" = "$cpu" || continue
     case $target_arch in
       # debian-all-test-cross architectures
@@ -1736,9 +1736,8 @@ 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 "$runc" != no; then
-    echo "RUNC=$runc" >> $config_host_mak
-    echo "CONTAINER_ENGINE=$container_engine" >> $config_host_mak
+if test "$container_command" != ""; then
+    echo "CONTAINER_COMMAND=$container_command" >> $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 4725c39807..0adddb6a5c 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -16,9 +16,8 @@ DOCKER_DEFAULT_REGISTRY := registry.gitlab.com/qemu-project/qemu
 endif
 DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),$(DOCKER_DEFAULT_REGISTRY))
 
-CONTAINER_ENGINE = auto
-DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py --engine $(CONTAINER_ENGINE)
-RUNC ?= $(shell $(DOCKER_SCRIPT) probe)
+CONTAINER_COMMAND ?= $(shell $(SRC_PATH)/tests/docker/docker.py probe)
+DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py --command "$(CONTAINER_COMMAND)"
 
 CUR_TIME := $(shell date +%Y-%m-%d-%H.%M.%S.$$$$)
 DOCKER_SRC_COPY := $(BUILD_DIR)/docker-src.$(CUR_TIME)
@@ -41,7 +40,7 @@ docker-qemu-src: $(DOCKER_SRC_COPY)
 # General rule for building docker images.
 docker-image-%: $(DOCKER_FILES_DIR)/%.docker
 	  $(call quiet-command,			\
-		DOCKER_BUILDKIT=1 $(RUNC) build		\
+		DOCKER_BUILDKIT=1 $(CONTAINER_COMMAND) build \
 		$(if $(DOCKER_V),,--quiet)		\
 		$(if $(NOCACHE),--no-cache,		\
 			$(if $(DOCKER_REGISTRY),--cache-from $(DOCKER_REGISTRY)/qemu/$*)) \
@@ -152,7 +151,7 @@ $(foreach i,$(filter-out $(DOCKER_PARTIAL_IMAGES),$(DOCKER_IMAGES)), \
 )
 
 docker:
-	@echo 'Build QEMU and run tests inside $(RUNC) containers'
+	@echo 'Build QEMU and run tests inside $(CONTAINER_COMMAND) containers'
 	@echo
 	@echo 'Available targets:'
 	@echo
@@ -219,10 +218,10 @@ docker-run: docker-qemu-src
 			$(IMAGE) --executable $(EXECUTABLE),		\
 			"  COPYING $(EXECUTABLE) to $(IMAGE)"))
 	$(call quiet-command,						\
-		$(RUNC) run 						\
+		$(CONTAINER_COMMAND) run 				\
 			--rm						\
 			$(if $(NOUSER),,				\
-				$(if $(filter docker,$(RUNC)),		\
+				$(if $(filter docker,$(CONTAINER_COMMAND)), \
 					-u $(UID),			\
 					--userns keep-id		\
 				)					\
diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 9e18b984f4..d2f39b5645 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -35,27 +35,6 @@
 
 DEVNULL = open(os.devnull, 'wb')
 
-class EngineEnum(enum.IntEnum):
-    AUTO = 1
-    DOCKER = 2
-    PODMAN = 3
-
-    def __str__(self):
-        return self.name.lower()
-
-    def __repr__(self):
-        return str(self)
-
-    @staticmethod
-    def argparse(s):
-        try:
-            return EngineEnum[s.upper()]
-        except KeyError:
-            return s
-
-
-USE_ENGINE = EngineEnum.AUTO
-
 def _bytes_checksum(bytes):
     """Calculate a digest string unique to the text content"""
     return hashlib.sha1(bytes).hexdigest()
@@ -73,12 +52,11 @@ def _file_checksum(filename):
 
 def _guess_engine_command():
     """ Guess a working engine command or raise exception if not found"""
-    commands = []
-
-    if USE_ENGINE in [EngineEnum.AUTO, EngineEnum.PODMAN]:
-        commands += [["podman"], ["podman-remote"], ["podman", "--remote"]]
-    if USE_ENGINE in [EngineEnum.AUTO, EngineEnum.DOCKER]:
-        commands += [["docker"], ["sudo", "-n", "docker"]]
+    commands = [["podman"],
+                ["podman-remote"],
+                ["podman", "--remote"],
+                ["docker"],
+                ["sudo", "-n", "docker"]]
     for cmd in commands:
         try:
             # 'version' is not sufficient to prove a working binary
@@ -222,8 +200,11 @@ def _dockerfile_verify_flat(df):
 
 class Docker(object):
     """ Running Docker commands """
-    def __init__(self):
-        self._command = _guess_engine_command()
+    def __init__(self, commandstr=None):
+        if commandstr is None:
+            self._command = _guess_engine_command()
+        else:
+            self._command = commandstr.split(" ")
 
         if ("docker" in self._command and
             "TRAVIS" not in os.environ and
@@ -411,8 +392,8 @@ def args(self, parser):
                             help="Run container using the current user's uid")
 
     def run(self, args, argv):
-        return Docker().run(argv, args.keep, quiet=args.quiet,
-                            as_user=args.run_as_current_user)
+        return Docker(args.command).run(argv, args.keep, quiet=args.quiet,
+                                        as_user=args.run_as_current_user)
 
 
 class BuildCommand(SubCommand):
@@ -445,7 +426,7 @@ def run(self, args, argv):
         dockerfile = _read_dockerfile(args.dockerfile)
         tag = args.tag
 
-        dkr = Docker()
+        dkr = Docker(args.command)
         if "--no-cache" not in argv and \
            dkr.image_matches_dockerfile(tag, dockerfile):
             if not args.quiet:
@@ -512,7 +493,7 @@ def args(self, parser):
                             help="Docker registry")
 
     def run(self, args, argv):
-        dkr = Docker()
+        dkr = Docker(args.command)
         dkr.command(cmd="pull", quiet=args.quiet,
                     argv=["%s/%s" % (args.registry, args.tag)])
         dkr.command(cmd="tag", quiet=args.quiet,
@@ -590,7 +571,7 @@ def run(self, args, argv):
         tmp.seek(0)
 
         # Run the build with our tarball context
-        dkr = Docker()
+        dkr = Docker(args.command)
         dkr.update_image(args.tag, tmp, quiet=args.quiet)
 
         return 0
@@ -601,7 +582,7 @@ class CleanCommand(SubCommand):
     name = "clean"
 
     def run(self, args, argv):
-        Docker().clean()
+        Docker(args.command).clean()
         return 0
 
 
@@ -610,7 +591,7 @@ class ImagesCommand(SubCommand):
     name = "images"
 
     def run(self, args, argv):
-        return Docker().command("images", argv, args.quiet)
+        return Docker(args.command).command("images", argv, args.quiet)
 
 
 class ProbeCommand(SubCommand):
@@ -619,7 +600,7 @@ class ProbeCommand(SubCommand):
 
     def run(self, args, argv):
         try:
-            docker = Docker()
+            docker = Docker(args.command)
             print(" ".join(docker._command))
         except Exception:
             print("no")
@@ -651,18 +632,16 @@ def run(self, args, argv):
                 cmd += ["-v", "%s:%s:ro,z" % (p, p)]
         cmd += [args.image, args.cc]
         cmd += argv
-        return Docker().run(cmd, False, quiet=args.quiet,
-                            as_user=True)
+        return Docker(args.command).run(cmd, False, quiet=args.quiet,
+                                        as_user=True)
 
 
 def main():
-    global USE_ENGINE
-
     parser = argparse.ArgumentParser(description="A Docker helper",
                                      usage="%s <subcommand> ..." %
                                      os.path.basename(sys.argv[0]))
-    parser.add_argument("--engine", type=EngineEnum.argparse, choices=list(EngineEnum),
-                        help="specify which container engine to use")
+    parser.add_argument("--command",
+                        help="specify which container engine command to use")
     subparsers = parser.add_subparsers(title="subcommands", help=None)
     for cls in SubCommand.__subclasses__():
         cmd = cls()
@@ -671,8 +650,6 @@ def main():
         cmd.args(subp)
         subp.set_defaults(cmdobj=cmd)
     args, argv = parser.parse_known_args()
-    if args.engine:
-        USE_ENGINE = args.engine
     return args.cmdobj.run(args, argv)
 
 
-- 
2.55.0