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

Allow puppetdb to be configure for masterless conf #163

Merged

Conversation

Spredzy
Copy link
Contributor

@Spredzy Spredzy commented Jan 15, 2015

Currently the module only allows PuppetDB to be configured in a
agent/master setup configuration, by configuring the [master] section of
the puppet.conf. This commit allows one to use this module to configure
the [main] section of the puppet.conf so the module can configure nodes to
use PuppetDB in a masterless setup.

Doc about puppet.conf configure in masterless way available here
https://docs.puppetlabs.com/puppetdb/2.2/connect_puppet_apply.html#manage-puppetconf

puppet_conf => $puppet_conf,
puppet_conf_section => $puppet_conf_section,
enable => $enable_reports,
require => $strict_validation ? {
true => Puppetdb_conn_validator['puppetdb_conn'],
default => Package[$terminus_package],
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you still need the terminus in a masterless configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ajroetker
Copy link
Contributor

I'm tempted to say that maybe the storeconfigs and report_processor classes should be moved to a different namespace now, maybe under puppetdb::puppet_conf::..., and maybe changing the master::config class to be puppetdb::consumer::config (using a $masterless = false variable to determine the config section). @kbarber I'm not sure if we'd save all that for a major release? or if just copying the classes for now, instead of moving might be the thing to do to keep the master classes around. My main concern is supporting masterless configuration in the master hierarchy :) love that it works with masterless though! :)

@Spredzy
Copy link
Contributor Author

Spredzy commented Jan 16, 2015

@ajroetker Thanks for reviewing. I agree about moving it out under another namespace, just wasn't make sure such a huge change will be accepted. If @kbarber agrees with the idea, I can make the needed changes.

@Spredzy
Copy link
Contributor Author

Spredzy commented Jan 22, 2015

Up ?

@kbarber
Copy link
Contributor

kbarber commented Jan 26, 2015

@ajroetker we can't change the API on a non-major boundary. We'd need to create new classes or something and find some way to proxy from the old ones to the new ones to keep backwards compatibility. Having said that, we could almost start considering a 5.0 release at this point ... since we probably will need this for PDB 3.0.0 later on, however there is some complexity in just doing that right now - my concern is about the package naming really for the terminus, since that might introduce some lack of backwards compatibility with PDB 2.2.x. Maybe if this all gets too nasty we should cut a stable & master branch. This needs some more thought, and to be truly honest with you - introducing complexity like this right now is somewhat bad timing, due to the pressures we are under :-).

But I get what you are trying to say about naming, it does make some sense to create some differently named classes. Not sure if I like the term 'consumer' however, seems very generic, I'd just say 'puppet' or something like that, since thats what its trying to manage really.

@kbarber
Copy link
Contributor

kbarber commented Jan 26, 2015

@ajroetker so ... I'd shelve your idea in a ticket. I like it, its nice, but lets try and do it for a major release and when we have the time. Mark it as a 'fix for 5.0.0' or something like that, so we know exactly when it can be worked on. I think the naming problem here isn't terrible, and most people would be forgiving if they saw it. At least we know about it :-).

@kbarber
Copy link
Contributor

kbarber commented Feb 3, 2015

@ajroetker lets talk about this one today.

@ajroetker
Copy link
Contributor

@Spredzy if you wouldn't mind we thought that instead of having $puppet_conf_section maybe having a $masterless = true "switch" would be a better route for this because you could also use that variable to set other nice masterless defaults like $manage_routes to false. Looks good otherwise!

@Spredzy Spredzy force-pushed the allow_masterless_configuration branch 3 times, most recently from 7100cb7 to ca933a2 Compare February 5, 2015 11:07
@Spredzy
Copy link
Contributor Author

Spredzy commented Feb 5, 2015

@kbarber @ajroetker updated based on your comments. Tests fails only on ruby 1.8.7, I think it might be related to rspec-core 3.2.0 that came out 2 days ago based on the trace. I noticed the same issue on puppetlabs-mongodb module

@kbarber
Copy link
Contributor

kbarber commented Feb 5, 2015

@Spredzy yeah, I'm sending up a pinning patch now. Double check your patch with a grep, there are still some mentions of the old parameter btw.

Currently the module only allows PuppetDB to be configured in a
agent/master setup configuration, by configuring the master section of
the puppet.conf and applying on the routes for such a configuration.
This commit allows one to use this module to configure the main section of
the puppet.conf and applying the proper routes so the module can configure
nodes to use PuppetDB in a masterless setup.

Doc about puppet.conf configure in masterless way available here
https://docs.puppetlabs.com/puppetdb/2.2/connect_puppet_apply.html#manage-puppetconf
@Spredzy Spredzy force-pushed the allow_masterless_configuration branch from ca933a2 to a826d85 Compare February 5, 2015 11:32
@Spredzy
Copy link
Contributor Author

Spredzy commented Feb 5, 2015

@kbarber Sorry I forgot to update the README. It has been fixed.

kbarber added a commit that referenced this pull request Feb 5, 2015
Allow puppetdb to be configure for masterless conf
@kbarber kbarber merged commit b482ad8 into puppetlabs:master Feb 5, 2015
@kbarber
Copy link
Contributor

kbarber commented Feb 5, 2015

@Spredzy thanks mate, looking good.

@Spredzy
Copy link
Contributor Author

Spredzy commented Feb 5, 2015

@kbarber @ajroetker Thanks to both of you for the reviews.

@JohnTheodore
Copy link

If I already have a puppet apply that happens on a box in a 30 minute cron job, with say /etc/puppet stuff, and /var/lib/puppet stuff, how can I install masterless puppetdb with these changes? Just do a puppet apply with class 'puppetdb', and params:masterless true? One issue is my puppet apply/config defaults to using /etc/puppet stuff, or /var/lib/puppet stuff.

Is there some simple install commands of puppetdb/postgres you guys used? Are there any examples you can provide?

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