| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-17 | |||
| 19:11:45 | mriedem | there may be some random roots growing between buildings downtown | |
| 19:12:07 | cburgess | mriedem Indeed | |
| 19:12:10 | superdan | mriedem: that patch is only changing the rest api which really shouldn't know or care or influence any of the cells stuff below | |
| 19:12:21 | superdan | so maybe I'll recheck it and see if it's the same set of failures or not | |
| 19:32:05 | openstackgerrit | John L. Villalovos proposed openstack/nova master: Use flake8-import-order https://review.openstack.org/435575 | |
| 19:37:12 | openstackgerrit | Sean Dague proposed openstack/nova master: Verify project_id when quotas are checked https://review.openstack.org/435010 | |
| 19:37:12 | openstackgerrit | Sean Dague proposed openstack/nova master: Verify project id for flavor access calls https://review.openstack.org/435432 | |
| 19:55:11 | melwitt | mriedem, superdan: I have a patch up for a blocker migration that operators can get stuck on upgrading from mitaka -> newton if they have soft-deleted records https://review.openstack.org/435546 | |
| 19:55:44 | superdan | oh yeah | |
| 19:55:56 | superdan | mriedem: that one needs to get in | |
| 20:00:14 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Only create vendordata_dynamic ksa session if needed https://review.openstack.org/435563 | |
| 20:00:20 | mriedem | i don't want to actually think today | |
| 20:00:34 | mriedem | that needs to go back to stable/newton? | |
| 20:00:44 | mriedem | because we haven't released https://review.openstack.org/#/c/434928/ yet | |
| 20:01:24 | superdan | it does yeah | |
| 20:01:33 | mriedem | btw, this just again goes to show how much no one is event close to trunk | |
| 20:01:49 | superdan | mriedem: well, it's actually probably not very common | |
| 20:01:59 | superdan | mriedem: but definitely hit-able | |
| 20:02:11 | mriedem | let me read up on the bug report and such | |
| 20:02:33 | superdan | it would only happen if you completely decommissioned compute nodes and never replaced them | |
| 20:03:23 | melwitt | it can also happen if you've ever deleted aggregates or pci devices. it's just that wasn't the case in our real world example | |
| 20:04:09 | superdan | melwitt: yeah, true I guess I forgot about those since the CN was the issue in the bug | |
| 20:05:04 | melwitt | superdan: yeah. I was debating whether to fix it only for compute_nodes or for the other tables too. I figured it's the same problem, and that assumes no apis ever read those soft-deleted records for anything later. I don't think they do | |
| 20:05:20 | mriedem | in the red hat bugzilla i guess | |
| 20:05:31 | mriedem | so i guess, | |
| 20:05:38 | superdan | was the aggregate move to api in ocata? | |
| 20:06:00 | melwitt | I thought it was earlier | |
| 20:06:08 | melwitt | I mean, it's definitely before ocata | |
| 20:06:09 | superdan | melwitt: if so we can't have deleted aggregates | |
| 20:06:11 | mriedem | newton | |
| 20:06:23 | mriedem | https://github.com/openstack/nova/commit/35748e7a9b3445a60522e088e38bf25e6bfef25e | |
| 20:06:30 | mriedem | i was thinking it's odd to update the migration itself, | |
| 20:06:34 | mriedem | but i guess if you're stuck on 330, | |
| 20:06:40 | mriedem | then you can't get past that anyway | |
| 20:06:41 | superdan | I guess that move happens after the sync though, if they both happened in newton | |
| 20:06:52 | superdan | mriedem: yeah | |
| 20:07:05 | superdan | mriedem: the other option is to convert all the deleted records, but that's kinda silly | |
| 20:07:18 | melwitt | there's a REST API for deleting an aggregate https://developer.openstack.org/api-ref/compute/?expanded=delete-aggregate-detail#delete-aggregate | |
| 20:07:22 | superdan | especially if we're about to delete them in the case of aggregates | |
| 20:07:31 | melwitt | so if that connects to an Aggregate.destroy(), that would do it | |
| 20:07:34 | superdan | melwitt: right, but once they're in the api db they can;t be soft deleted | |
| 20:07:43 | melwitt | ah, that's what you meant. yeah | |
| 20:07:55 | mriedem | so in mitaka i deleted a compute node, | |
| 20:08:14 | mriedem | actually i need to see the online migration code first | |
| 20:08:25 | melwitt | yeah, I have links up still I think | |
| 20:08:49 | melwitt | the online migration only migrates non-soft-deleted things and the blocker doesn't filter on deleted=0 so it just gets everything | |
| 20:09:00 | mriedem | what does https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L595 have to do with compute nodes? | |
| 20:09:36 | mriedem | you guys got some out of tree online migrations? | |
| 20:09:58 | mriedem | or is it just this? https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L597 | |
| 20:10:03 | mriedem | db.aggregate_uuids_online_data_migration, | |
| 20:10:07 | mriedem | that's the only mitaka one in that list | |
| 20:10:29 | mriedem | i should be looking at stable/newton i guess https://github.com/openstack/nova/blob/stable/newton/nova/cmd/manage.py#L786 | |
| 20:10:32 | melwitt | no, it looks like we removed the compute nodes one | |
| 20:10:33 | mriedem | which includes pci devices | |
| 20:10:37 | melwitt | after | |
| 20:10:40 | melwitt | yeah | |
| 20:11:40 | mriedem | oh nvm i see https://github.com/openstack/nova/blob/stable/mitaka/nova/cmd/manage.py#L930 | |
| 20:11:48 | mriedem | in mitaka we had an online migration for compute nodes | |
| 20:11:49 | mriedem | for the uuid | |
| 20:12:27 | mriedem | https://github.com/openstack/nova/blob/stable/mitaka/nova/db/sqlalchemy/api.py#L6531 | |
| 20:12:50 | mriedem | so that online migration can get soft deleted compute nodes with no uuid | |
| 20:12:52 | mriedem | right? | |
| 20:13:31 | melwitt | the online migration does not migrate soft-deleted compute nodes with no uuid | |
| 20:13:56 | melwitt | so I thought our choices are: migrate even soft-deleted things or skip soft-deleted things in the blocker | |
| 20:14:04 | mriedem | because of model_query | |
| 20:14:05 | mriedem | i suppose | |
| 20:14:07 | melwitt | yeah | |
| 20:14:16 | melwitt | model_query defaults to read_deleted='no' | |
| 20:14:18 | mriedem | and the context doesn't have read_deleted='yes' or whatever | |
| 20:14:19 | mriedem | ok | |
| 20:14:20 | mriedem | yup | |
| 20:14:55 | mriedem | ok so in mitaka we have soft deleted compute_nodes, online data migration doesn't get them b/c of model_query, but 330 blocked migration pulls them in b/c it's not using the orm and it explodes | |
| 20:15:01 | mriedem | because we don't filter on deleted==0 | |
| 20:15:07 | melwitt | the only gotcha would be if it were valid for any of those objects: compute nodes, aggregates, pci devices to be read in a deleted state | |
| 20:15:14 | mriedem | which is your fix | |
| 20:15:19 | melwitt | right | |
| 20:15:46 | mriedem | geez melwitt your commit message could have been several more paragraphs of explanation :) | |
| 20:16:11 | melwitt | because this fix would leave the soft-deleted unmigrated records around. the alternate fix would migrate potentially a lot of soft-deleted things for no reason | |
| 20:16:40 | mriedem | we don't read deleted on any of those records as far as i know | |
| 20:16:48 | melwitt | mriedem: yeah, sorry about that. I can add more info to it | |
| 20:16:56 | mriedem | melwitt: i'm only half joking | |
| 20:16:57 | mriedem | :) | |
| 20:16:58 | melwitt | yeah, that's what I thought too | |
| 20:17:08 | mriedem | superdan: do you know if we read deleted any of those? | |
| 20:17:21 | superdan | on compute nodes? | |
| 20:17:32 | melwitt | aggregates and pci devices also | |
| 20:17:38 | mriedem | i have like 1% knowledge of the pci_devices code | |
| 20:17:49 | superdan | definitely not pci_devices | |
| 20:18:32 | superdan | melwitt: if we did read aggregates, we'd generate on load: https://github.com/openstack/nova/blob/stable/newton/nova/objects/aggregate.py#L288 | |
| 20:18:34 | superdan | so we're okay | |
| 20:18:57 | melwitt | ah, nice | |
| 20:20:38 | mriedem | we mostly read deleted instances | |
| 20:20:43 | mriedem | so yeah i think we're ok there | |
| 20:21:18 | mriedem | and flavors | |
| 20:24:23 | mriedem | just testing the test quick | |
| 20:24:28 | mriedem | do we want/need a release note for this? | |
| 20:24:32 | mriedem | since it's going to be backported | |
| 20:24:54 | mriedem | like even a "fixes" note or "upgrade" note? as i said earlier, if you hit this you can't get past 330 anyway | |
| 20:25:02 | mriedem | so this is more about fixing it before people hit it | |
| 20:25:23 | mriedem | last time we backported a db schema migration to stable and released it i got some flak from red hat | |