| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-26 | |||
| 21:32:24 | jaypipes | dansmith: no, for move operations the definition of "correct" is fuzzy. | |
| 21:32:34 | dansmith | jaypipes: I disagree :) | |
| 21:33:18 | mriedem | ah i see | |
| 21:35:44 | mriedem | so this works the same if you're doing a resize and revert back to the source host i think | |
| 21:36:17 | jaypipes | I'm wondering if instances that are migrating are in RT.tracked_instances... | |
| 21:36:22 | jaypipes | if they aren't, we're good. | |
| 21:36:42 | dansmith | jaypipes: well, the existing RT stuff is not even correct, as you know | |
| 21:36:49 | bauzas | jaypipes: well, I don't think so | |
| 21:37:07 | bauzas | jaypipes: IIRC, tracked_instances is for existing instances | |
| 21:37:13 | dansmith | jaypipes: if you have the instance, you should be able to check instance.migration_context to know if it's moving | |
| 21:37:15 | bauzas | not for migrating ones | |
| 21:37:23 | jaypipes | ok, guys, I think we're good... | |
| 21:37:25 | dansmith | bauzas: for migrating ones on the source they should be there right? | |
| 21:37:26 | jaypipes | lemme explain. | |
| 21:37:27 | jaypipes | pls. | |
| 21:37:51 | bauzas | dansmith: for the source RT, yeah they should be there AFAIK | |
| 21:38:01 | dansmith | bauzas: right, destination node does not matter | |
| 21:38:05 | jaypipes | so, in update_available_resource(), we call _update_usage_from_instance(). this is the "auto-heal" thing. | |
| 21:38:13 | bauzas | correct | |
| 21:38:35 | jaypipes | within that method, we only delete the allocation if the instance is in DELETED or SHELVE_OFFLOADED state | |
| 21:38:40 | bauzas | the problem is how we could possibly have duplicate allocations for both target and source if source just removes the target allocations ? | |
| 21:38:47 | jaypipes | otherwise we don't touch the allocations. | |
| 21:38:53 | dansmith | jaypipes: eh? | |
| 21:39:07 | dansmith | jaypipes: we compare the generated ones to the ones from placement and re-put them if they differ | |
| 21:39:17 | dansmith | that's how we get allocations now | |
| 21:39:19 | bauzas | jaypipes: PUT /allocations/<instance_uuid> is cleaning up existing allocs, nope ? | |
| 21:39:49 | bauzas | FWIW, it's becoming late and I could be wrong | |
| 21:40:00 | dansmith | jaypipes: this: https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L890-L904 | |
| 21:40:16 | mriedem | edleafe: need to find out when the nova-compute service with the ironic driver is able to talk to ironic | |
| 21:40:57 | jaypipes | dansmith: when does update_instance_allocation() get called though? | |
| 21:41:03 | dansmith | jaypipes: oh, are you looking at the "is new or is old" bit of RT? | |
| 21:41:10 | jaypipes | dansmith: CORRECT! | |
| 21:41:33 | dansmith | jaypipes: okay, so that's a problem for the failure case then, | |
| 21:41:43 | jaypipes | dansmith: so we don't actually call that update_instance_allocation() unless it's either a brand new instance or it's DELETED/SHEVE_OFFLOADED | |
| 21:41:52 | dansmith | jaypipes: because that means we'll never heal over the double allocation on the source when the migration is canceled | |
| 21:42:05 | edleafe | mriedem: sure, but my point was that since the code runs in init_host, it won't get a second chance to run *after* the ironic service starts up | |
| 21:42:58 | mriedem | edleafe: at some point on startup the compute manager is calling get_inventory which has to refresh the node list | |
| 21:43:05 | mriedem | b/c of the pre_start_hook in the compute manager | |
| 21:43:15 | mriedem | so does that just not work today? or are we just not waiting long enough? | |
| 21:43:25 | dansmith | mriedem: edleafe: riht, that probably should run when we get a new node | |
| 21:43:33 | dansmith | because you can get/lose nodes on ironic at runtime | |
| 21:44:14 | dansmith | because we could have started with one node, migrated those, and then gained a couple more nodes later when someone shuts down another ironic compute in the hash ring | |
| 21:44:20 | edleafe | dansmith: I thougth a new node would be empty to start | |
| 21:44:25 | edleafe | no instance | |
| 21:44:26 | dansmith | and if that is done during upgrade, which it is, then you need to migrate it | |
| 21:44:29 | dansmith | edleafe: no, because ^ | |
| 21:45:04 | dansmith | edleafe: nova-computes shard off the full set of ironic nodes, | |
| 21:45:15 | dansmith | and if you were to upgrade one ironic compute, then shut down your old one, | |
| 21:45:31 | dansmith | you'd start with some nodes, and then later get a bunch more when the ring rebalances | |
| 21:45:44 | dansmith | after init_host(), at runtime, old nodes with instances that you now own and need to migrate | |
| 21:46:18 | edleafe | dansmith: so then what IYO would be a better place for this? | |
| 21:46:50 | dansmith | edleafe: I'd have to go dig just like you, but there's a place in there where we rebalance the ring (or balance it for the first time) and get a list of the nodes we own | |
| 21:46:51 | cfriesen_ | jaypipes: back at the last PTG did you arrive at any conclusions on how to handle the Intel CAT stuff? | |
| 21:46:54 | dansmith | edleafe: so .. there. :) | |
| 21:47:06 | jaypipes | cfriesen_: bad time to bring that question :) | |
| 21:47:14 | jaypipes | cfriesen_: how about discuss tomorrow? | |
| 21:48:36 | dansmith | edleafe: _refresh_hash_ring() is a good place to start | |
| 21:48:43 | cfriesen_ | jaypipes: sure | |
| 21:49:00 | dansmith | after you get the hash ring you could probably spawn your thread to go examine the instances on what the hash ring says are your nodes | |
| 21:49:48 | edleafe | dansmith: looking at that now... | |
| 21:50:05 | dansmith | edleafe: obviously I wasn't thinking about this possibility either, as I'm used to the world before this was here and there was pretty much only one ironic compute ever | |
| 21:51:23 | mriedem | it's curious that this blows up during init_host in the driver, but not when driver.get_available_nodes is called | |
| 21:51:27 | mriedem | which is shortly after | |
| 21:51:44 | mriedem | like in ocata, this is init_host: http://logs.openstack.org/63/485263/2/check/gate-tempest-dsvm-ironic-ipa-wholedisk-bios-agent_ipmitool-tinyipa-ubuntu-xenial-nv/de2c924/logs/screen-n-cpu.txt.gz#_2017-07-21_12_07_20_885 | |
| 21:51:52 | mriedem | and this is right after http://logs.openstack.org/63/485263/2/check/gate-tempest-dsvm-ironic-ipa-wholedisk-bios-agent_ipmitool-tinyipa-ubuntu-xenial-nv/de2c924/logs/screen-n-cpu.txt.gz#_2017-07-21_12_07_21_167 | |
| 21:55:33 | bauzas | folks it's late, so I'll disappear in a very short few | |
| 21:55:55 | mriedem | o/ | |
| 21:56:00 | bauzas | but like any release, just lemme know which changes I should review ASAP tomorrow morning my time | |
| 21:56:05 | bauzas | before we call the axe | |
| 21:56:44 | bauzas | and again, sorry for not having been there for half-Pike | |
| 21:56:48 | bauzas | \o | |
| 21:58:36 | bauzas | mriedem: before I leave, I'm torn by https://review.openstack.org/#/c/408955/ | |
| 21:59:16 | mriedem | edleafe: hold the phone, it was happening before the migrate flavors thing http://logs.openstack.org/80/461480/5/check/gate-tempest-dsvm-ironic-ipa-wholedisk-bios-agent_ipmitool-tinyipa-ubuntu-xenial-nv/834477a/logs/screen-n-cpu.txt.gz?level=TRACE#_Jul_19_03_34_01_687299 | |
| 22:02:23 | mriedem | blows up in ironic-api http://logs.openstack.org/80/461480/5/check/gate-tempest-dsvm-ironic-ipa-wholedisk-bios-agent_ipmitool-tinyipa-ubuntu-xenial-nv/834477a/logs/screen-ir-api.txt.gz#_Jul_19_03_34_01_680460 | |
| 22:03:13 | jgriffith | mriedem ildikov added a note here https://review.openstack.org/#/c/330285/106 regarding the Trace showing up in the logs | |
| 22:07:16 | openstackgerrit | Merged openstack/nova master: Updated from global requirements https://review.openstack.org/487473 | |
| 22:09:39 | mriedem | bauzas: torn how | |
| 22:09:40 | mriedem | ? | |
| 22:10:36 | mriedem | there are 3 other patches for that: api, docs and novaclient | |
| 22:10:42 | mriedem | so it's pretty damn late | |
| 22:12:51 | edleafe | mriedem: ah, so _refresh_cache() is the culprit. The migration change just adds that call a bit earlier | |
| 22:13:12 | mriedem | jgriffith: ack | |
| 22:13:26 | mriedem | edleafe: right, like <1 sec earlier | |
| 22:13:28 | mriedem | but still blowing up | |
| 22:13:36 | mriedem | i've adjusted the bug and routed it to ironic | |
| 22:13:51 | edleafe | mriedem: yeah, but we should be more graceful about it | |
| 22:14:00 | dansmith | mriedem: edleafe well, the bug of not migrating nodes we get via hash rebalance is still a thing | |
| 22:14:27 | edleafe | dansmith: agreed | |
| 22:15:26 | openstackgerrit | Eric Fried proposed openstack/nova master: Centralize keystoneauth1 Adapter conf option setup https://review.openstack.org/487581 | |
| 22:19:23 | openstackgerrit | Jay Pipes proposed openstack/nova master: claim resources in placement API during schedule() https://review.openstack.org/483566 | |
| 22:19:23 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: account for move operations in claim https://review.openstack.org/487589 | |
| 22:19:25 | jaypipes | mriedem, bauzas, dansmith: ding ding ding. round 2. ^^ | |
| 22:19:51 | mriedem | heh, can i be glass joe? | |
| 22:22:48 | openstackgerrit | Nicolas Simonds proposed openstack/nova master: libvirt: add support for virtio-net rx/tx queue sizes https://review.openstack.org/484997 | |
| 22:23:17 | jaypipes | mriedem: I'll work on my eye tattoo. | |
| 22:24:02 | mriedem | heh i guess we forgot about safe_connect on claim_resources | |
| 22:24:40 | openstackgerrit | Merged openstack/nova master: Remove the code related to extension loading from APIRouterV21 https://review.openstack.org/486414 | |
| 22:25:03 | openstackgerrit | Eric Fried proposed openstack/nova master: nova.utils.get_service_url() https://review.openstack.org/458257 | |
| 22:30:05 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix example in _serialize_allocations_for_consumer https://review.openstack.org/487614 | |
| 22:30:20 | mriedem | jaypipes: dansmith: so we don't need to handle anything different from the compute RT perspective for a move operation? | |