Idempotent config management

[jabclab]

In order to guarantee idempotence for management of config files (both redis-server and redis-sentinel) I propose the following set of changes which will lead to config files looking like:

$ cat /etc/redis/6379.conf
include /etc/redis/6379.dyn.conf # <-- file which contains what /etc/redis/6379.conf did before

This file is solely created if it was not present before. Therefore if Redis writes dynamic config via CONFIG REWRITE the file will not be touched. As the dynamic config will come after the include it will take precedence of the default config.

I've applied the same principle for the sentinel config too, e.g.

$ cat /etc/redis/sentinel_26379.conf
include /etc/redis/sentinel_6379.dyn.conf

This should solve #22 😄

[DavidWittman]

Thanks for putting in the time to take a crack at this @jabclab. I'll do some review this week.

[jabclab]

No probs @DavidWittman 😄

I've just spotted a small issue with the sentinel implementation during manual testing so maybe hold off merging for now. I'll let you know once I've got the fix in.

[jabclab]

The issue I'm hitting is that due to the CONFIG REWRITE of the sentinel processes I end up with a duplicate definition of the sentinel to master, e.g. the following is included in both /etc/redis/sentinel_26379.conf and /etc/redis/sentinel_26379.dyn.conf:

sentinel monitor master01 11.22.33.44 6379 2

This leads to:

*** FATAL CONFIG FILE ERROR ***
Reading the configuration file, at line 13
>>> 'sentinel monitor master01 11.22.33.44 6379 2'
Duplicated master name.

Trying to find an elegant solution now. The redis-server appears to be happy using the include paradigm though 😸

[jabclab]

OK, the issue boils down to the fact that duplicate sentinel * commands are not supported using the include paradigm.

Therefore I'm thinking of still creating the dynamic config file (/etc/redis/sentinel_26379.dyn.conf) but instead of using an include in /etc/redis/sentinel_26379.conf I'll iterate over each line in the dynamic file and use the lineinfile module. I think this will solve the problem 😄

[jabclab]

Hey @DavidWittman , I've updated the PR to handle sentinel config in a better way. It now writes the same /etc/redis/sentinel_26379.dyn.conf (this time as a hidden file) and then uses the lineinfile module to ensure each line of the default config is present in the /etc/redis/sentinel_26379.conf.

This isn't perfect as if, say, you want to change the port on which the sentinel monitoring is taking place it would probably lead to an error on service restart as there would be two sentinel monitors of the same name defined, e.g.

# /etc/redis/sentinel_26379.conf
sentinel monitor master01 11.22.33.44 6379 2
sentinel monitor master01 11.22.33.44 6380 2

Going forward I think we need to make it more intelligent and probably use redis-cli in order to determine state of the current sentinel setup.

I think this is change is still a significant improvement though, what d'you think?

😸

[jabclab]

Issue with the build was that I was using lineinfile without escaping double quotes. Therefore the sentinel log file ended up like:

...
logfile
...

i.e. logfile had no value therefore the following occurred:

 * Starting redis_26379...       *** FATAL CONFIG FILE ERROR ***
Reading the configuration file, at line 10
>>> 'logfile'
Bad directive or wrong number of arguments

[jabclab]

hey @DavidWittman , I think the approach I've taken so far is not an elegant way to go 😢 Managing the config file using ansible for sentinel seems too complicated/brittle. I think we either need to go with:

• creating a script with config defined in it which is run through redis-cli
• solely configuring the sentinel group once

So, I think the redis-server part of this PR still works nicely but the sentinel does not. D'you want me to close and reopen with just the redis-server parts?

[jabclab]

Closing for now as I think the handling of config in an idempotent way (especially in sentinel) needs further discussion and this code has become rather tightly coupled to the current sentinel.conf handling.