Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-07-11
16:56:16 dansmith mriedem: yeah
16:56:17 mdrabe ya
16:56:21 mriedem ildikov: honestly i'm not looking at the review priorities etherpad, but ok
16:56:35 mriedem ildikov: with <3 weeks i've got my list of priority changes to review starred already
16:57:19 openstackgerrit Alex Szarka proposed openstack/nova master: Transform compute_task notifications https://review.openstack.org/482629
16:57:25 ildikov mriedem: just like how others do I guess...
16:57:31 mriedem ildikov: that does remind me though that i/we need to reach out to claudiub on hyperv and the xenapi team about checking out stvnoyes' change for live migration
16:57:43 mriedem since that also has to be done in those other drivers that support live migration
16:58:16 claudiub o/
16:58:41 ildikov mriedem: Steve has changes in those parts too, but definitely needs a double check
16:58:46 ildikov claudiub: hi :)
16:58:52 claudiub hello. :)
16:59:05 mriedem ildikov: there are no hyperv/xenapi changes in https://review.openstack.org/#/c/463987/
16:59:09 mriedem claudiub: see ^
16:59:21 mriedem that's the change to use the new cinder volume attach flow for live migration
16:59:46 mriedem the hyperv and xenapi drivers would need to do something for that too, building on that patch, as that's only the libvirt driver and the framework code in the compute manager
16:59:52 ildikov mriedem: hmm, I thought I saw related changes somewhere... :/
17:00:18 mriedem claudiub: https://review.openstack.org/#/c/330285/ is the change that enables all of this new flow from the API
17:00:53 ildikov claudiub: so tl;dr, we have a new attach/detach API in Cinder starting from Ocata and we're on the mission to get that supported in Nova, hopefully in Pike
17:02:17 claudiub sooo... basically the idea is to connect the volumes during pre_live_migration and disconnect them if the live migration failed?
17:02:27 claudiub don't we already do that?
17:03:02 ildikov we have different calls for that now
17:03:22 ildikov as earlier we removed the volume connection, but we didn't really inform Cinder about it
17:03:48 ildikov just kinda wired the volume on the new host and then unwired it on the old one
17:09:20 claudiub anyways. will look into it.
17:10:03 mriedem claudiub: yeah it's new modeling and API calls now
17:12:16 ildikov claudiub: in case of libvirt we had an initialize_connection call in the driver for instance which we don't need anymore
17:12:39 ildikov claudiub: I don't seem to find anything like that for HyperV, but I can very easily miss something
17:13:06 claudiub anyways... unrelated, and since you're right here right now, on 3rd party CI section of the Cinder wiki ( https://wiki.openstack.org/wiki/Cinder/tested-3rdParty-drivers#Third_Party_CI_Requirements ) they say that "CI's do not need to kick off tests until Jenkin's has given a +1". does something like this also applies to nova as well?
17:13:53 ildikov claudiub: if there's anything we should modify there, plz add a note to the review or ping stvnoyes and me here if you would have any questions or comments to quickly chat about here
17:14:03 stvnoyes claudiub, mriedem - I did take a cursory look at xen and hyperV and those drivers didn't seem to interact with cinder during migration (unlike libvirt)
17:15:25 openstackgerrit Alex Szarka proposed openstack/nova master: Transform compute_task notifications https://review.openstack.org/482629
17:15:27 mriedem claudiub: we've never said anything about that in nova
17:16:04 claudiub mriedem: which is why I'm asking. :)
17:16:08 mriedem claudiub: i assume that's for trying to minimize the amount of traffic that the 3rd party ci needs to do
17:16:13 claudiub yeap
17:16:25 mriedem claudiub: i'm fine with that, b/c jenkins could fail with a pep8 issue or something and need to be rechecked
17:16:41 mriedem i don't exactly know *how* a 3rd party ci listens for a jenkins +1 on a change to trigger
17:16:48 claudiub cool, thanks. :)
17:16:51 mriedem but assume it's on the gerrit/zuul stream of events
17:17:16 claudiub yeah, the ci team will know how to handle them. :)
17:17:55 mriedem granted, if jenkins is +1 and cores approve it before any other 3rd party ci reports in with failures because it took too long, then that's a risk,
17:18:15 mriedem but in general i'm assuming that mostly only applies to changes to the drivers themselves, or major internal changes to compute manager
17:18:26 mriedem i.e. i'll wait for hyperv ci to be happy on a hyperv driver change
17:20:28 openstackgerrit Rawan Herzallah proposed openstack/nova master: Adding NVMEoF for libvirt driver https://review.openstack.org/482640
17:20:35 claudiub mriedem: it seems that there are details on how to wait for Jenkins' +1 on commits on the bottom of the wiki: https://wiki.openstack.org/wiki/Cinder/tested-3rdParty-drivers#Third_Party_CI_Requirements
17:25:22 mriedem gibi: should i mark this as WIP until you sort out the functional test issue with shelve? https://review.openstack.org/#/c/448779/
17:28:34 openstackgerrit Rawan Herzallah proposed openstack/nova master: Adding NVMEoF for libvirt driver https://review.openstack.org/482640
17:29:52 openstackgerrit Rawan Herzallah proposed openstack/nova master: Adding NVMEoF for libvirt driver https://review.openstack.org/482640
17:34:01 ildikov mriedem: thank you!
17:48:29 claudiub ildikov: stvnoyes: yeah, we don't really interact with with cider api inside the hyper-v driver. we only login the storage targets. the hyper-v ci already ran over the patch you mentioned, including LiveBlockMigrationTestJSON.test_iscsi_volume and other live migration tests, except for test_volume_backed_live_migration (has a skip_because(bug=...) decorator).
17:49:20 claudiub so it should be fine, but I'd still like to take a closer look into it.
17:49:44 ildikov claudiub: thanks for checking
17:50:08 claudiub no problem. :)
17:50:41 ildikov claudiub: we should have a patch to enable that test as well in the related ones
17:54:15 mriedem claudiub: the ci will pass the live migration change and pass b/c it's still using the old flow
17:54:50 mriedem claudiub: it has to run on both https://review.openstack.org/#/c/330285/ and https://review.openstack.org/#/c/463987/ together
17:55:04 mriedem sec
17:55:04 mriedem which, i have a patch for that
17:55:14 mriedem https://review.openstack.org/#/c/481290/
17:55:25 mriedem claudiub: can you get the hyperv ci to run on that ^?
17:55:32 mriedem oh nvm it did
17:56:20 mriedem yeah looks like it's happy
18:06:12 mriedem who can review this for the last +2 https://review.openstack.org/#/c/454322/ ? the cinder change that depends on it is approved https://review.openstack.org/#/c/454287/ and the test stack testing it is all green https://review.openstack.org/#/c/480778/
18:27:39 mriedem nevermind, looks like that entire stack is going to have to be rebased b/c the 2.50 microversion is going through the gate
18:39:20 jaypipes mriedem: had some comments on that anyway.
18:41:23 mriedem ok
18:50:26 mriedem jaypipes: replied,
18:50:33 mriedem see if my reply on the policy stuff makes sense
18:50:40 jaypipes k
18:52:48 openstackgerrit Merged openstack/nova master: Fix error message when support matrix entry is missing a driver https://review.openstack.org/480708
18:59:58 mriedem melwitt: i think we need to handle this differently https://review.openstack.org/#/c/446243/ - but you could hold off on updating it for awhile, i'm going to dive into the counting instances change at the bottom
19:00:02 mriedem for about 4 hours
19:00:37 melwitt mriedem: cool, saw the comment. makes sense
19:01:10 melwitt and thanks in advance for reviewing counting instances
19:03:20 dansmith mriedem: melwitt I don't really disagree, although I think that if they don't notice the deprecation message, they won't notice the error if we exit with zero, which means we're just kicking the can down the road
19:03:34 dansmith but if making it a no-op gets us out of deprecation jail, then whatever
19:04:34 melwitt yeah, fair point
19:04:35 mriedem yes it avoids deprecation jail
19:10:31 mriedem gd those giant ass try/except blocks
19:29:01 mriedem melwitt: how does check_deltas work with the 0 delta again?
19:29:31 melwitt mriedem: it just means check usage vs limit without a delta, delta of 0
19:30:59 melwitt mriedem: when we check quota before we create a resource, we want to check: count now + count delta against the limit
19:31:10 mriedem yeah
19:31:15 mriedem and after we create,
19:31:16 melwitt mriedem: when we check quota after we created a resource, we want to check: count now against the limit
19:31:25 mriedem we check count now against limit
19:31:28 melwitt right
19:31:28 mriedem after creating the thing
19:31:33 melwitt exactly
19:32:13 melwitt "now that we have created the thing, are we currently over quota" and that will be a "yes" if parallel requests raced and too many resources got created
19:33:23 melwitt (if too many got created and are > limit all together)
19:33:42 mriedem right, i was wondering if we needed to process the 'overs' kwarg out of TooManyInstances and delete more than one instance
19:34:18 melwitt oh, I see. I had thought no because each thread created one instance and would delete one instance
19:34:54 mriedem yeah i'm talking to myself in the review a bit
19:36:03 melwitt trying to think if there's a wrinkle with multi boot. I had thought only delete the one that registered as OverQuota and leave the rest, because it's going to set them all to ERROR anyway I think because of how multi boot works when one fails
19:36:21 mriedem yeah i was also thinking about multi create
19:36:35 mriedem and how, if possible, that can screw us here
19:37:22 melwitt yeah. at the least I should add a test for that scenario (recheck_quota fails during multi boot). I hadn't thought of it at the time
19:37:22 mriedem but we get the min and max from the request before _provision_instances calls check_num_instances_quota

Earlier   Later