Earlier  
Posted Nick Remark
#openstack-nova - 2017-02-01
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.
16:25:08 bauzas dansmith: now, say that we remove those conditionals, a GET /servers?uuid=<> would return a 404 in case the BuildRequest is deleted, right?
16:25:48 bauzas dansmith: if so, I think you found the best approach for fixing the issue, kudos
16:25:52 dansmith bauzas: if it's unmapped
16:26:04 bauzas dansmith: but this is mapped only after scheduling, right?
16:26:54 bauzas I mean
16:26:54 dansmith bauzas: yes
16:27:01 bauzas someone deletes an instance
16:27:07 bauzas then it deletes the mappings
16:27:17 bauzas so, bim
16:27:43 bauzas we go to the mappings equal None and then baaam, BuildRequestNotFound, pif paf 404
16:28:04 bauzas unless I missed that
16:28:11 bauzas s/that/something
16:32:07 openstackgerrit Lee Yarwood proposed openstack/nova: libvirt: Remove redundant bdm serial mangling and saving during swap_volume https://review.openstack.org/427364
16:35:23 kashyap Hi folks, has anyone seen this error with today's Git?
16:35:29 kashyap 017-02-01 15:55:46.453 | No hosts found to map to cell, exiting.
16:35:33 mriedem when conductor cleans up the build request, if it's already gone and it deletes the instance, why doesn't it also clean up the instance mapping?
16:35:38 mriedem dansmith: ^?
16:35:56 kashyap My current DevStack instance is failing with the above. /me looks at the logs
16:36:08 mriedem kashyap: you need to have the placement-api enabled
16:36:19 mriedem kashyap: http://lists.openstack.org/pipermail/openstack-dev/2017-January/111295.html
16:36:21 kashyap Ah!
16:36:23 kashyap That's it
16:37:01 bauzas mriedem: kashyap: not sure this error is related
16:37:11 bauzas but /me shrugs
16:37:18 kashyap bauzas: But mriedem is right that I don't have the placement API
16:37:23 bauzas sure
16:37:28 mriedem there won't be a host if n-cpu fails to start
16:37:39 bauzas ah true
16:37:40 mriedem which kills map_cell_and_hosts
16:37:41 mriedem or whatever
16:37:45 bauzas okat
16:37:53 bauzas whatever, like you said
16:38:02 trinaths1 Hi
16:38:27 bauzas mriedem: what do you mean "when conductor cleans up the build request" ?
16:38:39 bauzas mriedem: when we found a place, and then we create the instance?N
16:38:40 mriedem bauzas: https://github.com/openstack/nova/blob/d308d1f70e7e840b1b0c8f4307998d89f9a5ddff/nova/conductor/manager.py#L925-L949
16:39:03 kashyap mriedem: This is the only thing that I need, right? - 'placement-api' (Yep, looking here - http://logs.openstack.org/06/425806/8/check/gate-tempest-dsvm-cells-ubuntu-xenial/e958e02/logs/localrc.txt.gz)
16:39:05 mriedem if the build request is deleted before/during scheduling, we delete the instance from the cell, but why don't we also cleanup the instance mapping record?
16:39:16 mriedem kashyap: yes
16:39:39 dansmith mriedem: probably should.. we do in the compute/api if we get similarly interrupted by a delete
16:39:59 mriedem ok, was just thinking about orphans
16:40:06 alaski is the instance hard deleted? If there's an instance record there should be a mapping
16:40:19 mriedem alaski: it's not hard deleted no
16:40:21 dansmith alaski: oh good point
16:40:24 bauzas I was about to sat that
16:40:30 bauzas because we soft delete
16:40:44 bauzas but alaski beated me
16:40:45 mriedem the build request is hard-deleted, but if the instance exists then we don't care about the build request
16:40:56 mriedem yeah i suppose for nova list --deleted
16:40:59 dansmith yeah
16:41:00 mriedem touche
16:41:30 dansmith alaski: nice save from the sidelines :)
16:42:44 alaski dansmith: I lurk so I can occasionally seem smart still :)
16:43:01 dansmith alaski: not much danger of that being forgotten, but as you wish :)
16:44:02 mriedem now if he'd only tell me how to untangle this one test...
16:44:33 mordred mriedem: dammit man. now I'm participating in a database related thread on the mailing list
16:44:34 alaski have you tried turning it off and back on...
16:44:39 mordred mriedem: there is no way this ends well for anyone
16:54:42 bauzas mriedem: are you considering keeping that https://review.openstack.org/#/c/427775/1 or do you prefer to remove the conditions for the get() method ?
16:54:50 bauzas conditionals* even
16:54:56 bauzas I tend to prefer the latter
16:55:04 dansmith bauzas: we need to remove the TODOs regardless
16:55:18 bauzas we = who does that so I could +2 it ?
16:55:27 dansmith I already have the patch up
16:55:32 bauzas oh, nice
16:55:35 dansmith WIP, so don't go look yet
16:55:43 bauzas please be a gentleman, so I could CC it :)
16:56:27 dansmith wat?
16:57:32 dansmith oh fuck. that breaks every api samples test.. cripes.
17:06:01 bauzas dansmith: cells meeting or should we leave you having fun with api sample tests ?
17:06:12 dansmith bauzas: we just had it and you didn't show up
17:06:27 dansmith so we adjourned to do work
17:06:48 bauzas heh
17:11:41 mriedem so i can't figure this out,
17:11:42 mriedem https://github.com/openstack/nova/blob/master/nova/tests/unit/conductor/test_conductor.py#L1513
17:11:53 mriedem i changed that to self.assertEqual(3, build_and_run_instance.call_count)
17:11:59 mriedem and it always fails saying it's only called once
17:12:04 mriedem but there are 3 build requests in that test
17:12:35 mriedem as far as i can tell, things should be peachy
17:13:13 edleafe jaypipes: are you working on https://review.openstack.org/#/c/404472/, or do you want me to fix the tests?
17:14:55 bauzas mriedem: but we only schedule once with 3 requests in it, right ? https://github.com/openstack/nova/blob/master/nova/tests/unit/conductor/test_conductor.py#L1512
17:15:44 mriedem bauzas: it's asserting this is called https://github.com/openstack/nova/blob/d308d1f70e7e840b1b0c8f4307998d89f9a5ddff/nova/conductor/manager.py#L959
17:15:48 mriedem which is in a for loop
17:15:53 mriedem over the list of build requests

Earlier   Later