Add support for AmazonLinux.

[pillarsdotnet]

These changes should enable installation on AmazonLinux.

See #37 for context.

[edestecd]

I need to update the tests in this module, so they will pass again. I'l try and do that later today.

[edestecd]

I have fixed the tests. Please rebase from master

[pillarsdotnet]

Rebased. Tests passed. Probably should write additional tests for AmazonLinux.

[edestecd]

That would be nice ;)

[davidgibbons]

Is this going to be merged soon or should I just Puppetfile off the pillarsdotnet version?

[pillarsdotnet]

I need to write the rspec tests first. Sorry; will try to get to it soon.

[edestecd]

I'm going to cut another version to the forge as soon as this and one other is merged...

[pillarsdotnet]

Took a look at the existing rspec tests. The only distro-specific bits I see are in spec/acceptance/nodesets, and the centos one contains a link to a vagrantcloud box which no longer exists.

Sorry, but I'm not familiar enough with rspec to write a comprehensive set of tests for all supported distros, If someone else wants to write one for Centos, I'll take a stab at modifying it for AmazonLinux, but otherwise I'm done.

[pillarsdotnet]

Leveraged the stdlib service_provider() function to detect systemd usage, instead of relying on a specific distro name / version number.

[pillarsdotnet]

Don't know how to set facts so that rspec passes. Trying different options...

[edestecd]

Were using the rspec-puppet-facts gem, which parses your metadata.json and runs your spec tests against all supported oses. o you just need to add AmazonLinux here: https://github.com/edestecd/puppet-clamav/blob/master/metadata.json#L16

[edestecd]

The acceptance tests are pretty old and i'm not really running them with travis ci any more. I need to get them updated at some point.

[edestecd]

Looks like you changed a lot since last I reviewed it.

Before you had an entire new section in params for AmazonLinux. Did you realize that most of the options were the same as RedHat 6? So the only change needed was the pid file? I thought the package name and service name were a bit different than RedHat 6?

Also before you were modifying the pid file for Debian with a specific issue linked that made sense... Was that a mistake?

Do you have the old commit around? It might be nice for comparison...

[edestecd]

I see now you are modifying Debian still for the pid file, but also doing RedHat 7...

Can we get a link to the Debain issue in the comments above the code in params or maybe in the issue. Also is there a specific issue for Redhat or did you just notice it was needed? I need to spin up a Redhat 7 and see what the defaults are on a fresh install I guess to confirm that pid file should indeed not be set...

[edestecd]

Found it: Debian issue for systemd pid file: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=804132

I also agree that any systemd should not need a pid file,including RedHat 7,but would still like to confirm

[ubellavance]

@edestecd do you want me to open a ticket with Red Hat to get the info? We have subscriptions with them.

[edestecd]

The new code looks mostly good to me. If you could add AmazonLinux and the versions you have tested to metadata.json so it will be tested in spec and fix up this: https://github.com/pillarsdotnet/puppet-clamav/blob/fa87e6aa6a31bfbc97a483c95513bb6a67c59b29/spec/classes/clamav_spec.rb#L9

I think we would be good to go...

[edestecd]

@ubellavance I was looking for an existing redhat issue that indicated systemd should in fact not have a pid file set, but I can check it out by doing a vagrant install.

What is you experience on RedHat 7? Does it work with a pid file set (the default in the module)? Or do you use RedHat 7? If you have a minute to confirm on a fresh install that would also be nice... Just want to know if the stock config on a manual install of the package has the pid file set.

[ubellavance]

@edestecd I've been using your module with RHEL7 for about a year. Clamd config file (/etc/clamd.d/scan.conf) contains this line:

PidFile /var/run/clamd.scan/clamd.pid

I could easily test on a fresh install, but what do you want to test? If it can live w/o a pid file or does it bother to have one, or something else?

[edestecd]

@ubellavance I usually try an make the module default to what a fresh manual install defaults to. That pid file may have been left over from RedHat 6 and may not be needed on RedHat 7. Basically if a fresh manual install does not have a pid file set on RedHat 7 then we should not set it in the module, by default (in params.pp).

So we need to know what a fresh manual install config file looks like. Does it have PidFile set?

[ubellavance]

@edestecd Ok what you'd like to see is a simple manual install using yum? even easier...

[ubellavance]

It is commented out on a RHEL7 fresh install.

# grep Pid /etc/clamd.d/scan.conf
#PidFile /var/run/clamd.scan/clamd.pid

If I try to start it with the package default, it won't run, but if I follow the error messages (comment out the Example line and set a TCP port or a UNIX socket), it works.

[pillarsdotnet]

@edestecd -- Still isn't testing on Amazon Linux despite updates to metadata.json. I'm open to suggestions.

[pillarsdotnet]

I've tested locally on Amazon Linux 2017.03 and made appropriate changes. Sorry I lost the original patch-set; looks like there were quite a few deviations from the base Centos-6 distro.

[edestecd]

@ubellavance that makes sense that PidFile is commented out b default. The module does set the socket so it at least works out of the box, since the stock config does not...

[edestecd]

@pillarsdotnet that facts generator does not seem to support amazon: https://github.com/camptocamp/facterdb/tree/master/facts/2.5

[edestecd]

@pillarsdotnet The mysql module uses the same fact gem and does not list Amazon in the metadata.json: https://github.com/puppetlabs/puppetlabs-mysql/blob/master/metadata.json

Despite supporting it in manafests/patams.pp: https://github.com/puppetlabs/puppetlabs-mysql/blob/master/manifests/params.pp#L352

We should probably just leave it out then

[edestecd]

I'm pretty good with this except the two things I mentioned. It would be nice to get this merged soon... Thanks for all your work on this @pillarsdotnet. I know many will appreciate it!

[pillarsdotnet]

There appears to be a process for getting new entries added to facterdb, so I'll work on that next.

[pillarsdotnet]

Refactered params.pp; still think it could be better.

[pillarsdotnet]

Yay! Finally passed!

[pillarsdotnet]

Also submitted PR to facterdb: https://github.com/camptocamp/facterdb/pull/56

[pillarsdotnet]

facterdb merged https://github.com/camptocamp/facterdb/pull/56 so it should be possible to add AmazonLinux to the supported OS list.

[edestecd]

I promise to look at this by tomorrow...

[edestecd]

@pillarsdotnet Did we get a new release of the facterdb gem, so we can add it to the Gemfile?

[edestecd]

Looks like factordb did not release a new version yet. I'm cool with pointing our Gemfile to the master branch for now.

[pillarsdotnet]

I don't think we can "point our Gemfile to the master branch." Their github sources don't contain a pre-built gem file.

[edestecd]

I think all your git repo needs is a gemspec for bundler to be able to use it.

[edestecd]

Something like this should work:

gem 'facterdb', :git => 'https://github.com/camptocamp/facterdb.git', :branch => 'master'

[pillarsdotnet]

Okay, pushed; let's see if it passes tests.

[pillarsdotnet]

Well, it passes tests but doesn't actually test on Amazon Linux. Dunno why...

[edestecd]

Wow @pillarsdotnet thanks for continuing to work on this. Sorry for my absence lately. I need to get back to this module and get it updated for pdk and puppet 4. Do you use puppet 4 or 5? Hopefully we can get this merged soon. I hope to work on this some this week.

[pillarsdotnet]

I'm using puppet 4 and puppet 5 in different situations.

But we're no longer using clamav, so I probably won't be working on this anymore.

[pillarsdotnet]

Going to delete my fork, which will close this PR.

@edestecd -- I'm sorry, but I'm no longer working with Puppet nor with ClamAV.