This help formatter class reports the defaults we use for options taking an argument. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; Reviewed-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- ci/helper | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ci/helper b/ci/helper index 73a3f729da..8f34f4b59d 100755 --- a/ci/helper +++ b/ci/helper @@ -79,6 +79,7 @@ class Parser: "build", help="run a build in a container", parents=[containerparser, mesonparser], + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) buildparser.set_defaults(func=Application.action_build) @@ -87,6 +88,7 @@ class Parser: "test", help="run a build in a container (including tests)", parents=[containerparser, mesonparser], + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) testparser.set_defaults(func=Application.action_test) @@ -95,6 +97,7 @@ class Parser: "shell", help="start a shell in a container", parents=[containerparser], + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) shellparser.set_defaults(func=Application.action_shell) @@ -102,6 +105,7 @@ class Parser: listimagesparser = subparsers.add_parser( "list-images", help="list known container images", + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) listimagesparser.set_defaults(func=Application.action_list_images) @@ -110,6 +114,7 @@ class Parser: "refresh", help="refresh data generated with lcitool", parents=[lcitoolparser], + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) refreshparser.set_defaults(func=Application.action_refresh) -- 2.30.2

With the gradual rewrite of the Makefile to the 'helper' script will require helper functions that would better live in a separate util module. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; Reviewed-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- ci/util.py | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 ci/util.py diff --git a/ci/util.py b/ci/util.py new file mode 100644 index 0000000000..f9f8320276 --- /dev/null +++ b/ci/util.py @@ -0,0 +1,40 @@ +import json +import urllib.request +import urllib.parse + +from typing import Dict, List + + +def get_registry_uri(namespace: str, + gitlab_uri: str = "https://gitlab.com") -> str: + """ + Construct a v4 API URI pointing the namespaced project's image registry. + + :param namespace: GitLab project namespace, e.g. "libvirt/libvirt" + :param gitlab_uri: GitLab base URI, can be a private deployment + :param api_version: GitLab REST API version number + :return: URI pointing to a namespaced project's image registry + """ + + # this converts something like "libvirt/libvirt" to "libvirt%2Flibvirt" + namespace_urlenc = urllib.parse.quote_plus(namespace) + + project_uri = f"{gitlab_uri}/api/v4/projects/{namespace_urlenc}" + + uri = project_uri + "/registry/repositories" + return uri + + +def get_registry_images(uri: str) -> List[Dict]: + """ + List all container images that are currently available in the given GitLab + project. + + :param uri: URI pointing to a GitLab instance's image registry + :return: list of container image names + """ + + r = urllib.request.urlopen(uri + "?per_page=100") + + # read the HTTP response and load the JSON part of it + return json.loads(r.read().decode()) -- 2.30.2

This function checks whether there are any stale Docker images in the registry that can be purged. Since we're pulling available container images from our GitLab registry with the 'list-images' action, it could happen that we'd list old (already unsupported) images and make them available for the user to consume and run a build in them. Naturally, the build will most likely fail leaving the user confused. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- ci/helper | 46 +++++++++++++++++++++++++++++++++++++++++++++- ci/util.py | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/ci/helper b/ci/helper index 31cf72fbdf..0743f95e13 100755 --- a/ci/helper +++ b/ci/helper @@ -10,6 +10,7 @@ import pty import shutil import subprocess import sys +import textwrap import util @@ -130,7 +131,7 @@ class Parser: refreshparser = subparsers.add_parser( "refresh", help="refresh data generated with lcitool", - parents=[lcitoolparser], + parents=[lcitoolparser, gitlabparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) refreshparser.add_argument( @@ -139,6 +140,13 @@ class Parser: default=False, help="refresh data silently" ) + refreshparser.add_argument( + "--check-stale", + action="store", + choices=["yes", "no"], + default="yes", + help="check for existence of stale images on the GitLab instance" + ) refreshparser.set_defaults(func=Application.action_refresh) def parse(self): @@ -287,10 +295,46 @@ class Application: print("Available cross-compiler container images:\n") print(spacing + ("\n" + spacing).join(cross)) + def check_stale_images(self): + namespace = self.args.namespace + gitlab_uri = self.args.gitlab_uri + registry_uri = util.get_registry_uri(namespace, gitlab_uri) + lcitool_hosts = self.lcitool_get_hosts() + + stale_images = util.get_registry_stale_images(registry_uri, + lcitool_hosts) + if stale_images: + spacing = "\n" + 4 * " " + stale_fmt = [f"{k} (ID: {v})" for k, v in stale_images.items()] + stale_details = spacing.join(stale_fmt) + stale_ids = ' '.join([str(id) for id in stale_images.values()]) + registry_uri = util.get_registry_uri(namespace, gitlab_uri) + + msg = textwrap.dedent(f""" + The following images are stale and can be purged from the registry: + + STALE_DETAILS + + You can delete the images listed above using this shell snippet: + + $ for image_id in {stale_ids}; do + curl --request DELETE --header "PRIVATE-TOKEN: <access_token>" \\ + {registry_uri}/$image_id; + done + + You can generate a personal access token here: + + {gitlab_uri}/-/profile/personal_access_tokens + """) + print(msg.replace("STALE_DETAILS", stale_details)) + def action_refresh(self): self.refresh_containers() self.refresh_cirrus() + if self.args.check_stale == "yes" and not self.args.quiet: + self.check_stale_images() + def run(self): self.args.func(self) diff --git a/ci/util.py b/ci/util.py index f9f8320276..90d58454be 100644 --- a/ci/util.py +++ b/ci/util.py @@ -38,3 +38,44 @@ def get_registry_images(uri: str) -> List[Dict]: # read the HTTP response and load the JSON part of it return json.loads(r.read().decode()) + + +def get_image_distro(image_name: str) -> str: + """ + Extract the name of the distro in the GitLab image registry name, e.g. + ci-debian-9-cross-mipsel --> debian-9 + + :param image_name: name of the GitLab registry image + :return: distro name as a string + """ + name_prefix = "ci-" + name_suffix = "-cross-" + + distro = image_name[len(name_prefix):] + + index = distro.find(name_suffix) + if index > 0: + distro = distro[:index] + + return distro + + +def get_registry_stale_images(registry_uri: str, + supported_distros: List[str]) -> Dict[str, int]: + """ + Check the GitLab image registry for images that we no longer support and + which should be deleted. + + :param uri: URI pointing to a GitLab instance's image registry + :param supported_distros: list of hosts supported by lcitool + :return: dictionary formatted as: {<gitlab_image_name>: <gitlab_image_id>} + """ + + images = get_registry_images(registry_uri) + + stale_images = {} + for img in images: + if get_image_distro(img["name"]) not in supported_distros: + stale_images[img["name"]] = img["id"] + + return stale_images -- 2.30.2

As documented at [1], the common practice with respect to private attributes/methods naming is to prefix them with an underscore. [1] https://docs.python.org/3/tutorial/classes.html#private-variables Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- ci/helper | 122 +++++++++++++++++++++++++++--------------------------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/ci/helper b/ci/helper index 0743f95e13..70c9cb4f6b 100755 --- a/ci/helper +++ b/ci/helper @@ -84,8 +84,8 @@ class Parser: ) # Main parser - self.parser = argparse.ArgumentParser() - subparsers = self.parser.add_subparsers( + self._parser = argparse.ArgumentParser() + subparsers = self._parser.add_subparsers( dest="action", metavar="ACTION", ) @@ -98,7 +98,7 @@ class Parser: parents=[containerparser, mesonparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) - buildparser.set_defaults(func=Application.action_build) + buildparser.set_defaults(func=Application._action_build) # test action testparser = subparsers.add_parser( @@ -107,7 +107,7 @@ class Parser: parents=[containerparser, mesonparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) - testparser.set_defaults(func=Application.action_test) + testparser.set_defaults(func=Application._action_test) # shell action shellparser = subparsers.add_parser( @@ -116,7 +116,7 @@ class Parser: parents=[containerparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) - shellparser.set_defaults(func=Application.action_shell) + shellparser.set_defaults(func=Application._action_shell) # list-images action listimagesparser = subparsers.add_parser( @@ -125,7 +125,7 @@ class Parser: parents=[gitlabparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) - listimagesparser.set_defaults(func=Application.action_list_images) + listimagesparser.set_defaults(func=Application._action_list_images) # refresh action refreshparser = subparsers.add_parser( @@ -147,56 +147,56 @@ class Parser: default="yes", help="check for existence of stale images on the GitLab instance" ) - refreshparser.set_defaults(func=Application.action_refresh) + refreshparser.set_defaults(func=Application._action_refresh) def parse(self): - return self.parser.parse_args() + return self._parser.parse_args() class Application: def __init__(self): - self.basedir = pathlib.Path(__file__).resolve().parent - self.args = Parser().parse() + self._basedir = pathlib.Path(__file__).resolve().parent + self._args = Parser().parse() - if self.args.action == "refresh": - if not shutil.which(self.args.lcitool): + if self._args.action == "refresh": + if not shutil.which(self._args.lcitool): sys.exit("error: 'lcitool' not installed") - def make_run(self, target): + def _make_run(self, target): args = [ "-C", - self.basedir, + self._basedir, target, ] - if self.args.action in ["build", "test", "shell"]: + if self._args.action in ["build", "test", "shell"]: args.extend([ - f"CI_ENGINE={self.args.engine}", - f"CI_USER_LOGIN={self.args.login}", - f"CI_IMAGE_PREFIX={self.args.image_prefix}", - f"CI_IMAGE_TAG={self.args.image_tag}", + f"CI_ENGINE={self._args.engine}", + f"CI_USER_LOGIN={self._args.login}", + f"CI_IMAGE_PREFIX={self._args.image_prefix}", + f"CI_IMAGE_TAG={self._args.image_tag}", ]) - if self.args.action in ["build", "test"]: + if self._args.action in ["build", "test"]: args.extend([ - f"CI_MESON_ARGS={self.args.meson_args}", - f"CI_NINJA_ARGS={self.args.ninja_args}", + f"CI_MESON_ARGS={self._args.meson_args}", + f"CI_NINJA_ARGS={self._args.ninja_args}", ]) if pty.spawn(["make"] + args) != 0: sys.exit("error: 'make' failed") - def lcitool_run(self, args): - output = subprocess.check_output([self.args.lcitool] + args) + def _lcitool_run(self, args): + output = subprocess.check_output([self._args.lcitool] + args) return output.decode("utf-8") - def lcitool_get_hosts(self): - output = self.lcitool_run(["hosts"]) + def _lcitool_get_hosts(self): + output = self._lcitool_run(["hosts"]) return output.splitlines() - def generate_dockerfile(self, host, cross=None): + def _generate_dockerfile(self, host, cross=None): args = ["dockerfile", host, "libvirt"] - outdir = self.basedir.joinpath("containers") + outdir = self._basedir.joinpath("containers") outfile = f"ci-{host}.Dockerfile" if cross: @@ -204,27 +204,27 @@ class Application: outfile = f"ci-{host}-cross-{cross}.Dockerfile" outpath = outdir.joinpath(outfile) - if not self.args.quiet: + if not self._args.quiet: print(outpath) - output = self.lcitool_run(args) + output = self._lcitool_run(args) with open(outpath, "w") as f: f.write(output) - def generate_vars(self, host): + def _generate_vars(self, host): args = ["variables", host, "libvirt"] - outdir = self.basedir.joinpath("cirrus") + outdir = self._basedir.joinpath("cirrus") outfile = f"{host}.vars" outpath = outdir.joinpath(outfile) - if not self.args.quiet: + if not self._args.quiet: print(outpath) - output = self.lcitool_run(args) + output = self._lcitool_run(args) with open(outpath, "w") as f: f.write(output) - def refresh_containers(self): + def _refresh_containers(self): debian_cross = [ "aarch64", "armv6l", @@ -241,41 +241,41 @@ class Application: "mingw64", ] - for host in self.lcitool_get_hosts(): + for host in self._lcitool_get_hosts(): if host.startswith("freebsd-") or host.startswith("macos-"): continue - self.generate_dockerfile(host) + self._generate_dockerfile(host) if host == "fedora-rawhide": for cross in fedora_cross: - self.generate_dockerfile(host, cross) + self._generate_dockerfile(host, cross) if host.startswith("debian-"): for cross in debian_cross: if host == "debian-sid" and cross == "mips": continue - self.generate_dockerfile(host, cross) + self._generate_dockerfile(host, cross) - def refresh_cirrus(self): - for host in self.lcitool_get_hosts(): + def _refresh_cirrus(self): + for host in self._lcitool_get_hosts(): if not (host.startswith("freebsd-") or host.startswith("macos-")): continue - self.generate_vars(host) + self._generate_vars(host) - def action_build(self): - self.make_run(f&quot;ci-build(a){self.args.target}&quot;) + def _action_build(self): + self._make_run(f&quot;ci-build(a){self._args.target}&quot;) - def action_test(self): - self.make_run(f&quot;ci-test(a){self.args.target}&quot;) + def _action_test(self): + self._make_run(f&quot;ci-test(a){self._args.target}&quot;) - def action_shell(self): - self.make_run(f&quot;ci-shell(a){self.args.target}&quot;) + def _action_shell(self): + self._make_run(f&quot;ci-shell(a){self._args.target}&quot;) - def action_list_images(self): - registry_uri = util.get_registry_uri(self.args.namespace, - self.args.gitlab_uri) + def _action_list_images(self): + registry_uri = util.get_registry_uri(self._args.namespace, + self._args.gitlab_uri) images = util.get_registry_images(registry_uri) # skip the "ci-" prefix each of our container images' name has @@ -295,11 +295,11 @@ class Application: print("Available cross-compiler container images:\n") print(spacing + ("\n" + spacing).join(cross)) - def check_stale_images(self): - namespace = self.args.namespace - gitlab_uri = self.args.gitlab_uri + def _check_stale_images(self): + namespace = self._args.namespace + gitlab_uri = self._args.gitlab_uri registry_uri = util.get_registry_uri(namespace, gitlab_uri) - lcitool_hosts = self.lcitool_get_hosts() + lcitool_hosts = self._lcitool_get_hosts() stale_images = util.get_registry_stale_images(registry_uri, lcitool_hosts) @@ -328,15 +328,15 @@ class Application: """) print(msg.replace("STALE_DETAILS", stale_details)) - def action_refresh(self): - self.refresh_containers() - self.refresh_cirrus() + def _action_refresh(self): + self._refresh_containers() + self._refresh_cirrus() - if self.args.check_stale == "yes" and not self.args.quiet: - self.check_stale_images() + if self._args.check_stale == "yes" and not self._args.quiet: + self._check_stale_images() def run(self): - self.args.func(self) + self._args.func(self) if __name__ == "__main__": -- 2.30.2