Fix manage_service_file variable

[CallumBanbery]

Pull Request (PR) description

The manage_service_file is configured incorrectly and doesn't
allow for overriding the value with hieradata. Updaing the init.pp
class params allows the file to be passed in.

This Pull Request (PR) fixes the following issues

Fixes #308
Fixes #310

[alexjfisher]

I think this is fixing a bug, but also changing the default behaviour?

Currently $::redis::manage_service_file doesn't exist (that's gotta be a bug).

puppet-redis/manifests/config.pp

Line 36 in 950e9e3

manage_service_file => $::redis::manage_service_file,

ie we end up with

redis::instance {'default':
  manage_service_file => undef,
  ...
}

This should result in the default value of true being used?

puppet-redis/manifests/instance.pp

Line 212 in 950e9e3

$manage_service_file = true,

With this change, we will now correctly use the value from params.pp, but unfortunately that's a different default. Does this need to be updated too?

Are you able to add some spec tests to prove/disprove what's actually happening?

[alexjfisher]

Correction, your change is correct. I didn't notice that init.pp inherits::params. This means that $::redis::manage_service_file does exist (and is false not undef).

All the same, do you think you'd be able to add a spec test for the manage_service_file parameter?

[CallumBanbery]

@alexjfisher I've added tests for having the manage_service_file set to false and true.
The only thing that's strange is passing in the service_provider value. I tried passing it in the params but it didn't work.
e.g. (Didn't work)

let(:params) do
  {
    manage_service_file: true,
    service_provider: 'systemd'
  }
end

But passing the value in as a fact and it's fine.

puppet-redis/spec/classes/redis_spec.rb

Lines 8 to 16 in 30f9b8c

	let(:facts) do
	merged_facts = facts.merge(redis_server_version: '3.2.3')
	if facts[:operatingsystem].casecmp('archlinux') == 0
	merged_facts = merged_facts.merge(service_provider: 'systemd')
	end
	merged_facts
	end

I just don't know why that's required as it should really work from params because it's a class parameter.

[dhoppe]

@CallumBanbery Thank you very much for your contribution.