Friday, 2026-09-04

opendevreviewGoutham Pacha Ravi proposed openstack/nova master: Use a per-instance cephx identity for CephFS shares  https://review.opendev.org/c/openstack/nova/+/100190003:22
opendevreviewGoutham Pacha Ravi proposed openstack/nova master: Extract share management from ComputeManager  https://review.opendev.org/c/openstack/nova/+/99934903:22
opendevreviewGoutham Pacha Ravi proposed openstack/nova master: Support cold migration with virtiofs shares  https://review.opendev.org/c/openstack/nova/+/98963303:22
opendevreviewGoutham Pacha Ravi proposed openstack/nova master: Reconcile stale Manila share access rules  https://review.opendev.org/c/openstack/nova/+/100191603:22
opendevreviewGhanshyam Maan proposed openstack/nova master: [func test]Catch hanging task at graceful shutdown  https://review.opendev.org/c/openstack/nova/+/100326605:34
opendevreviewGhanshyam Maan proposed openstack/nova master: Fix the test_migrate_disk_and_power_off_crash_finish_revert_migration  https://review.opendev.org/c/openstack/nova/+/100391005:53
Ugglagouthamr "but i don't know if Uggla gibi thought that wasn't necessary when they wrote this up", If I remember well nothing was really specified about it. I set something just to highlight the share was somewhere managed by nova. 09:43
*** sfinucan is now known as stephenfin10:55
opendevreviewKamil Sambor proposed openstack/nova master: Replace StaticallyDelayingWrapper with futurist DelayingExecutor  https://review.opendev.org/c/openstack/nova/+/99741012:26
opendevreviewribaudr proposed openstack/nova master: Add regression test for bug 2162835  https://review.opendev.org/c/openstack/nova/+/99997713:17
opendevreviewribaudr proposed openstack/nova master: Fix CPU pinning reset during evacuate periodic race  https://review.opendev.org/c/openstack/nova/+/99997813:17
opendevreviewKamil Sambor proposed openstack/nova master: Replace StaticallyDelayingWrapper with futurist DelayingExecutor  https://review.opendev.org/c/openstack/nova/+/99741013:19
*** EugenMayer4401809 is now known as EugenMayer44018013:33
*** EugenMayer4401808 is now known as EugenMayer44018015:06
opendevreviewBalazs Gibizer proposed openstack/nova master: Fix parallel evacuations pushing instance to ERROR  https://review.opendev.org/c/openstack/nova/+/100399415:17
gibithis is a nasty one ^^15:17
gibi(all hail the automatic evacuation engines hammering the nova-api with parallel evac requests)15:17
opendevreviewGhanshyam Maan proposed openstack/nova master: Fix the test_migrate_disk_and_power_off_crash_finish_revert_migration  https://review.opendev.org/c/openstack/nova/+/100391015:30
opendevreviewGhanshyam Maan proposed openstack/nova master: [func test]Catch hanging task at graceful shutdown  https://review.opendev.org/c/openstack/nova/+/100326615:30
gmaangibi: regarding the test_migrate_disk_and_power_off_crash_finish_revert_migration   failure, I think I am going in same direction as you commented on the ashish patch. consider the graceful_shutdown_timeout in polling the instance state and if execution is more for this test then we can reduce the graceful_shutdown_timeout  for this test to make graceful shutdown fast.15:36
gmaanI am testing the polliing fix first and try to reduce the timeout. 15:37
gmaani tried to find the ashish change but could not due to its topic, i tried to check if any effort to run this test in threading mode and only searched eventlet-removal gerrit topic15:38
gmaanashigupt: ^^ and updated the topic for your change15:43
gmaandansmith: there is one bug for the task tracking leaking the live migration task on destination, its fix it ready if you hav time to look :https://review.opendev.org/c/openstack/nova/+/1003386/115:53
dansmithgmaan: ack I'm trying to focus on the manila FFE today since we can fix bugs later15:54
gmaansure15:54
opendevreviewGhanshyam Maan proposed openstack/nova master: Fix the test_migrate_disk_and_power_off_crash_finish_revert_migration  https://review.opendev.org/c/openstack/nova/+/100391016:32
ashiguptgmaan, ack thankyou16:36
gmaanashigupt: let me test the fix on my change and then I can update you, something you can check on monday16:37
ashiguptgmaan: sure thank you16:41
opendevreviewGhanshyam Maan proposed openstack/nova master: [func test]Catch hanging task at graceful shutdown  https://review.opendev.org/c/openstack/nova/+/100326616:51
dansmithmelwitt: gouthamr I'm a bit unahppy with the double-nested RPC-to-DB calls in the reconcile patch18:01
dansmithwe can get a list of all the instances on the current host without doing the list all by host.. and can't we add another query to the object to get all mappings for a list of uuids? that would turn it from n^2 queries to 218:03
dansmithbecause we're also doing a share object lookup for every migration we find, even old long-since-confirmed migrations forever and ever it seems18:05
dansmithif we had one "get all share mappings for list-of-uuids" call we could do that once with all the uuids we're looking for, from both the instance list and migration list18:06
* gouthamr guess so, orients.. 18:08
dansmithlike, let's say we have 500 instances on this host, none of which use shares currently.. and 10k migrations over the last several years, few of which are still on this host and none of which used shares.. we're going to do 10500 RPC calls to DB queries at startup and every hour until the cows come home right?18:11
dansmithfor operators that do slide puzzle migrations, every instance running in the system will have gone through every host eventually and those will all have confirmed migration records for any host they've ever been on AFAIK18:12
dansmith(sorry n^2 queries above is not right of course, it's just n^2 iterations)18:14
gouthamrdansmith: ack, yeah no this falls on that scale - although RPC can handle it, its inefficient.. so, get all instance UUIDs on this host, union in the instance UUIDs from the migration list, then one ShareMappingList.get_by_instance_uuids for that whole set... and iterate that instead...18:14
dansmithcc Uggla since you reviewed too ^18:14
* gouthamr #TIL new term: slide puzzle migrations - that's exactly what people do18:14
sean-k-mooneyon startup we pass a list of all the isntance to the driver via a new process instnace on startup up method after thehost start is done18:14
sean-k-mooney*after the host startup stable uuid check is done18:15
dansmithsean-k-mooney: right we already have the list of instances but this adds another18:15
sean-k-mooneyrigfht i jsut removed one duplciate list all isntance on host on startup this cycle18:16
sean-k-mooneyand got down to doing it only once18:16
sean-k-mooneyso if we can not go  back to 2 that would be nice.18:16
dansmithgouthamr: I'm not sure RPC can handle it TBH.. if we have 500 nodes in a cell and all of them are hammering the MQ conductors and DB with over 10k requests every time the periodic runs, or storming them at cluster restart.. that's not going to go well18:16
sean-k-mooneyim not really followign the full converstion i just saw scary numbers18:17
dansmithoh actually it's worse18:18
dansmithbecause then for each share we again query all the instances using that share without a host filter I guess18:18
gouthamrlet me look, yeah - i guess i can piggy back on "get_uuids_by_host" - pass it down to the helper that'll get invoked at init, and periodically18:18
sean-k-mooneygouthamr: what are you actully trying to do18:19
sean-k-mooneyis this for https://review.opendev.org/c/openstack/nova/+/1001916/19/nova/compute/share_management.py#58818:19
gmaanditto, even in native threading mode, RPC is not so cheap and can put other RPC requests in queue. these numbers are really scary :)18:19
dansmithsean-k-mooney: yes18:20
dansmithgmaan: agree18:20
gouthamrsean-k-mooney: yeah, i'm trying to identify migrations that have leaked manila access rules, find them and reap them periodically18:20
dansmithgmaan: native threading will actually let n-cpu hammer the other services even harder18:20
gmaanyeah18:20
sean-k-mooneyok so 1 call to manilla for all share access rules for the current host 1 2 calls to the db 1 for the isntance on this host and one for migrations?18:21
dansmithsean-k-mooney: that's what I suggested but apparently we can't do that?18:21
sean-k-mooneyoh ok 18:22
sean-k-mooneyso to that atht then we woudl neeed to reconstcut it ofrm the nova side18:23
sean-k-mooneywhat about not makign it a perodic and provide a heal share allocations command liek we did for placement allcoaitns in nova-manage18:23
sean-k-mooneythis repair shoudl nto be needed constantly right18:23
gouthamryeah unfortunately, there's no "give me all access rules for this client" sorta call available in manila.. it's "give me all access rules for this share".. 18:24
sean-k-mooneyso if we provide a nova-manage command for it operators can use it in the event of a bug18:24
sean-k-mooneyand there is no give me all the shares with assess rules for this host command18:24
gouthamryes18:26
sean-k-mooneyok so host on the share is the hostnaem of the thing exproting the share18:27
gouthamryes18:27
sean-k-mooneyjust looking at https://docs.openstack.org/api-ref/shared-file-system/#id1718:27
sean-k-mooneyack18:27
gouthamr(not compute host)18:27
dansmithsean-k-mooney: weren't you trying to get us to stop doing things like reading proc and sysfs directly in code?18:28
dansmithusing nova/filesystem?18:28
sean-k-mooneyyes ideally18:28
sean-k-mooneymanily ot have a central place that handels the resouce busy issues 18:28
sean-k-mooneyand make it more testable18:28
dansmithlooks like no such handler for proc in there, but this code reads /proc/mounts directly, so probably should go there.. although I'm surprised there's no os module for that18:29
sean-k-mooneythe place we had the speic issue that bit us was reading the online state fo cpu cores18:30
sean-k-mooneythat was randomly returnign a device busy sortly before/after onlining a core18:30
sean-k-mooneyas it was still goting though the state machine in the kernel18:30
sean-k-mooneyim not sure if proc would have the same issue18:31
dansmithsean-k-mooney: right this code would not run on macos and so if it's naked in tests will break that compatibility we just achieved18:31
sean-k-mooneysince its not actully reading form hardware18:31
gouthamrsean-k-mooney: on the self-healing at the host level vs giving the operator means.. we thought about this earlier.. draining a access needs host-local state (my_shared_fs_storage_ip/CONF.host + "is the share still hard mounted here").. operators could disable this periodic check though and let this only occur at startup (like other optional periodic tasks) 18:31
sean-k-mooneyoh well yes that too18:31
dansmithyeah making this not self-heal is bad, IMHO18:32
dansmithand requiring cron running nova-manage on compute nodes is no bueno18:32
sean-k-mooneyim not agaisnt self healing i just suggested providing a command because we evnetull moved form healign the allcoation automaticly to a commnd18:32
sean-k-mooneybut ya i assume its like cider where we neded lcoal state to do that form brick18:33
dansmithyeah but the command has to be run on each compute node, not centrally18:33
gouthamrtechnically, on macos, you're only running unit tests?18:33
sean-k-mooneydansmith: yes like with volumes18:33
sean-k-mooneywich i also agree is kind of a pain so we can likely park that18:33
sean-k-mooneyso the prople is how to do that without melting the rpc bus18:34
sean-k-mooney*problem18:34
gouthamr(i mocked the /proc/mounts calls .. i run unit tests locally on mac, and yes, this would _have_ to be mocked regardless)18:34
sean-k-mooneyit sound like a direct remotable obejct method on InstanceList that retuns the list of instnace that have attachment on a given host with the atachmetn info is going to be needed18:36
gouthamr(ese: https://review.opendev.org/c/openstack/nova/+/1001916/19/nova/tests/unit/compute/test_compute_mgr.py#4435)18:36
sean-k-mooneyso one rpc that give you only the instnace for a given host that have attachmets with the attachment info18:36
sean-k-mooneydansmith: was the concer only with instnace the were moved sepcificly?18:36
sean-k-mooneydoing that filtering on the python side seam like a bad time but we shoudl be able to do it semi effeictly in sql18:38
dansmithgouthamr: mm, nope functional should be working (I'm running now to see if something has regressed)18:39
sean-k-mooneywe would need to join on the migration table compute nodes tabel and isntance table and isntance sares table but we shoudl be abel to say give me all instnace that migrated to/from this host that had share attachmetns18:40
dansmithsean-k-mooney: and yeah sounds like maybe a single query method that returns "any share mapping we might care about" would be the best option.. we have a ShareMappingList already which is the right place to put it I thinjk18:40
gouthamrack, i ran functional too on a mac.. /me is running it again after collapsing the queries18:40
sean-k-mooneyya if its the mappitn we care about that sound better to me as well18:40
sean-k-mooneyi was not sure if we wanted the mappitns or the instnace or both18:41
dansmithsean-k-mooney: I'm going to commit comments on the concerns.. would be nice to have your weight applied18:41
dansmithsean-k-mooney: mappings AFAIK18:41
gouthamryes, mappings18:41
sean-k-mooneysure but as the questions i have been asking imply i have not been folloing the overall work closely18:42
melwittyeah so it would be get all share mappings for a list of instance uuids rather than doing one by one right18:42
sean-k-mooneybut i agree the scalaiblity of the current approch is concerning18:42
gouthamryeah.. the data model was forcing me with some of these calls in the code.. I'm working on your suggestions to stitch (InstanceList + MigrationList + ShareMappingList) and get a list to parse... one remotable RPC that does all this would help cleanly for sure. but, can i do that in Indri? or would you prefer to have it done now?18:42
melwittadding a ShareMappingList.get_by_instance_uuids() would be simple enough. do we need more than that?18:45
dansmithgouthamr: what you just described is what I just said we should do in my comment just now18:45
sean-k-mooneyor ShareMappingList.get_by_migration_host ?18:46
dansmithgouthamr: I don't think we can/should merge this the way it is, but it would suck to have to punt it all to indri as well.. since you'll have to add RPC changes it won't be backportable either18:46
dansmithmelwitt: instance uuids is not enough and it's not even the likely larger list18:47
gouthamryeah no.. i wasn't looking to backport this stuff18:47
gouthamrits super important for me though to make forward progress on the cold migration :( it makes all of this partially usable before we fix up the hard block on other instance actions that you folks pointed out at the PTG18:47
gouthamrthe backport was just going to be for: https://review.opendev.org/c/openstack/nova/+/1001900/18:48
dansmithyup18:48
gouthamrs/backport/backport request :) 18:48
* gouthamr reads comments on gerrit18:49
gouthamrty for this discussion and the comments!18:49
sean-k-mooneygouthamr: i have not looked at https://review.opendev.org/c/openstack/nova/+/1001900 in detail but just looking at the tile im not sure that is backportable18:53
gouthamrsean-k-mooney: oh, read on :P yes, we're moving auth, but it doesn't break anything running until you choose18:53
sean-k-mooneyit might be but we have to supprot n-2 compatiblity18:54
gouthamrsean-k-mooney: this will be supplemented with an OSSN/operator guidance on how the move will occur18:54
sean-k-mooneyso we need atuth to work with a patched souce node and unpatch dest18:55
sean-k-mooneyas an example18:55
sean-k-mooneyits too late on a friday for me to really ingest the details18:55
gouthamrsean-k-mooney: it will; on the unpatched destination, we'll use a "shared credential" until that's patched some day18:55
gouthamrsean-k-mooney: yes :) ty for the brainstorm! have a great weekend18:56
dansmithsean-k-mooney: it's really not ideal, I've already raised the concerns18:57
sean-k-mooneyack, im not really sure that shareing a cephx id is a problme18:58
dansmithsean-k-mooney: the virtiofs implementation in the tree is sort of "so leaky that it doesn't break upgrades" which is not good but also means that we _can't_ do automatic cleanup there anyway18:58
sean-k-mooneybut i woudl have to read the bug to knwo why it woudl be18:58
dansmithsean-k-mooney: it's a hardening thing18:58
sean-k-mooneysure but for nova rbd driver we mostly use the same cephx key18:59
sean-k-mooneytechnially it can vary per host18:59
sean-k-mooneybut i dont really knwo of any installer that does that18:59
sean-k-mooneywe just use a single nova key on all compute nodes18:59
sean-k-mooneyso using shared ceph keyrings between host is not generally a sefutiy issue but i dont really have an object to hardening19:00
dansmithI'm not sure our bad behavior is a justification to do it badly.. but those ceph shares are less user-visible than these I think19:01
dansmithand the current implementation sort of can't even track which hosts should and should not have access to a share as a result19:01
sean-k-mooneyack as i said i have no real issue with hardenign19:01
sean-k-mooneyjsut notign that it not the same as nova provisend rbd voluems19:01
sean-k-mooneyi think for cidner we are given a cephx key in teh conenciton info to ues19:02
sean-k-mooneypossibel per voluem?19:02
sean-k-mooneyand as i said ther is nothign prventign you minting a per host key for images_type=rbd today19:02
sean-k-mooneybut ya as you said if you have escaped to the host your already have other compomise vector to worry about19:03
* gouthamr had stepped away into a terminal19:05
sean-k-mooneyfor the peroupus of share attachment the client of the sheare was desiged to be nova rather then the instnace19:07
sean-k-mooneythe same way as nova is the sincel client for ceph voleusm for images_type=rbd19:07
dansmiththat's how the code is designed now, but I think that was probably wrong19:07
sean-k-mooneyit depend on your pesertive of who own the expore the end user or nova19:08
sean-k-mooneyif its nova which is howe we desgiend it its correct19:08
dansmithtreating all of nova as a single unit is a bad idea, whether it be shares, volumes, access to the MQ or DB, etc.. in 2010 I think that was a lot more normal but today it seems verging on irresponsible to me19:08
gouthamr++19:08
sean-k-mooneyif its the end user then the cephex key shoudl be in barbican liek cinder encyption keys19:08
sean-k-mooneyright as i said im oke with hardenign but that feels mroe like a security feature then soemthign we would activly backport19:09
gouthamrcephfs != rbd, for RBD, cephx is one of several layers. For a CephFS share, the cephx access rule is the access-control layer.. 19:10
dansmithit is and you could surely argue that it's not backport-worthy IMHO19:10
gouthamreven there though, we could make a better choice than having a single shared credential, imo..19:10
sean-k-mooneygouthamr: we dont require it to be shared in nova today taht up to the installer19:11
melwittso are we thinking a separate patch for the ShareMappingList.get_share_ids_for_host() to go in front of the reconcile patch?19:11
gouthamrmelwitt: i'll fold it in.. 19:11
melwittto try and minimize churn on the reconcile? ok19:11
gouthamrsean-k-mooney: ack, yes, but in this case, it's hardcoded so worse :(19:11
gouthamrmelwitt: it was the last patch in the series, so i can fix it up in isolation :) 19:12
gouthamrdansmith: i'm exploring psutil.disk_partitions.. i see its being used by quobyte: https://opendev.org/openstack/nova/src/branch/master/nova/virt/libvirt/volume/quobyte.py#L11219:13
dansmithgouthamr: aha, nice19:14
gouthamrdansmith: it solves my problem of not hemorrhaging on an inaccessible hard mount19:14
dansmithso realistically here.. unfortunately monday is a holiday for many of us19:16
dansmithI wish Uggla was around but I assume merging this on tuesday is pretty dang late19:16
dansmithI almost wish melwitt had been less of a responsible person and sent the bottom patches to the gate already :)19:16
* gouthamr is grinning, but also sad19:17
sean-k-mooneyso realsticlly , we cut rc1 without this next week, we can still merge this on master it just will be for next cycel if we reapprove the spec19:17
sean-k-mooneyand we can repprose that already19:18
dansmithI have no plans for this weekend other than recharging my brokenness.. I'm not really up for reviewing tomorrow but I guess I could come back later today if it happens that it's all ready19:18
dansmithbut it's feeling pretty rushed for sure19:19
melwittI can also review if updated today19:20
melwittand I can check up on it over the weekend19:21
gouthamrfrom my (probably biased) vantage point, the reconciliation of rules because we're trying not to wedge instances on cold migration is good hardening.. cold migration will be used no doubt if we shipped this even without the reconciliation - but, we're doing this right and i wouldn't do it differently.. 19:21
gouthamrI know of a few <redacted> clouds that would be happy to have this feature land in H.. but they're all really waiting for a day their can hot-attach cephfs and call it a day - it's killing their cloud right now when person 100001 is coming to them to enable this, and they have to tell them they'd have to roll out their own tenant-scoped ceph cluster19:22
dansmithgouthamr: we also know the reconciliation will not be backportable if it becomes more of a problem than you think19:23
gouthamryes19:23
melwittyeah, that's the problem, reconcile would not be backportable19:23
dansmithIME instances wedged in the middle of migrations is not that uncommon so I'm nervous about having no path for cleanup for something users can trigger19:23
gouthamrack; i'll flush this out and test it and have something for your eyes in a bit.. the psutil change was the simplest :) but, the share mapping object update, i'll deep-test19:24
melwittthat is my experience also. maybe the biggest complaint about nova is stuff stuck in a state requiring manual database intervention to recover19:25
dansmithyup19:25
dansmithgouthamr: okay then I'll take a break for a bit so I can be available for a bit later19:25
gouthamrack, thank you dansmith!19:25
gouthamrsame, melwitt and sean-k-mooney!19:25
opendevreviewGoutham Pacha Ravi proposed openstack/nova master: Reconcile stale Manila share access rules  https://review.opendev.org/c/openstack/nova/+/100191620:07
opendevreviewGoutham Pacha Ravi proposed openstack/nova master: Only load same-host instances when checking share usage  https://review.opendev.org/c/openstack/nova/+/100405720:07
* gouthamr is watching gate, but got all comments he think20:39
gouthamrs20:39
dansmithmelwitt: looks okay to me21:10
gouthamrdansmith: do you mind me folding follow up comments into a single commit post these merging21:16
gouthamrlike a low-risk follow up that'll land next week?21:16
dansmithwhich comments?21:16
gouthamryou're right about "is_mounted()".. it's a mount path that we're check21:17
gouthamrchecking*21:17
dansmithoh yeah that's why I said "not critical to fix right now"21:17
gouthamr++ ack, ty will watch for melwitt's comments as well21:18
* gouthamr steps away briefly21:18
melwittwill be looking21:39
gmaandansmith: thanks for review on task tracking fix, I replied to your commment https://review.opendev.org/c/openstack/nova/+/1003386/comments/25784b40_6a5b7aad21:42
gmaanif we are ok to do RPC versioning bupm,21:42
gmaan* RPC version bump, then i can add a new RPC cast to end the task which will not impact the dest rollback for old compute mixed env21:43
gmaanI was thinking to handle that generacally in part-3 21:43

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