Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-07-03
14:37:21 dvo-plv but it achive this exception https://review.opendev.org/c/openstack/nova/+/876075/15/nova/virt/hardware.py#1971
14:43:10 dvo-plv I did not get the vector which I should to move
14:43:26 dvo-plv -> selection_lists = self.query_client.select_destinations(
14:43:26 dvo-plv I can see that _resize method call > /opt/stack/nova/nova/conductor/tasks/migrate.py(345)_schedule()
14:44:44 dvo-plv and this rescheduler calls self.query_client.select_destinations method which call my filter, which call excepetion conflict
14:45:11 dansmith auniyal: do you want to get this fixed up so we can just merge it? https://review.opendev.org/c/openstack/nova/+/885756
14:45:16 dvo-plv but this exception does not affect on this https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L1086
14:45:28 dansmith we could do that before you fully fix the dangling bdms thing and then it will be off your plate and out of that stack
15:07:52 dvo-plv https://paste.opendev.org/show/byeH5cOKuKPovlDt9rGh/
15:07:52 dvo-plv it calls rpc server error instead of httconflict
15:11:59 dvo-plv https://paste.opendev.org/show/byB3mGcovbJN70D7KL6B/
15:11:59 dvo-plv it sets code 500 by default
15:28:24 dvo-plv sean-k-mooney, conflict exception absent here: https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L294
15:35:46 dvo-plv and here: https://github.com/openstack/nova/blob/master/nova/scheduler/manager.py#L147
15:35:51 dvo-plv but it does not help
15:39:48 auniyal dansmith, yes, MockPatch didn't work for me right now, so I removed WIP. later I might create new patch for same. if its fine we can merge this.
15:40:16 dansmith auniyal: I commented about this, but you were using it as a context manager right? You need to use it as a fixture
15:42:14 auniyal yes, context manager I tried earlier and also mock.patch('nova.volume.cinder.API.attachment_create', side_effect=self.fake_attachment_create).start()
15:42:47 auniyal but after your suggestion on MockPatch I started looking into it, but couldn't figure out it
15:43:35 auniyal this: $ grep 'Fixture.*MockPatch' -r nova/tests
15:45:31 dansmith $ grep 'fixtures.*Mock' -r nova/tests | wc -l ✭reimage-timeout
15:45:31 dansmith 181
15:45:31 dansmith $ grep 'fixtures.*Mock' -r nova/tests | wc -l ✭reimage-timeout
15:49:34 auniyal yes, it was your comment actually,
15:49:52 auniyal the files it shows has example of MockPatch
15:50:27 auniyal so I went through them, and tried to apply on cinder fixture, but it did not work,
15:50:40 auniyal I got the control-flow though
15:51:47 dansmith okay I'm confused
15:53:28 sean-k-mooney dvo-plv: so we seam to be getting to select destination before that runs which is far too late
15:55:28 sean-k-mooney dvo-plv: you should be checkign the image compatibality in _validate_flavor_image_nostatus https://github.com/openstack/nova/blob/4b454febf73cdd7b5be0a2dad272c1d7685fac9e/nova/compute/api.py#L718-L833
15:56:28 sean-k-mooney for resize its invoked here https://github.com/openstack/nova/blob/4b454febf73cdd7b5be0a2dad272c1d7685fac9e/nova/compute/api.py#L4253-L4272 but its also used in create and rebuild
15:57:17 sean-k-mooney dvo-plv: _validate_flavor_image_nostatus is the common place in the api that is ment to check compaitble before we ever do a api request to placemnt or an rpc to the conductor
16:00:13 sean-k-mooney you will also need to put a call to the check here too https://github.com/openstack/nova/blob/4b454febf73cdd7b5be0a2dad272c1d7685fac9e/nova/compute/api.py#L4268
16:00:22 sean-k-mooney to validate the bfv path
16:02:04 dvo-plv https://github.com/openstack/nova/blob/4b454febf73cdd7b5be0a2dad272c1d7685fac9e/nova/compute/api.py#L4266
16:02:04 dvo-plv Maybe it will be better to call this https://review.opendev.org/c/openstack/nova/+/876075/15/nova/virt/hardware.py#1971 before this statement
16:02:09 dvo-plv to call both if and else ?
16:02:18 dvo-plv to catch both if and else
16:02:31 dvo-plv Also I need to handle rebuild somewhere
#openstack-nova - 2023-07-04
06:27:19 opendevreview Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284
10:57:30 opendevreview Merged openstack/nova stable/yoga: Fix get_segments_id with subnets without segment_id https://review.opendev.org/c/openstack/nova/+/883724
11:41:27 sean-k-mooney dvo-plv: you could just add your check to _validate_flavor_image_numa_pci although that is strictly not what that is for but its called form _validate_flavor_image_nostatus
11:41:41 sean-k-mooney so if you add it to _validate_flavor_image_numa_pci it will be called everywhere it shoudl be
11:41:58 sean-k-mooney i actullly think that current if/else for BFV instance might be a bug
11:42:40 sean-k-mooney but that is unrealted to yoru change
11:43:20 dvo-plv I thought do it in the next way for resize https://paste.opendev.org/show/bUEZJuI6huYrnmAVSGsZ/
11:43:44 dvo-plv and this for rebuild https://paste.opendev.org/show/b5zFmXnXpVE4iNezPiSi/
11:44:14 sean-k-mooney i would kind of like to avoid adding the call in multiple places to make sure we dont miss a palce
11:45:03 sean-k-mooney what you suggest will work
11:45:14 sean-k-mooney we just need to allso ensure its called on server create
11:45:15 dvo-plv sure
11:45:42 sean-k-mooney so there are tree cases we need to check (create, resize and rebuild)
11:46:15 sean-k-mooney maybe rebuild
11:46:24 sean-k-mooney sorry maybe rescue
11:46:30 sean-k-mooney for rescue im conflcited
11:46:54 sean-k-mooney the rescue image is tempory so normally i would say just never set this for the rescue image
11:47:03 sean-k-mooney so we should not need to check it
11:47:43 sean-k-mooney ill leave it up to you if you want to add the 3 checks then that is ok
11:48:00 sean-k-mooney just add them in similar location to the existing _validate_flavor_image_nostatus checks
11:52:03 dvo-plv looks like we will not have ability to do this without additional changes
11:53:03 dvo-plv self._validate_flavor_image_nostatus(context, image, new_flavor, root_bdm=None,validate_pci=True) this method get not full image parameter
11:54:42 dvo-plv it pass only metadata https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4254
11:54:53 dvo-plv {'min_ram': '0', 'min_disk': '1', 'disk_format': 'raw', 'container_format': 'raw', 'properties': {'kernel_id': 'nokernel', 'ramdisk_id': 'nokernel', 'hw_virtio_packed_ring': 'true', 'base_image_ref': 'c456eb30-91d7-4f43-8f46-2efd9eccd744', 'boot_roles': 'reader,member,admin'}}
11:54:53 dvo-plv (Pdb) image
11:55:09 sean-k-mooney ack you actully only need the metadata not the full image
11:55:18 sean-k-mooney for yoru hardware.py funciton
11:55:28 dvo-plv but we need full object https://review.opendev.org/c/openstack/nova/+/876075/14/nova/virt/hardware.py#1941
11:56:11 sean-k-mooney oh you mean it gets a dict not the object?
11:56:13 dvo-plv https://review.opendev.org/c/openstack/nova/+/876075/14/nova/virt/hardware.py#1977
11:56:13 dvo-plv because exception needs next field
11:56:37 sean-k-mooney we dont neeed the iamge name error masge
11:56:46 dvo-plv yes, I need object + this object have to have name field
11:56:56 sean-k-mooney ya but we dont need the name filed
11:57:38 sean-k-mooney use the uuid c456eb30-91d7-4f43-8f46-2efd9eccd744 from base_image_ref if you want
11:58:01 dvo-plv okay I can use uuid
11:58:02 sean-k-mooney but you do not need the flavor or iamge name for the message
11:58:54 sean-k-mooney really you just need to say "Flavor has ... image has ...."
11:59:01 sean-k-mooney the user knows what flavor and image they use
12:02:04 dvo-plv ImageMeta(checksum=<?>,container_format='raw',created_at=<?>,direct_url=<?>,disk_format='raw',id=<?>,min_disk=1,min_ram=0,name=<?>,owner=<?>,properties=ImageMetaProps,protected=<?>,size=<?>,status=<?>,tags=<?>,updated_at=<?>,virtual_size=<?>,visibility=<?>)
12:02:04 dvo-plv Yes, I will rewrite log, because id also does not exists in some cases (Pdb) image_meta
12:02:28 sean-k-mooney ack
12:28:04 dvo-plv sean-k-mooney, I would like to clarify how properly I need to fix next unit test, which start to fall after adding those functional
12:28:05 dvo-plv https://github.com/openstack/nova/blob/master/nova/tests/unit/compute/test_compute.py#L14213
12:28:21 dvo-plv (Pdb) cont
12:28:21 dvo-plv {'id': 'fbb69a20-3717-439e-88ec-7954e6f801af', 'status': 'active', 'properties': {'hw_cpu_policy': 'bar'}}
12:28:21 dvo-plv (Pdb) image
12:28:21 dvo-plv -> image_meta = objects.ImageMeta.from_dict(image)
12:28:21 dvo-plv /opt/stack/nova/nova/compute/api.py(758)_validate_flavor_image_nostatus()
12:28:37 dvo-plv it fails which is expected, according to this verification
12:29:22 dvo-plv https://github.com/openstack/nova/blob/master/nova/objects/image_meta.py#L234
12:30:24 sean-k-mooney it shoudl not fail by adding your fucntion
12:30:35 sean-k-mooney did you change the object passed to _validate_flavor_image_nostatus
12:31:07 sean-k-mooney i was tryign to steer you away form that since i expecte that to cause a lot of unit/funcitonal test filaures
12:31:37 dvo-plv https://paste.opendev.org/show/b84ihg1M7WPIpDTSVyXc/
12:31:50 dvo-plv no I just make object from dict for verification
12:32:12 sean-k-mooney yes but that will have the same effect
12:32:26 sean-k-mooney it will cause the image proeprty validation to be enfroced
12:33:15 sean-k-mooney i understand why you wourl prefer to use the object
12:33:38 sean-k-mooney but it might be simpler to make get_packed_virtqueue_constraint use the dict
12:35:21 dvo-plv Okay, I will try to use dict instead, I believe that vit will be better if verification method will be similar to the other, which check conflcits

Earlier   Later