Allow to manage Log4j configuration

[evgenkisel]

A default configuration of log4j does not allow Kafka to delete old log files which were rotated.
As result it gets issues with disk space on Kafka instances.
The changes will allow to update log4j.properties files to change a logic of rotation for deleting old files.
Customers can enable the function and customize a size which triggers rotating and a count of stored log files.

This Pull Request (PR) fixes the following issues

Fixes #200
Fixes #85

[smortex]

Can you add unit tests that demonstrate that this works as intended so that we can avoid regressions in the future?

No need to test for all values in the template, I guess that checking that the file is properly managed depending on the value of $manage_log4j and that MaxFileSize / MaxBackupIndex are set to the expected values for an appender would be enough.

[evgenkisel]

@smortex will do.

[evgenkisel]

@smortex could you please provide docs or examples how to test values of class parameters with rspec-puppet?
I'm not a big expert in Puppet Rspec.

I found how to test the file resource when manage_log4j is true

context 'with  manage_log4j => true' do
  let(:params) { {'manage_log4j' => true} }
    it { is_expected.to contain_file('/opt/kafka/config/log4j.properties') }
end

but I can't find ways how to test the parameters $log_file_size and $log_file_count
for the $log_file_size we should be able that it's set to String in format 999MB

[evgenkisel]

@smortex thanks a lot for your explanation! Looks like I didn't understand a main goal of the Unit tests.
So, I updated Unit tests and deleted my changes from kafka::producer class, because the class doesn't run a daemon of Kafka as I understand. It's just for running Kafka producer console client.
The parameter $manage_log4j will be available only for the broker, consumer and mirror classes.

[evgenkisel]

@smortex could you please review it? I hope we can include the changes to the new release

[smortex]

Looks good, I added an in-line note for some point you may want to adjust.

Can you then squash your changes in a single commit (to avoid multiple "fix" commits which make digging in the code history harder)?

Thanks!

[evgenkisel]

@smortex thank you for the suggest about changing Pattern for the $log_file_size !

all changes are squashed to a13a5ed

please review it

[smortex]

I still see a lot of commits 😄

Your commits are interleaved with other ones in the master branch (you seem to have merged master at some point), so first rebase you changes on top of master:

git rebase origin/master

You then end up with your 13 commits on top of master. Run:

git rebase -i origin/master

This will open your editor with the commits where you can reorder / edit / reword / etc. If you just want to keep the commit message from the fist commit and squash all other commits into it, keep he first line untouched and change all other lines' "pick" to "fixup", save and exit, and you are ready to:

git push -f

[evgenkisel]

@smortex I'm sorry, but I'm not a big expert with Git :)) Now it's ok

[smortex]

LGTM. If nobody from voxpupuli who use this module approve and merge this, I am fine with merging this in a few days if no concerns are raised.

Thanks!

[bastelfreak]

For the integer here and in the other classes, can you enforce the minimal/maximal values?
example to allow 0 as lowest value and 10 as highest:

Suggested change

	Integer $log_file_count = $kafka::params::log_file_count
	Integer[0, 10] $log_file_count = $kafka::params::log_file_count

[evgenkisel]

@bastelfreak Updated like Integer[1, 50] $log_file_count

[evgenkisel]

@bastelfreak @smortex I'm sorry, but after my last changes it started two workflows and they are failed.
As I see it's not related with my changes. Maybe some conflicts between containers on Travis CI.
could you please re-run tests?