5:54 a.m.
On Thu, 2021-03-18 at 09:09 +0100, Erik Skultety wrote:
+ def check_stale_images(self):
+ if self.args.check_stale != "yes" or self.args.quiet:
+ return
I would prefer it if this check were to be performed in the
action_refresh() method to decide whether or not check_stale_images()
should be called.
+ print(f"""
+The following images are stale and can be purged from the registry:
+{stale_details}
+
+You can remove the above images over the API with the following code snippet:
"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
This will not result in a working shell snippet being displayed. What
you want is
f"""
$ 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""")
Empty line before the URL for readability.
I still think that we should use textwrap.dedent(), as it makes the
script much more readable at the cost of a single additional
string.replace() call:
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):
+ # refresh Dockerfiles and vars files
self.refresh_containers()
self.refresh_cirrus()
+ # check for stale images
+ self.check_stale_images()
The method names are really quite self-explanatory, so the comments
you're introducing don't add much IMHO... I'd say leave them out.
+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)
+
+ # extract distro names from the list of registry images
+ registry_distros = [get_image_distro(i["name"]) for i in images]
+
+ # - compare the distros powering the images in GitLab registry with
+ # the list of host available from lcitool
+ # - @unsupported is a set containing the distro names which we no longer
+ # support; we need to map these back to registry image names
+ unsupported = set(registry_distros) - set(supported_distros)
+ if unsupported:
+ stale_images = {}
+ for distro in unsupported:
+ for img in images:
+ # gitlab images are named like
"ci-<distro>-<cross_arch>?"
+ if distro in img["name"]:
+ stale_images[img["name"]] = img["id"]
+
+ return stale_images
As far as I can tell, this can be achieved in a much more
straightforward way with
def get_registry_stale_images(registry_uri, supported_distros):
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
At least from a quick test, the results appear to be the same. Am I
missing something?
--
Andrea Bolognani / Red Hat / Virtualization