mikal | sean-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 test | 00:21 |
sean-k-mooney[m] | normally also the notificaotn object change | 00:21 |
sean-k-mooney[m] | but in this case you dont have a notifiction object change | 00: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 rest | 00:23 |
mikal | sean-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 |
opendevreview | Takashi Kajinami proposed openstack/nova master: libvirt: Launch instances with stateless firmware https://review.opendev.org/c/openstack/nova/+/908890 | 01:52 |
*** __ministry is now known as Guest1021 | 02:02 | |
opendevreview | Merged openstack/nova master: Fix deepcopy usage for BlockDeviceMapping in get_root_info https://review.opendev.org/c/openstack/nova/+/920374 | 02:33 |
opendevreview | Zhang Hua proposed openstack/nova stable/2024.1: Fix deepcopy usage for BlockDeviceMapping in get_root_info https://review.opendev.org/c/openstack/nova/+/926696 | 03:41 |
opendevreview | Zhang Hua proposed openstack/nova stable/2023.2: Fix deepcopy usage for BlockDeviceMapping in get_root_info https://review.opendev.org/c/openstack/nova/+/926697 | 03:44 |
opendevreview | Zhang Hua proposed openstack/nova stable/2023.1: Fix deepcopy usage for BlockDeviceMapping in get_root_info https://review.opendev.org/c/openstack/nova/+/926698 | 03:46 |
*** bauzas_ is now known as bauzas | 03:51 | |
opendevreview | Masahito Muroi proposed openstack/nova master: Use boot_roles_count and boot_role_<count> key for system_metadata https://review.opendev.org/c/openstack/nova/+/925163 | 06:16 |
*** bauzas_ is now known as bauzas | 06:34 | |
*** elodilles_ooo is now known as elodilles | 06:44 | |
opendevreview | Amit Uniyal proposed openstack/nova master: Reproducer test for image property hw_architecture https://review.opendev.org/c/openstack/nova/+/926128 | 06:46 |
opendevreview | Amit Uniyal proposed openstack/nova master: Libvirt: updates resource provider trait list https://review.opendev.org/c/openstack/nova/+/926521 | 06:46 |
opendevreview | Amit Uniyal proposed openstack/nova master: VMware: updates resource provider trait list https://review.opendev.org/c/openstack/nova/+/926522 | 06:46 |
opendevreview | Amit Uniyal proposed openstack/nova master: zvm: updates resource provider trait list https://review.opendev.org/c/openstack/nova/+/926523 | 06:46 |
opendevreview | Amit Uniyal proposed openstack/nova master: WIP Ironic: updates resource provider trait list https://review.opendev.org/c/openstack/nova/+/926524 | 06:46 |
*** bauzas_ is now known as bauzas | 07:34 | |
IPO | Hello all ! | 08:06 |
IPO | sean-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/+/923395 | 08:11 |
sean-k-mooney[m] | IPO: im kind of conflicted, i personally dont really think that feature should exist in nova | 10:00 |
sean-k-mooney[m] | the global version specifically | 10: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 vms | 10:01 |
sean-k-mooney[m] | the way the fix is proposed its also pretty expensive | 10:03 |
sean-k-mooney[m] | every instqance create would result in a db query to alll cells. | 10:04 |
IPO | sean-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 either | 10:07 |
IPO | sean-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 |
IPO | sean-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 fix | 10: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 cloud | 10:19 |
sean-k-mooney[m] | thats why im not -1, the operator made a delibverate choice to opt in | 10:20 |
sean-k-mooney[m] | i could understand why this was useful for nova networks | 10:20 |
sean-k-mooney[m] | so that all vms had a unique fqdn | 10:21 |
sean-k-mooney[m] | but with neutron and designate im not sure what the usecase for global or project uniqness is | 10: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 that | 10: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 it | 10:26 |
IPO | sean-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 VMs | 10:34 |
IPO | sean-k-mooney: I will try to collect additional reasons with our team, to have more items in list. | 10:42 |
IPO | sean-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-mooney | ok | 11:05 |
sean-k-mooney | as i said its just not a priority for me but if others are happy to proceed then im ok with that | 11:05 |
IPO | sean-k-mooney: ok, I see, thank you for your time. | 11:12 |
opendevreview | Jan Gutter proposed openstack/placement master: DNM -- testing fix in external PR https://review.opendev.org/c/openstack/placement/+/926743 | 11:55 |
sean-k-mooney | gmann: melwitt: https://review.opendev.org/c/openstack/nova/+/925163 is a fairly simple review if ye have time later | 12:02 |
opendevreview | Jan Gutter proposed openstack/placement master: DNM -- testing fix in external PR https://review.opendev.org/c/openstack/placement/+/926743 | 12:18 |
opendevreview | Jan Gutter proposed openstack/placement master: DNM -- testing fix in external PR https://review.opendev.org/c/openstack/placement/+/926743 | 13:52 |
*** bauzas_ is now known as bauzas | 14:23 | |
stephenfin | Just 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 |
stephenfin | e.g. https://paste.openstack.org/show/bFOXilnynfyoGM2CCp7v/ | 15:30 |
stephenfin | (that's the output from my hacked OSC) | 15:30 |
opendevreview | Merged openstack/nova stable/2023.2: Disconnecting volume from the compute host https://review.opendev.org/c/openstack/nova/+/911396 | 15:41 |
melwitt | sean-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 note | 15:55 |
sean-k-mooney | stephenfin: i dont think that is a bug | 15:58 |
sean-k-mooney | the bdm are not just for cinder | 15:58 |
sean-k-mooney | you use them ot create multiple ephermaal disk, swap disk and for the root disk | 15:59 |
stephenfin | Right, 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 them | 15:59 |
sean-k-mooney | right so the api is corect to accept it | 16:00 |
sean-k-mooney | the comptue proably has a bug | 16:00 |
sean-k-mooney | or there is a bug in how we are merging the bdm and the image | 16:00 |
stephenfin | Interesting. Are you suggestion I should be able to override $flavor.disk with a BDM? | 16:00 |
melwitt | yeah, 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 |
stephenfin | s/suggestion/suggesting/ | 16:00 |
stephenfin | melwitt: Yes, thanks Mel :) | 16:01 |
sean-k-mooney | im sayiung that passing image_ref and also providing bdms for the root disk is proably undfiend right now | 16:01 |
sean-k-mooney | im incined to say that that might be something to reject with a 400 | 16:01 |
melwitt | seems 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 wanted | 16:02 |
stephenfin | Not 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-mooney | right | 16:02 |
sean-k-mooney | but im not sure you should be passsing an image uuid | 16:02 |
sean-k-mooney | and bdms with srouce type image and a uuid | 16:02 |
stephenfin | Yeah, 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-mooney | https://paste.openstack.org/show/bnHpudm9QI1H7gj7aVAm/ | 16:04 |
sean-k-mooney | that a pretty print of the payload | 16:04 |
sean-k-mooney | stephenfin: so what exactly happens in this case | 16:04 |
sean-k-mooney | i think the result should be the same since its the same uuid | 16:05 |
sean-k-mooney | are you expeciting the size to be 6 and your getting the flavor size | 16:05 |
sean-k-mooney | "destination_type": "local", is really intended only for the addtional ephmeral disks and volume | 16:06 |
sean-k-mooney | *volume_size | 16:06 |
sean-k-mooney | in that case is for when you want to have multiple | 16:06 |
stephenfin | just checking now | 16:06 |
sean-k-mooney | destination_type=volume volume_size is obvioulsy the cinder volume size | 16:06 |
melwitt | I 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 invalid | 16:07 |
sean-k-mooney | yep i would expect the bdm to have precidence but it shoudl not be able to exceed flavor.disk_gb | 16:08 |
melwitt | that would make it consistent. I assume that flavor disk size is ignored for BFV | 16:08 |
sean-k-mooney | for the non bfv case | 16:08 |
sean-k-mooney | melwitt: yes and no | 16:08 |
sean-k-mooney | it mostly is | 16:08 |
stephenfin | sean-k-mooney: so the answer is it depends :) | 16:09 |
sean-k-mooney | you shoudl set it to 0 for bfv flavors | 16:09 |
stephenfin | if I pass an imageRef, then the BFV is totally ignored | 16:09 |
sean-k-mooney | in 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 ectra | 16:09 |
stephenfin | if I do not pass an imageRef, then the server goes to error state due an ImageNotFound error | 16:09 |
sean-k-mooney | so it depend on how old your nova is | 16:09 |
melwitt | I see | 16:09 |
sean-k-mooney | but like train did the right thing | 16:09 |
sean-k-mooney | so its been correctly ignoring it for bfv for a long time | 16:10 |
stephenfin | this is using a relatively recent deploy of DevStack (commit 8c4a2e1b4759b9932bcad10810143e61ec9b4c36 from Jul 20) | 16:10 |
stephenfin | sean-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 use | 16:11 |
stephenfin | then again, they could just request N ephemeral disks, so it probably doesn't matter: the operator has to reply on quota to save them | 16:11 |
sean-k-mooney | stephenfin: they kind of alreayd have that however | 16:12 |
stephenfin | you mean with Cinder? | 16:12 |
sean-k-mooney | and we do allow you do do it for the addtional ephmeral disk and for bfv | 16:12 |
sean-k-mooney | no so you can have a flavor with ephmeral_disk=100GB | 16:12 |
stephenfin | oh, yes | 16:12 |
sean-k-mooney | and hten you can subdevided that into multiple disks via the bdm | 16:12 |
stephenfin | me trying to figure out whether I could do that is what led me down this rabbit hole :) | 16:13 |
sean-k-mooney | https://docs.openstack.org/nova/train/user/launch-instance-from-volume.html#create-volume-from-image-and-boot-instance | 16:13 |
stephenfin | fwict, it's not really subdividing it so much as completely ignoring what's in the flavor | 16:13 |
sean-k-mooney | well if its using 6GB i think that is correct | 16:14 |
sean-k-mooney | provided the flavor is > 6GB | 16:14 |
sean-k-mooney | well >= | 16:14 |
sean-k-mooney | https://docs.openstack.org/ocata/user-guide/cli-nova-launch-instance-from-volume.html | 16:15 |
sean-k-mooney | the 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-mooney | so it looks like we wanted the bdms to have precednce over the flavor | 16:16 |
stephenfin | Yup. That's what I'm seeing. Good point. I'd missed that nuance | 16:16 |
sean-k-mooney | i assume you followed the BFV example but changed source ot image and dest to local | 16: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 | NAME | 16:18 |
stephenfin | sort of - I hacked in a `--root` option to OSC | 16:19 |
stephenfin | it was easier than figuring out a `--block-device` invocation 0:) | 16:20 |
sean-k-mooney | ok is there a reason yoru doing this | 16: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-mooney | ok there are some things you can only do via the bdms | 16:20 |
sean-k-mooney | im confilictedc if i should tell you what those are | 16:21 |
sean-k-mooney | on one hand you might make it easy to do | 16:21 |
sean-k-mooney | on the other hand you might make it easy to do and ill have to support people doing it :) | 16:21 |
sean-k-mooney | the only way to specify a per disk device type is block_device_mapping_v2.device_type i.e. disk, cdrom. | 16:22 |
stephenfin | that 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-mooney | you can also set block_device_mapping_v2.disk_bus pre bdm | 16:23 |
sean-k-mooney | allowign diffent disks to have specific disk bus "fdc, ide, sata, scsi, usb, virtio, xen, lxc and uml." | 16:23 |
sean-k-mooney | so if you wanted your root disk to use stata and the rest to use virtio-blk or virtio-scsi | 16:24 |
sean-k-mooney | you should be able to express that in the bdms | 16:24 |
auniyal | hey 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.1 | 16:25 |
elodilles | auniyal: ACK, added to my TODO list, though probably will do only tomorrow | 16:27 |
auniyal | elodilles, thanks, yeah sure | 16:28 |
opendevreview | Pavlo Shchelokovskyy proposed openstack/nova master: Fix device type when booting from ISO image https://review.opendev.org/c/openstack/nova/+/909611 | 16:38 |
*** bauzas_ is now known as bauzas | 18:30 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!