| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-08-31 | |||
| 16:37:11 | sean-k-mooney | ya this is proably less readable then just doing object.whatever=somethign | |
| 16:37:29 | sean-k-mooney | its certenly more complicated and eaiser to mess up | |
| 16:37:29 | bauzas | and make sure we populate it from the info cache when we can | |
| 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 | |
| 18:38:43 | sean-k-mooney | yep | |
| 18:38:50 | artom | Ah, yeah true :) | |
| 18:39:14 | sean-k-mooney | go do dad taxi :) | |
| 20:27:36 | mnaser | is there a 'pre-release' checklist for nova | |
| 20:28:02 | mnaser | it'd be nice if some migrations that hit the same table were combined, s=>t includes adding vpmem and resources to instance_extra | |
| 20:28:14 | mnaser | so going through that painful migration once might be nicer/easier | |
| 20:33:00 | mriedem | mnaser: closest is probably here now https://docs.openstack.org/nova/latest/contributor/ptl-guide.html | |
| 20:33:20 | mriedem | https://wiki.openstack.org/wiki/Nova/ReleaseChecklist is older | |
| 20:34:26 | mriedem | with what your asking for there it's probably hard to do when you're working on separate features, db schema migrations aren't rolled together at the end of the release, they happen as features land | |
| 20:35:02 | mriedem | *you're | |
| 20:35:16 | mnaser | mriedem: yeah i guess going on the 'every commit is releasable' makes this a little impossible | |
| 20:36:19 | mriedem | those didn't actually migrate data did they? just alter table to add the column on a big table? | |