| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-27 | |||
| 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 | |
| 19:23:37 | efried | mriedem jaypipes mordred Having started poking at the cinderclient construction, I think ^this^ may be a better alternative to get_service_url | |
| 19:23:57 | mriedem | efried: the house is on fire | |
| 19:24:03 | dansmith | mriedem: jaypipes https://etherpad.openstack.org/p/y9sUcb6XW6 | |
| 19:24:17 | mriedem | efried: have sdague check out the service catalog stuff | |
| 19:24:21 | mriedem | he knows more about that than i do | |
| 19:24:24 | efried | mriedem Roger wilco. | |
| 19:29:07 | mordred | efried: yes - that's a great approach | |
| 19:29:35 | efried | mordred Cool, thanks for looking. | |
| 19:31:29 | mriedem | https://review.openstack.org/#/c/244489/ | |
| 19:37:00 | cfriesen_ | jaypipes: did you ever get anywhere with the issue we discussed at the end of June around duplicate scsi device numbers when using virtio-scsi? | |
| 19:37:28 | jaypipes | cfriesen_: nope :( | |
| 19:37:34 | mriedem | cfriesen_: the house is on fire | |
| 19:37:39 | cfriesen_ | jaypipes: I think bug 1702999 is related, as is the "cannot attach new volume to an instance" thread on the openstack-operators list | |
| 19:37:41 | openstack | bug 1702999 in OpenStack Compute (nova) "Can't attach volume if instance boot from volume and virtio-scsi is enabled in the image" [Undecided,Incomplete] https://launchpad.net/bugs/1702999 | |
| 19:38:12 | jaypipes | oh wait, yeah I think we did have a patch for that... | |
| 19:38:36 | jaypipes | cfriesen_: gimme a while... on call | |
| 19:38:57 | mriedem | cfriesen_: this? https://review.openstack.org/#/q/topic:bug/1686116 | |
| 19:42:12 | cfriesen_ | mriedem: looks like it might help. in the case I looked at it would boot (using sda) but trying to attach volumes would fail. | |
| 19:42:58 | cfriesen_ | might be the case that 1702999 is already fixed | |
| 19:49:10 | cdent | jaypipes: if you end up with something that has lose ends by the time you go to bed, feel free to let me know the state of things and I can poke in my morning | |
| 19:49:31 | jaypipes | cdent: thx Chris, will do. | |
| 19:57:30 | openstackgerrit | OpenStack Proposal Bot proposed openstack/python-novaclient master: Updated from global requirements https://review.openstack.org/488125 | |
| 19:59:18 | openstackgerrit | Doug Hellmann proposed openstack/nova master: add a redirect for the old cells landing page https://review.openstack.org/487932 | |
| 20:10:00 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/488034 | |
| 20:12:24 | jaypipes | dansmith: fuuug... so confirm_resize() doesn't run on the destination host. It runs on the source host. :( | |
| 20:12:43 | mriedem | yeah it doesn't call back into rt | |