Add option to Docker provisioner to build local images

[jpfuentes2]

Option would work like so:

Vagrant.configure("2") do |config|
  config.vm.provision "docker",
    images: %w( ubuntu ),
    build_images: [
      { path: "Dockerfile", args: "-t me/my_app" }
    ]
end

Please be aware this is my first time hacking on Vagrant and I was kind of rushed : )

I didn't see any tests regarding this provisioner on #2549. Please let me know if I need to add something.

Also, is there documentation other than the README for hacking on Vagrant? I'm wondering what the normal work flow is for testing this feature I added. Would I just create a Vagrantfile with my new feature and run bundle exec vagrant up?

Thanks!

[fgrehm]

@jpfuentes2

Thanks! :) I was actually going to work on that but you've beat me to it, so I'm 👍 for the feature and I'm assigning it to myself for review. I'll try to have a look at it tomorrow but I'm not 100% sold on that DSL yet (see below for more).

As per testing the provisioner, there was originally some tests for it back in vocker but I decided to drop them since I knew the API would change and I was using another mocking framework over there. But will work on backporting them as soon as I have a chance.

re: Hacking on Vagrant, I think the README is all we've got for now, if someone else has any other pointers please raise your hand :P /cc @phinze @tmatilai @gildegoma

@mitchellh do you have any thoughts on the feature / DSL? This is what I had in mind when I first thought of this feature:

config.vm.provision "docker" do |d|
  d.build_image 'image:optional-tag', '<full path to Dockerfile on guest>', 
                                      args: '<build args>'
end

WDYT? (@jpfuentes2 feel free to share your thoughts too :)

[fgrehm]

@jpfuentes2 before I forget, this variable doesn't seem to be used :)

[jpfuentes2]

Nice catch! I went through a few iterations and decided there was no need for a generic options hash. I'll remove it.

[jpfuentes2]

I'm 👍 on your suggested DSL. It seems like Vagrant prefers multiple method calls instead of passing an array so I think your plan is superior. It concisely represents common docker build usage and inclines users to always name their images which is something I do regardless.

[fgrehm]

Yeah, the idea of having the image tagged at all times is good to avoid having untagged images laying around and to allow us to do configuration merging from within Vagrant with less pain as they'll be stored as a Hash on the config object :)

So, I slept on this and figured out we probably need to do one more thing before bringing it in that is check whether the image is already available and skip building so that the provisioner is idempotent. On the best case scenario this doesn't represent an issue because Docker will reuse cached images, but if we have an ADD instruction along the way for example, further instructions won't be cached and a new image will be built on every vagrant provision run. Does that make sense?

Apart from that I think we should be good to go :)

@mitchellh and others, are you guys up for bringing this in on 1.4.x?

[jpfuentes2]

check whether the image is already available and skip building so that the provisioner is idempotent

When using Chef/Puppet provisioners I can make changes to the configuration of those systems, run vagrant provision and see my new changes applied immediately without having to vagrant ssh and do manual invocations in the VM. If we skip building existing images then I would be forced to bump the tag version of the image to force the build. I'm not sure that would be expected behavior, would it?

It was my understanding that Vagrant stopped provisioning on every vagrant up because some people use Shell provisioners and those are generally not idempotent. Implied here is that Vagrant presumes non-idempotent provision behavior by default. However, users can vagrant provision to their hearts desire afterwards respective of configuration updates.

[fgrehm]

@jpfuentes2 Yeah, it makes sense but I'm still on the fence... gonna have to think about it a bit more.

We just need to be consistent at least, if we end up building images on every provisioner run, we should also pull them on every run as well instead of skipping it as we currently do.

ping @mitchellh

[goshacmd]

👍 on this feature with @fgrehm's suggested DSL.

[mitchellh]

LGTM. Great job!

[mitchellh]

I'm updating it with the newer DSL now.

[fgrehm]

@mitchellh I noticed you ended up dropping the second parameter for always tagging the image that I proposed in favor of the args + -t option when you updated to the new DSL =(

Was that intentional?

[mitchellh]

I think so, thinking back I think it was because you can just use "args" to do that. Thoughts?

[fgrehm]

Well, it depends on what are your thoughts on #2901, if we agree on building the image only if doesn't exist, I'd say it'll be nice to have the parameter in place so we can perform the checking.

Either way, as @jpfuentes2 pointed out above, I also believe it represents the most common usage for docker build :-)

[jpfuentes2]

Yeah, I think it's worth considering my comment here: #2615 (comment)