[PATCHES] make ctdb runnable in non-FHS installs

[Michael Adam]

Thanks for the review, Martin!

On 2016-06-08 at 11:28 +1000, Martin Schwenke wrote:
> On Wed, 8 Jun 2016 01:30:02 +0200, Michael Adam <obnox at samba.org> wrote:
> 
> > While debugging an issue with ctdb, I thought (again)
> > it would be convenient to be able to run ctdb(d) in
> > an install that is not conforming to FHS.
> > 
> > We do already have ctdbd_wrapper to call the daemon,
> > but several places in event scripts and ctdbd_wrapper
> > itself directly call ctdbd. So this assumes that ctdb
> > is in the patch and is to be called without options.
> > Hence running from a non-fhs install-prefix does not
> > work at all.
> > 
> > This patch introduces ctdb_wrapper (like ctdbdb_wrapper)
> > but for the ctdb tool and much simpler, and it changes
> > all important places (event scripts and ctdbd_wrapper) to
> > call ctdb_wrapper instead of calling ctdb direclty.
> > 
> > So this patchset lets me successfully start and manage
> > a ctdb cluster from a non-FHS location.
> > 
> > Review appreciated!
> 
> Thanks!  I've been wanting to do something like this for a long time!
> 
> Comments:
> 
> * I don't think using "eval" gains you anything.  In most places you can
>   just change:
> 
>     ctdb -> $ctdb
> 
>   I think the exception is the wrapper.  I think the eval is needed
>   there because the wrapper adds single-quotes quotes around option
>   values so it can then maintain whitespace in the values when the
>   command-line is expanded using eval.  It might be work testing if
>   that actually works and, if not, we could simplify and allow
>   ctdb_wrapper to just use exec.  Similarly, we could decide to
>   simplify the code by not allowing whitespace in option values.  Blah,
>   blah, blah...  :-)

Thanks. I am never 100% sure about the usefulness of and
need to use eval. I just imitated patterns I saw elsewhere. :-)

So I'll simply go with your advice and (for now) only leave
the use in ctdbd_wrapper eval'd.

> * Copying and using maybe_set() only once is probably overkill.  I'd
>   suggest either:
> 
>   1. Moving it to the functions file (since it is loaded anyway),
>      perhaps with a rename (ctdb_options_maybe_set ?) and still calling
>      it.  This makes sense if we want to add more options.  Perhaps
>      CTDB_NODES, which is pulled from environment by ctdb command?
> 
>   2. Just inline the construction of ctdb_options.
> 
>   I think I prefer (1).  :-)

Good point, i'll change it accordingly.

> * I think you're solving 2 problems here:
> 
>   1. Help the event scripts find the ctdb command.
> 
>   2. Help the ctdb command process configuration options.

Very true.

>   Because (2) is valuable to external callers too, I wonder if you want
>   to change things so that /usr/local/bin/ctdb is the wrapper
>   and /usr/local/libexec/ctdb/ctdb_tool (or ctdb_cli, or similar) is
>   the binary.  Then everyone gets (2).  Then we can still do (1) by
>   using CTDB_TOOL="${CTDB_TOOL:-/usr/local/bin/ctdb}", or similar.
> 
> What do you think?

I thought the same thing actually, but I feared it would
be considered too radical. Hence I did not dare to propose
it. But your suggestion confirms my ideas.

I'll post a second version of the patchset with the above
changes, soon!

Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160608/b2e0b353/signature.sig>