/srv/irclogs.ubuntu.com/2015/03/11/#juju-dev.txt

menn0davecheney: I just added a mutex to control access to Deque and it completely ruins performance. given that I don't need it for my use case i'm going to leave it out. ppl who need goroutine safety can do that at a higher layer.00:17
=== arosales_ is now known as arosales
davecheneymenn0: ok, thanks, that was my question00:24
menn0davecheney: it beats me why, but using *list.List and list.New() is consistently a few % faster than using the list.List zero value.00:31
davecheneyprobably alignment00:31
menn0davecheney: it's not enough of a difference to worry me but it's interesting anyway00:31
davecheneyit won't matter in real life00:31
* menn0 agrees00:31
davecheneyyour dedication to performance is admirable00:32
davecheneybut i think it will be lost in the niose00:32
davecheneyjuju talks over the network00:32
davecheneythat's what takes all the time00:32
wallyworldaxw: can we chat when you get back?00:32
menn0davecheney: sure. i'm going to use the zero value.00:34
menn0davecheney, thumper, waigani_: updated the Deque PR. http://reviews.vapour.ws/r/1122/diff/2-3/00:48
thumpermenn0: I've given it a shipit, but it would be good manners to wait for davecheney and waigani_ :-)00:55
thumperI've got this cache file configstore thing done now (I think)00:55
thumperrunning full test suite00:55
waigani_menn0: shipit from me00:59
thumperhow do we know if the bot is hooked up for merging?01:01
* thumper is thinking of juju/utils01:02
menn0thumper: i'm guessing it isn't b/c merges are being done by real people instead of jujubot01:03
thumperhmm01:04
thumperI'm having a WTF moment01:04
thumperanyone else getting errors running tests in service/upstart?01:04
thumperI get five failures01:05
thumperall the same place01:05
thumperupstart_test.go:240:01:05
thumper    c.Assert(err, gc.ErrorMatches, ".*exit status 99.*")01:05
thumper... error string = "exit status 2"01:05
thumper... regex string = ".*exit status 99.*"01:05
jw4I wasn't a couple hours ago - I'll pull and test again01:14
thumperI get this test failure on master01:15
jw4nope, no new changes since I last ran a clean test run01:15
thumperboth when I rull all the tests, and just the tests for that package01:15
thumperwtf?01:15
jw4I'm on trusty01:15
thumperme too01:15
axwdavecheney: re "state", I sympathise, but I can't think of a better name. any suggestions for an alternative that describes persistent local state?01:18
wallyworldaxw: got time for a quick hangout?01:19
thumperinterestingly TestStart does the same sort of thing, but it passes01:19
axwwallyworld: sure, see you in tanzanite?01:19
* thumper tries to work out what is different01:19
wallyworldok01:19
jw4thumper, I'm curious what extra logging around line 168 of service/upstart/upstart.go would reveal?01:31
waigani_menn0, thumper: http://reviews.vapour.ws/r/1115/ - envusers is going to follow a similar path. So I'd like to get this reviewed first.01:31
thumperjw4: line 168?01:32
thumperTestRemoveStopped passes01:32
jw4in the Running() method?01:33
thumpersorry, thought you meant the _test file01:34
jw4i.e. it feels like some system level failure is happening (with a return code of 2) before the expected failure with return code of 9901:34
axwwallyworld: are you shippitting http://reviews.vapour.ws/r/1112/ ?01:35
wallyworldah bollocks, yes sorry01:35
wallyworlddone01:36
=== kadams54 is now known as kadams54-away
thumperWhat the actual fuck01:38
thumperdamn screwed test isolation problem01:38
thumperif I run the test by itself, it passes01:38
thumperbut the whole suite, it fails01:39
jw4:(01:39
* thumper pokes01:39
jw4although to be perfectly honest... when it comes to debugging.... words like WT(A)F are actually beautiful.  The programmers inverse of Eureka!01:40
thumperhah01:41
thumper[LOG] 0:00.074 DEBUG juju.service.upstart Running out: "\x1b[1;33mjuju status -e local\x1b[0m\nerror: cannot determine juju home, required environment variables are not set\n\x1b[1;33mjuju status -e wikienv\x1b[0m\nerror: cannot determine juju home, required environment variables are not set\n\x1b[1;33mjuju status -e kibanaenv\x1b[0m\nerror: cannot determine juju home, required environment variables are not set\n"01:41
thumperthat occurs before each failure01:41
jw4weird... seems like that should at minimum be logged at WARN?01:41
thumperthat is the logging line I added01:42
davecheneywhat is all that colorisation shit in there ?01:42
thumperbut the question is 'why' ?01:42
thumperdavecheney: I believe so01:42
thumperit looks like something is calling out to my shell01:42
thumperand I must have some weird executable in the path01:42
davecheneyand it's sourcing your .profile01:43
davecheneyso who knows what kind of stuff is in the environment01:43
jw4blech01:43
thumpersomething is so wrong with this01:44
thumperthe Running method swallows this error very easily01:44
thumperha01:46
thumperhaha01:46
thumperfucker01:46
* thumper has /home/tim/bin/status01:46
thumperwhich has colour output01:46
* thumper fixes01:46
thumperthe test fakes out the start/stop/status generally01:47
thumperbut upstart service Start calls Running, which calls the status command01:47
thumperwhich is getting fooled by my command01:47
* thumper thinks01:47
thumperwhy is it getting my path?01:47
thumperI thought our IsolationTestSuite fixed that01:48
jw4hahha01:48
thumperuh ha01:50
thumpertesting.BaseSuite doesn't use the IsolationSuite01:50
thumperit looks like someone looked at it01:50
thumperbut didn't make the change because too many tests failed01:50
thumperas they relied on things in the path01:50
davecheneythumper: rename the suite01:51
davecheneySortOfIsolationSuite01:51
thumperdavecheney: the problem is that we aren't using the IsolationSuite01:51
davecheney // SortOfIsolationSuite tries to isolate your tests, sort of, it works, mostly.01:51
thumperand I think we should be01:51
axwwallyworld: you're still going to disable the storage provisioners in this branch, right?01:52
wallyworldaxw: i disabled the workers01:52
axwwallyworld: cmd/jujud/agent/machine.go is still starting those workers...01:53
* thumper tries to think about how best to fix this01:53
wallyworldaxw: hmmm, ok, let me check - i modified the tests to ensure the workers weren't running01:53
axwoh I see, there's a bool01:54
axwwallyworld: don't disable *all* of the storage workers - the others work fine01:54
wallyworldaxw: yeah, i just saw that01:54
wallyworlddoh01:54
wallyworldwill fix01:54
=== kadams54-away is now known as kadams54
thumperhttps://github.com/juju/juju/pull/179402:03
thumperdavecheney, menn0: trivial one http://reviews.vapour.ws/r/1123/02:16
menn0thumper: will take a look once i've finished looking at Jesse's branch02:17
menn0waigani_: just looking at the lowercase user name branch02:27
menn0waigani_: is it supposed to be included in 1.23?02:27
menn0davecheney: are you happy for http://reviews.vapour.ws/r/1122/ to be merged now?02:29
davecheneymenn0: one final round of comments02:30
menn0waigani_: just finished reviewing your username case PR. I think I see a bug but otherwise good.02:31
davecheneymenn0: lgtm02:32
menn0davecheney: kk. I don't see them yet. Are you still working on them?02:32
davecheneywas going to argue about type blockT []interface{}02:32
davecheneybut i couldn't be bothered02:32
davecheneyit's too hot for pedantry today02:33
menn0davecheney: :)02:33
waigani_menn0: txn documentation says that an insert will go ahead even if the doc already exists (thats why there is an exists assert). I was going off that - but I'll add a test to make sure.02:34
thumperhere is another: http://reviews.vapour.ws/r/1124/diff/#02:34
thumperno type called State in there :)02:34
thumpercoffee time02:34
menn0waigani_: fair enough. if that works then great (even if it's a little confusing). the test would be good.02:34
waigani_menn0: actually, I was wondering about the blockT also, if it also caught davecheney's eye - maybe at least a comment?02:35
waigani_menn0: what does the "T" stand for?02:35
menn0waigani_: Type02:36
menn0waigani_: I wanted to use "block" for variable names02:36
waigani_right02:36
davecheneyif we're going to have the discussion02:37
davecheneylets not02:37
menn0+102:37
davecheneygofmt -r 'blockT -> []interface{}' -w *.go02:37
davecheneydone02:37
menn0-002:37
menn0:)02:37
davecheneymenn0: yellow card, only two's compliment allowed in this channel02:38
menn0ha02:38
wallyworldaxw: free for another chat?03:08
axwwallyworld: just a minute please, propsing a branch03:09
wallyworldsure, np03:09
beisner /o\  fyi 1.22beta6 does not appear to resolve bug 143004903:09
mupBug #1430049: unit "ceph/0" is not assigned to a machine when deploying with juju 1.22-beta5 <oil> <juju-core:Fix Committed by axwalk> <https://launchpad.net/bugs/1430049>03:09
axw-_________________-03:10
axwbeisner: ah, from status it's still running beta503:11
beisnerand...  \o/  fyi 1.22beta6 DOES appear to resolve bug 1420049 !03:11
mupBug #1420049: ppc64el - jujud: Syntax error: word unexpected (expecting ")") <deploy> <openstack> <regression> <uosci> <juju-core:In Progress by axwalk> <juju-core 1.22:Fix Released by axwalk> <https://launchpad.net/bugs/1420049>03:11
axwwallyworld: see you in tanzanite again03:12
axwbeisner: yay, thanks for verifying03:12
wallyworldaxw: sure03:12
beisneraxw, ah indded.   not sure juju-deployer can do equivalent to upload-tools (?).  so I'll have to wait for beta6 tools to hit the streams.03:16
rick_h_beisner: not following, deployer runs on bootstrapped env so you just need --upload-tools at bootstrap?03:17
rick_h_beisner: juju-quickstart takes --upload-tools flag?03:18
* rick_h_ rereads bug03:18
beisnerjust reproducing the same way we caught it (bootstrapping via juju-deployer -B)03:18
rick_h_beisner: ah, didn't notice that in the bug anywhere03:18
beisnerrick_h_, it's entirely possible that the problem exists with a bootstrep / then deploy.  but i'm not sure atm.03:19
rick_h_beisner: understood03:19
beisnerrick_h_, easy enough to test though, so i'll kick off a modified loop with beta6 and --upload-tools.03:20
beisnerrick_h_, will check back in tomorrow on that.03:22
rick_h_beisner: np, just hoped to save you some stream waiting03:22
beisnerrick_h_, yep, appreciate that.03:23
beisnerand she's off!03:27
beisnerthanks for the fix(es)!  checking out.03:32
mupBug #1282642 changed: Bootstrap prefers .jenv over environments.yaml <bootstrap> <ci> <config> <destroy-environment> <regression> <juju-core:Won't Fix> <https://launchpad.net/bugs/1282642>03:41
menn0thumper: review done. there's lots of comments but they're already pretty minor03:47
thumpermenn0: thanks03:47
menn0waigani_: ship it with some minor comments for http://reviews.vapour.ws/r/1115/03:48
wallyworldaxw: about to lose power here for 20 mins fyi03:50
axwwallyworld: okey dokey03:51
waigani_menn0: thanks03:55
=== kadams54 is now known as kadams54-away
=== kadams54-away is now known as kadams54
=== kadams54 is now known as kadams54-away
mupBug #1430639 was opened: juju can't bootstrap in LXC <juju-core:New> <https://launchpad.net/bugs/1430639>05:54
urulamawallyworld_: hey, no worries, see you next week05:54
wallyworld_urulama: ty, about to go :-)05:54
jamaxw: are you around?06:24
jamyou're the one that implemented getting "config-changed" events when the IP address changes, right?06:24
axwjam: hey, yes06:25
jamI'm looking at http://reviews.vapour.ws/r/1118/diff/# from dimiter and I'm thinking at least some of it isn't the right fix.06:25
axwlooking06:25
jamaxw: I *just* published my comments on it06:28
jamI'm testing that it fixes the test I saw failing now06:28
axwjam: I'll keep thinking, but I had the same thoughts as you initially: we should only reset seenConfigChanged/configChanges when the charm changes06:30
jamaxw: do you agree that it might suppress address events ?06:31
axwjam: yes06:32
jamaxw: have you seen the intermittent failures?06:33
axwI suppose we're missing a test then06:33
axwno, I haven't06:33
axwjam: that test could indeed trigger the address watcher twice: once for the initial, another time for the update06:36
jamaxw: I'm pretty sure that test was written when we didn't have an initial, thoughts on how to clean it up?06:41
axwjam: only thing I can think to do is to take out the SetAddresses, or move it to before the filter is started06:58
axwsame effect either way, but the latter might be easier to understand06:58
jamaxw: well the whole test is that we don't generate an event until we SetAddress, right?06:58
jam"TestIgnoreInitialAddressEvent"06:58
axwjam: sorry, I was thinking of TestConfigAndAddressEventsDiscarded06:58
* axw runs a test06:59
axwjam: that test is a bit weird. I would think it should be setting the charm URL, then checking that no config-changed is emitted, then setting the addresses and checking one is emitted07:01
axwjam: eh, except that an address change *always* gets generated. its watcher is started immediately, regardless of whether there's a charm URL07:08
axwjam: I think the test doesn't make sense anymore. It was written when the logic was different, https://github.com/juju/juju/blob/1.21/worker/uniter/filter.go07:10
jamaxw: so *something* should be testing the seenConfigChange sort of tests07:10
jamif we remove the tests, then we should be removing the "maybeConfigEvent" code07:11
jamaxw: interestingly in https://github.com/juju/juju/blob/1.21/worker/uniter/filter_test.go it says "we don't set addresses here" for TestInitialAddressEventIgnored code07:12
axwjam: the test doesn't make sense anymore. it's not that we *ignore* the initial address event (anymore), but we don't take any action until *both* the initial config AND address are received07:15
axwthe logic did change, the test should have been dropped07:16
axwthere are other tests that check that config-changed doesn't get emitted until the charm URL is set (which triggers input config events)07:16
axwany test that SetAddresses before setting the charm URL is probably broken07:17
jamaxw: so potentially one of the tests but TestConfigEvents is also failing with the same race07:17
axwjam: yeah, that initial SetAddresses in TestConfigEvents should be removed07:18
axwin TestConfigAndAddressEvents, we ought to wait for an event after setting the charm, then trigger a config change and an address change and wait for a coalesced config-changed07:20
dimitern:) morning07:20
axwin TestConfigAndAddressEventsDiscarded, drop the SetAddresses07:20
axwmorning dimitern07:20
axwor wait.. afternoon, if we're saying our own time :)07:21
dimitern:) hah yeah07:21
dimiterndid my controversial fix for the filter tests spawn a discussion?07:22
jamdimitern: morning07:22
jamdimitern: well, I tried to go back to the source of this code and get some input07:22
dimiternjam, yeah, looking at it again this morning I'm not quite sure why the logic around config events has to be so complicated07:23
jamdimitern: my guess is that axw didn't want to disturb existing charms when he updated the events to also trigger when address changes occur07:25
jam*probably* the issue was sending a config-changed event when the IP address changed the first time, but there was no config data yet.07:26
dimiternjam, axw, but why the address change reporting needs to be delayed?07:26
jamdimitern: "no config data yet" is my guess07:26
axwdimitern: config-changed mustn't be generated until there's a charm set. I don't recall why we went with config-changed over address-changed07:27
dimiternaxw, ah! that's the outcome of the address-changed discussion07:28
dimiternnow it starts to make more sense to me07:28
axwdimitern: yeah, I think we went with config-changed because networking wasn't ready yet, and William wanted address-changed to be network-aware07:28
axwso firing additional config-changed was seen as better than nothing, which is what we had before07:29
dimiternaxw, right, and IIRC the idea of having address-changed was abandoned07:29
axwI haven't heard any more of it07:30
axwwe abandoned automatically setting addresses07:30
axwI mean, updating07:30
axwbecause proxy charms07:30
dimiternI see, yeah07:31
dimiternanyway, I'd still like fwereade to have a look at that fix as well07:31
dimiternand ideally support it with at least *a test*07:32
axwdimitern: btw I agree with jam's analysis on the bug. some of the tests look broken to me, since as jam says, they're firing two address changes (the initial watcher response, and then another change due to SetAddresses)07:33
dimiternaxw, that looked really odd to me as well - why fire 2 address changes07:35
dimitern(the inevitable initial one and then setting another one)07:35
dimiternaxw, how about moving the setAddress before starting the filter?07:37
axwdimitern: which test?07:37
dimiternthat would make more sense to simulate a precondition07:37
dimiternaxw, TestConfigEvents I think07:38
axwdimitern: I think that'd be fine, or just drop the first SetAddresses and leave a comment.07:38
axwa comment might be in order either way, really07:38
dimiternaxw, yeah, considering 3 of us can't quite get the intent there for a while :)07:39
TheMuemorning o/08:34
dimiternTheMue, morning08:37
TheMuehmmpf, forgot to publish my review comments yesterday :/08:45
TheMuedimitern: would you mind another look at http://reviews.vapour.ws/r/1106/ ?08:45
dimiternTheMue, sure08:45
dimiternTheMue, in TestUnsetServices I'm not quite sure what you're testing - I can see foo=bar being set but why isn't it included in the next ServiceInfo change?09:00
TheMuedimitern: the first part, the setting, simply has been copied from the according setting test above to see if it works. and then, after destroying  the service, the test failed, because the settings still contained the blog-title09:03
TheMuedimitern: then, after implementing removed(), they are cleared as wanted09:03
TheMuedimitern: so it's simply to have a scenario09:04
dimiternTheMue, ok, that's good, but why is foo=bar that you set missing from the change?09:04
TheMuedimitern: here I would myself have to look how the helper setServiceConfigAttr() works. if it should not also clear this setting, then also one of the old tests above works wrong09:05
dimiternTheMue, yeah - or perhaps foo is an invalid setting for that charm and it's not getting set in the first place; if so then use one of the valid settings instead of foo (outlook IIRC was a valid setting for the testing wordpress charm)09:06
TheMuedimitern: Service.UpdateConfigSettings() seems to be the reason, will take a deeper look09:06
dimiternTheMue, if there is an error getting ignored somewhere, that's bad :)09:07
TheMuedimitern: oh, that may too be the reason. if it's so I'll change the test above too so that it works correctly09:07
dimiternTheMue, cheers09:07
axwwallyworld: I've got a branch adding scope to volume and filesystem tags, and Watch{Environ,Machine}Volumes09:18
axwwill propose soon09:18
axw(as I've run tests)09:18
voidspacedimitern: dooferlad: TheMue: can we postpone / delay standup by ten minutes? I have to take child number 1 to the childminders. (Just round the corner.)09:53
dooferladvoidspace: fine by me09:53
TheMuevoidspace: +109:53
dimiternvoidspace, np09:54
voidspacethanks guys, biab09:54
=== urulama_ is now known as urulama_otp
voidspacedimitern: TheMue: dooferlad: back10:06
TheMuecoming10:06
dimiternvoidspace, dooferlad, TheMue, omw10:08
dimiterndooferlad, I found the issue11:30
dooferladdimitern: cool!11:30
dooferladdimitern: what was it?11:30
dimiterndooferlad, :) well, it's good I added generation of /etc/network/interfaces for the lxc template as well, but I should've removed it on shutdown11:31
dooferladdimitern: ah.11:32
dimiterndooferlad, as the clone starts with the template's /e/n/i11:32
dimiterndooferlad, yeah, I wish I thought of that 2 days ago :/11:32
dooferladdimitern: indeed.11:34
dimitern:) OTOH I learned more about upstart, init, and cloud-init internals in the past week than I would've ever wanted11:37
TheMuedimitern: found it. you can write into your initial settings any stupid stuff you want. as long as the service itself doesn't know them (via Update...) they are removed with the first change event11:40
TheMuedimitern: so here it's no wonder that strange foo settings in the tests above are removed then11:40
dimiternTheMue, oh, that's nasty :) good catch11:40
TheMuedimitern: yeah, will change the service setting tests too11:42
dimiternTheMue, thanks!11:43
dimiterndooferlad, I found possibly the most resilient set of commands for /e/n/i - http://paste.ubuntu.com/10579764/12:13
dimiterndooferlad, "replace" works even when the lxc package pre-populates the routes (which it doesn't do *always*) so it won't fail due to duplicate routes12:14
dimiterndooferlad, and "|| true" ensures no errors if ifup failed to bring the NIC up completely or ip link set dev eth0 up was called separately, without the rest of the commands (like the network-interface-container.conf job "helpfully" does)12:17
* TheMue steps out for a moment12:34
voidspaceoops, before doing <Ctrl-a><del> on the sprint logistics spreadsheet make sure the focus is in the right place12:36
voidspaceI momentarily deleted the entire sheet...12:36
dimiternvoidspace, wow :) good to know12:36
voidspacedimitern: :-)12:37
voidspacedimitern: luckily undo works...12:37
dimiternvoidspace, indeed12:37
dimiternericsnow, wwitzel3, hey guys - are you around by any chance?12:40
dimiterntoo early perhaps12:41
perrito666dimitern: too early12:43
perrito666dimitern: at least for eric I believe is a little before 6a,12:43
dimiternperrito666, yep12:43
dimiternperrito666, I'm thinking of joining your standup today to discuss some ideas around testing and stubs12:45
perrito666dimitern: you are welcome to do so, bring candies though12:45
dimiternperrito666, :) will do12:47
perrito666well There is ongoing organization for the go meetup in my city, it will be a lovely two person lunch13:02
perrito666to all my fellow americans https://www.youtube.com/embed/br0NW9ufUUw13:07
=== kadams54 is now known as kadams54-away
dimiterndooferlad, ok, scratch that - even *more* resilient version is http://paste.ubuntu.com/10580025/13:25
dooferladdimitern: where did the address and netmask go?13:26
dimiterndooferlad, in a pre-up step13:26
dooferladdimitern: ohh, haven't seen that before13:27
dimiterndooferlad, and the type is now manual, not static - ifup is too smart and tries to run ip addr add before running any pre-up scrips13:27
dimiterndooferlad, still works with "ifup -a" or "ifup -a --allow auto" though, which is good13:28
dooferladdimitern: clearly need a pre-pre-up :-)13:28
dimiterndooferlad, *lol*13:28
dimiterndooferlad, I'd appreciate if you can  independently test whether it will also work for kvm containers13:29
dooferladdimitern: soon as I have this test fixed I will try it.13:30
dimiterndooferlad, cheers13:30
mupBug #1430639 changed: juju can't bootstrap in LXC <juju-core:Won't Fix> <https://launchpad.net/bugs/1430639>13:31
mupBug #1430791 was opened: Upgrade-juju is broken on most/all substrates <ci> <regression> <upgrade-juju> <juju-core:Triaged> <https://launchpad.net/bugs/1430791>13:31
dimiterndooferlad, it works like charm! very fast startup now - both the template and all the clones and no errors13:39
dooferladdimitern: sweet!13:39
dimiterndooferlad, now testing on precise13:39
dimiterndooferlad, I had to add a step to pre-render /e/n/i just before the container starts though13:40
=== kadams54-away is now known as kadams54
dimiternsinzui, hey re that upgrade issue13:43
dimiternsinzui, I've seen it happen on trunk since some time now - if you immediately try to call any command after upgrade-juju it always fails with "connection is shutdown", but if you retry after a few seconds it works13:44
dimiternsinzui, I guess that's something to do with updating the apiserver TLS cert like during bootstrap13:45
sinzuidimitern, CI has done that for more than a year13:45
sinzuiThis looks like something new13:45
dimiternsinzui, I think if you change the job to retry a few times when calling status after upgrade-juju successfully completes it will all go green (or mostly)13:46
dimiternsinzui, in a normal case, e.g. if you're not impatient and wait a few seconds before running anything after upgrade-juju, I can confirm it works OK on trunk13:47
dimiternsinzui, but you're also correct that it used to work, now the UX is slightly worse - I guess automated scripts and tools like juju-deployer will be affected unless they do retry commands after upgrade13:49
sinzuidimitern, yeah, abentley  and I are pondering the deployer case13:51
mattyw_katco, ping?14:01
katcomattyw_: hey hey14:01
mattyw_katco, morning - I gave you one minute after 9am so you'd have time to get coffee - so I reckon you owe me a favour ;)14:02
katcomattyw_: LOL14:02
natefinchwwitzel3: standup?14:04
jamkatco: just the person I wanted to ping :) You did LeadershipSettingsAccessor, right?14:04
katcojam: o/ yes i did14:05
wwitzel3natefinch: yep14:05
jamso I'm trying to sort through the various layers to enable the Uniter to call WatchLeadershipSettings14:06
katcojam: ok14:06
jamone concern is that both the APIServer internally and the API Client externally use LeadershipSettingsAccessor (I believe) which has14:06
jamWatchLeadershipSettings(serviceId)14:06
jambut IIRC, we should be passing in a unit.Tag for the api client, shouldn't we?14:06
jamkatco: do you have any actual end-to-end API client tests? Most of the tests I can see are stubbed out14:07
jamso I couldn't tell what you should actually be passing for serviceID14:07
katcojam: hm. this seems to be a point of confusion on the team... i think people have told me that tags are otw format only, client-facing are ids14:07
jambecause the tests are using "foobar"14:07
jamwhich isn't a valid unit ID14:08
jamnor a valid unit tag14:08
katcojam: https://github.com/juju/juju/blob/master/featuretests/leadership_test.go14:08
jamkatco: so names.Tag is an object that is generally used in clients, I believe14:08
jamvs a string14:08
katcojam: we place end-to-end tests in featuretests/ now, hopefully there is something in there that looks promising14:08
katcojam: huh. i swear fwereade and wallyworld were told me that tags were OtW format/internal use only and shouldn't be exposed to clients14:09
jamtag-as-a-string is the wire format, yes14:10
jamnames.Tag is used all over the place14:10
katcothey said this was a mistake we were trying to correct14:10
wallyworldand it should n't be14:10
katcolook at that14:10
wallyworldusers should pass in names / ids14:10
katcoi summoned wallyworld14:10
wallyworldwhich are converted to tags14:10
jamkatco: anyway that particular part I'll just adjust to your api, just trying to find out what to pass.14:11
jambut I realize I'm passing the Unit.Id() which isn't the service id14:11
wallyworldtags just serve to provide additional typing info for an id14:11
katcojam: does this help? https://github.com/juju/juju/blob/master/featuretests/leadership_test.go#L25614:11
jamkatco: those particular tests create a service then a unit, i need to go from a unit to its service id14:12
fwereadejam, katco, wallyworld: indeed, it's really a wire-format thing; and I'm not *especially* bothered about using tags in agent code, because sometimes it really does seems to make more sense; but we absolutely mustn't leak them into the UI14:12
katcojam: looks like this is one path: unit.Service().Tag().Id()14:13
fwereadejam, katco, wallyworld: (I am a *little* bothered by it, because it feels like it's close to being a layering violation, but I'm not really in love with the remote objects we use there either, and they'd be the other obvious source of typing information when we need it)14:14
katcojam: not sure if that's the best approach... especially if you're within a charm14:14
wallyworldour remoting approach kinda sucks14:14
fwereadekatco, wallyworld, jam: names.UnitService()?14:14
wallyworldshould be SOA; attaching remoting layer to domain objects is plain wrong14:15
jamfwereade: well the test suite has a self.service object14:15
katcofwereade: that sounds more correct from within the context of a unit14:15
jamand the uniter also has a self.service14:15
fwereadewallyworld, it does indeed, it's just a layer of unnecessary yuck :)14:15
wallyworldwish we could change it, but needs to evolve; so much legacy14:15
fwereadewallyworld, well, in general, there's no need to double down on the weird remote objects style14:16
dimiterndooferlad, success! both trusty and precise lxc containers work fine14:16
dooferladdimitern: :-D14:17
wallyworldfwereade: so long as new work is done correctly.... :-)14:17
fwereadewallyworld, I'd rather see new code that just talks to an api object directly rather than tack stuff onto the already-bloated remotes14:17
fwereadewallyworld, exactly14:17
wallyworldyup14:17
katcojam: does that help at all? i'd be happy to discuss further to help out with the api14:17
jamkatco: I found the problem I was having, now on to the next problem :)14:17
katcojam: ah the life of a techy ;)14:18
jamhm.. unknown watche id14:18
katcowallyworld: hey, exactly which type of robot are you?14:18
wallyworldkatco: WAL-E :-D14:19
katcoLOL14:19
katcooh my... i was not prepared for that awesome rebuttal14:19
wallyworld:-D14:19
katcowell played, sir.14:19
wallyworldfor once, yes14:19
wallyworldnow i do need sleep, catch you tomorrow14:19
katcocya14:19
jamnight wallyworld14:19
* katco makes more tea14:20
dimiternkatco, hey there14:30
dimiternfwereade, hey, can you spare 10m and have a look at http://reviews.vapour.ws/r/1118/ at some point please?14:32
katcodimitern: hey dimiter!14:32
katcoman my irc notifications don't seem to be working14:32
dimiternkatco, :) oh14:32
katcooh no wait... they were... i was looking at a different screen haha14:32
katcodimitern: how are you!14:33
dimiternkatco, I wanted to ask about goose cinder support and a test double for it14:33
katcodimitern: right, sorry i owe you an update on that14:33
dimiternkatco, good, thanks, how about you?14:33
katcodimitern: doing pretty good! busy busy14:33
dimiternkatco, :) I bet14:33
katcodimitern: so, i talked with wallyworld, and we'd like to log a bug for the test-doubles so we can land the patch and unblock the storage provider i'm working on14:34
dimiternkatco, ok, so no rush on the cinder stuff - just wanted to double check you've seen it14:34
katcodimitern: is that ok with you? i don't think it should be that hard to write14:34
katcodimitern: yeah, sorry i haven't responded =/ we were discussing all the various strategies14:34
dimiternkatco, it's ok, but how are you going to write the tests for the storage provider without test doubles?14:34
katcodimitern: tbh i haven't gotten that far. definitely will have some unit tests, and then i'd have to take a look at the tests axw and wallyworld have already written to see the strategy14:35
katcodimitern: it may be that i'll need to land a test double before i can write those tests. i don't know yet14:35
* perrito666 jumps to systemd14:35
ericsnowdimitern: that testing thread: https://lists.ubuntu.com/archives/juju-dev/2015-February/004164.html14:36
dimiternkatco, ok, it's fine to land it later, but we'll need proper "local live" tests with a test double like we do for other features14:36
dimiternericsnow, great, thanks for saving me some digging! :)14:37
ericsnowdimitern: :)14:37
katcodimitern: understood14:37
katcodimitern: i will file a bug today to ensure it doesn't fall off our radar14:38
dimiternkatco, awesome, thanks!14:38
katcodimitern: ty for the reviews/thoughts!14:38
dimiternkatco, np :)14:39
=== arosales_ is now known as arosales
jamkatco: ok, I do have a question now.14:48
jamkatco: AFAICT WatchLeadershipSettings isn't registering its watcher with the api server14:48
jambecause I can see the API response giving a NotifyWatcherId of 914:48
jamand then I see a call to NotifyWatcher Next 914:48
jamand it comes back with no-such-watcher-id14:49
jamnow, I can see a featuretest for Changes()14:49
jambut it seems to only assert that we get *a* change14:49
jamand the *first* change for watchers is generated client side14:49
jamso it may be that we aren't ever actually trying to watch for a real change14:50
* katco reading14:52
katcogm14:53
katco*hm14:53
katcolet me look at the test again14:53
katcojam: i see what you mean... let me check and see if we ignore that 1st even somewhere in the chain14:54
katcojam: here's the function that instantiates the new watchers: https://github.com/juju/juju/blob/master/api/uniter/uniter.go#L55-L5714:58
katcojam: it looks like it's being passed in a facade, do you know if instantiating a notifywatcher will register it with the api server?14:59
katcojam: either way, i think you're right that the test is probably incorrect14:59
jamkatco: so to go back to the featuretest quickly, if you just comment out the Merge line, does it still pass (I think it should because of initial event stuff)14:59
jamit passes when I try to comment out the Merge line15:00
katcojam: yeah i think the test is probably lying to us15:00
jamI tried a quick "call it twice" but currently trying to make that do what I think it should15:00
katcojam: is it better practice to consume the first event within WatchLeadershipSettings, or within the caller of that?15:01
jamkatco: so code that hangs on a watcher usually needs to do initialization15:03
jamso we expect an initial event15:03
jambecause Watchers always generate an initial event15:03
jamwe don't bother sending it over the wire15:03
katcojam: ah ok. so maybe consume that initial event in WatchLeadershipSettings?15:03
jamand client-side code always generates the event, and server-side code suppresses it15:03
jamkatco: so I got the test to fail like it should. I'll paste the diff15:04
katcocool15:04
ericsnowfwereade: ping15:05
jamkatco: http://paste.ubuntu.com/10580495/15:06
* katco looking15:06
katcojam: so it appears as thought the code is not registering the watcher properly, as you said15:07
jamkatco: https://github.com/juju/juju/blob/master/apiserver/upgrader/unitupgrader.go#L4615:08
jamis how we create a watcher, consume the initial event, and register that watcher with the API server resources15:08
katcojam: well.. is there a race in that test? do we check for changes before the merge can complete?15:08
jamkatco: so the diff that I posted15:09
jamthere will *always* be an initial change, generate client side15:09
jamkatco: https://github.com/juju/juju/blob/master/api/watcher/watcher.go#L16515:10
katcojam: right, but the second pull, L12... is it possible we hit that before L21 will produce an event?15:10
jamkatco: it is a channel15:11
jamif we get there, it will block15:11
jamit is actually a synchronous channel15:11
jamso if *either* side gets there they block until the other side gets there too15:11
jam(though one side should be a select loop, so it can handle shutdown, etc)15:11
katcodoh sorry, i mistook the ok for an indication of successfully retrieving from the channel15:11
jamtechnically, we should be doing something more like NotifyAsserter which will timeout rather than hang the test suite15:12
jamkatco: you get ok = false if the channel is *closed*15:12
jamkatco: in which case you do still get a return from the channel15:12
jambut the ok says "you're getting returned because the channel was closed, not because I had data for you"15:12
katcogotcha, thanks for the extanalpion15:12
katcowow15:12
katcoi butchered that word15:12
katcoexplanation15:12
jamkatco: funny, I read it just fine15:13
katcojam: have you ever seen that study that showed people read english words fine as long as the first and last letters are correct?15:13
jamyeah, multiple times15:13
jamand it gets worse and worse by the end15:13
katcowow... i just noticed i perfectly reversed the middle of that word15:14
katcoweird15:14
katcoanyway15:14
katcoso it looks like there's a bit of work to patch this stuff up15:14
katcoi'm working on storage stuff for tanzanite, but i realize leadership is due and important as well15:14
jamkatco: so you can use NotifyAsserter when working with channels like this. It does stuff like assert "there is exactly 1 message on the channel right now"15:15
katcoah cool15:15
katcowhich package is that in?15:15
jamkatco: I'm about 2 hours past EOD, I *might* come back and look into this tonight, but if you can get to it by my morning I'll be happy to review a patch15:15
jamkatco: github.com/juju/juju/testing IIRC15:15
katcojam: ok i'll pop it onto my queue and should have a patch by your tomorrow15:16
katcopop it onto my stack rather :p15:16
katcolifo ;)15:16
jamkatco: https://github.com/juju/juju/blob/master/worker/uniter/filter/filter_test.go is using NotifyAsserterC15:16
jamif you want examples15:16
katcoperfect ty15:16
katcojam: ty for troubleshooting that, and sorry for the issues15:17
jamit does make tests a bit slower sometimes, because if you want to assert that there isn't any data pending on a channel, it waits 50ms to ensure that there really isn't anything there.15:17
jamkatco: np. I'm glad I could track it down. I don't quite know whether my code is correct or not yet :)15:17
katcojam: haha isn't that a fun time in a change? :)15:18
jamfwereade: if you're around git@github.com:jameinel/juju leader-settings-tests is the work in progress15:18
jamI think it is close to correct, caveat it crashes the Uniter because of the LeadershipSettingsWatcher bug15:18
mupBug #1430839 was opened: juju-run and juju-backup break 'juju help plugins' <juju-core:New> <https://launchpad.net/bugs/1430839>15:19
jamok, EOD, have a good one15:20
katcojam: tc!15:22
dimiterndooferlad, I did find an issue with lxc-clone: true and lxc-clone-aufs: true - the rootfs is no longer a dir in the clone, but a snapshot (<lxc-dir>/name/delta0)15:35
=== meetingology` is now known as meetingology
mattyw_ericsnow, are you still the reviewboard go to fella?16:15
ericsnowmattyw_: depends on your question :)16:15
mattyw_ericsnow, a review not being picked up https://github.com/juju/juju/pull/180216:15
mattyw_ericsnow, and shall I just do it by hand?16:16
perrito666this might be nice/useful for those of you who still hate git https://github.com/arialdomartini/oh-my-git16:22
katcoTheMue: ping16:35
ericsnowmattyw_: yeah, do it by hand (rbt post)16:36
mattyw_ericsnow, will do thanks16:36
katcoperrito666: i am the least helpful person on git commands because i use this exclusively: https://www.youtube.com/watch?v=zobx3T7hGNA16:37
ericsnowmattyw_: yeah, sorry, it's a consequence of switching host and having to upgrade reviewboard and reviewboard changing APIs between micro versions :(16:37
ericsnowmattyw_: actually don't do it by hand16:38
ericsnowmattyw_: I forced GH to redeliver the PR event and it worked this time16:38
mattyw_ericsnow, awesome, thank you very much16:39
TheMuekatco: pong16:41
katcoTheMue: hey i saw you had a topic scheduled for Nuremberg re: timeboxed iterations16:42
katcoTheMue: would you mind if i piggy-back on that to discuss timeboxed estimation? or should that be a separate topic?16:42
TheMuekatco: yep, would like to talk about it based on my experiences16:42
TheMuekatco: no, would appreciate it. estimation is beside reqeng one of my favorite topics16:43
katcoTheMue: awesome! :)16:43
katcoTheMue: i'll just /katco on the owners then?16:44
TheMuekatco: yes, and we both then could prepare for it during the next weeks16:45
mattyw_katco, TheMue if I'm available for that session I'll be in it16:45
katcoTheMue: sounds good. tyvm for bringing that up16:45
TheMuemattyw_: +116:45
TheMuekatco: yw16:45
mupBug #1430898 was opened: run-unit-tests ppc64el timeout <ci> <gccgo> <ppc64el> <regression> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1430898>16:49
voidspaceTheMue: ping17:02
mgzmattyw_: have you got a mo to do second review on reviews.vapour.ws/r/1119 please? :)17:04
voidspacedimitern: ok, so the test is failing because the allocated addresses aren't being found17:12
voidspacedimitern: so a genuine failure...17:12
voidspace:-)17:12
mattyw_mgz, will do17:13
dimiternvoidspace, hmm.. that's odd - are they allocated to the container?17:13
voidspaceyep17:13
voidspaceI'm calling AllocateTo17:13
voidspaceI haven't yet tested the new state code that fetches them though17:13
voidspacedimitern: probably time to write a test for that code...17:13
voidspace:-)17:13
dimiternvoidspace, ah, that might be it :)17:13
voidspacethis is why we tset17:14
voidspace*test even17:14
voidspacedimitern: and indeed a simple test for the new state method fails to return any addresses17:24
voidspaceoops17:24
dimiternvoidspace, that's odd, might be related to the multi-env changes in state (all PKs have "<envUUID>:" prepended)17:25
voidspacedebugging17:25
dimiternvoidspace, what's you query?17:25
voidspaceit's probably me17:25
voidspacedimitern:         iter := addresses.Find(bson.D{{"machineid", machineId}}).Iter()17:26
voidspacepretty simple17:26
dimiternvoidspace, right.. let me have a look in state17:26
voidspacedimitern: no, I'm being dumb17:27
voidspacedimitern: I think I'm fetching the wrong field for the address17:27
voidspaceshould be Value and I'm using Address17:27
voidspacedimitern: pretty sure that's it17:28
voidspacerunning the test now17:28
dimiternvoidspace, ah, ok - there's not even an address there17:28
voidspaceyeah :-D17:29
voidspacestate test passes, now trying apiserver/provisioner17:30
voidspacedimitern: yay - fails for the right reason now - wrong error message17:31
dimiternvoidspace, good! :)17:32
voidspaceawesome17:32
voidspacethe rest should be plain sailing ;-)17:32
dimiternalways nice to hear :)17:32
mattyw_mgz, done17:38
bodie_coreycb, you around?17:39
mgzmattyw_: thanks!17:41
gsamfira1mattyw_: Thank you :)17:47
sinzuinatefinch, dimitern katco: do either of you have a few minutes to review this momentous branch http://reviews.vapour.ws/r/1132/17:48
mattyw_sinzui, I could do technically, but I'm not graduated so it wouldn't help :)17:49
dimiternsinzui, sure, looking17:49
dimiternsinzui, done17:50
sinzuithank you dimitern17:50
natefinchwwitzel3: how goes?18:00
perrito666bbl18:14
mupBug #1430943 was opened: Subordinate charms are not displayed in `juju status`. <juju-core:New> <https://launchpad.net/bugs/1430943>18:20
lazyPowerWhen are we going to land enablement on cloud providers of windows images? We have the series available, but cannot do anything with it outside of a MAAS host - just curious if this work is scheduled for this cycle or an upcoming.18:33
voidspacebiab18:41
=== kadams54 is now known as kadams54-away
=== kadams54-away is now known as kadams54
natefinchlazyPower:  re: windows, we have steps to enable it on a private openstack.  For public clouds, it's tricky, because we need special windows images... I don't know what the status of getting our windows images available on the various clouds is. alexisb might have an idea.19:22
lazyPowernatefinch: i figured that was the clincher, needing to get the baked in cloudebase-cloudinit service on the core windows image with sysprep19:22
lazyPoweras we dont support custom AMI's or anything - its kind of a non starter atm right?19:23
natefinchlazyPower: custom AMI's is something we've been talking about... we should bring it up in nuremberg, that might fix a lot of our problems (and a lot of people want custom AMI's anyway)19:23
lazyPowernatefinch: we're going to have fun supporting that one :)19:24
lazyPowermay the games begin!19:24
perrito666can anyone with superpower rubberstamp this? http://reviews.vapour.ws/r/1088/19:29
natefinchlazyPower: gah you broke ctrl+mousewheel zooming on your site19:37
lazyPoweri did?19:37
lazyPowerhah, i did19:37
lazyPowerOLE!19:37
natefinchperrito666: reviewing19:38
natefinchperrito666: oops, or not: There was an error displaying this diff.19:38
perrito666natefinch: well, my day has been on those lines so far19:39
natefinchlazyPower: some custom scrolling javascript stuff, huh?  I can tell the page scrolls differently than normal, though I couldn't explain how19:39
perrito666today I had to drive at the worse hour to the exact center of the village just because two peoeple could not properly pass an envelope from one to the other so I could get it in another moment19:40
lazyPowernatefinch: source is here :) https://github.com/chuckbutler/pelican-porto19:41
natefinchlazyPower: betcha it's the scroll to top thingy.... but my knowledge of javascript doesn't really go much beyond the basics.19:45
natefinchlazyPower: which is to say: I'm better at filing bugs than fixing them ;)19:46
natefinchperrito666: I think you can just do rbt post -u from the correct branch locally, and it'll update...19:47
perrito666natefinch: that is not my proposal, actually19:50
natefinchperrito666: oh, you're right.  sorry, wasn't paying attention19:51
natefinchman, I hate testing with local provider... I never know if the problem I'm having is because local is a special kind of stupid, or if it's a general problem20:01
beisnerlol natefinch20:04
natefinchI guess that should be "a stupid kind of special"  but both are mostly correct20:05
=== kadams54 is now known as kadams54-away
=== kadams54-away is now known as kadams54
perrito666ericsnow: do you know why https://github.com/juju/juju/pull/1805 is not being reviewboardized_20:39
ericsnowperrito666: non-ascii characters20:39
ericsnowperrito666: it's a bug in the GH hook20:40
ericsnowperrito666: try using rbt post20:40
perrito666I think I missed the chars20:40
perrito666do you see them?20:40
ericsnowperrito666: nope20:41
ericsnowperrito666: could be in the metadata GH sends20:41
perrito666ah20:41
perrito666mmm and now rbt cannot reach reviewboard20:41
perrito666... what a day20:41
=== kadams54 is now known as kadams54-away
=== kadams54-away is now known as kadams54
katcowho is familiar with watchers?20:57
ericsnowkatco: like with who watches them or something21:05
ericsnowkatco: <wink>21:05
katcoericsnow: the comedian.21:05
katcoericsnow: https://www.youtube.com/watch?v=YhJMAaix0CA21:05
thumpersinzui: re bug 1430898, it may be related to bug 1430791.21:19
mupBug #1430898: run-unit-tests ppc64el timeout <ci> <gccgo> <ppc64el> <regression> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1430898>21:19
mupBug #1430791: Upgrade-juju is broken on most/all substrates <ci> <regression> <upgrade-juju> <juju-core:In Progress by waigani> <https://launchpad.net/bugs/1430791>21:19
thumpersinzui: waigani is working on the latter, and once it lands, I may get you to see if it fixes the former21:19
sinzuithumper, That would be nice21:19
waiganithumper, sinzui: http://reviews.vapour.ws/r/1133/21:24
thumperwaigani: you have checked in state/main21:25
thumperwaigani: can you reset on master and remove it plz?21:25
waiganithumper: ugh, sorry21:26
waiganithumper: gone21:28
thumperwaigani: +1, make sure you use the "fixes-1430791" bit in the merge instruction21:30
=== kadams54 is now known as kadams54-away
perrito666anyone knows how to tell rbt to ignore ssl certs?22:25
jw4ericsnow, did you get your watcher questions answered?22:26
ericsnowjw4: :)22:26
jw4:)22:27
jw4Looks like it was katco with the original question too22:27
katcojw4: i've got a crack team of wallyworld looking into it :)22:27
jw4katco, awesome - :)22:28
katcojw4: we are just seeing some odd test behavior. we register a watcher on the apiserver, but then it seems it can't find it.22:28
jw4hmm interesting22:28
perrito666ericsnow: know how to ask rbt to ignore ssl cert?22:28
perrito666mm and now rbt asks me for a user22:45
ericsnowperrito666: https://github.com/juju/juju/blob/master/doc/contributions/reviewboard.md#rbt-authentication22:45
perrito666ericsnow: I have read the code, it seems there is no way, in my version to bypas ssl22:46
perrito666but anyway I would like this to work as is :p22:46
ericsnowperrito666: why do you want to bypass SSL?22:47
ericsnowperrito666: (I don't know if there's a way)22:47
perrito666ericsnow: I an getting an error in my vivid install but Ill recreate the virtualenv just in case22:47
ericsnowperrito666: the phrase *bleeding* edge comes to mind :)22:49
menn0thumper or davecheney: jujud test fixes http://reviews.vapour.ws/r/1137/  (noticed while looking at an unrelated panic)23:25

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