| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-04-01 | |||
| 17:07:43 | gmann | sean-k-mooney: I am searhing via image id from the tempest test failing traceback: | |
| 17:07:52 | sean-k-mooney | https://zuul.opendev.org/t/openstack/build/54c93553fc6c4797b25077fb29a89338/log/controller/logs/screen-c-vol.txt#4972-5037 | |
| 17:08:02 | dansmith | gibi: that ^ tests the full workflow in a single test.. there's a lot of setup so I'm not sure if it's really useful to split that into multiple cases | |
| 17:08:40 | dansmith | gibi: also not sure its feasible to test anything other than the happy path there (the cache issue is covered in unit tests) | |
| 17:09:09 | sean-k-mooney | gmann: do you know where that 3eb80d77-c55b-40d1-bf5d-1e22607350e8 is coming form | |
| 17:09:37 | sean-k-mooney | we ca see in the start fo the test logs that the id of the create image is 439cbb04-d5ef-480f-89c9-84bf2d4c749b | |
| 17:10:02 | dansmith | I assume testing things like startup failures due to improper config of a VF or something are not really needed to be covered in functional since they're covered in unit | |
| 17:10:17 | sean-k-mooney | gmann: oh this is doing multiple operations in on etest | |
| 17:10:58 | gmann | sean-k-mooney: its is from cinder upload_volume https://github.com/openstack/tempest/blob/80c0477f78c71a2bd2e1a324c41cd2f50329b200/tempest/api/volume/test_volumes_actions.py#L123 | |
| 17:12:08 | sean-k-mooney | so what happing is the raw iamge is uploading fine | |
| 17:12:13 | sean-k-mooney | and the qcow is failing | |
| 17:13:00 | sean-k-mooney | gmann: the qcow fails because of https://zuul.opendev.org/t/openstack/build/54c93553fc6c4797b25077fb29a89338/log/controller/logs/screen-c-vol.txt#4972-5037 | |
| 17:13:41 | sean-k-mooney | its failing in the format inspector code | |
| 17:14:08 | dansmith | on the cinder side | |
| 17:14:12 | sean-k-mooney | yep | |
| 17:14:27 | dansmith | and that's their oooold integrated format inspector code | |
| 17:14:32 | gmann | but in that case, I expect upload_volume to fail instead of giving image_id | |
| 17:14:47 | sean-k-mooney | so its a ceph backed cinder volume. and we are asking cidner ot upload the volume as an image in qcow2 format | |
| 17:14:58 | dansmith | and apparently because filename is none | |
| 17:15:23 | sean-k-mooney | gmann: i have not looked at the cidner code but i belive the worklaod is create teh glance image then upload the data | |
| 17:15:37 | sean-k-mooney | so they are probaly failing in between those two steps | |
| 17:17:07 | sean-k-mooney | gmann: form the looks of that trace the ceph cinder volume driver does not support converting the format when uploading a volume to an image | |
| 17:17:27 | sean-k-mooney | gmann: so in the near term we proably should tweak that job to only use raw | |
| 17:17:47 | sean-k-mooney | and if cidner ever support that with ceph backed volumes we can turn it back on | |
| 17:19:50 | sean-k-mooney | looking at the trace they are either expectign a host monted volume or a qemu rbd path to be passed as the path to qemu image. but its obviously not passing either given its None | |
| 17:23:08 | sean-k-mooney | dansmith: gmann i think the short term fix would be to add disk_format=raw here https://github.com/openstack/nova/blob/master/.zuul.yaml#L712 | |
| 17:23:22 | sean-k-mooney | based on the default change in https://review.opendev.org/c/openstack/tempest/+/938592/4/tempest/config.py | |
| 17:25:05 | sean-k-mooney | qcow makes sensce fo lvm josb perhaps since the volujme will be a local block device and they can call qemu image on it | |
| 17:25:17 | dansmith | why not have the cinder team confirm and do that if so? | |
| 17:25:22 | dansmith | isn't this on a stable job? | |
| 17:25:37 | sean-k-mooney | its likely on master too | |
| 17:25:44 | sean-k-mooney | but yes | |
| 17:26:02 | dansmith | how would that not be 100% fail? | |
| 17:26:26 | gmann | but this is not 100% failure in stable and master pass correctly ? | |
| 17:26:27 | dansmith | my point is, let's not just go papering over some failure with a devstack change as this seems like it should be a cinder bug no? | |
| 17:26:47 | sean-k-mooney | right i think it is a ciner bug | |
| 17:26:48 | gmann | dansmith: yeah, that what I was wondering, its not 100% | |
| 17:27:06 | dansmith | right, so I say kick it over to them | |
| 17:27:08 | sean-k-mooney | so i dont see it as papering over | |
| 17:27:16 | sean-k-mooney | we need to report it to them as a gate blocker | |
| 17:27:40 | sean-k-mooney | and then eitehr revert the tempst defualt change until its fixed or just skip it in our job until fixed | |
| 17:27:56 | sean-k-mooney | but i agree the first step is get cinder involved | |
| 17:27:56 | dansmith | if glance is not using ceph as well, then asking them to upload a qcow2 image is totally legit | |
| 17:28:09 | sean-k-mooney | glance is usign file and ceph | |
| 17:28:25 | sean-k-mooney | so yes uploading qcow is ligit in the job | |
| 17:28:32 | sean-k-mooney | given file is the default | |
| 17:28:49 | sean-k-mooney | even if it was not the default it would be ligit but slow | |
| 17:28:56 | dansmith | yeah | |
| 17:29:11 | sean-k-mooney | we also have force image convertion to raw in this job | |
| 17:29:30 | sean-k-mooney | which could be a diffent issue | |
| 17:29:50 | sean-k-mooney | its not failing in galnce however so its not the current issue | |
| 17:32:10 | supamatt | :wave: hi folks this change broke security groups https://review.opendev.org/c/openstack/nova/+/811521 , if any security groups with the same name exist in the project (whether the VM is using that sec group or not). Results in VMs not being able to be built with an error saying "Multiple security groups found matching |
|
| 17:33:08 | sean-k-mooney | if you relying on passign the name instead of the uuid | |
| 17:33:54 | sean-k-mooney | it woudl also only fail if that conflicting named security group was shared with you | |
| 17:34:37 | sean-k-mooney | supamatt: im not really sure how to adress the conflicting requirements | |
| 17:34:53 | sean-k-mooney | prefering the local one seams incorrect | |
| 17:35:11 | sean-k-mooney | if there is a name conflict it should be raised as an error by nova | |
| 17:36:05 | masahito | gmann: gibi: hi please review this api bug fix https://review.opendev.org/c/openstack/nova/+/939658 The v2.100 follows the same fix, so it might be ok for W+1. | |
| 17:36:48 | supamatt | sean-k-mooney: that's the thing, I am passing the uuid to nova. But nova is errors if any security has the same name in a project, regardless if I use that sec group to provision a vm. | |
| 17:37:30 | sean-k-mooney | supamatt: on ok | |
| 17:37:34 | sean-k-mooney | *oh | |
| 17:37:41 | sean-k-mooney | that simpler to fix | |
| 17:37:58 | sean-k-mooney | supamatt: have you filed a bug for this yet | |
| 17:38:17 | supamatt | I was checking lp now to see if there was an existing one | |
| 17:38:32 | supamatt | but it seems I am the first to see this, so I will file one | |
| 17:39:08 | sean-k-mooney | if you pass a name then the current behavior is arguabel correct. we could prefer the one from the current project but that might eb surprisign, if you pass a uuid we shoudl use the one you said even if there are duplicate names so that a ligit bug | |
| 17:40:38 | sean-k-mooney | supamatt: so taht a new feature in epoxy which release tomorrow | |
| 17:41:10 | sean-k-mooney | we wont have a fix for it in the release but we likely will do another release after the ptg | |
| 17:51:11 | sean-k-mooney | supamatt: https://review.opendev.org/c/openstack/nova/+/811521/13/nova/network/neutron.py#851 that where the but is | |
| 17:51:34 | sean-k-mooney | supamatt: the current algorthim is raising based on the aviabel secuirty groups to the user not the requsted one | |
| 17:52:26 | sean-k-mooney | we need to change the data stuctures and the algorthim slightly | |
| 17:53:41 | gmann | masahito: ack, will check soon | |
| 17:57:51 | gmann | dansmith: sean-k-mooney: elodilles: on test_volume_upload ceph failure, I found the cinder fix which is being backported to stable branches one by one. https://review.opendev.org/q/I32b505aa69c71b62e7e3a52d65d38165d34e97d8 | |
| 17:58:31 | sean-k-mooney | gmann ack | |
| 17:58:34 | gmann | we just need to wait for that to be merged on all stable | |
| 17:58:38 | dansmith | gotcha | |
| 17:58:59 | dansmith | I'm also a bit surprised (not really) they have not converted to oslo format inspector | |
| 17:59:08 | dansmith | I even prototyped the patch for them | |
| 17:59:49 | sean-k-mooney | hum ya the failure was on stable/2025.1 | |
| 18:00:05 | sean-k-mooney | that means they are also missing some of the mutliiple format things | |
| 18:00:18 | sean-k-mooney | for iso supprot and vfat supprot | |
| 18:00:26 | dansmith | yeah | |
| 18:00:33 | dansmith | not sure if that's an issue for them or not | |
| 18:00:51 | sean-k-mooney | well it mean you wont be able to create volumes form some isos presumable | |
| 18:01:09 | sean-k-mooney | that a pretty neich usecase given you can only use them properly with hacks | |
| 18:01:10 | dansmith | I'm still a bit surprised that fix that gmann found is not deterministic in its fail pattern | |
| 18:01:18 | elodilles | gmann: ah, cool, sounds great! thanks! | |
| 18:01:43 | sean-k-mooney | dansmith: ya that odd to me too | |
| 18:04:51 | sean-k-mooney | dansmith: that unlink https://review.opendev.org/c/openstack/cinder/+/945616/2/cinder/volume/drivers/rbd.py#2126 bug | |
| 18:05:22 | sean-k-mooney | upload can raise so there should at least be a try finally block here | |
| 18:06:05 | dansmith | unlink you mean? | |
| 18:06:20 | dansmith | it also looks like they're using two strategies to unlink the temp file no? | |
| 18:06:39 | sean-k-mooney | ya the os.unlink of the temp export of the volume | |
| 18:06:41 | dansmith | oh, I guess the context only unlinks on error | |
| 18:06:54 | dansmith | but yeah, that could definitely fail | |
| 18:07:09 | sean-k-mooney | oh they are using fileutils.remove_path_on_error(tmp_file): | |
| 18:07:16 | sean-k-mooney | to handel the excption case | |
| 18:07:26 | sean-k-mooney | and then the explcit unlink for happy path | |
| 18:07:34 | dansmith | right, seems like they should use try..finally: unlink() | |
| 18:07:41 | sean-k-mooney | that works... but not how i would have done that | |