[00:17] davecheney: 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. === arosales_ is now known as arosales [00:24] menn0: ok, thanks, that was my question [00:31] davecheney: 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] probably alignment [00:31] davecheney: it's not enough of a difference to worry me but it's interesting anyway [00:31] it won't matter in real life [00:31] * menn0 agrees [00:32] your dedication to performance is admirable [00:32] but i think it will be lost in the niose [00:32] juju talks over the network [00:32] that's what takes all the time [00:32] axw: can we chat when you get back? [00:34] davecheney: sure. i'm going to use the zero value. [00:48] davecheney, thumper, waigani_: updated the Deque PR. http://reviews.vapour.ws/r/1122/diff/2-3/ [00:55] menn0: I've given it a shipit, but it would be good manners to wait for davecheney and waigani_ :-) [00:55] I've got this cache file configstore thing done now (I think) [00:55] running full test suite [00:59] menn0: shipit from me [01:01] how do we know if the bot is hooked up for merging? [01:02] * thumper is thinking of juju/utils [01:03] thumper: i'm guessing it isn't b/c merges are being done by real people instead of jujubot [01:04] hmm [01:04] I'm having a WTF moment [01:04] anyone else getting errors running tests in service/upstart? [01:05] I get five failures [01:05] all the same place [01:05] upstart_test.go:240: [01:05] c.Assert(err, gc.ErrorMatches, ".*exit status 99.*") [01:05] ... error string = "exit status 2" [01:05] ... regex string = ".*exit status 99.*" [01:14] I wasn't a couple hours ago - I'll pull and test again [01:15] I get this test failure on master [01:15] nope, no new changes since I last ran a clean test run [01:15] both when I rull all the tests, and just the tests for that package [01:15] wtf? [01:15] I'm on trusty [01:15] me too [01:18] davecheney: re "state", I sympathise, but I can't think of a better name. any suggestions for an alternative that describes persistent local state? [01:19] axw: got time for a quick hangout? [01:19] interestingly TestStart does the same sort of thing, but it passes [01:19] wallyworld: sure, see you in tanzanite? [01:19] * thumper tries to work out what is different [01:19] ok [01:31] thumper, I'm curious what extra logging around line 168 of service/upstart/upstart.go would reveal? [01:31] 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:32] jw4: line 168? [01:32] TestRemoveStopped passes [01:33] in the Running() method? [01:34] sorry, thought you meant the _test file [01:34] i.e. it feels like some system level failure is happening (with a return code of 2) before the expected failure with return code of 99 [01:35] wallyworld: are you shippitting http://reviews.vapour.ws/r/1112/ ? [01:35] ah bollocks, yes sorry [01:36] done === kadams54 is now known as kadams54-away [01:38] What the actual fuck [01:38] damn screwed test isolation problem [01:38] if I run the test by itself, it passes [01:39] but the whole suite, it fails [01:39] :( [01:39] * thumper pokes [01:40] although to be perfectly honest... when it comes to debugging.... words like WT(A)F are actually beautiful. The programmers inverse of Eureka! [01:41] hah [01:41] [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] that occurs before each failure [01:41] weird... seems like that should at minimum be logged at WARN? [01:42] that is the logging line I added [01:42] what is all that colorisation shit in there ? [01:42] but the question is 'why' ? [01:42] davecheney: I believe so [01:42] it looks like something is calling out to my shell [01:42] and I must have some weird executable in the path [01:43] and it's sourcing your .profile [01:43] so who knows what kind of stuff is in the environment [01:43] blech [01:44] something is so wrong with this [01:44] the Running method swallows this error very easily [01:46] ha [01:46] haha [01:46] fucker [01:46] * thumper has /home/tim/bin/status [01:46] which has colour output [01:46] * thumper fixes [01:47] the test fakes out the start/stop/status generally [01:47] but upstart service Start calls Running, which calls the status command [01:47] which is getting fooled by my command [01:47] * thumper thinks [01:47] why is it getting my path? [01:48] I thought our IsolationTestSuite fixed that [01:48] hahha [01:50] uh ha [01:50] testing.BaseSuite doesn't use the IsolationSuite [01:50] it looks like someone looked at it [01:50] but didn't make the change because too many tests failed [01:50] as they relied on things in the path [01:51] thumper: rename the suite [01:51] SortOfIsolationSuite [01:51] davecheney: the problem is that we aren't using the IsolationSuite [01:51] // SortOfIsolationSuite tries to isolate your tests, sort of, it works, mostly. [01:51] and I think we should be [01:52] wallyworld: you're still going to disable the storage provisioners in this branch, right? [01:52] axw: i disabled the workers [01:53] wallyworld: cmd/jujud/agent/machine.go is still starting those workers... [01:53] * thumper tries to think about how best to fix this [01:53] axw: hmmm, ok, let me check - i modified the tests to ensure the workers weren't running [01:54] oh I see, there's a bool [01:54] wallyworld: don't disable *all* of the storage workers - the others work fine [01:54] axw: yeah, i just saw that [01:54] doh [01:54] will fix === kadams54-away is now known as kadams54 [02:03] https://github.com/juju/juju/pull/1794 [02:16] davecheney, menn0: trivial one http://reviews.vapour.ws/r/1123/ [02:17] thumper: will take a look once i've finished looking at Jesse's branch [02:27] waigani_: just looking at the lowercase user name branch [02:27] waigani_: is it supposed to be included in 1.23? [02:29] davecheney: are you happy for http://reviews.vapour.ws/r/1122/ to be merged now? [02:30] menn0: one final round of comments [02:31] waigani_: just finished reviewing your username case PR. I think I see a bug but otherwise good. [02:32] menn0: lgtm [02:32] davecheney: kk. I don't see them yet. Are you still working on them? [02:32] was going to argue about type blockT []interface{} [02:32] but i couldn't be bothered [02:33] it's too hot for pedantry today [02:33] davecheney: :) [02:34] 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] here is another: http://reviews.vapour.ws/r/1124/diff/# [02:34] no type called State in there :) [02:34] coffee time [02:34] waigani_: fair enough. if that works then great (even if it's a little confusing). the test would be good. [02:35] menn0: actually, I was wondering about the blockT also, if it also caught davecheney's eye - maybe at least a comment? [02:35] menn0: what does the "T" stand for? [02:36] waigani_: Type [02:36] waigani_: I wanted to use "block" for variable names [02:36] right [02:37] if we're going to have the discussion [02:37] lets not [02:37] +1 [02:37] gofmt -r 'blockT -> []interface{}' -w *.go [02:37] done [02:37] -0 [02:37] :) [02:38] menn0: yellow card, only two's compliment allowed in this channel [02:38] ha [03:08] axw: free for another chat? [03:09] wallyworld: just a minute please, propsing a branch [03:09] sure, np [03:09] /o\ fyi 1.22beta6 does not appear to resolve bug 1430049 [03:09] Bug #1430049: unit "ceph/0" is not assigned to a machine when deploying with juju 1.22-beta5

[03:10] -_________________- [03:11] beisner: ah, from status it's still running beta5 [03:11] and... \o/ fyi 1.22beta6 DOES appear to resolve bug 1420049 ! [03:11] Bug #1420049: ppc64el - jujud: Syntax error: word unexpected (expecting ")")

[03:12] wallyworld: see you in tanzanite again [03:12] beisner: yay, thanks for verifying [03:12] axw: sure [03:16] axw, 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:17] beisner: not following, deployer runs on bootstrapped env so you just need --upload-tools at bootstrap? [03:18] beisner: juju-quickstart takes --upload-tools flag? [03:18] * rick_h_ rereads bug [03:18] just reproducing the same way we caught it (bootstrapping via juju-deployer -B) [03:18] beisner: ah, didn't notice that in the bug anywhere [03:19] rick_h_, it's entirely possible that the problem exists with a bootstrep / then deploy. but i'm not sure atm. [03:19] beisner: understood [03:20] rick_h_, easy enough to test though, so i'll kick off a modified loop with beta6 and --upload-tools. [03:22] rick_h_, will check back in tomorrow on that. [03:22] beisner: np, just hoped to save you some stream waiting [03:23] rick_h_, yep, appreciate that. [03:27] and she's off! [03:32] thanks for the fix(es)! checking out. [03:41] Bug #1282642 changed: Bootstrap prefers .jenv over environments.yaml

[03:47] thumper: review done. there's lots of comments but they're already pretty minor [03:47] menn0: thanks [03:48] waigani_: ship it with some minor comments for http://reviews.vapour.ws/r/1115/ [03:50] axw: about to lose power here for 20 mins fyi [03:51] wallyworld: okey dokey [03:55] menn0: thanks === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 === kadams54 is now known as kadams54-away [05:54] Bug #1430639 was opened: juju can't bootstrap in LXC [05:54] wallyworld_: hey, no worries, see you next week [05:54] urulama: ty, about to go :-) [06:24] axw: are you around? [06:24] you're the one that implemented getting "config-changed" events when the IP address changes, right? [06:25] jam: hey, yes [06:25] I'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] looking [06:28] axw: I *just* published my comments on it [06:28] I'm testing that it fixes the test I saw failing now [06:30] jam: I'll keep thinking, but I had the same thoughts as you initially: we should only reset seenConfigChanged/configChanges when the charm changes [06:31] axw: do you agree that it might suppress address events ? [06:32] jam: yes [06:33] axw: have you seen the intermittent failures? [06:33] I suppose we're missing a test then [06:33] no, I haven't [06:36] jam: that test could indeed trigger the address watcher twice: once for the initial, another time for the update [06:41] axw: I'm pretty sure that test was written when we didn't have an initial, thoughts on how to clean it up? [06:58] jam: only thing I can think to do is to take out the SetAddresses, or move it to before the filter is started [06:58] same effect either way, but the latter might be easier to understand [06:58] axw: well the whole test is that we don't generate an event until we SetAddress, right? [06:58] "TestIgnoreInitialAddressEvent" [06:58] jam: sorry, I was thinking of TestConfigAndAddressEventsDiscarded [06:59] * axw runs a test [07:01] jam: 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 emitted [07:08] jam: eh, except that an address change *always* gets generated. its watcher is started immediately, regardless of whether there's a charm URL [07:10] jam: 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.go [07:10] axw: so *something* should be testing the seenConfigChange sort of tests [07:11] if we remove the tests, then we should be removing the "maybeConfigEvent" code [07:12] axw: 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 code [07:15] jam: 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 received [07:16] the logic did change, the test should have been dropped [07:16] there are other tests that check that config-changed doesn't get emitted until the charm URL is set (which triggers input config events) [07:17] any test that SetAddresses before setting the charm URL is probably broken [07:17] axw: so potentially one of the tests but TestConfigEvents is also failing with the same race [07:18] jam: yeah, that initial SetAddresses in TestConfigEvents should be removed [07:20] in 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-changed [07:20] :) morning [07:20] in TestConfigAndAddressEventsDiscarded, drop the SetAddresses [07:20] morning dimitern [07:21] or wait.. afternoon, if we're saying our own time :) [07:21] :) hah yeah [07:22] did my controversial fix for the filter tests spawn a discussion? [07:22] dimitern: morning [07:22] dimitern: well, I tried to go back to the source of this code and get some input [07:23] jam, yeah, looking at it again this morning I'm not quite sure why the logic around config events has to be so complicated [07:25] dimitern: my guess is that axw didn't want to disturb existing charms when he updated the events to also trigger when address changes occur [07:26] *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] jam, axw, but why the address change reporting needs to be delayed? [07:26] dimitern: "no config data yet" is my guess [07:27] dimitern: config-changed mustn't be generated until there's a charm set. I don't recall why we went with config-changed over address-changed [07:28] axw, ah! that's the outcome of the address-changed discussion [07:28] now it starts to make more sense to me [07:28] dimitern: yeah, I think we went with config-changed because networking wasn't ready yet, and William wanted address-changed to be network-aware [07:29] so firing additional config-changed was seen as better than nothing, which is what we had before [07:29] axw, right, and IIRC the idea of having address-changed was abandoned [07:30] I haven't heard any more of it [07:30] we abandoned automatically setting addresses [07:30] I mean, updating [07:30] because proxy charms [07:31] I see, yeah [07:31] anyway, I'd still like fwereade to have a look at that fix as well [07:32] and ideally support it with at least *a test* [07:33] dimitern: 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:35] axw, that looked really odd to me as well - why fire 2 address changes [07:35] (the inevitable initial one and then setting another one) [07:37] axw, how about moving the setAddress before starting the filter? [07:37] dimitern: which test? [07:37] that would make more sense to simulate a precondition [07:38] axw, TestConfigEvents I think [07:38] dimitern: I think that'd be fine, or just drop the first SetAddresses and leave a comment. [07:38] a comment might be in order either way, really [07:39] axw, yeah, considering 3 of us can't quite get the intent there for a while :) [08:34] morning o/ [08:37] TheMue, morning [08:45] hmmpf, forgot to publish my review comments yesterday :/ [08:45] dimitern: would you mind another look at http://reviews.vapour.ws/r/1106/ ? [08:45] TheMue, sure [09:00] TheMue, 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:03] dimitern: 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-title [09:03] dimitern: then, after implementing removed(), they are cleared as wanted [09:04] dimitern: so it's simply to have a scenario [09:04] TheMue, ok, that's good, but why is foo=bar that you set missing from the change? [09:05] dimitern: 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 wrong [09:06] TheMue, 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] dimitern: Service.UpdateConfigSettings() seems to be the reason, will take a deeper look [09:07] TheMue, if there is an error getting ignored somewhere, that's bad :) [09:07] dimitern: oh, that may too be the reason. if it's so I'll change the test above too so that it works correctly [09:07] TheMue, cheers [09:18] wallyworld: I've got a branch adding scope to volume and filesystem tags, and Watch{Environ,Machine}Volumes [09:18] will propose soon [09:18] (as I've run tests) [09:53] dimitern: 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] voidspace: fine by me [09:53] voidspace: +1 [09:54] voidspace, np [09:54] thanks guys, biab === urulama_ is now known as urulama_otp [10:06] dimitern: TheMue: dooferlad: back [10:06] coming [10:08] voidspace, dooferlad, TheMue, omw [11:30] dooferlad, I found the issue [11:30] dimitern: cool! [11:30] dimitern: what was it? [11:31] dooferlad, :) well, it's good I added generation of /etc/network/interfaces for the lxc template as well, but I should've removed it on shutdown [11:32] dimitern: ah. [11:32] dooferlad, as the clone starts with the template's /e/n/i [11:32] dooferlad, yeah, I wish I thought of that 2 days ago :/ [11:34] dimitern: indeed. [11:37] :) OTOH I learned more about upstart, init, and cloud-init internals in the past week than I would've ever wanted [11:40] dimitern: 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 event [11:40] dimitern: so here it's no wonder that strange foo settings in the tests above are removed then [11:40] TheMue, oh, that's nasty :) good catch [11:42] dimitern: yeah, will change the service setting tests too [11:43] TheMue, thanks! [12:13] dooferlad, I found possibly the most resilient set of commands for /e/n/i - http://paste.ubuntu.com/10579764/ [12:14] dooferlad, "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 routes [12:17] dooferlad, 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:34] * TheMue steps out for a moment [12:36] oops, before doing on the sprint logistics spreadsheet make sure the focus is in the right place [12:36] I momentarily deleted the entire sheet... [12:36] voidspace, wow :) good to know [12:37] dimitern: :-) [12:37] dimitern: luckily undo works... [12:37] voidspace, indeed [12:40] ericsnow, wwitzel3, hey guys - are you around by any chance? [12:41] too early perhaps [12:43] dimitern: too early [12:43] dimitern: at least for eric I believe is a little before 6a, [12:43] perrito666, yep [12:45] perrito666, I'm thinking of joining your standup today to discuss some ideas around testing and stubs [12:45] dimitern: you are welcome to do so, bring candies though [12:47] perrito666, :) will do [13:02] well There is ongoing organization for the go meetup in my city, it will be a lovely two person lunch [13:07] to all my fellow americans https://www.youtube.com/embed/br0NW9ufUUw === kadams54 is now known as kadams54-away [13:25] dooferlad, ok, scratch that - even *more* resilient version is http://paste.ubuntu.com/10580025/ [13:26] dimitern: where did the address and netmask go? [13:26] dooferlad, in a pre-up step [13:27] dimitern: ohh, haven't seen that before [13:27] dooferlad, and the type is now manual, not static - ifup is too smart and tries to run ip addr add before running any pre-up scrips [13:28] dooferlad, still works with "ifup -a" or "ifup -a --allow auto" though, which is good [13:28] dimitern: clearly need a pre-pre-up :-) [13:28] dooferlad, *lol* [13:29] dooferlad, I'd appreciate if you can independently test whether it will also work for kvm containers [13:30] dimitern: soon as I have this test fixed I will try it. [13:30] dooferlad, cheers [13:31] Bug #1430639 changed: juju can't bootstrap in LXC [13:31] Bug #1430791 was opened: Upgrade-juju is broken on most/all substrates [13:39] dooferlad, it works like charm! very fast startup now - both the template and all the clones and no errors [13:39] dimitern: sweet! [13:39] dooferlad, now testing on precise [13:40] dooferlad, I had to add a step to pre-render /e/n/i just before the container starts though === kadams54-away is now known as kadams54 [13:43] sinzui, hey re that upgrade issue [13:44] sinzui, 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 works [13:45] sinzui, I guess that's something to do with updating the apiserver TLS cert like during bootstrap [13:45] dimitern, CI has done that for more than a year [13:45] This looks like something new [13:46] sinzui, 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:47] sinzui, 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 trunk [13:49] sinzui, 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 upgrade [13:51] dimitern, yeah, abentley and I are pondering the deployer case [14:01] katco, ping? [14:01] mattyw_: hey hey [14:02] 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] mattyw_: LOL [14:04] wwitzel3: standup? [14:04] katco: just the person I wanted to ping :) You did LeadershipSettingsAccessor, right? [14:05] jam: o/ yes i did [14:05] natefinch: yep [14:06] so I'm trying to sort through the various layers to enable the Uniter to call WatchLeadershipSettings [14:06] jam: ok [14:06] one concern is that both the APIServer internally and the API Client externally use LeadershipSettingsAccessor (I believe) which has [14:06] WatchLeadershipSettings(serviceId) [14:06] but IIRC, we should be passing in a unit.Tag for the api client, shouldn't we? [14:07] katco: do you have any actual end-to-end API client tests? Most of the tests I can see are stubbed out [14:07] so I couldn't tell what you should actually be passing for serviceID [14:07] jam: 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 ids [14:07] because the tests are using "foobar" [14:08] which isn't a valid unit ID [14:08] nor a valid unit tag [14:08] jam: https://github.com/juju/juju/blob/master/featuretests/leadership_test.go [14:08] katco: so names.Tag is an object that is generally used in clients, I believe [14:08] vs a string [14:08] jam: we place end-to-end tests in featuretests/ now, hopefully there is something in there that looks promising [14:09] jam: huh. i swear fwereade and wallyworld were told me that tags were OtW format/internal use only and shouldn't be exposed to clients [14:10] tag-as-a-string is the wire format, yes [14:10] names.Tag is used all over the place [14:10] they said this was a mistake we were trying to correct [14:10] and it should n't be [14:10] look at that [14:10] users should pass in names / ids [14:10] i summoned wallyworld [14:10] which are converted to tags [14:11] katco: anyway that particular part I'll just adjust to your api, just trying to find out what to pass. [14:11] but I realize I'm passing the Unit.Id() which isn't the service id [14:11] tags just serve to provide additional typing info for an id [14:11] jam: does this help? https://github.com/juju/juju/blob/master/featuretests/leadership_test.go#L256 [14:12] katco: those particular tests create a service then a unit, i need to go from a unit to its service id [14:12] jam, 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 UI [14:13] jam: looks like this is one path: unit.Service().Tag().Id() [14:14] jam, 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] jam: not sure if that's the best approach... especially if you're within a charm [14:14] our remoting approach kinda sucks [14:14] katco, wallyworld, jam: names.UnitService()? [14:15] should be SOA; attaching remoting layer to domain objects is plain wrong [14:15] fwereade: well the test suite has a self.service object [14:15] fwereade: that sounds more correct from within the context of a unit [14:15] and the uniter also has a self.service [14:15] wallyworld, it does indeed, it's just a layer of unnecessary yuck :) [14:15] wish we could change it, but needs to evolve; so much legacy [14:16] wallyworld, well, in general, there's no need to double down on the weird remote objects style [14:16] dooferlad, success! both trusty and precise lxc containers work fine [14:17] dimitern: :-D [14:17] fwereade: so long as new work is done correctly.... :-) [14:17] wallyworld, I'd rather see new code that just talks to an api object directly rather than tack stuff onto the already-bloated remotes [14:17] wallyworld, exactly [14:17] yup [14:17] jam: does that help at all? i'd be happy to discuss further to help out with the api [14:17] katco: I found the problem I was having, now on to the next problem :) [14:18] jam: ah the life of a techy ;) [14:18] hm.. unknown watche id [14:18] wallyworld: hey, exactly which type of robot are you? [14:19] katco: WAL-E :-D [14:19] LOL [14:19] oh my... i was not prepared for that awesome rebuttal [14:19] :-D [14:19] well played, sir. [14:19] for once, yes [14:19] now i do need sleep, catch you tomorrow [14:19] cya [14:19] night wallyworld [14:20] * katco makes more tea [14:30] katco, hey there [14:32] fwereade, hey, can you spare 10m and have a look at http://reviews.vapour.ws/r/1118/ at some point please? [14:32] dimitern: hey dimiter! [14:32] man my irc notifications don't seem to be working [14:32] katco, :) oh [14:32] oh no wait... they were... i was looking at a different screen haha [14:33] dimitern: how are you! [14:33] katco, I wanted to ask about goose cinder support and a test double for it [14:33] dimitern: right, sorry i owe you an update on that [14:33] katco, good, thanks, how about you? [14:33] dimitern: doing pretty good! busy busy [14:33] katco, :) I bet [14:34] dimitern: 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 on [14:34] katco, ok, so no rush on the cinder stuff - just wanted to double check you've seen it [14:34] dimitern: is that ok with you? i don't think it should be that hard to write [14:34] dimitern: yeah, sorry i haven't responded =/ we were discussing all the various strategies [14:34] katco, it's ok, but how are you going to write the tests for the storage provider without test doubles? [14:35] dimitern: 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 strategy [14:35] dimitern: it may be that i'll need to land a test double before i can write those tests. i don't know yet [14:35] * perrito666 jumps to systemd [14:36] dimitern: that testing thread: https://lists.ubuntu.com/archives/juju-dev/2015-February/004164.html [14:36] katco, 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 features [14:37] ericsnow, great, thanks for saving me some digging! :) [14:37] dimitern: :) [14:37] dimitern: understood [14:38] dimitern: i will file a bug today to ensure it doesn't fall off our radar [14:38] katco, awesome, thanks! [14:38] dimitern: ty for the reviews/thoughts! [14:39] katco, np :) === arosales_ is now known as arosales [14:48] katco: ok, I do have a question now. [14:48] katco: AFAICT WatchLeadershipSettings isn't registering its watcher with the api server [14:48] because I can see the API response giving a NotifyWatcherId of 9 [14:48] and then I see a call to NotifyWatcher Next 9 [14:49] and it comes back with no-such-watcher-id [14:49] now, I can see a featuretest for Changes() [14:49] but it seems to only assert that we get *a* change [14:49] and the *first* change for watchers is generated client side [14:50] so it may be that we aren't ever actually trying to watch for a real change [14:52] * katco reading [14:53] gm [14:53] *hm [14:53] let me look at the test again [14:54] jam: i see what you mean... let me check and see if we ignore that 1st even somewhere in the chain [14:58] jam: here's the function that instantiates the new watchers: https://github.com/juju/juju/blob/master/api/uniter/uniter.go#L55-L57 [14:59] jam: 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] jam: either way, i think you're right that the test is probably incorrect [14:59] katco: 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) [15:00] it passes when I try to comment out the Merge line [15:00] jam: yeah i think the test is probably lying to us [15:00] I tried a quick "call it twice" but currently trying to make that do what I think it should [15:01] jam: is it better practice to consume the first event within WatchLeadershipSettings, or within the caller of that? [15:03] katco: so code that hangs on a watcher usually needs to do initialization [15:03] so we expect an initial event [15:03] because Watchers always generate an initial event [15:03] we don't bother sending it over the wire [15:03] jam: ah ok. so maybe consume that initial event in WatchLeadershipSettings? [15:03] and client-side code always generates the event, and server-side code suppresses it [15:04] katco: so I got the test to fail like it should. I'll paste the diff [15:04] cool [15:05] fwereade: ping [15:06] katco: http://paste.ubuntu.com/10580495/ [15:06] * katco looking [15:07] jam: so it appears as thought the code is not registering the watcher properly, as you said [15:08] katco: https://github.com/juju/juju/blob/master/apiserver/upgrader/unitupgrader.go#L46 [15:08] is how we create a watcher, consume the initial event, and register that watcher with the API server resources [15:08] jam: well.. is there a race in that test? do we check for changes before the merge can complete? [15:09] katco: so the diff that I posted [15:09] there will *always* be an initial change, generate client side [15:10] katco: https://github.com/juju/juju/blob/master/api/watcher/watcher.go#L165 [15:10] jam: right, but the second pull, L12... is it possible we hit that before L21 will produce an event? [15:11] katco: it is a channel [15:11] if we get there, it will block [15:11] it is actually a synchronous channel [15:11] so if *either* side gets there they block until the other side gets there too [15:11] (though one side should be a select loop, so it can handle shutdown, etc) [15:11] doh sorry, i mistook the ok for an indication of successfully retrieving from the channel [15:12] technically, we should be doing something more like NotifyAsserter which will timeout rather than hang the test suite [15:12] katco: you get ok = false if the channel is *closed* [15:12] katco: in which case you do still get a return from the channel [15:12] but the ok says "you're getting returned because the channel was closed, not because I had data for you" [15:12] gotcha, thanks for the extanalpion [15:12] wow [15:12] i butchered that word [15:12] explanation [15:13] katco: funny, I read it just fine [15:13] jam: 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] yeah, multiple times [15:13] and it gets worse and worse by the end [15:14] wow... i just noticed i perfectly reversed the middle of that word [15:14] weird [15:14] anyway [15:14] so it looks like there's a bit of work to patch this stuff up [15:14] i'm working on storage stuff for tanzanite, but i realize leadership is due and important as well [15:15] katco: 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] ah cool [15:15] which package is that in? [15:15] katco: 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 patch [15:15] katco: github.com/juju/juju/testing IIRC [15:16] jam: ok i'll pop it onto my queue and should have a patch by your tomorrow [15:16] pop it onto my stack rather :p [15:16] lifo ;) [15:16] katco: https://github.com/juju/juju/blob/master/worker/uniter/filter/filter_test.go is using NotifyAsserterC [15:16] if you want examples [15:16] perfect ty [15:17] jam: ty for troubleshooting that, and sorry for the issues [15:17] it 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] katco: np. I'm glad I could track it down. I don't quite know whether my code is correct or not yet :) [15:18] jam: haha isn't that a fun time in a change? :) [15:18] fwereade: if you're around git@github.com:jameinel/juju leader-settings-tests is the work in progress [15:18] I think it is close to correct, caveat it crashes the Uniter because of the LeadershipSettingsWatcher bug [15:19] Bug #1430839 was opened: juju-run and juju-backup break 'juju help plugins' [15:20] ok, EOD, have a good one [15:22] jam: tc! [15:35] dooferlad, 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 (/name/delta0) === meetingology` is now known as meetingology [16:15] ericsnow, are you still the reviewboard go to fella? [16:15] mattyw_: depends on your question :) [16:15] ericsnow, a review not being picked up https://github.com/juju/juju/pull/1802 [16:16] ericsnow, and shall I just do it by hand? [16:22] this might be nice/useful for those of you who still hate git https://github.com/arialdomartini/oh-my-git [16:35] TheMue: ping [16:36] mattyw_: yeah, do it by hand (rbt post) [16:36] ericsnow, will do thanks [16:37] perrito666: i am the least helpful person on git commands because i use this exclusively: https://www.youtube.com/watch?v=zobx3T7hGNA [16:37] mattyw_: yeah, sorry, it's a consequence of switching host and having to upgrade reviewboard and reviewboard changing APIs between micro versions :( [16:38] mattyw_: actually don't do it by hand [16:38] mattyw_: I forced GH to redeliver the PR event and it worked this time [16:39] ericsnow, awesome, thank you very much [16:41] katco: pong [16:42] TheMue: hey i saw you had a topic scheduled for Nuremberg re: timeboxed iterations [16:42] TheMue: would you mind if i piggy-back on that to discuss timeboxed estimation? or should that be a separate topic? [16:42] katco: yep, would like to talk about it based on my experiences [16:43] katco: no, would appreciate it. estimation is beside reqeng one of my favorite topics [16:43] TheMue: awesome! :) [16:44] TheMue: i'll just /katco on the owners then? [16:45] katco: yes, and we both then could prepare for it during the next weeks [16:45] katco, TheMue if I'm available for that session I'll be in it [16:45] TheMue: sounds good. tyvm for bringing that up [16:45] mattyw_: +1 [16:45] katco: yw [16:49] Bug #1430898 was opened: run-unit-tests ppc64el timeout [17:02] TheMue: ping [17:04] mattyw_: have you got a mo to do second review on reviews.vapour.ws/r/1119 please? :) [17:12] dimitern: ok, so the test is failing because the allocated addresses aren't being found [17:12] dimitern: so a genuine failure... [17:12] :-) [17:13] mgz, will do [17:13] voidspace, hmm.. that's odd - are they allocated to the container? [17:13] yep [17:13] I'm calling AllocateTo [17:13] I haven't yet tested the new state code that fetches them though [17:13] dimitern: probably time to write a test for that code... [17:13] :-) [17:13] voidspace, ah, that might be it :) [17:14] this is why we tset [17:14] *test even [17:24] dimitern: and indeed a simple test for the new state method fails to return any addresses [17:24] oops [17:25] voidspace, that's odd, might be related to the multi-env changes in state (all PKs have ":" prepended) [17:25] debugging [17:25] voidspace, what's you query? [17:25] it's probably me [17:26] dimitern: iter := addresses.Find(bson.D{{"machineid", machineId}}).Iter() [17:26] pretty simple [17:26] voidspace, right.. let me have a look in state [17:27] dimitern: no, I'm being dumb [17:27] dimitern: I think I'm fetching the wrong field for the address [17:27] should be Value and I'm using Address [17:28] dimitern: pretty sure that's it [17:28] running the test now [17:28] voidspace, ah, ok - there's not even an address there [17:29] yeah :-D [17:30] state test passes, now trying apiserver/provisioner [17:31] dimitern: yay - fails for the right reason now - wrong error message [17:32] voidspace, good! :) [17:32] awesome [17:32] the rest should be plain sailing ;-) [17:32] always nice to hear :) [17:38] mgz, done [17:39] coreycb, you around? [17:41] mattyw_: thanks! [17:47] mattyw_: Thank you :) [17:48] natefinch, dimitern katco: do either of you have a few minutes to review this momentous branch http://reviews.vapour.ws/r/1132/ [17:49] sinzui, I could do technically, but I'm not graduated so it wouldn't help :) [17:49] sinzui, sure, looking [17:50] sinzui, done [17:50] thank you dimitern [18:00] wwitzel3: how goes? [18:14] bbl [18:20] Bug #1430943 was opened: Subordinate charms are not displayed in `juju status`. [18:33] When 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:41] biab === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 [19:22] lazyPower: 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] natefinch: i figured that was the clincher, needing to get the baked in cloudebase-cloudinit service on the core windows image with sysprep [19:23] as we dont support custom AMI's or anything - its kind of a non starter atm right? [19:23] lazyPower: 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:24] natefinch: we're going to have fun supporting that one :) [19:24] may the games begin! [19:29] can anyone with superpower rubberstamp this? http://reviews.vapour.ws/r/1088/ [19:37] lazyPower: gah you broke ctrl+mousewheel zooming on your site [19:37] i did? [19:37] hah, i did [19:37] OLE! [19:38] perrito666: reviewing [19:38] perrito666: oops, or not: There was an error displaying this diff. [19:39] natefinch: well, my day has been on those lines so far [19:39] lazyPower: some custom scrolling javascript stuff, huh? I can tell the page scrolls differently than normal, though I couldn't explain how [19:40] today 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 moment [19:41] natefinch: source is here :) https://github.com/chuckbutler/pelican-porto [19:45] lazyPower: betcha it's the scroll to top thingy.... but my knowledge of javascript doesn't really go much beyond the basics. [19:46] lazyPower: which is to say: I'm better at filing bugs than fixing them ;) [19:47] perrito666: I think you can just do rbt post -u from the correct branch locally, and it'll update... [19:50] natefinch: that is not my proposal, actually [19:51] perrito666: oh, you're right. sorry, wasn't paying attention [20:01] man, 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 problem [20:04] lol natefinch [20:05] I guess that should be "a stupid kind of special" but both are mostly correct === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 [20:39] ericsnow: do you know why https://github.com/juju/juju/pull/1805 is not being reviewboardized_ [20:39] perrito666: non-ascii characters [20:40] perrito666: it's a bug in the GH hook [20:40] perrito666: try using rbt post [20:40] I think I missed the chars [20:40] do you see them? [20:41] perrito666: nope [20:41] perrito666: could be in the metadata GH sends [20:41] ah [20:41] mmm and now rbt cannot reach reviewboard [20:41] ... what a day === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 [20:57] who is familiar with watchers? [21:05] katco: like with who watches them or something [21:05] katco: [21:05] ericsnow: the comedian. [21:05] ericsnow: https://www.youtube.com/watch?v=YhJMAaix0CA [21:19] sinzui: re bug 1430898, it may be related to bug 1430791. [21:19] Bug #1430898: run-unit-tests ppc64el timeout [21:19] Bug #1430791: Upgrade-juju is broken on most/all substrates [21:19] sinzui: waigani is working on the latter, and once it lands, I may get you to see if it fixes the former [21:19] thumper, That would be nice [21:24] thumper, sinzui: http://reviews.vapour.ws/r/1133/ [21:25] waigani: you have checked in state/main [21:25] waigani: can you reset on master and remove it plz? [21:26] thumper: ugh, sorry [21:28] thumper: gone [21:30] waigani: +1, make sure you use the "fixes-1430791" bit in the merge instruction === kadams54 is now known as kadams54-away [22:25] anyone knows how to tell rbt to ignore ssl certs? [22:26] ericsnow, did you get your watcher questions answered? [22:26] jw4: :) [22:27] :) [22:27] Looks like it was katco with the original question too [22:27] jw4: i've got a crack team of wallyworld looking into it :) [22:28] katco, awesome - :) [22:28] jw4: 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] hmm interesting [22:28] ericsnow: know how to ask rbt to ignore ssl cert? [22:45] mm and now rbt asks me for a user [22:45] perrito666: https://github.com/juju/juju/blob/master/doc/contributions/reviewboard.md#rbt-authentication [22:46] ericsnow: I have read the code, it seems there is no way, in my version to bypas ssl [22:46] but anyway I would like this to work as is :p [22:47] perrito666: why do you want to bypass SSL? [22:47] perrito666: (I don't know if there's a way) [22:47] ericsnow: I an getting an error in my vivid install but Ill recreate the virtualenv just in case [22:49] perrito666: the phrase *bleeding* edge comes to mind :) [23:25] thumper or davecheney: jujud test fixes http://reviews.vapour.ws/r/1137/ (noticed while looking at an unrelated panic)