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

Support new non-AIO puppet Debian packages #868

Merged
merged 4 commits into from
May 16, 2023

Conversation

jcharaoui
Copy link
Contributor

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

Copy link
Member

@ekohl ekohl left a 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.

@@ -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) {
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Member

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
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Contributor

@anarcat anarcat left a 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. )

@jcharaoui jcharaoui changed the title Support new non-AIO puppetserver Debian package Support new non-AIO puppet Debian packages Mar 23, 2023
manifests/params.pp Outdated Show resolved Hide resolved
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.
@jcharaoui jcharaoui force-pushed the non-aio-debian-puppetserver branch from 112748d to 0a6fdd7 Compare May 6, 2023 16:48
@jcharaoui jcharaoui force-pushed the non-aio-debian-puppetserver branch from 0a6fdd7 to 4a6a4b9 Compare May 6, 2023 16:54
@ekohl ekohl merged commit b3c3deb into theforeman:master May 16, 2023
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.

4 participants