[v2] tests/docker: start using libvirt-ci's "lcitool" for dockerfiles

[PATCH v2 12/25] tests/docker: add script for automating container refresh

Posted by Daniel P. Berrangé 4 years, 10 months ago

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/docker/dockerfiles/refresh | 53 ++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100755 tests/docker/dockerfiles/refresh

diff --git a/tests/docker/dockerfiles/refresh b/tests/docker/dockerfiles/refresh
new file mode 100755
index 0000000000..b1d99963e9
--- /dev/null
+++ b/tests/docker/dockerfiles/refresh
@@ -0,0 +1,53 @@
+#!/usr/bin/python3
+#
+# Re-generate container recipes
+#
+# This script uses the "lcitool" available from
+#
+#   https://gitlab.com/libvirt/libvirt-ci
+#
+# Copyright (c) 2020 Red Hat Inc.
+#
+# 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.
+
+import sys
+import os
+import subprocess
+
+if len(sys.argv) != 2:
+   print("syntax: %s PATH-TO-LCITOOL" % sys.argv[0], file=sys.stderr)
+   sys.exit(1)
+
+lcitool_path=sys.argv[1]
+
+def atomic_write(filename, content):
+   try:
+      with open(filename + ".tmp", "w") as fp:
+         print(content, file=fp, end="")
+         os.replace(filename + ".tmp", filename)
+   except Exception as ex:
+      os.unlink(filename + ".tmp")
+      raise
+
+def generate_image(filename, host, cross=None, trailer=None):
+   print("Generate %s" % filename)
+   args = [lcitool_path, "dockerfile"]
+   if cross is not None:
+      args.extend(["--cross", cross])
+   args.extend([host, "qemu"])
+   lcitool=subprocess.run(args, capture_output=True)
+
+   if lcitool.returncode != 0:
+      raise Exception("Failed to generate %s: %s" % (filename, lcitool.stderr))
+
+   content = lcitool.stdout.decode("utf8")
+   if trailer is not None:
+      content += trailer
+   atomic_write(filename, content)
+
+try:
+   pass
+except Exception as ex:
+   print(str(ex), file=sys.stderr)
-- 
2.29.2

Re: [PATCH v2 12/25] tests/docker: add script for automating container refresh

Posted by Philippe Mathieu-Daudé 4 years, 10 months ago

On 1/14/21 2:02 PM, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/docker/dockerfiles/refresh | 53 ++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100755 tests/docker/dockerfiles/refresh
> 
> diff --git a/tests/docker/dockerfiles/refresh b/tests/docker/dockerfiles/refresh
> new file mode 100755
> index 0000000000..b1d99963e9
> --- /dev/null
> +++ b/tests/docker/dockerfiles/refresh
> @@ -0,0 +1,53 @@
> +#!/usr/bin/python3
> +#
> +# Re-generate container recipes
> +#
> +# This script uses the "lcitool" available from
> +#
> +#   https://gitlab.com/libvirt/libvirt-ci
> +#
> +# Copyright (c) 2020 Red Hat Inc.
> +#
> +# 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.
> +
> +import sys
> +import os
> +import subprocess
> +
> +if len(sys.argv) != 2:
> +   print("syntax: %s PATH-TO-LCITOOL" % sys.argv[0], file=sys.stderr)
> +   sys.exit(1)
> +
> +lcitool_path=sys.argv[1]
> +
> +def atomic_write(filename, content):
> +   try:
> +      with open(filename + ".tmp", "w") as fp:
> +         print(content, file=fp, end="")
> +         os.replace(filename + ".tmp", filename)
> +   except Exception as ex:
> +      os.unlink(filename + ".tmp")
> +      raise
> +
> +def generate_image(filename, host, cross=None, trailer=None):
> +   print("Generate %s" % filename)
> +   args = [lcitool_path, "dockerfile"]
> +   if cross is not None:
> +      args.extend(["--cross", cross])
> +   args.extend([host, "qemu"])
> +   lcitool=subprocess.run(args, capture_output=True)
> +
> +   if lcitool.returncode != 0:
> +      raise Exception("Failed to generate %s: %s" % (filename, lcitool.stderr))
> +
> +   content = lcitool.stdout.decode("utf8")
> +   if trailer is not None:
> +      content += trailer
> +   atomic_write(filename, content)
> +
> +try:
> +   pass

This code would be easier to understand if you place this
patch before the "tests/docker: auto-generate ..." ones,
and add a comment (or directly add these 3 lines in the
first "auto-generate" patch).

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +except Exception as ex:
> +   print(str(ex), file=sys.stderr)
>

Re: [PATCH v2 12/25] tests/docker: add script for automating container refresh

Posted by Wainer dos Santos Moschetta 4 years, 10 months ago

Hi,

On 1/14/21 10:02 AM, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/docker/dockerfiles/refresh | 53 ++++++++++++++++++++++++++++++++
>   1 file changed, 53 insertions(+)
>   create mode 100755 tests/docker/dockerfiles/refresh
>
> diff --git a/tests/docker/dockerfiles/refresh b/tests/docker/dockerfiles/refresh

My suggestion is to rename the file to gen_dockerfiles.py and move to 
the parent directory.

To the best of my knowledge this community isn't picky about the style 
of Python code but if you run pylint you gonna see some warns worth a 
fix (mostly about bad indentation).

> new file mode 100755
> index 0000000000..b1d99963e9
> --- /dev/null
> +++ b/tests/docker/dockerfiles/refresh
> @@ -0,0 +1,53 @@
> +#!/usr/bin/python3
> +#
> +# Re-generate container recipes
> +#
> +# This script uses the "lcitool" available from
> +#
> +#   https://gitlab.com/libvirt/libvirt-ci

Shouldn't we document somewhere else the use of this tool in QEMU? For 
those who don't know its purpose, how it can be installed/extended, 
etc... maybe having a README under tests/docker or just appending some 
words on "Docker based tests" section from docs/devel/testing.rst?

- Wainer

> +#
> +# Copyright (c) 2020 Red Hat Inc.
> +#
> +# 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.
> +
> +import sys
> +import os
> +import subprocess
> +
> +if len(sys.argv) != 2:
> +   print("syntax: %s PATH-TO-LCITOOL" % sys.argv[0], file=sys.stderr)
> +   sys.exit(1)
> +
> +lcitool_path=sys.argv[1]
> +
> +def atomic_write(filename, content):
> +   try:
> +      with open(filename + ".tmp", "w") as fp:
> +         print(content, file=fp, end="")
> +         os.replace(filename + ".tmp", filename)
> +   except Exception as ex:
> +      os.unlink(filename + ".tmp")
> +      raise
> +
> +def generate_image(filename, host, cross=None, trailer=None):
> +   print("Generate %s" % filename)
> +   args = [lcitool_path, "dockerfile"]
> +   if cross is not None:
> +      args.extend(["--cross", cross])
> +   args.extend([host, "qemu"])
> +   lcitool=subprocess.run(args, capture_output=True)
> +
> +   if lcitool.returncode != 0:
> +      raise Exception("Failed to generate %s: %s" % (filename, lcitool.stderr))
> +
> +   content = lcitool.stdout.decode("utf8")
> +   if trailer is not None:
> +      content += trailer
> +   atomic_write(filename, content)
> +
> +try:
> +   pass
> +except Exception as ex:
> +   print(str(ex), file=sys.stderr)