/srv/irclogs.ubuntu.com/2017/02/21/#juju-dev.txt

babbageclunkwallyworld, axw: review please? https://github.com/juju/juju/pull/701000:13
wallyworldok00:14
babbageclunkwallyworld: thanks - ended up being a bit stupid00:16
wallyworlda one liner00:16
wallyworldbabbageclunk: lgtm00:17
babbageclunkwallyworld: yup - the better fix is lots of lines across 3 packages00:17
babbageclunkwallyworld: thanks00:18
wallyworldfor 2.1 this is fine IMO00:18
axwbabbageclunk: is that change safe for concurrent AddCharmWithAuthorization calls? will two calls to repo.Get end up getting the same bundle, and one will remove it from underneath the other?00:30
menn0wallyworld, axw: during cloud-init, the proxy settings are only used to write out to ~ubuntu/.juju-proxy01:19
wallyworldmenn0: yeah, that's what i feared01:19
menn0wallyworld, axw: which is dumb anyway01:19
axwmenn0: no, we do also set them for the cloud-init scripts01:19
menn0wallyworld, axw: the proxy settings aren't used in any other way01:19
axwmenn0: see `exportedProxyEnv := w.icfg.ProxySettings.AsScriptEnvironment()`01:19
axwmenn0: in cloudconfig/userdata_unix.go01:20
axwerr, userdatacfg_unix.go01:20
menn0axw: you're right. I missed that the proxy settings were executed01:20
menn0axw, wallyworld: so the one thing we should fix is that the proxy settings should probably be written out to the global profile (which affects all users) instead of just the ubuntu user01:22
menn0axw, wallyworld: do you agree?01:22
axwmenn0: seems reasonable01:22
wallyworldi think so01:22
wallyworldi wonder why we restricted to ~ubuntu in the first place01:22
menn0axw, wallyworld: which is fine except maybe for the manual provider01:22
babbageclunkaxw: ugh, you're right. Ok, I'll have to go for the more invasive way.01:23
babbageclunkaxw: thanks though! Luckily the merge failed.01:23
axwmenn0: if you manually provision, you're handing over the reins. I think it's fine to do that. ideally we would be able to undo on agent uninstall tho01:24
axwbabbageclunk: nps01:24
menn0axw: yep... which should be doable01:24
menn0wallyworld, axw: it looks like we don't bother with the proxy settings on windows01:24
* menn0 files some bugs01:25
axwwallyworld babbageclunk: I'm going to be out both standups tomorrow and thursday (dentist, then school assembly)02:22
wallyworldno worries02:22
axwwallyworld: seems you were right, LXD talks to the source directly02:30
wallyworldaxw: it was a guess rather than being right :-)02:30
axwwallyworld: but the client does need to connect to the source and the LXD as well02:30
axwwallyworld: so I think it's that initial bit where it's trying to get the info that's going to be the problem02:31
wallyworldright, and that's something we can fix at our end02:31
wallyworldalbeit a bit hackish perhaps02:31
thumperwell... bollocks, and more bollocks02:40
thumpersomehow in my massive pile of changes, I seem to have broken some of the macaroon stuff02:41
thumperseems that what used to end up having a successful post to /auth/discharge, now gets:02:41
thumpercannot get discharge from \"https://localhost:35994/auth\": cannot acquire discharge: cannot http POST to \"https://localhost:35994/auth/discharge\": Post https://localhost:35994/auth/discharge: x509: certificate signed by unknown authority02:41
thumpersince this is a test... the tlsConfig should be set to use the CA cert... I think02:42
thumperbut this is in the http bakery code...02:42
thumperso NIF02:42
thumperNFI02:42
* thumper diggs through the code changes to see if I have inadvertantly deleted something important02:42
thumperhmm...02:44
* thumper thinks some...02:44
thumperhmm.... I have a cunning plan02:45
thumperhopefully not too cunning...02:45
thumpergah...02:46
thumperI think this is another "localhost" != "127.0.0.1" issue with the bakery apiHost02:47
* thumper stretches his brain02:53
thumperthis may or may not work...03:22
thumperI'm hoping it will03:22
thumperit'll save a lot of pain03:22
thumperyay, that fixed the macaroon issue03:24
thumperphew03:24
thumpernow for the others...03:24
thumperI may be able to revert some of these other shitty changes I have done03:24
thumperreduce the diff size03:24
thumperhazaah03:24
* thumper afk for a bit -- kid pickup03:38
=== thumper is now known as thumper-afk
* thumper-afk headdesks04:22
thumper-afkoh04:22
=== thumper-afk is now known as thumper
thumperFFS04:22
* thumper takes a deep breath04:22
thumperwas trying to work out why the mutex values I added weren'04:23
thumperweren't stopping concurrent writes04:23
thumperthen I noticed the reciever wasn't a pointer04:23
thumperso every method call got its own mutex04:23
thumperawesome04:23
anastasiamacnice :( what area r u in?04:25
thumperrpc/jsoncodec04:28
thumperit was my doing though, adding the mutex04:28
thumperbecause gorilla/websocket has checks for concurrent writes / reads04:28
blahdeblahthumper: still around?04:53
thumperkinda04:54
thumperwhazzup04:54
thumper?04:54
blahdeblahremember the "mad-hack" you guided me through when we had missing txn-revnos?04:54
thumperah... yeah04:54
blahdeblahIt's just happened to me for the 3rd time in a different environment, with the same charm.04:55
blahdeblahWondering if it's worth logging a bug04:55
blahdeblah(given that it's 1.25.x)04:55
thumperreally? that is kinda batshit crazy04:56
blahdeblahIKR!04:56
thumperhave you been doing any juju upgrades?04:57
blahdeblahthis is 1.25.6 and has been for quite a while04:57
thumperhmm...04:57
thumperis it an environment that is resistent to upgrading?04:58
blahdeblahWell, between 1.25.7 and 1.25.9, we were quite resistent to upgrading... ;-)04:58
blahdeblahI don't have a problem with upgrading to 1.25.10, but is that going to fix anything?04:59
thumpernot sure...05:00
thumperI'd have to go look at the 1.25.6 codebase05:00
thumperblahdeblah: file a bug with as much info as you have, and I'll take a look to diagnose tomorrow05:01
thumperit is possible that 1.25.10 will fix, but it probably makes sense to look05:01
thumperif I can't find a good answer, at least we can write that on the bug05:01
blahdeblahthumper: I kept detailed notes about what we did and last time just stepped through and fixed it05:01
thumperassign to me too05:01
blahdeblahSo I'm happy to go through again and do the same; or would you prefer I left it until we can dive in further?05:02
thumperblahdeblah: no, don't wait to fix05:10
thumperI'll just be reading code05:10
* thumper thrashes his computer some more with a full test run05:11
thumperbabbageclunk, menn0: I found a way around the whole "localhost" != "127.0.0.1" issue05:11
thumpermuch cleaner05:11
thumperthank god05:11
blahdeblahthumper: OK - thanks05:15
wallyworldjam: if you get a chance, i'd appreciate a look at the IngressNetworksForRelation() API implementation in https://github.com/juju/juju/pull/7012. For now I just get state.AllIPAddresses() and filter multicast and loopback. I also filter out IPv6 and AWS didn't like it when updating the security groups05:33
jamwallyworld: looking05:38
wallyworldthanks05:38
thumperhey y'all, got a groovey review: https://github.com/juju/juju/pull/701305:40
thumperalthough babbageclunk did say he'd take it05:40
thumpergiven that it is huge05:41
wallyworldnot thaaaaaat big05:41
* thumper is done05:42
thumperlaters folks05:43
mupBug #1666396 opened: Missing txn-revnos in mongodb leads to missing status updates <canonical-is> <juju-core:New for thumper> <https://launchpad.net/bugs/1666396>05:54
axwanastasiamac: would you please review https://github.com/juju/juju/pull/7014?06:53
anastasiamacaxw: in a sec but will :D06:53
jamaxw: "the proxy should reject attempts to connect to its own lxdbr0 address"07:01
jamaxw: I'm not sure that should be "the proxy should"07:01
axwjam: it should :)  what isn't clear?07:02
axwjam: the idea is to show that no_proxy is working07:02
jamaxw: so you said the host's IP address on lxdbr0 is 10.250.243.1, and then you set the proxy as 10.250.243.1 if the proxy is rejecting attempts to connect to that address07:02
axwjam: I did also test without no_proxy to show that it *does* fail07:03
jamthen nothing can talk to the proxy07:03
axwjam: specifically, it should reject attempts to connect to 10.250.243.1 *through* the proxy07:03
axwI'll reword07:03
jamaxw: k, that could be spelled a bit clearer07:03
jamcause I'm pretty sure you're connecting *to* the proxy via that IP :)07:04
jamaxw: ugh, just spent 10 minutes searching for the right version, cause we have "github.com/juju/juju/utils/proxy" *and* "github.cqm/juju/utils/proxy"07:16
axwjam: heh yeah :/07:16
jamaxw: we should probably update that code to support CIDR notation07:17
jamI see a fair bit of stuff that plays games with the strings instead of using net.SplitHostPort, etc.07:18
jamaxw: hasPort() => strings.LastIndex(':') > strings.LastIndex(']')07:19
axwjam: would be nice, but I don't think $no_proxy supports CIDR normally?07:19
jamseems ... fishy07:19
axwjam: it does a bit. I think it was lifted from net/http07:20
jamaxw: yeah, looking at docs it seems it is intended to support domain suffixes instead of IP prefxies07:20
jamprefixes07:20
wallyworldaxw: your PR relies on having env vars for proxies to use in the CLI command. and uses model config for the agent related stuff. IIANM. Is that an issue that these 2 sources could be out of sync?07:38
menn0axw, jam: the fishy string monkeying in the net/http proxy handling code has been fixed in recent versions of the Go stdlib07:57
jammenn0: sounds like we should update our fork07:58
jamwallyworld: I'm somewhat ok with that. "What are the PROXY settings *of my laptop* is not the same as what are the PROXY settings for my cloud"07:59
menn0jam: our fork in gh/juju/juju/utils/proxy does the right thing I believe07:59
jammenn0: hasPort still does strings.Find07:59
jamsorry strings.LastIndex07:59
menn0jam: ok right. well in Go 1.6 it's much worse :)08:00
wallyworldjam: yeah, that does make sense, but it can lead to confusion for local lxd cloud where everything runs on same host08:00
menn0jam: the code looks like current master08:00
menn0(Go master)08:00
menn0I believe08:00
jammenn0: net.http.ResetCachedEnvironment()08:01
jamseems to be a thing08:01
menn0jam: only in an export_test.go08:02
jamyeah08:02
jamI just tracked that down08:02
jamblah08:02
menn0hence all the trickery08:02
jammenn0: one of those "if its useful in tests, it would *probably* be useful in production for other people"08:02
menn0jam: indeed :)08:03
jamnobody would ever change proxy settings on the fly, right?08:03
axwwallyworld: that's existing behaviour, didn't want to muck with it right now08:10
wallyworldno worries, it does make sense that that's what it needs to be08:10
axwwallyworld: i.e. we previously would have honoured $http_proxy in the client, but not used http-proxy from --config in the client08:10
wallyworldwe should make sure doc is clear08:11
wallyworldand maybe print a message if there are differences when using local provider or something? maybe that's overkill08:11
axwjam: thanks for the review08:20
axwwallyworld: we could print out what proxy settings are detected, might be useful. but not in this PR08:21
=== jamespag` is now known as jamespage
perrito666does anyone know if tim replicated https://bugs.launchpad.net/juju/+bug/1634178 ?11:51
mupBug #1634178: login doesn't prompt password <ci> <intermittent-failure> <jujuqa> <login> <juju:In Progress by thumper> <juju 2.0:Won't Fix> <juju 2.1:Triaged> <https://launchpad.net/bugs/1634178>11:51
wallyworldjam: thanks for review, i've replied to comments etc. i'll take another look, but I could not see subnets being populated from the provider at all - the provider (the aws one I looked at) seemed to populate only the ipadresses collection11:57
jamwallyworld: the provider isn't populating subnets, that's the discussion I had with you11:58
jamwallyworld: the ipaddresses is only being populated in 2.1 by the JUju machiner watching the local ip devices11:58
wallyworldjam: oh, i thought we said maas and aws did (via their NetworkInterface implementation11:58
jamwallyworld: but you're missing a bunch of the information, like all the subnets that will be used in the next add-unit11:58
jamwallyworld: they aren't?11:59
jamaws and MAAS should be11:59
wallyworldnot that i saw - i traced the code and it all seemed to point to ipaddresses being poulated11:59
wallyworldi'll need to look again11:59
wallyworldi started using AllSubnets() and it never seemed to get populatred11:59
jamwallyworld: all of the "juju deploy --bind space" stuff requires subnets to have values11:59
wallyworldhence i looked at the code and saw ipaddresses11:59
wallyworldok, i'll take another look12:00
wallyworldi may have misread something12:00
wallyworldjam: i'm looking at the discover spaces worker. that seems to want to update the subnets collection, right?12:09
jamwallyworld: yeah12:10
wallyworldjam: but aws provider does not support spaces discovery12:10
wallyworldso maybe it's only maas that will have subnets collection populated12:10
jamwallyworld: so for AWS, you do manual "juju add-space" and "juju add-subnet" if you want to use "juju deploy --bind", because we don't know what subnet is in what space if you don't tell us.12:11
wallyworldjam: right, so that screws up the firewaller and cmr12:11
wallyworldwe want cmr to work without adding spaces12:11
wallyworldwithout requiring spaces12:11
jamwallyworld: so update the discovery mechanic to record subnets without requiring spaces12:12
jamwallyworld: its still *far* better12:12
jamelse you run into "what machines have I actually deployed so far"12:12
jaminstead of "what subnets could be in use as soon as I add-unit"12:12
wallyworldright, i didn't didn;t think i needed to do that as i thought aws provider populated its stuff12:12
wallyworldi'm just seeing a lot of this code for the first time12:12
wallyworldnot sure what's there already12:12
jamwallyworld: to be fair, *it should have*12:12
jamwallyworld: (I only saw concrete code a few months ago as well)12:13
wallyworldand i'm getting my head across the model etc12:13
jamwallyworld: well, I'm pretty solid on how things "should" be, and loosely aware of how things *are*. :)12:13
wallyworldok, so i'll rework the PR to use subnets; the fallabck to 0.0.0.0/0 will work, and then i'll need to add stuff to the providers12:13
wallyworldeverything is awesome12:14
wallyworld"how things are" is interesting, a fair bit to do still12:14
wallyworldit seems12:14
jamindeed12:17
wallyworldjam: so would it be reasonable to modify the discover spaces worker to say that is the provder doesn't support spaces discovery, just ask it for all subnets (if it supports that which i think aws does) and whack those in the subnets collection? my fear is that subnets appear to have a space name attribute and that if we record subnets with a blank space name, things will screw up later12:17
jamwallyworld: we're currently treating "" as the "unknown" space elsewher, I think its fine12:21
wallyworldjam: ok, i'll see how it goes. i'll look to land this PR after changing t subnets; do a followup which adds the subnet listener to the worker so that as subnets are added async the ingress rules are updated; start fixing the proivders starting with aws to populate subnets info. i wasn't budgeting to that that last bit, nor updating the other providers; hopefully it won't all blow out too much12:24
rogpeppe1does anyone have strong feelings about removing colors from vanilla loggo and putting them into a separate package under utils?13:40
rogpeppe1we've had problems with all our services producing unparsable logs because of the default behaviour13:41
=== rogpeppe1 is now known as rogpeppe
rogpeppeand i don't think it's right that a low level logging package should be pulling a bunch of special dependencies for that purpose13:41
perrito666as long as you dont break juju default behavior13:43
rogpeppewithout colors, loggo has no external dependencies. with them, it depends on 6 large external repositories13:43
rogpeppeperrito666: we'll change juju to use the colour writer13:43
rogpeppeperrito666: its behaviour won't change until the loggo dep is updated in juju13:44
perrito666rogpeppe: wel without colors I would not use juju logger :p so they make sense to me13:44
perrito666rogpeppe: I dont have objections then13:44
rogpeppejam, wallyworld: ^ ?13:44
perrito666rogpeppe: I would feel better if you enlisted the opinion of thumper since he implemented the colour fancines13:45
* perrito666 wrote color in brit english just for rogpeppe13:45
rogpeppeperrito666: ok, i'll post a mail to the list13:46
wallyworldrogpeppe: yeah, best tto ask thumper; i do see you have a point though13:46
rogpeppeperrito666: i never see thumper in my time zone13:46
perrito666rogpeppe: yes, he seems to be in the exact oposite, would you like for me to ping him so he answers your mail promptly?13:47
rogpeppeperrito666: please13:47
perrito666rogpeppe: noted13:47
rogpeppeperrito666: ta13:47
rogpeppeperrito666: i'm sure he'll see my mail to the list anyway13:47
perrito666rogpeppe: certainly just making sure he answers asap13:47
rogpeppeperrito666: i've sent a message to the list14:02
perrito666rogpeppe: tx, I added an item to my todo to talk to thumper as soon as he says good mornig14:02
rogpeppeperrito666: thanks!14:03
perrito666jam: ping me if you are back around15:16
=== mup_ is now known as mup
cmarsall right, i forget, if i'm going to propose a PR into juju, what should I clone off of / propose merging with? master? develop? staging? whaaa?18:55
perrito666cmars: hard question18:55
cmarslol18:55
perrito666I believe it would be safe to branch staging and propose into develop18:56
cmarsperrito666, ok, thanks!18:56
perrito666cmars: although you might want to ask someone else too, I can never recall if staging is up to date with 2.118:57
perrito666sinzui: ?18:57
sinzuiperrito666: cmars, for the last 10 days, staging and master have been getting develops blesses. staging is testing the bless develop got a few hours ago.18:58
sinzuiperrito666: cmars staging/master is NEVER a stable branch. 2.1 and 2.0 are their own histories...18:59
sinzuiwhich makes you wonder about the intent to always release off og master18:59
cmarshmm19:01
cmarsi'll start with staging and cherrypick it wherever it needs to go19:01
=== frankban is now known as frankban|afk
babbageclunkmorning!19:57
perrito666babbageclunk: morning19:58
babbageclunkperrito666: feel like a fairly easy review? https://github.com/juju/charmrepo/pull/11020:10
thumperwell.... shit20:13
perrito666thumper: ah, one of those mornings20:17
perrito666thumper: btw, there is a rogpeppe mail that is pretty much just for you20:17
perrito666babbageclunk: if you can hold until I return from the supermarket ill gladly review that20:17
perrito666also if any of you is very familiar with how juju handled tools in 1.25 there is someone needing help in the private channel :)20:19
babbageclunkperrito666: dude, if you don't want to just say so. ;)20:19
babbageclunkthumper: feel like a fairly easy review? https://github.com/juju/charmrepo/pull/11020:19
thumperperrito666: saw it20:20
thumperbabbageclunk: how about a call, you can explain you change, and I need a teddy bear20:20
babbageclunkok20:26
babbageclunkthumper: ^20:26
thumperbabbageclunk: https://hangouts.google.com/hangouts/_/canonical.com/lol-wat20:26
perrito666back20:43
=== jog_ is now known as jog
wallyworldmenn0: re bug 1666660 - normally we remove the primary document (the model) before any satellite documents. I assume that will be the focus of your fix?21:53
mupBug #1666660: list-models sometimes errors after a migration (before model has been removed) <ci> <model-migration> <juju:New for menno.smits> <https://launchpad.net/bugs/1666660>21:53
=== frankban|afk is now known as frankban
menn0wallyworld: I'm not working on that at the moment and haven't got a specific solution in mind22:06
wallyworldmenn0: no worries, was just curious based on the description22:07
menn0wallyworld: looking at removeAllModelDocs, it removes the model doc last22:07
menn0wallyworld: which will be the problem22:07
wallyworldyep22:07
menn0wallyworld: at the point it's called the model is marked Dead22:07
menn0wallyworld: I suspect that model listing and lookup should avoid Dead models22:08
wallyworldwe've had similar issues in the past with other entity->subordinate object relationships22:08
wallyworldyay mgo txns and not being atomic22:08
wallyworld"txns"22:09
menn0wallyworld: understood22:09
menn0wallyworld: in this case I think it makes sense for removeAllModelDocs to remove the model last22:09
menn0wallyworld: in case it fails, leaving the model doc behind makes it possible to retry22:10
menn0wallyworld: I think GetModel should ignore Dead models22:10
=== mup_ is now known as mup
=== mup_ is now known as mup
=== frankban is now known as frankban|afk
=== thumper is now known as thumper-dogwalk
blahdeblahthumper-dogwalk: Did https://bugs.launchpad.net/juju-core/+bug/1666396 make sense?  Please let me know if you need any further information.23:13
mupBug #1666396: Missing txn-revnos in mongodb leads to missing status updates <canonical-is> <juju-core:New for thumper> <https://launchpad.net/bugs/1666396>23:13

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