Wednesday, 2018-05-09

*** gman-tx has joined #openstack-powervm00:08
*** gman-tx has quit IRC00:12
*** gman-tx has joined #openstack-powervm00:53
*** username_ has joined #openstack-powervm01:22
*** username_ is now known as username__01:23
*** gman-tx has quit IRC02:27
*** username__ has quit IRC02:35
*** chas_ has joined #openstack-powervm05:38
*** chas_ has quit IRC05:38
*** mujahidali has joined #openstack-powervm05:51
*** edmondsw has joined #openstack-powervm08:35
*** edmondsw has quit IRC08:40
*** Mujahid_ has joined #openstack-powervm09:19
*** mujahidali has quit IRC09:19
*** Mujahid_ has quit IRC12:03
*** edmondsw has joined #openstack-powervm12:10
*** gman-tx has joined #openstack-powervm12:47
openstackgerritMerged openstack/nova-powervm master: Properly set vios_ids when discovering initiator
*** esberglu has joined #openstack-powervm13:58
esbergluedmondsw: efried:
openstackLaunchpad 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 night14:00
esbergluSo CI is down again, can't delete instances14:01
esberglu is the change that broke us14:02
esbergluSince we can't backport pypowervm req bumps we will need to fix in nova-powervm14:02
efriedesberglu: wtf, they merged the constraint bump for a broken version to queens??14:12
edmondswrevert that?14:12
efriedesberglu: And in queens, we can't blacklist that version in our requirements.txt because it won't match g-r?14:13
efriedWe can resurrect I suppose.  Or you could patch it in.14:14
efriedwhile we fight the good fight.14:14
efriedesberglu: In any case, you should comment on that merged requirements patch letting them know that it includes the busted unicode-ification thing.14:16
esbergluefried: I believe you're correct on blacklisting14:21
esbergluI've patched 563314 into CI for now just to get it back online14:21
*** tjakobs has joined #openstack-powervm14:23
esbergluefried: Wait isn't supposed to fix this issue?14:26
efriedesberglu: Yeah, but it depends what commits are in which release.14:26
esbergluefried: That's in the release14:26
efriedesberglu: Then how did we break?14:27
esbergluefried: I'm not sure. 1.31.3 is installed on the CI neos14:28
esbergluAnd the fix is in the that release14:28
esbergluBefore either of those changes it was using str()14:31
*** mujahidali has joined #openstack-powervm14:47
*** edmondsw has quit IRC15:04
*** gman-tx has quit IRC15:57
*** gman-tx has joined #openstack-powervm15:59
*** gman-tx_ has joined #openstack-powervm16:14
*** gman-tx has quit IRC16:15
*** gman-tx_ is now known as gman-tx16:16
*** gman-tx has quit IRC16:36
*** mujahidali has quit IRC16:54
*** edmondsw has joined #openstack-powervm19:54
edmondswmdrabe can you review ?20:14
edmondswchance to show off those new +2 skillz ;)20:14
mdrabeyep lookin20:16
esbergluedmondsw: I talked to the nova cores, they approved of the CI changes and are dropping the block on snapshot and localdisk20:16
edmondswesbergul \o/20:16
edmondswesberglu so are we next in the runway queue?20:17
esbergluedmondsw: Yes20:17
esbergluedmondsw: Also, Mujahid was able to create volumes in one of the tempest vms, and I confirmed that they are exist on the SAN20:17
efriedesberglu: Looks like you still have a -1 from edmondsw on localdisk20:17
edmondswesberglu ^ I was just going to say20:18
esbergluefried: Yep, planning on hitting that by EOD20:18
edmondswesberglu so how far does that go in getting us to an on-demand CI job?20:19
esbergluedmondsw: Remaining stuff to do20:19
esberglu1) Make sure that we can attach/detach those volumes to instances within the AIO vm20:19
esberglu2) Put together a list of tempest tests for the on-demand job and get them passing20:20
esberglu3) Automate it all20:20
esbergluSo not that far20:20
edmondswbut we're moving. that's something20:21
esbergluBut I thought it would take longer to get where we are right now, so things are looking ok20:21
esbergluedmondsw: Regarding your race condition comment, I'm not sure I follow20:22
esbergluDoesn't setUp always get run prior to the test? Where does the race come in?20:22
efriedyes, setUp gets run once per test case, before the test case method itself begins.20:23
edmondswesberglu if another test that calls find_vg runs before that line, the assert would fail20:24
efriedSo this is saying that we call find_vg as part of LocalStorage init, right?20:24
efriededmondsw: No20:24
efriedthe setUp runs individually for each test case20:24
edmondswoh it does? I thought it was once for the class20:24
efriedThat's a different method20:25
efriedsetUpClass or something, don't remember20:25
esberglusetUpClass does that20:25
efriedit's seldom used.20:25
edmondswok, then it's probably fine then20:25
efriedI 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
esbergluedmondsw: efried: New patch is up for localdisk20:37
edmondswesberglu lgtm thanks!20:40
esbergluefried: Okay finally back to the oslo.versionedobjects issue from earlier20:48
efrieddiscover anything new?20:48
esbergluCurrently hitting both queens and pike, IT and OOT20:48
esbergluNo I'm just getting back into it20:48
efriedGuess 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
esbergluefried: For the CI undercloud we are running 1.31.3, which is broken for us20:50
efried"broken for us" meaning it has the bad unicode crap in it?20:51
esbergluHowever it has which we thought would work20:51
esberglu*1.31.3 has20:52
esbergluefried: As in ValueError: invalid literal for int() with base 10: '4E27E1E6-6A24-4F0A-8E7B-2BBE7B4A28BA'20:52
esbergluefried: Is that not what we want to be returning?20:53
esbergluThe coerce method20:53
openstackgerritMerged openstack/nova-powervm master: Sanitize the config drive UUID
esbergluefried: In pypowervm we're checking if it's an instance of str, isn't20:56
esberglu"%s" % value20:56
esbergluA string?20:57
esbergluMust not be20:57
efriedesberglu: Yes, it oughtta be.20:57
efriedAre we running py2 here?20:57
esbergluefried: ya20:57
efriededmondsw: Still around?20:59
edmondswefried o/20:59
efriedThing is, I think we have to use str, not six.text_type.21:00
efriedbecause we *have* to satisfy the (broken, non-six-compliant) isinstance(str) check in pypowervm.21:00
esbergluedmondsw: Not sure if you were around earlier, but is now hitting the stable branches21:01
openstackLaunchpad 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
edmondswtrying to parse21:01
efriededmondsw: A recap might be helpful for all of us.21:02
edmondswyeah, I'm remembering pieces of this, but not the whole picture21:02
efriedpypowervm 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
efriedWe fixed this via pypowervm 1.1.15 in master, but we can't backport that req change to stable.21:03
efriedThey fixed oslo-versionedobjects, or so we thought, and backported the fix to pike/queens.  BUT we still seem to be broken.21:03
efriedSo since we can't fix pypowervm, we have to fix this in nova-powervm21:03
esbergluefried: I should just be able to port
esberglueverywhere this is hitting21:04
edmondswbut why didn't the fix in oslo-versionedobjects work?21:05
esberglubut I think you're saying you're concerned that it has to be str() there, not six.text_type()?21:05
efriedThat's what we're not sure about.21:05
esberglu - Change that broke us21:05
efriedI'm wondering if we're really not running the version we think21:05
esberglu - Change that we thought would fix21:05
efriednevertheless, we know we can work around it via (nova-powervm stringify UUID)21:05
efriedbecause when esberglu patches that in, we're good, right esberglu?21:06
esbergluefried: Only confirmed on queens OOT so far, but yes21:06
efriedSo right now, other than that being WIP, you were objecting because I used str instead of six.text_type.21:06
edmondswesberglu point us to a failing CI run?21:06
efriedI'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
efriedNot isinstance(six.text_type)21:07
edmondswefried yep, that makes sense21:07
efriedstr() still exists in py3.  Whether it does something "different" is irrelevant, because isinstance(str(...), str) will pass in both py2 and py321:07
efriedbut yeah, I should put a NOTE there explaining that.21:07
edmondswefried but I still want to figure out why the oslo fix didn't work and how to fix that21:07
efriedagree with that.21:08
edmondswefried ++ on adding a comment21:08
efriedso do we leave CI broken in the meantime?  Or patch in the meantime?  Or what?21:08
edmondswpatch in the meantime21:08
efriedI guess I'll respin the patch anyway, and hope we don't wind up needing to commit it?21:09
efriedAnd we'll have to drop it into master and then backport it, but then unwind it in master.21:09
efriedwhich is ick21:09
efriedwaidaminute, esberglu how are you patching it into stable?  Does it basically just cherry-pick the delta in?21:10
esbergluI'll have the CI change up shortly. I'm not going to propose backports for now, I can just cherry-pick it in21:10
esbergluWill need to propose a corresponding patch to cherry-pick for nova though21:10
esbergluI take that back, it doesn't cherry-pick cleanly to pike21:14
efriedrightright, we have to do this for nova too, ick.21:14
edmondswefried I looked at the oslo change... it was not a revert. Apparently there is a difference between `"%s" % value` and `str(value)`...21:15
efriedyeah, I knew it wasn't a revert, but "%s" % value ought to be a friggin str21:15
esbergluedmondsw: Yep that's where we got as well, but we aren't sure why they aren't the same21:16
edmondsw>>> s = '%s' % 'foo'21:16
edmondsw>>> isinstance(s, str)21:16
edmondswnot understanding..21:17
edmondsw>>> s = '%s' % u'foo'21:17
edmondsw>>> isinstance(s, str)21:17
edmondswso to make it work like before, you'd actually have to revert and use str()21:18
edmondswefried ^21:18
efriedand lookie here:21:19
efriedIn [2]: isinstance(str('%s' % u'foo'), str)21:19
efriedOut[2]: True21:19
efriedIn [4]: isinstance(six.text_type('%s' % u'foo'), str)21:19
efriedOut[4]: False21:19
efriedso we absolutely have to use str()21:19
efriedesberglu: ya know, it would actually be better if you proposed the nova change, so I can +2 it.21:19
efriedyou got space for that?21:20
edmondswor I can21:20
efriedthat works too.21:20
edmondswso what do we do about oslo?21:20
efriedThe commit message needs to have allll the pointers.  Breaking and "fixing" oslo-versionedobjects patches, pypowervm fix...21:20
efriedfuck nothing I guess.21:20
efriedI mean we could propose a patch that gets us back to str()21:21
efriedbut I don't know if it'd fly.21:21
efriedAnd then we'd have to get that in a release, and the release posted to g-r in stable etc.21:21
efriedpretty big PITA considering we have a workaround.21:21
edmondswyeah, I'm thinking it probably wouldn't fly21:21
edmondswwe should tell them what's going on, though21:21
edmondswsince this did break compat21:22
edmondswprobably no way around it, but they outta know21:22
esbergluedmondsw: efried: I have all the links handy I can propose the changes21:22
efriededmondsw: agree.  esberglu: ++21:23
efriedshould I continue with the n-p one or do you want to take it over?21:23
edmondswefried ideally, oslo would use six.PY2 to know they should str()21:23
edmondswand that would solve the backward compat issue21:23
esbergluefried: We can abandon the master change right? And I can resubmit to queens21:24
efriedIn n-p we can probably get away with that.  Not in nova.21:24
edmondswefried 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 master21:25
edmondswI mean directly in stable/queens21:25
efriedyes to the former, no the latter won't fly.21:25
efriedIn nova we'll have to propose to master with a TODO(): get rid of this when pypowervm 1.1.15 is minimum.21:25
efriedThen backport21:25
efriedThen propose a change to master to get rid of it.21:25
edmondswoh, we're still not at 1.1.15 min?21:26
efriedWe are.21:26
esbergluThat seems excessive21:26
efriedwe can propose both of the master changes in a series to make it clear what we're doing.21:26
efriedHell, you can ask the process nazis in -nova if you like, but I'm pretty sure dem's da rulez.21:26
esbergluFor n-p we're cool with direct to queens?21:27
efrieddo we have a bug open for this already?21:29
edmondswesberglu 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 us21:29
openstackLaunchpad 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
efriededmondsw: Back to Vlad - we need to backport to queens, yah?21:36
openstackgerritEric Fried proposed openstack/nova-powervm stable/queens: Fix boot volume connectivity type discovery
efriededmondsw: ^21:37
efriedThat will be the easiest way to get him back to stable - if he pulls that branch down.21:37
edmondswefried yep21:38
edmondswefried it ok that the commit message comments about cherry-pick aren't there when you pick that way?21:39
efriedah rats, we get that when the master one is merged when cherry-picked, but this wasn't yet.21:40
efriedI can add it manually.21:40
openstackgerritEric Fried proposed openstack/nova-powervm stable/queens: Fix boot volume connectivity type discovery
efriededmondsw: Done ^21:41
efriededmondsw: And sent the fup email.  Thanks for noticing thot.21:42
edmondswtx for fixing21:42
efried(^ dvorak typo)21:42
edmondswefried clean cherry pick, ok to fast approve?21:43
efriedLet's get the master one in the gate first, but sure.21:44
edmondswyeah, I'll wait on CI21:44
edmondswmaster is in the gate21:44
edmondswesberglu note that efried promised your nova commit message would have all the gory details21:52
openstackgerritMerged openstack/nova-powervm master: Fix boot volume connectivity type discovery
efriededmondsw: based on ==> 4:22:55 PM - esberglu: edmondsw: efried: I have all the links handy I can propose the changes21:53
*** tjakobs has quit IRC21:59
edmondswefried I wonder if we'll need to backport ...22:04
edmondswI've got a question out to Ken on that22:05
edmondswbut I suspect we will22:05
openstackgerritEric Berglund proposed openstack/nova-powervm stable/queens: Stringify instance UUID
esbergluedmondsw: efried: ^22:08
esbergluGonna be afk for a while, I can put the nova fix up later this evening22:09
*** esberglu has quit IRC22:10
*** edmondsw has quit IRC23:10
*** edmondsw has joined #openstack-powervm23:11
*** edmondsw has quit IRC23:15

Generated by 2.15.3 by Marius Gedminas - find it at!