Add replication role detection from runtime Redis information #128
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danielkza
force-pushed
the
replication-role-detection
branch
from
72b5b5a
to
aa6779e
Compare
|
@DavidWittman I did some work to try and solve the problems that #25 couldn't, by attempting to merge rewritten configuration with our managed config at daemon startup time, with some other assorted fixes. Please check it out if you can. |
danielkza
force-pushed
the
replication-role-detection
branch
3 times, most recently
from
3305c0f
to
b5187fe
Compare
Export Redis information (as retrieved by the INFO) command as local
facts for each configured server/sentinel instance.
Those facts are created in addition to the existing global facts, and
are identified by keys in the form of `redis_{{ redis_port }}` and
`redis_sentinel_{{ redis_sentinel_port }}`.
The script will copy over the contents of the main configuration file, then any new lines added to the running config file after the CONFIG REWRITE tag line. This way Redis Sentinel information and dynamic master/slave status is not lost when re-configuring instances.
danielkza
force-pushed
the
replication-role-detection
branch
from
b5187fe
to
4ed1988
Compare
The service module doesn't accept implementation-specific arguments before Ansible 2.2.
The Sentinel config file contains the Redis password, and therefore, cannot be publicly readable. Also ensure files have the redis group set.
- Reduce verbosity - Only set permissions of dynamic config file (not it's containing directory) - Correctly use redis user and group from template
danielkza
force-pushed
the
replication-role-detection
branch
from
4ed1988
to
fa8e1a8
Compare
|
I fixed systemd quoting issues (as #30) by rebasing some commits. |
|
It seems Ansible 1.9 doesn't support the systemd module :( edit: scratch that, it's actually just in 2.2. I guess I need to call daemon-reload manually instead. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make the role aware of the runtime-assigned replication roles of existing Redis instances to avoid forcing role changes to instances (which might cause downtime).
Do it by sourcing data from the
INFORedis command through a fact script, then using that information to determine whether an already running instance is a master or a slave. Then, if the option is enabled, ignore the manually assigned role in favor of the detected role. That fact script can also possibly replace the existingredisone, which is really not that useful, since a) you likely already have the data it contains from the inventory b) it doesn't support/can't distinguish more than once instance.Hopefully is enough to fix #22 without requiring any weird configuration generation.Unfortunately that was too optimistic, it seems some weird stuff will be necessary.Tests are pending - I'm posting this for some feedback before going further.