With Liberty and later, networking-calico could use events.AFTER_CREATE/INIT to start its etcd connections, instead of the complex current scheme

Bug #1640969 reported by Nell Jerram
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
networking-calico
New
Undecided
Unassigned

Bug Description

IRC discussion just now:

<ijw> neiljerram: yo
<neiljerram> ijw, hi
<ijw> I think we have a problem using etcd watches from Neutron - do you do that yourselves, at all?
<neiljerram> ijw, we do indeed
<neiljerram> ijw, actually let me be sure ... I assume you mean from the Neutron server?
<ijw> Yup
<ijw> Issue I'm seeing is that the watch seems to be starting before the worker processes are forked, and we have a shared FD at the end of it
<ijw> I wondered if you'd had that or if you had a way to delay the watching
<neiljerram> ijw, Yes, we use an etcd watcher as part of getting agent and port statuses back from etcd into the Neutron DB
<neiljerram> ijw, Yes, we hit that forking issue in early days... Need to refresh my memory how we deal with it.
<ijw> Please tell me what it is when you remember, it's a right pain at the moment
<kevinbenton> ijw: there is an event you can listen for i believe
<openstackgerrit> Nakul Dahiwade proposed openstack/neutron: OVO for NetworkDhcpAgentBinding https://review.openstack.org/370452
<openstackgerrit> Nakul Dahiwade proposed openstack/neutron: Agent to OVO https://review.openstack.org/297887
<kevinbenton> ijw: bleck
<kevinbenton> ijw: this is what calico is doing https://github.com/openstack/networking-calico/blob/master/networking_calico/plugins/ml2/drivers/calico/mech_calico.py#L316-L326
<neiljerram> ijw, see https://github.com/openstack/networking-calico/blob/master/networking_calico/plugins/ml2/drivers/calico/mech_calico.py#L252-262
<kevinbenton> at one point we did have a post fork callback you could listen for just for this case
<neiljerram> kevinbenton, was that post fork callback removed again, or could we still use that?
<kevinbenton> neiljerram: i believe you can listen for resources.PROCESS events.AFTER_INIT
<neiljerram> kevinbenton, Yes, that sounds right.
<kevinbenton> neiljerram, ijw: yeah, background on it here https://review.openstack.org/#/c/189391/
<neiljerram> kevinbenton, Back when we last looked at this, I think we decided to stick with the solution that you now see, because we still wanted networking-calico master to work with Neutron releases that didn't include that callback.
<kevinbenton> neiljerram: i see
<neiljerram> kevinbenton, But not we only want master to work with >= Kilo, and it might have been Kilo when that callback was introduced.
<neiljerram> *now
<ijw> kevinbenton: rather than read quite a meaty patch on gerrit, do you happen to know either how that's used or have an example of where it's used?
<kevinbenton> ijw: you subscribe to that event
<ijw> ok
<kevinbenton> ijw: the receiving function will be called only after forking has happened
<kevinbenton> ijw: so at that point you can setup your etc watcher
<ijw> Hm, so that should work out then, I'll just move my init from where it is into a callback
<ijw> In this instance, kick a thread off, but yeah, all the same thing
<kevinbenton> ijw: keep in mind that each fork will receive this event
<kevinbenton> ijw: so if you only want one single process spawned, you'll have to do some coordination of your own
<kevinbenton> ijw: this event just tells each fork that forking is done
<neiljerram> ( kevinbenton ijw Actually that event was merged shortly before Liberty )
<kevinbenton> neiljerram, ijw: keep in mind if you do want code compatible with older releases, the event name changed
<ijw> No, that's fine, I knew I'd be running more threads than entirely necessary (sucks, probably wants attention at some point, but not the end of the world)
<ijw> Older than Liberty
<ijw> ?
<kevinbenton> no, Liberty uses events.AFTER_CREATE
<kevinbenton> newest is AFTER_INIT
<kevinbenton> i think it changed in newton
<neiljerram> kevinbenton, ah, thanks for noting that. That's an easily manageable detail though.
<kevinbenton> so you would just need two subscribe calls
<kevinbenton> neiljerram: right
<neiljerram> So action with me to review our Kilo support story... :-) ijw thanks for reminding me about this!
<ijw> kevinbenton: so, simply register whichever of those constants actually exists?
<kevinbenton> ijw: yep
<kevinbenton> ijw: well no
<kevinbenton> ijw: AFTER_CREATE will always exist i think
<kevinbenton> because that's used for 'normal' resources
<kevinbenton> ijw: so use AFTER_INIT if it exists
<kevinbenton> ijw: else AFTER_CREATE

Revision history for this message
Nell Jerram (neil-jerram) wrote :

However, at the moment networking-calico works with >= Kilo (as discussed in https://bugs.launchpad.net/networking-calico/+bug/1640967), but the required AFTER_INIT/AFTER_CREATE notification was only added for Liberty. So, to adopt the latter approach, we'd either need to rethink our Kilo support, or to retain the existing code as well as a new notification-based approach, but only use the existing code with Kilo.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.