| *** yamamoto has quit IRC | 00:52 | |
| *** yamamoto has joined #openstack-dragonflow | 01:47 | |
| *** yamamoto has quit IRC | 02:11 | |
| *** yamamoto has joined #openstack-dragonflow | 02:15 | |
| *** yamamoto has quit IRC | 02:19 | |
| *** yamamoto has joined #openstack-dragonflow | 02:24 | |
| *** yamamoto has quit IRC | 02:28 | |
| *** yamamoto has joined #openstack-dragonflow | 03:29 | |
| *** yamamoto has quit IRC | 03:35 | |
| *** yamamoto has joined #openstack-dragonflow | 05:09 | |
| *** yamamoto has quit IRC | 05:10 | |
| *** yamamoto has joined #openstack-dragonflow | 05:13 | |
| openstackgerrit | Dima Kuznetsov proposed openstack/dragonflow master: Remove leftovers of port status notifier https://review.openstack.org/492028 | 07:31 |
|---|---|---|
| *** NatanBro has joined #openstack-dragonflow | 07:32 | |
| *** yamamoto has quit IRC | 08:20 | |
| openstackgerrit | Dima Kuznetsov proposed openstack/dragonflow master: Fix update FIP status https://review.openstack.org/492040 | 08:20 |
| *** yamamoto has joined #openstack-dragonflow | 08:30 | |
| openstackgerrit | Merged openstack/dragonflow master: Switch from oslosphinx to openstackdocstheme https://review.openstack.org/487810 | 08:43 |
| *** yamamoto has quit IRC | 09:13 | |
| openstackgerrit | Merged openstack/dragonflow master: Add 'rm -f .testrepository/times.dbm' command in testenv https://review.openstack.org/487799 | 09:35 |
| *** yamamoto has joined #openstack-dragonflow | 10:14 | |
| *** yamamoto has quit IRC | 10:20 | |
| *** yamamoto has joined #openstack-dragonflow | 10:42 | |
| *** NatanBro has quit IRC | 11:20 | |
| openstackgerrit | Dima Kuznetsov proposed openstack/dragonflow master: [05/xx] Change DNAT to rely on Provider app for bridge access https://review.openstack.org/475362 | 11:42 |
| *** yamamoto has quit IRC | 12:00 | |
| oanson | dimak, ping | 12:58 |
| dimak | hey | 12:58 |
| oanson | Regarding https://review.openstack.org/#/c/489994/ | 12:58 |
| dimak | Yes | 12:59 |
| oanson | I think we discussed the 'common' dispatcher code (as used also in bgp service) is to be moved out of df_local_controller, right? | 12:59 |
| dimak | We did | 13:00 |
| *** yamamoto has joined #openstack-dragonflow | 13:00 | |
| dimak | oanson, you suggest it is related to the above patcH? | 13:01 |
| oanson | YEs | 13:04 |
| oanson | Yes* | 13:04 |
| oanson | As I said, I don't like it that we have 2 different dispatch methods in df controller | 13:04 |
| dimak | What 2? | 13:05 |
| oanson | 1. Using model events. 2. <action>_<table_name> | 13:05 |
| oanson | Though I'm not sure how to do it differently - the second method promises that code will run *before* application code | 13:06 |
| oanson | Isn't that code a bit overlapping the port locator object logic? | 13:07 |
| dimak | Port locator is just a lookup | 13:07 |
| dimak | controller handles port state | 13:07 |
| oanson | Why not use it to decide which event to fire? The state here is just is_local or is_remove. That exists in port_locator as well | 13:08 |
| *** yamamoto has quit IRC | 13:08 | |
| oanson | I'm just trying to think of an elegant way to do this. If we can remove this code from df controller entirely and put it somewhere else, that would be great | 13:08 |
| dimak | Not sure I understood your idea, what do you want to use to figure out what event should be fired | 13:10 |
| oanson | port_locator already has an is_local and is_remote function. We don't really need _local_ports and _remote_ports in df controller. | 13:11 |
| oanson | Then update_lport (I know you want it removed) or an event handler on df controller just reads the info from there. | 13:11 |
| dimak | I don't want update_lport, it's too early of an entrypoint | 13:12 |
| dimak | I think I can remote _local/remote_ports in the patch that adds port_locator | 13:13 |
| dimak | though wait, I'm not sure | 13:13 |
| oanson | It is getting a bit intricate. I am not sure I understand the flow too well | 13:14 |
| oanson | For instance, how does migration fit in all this? Or do you want to solve migration later? | 13:14 |
| dimak | First of all, yes | 13:14 |
| dimak | and my plan is, the app updates the correct binding in port locator, then emits the bind/unbind event | 13:15 |
| oanson | Without migration, it should be easy enough. You can use port_locator to know what kind of event you need. | 13:15 |
| dimak | this way binding is stick and the update will not wipe it | 13:15 |
| dimak | then once update arrives, migration app will clear the binding | 13:16 |
| oanson | That makes sense - have migration just change the binding without touching the object | 13:16 |
| dimak | Yes | 13:16 |
| oanson | Maybe we need to extend migration - make it a full fledged 'port location handling app'. | 13:16 |
| oanson | But then you have a mandatory app. | 13:17 |
| dimak | I prefer to start with something rather simple | 13:17 |
| dimak | If its mandatory, why is it an app | 13:17 |
| oanson | Because it is not the controller's responsibility. | 13:18 |
| oanson | I don't think mandatory apps are a problem. They exist everywhere else. | 13:18 |
| oanson | (All operating systems, agents in Neutron implementations, etc.) | 13:18 |
| oanson | Well, almost all OSes. Maybe not DOS :) | 13:19 |
| oanson | And it's only mandatory if you care about port binding. So maybe e.g. L2, L3 don't actually care about it, as long as it's bound. | 13:20 |
| dimak | I can move controller an app, I'll still need port locator lookup | 13:20 |
| oanson | That's all right | 13:21 |
| oanson | The app looks up the info in the locator, and then fires the relevant event | 13:21 |
| dimak | And I'm still not sold on why its an app | 13:21 |
| oanson | You agree it shouldn't be in the controller? | 13:21 |
| dimak | Kind-ish? | 13:22 |
| dimak | It depends on what we want to keep in there | 13:22 |
| oanson | In previous discussions we wanted minimal model-related logic | 13:22 |
| oanson | Except maybe core models | 13:22 |
| oanson | dimak, ? | 13:25 |
| dimak | oanson, yes, sorry | 13:33 |
| oanson | No worries | 13:34 |
| dimak | We can move the callbacks elsewhere, it still doesn't mean it has to be an app | 13:34 |
| oanson | So where will you put them? | 13:34 |
| dimak | dragonflow.controller.not_an_app? :) | 13:35 |
| dimak | They can be in port_locator | 13:35 |
| dimak | though not really related | 13:35 |
| oanson | Sure. What makes it a non-app? | 13:35 |
| dimak | its a few methods that translate one kind of event into another | 13:36 |
| oanson | I have put it on the model | 13:36 |
| dimak | It's semantics | 13:36 |
| oanson | Not sure it's the best idea. | 13:36 |
| oanson | So is the whole software-engineering thing | 13:36 |
| dimak | I don't think it belongs on the model | 13:36 |
| dimak | its strictly controller code | 13:36 |
| oanson | I don't think we have a consensus here :( | 13:38 |
| oanson | Let's do it like this: Keep patch 1 as is. In patch 2, where you introduce port_locator, remove the bound/unbound events in df local controller | 13:39 |
| oanson | We'll discuss what to do with the base events later. | 13:40 |
| oanson | I am not convinced about removing update_lport, but we can revisit that later | 13:40 |
| dimak | Sure, I'll get to it tomorrow, I'm fiddling with DNAT for a bit | 13:41 |
| dimak | Why do you want to keep update_lport" | 13:41 |
| dimak | ? | 13:41 |
| oanson | It feels a bit strange to fire an event, only to catch it again. | 13:41 |
| oanson | And I don't like having to dispatch methods in the same class. It feels overly complex | 13:42 |
| dimak | Why? maybe some apps will want to listen to update events regardless of locality | 13:43 |
| oanson | Especially since df controller doesn't need to catch events from applications now that we reuse port_locator | 13:43 |
| oanson | Probably. | 13:43 |
| dimak | after all, lport implements basic events | 13:43 |
| oanson | So update_lport can fire the original event as well | 13:43 |
| oanson | I think l2, l3 is a use case for what you're saying | 13:43 |
| openstackgerrit | Dima Kuznetsov proposed openstack/dragonflow master: [06/xx] Change DNAT to rely on Provider app for bridge access https://review.openstack.org/475362 | 15:09 |
| openstackgerrit | Dima Kuznetsov proposed openstack/dragonflow master: [WIP][05/xx] Move FIP status updates to L3 plugin https://review.openstack.org/493359 | 15:09 |
| openstackgerrit | Merged openstack/dragonflow master: Restore QoS fullstack tests https://review.openstack.org/492413 | 17:35 |
| openstackgerrit | Merged openstack/dragonflow master: Don't send a value in delete pubsub event https://review.openstack.org/491778 | 17:37 |
| openstackgerrit | Merged openstack/dragonflow master: Remove leftovers of port status notifier https://review.openstack.org/492028 | 17:37 |
| openstackgerrit | Dima Kuznetsov proposed openstack/dragonflow master: [WIP][05/xx] Move FIP status updates to L3 plugin https://review.openstack.org/493359 | 18:29 |
| openstackgerrit | Merged openstack/dragonflow master: BGP service: Register only relevant models https://review.openstack.org/492215 | 20:58 |
| *** yamamoto has joined #openstack-dragonflow | 21:09 | |
| *** yamamoto has quit IRC | 21:20 | |
| *** yamamoto has joined #openstack-dragonflow | 21:56 | |
| *** yamamoto has quit IRC | 21:57 | |
| *** yamamoto has joined #openstack-dragonflow | 21:58 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!