| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-18 | |||
| 20:34:21 | jroll | edleafe: that feels very wrong, is that code merged or is there still time to review it? :) | |
| 20:34:22 | mriedem | jroll: yeah it's not completely baked for ironic yet obviously | |
| 20:34:28 | jroll | right | |
| 20:34:30 | mriedem | jroll: the code to use it isn't merged yet | |
| 20:34:35 | mriedem | https://review.openstack.org/#/c/511358/40 | |
| 20:34:39 | jroll | awesome, ty | |
| 20:34:46 | mriedem | i have https://review.openstack.org/#/c/527289/ and am debugging it | |
| 20:34:47 | jaypipes | mriedem: so I have to figure out how to adapt the functional test base so that only the _create_server_group() uses the microversion at 2.59 and everything else uses 2.1... | |
| 20:34:50 | jaypipes | fun. | |
| 20:34:52 | edleafe | jroll: yeah, we discussed updating that for unique host/node, but the first round it's just host | |
| 20:35:20 | jroll | edleafe: gotcha, will take a look around | |
| 20:35:27 | edleafe | jroll: the code to create the alternates has merged. | |
| 20:40:17 | melwitt | jaypipes: it's ghetto, but you could just do what mriedem suggested and add something like this to _boot_a_server_to_group https://github.com/openstack/nova/blob/master/nova/tests/functional/test_servers.py#L1449 | |
| 20:40:54 | melwitt | I don't know who figured out that's always the image uuid of the first image but that's what it appears to me | |
| 20:41:20 | mriedem | jaypipes: you can use the self.api client directly | |
| 20:41:30 | mriedem | if you want to avoid the base class method to create the server group | |
| 20:44:01 | jaypipes | mriedem: I am looking at nova/tests/functional/api/client.py to try and understand how to pass a specific microversion... I don't see it. | |
| 20:44:11 | mriedem | oh right... | |
| 20:44:22 | mriedem | you can't, and the client specifically blows up if you try passing your own header | |
| 20:44:39 | mriedem | jaypipes: please place your nuts here < > so i can kick you square in them | |
| 20:44:40 | jaypipes | mriedem: looks like I will need to do somethin glike this: | |
| 20:44:59 | mriedem | self.api.microversion = '2.59' | |
| 20:45:05 | mriedem | self.api.post_server_group() | |
| 20:45:07 | jaypipes | self.api.microversion = '2.59'; do something...; self.api.microversion = orig | |
| 20:45:11 | mriedem | self.api.microversion = old_value | |
| 20:45:12 | mriedem | yeah | |
| 20:45:13 | jaypipes | heh, jinux | |
| 20:45:20 | jaypipes | or even jinx. | |
| 20:45:33 | mriedem | jinux is your personal linux distro? | |
| 20:45:35 | jaypipes | I guess jinux is linux jinx. | |
| 20:45:36 | mriedem | jay's linux | |
| 20:45:40 | jaypipes | heh | |
| 20:45:57 | mriedem | this is the thing that blows up if you try to pass a header override https://github.com/openstack/nova/blob/master/nova/tests/functional/api/client.py#L179 | |
| 20:46:23 | mriedem | however, i think that was added when this was all written by sdague and was very specific to how the tests worked with scenarios | |
| 20:46:50 | jaypipes | I'll just add the microversion override as above with the reset right after | |
| 20:46:54 | mriedem | i'd be cool with removing that so you can run tests at a default minimum microversion and then make certain calls with a higher microversion, or vice-versa | |
| 20:48:37 | mriedem | btw, this is a useful code comment http://git.openstack.org/cgit/openstack/nova/tree/nova/scheduler/filters/retry_filter.py#n37 | |
| 20:49:09 | jaypipes | mriedem: about equally as useful as the majority of code comments in the scheduler filters. | |
| 20:49:43 | melwitt | lol | |
| 20:50:05 | jaypipes | also, god help whomever needs to change "tenant" -> "project" when we no longer support keystone 2.0.' | |
| 20:50:57 | mriedem | edleafe: ok this is really confusing http://logs.openstack.org/89/527289/1/check/ironic-tempest-dsvm-ipa-wholedisk-agent_ipmitool-tinyipa-multinode/406d4ab/logs/screen-n-sch.txt.gz#_Dec_12_22_23_47_783964 | |
| 20:51:04 | mriedem | we have 12 candidate hosts, | |
| 20:51:15 | mriedem | initial select_destinations call for this create request | |
| 20:51:25 | mriedem | and we log "Re-scheduling is disabled" 12 times, once per host, because of the RetryFilter, | |
| 20:51:43 | mriedem | and i think it's all because we don't populate the 'retry' field in the RequestSpec before calling select_destinations the first time | |
| 20:52:04 | mriedem | during create we call the scheduler here http://git.openstack.org/cgit/openstack/nova/tree/nova/conductor/manager.py#n1016 | |
| 20:52:15 | mriedem | and we don't populate retry until http://git.openstack.org/cgit/openstack/nova/tree/nova/conductor/manager.py#n1108 | |
| 20:52:23 | mriedem | i'm not sure if that was ever by design | |
| 20:52:49 | mriedem | looking at the old build_instances method, we'd populate retry before calling the scheduler http://git.openstack.org/cgit/openstack/nova/tree/nova/conductor/manager.py#n543 | |
| 20:53:19 | mriedem | again, probably not our issue here since max_attempts=3 by default, and i verified that in this job, but it sure is confusing in the logs | |
| 20:57:52 | mriedem | https://bugs.launchpad.net/nova/+bug/1738876 | |
| 20:57:52 | openstack | Launchpad bug 1738876 in OpenStack Compute (nova) "Scheduler logs "Re-scheduling is disabled" on initial server create scheduling even though max_attempts>0" [Medium,Triaged] | |
| 21:09:32 | edleafe | mriedem: with alternates, isn't the RetryFilter supposed to go away? | |
| 21:09:58 | mriedem | i suppose it would over time | |
| 21:10:07 | edleafe | since you won't try the same host more than once? | |
| 21:10:32 | edleafe | IOW, it'll always be a noop | |
| 21:17:18 | mriedem | edleafe: this shows that we're getting duplicate alternates http://logs.openstack.org/89/527289/1/check/ironic-tempest-dsvm-ipa-wholedisk-agent_ipmitool-tinyipa-multinode/22826a2/logs/screen-n-super-cond.txt.gz#_Dec_15_23_17_04_949754 | |
| 21:17:29 | mriedem | Selected host: ubuntu-xenial-inap-mtl01-0001472200; Alternates: [u'ubuntu-xenial-inap-mtl01-0001472200', u'ubuntu-xenial-inap-mtl01-0001472200'] | |
| 21:19:12 | mriedem | hmm | |
| 21:19:13 | mriedem | https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L361 | |
| 21:19:17 | mriedem | host not in selected_hosts | |
| 21:19:30 | mriedem | we don't have a HostState.__eq__ override | |
| 21:20:02 | edleafe | mriedem: wait - a HostState object isn't equal to itself? | |
| 21:21:42 | openstackgerrit | Merged openstack/nova stable/pike: Make request_spec.spec MediumText https://review.openstack.org/528330 | |
| 21:22:26 | mriedem | edleafe: i'm not sure if that is the issue, | |
| 21:22:43 | mriedem | but https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L361 is the place where we'd filter out duplicate alternates wouldn't it? | |
| 21:23:07 | mriedem | oh wait, no | |
| 21:23:57 | mriedem | maybe the logging you added to conductor is just incomplete because it doesn't also log the nodename? | |
| 21:24:33 | edleafe | that could be the case | |
| 21:24:54 | edleafe | I have some unit tests that verify that there aren't dupes added to alternates | |
| 21:25:55 | mriedem | ok, so in the case of ironic, we could be getting alternate nodes properly from the scheduler | |
| 21:26:07 | edleafe | I can add nodename and push a revision if you want to test that theory | |
| 21:26:08 | mriedem | but the reschedule logic doesn't take that into account | |
| 21:26:24 | mriedem | edleafe: yeah let's do that - i left some other comments in conductor manager about that log message | |
| 21:28:22 | edleafe | ok | |
| 21:28:54 | mriedem | this also explains why http://logs.openstack.org/89/527289/1/check/ironic-tempest-dsvm-ipa-wholedisk-agent_ipmitool-tinyipa-multinode/22826a2/ passed the 2nd time i checked it, | |
| 21:29:09 | mriedem | it's just totally random if we pick one of the alternates which is a different node on a different hose when we reschedule | |
| 21:29:36 | mriedem | jroll: so to answer your question, reschedules with alternate ironic nodes might work, but it's not designed to work yet :) | |
| 21:29:46 | jroll | heh, cool | |
| 21:30:47 | openstackgerrit | Ed Leafe proposed openstack/nova master: Make conductor pass and use host_lists https://review.openstack.org/511358 | |
| 21:31:36 | edleafe | mriedem: ^^ now with improved logging! | |
| 21:32:11 | openstackgerrit | Jay Pipes proposed openstack/nova master: WIP - Support aggregate affinity filters https://review.openstack.org/528804 | |
| 21:35:12 | mriedem | edleafe: you changed one thing, broke one thing, and missed some other things :) | |
| 21:35:37 | edleafe | about par for the course | |
| 21:35:42 | mriedem | before you change, hold up | |
| 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 | |