Make it possible to install sentinel independently

[tomsajan]

Pull Request (PR) description

This PR introduces the possibility to install redis::sentinel standalone, without redis::server. It comes in handy when you need your sentinels to be for example on a different node than your redis server.

The change itself is a pretty straightforward, I am just adding a require_redis parameter, that makes the requirement of redis class conditional. The default value is true, which includes the redis and it is therefore backward compatible with the current setup.

The only downside of this approach I see currently is that in case someone needs a standalone redis-sentinel from a managed repository, the repository must be added manually as the redis::preinstall is no longer included (as was previously with the redis class)

This Pull Request (PR) fixes the following issues

• installation of standalone sentinel (no open issue for this one, so far)

Thank you for any input :)

[tomsajan]

Regarding the puppet5/ubuntu 20.04 tests, the errors don't look like they were caused by my changes...

[ghoneycutt]

This looks good to me though it needs the addition of tests that show the presence or absence of the redis class based on the new require_redis parameter.

[ghoneycutt]

style nit - drop the leading :: so it reads redis::sentinel

[tomsajan]

Ok thanks for the tests suggestion. I will take a look at it :)

[vox-pupuli-tasks]

Dear @tomsajan, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

[gizmo15]

Hi,

Can i help to push this PR?
I have this need at work.

thanks

[ekohl]

@gizmo15 it's probably best to submit your own PR.

[tomsajan]

Thanks for taking interest in this. To be honest, this slipped from my mind. I'll try to make it work.
What is required from this to be accepted? I suppose

• rebase to current upstream
• drop leading :: and add tests as suggested by @ghoneycutt ?
  I may have need some helm with the tests, as I didn't do it yet in puppet, but I will take a look a try my best

[tomsajan]

Hi, so I merged it to the current master.
Could someone advise me please on how to pass the CI tests? Static tests reports outdated reference.md, but I adjusted it as good as I was able and it still complains. :(

Not sure though on how to tackle tests to verify the presence or absence of the redis class...

Also, this MR still has the labe merge-conflicts, although there are non now.

Thanks and have a great day :)

[ekohl]

I would recommend to rebase it on master. We don't like it when a PR merges in master. Though I think the bot may not be able to remove the label.

[flepoutre]

Hello, any news about this PR ?
Thanks.

[yakatz]

It doesn't appear it was ever updated as requested during the review, so it can't be merged as-is. The original author would have to update it, or someone else could create a new pull request based on it.

[tomsajan]

Let me give it a try for a day or two :)

[tomsajan]

So I squashed my changes and rebased it all to current master.
During the rebase I renamed the option to contain_redis because now contain is used instead of require.
There are still some tests that fail.

I am not sure I will be able to test this code as I don't use puppet anymore in my infrastructure, but I will try to come up with something tomorrow.

[flepoutre]

@tomsajan Hello, thanks for your feedback. I don't really understand the Puppet CI error 🤷‍♂️
Do you have any idea please ?
Thanks.