search

saltstack-formulas / haproxy-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
28 stars 122 forks source link

Minor improvements #10

Closed boltronics closed 9 years ago

boltronics commented 9 years ago

I found the formatting of the haproxy.jinja template file to output lots of unnecessary whitespace, and added debug comments that also didn't begin on a new line. Seemed like a good idea to remove that.

Also, I found the /etc/default/haproxy ENABLED setting doesn't always exist. eg. Debian Wheezy and Jessie (at least) don't have it, and don't honour it even if they do. Hence I switched to using file.replace instead of file.managed. I haven't tested this much, but I believe it should work on Ubuntu or whatever honours ENABLED - if it doesn't already exist (even in commented form), Salt doesn't do anything here now. The haproxy-init-enable and haproxy-init-disable files should now be redundant and can be deleted.

So lastly, I implemented a solution to stop haproxy (and disable the service at startup) via the service state module, if haproxy:enable pillar data is set to False. That seems a more standard and compatible way of managing this to me.

If you deem any of these changes useful enough to pull, you can consider them licensed under the existing Apache License version 2.0. Cheers.

nmadhok commented 9 years ago

@boltronics Rebase needed

boltronics commented 9 years ago

It wasn't needed back when I posted this 9 days ago...

boltronics commented 9 years ago

It's late here. I'll look at it tomorrow.

gravyboat commented 9 years ago

@boltronics Yeah sorry for the delays on getting this merged. We're all just volunteers that do this when we have time so we occasionally miss things or merge things in the wrong order. If you rebase we should be able to get this in, the changes look good!

boltronics commented 9 years ago

Thanks @gravyboat. I understand.

Looks like my ddc5b79 was added as a part of c00502a two days ago, so it has been dropped.

1ffa423 is new and (like b4ee8e8) optimises the formatting for the resultant haproxy.cfg (as opposed to the jinja file itself). For the Pillar data I tested with (not a huge variety, admittedly), it looks more consistent IMO, with all category headings prefixed with two new lines.

#------------------
# some header
#------------------
config
    more config
    even more config

#------------------
# another header
#------------------
config
    more config
    even more config
gravyboat commented 9 years ago

Looks good to me, thanks!