| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-05 | |||
| 19:45:21 | mriedem | the callback method might be a pita | |
| 19:52:05 | mriedem | eww this gets into the whole soft delete mess | |
| 19:52:16 | mriedem | but i think we already figured out that you can't soft delete an instance in cell0 | |
| 19:52:45 | mriedem | remember getting caught up in this on the notification patch | |
| 19:52:46 | dansmith | correct | |
| 19:53:03 | dansmith | because there's no way to undelete it | |
| 19:53:14 | mriedem | in the 'normal' flow we set the task_state to 'deleting' before cleaning up bdms and all that | |
| 19:54:28 | mriedem | w/o that we just go from ERROR/None to DELETED/None to destroyed | |
| 19:57:09 | mriedem | god also https://github.com/openstack/nova/blob/master/nova/compute/api.py#L1913-L1930 | |
| 19:58:26 | mriedem | https://review.openstack.org/#/c/61462/ | |
| 19:59:20 | mriedem | so because someone thought we should deleted the shelved snapshot image when the instance is deleted, we did | |
| 20:05:02 | mriedem | this might be easier than i think for shelved offloaded. | |
| 20:10:13 | mriedem | because if the instance is shelved offloaded, it's in a cell | |
| 20:10:26 | mriedem | so the if not instance.host check at the very beginning is just not considering that case | |
| 20:10:32 | mriedem | and assumes the instance never got to a cell | |
| 20:12:11 | dansmith | if it's offloaded it's in a cell yeah | |
| 20:12:37 | mriedem | this is literally the fix: | |
| 20:12:40 | mriedem | instance.vm_state == vm_states.SHELVED_OFFLOADED): | |
| 20:12:40 | mriedem | if not (instance.host or | |
| 20:13:06 | melwitt | yeah, later code does "if not instance.host and not shelved_offloaded:" | |
| 20:13:10 | openstackgerrit | Eric Fried proposed openstack/nova master: PowerVM Driver: SSP emphemeral disk support https://review.openstack.org/443189 | |
| 20:13:42 | melwitt | and vm_states.ERROR should be in there too, which is what the patch I've had up for ages does | |
| 20:14:21 | mriedem | so i was trying to think of what we miss now in the first local delete block for cells v2 where instance.host is None | |
| 20:14:24 | mriedem | and it's not shelved offloade | |
| 20:14:28 | mriedem | i think that's just the cell0 case | |
| 20:15:05 | mriedem | where i don't think we need to worry about cleaning up bdms, because conductor doesn't create the bdms in cell0 | |
| 20:15:48 | mriedem | _bury_in_cell0 creates the instance in cell0, then deletes the build request | |
| 20:23:37 | mriedem | crap we might have this same issue in newton https://github.com/openstack/nova/blob/stable/newton/nova/compute/api.py#L1682 | |
| 20:31:23 | openstackgerrit | Scott Moser proposed openstack/nova master: Limit exposure of network device types to the guest. https://review.openstack.org/400883 | |
| 20:31:33 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Regression test for local delete with an attached volume https://review.openstack.org/449334 | |
| 20:31:34 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Perform old-style local delete for shelved offloaded instances https://review.openstack.org/453859 | |
| 20:31:34 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Set size/status during image create with FakeImageService https://review.openstack.org/449702 | |
| 20:31:44 | mriedem | thar she blar | |
| 20:32:56 | dtp | mriedem how do you guys do rollbacks w/o downgrade on the migrations? | |
| 20:33:03 | mriedem | dtp: we dont | |
| 20:33:22 | mriedem | roll forward to the maxxx | |
| 20:33:33 | mriedem | i usually do the dew while rolling forward | |
| 20:34:07 | dtp | nova-manage db sync supports downgrade | |
| 20:34:31 | mriedem | it might say it does | |
| 20:34:35 | mriedem | which would be a bug | |
| 20:34:57 | dtp | i think it actually does....or else my upgrade is already idempotent | |
| 20:35:18 | dtp | because 358 > 359 > 358 > 359 works | |
| 20:35:40 | mriedem | well it works for your migration b/c you implemented downgrade | |
| 20:36:09 | mriedem | as i said in the review, i'm surprised we didn't have a test that would fail saying we don't support downgrade | |
| 20:36:18 | dtp | ok ok | |
| 20:36:38 | dtp | thanks for the review, btw | |
| 20:37:12 | mriedem | np | |
| 20:37:57 | mriedem | dtp: so i guess we weren't idempotent when adding the compute_nodes.uuid column either https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/migrate_repo/versions/314_add_resource_provider_tables.py#L78 | |
| 20:38:43 | mriedem | but we should be, e.g. https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/migrate_repo/versions/358_bdm_attachment_id.py#L27 | |
| 20:39:24 | dtp | makes sense | |
| 20:40:04 | mriedem | dtp: btw, when you said nova-manage db sync supports downgrade, you mean by passing in the --version right? | |
| 20:40:37 | dtp | yep | |
| 20:40:44 | mriedem | yeah i think if you tried going further, it'd explode | |
| 20:42:01 | dtp | surely | |
| 20:43:39 | mriedem | huh clearly test_no_migrations_have_downgrade failed it's job | |
| 20:43:42 | edleafe | OK, what am I doing wrong? I'm making a commit to the os-traits library. I have git-review installed, and a .gitreview file in the directory, just as in my nova project. But when I commit, I'm not getting a Change-Id added to the commit message. | |
| 20:43:58 | mriedem | edleafe: git review -s | |
| 20:44:02 | mriedem | in the repo | |
| 20:44:12 | mriedem | you probably don't have the gerrit remote | |
| 20:44:31 | edleafe | mriedem: that worked, but I did have the remote | |
| 20:44:45 | mriedem | maybe git review -s installs a hook or something, idk | |
| 20:45:23 | edleafe | dunno, but thanks for setting me straight | |
| 20:46:44 | melwitt | edleafe: you have to do git review -s anytime you clone a new repo, fwiw | |
| 20:47:58 | edleafe | melwitt: yeah, I thought I had, but I guess not... | |
| 20:48:02 | edleafe | thanks | |
| 20:51:36 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif master: Updated from global requirements https://review.openstack.org/451049 | |
| 20:57:51 | cdent | edleafe: that os-traits stuff you did is very satisfying | |
| 21:00:33 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix test_no_migrations_have_downgrade https://review.openstack.org/453872 | |
| 21:07:39 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/453896 | |
| 21:08:58 | edleafe | cdent: a lot easier on the eyes, no? | |
| 21:09:05 | cdent | quite | |
| 21:20:27 | openstackgerrit | Dan Peschman proposed openstack/nova master: Add uuid to Service model https://review.openstack.org/453025 | |
| 21:21:04 | mriedem | oh dansmith that reminds me, did the functional tests for https://review.openstack.org/#/c/438025/ work? | |
| 21:21:10 | mriedem | oh https://review.openstack.org/#/c/452006/2 | |
| 21:21:53 | dansmith | yeah I think I got it working | |
| 21:22:26 | mriedem | you should take that off the end of the series | |
| 21:22:33 | mriedem | since it's testing something that's already merged | |
| 21:22:37 | mriedem | we can just work that in parallel | |
| 21:56:40 | dansmith | mriedem: sure | |
| 21:58:32 | mriedem | dansmith: there are some unnecessary changes in https://review.openstack.org/#/c/439891/ i think, so i'll let you check the comments, but otherwise i'm +2 | |
| 22:02:43 | dansmith | mriedem: yeah they're all rebase noise, I'll fix | |
| 22:02:53 | mriedem | cool | |
| 22:06:28 | mriedem | where does the compute call the scheduler on init if not _update_scheduler_instance_info? | |
| 22:06:50 | dansmith | there, | |
| 22:07:04 | dansmith | which is a synchronous call all the way into scheduler, | |
| 22:07:07 | dansmith | which will look up hostmapping | |
| 22:07:10 | dansmith | and fail | |
| 22:07:14 | dansmith | if we haven't created it yet | |
| 22:07:32 | mriedem | that's an rcp cast | |
| 22:07:35 | mriedem | *rpc | |
| 22:07:54 | dansmith | not if we have CastAsCall | |
| 22:08:00 | mriedem | oh | |
| 22:08:01 | mriedem | heh | |
| 22:08:04 | dansmith | which is the devil | |
| 22:10:39 | mriedem | alright that's fun | |
| 22:10:42 | mriedem | left a comment to remind myself | |
| 22:11:07 | mriedem | also, i see that none of that compute -> scheduler stuff is necessary if you're not using the filter scheduler, but meh | |
| 22:12:59 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/453896 | |
| 22:16:27 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Mention track_instance_changes in scheduler.driver conf help https://review.openstack.org/453916 | |
| 22:35:29 | openstackgerrit | Dan Smith proposed openstack/nova master: Add functional test for two-cell scheduler behaviors https://review.openstack.org/452006 | |