fix FreeBSD support #455
fix FreeBSD support #455
Conversation
manifests/instance.pp
Outdated
| @@ -414,7 +414,7 @@ | |||
| } | |||
| } | |||
|
|
|||
| if $manage_service_file { | |||
| if $manage_service_file and $facts['kernel'] == 'Linux' { | |||
There was a problem hiding this comment.
manage_service_file seems to be false by default (from init.pp). I am wondering if it would make sense to use hiera to set it to true on systemd systems and false otherwise. That way you would not need to change its value to make the module useful (assuming you have a mix of Linux and FreeBSD systems and a single profile that setup redis on these systems).
If I am guessing wrong, maybe we can rely on stdlib's service_provider fact rather than the kernel one:
| if $manage_service_file and $facts['kernel'] == 'Linux' { | |
| if $manage_service_file and $facts['service_provider'] == 'systemd' { |
There was a problem hiding this comment.
I like the idea of using the service_provider fact, now implemented in the most recent commit.
However, relying on manage_service_file would be great, but there is no module data directory right now. Instead I've changed the parameter to use the value from $redis::manage_service_file. I hope this will make it possible to set a global default on FreeBSD systems.
There was a problem hiding this comment.
I have to add that using $redis::manage_service_file would change the default value from true to false, which is probably a bad idea for other operating systems. So I should probably revert this and just rely on the service_provider fact in both cases?
manifests/instance.pp
Outdated
| @@ -414,7 +414,7 @@ | |||
| } | |||
| } | |||
|
|
|||
| if $manage_service_file { | |||
| if $manage_service_file and and $facts['service_provider'] == 'systemd' { | |||
There was a problem hiding this comment.
and and causing the CI failure 😄
|
@fraenki thanks for the PR, can you please ass FreeBSD to the metadata.json? |
|
Thank your both for your review :) |
Pull Request (PR) description
Avoid systemd-related code when running on non-Linux operating systems.
This Pull Request (PR) fixes the following issues
No issue found, just noticed that FreeBSD support is broken.