Add ubuntu 18.04 support

[cliff-wakefield]

Under Ubuntu 18.04 it appears the SNMP user and group have changed.

Error: Could not find user snmp
Error: /Stage[main]/Snmp/File[var-net-snmp]/owner: change from 'Debian-snmp' to 'snmp' failed: Could not find user snmp
Error: Could not find group snmp
Error: /Stage[main]/Snmp/File[var-net-snmp]/group: change from 'Debian-snmp' to 'snmp' failed: Could not find group snmp
Notice: /Stage[main]/Snmp/Service[snmpd]: Dependency File[var-net-snmp] has failures: true
Warning: /Stage[main]/Snmp/Service[snmpd]: Skipping because of failed dependencies

This request simply adds a condition to set the user and group correctly in params.pp

[cliff-wakefield]

I have no idea why the individual test is failing.

I can't be because of the 4 lines I've added, or at least I can't see how it could be??

Any ideas?

[elfrinjo]

Hi, there is a missing space between 'Ubuntu' and and

[cliff-wakefield]

Ok fixed the space, but the tests still fail.

I also created another fork with a change to just the README it still fails the same test, with no actual code change.

[bastelfreak]

Hi @cliff-wakefield, thanks for the PR! Can you please rebase against our latest master? That will pull in #134. Please add Ubuntu 18.04 to our metadata.json.

[bastelfreak]

Those facts are legacy and we try to get rid of them in new code + migrate the existing codebase in parallel. Can you please use $facts['os']['name'] and $facts['os']['release']['major']?

---

The tests will fail with that, can you please add a rspec-puppet-facts based test for this?

[cliff-wakefield]

I have rebased to master, I have updated the metadata.

[cliff-wakefield]

Switching params.pp to use the facts hash is much larger change, that may well need doing but is larger in scope than just adding Ubuntu 18.04 support.

[bastelfreak]

I did some inline comments. Let us know if you've got any questions. You can reply here or reach out to us on our IRC channel #voxpupuli on freenode or https://puppetcommunity.slack.com

[cliff-wakefield]

Looking at the code again the fix done under PR #134 fixes the issue I believe anyway as it now numerically compares the versions of Ubuntu so I don't think this PR is needed?

[bastelfreak]

It only compares the Debian version at the moment, right?

[rogermartensson]

I just did a test on the changed line in params.pp in PR #134.

if $::operatingsystem == 'Debian' and versioncmp($::operatingsystemmajrelease, '9') >= 0 {

It didn't work for me.
If it was to work then it probably would trigger for Ubuntu older than 18.04. Would that make installation these older versions fail?

So I guess there is a need to test for Ubuntu 18.04. Maybe converts the number test to use versioncmp and in the PR. Then Travis might be more happy?

[ekohl]

Could you rebase this? There are a lot of irrelevant changes from master in this PR that make it hard to see your changes.

[cliff-wakefield]

@ekohl rebased and standardised the Operating systems version test using versioncmp.

[alexjfisher]

Can this also be squashed before merge?

[amateo]

I think this issue is related in #168