| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-05-19 | |||
| 13:52:27 | sean-k-mooney | gibi: im going to take a look at the pci spec again soon | |
| 13:52:43 | gibi | sean-k-mooney: thanks. only the pci alias part changed the rest is just fixing nits | |
| 13:52:48 | bauzas | Uggla: gibi: I don't see in https://etherpad.opendev.org/p/unshelve-to-host#L166 what we should do with original_az=None, host=hostB and AZ=something | |
| 13:53:01 | sean-k-mooney | ack | |
| 13:53:08 | bauzas | should we conflict or verify that the host is within the requested AZ ? | |
| 13:53:18 | bauzas | this was an open question in the spec review | |
| 13:53:32 | gibi | ahh good point I will add a line | |
| 13:53:41 | gibi | or you cabn | |
| 13:54:24 | gibi | I think that should be similar to AZ1 -> AZ2 + host1 case | |
| 13:54:40 | sean-k-mooney | bauzas: personally no | |
| 13:54:51 | sean-k-mooney | bauzas: i think we should let the scheduler handel that | |
| 13:55:14 | bauzas | sean-k-mooney: gibi: ok I added a line | |
| 13:55:17 | gibi | thanks | |
| 13:55:28 | sean-k-mooney | if its not we will get a no valid host but i was ok with leaving that to the patch review honestly | |
| 13:55:38 | sean-k-mooney | i dotn really mind checin in the api | |
| 13:55:44 | gibi | I'm OK to let the schedule reject it | |
| 13:55:47 | gibi | all these cases | |
| 13:55:56 | sean-k-mooney | but there are many other factors that could cause it to rejected | |
| 13:56:14 | sean-k-mooney | so schduler/placment is really the only thing that know if that is ok or not | |
| 13:56:40 | sean-k-mooney | like isolated_aggrate/tenating isolation filter/host aggreate metadta ectra | |
| 13:56:47 | bauzas | gibi: there could be a corner case tho | |
| 13:56:51 | gibi | yeah, we cloud do a check in the api and make a better error message but that is not strictly needed | |
| 13:57:09 | bauzas | gibi: if we leave the scheduler return NoValidHost, | |
| 13:57:24 | bauzas | the RequestSpec would have been modified before | |
| 13:57:32 | gibi | ohh true | |
| 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) | |