Fix: select systemd for ubuntu correctly

[jk2l]

Patch for #240

[bastelfreak]

Maybe we can simplify the logic a bit. We use camptocamp/systemd as a dependency, the module provides a fact. We probably should use this instead of parsing operatingsystem versions.

[jk2l]

I have done the code changes, and adjusted the tests. But i don't have a system with systemd to test it. so the facter part is solidly base on assumption of the input and output from facter. Need someone help evaluate that section

[jk2l]

The refactoring fail:

• First refactoring patch is correct but failed for having quoted boolean word. Maybe we should add puppet-lint skip quoted boolean check?
• Second refactoring failed when run on my server. The facter $::systemd give me 'false' as string which evaluated as true

Feedback require

[jk2l]

@bastelfreak who is in charge for the development guideline within this repo?

[bastelfreak]

The voxpupuli team is. We define our guidelines in https://github.com/voxpupuli/modulesync_config and they apply to all modules in our namespace. I don't think that we should disable the quoted-boolean checks, bools shouldn't be quoted. I will test the code tomorrow on a systemd box. Facter on puppet3 is a bit painful because it doesn't know any datatypes except for strings :(

[bastelfreak]

@jk2l I tested the fact on a CentOS7 and an older Debian box. It correctly returns true/false, but as strings. We have two options here I think:

• Compare the result from $::systemd as strings
• cast the string into a bool with any2bool

My personal preference is any2bool. Then we should be save even when facter will return the correct datatype.

[igalic]

is there a reason we don't use rspec-facts here?

[bastelfreak]

this would allow us to shorten the overhead. but then we need am itchy case block to mock the systemd fact depending on the OS :(

[jk2l]

I only start involve with puppet and open source (only learned rspec last month), so i am still not familiar with the ruby and rspec (PHP background btw).

i can implement the changes, but it will be great if you can show me any reference or guideline that i can follow. as most of the stuff i pick up by reading the github source code directly (it is a bit hard to find the documentation on this)

[bastelfreak]

rspec-facts is a cool tool which parses the metadata.json and generates a hash of facts for each OS version in the metadata.json. I already implemented that for a few spec tests: https://github.com/voxpupuli/puppet-zabbix/blob/master/spec/classes/sender_spec.rb#L7-L13

you can always join the #voxpupuli IRC channel on freenode if you've question. direct chat will make it easier for both of us - we're always happy to help.

[jk2l]

I never use IRC before. I just joined it but i am not sure did I do it right. it seem like a total empty channel

[jk2l]

anyway @bastelfreak i wonder would it be overkill or better if I move the init script section into seperate class and write a unit tests for that instead. which can get rid of duplicate codes in both agent and server section and also the init script coding is not really depends on other system facts.

[jk2l]

Finally figured out how to test between different systems and robocop. This should be the last patch

[bastelfreak]

I think we can remove that if? There are multiple other distributions that also ship systemd. the next if with any2bool() should be enough

[jk2l]

I just realise the any2bool is only available on stdlib master branch. The puppet forge release does not have it yet. Using any2bool also need to move our dependency of stdlib from >= 4.4.0 to newer version that does not exist yet

[jk2l]

@bastelfreak Feedback require

[bastelfreak]

this may be the reason why the documentation for that is a bit low. I think this is the best function for our usecase. I will see if we can get a release of stdlib.