Add performance tuning for ulimits

[Cornellio]

This change adds ulimits performance tuning whereby the maximum open file handles is increased by adding riak hard nofile and riak soft nofile in /etc/security/limits.conf. When running with default values the config file will be modified like so:

riak hard nofile 65536 riak soft nofile 65536

Custom values can be set by the user by passing parameters like this:

class { '::riak':
  ulimits_nofile_soft => 88536,
  ulimits_nofile_hard => 98536,
}

[mbbroberg]

Thanks @Cornellio! This work is great. I'd like @basho-labs/puppet-committers to take a look if either is free.

[Cornellio]

Thank you @mjbrender, not clear why the Travis CI builds failed. I will look into it. Rechecking Beaker tests on my system and so far they are passing on centos-66-x64 and debian-78-x64.

[Cornellio]

I've typed them as Integers.

[Cornellio]

Hi @danieldreier,

Regarding the failed Travis CI builds, I'm trying to get this resolved but am at a loss as to why they fail. Would you be able to comment? The local rspec tests also fail when I do bundle exec rake spec. This fails both on my forked and modified repo and the upstream https://github.com/basho-labs/puppet-riak. It's curious that Travis CI tests pass on your upstream repo yet doing rspec tests on the same code fails.

Thank you

[mbbroberg]

Man, Travis CI is being mean. I'm trying to work with it locally as well. @danieldreier, any observations? @haf, you around?

[haf]

key_source has been removed https://github.com/basho-labs/puppet-riak/blob/master/manifests/repository/debian.pp#L11

Source of Apt::Source https://github.com/puppetlabs/puppetlabs-apt/blob/master/manifests/source.pp#L3

Failing Invalid parameter key_source on Apt::Source[riak] at /home/travis/build/basho-labs/puppet-riak/spec/fixtures/modules/riak/manifests/repository/debian.pp:8 on node testing-worker-linux-f25aadab-2-20180-linux-15-61540392

[Cornellio]

Nice catch @haf. This fixes it in my local rspec tests, yet on travis ci I get mixed results. https://travis-ci.org/Cornellio/puppet-riak/builds/61655691. Some tests pass, some fail, depending on Ruby versions. Not sure what to do.

[haf]

It seems the future parser barfs on $::puppetversion, see:

https://travis-ci.org/Cornellio/puppet-riak/jobs/61655693#L436

Is it illegal to use global variables now? /cc @hlindberg Or is the version comparison in the Apt module something that makes the type system annoyed?

Ref https://github.com/puppetlabs/puppetlabs-apt/commit/21a2462a58938d126b66985c0b0fa1f8e09f27cd

[hlindberg]

@haf It is not illegal to lookup global variables - the error is that the variable is not set which means strict variables is turned on. Is the variable in question actually set? It is supposed to come from facter, be a node parameters, and then set as a node/top scope variable. Cannot see type system having anything to do with this problem.

[haf]

@hlindberg thank you! That helps; means it's probably an integration test-framework error.

[Cornellio]

I placed a separate PR #67 containing only the removal of key_source, instead of mixing it in here.

[Cornellio]

Is there a way forward here? Considering we're stuck on an integration testing issue, I don't know what to do. Can we change the tests in some way?

[danieldreier]

@Cornellio I'll review tonight, can't during work.

[haf]

Ah! So it was THAT which failed the tests!

[danieldreier]

+1, thanks for putting work into this.

[mbbroberg]

Typos are evil :see_no_evil:.

How's everyone feeling with a merge here? I love seeing the "All is well" flag.

[Cornellio]

With a certain bias, I'm feeling good about it now that tests are passing. I'm not clear what got them to pass though.

[danieldreier]

I'm fine with this being merged. We should probably decide how we want to do code review on this project, if we want to do one +1 and then somebody else merges, just have somebody else merge, etc.

I think that two +1's (the second one being the merge) is good for code quality but I'm not sure the project is big enough to support it.