| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-03-26 | |||
| 14:25:34 | PetrTuma | images (that differ in hw_numa_nodes and few other NUMA related properties). So far none of the rebuilds had been blocked. I added some extra logging to see whether the _validate_numa_rebuild method is actually called or not. It looks like it isn't, I don't see my message logged in any logs. I'm quite at loss what might be wrong. Do you have any | |
| 14:25:34 | PetrTuma | sean-k-mooney Hello, I had some time this week to test the https://review.opendev.org/#/c/703116/3 and the related issue in Rocky. I was able to reproduce original issue (thanks again for the help last week) and now I'm back to testing of the fix for that issue. Once again I updated code in my nova containers and I'm testing the rebuild between two | |
| 14:25:35 | PetrTuma | idea? | |
| 14:26:29 | sean-k-mooney | you updated the nova api container right | |
| 14:26:38 | PetrTuma | yes | |
| 14:26:56 | sean-k-mooney | just checking you didnt update the compute one :) | |
| 14:27:37 | sean-k-mooney | PetrTuma: what release is it? | |
| 14:27:44 | PetrTuma | Rocky | |
| 14:28:06 | dansmith | gibi: do you want me to circle back on that review and re-affirm my -1 with a summary here, or wait for the meeting or something else? | |
| 14:28:09 | sean-k-mooney | i tihnk rocky has the chagne that force the image to be checked if it changes | |
| 14:28:17 | sean-k-mooney | in older releases we did not do that | |
| 14:29:17 | sean-k-mooney | dansmith: you could propose a minor update to the spec with what you want to see? | |
| 14:29:32 | dansmith | I could | |
| 14:29:47 | openstackgerrit | Merged openstack/nova master: Non-Admin user can filter their instances by more filters https://review.opendev.org/701609 | |
| 14:29:53 | openstackgerrit | Merged openstack/nova master: Add default cpu model for AArch64 https://review.opendev.org/709494 | |
| 14:30:53 | PetrTuma | sean-k-mooney do you mean other change than the one I use? | |
| 14:31:49 | hrw | yes! | |
| 14:31:50 | gibi | dansmith: I would be glad for a summary back on the review | |
| 14:32:06 | sean-k-mooney | PetrTuma: yes so a cople of release ago we made a change to cause rebuild with a different image to query the schduler to determin if the current host was valid | |
| 14:32:28 | dansmith | gibi: okay | |
| 14:32:31 | gibi | dansmith: thank you | |
| 14:32:36 | sean-k-mooney | PetrTuma: that is one possiblity as to why this code is not beeing called. if it is not in rocky. although i tought that change was older then that | |
| 14:32:51 | gibi | dansmith: I will try to reach gmann to comment back | |
| 14:33:05 | gibi | dansmith: either on the meeting or separately | |
| 14:34:05 | PetrTuma | sean-k-mooney Ok, I'll try to find the change and confirm this, thanks for the moment | |
| 14:36:37 | gmann | reading logs... | |
| 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: 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 | |