-
Notifications
You must be signed in to change notification settings - Fork 614
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ALL postgresql.conf options should be puppeted #26
Comments
Ahhh.... I knew this day would come. :) So, the options I can see for this are to add more of the "file_line" parameters, or to switch over to using a fully-templated copy of the postgresql.conf file. The main reason I didn't do a template from the outset was that there are differences between the contents of this file for different versions of postgres and for different OS distributions, so I was worried about coming up with a template that would be compatible with all of them. I felt like the "file_line" approach was a bit safer. Do you have some specific concerns about its scalability? I am definitely in favor of moving forward on this front, and have to some degree just been waiting for someone to bring up the issue so that I could get their thoughts on it. Any particular suggestions? |
Today I was looking for a way to manage the write ahead log settings, and I stumbled upon a fork where the owners introduced a custom puppet type to manage the config file. Maybe you could ask them to lend you their solution? :) |
Thanks for the heads up, I will definitely ping them! |
Hi, i just read a post on planet postgresql about managing the conf file programmatically... this should be easy to implement and supported for nearly every pg version... |
I think I found the link you are referring to http://momjian.us/main/blogs/pgblog/2012.html#December_7_2012 So would we make postgresconf.d directory and have puppet put all the parameters there? This sounds like it might be the best solution. The puppet type would be interesting too, but I'm all for choosing one and implementing it as soon as possible. This is the last major feature blocking me from deploying it in my environment. |
There's something I don't quite get about this solution: the included file still won't be puppetized (or am I wrong?), so when you want to change something in there, don't you have the same problem? |
@ak0ska I think that included file would be puppetized; it could be done either via this module or via a normal puppet file resource. The latter might be a simpler first step, and support for the former could be added later. This would also help to prevent us from having to muck around too much with the default config files that ship with the various distros. I think I'm liking this idea. It might be as simple as adding an extra "file_line" resource to beforeservice.pp. My only real concern would be to make sure that versions of pg back to 8.1 support this "include" feature, and if not, we need to make sure the module can handle it accordingly. I'm not sure when I will have time to work on / test this stuff, but if any of you are inclined to submit a pull request I would be interested in helping get it into the code! |
From http://www.postgresql.org/docs/8.2/static/config-setting.html it seems the Also, versions 8.2 and 8.1 are no longer supported at all upstream. Does this Puppet module really need to be compatible with these old versions? |
Well, we are in the sticky spot of dealing with the fact that RHEL5 is still in use and still defaults to 8.1. However, I think it would be fine to say that this particular feature isn't supported in 8.1, and the more that I think about it, the more that I think that this 'include' approach is the best, most flexible solution. So at this point I'd accept a pull that added support for this... or I will probably add it myself next time I get some time to work on this module. Thanks for looking that up! |
The guys over at https://github.com/hercules-team/augeas are busy working on a new Augeas release (there hasn't been one in over a year, I think), and have many new lenses, including one for Until then it is probably easiest to use the |
Agreed. The 'include' seems like a great, easy solution for the time being. We could get fancier later if we wanted to but even then the include could still work and could still provide value. |
FYI, I have a branch that basically implements the https://github.com/cprice-puppet/puppet-postgresql/tree/feature/master/adding-include-to-pg-conf This wouldn't necessarily preclude the idea of doing something more fancy (augeas etc.) in the future. |
Sweet, will this be merged soon? I can't wait to stop shipping flat files. :) |
If people don't think that looks too horrible ( @kbarber ?) then we can probably get it merged pretty soon here. We need something like this for Puppet Enterprise soon anyway, so I expect we'll have someone reviewing this and deciding whether to merge it within the next week-ish at the latest. If this doesn't get merged then we will have to get something else in to provide this functionality because it's blocking us :) |
Note that the "include" functionality has now been merged ( #105 ). I'm leaving this issue open because I still think it's worth discussing a more robust solution, but I'm hopeful that this short-term solution should be forward-compatible. |
@cprice-puppet We should totally work on something whereby all options can be passed to postgresql.conf, a template hash -> option pattern could be used here for example to give us unlimited coverage. Of course validation then becomes a problem, but we could solve that in many ways. Let me change the title to cover that kind of scope. |
Agree! |
Fixed in 3.x |
In postgresql.conf, there's a number of settings (ssl, max_connections, data_directory,...) that need to be changed in our environment, but the only one that can be puppeted is listen_addresses, which is done through the (non-scalable) file_line in beforeservice.pp.
How should adding on to the changeable options for postgresql.conf be handled?
The text was updated successfully, but these errors were encountered: