| *** gman-tx has joined #openstack-powervm | 00:08 | |
| *** gman-tx has quit IRC | 00:12 | |
| *** gman-tx has joined #openstack-powervm | 00:53 | |
| *** username_ has joined #openstack-powervm | 01:22 | |
| *** username_ is now known as username__ | 01:23 | |
| *** gman-tx has quit IRC | 02:27 | |
| *** username__ has quit IRC | 02:35 | |
| *** chas_ has joined #openstack-powervm | 05:38 | |
| *** chas_ has quit IRC | 05:38 | |
| *** mujahidali has joined #openstack-powervm | 05:51 | |
| *** edmondsw has joined #openstack-powervm | 08:35 | |
| *** edmondsw has quit IRC | 08:40 | |
| *** Mujahid_ has joined #openstack-powervm | 09:19 | |
| *** mujahidali has quit IRC | 09:19 | |
| *** Mujahid_ has quit IRC | 12:03 | |
| *** edmondsw has joined #openstack-powervm | 12:10 | |
| *** gman-tx has joined #openstack-powervm | 12:47 | |
| openstackgerrit | Merged openstack/nova-powervm master: Properly set vios_ids when discovering initiator https://review.openstack.org/566955 | 13:57 |
|---|---|---|
| *** esberglu has joined #openstack-powervm | 13:58 | |
| esberglu | edmondsw: efried: https://bugs.launchpad.net/nova-powervm/+bug/1766692 | 14:00 |
| openstack | Launchpad bug 1766692 in pypowervm "instance.uuid no longer being a str breaks powervm scsi disconnect" [Critical,Fix released] - Assigned to Eric Fried (efried) | 14:00 |
| esberglu | ^ that's hitting queens as of last night | 14:00 |
| esberglu | So CI is down again, can't delete instances | 14:01 |
| esberglu | https://review.openstack.org/#/c/566890/ is the change that broke us | 14:02 |
| esberglu | Since we can't backport pypowervm req bumps we will need to fix in nova-powervm | 14:02 |
| efried | esberglu: wtf, they merged the constraint bump for a broken version to queens?? | 14:12 |
| edmondsw | revert that? | 14:12 |
| efried | esberglu: And in queens, we can't blacklist that version in our requirements.txt because it won't match g-r? | 14:13 |
| efried | We can resurrect https://review.openstack.org/#/c/563314/1/nova_powervm/virt/powervm/vm.py I suppose. Or you could patch it in. | 14:14 |
| efried | while we fight the good fight. | 14:14 |
| efried | esberglu: In any case, you should comment on that merged requirements patch letting them know that it includes the busted unicode-ification thing. | 14:16 |
| esberglu | efried: I believe you're correct on blacklisting | 14:21 |
| esberglu | I've patched 563314 into CI for now just to get it back online | 14:21 |
| efried | ack | 14:21 |
| *** tjakobs has joined #openstack-powervm | 14:23 | |
| esberglu | efried: Wait isn't https://review.openstack.org/#/c/561674/ supposed to fix this issue? | 14:26 |
| efried | esberglu: Yeah, but it depends what commits are in which release. | 14:26 |
| esberglu | efried: That's in the release | 14:26 |
| efried | esberglu: Then how did we break? | 14:27 |
| esberglu | efried: I'm not sure. 1.31.3 is installed on the CI neos | 14:28 |
| esberglu | https://github.com/openstack/oslo.versionedobjects/tree/1.31.3 | 14:28 |
| esberglu | And the fix is in the that release | 14:28 |
| esberglu | efried: https://review.openstack.org/#/c/559815/1/oslo_versionedobjects/fields.py | 14:31 |
| esberglu | Before either of those changes it was using str() | 14:31 |
| *** mujahidali has joined #openstack-powervm | 14:47 | |
| *** edmondsw has quit IRC | 15:04 | |
| *** gman-tx has quit IRC | 15:57 | |
| *** gman-tx has joined #openstack-powervm | 15:59 | |
| *** gman-tx_ has joined #openstack-powervm | 16:14 | |
| *** gman-tx has quit IRC | 16:15 | |
| *** gman-tx_ is now known as gman-tx | 16:16 | |
| *** gman-tx has quit IRC | 16:36 | |
| *** mujahidali has quit IRC | 16:54 | |
| *** edmondsw has joined #openstack-powervm | 19:54 | |
| edmondsw | mdrabe can you review https://review.openstack.org/#/c/566420/ ? | 20:14 |
| edmondsw | chance to show off those new +2 skillz ;) | 20:14 |
| mdrabe | yep lookin | 20:16 |
| esberglu | edmondsw: I talked to the nova cores, they approved of the CI changes and are dropping the block on snapshot and localdisk | 20:16 |
| edmondsw | esbergul \o/ | 20:16 |
| edmondsw | esberglu so are we next in the runway queue? | 20:17 |
| esberglu | edmondsw: Yes | 20:17 |
| edmondsw | great | 20:17 |
| esberglu | edmondsw: Also, Mujahid was able to create volumes in one of the tempest vms, and I confirmed that they are exist on the SAN | 20:17 |
| efried | esberglu: Looks like you still have a -1 from edmondsw on localdisk | 20:17 |
| edmondsw | esberglu ^ I was just going to say | 20:18 |
| esberglu | efried: Yep, planning on hitting that by EOD | 20:18 |
| efried | dig | 20:18 |
| edmondsw | esberglu so how far does that go in getting us to an on-demand CI job? | 20:19 |
| esberglu | edmondsw: Remaining stuff to do | 20:19 |
| esberglu | 1) Make sure that we can attach/detach those volumes to instances within the AIO vm | 20:19 |
| esberglu | 2) Put together a list of tempest tests for the on-demand job and get them passing | 20:20 |
| esberglu | 3) Automate it all | 20:20 |
| esberglu | So not that far | 20:20 |
| edmondsw | but we're moving. that's something | 20:21 |
| esberglu | But I thought it would take longer to get where we are right now, so things are looking ok | 20:21 |
| edmondsw | ++ | 20:21 |
| esberglu | edmondsw: Regarding your race condition comment, I'm not sure I follow | 20:22 |
| esberglu | https://review.openstack.org/#/c/549300/25/nova/tests/unit/virt/powervm/disk/test_localdisk.py@80 | 20:22 |
| esberglu | Doesn't setUp always get run prior to the test? Where does the race come in? | 20:22 |
| efried | yes, setUp gets run once per test case, before the test case method itself begins. | 20:23 |
| edmondsw | esberglu if another test that calls find_vg runs before that line, the assert would fail | 20:24 |
| efried | So this is saying that we call find_vg as part of LocalStorage init, right? | 20:24 |
| efried | edmondsw: No | 20:24 |
| efried | the setUp runs individually for each test case | 20:24 |
| edmondsw | oh it does? I thought it was once for the class | 20:24 |
| efried | That's a different method | 20:25 |
| efried | setUpClass or something, don't remember | 20:25 |
| esberglu | setUpClass does that | 20:25 |
| efried | it's seldom used. | 20:25 |
| edmondsw | ok, then it's probably fine then | 20:25 |
| efried | I am not positive how the internals work, but I think the test runner creates a separate instance of the class for each test case, runs setUp, and then runs that individual test case. | 20:26 |
| esberglu | edmondsw: efried: New patch is up for localdisk | 20:37 |
| efried | ack | 20:37 |
| edmondsw | ack | 20:37 |
| edmondsw | esberglu lgtm thanks! | 20:40 |
| esberglu | efried: Okay finally back to the oslo.versionedobjects issue from earlier | 20:48 |
| efried | discover anything new? | 20:48 |
| esberglu | Currently hitting both queens and pike, IT and OOT | 20:48 |
| esberglu | No I'm just getting back into it | 20:48 |
| efried | Guess first thing is to validate a) what version of that package we're actually running against in those envs, and b) whether that version is the broken one. | 20:49 |
| esberglu | efried: For the CI undercloud we are running 1.31.3, which is broken for us | 20:50 |
| efried | "broken for us" meaning it has the bad unicode crap in it? | 20:51 |
| esberglu | However it has https://review.openstack.org/#/c/561674/ which we thought would work | 20:51 |
| esberglu | *1.31.3 has | 20:52 |
| esberglu | efried: As in ValueError: invalid literal for int() with base 10: '4E27E1E6-6A24-4F0A-8E7B-2BBE7B4A28BA' | 20:52 |
| efried | hum | 20:52 |
| esberglu | efried: Is that not what we want https://review.openstack.org/#/c/561674/2/oslo_versionedobjects/fields.py to be returning? | 20:53 |
| esberglu | The coerce method | 20:53 |
| openstackgerrit | Merged openstack/nova-powervm master: Sanitize the config drive UUID https://review.openstack.org/428433 | 20:55 |
| esberglu | efried: In pypowervm we're checking if it's an instance of str, isn't | 20:56 |
| esberglu | "%s" % value | 20:56 |
| esberglu | A string? | 20:57 |
| esberglu | Must not be | 20:57 |
| efried | esberglu: Yes, it oughtta be. | 20:57 |
| efried | Are we running py2 here? | 20:57 |
| esberglu | efried: ya | 20:57 |
| efried | edmondsw: Still around? | 20:59 |
| edmondsw | efried o/ | 20:59 |
| efried | Thing is, I think we have to use str, not six.text_type. | 21:00 |
| efried | because we *have* to satisfy the (broken, non-six-compliant) isinstance(str) check in pypowervm. | 21:00 |
| esberglu | edmondsw: Not sure if you were around earlier, but https://bugs.launchpad.net/nova-powervm/+bug/1766692 is now hitting the stable branches | 21:01 |
| openstack | Launchpad bug 1766692 in pypowervm "instance.uuid no longer being a str breaks powervm scsi disconnect" [Critical,Fix released] - Assigned to Eric Fried (efried) | 21:01 |
| edmondsw | trying to parse | 21:01 |
| efried | edmondsw: A recap might be helpful for all of us. | 21:02 |
| edmondsw | yeah, I'm remembering pieces of this, but not the whole picture | 21:02 |
| efried | pypowervm was using isinstance(str) to pre-check whether the thing we were looking at might be a UUID. After the oslo-versionedobjects breakage, a UUID was no longer satisfying that check, so we fell through to the int() branch, which broke on the (now unicode, I think) uuid. | 21:03 |
| efried | We fixed this via pypowervm 1.1.15 in master, but we can't backport that req change to stable. | 21:03 |
| efried | They fixed oslo-versionedobjects, or so we thought, and backported the fix to pike/queens. BUT we still seem to be broken. | 21:03 |
| efried | So since we can't fix pypowervm, we have to fix this in nova-powervm | 21:03 |
| esberglu | efried: I should just be able to port https://review.openstack.org/#/c/563314/ | 21:04 |
| esberglu | everywhere this is hitting | 21:04 |
| efried | via https://review.openstack.org/#/c/563314/ | 21:04 |
| edmondsw | but why didn't the fix in oslo-versionedobjects work? | 21:05 |
| esberglu | but I think you're saying you're concerned that it has to be str() there, not six.text_type()? | 21:05 |
| efried | That's what we're not sure about. | 21:05 |
| esberglu | https://review.openstack.org/#/c/559815/ - Change that broke us | 21:05 |
| efried | I'm wondering if we're really not running the version we think | 21:05 |
| esberglu | https://review.openstack.org/#/c/561674/ - Change that we thought would fix | 21:05 |
| efried | nevertheless, we know we can work around it via https://review.openstack.org/#/c/563314/ (nova-powervm stringify UUID) | 21:05 |
| efried | because when esberglu patches that in, we're good, right esberglu? | 21:06 |
| esberglu | efried: Only confirmed on queens OOT so far, but yes | 21:06 |
| efried | So right now, other than that being WIP, you were objecting because I used str instead of six.text_type. | 21:06 |
| edmondsw | esberglu point us to a failing CI run? | 21:06 |
| efried | I'm saying str is actually the appropriate "fix" here, because we're explicitly trying to get around a known-broken isinstance(str) in pypowervm. | 21:06 |
| efried | Not isinstance(six.text_type) | 21:07 |
| esberglu | http://184.172.12.213/89/544689/3/check/nova-powervm-out-of-tree-pvm/29d7916/ | 21:07 |
| edmondsw | efried yep, that makes sense | 21:07 |
| efried | str() still exists in py3. Whether it does something "different" is irrelevant, because isinstance(str(...), str) will pass in both py2 and py3 | 21:07 |
| efried | but yeah, I should put a NOTE there explaining that. | 21:07 |
| edmondsw | efried but I still want to figure out why the oslo fix didn't work and how to fix that | 21:07 |
| efried | agree with that. | 21:08 |
| edmondsw | efried ++ on adding a comment | 21:08 |
| efried | so do we leave CI broken in the meantime? Or patch in the meantime? Or what? | 21:08 |
| edmondsw | patch in the meantime | 21:08 |
| efried | I guess I'll respin the patch anyway, and hope we don't wind up needing to commit it? | 21:09 |
| efried | And we'll have to drop it into master and then backport it, but then unwind it in master. | 21:09 |
| efried | which is ick | 21:09 |
| efried | waidaminute, esberglu how are you patching it into stable? Does it basically just cherry-pick the delta in? | 21:10 |
| esberglu | I'll have the CI change up shortly. I'm not going to propose backports for now, I can just cherry-pick it in | 21:10 |
| esberglu | Will need to propose a corresponding patch to cherry-pick for nova though | 21:10 |
| esberglu | I take that back, it doesn't cherry-pick cleanly to pike | 21:14 |
| efried | rightright, we have to do this for nova too, ick. | 21:14 |
| edmondsw | efried I looked at the oslo change... it was not a revert. Apparently there is a difference between `"%s" % value` and `str(value)`... | 21:15 |
| efried | yeah, I knew it wasn't a revert, but "%s" % value ought to be a friggin str | 21:15 |
| esberglu | edmondsw: Yep that's where we got as well, but we aren't sure why they aren't the same | 21:16 |
| edmondsw | >>> s = '%s' % 'foo' | 21:16 |
| edmondsw | >>> isinstance(s, str) | 21:16 |
| edmondsw | True | 21:16 |
| edmondsw | :( | 21:16 |
| edmondsw | not understanding.. | 21:17 |
| edmondsw | ahh... | 21:17 |
| edmondsw | >>> s = '%s' % u'foo' | 21:17 |
| edmondsw | >>> isinstance(s, str) | 21:17 |
| edmondsw | False | 21:17 |
| edmondsw | so to make it work like before, you'd actually have to revert and use str() | 21:18 |
| edmondsw | efried ^ | 21:18 |
| efried | and lookie here: | 21:19 |
| efried | In [2]: isinstance(str('%s' % u'foo'), str) | 21:19 |
| efried | Out[2]: True | 21:19 |
| efried | In [4]: isinstance(six.text_type('%s' % u'foo'), str) | 21:19 |
| efried | Out[4]: False | 21:19 |
| efried | so we absolutely have to use str() | 21:19 |
| efried | esberglu: ya know, it would actually be better if you proposed the nova change, so I can +2 it. | 21:19 |
| efried | you got space for that? | 21:20 |
| edmondsw | or I can | 21:20 |
| efried | that works too. | 21:20 |
| edmondsw | so what do we do about oslo? | 21:20 |
| efried | The commit message needs to have allll the pointers. Breaking and "fixing" oslo-versionedobjects patches, pypowervm fix... | 21:20 |
| efried | fuck nothing I guess. | 21:20 |
| efried | I mean we could propose a patch that gets us back to str() | 21:21 |
| efried | but I don't know if it'd fly. | 21:21 |
| efried | And then we'd have to get that in a release, and the release posted to g-r in stable etc. | 21:21 |
| efried | pretty big PITA considering we have a workaround. | 21:21 |
| edmondsw | yeah, I'm thinking it probably wouldn't fly | 21:21 |
| edmondsw | we should tell them what's going on, though | 21:21 |
| edmondsw | since this did break compat | 21:22 |
| edmondsw | probably no way around it, but they outta know | 21:22 |
| esberglu | edmondsw: efried: I have all the links handy I can propose the changes | 21:22 |
| efried | edmondsw: agree. esberglu: ++ | 21:23 |
| efried | should I continue with the n-p one or do you want to take it over? | 21:23 |
| edmondsw | efried ideally, oslo would use six.PY2 to know they should str() | 21:23 |
| edmondsw | and that would solve the backward compat issue | 21:23 |
| esberglu | efried: We can abandon the master change right? And I can resubmit to queens | 21:24 |
| efried | In n-p we can probably get away with that. Not in nova. | 21:24 |
| edmondsw | efried is nova gonna make us merge in master just to cherry-pick and then revert it? I'd argue they should let us just merge directly in master | 21:25 |
| edmondsw | I mean directly in stable/queens | 21:25 |
| efried | yes to the former, no the latter won't fly. | 21:25 |
| efried | In nova we'll have to propose to master with a TODO(): get rid of this when pypowervm 1.1.15 is minimum. | 21:25 |
| efried | Then backport | 21:25 |
| efried | Then propose a change to master to get rid of it. | 21:25 |
| edmondsw | oh, we're still not at 1.1.15 min? | 21:26 |
| efried | We are. | 21:26 |
| esberglu | That seems excessive | 21:26 |
| efried | we can propose both of the master changes in a series to make it clear what we're doing. | 21:26 |
| efried | Hell, you can ask the process nazis in -nova if you like, but I'm pretty sure dem's da rulez. | 21:26 |
| edmondsw | ick | 21:26 |
| esberglu | For n-p we're cool with direct to queens? | 21:27 |
| edmondsw | yes | 21:27 |
| efried | yes | 21:27 |
| efried | do we have a bug open for this already? | 21:29 |
| edmondsw | esberglu I would go ahead and propose directly to queens for nova as well. If they make us do otherwise, so be it, but make them make us | 21:29 |
| edmondsw | efried https://bugs.launchpad.net/nova-powervm/+bug/1766692 | 21:29 |
| openstack | Launchpad bug 1766692 in pypowervm "instance.uuid no longer being a str breaks powervm scsi disconnect" [Critical,Fix released] - Assigned to Eric Fried (efried) | 21:29 |
| efried | good | 21:29 |
| efried | edmondsw: Back to Vlad - we need to backport https://review.openstack.org/#/c/566420/ to queens, yah? | 21:36 |
| openstackgerrit | Eric Fried proposed openstack/nova-powervm stable/queens: Fix boot volume connectivity type discovery https://review.openstack.org/567418 | 21:37 |
| efried | edmondsw: ^ | 21:37 |
| efried | That will be the easiest way to get him back to stable - if he pulls that branch down. | 21:37 |
| edmondsw | efried yep | 21:38 |
| edmondsw | efried it ok that the commit message comments about cherry-pick aren't there when you pick that way? | 21:39 |
| efried | ah rats, we get that when the master one is merged when cherry-picked, but this wasn't yet. | 21:40 |
| efried | I can add it manually. | 21:40 |
| openstackgerrit | Eric Fried proposed openstack/nova-powervm stable/queens: Fix boot volume connectivity type discovery https://review.openstack.org/567418 | 21:41 |
| efried | edmondsw: Done ^ | 21:41 |
| efried | edmondsw: And sent the fup email. Thanks for noticing thot. | 21:42 |
| efried | that. | 21:42 |
| edmondsw | tx for fixing | 21:42 |
| efried | (^ dvorak typo) | 21:42 |
| edmondsw | efried clean cherry pick, ok to fast approve? | 21:43 |
| efried | Let's get the master one in the gate first, but sure. | 21:44 |
| edmondsw | yeah, I'll wait on CI | 21:44 |
| edmondsw | master is in the gate | 21:44 |
| efried | dig | 21:45 |
| edmondsw | esberglu note that efried promised your nova commit message would have all the gory details | 21:52 |
| openstackgerrit | Merged openstack/nova-powervm master: Fix boot volume connectivity type discovery https://review.openstack.org/566420 | 21:53 |
| efried | edmondsw: based on ==> 4:22:55 PM - esberglu: edmondsw: efried: I have all the links handy I can propose the changes | 21:53 |
| *** tjakobs has quit IRC | 21:59 | |
| edmondsw | efried I wonder if we'll need to backport https://review.openstack.org/#/c/428433/ ... | 22:04 |
| edmondsw | I've got a question out to Ken on that | 22:05 |
| edmondsw | but I suspect we will | 22:05 |
| efried | nod | 22:05 |
| openstackgerrit | Eric Berglund proposed openstack/nova-powervm stable/queens: Stringify instance UUID https://review.openstack.org/567434 | 22:06 |
| esberglu | edmondsw: efried: ^ | 22:08 |
| esberglu | Gonna be afk for a while, I can put the nova fix up later this evening | 22:09 |
| *** esberglu has quit IRC | 22:10 | |
| *** edmondsw has quit IRC | 23:10 | |
| *** edmondsw has joined #openstack-powervm | 23:11 | |
| *** edmondsw has quit IRC | 23:15 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!