Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-11
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,
17:22:20 dansmith is it's a cast, and we could check the status of the migration before we make that call, but it might be different when it would land on the compute and do its thing
17:22:34 dansmith which maybe isn't a problem because it's just less permissive not more
17:23:38 mriedem dansmith: yeah, worst case is the api races and then the compute ignores the abort request, because i think it's ultimately the libvirt driver that has to ack the abort request
17:23:39 dansmith mriedem: and we'd have to have the migration in rpcapi in order to know the status and whether or not we need to abort based on theversion we're pinned to
17:24:37 mriedem oh because we don't pass the migration record itself to live_migration_abort, just the id
17:24:51 dansmith right
17:25:07 mriedem hmm, well, we have to bump the compute service version anyway, so we can do that in the same version bump
17:25:29 melwitt cfriesen: I think you might need this https://review.openstack.org/#/c/515008
17:25:42 mriedem i was just looking that up :)
17:25:52 dansmith mriedem: well you don't actually have to change the rpc call, you just have to change the interface to the rpcapi method
17:26:03 dansmith mriedem: or you do it based on service version in compute/api before you get that far
17:26:13 cfriesen melwitt: looks promising, thanks
17:26:34 mriedem dansmith: right so pass migration object to rpcapi method, if can't send new version, send migration.id

Earlier   Later