Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Added redis RA, patched IPaddr2 to support multiple virtual IPs #24

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
7 participants

Please review, thanks.

Martin Walter added some commits Jul 15, 2011

Martin Walter The deletion of the primary IP via the ip command will also remove th…
…e secondary IP. Thus, trying to delete the secondary IP after the primary IP was deleted will result in an error. This fix assures that delete_interface() will only result in an error if ' addr delete' has failed and the ip is still listed in addr show.
2c6d3bc
Martin Walter Adds the RA for the redis service, based on that of mysql. Supports m…
…aster/slave configurations of a depth of 1.
d0b3870

"binary" variables normally honor $PATH; you should be able to change this to just redis-server.

"binary" variables normally honor $PATH; you should be able to change this to just redis-cli.

This is not necessary if you define a non-absolute path for binary.

Missing local?

Wrong return code. Should be $OCF_ERR_INSTALLED (the config file might be OK on a different node).

start-stop-daemon is Debian specific. You should replace this with invoking the daemon with >/dev/null 2>&1 &.

Redis will not perform a change of uid by itself. It is usually ran using runit (or other daemon tool) or start-stop daemon. Even on red-hat based distros where this is not present most people will compile the tool to get it running correctly. What I mean is this is probably the best way to run redit at the moment.

If this really does depend on start-stop-daemon, there would have to be a check_binary start-stop-daemon either in start or validate.

Btw: it seems that the original submitter abandoned this effort, so rather than commenting here, it may be wise to fork the repo and implement the changes you consider appropriate. Then we can pick up the review again.

The mysql RA uses ocf_is_ms to test whether it is configured as a Master/Slave set. It does so strictly for maintaining compatibility with existing versions, which always ran mysql as a standard primitive. This is a new resource agent. Are you absolutely sure you need to cater for both modes?

start-stop-daemon is Debian specific. You should replace this with retrieving the pid from $OCF_RESKEY_pid, and then shutting down the process with kill -TERM or kill -QUIT, whichever is applicable.

This part really is not necessary.

You are using a number of possible bashisms (kill -SIG instead of kill -s SIG, local foo=bar instead of local foo; foo=bar), which will not work if /bin/sh is a shell other than bash.

Either remove the bashisms (/usr/bin/checkbashisms will help), or set the interpreter to /bin/bash explicitly.

Member

fghaas commented Jul 25, 2011

Thanks for the contribution. See line notes for redis RA please. I'll review the other patch separately.

fghaas commented on 2c6d3bc Jul 25, 2011

The issue is valid, but the approach to solve it is a bit short-sighted.

ip_stop invokes ip_served to determine whether the configured IP is currently being served by the system. If it is not, then ip_stop immediately exits successfully, and never invokes delete_interface. If that is not working for you and you are observing that the RA attempts to delete a secondary IP that is no longer there (because its primary IP had been removed), then I suggest we need to fix ip_served, not delete_interface.

@fghaas, you're suggestion is of course good, but I'm not sure how this could happened at all, if find_interface() works correctly. @kskmori, could you please take a look.

I haven't been able to reproduce it either.
Only I can guess that it could be a race condition that deletion of the primary IP is completed between find_interface() and delete_interface() for a secondary IP when two IPaddr2s are configured without any ordering.

But even if we applied this patch there still be another possibility that the monitor for a secondary IP may be issued and failed between the primary IP stop and the secondary IP stop. That would be also confusing.

So I think that the complete solution would be, as already noted in the meta-data of IPaddr2, 1) always assign a "static" IP address as the primary, or 2) add a kernel parameter so that a secondary IP would promote when the primary IP was deleted.

fghaas commented on d0b3870 Nov 28, 2011

Martin, have you abandoned this effort? It seems that coredump@a4a5b6b has picked this up and is willing to develop this further.

Owner

metmajer replied Nov 28, 2011

Okay, not a problem. Thanks for getting this started. I will ask José (coredump) to drop your IPaddr2 patch from his fork, though, so we can separate the "new RA" patch from the other which fixes an existing RA.

Erm.. How do I do that? :P My github-fu is lacking.

Ping me on IRC and I'll walk you through it.

How's the state on this subject? The groundwork (especially the M/S part) looked really promising to me, from the first point of view, it is exactly what I have to have. :)

I have lost interest in this matter (and there are obviously more talented people around). However, I am happy to hear that my has some value for you. Otherwise, take a look a the other people's forks.

Contributor

dmuhamedagic commented Oct 30, 2012

@coredump, any news here?

Member

krig commented Jan 8, 2015

This request should be closed as it is superseded by the request in #212.

Contributor

dmuhamedagic commented Jan 14, 2015

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment