Wednesday, 2021-11-17

opendevreviewGhanshyam proposed openstack/nova-specs master: Allow project admin to list hypervisors  https://review.opendev.org/c/openstack/nova-specs/+/79301100:06
gmanndansmith: sean-k-mooney melwitt gibi ^^ please review the 'project admin boot server on host' spec. I have proposed to modify the existing field but if that break user I am ok on adding new field too.00:10
gmanngibi: also please remove your procedural -2. 00:10
gibigmann: I dropped the -2, I will review the spec later today06:22
opendevreviewRajat Dhasmana proposed openstack/nova-specs master: Add spec for volume backed server rebuild  https://review.opendev.org/c/openstack/nova-specs/+/80962106:27
bauzasgood morning Nova07:03
*** brinzhang_ is now known as brinzhang07:26
*** akekane_ is now known as abhishekk07:53
stephenfinlyarwood: Can I just say that our volume attachment APIs are exceptionally...weird? :-D There doesn't seem to be any way to fetch an attachment by its own ID (you have to use the server and volume IDs), and you need to pass a volume ID when updating an attachment, even if you're only changing the delete on termination behavior09:42
stephenfin(the students we're mentoring are running into bugs with openstacksdk caused by, I think, misunderstandings of how that API is supposed to work)09:43
lyarwoodstephenfin: \o morning (just back online after a few days off sick) 09:52
lyarwoodstephenfin: wait my APIs? ^_^09:52
stephenfinlyarwood: Oh, sorry to hear, hope you're feeling better /o\09:53
lyarwoodstephenfin: They pre-date me buddy but you're right that there's no support for Nova's attachment UUID to be used to lookup things09:53
stephenfinI wasn't blaming you but if you want to take ownership09:53
stephenfinYou just seemed like someone that would appreciate such comments 0:)09:53
lyarwoodstephenfin: Aye it's a valid RFE of sorts09:53
gibiit is good to see that stephen is bringing back honest feedback about our API usability. 10:06
gibi*stephenfin10:07
gibiI have no clue who has the time to fix it10:07
gibibut still the feedback is appreciated10:07
stephenfinIt could be something for future students to work on. I'll mention it to diablo_rojo10:08
sean-k-mooney[m]im really not sure why we have a volume attaments api in nova iteslf. to me the atacment is not something we shoudl be exposing to users we should just have the volume resource associated with the server10:09
sean-k-mooney[m]we dont expose port bindign in our api. they exist in neutorn but not in nova10:10
stephenfinsean-k-mooney[m]: how would propose modifying e.g. the delete on termination behaviour for an attached volume in that scenario10:10
stephenfinor doing volume swaps10:10
stephenfin*how would you10:10
sean-k-mooney[m]it would be an atibute on the volume10:10
sean-k-mooney[m]and or set on the cinder resouce10:10
stephenfinwhat about multi-attach volumes?10:10
sean-k-mooney[m]not on novas10:10
sean-k-mooney[m]you can have volume attaments they shoould just not be part fo the nova api10:11
stephenfinthis isn't a cinder thing, right? nova decides whether to delete $thing or not10:11
sean-k-mooney[m]nova should have  /server/uuid/volumes10:11
stephenfinthat's what we have :)10:11
sean-k-mooney[m]that should just list the volume not the attacments10:12
stephenfin /server/{server_id}/os-volumes10:12
sean-k-mooney[m]all other data should be in cinder10:12
stephenfinactually, no, tell a lie: the API is '/servers/{server_id}/os-volume_attachments'10:13
sean-k-mooney[m]conceptually if you are working wiht the nova api you should not be thinking in terms of volume attachment but server and volume reosuces10:13
stephenfinhttps://docs.openstack.org/api-ref/compute/?expanded=list-volume-attachments-for-an-instance-detail#list-volume-attachments-for-an-instance10:13
sean-k-mooney[m]really all nova should be tracking in its api is the volume uuid is associated with the server10:15
sean-k-mooney[m]perhaps the status of the volume too10:15
stephenfinthe mount point?10:15
sean-k-mooney[m]not the mount point the tag yes since that is a nova concept10:16
sean-k-mooney[m]we dont actully guarentee the mount point and in libvirt it is jut not correct10:16
sean-k-mooney[m]if you specifiy the mount point name there is no way for libvit to enforce it10:17
stephenfinoh, I didn't know that10:17
stephenfinwe should _probably_ mention that in the API ref10:17
lyarwoodit is10:17
lyarwoodhttps://docs.openstack.org/api-ref/compute/?expanded=attach-a-volume-to-an-instance-detail#attach-a-volume-to-an-instance10:18
lyarwoodName of the device such as, /dev/vdb. Omit or set this parameter to null for auto-assignment, if supported. If you specify this parameter, the device must not exist in the guest operating system. Note that as of the 12.0.0 Liberty release, the Nova libvirt driver no longer honors a user-supplied device name. This is the same behavior as if the device name parameter is not supplied on the request.10:18
stephenfinlyarwood++ Sweet. I was looking at https://docs.openstack.org/api-ref/compute/?expanded=list-volume-attachments-for-an-instance-detail#list-volume-attachments-for-an-instance and thought it would be mentioned there also10:18
stephenfinonce place is good enough though, for sure10:18
lyarwoodand I've wanted to remove it entirely from the response but that's going to take reworking the entire attach flow between the API and compute to drop some useless RPC stuff10:19
lyarwoodtbh we could also list it in the GET docs10:19
sean-k-mooney[m]stephenfin: so ya i dont think we should have the volume attaments api we have currently and i dont think we should mirror that for the manilla shares going forward10:20
lyarwoodsean-k-mooney: I've updated the manila spec FWIW10:22
sean-k-mooney[m]just opened it10:23
sean-k-mooney[m]ill review it this morning10:23
sean-k-mooney[m]i have a doctors appointment in an hour so i might loop back with you later10:23
sean-k-mooney[m]did you see my comment regarding the vm memory10:24
sean-k-mooney[m]oh you going to require file backed memory i almost feel like -2 for that10:25
lyarwoodyeah I've suggested going with the simple option for now and queuing the image property work for later on10:25
lyarwoodsure go ahead10:25
sean-k-mooney[m]requireing hugepages i could live with10:25
sean-k-mooney[m]file backed memory is not somethign we can schdule on today10:25
sean-k-mooney[m]so there is no way to enforce it so the vm will just not be able to access the shares if it lands on a host without it10:26
lyarwoodwell the compute would fail the request at that point10:27
lyarwoodlate on but still10:27
lyarwoodwe wouldn't have the attachment 10:27
sean-k-mooney[m]your expecting a build failure. if its like normal vhost user10:27
sean-k-mooney[m]it will boot but the connectivy wont work10:27
lyarwoodwell no10:27
lyarwoodfile backed memory is a configurable on the compute right?10:28
sean-k-mooney[m]yes10:28
lyarwoodand with this spec we are only talking about a basic attach share flow10:28
lyarwoodif we support shelved it would make this harder to assert but either way10:28
lyarwoodduring the attach or boot we'd be able to tell if the compute supported file backed or not10:28
sean-k-mooney[m]i guess since this is not boot its not as bad10:29
lyarwoodif we support shelved it would be that's more awkward yeah10:30
sean-k-mooney[m]well the other issue is10:30
sean-k-mooney[m]as a normal user you cant tell if file backed memory is used10:31
sean-k-mooney[m]so you dont know if it will work10:31
lyarwoodYeah it's awkward for end users, admins would need file backed host aggregates for this to work I guess10:32
lyarwoodbut without the image property stuff this is the best we can do in the short term tbh10:32
lyarwoodso it's either deliver something this cycle or back it up behind a pile of other work10:32
sean-k-mooney[m]yes but the vm when it was booted did not request “must be able to attached shares” in any way10:33
sean-k-mooney[m]well lets just say use file backed memory or hugepages10:33
sean-k-mooney[m]and i can look at creating the new imge/flavor extra spec in a sperate spec10:34
sean-k-mooney[m]i think it would be a good addtion outside of this feature10:34
sean-k-mooney[m]lyarwood:  hugepages also solve the requirement for vhost-user and are user requestable today via flavor or image10:35
sean-k-mooney[m]kashyap by the way do you recall i mention that file backed memory seamed to be not allocating memory form the file10:36
kashyapsean-k-mooney[m]: Very vaguely :)10:36
kashyapsean-k-mooney[m]: Can you refresh my memory, please?  Is there a ticket/bug for this?10:37
sean-k-mooney[m]kashyap i have been wondering the last day or two could that be related to tb-cache or something similar10:37
sean-k-mooney[m]no i just deployed it at home to test it10:37
kashyapInteresting.  Can you share your guest XML + QEMU command-line to see if I can reproduce it10:37
sean-k-mooney[m]then booted vms and could not over subscibe my ram with OOM10:37
sean-k-mooney[m]well ill have to repoduce it my self in a test envionent10:38
sean-k-mooney[m]but if i do i can share it with you10:38
sean-k-mooney[m]lyarwood:  are you setting up a deployment with file backed memory for your manila dev?10:38
lyarwoodyeah I plan to10:39
sean-k-mooney[m]ok can you try to reverify the behavior10:39
* lyarwood nuked his original one last week before going off sick10:39
lyarwoodyeah sure10:39
kashyaplyarwood: Hope you're hale and hearty now10:40
kashyapsean-k-mooney[m]: Yeah, that behaviour does sound like tb-cache thing10:40
lyarwoodkashyap: yup back to normal now thanks10:40
gibican I get a second core on this bugfix (bauzas and sean-k-mooney[m] are already positive on it) https://review.opendev.org/c/openstack/nova/+/813419 ?10:40
lyarwoodgibi: queued10:40
gibilyarwood: thanks! I'm glad you are back!10:41
sean-k-mooney[m]basiclly i just tried to boot 6 8G vms on a host with 48G of ram and the 6th one triggered OOM10:41
sean-k-mooney[m]gibi:  my +1 dissapeared at some point but its back on it. the config help text is now better then some of our dedicated docs :)10:46
gibisean-k-mooney[m]: thanks. bauzas pushed me to have proper config doc and even config value validation10:46
sean-k-mooney[m]i proably would have skipped the validation because its easy to miss updating that if we add a new type10:47
sean-k-mooney[m]but we will proably rememeber10:47
sean-k-mooney[m]on the other hand i am seeing a lot of issue related to edgecases with the network vif plugged events10:48
sean-k-mooney[m]which makes me think we need a systematic soluntion to this problem sooner rather then later10:48
sean-k-mooney[m]i might see if i can revie the work to pass the driver form neutron to nova this cycle10:49
sean-k-mooney[m]but without neutron telling us when the event is sent i feel like we will continue to have wack a mole issues10:50
gibilyarwood: sean-k-mooney[m]: feel free to add me as a reviewer11:11
lyarwoodsean-k-mooney: https://review.opendev.org/c/openstack/nova/+/811716 - can you also hit this again today please?12:28
gibilyarwood: I getting pretty confident that the kernel panic on stable/victoria in nova-live-migration job happens because we are live migrating a guest that is not booted fully up yet. When I added 30 sec sleep before the live migration then the problem dissapeared (5/5 run green) 13:06
gibilyarwood: from the console log I see that that without the sleep tempest would trigger live migration even 10 seconds before the guest fully booted13:07
gibiyou can see the run results here https://review.opendev.org/c/openstack/nova/+/81756413:07
lyarwoodgibi: kk I was sure I tested my PINGABLE/SSHABLE series against it last week and it still failed13:07
gibiso I think your idea to wait for pingable is a good direction13:07
gibihm, interesting13:07
lyarwoodlet me look again13:07
lyarwoodhttps://review.opendev.org/c/openstack/nova/+/817636 13:08
lyarwoodI want to say that was against https://review.opendev.org/c/openstack/tempest/+/817635/213:08
lyarwoodI'm just cleaning the series up again now and can retest13:08
gibilyarwood: I don't see kernel panic in the runs of https://review.opendev.org/c/openstack/nova/+/817636, but there are other errors. Let's re-test it and see where we are13:10
opendevreviewLee Yarwood proposed openstack/nova stable/victoria: DNM - Testing volume detach failures  https://review.opendev.org/c/openstack/nova/+/81763613:30
sean-k-mooneylyarwood: yes will do13:42
lyarwoodthanks13:43
sean-k-mooneyya ok im +1 on that ill get to your spec ater the call im on is over13:48
*** akekane_ is now known as abhishekk14:00
opendevreviewArtom Lifshitz proposed openstack/nova master: DNM: Test token expiration during live migration  https://review.opendev.org/c/openstack/nova/+/81777814:20
opendevreviewArtom Lifshitz proposed openstack/nova master: DNM: Test token expiration during live migration  https://review.opendev.org/c/openstack/nova/+/81777814:28
bauzasfolks, in case you don't know, the OpenInfra keynote is starting in 8 mins14:52
gmanngibi: thanks15:52
gibigmann: sorry I had no time to go back and properly review that today15:52
gibi:/15:52
gmannno worry.15:53
whoami-rajatlyarwood, around?16:13
lyarwoodwhoami-rajat: hey yeah16:13
lyarwoodon a call but can chat16:13
whoami-rajathey16:13
whoami-rajatack16:14
whoami-rajatso i don't have any issue with your suggestion of keeping the tried and tested way of nova doing the attachment update, but the team agreed on other flow so don't want to go back and forth16:14
lyarwoodyeah appreciate that, I wasn't at PTG so wasn't part of the discussions16:15
lyarwoodbut as someone maintaining this area more than most I'm still against passing the connector around like this16:16
lyarwoodit also keeps the cinder implementation straight forward etc so it's a win win in my view16:16
lyarwoodif other nova-specs-cores are against this then they can speak out in the spec 16:17
whoami-rajatack, makes sense to me as the code becomes easier to maintain and debug that way, not sure how much optimization that one less API call does16:18
whoami-rajati tried discussing the same with other cores in yesterday's nova meeting but we went out of time16:19
whoami-rajatso i will update the spec and see if people are against it and we require further discussion on it16:20
whoami-rajatthanks lyarwood for your inputs16:22
lyarwoodAwesome thanks and yeah agree, it's avoiding a single c-api call from n-cpu but c-vol will still have the do the same work so it's a tiny optimisation 16:22
opendevreviewArtom Lifshitz proposed openstack/nova master: DNM: Test token expiration during live migration  https://review.opendev.org/c/openstack/nova/+/81777816:44
opendevreviewArtom Lifshitz proposed openstack/nova master: DNM: Test token expiration during live migration  https://review.opendev.org/c/openstack/nova/+/81777817:39
lyarwoodgibi: https://42950ae1f17575ae9a7c-fe6c968e98fdbd85f0f135fdbc9bd3ed.ssl.cf2.rackcdn.com/817636/2/check/nova-live-migration/66b064b/testr_results.html - so that worked pretty well aside from the SG group DELETE blowing up but I can fix that up17:40
lyarwoodgibi: had to wait 32 seconds for sshd to start in the instance17:40
gibithat seems align with the logs I collected. The kernel needed more than 10 seconds to boot up17:41
gibiunfortunately there is no timestamps in the cloud init part17:41
gibibut dhcp definetly needs seconds to finish17:41
sean-k-mooneylyarwood: that being waiting for ssh/ping to work? or soemthing else17:50
lyarwoodyeah ssh17:50
sean-k-mooneycores can restore patches that are abandoned that belong to others right17:51
sean-k-mooneyi messaged the autor via gerrit but if they dont respond in a few days i might ask ye to unabandong a chagne 17:52
melwittsean-k-mooney: yeah cores can restore patch. which patch is it? I can do it18:01
sean-k-mooneyhttps://review.opendev.org/c/openstack/nova/+/74152918:02
melwittdone18:02
sean-k-mooneythanks18:02
sean-k-mooneysince it was there idea i want to give creit where credit is due and reuse there patch rather then start a new one18:03
melwittmakes sense ++18:03
opendevreviewLee Yarwood proposed openstack/nova-specs master: Repropose Add libvirt support for flavor and image defined ephemeral encryption  https://review.opendev.org/c/openstack/nova-specs/+/81086819:36

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