| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-11 | |||
| 19:13:39 | openstack | Launchpad bug 1763766 in OpenStack Compute (nova) "nova needs to disallow resource consumption changes on image rebuild" [Medium,Triaged] | |
| 19:13:40 | openstack | Launchpad bug 1804502 in OpenStack Compute (nova) "Rebuild server with NUMATopologyFilter enabled fails (in some cases)" [Undecided,In progress] - Assigned to David Hill (david-hill-ubisoft) | |
| 19:14:03 | eandersson | Would it be possible to add an option to allow rebuilds to re-schedule the VM? | |
| 19:14:23 | eandersson | something like openstack server rebuild <id> --migrate/reschedule/bla | |
| 19:14:28 | sean-k-mooney | like evauate | |
| 19:14:46 | eandersson | I can't reproduce this as well | |
| 19:15:26 | sean-k-mooney | rebuild is ment to be posiblein virt driver that dont support migration | |
| 19:16:29 | sean-k-mooney | there is noting in principal that would disallow a driver form having rebiuld be to a different host like a cold migration but it is not how its currelty impelmeted | |
| 19:16:49 | sean-k-mooney | we would have to change the noop claim to a move claim if the inplcem rebuild did not succesed | |
| 19:17:29 | sean-k-mooney | or expose it at the api like you suggest and make the resuling to a different host user contolerale | |
| 19:18:52 | sean-k-mooney | ingoring numa for a second since image can have tratis we need to check with placmenet in anycase to see it the traits on the new image are valid on the current host | |
| 19:20:44 | sean-k-mooney | eandersson: what is the behavior you would like to see | |
| 19:21:11 | eandersson | Better error message at least :p | |
| 19:21:40 | eandersson | But not sure what the ideal would be. | |
| 19:21:50 | efried | stephenfin, sean-k-mooney, luyao, alex_xu: I'm +2 on everything vpmem except: | |
| 19:21:50 | efried | - https://review.opendev.org/#/c/678455/ where I'll proxy a vote from sean-k-mooney | |
| 19:21:50 | efried | - the bottom where I'll flip from -2 to +2 when we're ready | |
| 19:21:53 | eandersson | Maybe just documentation on how to avoid this on image builds. | |
| 19:22:02 | efried | I'm now going to sleep. | |
| 19:22:06 | eandersson | It would be nice as an admin to have a way to fix it at least. | |
| 19:22:33 | sean-k-mooney | eandersson: well you cant avoid it if the instance has a numa toplogy | |
| 19:22:44 | sean-k-mooney | you can work around it via cold/live migation | |
| 19:22:50 | sean-k-mooney | but that it | |
| 19:23:13 | eandersson | Maybe always force reschedule on rebuild when numa is a thing | |
| 19:23:37 | sean-k-mooney | rebuild are not allowed to change host today | |
| 19:23:42 | sean-k-mooney | so there is no rescudler | |
| 19:24:03 | sean-k-mooney | whats actully happening is the numa toplogy filter is treating the rebuild like a new isntance spawn | |
| 19:24:09 | sean-k-mooney | so you need double the resouces | |
| 19:24:26 | eandersson | oh so that is why some of them worked | |
| 19:24:32 | sean-k-mooney | yep | |
| 19:24:49 | sean-k-mooney | if the instance does not have a numa toplogy or there is enough space it works | |
| 19:25:35 | sean-k-mooney | we can actully now tell if its a rebuild in the filter code but we have not added special handeling for that in the numa toplogy filter | |
| 19:26:17 | sean-k-mooney | the correct thign to do is to check if the numa topldoy woudl change. reject the rebuild if its true or skip the numa toplogy filter if not | |
| 19:26:34 | sean-k-mooney | that is more or less want i want to code up | |
| 19:28:03 | sean-k-mooney | if posible i would like to do the "rebuild chage numa toplogy" check at the api and return an expcti error | |
| 19:28:25 | sean-k-mooney | but i have not figured out all the pices yet | |
| 19:31:01 | mriedem | sean-k-mooney: if rebuild + new image + numa sometimes works if the new image numa topo fits on the same host, why wouldn't you just let the scheduler still try to fit it? | |
| 19:31:16 | mriedem | and only outright skip if the image is new + rebuild + none of the numa properties changed | |
| 19:31:30 | sean-k-mooney | mriedem: we dont update the instance numa toplogy in the db | |
| 19:31:35 | sean-k-mooney | because that is dont in the claim | |
| 19:31:37 | mriedem | and we don't claim | |
| 19:31:39 | sean-k-mooney | and we do a noop claim | |
| 19:31:43 | mriedem | yeah ok | |
| 19:31:52 | sean-k-mooney | so if we fixed that too then sure | |
| 19:31:54 | mriedem | so like new image + volume-backed server we just fail fast in the api today | |
| 19:32:21 | sean-k-mooney | thats becasue we dont supprot volume backed rebuild in general? | |
| 19:32:27 | mriedem | no, | |
| 19:32:39 | mriedem | it's because we don't replace the root volume with the new image | |
| 19:33:09 | mriedem | there are approved cinder/nova specs to support all of that in the past and cinder did a bunch of work to plumb a new api for us, but the nova side changes all stalled | |
| 19:33:41 | mriedem | gibi_bus: did you go to bed yet? | |
| 19:34:52 | sean-k-mooney | this liekly will become a spec in anycase. but what i wanted to do was first do the api/filer check to block it. then explore either updating the topology on rebuld and validatin git or otherwise having a more ocmplet fix | |
| 19:35:17 | sean-k-mooney | if it could fix then i would liek to allow it | |
| 19:35:32 | sean-k-mooney | but only if the toploy you ask for in the image is what you actlly end up with | |
| 19:35:48 | sean-k-mooney | which is not what would happen without updating the laim | |
| 19:35:51 | sean-k-mooney | *claim | |
| 19:37:40 | sean-k-mooney | the otherthing with allowing the toplogy to change on a rebuild would be keeping placmenet in sysnc if/when we model numa in placmenet | |
| 19:38:37 | sean-k-mooney | i dont want to complicate numa in plamcent more by having to deal with this at the same time | |
| 19:40:34 | mriedem | i could use some opinions on alternatives i've proposed for some new error handling during prep_resize in gibi's bottom unapproved change https://review.opendev.org/#/c/676980/20/nova/compute/manager.py@4529 which is the only thing holding it up right now | |
| 19:40:49 | mriedem | since i'll probably be the one implementing them today so we can still meet FF for the bw provider migration stuff | |
| 19:42:01 | mriedem | sean-k-mooney: yeah i'd focus on whatever you can that is backportable first | |
| 19:43:41 | sean-k-mooney | yep thats the plan. its in our team backlog for U and ill likely volenterr to be the person to fix it so im going to fix what can be backport first then consider how to imporve it later. | |
| 19:43:56 | sean-k-mooney | thanks for the link to the other bug by the way | |
| 19:44:03 | sean-k-mooney | i forgot that was still open | |
| 19:45:06 | openstackgerrit | Merged openstack/nova master: Make _revert_allocation nested allocation aware https://review.opendev.org/676138 | |
| 19:45:07 | sean-k-mooney | im jsut looking at https://review.opendev.org/#/c/676980/20/nova/compute/manager.py@4529 | |
| 19:45:42 | sean-k-mooney | i dont really have context right now but what alternitve were you tinking of | |
| 19:49:32 | mriedem | they are listed below | |
| 19:49:57 | sean-k-mooney | ok ill keep reading | |
| 19:50:06 | mriedem | gibi added code to the prep_resize flow that can result in a BuildAbortException getting raised and in that case we shouldn't try to reschedule | |
| 19:50:15 | mriedem | he added handling for BuildAbortException, | |
| 19:50:19 | sean-k-mooney | the finally block will be run by the way when we catch the BuildAbortException | |
| 19:50:27 | sean-k-mooney | i was just comming that in gerrit | |
| 19:50:29 | mriedem | but was missing some things we do when we don't reschedule from a prep_resize failure, like recording a fault and sending an error notification | |
| 19:56:17 | gibi | mriedem: I'm not sleeping yet but I'm already short on brainpower | |
| 19:56:40 | mriedem | gibi: ok if you can look at https://review.opendev.org/#/c/676980/20 before going to bed i can try to update it | |
| 19:56:49 | mriedem | but it's looking non-trivial | |
| 19:57:04 | mriedem | otherwise i'll just move up the series since those changes look simple | |
| 19:59:25 | gibi | now I read your comments. it seems non trivial indeed. Would it be too bad letting the re-schedule happen as a first solution. And optimizing out the unnecessary rechedule later? | |
| 20:01:13 | gibi | mriedem: I cannot implement the functional test and the proper cleanup today. I can do that tomorrow morning. Or If you able to implement it then I can try to review it | |
| 20:02:36 | sean-k-mooney | donnyd: the new lable job passed https://review.opendev.org/#/c/680738/ so im going to call it a day | |
| 20:03:57 | gibi | I will be back in 15 minutes | |
| 20:05:12 | mriedem | gibi: i could let the BuildAbortException handling slide to a follow up but the fact that i can comment out the new code and none of the functional tests fail is kind of a blocker for me | |
| 20:05:39 | gibi | mriedem: ack, that was a surprise for me too. | |
| 20:06:22 | gibi | mriedem: I suggest you move forward with the reviews and I will respin this in the morning with a proper functional test | |
| 20:07:05 | gibi | mriedem: as I run out of time today and I don't want to ask you to troubleshoot those functional tests | |
| 20:07:40 | openstackgerrit | Luyao Zhong proposed openstack/nova master: Add functional tests for virtual persistent memory https://review.opendev.org/678470 | |
| 20:08:29 | mriedem | gibi: yup sounds like a plan | |
| 20:11:44 | openstackgerrit | Merged openstack/nova master: Nova object changes for forbidden aggregates request filter https://review.opendev.org/671072 | |
| 20:21:37 | gibi | shover thoughts: the computes in those tests are symmetrical. they have the same PF setup. PFs are named the same on host1 and on host2 and we are "updating" the PF name in the PCI request. | |
| 20:21:55 | gibi | when we migrate from host1 to host2 | |
| 20:22:12 | gibi | so I can imagine that without the update the PF names remains the same | |
| 20:22:30 | gibi | and everything works | |
| 20:22:30 | donnyd | sean-k-mooney: woot woot | |
| 20:43:04 | openstackgerrit | Merged openstack/nova master: Improve SEV documentation and other minor tweaks https://review.opendev.org/681254 | |
| 20:43:14 | openstackgerrit | Merged openstack/nova master: Introduce live_migration_claim() https://review.opendev.org/635669 | |
| 20:48:16 | gibi | I was able to disprove my shover thoughts. | |
| 20:48:36 | gibi | I'm going to sleep now. Talk to you tomorrow | |
| 20:49:54 | gibi | and I disproved my disproval | |
| 20:51:08 | openstackgerrit | Merged openstack/python-novaclient master: doc: Add support microversions for options https://review.opendev.org/681174 | |
| 20:53:21 | openstackgerrit | Merged openstack/os-traits master: Build pdf docs https://review.opendev.org/681465 | |