CTDB ipreallocated event handling

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

CTDB ipreallocated event handling

Samba - samba-technical mailing list
Hi Martin,

I would like to understand the handling and the of the ipreallocated
event and the things connected with it. I've a few notes and questions.

According to the README in the events.d folder, the event is triggered
after "releaseip", "takeip" and "updateip" events.
But at the end of the file I read the following:

* The "ipreallocated" event is run on all nodes.  It is even run if no
  "takeip", "releaseip" or "updateip" events were triggered.

This sounds for me, that one of the statements is not correct :-)

As far as I can see, the event is triggered every time, when the ip
address allocation could have been changed - and this is nice. We should
specify the exact behaviour in the 'ipreallocated' passage.


The next issue is the usage of the service_reconfigure() function in the
event scripts.
According to the README, "an explicit ipreallocated event handler is
usually not necessary", because the service_reconfigure() function "will
be implicitly run before the ipreallocated event".

Who does call the service_reconfigure() function and why this is done
before the ipreallocated event?

The 60.nfs event script has a ipreallocated handler which calls the
ctdb_service_reconfigure() function explicitly. Is this fine? Wouldn't
this call service_reconfigure() twice? The first time by the magic call
before the ipreallocated event and the second with the ipreallocated
event? Or is the ipreallocated event ignored, because the first magic
call un-sets the "need reconfigure"-flag?

I would be glad if you can help me to clarify this :-)

Best regards,
Björn

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CTDB ipreallocated event handling

Samba - samba-technical mailing list
Hi Björn

On Tue, 11 Jul 2017 16:29:49 +0200, Bjoern Baumbach <[hidden email]>
wrote:

> I would like to understand the handling and the of the ipreallocated
> event and the things connected with it. I've a few notes and questions.
>
> According to the README in the events.d folder, the event is triggered
> after "releaseip", "takeip" and "updateip" events.
> But at the end of the file I read the following:
>
> * The "ipreallocated" event is run on all nodes.  It is even run if no
>   "takeip", "releaseip" or "updateip" events were triggered.
>
> This sounds for me, that one of the statements is not correct :-)
>
> As far as I can see, the event is triggered every time, when the ip
> address allocation could have been changed - and this is nice. We should
> specify the exact behaviour in the 'ipreallocated' passage.
Right, the documentation is unclear. Patch attached to try to fix
that.  :-)

> The next issue is the usage of the service_reconfigure() function in the
> event scripts.
> According to the README, "an explicit ipreallocated event handler is
> usually not necessary", because the service_reconfigure() function "will
> be implicitly run before the ipreallocated event".
>
> Who does call the service_reconfigure() function and why this is done
> before the ipreallocated event?
>
> The 60.nfs event script has a ipreallocated handler which calls the
> ctdb_service_reconfigure() function explicitly. Is this fine? Wouldn't
> this call service_reconfigure() twice? The first time by the magic call
> before the ipreallocated event and the second with the ipreallocated
> event? Or is the ipreallocated event ignored, because the first magic
> call un-sets the "need reconfigure"-flag?
>
> I would be glad if you can help me to clarify this :-)
The documentation was out of date.

When I was young and foolish (yes, last week... or perhaps last year) I
believed in having a lot of implicit magic to do things
automatically... and obfuscate the code.  So I added a lot of
features so I could write less, but more confusing, code... ;-)

We used to have a magic function service_check_reconfigure() that was
called in the boiler-plate before the case statement in each event
script. That function would check if the event was "ipreallocated" and
if ctdb_service_needs_reconfigure() was true.  If so, it would run
ctdb_service_reconfigure(), which runs service_reconfigure() - yes, 2
different functions!

Now that service_check_reconfigure() is gone, we check
ctdb_service_needs_reconfigure() explicitly in the "ipreallocated" event and run ctdb_service_reconfigure(), which we still have because
it does a bit of magic that we don't quite want to get rid of without
thinking carefully.

All of this will change quite a bit if we get through the grand
redesign/reimplementation that I outlined in my SambaXP talk this
year...

peace & happiness,
martin

0001-ctdb-docs-Update-documentation-of-ipreallocated-even.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CTDB ipreallocated event handling

Samba - samba-technical mailing list
Hi Martin,

thank you very much for the explanation and updating the docs!

On 07/12/2017 04:40 AM, Martin Schwenke wrote:
> The documentation was out of date.
>
> Now that service_check_reconfigure() is gone, we check
> ctdb_service_needs_reconfigure() explicitly in the "ipreallocated" event and run ctdb_service_reconfigure(), which we still have because
> it does a bit of magic that we don't quite want to get rid of without
> thinking carefully.

This was a good decision. Handling the changes in the "ipreallocated"
event is much more comprehensible :-)

The updated documentation looks much better.

Adding the "updateip" event to the following part might help some users,
who like to implement their own event scripts:

-* An event script can use ctdb_service_set_reconfigure() in "takeip"
-  or "releaseip" events to flag that its service needs to be
+* An event script can use ctdb_service_set_reconfigure() in "takeip",
+  "releaseip" or "updateip" events to flag that its service needs to be
   reconfigured. The "ipreallocated" event can then use

Best regards,
Björn

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Besuchen Sie uns vom 10.-12.10.2017 auf der it-sa
IT-Security Messe in Nürnberg, Halle 9, Stand 204
Freitickets: http://it-sa.de/voucher Code:A361597

Meet us at IT security fair it-sa in Nuremberg
October 10th-12th 2017 in hall 9 at booth 204
tickets:http://it-sa.de/voucher code: A361597

Meet us at Storage Developer Conference (SDC)
Santa Clara, CA USA, September 11th-14th 2017

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CTDB ipreallocated event handling

Samba - samba-technical mailing list
Hi Björn,

On Wed, 12 Jul 2017 13:28:13 +0200, Bjoern Baumbach <[hidden email]>
wrote:

> thank you very much for the explanation and updating the docs!
>
> On 07/12/2017 04:40 AM, Martin Schwenke wrote:
> > The documentation was out of date.
> >
> > Now that service_check_reconfigure() is gone, we check
> > ctdb_service_needs_reconfigure() explicitly in the "ipreallocated"
> > event and run ctdb_service_reconfigure(), which we still have
> > because it does a bit of magic that we don't quite want to get rid
> > of without thinking carefully.  
>
> This was a good decision. Handling the changes in the "ipreallocated"
> event is much more comprehensible :-)
>
> The updated documentation looks much better.

Yay!  :-)

> Adding the "updateip" event to the following part might help some
> users, who like to implement their own event scripts:
>
> -* An event script can use ctdb_service_set_reconfigure() in "takeip"
> -  or "releaseip" events to flag that its service needs to be
> +* An event script can use ctdb_service_set_reconfigure() in "takeip",
> +  "releaseip" or "updateip" events to flag that its service needs to
> be reconfigured. The "ipreallocated" event can then use

Are you happy for me to squash that change in with my patch and re-post?

peace & happiness,
martin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CTDB ipreallocated event handling

Samba - samba-technical mailing list
On Thu, 13 Jul 2017 15:30:33 +1000, Martin Schwenke via samba-technical
<[hidden email]> wrote:

> On Wed, 12 Jul 2017 13:28:13 +0200, Bjoern Baumbach <[hidden email]>
> wrote:
>
> > thank you very much for the explanation and updating the docs!
> >
> > On 07/12/2017 04:40 AM, Martin Schwenke wrote:  
> > > The documentation was out of date.
> > >
> > > Now that service_check_reconfigure() is gone, we check
> > > ctdb_service_needs_reconfigure() explicitly in the "ipreallocated"
> > > event and run ctdb_service_reconfigure(), which we still have
> > > because it does a bit of magic that we don't quite want to get rid
> > > of without thinking carefully.    
> >
> > This was a good decision. Handling the changes in the "ipreallocated"
> > event is much more comprehensible :-)
> >
> > The updated documentation looks much better.  
>
> Yay!  :-)
>
> > Adding the "updateip" event to the following part might help some
> > users, who like to implement their own event scripts:
> >
> > -* An event script can use ctdb_service_set_reconfigure() in "takeip"
> > -  or "releaseip" events to flag that its service needs to be
> > +* An event script can use ctdb_service_set_reconfigure() in "takeip",
> > +  "releaseip" or "updateip" events to flag that its service needs to
> > be reconfigured. The "ipreallocated" event can then use  
>
> Are you happy for me to squash that change in with my patch and re-post?
Assuming the answer to this is "yes", so updated patch is attached.  :-)

Please review and maybe push...

peace & happiness,
martin

0001-ctdb-docs-Update-documentation-of-ipreallocated-even.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CTDB ipreallocated event handling

Samba - samba-technical mailing list
On Thu, Jul 13, 2017 at 4:18 PM, Martin Schwenke via samba-technical <
[hidden email]> wrote:

> On Thu, 13 Jul 2017 15:30:33 +1000, Martin Schwenke via samba-technical
> <[hidden email]> wrote:
>
> > On Wed, 12 Jul 2017 13:28:13 +0200, Bjoern Baumbach <[hidden email]>
> > wrote:
> >
> > > thank you very much for the explanation and updating the docs!
> > >
> > > On 07/12/2017 04:40 AM, Martin Schwenke wrote:
> > > > The documentation was out of date.
> > > >
> > > > Now that service_check_reconfigure() is gone, we check
> > > > ctdb_service_needs_reconfigure() explicitly in the "ipreallocated"
> > > > event and run ctdb_service_reconfigure(), which we still have
> > > > because it does a bit of magic that we don't quite want to get rid
> > > > of without thinking carefully.
> > >
> > > This was a good decision. Handling the changes in the "ipreallocated"
> > > event is much more comprehensible :-)
> > >
> > > The updated documentation looks much better.
> >
> > Yay!  :-)
> >
> > > Adding the "updateip" event to the following part might help some
> > > users, who like to implement their own event scripts:
> > >
> > > -* An event script can use ctdb_service_set_reconfigure() in "takeip"
> > > -  or "releaseip" events to flag that its service needs to be
> > > +* An event script can use ctdb_service_set_reconfigure() in "takeip",
> > > +  "releaseip" or "updateip" events to flag that its service needs to
> > > be reconfigured. The "ipreallocated" event can then use
> >
> > Are you happy for me to squash that change in with my patch and re-post?
>
> Assuming the answer to this is "yes", so updated patch is attached.  :-)
>
> Please review and maybe push...
>
> peace & happiness,
> martin
>

Pushed to autobuild.

Amitay.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CTDB ipreallocated event handling

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On 07/13/2017 08:18 AM, Martin Schwenke via samba-technical wrote:
>> Are you happy for me to squash that change in with my patch and re-post?
> Assuming the answer to this is "yes", so updated patch is attached.  :-)

Yes! Thank you :-)

Best regards,
Björn

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Loading...