| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-11 | |||
| 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 | |
| 19:37:45 | melwitt | oh ... that's right | |
| 19:37:56 | mriedem | i'm not sure if we can get into that failure | |
| 19:38:05 | melwitt | it checks them all at the same time. but I do know if it fails max_count it will check again with min_count | |
| 19:38:57 | melwitt | er sorry, it will calculate how many it can fit and if it's at least min_count, it will go ahead with how many it thinks will fit | |
| 19:39:10 | melwitt | I found that part pretty confusing | |
| 19:39:45 | mriedem | yeah, elif min_count <= allowed <= max_count: | |
| 19:39:55 | mriedem | user_id=user_id) | |
| 19:39:55 | mriedem | allowed, project_id=project_id, | |
| 19:39:55 | mriedem | return check_num_instances_quota(context, instance_type, min_count, | |
| 19:40:02 | mriedem | so allowed becomes the new max_count | |
| 19:40:20 | mriedem | so give me the min i want plus fill up the rest of my quota | |
| 19:41:19 | melwitt | right, if max was more than quota | |
| 19:48:58 | jaypipes | mriedem: answered you. ack on the reason for not adding a new policy rule. | |
| 19:49:29 | mriedem | double ack good buddy 10-4 | |
| 20:18:10 | mriedem | hmm | |
| 20:18:20 | mriedem | melwitt: we should see if tempest has a soft delete / restore test | |
| 20:19:21 | claudiub | mriedem: heya. any idea where the nova PCI passthrough / SR-IOV guide went? | |
| 20:19:32 | mriedem | melwitt: there isn't - there are only negative tests in tempest for restore | |
| 20:19:36 | mriedem | test_restore_nonexistent_server_id | |
| 20:19:40 | mriedem | test_restore_server_invalid_state | |
| 20:19:42 | claudiub | can't seem to find anymore, only some wiki since juno | |
| 20:20:26 | mriedem | claudiub: hmm, the docs are all being migrated from the openstack-manuals repo | |
| 20:20:43 | mriedem | https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:doc-migration | |
| 20:20:54 | mtreinish | mriedem: I like how you know that off the top of your head | |