| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-27 | |||
| 18:27:01 | openstackgerrit | Merged openstack/python-novaclient master: Allow tuple as for nics value https://review.openstack.org/475816 | |
| 18:27:01 | jaypipes | guys, won't https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1060 skip instances that are currently being mnoved? | |
| 18:27:02 | dansmith | mriedem: not if you'reon the late end of the race | |
| 18:27:15 | dansmith | currently being moved is not the problem | |
| 18:27:23 | dansmith | "just moved a half second ago" is the problem right? | |
| 18:27:35 | jaypipes | but instance.host will be not None. | |
| 18:27:48 | jaypipes | the only time instance.host is None is when the instance is deleted. | |
| 18:28:38 | dansmith | jaypipes: actually not | |
| 18:28:50 | dansmith | jaypipes: that's reversed.. None means "not yet booted".. it's still $host after you delete | |
| 18:28:55 | dansmith | it's skipping not yet booted instances | |
| 18:29:09 | dansmith | if the hostname doesn't match you'll continue on to delete it because it's not on our host right? | |
| 18:29:17 | jaypipes | ah, shit. | |
| 18:29:21 | dansmith | since this logic was all based on just accounting for the local host, I don't think it quite works | |
| 18:29:22 | dansmith | plus, | |
| 18:29:32 | dansmith | there is a race between fetchign the instance and its host flipping to the other side | |
| 18:30:09 | mriedem | right the not instance.host is for scheduling instances the first time | |
| 18:30:11 | mriedem | per the comment | |
| 18:30:17 | mriedem | "# Allocations related to instances being scheduled should not be # deleted if we already wrote the allocation previously." | |
| 18:30:22 | dansmith | yeah | |
| 18:30:27 | dansmith | this was the "leafe race" | |
| 18:30:28 | mriedem | "already wrote the allocation previously" == in the scheduler | |
| 18:30:32 | mriedem | right | |
| 18:30:58 | jaypipes | but allocations_to_delete won't contain migrating instances. Because migrating instances are in RT.tracked_instances, no? | |
| 18:31:16 | dansmith | jaypipes: not at the instance after we complete the migration right? | |
| 18:31:47 | mriedem | that i don't know | |
| 18:31:49 | mriedem | b/c it's f'ed | |
| 18:31:50 | dansmith | jaypipes: either way, this is not a big deal: | |
| 18:32:10 | dansmith | just pull the allocation like we do, and if we're not in it, we don't touch it | |
| 18:32:28 | dansmith | and maybe something else about if there are two in there, then don't delete either, although not sure if that's important or not | |
| 18:32:43 | jaypipes | dansmith: sure, that's a fairly safe thing. | |
| 18:33:30 | jaypipes | ok, guys, lemme update the move patch. | |
| 18:33:31 | mriedem | just looking at where this starts from, the periodic is in here https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L667 | |
| 18:33:40 | mriedem | jaypipes: just do it on top of that one since it's in the gate | |
| 18:33:49 | mriedem | we get the instances for the host and node (source) | |
| 18:34:09 | mriedem | clear out all tracked instances https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1030 | |
| 18:34:42 | dansmith | jaypipes: mriedem: ++ for not interrupting the one in the gate | |
| 18:34:48 | mriedem | then we go here https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1044 | |
| 18:34:54 | mriedem | which is where i get lost | |
| 18:35:51 | bauzas | mriedem: what's the concern? | |
| 18:35:52 | mriedem | i think it just gets added here https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L995 | |
| 18:36:01 | mriedem | because we clear self.tracked_instances before calling that method | |
| 18:36:05 | jaypipes | yeah | |
| 18:36:08 | mriedem | so is_new_instance = uuid not in self.tracked_instances will be True | |
| 18:36:16 | mriedem | and then self.tracked_instances[uuid] = obj_base.obj_to_primitive(instance) | |
| 18:36:24 | bauzas | mriedem: you want to know whether migrating instances are in tracked_instances ? | |
| 18:36:36 | dansmith | mriedem: even still, we pull the allocation, we should never just blindly delete it without looking to make sure it's what we expect, which is kinda the point of the generation stuff, but in reverse here | |
| 18:36:48 | jaypipes | bauzas: they aren't. they are. depends on when in the update_available_resource() method you're looking at ;) | |
| 18:36:51 | bauzas | mriedem: if that's the question, I don't think so | |
| 18:37:02 | bauzas | mriedem: because we lookup at all the existing instances | |
| 18:37:03 | jaypipes | dansmith: ack | |
| 18:37:07 | mriedem | dansmith: yeah, that seems simplest too | |
| 18:37:16 | dansmith | anybody else looking forward to most of this code going away? :) | |
| 18:37:21 | mriedem | o/ | |
| 18:37:28 | bauzas | so since we update the instance.host once the migration is done, the target RT doesn't see it | |
| 18:37:31 | mriedem | so here is another question, which no one is going to like | |
| 18:37:44 | mriedem | this is going to be a change in how the compute is behaving | |
| 18:37:48 | mriedem | and ocata computes won't have this | |
| 18:37:50 | mriedem | so, | |
| 18:38:13 | jaypipes | dansmith: hmm... | |
| 18:38:20 | mriedem | do we (1) make claims in the scheduler dependent on pike computes, or (2) throw to the wind and rely on the dest periodic self-heal fixing the allocations? | |
| 18:38:35 | jaypipes | dansmith: so we're calling get_allocations_for_resource_)provider() and passing in the source compute node UUID. | |
| 18:38:55 | jaypipes | dansmith: so we're guaranteed that the only allocations returned are instances that are "on the source host" according to placement. | |
| 18:39:09 | dansmith | mriedem: well the healing doesn't happen all the time, as he pointed out yesterday, only when instances are added/removed, right? | |
| 18:39:22 | dansmith | mriedem: so I'm not sure we'll actually heal over stuff that ocata computes don't do :/ | |
| 18:39:56 | dansmith | jaypipes: delete_allocation_for_instance() operates only on an instance uuid | |
| 18:40:16 | dansmith | jaypipes: if we call that after the flip happened for whatever reason we'll do the wrong thing | |
| 18:40:31 | dansmith | jaypipes: even if we started ten minutes ago and got blocked for a while or something | |
| 18:40:47 | dansmith | jaypipes: really we should use generation on delete to make sure we don't delete something we're not intending to :/ | |
| 18:40:49 | jaypipes | dansmith: right, but didn't you want me to check to see if the allocation had the source compute node UUID in it and if not, don't call delete allocation? | |
| 18:41:08 | dansmith | jaypipes: yes, but delete allocation is only fetching via instance uuid | |
| 18:41:21 | dansmith | https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L999-L1001 | |
| 18:41:36 | jaypipes | dansmith: I understand that, but get_allocations_for_resource_provider() will always only return allocations that "have the source compute node UUID in them" | |
| 18:41:53 | jaypipes | dansmith: so that logic isn't going to filter anything out... | |
| 18:41:59 | mriedem | hangout? | |
| 18:42:05 | dansmith | jaypipes: yeah dude I get that, but we query that way, then time passes, then we delete | |
| 18:42:50 | jaypipes | mriedem: sure | |
| 18:43:09 | dansmith | https://hangouts.google.com/call/5pmzfm5wpfckpptt4l5hxjw5cyu | |
| 18:45:04 | bauzas | can I join ? | |
| 18:45:09 | bauzas | :) | |
| 18:45:13 | bauzas | need more context | |
| 18:45:20 | mriedem | everyone can join | |
| 18:46:53 | smcginnis | Not if you're in China. | |
| 18:46:57 | smcginnis | :) | |
| 18:47:02 | mriedem | nothing stopping them | |
| 18:47:09 | mriedem | climb that mountain | |
| 18:47:32 | smcginnis | :D | |
| 18:52:17 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L667 | |
| 18:52:30 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1030 | |
| 18:52:41 | mriedem | if instance.vm_state not in vm_states.ALLOW_RESOURCE_REMOVAL: | |
| 18:52:49 | dansmith | https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1007 | |
| 18:56:51 | mriedem | on unshelve we set the host/node on the instance here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4423 | |
| 18:57:01 | mriedem | and change the vm_state here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4440 | |
| 19:01:30 | openstackgerrit | Merged openstack/nova master: stabilize test_create_delete_server functional test https://review.openstack.org/487772 | |
| 19:11:50 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L797 | |
| 19:13:24 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1012 | |
| 19:14:45 | mriedem | instance_claim updating allocations https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L224 | |
| 19:16:13 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/488034 | |
| 19:19:10 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif master: Updated from global requirements https://review.openstack.org/488086 | |
| 19:21:33 | openstackgerrit | OpenStack Proposal Bot proposed openstack/python-novaclient master: Updated from global requirements https://review.openstack.org/488125 | |
| 19:22:40 | openstackgerrit | Eric Fried proposed openstack/nova master: nova.utils.get_endpoint_data() https://review.openstack.org/488137 | |