| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-18 | |||
| 21:36:57 | mriedem | so actually, this should work for ironic | |
| 21:37:27 | mriedem | there is nothing in the scheduler or conductor retry loop code, that i can see, which is filtering alternates based on just the host | |
| 21:38:12 | mriedem | so the failure we hit in the ironic job originally where it tried to upcall, is probably just do to not handling the final retry properly and killing it | |
| 21:38:21 | mriedem | is_retry = host_lists is not None | |
| 21:38:30 | mriedem | on the last reschedule, that could be None or an empty list yeah? | |
| 21:39:05 | edleafe | how could it be None? | |
| 21:40:20 | mriedem | # 'host_lists' will be None in one of two cases: when running | |
| 21:40:20 | mriedem | # cellsv1, or during a reschedule from a pre-Queens compute. | |
| 21:40:49 | edleafe | yeah | |
| 21:40:50 | mriedem | i think i explain the problem scenario here https://review.openstack.org/#/c/511358/41/nova/conductor/manager.py@599 | |
| 21:41:00 | edleafe | not on a reschedule | |
| 21:41:03 | mriedem | we have 2 alternates, and let's say we're on the last one | |
| 21:41:10 | mriedem | we pop that last entry from the host_list | |
| 21:41:22 | mriedem | pass it to compute, which fails, and reschedules, and passes that empty list back to build_instances | |
| 21:41:52 | edleafe | yup | |
| 21:41:57 | edleafe | it won't be None | |
| 21:42:11 | mriedem | i think i'm confusing myself | |
| 21:43:02 | edleafe | it will be empty if there are no more alternates | |
| 21:43:06 | edleafe | but it will be a list | |
| 21:44:50 | edleafe | mriedem: https://review.openstack.org/#/c/511358/41/nova/compute/manager.py@1905 | |
| 21:45:18 | mriedem | yeah the list of lists there really confuses me | |
| 21:45:23 | mriedem | you said you're doing that because of [instance] | |
| 21:45:30 | mriedem | but i don't think we need to follow that pattern here | |
| 21:46:17 | edleafe | conductor gets [instance], so it need [host_list] | |
| 21:46:27 | edleafe | compute gets instance, so it needs host_list | |
| 21:46:28 | mriedem | so the zip works? | |
| 21:46:52 | edleafe | that's one of many reasons | |
| 21:47:03 | edleafe | it needs a host_list for each instance | |
| 21:47:43 | mriedem | ok, not a major issue for me atm | |
| 21:47:53 | mriedem | for that one ci job failure, this is the reason the first selected node failed | |
| 21:47:54 | mriedem | http://logs.openstack.org/89/527289/1/check/ironic-tempest-dsvm-ipa-wholedisk-agent_ipmitool-tinyipa-multinode/406d4ab/logs/screen-n-cpu.txt.gz#_Dec_12_22_23_48_399346 | |
| 21:47:58 | mriedem | ComputeHostNotFound: Compute host 5 could not be found. | |
| 21:49:05 | edleafe | any idea why that host couldn't be found? | |
| 21:49:22 | mriedem | nope | |
| 21:49:34 | mriedem | we might not be using the proper cell context | |
| 21:52:02 | mriedem | so we hit the first selected node, which fails because of compute node not found, | |
| 21:52:09 | mriedem | and we cast up to the cell conductor to reschedule and hit this http://logs.openstack.org/89/527289/1/check/ironic-tempest-dsvm-ipa-wholedisk-agent_ipmitool-tinyipa-multinode/406d4ab/logs/screen-n-cond-cell1.txt.gz#_Dec_12_22_23_48_751837 | |
| 21:55:30 | mriedem | AHA | |
| 21:55:38 | mriedem | i see the problemo | |
| 21:55:39 | mriedem | oh ed | |
| 21:55:43 | mriedem | ed ed ed | |
| 21:56:24 | edleafe | you were getting me confused. How could the compute call back when the compute wasn't found? | |
| 21:58:52 | mriedem | comments line | |
| 21:58:53 | mriedem | *inline | |
| 21:59:01 | mriedem | you were passing host_list using the wrong arg | |
| 21:59:04 | edleafe | looking | |
| 21:59:10 | mriedem | that's why it was None when it came back up and we tried to hit the scheduler again | |
| 21:59:27 | mriedem | use kwargs ftw | |
| 22:00:10 | mriedem | so i think ironic reschedules should be totally fine | |
| 22:00:11 | mriedem | jroll: ^ | |
| 22:00:36 | mriedem | code as is is busted so it wasn't trying alternates | |
| 22:00:42 | mriedem | once we fix that, things should be good | |
| 22:01:33 | jroll | nice | |
| 22:04:25 | openstackgerrit | Ed Leafe proposed openstack/nova master: Make conductor pass and use host_lists https://review.openstack.org/511358 | |
| 22:06:40 | edleafe | mriedem: ^^ updated | |
| 22:07:28 | mriedem | edleafe: ok, that should fix the actual reschedule issue, but still missing comments from PS41 so re-applied my -1 | |
| 22:07:31 | mriedem | if you're about end of day, | |
| 22:07:38 | mriedem | then ill kick off the ironic job again | |
| 22:07:44 | mriedem | and we can check results in the morn | |
| 22:08:04 | edleafe | I just wanted to get you that fix first | |
| 22:08:09 | edleafe | working on the other comments | |
| 22:08:11 | mriedem | ok | |
| 22:11:59 | edleafe | mriedem: re: your question on https://review.openstack.org/#/c/511358/41/nova/conductor/manager.py@578 | |
| 22:12:18 | edleafe | Should that failure be handled at the compute level, or up here in the conductor? | |
| 22:13:57 | mriedem | edleafe: conductor | |
| 22:14:07 | mriedem | scheduler_utils.populate_retry is what raises MaxRetriesExceeded today | |
| 22:14:39 | mriedem | so, maybe populate_retry just handles my issue on L578 | |
| 22:14:43 | mriedem | since it comes before we'd get that far | |
| 22:16:16 | edleafe | The question is at this point, if we have an empty host_list, should I raise a MaxRetriesExceeded exception? | |
| 22:16:53 | mriedem | i don't think we'll get that far | |
| 22:16:58 | mriedem | populate_retry should fail before that | |
| 22:17:09 | mriedem | if you're asking about just being explicit... | |
| 22:17:31 | mriedem | it's definitely confusing that populate_retry is also the "check if you've gone over the retry limit thing" | |
| 22:17:46 | edleafe | that confused me too | |
| 22:17:54 | mriedem | i'd be fine with an explicit check | |
| 22:18:13 | edleafe | so if we reach 578 and have an empty host_list, something is seriously wrong | |
| 22:18:27 | mriedem | yes | |
| 22:19:13 | edleafe | what would be the best exception to raise there? Besides a YoureHosedException? | |
| 22:20:01 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: libvirt: Provide VGPU inventory for a single GPU type https://review.openstack.org/514763 | |
| 22:20:02 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: WIP: libvirt: create vGPU for instance https://review.openstack.org/528832 | |
| 22:20:16 | mriedem | edleafe: MaxRetriesExceedd? | |
| 22:20:21 | mriedem | *exceeded | |
| 22:20:24 | mriedem | that, or NoValidHost | |
| 22:20:57 | mriedem | honestly L578 shouldn't be there | |
| 22:20:58 | edleafe | Well, we found valid hosts, but just couldn't build on 'em | |
| 22:21:09 | mriedem | the loop should assume we have a host per instance | |
| 22:21:18 | mriedem | if we don't, something higher up in the method should have caught it | |
| 22:21:25 | edleafe | ok, I'll remove it | |
| 22:21:48 | mriedem | this is what should catch that case https://review.openstack.org/#/c/511358/42/nova/conductor/manager.py@545 | |
| 22:22:17 | mriedem | it's definitely confusing to have the retry information spread and maintained across filter properties, request spec and the host list stuff, but that's what we have | |
| 22:22:48 | mriedem | if for some reason populate_retry didn't catch it and host_lists was empty, then we have to handle it explicitly | |
| 22:23:01 | mriedem | and, host_lists wouldn't be empty, because it'd be [[]] right? | |
| 22:23:11 | mriedem | a list with a single entry, which is an empty list | |
| 22:23:14 | edleafe | yeah, that's the plan | |
| 22:23:33 | mriedem | >>> if [[]]: | |
| 22:23:33 | mriedem | ... print 'empty lists are fun' | |
| 22:23:33 | mriedem | ... | |
| 22:23:33 | mriedem | empty lists are fun | |
| 22:23:42 | edleafe | ok, what about this scenario: | |
| 22:23:49 | edleafe | max_attempts=5 | |