| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-19 | |||
| 19:52:05 | mriedem | however, since we don't bury it, and the build request is gone, and we didn't create the instance, we could have an orphaned instance mapping, yeah? | |
| 19:52:33 | jgwentworth | no, just didn't want to lose my other nick from not using it | |
| 19:54:31 | melwitt | concatenating not working | |
| 19:54:43 | mriedem | ok, was going to ping you on ^ as well | |
| 19:55:02 | melwitt | what you're saying makes sense though | |
| 19:55:44 | melwitt | interestingly recently I was trying to figure out how an instance could possibly be in both cell0 and cell1 in a single cell env, since that's happened to rdo cloud several times. but I found no way | |
| 19:56:05 | melwitt | so if you saw how that could be, lmk | |
| 19:59:02 | dansmith | mriedem: maybe? | |
| 19:59:19 | melwitt | so, we create instance mapping with cell_id=NULL in compute/api. so if we get to conductor and no build request found, we add a placeholder to the instance list | |
| 20:00:15 | melwitt | we don't create an instance record | |
| 20:01:02 | mriedem | right we'd skip those here https://github.com/openstack/nova/blob/74aebe0d4e5a978a40011e890aee9e70e98246c4/nova/conductor/manager.py#L1391 | |
| 20:01:29 | dansmith | yeah was just reading that loop | |
| 20:01:40 | dansmith | we assume that if instance is none that it's because we buried it already I think | |
| 20:01:42 | mriedem | while i'm looking at this code, we should probably make this fatal now :) https://github.com/openstack/nova/blob/74aebe0d4e5a978a40011e890aee9e70e98246c4/nova/conductor/manager.py#L1238 | |
| 20:01:58 | melwitt | we delete all the stuff if quota recheck fails. but if someone has requested a delete of the thing, wouldn't the compute/api delete the instance mapping? I can't remember where we do that for delete | |
| 20:02:03 | mriedem | dansmith: or it's already been deleted via the build request before we got that far | |
| 20:02:23 | mriedem | melwitt: no we don't delete the instance mapping when deleting the instance (or build request) in the api | |
| 20:02:41 | mriedem | that's why surya had to add it to the archive_deleted_rows cmd | |
| 20:02:51 | dansmith | mriedem: yeah, we get instance=None if the build request failed, do we get it for other reasons? | |
| 20:03:04 | melwitt | oh, ever? if we never do, then is it an issue? | |
| 20:03:06 | dansmith | ohh, | |
| 20:03:08 | dansmith | I see, | |
| 20:03:09 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add extra spec parameter and image property for memory encryption https://review.opendev.org/664420 | |
| 20:03:18 | dansmith | we bury it right away if we get hostmappingnotfound | |
| 20:03:33 | dansmith | but if we get instance mapping not found, we don't bury, but instances.append(None) | |
| 20:03:40 | dansmith | the loop at the bottom assumes the former behavior | |
| 20:04:07 | dansmith | the latter looks like it was added for quota counting reasons | |
| 20:04:22 | melwitt | yeah, it was | |
| 20:04:26 | mriedem | dansmith: you mean build request not found | |
| 20:04:34 | melwitt | the append None thing | |
| 20:04:36 | mriedem | https://github.com/openstack/nova/blob/74aebe0d4e5a978a40011e890aee9e70e98246c4/nova/conductor/manager.py#L1350 | |
| 20:04:42 | openstackgerrit | Adam Spiers proposed openstack/nova master: Use fake flavor instead of empty dict in test https://review.opendev.org/662555 | |
| 20:04:43 | openstackgerrit | Adam Spiers proposed openstack/nova master: Pass extra_specs to flavor in vif tests https://review.opendev.org/662556 | |
| 20:04:43 | openstackgerrit | Adam Spiers proposed openstack/nova master: Extract SEV-specific bits on host detection https://review.opendev.org/636334 | |
| 20:04:44 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add |
|
| 20:04:44 | openstackgerrit | Adam Spiers proposed openstack/nova master: Allow guest devices to include |
|
| 20:04:45 | openstackgerrit | Adam Spiers proposed openstack/nova master: Detect that SEV is required and enable iommu for devices https://review.opendev.org/644565 | |
| 20:04:45 | openstackgerrit | Adam Spiers proposed openstack/nova master: Use |
|
| 20:04:46 | openstackgerrit | Adam Spiers proposed openstack/nova master: Enable memory locking if SEV is requested https://review.opendev.org/662558 | |
| 20:04:48 | dansmith | mriedem: yeah sorry | |
| 20:04:56 | dansmith | mriedem: yeah that was melwitt's main counting quotas patch | |
| 20:05:53 | dansmith | which added the instances.append(None) for the host not found bury case as well | |
| 20:06:04 | dansmith | https://github.com/openstack/nova/commit/5c90b25e49d47deb7dc6695333d9d5e46efe8665#diff-378a96ec6159d0a2f8ec7ab71bc3843bR946 | |
| 20:06:40 | dansmith | although it looks like we were skipping those both anyway | |
| 20:07:11 | mriedem | yeah, and at some point in talking about https://bugs.launchpad.net/nova/+bug/1784074 we had talked about squashing those two for loops into one again, but i've lost track of the discussion about that (though i think i know how to find it) | |
| 20:07:12 | openstack | Launchpad bug 1784074 in OpenStack Compute (nova) "Instances end up with no cell assigned in instance_mappings" [Medium,In progress] - Assigned to Matt Riedemann (mriedem) | |
| 20:07:15 | openstackgerrit | Eric Fried proposed openstack/nova master: Clarify --before help text in nova manage https://review.opendev.org/661289 | |
| 20:07:18 | mriedem | i'm not really signing up to do that refactor now though, | |
| 20:07:28 | dansmith | yeah | |
| 20:07:32 | mriedem | just thinking it makes sense for us to delete the instance mapping in this block https://github.com/openstack/nova/blob/74aebe0d4e5a978a40011e890aee9e70e98246c4/nova/conductor/manager.py#L1350 | |
| 20:07:45 | dansmith | I think we should just bury in that case, no? | |
| 20:07:47 | mriedem | b/c if the BR is gone, and the instance isn't created in a cell yet, the instance mapping shouldn't exist either - it's orphaned | |
| 20:07:54 | melwitt | I've been wanting ("wanting") to do that refactor but, you know how it goes | |
| 20:07:55 | mriedem | burying it would bring it back | |
| 20:08:00 | mriedem | from the br zombieland | |
| 20:08:16 | dansmith | mriedem: right, but isn't that the expectation of the api user? | |
| 20:08:17 | melwitt | but how does that hurt anything? if we never delete instance mappings? | |
| 20:08:24 | dansmith | or is this only in the multi-create case? | |
| 20:08:52 | melwitt | *if we never delete instance mappings anyway? | |
| 20:09:07 | dansmith | for single create, once you get back a uuid you expect to be able to list that thing (with deleted=yes) until archive | |
| 20:09:24 | dansmith | but I guess maybe if you've deleted it it could be up for immediate archive and thus deleting the mapping is equivalent | |
| 20:09:26 | dansmith | so yeah, either I guess | |
| 20:09:42 | mriedem | melwitt: it could be a problem in a tight window if you got that mapping via build request as a marker when listing/paging | |
| 20:09:54 | melwitt | oh, I see | |
| 20:10:20 | mriedem | dansmith: if you delete the build request before it's an instance, i don't think we honor reading it as deleted=yes anyway, | |
| 20:10:24 | mriedem | because BRs are hard deleted | |
| 20:10:50 | mriedem | melwitt: at least that was part of the reasoning behind https://review.opendev.org/#/c/575556/ | |
| 20:11:13 | dansmith | mriedem: ack yeah I guess I remember that bit of cheating | |
| 20:11:51 | mriedem | it would have been like newton-era talk about that cheating being ok, likely in a small hotel conf room in hillsboro | |
| 20:11:53 | mriedem | if i remember correctly | |
| 20:12:00 | mriedem | with jpenick squashed on the floor in a corner | |
| 20:12:07 | melwitt | lol | |
| 20:12:27 | dansmith | yar | |
| 20:12:44 | mriedem | the good old days... | |
| 20:12:54 | melwitt | it was wharm in there | |
| 20:12:56 | mriedem | anyway, i'm about to abandon https://review.opendev.org/#/c/575556/ but was just looking at my last comment there | |
| 20:13:11 | mriedem | melwitt: unless you were directly underneath where the air shot out and down on you, which was my case | |
| 20:13:18 | mriedem | then it was cold and contact-drying | |
| 20:13:25 | melwitt | lol! | |
| 20:13:30 | melwitt | well I'll be | |
| 20:32:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Delete InstanceMapping in conductor if BuildRequest is already deleted https://review.opendev.org/666438 | |
| 20:35:29 | openstackgerrit | melanie witt proposed openstack/nova master: rbd: use MAX_AVAIL stat for reporting bytes available https://review.opendev.org/556692 | |
| 20:40:47 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add functional test for AggregateMultiTenancyIsolation + migrate https://review.opendev.org/571265 | |
| 21:00:13 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: don't log error if guest gone during interface detach https://review.opendev.org/610727 | |
| 21:04:31 | mriedem | relatively simple older patch here that has a +2 on it - it's mostly docstring https://review.opendev.org/#/c/633212/ | |
| 21:28:27 | sean-k-mooney | mriedem: just back after dinner ill take a look at https://review.opendev.org/#/c/644881/ now | |
| 21:31:13 | mriedem | i'll be gone in 30 minutes | |
| 21:31:56 | sean-k-mooney | mriedem: the use of hybrid plug is based on the local config on the compute host which is read by the agent and hybrid plug amoung other things is stored in the neutron db | |
| 21:32:15 | sean-k-mooney | so when the ml2 driver bind the port to the hsot is pass an agent context like our host_state object | |
| 21:32:19 | mriedem | mnaser: figured out why my recreate test in https://review.opendev.org/#/c/647603/ wasn't failing | |
| 21:32:26 | sean-k-mooney | which it check to determin if hybid plug should be used | |
| 21:32:31 | mriedem | mnaser: only took a couple of months | |
| 21:32:46 | mriedem | sean-k-mooney: but that's per neutron agent right? | |
| 21:32:53 | sean-k-mooney | yes | |
| 21:33:03 | mriedem | the commit message makes it sound like if you're ovs hybrid plug, it's deployment-wide | |
| 21:33:23 | sean-k-mooney | it does. ill comment on the comment and correct it | |
| 21:33:46 | sean-k-mooney | for tools like triplo it typeiclay configred globally | |
| 21:33:54 | sean-k-mooney | but it actully set per host | |
| 21:34:49 | sean-k-mooney | at bind time the ml2 driver is passed the agent context for the agent on the host that nova selected and it determins what to do based on the configurtion of that host | |
| 21:34:53 | mnaser | mriedem: what object should it be? | |