| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-02 | |||
| 16:35:45 | kashyap | Thanks for the links | |
| 16:37:27 | melwitt | mriedem: oh, bummer, didn't realize it was that unreliable. was thinking they'd be regression tests | |
| 16:37:41 | mriedem | +2 on the bottom change | |
| 16:37:54 | mriedem | melwitt: they might be better, not really sure, would have to run them through a bunch of times to see | |
| 16:38:06 | mriedem | https://bugs.launchpad.net/nova/+bug/1524898 | |
| 16:38:08 | openstack | Launchpad bug 1524898 in OpenStack Compute (nova) "Volume based live migration aborted unexpectedly" [High,In progress] | |
| 16:38:21 | melwitt | yep, understood | |
| 16:38:26 | mriedem | now that we're using pike UCA, maybe that's not as bad | |
| 16:40:07 | melwitt | ah, to get newer libvirt and qemu | |
| 16:40:20 | mriedem | yes we now have libvirt 3.6.0 and qemu 2.10 | |
| 16:42:53 | mriedem | so we have control over the live migration job per-branch, but the neutron multi-node job runs live migration tests too, and that would maybe start failing on stable branches if we enabled the volume-backed test in tempest; but the neutron multinode job might be non-voting on stable too... | |
| 16:43:00 | mriedem | all things that must be considered... | |
| 16:43:49 | melwitt | good points | |
| 16:46:32 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: add nested resource providers https://review.openstack.org/377138 | |
| 16:46:32 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: allow filter providers in tree https://review.openstack.org/377215 | |
| 16:46:33 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: adds REST API for nested providers https://review.openstack.org/384807 | |
| 16:46:33 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: update client to set parent provider https://review.openstack.org/385693 | |
| 16:46:35 | jaypipes | efried: ^^ n-r-p rebased. | |
| 16:46:44 | jaypipes | efried: and addressed your comments. thanks. | |
| 16:46:52 | efried | jaypipes Ack. | |
| 16:55:24 | efried | jaypipes https://review.openstack.org/#/c/384807/ -- placement-api-ref update for ?tree= ? | |
| 16:55:44 | jaypipes | efried: good spot. will add. | |
| 16:56:12 | efried | jaypipes Durn patch-level comments always getting lost. | |
| 16:56:43 | jaypipes | efried: currently pulling in the "lost PS3 changes" in https://review.openstack.org/#/c/516778. sorry about that... | |
| 16:56:54 | efried | Cool | |
| 16:57:22 | melwitt | mdbooth: question in this unit test patch, what's the mis-named field? I didn't notice anything in this change or the one above it https://review.openstack.org/#/c/502126/3//COMMIT_MSG | |
| 16:59:10 | mdbooth | melwitt: IIRC the error was in the subsequent test | |
| 16:59:27 | mdbooth | But it wasn't picked up because it was just a mock | |
| 17:00:11 | melwitt | mdbooth: "subsequent test" meaning the test in the next patch right? | |
| 17:01:11 | mdbooth | melwitt: IIRC, yes? | |
| 17:01:27 | mdbooth | I've slept many times since writing that. Almost certainly yes. | |
| 17:01:46 | mdbooth | It doesn't really matter, though, because exactly which test it is doesn't matter. | |
| 17:02:06 | mdbooth | The point is that the way it was written allowed a test to assume the existence of a field which doesn't exist. | |
| 17:03:32 | melwitt | mdbooth: yeah, I get the point, this commit message is so vague there's no way to see or understand what the problem was, is all | |
| 17:03:53 | mdbooth | The problem is it's a Mock, which means anything goes. | |
| 17:04:09 | mdbooth | It could have been anything. Exactly what it was doesn't really matter. | |
| 17:04:11 | openstackgerrit | Jay Pipes proposed openstack/nova master: begin refactor AllocCandidates._get_by_filters() https://review.openstack.org/516778 | |
| 17:04:12 | openstackgerrit | Jay Pipes proposed openstack/nova master: build ProviderSummary objects in sep function https://review.openstack.org/516779 | |
| 17:04:12 | openstackgerrit | Jay Pipes proposed openstack/nova master: build alloc request resources for shared resources https://review.openstack.org/516780 | |
| 17:04:13 | openstackgerrit | Jay Pipes proposed openstack/nova master: create allocation request for single provider https://review.openstack.org/516781 | |
| 17:04:13 | openstackgerrit | Jay Pipes proposed openstack/nova master: finish refactor AllocCandidates._get_by_filters() https://review.openstack.org/516782 | |
| 17:04:14 | openstackgerrit | Jay Pipes proposed openstack/nova master: required traits for no sharing providers https://review.openstack.org/517027 | |
| 17:04:14 | openstackgerrit | Jay Pipes proposed openstack/nova master: WIP: handle traits with sharing providers https://review.openstack.org/517119 | |
| 17:04:17 | jaypipes | efried: k, done ^ | |
| 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/ | |