| 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 | |