opendevreview | Brin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from Flavor Access APIs https://review.opendev.org/c/openstack/nova/+/767704 | 06:06 |
---|---|---|
*** akekane_ is now known as abhishekk | 06:46 | |
opendevreview | Brin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from List/Show usage APIs https://review.opendev.org/c/openstack/nova/+/768509 | 06:46 |
opendevreview | Brin Zhang proposed openstack/nova master: Replace tenants* with projects* of policies https://review.opendev.org/c/openstack/nova/+/765315 | 06:46 |
opendevreview | Brin Zhang proposed openstack/nova master: Replace os-simple-tenant-usage with os-simple-project-usage https://review.opendev.org/c/openstack/nova/+/842288 | 06:46 |
opendevreview | Brin Zhang proposed openstack/nova master: Replace tenant_id with project_id in os-quota-sets path https://review.opendev.org/c/openstack/nova/+/768851 | 06:46 |
opendevreview | Brin Zhang proposed openstack/nova master: Replace tenant_id with project_id in Limits API https://review.opendev.org/c/openstack/nova/+/768862 | 06:46 |
opendevreview | Brin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from Flavor Access APIs https://review.opendev.org/c/openstack/nova/+/767704 | 07:51 |
opendevreview | Brin Zhang proposed openstack/nova master: Replaces tenant_id with project_id from List/Show usage APIs https://review.opendev.org/c/openstack/nova/+/768509 | 07:52 |
seyeongkim | hello, 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 |
Uggla | gibi, hello if you can have a look at https://review.opendev.org/c/openstack/nova-specs/+/831506 | 08:23 |
gibi | I'm looking at the manial one right now but then I can look the unshelve too | 08:25 |
Uggla | gibi, oh cool as well thx. | 08:32 |
bauzas | Uggla: IMHO we shouldn't add a specific parameter for asking to pin an AZ | 08:36 |
bauzas | if the user asked for a specific AZ, it's pinned | 08:37 |
bauzas | so if the operator wants to move the instance to another AZ, OK, but then it should be pinned again | 08:37 |
bauzas | if the user didn't ask for a specific AZ, then it's not pinned | 08:38 |
bauzas | so, if the operator moves the instance to another AZ, then meh | 08:38 |
Uggla | bauzas, sure but if he unpins an instance he needs to recreate it to pin it ? | 08:38 |
bauzas | Uggla: who "he" ? | 08:38 |
bauzas | the admin ? | 08:38 |
bauzas | or the user ? | 08:38 |
Uggla | bauzas, user | 08:39 |
bauzas | but the user only creates the instance | 08:39 |
bauzas | he doesn't have a "pinned" parameter | 08:39 |
bauzas | he doesn't know about the environment | 08:39 |
bauzas | but what he knows is that the environment has multiple AZs and he just wants his instance to be in one of them | 08:40 |
bauzas | then, he'll create the instance by asking --az AZ1 | 08:40 |
bauzas | which will be pinning the AZ | 08:40 |
Uggla | bauzas, yep, then he can shelve/unshelve with AZ=None to unpin it | 08:41 |
bauzas | then, 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 moved | 08:41 |
Uggla | bauzas, why operator ? user can do it as well. Am I wrong ? | 08:42 |
bauzas | Uggla: 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 AZ | 08:42 |
bauzas | Uggla: no, because endusers can't unshelve to an AZ, right? | 08:43 |
bauzas | you need to be admin | 08:43 |
Uggla | bauzas, user can unshelve to an AZ not to a host | 08:43 |
bauzas | oh my bad then | 08:43 |
bauzas | Uggla: 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 AZ | 08:44 |
bauzas | and if you unshelve az=AZ2, that exactly means like create with --az AZ2 | 08:44 |
Uggla | bauzas, agree but there is no way to pin the instance to an az without recreating it. | 08:46 |
bauzas | Uggla: or shelving again | 08:47 |
bauzas | right? | 08:47 |
Uggla | bauzas, no due to new behavior | 08:49 |
bauzas | Uggla: if an instance was unshelved with az=None, the ReqSpec.az field will be None, right? | 08:49 |
bauzas | oh I see | 08:50 |
Uggla | yep but not way to set it again to az=foo. | 08:50 |
Uggla | No AZ | AZ but, no host | **Schedule in the AZ, keep the | | 08:50 |
Uggla | | | | reqspec.AZ as None** | 08:50 |
bauzas | then if you pass shelve/unshelve with az=something, it will move to this new AZ without pinning it | 08:50 |
bauzas | I see | 08:50 |
Uggla | bauzas, yes | 08:50 |
bauzas | well, I'd say it's a hard choice to make | 08:51 |
bauzas | but the point is, if an instance is unpinned, you have no way to pin it | 08:51 |
bauzas | problem is, the existing behaviour pins it even if you don't want | 08:52 |
bauzas | Uggla: honestly, this is debatable | 08:52 |
Uggla | bauzas, yes it was raised by sean as a question, but playing with devstack I realized that it could be non conveniant. | 08:52 |
bauzas | Uggla: do we agree on the fact that you explicitely need to set az:none in order to unpin the instance, right? | 08:53 |
Uggla | bauzas, yep sure | 08:53 |
bauzas | this can't be done without the caller explicitely opting into it | 08:53 |
bauzas | you can unshelve without any parameter | 08:54 |
bauzas | which will leave the instance in the same AZ | 08:54 |
bauzas | you can now unshelve to a host | 08:54 |
Uggla | bauzas, yep | 08:54 |
bauzas | which will leave this instance to this AZ | 08:54 |
bauzas | (be pinned) | 08:54 |
bauzas | you can unshelve to another AZ | 08:55 |
bauzas | which will move this instance to the new wanted AZ, still pinned tho | 08:55 |
Uggla | bauzas, if it is pinned at boot time it will remain pinned | 08:55 |
bauzas | so, frankly, the caller exactly has to call "I want my instance to be unpinned" | 08:55 |
Uggla | bauzas, yep absolutely, but if you do that it will remain unpinned. With no way to pin it again without recreating it. | 08:56 |
Uggla | bauzas, 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 |
bauzas | then you probably have right | 08:58 |
bauzas | s/have/are | 08:59 |
bauzas | I see your honest concern, people would want to tell "I want my instance be stuck somewhere now" | 08:59 |
Uggla | bauzas, by the way I have no real interest to change as my code is working... :) | 08:59 |
bauzas | problem is, I don't know which exact semantics to write | 09:00 |
bauzas | Uggla: man, you litterally just did put your left food in some poop, you know | 09:00 |
bauzas | this is all your honot | 09:00 |
bauzas | honor | 09:00 |
bauzas | but this is the joy of review times | 09:01 |
bauzas | either reviewers or even you think about something new you haven't written, and then you put your code in the trash | 09:01 |
Uggla | bauzas, 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 |
bauzas | I literally see this coming, indeed. | 09:03 |
bauzas | the problem is that 'AZ pinning' is something we don't explicitely mention | 09:03 |
Uggla | bauzas, btw it was the case with the old microversion. | 09:03 |
bauzas | this is more a tribal knowledge | 09:04 |
bauzas | and a lot of people are confused by the multiple config options and parameters we have | 09:04 |
Uggla | bauzas, 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 |
Uggla | bauzas, 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 |
gibi | I've just read the scrollback about unshelve | 09:13 |
bauzas | Uggla: 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 meaningless | 09:13 |
gibi | do we have a way forward or we are still in the open? | 09:13 |
bauzas | gibi: I think we're basically arriving to a consensus | 09:14 |
gibi | which is? :) | 09:14 |
bauzas | but there is one left question about the unpinned AZ | 09:14 |
gibi | do we need to consider the value of schedule_default_az during unshelve if the AZ param is not provided? | 09:15 |
* gibi hides away | 09:15 | |
bauzas | gibi: 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 it | 09:15 |
bauzas | gibi: 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 |
bauzas | they have different value per nova-api service | 09:16 |
gibi | personally I don't want to have schedule_default_az in the picture | 09:16 |
gibi | as it is extra complication | 09:17 |
bauzas | gibi: don't disagree with this | 09:17 |
gibi | no I did not know that there are people out there abusing schedule_default_az | 09:17 |
bauzas | problem is, unshelve to AZ is end-user API | 09:17 |
bauzas | and that's a problem | 09:17 |
gibi | AZ is an end user thing | 09:17 |
gibi | during boot too | 09:17 |
gibi | (excpet for schedule_default_az ;) ) | 09:17 |
bauzas | yes I know | 09:18 |
bauzas | that ^ | 09:18 |
gibi | sure | 09:18 |
bauzas | we're mixing wishes of the enduser with operator toughts | 09:18 |
gibi | yepp | 09:18 |
* Uggla out 10mn | 09:18 | |
gibi | and most of the time operator > end user | 09:18 |
gibi | except for schedule_default_az | 09:18 |
bauzas | but this is clear that user choice supersedes the schedule_default_az thing | 09:18 |
gibi | as that is just a default that can be overridden by the end user | 09:19 |
bauzas | so this is bikeshed | 09:19 |
bauzas | yeah | 09:19 |
bauzas | let's put schedule_default_az out of the picture :) | 09:19 |
* bauzas also has his kid to drive back from school | 09:19 | |
gibi | pretty please document this decision that schedule_default_az will not be used during unshelve | 09:20 |
gibi | we seriously need to beef up our doc around this | 09:20 |
gibi | to have something to point at when bugs appeare | 09:20 |
bauzas | possibly | 09:21 |
bauzas | that's what I said, AZs are tribal knowledge atm | 09:21 |
* bauzas drops | 09:21 | |
gibi | Uggla: 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 |
Uggla | gibi, 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 |
gibi | ahh so that would be a new API param | 09:36 |
Uggla | gibi, yes sorry if it is not clear. | 09:36 |
gibi | and above bauzas suggested that we don't add that, isn't it? | 09:36 |
gibi | no worries | 09:36 |
Uggla | gibi, yes | 09:36 |
gibi | ack | 09:36 |
gibi | I will try to summarise my current understanding about the agreement from above in the review | 09:37 |
Uggla | gibi, but bauzas said that before the discussion about pin/unpin of AZ. | 09:37 |
gibi | /o\ ETOOCOMPLEX | 09:37 |
gibi | anyhow | 09:38 |
gibi | I will try to write something in the review :D | 09:38 |
gibi | considering all the discussion happened before | 09:38 |
Uggla | gibi, hum yes I think that as well writting it. :) | 09:38 |
Uggla | gibi, but take care about the client issue as well. | 09:39 |
gibi | do you mean the CLI issue? | 09:39 |
Uggla | gibi, yes I introduced a param unpin-az to avoid issues with --availability-zone None --> None treated as a string "None". | 09:40 |
gibi | I assume if we come up with an API semantic that is accepted then will be able to map that to a meaningful CLI some way | 09:41 |
Uggla | gibi, so client as 3 param --availability-zone, --host, --unpin-az --> API only 2 --availability-zone and host. | 09:42 |
gibi | and in the CLI we can have extra params like unpin-az if we need it | 09:42 |
gibi | yep, CLI is easy :D | 09:42 |
Uggla | gibi, yes but it could be tricky without the mutually exclusive stuff. | 09:43 |
Uggla | gibi, ex on the cli user could request --availability-zone nova --unpin-az --> with is not possible. | 09:43 |
gibi | yepp, that we can catch that on the client side | 09:44 |
Uggla | s/with/which/ | 09:44 |
gibi | --availability-zone and --unpin-az is mutually exlisive on the client side | 09:44 |
Uggla | gibi, today all params are mutually exclusive. So no pb. | 09:45 |
Uggla | gibi, and this is the case on the CLI and API. | 09:46 |
Uggla | gibi, btw so we ended up to not support pin of an az via unshelve ? | 09:48 |
gibi | that is how I read bauzas above | 09:49 |
Uggla | gibi, to be honest it is unclear to me. :) | 09:50 |
gibi | but I think dansmith had other opinion | 09: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 |
Uggla | gibi, should we try to put that point to today meeting agenda ? | 09:52 |
gibi | if you can ask the proper questions, then yes we can try | 09:53 |
opendevreview | Brin Zhang proposed openstack/nova master: Replace os-simple-tenant-usage with os-simple-project-usage https://review.opendev.org/c/openstack/nova/+/842476 | 09:54 |
Uggla | gibi, I put the question in the agenda. If you want to have look. | 10:10 |
opendevreview | John Garbutt proposed openstack/nova master: Ironic: retry when node not available https://review.opendev.org/c/openstack/nova/+/842478 | 10:14 |
gibi | bauzas: Uggla: left a comment to the unshelve spec pointing to an extended table in the etherpad | 10:23 |
sean-k-mooney | gibi: looks like the resize patch was hit by infra issue again | 10:48 |
sean-k-mooney | im seeing some mysql failures | 10:48 |
gibi | something is going on I saw mysql issues on other tempest patches this week | 10:56 |
sean-k-mooney | gibi: i think dan would prefer if the value provided for the az paramater is unconditionally used to update the request spec | 10:58 |
sean-k-mooney | so if its not provide we dont update | 10:58 |
gibi | yes I think so | 10:58 |
sean-k-mooney | if you provide a sentenal proably {} we woudl set it to python None | 10:59 |
sean-k-mooney | any string value jsut gets set in the request spec | 10:59 |
sean-k-mooney | actully we could use null | 10:59 |
sean-k-mooney | unqutated as in the json value null for python None that is the write sentenial | 11:00 |
sean-k-mooney | im ok with taht as it provides a way to pin and unpine and its simple to reason about | 11:00 |
sean-k-mooney | that just meas we get rid of the idea of az and host being mutally exclucive | 11:01 |
gibi | sean-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 suggested | 11:02 |
sean-k-mooney | so az unset means use current requstspec value, az=null means update to python None az="any string" means update to that stirng | 11:02 |
sean-k-mooney | i have it open but have not read it yet was reading the scrollback here | 11:02 |
gibi | sean-k-mooney: yepp | 11:03 |
sean-k-mooney | so the only delta is im being explict about types | 11:03 |
sean-k-mooney | so we are not reserving the string None to by python None | 11:03 |
sean-k-mooney | we are using the json value null | 11:04 |
gibi | also the previous proposal did not pin when no AZ (boot) -> AZ (unshelve) was requested | 11:04 |
gibi | but the new one pins | 11:04 |
gibi | as per dansmith's point | 11:04 |
sean-k-mooney | why is no unpinning allow if the host is requested | 11:05 |
sean-k-mooney | would it not be simplet to jsut entirly decoule the two paramters | 11:06 |
gibi | sean-k-mooney: do you mean L182? | 11:06 |
gibi | L128 | 11:06 |
sean-k-mooney | yes | 11:06 |
gibi | I think it can lead to contradiction | 11:07 |
sean-k-mooney | i dont think so | 11:07 |
gibi | no AZ host1 -> pin to the AZ of host1 | 11:07 |
gibi | but AZ=None host1 -> not pin due to AZ=None or pin due to host1? | 11:08 |
sean-k-mooney | no your miss understanding what i was suggesting | 11:08 |
sean-k-mooney | i was suggesting there is no implict change to az by passign host | 11:08 |
sean-k-mooney | so if the host is in a differnt az it will fail | 11:08 |
sean-k-mooney | if you dont care about the az but just want the host then | 11:09 |
sean-k-mooney | az=null host=host-1 | 11:09 |
gibi | so you would change both L127 and 128 but keep L130? | 11:09 |
kashyap | Zuul 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-mooney | gibi: yes i think so | 11:10 |
sean-k-mooney | gibi: if the aim is to simplfy i think we shoudl remove all the magic | 11:10 |
sean-k-mooney | so there is no implict updates of the az | 11:11 |
gibi | the question is then what happens today if the instance is booted with --availability-zone :host1 (so no AZ but host provided) | 11:11 |
gibi | do we pin today to the AZ of host1? if not then I agree with you | 11:11 |
sean-k-mooney | gibi: i dont think that si vaild | 11:11 |
gibi | let me try | 11:11 |
sean-k-mooney | i think to use the host via the az an az is required | 11:11 |
opendevreview | Stephen Finucane proposed openstack/nova master: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842528 | 11:19 |
opendevreview | Stephen Finucane proposed openstack/nova master: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842528 | 11:20 |
gibi | sean-k-mooney: "--availability-zone :gibi-devstack-aio" is allowed and it forces the host | 11:20 |
sean-k-mooney | that is not documented anywhere that im aware of | 11:20 |
sean-k-mooney | i would have filed a bug for that | 11:21 |
gibi | but does not set the AZ in the request spec | 11:21 |
sean-k-mooney | so your saying this feature is alreay supported | 11:21 |
gibi | we can force the host without pinning to the AZ of the host | 11:21 |
sean-k-mooney | well we dont allow host for unshleve in the az | 11:21 |
sean-k-mooney | gibi: well you can force and pin too | 11:22 |
gibi | yepp | 11:22 |
sean-k-mooney | by passing the correct az | 11:22 |
stephenfin | sean-k-mooney: gibi: That's a dead simple nova/neutron patch for whenever you've time ^ | 11:22 |
gibi | stephenfin: ack | 11:22 |
sean-k-mooney | ack | 11:22 |
gibi | sean-k-mooney: so based on this I think it is OK to decouple the host from AZ in unshelve too | 11:23 |
sean-k-mooney | that reminds me we dont currently unbind port when we shelve offload. we really shoudl fix that | 11:23 |
sean-k-mooney | i summareised my toughs "again" on line 139+ | 11:23 |
gibi | sean-k-mooney: just not blinding unbind but keep the device_id set and check with neutron that this keeps the port reserved | 11:24 |
gibi | sean-k-mooney: thanks | 11:24 |
sean-k-mooney | Uggla: rememebr when we said this was a simple non contoversial spec | 11:24 |
sean-k-mooney | gibi: unbinding never touches the device_id | 11:24 |
sean-k-mooney | that woudl be a detach | 11:24 |
sean-k-mooney | unbinding is just resetting the binding:host-id | 11:25 |
sean-k-mooney | and possible you could clear the port_profile bits | 11:25 |
sean-k-mooney | but ya we shoudl keep device_id set when unbinding for shelve_offload | 11:25 |
sean-k-mooney | to keep the port owned by the vm | 11:26 |
sean-k-mooney | stephenfin: i was going to ask why the continue but i sse wew try and use the port profile later | 11:27 |
sean-k-mooney | ya this looks correct to me. | 11:27 |
stephenfin | Yeah. 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 |
stephenfin | Unrelated, but if anyone hasn't gotten an ultrawide monitor yet, you need to get on that | 11:28 |
stephenfin | <3 | 11:28 |
sean-k-mooney | well the second excpetion handeler | 11:29 |
sean-k-mooney | is only invoked if we do find the port | 11:29 |
sean-k-mooney | but there is a differnt excption | 11: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-mooney | but i guess that still means we dont have the port object | 11:29 |
sean-k-mooney | so ya it likely is not correct | 11:30 |
kashyap | stephenfin: What is the model that you have? | 11:30 |
kashyap | (Monitor model, i.e.) | 11:30 |
stephenfin | HP M34d | 11:30 |
sean-k-mooney | stephenfin: you have seeen the one on my desk | 11:30 |
stephenfin | 34" ultrawide, 3440x1440 resolution, USB C Power Delivery (which is A+) | 11:31 |
sean-k-mooney | mine is similar 34" 21:9 | 11:31 |
sean-k-mooney | 3440x1440@100hz | 11:31 |
stephenfin | Only 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 screen | 11:31 |
sean-k-mooney | predates usbc however | 11:31 |
stephenfin | sean-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 regardless | 11:32 |
stephenfin | I'm not gaming so it doesn't matter much anyway | 11:32 |
sean-k-mooney | i have been debating between going to 2 ultrawids or 1 large 4k display for a while but didnt find a cheep ultrawide | 11:32 |
sean-k-mooney | hum also curved | 11:34 |
stephenfin | This was a 479€ which seemed reasonable for what it had. I could justify more than doubling that for a 49" | 11:34 |
sean-k-mooney | that is the one think i didnt want on this | 11:34 |
stephenfin | Also, you'd *have* to step up to Thunderbolt for that | 11:35 |
stephenfin | Don't think even USB 3.2 has the bandwidth for that | 11:35 |
sean-k-mooney | am maybe with display stream compression | 11:35 |
stephenfin | (i.e. 5120 x 1440 resolution) | 11:35 |
sean-k-mooney | but it depsn on the resolution | 11:35 |
sean-k-mooney | ya 4k ultrawid is nice | 11:35 |
sean-k-mooney | that is what i was loking at | 11:35 |
sean-k-mooney | but $$$ | 11:36 |
sean-k-mooney | i 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 irc | 11:37 |
sean-k-mooney | right now my laptop (left of ultra wide) is my email screen and portait monitor (right of ultra wide) is irc | 11:38 |
sean-k-mooney | im just worried they might be two high | 11:38 |
gibi | sean-k-mooney: see the new table at https://etherpad.opendev.org/p/unshelve-to-host#L166 | 11:47 |
gibi | is this what you are after? | 11:47 |
sean-k-mooney | kind of i woudl prefer if we use null and queted strings to be explcit still reading it | 11:52 |
gibi | when I say "no AZ" it means the availability_zone field is not in the request body | 11:52 |
sean-k-mooney | yep i know | 11:53 |
sean-k-mooney | i was talkign about AZ=None | 11:53 |
gibi | so that is not the same as availability_zone = null | 11:53 |
gibi | ahh | 11:53 |
gibi | OK | 11:53 |
sean-k-mooney | that is ambiquious | 11:53 |
gibi | let me fix that | 11:53 |
sean-k-mooney | that could mean AZ="None" or AZ=null | 11:53 |
sean-k-mooney | cool | 11:54 |
gibi | done | 11:54 |
gibi | yepp thanks for the quotes | 11:55 |
sean-k-mooney | so yes that is what i was thinking | 11:56 |
gibi | cool | 11:56 |
sean-k-mooney | i was also thinking of just leaving the host->az check to the schduler | 11:56 |
sean-k-mooney | we could do it in the api if we want too | 11:56 |
gibi | Uggla, bauzas, dansmith: please check the table in https://etherpad.opendev.org/p/unshelve-to-host#L166 at least Sean and I agree on that now | 11:56 |
sean-k-mooney | but we could jsut convert the no valid host to a better error | 11:56 |
gibi | yeah, good point | 11:57 |
gibi | that will be NoValid host if we not check in the api | 11:57 |
gibi | but that is fine | 11:57 |
sean-k-mooney | im going to get coffee quickly brb | 11:58 |
gibi | sean-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 offload | 12:00 |
sean-k-mooney | gibi: that is not an unbind then | 12:05 |
gibi | the function is called unbind :) | 12:05 |
sean-k-mooney | at least not at the nutron api level | 12:06 |
sean-k-mooney | right its miss named | 12:06 |
sean-k-mooney | that is unbind_and_detach | 12:06 |
gibi | https://review.opendev.org/c/openstack/nova/+/842528/2/nova/network/neutron.py#616 | 12:06 |
sean-k-mooney | ya that function should be renamed | 12:06 |
sean-k-mooney | that is actully a detach | 12:06 |
sean-k-mooney | its a detach and clearing of dns_name | 12:07 |
sean-k-mooney | actully does this even do an unbind | 12:08 |
sean-k-mooney | ah i t does line644 | 12:08 |
sean-k-mooney | constants.BINDING_HOST_ID: None, | 12:09 |
sean-k-mooney | is the actual unbind | 12:09 |
sean-k-mooney | gibi: this is being used as part of instance delete which is why its doing all the other stuff | 12:10 |
gibi | yeah, this is correct during delete but it would not be corrrect during offload | 12:10 |
sean-k-mooney | its just combining the unbind with teh other operations needed to reset the port | 12:10 |
sean-k-mooney | right but that function is not only doing an unbind | 12:11 |
sean-k-mooney | i never said we shoudl use that funciton | 12:11 |
gibi | sure | 12:12 |
gibi | it is about terminology | 12:12 |
sean-k-mooney | we shoudl proably rename that "release_ports" or "reset_ports" | 12:12 |
gibi | yeah I agree | 12:13 |
sean-k-mooney | right terminology wise that function is missnamed | 12:13 |
sean-k-mooney | if i propose a patch for unshelve | 12:13 |
sean-k-mooney | ill porpsoe a clean up patch to rename this | 12:13 |
gibi | ack | 12:14 |
sean-k-mooney | speaking of patchs my spawn_n patch actully passed ci this time | 12:15 |
sean-k-mooney | well ignoreing pep8 | 12:15 |
sean-k-mooney | but it did not seam to really decrease the number of greenlets | 12:15 |
sean-k-mooney | i guess becasue when we are counting them | 12:15 |
sean-k-mooney | we count the wrapped greenlets | 12:15 |
sean-k-mooney | and the freestanding ones | 12:15 |
sean-k-mooney | so as you predicted it has no visabel effect | 12:16 |
gibi | the code should only count naked greenlets that are not wrapped | 12:18 |
gibi | in a GreenThred | 12:19 |
sean-k-mooney | hum well the number looked identical | 12:19 |
gibi | https://review.opendev.org/c/openstack/nova/+/841040/6/nova/compute/manager.py#10150 | 12:19 |
sean-k-mooney | its possible it did not work i coudl add a func test to check | 12:19 |
gibi | greenlets might be special from the GC perspective as they are implemented in a C extension | 12:19 |
gibi | so this can be a side effect of my hack to looking at the GC | 12:20 |
sean-k-mooney | maybe | 12:20 |
sean-k-mooney | not really sure | 12:20 |
gibi | me neither | 12:20 |
sean-k-mooney | will isinstance look at inheritance | 12:21 |
gibi | yepp | 12:21 |
gibi | it does | 12:21 |
sean-k-mooney | and GreenThread contains but is not a greenlet | 12:21 |
Uggla | gibi, 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-mooney | gibi: https://github.com/eventlet/eventlet/blob/master/eventlet/greenthread.py#L163= | 12:22 |
Uggla | gibi, although I think it is a better choice. | 12:22 |
gibi | GreenThread derives from greenlet https://github.com/eventlet/eventlet/blob/master/eventlet/greenthread.py#L163 | 12:23 |
gibi | sean-k-mooney: you were faster | 12:23 |
sean-k-mooney | gibi: yep so greenlet includes all the greenthread too | 12:23 |
gibi | so isinstance(GreenThread(), greenlet) is True | 12:23 |
sean-k-mooney | ya | 12:23 |
gibi | the count does not | 12:23 |
gibi | as I have an if | 12:23 |
gibi | naked = [g for g in greenlets if g not in greenthreads] | 12:24 |
sean-k-mooney | oh ya | 12:24 |
gibi | so only count those greenlets that are not counted for greenthread | 12:24 |
gibi | s | 12:24 |
sean-k-mooney | did see that look was lookign jsut at greenlets and ghreenthreads | 12:24 |
gibi | ah jeah I forgot to log _naked_ greenlet | 12:25 |
gibi | hm | 12:25 |
gibi | I idd | 12:25 |
gibi | did | 12:25 |
sean-k-mooney | you jsut didnt log total | 12:25 |
gibi | so naked here means not wrapped in a GreenThread | 12:25 |
gibi | yeah I did not log total as that would count things twice | 12:25 |
gibi | i.e. a GreenThread would also be counted as greenlet too | 12:26 |
sean-k-mooney | Uggla: no we are redefiening what the qeust means | 12:26 |
sean-k-mooney | we are defien that if you pass an AZ you are requesting for it to be pinned to that az | 12:26 |
sean-k-mooney | which is the current unshelve to az beahvior | 12:26 |
gibi | yepp | 12:27 |
Uggla | sean-k-mooney, gibi , I'have just got it. Seems ok for me. | 12:27 |
sean-k-mooney | Uggla: so we found that odd in the previous case beacue of the implict magic of updating az somethimes but nota always | 12:27 |
sean-k-mooney | but now that its only ever updated explcitly in this table | 12:28 |
sean-k-mooney | its consistent | 12:28 |
Uggla | sean-k-mooney, I agree it is better now and unpin/pin is possible which is cool. | 12:28 |
sean-k-mooney | Uggla: for better or worse this is why we have specs for api changes | 12:30 |
sean-k-mooney | Uggla: something that seams simple like "just unshalve to this host" can have subtel issues with considtency | 12:30 |
sean-k-mooney | so hopefully this has not put you off specs | 12:30 |
Uggla | sean-k-mooney, completely understand, I was worried by the fact that unpin was not possible. Now the spec is better. | 12:31 |
sean-k-mooney | gibi: ill proably respin my monkey_patch change and add a release note then bring it up in the next team meeting | 12:31 |
sean-k-mooney | gibi: basiclly so we want to proceed with it to get operator feedback or abandon it | 12:32 |
sean-k-mooney | defaulting to False of cource so its opt in not opt out | 12:32 |
Uggla | sean-k-mooney, I have also learn to take time to mature the spec before doing the code.... ;) | 12:33 |
sean-k-mooney | hehe 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 cases | 12:34 |
sean-k-mooney | often it can be helpfull to get something mostly working but defer the docs and test code till after the spec is approved | 12:35 |
sean-k-mooney | but only for small changes | 12:35 |
Uggla | sean-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 |
Uggla | gibi, fyi I have removed the point for today's call. | 12:38 |
Uggla | sean-k-mooney, gibi I'm gonna update the spec with these new data. If you are ok ? | 12:39 |
sean-k-mooney | for spec design stuff we try to keep as much of it public as we can anyway | 12:39 |
sean-k-mooney | Uggla: am sure you could | 12:40 |
sean-k-mooney | Uggla: 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 irc | 12:41 |
Uggla | sean-k-mooney, ok | 12:42 |
sean-k-mooney | basically 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 anyway | 12:42 |
gibi | sean-k-mooney: re spawn_n, OK, cool, maybe we can add mnaser to that patch | 12:43 |
gibi | Uggla: maybe before you update the spec ping bauzas to look at the table :D | 12:44 |
bauzas | hmmm ? | 12:44 |
gibi | bauzas: we have a new behavior table in https://etherpad.opendev.org/p/unshelve-to-host#L166 for unshelve | 12:44 |
gibi | it changes around couple of things | 12:45 |
Uggla | gibi don't want to disturb bauzas during the nap time... :D | 12:45 |
gibi | nap time :D | 12:45 |
bauzas | not really napping | 12:46 |
bauzas | just sweating here | 12:46 |
bauzas | 28°c in the office room | 12:46 |
bauzas | Uggla: tbc, I hate the AZ hack for create | 12:47 |
gibi | yesterday was cold enough that I was able to bring down the room temp to 22C here | 12:47 |
bauzas | Uggla: I mean, the -az az_anything:host one | 12:47 |
Uggla | bauzas, 27°C here as well. | 12:48 |
bauzas | Uggla: I would have preferred to have a specific --host parameter instead of using the --az one | 12:48 |
sean-k-mooney | bauzas: you shoudl tell use how you really feel about it :P | 12:48 |
gibi | bauzas: yeah --az is a mess | 12:48 |
sean-k-mooney | bauzas: yep which we have now | 12:49 |
bauzas | sean-k-mooney: about the weather, you mean ? | 12:49 |
bauzas | or about the AZ hack ? :D | 12:49 |
sean-k-mooney | no i was being sarcatic im well aware of you "love"/hate of --az | 12:49 |
sean-k-mooney | i dont disagree with you either its one of the warts in our api | 12:50 |
sean-k-mooney | escpailly since they dont even really exist form a data model point of view | 12:50 |
sean-k-mooney | everything about AZs is a bit of a hack | 12:50 |
sean-k-mooney | fortunetly palcement aggreates have learned form that mistake | 12:51 |
Uggla | bauzas, 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 :D | 12:58 |
bauzas | hah | 12:59 |
Uggla | bauzas, but what do you think of Gibi's behavior proposed table ? | 13:04 |
bauzas | otp | 13:04 |
bauzas | over the phone for brian | 13:04 |
*** dasm|off is now known as dasm | 13:21 | |
opendevreview | Balazs Gibizer proposed openstack/nova-specs master: PCI device tracking in Placement https://review.opendev.org/c/openstack/nova-specs/+/791047 | 13:27 |
gibi | sean-k-mooney, melwitt: hopefully the last update. I fixed the small nits and made a decision about pci alias forbidden / required traits ^^ | 13:28 |
bauzas | gibi: I need to review your spec | 13:39 |
bauzas | just for my knowledge at least :p | 13:39 |
gibi | sure, go for it | 13:39 |
bauzas | Uggla: /me looks at the etherpad again | 13:39 |
bauzas | gibi: your spec is open as a tab for a too long time | 13:40 |
gibi | bauzas: you have the benefit now that the content is settled and questions are answered | 13:40 |
bauzas | :) | 13:41 |
bauzas | gibi: 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/+/840217 | 13:43 |
gibi | bauzas: I let the others pull the trigger on that :) | 13:43 |
bauzas | :) | 13:44 |
sean-k-mooney | i can set +w if ther are no outstanding questions | 13:50 |
sean-k-mooney | im stil happy with it as it is so i was just waiting for gibi and melwitt to review | 13:50 |
sean-k-mooney | gibi: if your good to proceed ill send it on its way | 13:51 |
gibi | I accept it | 13:51 |
bauzas | cool, no rush | 13:51 |
gibi | so go and send it | 13:51 |
sean-k-mooney | done | 13:51 |
sean-k-mooney | gibi: im going to take a look at the pci spec again soon | 13:52 |
gibi | sean-k-mooney: thanks. only the pci alias part changed the rest is just fixing nits | 13:52 |
bauzas | Uggla: 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=something | 13:52 |
sean-k-mooney | ack | 13:53 |
bauzas | should we conflict or verify that the host is within the requested AZ ? | 13:53 |
bauzas | this was an open question in the spec review | 13:53 |
gibi | ahh good point I will add a line | 13:53 |
gibi | or you cabn | 13:53 |
gibi | I think that should be similar to AZ1 -> AZ2 + host1 case | 13:54 |
sean-k-mooney | bauzas: personally no | 13:54 |
sean-k-mooney | bauzas: i think we should let the scheduler handel that | 13:54 |
bauzas | sean-k-mooney: gibi: ok I added a line | 13:55 |
gibi | thanks | 13:55 |
sean-k-mooney | if its not we will get a no valid host but i was ok with leaving that to the patch review honestly | 13:55 |
sean-k-mooney | i dotn really mind checin in the api | 13:55 |
gibi | I'm OK to let the schedule reject it | 13:55 |
gibi | all these cases | 13:55 |
sean-k-mooney | but there are many other factors that could cause it to rejected | 13:55 |
sean-k-mooney | so schduler/placment is really the only thing that know if that is ok or not | 13:56 |
sean-k-mooney | like isolated_aggrate/tenating isolation filter/host aggreate metadta ectra | 13:56 |
bauzas | gibi: there could be a corner case tho | 13:56 |
gibi | yeah, we cloud do a check in the api and make a better error message but that is not strictly needed | 13:56 |
bauzas | gibi: if we leave the scheduler return NoValidHost, | 13:57 |
bauzas | the RequestSpec would have been modified before | 13:57 |
gibi | ohh true | 13:57 |
bauzas | so we're not idempotent | 13:57 |
sean-k-mooney | well we proably should not save it until after schduing | 13:57 |
gibi | yepp we need to rollback the change or not save it | 13:57 |
gibi | if that is not possible architecturally then lets do a check in the API | 13:58 |
sean-k-mooney | well we will need to cacht the novaild host and rollback in anycase if we do the save before | 13:58 |
bauzas | wai | 13:58 |
bauzas | wait | 13:58 |
bauzas | in order to verify that the host is in an AZ | 13:58 |
bauzas | we need to lookup its aggregate, right | 13:59 |
sean-k-mooney | yep | 13:59 |
bauzas | but, | 13:59 |
bauzas | this is cell-specific | 13:59 |
sean-k-mooney | no its not | 13:59 |
sean-k-mooney | its in the api db | 13:59 |
sean-k-mooney | the host aggreate are in teh api db | 13:59 |
bauzas | oh stupid me, you're right | 13:59 |
sean-k-mooney | the instance.az is in the cell db | 13:59 |
bauzas | we debated the place of the aggregates table for a while when we designed cells v2 | 13:59 |
sean-k-mooney | i think | 14:00 |
bauzas | sean-k-mooney: yes, instance.az | 14:00 |
bauzas | reqspec.az is in the API DB | 14:00 |
sean-k-mooney | ah yes | 14:00 |
sean-k-mooney | so we dont need instance.az | 14:00 |
bauzas | so, there is no cell downcall | 14:00 |
sean-k-mooney | yep | 14:00 |
bauzas | yup, we don't need it | 14:00 |
bauzas | so, this is cheap to verify it by the api service | 14:00 |
bauzas | in this case, I'm in favor of doing the lookup at the unshelve time | 14:01 |
bauzas | in the api service | 14:01 |
bauzas | and return synchronously a "sorry dude" | 14:01 |
bauzas | instead of trying to manage a late persistence or a rollback of the reqspec | 14:01 |
Uggla | agree, it sounds simpler. | 14:02 |
bauzas | and this is synchronous | 14:03 |
sean-k-mooney | bauzas: ok but we still need to handel rollback of the request spec if we fail to unshleve for any reason | 14:04 |
bauzas | sean-k-mooney: agreed | 14:04 |
bauzas | which wasn't the case now, right? | 14:05 |
sean-k-mooney | well i have not looked at he code to check | 14:05 |
bauzas | I see the .save() call, but I don't see the conductor managing the exception | 14:05 |
sean-k-mooney | do you have a link | 14:05 |
bauzas | yup, sec | 14:05 |
bauzas | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4470 | 14:06 |
sean-k-mooney | i guess we can also just document that the request spec will always be updted | 14:06 |
bauzas | sec, better with a permalink | 14:06 |
bauzas | https://github.com/openstack/nova/blob/4939318649650b60dd07d161b80909e70d0e093e/nova/compute/api.py#L4470 | 14:06 |
sean-k-mooney | i just generally effect failed operation to not change things | 14:06 |
sean-k-mooney | ya so right now it changing it uncondtionaly | 14:07 |
sean-k-mooney | i guess that fine | 14:07 |
sean-k-mooney | its also doing the validation now | 14:07 |
bauzas | https://github.com/openstack/nova/blob/4939318649650b60dd07d161b80909e70d0e093e/nova/conductor/manager.py#L1040-L1045 | 14:07 |
sean-k-mooney | https://github.com/openstack/nova/blob/4939318649650b60dd07d161b80909e70d0e093e/nova/compute/api.py#L4454= | 14:07 |
sean-k-mooney | in the api | 14:07 |
bauzas | the conductor is not handling the AZ case on the NoValidHost case | 14:07 |
sean-k-mooney | ya | 14:07 |
bauzas | so, now, if you unshelve to another AZ, and you fail, your instance gets pinned in the new AZ | 14:07 |
sean-k-mooney | yep | 14:08 |
bauzas | still shelved but pinned | 14:08 |
sean-k-mooney | so we either maintain that behavior | 14:08 |
sean-k-mooney | or we treat it as a bug | 14:08 |
sean-k-mooney | and fix that spereatly | 14:08 |
bauzas | good point | 14:08 |
bauzas | this is a bug | 14:08 |
sean-k-mooney | ok then we dont need to cover it in the spec | 14:09 |
bauzas | fixing it now would benefit to the unshelve to host implementation | 14:09 |
bauzas | but wait | 14:09 |
bauzas | the rollback isn't trivial | 14:09 |
bauzas | what field should we set if we rollback ? | 14:09 |
opendevreview | Merged openstack/nova-specs master: Proposes to remove keypair generation https://review.opendev.org/c/openstack/nova-specs/+/840217 | 14:09 |
bauzas | I mean, what value for reqspec.az to restore ? | 14:09 |
bauzas | thanks whoever it was ^ | 14:10 |
bauzas | anyway, I'm bikeshedding | 14:11 |
bauzas | this is unrelated to the spedc | 14:11 |
bauzas | we won't fix all the first-class problems of the world with this spec | 14:11 |
sean-k-mooney | aw | 14:12 |
sean-k-mooney | but what about world hunger | 14:12 |
bauzas | (technically, Placement could resolve the World-class starvation issue we have) | 14:12 |
sean-k-mooney | surely that is in scope right | 14:12 |
bauzas | heh, placement can help | 14:12 |
bauzas | atm, this is just some western countries who use specific required traits | 14:13 |
bauzas | and gather all the resources | 14:13 |
gibi | so 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 trial | 14:13 |
sean-k-mooney | hehe Uggla for context the example demo of placment was makign a sandwich and tracking inventories of food in your fridge | 14:13 |
bauzas | sean-k-mooney: I showcased him during our knowledge transfert :) | 14:14 |
bauzas | I used cdent's slides about ham and leafs | 14:14 |
sean-k-mooney | gibi: ya it can i dont think its a big issue | 14:14 |
bauzas | gibi: you make a valid point | 14:14 |
bauzas | we should open a bug report for not forgeting it | 14:14 |
gibi | ack | 14:15 |
bauzas | the resolution isn't trivial | 14:15 |
bauzas | but at least we can assess in the report that a workaround is to ask for another unshelve | 14:15 |
bauzas | damn, I need to taxi again my kid | 14:15 |
Uggla | bauzas sold me that unshelve to host would be an easy trivial change, self contained to start learning.... ;) | 14:36 |
gibi | Uggla: at least the original use case was clear and agreed. We just made sure that all the possible state transitions are covered | 14:39 |
gibi | and that took a bit of time | 14:39 |
gibi | but this is still fairly simple | 14:40 |
Uggla | gibi, yes I'm just kidding at bauzas. ;) | 14:42 |
gibi | :) | 14:43 |
opendevreview | John Garbutt proposed openstack/nova master: Ironic: retry when node not available https://review.opendev.org/c/openstack/nova/+/842478 | 14:48 |
opendevreview | Merged openstack/nova master: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842528 | 15:49 |
opendevreview | Stephen Finucane proposed openstack/nova stable/yoga: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842584 | 17:48 |
opendevreview | Stephen Finucane proposed openstack/nova stable/xena: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842586 | 17:53 |
opendevreview | Stephen Finucane proposed openstack/nova stable/wallaby: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842587 | 17:57 |
opendevreview | Stephen Finucane proposed openstack/nova stable/victoria: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842588 | 17:57 |
opendevreview | Stephen Finucane proposed openstack/nova stable/victoria: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842588 | 17:58 |
opendevreview | Stephen Finucane proposed openstack/nova stable/ussuri: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842590 | 17:59 |
opendevreview | Stephen Finucane proposed openstack/nova stable/train: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842592 | 18:03 |
*** dasm is now known as dasm|off | 21:26 | |
opendevreview | Ghanshyam proposed openstack/nova master: DNM: testing py38|9 fips on c8|9s https://review.opendev.org/c/openstack/nova/+/842640 | 22:30 |
opendevreview | Ghanshyam proposed openstack/nova master: DNM: testing py38|9 fips on c8|9s https://review.opendev.org/c/openstack/nova/+/842640 | 22:36 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!