-
Notifications
You must be signed in to change notification settings - Fork 229
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
Support new non-AIO puppet Debian packages #868
Support new non-AIO puppet Debian packages #868
Conversation
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 this! I appreciate how small it is.
manifests/server/puppetserver.pp
Outdated
@@ -240,7 +240,7 @@ | |||
} | |||
|
|||
unless $facts['os']['family'] == 'FreeBSD' or | |||
($facts['os']['family'] == 'Debian' and $puppet::params::aio_package) { | |||
($facts['os']['family'] == 'Debian' and $puppet::params::aio_package) { |
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 consider this a bug in the indent rule. Not going to dive into it in the weekend, just want to let you know.
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.
OK, just let me know if you want me to remove that commit from the branch.
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.
In voxpupuli/puppet-lint-strict_indent-check#28 I already added examples for this, though I suspect that in this case you should use braces around the statements.
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 not sure where you're suggesting I use braces ({}
) in there?
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 round braces: ()
, but those don't appear to help either. But in this case I think we can simplify it to unless $puppet::params::aio_package
.
Though I now wonder why we ensure /opt/puppetlabs/server/apps/puppetserver/config/services.d
exists. It was added in 68c2d50, but I question why. It's part of the puppetserver RPM on EL8 and we don't place any files in that directory. Given the commit message I think we can actually drop it now: #869
@@ -169,6 +169,8 @@ | |||
context => '/files/etc/rc.conf', | |||
changes => ["set puppetserver_java_opts '\"${jvm_cmd}\"'"], | |||
} | |||
} elsif $facts['os']['family'] == 'Debian' and !$puppet::params::aio_package { | |||
$server_gem_paths = ['${jruby-puppet.gem-home}', '/usr/lib/puppetserver/vendored-jruby-gems'] # lint:ignore:single_quote_string_with_variables |
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.
Given the repetition in declaring a vendored-jruby-gems dir, is this something that belongs in params.pp
?
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.
Declaring $server_gem_paths
in puppet::params
was my first idea too, however it doesn't work because the variable isn't a class parameter of puppet::server::puppetserver
.
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.
there's a bunch of CI warnings, but once those are fixed I fully support this proposal. )
0556430
to
2ea4f65
Compare
The upcoming release of Debian will ship with a new non-AIO puppetserver package, and these configuration tweaks are needed to make the module compatible with it.
112748d
to
0a6fdd7
Compare
0a6fdd7
to
4a6a4b9
Compare
The upcoming release of Debian will ship with a new non-AIO puppetserver package, and these configuration tweaks are needed to make the module compatible with it.
https://packages.debian.org/search?keywords=puppetserver