| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-08-31 | |||
| 16:33:28 | sean-k-mooney | this is where its filtered out | |
| 16:33:31 | sean-k-mooney | https://github.com/openstack/nova/blob/b5d48043466b53fbdfe7b93c2e4efd449904e593/nova/objects/request_spec.py#L655-L657 | |
| 16:34:39 | sean-k-mooney | which we call in create before we create it in the db | |
| 16:34:51 | sean-k-mooney | then we load back from the db object | |
| 16:35:41 | bauzas | sean-k-mooney: that's the standard way of providing a ovo field that's not persisted | |
| 16:36:01 | sean-k-mooney | "standard" i was expecting to have a flag in the field deffintion | |
| 16:36:12 | sean-k-mooney | like nullable=false | |
| 16:36:20 | bauzas | ie. you explicitely provide a lazy-loading mechanism for getting it, but you also delete its primitive before persisting to the DB | |
| 16:36:20 | sean-k-mooney | but persit=false instead | |
| 16:36:36 | bauzas | sean-k-mooney: oh no, that's made thru code patterns :) | |
| 16:37:04 | bauzas | so, I could technically add a new field | |
| 16:37:09 | bauzas | make it lazy-loadable | |
| 16:37:11 | sean-k-mooney | ya this is proably less readable then just doing object.whatever=somethign | |
| 16:37:29 | bauzas | and make sure we populate it from the info cache when we can | |
| 16:37:29 | sean-k-mooney | its certenly more complicated and eaiser to mess up | |
| 16:38:04 | bauzas | sean-k-mooney: well, I know our objects before they were called o.vo :) | |
| 16:38:14 | bauzas | that certainly helps :) | |
| 16:38:33 | sean-k-mooney | yep but that does not mean we should keep doing it this way | |
| 16:38:38 | sean-k-mooney | its rather archane | |
| 16:38:50 | sean-k-mooney | i can follow it but its not obvious and its not a clean approch | |
| 16:38:54 | sean-k-mooney | it certenly works | |
| 16:39:08 | sean-k-mooney | but this is definetly techdebth | |
| 16:39:25 | sean-k-mooney | addign a property to the class and using it would be cleaner | |
| 16:39:40 | sean-k-mooney | or addign it as a flag on the field and doing it in ovo | |
| 16:40:04 | sean-k-mooney | i guess its not really ovo but the db code | |
| 16:48:42 | bauzas | sean-k-mooney: I'll make a proposal tomorrow morning | |
| 16:50:02 | sean-k-mooney | we shoudl put the segment info in the nova.network.vifobject | |
| 16:50:10 | sean-k-mooney | then get those form the cache | |
| 16:50:43 | sean-k-mooney | https://github.com/openstack/nova/blob/b5d48043466b53fbdfe7b93c2e4efd449904e593/nova/network/model.py#L380-L404 | |
| 16:50:59 | sean-k-mooney | well nova.network.model.Vif | |
| 16:51:21 | sean-k-mooney | or nova.network.model.VIF i guess is the corerct capitalisation | |
| 16:52:07 | sean-k-mooney | we can add a new segment filed to that object | |
| 16:52:26 | sean-k-mooney | these are not ovos | |
| 16:53:35 | sean-k-mooney | but those are what is stored in the info cache | |
| 16:54:16 | sean-k-mooney | they are constucted here https://github.com/openstack/nova/blob/b5d48043466b53fbdfe7b93c2e4efd449904e593/nova/network/neutron.py#L3026-L3076 | |
| 16:58:57 | sean-k-mooney | this is called via allocate_for_instance which is also what populates the info cache | |
| 17:02:09 | sean-k-mooney | oh... | |
| 17:02:48 | sean-k-mooney | that is called first on the compute host. because we only auto allocate networks on compute host if we are passed network ids | |
| 17:09:27 | bauzas | sean-k-mooney: again, I won't modify the vif object | |
| 17:09:35 | bauzas | I think I have everything I need | |
| 17:09:53 | bauzas | we have the requested networks at boot time, so we can directly pass them thru the spec object | |
| 17:10:11 | bauzas | for the move operations, we can recalculate the requested networks from the vifs | |
| 17:10:15 | bauzas | that's it | |
| 17:10:43 | bauzas | hopefully, tomorrow I should be able to provide a new re | |
| 17:10:44 | sean-k-mooney | yes although im suggestign we should have the segments in the vif object anyway | |
| 17:10:45 | bauzas | rev | |
| 17:10:55 | sean-k-mooney | i.e. we should add them | |
| 17:11:24 | bauzas | sean-k-mooney: I'm more in favor of what gibi suggested in the spec review, ie. getting the aggregates directly from neutron as an a-c query | |
| 17:11:32 | bauzas | like we do for bw-aware instances | |
| 17:11:41 | sean-k-mooney | bauzas: yes that is what i orginally suggested | |
| 17:11:43 | bauzas | but that's next release | |
| 17:11:49 | sean-k-mooney | but that not going to happen this release | |
| 17:11:53 | bauzas | yup | |
| 17:11:56 | sean-k-mooney | but even if we get them form neutron | |
| 17:12:03 | sean-k-mooney | we will need to store them in the vif object | |
| 17:12:13 | sean-k-mooney | that is why i was suggestign adding them there | |
| 17:12:24 | bauzas | either way, kids go back to school tomorrow morning, stopping now | |
| 17:12:25 | sean-k-mooney | anyway ill take a look at your patch tomorrow | |
| 17:13:05 | bauzas | sean-k-mooney: bye and thanks for all the fish | |
| 17:27:42 | sean-k-mooney | yum fish.... i was ment to get sushi yesterday but resturant nice resturant was not delivering. i think they are still offline today but someday this week ill will order some and it will be awsome. for now im just oging to go make dinner o/ | |
| 18:05:27 | sean-k-mooney | gibi: ill try to test your sriov series tomorrow but feel free to remind me to if i forget. | |
| 18:27:38 | sean-k-mooney | artom: added a few more details to https://review.opendev.org/#/c/747451/4 but im +1 on the patch | |
| 18:27:59 | sean-k-mooney | basically i just noted the binding point for the singel port binding workflow to show that that is also valid | |
| 18:28:15 | sean-k-mooney | and responded to lee's question regarding asserting the host | |
| 18:28:47 | sean-k-mooney | melwitt: ^ if you want me to expand on anything in particalar let me know but it looks correct. | |
| 18:31:21 | artom | sean-k-mooney, so when I tested this I didn't have any of the binding stuff in the vifs | |
| 18:31:31 | artom | Though in retrospect maybe I was running without the extension? | |
| 18:32:40 | sean-k-mooney | how do you mean | |
| 18:33:09 | sean-k-mooney | when you tested this with devstack or unit/functional tests | |
| 18:33:16 | artom | sean-k-mooney, devstack | |
| 18:33:30 | sean-k-mooney | which vifs did you check | |
| 18:33:39 | artom | sean-k-mooney, all of them :P | |
| 18:34:08 | artom | sean-k-mooney, even the network_info from the Neutron API didn't have them | |
| 18:34:28 | sean-k-mooney | which binding info are you looking for | |
| 18:34:45 | sean-k-mooney | the host id is not in the nova.network.model.VIF object | |
| 18:34:57 | sean-k-mooney | which is what is in the network info | |
| 18:35:09 | artom | sean-k-mooney, I guess it converts | |
| 18:35:24 | artom | Err, dad taxi time | |
| 18:35:27 | artom | Back in a bit | |
| 18:35:29 | sean-k-mooney | do you mean the migrating_to fields were not in the binding profile | |
| 18:35:32 | sean-k-mooney | sure | |
| 18:35:53 | artom | sean-k-mooney, that was there | |
| 18:35:54 | artom | http://paste.openstack.org/ | |
| 18:35:56 | artom | Err | |
| 18:36:01 | artom | http://paste.openstack.org/show/797304/ | |
| 18:36:07 | artom | I actually saved it at the time | |
| 18:36:19 | artom | To compare the nw_info from the API vs the one from the cache | |
| 18:36:23 | artom | That link is the one from the API | |
| 18:36:28 | artom | It has migrating_to | |
| 18:36:31 | artom | But that's it | |
| 18:36:36 | artom | The one in the cache didn't have it | |
| 18:36:42 | sean-k-mooney | ah ok | |
| 18:36:52 | sean-k-mooney | then ya i guess you could assert that | |
| 18:37:01 | artom | http://paste.openstack.org/show/797305/ is from the cache | |
| 18:37:04 | sean-k-mooney | the one in the cache has not been update at all then | |
| 18:37:19 | artom | OK, really have to bounce | |
| 18:37:43 | sean-k-mooney | artom: cool but just so you know you just showed there is something you can test in a follow up patch :P | |
| 18:38:07 | sean-k-mooney | you also showed that its doint the right thing | |
| 18:38:37 | artom | sean-k-mooney, checking that "migrating_to" is *not* in the VIF? | |
| 18:38:39 | sean-k-mooney | since the cache does not have any of the chagne done during migration | |