Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-02-01
15:19:58 mriedem vsaienk0: http://lists.openstack.org/pipermail/openstack-dev/2017-January/111295.html
15:20:13 mriedem vsaienk0: https://bugs.launchpad.net/nova/+bug/1660973 is a duplicate
15:20:13 openstack Launchpad bug 1660973 in networking-bgpvpn ""No hosts found to map to cell, exiting" / PlacementNotConfigured exception" [Critical,In progress] - Assigned to Thomas Morin (tmmorin-orange)
15:20:49 dansmith mriedem: you sure that's the same?
15:20:57 mriedem it's not
15:20:58 mriedem nvm
15:21:02 mriedem http://logs.openstack.org/75/427675/2/check/gate-tempest-dsvm-ironic-ipa-wholedisk-agent_ipmitool-tinyipa-multinode-ubuntu-xenial-nv/3ff2401/logs/screen-n-cpu.txt.gz?level=TRACE is clean
15:21:11 dansmith yeah
15:21:22 dansmith n-cond too, an that's the important one for the mapping bug
15:21:53 vsaienk0 mriedem: on ironic multinode job placement api is enabled
15:22:04 mriedem vsaienk0: then it's something else
15:22:06 dansmith vsaienk0: is that test trying to target specific hosts with a boot?
15:22:53 mordred mriedem: done
15:23:03 mordred mriedem: I believe I said the same thing as sdague - exept worse and with more words
15:23:34 mriedem mordred: that's why i pinged you :)
15:24:54 vsaienk0 dansmith: yes, test_schedule_to_all_nodes is failing
15:25:21 dansmith vsaienk0: I see that from the logs, I'm asking what the test is doing, but I'll go look for myself
15:25:52 jaypipes sileht: thx for your fix on https://review.openstack.org/#/c/427667/. zzzeek, if you can give your attention to this patch, that would be appreciated. I will leave it to mriedem, sdague or bauzas to +W.
15:26:24 vsaienk0 dansmith: that is nova test, I don't know what exactly it is doing
15:26:36 dansmith vsaienk0: okay, yeah, so it's trying to specifically boot an instance targeted at a node
15:26:52 jaypipes sileht: for the record, the reason that uuid was the grouping expression there was that the original query I was using only returned the UUIDs of the resource providers :)
15:27:10 dansmith vsaienk0: so it may be similar to the issue mriedem pointed above, but manifesting differently, give me a sec to look at the logs
15:27:22 jaypipes rpodolyaka: thx for the heads up on https://review.openstack.org/#/c/427667/ :)
15:27:28 bauzas sileht: oh, nice catch, thanks for fixing that
15:28:11 dansmith vsaienk0: is this failing all the time now or just occasionally
15:28:30 vsaienk0 dansmith: all the time
15:28:45 dansmith vsaienk0: okay, let me put up a hacky devstack patch to try something
15:30:11 vsaienk0 dansmith thanks!
15:30:28 bauzas sileht: jaypipes: and FWIW, rpodolyaka is right, I did the GROUP BY because I wanted the HAVING clause
15:30:47 bauzas sileht: jaypipes: but that should work by the PK too
15:30:54 rpodolyaka jaypipes: np!
15:31:43 dansmith vsaienk0: can you do a canary change against ironic, depending on this? https://review.openstack.org/427772
15:32:51 vsaienk0 dansmith: done https://review.openstack.org/#/c/427675/ waiting for results, thanks!
15:33:25 dansmith vsaienk0: okay, this is the thing I was pointing to yesterday, which I suspect you're hitting. if this fixes it, we'll need to put it in a proper place of course
15:34:25 openstackgerrit Christopher Martin proposed openstack/nova: Stop using mox in test_compute_cells.py https://review.openstack.org/389750
15:34:54 dansmith vsaienk0: oh you guys are a devstack plugin, well that's why I couldn't find stuff... going forward we can just put this in your lib/ironic thing, so .. easier
15:36:35 sfinucan bauzas: Think there's a very minor logging mistake in https://review.openstack.org/#/c/427200, though otherwise LGTM
15:37:10 bauzas sfinucan: you can hurt me with a -1, I won't hit you :p
15:37:41 sfinucan bauzas: I don't usually -1 for things I'm not 100% sure are an issue, but you asked for it ;)
15:37:44 bauzas and I even prefer or I could miss the comments :)
15:38:11 bauzas sfinucan: well, I'm French so arguing is like my 2nd nature
15:39:01 openstackgerrit Matt Riedemann proposed openstack/nova: Ensure build request exists before creating instance https://review.openstack.org/427775
15:39:02 mriedem dansmith: melwitt: bauzas: ^ i'm assuming some sort of transactional lock would be best there, but not sure how to do it
15:40:18 dansmith mriedem: you can't, they're in separate databases
15:40:25 mriedem thought about that
15:40:26 mriedem yeah
15:40:28 dansmith mriedem: you should create the instance, then try to delete the buildreq, and if that fails, delete the instance
15:40:31 dansmith like we do below
15:42:04 mriedem we have a window either way
15:42:16 mriedem i'd rather not create the instance in the first place
15:42:44 mriedem if the build request doesn't exist before we create the instance, this catches it, if the build request doesn't exist after we create the instance, the existing check at the bottom handles it
15:43:20 dansmith mriedem: there's no window this way,
15:43:40 dansmith mriedem: and the check at the bottom will have to undo a bunch of other stuff, and will have done actions, notifications, etc
15:43:44 dansmith mriedem: but your call
15:44:14 dansmith mriedem: actually, yeah, that's fine
15:44:34 mriedem huh? why my change we don't create the instance, and continue, so the actions and notifications don't happen
15:44:54 dansmith mriedem: I meant if you delay the delete until below
15:44:58 mriedem s/why/with/
15:45:42 openstackgerrit Andy McCrae proposed openstack/nova: Allow placement endpoint interface to be set https://review.openstack.org/426163
15:45:43 dansmith since we don't have the instance mapping until a little later, we can't delete the build request yet up there, which means we have to wait
15:47:38 bauzas mriedem: dansmith: catching up your convo
15:47:38 openstackgerrit Lee Yarwood proposed openstack/nova: libvirt: Remove redundant bdm serial mangling and saving during swap_volume https://review.openstack.org/427364
15:47:57 dansmith mriedem: let me try to linearize the mess that came out of my head above
15:48:26 dansmith mriedem: when we create the instance where we do today, it's not find-able as an instance by the api until the instance mapping is created below
15:48:32 bauzas dansmith: mriedem: I think most of the corner cases about synchronizing have been written by alaski in its conductor method
15:48:38 bauzas lemme find it
15:48:54 dansmith mriedem: so if we delete the buildreq due to an external delete,
15:49:23 dansmith mriedem: and then create the instance, it's still not find-able until we add the instance mapping and then delete the buildreq
15:49:55 dansmith mriedem: so there might be a race between creating the mapping and noticing the buildreq is gone, but that is an extremely short time period
15:49:56 dansmith make sense?
15:51:41 mriedem dansmith: yeah, and my patch makes that window even smaller yes?
15:52:30 dansmith mriedem: hang on, I'm reading _get_instance()
15:52:47 mriedem dansmith: are you suggesting we move all of https://github.com/openstack/nova/blob/d308d1f70e7e840b1b0c8f4307998d89f9a5ddff/nova/conductor/manager.py#L925-L949 to right after the instance is created, before the notification and action stuff?
15:53:37 mriedem if we moved that up, we wouldn't need to keep track of bdms to cleanup...
15:54:09 mriedem i'm going to start on fixing the return/continue bug
15:54:48 dansmith mriedem: check this out: https://github.com/openstack/nova/blob/master/nova/compute/api.py#L2295-L2302
15:55:03 dansmith mriedem: that will short-circuit the logic we put in to make this atomic across both dbs
15:55:20 dansmith mriedem: and it says we should remove it once people have to use cellsv2
15:55:39 dansmith that will effectively go straight to the default cell without requiring the buildreq/instmap lock that we have in place
15:55:43 mriedem yeah i saw that last night, and remember specifically asking alaski to put that TODO in there
15:56:07 dansmith so, can we (a) remove that and see how it goes, (b) merge it if it looks good and see if the fails go away?
15:58:43 openstackgerrit Dan Smith proposed openstack/nova: WIP: Remove pre-cellsv2 short circuit in instance get https://review.openstack.org/427782
15:59:17 mriedem dansmith: would we get into that else in this case though? the inst_map could exist and have the cell mapping set
15:59:45 mriedem i guess if that's the case, the cleanup routine in conductor should have already deleted the instance
16:00:13 mriedem crap i havea 1x1, bbiab
16:04:11 dansmith hmm
16:08:01 bauzas dansmith: mriedem: so, I actually wonder why we wouldn't reproduce the logic in build_instances() where when refreshing the instance, we accept that there could be an InstanceNotFound ?
16:08:19 bauzas dansmith: mriedem: but here, we would just access the BuildRequest record instead
16:08:43 bauzas when we get_new_instance()
16:08:57 dansmith bauzas: until we have updated the instance mapping the instance itself could not be deleted, and the buildreq is how that happens
16:09:23 bauzas I'm missing a race condition here then
16:10:20 dansmith the one on L2311 also introduces a race between deleting the map and the instance, so I'll remove that one too
16:11:18 openstackgerrit Ildiko Vancsa proposed openstack/nova: Implement new attach/detach Cinder flow https://review.openstack.org/330285
16:11:19 openstackgerrit Ildiko Vancsa proposed openstack/nova: Remove check_attach https://review.openstack.org/335358
16:13:40 bauzas dansmith: both conditions are for existing instances that needed to have instance mappings, right?
16:14:03 bauzas dansmith: for a fresh new ocata instance, we suppose the mapping is created, right?
16:14:40 dansmith bauzas: as of ocata, all instances old and new have said mappings, yes
16:15:13 bauzas okay, so I agree with you that we should wipe those conditionals
16:15:22 bauzas because it's confusing
16:16:45 dansmith s/confusing/wrong.

Earlier   Later