| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-27 | |||
| 18:39:27 | mriedem | oh which reminds me, when do we clear the queued_for_delete flag once it's set.... | |
| 18:42:10 | mriedem | uh, i think we might just not want to do queued_for_delete at all, | |
| 18:42:31 | mriedem | and 500 or 503 if you try to delete an instance in a down cell, like any other action on a server that's in a down cell like PUT /servers/{id} | |
| 18:49:04 | mriedem | random aside - can we delete this compat code from RT now? https://github.com/openstack/nova/blob/c8b93fa2493dce82ef4c0b1e7a503ba9b81c2e86/nova/compute/resource_tracker.py#L1206 | |
| 18:49:16 | mriedem | if you're in rocky, you shouldn't have ocata computes | |
| 18:49:57 | melwitt | mriedem: well, blocking a delete is new and undesirable behavior that we haven't had before. that's my first thought | |
| 18:50:13 | mriedem | it's not new if your database is down | |
| 18:50:16 | mriedem | which is the case here | |
| 18:51:14 | mriedem | and it's not like once the cell computes come back up they can up-call to the instance mappings table to see, oh while i was dead the user wanted to delete this instance | |
| 18:51:23 | melwitt | so the pre cells v2 behavior when nova-api is up but the database is down is what, a 500? | |
| 18:51:31 | mriedem | melwitt: i'm sure | |
| 18:52:39 | mriedem | queued_for_delete means, to me anyway, that we have some kind of periodic in the control plane services that is trying to hit the cells for instance mappings that are queued for delete and once it reaches a cell that's back up, it does the actual delete | |
| 18:52:42 | mriedem | that's not being proposed | |
| 18:53:36 | mriedem | sort of like how the _reclaim_queued_deletes periodic works in the nova-compute service for SOFT_DELETED instances | |
| 18:53:41 | melwitt | yeah, I get where you're coming from on that. and I'm trying to remember why then we wanted queued_for_delete in the first place. the ability to list/show "deleted" instance shells? since 'novs list --deleted' is a thing you can do | |
| 18:54:01 | melwitt | *nova list --deleted | |
| 18:54:11 | mriedem | maybe, but the spec calls it out as being needed for quota calculation | |
| 18:54:51 | melwitt | okay, I don't think it was supposed to, that is, we had thought we could piggyback the quota stuff on that column. the column itself had other purpose prior to that | |
| 18:55:05 | mriedem | well i guess it's not explicitly linking that in, but it says it's needed for nova list, nova show and nova boot | |
| 18:55:43 | mriedem | "``nova boot`` should not succeed if that project has any living VMs in the | |
| 18:55:43 | mriedem | down cell until an all-cell-iteration independent solution for quota | |
| 18:55:43 | mriedem | calculation is implemented through `quotas using placement`_." | |
| 18:55:53 | melwitt | yeah. my guess is that it's for the case where an instance was deleted in the past from some cell, then that cell goes down, then someone does 'nova list --deleted', we'd have no way of knowing deleted or not without access to the cell database | |
| 18:56:02 | mriedem | *living* VMs means, i assume, that queued_for_delete would be considered there during the quota calculation | |
| 18:57:25 | melwitt | because if an instance was deleted from a cell in the past, if the cell goes down, without queued_for_delete the instance would reappear in the nova list | |
| 18:57:33 | melwitt | if we're going off instance mappings and nothing else | |
| 18:58:52 | mriedem | so by that logic, we'd always need to set InstanceMapping.queued_for_delete=True when deleting an instance even if the cell is up and we actually do delete the instance from the cell | |
| 18:59:43 | melwitt | yeah, I thought that was how it's supposed to work. mark queued_for_delete=True when a delete request comes in, and that just means "the user intended to delete this" and does not indicate success or not | |
| 19:02:23 | mriedem | ok, and we never set it back to queued_for_delete=False? | |
| 19:02:28 | mriedem | that's what is bothering me | |
| 19:02:29 | melwitt | and that "queued" was a misnomer in that there's no queuing system at the API level for deletes. maybe better named "delete_requested" or something. guess we should double check with tssurya and dansmith. | |
| 19:02:35 | melwitt | I don't think so, no | |
| 19:03:13 | mriedem | we've said in the spec that we're just not going to include instances from down cells if there are filter parameters, | |
| 19:03:30 | mriedem | we could just consider 'nova list --deleted' as a filter parameter that we're not going to include results from down cells | |
| 19:03:34 | mriedem | b/c we don'tknow | |
| 19:04:54 | melwitt | yeah, that's true. but as we talk I'm realizing I think the case we are trying to avoid is, deleted instance reappearing in a plain 'nova list' | |
| 19:05:07 | melwitt | or wait, | |
| 19:05:20 | melwitt | you're saying omit that one because it's in a down cell | |
| 19:05:32 | melwitt | yeah, I dunno. | |
| 19:05:57 | melwitt | the original thinking was show instance "shells" for instances in down cells | |
| 19:06:18 | mriedem | yes which we still plan on doing | |
| 19:06:21 | melwitt | we need to know when not to show a shell if it's deleted or was intended to be deleted | |
| 19:06:32 | melwitt | that's where queued_for_delete comes in, I think | |
| 19:08:29 | mriedem | https://review.openstack.org/#/c/557369/5/specs/rocky/approved/handling-down-cell.rst@103 | |
| 19:08:49 | mriedem | "But then it's weird if I issue DELETE /servers/{id}, get a 202, and then my server status is always UNKNOWN and eventually goes to ACTIVE once the cell is back up - we aren't really doing any queuing in that case." | |
| 19:09:14 | mriedem | so maybe we need to: | |
| 19:09:32 | mriedem | actually nvm | |
| 19:10:13 | mriedem | was going to say, always set queued_for_delete on any delete request, but if the cell is down and we don't delete the instance, the api is going to start hiding it | |
| 19:10:25 | mriedem | and then it's going to show up again once the cell is back | |
| 19:10:45 | mriedem | i want to say: | |
| 19:10:58 | mriedem | 1. delete fails if the cell is down as it would today, don't touch queued_for_delete | |
| 19:11:07 | melwitt | yeah, I thought that's what the deal was going to be though. queued_for_delete was supposed to be a minimal effort to show something correct when cells are down, AFAIK | |
| 19:11:14 | mriedem | 2. if we actually get the instance and queue a delete of the instance and the cell is up, flip queued_for_delete=True | |
| 19:11:45 | melwitt | and to not have deleted instances reappear in nova list once a cell goes down | |
| 19:12:08 | mriedem | i think you can only do that if you actually issued an actual working delete request | |
| 19:12:15 | mriedem | either local in the api or cast to the compute | |
| 19:12:16 | mriedem | but the cell is up | |
| 19:12:46 | mriedem | otherwise we're lying if we say it's queued for delete while the cell is down and never actually delete it once the cell comes back up | |
| 19:13:07 | mriedem | i think DELETE /servers/{id} has to 500 if the cell is down | |
| 19:13:08 | melwitt | okay, yeah I agree with that. so you're just saying wait to set queued_for_delete until a delete is actually initiated with an up cell | |
| 19:13:17 | mriedem | yeah i think so | |
| 19:13:25 | melwitt | that's fair enough | |
| 19:13:40 | mriedem | because in that case we should have actually deleted the instance in the cell | |
| 19:13:54 | melwitt | right | |
| 19:14:08 | mriedem | and then queued_for_delete is only read while listing/showing over down cells | |
| 19:14:17 | mriedem | and never goes from True back to False | |
| 19:14:31 | melwitt | ++ | |
| 19:17:39 | melwitt | I'm gonna try to do a check with devstack to verify what is returned for a delete when the database is down | |
| 19:33:20 | mriedem | i need a palate cleanser | |
| 19:40:24 | dansmith | mriedem: before I go write examples in the docs that don't satisfy what you're looking for, can you review my response here first? https://review.openstack.org/#/c/578163/2/doc/source/user/cells.rst | |
| 19:44:51 | mriedem | never satisfied | |
| 19:44:53 | mriedem | replied | |
| 19:45:02 | mriedem | i've heard that snickers really satisfies | |
| 19:54:53 | openstackgerrit | Dan Smith proposed openstack/nova master: Allow templated cell_mapping URLs https://review.openstack.org/578163 | |
| 20:02:38 | SpamapS | mriedem: we are very far from needing multi-cell yet. Our only Pike clouds are greenfield new deployments. We'll move our bigger clouds last, and will likely be on Rocky+ by then. | |
| 20:05:21 | mriedem | ok | |
| 20:06:23 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use consumer generation in _heal_allocations_for_instance https://review.openstack.org/577905 | |
| 20:15:06 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Wait for vif plugging during live migration job https://review.openstack.org/578551 | |
| 20:36:27 | openstackgerrit | Rick Bartra proposed openstack/nova master: Add granularity to os-services API https://review.openstack.org/578553 | |
| 21:21:43 | Nova | Hi Eric, please ping me when you can | |
| 21:22:33 | Sundar | efried: Please ping me when you have a moment | |
| 21:28:16 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Make nova-lvm run in check on libvirt changes and compute API tests https://review.openstack.org/569149 | |
| 21:30:54 | openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Usages per project and user (v1.8, v1.9) https://review.openstack.org/514646 | |
| 21:30:55 | openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: CLI allocation candidates (v1.10) https://review.openstack.org/514647 | |
| 21:30:56 | openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: New dict format of allocations (v1.11, v1.12) https://review.openstack.org/542819 | |
| 21:30:57 | openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Transactionally update allocations (v1.13) https://review.openstack.org/546674 | |
| 21:30:58 | openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Add nested resource providers (v1.14) https://review.openstack.org/546675 | |
| 21:30:59 | openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Limit allocation candidates (v1.15, v1.16) https://review.openstack.org/548043 | |
| 21:31:00 | openstackgerrit | Matt Riedemann proposed openstack/osc-placement master: Allocation candidates parameter: required (v1.17) https://review.openstack.org/548326 | |
| 21:31:15 | efried | Sundar: Hello | |
| 21:32:28 | Sundar | efried: Can you please try out https://drive.google.com/open?id=1ICdNeG3bbX4l-Qkj1Q8IzF8QmLsn4U7K ? | |
| 21:32:54 | Sundar | This patch adds the sequence diag | |
| 21:33:52 | efried | Sundar: Okay. We're still busted on the doc build in general, but I can try to build it locally. At a glance, I believe you might need semicolons at the end of each line. | |
| 21:43:27 | openstackgerrit | Merged openstack/nova master: libvirt: remove old rbd snapshot removal error handling https://review.openstack.org/566369 | |
| 21:57:21 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (1) https://review.openstack.org/572018 | |
| 21:57:30 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (2) https://review.openstack.org/573481 | |
| 21:57:37 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (3) https://review.openstack.org/574104 | |
| 21:57:46 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (4) https://review.openstack.org/574106 | |
| 21:57:53 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (5) https://review.openstack.org/574110 | |
| 22:06:24 | openstackgerrit | Merged openstack/nova master: Handle network-changed event for a specific port https://review.openstack.org/465787 | |