[PATCHES] make ctdb runnable in non-FHS installs

Martin Schwenke martin at meltin.net
Fri Jun 10 10:28:19 UTC 2016


On Fri, 10 Jun 2016 09:44:54 +0200, Michael Adam <obnox at samba.org>
wrote:

> On 2016-06-10 at 15:43 +1000, Martin Schwenke wrote:
> > Hi Michael,
> > 
> > On Thu, 9 Jun 2016 17:57:21 +0200, Michael Adam <obnox at samba.org> wrote:
> > 
> > > After a long fruitful IRC discussion with Martin
> > > and Amitay, we are sticking with #1, i.e. not renaming
> > > ctdb and naming the wrapper ctdb. This poses too many
> > > problems.
> > > 
> > > Instead the updated patchset first makes sure that
> > > the proper ctdb binary can always be found as $CTDB
> > > in the scripts (and is used in the scripts). For the
> > > tests, a simple CTDB=ctdb override did the trick.
> > 
> > The hunk in the last patch that does the wscript sed magic needs to
> > come into the first patch, so that "/usr/local/bin" gets substituted on
> > installation.
> 
> Indeed! Done.
> 
> > Can you please move the patch that sets CTDB in the test code to be
> > second in the series?  That way it comes before the other patches that
> > use $CTDB in eventscripts, and that means that tests will pass for
> > each patch in the series.
> 
> Good pont. Done.
> 
> > In that patch it turns out I was wrong about needing something in
> > local_daemons.bash.  This is already done in integration.bash,
> > which is used by "simple" and "complex" tests.  Sorry I got that
> > wrong!
> 
> Correct. Just verified (with a complete successfull ctdb autobuild)
> and removed the hunk. Somehow I originally thought this was needed...
> 
> > For the hunk that remains, you could do the export on the same
> > line as the assignment, but that's no big deal...  :-)
> 
> Done.
> 
> > > Node: I provided patches to make use of $CTDB
> > > instead of verbatim 'ctdb' as one patch per file.
> > > I like this better, but if people insist, we could
> > > also squash those into one big sweeping change patch.
> > 
> > No worries...  :-)
> > 
> > > Finally the super simple wrapper is provided
> > > as ctdb_wrapper. This is mainly convenient
> > > when a non-default CTDB_SOCKET is set in the conf file.
> > 
> > > Note: instead of doing the maybe_set thing to
> > > turn CTDB_SOCKET into a --socket option, we could
> > > use export CTDB_SOCKET, but I like cmdline options
> > > better and we don't need the export for other purposes.
> > 
> > Can we please drop the wrapper (and the maybe_set() refactoring) for
> > now, get the $CTDB stuff upstream and discuss the wrapper further?
> 
> Sure!
> 
> > Before I added all of the sed magic to make things install cleanly, I
> > thought about adding a wrapper... but I'm not sure it serves a useful
> > purpose anymore.  For a regular installation CTDB_SOCKET should not be
> > set - the socket location should be set at compile time.  For testing
> > with local daemons, the preference is to use onnode with
> > $CTDB_NODES_SOCKETS set, the way local_daemons.bash does it.
> 
> OK. Onnode can also read $CTDB_BASE/nodes to build the list
> of nodes, which seems even more practical to me.
> 
> > This provides a nice abstraction layer so tests can be written
> > that can run against both local daemons and a real cluster,
> > like the "simple" tests.
> > 
> > If we have a wrapper then it will also need a manpage, since it is
> > installed in a bin directory.
> 
> True.
> 
> > Also, I honestly think it will confuse people and they won't know which
> > command to use...
> 
> Sold. I withdraw the ctdb_wrapper! :-)
> 
> Attached find the updated patchset.
> 
> Thanks for the reviews!

Reviewed-by: Martin Schwenke <martin at meltin.net>

Pushed...

peace & happiness,
martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160610/e5b878a0/attachment.sig>


More information about the samba-technical mailing list