Default should be against latest version

[txomon]

If it's not recommended to clean config d dir, the default should be False

[iggy]

Good luck with that. I fought it in the first place (and the only reason there's even an option).

[puneetk]

This is good, 2015.0 adds a _schedule.conf to the /etc/salt/master.d directory which we shouldn't remove.

[aboe76]

@puneetk the _schedule.conf is already in a exclude_pat in the file.recurse of the salt.master:
see line 13 in master.sls

    - exclude_pat: _*

[aboe76]

@txomon where did you find the recommendation that config d dir shouldn't be cleaned?

[iggy]

This just goes to show you that the salt-formula erasing config files it didn't create in the first place flies in the face of the principle of least surprise.

[puneetk]

@aboe76 not sure why this is done, but ok

[aboe76]

@iggy, the reason why this modules cleans /etc/salt/master.d is you want salt to configure salt, than there shouldn't be any other files that could potentially change the configuration in any way that the salt-formula dictates...because that is what you are doing when you are using the satl-formula.

Leaving anything behind in /etc/salt/master.d/ that has alphabetically order later that f_defaults.conf overrides anything that's in f_defaults.conf.

[iggy]

I didn't ask why, I merely pointed out that a user tried to use this formula and was shocked that the formula erased files that it didn't create (and by some accounts had no business touching).

[iggy]

P.S. if your goal is to start from a bare config, the formula should remove /etc/salt/{master,minion} too...

[txomon]

I totally agree with @iggy, I expected to write /etc/salt/{minion,master} and leave all the *.d for me

[txomon]

BTW, the recommendation is in the pillar.example:

https://github.com/saltstack-formulas/salt-formula/blob/master/pillar.example#L2-L4

[aboe76]

@iggy and @txomon thanks for pointing this out.

So can we get a consensus what this module should be doing, my thoughts on this is that /etc/salt/master and /etc/salt/minion shouldn't be touched at all and be left as the package provides, that way any upgrade and update on the config file works by just installing the package, and if you wan't to override something you add a file or put it in /etc/salt/master.d or in /etc/salt/minion.d as a conf file.

that is what the module was designed for in the first place, by allowing the minion and master files to be clean and pristine from package and just override in the config d dir.s

[txomon]

@aboe76 I think the difference here between managing .d and leaving the root one and the opposite would be to know what the package does.

If the package leaves a totally uncommented file at /etc/salt/{master,minion} then I would agree with you that we shouldn't touch that file and manage /etc/salt/{master,minion}.d/.

If the package leaves a partially uncommented file, that the user will have to modify to make it work, then I would manage /etc/salt/{master,minion} inserting the same comments as they are, as there will only be one configuration file.

The selection choice would be done with grep -v -e '^$' -e '^#' /etc/salt/master.

However, if this is the first case (which seems to), the current config is already explained at the commented file, so the managed file should be really clear on what it does. I mean at the very least #131.

Obviously, how the formula works should be documented (we manage X you can override like Y), so that the user, trying to save time, doesn't have to read all the code and execute it till he realizes what's happening.

[andrew-vant]

I think I'm the one that originally implemented this, in #51. My reasoning for clean: true at the time was that I didn't want to cause a situation where the administrator removed a minion.d file from the salt tree, only to find that it was left in place on the server after the next highstate application (which, to me, would be surprising and unwanted behavior).

I'm no longer sure if that was the best way to handle it. I want clean: true for all my deployments because I am entirely managing my config with salt-formula, but I can see the argument against it being the default.

[txomon]

I have been thinking more on this. And I have just changed my opinion on to the opposite.

As sysadmins, we will be modifying the /etc files just because they are the main configuration files, and they just add noise. It is easier to manage a etc folder with just the files you need than boilerplates and the precise configuration files.

In this case we are "lucky" because we can override /etc/salt/master with the .d, but not all the programs have such ability, and no body doubts about modifying a packaged /etc file. Also there is another reason that slipped my previous idea development.

Package provisions the config file so that you know what is to be configured. If you happen to change it, it will give you a warning. If we now override through a .d, we won't notice if the packaged file is modified!

I thereby think that we should go back to the /etc/salt/{minion,master} deployment, but do not make clean: True the default, that's up to each sysadmin.

[iggy]

Did we ever come to a consensus? This needs to be rebased either way.

[aboe76]

@iggy I don't mind the change in defaults, to clean => False, as long as the option remains, to turn it back on .

[iggy]

Yeah, definitely leaving the option, I've already hacked and slashed all my code to deal with it not even having an option (and defaulting to cleaning).

[puneetk]

@txomon can you please rebase, looks like using an option to clean or not is how we are doing it. thanks

[txomon]

The problem here is that you convinced me, and the default should be True.

I now believe that if something is managed by salt, no default files should remain, for example, /etc/salt/{master,minion} should be written by salt, and that .d dirs should be cleaned by default...

Opinions on this appreciated!

[txomon]

The thing here is to try to have uniformity. What do other formulas do with all the default config files? If those config files are overwritten (which is the case for apt etc.). Then this formula should overwrite /etc/salt/{minion,master}. Else, we don't have a good guideline.

Is there anything in the docs on saltstack? I don't remember, but maybe this would be a good addition.

[nmadhok]

There's no point arguing about this. Some people think it should clean all configs and some people think they should remain. The purpose of salt-formula is to install salt related packages in the first place and configure it which is why the default is True to clean custom config files which could come in the way of salt formula installation. Closing this PR.