Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[docker] fix image name when using sha256 for specs #3326

Merged
merged 2 commits into from
May 3, 2017

Conversation

xvello
Copy link
Contributor

@xvello xvello commented May 2, 2017

What does this PR do?

Some orchestrators init containers with the image sha256 instead of the
image name, and docker inspect returns this. If it's the case, make
a docker image inspect call to get the image name and cache the result

No cache invalidation is coded as image deletion don't trigger events and obsolete image count should be significant on a typical host

Motivation

Compatibility with k8s 1.6 & Nomad

Testing Guidelines

Added unit test for the three cases we support

@xvello xvello requested a review from hkaj May 2, 2017 13:04
@xvello xvello changed the title fix docker image name getting when using sha256 for specs [docker] fix image name when using sha256 for specs May 2, 2017
@xvello xvello added this to the 5.13.1 milestone May 2, 2017
Some orchestrators init containers with the image sha256 instead of the
image name, and docker inspect returns this. If it's the case, make
a docker image inspect call to get the image name and cache the result
@xvello xvello force-pushed the xvello/sha_to_image_name branch from 60bb7fe to e0655cf Compare May 2, 2017 13:20
Copy link
Member

@hkaj hkaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor comment on exception handling, otherwise LGTM

name = image_spec.get('RepoTags')[0]
self._image_sha_to_name_mapping[image] = name
return name
except Exception:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This exception handling is risky. Either we expect a precise error and we should only catch that, or we catch everything with Exception but log something. Otherwise we open the door to sneaky bugs.

name = name.split('@')[0] # Last resort, we get the name with no tag
self._image_sha_to_name_mapping[image] = name
return name
except Exception:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diitto

return name
except Exception:
pass
except Exception:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@xvello xvello force-pushed the xvello/sha_to_image_name branch from 0c7734a to 48d774e Compare May 2, 2017 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants