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 Neil Jerram on 2016-11-11
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
networking-calico
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

Neil 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  Edit
Everyone can see this information.

Other bug subscribers