Tuesday, 2017-03-28

openstackgerritMonty Taylor proposed openstack-infra/nodepool feature/zuulv3: Fetch list of AZs from nova if it's not configured  https://review.openstack.org/45034500:14
mordredShrews: I rebased on your docs patch. Also, I think I got your review comment, but I may not have00:15
Shrewsmordred: you did not. need me to correct the placement?00:28
Shrewsmordred: basically, the first 'if' in that 'for' is for pre-existing nodes, and the 2nd 'if' is for new nodes. we need to choose the az outside the for so it's the same for both types00:30
Shrewsactually, that's confusing since there are many 'fors' and 'ifs'. i'll put up a patch set tomorrow for you.00:31
*** jamielennox is now known as jamielennox|away00:54
*** jamielennox|away is now known as jamielennox00:59
*** bstinson has quit IRC01:30
*** bstinson has joined #zuul01:32
*** jamielennox is now known as jamielennox|away04:09
*** jamielennox|away is now known as jamielennox04:30
*** MarkMielke has quit IRC04:35
*** bhavik1 has joined #zuul04:49
*** isaacb has joined #zuul06:26
*** jamielennox is now known as jamielennox|away06:36
openstackgerritTobias Henkel proposed openstack-infra/zuul feature/zuulv3: Change mutex to counting semaphore  https://review.openstack.org/44256307:12
*** jamielennox|away is now known as jamielennox07:21
tobiash_jeblair: ^^ reworked regarding your comments but it looks like I have to rebase this again (merge failed) once your secrets work is merged or rebased07:31
*** hashar has joined #zuul07:35
*** bhavik1 has quit IRC08:00
*** rcarrillocruz has joined #zuul08:01
openstackgerritTobias Henkel proposed openstack-infra/zuul feature/zuulv3: Use unicode for change number extraction  https://review.openstack.org/45070410:24
*** openstackgerrit has quit IRC11:33
*** hashar is now known as hasharLunch11:47
*** hasharLunch is now known as hashar12:32
mordredShrews: ok. I see where you're talking about12:36
Shrewsmordred: did you see the import failure for that?12:42
mordrednope - but will look at that next12:42
Shrewsmaybe we need to add concurrent lib to shade's requirements.txt12:43
Shrewshrm, no. that shouldn't be necessary12:44
mordredShrews: https://review.openstack.org/45076512:49
mordredShrews: yes - "futures" is a backport from 3.2 to 2.7 ...12:49
mordredShrews: and we had it in our test-requirements - so we didn't notice it was missing from our requirements12:49
Shrewsah12:49
mordredShrews: and I'm guessing we were saved in nodepool accidentally due to a transitive depend from one of the things we recently removed - like zmq or something12:50
mordredanyway - we sohuld land that requirements patch to shade and release asap - shade is just broken now12:50
mordredactually - let me add a release note real quick12:50
Shrewsi +3'd. but can +3 again12:51
mordredShrews: updated12:52
mordredShrews: btw - it was heatclient12:54
Shrewslol12:54
mordredShrews: heatclient pulled in futures as a transitive - so it really was just the 1.18.0 release that was problematic12:54
mordred*phew* I was worried that we had more breakage out there than that12:55
*** openstackgerrit has joined #zuul12:56
openstackgerritMonty Taylor proposed openstack-infra/nodepool feature/zuulv3: Fetch list of AZs from nova if it's not configured  https://review.openstack.org/45034512:56
mordredShrews: that once again won't work, as I bumped the requirement to 1.18.112:56
mordredbut hopefully the logic is correct this time12:56
* Shrews looks12:57
pabelangerdanke!12:57
mordredShrews: it's worth noting that it may result in not reusing nodes in the case where there are more than one az and there are nodes available in one az but the random choice picks one from another az12:58
pabelangerjust hit that issue12:58
Shrewsmordred: close. we need to have an 'if not self.chosen_az' surrounding that since that code can be executed more than once12:58
mordredShrews: ah - k, cool12:58
mordredShrews: I don't think we necessarily need to worry too much about skipping node reuse in a multi-az world just yet - but it's worth noting12:58
Shrewsmordred: well... wait a sec13:00
Shrewsmordred: oh, geez. i TOTALLY told you the wrong thing13:01
mordredawesome13:01
Shrewsbecause i suck13:01
mordredyah - I was just about to say I think we want the code back where it was - because if it is there we'll only reset it if it hasn't been set and if we haven't grabbed a node from existing nodes13:02
mordredalthough I do think a trap to make sure that node.az is in self.pool.azs if there is one would be a good idea - just in case someone changes the config and there are existing nodes floating out there13:03
Shrewsmordred: your PS2 was correct13:03
openstackgerritTobias Henkel proposed openstack-infra/nodepool feature/zuulv3: Remove unused timing constants  https://review.openstack.org/45077013:03
Shrewsmordred: and that trap sounds like a good idea too13:04
mordredk. patch coming13:05
Shrewsthough then those nodes would never be used13:05
mordredyah ... I think that's overthikning it :)13:06
openstackgerritMonty Taylor proposed openstack-infra/nodepool feature/zuulv3: Fetch list of AZs from nova if it's not configured  https://review.openstack.org/45034513:06
mordredShrews: I believe at this point we REALLY understand that patch13:07
Shrewsyes13:07
Shrewssorry i confused you earlier13:07
mordredShrews: no - it was good - I grok the code in questoin better as a result :)13:09
Shrewsgood. explain it to me now  :)13:10
Shrewsmordred: btw, it seems our ansible friends are getting closer to gerrit style functionality via their bot: https://github.com/ansible/ansibullbot/pull/44513:18
ShrewsX number of 'shipit' comments merge the PR13:19
mordredShrews: heh13:24
mordredShrews: well, we should maybe explain to them why we have both a +2 and a +A vote13:24
mordredbut also neat13:24
openstackgerritTobias Henkel proposed openstack-infra/nodepool feature/zuulv3: Log return code on failed dib build  https://review.openstack.org/45078213:30
pabelangermordred: I'm going to forward port 358730 to feature/zuulv3 for nodepool. It is our server console patch13:30
pabelangerto avoid the large changes in nodepool yesterday13:31
*** isaacb has quit IRC13:32
mordredpabelanger: oh - wait a sec13:33
mordredpabelanger: that patch needs to be updated now that we have a new shade out which does not need the task_manager changes13:34
pabelangerack13:34
mordredpabelanger: which is to say - yes, plesae do - but skip the part where patches task_manager13:35
mordredpabelanger: and probably rebase it on top of https://review.openstack.org/450345 so that you pick up the requirements bump to 1.18.1 (which we'll release in just a few minutes)13:35
pabelangerwill do13:36
openstackgerritDirk Mueller proposed openstack-infra/nodepool master: [WIP]: Add opensuse 42.2 DIB testing  https://review.openstack.org/45004514:01
tobiash_mordred: regarding https://review.openstack.org/450345, is the double '_' intended? In provider manager self.__azs = None14:03
mordredtobiash_: you know - I should fix that :14:06
mordred:)14:06
tobiash_didn't know if there's some python magic behind that ;)14:07
jeblairthere is some python magic -- "__foo" becomes _ClassName__foo, so you can use it to automatically avoid collisions with subclasses.14:26
Shrewsthat's a good catch. easily missed14:33
mordredremote:   https://review.openstack.org/450345 Fetch list of AZs from nova if it's not configured14:55
mordredupdated14:55
mordredyou want to re +2 https://review.openstack.org/#/c/449140 ?14:56
Shrewsmordred: maybe we need to have the providermanager reconfigure() method reset self.__az ?14:59
Shrewsto your earlier point on the AZ list changing in the config14:59
mordredShrews: ++15:06
Shrewspabelanger: care to +2 https://review.openstack.org/449140 again? had to fix merge conflicts15:31
pabelangerShrews: +315:33
Shrewsthx15:33
openstackgerritMerged openstack-infra/nodepool feature/zuulv3: Docs: Remove cron references  https://review.openstack.org/44914015:41
openstackgerritMerged openstack-infra/nodepool feature/zuulv3: Docs: Correct availability-zones documentation.  https://review.openstack.org/44914715:42
openstackgerritMerged openstack-infra/nodepool feature/zuulv3: Docs: Remove refs to removed nodepool commands  https://review.openstack.org/44915215:43
mordredShrews: wait - so on the reconfigure thing ...15:52
Shrewsmostly thinking outloud on that15:53
mordredself.__az is the cached list of az's from the cloud - I'm not sure we need to reconfigure cached clouds things on reconfigure - unless we want to blank the flavor cache too15:53
Shrewsmordred: i was thinking in the case the AZs were listed explicitly15:54
mordredyah - that I think is going to get covered already, no?15:54
Shrewsself.__az won't change in that case, will it?15:54
ShrewsgetAZs() would return the outdated list15:56
mordredgetAZs isn't tied to the configured list15:57
mordredgetAZs only returns the azs detected from the cloud15:57
mordredself.pool.azs is the configured list15:57
Shrewsgah15:57
Shrewsignore me15:58
* mordred hands Shrews a pie15:58
Shrewsmy brain is just totally off today15:58
* Shrews goes for lunch to attempt to reset his brain15:59
pabelangerso, question / bikeshad, if we are calling it nodepool-launcher, and our service is nodepool-launcher, would the binary we call also be called nodepool-launcher?16:08
SpamapSI think we should call the binary Roger16:11
SpamapSit's a lovely name16:11
jeblairpabelanger: yes, i think we should call it nodepool-launcher (or Gerald)16:12
pabelanger:_16:13
pabelanger:)16:13
mordredI vote henry16:14
SpamapSRoger Henry Gerald Nodepool Launcher III, Lord of High SSHia.16:16
pabelangerI lol'd16:17
jeblairi really want to manipulate the proc table so it shows up as that, but fungi would kill me16:18
clarkbif you really want to get fun like that you coudl do what elasticserach and other clusters do16:18
fungialso not very portable across platforms16:18
clarkbthey randomly pull from a predefined list of themed names for naming their cluster nodes within the cluster16:18
clarkbso each laucnher could register with zk as gerald and ford and theodore and george16:19
jeblairclarkb: oh nice16:19
fungi(zeddmore, venkman, stantz, spengler...)16:21
clarkbjeblair: I've configured our ES servers to use their hostnamess as their names but by default its comic book characters?16:23
clarkbsomething like that16:23
*** hashar has quit IRC16:25
openstackgerritPaul Belanger proposed openstack-infra/nodepool feature/zuulv3: Rename nodepoold to nodepool-launcher  https://review.openstack.org/45087716:29
*** rcarrillocruz has quit IRC16:33
openstackgerritK Jonathan Harker proposed openstack-infra/zuul feature/zuulv3: Catch and log url pattern formatting errors  https://review.openstack.org/45046816:35
openstackgerritPaul Belanger proposed openstack-infra/nodepool feature/zuulv3: Rename nodepoold to nodepool-launcher  https://review.openstack.org/45087716:38
pabelangerI'll update puppet-nodepool shortly16:46
SpamapShm16:48
SpamapSso I'm messing with test_timer_sshkey16:48
SpamapSit seems nothing writes .ssh_wrapper_$connection anymore16:48
jeblairSpamapS: hrm, i thought the merger still did that?16:49
clarkbjeblair: didn't we remove it because it was causing problems?16:49
clarkbI think that happened at ptg16:49
SpamapSjeblair: I see it being read.16:49
SpamapSI don't see it being written.16:49
pabelangerclarkb: I thought so too16:50
jeblairhttp://git.openstack.org/cgit/openstack-infra/zuul/tree/zuul/merger/merger.py?h=feature/zuulv3#n22116:50
SpamapShttps://github.com/openstack-infra/zuul/blob/feature/zuulv3/zuul/merger/merger.py#L257-L26016:51
SpamapSthat's never called16:51
SpamapSoh no that's the method above16:51
jeblairSpamapS: what seems more likely is this:16:51
jeblairSpamapS: that because the merger isn't running pre-run merge checks on all changes yet, that doesn't occur during the test16:52
SpamapSso the wrapper file isn't written anymore, but the ssh command is still calculated?16:52
jeblairSpamapS: (however, the executor's merger will do that, but the test isn't looking for that)16:52
jeblairif that's correct, then the two solutions are: 1) depend on the pre-merge check being added again, or 2) switch to checking for the executor merger's ssh wrapper16:53
SpamapSSo should we enable this test as part of implementing pre-run merge checks?16:53
jeblairSpamapS: i'd stack it on top as a separate change16:55
jeblairnow is probably a good time for me to look at the test failures in 44627516:56
SpamapSwhere did that test even come from? it's not in master at all16:56
* SpamapS asks git16:56
SpamapShttp://git.openstack.org/cgit/openstack-infra/zuul/commit/zuul/merger/merger.py?h=feature/zuulv3&id=da90a50b794f18f74de0e2c7ec3210abf79dda2416:57
jeblairSpamapS: https://review.openstack.org/430872 was to master16:59
SpamapSyeah, the test isn't in master anymore16:59
jlkSpamapS: I rebased my patch set on top of the Changish refactor and NullChange removal. There wasn't much interaction with it, a single patch. I haven't pushed it up yet tho because I'm coordinating with jesusaur on some other patches to rebase on top.16:59
SpamapSjlk: Well that's better than "it blew the whole thing up". Anyway, they're in feature/zuulv3 now so your rebase is a safe one. :)17:00
jlkyeah that's what I meant. I rebased on tip of feature/zuulv3 which had your patches in it17:01
jlkmine were in merge conflict because of that.17:01
jeblairSpamapS: http://git.openstack.org/cgit/openstack-infra/zuul/tree/tests/test_scheduler.py#n308017:01
SpamapSzomg17:02
SpamapSignore me17:02
SpamapSwow17:02
SpamapSmy master clone was a local clone17:02
jlkgeez clint. :D17:03
SpamapSjeblair: I don't see an obvious story/task for adding pre-run merge checks to v3. Is it buried in something else?17:03
SpamapSbecause would like to just move this one to blocked and mention what it's blocked on in the task comments.17:04
jeblairSpamapS:  https://review.openstack.org/446275 says 346817:05
SpamapSoh already in flight, cool17:05
jesusaurSpamapS: it's part of re-enabling test_build_configuration_conflict17:05
SpamapSI can just stack on that.. :)17:05
SpamapSstill nothing writing that file17:10
jeblairSpamapS: i'm not sure that change is sufficiently functional to do that yet17:10
SpamapSYeah now I see it's still a little explodey in its own tests. K17:11
openstackgerritClint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Re-enable test_timer_sshkey  https://review.openstack.org/45089717:15
SpamapSkk, there's the test stacked on top. Does not work, but I'll just move the task to blocked and rebase once the parent starts passing17:16
jesusaurSpamapS: I've been having trouble tracking down the failures in test_v3.py in the parent change, if you have some spare cycles I would appreciate some help on that17:17
SpamapSjesusaur: sure! I'll switch gears into that now. It needs a rebase on feature/zuulv3, can we start from there?17:18
jesusauroh, yeah, I'll rebase it now17:18
SpamapSCool, need to walk my dog then I'll jump in to the failures17:19
openstackgerritK Jonathan Harker proposed openstack-infra/zuul feature/zuulv3: Perform pre-launch merge checks  https://review.openstack.org/44627517:20
jeblairjesusaur, SpamapS: left comments on PS9 of 44627517:24
jeblairjesusaur, SpamapS: and also PS817:25
jeblairbecause i was apparently moving around during a rebase :)17:25
openstackgerritPaul Belanger proposed openstack-infra/nodepool feature/zuulv3: Rename nodepoold to nodepool-launcher  https://review.openstack.org/45087717:36
jeblairjesusaur: let me know if those make sense or not17:47
jesusaurjeblair: thanks, for the reviews, but I'm a little confused with the comment about wanting to schedule the merge in _processOneItem17:48
jesusaurjeblair: don't we want the merge to have already been performed at that point so that didMergerFail would be accurate?17:48
jeblairjesusaur: processOneItem runs frequently and iteratively -- it looks at every item and makes sure that it makes progress if needed.  so it used to call prepareRef (which is the thing that triggered the merger), and then, the next time it got called for that item, if the merger was done, it would call launchJobs17:51
jesusauroh, I see, so we need to add a function similar to prepareRef that will return a boolean based on whether or not the merge has finished17:56
jesusaurright now that's kind of implicitly coming from prepareLayout17:57
jeblairjesusaur: right; likely some combo of the old prepareRef (which has the state machine in it) and the new scheduleMerge (which has the new stuff about files).  they already have some overlap.17:58
jeblairjesusaur: and yes -- most of the state machine stuff is in prepareLayout now.  but should probably be moved into the prepareRef/scheduleMerge function.17:59
jesusaurok17:59
openstackgerritPaul Belanger proposed openstack-infra/nodepool feature/zuulv3: Rename nodepoold to nodepool-launcher  https://review.openstack.org/45087718:26
openstackgerritDavid Shrewsbury proposed openstack-infra/zuul feature/zuulv3: Fix dev doc link in README.rst  https://review.openstack.org/45092218:29
openstackgerritK Jonathan Harker proposed openstack-infra/zuul feature/zuulv3: Perform pre-launch merge checks  https://review.openstack.org/44627518:49
jesusaurSpamapS: I've pushed a new patchset that works a bit better :)18:51
jesusaurexcept for pep8 :(18:52
SpamapSjesusaur: cool, was just looking at jeblair's comments18:54
openstackgerritK Jonathan Harker proposed openstack-infra/zuul feature/zuulv3: Perform pre-launch merge checks  https://review.openstack.org/44627518:55
openstackgerritMerged openstack-infra/zuul feature/zuulv3: Fix dev doc link in README.rst  https://review.openstack.org/45092218:56
openstackgerritPaul Belanger proposed openstack-infra/nodepool feature/zuulv3: Rename nodepoold to nodepool-launcher  https://review.openstack.org/45087718:58
jesusaurhrm, still getting a bunch of failures19:09
*** hashar has joined #zuul19:23
*** hashar_ has joined #zuul19:24
*** hashar_ has quit IRC19:24
*** hashar has quit IRC19:24
*** hashar has joined #zuul19:24
pabelangermordred: jeblair: Shrews: here is something new, we are defaulting for ipv6 for dsvm jobs now: http://logs.openstack.org/70/450770/1/check/gate-dsvm-nodepool/03f6229/logs/screen-nodepool.txt.gz#_2017-03-28_16_17_35_89919:47
pabelangerand for some reason, we are not able to boot it properly19:48
pabelangeralso: http://logs.openstack.org/52/449152/3/check/gate-dsvm-nodepool/598f3e3/logs/screen-nodepool.txt.gz#_2017-03-27_21_24_13_73419:54
Shrewspabelanger: for the second thing, https://review.openstack.org/44973719:56
Shrewspabelanger: for the first thing, https://review.openstack.org/#/c/449705/20:04
pabelangerShrews: thanks. I think the fix is to force ipv4 in clouds.yaml20:05
Shrewspabelanger: yeah. also curious that the gate-dsvm-nodepool-src-nv jobs seem to be cut short now20:06
Shrewsthey're finishing in under 20m20:06
pabelangerShrews: I have fixed that20:07
pabelangerwas a bug in JJB20:07
Shrewsoh good20:07
Shrewspabelanger: do we need to update nl01 nodepool.yaml for force ipv4?20:07
pabelangerShrews: maybe? infracloud is ipv4 only20:08
pabelangerbut rax we will20:08
Shrewsso yeah then20:08
Shrewsoh, i see. v6 not available in infracloud. i guess we're good for now then20:09
pabelangerya20:10
pabelangerRAX does support ipv6, however we still need to patch glean to support centos /fedora20:11
clarkbShrews: pabelanger you can test the ipv6 stuff using devstack since its enabled there by default20:16
clarkb(and why we tripped over the ipv6_dhcp thing with glean recently)20:17
pabelangerk20:18
pabelangerI'm not sure why we fail to SSH right now20:18
pabelangertrying to figure it out20:18
clarkbpabelanger: oh! is it because neutron + nova say use ipv6 but then glean doesn't configure ipv6?20:47
pabelangeryes, I think that is the isuse20:48
pabelangerit works in osic because dhcp20:48
pabelangerbut I don't think we have tested static20:48
pabelangerclarkb: Wait, is it type=ipv6?20:49
pabelangeror something else20:49
clarkbpabelanger: the type is ipv6_dhcp (whcih is wrong) so my fix to glean was to skip it20:50
pabelangerya20:50
clarkbpabelanger: but if shade interprets what nova/neutron are saying to mean "use ipv6" then we'd break20:50
pabelangerwe won't setup ipv6 then20:50
clarkband I think we recently merged a change to nodepool to stop setting that directly and rely on shade/occ20:50
pabelangerbecause we expect ipv6 only20:50
pabelangeryup20:50
clarkb(guessing no one checked the integration job on that)20:50
clarkbif its not too terrible I would revert for now20:51
pabelangerha20:51
clarkbother option is to tell occ/shade in devstack to prefer ipv420:51
pabelangeryes20:51
pabelangerhttps://review.openstack.org/#/c/449705/20:51
clarkbbut revert bceause its broken20:51
pabelangerlet me get clouds.yaml change in place20:51
clarkbthere is currently no way for nova + neutron + glean + nodepool to operate properly with ipv6 without modifying your configs20:52
pabelangerOS_FORCE_IPV4 is a thing in os-client-config20:54
pabelangermordred: ^does that sound right?20:54
clarkbworth noting this will also be a problem with rax and centos/fedora images20:55
pabelangerYa, its been on my list to add ipv6 to centos for a while20:55
pabelangerI should break down and do that20:56
clarkboh and osic20:56
clarkbthoughts on making that job voting as soon as its fixed?20:57
clarkbthe major chrun should be over at thsi point aiui20:57
pabelangerlooks like we can set force_ipv4 or routes_ipv6_externally: false20:58
clarkbI would set force_ipv4 as thats more direct20:59
clarkbthere is no ipv6 there despite what nova/neutron say (due to glean)20:59
mordredclarkb, pabelanger: the change we merged I thought was only to the zuulv3 branch - but I think we should do force_ipv421:00
clarkbmordred: yes I think that is correct21:00
pabelangerk, I'll somehow sed clouds.yaml21:01
clarkbpabelanger: you can use pyyaml to import it, add your key then spit it back out again21:01
clarkbmight be simplest to get correct that way21:01
pabelangerk21:01
pabelangerclarkb: actually, I can patch devstack for this21:08
pabelangerit has an update_clouds_yaml.py already21:08
clarkbpabelanger: but do we want to disable it in devstack globally?21:08
clarkbits a glean issue end of the day21:08
pabelangerclarkb: right, not change devstack but update the script an have us call directly21:09
clarkbah gotcha21:09
pabelangerOh21:13
pabelangeractually21:13
pabelangerthis is easier21:13
*** hashar has quit IRC21:15
openstackgerritPaul Belanger proposed openstack-infra/nodepool feature/zuulv3: Force os-client-config to use ipv4  https://review.openstack.org/45098321:16
pabelangerclarkb: mordred: ^I think that is what we need21:16
mordredpabelanger: that works?21:17
pabelangernot sure21:17
pabelangerI was reading os-client-config21:17
clarkbthere are like 3 clouds in the couds.yaml devsatck writes now21:18
clarkbso it will only affect the last one I think21:18
* clarkb looks21:18
pabelangerI think client is global21:18
mordredyah - the docs say that works21:18
mordredalthough I had totally forgotten doing any such thing - let me test real quick21:18
pabelangerotherwise we need routes_ipv6_externally: false21:19
pabelangerfor each cloud21:19
clarkboh thats its own top level thing gotcha21:19
clarkbwell its self testing21:19
clarkbso we just gotta wait21:19
pabelangerhowever, we could totally boot an ipv4 and ipv6 image for testing21:19
pabelangersome day21:19
mordredah - neat21:20
mordredyes - I remember writing this code now21:20
mordredpabelanger: fwiw, force-ipv4 can also be set on a per-cloud basis21:20
mordredpabelanger: setting it in the client stanza like that sets it as a global default so that any cloud stanzas that do not contain force-ipv4 will default to the value in the client stanza21:21
mordredpabelanger: but thank you for reminding me that we'd made that global setting! :)21:21
clarkbas for fixing glean we need to watch what frickler's change to nova does and then update once people like the solution proposed21:21
mordredturns out to have made this task much easier21:21
mordred++21:22
mordredand then I guess actually add support for centos yeah?21:22
clarkbbut looks like we'd change type=ipv6 to also match type=ipv6_slaac and separately add dhcp support21:22
clarkbmordred: ya21:22
pabelangerclarkb: which review is that, want to monitor21:23
clarkbpabelanger: let me find it again21:23
clarkbhttps://review.openstack.org/#/c/450297/1 that stack there21:24
pabelangermaybe we should run a 3rdparty CI system :)21:25
pabelangerand report back failures21:25
mordredpabelanger: :)21:29
clarkbyou can depends on with a glean patch and it shoudl all be tested properly21:30
clarkbbut looks like things may still be in flux a little21:30
clarkbthey are tring to understand how to not break cloud init according to the bug21:30
pabelangerif network['type'].startswith('ipv6')21:31
pabelangerya, we'll need to do that now too21:31
clarkbno you have to be even more specific21:42
clarkbbceause slaac and dhcpv6 are different /etc/network/interfaces.d/ configs21:43
pabelangerneat21:48
pabelangerya, that is going to be fun /s21:48
jeblairmordred, fungi, clarkb, jhesketh, pabelanger: i need to sign up cores to review the secrets stack.  fungi has volunteered to at least review the 2 crypto-critical changes; not sure about the rest of the stack.  i need at least one more for the whole stack.  who's interested?21:48
jeblair(it's really not bad; i just don't want to skimp on it)21:49
pabelangerjeblair: I can give a once over again, but my crypto isn't the best21:49
jeblairpabelanger: that's okay, there's a lot of zuul internal stuff too21:50
mordredjeblair: yah - I will aso review it - but I'm not fungi, so I'd love as many of us as is reasonable to21:50
jeblairjlk, SpamapS: i'd also be more comfortable if at least one of you +1d the stack21:50
jeblairmordred: thanks -- and you have a head start since you reviewed the first one already (thanks!) :)21:50
jeblair(i haven't forgotten the key size conversation, but i consider that something that's open to change until release, at least)21:51
jeblairpabelanger, mordred, fungi, SpamapS, jlk, (anyone else): the stack starts here: https://review.openstack.org/406382 and should be ready to go.21:52
* jlk looks21:53
pabelangerI'll look this evening21:53
jeblairpabelanger, mordred, fungi, SpamapS, jlk, (anyone else): note https://review.openstack.org/447087 is a refactor in the middle of the stack, so if you're going to leave a review comment of "it would be nice to put all these methods in one file..." be aware of that.  :)21:54
jlkhaha21:55
jlkgood to know21:55
mordredjeblair: what if I want you to just rename all of the methods?21:57
SpamapSjeblair: thanks for the heads up :-D21:57
SpamapSmordred: rename all the methods to variations of "theLobster()" ?21:57
SpamapSlike instead of prepareLayout() it should now be boilLobster()21:58
jeblairdo not boil gerald21:58
SpamapSand instead of submitMerge() it should be heatWaterToBoiling()21:58
SpamapSIn all seriousness.. my high school friends and I wrote a C++ obfuscator that replaced all function and variable names with random adjectives before the words 'lobster', 'crab', or 'fork'21:59
mordredjeblair: that sounds like a good bool - _should_boil_gerald = False21:59
mordredSpamapS: ahahahaha21:59
SpamapS#GetThoseNerds22:00
pabelangercan people keep an eye on https://review.openstack.org/#/c/450983/ if all tests are green, plz merge! It will fix our nodepool dsvm jobs22:00
pabelangermust eat now22:00
jeblairclarkb, pabelanger: so the idea is do that until nova decides how to spell their ipv6 option, then fix glean, then revert that?22:02
jeblair(also, is this having a production impact in osic?)22:02
clarkbjeblair: ya22:02
clarkbjeblair: no prod impac22:03
clarkbjeblair: whatever change in nova is much newer than any of our ipv6 clouds22:03
jeblairgotcha.  so as long as they don't update.... :)22:03
clarkbya22:03
jeblairsomeone send mudpuppy a cruise ticket22:03
clarkboh actually I know what it was22:06
clarkbthe nova change I linked to initially caused nova to start writing the info to config drive/metadata, buefore that it was explicitly optin22:07
clarkbso in osic we had been relying on glean not configuring anything to do with ipv6 and likely just getting default use slaac and be happy from distro defaults there22:07
clarkbso ya if they upgrade to pike we'd be in a world of hurt22:07
SpamapShas anyone bothered to check if we're using the libyaml path in our test suite runs btw?22:10
SpamapSgetting up over 10 minutes for a unit test run, might be worth it as the pure python one is sometimes 400x slower22:11
SpamapSs/400x/40x/ ... oops ;)22:11
clarkbSpamapS: just add libyaml-dev to bindep22:12
clarkboh except if we grab a wheel not built against it then we may still not use it22:12
clarkbbut I would start there and then inspect the tox log22:12
SpamapSclarkb: my past experience was that you also had to force in CSafeLoader in some cases.22:12
jlkjeblair: general question on the stack, is there any work/thought done to prevent logging of the secret values anywhere? As a service provider, I would not like to accidentally leak my consumer's secrets.22:12
SpamapSbut maybe they fixed some of that22:13
jeblairjlk: i agree.  i believe by the end of the stack there is a model.Secret object with a __repr__ method which intentionally only includes the name, not the decrypted form.  typically logging in zuul relies on either str or repr methods of data structure objects  (like "something happened with %s" % (secret)) which should make it easy for a dev to do the right thing22:17
jlkcool.22:17
SpamapSjlk: I recall discussion in OpenStack about being able to hand oslo.log a list of strings to XXX out.22:17
SpamapSdon't know that it ever went anywhere22:17
jlkIt looks like secrets are global to the tenant? I'm wondering how that would if say "github.com" is one tenant.22:18
jeblairjlk: but you know what, the decrypted form is in a ".data" attribute, which is a bit... generic.  if we renamed that to ".decrypted_secret_data" we could easily grep for all direct references to it.  that's maybe worth doing?22:18
SpamapSjlk: hah, I don't think that's how we're going to do things.22:18
jlkjeblair: that might be worth doing, giving it some extra flavor might make future devs careful about what they do with that data.22:18
jlkSpamapS: well, I didn't think we'd be writing layouts for every consumer,that'd be a TON of repeated code22:19
jeblairyeah, that way we can seek out anything that touches the decrypted form, whether for logging or just "why is that using the decrypted form there?" kinds of things.22:19
SpamapSBut even if secrets were global to all of github.com ... only zuul can decrypt22:19
jlkSpamapS: sure, but that means everybody has to play nice with names, who gets to own which secret name?22:19
SpamapSyeah you'd almost have to just enforce 1:1 with repo22:20
SpamapSor at least namespace them22:20
jeblairjlk: i think that's true for job names too, which is why i think you may want tenants.22:20
jeblairbasically, tenancy is the namespace operator22:20
SpamapSjeblair: can jobs be shared across tenant though?22:21
SpamapSBecause like, we'll have a base-nodejs job that does all the pre/post node stuff..22:21
jlkokay this is a distraction that is not relevant to the patch review.22:21
SpamapSwell it does kind of speak to the use case of tenants and secrets22:22
jeblairSpamapS: not internally, but via adding the repo to multiple tenants (which is a thing).22:22
jeblairSpamapS: so a "common-project-config" repo can be added to every tenant.22:22
jlkeach tenant could point back to the same config repo22:22
jeblairright22:22
jlkmakes sense.22:23
SpamapSThat seems like the opposite of a lot of duplicate code then.22:23
jlkWe're going to want to enforce some consistency across the whole of say github, like the pipeline names, behaviors.22:23
jeblairSpamapS: yep, it's the *same* code over and over :)  so i think it's good for the bonnyci setup.22:23
SpamapSWould it make sense to have a 'domain' level that encapsulates multiple 'tenants' ?22:24
SpamapSI haven't thought about what would be repeated though.22:24
jlkIs there also a way to prevent a repo from overriding core behaviors?22:25
SpamapSjlk: yeah, final22:25
jeblair(it's possible once we're past the threshold of premature optimization, we may want to make sure that zuul's internal data structures are optimized for heavy repitition like that... but later)22:25
jeblairSpamapS: re domain -- maybe, and probably not too hard, but my feeling is that we'll be better poised to examine that once we run into some problems we have trouble solving with tenants22:25
SpamapSI'm also a fan of having a single namespace level, and then optimizing for composition within a namespace.22:27
SpamapSI don't mind if every github.com tenant has a few lines of config repo references.22:28
SpamapSand said config repos can setup the pipelines, base jobs,tc.22:28
SpamapSetc22:28
jlkyeah that's pretty reasonable.22:31
SpamapSjlk: as long as the 'final' attribute is used properly so that we can keep users to a reasonable amount of conformity.22:33
jlkWonder how costly it'll be to iterate through all the tenants to find a place to deal with an event.22:34
jlkor how big the in-memory config will be after reading in the same yaml X times, where X == number of tenants22:34
jeblairjlk: yeah, that's the sort of thing i was alluding to earlier where we may want to optimize some stuff later22:36
jlkyup, shelved until we have data.22:36
jeblair(an emergency stop-gap solution if we hit performance problems before we can fix that would be to shard tenants across multiple zuul installations.  ultimately, in a very broad installation, that may be necessary anyway, at least until zuulv4 [fully distributed schedulers])22:39
SpamapSjlk: the events would arrive at  ${HOST}/${TENANT}/stuff22:40
jlkSpamapS: well... that's not how its set up right now22:40
SpamapSjlk: I know. :-/22:40
jlkgithub is the first thing adding a webhook, and we've made it global22:40
SpamapSbut some day maybe we'll have a working GH integration and we can get those things done without asking everybody to update. :)22:40
jlkmostly because we can't really do custom web hook URLs for every github consumer22:40
jlkyou get one URL I think.22:40
SpamapSjlk: /tenant isn't much different than a header with a repo, but your point is definitely made: indexes will be needed at some level.22:41
SpamapSjlk: I -1'd the first patch, pretty minor stuff tho... digging deeper into the stack now22:41
jlkSpamapS: from the github side, we get 1 URL we can put in our Integration. It does not appear to be dynamic at all, it's a static URL.22:43
jlkif Zuul is going to want things to be sorted into tenant buckets, we'd have to put something in front of zuul to ingest the github hook, to re-write the URL to land it in an appropriate tenant bucket.22:43
SpamapSjlk: I think we can make the GH driver build and maintain a repo<->tenant index. To your original point: it will not be free.22:45
jlkah right, I see what you mean by doing it there.22:45
jlk_that_ I think is kind of happening already?22:46
jlkmaybe not. I don't have any tests written that do multiple tenants and github.22:46
SpamapSYeah, sort of :)22:46
SpamapSclarkb: looping back to libyaml ... I'm having a hard time getting a fast yaml on my system at all.22:51
clarkbSpamapS: have a test yaml file I can use to check?22:51
SpamapSone would think 'pip install yaml' on a box with libyaml-dev would suffice22:52
clarkbI can try locally22:52
clarkbSpamapS: ya thats what I expect22:52
SpamapSfor speed? I may22:52
SpamapSjust to see if you have libyaml support, just look for yaml.CSafeLoader22:52
clarkbSpamapS: oh support at all you mean I can cehck that in a moment22:52
mordredSpamapS: I do not get yaml.CSafeLoader locally22:53
SpamapSalso look in your yaml install... the .so would be there22:54
clarkbneither do I22:54
SpamapS /usr/lib/python2.7/dist-packages/_yaml.so22:55
*** dkranz has quit IRC22:55
SpamapSUbuntu's python-yaml has that22:55
openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: Fix constructor arguments to source  https://review.openstack.org/45110222:55
SpamapS>>> yaml.CSafeLoader22:55
SpamapS<class 'yaml.cyaml.CSafeLoader'>22:55
SpamapSand as a result ^^22:55
mordredyup22:55
clarkbpython setup.py --with-libyaml install22:56
mordredSpamapS: so - I just got it from python setup.py22:56
clarkbapparently necessary to get it22:56
mordredinstall22:56
mordredwithout --with-libyaml22:56
clarkbmordred: but not pip install?22:56
SpamapSIIRC there's a pypi module that is just yaml that enforces libyaml22:56
mordredbut pip instlal didn't do that for me - I _think_ because I had a wheel cached locally from before I had libyaml-dev22:56
mordredtesting now22:56
SpamapSI thought you could get it just by building the extension when libyaml is linkable22:56
jeblairlet's all just use mordred's machine22:56
SpamapSpip install I think is grabbing a wheel22:56
SpamapSand the wheel is being overly generic for wheel reasons22:57
SpamapSI'm sorry for this bikeshed-ish thing.. I am hoping it saves us a couple of minutes per full tox run22:57
mordredSpamapS: nope - I think it's local wheel cache22:57
clarkb error: option --with-libyaml not recognized22:57
mordredSpamapS: I just deleted my pip cache and pip installed again22:57
clarkbmordred: aha!22:57
clarkbya I bet thats it22:57
mordredand it built against libyaml-dev22:58
SpamapSah so it gets cached wrong22:58
SpamapSand not invalidated22:58
mordredwell - if it gets cached before you installed libyaml-dev - yeah22:58
clarkbso adding libyaml-dev to bindep won't be engouh you also have to bypass our wheel mirror22:58
mordredah - yah - the wheel mirror in the gate makes this fun22:59
mordred/home/mordred/.cache/pip/wheels/2c/f7/79/13f3a12cd723892437c0cfbde1230ab4d82947ff7b3839a4fc/PyYAML-3.12-cp27-cp27mu-linux_x86_64.whl is the wheel that just built for me, fwiw22:59
mordredclarkb: this seems like an interestingly difficult problem to solve generally for the gate23:00
jeblairso, if the gate wheel mirror ran with libyaml-dev installed, we'd get a benefit?23:00
clarkbis libyaml support used by default? could we get away with putting libyaml-dev on the mirror builders?23:00
clarkband not break everyone without libyaml installed?23:00
jeblair(wheel mirror *build* i mean)23:00
SpamapSI believe there's a way to do this by wrapping PyYAML in a pypi package that enforces libyaml-dev23:00
mordredjeblair: we could - but it would mean any project installing pyyaml without libyaml in their bindep would get screwed I think23:00
SpamapSI think somebody may have done it23:00
* mordred checks23:00
SpamapSbut I can't seem to find it23:00
clarkbmordred: thats the question23:00
clarkbits possible pyyaml gracefully degreads23:01
mordredow. it's VERY hard to get ubuntu without libyaml23:01
jeblairmordred: want to apt-get remove pyyaml and see if your existing venv....23:01
jeblairok :)23:01
mordredunintsalling it uninstalls ubuntu-standard23:01
jlklolol23:01
clarkbso maybe its fine then :)23:02
mordredso - I think for the most part in the gate we're completely safe to build the ubuntu wheel mirror with libyaml-dev installed23:02
SpamapShttps://pypi.python.org/pypi/rtyaml23:02
mordredand that we should - since it'll make the gate better23:02
jlk"See, if you were just using a container and only installing what you wanted"....23:02
clarkbis it in centos standard and fedora standard? >_<23:02
mordredand us doing that in our wheel mirror should not impact anyone locally23:02
clarkbjlk: its actually what we want23:02
clarkbso we're good :)23:03
jlkthis is one of those fun things where you start to wonder if one use gets tested, and the other doesn't23:03
jlkand one way has bugs, the other doesn't.23:03
jlkand then we all go drink.23:03
clarkbmordred: we'll also likely need to remove pyyaml from the existing wheel cache on the builder23:03
clarkbbut then we'll be good23:03
mordredclarkb: yah23:03
SpamapSThere are things the C one can't do23:03
SpamapSthey're all things that you almost never want to do with YAML23:03
mordredSpamapS: :)23:03
SpamapSI wonder if this rtyaml is a better option long term.23:04
SpamapSIt would most likely break some things23:04
pabelangerkeep in mind, we disable wheel mirrors on pep8 jobs today23:06
clarkbpabelanger: which is fine it will just install pyyaml without libyaml bindings23:07
clarkbor you can list libyaml-dev and get them there too23:07
pabelangerclarkb: right, moving forward we could add a playbook to opt-in / opt-out for jobs23:07
pabelangerwe could do that today actually23:07
SpamapSAlso the native python yaml loader creates a ton of redundant objects and leaks tons and tons of RAM23:07
SpamapSI seem to have lost my benchmark scripts..23:10
SpamapSbut I do have my "make giant YAML files" scripts and my big yaml files23:10
jlkForever tagging SpamapS as "Big YAML" in my mind.23:11
pabelangerclarkb: jeblair: I should I add a TODO to about removing this once we've updated glean? https://review.openstack.org/#/c/450983/23:15
clarkbpabelanger: sure23:17
jeblair++23:17
SpamapS"I love it when you call me Big YA-ML  Throw white space in the air, if you a true player"23:17
openstackgerritPaul Belanger proposed openstack-infra/nodepool feature/zuulv3: Rename nodepoold to nodepool-launcher  https://review.openstack.org/45087723:17
jlkis status.json and it's ilk all now tenant scoped? Is there no longer a global status?23:18
openstackgerritPaul Belanger proposed openstack-infra/nodepool feature/zuulv3: Rename nodepoold to nodepool-launcher  https://review.openstack.org/45087723:19
openstackgerritPaul Belanger proposed openstack-infra/nodepool feature/zuulv3: Force os-client-config to use ipv4  https://review.openstack.org/45098323:19
pabelangerjlk: I believe so23:19
jeblairjlk: there are some loose ends there, but that's the intent23:22
SpamapSI think I have a patch that will make use use libyaml23:25
openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: Add canonical hostname to source object  https://review.openstack.org/45111023:26
jeblairjlk: ^ start of work we talked about last week.  i think/hope i'm going to be able to do this in a bunch of small patches.23:27
jeblairmordred: fyi ^23:27
SpamapSoy23:33
SpamapSso far, configloader is unhappy with CSafeLoader23:33
jeblairSpamapS: it's worth doing this work on the end of the secrets stack, where i made changes to configloader and yaml loading23:33
SpamapSjeblair: Ah I hadn't got that far down. Good idea.23:34
SpamapShttp://paste.openstack.org/show/604586/23:34
SpamapSin case you're curious.. haven't debugged it yet, but all tests are basically exploding on that23:35
Shrewsalways so much conversation after my EOD23:35
openstackgerritClint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Use libyaml if possible  https://review.openstack.org/45111323:36
SpamapSShrews: it's so much easier to talk without your superior intellect and education around.23:37
SpamapS^^ that's my (fairly broken still) patch to try and use libyaml if it's available.23:37
ShrewsSpamapS: that's blatantly false on so many levels23:39
SpamapSmmmmm failing fast23:39
SpamapSRan 228 (+35) tests in 399.195s (-507.783s)23:39
SpamapSFAILED (id=35, failures=142 (+133), skips=31)23:39
jlkIs this semaphore change really part of the secrets stack?23:42
clarkbSpamapS: I think thats voluptuous schema failing to apply conf because conf is None?23:46
clarkbSpamapS: the getSchema is returning the schema object which is then called against with (conf) and thats what raises23:47
clarkboh "None", line 1 column 1 is the _start_mark value23:48
clarkbso its breaking right away maybe?23:48
SpamapSclarkb: dunno, I'm just now running with a debugger in line23:49
* jlk out.23:49

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!