| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-18 | |||
| 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 | empty lists are fun | |
| 22:23:33 | mriedem | ... | |
| 22:23:33 | mriedem | ... print 'empty lists are fun' | |
| 22:23:33 | mriedem | >>> if [[]]: | |
| 22:23:42 | edleafe | ok, what about this scenario: | |
| 22:23:49 | edleafe | max_attempts=5 | |
| 22:23:58 | edleafe | only 2 alternates were found | |
| 22:24:19 | edleafe | the populate_retry thing won't catch that, will it? | |
| 22:24:30 | mriedem | no it wouldn't, | |
| 22:24:41 | mriedem | in that case, with the old flow, we'd call the scheduler and get NoValidHost | |
| 22:25:01 | mriedem | ok so given that, we can't rely on populate_retry | |
| 22:25:14 | edleafe | So I'll leave the check for an empty list, and raise MaxRetriesExceeded | |
| 22:25:35 | mriedem | well, L578 isn't the place to do that | |
| 22:25:41 | mriedem | we need it to fail in the try/except above | |
| 22:25:51 | mriedem | so it sets the vm_state to ERROR | |
| 22:26:09 | mriedem | so how about an else to this? "if not is_retry:" | |
| 22:26:48 | mriedem | if not is_retry: select_dests elif not host_list[0]: raise MaxRetriesExceeded | |
| 22:26:49 | edleafe | ok, I'll move the check there | |
| 22:27:49 | mriedem | note that in that case, we shouldn' try to destroy the build request in the except block | |
| 22:28:14 | edleafe | why not? | |
| 22:28:20 | mriedem | because if we're in a cell conductor we don't have access to the api db | |
| 22:28:32 | mriedem | and rather than BuildRequestNotFound, we'll get CantStartEngineError | |
| 22:28:41 | openstack | Launchpad bug 1733933 in OpenStack Compute (nova) "nova-conductor is masking error when rescheduling" [Low,Triaged] | |
| 22:28:41 | mriedem | ala https://bugs.launchpad.net/nova/+bug/1733933 | |
| 22:29:00 | edleafe | ah. I don't know how you keep all that straight | |
| 22:29:27 | mriedem | well, that's also what failed here 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 | |
| 22:29:41 | mriedem | er this i mean 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_940236 | |
| 22:29:52 | mriedem | File "/opt/stack/new/nova/nova/conductor/manager.py", line 478, in _destroy_build_request | |
| 22:30:26 | mriedem | i'll leave a note in the code so we don't forget | |
| 22:34:07 | mriedem | edleafe: ok commented. after thinking through that, it might be best to leave L578 | |
| 22:34:17 | mriedem | because host_lists is going to be per-instance right? | |
| 22:34:31 | mriedem | although, if we're rescheduling, it will be a single entry yeah? | |
| 22:35:23 | edleafe | yeah, always single on reschedule | |
| 22:36:34 | edleafe | also, the populate_retry method pops the exc_reason from the filter_properties, so it isn't available later in build_instances | |
| 22:36:56 | edleafe | why not check above as you mentioned before? | |
| 22:37:04 | mriedem | we could - i left yet another comment | |
| 22:37:15 | mriedem | i think the build request cleanup we have in here is likely latent broken | |
| 22:37:19 | mriedem | if we're rescheduling | |
| 22:37:43 | mriedem | if we get here https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L559 because populate_retry raised MaxRetriesExceeded, | |
| 22:37:50 | mriedem | we shouldn't try to delete the build request | |
| 22:38:01 | mriedem | melwitt: you might want to check my thinking here | |
| 22:41:00 | melwitt | yeah ... the way that code is written, it's assuming we have an instance record. and I thought we do indeed have an instance in ERROR state if max retries were reached | |
| 22:41:53 | melwitt | I can't remember | |
| 22:42:46 | melwitt | also, that's cells v1 only code where I think the instance record is created right away in the compute/api. so a retries exceeded would have an instance record | |
| 22:42:48 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Don't try to delete build requests on reschedules exceeded https://review.openstack.org/528835 | |
| 22:42:49 | mriedem | melwitt: edleafe: ^ it's a start | |
| 22:42:57 | mriedem | gotta run | |
| 22:43:12 | edleafe | mriedem: ok, will post an update soon | |
| 22:43:30 | edleafe | and I renamed 'is_retry' to 'is_reschedule' | |
| 22:43:43 | edleafe | just seems more accurate | |
| 22:46:08 | melwitt | oh, cells v1 AND reschedules go through build_instances. guh | |
| 22:47:41 | edleafe | melwitt: fun, right? | |
| 22:48:08 | melwitt | totally | |
| 23:14:26 | rybridges | Hey guys | |
| 23:14:58 | rybridges | Any idea why running suspending an instance would put it into an error state? Here is the log from nova-compute https://pastebin.com/jTedyZVJ | |
| 23:15:48 | mgagne | can't wait for policy-in-code to land in all projects... =) | |
| 23:16:21 | mgagne | I guess that would be one major reason I would upgrade | |
| 23:16:40 | cfriesen | rybridges: is libvirtd running? | |
| 23:17:00 | rybridges | here is the output of server show https://pastebin.com/8eQiz3hA | |
| 23:17:03 | cfriesen | rybridges: check the libvirtd/qemu logs | |
| 23:17:04 | rybridges | yes libvirtd is running | |
| 23:17:38 | rybridges | this is happening intermittently | |
| 23:17:42 | rybridges | sometimes it happens | |
| 23:17:45 | rybridges | other times it is fine | |
| 23:18:56 | cfriesen | your nova logs indicate that the call to virDomainManagedSave() failed with a connection reset...this implies an issue with libvirtd, I think. | |
| 23:19:02 | cfriesen | I gotta run, suppertime | |
| 23:21:26 | rybridges | right | |
| 23:21:30 | rybridges | i figured that much | |
| 23:21:35 | rybridges | why would that call be failing? | |
| 23:21:42 | rybridges | i dont see anything in qemu logs | |
| 23:22:08 | rybridges | i am grepping for all sorts of stuff in /var/log/libvirt/qemu | |
| 23:22:17 | rybridges | the hypervisor is rhel7 btw | |
| 23:30:27 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: [placement] Add functional tests for resource class API https://review.openstack.org/524506 | |
| 23:31:15 | melwitt | rybridges: you'll want to look at the libvirt daemon logs, libvirtd.log. also look at /var/log/messages | |