| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-03-26 | |||
| 14:19:06 | nightmare_unreal | cool :) Thanks | |
| 14:19:21 | sean-k-mooney | e.g. tox -e functional-py36 -- "compute|libvirt" | |
| 14:19:48 | nightmare_unreal | in quotes can i specify exact class name ? | |
| 14:19:55 | nightmare_unreal | like in your example | |
| 14:20:04 | sean-k-mooney | yes | |
| 14:20:11 | nightmare_unreal | awesome, thanks | |
| 14:20:21 | sean-k-mooney | the quote are jsut so the shell does not interperate the | as a pipe | |
| 14:20:54 | sean-k-mooney | so in this case is an or in the regex so that will run all gets with compute or libvirt in the fully qulified function name | |
| 14:21:29 | nightmare_unreal | understood | |
| 14:21:43 | gibi | dansmith, sean-k-mooney: from technical perspective I'm OK to have PUT or PATCH as well. I was convinced about both at some point in the past. From process perspective I feel bad that we might change direction of the API design 2 weeks before FF. | |
| 14:22:26 | gibi | I will raise the discussion again on the team meeting as I hope gmann will be there | |
| 14:22:31 | dansmith | gibi: I would hate to change our API so significantly on a process technicality | |
| 14:22:54 | dansmith | I'd much rather help brinzhang_ get it right, and/or approve a little extra time | |
| 14:23:09 | dansmith | the API stability is serious business to me and much more important to get it right than not | |
| 14:24:05 | gibi | dansmith: OK that is a good point. Let's double check with gmann but I'm OK to give extra time after FF if brinzhang_ needs time to change | |
| 14:24:16 | dansmith | meaning, it's not easily changeable later to allow something to land before a deadline | |
| 14:24:22 | dansmith | we have enough weird "huh, why is this different than *everything* else" warts on the API.. let's not make it worse | |
| 14:25:17 | openstackgerrit | Kevin Zhao proposed openstack/nova master: fix scsi disk unit number of the attaching volume when cdrom bus is scsi https://review.opendev.org/712607 | |
| 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 | |