Skip to content
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

Enable the module to manage entries in $confdir/config.ini #176

Merged
merged 1 commit into from
Apr 22, 2015
Merged

Enable the module to manage entries in $confdir/config.ini #176

merged 1 commit into from
Apr 22, 2015

Conversation

buzzdeee
Copy link
Contributor

@buzzdeee buzzdeee commented Apr 9, 2015

with regard to the command-processing section.

Added new class server/config_ini.pp to manage contents of the config.ini.
Three new parameters added:

  • command_threads
  • store_usage
  • temp_usage

All three default to 'undef'. This makes sure (potential) custom settings
done to that file with regard to above three variables are 'absent',
and let PuppetDB built-in defaults take care.

Documentation to the README.md added, as well as unit tests.

My use-case was, that I have on some nodes a too small /var partition,
so I had to lower the values of store-usage and temp-usage in the config.ini
manually.

@wkalt
Copy link
Contributor

wkalt commented Apr 16, 2015

this looks good to me

@@ -55,6 +55,9 @@
$manage_firewall = $puppetdb::params::manage_firewall,
$java_args = $puppetdb::params::java_args,
$max_threads = $puppetdb::params::max_threads,
$command_threads = $puppetdb::params::command_threads,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these all default to undef, you could probably just explicitly set them to undef here.

@ajroetker
Copy link
Contributor

👍 aside from the undef comment which I could live without!

@buzzdeee
Copy link
Contributor Author

I added a file resource to the server/config_ini.pp file to take care that it exists before trying to manipulate it. On OpenBSD, its owned by root:wheel, however, that is usually the user puppet runs on.
I was unsure, if I should explicitly manage the owner/group of the file. i.e. owner => root, group => 0.
since the permissions allow everyone to read, i.e. 0644, so puppetdb should be fine with it.

I kept the parameter definitions to undef in param.pp, because they are passed in in init.pp, and server.pp. If everyone ever wants to change them, it can be done centrally in init.pp, so I hope that is fine.

Unfortunately, the travis tests currently fail, because of updates/changes in the "concat" modules git repo. I didn't wanted to mess with the specs, therefore I haven't done anything in that regard ;)

@kbarber
Copy link
Contributor

kbarber commented Apr 20, 2015

Looks like we pull in another dependency for concat now, it wraps a native tool from electrical now: https://github.com/puppetlabs/puppetlabs-postgresql/pull/612/files. I guess we can either pin, or bump.

@kbarber
Copy link
Contributor

kbarber commented Apr 21, 2015

Trying to fix the testing issues here: #177

command-processing section.

Added new class server/config_ini.pp to manage contents of the config.ini.
Three new parameters added:
  * command_threads
  * store_usage
  * temp_usage

All three default to 'undef'. This makes sure (potential) custom settings
done to that file with regard to above three variables are 'absent',
and let PuppetDB built-in defaults take care.

Documentation to the README.md added, as well as unit tests.

My use-case was, that I have on some nodes a too small /var partition,
so I had to lower the values of store-usage and temp-usage in the config.ini
manually.
@buzzdeee
Copy link
Contributor Author

rebased, and specs seem to be all green again.

kbarber added a commit that referenced this pull request Apr 22, 2015
Enable the module to manage entries in $confdir/config.ini
@kbarber kbarber merged commit 4940938 into puppetlabs:master Apr 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants