| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-05-19 | |||
| 13:57:33 | bauzas | so we're not idempotent | |
| 13:57:37 | sean-k-mooney | well we proably should not save it until after schduing | |
| 13:57:48 | gibi | yepp we need to rollback the change or not save it | |
| 13:58:02 | gibi | if that is not possible architecturally then lets do a check in the API | |
| 13:58:38 | sean-k-mooney | well we will need to cacht the novaild host and rollback in anycase if we do the save before | |
| 13:58:38 | bauzas | wai | |
| 13:58:39 | bauzas | wait | |
| 13:58:52 | bauzas | in order to verify that the host is in an AZ | |
| 13:59:04 | bauzas | we need to lookup its aggregate, right | |
| 13:59:09 | sean-k-mooney | yep | |
| 13:59:12 | bauzas | but, | |
| 13:59:17 | bauzas | this is cell-specific | |
| 13:59:21 | sean-k-mooney | no its not | |
| 13:59:23 | sean-k-mooney | its in the api db | |
| 13:59:32 | sean-k-mooney | the host aggreate are in teh api db | |
| 13:59:34 | bauzas | oh stupid me, you're right | |
| 13:59:55 | sean-k-mooney | the instance.az is in the cell db | |
| 13:59:58 | bauzas | we debated the place of the aggregates table for a while when we designed cells v2 | |
| 14:00:00 | sean-k-mooney | i think | |
| 14:00:09 | bauzas | sean-k-mooney: yes, instance.az | |
| 14:00:17 | bauzas | reqspec.az is in the API DB | |
| 14:00:23 | sean-k-mooney | ah yes | |
| 14:00:29 | sean-k-mooney | so we dont need instance.az | |
| 14:00:30 | bauzas | so, there is no cell downcall | |
| 14:00:35 | sean-k-mooney | yep | |
| 14:00:36 | bauzas | yup, we don't need it | |
| 14:00:48 | bauzas | so, this is cheap to verify it by the api service | |
| 14:01:12 | bauzas | in this case, I'm in favor of doing the lookup at the unshelve time | |
| 14:01:17 | bauzas | in the api service | |
| 14:01:28 | bauzas | and return synchronously a "sorry dude" | |
| 14:01:51 | bauzas | instead of trying to manage a late persistence or a rollback of the reqspec | |
| 14:02:51 | Uggla | agree, it sounds simpler. | |
| 14:03:27 | bauzas | and this is synchronous | |
| 14:04:26 | sean-k-mooney | bauzas: ok but we still need to handel rollback of the request spec if we fail to unshleve for any reason | |
| 14:04:53 | bauzas | sean-k-mooney: agreed | |
| 14:05:07 | bauzas | which wasn't the case now, right? | |
| 14:05:35 | sean-k-mooney | well i have not looked at he code to check | |
| 14:05:38 | bauzas | I see the .save() call, but I don't see the conductor managing the exception | |
| 14:05:45 | sean-k-mooney | do you have a link | |
| 14:05:57 | bauzas | yup, sec | |
| 14:06:04 | bauzas | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4470 | |
| 14:06:10 | sean-k-mooney | i guess we can also just document that the request spec will always be updted | |
| 14:06:18 | bauzas | sec, better with a permalink | |
| 14:06:19 | bauzas | https://github.com/openstack/nova/blob/4939318649650b60dd07d161b80909e70d0e093e/nova/compute/api.py#L4470 | |
| 14:06:21 | sean-k-mooney | i just generally effect failed operation to not change things | |
| 14:07:02 | sean-k-mooney | ya so right now it changing it uncondtionaly | |
| 14:07:05 | sean-k-mooney | i guess that fine | |
| 14:07:13 | sean-k-mooney | its also doing the validation now | |
| 14:07:17 | bauzas | https://github.com/openstack/nova/blob/4939318649650b60dd07d161b80909e70d0e093e/nova/conductor/manager.py#L1040-L1045 | |
| 14:07:18 | sean-k-mooney | https://github.com/openstack/nova/blob/4939318649650b60dd07d161b80909e70d0e093e/nova/compute/api.py#L4454= | |
| 14:07:20 | sean-k-mooney | in the api | |
| 14:07:34 | bauzas | the conductor is not handling the AZ case on the NoValidHost case | |
| 14:07:58 | sean-k-mooney | ya | |
| 14:07:59 | bauzas | so, now, if you unshelve to another AZ, and you fail, your instance gets pinned in the new AZ | |
| 14:08:07 | sean-k-mooney | yep | |
| 14:08:09 | bauzas | still shelved but pinned | |
| 14:08:25 | sean-k-mooney | so we either maintain that behavior | |
| 14:08:32 | sean-k-mooney | or we treat it as a bug | |
| 14:08:38 | sean-k-mooney | and fix that spereatly | |
| 14:08:53 | bauzas | good point | |
| 14:08:56 | bauzas | this is a bug | |
| 14:09:06 | sean-k-mooney | ok then we dont need to cover it in the spec | |
| 14:09:13 | bauzas | fixing it now would benefit to the unshelve to host implementation | |
| 14:09:23 | bauzas | but wait | |
| 14:09:30 | bauzas | the rollback isn't trivial | |
| 14:09:46 | bauzas | what field should we set if we rollback ? | |
| 14:09:59 | opendevreview | Merged openstack/nova-specs master: Proposes to remove keypair generation https://review.opendev.org/c/openstack/nova-specs/+/840217 | |
| 14:09:59 | bauzas | I mean, what value for reqspec.az to restore ? | |
| 14:10:08 | bauzas | thanks whoever it was ^ | |
| 14:11:30 | bauzas | anyway, I'm bikeshedding | |
| 14:11:35 | bauzas | this is unrelated to the spedc | |
| 14:11:52 | bauzas | we won't fix all the first-class problems of the world with this spec | |
| 14:12:08 | sean-k-mooney | aw | |
| 14:12:15 | sean-k-mooney | but what about world hunger | |
| 14:12:23 | bauzas | (technically, Placement could resolve the World-class starvation issue we have) | |
| 14:12:26 | sean-k-mooney | surely that is in scope right | |
| 14:12:35 | bauzas | heh, placement can help | |
| 14:13:18 | bauzas | atm, this is just some western countries who use specific required traits | |
| 14:13:33 | bauzas | and gather all the resources | |
| 14:13:53 | gibi | so a failed unshelve can be retried so I'm not worrying about persisting the wrong data, as that can be overwritten by the next unshelve trial | |
| 14:13:59 | sean-k-mooney | hehe Uggla for context the example demo of placment was makign a sandwich and tracking inventories of food in your fridge | |
| 14:14:19 | bauzas | sean-k-mooney: I showcased him during our knowledge transfert :) | |
| 14:14:37 | bauzas | I used cdent's slides about ham and leafs | |
| 14:14:39 | sean-k-mooney | gibi: ya it can i dont think its a big issue | |
| 14:14:47 | bauzas | gibi: you make a valid point | |
| 14:14:58 | bauzas | we should open a bug report for not forgeting it | |
| 14:15:03 | gibi | ack | |
| 14:15:04 | bauzas | the resolution isn't trivial | |
| 14:15:24 | bauzas | but at least we can assess in the report that a workaround is to ask for another unshelve | |
| 14:15:49 | bauzas | damn, I need to taxi again my kid | |
| 14:36:57 | Uggla | bauzas sold me that unshelve to host would be an easy trivial change, self contained to start learning.... ;) | |
| 14:39:11 | gibi | Uggla: at least the original use case was clear and agreed. We just made sure that all the possible state transitions are covered | |
| 14:39:20 | gibi | and that took a bit of time | |
| 14:40:36 | gibi | but this is still fairly simple | |
| 14:42:13 | Uggla | gibi, yes I'm just kidding at bauzas. ;) | |
| 14:43:08 | gibi | :) | |
| 14:48:50 | opendevreview | John Garbutt proposed openstack/nova master: Ironic: retry when node not available https://review.opendev.org/c/openstack/nova/+/842478 | |
| 15:49:16 | opendevreview | Merged openstack/nova master: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842528 | |
| 17:48:02 | opendevreview | Stephen Finucane proposed openstack/nova stable/yoga: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842584 | |
| 17:53:49 | opendevreview | Stephen Finucane proposed openstack/nova stable/xena: neutron: Unbind remaining ports after PortNotFound https://review.opendev.org/c/openstack/nova/+/842586 | |