Thursday, 2022-05-19

opendevreviewBrin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from Flavor Access APIs  https://review.opendev.org/c/openstack/nova/+/76770406:06
*** akekane_ is now known as abhishekk06:46
opendevreviewBrin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from List/Show usage APIs  https://review.opendev.org/c/openstack/nova/+/76850906:46
opendevreviewBrin Zhang proposed openstack/nova master: Replace tenants* with projects* of policies  https://review.opendev.org/c/openstack/nova/+/76531506:46
opendevreviewBrin Zhang proposed openstack/nova master: Replace os-simple-tenant-usage with os-simple-project-usage  https://review.opendev.org/c/openstack/nova/+/84228806:46
opendevreviewBrin Zhang proposed openstack/nova master: Replace tenant_id with project_id in os-quota-sets path  https://review.opendev.org/c/openstack/nova/+/76885106:46
opendevreviewBrin Zhang proposed openstack/nova master: Replace tenant_id with project_id in Limits API  https://review.opendev.org/c/openstack/nova/+/76886206:46
opendevreviewBrin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from Flavor Access APIs  https://review.opendev.org/c/openstack/nova/+/76770407:51
opendevreviewBrin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from List/Show usage APIs  https://review.opendev.org/c/openstack/nova/+/76850907:52
seyeongkimhello, I have a question about lp https://bugs.launchpad.net/nova/+bug/1950186 if this can be fixed in near future or if there is any plan? Thanks in advance.08:11
Ugglagibi, hello if you can have a look at https://review.opendev.org/c/openstack/nova-specs/+/83150608:23
gibiI'm looking at the manial one right now but then I can look the unshelve too08:25
Ugglagibi, oh cool as well thx.08:32
bauzasUggla: IMHO we shouldn't add a specific parameter for asking to pin an AZ08:36
bauzasif the user asked for a specific AZ, it's pinned08:37
bauzasso if the operator wants to move the instance to another AZ, OK, but then it should be pinned again08:37
bauzasif the user didn't ask for a specific AZ, then it's not pinned08:38
bauzasso, if the operator moves the instance to another AZ, then meh08:38
Ugglabauzas, sure but if he unpins an instance he needs to recreate it to pin it ?08:38
bauzasUggla: who "he" ?08:38
bauzasthe admin ?08:38
bauzasor the user ?08:38
Ugglabauzas, user08:39
bauzasbut the user only creates the instance08:39
bauzashe doesn't have a "pinned" parameter08:39
bauzashe doesn't know about the environment08:39
bauzasbut what he knows is that the environment has multiple AZs and he just wants his instance to be in one of them08:40
bauzasthen, he'll create the instance by asking --az AZ1 08:40
bauzaswhich will be pinning the AZ08:40
Ugglabauzas, yep, then he can shelve/unshelve with AZ=None to unpin it08:41
bauzasthen, if some operator wants to move the instance to another AZ, then he needs to explain to the user that the instance will be moved, as the user will see a different AZ for his instance after it's moved08:41
Ugglabauzas, why operator ? user can do it as well. Am I wrong ?08:42
bauzasUggla: correct, but what I'm explaining is that if the operator does this (unshelve with AZ to be None), then he will explain his user that the instance could be moved to any AZ08:42
bauzasUggla: no, because endusers can't unshelve to an AZ, right?08:43
bauzasyou need to be admin08:43
Ugglabauzas, user can unshelve to an AZ not to a host08:43
bauzasoh my bad then08:43
bauzasUggla: well, then we need to document the fact that unshelving to AZ=None means the instance could be moved after to *any* AZ, exactly like when you create it without asking for an AZ08:44
bauzasand if you unshelve az=AZ2, that exactly means like create with --az AZ208:44
Ugglabauzas, agree but there is no way to pin the instance to an az without recreating it.08:46
bauzasUggla: or shelving again08:47
bauzasright?08:47
Ugglabauzas, no due to new behavior08:49
bauzasUggla: if an instance was unshelved with az=None, the ReqSpec.az field will be None, right?08:49
bauzasoh I see08:50
Ugglayep but not way to set it again to az=foo.08:50
UgglaNo AZ    | AZ but, no host        | **Schedule in the AZ, keep the         |08:50
Uggla|          |                        | reqspec.AZ as None**   08:50
bauzasthen if you pass shelve/unshelve with az=something, it will move to this new AZ without pinning it08:50
bauzasI see08:50
Ugglabauzas, yes08:50
bauzaswell, I'd say it's a hard choice to make08:51
bauzasbut the point is, if an instance is unpinned, you have no way to pin it08:51
bauzasproblem is, the existing behaviour pins it even if you don't want08:52
bauzasUggla: honestly, this is debatable08:52
Ugglabauzas, yes it was raised by sean as a question, but playing with devstack I realized that it could be non conveniant.08:52
bauzasUggla: do we agree on the fact that you explicitely need to set az:none in order to unpin the instance, right?08:53
Ugglabauzas, yep sure08:53
bauzasthis can't be done without the caller explicitely opting into it08:53
bauzasyou can unshelve without any parameter08:54
bauzaswhich will leave the instance in the same AZ08:54
bauzasyou can now unshelve to a host08:54
Ugglabauzas, yep 08:54
bauzaswhich will leave this instance to this AZ08:54
bauzas(be pinned)08:54
bauzasyou can unshelve to another AZ08:55
bauzaswhich will move this instance to the new wanted AZ, still pinned tho08:55
Ugglabauzas, if it is pinned at boot time it will remain pinned08:55
bauzasso, frankly, the caller exactly has to call "I want my instance to be unpinned"08:55
Ugglabauzas, yep absolutely, but if you do that it will remain unpinned. With no way to pin it again without recreating it.08:56
Ugglabauzas, I'm fine with that but take the opportunity to raise this fact as we want to review mutual exclusive stuff (which may cause an issue on the client side).08:57
bauzasthen you probably have right08:58
bauzass/have/are08:59
bauzasI see your honest concern, people would want to tell "I want my instance be stuck somewhere now"08:59
Ugglabauzas, by the way I have no real interest to change as my code is working... :)08:59
bauzasproblem is, I don't know which exact semantics to write09:00
bauzasUggla: man, you litterally just did put your left food in some poop, you know09:00
bauzasthis is all your honot09:00
bauzashonor09:00
bauzasbut this is the joy of review times09:01
bauzaseither reviewers or even you think about something new you haven't written, and then you put your code in the trash09:01
Ugglabauzas, I know, but I think if we don't provide this, I'm pretty sure user will complain saying there is no way to unpin an instance and recreating it is painful.09:02
bauzasI literally see this coming, indeed.09:03
bauzasthe problem is that 'AZ pinning' is something we don't explicitely mention09:03
Ugglabauzas, btw it was the case with the old microversion.09:03
bauzasthis is more a tribal knowledge09:04
bauzasand a lot of people are confused by the multiple config options and parameters we have09:04
Ugglabauzas, of course it means we have to explain this very well.09:05
bauzas(you can even pin all your instance to a specific AZ, even if not specified by the client, thanks to the very errorprone schedule_default_az option)09:05
Ugglabauzas, to come up with what you said earlier. My previous colleagues called me "Le chat noir" if something can go wrong. Then it is for me. :)09:11
gibiI've just read the scrollback about unshelve09:13
bauzasUggla: you're absolutely not alone in this situation. In the past, I don't count the number of revisions I made based on feedback and myself reconsidering a few things, sometimes even restoring an old revision as a fresh new PS because eventually we considered all the things we discussed over a couple of revisions were meaningless09:13
gibido we have a way forward or we are still in the open?09:13
bauzasgibi: I think we're basically arriving to a consensus09:14
gibiwhich is? :)09:14
bauzasbut there is one left question about the unpinned AZ09:14
gibido we need to consider the value of schedule_default_az during unshelve if the AZ param is not provided?09:15
* gibi hides away09:15
bauzasgibi: which is to continue to do what we agreed on before, but either documenting the 'unpinned can't be pinned again' thing, or provide some explicit parameter for it09:15
bauzasgibi: you know that this option is only used by the api service and some ops use that for RR load-balancing between AZs ?09:16
bauzasthey have different value per nova-api service09:16
gibipersonally I don't want to have schedule_default_az in the picture09:16
gibias it is extra complication09:17
bauzasgibi: don't disagree with this09:17
gibino I did not know that there are people out there abusing schedule_default_az09:17
bauzasproblem is, unshelve to AZ is end-user API09:17
bauzasand that's a problem09:17
gibiAZ is an end user thing09:17
gibiduring boot too09:17
gibi(excpet for schedule_default_az ;) )09:17
bauzasyes I know09:18
bauzasthat ^09:18
gibisure09:18
bauzaswe're mixing wishes of the enduser with operator toughts09:18
gibiyepp09:18
* Uggla out 10mn09:18
gibiand most of the time operator > end user09:18
gibiexcept for schedule_default_az09:18
bauzasbut this is clear that user choice supersedes the schedule_default_az thing09:18
gibias that is just a default that can be overridden by the end user09:19
bauzasso this is bikeshed09:19
bauzasyeah09:19
bauzaslet's put schedule_default_az out of the picture :)09:19
* bauzas also has his kid to drive back from school09:19
gibipretty please document this decision that schedule_default_az will not be used during unshelve09:20
gibiwe seriously need to beef up our doc around this09:20
gibito have something to point at when bugs appeare09:20
bauzaspossibly09:21
bauzasthat's what I said, AZs are tribal knowledge atm09:21
* bauzas drops09:21
gibiUggla: I'm not sure I get the meaning of the colums of the table in your comment https://review.opendev.org/c/openstack/nova-specs/+/831506/4#message-7d59a1d14206c958ad82d974af5f2a2c00ad255c 09:30
Ugglagibi, I just introduced a new param "az constraint". That would be used to choose if you want to pin/unpin the az of an instance.09:36
gibiahh so that would be a new API param09:36
Ugglagibi, yes sorry if it is not clear.09:36
gibiand above bauzas suggested that we don't add that, isn't it?09:36
gibino worries09:36
Ugglagibi, yes09:36
gibiack09:36
gibiI will try to summarise my current understanding about the agreement from above in the review09:37
Ugglagibi, but bauzas said that before the discussion about pin/unpin of AZ.09:37
gibi /o\ ETOOCOMPLEX09:37
gibianyhow09:38
gibiI will try to write something in the review :D09:38
gibiconsidering all the discussion happened before09:38
Ugglagibi, hum yes I think that as well writting it. :)09:38
Ugglagibi, but take care about the client issue as well.09:39
gibido you mean the CLI issue?09:39
Ugglagibi, yes I introduced a param unpin-az to avoid issues with --availability-zone None  --> None treated as a string "None".09:40
gibiI assume if we come up with an API semantic that is accepted then will be able to map that to a meaningful CLI some way09:41
Ugglagibi, so client as 3 param --availability-zone, --host, --unpin-az  --> API only 2 --availability-zone and host.09:42
gibiand in the CLI we can have extra params like unpin-az if we need it09:42
gibiyep, CLI is easy :D09:42
Ugglagibi, yes but it could be tricky without the mutually exclusive stuff.09:43
Ugglagibi, ex on the cli user could request --availability-zone nova --unpin-az  --> with is not possible.09:43
gibiyepp, that we can catch that on the client side 09:44
Ugglas/with/which/09:44
gibi--availability-zone and  --unpin-az is mutually exlisive on the client side09:44
Ugglagibi, today all params are mutually exclusive. So no pb.09:45
Ugglagibi, and this is the case on the CLI and API.09:46
Ugglagibi, btw so we ended up to not support pin of an az via unshelve ?09:48
gibithat is how I read bauzas above09:49
Ugglagibi, to be honest it is unclear to me. :)09:50
gibibut I think dansmith had other opinion09:51
gibi"I agree, it's very confusing to sometimes provide an AZ and pin and other times not. That seems like something we should avoid."09:51
Ugglagibi, should we try to put that point to today meeting agenda ?09:52
gibiif you can ask the proper questions, then yes we can try09:53
opendevreviewBrin Zhang proposed openstack/nova master: Replace os-simple-tenant-usage with os-simple-project-usage  https://review.opendev.org/c/openstack/nova/+/84247609:54
Ugglagibi, I put the question in the agenda. If you want to have look.10:10
opendevreviewJohn Garbutt proposed openstack/nova master: Ironic: retry when node not available  https://review.opendev.org/c/openstack/nova/+/84247810:14
gibibauzas: Uggla: left a comment to the unshelve spec pointing to an extended table in the etherpad10:23
sean-k-mooneygibi: looks like  the resize patch was hit by infra issue again10:48
sean-k-mooneyim seeing some mysql failures10:48
gibisomething is going on I saw mysql issues on other tempest patches this week10:56
sean-k-mooneygibi: i think dan would prefer if the value provided for the az paramater is unconditionally used to update the request spec10:58
sean-k-mooneyso if its not provide we dont update10:58
gibiyes I think so10:58
sean-k-mooneyif you provide a sentenal proably {} we woudl set it to python None10:59
sean-k-mooneyany string value jsut gets set in the request spec10:59
sean-k-mooneyactully we could use null10:59
sean-k-mooneyunqutated as in the json value null for python None that is the write sentenial11:00
sean-k-mooneyim ok with taht as it provides a way to pin and unpine and its simple to reason about11:00
sean-k-mooneythat just meas we get rid of the idea of az and host being mutally exclucive11:01
gibisean-k-mooney: have you checked the new table in https://etherpad.opendev.org/p/unshelve-to-host#L102 ? I think this is now in sync with what you wrote and what Dan suggested11:02
sean-k-mooneyso az unset means use current requstspec value, az=null means update to python None az="any string" means update to that stirng11:02
sean-k-mooneyi have it open but have not read it yet was reading the scrollback here11:02
gibisean-k-mooney: yepp 11:03
sean-k-mooneyso the only delta is im being explict about types11:03
sean-k-mooneyso we are not reserving the string None  to by python None11:03
sean-k-mooneywe are using the json value null11:04
gibialso the previous proposal did not pin when no AZ (boot) -> AZ (unshelve) was requested11:04
gibibut the new one pins11:04
gibias per dansmith's point11:04
sean-k-mooneywhy is no unpinning allow if the host is requested11:05
sean-k-mooneywould it not be simplet to jsut entirly decoule the two paramters11:06
gibisean-k-mooney: do you mean L182?11:06
gibiL12811:06
sean-k-mooneyyes11:06
gibiI think it can lead to contradiction11:07
sean-k-mooneyi dont think so11:07
gibino AZ host1 -> pin to the AZ of host111:07
gibibut AZ=None host1 -> not pin due to AZ=None or pin due to host1?11:08
sean-k-mooneyno your miss understanding what i was suggesting11:08
sean-k-mooneyi was suggesting there is no implict change to az by passign host11:08
sean-k-mooneyso if the host is in a differnt az it will fail11:08
sean-k-mooneyif you dont care about the az but just want the host then 11:09
sean-k-mooneyaz=null host=host-111:09
gibiso you would change both L127 and 128 but keep L130?11:09
kashyapZuul blessed this, can anyone just put this through: https://review.opendev.org/c/openstack/nova/+/838926 (libvirt: Add a workaround to skip compareCPU() on destination)11:09
sean-k-mooneygibi: yes i think so11:10
sean-k-mooneygibi: if the aim is to simplfy i think we shoudl remove all the magic11:10
sean-k-mooneyso there is no implict updates of the az11:11
gibithe question is then what happens today if the instance is booted with --availability-zone :host1 (so no AZ but host provided)11:11
gibido we pin today to the AZ of host1? if not then I agree with you11:11
sean-k-mooneygibi: i dont think that si vaild11:11
gibilet me try11:11
sean-k-mooneyi think to use the host via the az an az is required11:11
opendevreviewStephen Finucane proposed openstack/nova master: neutron: Unbind remaining ports after PortNotFound  https://review.opendev.org/c/openstack/nova/+/84252811:19
opendevreviewStephen Finucane proposed openstack/nova master: neutron: Unbind remaining ports after PortNotFound  https://review.opendev.org/c/openstack/nova/+/84252811:20
gibisean-k-mooney: "--availability-zone :gibi-devstack-aio" is allowed and it forces the host11:20
sean-k-mooneythat is not documented anywhere that im aware of11:20
sean-k-mooneyi would have filed a bug for that11:21
gibibut does not set the AZ in the request spec11:21
sean-k-mooneyso your saying this feature is alreay supported11:21
gibiwe can force the host without pinning to the AZ of the host11:21
sean-k-mooneywell we dont allow host for unshleve in the az11:21
sean-k-mooneygibi: well you can force and pin too11:22
gibiyepp11:22
sean-k-mooneyby passing the correct az11:22
stephenfinsean-k-mooney: gibi: That's a dead simple nova/neutron patch for whenever you've time ^11:22
gibistephenfin: ack11:22
sean-k-mooneyack11:22
gibisean-k-mooney: so based on this I think it is OK to decouple the host from AZ in unshelve too11:23
sean-k-mooneythat reminds me we dont currently unbind port when we shelve offload. we really shoudl fix that11:23
sean-k-mooneyi summareised my toughs "again" on line 139+11:23
gibisean-k-mooney: just not blinding unbind but keep the device_id set and check with neutron that this keeps the port reserved11:24
gibisean-k-mooney: thanks11:24
sean-k-mooneyUggla: rememebr when we said this was a simple non contoversial spec11:24
sean-k-mooneygibi: unbinding never touches the device_id11:24
sean-k-mooneythat woudl be a detach11:24
sean-k-mooneyunbinding is just resetting the binding:host-id11:25
sean-k-mooneyand possible you could clear the port_profile bits11:25
sean-k-mooneybut ya we shoudl keep device_id set when unbinding for shelve_offload11:25
sean-k-mooneyto keep the port owned by the vm11:26
sean-k-mooneystephenfin: i was going to ask why the continue but i sse wew try and use the port profile later11:27
sean-k-mooneyya this looks correct to me.11:27
stephenfinYeah. I'm not too sure about that second exception handler (if we can't fetch the port, I'm not sure how much we will fare with updating it) but that's a problem for another day ;-)11:28
stephenfinUnrelated, but if anyone hasn't gotten an ultrawide monitor yet, you need to get on that11:28
stephenfin<311:28
sean-k-mooneywell the second excpetion handeler11:29
sean-k-mooneyis only invoked if we do find the port11:29
sean-k-mooneybut there is a differnt excption11:29
stephenfin(new purchase this week - what have I been doing all these years? the extra pixel density makes the world of difference too)11:29
sean-k-mooneybut i guess that still means we dont have the port object11:29
sean-k-mooneyso ya it likely is not correct11:30
kashyapstephenfin: What is the model that you have?11:30
kashyap(Monitor model, i.e.)11:30
stephenfinHP M34d11:30
sean-k-mooneystephenfin: you have seeen the one on my desk11:30
stephenfin34" ultrawide, 3440x1440 resolution, USB C Power Delivery (which is A+)11:31
sean-k-mooneymine is similar 34" 21:9 11:31
sean-k-mooney3440x1440@100hz11:31
stephenfinOnly thing it's lacking is Thunderbolt which would be handy since I could daisy chain a second monitor. As things stand, I'm just using my laptop screen11:31
sean-k-mooneypredates usbc however11:31
stephenfinsean-k-mooney: sure have. This can do 100Hz if you're willing to put up with USB 2.0 speeds for the other ports, but Fedora seems not to drive 60Hz regardless11:32
stephenfinI'm not gaming so it doesn't matter much anyway11:32
sean-k-mooneyi have been debating between going to 2 ultrawids or 1 large 4k display for a while but didnt find a cheep ultrawide11:32
sean-k-mooneyhum also curved11:34
stephenfinThis was a 479€ which seemed reasonable for what it had. I could justify more than doubling that for a 49"11:34
sean-k-mooneythat is the one think i didnt want on this11:34
stephenfinAlso, you'd *have* to step up to Thunderbolt for that11:35
stephenfinDon't think even USB 3.2 has the bandwidth for that11:35
sean-k-mooneyam maybe with display stream compression11:35
stephenfin(i.e. 5120 x 1440 resolution)11:35
sean-k-mooneybut it depsn on the resolution11:35
sean-k-mooneyya 4k ultrawid is nice11:35
sean-k-mooneythat is what i was loking at11:35
sean-k-mooneybut $$$11:36
sean-k-mooneyi have basically been thinking about getting rid of my virtical monitor and laptop off my desk and addin a seocnd ultrawid on top of my current one for email and irc11:37
sean-k-mooneyright now my laptop (left of ultra wide) is my email screen and portait monitor (right of ultra wide) is irc11:38
sean-k-mooneyim just worried they might be two high11:38
gibisean-k-mooney: see the new table at https://etherpad.opendev.org/p/unshelve-to-host#L16611:47
gibiis this what you are after?11:47
sean-k-mooneykind of i woudl prefer if we use null and queted strings to be explcit still reading it11:52
gibiwhen I say "no AZ" it means the availability_zone field is not in the request body11:52
sean-k-mooneyyep i know11:53
sean-k-mooneyi was talkign about AZ=None11:53
gibiso that is not the same as availability_zone = null11:53
gibiahh11:53
gibiOK11:53
sean-k-mooneythat is ambiquious11:53
gibilet me fix that11:53
sean-k-mooneythat could mean AZ="None" or AZ=null11:53
sean-k-mooneycool11:54
gibidone11:54
gibiyepp thanks for the quotes11:55
sean-k-mooneyso yes that is what i was thinking11:56
gibicool11:56
sean-k-mooneyi was also thinking of just leaving the host->az check to the schduler11:56
sean-k-mooneywe could do it in the api if we want too11:56
gibiUggla, bauzas, dansmith: please check the table in https://etherpad.opendev.org/p/unshelve-to-host#L166 at least Sean and I agree on that now11:56
sean-k-mooneybut we could jsut convert the no valid host to a better error11:56
gibiyeah, good point11:57
gibithat will be NoValid host if we not check in the api11:57
gibibut that is fine11:57
sean-k-mooneyim going to get coffee quickly brb11:58
gibisean-k-mooney: about the unbind, stephenfin's patch just shows that unbind removes the device_id from the port so we cannot bindly unbind at offload12:00
sean-k-mooneygibi: that is not an unbind then12:05
gibithe function is called unbind :)12:05
sean-k-mooneyat least not at the nutron api level12:06
sean-k-mooneyright its miss named12:06
sean-k-mooneythat is unbind_and_detach12:06
gibihttps://review.opendev.org/c/openstack/nova/+/842528/2/nova/network/neutron.py#61612:06
sean-k-mooneyya that function should be renamed 12:06
sean-k-mooneythat is actully a detach12:06
sean-k-mooneyits a detach and clearing of dns_name12:07
sean-k-mooneyactully does this even do an unbind12:08
sean-k-mooneyah i t does line64412:08
sean-k-mooney  constants.BINDING_HOST_ID: None,12:09
sean-k-mooneyis the actual unbind12:09
sean-k-mooneygibi: this is being used as part of instance delete which is why its doing all the other stuff12:10
gibiyeah, this is correct during delete but it would not be corrrect during offload12:10
sean-k-mooneyits just combining the unbind with teh other operations needed to reset the port12:10
sean-k-mooneyright but that function is not only doing an unbind12:11
sean-k-mooneyi never said we shoudl use that funciton12:11
gibisure12:12
gibiit is about terminology12:12
sean-k-mooneywe shoudl proably rename that "release_ports" or "reset_ports"12:12
gibiyeah I agree12:13
sean-k-mooneyright terminology wise that function is missnamed12:13
sean-k-mooneyif i propose a patch for unshelve12:13
sean-k-mooneyill porpsoe a clean up patch to rename this12:13
gibiack12:14
sean-k-mooneyspeaking of patchs my spawn_n patch actully passed ci this time12:15
sean-k-mooneywell ignoreing pep812:15
sean-k-mooneybut it did not seam to really decrease the number of greenlets12:15
sean-k-mooneyi guess becasue when we are counting them 12:15
sean-k-mooneywe count the wrapped greenlets12:15
sean-k-mooneyand the freestanding ones12:15
sean-k-mooneyso as you predicted it has no visabel effect12:16
gibithe code should only count naked greenlets that are not wrapped12:18
gibiin a GreenThred12:19
sean-k-mooneyhum well the number looked identical12:19
gibihttps://review.opendev.org/c/openstack/nova/+/841040/6/nova/compute/manager.py#1015012:19
sean-k-mooneyits possible it did not work i coudl add a func test to check12:19
gibigreenlets might be special from the GC perspective as they are implemented in a C extension12:19
gibiso this can be a side effect of my hack to looking at the GC12:20
sean-k-mooney maybe12:20
sean-k-mooneynot really sure12:20
gibime neither12:20
sean-k-mooneywill isinstance look at inheritance12:21
gibiyepp12:21
gibiit does12:21
sean-k-mooneyand GreenThread contains but is not a greenlet12:21
Ugglagibi, looking at the table, line 3 we came back to the original behavior right ? Which is something we wanted to avoid. (Not break user choice to keep the instance floating.)12:22
sean-k-mooneygibi: https://github.com/eventlet/eventlet/blob/master/eventlet/greenthread.py#L163=12:22
Ugglagibi, although I think it is a better choice.12:22
gibiGreenThread derives from greenlet https://github.com/eventlet/eventlet/blob/master/eventlet/greenthread.py#L16312:23
gibisean-k-mooney: you were faster12:23
sean-k-mooneygibi: yep so greenlet includes all the greenthread too12:23
gibiso isinstance(GreenThread(), greenlet) is True12:23
sean-k-mooneyya12:23
gibithe count does not 12:23
gibias I have an if12:23
gibinaked = [g for g in greenlets if g not in greenthreads]12:24
sean-k-mooneyoh ya12:24
gibiso only count those greenlets that are not counted for greenthread12:24
gibis12:24
sean-k-mooneydid see that look was lookign jsut at greenlets and ghreenthreads12:24
gibiah jeah I forgot to log _naked_ greenlet12:25
gibihm12:25
gibiI idd12:25
gibidid12:25
sean-k-mooneyyou jsut didnt log total12:25
gibiso naked here means not wrapped in a GreenThread12:25
gibiyeah I did not log total as that would count things twice12:25
gibii.e. a GreenThread would also be counted as greenlet too12:26
sean-k-mooneyUggla: no we are redefiening what the qeust means12:26
sean-k-mooneywe are defien that if you pass an AZ you are requesting for it to be pinned to that az12:26
sean-k-mooneywhich is the current unshelve to az beahvior12:26
gibiyepp12:27
Ugglasean-k-mooney, gibi , I'have just got it. Seems ok for me.12:27
sean-k-mooneyUggla: so we found that odd in the previous case beacue of the implict magic of updating az somethimes but nota always12:27
sean-k-mooneybut now that its only ever updated explcitly in this table12:28
sean-k-mooneyits consistent12:28
Ugglasean-k-mooney, I agree it is better now and unpin/pin is possible which is cool.12:28
sean-k-mooneyUggla: for better or worse this is why we have specs for api changes12:30
sean-k-mooneyUggla: something that seams simple like "just unshalve to this host" can have subtel issues with considtency12:30
sean-k-mooneyso hopefully this has not put you off specs12:30
Ugglasean-k-mooney, completely understand, I was worried by the fact that unpin was not possible. Now the spec is better.12:31
sean-k-mooneygibi: ill proably respin my monkey_patch change and add a release note then bring it up in the next team meeting12:31
sean-k-mooneygibi: basiclly so we want to proceed with it to get operator feedback or abandon it12:32
sean-k-mooneydefaulting to False of cource so its opt in not opt out12:32
Ugglasean-k-mooney, I have also learn to take time to mature the spec before doing the code.... ;)12:33
sean-k-mooneyhehe well there are pros and cons to that technially you are ment to wait for the spec to be approve to start on the code however doing a quick poc can help with writing the spec in some cases12:34
sean-k-mooneyoften it can be helpfull to get something mostly working but defer the docs and test code till after the spec is approved12:35
sean-k-mooneybut only for small changes12:35
Ugglasean-k-mooney, I agree, for the unshelve one I would have wait before redoing it. On the virtiofs, doing it in // helps me to better understand and to have questions.12:35
Ugglagibi, fyi I have removed the point for today's call.12:38
Ugglasean-k-mooney, gibi I'm gonna update the spec with these new data. If you are ok ?12:39
sean-k-mooneyfor spec design stuff we try to keep as much of it public as we can anyway12:39
sean-k-mooneyUggla: am sure  you could12:40
sean-k-mooneyUggla: when we bring up topic in internal calls it generally for high level brain stroming and we dig into the detail in specs,mailing list or public irc12:41
Ugglasean-k-mooney, ok 12:42
sean-k-mooneybasically we gather ideas privately and publicly but try to do all decision making and deep technical discusion as public as possibel to allow others to chime in. so we would not have made a discusion in the internal call anyway12:42
gibisean-k-mooney: re spawn_n, OK, cool, maybe we can add mnaser to that patch12:43
gibiUggla: maybe before you update the spec ping bauzas to look at the table :D12:44
bauzashmmm ?12:44
gibibauzas: we have a new behavior table in https://etherpad.opendev.org/p/unshelve-to-host#L166 for unshelve12:44
gibiit changes around couple of things12:45
Ugglagibi don't want to disturb bauzas during the nap time... :D12:45
gibinap time :D12:45
bauzasnot really napping12:46
bauzasjust sweating here12:46
bauzas28°c in the office room12:46
bauzasUggla: tbc, I hate the AZ hack for create12:47
gibiyesterday was cold enough that I was able to bring down the room temp to 22C here12:47
bauzasUggla: I mean, the -az az_anything:host one12:47
Ugglabauzas, 27°C here as well. 12:48
bauzasUggla: I would have preferred to have a specific --host parameter instead of using the --az one12:48
sean-k-mooneybauzas: you shoudl tell use how you really feel about it :P12:48
gibibauzas: yeah --az is a mess12:48
sean-k-mooneybauzas: yep which we have now12:49
bauzassean-k-mooney: about the weather, you mean ?12:49
bauzasor about the AZ hack ? :D12:49
sean-k-mooneyno i was being sarcatic im well aware of you "love"/hate of --az12:49
sean-k-mooneyi dont disagree with you either its one of the warts in our api12:50
sean-k-mooneyescpailly since they dont even really exist form a data model point of view12:50
sean-k-mooneyeverything about AZs is a bit of a hack12:50
sean-k-mooneyfortunetly palcement  aggreates have learned form that mistake12:51
Ugglabauzas, regarding the hack, you are consistent, I think this is one of the fist thing you told me. Probably in my 2nd day at RH :D12:58
bauzashah12:59
Ugglabauzas, but what do you think of Gibi's behavior proposed table ?13:04
bauzasotp13:04
bauzasover the phone for brian13:04
*** dasm|off is now known as dasm13:21
opendevreviewBalazs Gibizer proposed openstack/nova-specs master: PCI device tracking in Placement  https://review.opendev.org/c/openstack/nova-specs/+/79104713:27
gibisean-k-mooney, melwitt: hopefully the last update. I fixed the small nits and made a decision about pci alias forbidden / required traits ^^13:28
bauzasgibi: I need to review your spec13:39
bauzasjust for my knowledge at least :p13:39
gibisure, go for it13:39
bauzasUggla: /me looks at the etherpad again13:39
bauzasgibi: your spec is open as a tab for a too long time13:40
gibibauzas: you have the benefit now that the content is settled and questions are answered13:40
bauzas:)13:41
bauzasgibi: sean-k-mooney: melwitt: gmann: I captured the agreement on my spec, can we move forward and accept it ? https://review.opendev.org/c/openstack/nova-specs/+/84021713:43
gibibauzas: I let the others pull the trigger on that :)13:43
bauzas:)13:44
sean-k-mooneyi can set +w if ther are no outstanding questions13:50
sean-k-mooneyim stil happy with it as it is so i was just waiting for gibi and melwitt  to review13:50
sean-k-mooneygibi: if your good to proceed ill send it on its way13:51
gibiI accept it13:51
bauzascool, no rush13:51
gibiso go and send it13:51
sean-k-mooneydone13:51
sean-k-mooneygibi: im going to take a look at the pci spec again soon13:52
gibisean-k-mooney: thanks. only the pci alias part changed the rest is just fixing nits13:52
bauzasUggla: gibi: I don't see in https://etherpad.opendev.org/p/unshelve-to-host#L166 what we should do with original_az=None, host=hostB and AZ=something13:52
sean-k-mooneyack13:53
bauzasshould we conflict or verify that the host is within the requested AZ ?13:53
bauzasthis was an open question in the spec review13:53
gibiahh good point I will add a line13:53
gibior you cabn13:53
gibiI think that should be similar to AZ1 -> AZ2 + host1 case13:54
sean-k-mooneybauzas: personally no13:54
sean-k-mooneybauzas: i think we should let the scheduler handel that13:54
bauzassean-k-mooney: gibi: ok I added a line13:55
gibithanks13:55
sean-k-mooneyif its not we will get a no valid host but i was ok with leaving that to the patch review honestly13:55
sean-k-mooneyi dotn really mind checin in the api 13:55
gibiI'm OK to let the schedule reject it 13:55
gibiall these cases13:55
sean-k-mooneybut there are many other factors that could cause it to rejected13:55
sean-k-mooneyso schduler/placment is really the only thing that know if that is ok or not13:56
sean-k-mooneylike isolated_aggrate/tenating isolation filter/host aggreate metadta ectra13:56
bauzasgibi: there could be a corner case tho13:56
gibiyeah, we cloud do a check in the api and make a better error message but that is not strictly needed13:56
bauzasgibi: if we leave the scheduler return NoValidHost,13:57
bauzasthe RequestSpec would have been modified before13:57
gibiohh true13:57
bauzasso we're not idempotent13:57
sean-k-mooneywell we proably should not save it until after schduing13:57
gibiyepp we need to rollback the change or not save it13:57
gibiif that is not possible architecturally then lets do a check in the API13:58
sean-k-mooneywell we will need to cacht the novaild host and rollback in anycase if we do the save before13:58
bauzaswai13:58
bauzaswait13:58
bauzasin order to verify that the host is in an AZ13:58
bauzaswe need to lookup its aggregate, right13:59
sean-k-mooneyyep13:59
bauzasbut,13:59
bauzasthis is cell-specific13:59
sean-k-mooneyno its not13:59
sean-k-mooneyits in the api db13:59
sean-k-mooneythe host aggreate are in teh api db13:59
bauzasoh stupid me, you're right13:59
sean-k-mooneythe instance.az is in the cell db13:59
bauzaswe debated the place of the aggregates table for a while when we designed cells v213:59
sean-k-mooneyi think14:00
bauzassean-k-mooney: yes, instance.az14:00
bauzasreqspec.az is in the API DB14:00
sean-k-mooneyah yes14:00
sean-k-mooneyso we dont need instance.az14:00
bauzasso, there is no cell downcall14:00
sean-k-mooneyyep14:00
bauzasyup, we don't need it14:00
bauzasso, this is cheap to verify it by the api service14:00
bauzasin this case, I'm in favor of doing the lookup at the unshelve time14:01
bauzasin the api service14:01
bauzasand return synchronously a "sorry dude"14:01
bauzasinstead of trying to manage a late persistence or a rollback of the reqspec14:01
Ugglaagree, it sounds simpler.14:02
bauzasand this is synchronous14:03
sean-k-mooneybauzas: ok but we still need to handel rollback of the request spec if we fail to unshleve for any reason14:04
bauzassean-k-mooney: agreed14:04
bauzaswhich wasn't the case now, right?14:05
sean-k-mooneywell i have not looked at he code to check14:05
bauzasI see the .save() call, but I don't see the conductor managing the exception14:05
sean-k-mooneydo you have a link14:05
bauzasyup, sec14:05
bauzashttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L447014:06
sean-k-mooneyi guess we can also just document that the request spec will always be updted14:06
bauzassec, better with a permalink 14:06
bauzashttps://github.com/openstack/nova/blob/4939318649650b60dd07d161b80909e70d0e093e/nova/compute/api.py#L447014:06
sean-k-mooneyi just generally effect failed operation to not change things14:06
sean-k-mooneyya so right now it changing it uncondtionaly14:07
sean-k-mooneyi guess that fine14:07
sean-k-mooneyits also doing the validation now14:07
bauzashttps://github.com/openstack/nova/blob/4939318649650b60dd07d161b80909e70d0e093e/nova/conductor/manager.py#L1040-L104514:07
sean-k-mooneyhttps://github.com/openstack/nova/blob/4939318649650b60dd07d161b80909e70d0e093e/nova/compute/api.py#L4454=14:07
sean-k-mooneyin the api14:07
bauzasthe conductor is not handling the AZ case on the NoValidHost case14:07
sean-k-mooneyya14:07
bauzasso, now, if you unshelve to another AZ, and you fail, your instance gets pinned in the new AZ 14:07
sean-k-mooneyyep14:08
bauzasstill shelved but pinned14:08
sean-k-mooneyso we either maintain that behavior 14:08
sean-k-mooneyor we treat it as a bug14:08
sean-k-mooneyand fix that spereatly14:08
bauzasgood point14:08
bauzasthis is a bug14:08
sean-k-mooneyok then we dont need to cover it in the spec14:09
bauzasfixing it now would benefit to the unshelve to host implementation14:09
bauzasbut wait14:09
bauzasthe rollback isn't trivial14:09
bauzaswhat field should we set if we rollback ?14:09
opendevreviewMerged openstack/nova-specs master: Proposes to remove keypair generation  https://review.opendev.org/c/openstack/nova-specs/+/84021714:09
bauzasI mean, what value for reqspec.az to restore ?14:09
bauzasthanks whoever it was ^14:10
bauzasanyway, I'm bikeshedding14:11
bauzasthis is unrelated to the spedc14:11
bauzaswe won't fix all the first-class problems of the world with this spec14:11
sean-k-mooneyaw14:12
sean-k-mooneybut what about world hunger14:12
bauzas(technically, Placement could resolve the World-class starvation issue we have)14:12
sean-k-mooneysurely that is in scope right14:12
bauzasheh, placement can help14:12
bauzasatm, this is just some western countries who use specific required traits14:13
bauzasand gather all the resources14:13
gibiso a failed unshelve can be retried so I'm not worrying about persisting the wrong data, as that can be overwritten by the next unshelve trial14:13
sean-k-mooneyhehe Uggla for context the example demo of placment was makign a sandwich and tracking inventories of food in your fridge14:13
bauzassean-k-mooney: I showcased him during our knowledge transfert :)14:14
bauzasI used cdent's slides about ham and leafs14:14
sean-k-mooneygibi: ya it can i dont think its a big issue 14:14
bauzasgibi: you make a valid point14:14
bauzaswe should open a bug report for not forgeting it14:14
gibiack14:15
bauzasthe resolution isn't trivial 14:15
bauzasbut at least we can assess in the report that a workaround is to ask for another unshelve14:15
bauzasdamn, I need to taxi again my kid14:15
Ugglabauzas sold me that unshelve to host would be an easy trivial change, self contained to start learning.... ;)14:36
gibiUggla: at least the original use case was clear and agreed. We just made sure that all the possible state transitions are covered 14:39
gibiand that took a bit of time14:39
gibibut this is still fairly simple14:40
Ugglagibi, yes I'm just kidding at bauzas. ;)14:42
gibi:)14:43
opendevreviewJohn Garbutt proposed openstack/nova master: Ironic: retry when node not available  https://review.opendev.org/c/openstack/nova/+/84247814:48
opendevreviewMerged openstack/nova master: neutron: Unbind remaining ports after PortNotFound  https://review.opendev.org/c/openstack/nova/+/84252815:49
opendevreviewStephen Finucane proposed openstack/nova stable/yoga: neutron: Unbind remaining ports after PortNotFound  https://review.opendev.org/c/openstack/nova/+/84258417:48
opendevreviewStephen Finucane proposed openstack/nova stable/xena: neutron: Unbind remaining ports after PortNotFound  https://review.opendev.org/c/openstack/nova/+/84258617:53
opendevreviewStephen Finucane proposed openstack/nova stable/wallaby: neutron: Unbind remaining ports after PortNotFound  https://review.opendev.org/c/openstack/nova/+/84258717:57
opendevreviewStephen Finucane proposed openstack/nova stable/victoria: neutron: Unbind remaining ports after PortNotFound  https://review.opendev.org/c/openstack/nova/+/84258817:57
opendevreviewStephen Finucane proposed openstack/nova stable/victoria: neutron: Unbind remaining ports after PortNotFound  https://review.opendev.org/c/openstack/nova/+/84258817:58
opendevreviewStephen Finucane proposed openstack/nova stable/ussuri: neutron: Unbind remaining ports after PortNotFound  https://review.opendev.org/c/openstack/nova/+/84259017:59
opendevreviewStephen Finucane proposed openstack/nova stable/train: neutron: Unbind remaining ports after PortNotFound  https://review.opendev.org/c/openstack/nova/+/84259218:03
*** dasm is now known as dasm|off21:26
opendevreviewGhanshyam proposed openstack/nova master: DNM: testing py38|9 fips on c8|9s  https://review.opendev.org/c/openstack/nova/+/84264022:30
opendevreviewGhanshyam proposed openstack/nova master: DNM: testing py38|9 fips on c8|9s  https://review.opendev.org/c/openstack/nova/+/84264022:36

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