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

Updated to support the new docker build --pull option #626

Closed
wants to merge 0 commits into from

Conversation

zaramckeown
Copy link

Within the docker build options there is now --pull. This enables an attempt to pull a newer version of the image always.

Additional information can be found here:
https://docs.docker.com/engine/reference/commandline/build/

@rhuss
Copy link
Collaborator

rhuss commented Nov 25, 2016

Thanks a lot for the PR !

I wonder a bit how this overlaps with the global configuration autoPull == always which also works for older versions. This works for both, the Dockerfile mode and the XML mode (see autoPullBaseImage()

It might make sense to move the global option to a more specific place within the build configuration to allow a more fine granular tuning of this mode.

Or do I misunderstand the purpose of --pull (its about the base image, or ?)

@zaramckeown
Copy link
Author

Thanks for pointing this out, the 'autopull=always' option, this meaning of pull always had been misunderstood. The pull option being part of the build command would allow for more granular tuning, as this could be now specified on a per image basis. In addition it enables the image pull to be done automatically as part of the docker build command therefore being done as part of a single docker request instead of multiple.

@rhuss
Copy link
Collaborator

rhuss commented Dec 20, 2016

Agreed that 'autoPull' is probably a bad name. Maybe a 'pullPolicy' would have been better (as it is used also for Kubernetes).

Agreed that it might make sense to have this on an per image basis. If so, I would like to keep the sematics and introduce an 'autoPull' option in the build config (even when its a bad name, but for backwards compatibility it make sense to keep it for now), where the build autoPull overrides any global autoPull and, based on the Docker API version in use, uses either a docker intrinsic --pull or the current version. If the later it too complicated I would stick to the current behaviour for backwards compatibility (and because I don't think that extra REST call is very harmful).

Would it possible if you could adapt the PR accordingly ?

@rhuss
Copy link
Collaborator

rhuss commented Apr 6, 2019

@rohanKanojia yet another PR, which I think would make sense, but where we didn't get any feedback later. Maybe you could put this on your, list, too ?

@rohanKanojia
Copy link
Member

crap, not again! Whenever I push to author's master PR gets closed :-(

@rhuss
Copy link
Collaborator

rhuss commented Apr 7, 2019

@rohanKanojia thanks, but to be clear, my intentation was not to introduce --pull as option to docker daemon, as we already do all the pulling of the base image on our own. Instead it was about introducing a pullPolicy as described in #626 (comment)

@rhuss
Copy link
Collaborator

rhuss commented Apr 7, 2019

BTW, I think one can reopen a PR if it has been closed by accident. Have to check that though.

@rohanKanojia
Copy link
Member

oh, I thought you wanted me to fix conflicts here.

@rhuss
Copy link
Collaborator

rhuss commented Apr 7, 2019

You can reopen a PR when you push a new commit to it. Not sure why it gets closed when you push. strange.

@rohanKanojia
Copy link
Member

When I push to their master branch, it gets closed. I get reopen button disabled.

Screenshot from 2019-04-07 14-11-12

@rohanKanojia
Copy link
Member

@rhuss : Are you talking about renaming autoPull to pullPolicy ? I'm not able to understand your comment properly. In your comment you're talking about autoPull option

@rhuss
Copy link
Collaborator

rhuss commented Apr 7, 2019

It's all about adding autoPull also to an image BuildConfiguration (which would override the global autoPull). Currently, it's a global parameter for all images, but it would be nice to enable it for single build configuration while keeping it disabled for others.

Tbh, I don't think this is a mission-critical PR if its hard to implement. We should still stick to our current behaviour to make an autoPull on our own instead of switching to --pull as the current solution works perfectly well.

@rohanKanojia
Copy link
Member

Hmm, well It was quite confusing for me. I don't think it's hard to implement since I just wasn't sure how to proceed.

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