Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-11-02
17:04:31 efried woot
17:04:32 melwitt mdbooth: yes, I get that. I wanted to see where a "mis-named field" was occuring for my own edification
17:06:26 melwitt I'll pull down the change and revert the bottom patch to see what was going on
17:07:11 mdbooth melwitt: It's really not worth it.
17:07:26 mdbooth melwitt: Couple of reasons:
17:07:39 melwitt well, I want to learn what was going on, so I'm going to do it
17:07:39 mdbooth Firstly, it might not even have been that patch as I've reordered them a bunch of times
17:07:46 melwitt since it doesn't say it in the commit message
17:07:56 mdbooth Secondly, I'll have fixed the issue already, so you won't see a failure regardless
17:08:24 melwitt well, we'll see
17:08:29 mdbooth So in your spelunking you're going to have to look for both the patch affected, and the revision of that patch which contained a bug which was picked up by functional but not unit testing
17:08:53 mdbooth And even when you know, it still doesn't matter, because it's just a general improvement anyway to not use a mock when you can use a real object
17:09:28 melwitt no, I'm just going to try reverting the bottom patch and maybe that'll show the problem. if not, then agreed
17:09:28 mdbooth Absolutely not worth the effort, imho
17:09:41 mdbooth It won't show the problem
17:09:48 mdbooth I'll most definitely have fixed the bug
17:10:26 melwitt that doesn't really make sense, you fixed the bug before changing to a real object?
17:10:42 mdbooth So, I wrote a unit test.
17:10:54 mdbooth In that test I accessed object.fo0
17:11:06 mdbooth The test passed anyway, because object was a mock
17:11:16 mdbooth But functional testing failed because it didn't use a mock
17:11:27 mdbooth I found this and changed the unit test to also not use a mock
17:11:31 mdbooth But I also fixed the bug
17:11:34 openstackgerrit Jay Pipes proposed openstack/nova master: placement: adds REST API for nested providers https://review.openstack.org/384807
17:11:35 openstackgerrit Jay Pipes proposed openstack/nova master: placement: update client to set parent provider https://review.openstack.org/385693
17:11:51 jaypipes efried: ^ fixed tetsuro's niggle and the tree= param.
17:12:04 efried ack
17:12:07 mdbooth This is a preventative patch to ensure I can't write a unit test with a similar bug in the future
17:12:31 mdbooth We've spent too long on it already, lets just drop it.
17:12:34 melwitt mdbooth: oh, I see
17:23:55 openstackgerrit Eric Fried proposed openstack/nova master: placement: Parse granular resources & traits https://review.openstack.org/514091
17:23:55 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Numbered groupings to GET /allocation_candidates https://review.openstack.org/514092
17:23:56 stephenfin dansmith: This looks like something you care about https://review.openstack.org/#/c/517158/
17:40:10 openstackgerrit Stephen Finucane proposed openstack/nova master: Cleanup update_instance cell mapping handling https://review.openstack.org/467380
17:40:10 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove cells v2 transition code from update_instance https://review.openstack.org/467382
18:00:08 openstackgerrit Eric Fried proposed openstack/nova master: Parse granular resources/traits from extra_specs https://review.openstack.org/515151
18:00:09 openstackgerrit Eric Fried proposed openstack/nova master: Granularize resources_from_{flavor|request_spec} https://review.openstack.org/515223
18:00:10 openstackgerrit Eric Fried proposed openstack/nova master: ResourceRequest.to_querystring() https://review.openstack.org/515811
18:00:34 efried jaypipes Both series are now fork-based on the common RequestGroup patch ^ ----^
18:05:27 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: libvirt: do not remove inst_base when volume-backed during resize https://review.openstack.org/517388
18:10:06 jaypipes efried: cool. reviewing now.
18:12:54 openstackgerrit Matt Riedemann proposed openstack/nova stable/ocata: libvirt: do not remove inst_base when volume-backed during resize https://review.openstack.org/517390
18:15:18 openstackgerrit Matt Riedemann proposed openstack/nova master: libvirt: Don't VIR_MIGRATE_NON_SHARED_INC without migrate_disks https://review.openstack.org/507202
18:37:27 mriedem easy fix https://review.openstack.org/#/c/511459/
18:54:42 melwitt mriedem: I noticed there are several other places in the file (in log calls) where the exception message isn't converted to unicode. is that because the log statement won't raise in that case? and the more important thing are the ones that are messages in exceptions we'll raise?
18:58:31 mriedem not sure, i thought six.text_type(ex) handled a lot of this for us,
18:58:37 mriedem but might have been a different type of error
18:58:59 melwitt oh, does LOG.x do text_type() underneath?
18:59:12 mriedem no
18:59:26 openstackgerrit Merged openstack/nova master: Implement power_off/power_on for the FakeDriver https://review.openstack.org/509935
19:00:27 mriedem melwitt: e.g. https://github.com/openstack/nova/commit/73fcf4628089dd784889062e916b80d3fc9988a2#diff-d362bac3c354dd65e061ca91a8d1e14e
19:01:10 melwitt k. just curious
19:01:12 mriedem so might just be a case where we used to cast things using str()
19:01:26 mriedem which makes me nervous about https://github.com/openstack/nova/commit/8fa97d91c005147cd3eba7c6f03136ade84ba46d#diff-d362bac3c354dd65e061ca91a8d1e14e
19:02:07 mriedem which landed in queens, which means no one in asia has probably tried testing with that yet
19:03:21 mriedem heh, i guess that was in the run up to https://review.openstack.org/#/c/507202/
19:03:28 mriedem but i don't see how it's related
19:04:36 melwitt oh, this part huh https://github.com/openstack/nova/commit/8fa97d91c005147cd3eba7c6f03136ade84ba46d#diff-ec64afccc111bbe81f5c85b27d73b41bR652 should be a to_unicode call?
19:05:03 melwitt er, the comment says should not be unicode. I guess only if it's a name or such
19:05:49 mriedem idk, i don't speak unicode
19:09:11 mriedem edleafe: http status code question
19:09:59 mriedem here is the scenario:
19:10:10 mriedem admin tries to migrate/evacuate an instance with a specified destination host,
19:10:32 mriedem for the dest host machinery to work, we need to put it into the *existing* request spec for the instance and pass that through api->conductor->scheduler
19:11:13 mriedem we have some compat code from newton where if the instance request spec isn't found, we silently create one (without the requested dest host) and pass it to the scheduler, and you might accidentally migrate the instance to what you specified, but it's definitely not intentional
19:11:45 mriedem so, if i were going to change that "if not request_spec: create one and fake it" thing to be a hard error in the API, is that a 400 response? i don't think 409 is correct because the user can't fix it.
19:20:32 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM Driver: config drive https://review.openstack.org/409404
19:21:05 edleafe mriedem: so it wouldn't matter which host they specified?
19:21:13 edleafe mriedem: if not, then yeah, 400
19:22:29 mriedem right, request 1 with host: 400: we don't have a request spec to do this, request 2 w/o host: cool, we pick a host for you as before
19:22:54 mriedem the 400 would have to say something about not being able to support migrating with a target host
19:22:59 mriedem as a hint you need to remove the host and retry
19:23:55 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM Driver: config drive https://review.openstack.org/409404
19:25:35 openstackgerrit Merged openstack/nova master: Add 'done' to migration_get_in_progress_by_host_and_node filter https://review.openstack.org/511351
19:25:59 edleafe Oh, so it *would* succeed if they modify the request
19:26:30 mriedem yeah, is that a 409 then?
19:26:34 edleafe Then I would go with the 409, with an explanation about not supporting a target host for that instance
19:26:38 mriedem 409: can't migrate with a target host
19:26:42 mriedem ok
19:27:21 edleafe Yeah, 400 usually means that the client request is malformed or otherwise invalid
19:29:09 mriedem bauzas: have we ever consider *not* persistent RequestSpec.requested_destination()
19:29:18 mriedem since that seems to be a per-request kind of thing only for move operatoins
19:30:44 mriedem or, we should null out reqspec.requested_destination.host/node in reset_forced_destinations
19:31:54 openstackgerrit OpenStack Proposal Bot proposed openstack/os-vif master: Updated from global requirements https://review.openstack.org/511035
19:40:37 openstackgerrit OpenStack Proposal Bot proposed openstack/os-vif master: Updated from global requirements https://review.openstack.org/511035
20:33:24 openstackgerrit Eric Fried proposed openstack/nova master: placement: Parse granular resources & traits https://review.openstack.org/514091
20:33:25 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Numbered groupings to GET /allocation_candidates https://review.openstack.org/514092
20:33:36 efried jaypipes Placement side ^
20:45:42 openstackgerrit Merged openstack/nova master: libvirt: properly decode error message from qemu guest agent https://review.openstack.org/511459
20:53:35 openstackgerrit Eric Fried proposed openstack/nova master: Parse granular resources/traits from extra_specs https://review.openstack.org/515151
20:53:35 openstackgerrit Eric Fried proposed openstack/nova master: Granularize resources_from_{flavor|request_spec} https://review.openstack.org/515223
20:53:36 openstackgerrit Eric Fried proposed openstack/nova master: ResourceRequest.to_querystring() https://review.openstack.org/515811
20:53:43 efried jaypipes Scheduler side ^
20:58:37 mnaser darn
20:58:43 mnaser another cell v2 quirk
20:59:03 mnaser we have a few instances that started to get cell_id=NULL assigned in mappings
20:59:19 mnaser but they exist in the cell with status=error
20:59:40 mnaser so they appear in lists but cannot be retrieved
21:00:12 melwitt mnaser: are they instances that failed to schedule? if so, they are indeed not assigned to a cell, but are inside the cell0 database
21:00:50 mnaser melwitt: but i believe they should at least be delete-able or i should be able to query them

Earlier   Later