| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-11 | |||
| 16:12:33 | mriedem | why would we care? | |
| 16:12:45 | mriedem | if you're out of tree, you're out of tree, and we don't block in-tree changes because of that | |
| 16:12:57 | mriedem | best we do at times is put something in the ML as a heads up for known internal interface changes | |
| 16:14:55 | bauzas | that remembers me some pg discussion... | |
| 16:14:58 | sean-k-mooney | not run by nova run by the maintiners of nova-lxd e.g. canonical | |
| 16:15:17 | bauzas | which I think we cleary made the statement that non-supported things for a project are potentially subject to break | |
| 16:15:32 | mriedem | bauzas: we have jobs we can run for pg testing if we ant | |
| 16:15:33 | mriedem | *want | |
| 16:15:34 | bauzas | hence other projects gating on that project shouldn't really support that too | |
| 16:15:36 | sean-k-mooney | mriedem: oh im not expecting nova to block in tree change i just expcting that nova-lxd team could benifit form know that its broken | |
| 16:15:39 | mriedem | there is a neutron job in the experimental queue that runs with pg | |
| 16:16:09 | mriedem | sean-k-mooney: nova-lxd team can listen on nova changes, run jobs and post results for failures somewhere if they want to detect them early | |
| 16:17:06 | bauzas | mriedem: I'm not saying it's impossible | |
| 16:17:16 | bauzas | mriedem: just that we broke ceilometer once | |
| 16:17:36 | bauzas | because ceilo was gating something we don't test | |
| 16:17:44 | mriedem | sure, i'm sure ceilometer broke us once indirectly too when they were part of tempest, 3 years ago | |
| 16:18:14 | bauzas | and the result was like a best-effort fix and a summit discussion that led to "come and help us make pg first-grade if your production runs on something we don't test" | |
| 16:18:34 | mriedem | there is nothing 2nd grade about pg wrt nova | |
| 16:18:51 | bauzas | I think we're diverting anyway | |
| 16:19:18 | bauzas | I just tried to explain that I feel you shouldn't blame yourself for breaking ansible if they gate on something we don't test | |
| 16:19:26 | dansmith | mriedem: this seems like a legit "stop the bleeding" patch for stable.. I saw you reviewed it once, but.. are you cool with it? https://review.openstack.org/#/c/543569 | |
| 16:19:34 | bauzas | we can certainly fix that tho | |
| 16:19:37 | mriedem | bauzas: i don't really blame myself :) | |
| 16:19:41 | sahid | mriedem: if you have a moment that one looks ready and important enough for some use-cases https://review.openstack.org/#/c/553072/ | |
| 16:19:44 | mriedem | sucks for odyssey4me though | |
| 16:20:05 | mriedem | jesus, did the red hat team bug meeting just end? | |
| 16:20:05 | bauzas | mriedem: I agree, and that requires a fix | |
| 16:20:22 | bauzas | mriedem: nah, not bugs | |
| 16:20:35 | bauzas | but yes, meeting ended | |
| 16:21:23 | mriedem | dansmith: i've just been putting off going through it again because it's hairy on stable | |
| 16:21:31 | mriedem | i'm not opposed to it no | |
| 16:21:33 | dansmith | it's big, | |
| 16:21:37 | dansmith | but it doesn't seem that hairy | |
| 16:21:56 | mriedem | the fact it was re-introducing a regression in the earlier PS indicates to me it's hairy | |
| 16:22:05 | dansmith | it's checking a thing and raising an exception to avoid you killing data puppies | |
| 16:22:29 | dansmith | okay I guess I don't quite know what that means | |
| 16:22:55 | mriedem | https://review.openstack.org/#/c/543569/1/nova/virt/libvirt/driver.py@1216 | |
| 16:23:03 | mriedem | meaning it's easy to break stuff | |
| 16:23:34 | dansmith | ah I see | |
| 16:23:42 | mriedem | anyway, i told lyarwood i'd go through it again last week and forgot | |
| 16:23:44 | dansmith | weyall.. data corruption seems more bad | |
| 16:23:55 | mriedem | and with no more eol, we have time to fix it if breaks something | |
| 16:24:16 | dansmith | by "time" you mean "forever and ever" right? | |
| 16:24:31 | odyssey4me | mriedem eh? what sucks for me? :) | |
| 16:35:06 | openstackgerrit | sahid proposed openstack/nova master: libvirt: move version to string in utils https://review.openstack.org/560455 | |
| 16:35:38 | mriedem | dansmith: i'll trade you https://review.openstack.org/#/c/536722/ | |
| 16:36:08 | dansmith | mriedem: seems a little lopsided | |
| 16:39:33 | mriedem | sahid: comments inline | |
| 16:41:43 | openstackgerrit | Merged openstack/nova master: Expose shutdown retry interval as config setting https://review.openstack.org/552483 | |
| 16:44:26 | dansmith | mriedem: you okay with this? https://review.openstack.org/#/c/484997/14/nova/conf/libvirt.py | |
| 16:44:42 | dansmith | I suggested something similar, but not exactly this.. although this works and is easier than what I suggested | |
| 16:44:54 | dansmith | it gets us config load-time verification of that parameter | |
| 16:44:57 | dansmith | instead of at runtime | |
| 16:45:12 | dansmith | but I don't think we have anywhere else that we subclass an opt (even though that's suggested by the oslo docs AFAICT) | |
| 16:45:46 | mriedem | you just pulled me out of https://review.openstack.org/#/c/543569/ | |
| 16:46:05 | dansmith | it needs only a glance | |
| 16:46:22 | mriedem | so does https://review.openstack.org/#/c/560454/ | |
| 16:47:18 | mriedem | agree with load-time config validation | |
| 16:47:44 | mriedem | wouldn't it be simple to do a subclass inline in that module | |
| 16:47:45 | mriedem | ? | |
| 16:49:05 | dansmith | mriedem: wrong link? | |
| 16:50:53 | mriedem | no, i asked for a review on that earlier - since it's kicking things out of CI on master | |
| 16:51:28 | dansmith | I'm confused | |
| 16:51:56 | dansmith | mriedem: melwitt tssurya: do we need a cells meeting today? I have nothing new | |
| 16:51:58 | mriedem | you asked me to review lee's thing, then while i was doing that, you asked me to look at sahid's thing, so in kind i threw that bug fix at you while you were reviewing sahid's thing :) | |
| 16:52:06 | dansmith | and I reviewed some of tssurya's patches to make myself feel better this morning | |
| 16:52:16 | mriedem | i dont think i have anything | |
| 16:52:24 | tssurya | me niether | |
| 16:52:26 | melwitt | I was just about to ask that. I don't think I have anything either | |
| 16:52:33 | tssurya | dansmith: thanks for the reviews | |
| 16:52:46 | mriedem | i reviewed tssurya's patch to delete RPs and host mappings when we delete compute services but my comments weren't controversial | |
| 16:52:51 | dansmith | sweet, cancelled thusly | |
| 16:53:52 | tssurya | jaypipes: I am still working on the func test, haven't got very far (dangling between understanding service fixtures) , however I will try again tomorrow and ping you if it becomes too much for me | |
| 16:54:23 | mriedem | tssurya: i can probably help with that, | |
| 16:54:40 | mriedem | could be a patch before yours in the series to show that we didn't delete HMs and RPs when we deleted a compute service, and then yours could build on top to assert that we do | |
| 16:55:10 | dansmith | yeah I like that | |
| 16:55:18 | dansmith | I would call that "mriedem style" | |
| 16:55:20 | mriedem | it would also keep the fix patch smaller | |
| 16:55:26 | mriedem | like gangham style? | |
| 16:55:28 | imacdonn | I can certainly related to getting all tangled up in fixtures :/ | |
| 16:55:35 | imacdonn | relate* | |
| 16:55:44 | dansmith | mriedem: similar, yeah | |
| 16:55:59 | tssurya | mriedem: that would be really awesome for me, writing the test_services from stratch is causing me a headache because its my first go | |
| 16:56:42 | tssurya | imacdonn: :) | |
| 16:59:37 | imacdonn | mriedem dansmith speaking of which ... I tried to implement that compute-is-configured-to-use-placement check based on auth_type, but I'm back in unit-test hell ... 27 tests that start a compute service trip on it .. and if I set auth_type in the conf fixture, a bazillion other tests fail ... some due to auth_url not set | |
| 17:04:27 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: Localdisk https://review.openstack.org/549300 | |
| 17:08:22 | mriedem | dansmith: lyarwood: done https://review.openstack.org/#/c/543569/ | |
| 17:10:57 | mriedem | dansmith: you should hit https://review.openstack.org/#/c/559987/ while we wait for lee to fix that up | |
| 17:11:27 | dansmith | ya | |
| 17:19:14 | mriedem | dansmith: left some replies in https://review.openstack.org/#/c/536722/ based on earlier discussions | |
| 17:19:31 | mriedem | as for the cleanup routine, i think some of that is going to have to just be impl trial and error, but i left some ideas | |
| 17:20:03 | dansmith | mriedem: how is it not a new rpc call? | |
| 17:20:19 | dansmith | oh because live_migration_abort() I guess? | |
| 17:20:22 | mriedem | yes | |
| 17:20:29 | mriedem | read the rest of my comment :) | |
| 17:21:26 | mriedem | there is one thing i'm not totally sure about, and it's that http://pythonhosted.org/futures/#concurrent.futures.Executor.shutdown says "If wait is False then this method will return immediately and the resources associated with the executor will be freed when all pending futures are done executing. Regardless of the value of wait, the entire Python program will not exit until all pending futures are done executing." | |
| 17:21:53 | cfriesen | melwitt: lyarwood: I'm seeing something weird on Pike: http://paste.openstack.org/show/718967/ Basically heat creates an instance and attaches volumes to it, then on stack teardown it runs into problems detaching the volume from the instance. Eventually the token times out and it retries, only this time libvirt doesn't actually recognize the specified device (making me wonder whether it had actually been detached at this | |
| 17:21:55 | cfriesen | point.) | |
| 17:21:57 | mriedem | however, if we do the done callback and Futures.cancel() thing i laid out, i think the pool will exit once we cancel those futures | |
| 17:22:05 | dansmith | mriedem: so the thing that sucks about that, IMHO, | |