Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-06-22
10:31:51 sean-k-mooney oh
10:31:56 sean-k-mooney sorry you ar just setting the property
10:32:10 sean-k-mooney ya so that is why that property stores its data in the profile
10:32:20 sean-k-mooney to avoid the ovo change and be backportable
10:32:22 sean-k-mooney ok
10:32:32 sean-k-mooney so yes we can just set the property to true uncondtionally
10:33:37 sean-k-mooney but we als have https://github.com/openstack/nova/blob/5979c648462b03a2fe90148f20f099c964cdd298/nova/network/model.py#L405
10:33:41 sean-k-mooney ok im confused
10:34:01 sean-k-mooney i tought that was an ovo issue but maybe not
10:34:06 stephenfin those are just dics
10:34:08 stephenfin *dicts
10:34:17 stephenfin so we can store whatever we want in them
10:34:17 sean-k-mooney ok so we just pass delegate_create=True
10:34:44 sean-k-mooney stephenfin: they are but they are also https://github.com/openstack/nova/blob/master/nova/objects/instance_info_cache.py#L38
10:35:21 sean-k-mooney i guess thats a network model
10:35:31 stephenfin all that that's doing is calling '.json' on the object https://github.com/openstack/nova/blob/5979c648462b03a2fe90148f20f099c964cdd298/nova/objects/fields.py#L1084
10:35:37 stephenfin so it doesn't really matter
10:36:08 sean-k-mooney well its is using https://github.com/openstack/nova/blob/5979c648462b03a2fe90148f20f099c964cdd298/nova/network/model.py#L512
10:36:19 sean-k-mooney ok i guess its fine
10:36:28 sean-k-mooney i had issue adding other filds in the past
10:36:37 sean-k-mooney but maybe i was chanign something else
10:36:52 sean-k-mooney in anycase i think your right we can just set it to true there
10:37:11 sean-k-mooney but we need to ensure in the migration path we set it to false wehn its not present orginally
11:13:44 gibi stephenfin, sean-k-mooney: for me https://review.opendev.org/c/openstack/nova/+/797142 looks fine, but sean-k-mooney has a -1 on it so I'm affraid of approving it
11:15:55 sean-k-mooney gibi: thats what we were discussign above stephen fixed it for migration but its still using the wrong interface type on boot
11:16:17 gibi OK, so there will be changes. thanks
11:16:26 sean-k-mooney so with that patch as is we boot with interface type=bridge then we migrate with ethernet and hard reboot back to bridge
11:16:49 sean-k-mooney the orignal intent was to always use ethernet
11:17:21 sean-k-mooney gibi: technically as is this should actully fix the migration issue we were trying to fix
11:17:35 sean-k-mooney but its a little odd to change the type just for the migration
11:17:54 sean-k-mooney im also worried that this current state breaks ovs on windows
11:18:15 sean-k-mooney so i would prefer to fix the boot case.
11:18:31 opendevreview Merged openstack/nova stable/victoria: [neutron] Get only ID and name of the SGs from Neutron https://review.opendev.org/c/openstack/nova/+/787252
11:19:03 opendevreview Stephen Finucane proposed openstack/nova master: libvirt: Always delegate OVS plug to os-vif https://review.opendev.org/c/openstack/nova/+/797428
11:20:05 stephenfin sean-k-mooney: gibi: ^ there's the follow-up patch. I need to figure out test coverage (currently all unit tests are passing and I'm running functional tests now, but I guess I should add a new test). We can proceed with the current patch as-is though IMO
11:20:13 stephenfin it definitely fixes one issue
11:20:57 sean-k-mooney well what about for backporting
11:21:20 sean-k-mooney unless you are going to squash all 3 patches you would have to undo the squashing you have already done
11:22:15 stephenfin Not necessarily. The main patch only ever fixed the issue for live migration but had a bug which we've squashed the fix in for. This new patch stands on its own feet
11:22:25 sean-k-mooney it was ment to do it always
11:22:46 sean-k-mooney ti did do it always before i put in the detection mechanium for upgrades
11:23:00 stephenfin Also, I don't think hard reboot is an issue. We'll have set the 'delegate_create' attribute of the VIF entries as part of live migration and nothing unsets that
11:23:15 sean-k-mooney stephenfin: well i would prefer not to backport it in its current state
11:23:17 stephenfin So it'll change during live migration but it shouldn't change after a hard reboot
11:23:34 sean-k-mooney it will revert back to bridge in a hard reboot
11:23:46 sean-k-mooney without the final patch
11:23:52 stephenfin why?
11:24:04 sean-k-mooney because bridge is what we use for spawn
11:24:23 stephenfin unless the VIF objects in network_info have delegate_create=True
11:24:32 sean-k-mooney which they wont
11:24:49 stephenfin again, why?
11:24:50 sean-k-mooney the migration data vifs are not the same as the ones in the info cache
11:25:15 sean-k-mooney the ones in the info cache get rebuilt form neutron by the perodic task
11:25:26 sean-k-mooney or whenever we get a network-changed event
11:25:40 sean-k-mooney so delegate_create=True will be lost
11:25:56 sean-k-mooney if its ever populated in it in the first place
11:26:32 stephenfin ah, TIL
11:26:50 stephenfin okay, so I'll squash the third patch in too so
11:27:31 sean-k-mooney ok am can we wait for grenade to run on the 3rd patch then we can merge the second one?
11:27:39 sean-k-mooney i just want to make sure that it is not breaking anything
11:29:38 stephenfin we really need to mute that SQLAlchemy warning
11:29:44 sean-k-mooney yep
11:29:52 stephenfin I've a fix proposed to oslo.db
11:29:59 stephenfin https://review.opendev.org/c/openstack/oslo.db/+/797426
11:30:00 sean-k-mooney ah nice
11:30:22 sean-k-mooney i was assuming we woudl set that to false
11:30:54 sean-k-mooney are we ok with caching softdelete or json data
11:31:55 sean-k-mooney The requirements for cacheable elements is that they are hashable and also that they indicate the same SQL rendered for expressions using this type every time for a given cache value.
11:31:57 sean-k-mooney ok
11:32:02 sean-k-mooney so ya that is correct
11:32:10 sean-k-mooney cache_ok applies
12:39:30 sean-k-mooney stephenfin: the full job suite is not finished yet but the live migration job passed and is now using ethernet so thats good to see just waiting on grenade now
12:52:07 sean-k-mooney gibi: gmann so intersing problem, apparently we do not validate server_group policy strings
12:52:30 sean-k-mooney we have never intended the server group to be user extendable right?
12:52:47 sean-k-mooney e.g. by using out of tree filters and weighers?
12:59:22 sean-k-mooney actully we do have schema validataion for thsi
12:59:24 sean-k-mooney https://github.com/openstack/nova/blob/5979c648462b03a2fe90148f20f099c964cdd298/nova/api/openstack/compute/schemas/server_groups.py#L22-L67
12:59:44 sean-k-mooney but i guess that is not workign i need to confrim what is being repoted downstream
13:10:40 stephenfin sean-k-mooney: gibi: gmann: Yeah, there's a request to undo the validation of what we pass to the API in OSC because its seems someone is relying on this broken behavior https://storyboard.openstack.org/#!/story/2008975
13:13:19 sean-k-mooney so i thikn we need to figure out why the json schema validation is not rejectign there request and fix that and backport it in thet api
13:14:01 sean-k-mooney we could have a workaround config option to disable that but this is not a valid extenion point today
13:15:01 gibi I agree that we should validate the policy
13:15:07 gibi so let's fix it
13:15:10 gibi and backport it
13:15:29 gibi I'm meh on providing the workaround config
13:15:39 gibi if we do that the the default should be still to validate the policy
13:15:39 sean-k-mooney the real question is why is the existing code not validating it when we have the json scmea files to do that and unit test that apprently are testing the validation
13:16:08 sean-k-mooney gibi: yep agreed if we have a config option it shoudl default to validating
13:16:25 gibi good question. unfortunately I have swamped with other things right now
13:57:33 sean-k-mooney am.... wehre do we actully do the jsonscema validation fo the api schema https://codesearch.opendev.org/?q=jsonschema&i=nope&files=&excludeFiles=&repos=openstack/nova
14:27:07 sean-k-mooney ah this is how we import them https://codesearch.opendev.org/?q=from%20nova.api.openstack.compute.schemas%20import&i=nope&files=nova%2Fapi&excludeFiles=&repos=openstack/nova
14:27:42 sean-k-mooney and this is where we use the server group schema https://opendev.org/openstack/nova/src/branch/master/nova/api/openstack/compute/server_groups.py#L26
14:29:15 sean-k-mooney and this is where we apply the vlaidations https://opendev.org/openstack/nova/src/branch/master/nova/api/openstack/compute/server_groups.py#L182-L184
14:42:56 gibi sean-k-mooney: there is a bug around VDPA https://bugs.launchpad.net/nova/+bug/1933096
14:43:42 sean-k-mooney that should not happen
14:43:53 sean-k-mooney we have a version check for this
14:44:23 sean-k-mooney let me triple check but im pretty sure we only try to use that if libvirt is knew enouch
14:47:11 sean-k-mooney its used here https://github.com/openstack/nova/blob/50fdbc752a9ca9c31488140ef2997ed59d861a41/nova/virt/libvirt/host.py#L1338
14:48:36 sean-k-mooney which is called form here https://github.com/openstack/nova/blob/50fdbc752a9ca9c31488140ef2997ed59d861a41/nova/virt/libvirt/vif.py#L537-L540
14:49:03 sean-k-mooney i guess we dont have a libvirt verion check here

Earlier   Later