| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-03-26 | |||
| 14:37:00 | sean-k-mooney | PetrTuma: the check for the need to schdule happens here https://zuul.opendev.org/t/openstack/build/cfebf394494246beabb5f417ec1d5c64/logs which is after the check i added | |
| 14:37:45 | sean-k-mooney | PetrTuma: which is here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3484-L3485 | |
| 14:37:51 | sean-k-mooney | PetrTuma: so that is not the issue | |
| 14:39:20 | sean-k-mooney | PetrTuma: so the only condition that should prevent the _validate_numa_rebuild function running is if orig_image_ref != image_href: | |
| 14:41:20 | dansmith | gibi: done | |
| 14:44:50 | openstackgerrit | jayaditya gupta proposed openstack/nova master: Support for nova-manage placement heal_allocations --cell https://review.opendev.org/714459 | |
| 14:45:07 | gibi | dansmith: thanks again. and sorry that I'm more tense than usual | |
| 14:45:35 | dansmith | gibi: np, I understand the pressure | |
| 14:46:04 | gibi | I guess some of it is new to me :) | |
| 14:46:58 | dansmith | I believe you signed up for it :) | |
| 14:47:03 | gibi | I did | |
| 14:47:09 | gibi | and I'm learning every day | |
| 14:47:22 | gmann | dansmith: gibi sean-k-mooney actually PUT is swap volume thing which is not actually updating resources but kind of migrating the storage from your machine. So PUT(swap) is always confusing and complex API from nova. I am not sure how useful that is for people. I remember a bug when some of our customer trying swap back and forth and getting error. | |
| 14:47:42 | gmann | my main concern on not using PUT was- not to make this API over scoped and keep it for swap only. | |
| 14:48:38 | gmann | i am ok with any method, PUT also ok for consistency to our APIs but new one not in existing PUT which is swap | |
| 14:49:00 | gmann | can we do it via server PUT ? | |
| 14:49:46 | sean-k-mooney | gmann: but thats the thing PUT should not be swap. PUT + changeing the volume id sure | |
| 14:49:56 | sean-k-mooney | but PUT alone should not have been swap | |
| 14:50:35 | dansmith | I have an important call in ten minutes which I need to get ready for and pay close attention to, but after that I'll be back to discuss further if we need | |
| 14:50:46 | gmann | yeah that is not best design but we cannot change that now but can avoid making it more comlex | |
| 14:51:04 | gmann | dansmith: sure. | |
| 14:51:35 | dansmith | IMHO, anything that requires the client to do something different than the obvious thing of PUTting the resource with delete-on-termination changed is more complex | |
| 14:52:06 | sean-k-mooney | gmann: acutlly we could change it in a microversion but not in ussuri at this point. i do tend to agree with ^ on that point | |
| 14:53:37 | sean-k-mooney | * with dansmith | |
| 14:54:08 | gmann | you mean making swap as action(though action are not good but at least consistent to our API ) and PUT for delete-on-termination changed or any future modification in volume things ? | |
| 14:54:41 | sean-k-mooney | gmann: we dont need to make it an action although that is one option. we just need to stop thing of PUT as swap | |
| 14:54:45 | johnthetubaguy | isn't the simplest just that PUT does different things if the body includes volume_id or delete_on_termination | |
| 14:54:47 | gmann | dansmith: +1 on that. but does not client will be confused with PUT = swap + actual updating.. | |
| 14:55:03 | sean-k-mooney | PUT is just an update and if put change the volume id its sideffect is a swap underneat | |
| 14:55:35 | gmann | johnthetubaguy: in implementation, there is no challenge it is easy. i am thinking on usage point of view if we are making this API over-scoped | |
| 14:55:54 | sean-k-mooney | johnthetubaguy: well it does one thing update the attachment recored, and that update can have other sideffect such as swap | |
| 14:56:47 | sean-k-mooney | if we consider the attachment ot be declaritive rahter then inperitive this is totally natural | |
| 14:56:54 | gmann | i think swap storage is very explicit things to use and adding it as side-effect behind some operation is not good API | |
| 14:57:03 | johnthetubaguy | for me its a consistency thing, PUT for partial update is what we do on other resources | |
| 14:57:28 | sean-k-mooney | PUT sematicaly is not ment ot be a partil update but it is how we use it yes | |
| 14:57:49 | gmann | yeah, i completely agree on that. but we have used this PUT for swap that is the only things i am worried about. | |
| 14:58:12 | johnthetubaguy | gmann: there are two ways of thinking about this though, they are both a partial update of the attachment, one just does more things than the other | |
| 14:59:04 | johnthetubaguy | if we ignore the existing Cinder specific API, we wouldn't be talking about this right, its just a PUT for every other API, and I think that is the simplest thing for 99% of our API users | |
| 14:59:41 | johnthetubaguy | but hey, best to discuss later I suspect | |
| 14:59:52 | gmann | can we move the swap to action API and use PUT for these kind of updates ? like our host/services/hypervisors APIs were not good and we combined them . | |
| 15:00:44 | johnthetubaguy | gmann: certainly an option, but its a cinder only, largely internal to OpenStack API, that frankly I am tempted to remove from the API docs to avoid confusion | |
| 15:00:50 | sean-k-mooney | gmann: we could but what benifit does that serve | |
| 15:01:02 | johnthetubaguy | seems like busy work to me | |
| 15:01:22 | johnthetubaguy | we don't have bandwidth for the pants of fire stuff right now | |
| 15:02:00 | gmann | ok | |
| 15:02:43 | openstackgerrit | Merged openstack/nova master: Add transform_image_metadata request filter https://review.opendev.org/665775 | |
| 15:05:29 | Sundar | dansmith, sean-k-mooney, gibi, brinzhang_, alex_xu: I am creating a list of followups to the Cyborg-Nova patch series: https://etherpad.openstack.org/p/cyborg-nova-followup . It is WIP. But, if you have any comments on the structure of the doc or the categories of the tasks, please LMK. | |
| 15:06:38 | gibi | Sundar: ack | |
| 15:08:38 | gmann | if we consider volume swap from nova perspective/API as just an update to resource then i am ok and hope it does not confuse users. having clear doc about what this API does as per different ways of request. | |
| 15:10:12 | gmann | gibi: brinzhang_ on instance event things. +1 on having a clear doc to explain how operator can use policy in which situation and with risk. | |
| 15:11:59 | gmann | gibi: brinzhang_ I was waiting for dansmith reply on that- if we can exclude non-nova exception from 'details' and only show nova exception ?. though it will be much clear if we can exclude few nova exceptions also which are not fixable by non-admin. | |
| 15:16:01 | gmann | if that is hard to do/decide the gray list of exceptions for non-admin then I am ok with only have a clear policy doc saying the risk and usage of this policy. | |
| 15:18:13 | nightmare_unreal | mriedem: how will I know what resource value to overwrite ? I am refering to TO-DO overwrite allocations | |
| 15:21:53 | mriedem | nightmare_unreal: i guess your test would need to change something about the instance allocations out of band (via the placement API directly) and then run heal_allocations with your new flag which will overwrite those allocations back to the instance flavor | |
| 15:22:19 | Sundar | sean-k-mooney: Re. https://review.opendev.org/#/c/673735/46/nova/conductor/manager.py@1632, since this is the instance creation code path, failures will cause rescheduling rather than put the instance in error status for the user to clean up, right? | |
| 15:22:23 | mriedem | so i guess you could like simulate a busted same-host resize and double the allocation values for the instance? run heal and then assert the allocations are back to the instance.flavor values | |
| 15:23:26 | nightmare_unreal | i have not yet reached to write test :/ , was fixing my previous patch. Yet to write code for the overwrite. | |
| 15:31:02 | mriedem | ok well i think the actual change is just plumbing a new type of force or overwrite flag or something down to where that conditional is i showed you the other day | |
| 15:31:11 | mriedem | where it determines if it should call put_allocations or not | |
| 15:31:40 | nightmare_unreal | yes I think so too :) | |
| 15:31:43 | nightmare_unreal | afk | |
| 15:32:11 | nightmare_unreal | Thanks | |
| 15:32:33 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tox: Integrate mypy https://review.opendev.org/676208 | |
| 15:32:34 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Update and correct typing information https://review.opendev.org/714694 | |
| 15:32:34 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Add typing information https://review.opendev.org/714695 | |
| 15:32:35 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Split instance NUMA object tests https://review.opendev.org/714696 | |
| 15:32:35 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Replace 'cpu_pinning_requested' helper https://review.opendev.org/714697 | |
| 15:32:36 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Don't consider overhead CPUs for unpinned instances https://review.opendev.org/714698 | |
| 15:32:36 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Remove handling of pre-Train compute nodes https://review.opendev.org/714699 | |
| 15:32:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Add validation for 'cpu_realtime_mask' https://review.opendev.org/468203 | |
| 15:32:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Tweak the 'cpu_realtime_mask' handling slightly https://review.opendev.org/461456 | |
| 15:32:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Rework 'get_realtime_constraint' https://review.opendev.org/714700 | |
| 15:32:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Invert order of NUMA topology generation https://review.opendev.org/714701 | |
| 15:33:23 | gibi | gmann: do I understand correctly that you also got convinced that the PUT solution is OK | |
| 15:33:26 | gibi | ? | |
| 15:36:55 | gmann | gibi: yeah. I am ok to add in existing PUT as making other PUT or action API is a big change from implementation and users perspective. having api-ref clear about this PUT = swap + updating resources based on how you use this API. | |
| 15:37:18 | gibi | gmann: ack, thanks | |
| 15:37:53 | gmann | though it makes this API more complex but its tradeoff as dansmith mentioned that any other way is not less complex for client. | |
| 15:40:04 | sean-k-mooney | gmann: i think calling it a swap + updating resources is still the wrong way to think about it | |
| 15:40:19 | gmann | gibi: on the instance fault things: i will wai for dansmith to come back and discuss if somehow we can minimize the info leak to non-admin. other part like policy things is ok for me. | |
| 15:40:25 | sean-k-mooney | gmann: it is just declaritivly updatin the resouces and a sidefect of that can be to trigger swap | |
| 15:40:43 | sean-k-mooney | that is how we should explain it to endusers | |
| 15:40:55 | gibi | gmann: re instance fault: thanks | |
| 15:41:08 | johnthetubaguy | gmann: my preference is to not document the crazy cinder API, and move that to some developer docs or something? | |
| 15:41:36 | johnthetubaguy | should stop some confusion for the 99% of API users | |
| 15:41:37 | sean-k-mooney | johnthetubaguy: do end users ever call that directly | |
| 15:41:40 | gmann | sean-k-mooney: humm, having the swap as side-efect is my concern. that is big change to enduser machine so it has to be very explicit operation from usage doc side. | |
| 15:41:47 | johnthetubaguy | sean-k-mooney: they do, but they never should | |
| 15:41:58 | johnthetubaguy | its for cinder only, else odd things happen | |
| 15:42:07 | sean-k-mooney | gmann: not really that is how i alwasy tought of it already | |
| 15:42:42 | sean-k-mooney | johnthetubaguy: ok so this is just for the callback form cinder | |
| 15:42:51 | johnthetubaguy | that is my understanding, yes | |
| 15:42:56 | johnthetubaguy | hence the big red warning | |
| 15:43:04 | gmann | johnthetubaguy: can we make that internal than ? at least from doc side. i mean api-ref not at all talk about swap operation for users as currently we have ? | |
| 15:43:09 | sean-k-mooney | johnthetubaguy: and yes im pretty sure we have had downstream bugs filed as a result of users calling it directly | |
| 15:43:20 | johnthetubaguy | gmann: +1 that | |
| 15:43:24 | gmann | sean-k-mooney: yeah we had few in past | |
| 15:43:35 | johnthetubaguy | sean-k-mooney: yeah, what they wanted to do was call the Cinder swap API, but they got confused | |
| 15:43:39 | gibi | for me the difference is like change my house (swap) or repaint the kitchen in my house (update) | |
| 15:44:36 | sean-k-mooney | actully i think our customer wanted to bypass a check in the ciner side a and force it so that is why they used nova | |