[Qemu-devel] [PATCH v1 0/8] Docker tweaks for TCG testing

[Qemu-devel] [PATCH v1 0/8] Docker tweaks for TCG testing

[Alex Bennée]

Hi Fam,

These are the docker specific changes from my current TCG testing
branch:

  https://github.com/stsquad/qemu/tree/testing/tcg-tests-revival-v8

Once the CI tests have worked their way through I'm hoping to submit
the pull-req to get this merged. However I've been hacking about in
docker.py and rather than sending the whole 50+ series I've split them
out for easy review. The ones that currently need review are:

: patch 0005/docker docker.py use ps to probe usage.patch
: patch 0006/docker docker.py don t conflate checksums for ext.patch
: patch 0007/docker docker.py add check sub command.patch
: patch 0008/docker docker.py adding age check command.patch

If you get a chance could you have a quick look through?

Alex Bennée (8):
  docker: Add "cc" subcommand
  docker: extend "cc" command to accept compiler
  docker: allow "cc" command to run in user context
  docker: Makefile.include introduce DOCKER_SCRIPT
  docker: docker.py use "ps" to probe usage
  docker: docker.py don't conflate checksums for extra_files
  docker: docker.py add check sub-command
  docker: docker.py adding age check command

 tests/docker/Makefile.include |  10 ++--
 tests/docker/docker.py        | 105 ++++++++++++++++++++++++++++++++--
 2 files changed, 106 insertions(+), 9 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v1 1/8] docker: Add "cc" subcommand

[Alex Bennée]

Signed-off-by: Fam Zheng <famz@redhat.com>
[AJB: add if args.paths check]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v2
  - add if args.paths check to avoid iterating null argument
---
 tests/docker/docker.py | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 306e14cf69..4cd6f9b8ec 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -409,6 +409,31 @@ class ProbeCommand(SubCommand):
         return
 
 
+class CcCommand(SubCommand):
+    """Compile sources with cc in images"""
+    name = "cc"
+
+    def args(self, parser):
+        parser.add_argument("--image", "-i", required=True,
+                            help="The docker image in which to run cc")
+        parser.add_argument("--source-path", "-s", nargs="*", dest="paths",
+                            help="""Extra paths to (ro) mount into container for
+                            reading sources""")
+
+    def run(self, args, argv):
+        if argv and argv[0] == "--":
+            argv = argv[1:]
+        cwd = os.getcwd()
+        cmd = ["--rm", "-w", cwd,
+               "-v", "%s:%s:rw" % (cwd, cwd)]
+        if args.paths:
+            for p in args.paths:
+                cmd += ["-v", "%s:%s:ro,z" % (p, p)]
+        cmd += [args.image, "cc"]
+        cmd += argv
+        return Docker().command("run", cmd, args.quiet)
+
+
 def main():
     parser = argparse.ArgumentParser(description="A Docker helper",
             usage="%s <subcommand> ..." % os.path.basename(sys.argv[0]))
-- 
2.17.1

[Qemu-devel] [PATCH v1 2/8] docker: extend "cc" command to accept compiler

[Alex Bennée]

When calling our cross-compilation images we want to call something
other than the default cc.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
v2
  - use arg.cc default to simplify logic
---
 tests/docker/docker.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 4cd6f9b8ec..663dfcba4b 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -416,6 +416,8 @@ class CcCommand(SubCommand):
     def args(self, parser):
         parser.add_argument("--image", "-i", required=True,
                             help="The docker image in which to run cc")
+        parser.add_argument("--cc", default="cc",
+                            help="The compiler executable to call")
         parser.add_argument("--source-path", "-s", nargs="*", dest="paths",
                             help="""Extra paths to (ro) mount into container for
                             reading sources""")
@@ -429,7 +431,7 @@ class CcCommand(SubCommand):
         if args.paths:
             for p in args.paths:
                 cmd += ["-v", "%s:%s:ro,z" % (p, p)]
-        cmd += [args.image, "cc"]
+        cmd += [args.image, args.cc]
         cmd += argv
         return Docker().command("run", cmd, args.quiet)
 
-- 
2.17.1

[Qemu-devel] [PATCH v1 3/8] docker: allow "cc" command to run in user context

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/docker/docker.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 663dfcba4b..c83fd89985 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -418,6 +418,8 @@ class CcCommand(SubCommand):
                             help="The docker image in which to run cc")
         parser.add_argument("--cc", default="cc",
                             help="The compiler executable to call")
+        parser.add_argument("--user",
+                            help="The user-id to run under")
         parser.add_argument("--source-path", "-s", nargs="*", dest="paths",
                             help="""Extra paths to (ro) mount into container for
                             reading sources""")
@@ -431,6 +433,8 @@ class CcCommand(SubCommand):
         if args.paths:
             for p in args.paths:
                 cmd += ["-v", "%s:%s:ro,z" % (p, p)]
+        if args.user:
+            cmd += ["-u", args.user]
         cmd += [args.image, args.cc]
         cmd += argv
         return Docker().command("run", cmd, args.quiet)
-- 
2.17.1

[Qemu-devel] [PATCH v1 4/8] docker: Makefile.include introduce DOCKER_SCRIPT

[Alex Bennée]

Define this in one place to make it easy to re-use.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/docker/Makefile.include | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 74fd51c22c..8afb383478 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -15,6 +15,8 @@ DOCKER_TESTS := $(notdir $(shell \
 
 DOCKER_TOOLS := travis
 
+DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py
+
 TESTS ?= %
 IMAGES ?= %
 
@@ -38,7 +40,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
 		echo WARNING: EXECUTABLE is not set, debootstrap may fail. 2>&1 ; \
 	fi
 	$(call quiet-command,\
-		$(SRC_PATH)/tests/docker/docker.py build qemu:$* $< \
+		$(DOCKER_SCRIPT) build qemu:$* $< \
 		$(if $V,,--quiet) $(if $(NOCACHE),--no-cache) \
 		$(if $(NOUSER),,--add-current-user) \
 		$(if $(EXTRA_FILES),--extra-files $(EXTRA_FILES))\
@@ -133,11 +135,11 @@ docker-run: docker-qemu-src
 	fi
 	$(if $(EXECUTABLE),						\
 		$(call quiet-command,					\
-			$(SRC_PATH)/tests/docker/docker.py update 	\
+			$(DOCKER_SCRIPT) update 			\
 			$(IMAGE) $(EXECUTABLE),				\
 			"  COPYING $(EXECUTABLE) to $(IMAGE)"))
 	$(call quiet-command,						\
-		$(SRC_PATH)/tests/docker/docker.py run 			\
+		$(DOCKER_SCRIPT) run 					\
 			$(if $(NOUSER),,-u $(shell id -u)) 		\
 			--security-opt seccomp=unconfined		\
 			$(if $V,,--rm) 					\
@@ -167,4 +169,4 @@ docker-run-%:
 	@$(MAKE) docker-run TEST=$(CMD) IMAGE=qemu:$(IMAGE)
 
 docker-clean:
-	$(call quiet-command, $(SRC_PATH)/tests/docker/docker.py clean)
+	$(call quiet-command, $(DOCKER_SCRIPT) clean)
-- 
2.17.1

[Qemu-devel] [PATCH v1 5/8] docker: docker.py use "ps" to probe usage

[Alex Bennée]

The "images" command is a fairly heavyweight command to run as it
involves searching the whole docker file-system inventory. On a
machine with a lot of images this makes start-up fairly expensive.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/docker.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index c83fd89985..3072bf4a31 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -49,7 +49,7 @@ def _guess_docker_command():
     commands = [["docker"], ["sudo", "-n", "docker"]]
     for cmd in commands:
         try:
-            if subprocess.call(cmd + ["images"],
+            if subprocess.call(cmd + ["ps"],
                                stdout=DEVNULL, stderr=DEVNULL) == 0:
                 return cmd
         except OSError:
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 5/8] docker: docker.py use "ps" to probe usage

[Philippe Mathieu-Daudé]

On 06/18/2018 10:07 AM, Alex Bennée wrote:
> The "images" command is a fairly heavyweight command to run as it
> involves searching the whole docker file-system inventory. On a
> machine with a lot of images this makes start-up fairly expensive.

"version" would be even quicker.

> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  tests/docker/docker.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index c83fd89985..3072bf4a31 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -49,7 +49,7 @@ def _guess_docker_command():
>      commands = [["docker"], ["sudo", "-n", "docker"]]
>      for cmd in commands:
>          try:
> -            if subprocess.call(cmd + ["images"],
> +            if subprocess.call(cmd + ["ps"],
>                                 stdout=DEVNULL, stderr=DEVNULL) == 0:
>                  return cmd
>          except OSError:
> 

[Qemu-devel] [PATCH v1 6/8] docker: docker.py don't conflate checksums for extra_files

[Alex Bennée]

This just gets confusing especially as the helper function doesn't
even take into account any extra files (or the executable). Currently
the actual check just ignores them and also passes the result through
_dockerfile_preprocess so we fix that too.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/docker.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 3072bf4a31..325915a6f9 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -201,8 +201,10 @@ class Docker(object):
 
         tmp_df.write("\n")
         tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" %
-                     _text_checksum("\n".join([dockerfile] +
-                                    extra_files_cksum)))
+                     _text_checksum(_dockerfile_preprocess(dockerfile)))
+        for f, c in extra_files_cksum:
+            tmp_df.write("LABEL com.qemu.%s-checksum=%s" % (f, c))
+
         tmp_df.flush()
 
         self._do_check(["build", "-t", tag, "-f", tmp_df.name] + argv + \
@@ -317,7 +319,7 @@ class BuildCommand(SubCommand):
                 _copy_binary_with_libs(args.include_executable, docker_dir)
             for filename in args.extra_files or []:
                 _copy_with_mkdir(filename, docker_dir)
-                cksum += [_file_checksum(filename)]
+                cksum += [(filename, _file_checksum(filename))]
 
             argv += ["--build-arg=" + k.lower() + "=" + v
                         for k, v in os.environ.iteritems()
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 6/8] docker: docker.py don't conflate checksums for extra_files

[Philippe Mathieu-Daudé]

On 06/18/2018 10:07 AM, Alex Bennée wrote:
> This just gets confusing especially as the helper function doesn't
> even take into account any extra files (or the executable). Currently
> the actual check just ignores them and also passes the result through
> _dockerfile_preprocess so we fix that too.

Looks correct.

> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  tests/docker/docker.py | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index 3072bf4a31..325915a6f9 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -201,8 +201,10 @@ class Docker(object):
>  
>          tmp_df.write("\n")
>          tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" %
> -                     _text_checksum("\n".join([dockerfile] +
> -                                    extra_files_cksum)))
> +                     _text_checksum(_dockerfile_preprocess(dockerfile)))
> +        for f, c in extra_files_cksum:
> +            tmp_df.write("LABEL com.qemu.%s-checksum=%s" % (f, c))
> +
>          tmp_df.flush()
>  
>          self._do_check(["build", "-t", tag, "-f", tmp_df.name] + argv + \
> @@ -317,7 +319,7 @@ class BuildCommand(SubCommand):
>                  _copy_binary_with_libs(args.include_executable, docker_dir)
>              for filename in args.extra_files or []:
>                  _copy_with_mkdir(filename, docker_dir)
> -                cksum += [_file_checksum(filename)]
> +                cksum += [(filename, _file_checksum(filename))]
>  
>              argv += ["--build-arg=" + k.lower() + "=" + v
>                          for k, v in os.environ.iteritems()
> 

[Qemu-devel] [PATCH v1 7/8] docker: docker.py add check sub-command

[Alex Bennée]

This command allows you to check if we need to re-build a docker
image. If the image isn't in the repository or the checksums don't
match then we return false and some text (for processing in
makefiles).

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
  - fix prints to be "modern"
  - PEP8 fixes
  - split inspect into function
---
 tests/docker/docker.py | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 325915a6f9..c157ff826a 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -179,8 +179,14 @@ class Docker(object):
                                        stderr=subprocess.STDOUT,
                                        **kwargs)
 
+    def inspect_tag(self, tag):
+        try:
+            return self._output(["inspect", tag])
+        except subprocess.CalledProcessError:
+            return None
+
     def get_image_dockerfile_checksum(self, tag):
-        resp = self._output(["inspect", tag])
+        resp = self.inspect_tag(tag)
         labels = json.loads(resp)[0]["Config"].get("Labels", {})
         return labels.get("com.qemu.dockerfile-checksum", "")
 
@@ -442,6 +448,36 @@ class CcCommand(SubCommand):
         return Docker().command("run", cmd, args.quiet)
 
 
+class CheckCommand(SubCommand):
+    """Check if we need to re-build a docker image out of a dockerfile.
+    Arguments: <tag> <dockerfile>"""
+    name = "check"
+
+    def args(self, parser):
+        parser.add_argument("tag",
+                            help="Image Tag")
+        parser.add_argument("dockerfile",
+                            help="Dockerfile name")
+
+    def run(self, args, argv):
+        dockerfile = open(args.dockerfile, "rb").read()
+        tag = args.tag
+
+        dkr = Docker()
+        info = dkr.inspect_tag(tag)
+        if info is None:
+            print("Image does not exist")
+            return 1
+
+        if dkr.image_matches_dockerfile(tag, dockerfile):
+            if not args.quiet:
+                print("Image is up to date")
+            return 0
+        else:
+            print("Image needs updating")
+            return 1
+
+
 def main():
     parser = argparse.ArgumentParser(description="A Docker helper",
             usage="%s <subcommand> ..." % os.path.basename(sys.argv[0]))
-- 
2.17.1

[Qemu-devel] [PATCH v1 8/8] docker: docker.py adding age check command

[Alex Bennée]

This is useful for querying if an image is too old.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v7
  - use cached info
---
 tests/docker/docker.py | 46 +++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index c157ff826a..da1ed4e879 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -29,6 +29,7 @@ from tarfile import TarFile, TarInfo
 from StringIO import StringIO
 from shutil import copy, rmtree
 from pwd import getpwuid
+from datetime import datetime,timedelta
 
 
 FILTERED_ENV_NAMES = ['ftp_proxy', 'http_proxy', 'https_proxy']
@@ -185,6 +186,9 @@ class Docker(object):
         except subprocess.CalledProcessError:
             return None
 
+    def get_image_creation_time(self, info):
+        return json.loads(info)[0]["Created"]
+
     def get_image_dockerfile_checksum(self, tag):
         resp = self.inspect_tag(tag)
         labels = json.loads(resp)[0]["Config"].get("Labels", {})
@@ -456,11 +460,14 @@ class CheckCommand(SubCommand):
     def args(self, parser):
         parser.add_argument("tag",
                             help="Image Tag")
-        parser.add_argument("dockerfile",
-                            help="Dockerfile name")
+        parser.add_argument("dockerfile", default=None,
+                            help="Dockerfile name", nargs='?')
+        parser.add_argument("--checktype", choices=["checksum", "age"],
+                            default="checksum", help="check type")
+        parser.add_argument("--olderthan", default=60, type=int,
+                            help="number of minutes")
 
     def run(self, args, argv):
-        dockerfile = open(args.dockerfile, "rb").read()
         tag = args.tag
 
         dkr = Docker()
@@ -469,13 +476,32 @@ class CheckCommand(SubCommand):
             print("Image does not exist")
             return 1
 
-        if dkr.image_matches_dockerfile(tag, dockerfile):
-            if not args.quiet:
-                print("Image is up to date")
-            return 0
-        else:
-            print("Image needs updating")
-            return 1
+        if args.checktype == "checksum":
+            if not args.dockerfile:
+                print("Need a dockerfile for tag:%s" % (tag))
+                return 1
+
+            dockerfile = open(args.dockerfile, "rb").read()
+
+            if dkr.image_matches_dockerfile(tag, dockerfile):
+                if not args.quiet:
+                    print("Image is up to date")
+                return 0
+            else:
+                print("Image needs updating")
+                return 1
+        elif args.checktype == "age":
+            timestr = dkr.get_image_creation_time(info).split(".")[0]
+            created = datetime.strptime(timestr, "%Y-%m-%dT%H:%M:%S")
+            past = datetime.now() - timedelta(minutes=args.olderthan)
+            if created < past:
+                print ("Image created @ %s more than %d minutes old" %
+                       (timestr, args.olderthan))
+                return 1
+            else:
+                if not args.quiet:
+                    print ("Image less than %d minutes old" % (args.olderthan))
+                return 0
 
 
 def main():
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 8/8] docker: docker.py adding age check command

[Philippe Mathieu-Daudé]

On 06/18/2018 10:07 AM, Alex Bennée wrote:
> This is useful for querying if an image is too old.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v7
>   - use cached info
> ---
>  tests/docker/docker.py | 46 +++++++++++++++++++++++++++++++++---------
>  1 file changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index c157ff826a..da1ed4e879 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -29,6 +29,7 @@ from tarfile import TarFile, TarInfo
>  from StringIO import StringIO
>  from shutil import copy, rmtree
>  from pwd import getpwuid
> +from datetime import datetime,timedelta

datetime,<space>timedelta

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>  
>  
>  FILTERED_ENV_NAMES = ['ftp_proxy', 'http_proxy', 'https_proxy']
> @@ -185,6 +186,9 @@ class Docker(object):
>          except subprocess.CalledProcessError:
>              return None
>  
> +    def get_image_creation_time(self, info):
> +        return json.loads(info)[0]["Created"]
> +
>      def get_image_dockerfile_checksum(self, tag):
>          resp = self.inspect_tag(tag)
>          labels = json.loads(resp)[0]["Config"].get("Labels", {})
> @@ -456,11 +460,14 @@ class CheckCommand(SubCommand):
>      def args(self, parser):
>          parser.add_argument("tag",
>                              help="Image Tag")
> -        parser.add_argument("dockerfile",
> -                            help="Dockerfile name")
> +        parser.add_argument("dockerfile", default=None,
> +                            help="Dockerfile name", nargs='?')
> +        parser.add_argument("--checktype", choices=["checksum", "age"],
> +                            default="checksum", help="check type")
> +        parser.add_argument("--olderthan", default=60, type=int,
> +                            help="number of minutes")
>  
>      def run(self, args, argv):
> -        dockerfile = open(args.dockerfile, "rb").read()
>          tag = args.tag
>  
>          dkr = Docker()
> @@ -469,13 +476,32 @@ class CheckCommand(SubCommand):
>              print("Image does not exist")
>              return 1
>  
> -        if dkr.image_matches_dockerfile(tag, dockerfile):
> -            if not args.quiet:
> -                print("Image is up to date")
> -            return 0
> -        else:
> -            print("Image needs updating")
> -            return 1
> +        if args.checktype == "checksum":
> +            if not args.dockerfile:
> +                print("Need a dockerfile for tag:%s" % (tag))
> +                return 1
> +
> +            dockerfile = open(args.dockerfile, "rb").read()
> +
> +            if dkr.image_matches_dockerfile(tag, dockerfile):
> +                if not args.quiet:
> +                    print("Image is up to date")
> +                return 0
> +            else:
> +                print("Image needs updating")
> +                return 1
> +        elif args.checktype == "age":
> +            timestr = dkr.get_image_creation_time(info).split(".")[0]
> +            created = datetime.strptime(timestr, "%Y-%m-%dT%H:%M:%S")
> +            past = datetime.now() - timedelta(minutes=args.olderthan)
> +            if created < past:
> +                print ("Image created @ %s more than %d minutes old" %
> +                       (timestr, args.olderthan))
> +                return 1
> +            else:
> +                if not args.quiet:
> +                    print ("Image less than %d minutes old" % (args.olderthan))
> +                return 0
>  
>  
>  def main():
>