Earlier  
Posted Nick Remark
#openstack-nova - 2025-04-01
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 dansmith if glance is not using ceph as well, then asking them to upload a qcow2 image is totally legit
17:27:56 sean-k-mooney but i agree the first step is get cinder involved
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 . Use an ID to be more specific"
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
18:07:46 dansmith yeah
18:08:08 sean-k-mooney dont we use context lib.exit_stack or something like that to handel it in nova/oslo
18:08:34 dansmith in some places yeah but I don't think it's really necessary here
18:08:35 sean-k-mooney we might not even need that
18:08:54 sean-k-mooney right that really only needed if we have multipel resocue we need to clean up
18:10:27 sean-k-mooney gmann: are we sure this is not just hard failing but it looks like its only failign sometimes due to the backport fo that patch

Earlier   Later