[salt] [bugfix] Restablize Salt provisioner

[jcockhren]

Hey all! This PR answers #6276. For the brave/bored, feel free to drill down through the referenced issues and PRs.

For the everyone else, this PR reverts the show stopper bug introduced in #5435 and the work done on top of it in #5936. My goal was to simply bring the salt provisioner back to a working state. The issue seeming to involve vagrant using the Salt dummy config file instead of the file set with the minion_config setting.

The Good

At provision time, the user supplied minion_config is copied to the inside the vagrant VM as you would expect. Therefore, the provisioner no longer hangs at "Bootstrapping Salt... (this may take a while)". This unblocks the Salt community. 🎊

The Bad

There were no provision tests for the salt provisioner to begin with. For the past 2 weeks, I've attempted to add some testing around the provisioner, however I've been unsuccessful. 😞 I would like the time/space to talk about why testing around the salt provisioner can not leverage the approach used in the ansible, shell and chef provisioners, but for the sake of brevity let's make that a separate issue, if that's 🆒.

@mitchellh @timoguin @cachedout @thatch45 @wkral

[timoguin]

@jcockhren and I have talked about this quite a bit and think this is the right direction. It seems the functionality I added was already there, but it wasn't clear in the documentation.

[wwentland]

How does this relate to #6073 ?

[jcockhren]

@BABILEN
So people get the context, #6073 addresses #5973.

TBH, the errors thrown when this provisioner vomits, are misleading. The error shown in #5973 stopped occurring on Debian Jessie with this PR as well. In fact, that error only appeared for me when using a debian/jessie base box but not ubuntu/trusty. Therefore this PR and #6073 seem related.

This PR fixes the bug introduced with the configuration. If the configuration vomits, then this provisioner uploads a dummy config file. Basically never considering the value of minion_config.

[wwentland]

Thanks for the explanation!

[johntron]

Now we just need @sethvargo or @mitchellh to finally merge one of these PRs so we can finally close #6382, #6276, #5973, and #6073 and avoid all the confusion.

[wwentland]

What prevents this PR from being merged? Anything we can do to solve any potential issue?

[mitchellh]

I believe this is fixed by #6073. Can someone advise on if this is different in any way? I believe this is fixed. Sorry for the delay.

[timoguin]

@mitchellh, this is related pretty closely to #6073 but it fixes a different issue introduced in 1.7.4 by yours truly in PR #5435.

It should be merged as well.

[jcockhren]

@mitchellh

1. Stop trying to upload minion config to privileged directory #6073 forces every single user of the salt provisioner to have the setting salt.bootstrap_options = "-P -c /tmp" in order to use it. Without that option, the provision step for all Vagrantfiles up until 1.7.2 will still hang when attempting to vagrant up from scratch. I know this because I've tested @johntron's branch from his fork using the following simple Vagrantfile using the salt provisioner:

Vagrant.configure(2) do |config|
  config.vm.box = "ubuntu/trusty64"
  config.vm.box_check_update = false
  config.vm.synced_folder "salt/states/", "/srv/salt/"

  config.vm.provider "virtualbox" do |vb|
    vb.memory = "1024"
  end

  config.vm.provision :salt do |salt|
    salt.minion_config = 'salt/minion'
    salt.run_highstate = true
    salt.install_args = 'git v2015.5.5'
    salt.verbose = true
  end
end

The result being, vagrant hangs during provisioning when attempting to run highstate. (note, I haven't included the state and config). Here's a gist containing the supporting files needed to test @johntron 's PR
2. Now use the same gist to checkout out my PR and you'll see that highstate successfully completes.

[mitchellh]

Got it, I'll merge this one as well, does that fix it @jcockhren

Sorry we don't have anyone here maintaining or testing Salt on a daily basis we rely on the community, so I apologize for my ignorance on the issue.