| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-07-03 | |||
| 10:15:31 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/exception.py#L67 | |
| 10:16:14 | dvo-plv | So, all this changes is not required to raise http conflict ? https://paste.opendev.org/show/bzaJaNk53E0OX9otfzzI/ | |
| 10:16:27 | dvo-plv | It should handle is some other way ? | |
| 10:16:28 | sean-k-mooney | it is part of INVALID_FLAVOR_IMAGE_EXCEPTIONS https://github.com/openstack/nova/blob/4b454febf73cdd7b5be0a2dad272c1d7685fac9e/nova/api/openstack/compute/servers.py#L58 | |
| 10:17:06 | sean-k-mooney | and that gets converted to a 400 bad reuest for resize here https://github.com/openstack/nova/blob/4b454febf73cdd7b5be0a2dad272c1d7685fac9e/nova/api/openstack/compute/servers.py#L1086-L1087 | |
| 10:18:01 | sean-k-mooney | so if you copy test_resize_server_negative_invalid_state and update it with the logic form your test and assert a 400 is retured then that should work | |
| 10:19:02 | sean-k-mooney | so duplicate https://github.com/openstack/nova/blob/4b454febf73cdd7b5be0a2dad272c1d7685fac9e/nova/tests/functional/test_servers.py#L792C9-L821 and update it image/flavor infor required for your test case | |
| 10:19:20 | dvo-plv | Okay, I will try to use it as template now | |
| 10:19:22 | sean-k-mooney | you shoudl get back a client.OpenStackApiException with reponce code 400 | |
| 10:19:24 | dvo-plv | thank you | |
| 10:20:08 | sean-k-mooney | ok i need to step away for a bit and pick up a collegue form there house | |
| 10:20:12 | sean-k-mooney | ill be back later | |
| 11:16:45 | dvo-plv | sean-k-mooney, I have rewrote test in the next way and I still get the same empty response | |
| 11:16:46 | dvo-plv | https://paste.opendev.org/show/bSbVh7CP0oKa1eQdVNUj/ | |
| 13:23:19 | opendevreview | Amit Uniyal proposed openstack/nova master: Refactor CinderFixture https://review.opendev.org/c/openstack/nova/+/885756 | |
| 13:23:20 | opendevreview | Amit Uniyal proposed openstack/nova master: Reproducer for dangling bdms https://review.opendev.org/c/openstack/nova/+/881457 | |
| 13:23:20 | opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | |
| 13:23:21 | opendevreview | Amit Uniyal proposed openstack/nova master: Allow to set attachment-id for bdm tests https://review.opendev.org/c/openstack/nova/+/887515 | |
| 13:42:02 | dvo-plv | I never catch this exception | |
| 13:42:03 | dvo-plv | https://paste.opendev.org/show/bBkeTtneX6hnX4kUj79F/ | |
| 13:43:16 | dvo-plv | Maybe I need to wrap my method with some decorator ? | |
| 13:59:55 | sean-k-mooney | it would be similer to look at this if you pushed your change as a seperate commit on top of the curren tone | |
| 14:00:12 | sean-k-mooney | but you should not really be modifying _resize | |
| 14:01:38 | sean-k-mooney | if your not getting to INVALID_FLAVOR_IMAGE_EXCEPTIONS on line 42 | |
| 14:02:00 | sean-k-mooney | that implies you likely are not blocking the resize proeprly | |
| 14:03:21 | sean-k-mooney | i.e. the code path might not be calling the fungiton that does the validation | |
| 14:14:19 | sean-k-mooney | dvo-plv: i belive the validation code is defeintly run for server create yes. | |
| 14:14:33 | sean-k-mooney | i.e. if you create a server with a conflicting image/flavor | |
| 14:15:01 | sean-k-mooney | so perhaps you should start with a funcitonal test to confirm that | |
| 14:15:15 | sean-k-mooney | and then if the that works and the resize one does not we then definetly have a bug | |
| 14:29:52 | opendevreview | Amit Uniyal proposed openstack/nova master: Refactor CinderFixture https://review.opendev.org/c/openstack/nova/+/885756 | |
| 14:29:53 | opendevreview | Amit Uniyal proposed openstack/nova master: Reproducer for dangling bdms https://review.opendev.org/c/openstack/nova/+/881457 | |
| 14:29:53 | opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | |
| 14:33:47 | opendevreview | Danylo Vodopianov proposed openstack/nova master: Packed virtqueue support was added. https://review.opendev.org/c/openstack/nova/+/876075 | |
| 14:34:19 | dvo-plv | sean-k-mooney, I just uploaded test | |
| 14:36:14 | dvo-plv | the problem which I get see in the debugger, it does not achive this execption | |
| 14:36:15 | dvo-plv | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L1086 | |
| 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 | I can see that _resize method call > /opt/stack/nova/nova/conductor/tasks/migrate.py(345)_schedule() | |
| 14:43:26 | dvo-plv | -> selection_lists = self.query_client.select_destinations( | |
| 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 | it calls rpc server error instead of httconflict | |
| 15:07:52 | dvo-plv | https://paste.opendev.org/show/byeH5cOKuKPovlDt9rGh/ | |
| 15:11:59 | dvo-plv | it sets code 500 by default | |
| 15:11:59 | dvo-plv | https://paste.opendev.org/show/byB3mGcovbJN70D7KL6B/ | |
| 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 | 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:04 | dvo-plv | https://github.com/openstack/nova/blob/4b454febf73cdd7b5be0a2dad272c1d7685fac9e/nova/compute/api.py#L4266 | |
| 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 | (Pdb) image | |