-
Notifications
You must be signed in to change notification settings - Fork 23
Puppetserver support (third try) #200
base: master
Are you sure you want to change the base?
Puppetserver support (third try) #200
Conversation
Provide rudimentary tests for installing puppetserver. Not expected to pass yet. This is likely to run into memory size issues because beaker VMs are created with 512mb and I'm not aware of an obvious way to change that.
Configure a basic puppetserver. This puts just enough configuration in place to get a basic running instance. Some of the configuration parameters are not implemented yet; they're placeholders to facilitate future development.
value => $jvm_memory, | ||
} | ||
ini_subsetting {'puppetserver_maxpermsize': | ||
subsetting => '-XX:MaxPermSize=', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will not work in java8
well, it'll work, but it'll be ignored with a warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty ignorant of java version differences; what would we need to do for java 8? I'm simply re-implementing the stock puppetserver configuration here. I'd be glad to switch based on java version, or to parameterize it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the equivalent in java 8 is: -XX:MaxMetaspaceSize=256m
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can use $::java_major_version
to distinguish this.
perm = $::java_major_version ? {
8 => '-XX:MaxMetaspaceSize=',
default => '-XX:MaxPermSize=',
}
i hope this also works if someone's not using puppetlabs-java, while using puppet 4 ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion - added it.
Actually - our travis-ci tests do a required test iteration with future parser enabled. Puppet 3.7.3 future parser and puppet 4 syntax should be virtually identical. The agent install will be a bit different - facter, puppet, etc all bundled together in one package and paths will be different. The recent abstraction work we've done on this module should help, too.
I'm also not sure I want to add a dependency on puppetlabs-java. I've wrapped the whole thing in a conditional; if we end up needing to manage java further we can circle back.
Based on @igalic's suggestion, this sets the puppetserver_maxpermsize ini subsetting conditionally based on java version if the java_major_version fact from the puppetlabs-java module is available.
This is my third try at adding basic puppetserver support. This works under Debian Wheezy when I apply tests/puppetserver.pp; a very basic puppetserver setup comes up, though I haven't done more extensive testing to validate it in real depth.
@igalic - this isn't really intended to compete with PR #193 - I just wanted to clean up what I'd done previously so we could collaborate more effectively.
As I described in that PR, there are a ton more settings we could manage, but as an MVP this should be enough to get us started.
Any review would be much appreciated.