Re: [Qemu-devel] [PATCH v3 01/13] tests: Add utilities for docker testing

[Fam Zheng]

On Fri, 03/11 15:04, Alex Bennée wrote:
> 
> Fam Zheng <address@hidden> writes:
> 
> > docker_run: A wrapper for "docker run" (or "sudo -n docker run" if
> > necessary), which takes care of killing and removing the running
> > container at SIGINT.
> >
> > docker_clean: A tool to tear down all the containers including inactive
> > ones that are started by docker_run.
> >
> > docker_build: A tool to compare an image from given dockerfile and
> > rebuild it if they're different.
> 
> This commit text needs updating with the actual calling conventions.

Will do.

> 
> >
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> >  tests/docker/docker.py | 180 
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 180 insertions(+)
> >  create mode 100755 tests/docker/docker.py
> >
> > diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> > new file mode 100755
> > index 0000000..22f537c
> > --- /dev/null
> > +++ b/tests/docker/docker.py
> > @@ -0,0 +1,180 @@
> > +#!/usr/bin/env python2
> > +#
> > +# Docker controlling module
> > +#
> > +# Copyright (c) 2016 Red Hat Inc.
> > +#
> > +# Authors:
> > +#  Fam Zheng <address@hidden>
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2
> > +# or (at your option) any later version. See the COPYING file in
> > +# the top-level directory.
> 
> It's worth running pylint over this file. There are a number of
> missing newlines/spaces/long lines that aren't PEP friendly.

I'll run this through pylint.

> 
> > +
> > +import os
> > +import sys
> > +import subprocess
> > +import json
> > +import hashlib
> > +import atexit
> > +import uuid
> > +import argparse
> > +
> > +class Docker(object):
> > +    """ Running Docker commands """
> > +    def __init__(self):
> > +        self._command = self._guess_command()
> > +        self._instances = []
> > +        atexit.register(self._kill_instances)
> > +
> > +    def _do(self, cmd, quiet=True, **kwargs):
> > +        if quiet:
> > +            kwargs["stdout"] = subprocess.PIPE
> > +        return subprocess.call(self._command + cmd, **kwargs)
> > +
> > +    def _do_kill_instances(self, only_known, only_active=True):
> > +        cmd = ["ps", "-q"]
> > +        if not only_active:
> > +            cmd.append("-a")
> > +        for i in self._output(cmd).split():
> > +            resp = self._output(["inspect", i])
> > +            labels = json.loads(resp)[0]["Config"]["Labels"]
> > +            active = json.loads(resp)[0]["State"]["Running"]
> > +            if not labels:
> > +                continue
> > +            instance_uuid = labels.get("com.qemu.instance.uuid", None)
> > +            if not instance_uuid:
> > +                continue
> > +            if only_known and instance_uuid not in self._instances:
> > +                continue
> > +            print "Terminating", i
> > +            if active:
> > +                self._do(["kill", i])
> > +            self._do(["rm", i])
> > +
> > +    def clean(self):
> > +        self._do_kill_instances(False, False)
> > +        return 0
> > +
> > +    def _kill_instances(self):
> > +        return self._do_kill_instances(True)
> > +
> > +    def _output(self, cmd, **kwargs):
> > +        return subprocess.check_output(self._command + cmd,
> > +                                       stderr=subprocess.STDOUT,
> > +                                       **kwargs)
> > +
> > +    def _guess_command(self):
> > +        commands = [["docker"], ["sudo", "-n", "docker"]]
> > +        for cmd in commands:
> > +            if subprocess.call(cmd + ["images"],
> > +                               stdout=subprocess.PIPE,
> > +                               stderr=subprocess.PIPE) == 0:
> > +                return cmd
> > +        commands_txt = "\n".join(["  " + " ".join(x) for x in commands])
> > +        raise Exception("Cannot find working docker command. Tried:\n%s" % 
> > commands_txt)
> > +
> > +    def get_image_dockerfile_checksum(self, tag):
> > +        resp = self._output(["inspect", tag])
> > +        labels = json.loads(resp)[0]["Config"].get("Labels", {})
> > +        return labels.get("com.qemu.dockerfile-checksum", "")
> > +
> > +    def checksum(self, text):
> > +        return hashlib.sha1(text).hexdigest()
> > +
> > +    def build_image(self, tag, dockerfile, df, quiet=True, argv=[]):
> > +        tmp = dockerfile + "\n" + \
> > +              "LABEL com.qemu.dockerfile-checksum=%s" % 
> > self.checksum(dockerfile)
> > +        tmp_df = df + ".tmp"
> > +        tmp_file = open(tmp_df, "wb")
> > +        tmp_file.write(tmp)
> > +        tmp_file.close()
> > +        self._do(["build", "-t", tag, "-f", tmp_df] + argv + 
> > [os.path.dirname(df)],
> > +                 quiet=quiet)
> > +        os.unlink(tmp_df)
> 
> Use python's tempfile to do this. It handles all the lifetime issues for
> you automatically - the file gets removed when the object goes out of scope.

Okay, will do.

> 
> > +
> > +    def image_matches_dockerfile(self, tag, dockerfile):
> > +        try:
> > +            checksum = self.get_image_dockerfile_checksum(tag)
> > +        except:
> > +            return False
> > +        return checksum == self.checksum(dockerfile)
> > +
> > +    def run(self, cmd, keep, quiet):
> > +        label = uuid.uuid1().hex
> > +        if not keep:
> > +            self._instances.append(label)
> > +        ret = self._do(["run", "--label", "com.qemu.instance.uuid=" + 
> > label] + cmd, quiet=quiet)
> > +        if not keep:
> > +            self._instances.remove(label)
> > +        return ret
> > +
> > +class SubCommand(object):
> > +    """A SubCommand template base class"""
> > +    name = None # Subcommand name
> > +    def args(self, parser):
> > +        """Setup argument parser"""
> > +        pass
> > +    def run(self, args, argv):
> > +        """Run command.
> > +        args: parsed argument by argument parser.
> > +        argv: remaining arguments from sys.argv.
> > +        """
> > +        pass
> > +
> > +class RunCommand(SubCommand):
> > +    """Invoke docker run and take care of cleaning up"""
> > +    name = "run"
> > +    def args(self, parser):
> > +        parser.add_argument("--keep", action="store_true",
> > +                            help="Don't remove image when the command 
> > completes")
> > +        parser.add_argument("--quiet", action="store_true",
> > +                            help="Run quietly unless an error
> > occured")
> 
> I suspect --quiet should be a shared global flag.

Will change, so the --verbose in build subcommand below will be expressed in
!quiet.

> 
> Also it would be worth adding help text to show the remaining args are
> passed "as is" to the docker command line.

Yes, good point.

> 
> > +    def run(self, args, argv):
> > +        return Docker().run(argv, args.keep, quiet=args.quiet)
> > +
> > +class BuildCommand(SubCommand):
> > +    """ Build docker image out of a dockerfile"""
> > +    name = "build"
> > +    def args(self, parser):
> > +        parser.add_argument("tag",
> > +                            help="Image Tag")
> > +        parser.add_argument("dockerfile",
> > +                            help="Dockerfile name")
> > +        parser.add_argument("--verbose", "-v", action="store_true",
> > +                            help="Print verbose information")
> 
> I suspect --verbose should be a shared global flag.
> 
> > +
> > +    def run(self, args, argv):
> > +        dockerfile = open(args.dockerfile, "rb").read()
> > +        tag = args.tag
> > +
> > +        dkr = Docker()
> > +        if dkr.image_matches_dockerfile(tag, dockerfile):
> > +            if args.verbose:
> > +                print "Image is up to date."
> > +            return 0
> > +
> > +        quiet = not args.verbose
> > +        dkr.build_image(tag, dockerfile, args.dockerfile, quiet=quiet, 
> > argv=argv)
> > +        return 0
> 
> I've seen this hang. Do builds always succeed?

It does "{apt-get,yum,dnf} install", which could block due to network issues.

> 
> > +
> > +class CleanCommand(SubCommand):
> > +    """Clean up docker instances"""
> > +    name = "clean"
> > +    def run(self, args, argv):
> > +        Docker().clean()
> > +        return 0
> > +
> > +def main():
> > +    parser = argparse.ArgumentParser()
> > +    subparsers = parser.add_subparsers()
> > +    for cls in SubCommand.__subclasses__():
> > +        cmd = cls()
> > +        subp = subparsers.add_parser(cmd.name, help=cmd.__doc__)
> > +        cmd.args(subp)
> > +        subp.set_defaults(cmdobj=cmd)
> > +    args, argv = parser.parse_known_args()
> > +    return args.cmdobj.run(args, argv)
> 
> There are some niggles with help:
> 
> 14:40 address@hidden/x86_64  
> [qemu.git/review/docker-v3]>./tests/docker/docker.py --help
> usage: docker.py [-h] {run,build,clean} ...
> 
> positional arguments:
>   {run,build,clean}
> 
> Positional? Really. You can only have one command at a time.

That's the default output of Python's argparse module.

> 
>       run              Invoke docker run and take care of cleaning up
>       build            Build docker image out of a dockerfile
>       clean            Clean up docker instances
> 
> optional arguments:
>   -h, --help         show this help message and exit
> 
> OK that's useful, but do we have args for build?
> 
> 14:43 address@hidden/x86_64  
> [qemu.git/review/docker-v3]>./tests/docker/docker.py --help build
>   usage: docker.py [-h] {run,build,clean} ...
> 
> positional arguments:
>   {run,build,clean}
>       run              Invoke docker run and take care of cleaning up
>       build            Build docker image out of a dockerfile
>       clean            Clean up docker instances
> 
> optional arguments:
>   -h, --help         show this help message and exit
> 
> Hmm same result. We have to call like this:
> 
> 14:43 address@hidden/x86_64  [qemu.git/review/docker-v3]  
> >./tests/docker/docker.py build --help
>   usage: docker.py build [-h] [--verbose] tag dockerfile
> 
> positional arguments:
>   tag            Image Tag
>   dockerfile     Dockerfile name
> 
> optional arguments:
>   -h, --help     show this help message and exit
>   --verbose, -v  Print verbose information
> 
> Maybe there is someway to make this clearer.

I'll try. Thanks for your input!

Fam

> 
> > +
> > +if __name__ == "__main__":
> > +    sys.exit(main())
> 
> 
> --
> Alex Bennée