-
-
Notifications
You must be signed in to change notification settings - Fork 127
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
Add support for FreeBSD #282
Conversation
@martijndegouw thanks for the PR! Can you u take a look at the failing spec tests and also add FreeBSD to the metadata.json? |
aabf26d
to
0162a27
Compare
Dear @martijndegouw, thanks for the PR! This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
8bfaab3
to
a43ef2e
Compare
744a02f
to
d367886
Compare
@martijndegouw thanks for the updates. can you take a look at the failing tests? |
@@ -230,50 +230,54 @@ | |||
end | |||
|
|||
context 'when forwarder not already installed' do |
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.
instead of the additional level in indentation, you can do the following:
context 'when forwarder not already installed' do | |
context 'when forwarder not already installed', unless: facts[:kernel] == 'FreeBSD' do |
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 can change that, if that's the preferred way. I just copied it over from the tests above.
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 guess it would be nice to have a test for service_provider == freebsd as well. I'll give that a try.
5092d1c
to
e1fd0ad
Compare
@@ -261,6 +282,7 @@ | |||
'Debian': { $package_provider = 'dpkg' } | |||
'Solaris': { $package_provider = 'sun' } | |||
'Suse': { $package_provider = 'rpm' } | |||
'FreeBSD': { $package_provider = 'pkgng' } |
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.
Is setting this required? in 99% of the cases undef is fine and puppet will figure it out automatically.
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 just added it for completeness, but it looks like there is code depending on it in
if $splunk::forwarder::package_provider and !($splunk::forwarder::package_provider in ['apt','chocolatey','yum']) { |
To be honest, I did not test it without setting is specifically.
spec/classes/forwarder_spec.rb
Outdated
@@ -231,7 +231,7 @@ | |||
|
|||
context 'when forwarder not already installed' do | |||
let(:facts) do | |||
facts.merge(splunkforwarder_version: nil, service_provider: 'systemd') | |||
facts.merge(splunkforwarder_version: '7.3.3', service_provider: facts[:kernel] == 'FreeBSD' ? 'freebsd' : 'systemd') |
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.
wh00ps
e1fd0ad
to
7e25b78
Compare
Add support for FreeBSD. Tested on FreeBSD 12.1