| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-19 | |||
| 23:20:46 | mriedem | if we're live migrating or evacuating | |
| 23:21:16 | mnaser | i guess thats why it says todo :> | |
| 23:21:28 | mriedem | heh | |
| 23:21:48 | mriedem | melwitt: ^ a todo to keep in mind if we ever want to use placement allocations to mine data for counting quotas | |
| 23:21:54 | mriedem | we aren't storing the correct user_id for all allocations | |
| 23:23:34 | melwitt | so we should have one claim per allocation or? | |
| 23:24:00 | mriedem | when migrating or evacuating, by default the context is the admin | |
| 23:24:03 | mriedem | b/c those are admin apis | |
| 23:24:25 | mriedem | so the user_id we're storing in the allocation for the instance is from the admin, but the project_id should come from the instance, which is the user | |
| 23:24:35 | melwitt | yeah, I see. guh | |
| 23:28:57 | melwitt | does it maybe work out because allocations are updated by the compute host every update interval? would it auto heal the user/project once we fix it? | |
| 23:29:07 | mriedem | no | |
| 23:29:15 | mriedem | computes don't mess with allocations once you're upgraded to pike | |
| 23:30:20 | mnaser | mriedem: https://bugs.launchpad.net/nova/+bug/1739318 | |
| 23:30:21 | openstack | Launchpad bug 1739318 in OpenStack Compute (nova) "Online data migration context does not contain project_id" [Undecided,New] | |
| 23:30:28 | mriedem | mnaser: thanks | |
| 23:30:31 | melwitt | hm, I thought that's what update_available_resource did | |
| 23:30:39 | mriedem | melwitt: used to did | |
| 23:30:42 | mnaser | also looks like the claim resources which did `project_id = spec_obj.project_id` was moved to scheduler utils | |
| 23:30:44 | melwitt | damn | |
| 23:30:46 | mnaser | so that might make things more challenging to backport. | |
| 23:31:05 | mnaser | (or if you have to solve the user_id one) | |
| 23:36:45 | mnaser | mriedem: so is it time to write a little script to iterate all request specs, and those will null, look up the project_id from instances table and update it again with the project_id in there? | |
| 23:38:15 | mriedem | mnaser: i think we might need that too, but i have a workaround i think we can use for now, | |
| 23:38:27 | mriedem | plus fixing that busted migration routine for people that haven't hit this yet | |
| 23:38:54 | mnaser | mriedem: im working on a small fix for that busted migration routine as it seems pretty trivial | |
| 23:41:43 | mnaser | mriedem: im noticing a lot (most fields) are nullable=True .. can I drop that for project_id or is that a design decision? | |
| 23:42:21 | mnaser | if i cant drop it, i can raise an exception in from_components if context.project_id is none (and add a unit test for that), then fix the layer above it to make sure it always supplies a project_id | |
| 23:42:27 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Workaround missing RequestSpec.project_id when moving an instance https://review.openstack.org/529185 | |
| 23:42:27 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use instance.project_id when creating request specs for old instances https://review.openstack.org/529184 | |
| 23:42:32 | mriedem | mnaser: this is my start ^ | |
| 23:42:45 | mnaser | oh okay :P | |
| 23:43:00 | mnaser | c'mon gerrit | |
| 23:45:05 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Workaround missing RequestSpec.project_id when moving an instance https://review.openstack.org/529185 | |
| 23:45:11 | mriedem | ^ handles the other cases | |
| 23:47:02 | mriedem | tonyb: think we might want to hold up https://review.openstack.org/#/c/529102/ for https://review.openstack.org/529184 | |
| 23:47:20 | mnaser | mriedem: the patch for the fix looks good, but just a question, do you want to drop nullable=True to make sure that it will never save (in case we ever likely run into this again?) | |
| 23:47:39 | mriedem | mnaser: that will require a version bump on the object and isn't something we can backport | |
| 23:47:49 | mriedem | it's something we can do on master, but not critical atm | |
| 23:47:51 | mnaser | ah okay, figured there was a reason behind it | |
| 23:48:27 | mriedem | i'll leave a todo | |
| 23:49:37 | mriedem | mnaser: i don't suppose you have a recreate of this in staging that you can test out with the workaround patch? | |
| 23:50:06 | mnaser | mriedem: i dont think i can recreate this scenario.. we just rebuilt our local dev cloud from scratch a few weeks ago :( | |
| 23:50:11 | mnaser | it was too bad because it was running since newton | |
| 23:50:39 | mriedem | ok, we could probably recreate it though with devstack. create a new instance, delete it's request spec from the db directly, then run the migration routine | |
| 23:50:45 | mriedem | then try to migrate that instance | |
| 23:51:42 | mnaser | mriedem: we probably dont have to get that far, probably seeing project_id non null in request_specs table would probably be enough to show that this bug specifically was resolved | |
| 23:51:58 | openstackgerrit | Merged openstack/nova master: Deduplicate instance.create notification samples https://review.openstack.org/523456 | |
| 23:52:04 | mriedem | true | |
| 23:52:12 | mriedem | i mean, you could just test this in prod, but...i didn't want to ask | |
| 23:52:26 | mnaser | mriedem: i could probably patch up the live migration one only | |
| 23:52:33 | mnaser | since really nothing can break there because its an admin api only | |
| 23:52:54 | mnaser | i wouldnt be able to test the migrate and conductor changes as those are too critical tbh | |
| 23:53:42 | mnaser | by conductor, the conductor manager change that is | |
| 23:55:05 | mriedem | yeah | |
| 23:55:08 | mnaser | oh fun times | |
| 23:55:15 | mnaser | this will conflict in stable/pike | |
| 23:55:46 | mnaser | _get_request_spec_for_select_destinations doesnt exist in stable/pike .. not in my stable/pike | |
| 23:56:03 | mnaser | tasks | |
| 23:57:52 | mriedem | that code is in _find_destination in pike | |
| 23:58:33 | mnaser | ok i see | |
| 23:58:33 | mriedem | https://github.com/openstack/nova/blob/stable/pike/nova/conductor/tasks/live_migrate.py#L264 | |
| #openstack-nova - 2017-12-20 | |||
| 00:00:34 | mnaser | well, here goes nothing | |
| 00:01:05 | mnaser | ok, conductor backup with patch, gonna try a live migration | |
| 00:01:28 | mnaser | status=MIGRATING | |
| 00:01:47 | mnaser | let me watch nova-compute to check everything is ok | |
| 00:02:20 | mnaser | live migrated successfully *without* forcing host, let me try forcing host | |
| 00:03:43 | mnaser | mriedem: fixed for both forced host and unspecified host for live migrations | |
| 00:04:02 | mriedem | sweet | |
| 00:04:05 | mnaser | thank you so much, moving forwards i guess we'll have to find a way to clean this up | |
| 00:04:08 | mriedem | about done with tests for the workaround | |
| 00:06:32 | openstackgerrit | Merged openstack/nova master: Remove the objects which related to the old v2 API implementation https://review.openstack.org/519997 | |
| 00:06:40 | openstackgerrit | Merged openstack/nova master: Remove the objects for describing the extension for v2.1 API https://review.openstack.org/519998 | |
| 00:06:58 | mnaser | found some other VMs in error state which failed to delete ... "{"message": "Field `disabled' cannot be None", "code": 500, "details": " File \"/usr/lib/python2.7/site-packages/nova/compute/manager.py\", line 205," | |
| 00:07:03 | mnaser | investigation round #2 | |
| 00:07:04 | openstackgerrit | Merged openstack/nova master: libvirt: throw NotImplementedError if qga is not responsive when setting password https://review.openstack.org/523828 | |
| 00:07:11 | openstackgerrit | Merged openstack/nova master: Update and complete volume attachments during resize https://review.openstack.org/527228 | |
| 00:08:25 | mriedem | mnaser: service record maybe? | |
| 00:08:33 | mriedem | nova.services table | |
| 00:09:24 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Workaround missing RequestSpec.project_id when moving an instance https://review.openstack.org/529185 | |
| 00:09:30 | mriedem | here is the workaround patch with tests ^ | |
| 00:09:34 | mnaser | http://paste.openstack.org/show/629406/ | |
| 00:09:41 | mnaser | got the full trace back, ill start digging into why | |
| 00:10:07 | mnaser | i guess its a flavor where disabled is none | |
| 00:10:14 | mriedem | yeah... | |
| 00:10:32 | mnaser | instance created at 2015-03-24T09:14:09Z | |
| 00:10:50 | mriedem | so i think in that case, the flavor we're using is pulled out of the embedded flavor stored with the instance | |
| 00:10:59 | mriedem | from the instance_extra table | |
| 00:11:03 | mnaser | most very likely | |
| 00:11:20 | mriedem | not sure why disabled would be null | |
| 00:11:32 | mnaser | let me double check the db | |
| 00:12:01 | mriedem | the disabled column in the db defaults to False | |
| 00:13:29 | mnaser | this is from instance_extra? | |
| 00:14:20 | mriedem | yeah, self.flavor = flavor_payload.FlavorPayload(flavor=instance.flavor) | |
| 00:14:26 | mnaser | http://paste.openstack.org/show/629410/ | |
| 00:14:31 | mriedem | instance.flavor pulls the embedded flavor out of the instance_extra table | |
| 00:15:54 | mnaser | if i recall, the embedded flavor didnt exist before | |
| 00:16:08 | mnaser | could it be possible that when whatever migration ran that did that, it didnt add it? | |
| 00:16:10 | mriedem | we started embedding the flavor in kilo | |
| 00:16:22 | mriedem | flavors were the first online data migration in nova i think | |