From b953727881d1660d5b51af930f47843b0da00da4 Mon Sep 17 00:00:00 2001 From: Tully Foote Date: Fri, 24 Jan 2025 11:58:06 -0800 Subject: [PATCH 1/3] improve robustness of image cleanup --- src/rocker/core.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/rocker/core.py b/src/rocker/core.py index 03af51c3..756176e9 100644 --- a/src/rocker/core.py +++ b/src/rocker/core.py @@ -251,12 +251,24 @@ def docker_build(docker_client = None, output_callback = None, **kwargs): print("no more output and success not detected") return None -def docker_remove_image(image_id, docker_client = None, output_callback = None, **kwargs): +def docker_remove_image( + image_id, + docker_client = None, + output_callback = None, + fail_on_error = False, + force = False, + **kwargs): if not docker_client: docker_client = get_docker_client() - docker_client.remove_image(image_id) + try: + docker_client.remove_image(image_id, force=force) + except docker.errors.APIError as ex: + ## removing the image can fail if there's child images + if fail_on_error: + return False + return True class SIGWINCHPassthrough(object): def __init__ (self, process): @@ -421,7 +433,8 @@ def run(self, command='', **kwargs): def clear_image(self): if self.image_id: - docker_remove_image(self.image_id) + if not docker_remove_image(self.image_id): + print(f'Failed to clear image {self.image_id} it likely has child images.') self.image_id = None self.built = False From 4a6e19171765e417d211ec9a139a66987bca5645 Mon Sep 17 00:00:00 2001 From: Tully Foote Date: Fri, 24 Jan 2025 21:54:42 -0800 Subject: [PATCH 2/3] Don't automatically clean up a named image That makes the name useless --- src/rocker/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rocker/cli.py b/src/rocker/cli.py index e74791b7..70d2cfbc 100644 --- a/src/rocker/cli.py +++ b/src/rocker/cli.py @@ -69,13 +69,13 @@ def main(): exit_code = dig.build(**vars(args)) if exit_code != 0: print("Build failed exiting") - if not args_dict['persist_image']: + if not (args_dict['persist_image'] or args_dict.get('image_name')): dig.clear_image() return exit_code # Convert command into string args.command = ' '.join(args.command) result = dig.run(**args_dict) - if not args_dict['persist_image']: + if not (args_dict['persist_image'] or args_dict.get('image_name')): print(f'Clearing Image: {dig.image_id}s\nTo not clean up use --persist-images') dig.clear_image() return result From a6a31493178a8c5b396726e913bdf284e123117e Mon Sep 17 00:00:00 2001 From: Tully Foote Date: Sat, 25 Jan 2025 02:49:03 -0800 Subject: [PATCH 3/3] Read aux ID field to get image name if not in stream I'm not sure if this is a good thing to add it's undocumented and potentially fragile. I was trying it when looking at #309 for a root cause, but determined that the error was actually the build failing. So the image id detection wasn't necessary. The canonicalization though may be valuable to merge w/o the aux reading logic. --- src/rocker/core.py | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/rocker/core.py b/src/rocker/core.py index 756176e9..f51c286c 100644 --- a/src/rocker/core.py +++ b/src/rocker/core.py @@ -227,28 +227,55 @@ def get_user_name(): userinfo = pwd.getpwuid(os.getuid()) return getattr(userinfo, 'pw_' + 'name') +def canonicalize_image_id(image_id, docker_client=None): + if not docker_client: + docker_client = get_docker_client() + inspect_info = docker_client.inspect_image(image_id) + # print("image info ", inspect_info) + #TODO(tfoote) Consider canonicalizing this long instead of truncated especially with podman support + return inspect_info['Id'].split(':')[1][0:12] + def docker_build(docker_client = None, output_callback = None, **kwargs): image_id = None + aux_image_id = None if not docker_client: docker_client = get_docker_client() kwargs['decode'] = True for line in docker_client.build(**kwargs): + # Undocumented way to get image_id from alternative element from stream + # example seen {'aux': {'ID': 'sha256:e328328fb3297a7c0f6cc5c2bf460d976f8e69e87a0706e6b8a211f42afa025c'}} + found_aux_image_id = None + found_aux_image_id = line.get('aux', {}).get('ID') + if found_aux_image_id: + aux_image_id = found_aux_image_id + output = line.get('stream', '').rstrip() + + #Reset aux_image_id if a new layer is detected + if aux_image_id and output.startswith('Step '): + aux_image_id = None + print("resetting aux id for new step: ", output[0:15], "...") + if not output: # print("non stream data", line) continue if output_callback is not None: output_callback(output) + # Fallback detect image_id via string matching + if not image_id: + match = re.match(r'Successfully built ([a-z0-9]{12})', output) + if match: + image_id = match.group(1) - match = re.match(r'Successfully built ([a-z0-9]{12})', output) - if match: - image_id = match.group(1) + # Fallback to aux based image id if found + if not image_id and aux_image_id: + image_id = aux_image_id if image_id: - return image_id + return canonicalize_image_id(image_id, docker_client=docker_client) else: - print("no more output and success not detected") + print('Docker Build Failed: Output stream from docker_build finished but no image_id detected.') return None def docker_remove_image(