With Liberty and later, networking-calico could use events.AFTER_CREATE/INIT to start its etcd connections, instead of the complex current scheme
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 NetworkDhcpAgen
<openstackgerrit> Nakul Dahiwade proposed openstack/neutron: Agent to OVO https:/
<kevinbenton> ijw: bleck
<kevinbenton> ijw: this is what calico is doing https:/
<neiljerram> ijw, see https:/
<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:/
<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
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.