Added image tag option #204
Conversation
| @@ -76,17 +76,15 @@ def pull(self, image_object): | |||
| except IndexError: | |||
| self.logger.error('Malformed or missing tag. Skipping...') | |||
| raise ConnectionError | |||
| if self.config.latest and image.tags[0][-6:] != 'latest': | |||
|
|
|||
| if self.config.image_tag: | |||
There was a problem hiding this comment.
If i am not mistaken, image.tags[0] is ALWAYS the tag used to run the image. This is our design intentionally. Random tags make it an unreasonable automation and it might be better suited for you to use a shared tag as your run tag that has updates. This is all assuming that my initial statement is true
There was a problem hiding this comment.
This was not my experience. For me, it was the last tag that was running. I had to untag the others before updates worked. While that isn't a big deal, it's an attempt to make the behaviour less random and more deterministic / explicit.
Regardless, this is merely an extension to the 'latest' feature; if that exists, I don't see a reason why it wouldn't be made slightly more flexible. If you only have the option to access this feature with the 'latest' tag, you can only use this on a single environment, but in my use case, I would like to use it on two (development and staging).
I would in fact argue that setting an image tag makes it less random, as I don't think image.tags[0] is reliable in the sense that I couldn't find any documentation that specifies how it is supposed to work. In other words, there don't seem to be any guarantees its behaviour will be backwards compatible.
There was a problem hiding this comment.
image.tags[0] is not reliable with multiple tags (I have just tested). You are correct. but container.attrs['Config']['Image'] Gives the exact tag used to run and that IS what our goal was. Im going to make a new pr with the above change.
There was a problem hiding this comment.
Up to you of course. I do think it is inconsistent to leave in the 'latest' feature.
More importantly however, in your change, you did not address the neglected else in the pull method. The elif's are not exhaustive, causing an issue later on where the function returns None instead of raising an exception. I've seen this happen when trying to pull an image from gitlab.com; the unauthorized check didn't work.
There was a problem hiding this comment.
tbh im not a fan of latest existing at all. That is left from legacy code. Ill talk to the devs about removing it. As for the bug you mentioned, please capture the logs surrounding it so i can get more details and make an issue request for it and ill get it cleaned up :)
|
The pr i just made should resolve this |
I found that image.tags[0] in dockerclient.Container.pull() doesn't always return what I want. In my situation, I had a container that had a number of tags (one for every commit, as I tag them with the git commit hash), as well as the last and most actual tag (which was 'staging', because I switched to tagging my releases with a more readable tag halfway during implementation of ouroboros). This however caused the container to never update, as it picked the first tag always, which didn't make sense for me anymore.
This is most probably not an issue for most people. However, I can imagine that people (myself included) wouldn't mind an option to just set the tag to whatever they want. Hence this pull request.
Furthermore, while testing I ran into some issues, especially one that caused the latest_image to be None in dockerclient.Container.update() due to an unhandled ApiError when pulling. So I proposed a fix for that as well.