Wednesday, 2024-08-21

mikalsean-k-mooney: how do I delineate what is the object change vs everything else? For example for https://review.opendev.org/c/openstack/nova/+/924844 are we literally saying just pull the nova/objects/console_auth_token.py bit and associated tests out into its own change?00:19
sean-k-mooney[m]yep more or less jut that and the test00:21
sean-k-mooney[m]normally also the notificaotn object change00:21
sean-k-mooney[m]but in this case you dont have a  notifiction object change00:21
sean-k-mooney[m]as i said this is really 3 patches in one. im not genrally against that but its an object change followed by the db change followed by the rest00:23
mikalsean-k-mooney: I can give that a go... I'll start with the nova console change instead of the extra spec one I think though. Give me a day or two.01:19
opendevreviewTakashi Kajinami proposed openstack/nova master: libvirt: Launch instances with stateless firmware  https://review.opendev.org/c/openstack/nova/+/90889001:52
*** __ministry is now known as Guest102102:02
opendevreviewMerged openstack/nova master: Fix deepcopy usage for BlockDeviceMapping in get_root_info  https://review.opendev.org/c/openstack/nova/+/92037402:33
opendevreviewZhang Hua proposed openstack/nova stable/2024.1: Fix deepcopy usage for BlockDeviceMapping in get_root_info  https://review.opendev.org/c/openstack/nova/+/92669603:41
opendevreviewZhang Hua proposed openstack/nova stable/2023.2: Fix deepcopy usage for BlockDeviceMapping in get_root_info  https://review.opendev.org/c/openstack/nova/+/92669703:44
opendevreviewZhang Hua proposed openstack/nova stable/2023.1: Fix deepcopy usage for BlockDeviceMapping in get_root_info  https://review.opendev.org/c/openstack/nova/+/92669803:46
*** bauzas_ is now known as bauzas03:51
opendevreviewMasahito Muroi proposed openstack/nova master: Use boot_roles_count and boot_role_<count> key for system_metadata  https://review.opendev.org/c/openstack/nova/+/92516306:16
*** bauzas_ is now known as bauzas06:34
*** elodilles_ooo is now known as elodilles06:44
opendevreviewAmit Uniyal proposed openstack/nova master: Reproducer test for image property hw_architecture  https://review.opendev.org/c/openstack/nova/+/92612806:46
opendevreviewAmit Uniyal proposed openstack/nova master: Libvirt: updates resource provider trait list  https://review.opendev.org/c/openstack/nova/+/92652106:46
opendevreviewAmit Uniyal proposed openstack/nova master: VMware: updates resource provider trait list  https://review.opendev.org/c/openstack/nova/+/92652206:46
opendevreviewAmit Uniyal proposed openstack/nova master: zvm: updates resource provider trait list  https://review.opendev.org/c/openstack/nova/+/92652306:46
opendevreviewAmit Uniyal proposed openstack/nova master: WIP Ironic: updates resource provider trait list  https://review.opendev.org/c/openstack/nova/+/92652406:46
*** bauzas_ is now known as bauzas07:34
IPOHello all !08:06
IPOsean-k-mooney: Hello, Sean! What do you think: is there a chance to review and comment the fix: https://review.opendev.org/c/openstack/nova/+/92339508:11
sean-k-mooney[m]IPO:  im kind of conflicted, i personally dont really think that feature should exist in nova10:00
sean-k-mooney[m]the global version specifically10:00
sean-k-mooney[m]because it allow you to probe for information you otherwise should not have. i.e. the hostname of other tenants vms10:01
sean-k-mooney[m]the way the fix is proposed its also pretty expensive10:03
sean-k-mooney[m]every instqance create would result in a db query to alll cells.10:04
IPOsean-k-mooney: Thank you for comment. Interesting point. But in this case maybe the some way of variant, which was proposed earlier, should been implemented. But this will allow to check VMs name in one cell, but for all projects, which is not allowed either10:07
IPOsean-k-mooney: And finally it looks like this feature maybe be cancelled at all as a potential security risk ?10:09
sean-k-mooney[m]what use case made you interested in supporting this across cells?10:11
IPOsean-k-mooney: Good question ! Basically we don't use cells in prods and some case of fix, implemented before (https://review.opendev.org/c/openstack/nova/+/860938) suits our needs. But there was the comment that we need fix to check it for all cells. Thant is why we start to impliment new fix10:17
sean-k-mooney[m]thats not quite what i was asking. why to you enable project or global uniquness?10:19
sean-k-mooney[m]the security issue is minor and its something that deployers opt into for there cloud10:19
sean-k-mooney[m]thats why im not -1, the operator made a delibverate choice to opt in10:20
sean-k-mooney[m]i could understand why this was useful for nova networks10:20
sean-k-mooney[m]so that all vms had a unique fqdn10:21
sean-k-mooney[m]but with neutron and designate im not sure what the usecase for global or project uniqness is10:21
sean-k-mooney[m]IPO:  if other cores think this should be supported and fixed then thats fine we can proceed with the patch its just not something i personally think we should spend time on but if you have a usecase that needs it i dont want to block that10:25
sean-k-mooney[m]i just dont really want to invest much of my time in advocating for it when i dont really belive in it10:26
IPOsean-k-mooney: thank you for extra comments. I have clear view of your point of view about this issue. One of reasons we faced - we have naming convention for VMs. It is maintained by our self-made portal. But there is extra ability to work with cloud - via CLI or API and we can't control names users can choose for VMs10:34
IPOsean-k-mooney: I will try to collect additional reasons with our team, to have more items in list.10:42
IPOsean-k-mooney: Thank you, Sean! I will return to this topic as soon as will have additional info. Have a good day !11:05
sean-k-mooneyok11:05
sean-k-mooneyas i said its just not a priority for me but if others are happy to proceed then im ok with that11:05
IPOsean-k-mooney: ok, I see, thank you for your time.11:12
opendevreviewJan Gutter proposed openstack/placement master: DNM -- testing fix in external PR  https://review.opendev.org/c/openstack/placement/+/92674311:55
sean-k-mooneygmann: melwitt: https://review.opendev.org/c/openstack/nova/+/925163 is a fairly simple review if ye have time later12:02
opendevreviewJan Gutter proposed openstack/placement master: DNM -- testing fix in external PR  https://review.opendev.org/c/openstack/placement/+/92674312:18
opendevreviewJan Gutter proposed openstack/placement master: DNM -- testing fix in external PR  https://review.opendev.org/c/openstack/placement/+/92674313:52
*** bauzas_ is now known as bauzas14:23
stephenfinJust for lolz, I attempted to create a BDM with boot_index=0 and destination_type=local (i.e. the root disk). Nova ignores/overwrites it. Is the fact that it accepts it in the first place a bug in our validation code?15:30
stephenfine.g. https://paste.openstack.org/show/bFOXilnynfyoGM2CCp7v/15:30
stephenfin(that's the output from my hacked OSC)15:30
opendevreviewMerged openstack/nova stable/2023.2: Disconnecting volume from the compute host  https://review.opendev.org/c/openstack/nova/+/91139615:41
melwittsean-k-mooney: fyi in case you missed it, I updated https://review.opendev.org/q/topic:%22bp/unified-limits-nova-unset-limits%22+is:open+project:openstack/nova after we chatted about [workarounds] and a release note15:55
sean-k-mooneystephenfin: i dont think that is a bug 15:58
sean-k-mooneythe bdm are not just for cinder 15:58
sean-k-mooneyyou use them ot create multiple ephermaal disk, swap disk and for the root  disk15:59
stephenfinRight, I know all that (that's what 'destination_type' is there for, after all). My point is that the nova-api/nova-compute ignores what the user is giving rather than spitting it back at them15:59
sean-k-mooneyright so the api is corect to accept it16:00
sean-k-mooneythe comptue proably has a bug16:00
sean-k-mooneyor there is a bug in how we are merging the bdm and the image 16:00
stephenfinInteresting. Are you suggestion I should be able to override $flavor.disk with a BDM?16:00
melwittyeah, I checked and the api-ref example actually shows boot_index=0 (https://docs.openstack.org/api-ref/compute/#id11) however I think what stephenfin is saying is nova isn't taking the disk size from the BDM and is instead using what's in the flavor in the local case?16:00
stephenfins/suggestion/suggesting/16:00
stephenfinmelwitt: Yes, thanks Mel :)16:01
sean-k-mooneyim sayiung that passing image_ref and also providing bdms for the root disk is proably undfiend right now16:01
sean-k-mooneyim incined to say that that might be something to reject with a 40016:01
melwittseems like a nova bug although if flavor and bdm are in conflict then I guess maybe it should reject it saying conflict because we can't assume which one they really wanted16:02
stephenfinNot so. If I pass destination_type=volume instead of destination_type=local, things work just fine (that's a BFV instance)16:02
sean-k-mooneyright16:02
sean-k-mooneybut im not sure you should be passsing an image uuid16:02
sean-k-mooneyand bdms with srouce type image and a uuid16:02
stephenfinYeah, fair point. I'm thinking about OSC but from a pure API level, you'd set imageRef to nll and set uuid in the relevant BDM instead 16:04
sean-k-mooneyhttps://paste.openstack.org/show/bnHpudm9QI1H7gj7aVAm/16:04
sean-k-mooneythat a pretty print of the payload16:04
sean-k-mooneystephenfin: so what exactly happens in this case16:04
sean-k-mooneyi think the result should be the same since its the same uuid16:05
sean-k-mooneyare you expeciting the size to be 6 and your getting the flavor size16:05
sean-k-mooney"destination_type": "local", is really intended only for the addtional ephmeral disks and volume16:06
sean-k-mooney*volume_size16:06
sean-k-mooneyin that case is for when you want to have multiple16:06
stephenfinjust checking now16:06
sean-k-mooneydestination_type=volume volume_size is obvioulsy the cinder volume size16:06
melwittI guess if we consider how it works for volume (bdm disk size takes precedence) maybe we should do that with local too? rather than it being treated as invalid16:07
sean-k-mooneyyep i would expect the bdm to have precidence but it shoudl not be able to exceed flavor.disk_gb16:08
melwittthat would make it consistent. I assume that flavor disk size is ignored for BFV16:08
sean-k-mooneyfor the non bfv case16:08
sean-k-mooneymelwitt: yes and no16:08
sean-k-mooneyit mostly is16:08
stephenfinsean-k-mooney: so the answer is it depends :)16:09
sean-k-mooneyyou shoudl set it to 0 for bfv flavors16:09
stephenfinif I pass an imageRef, then the BFV is totally ignored16:09
sean-k-mooneyin older release we used to claim it if not but in newer release if its BFV even if the falvor is not 0 we 0 it in placment ectra16:09
stephenfinif I do not pass an imageRef, then the server goes to error state due an ImageNotFound error 16:09
sean-k-mooneyso it depend on how old your nova is 16:09
melwittI see16:09
sean-k-mooneybut like train did the right thing16:09
sean-k-mooneyso its been correctly ignoring it for bfv for a long time16:10
stephenfinthis is using a relatively recent deploy of DevStack (commit 8c4a2e1b4759b9932bcad10810143e61ec9b4c36 from Jul 20)16:10
stephenfinsean-k-mooney: I was thinking that I would get a disk size of 6, but idk if that's necessarily something we want to support. Flavors are there for a reason and allowing users to do this provides a way to side-step what the operators want them to use16:11
stephenfinthen again, they could just request N ephemeral disks, so it probably doesn't matter: the operator has to reply on quota to save them16:11
sean-k-mooneystephenfin: they kind of alreayd have that however16:12
stephenfinyou mean with Cinder? 16:12
sean-k-mooneyand we do allow you do do it for the addtional ephmeral disk and for bfv16:12
sean-k-mooneyno so you can have a flavor with ephmeral_disk=100GB16:12
stephenfinoh, yes16:12
sean-k-mooneyand hten you can subdevided that into multiple disks via the bdm16:12
stephenfinme trying to figure out whether I could do that is what led me down this rabbit hole :)16:13
sean-k-mooneyhttps://docs.openstack.org/nova/train/user/launch-instance-from-volume.html#create-volume-from-image-and-boot-instance16:13
stephenfinfwict, it's not really subdividing it so much as completely ignoring what's in the flavor16:13
sean-k-mooneywell if its using 6GB i think that is correct16:14
sean-k-mooneyprovided the flavor is > 6GB16:14
sean-k-mooneywell >=16:14
sean-k-mooneyhttps://docs.openstack.org/ocata/user-guide/cli-nova-launch-instance-from-volume.html16:15
sean-k-mooneythe note makes it pretty clear for swap and ephermeral 16:16
sean-k-mooney"The flavor defines the maximum swap and ephemeral disk size. You cannot exceed these maximum values."16:16
sean-k-mooneyso it looks like we wanted the bdms to have precednce over the flavor16:16
stephenfinYup. That's what I'm seeing. Good point. I'd missed that nuance16:16
sean-k-mooneyi assume you followed the BFV example but changed source ot image and dest to local16:18
sean-k-mooney$ openstack server create --flavor FLAVOR --block-device \16:18
sean-k-mooney  source=SOURCE,id=ID,dest=DEST,size=SIZE,shutdown=PRESERVE,bootindex=INDEX \16:18
sean-k-mooney  NAME16:18
stephenfinsort of - I hacked in a `--root` option to OSC16:19
stephenfinit was easier than figuring out a `--block-device` invocation 0:)16:20
sean-k-mooneyok is there a reason yoru doing this16:20
stephenfin<stephenfin> me trying to figure out whether I could do that is what led me down this rabbit hole :)16:20
sean-k-mooneyok there are some things you can only do via the bdms16:20
sean-k-mooneyim confilictedc if i should tell you what those are16:21
sean-k-mooneyon one hand you might make it easy to do 16:21
sean-k-mooneyon the other hand you might make it easy to do and ill have to support people doing it :)16:21
sean-k-mooneythe only way to specify a per disk device type is block_device_mapping_v2.device_type i.e. disk, cdrom.16:22
stephenfinthat ship has sailed. There are already a load of newish BDM-related options that I added over the past few years (like the `--block-device` option you refer to)16:22
sean-k-mooneyyou can also set block_device_mapping_v2.disk_bus  pre bdm16:23
sean-k-mooneyallowign diffent disks to have specific disk bus "fdc, ide, sata, scsi, usb, virtio, xen, lxc and uml."16:23
sean-k-mooneyso if you wanted your root disk to use stata and the rest to use virtio-blk or virtio-scsi16:24
sean-k-mooneyyou should be able to express that in the bdms16:24
auniyalhey elodilles, sean-k-mooney,  stephenfin, 16:25
auniyal(did not wanted to disturb on-going chat, but its late for me, so when you have some time) can you please look into these https://review.opendev.org/q/topic:%22lp/2012365%22 ;  backports for stable/2023.1 merged in stable/2023.116:25
elodillesauniyal: ACK, added to my TODO list, though probably will do only tomorrow16:27
auniyalelodilles, thanks, yeah sure16:28
opendevreviewPavlo Shchelokovskyy proposed openstack/nova master: Fix device type when booting from ISO image  https://review.opendev.org/c/openstack/nova/+/90961116:38
*** bauzas_ is now known as bauzas18:30

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!