Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-03-26
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: 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.

Earlier   Later