vmware-archive / salty-vagrant

Use Salt as a Vagrant provisioner.
BSD 2-Clause "Simplified" License
372 stars 62 forks source link

Enhancement: add config option for masterless #64

Open kamalmarhubi opened 11 years ago

kamalmarhubi commented 11 years ago

I brought up a related idea (separate provisioner for masterless) in the vagrant-salt-natively issue.

My motivation is that a common use case for vagrant is to use it with baked-in config (ie without a CM server somewhere). This is made very straightforward for Chef and Puppet, but in the simplest case for Salt will require creating a single-line (file_client: local) minion config file, and adding a line in the Vagrantfile to point to it.

My initial suggestion was to add a masterless provisioner, mirroring the setup that Chef and Puppet have.

@akoumjian's comment:

I'm not sure I really like that design. I would rather not make any assumptions about people's setups (as many do not user masterless). I would rather have lots of documented examples for different configs, rather than building a separate provisioner for what is essentially just a flag in a config file.

I agree with this—a separate provisioner is silly. However, I still think the common use case of masterless could be streamlined a bit. My thought: add a salt.masterless config option that ensures that file_client: local is in the minion config. This includes creating an otherwise empty minion config if none is given.

I think the existence of the accept_keys flag supports this addition, as that too is ‘just a flag in a config file’, yet warrants a convenience option in the vagrant configuration.

akoumjian commented 11 years ago

I do want to make getting started as easy as possible, so I'm considering this. However, your last comment is incorrect as the accept_keys flag is simply to run salt-key -A imperatively, not to change the auto_accept feature in the master config.

My main hesitation comes from not wanting to get into the config templating and editing business. The road can be difficult to maintain, and hard to anticipate what users will give you. It will be confusing when users provide their own minion configs, because then the options are that A) they forget to set file_client: local and don't understand why it's not working anymore or B) we modify a file they are putting in without their knowledge, possibly causing unexpected results.

The beauty of the way it works now is that "it's just salt", might like salt "is just python".

My solution so far has been trying to provide example folders of Vagrantfiles and minion configs, along with trying to point out important options in the docs.

One alternative would be to re-add the salt.masterless config and in that cause just as the --local option to the salt-call command for highstate. However, once again the user will be confused when they run salt-call manually and it breaks looking for a master.

kamalmarhubi commented 11 years ago

I do want to make getting started as easy as possible, so I'm considering this.

Yeah, my motivation comes as someone who picked up salt and vagrant at the same time. I don't like Chef being an EDSL—this is one place where I don't think configuration should be code. I played with Puppet, but it seemed overly complicated for what I wanted, so I hunted for alternatives. I prefer the salt state setup in general, with a minor thought that salt+vagrant could be slightly smoother.

If this is going to be merged in to the default vagrant install, I was thinking that smoothness might be appreciated by other people, who like me, could be coming to salt by way of vagrant rather than the other way around.

However, your last comment is incorrect as the accept_keys flag is simply to run salt-key -A imperatively, not to change the auto_accept feature in the master config. [...] My main hesitation comes from not wanting to get into the config templating and editing business. The road can be difficult to maintain, and hard to anticipate what users will give you.

My bad, I hadn't looked at how the accept_keys flag worked. Given that, then yeah messing with config might be bad. Firmly in theory-land (ie, no thoughts about implementation concerns), let's imagine the scenarios:

file_client / salt.masterless absent false true
absent remote remote local
remote remote remote local
local local ??? local

(Entries are minion config's file_client and Vagrantfile's salt.masterless config flag, with the table value being the file_client value in the minion config that is actually placed in /etc/salt/minion.

I'm imagining here that the entry in the Vagrantfile overrides the minion config provided, if any. It could warn if it's overriding the minion config. I think this is surprise-free except for the ??? entry: the file_client in the provided minion config is set to local, but the Vagrantfile's salt.masterless is explicitly set to false. I can see this one going either way, or perhaps it would be best to error out.

Thoughts?

Caustic commented 11 years ago

Are you still considering this? I would very much appreciate a configuration option myself. I understand your reasoning for having the user explicitly include it in the minion config file, but I would guess that most people running a masterless setup pass in the --local option to salt-call anyways instead of defining it in their minion config.

Salt's official docs for Masterless Quickstart doesn't even mention the file_client: local option.

Furthermore, it enables users to have the same minion configuration file for vagrant and production if they so desire.

akoumjian commented 11 years ago

I could see the use for a flag which simply adds the '--local' string to salt-call, on the automatic highstate run. However, the minion service is still going to be looking for a remote master and will produce a bunch of errors in the log. Then there's the task of educating the new user that if they want to run any salt commands, they need to be using the --local flag as well.

I'm going to draw the line at editing / templating config files. Better examples and better documentation can make this really painless.

I will update the quickstart guide to include a reference to the file_client option, since that would be one of the 'next steps'.