| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-02-27 | |||
| 13:58:05 | bauzas | tkajinam: my hard effort for today will be to purge the status etherpad and you're part of it :) | |
| 13:58:19 | tkajinam | \o/ | |
| 14:00:36 | tkajinam | this conflicts with the sev-es patches but I'm fine with pushing the sev-es work to next cycle and let that multipath change go first, given the complexity of sev-es series | |
| 14:01:46 | bauzas | thanks for telling the priorities | |
| 14:26:49 | opendevreview | Takashi Kajinami proposed openstack/nova master: libvirt: Add new option to enforce multipath volume connections https://review.opendev.org/c/openstack/nova/+/845660 | |
| 14:29:06 | tkajinam | os-brick made a breaking interface change and that is part of the latest release. ugh | |
| 14:37:19 | opendevreview | Takashi Kajinami proposed openstack/nova master: libvirt: Add new option to enforce multipath volume connections https://review.opendev.org/c/openstack/nova/+/845660 | |
| 15:01:07 | sean-k-mooney | tkajinam: looks like its small enough to add to the review list | |
| 15:01:14 | sean-k-mooney | for today | |
| 15:01:23 | sean-k-mooney | but i guess we will see | |
| 15:15:58 | sean-k-mooney | tkajinam: +2 with comments | |
| 15:16:20 | dansmith | tkajinam: I had a couple comments but didn't -1 | |
| 15:16:26 | dansmith | tkajinam: if you fix them I'll be +2+W | |
| 15:19:07 | bauzas | tkajinam: I have one concern https://review.opendev.org/c/openstack/nova/+/845660/15/nova/virt/libvirt/driver.py#1177 | |
| 15:19:08 | sean-k-mooney | dansmith: replied im not agaisnt InvalidConfiguration but it think its subtlly diffent | |
| 15:20:08 | bauzas | tkajinam: you were litterally speaking about how os-brick can break some API by a new major version, I'm a bit afraid of us pulling a hard dependency on some osbrick submodule at some driver initialization | |
| 15:20:32 | sean-k-mooney | dansmith: so if its updated im ok with that but i generally think of InvalidConfiguration as stricly configuring conflicting options or missign requried ones based on the current set values | |
| 15:21:09 | sean-k-mooney | dansmith: but i dont think i would be confused if this was InvalidConfiguration as an operator either | |
| 15:22:02 | dansmith | sean-k-mooney: I replied | |
| 15:22:51 | dansmith | bauzas: only if multipathd is configured as required and you could sidestep that problem by changing it to not if you had to for a package conflict | |
| 15:23:07 | dansmith | bauzas: to me, the more of this kind of stuff we delegate to brick (no matter where it is in their module hierarchy) the better | |
| 15:23:28 | dansmith | if they break their api and thus us, then so be it, like any other library | |
| 15:23:36 | bauzas | dansmith: sure, that's not the fact that we delegate to brick at startup | |
| 15:23:57 | bauzas | that's about calling a specific brick connector module to get that | |
| 15:24:01 | dansmith | it's that it's using a given driver | |
| 15:24:02 | sean-k-mooney | we also dont have a connecto obejct to use | |
| 15:24:11 | bauzas | while connectors do that already | |
| 15:24:12 | dansmith | sure, but the only thing that would break that should be a requirements change | |
| 15:24:14 | sean-k-mooney | we woud need to constract athat for a given voluem first | |
| 15:24:44 | bauzas | tbc, I haven't -1 the patch | |
| 15:24:51 | sean-k-mooney | bauzas: what i would prefer to do is add a privsep funciton in nova that just does what the current so brick funtion does | |
| 15:25:01 | bauzas | but I'm debating the alternatives, since I'm not at all a brick expert | |
| 15:25:07 | dansmith | sean-k-mooney: I'm very -1 on that | |
| 15:25:20 | bauzas | sean-k-mooney: sorry but I'm like dan | |
| 15:25:23 | sean-k-mooney | well i just dont like having to constuct rootwap calls | |
| 15:25:26 | sean-k-mooney | in nova | |
| 15:25:28 | bauzas | I don't want nova to do brick things | |
| 15:25:31 | dansmith | sean-k-mooney: when brick gets changed because multipathd changes, we'll never update our copy and we'll break | |
| 15:25:35 | dansmith | that's a terrible idea, IMHO | |
| 15:25:39 | sean-k-mooney | if there was a high level fonction that didnt need that it woudl be fine | |
| 15:25:44 | dansmith | bauzas: right, this is brick's domain, even if it's ugly | |
| 15:25:48 | bauzas | that's my point | |
| 15:25:55 | bauzas | I'm ok with calling brick | |
| 15:25:59 | sean-k-mooney | i dont have an issue using brick | |
| 15:26:04 | bauzas | I'm a bit torn with calling brick on a specific subdomain | |
| 15:26:22 | sean-k-mooney | i just dislike that we have to do the utils.get_root_helper() part | |
| 15:26:39 | dansmith | sean-k-mooney: oh you want to privsep wrap the brick call? that's different than what you suggested | |
| 15:26:49 | bauzas | sean-k-mooney: https://github.com/openstack/os-brick/blob/6e83ac6eeee8f3a4b3265a4e927dca1bc190e088/os_brick/initiator/connectors/iscsi.py#L523 | |
| 15:27:00 | bauzas | we could just call supports_multipath | |
| 15:27:01 | sean-k-mooney | dansmith: no i want brick to update the fucntio to use prvisep internall | |
| 15:27:04 | sean-k-mooney | and we just call it | |
| 15:27:12 | bauzas | that's what I wrote in my comment | |
| 15:27:22 | dansmith | bauzas: that's a connector object though | |
| 15:27:31 | bauzas | instead of using what looks to me an internal brick API | |
| 15:27:59 | bauzas | dansmith: doesn't it doing the same thing ? | |
| 15:28:01 | sean-k-mooney | bauzas that need us to constuct a connector which whil we could proably fake normlay need connection info form cinder | |
| 15:28:02 | dansmith | sean-k-mooney: okay that's fine, but.. don't we call brick things all over the place? are they all wrapped in privsep? | |
| 15:28:12 | bauzas | ah | |
| 15:28:14 | sean-k-mooney | dansmith: no | |
| 15:28:15 | dansmith | bauzas: we don't have a connector object though.. and we wouldn't here | |
| 15:28:30 | dansmith | sean-k-mooney: which no? :) | |
| 15:28:35 | sean-k-mooney | i was punting on that i just wanted a commnt to be added to know we need to clean this up in the future | |
| 15:28:41 | bauzas | I wish we had then a more abstract API to consume | |
| 15:29:04 | bauzas | but I'm cool with merging it as it is, provided we document it may break :) | |
| 15:29:05 | dansmith | sean-k-mooney: comment works :) | |
| 15:29:14 | bauzas | heh, jinxed | |
| 15:29:20 | sean-k-mooney | i was ok if not supper happy with multipath_running = linuxscsi.LinuxSCSI.is_multipath_running ... | |
| 15:29:36 | sean-k-mooney | a comment woudl be nice i just didnt want to ask for a respin for that | |
| 15:29:39 | dansmith | bauzas: I don't think that's a private brick api | |
| 15:29:41 | sean-k-mooney | on its own | |
| 15:30:03 | dansmith | bauzas: it's underscored in the connector usage, but it's api is not underscored | |
| 15:30:17 | bauzas | you're right | |
| 15:30:44 | bauzas | anyway, I'm cool with shipping as it is, hence my +1 but I'd appreciate if we could comment out that call | |
| 15:30:49 | bauzas | comment s/out// | |
| 15:31:31 | bauzas | (english and all your adverbs) | |
| 15:35:13 | sean-k-mooney | tkajinam: so tl;dr dansmith was right and if you adress there intial comemnts i think we are all ok with your patch | |
| 15:35:14 | dansmith | sean-k-mooney: just to circle back.. I'm not sure InvalidConfiguration should really inherit from Invalid, but raising Invalid itself defaults to "400 Bad Request" because that's really for HTTP Invalid.. I know he's overriding the message, | |
| 15:35:22 | dansmith | but it's just not what Invalid should be used for, IMHO | |
| 15:36:21 | sean-k-mooney | your right i was orgianly considerign if it shoudl be a RuntimeError but decied not to suggest that | |
| 15:36:40 | sean-k-mooney | partly because i was nto sure RuntimeError woudl abort startup | |
| 15:37:19 | dansmith | yeah, RuntimeError is interesting, but I tend to think that's more useful for things like "This shouldn't happen" .. programming errors more than something the operator could have done | |
| 15:37:28 | dansmith | anyway, it's all a bit pedantic anyway | |
| 15:38:28 | sean-k-mooney | to me that is what the second test really is, the config is valid but the runtime depency is not met. so either this should be some kind of custome excpetion or we use `InvalidConfiguration` adn agree that you were right before you confince me otherwise :) | |
| 15:38:42 | sean-k-mooney | s/confince/convince/ | |
| 15:54:49 | masahito | hi nova folks, could you review the API bug fix? https://review.opendev.org/c/openstack/nova/+/939658 It needs +W now. | |
| 16:47:59 | jizaymes | processed that correctly and added the ubuntu user ssh authorized key, but somehow the functionality within nova is now not working. no clear log events in the neutron-ovn-metadata service logs or nova logs. Recommendations? | |
| 16:47:59 | jizaymes | Hello. I made the mistake of trying to rebuild my linux images for openstack (2024.2) to add qemu-agent baked in -- that worked but couldn't SSH in. I then reverted my images to the vanilla ubuntu cloud images (tried 22 and 24, no customization.) In all cases now, cloud-init sets the IP and dns servers, etc, but no longer injects the ssh key for the ubuntu user. I ran the image separately on a kvm box with a cloud-init .iso I made and it | |
| 16:58:57 | opendevreview | Doug Szumski proposed openstack/nova master: Stop corrupting ephemeral volumes during cold migration https://review.opendev.org/c/openstack/nova/+/940900 | |
| 17:08:04 | jizaymes | seems to match this thread, but I am not sure what their resolution was https://lists.openstack.org/archives/list/openstack-discuss@lists.openstack.org/thread/XPHA7B5D6SIZZPLXT7CLLXX5OHYPTYSW/#XPHA7B5D6SIZZPLXT7CLLXX5OHYPTYSW | |
| 17:13:41 | dougszu | sean-k-mooney: I've had a go at removing the ephemeral volume backing file in the above patch - it doesn't seem too bad. Any thoughts when you've got a moment would be great | |
| 17:21:19 | sean-k-mooney | dougszu: skiming it, ack on updating to also ensure new vms dont have backing files for ephmeeral, (it sould also be done for swap) today is feature freeze (upstream and a seocnd one downstream) so im kind of maxed out on reviews currently | |
| 17:21:45 | sean-k-mooney | dougszu: i can try and take a look at this next week dougszu feel free to ping me about it | |
| 17:22:04 | dougszu | many thanks! will do | |
| 17:22:54 | sean-k-mooney | live migration is harder so one appoch would be to do that in a diffent patch. | |
| 17:23:19 | sean-k-mooney | focus this on new vms (inculding unshelve) and cold migrate/rezise | |
| 17:23:30 | sean-k-mooney | if we flattened images on hard reboot | |
| 17:23:47 | sean-k-mooney | or agent restart that might also be an option | |
| 17:24:26 | sean-k-mooney | everngly all vms would not have backign files for ephmeral and swap and so live migrate woudl eventurally resolve | |
| 17:24:37 | sean-k-mooney | thats kind of a cop out but | |
| 17:24:46 | sean-k-mooney | incremental progress is still better then none | |