| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-03-26 | |||
| 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: libvirt: Add typing information https://review.opendev.org/714695 | |
| 15:32:34 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Update and correct typing information https://review.opendev.org/714694 | |
| 15:32:35 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Replace 'cpu_pinning_requested' helper https://review.opendev.org/714697 | |
| 15:32:35 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Split instance NUMA object tests https://review.opendev.org/714696 | |
| 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:36 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Don't consider overhead CPUs for unpinned instances https://review.opendev.org/714698 | |
| 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:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Add validation for 'cpu_realtime_mask' https://review.opendev.org/468203 | |
| 15:32:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Invert order of NUMA topology generation https://review.opendev.org/714701 | |
| 15:32:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Rework 'get_realtime_constraint' https://review.opendev.org/714700 | |
| 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 | |
| 15:45:35 | johnthetubaguy | gibi: or you swap out your oven to a hot one, or just change tell it to self destruct when your pizza is done. its the same thing you are changing. | |
| 15:45:45 | openstackgerrit | Stephen Finucane proposed openstack/nova master: api: Add framework for extra spec validation https://review.opendev.org/704643 | |
| 15:45:46 | openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Add documentation for flavor extra specs https://review.opendev.org/710037 | |
| 15:45:46 | sean-k-mooney | anyway when we have had issue related to swap volume we havne pretty much always fixed it once for them and told them not to do that again | |
| 15:45:46 | openstackgerrit | Stephen Finucane proposed openstack/nova master: api: Add microversion 2.84, extra spec validation https://review.opendev.org/708436 | |
| 15:45:55 | stephenfin | gibi: I'd to rebase that ^ after losing the race for 2.83. Could you hit https://review.opendev.org/#/c/708436/9 again? | |
| 15:46:18 | johnthetubaguy | sean-k-mooney: yeah, sounds like they were doing something bad. something we shouldn't tell them to do in the docs | |
| 15:46:19 | stephenfin | and johnthetubaguy, any chance you'll stick that on your list? iirc, that was also on your list of gripes ^ | |
| 15:46:21 | gibi | johnthetubaguy: yeah like that. replace the whole v.s. tweak a prt of the whole | |
| 15:46:45 | gibi | stephenfin: sure | |
| 15:46:56 | johnthetubaguy | stephenfin: well played, and yes on both counts | |
| 15:47:05 | gibi | stephenfin: I think I caused you to lose :) | |
| 15:47:34 | stephenfin | well, thankfully it's just lyarwood and I duking it out for 2.84 now, afaict :) | |
| 15:48:05 | gibi | :) | |