Earlier  
Posted Nick Remark
#openstack-nova - 2017-12-18
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
23:31:37 rybridges by "libvirt daemon logs" do you mean the journalctl?

Earlier   Later