[00:00] all: master is not passing the tests for me [00:00] uniter, api client, worker/uniter [00:00] all fail [00:00] anastasiamac: I actually added anothe rcomment but apparently did not go trough reviewboard [00:01] anastasiamac: try to change the uses of fmt.Errorf for errors.Annotate [00:02] perrito666:of course! thnx :-) will do [00:03] perrito666: constructive suggestions r an amazing feedback, especially atm ;-) [00:04] perrito666: english hard atm too... [00:06] anastasiamac: I am south american I am certainly in no position to criticize anyone's english [00:08] perrito666: :-) [00:22] davecheney: master as of 9am this morning passed for me [00:22] davecheney: what is your last commit? [00:23] thumper: i just pulled master [00:23] i was trying to get my branch up to date and the tests were all failing [00:23] so i switch to master [00:23] and they are still failing [00:23] davecheney: let me try === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: None [00:24] davecheney: running now [00:29] thumper: eg, http://paste.ubuntu.com/8726080/ [00:30] http://paste.ubuntu.com/8726089/ [00:31] works here [00:32] smells of timing issue [00:32] well shit [00:32] ok, i'll break out my tools [00:33] oh hang on [00:33] these are the actions [00:33] tests [00:33] i raised bugs about this before [00:34] I thought this was parallel charm upload? [00:35] thumper: what rev of charm.v2 do you have ? [00:35] i don't trust godeps any more [00:35] um... [00:35] it should be charm.v4 [00:35] I think [00:36] * thumper looks [00:36] menn0_: added another step for the migrations. state/watcher.go update Watcher.merge() - change.Id -> localID [00:36] davecheney: yeah, I'm using charm.v4 [00:36] thumper: can you tell me the exact rev [00:36] hey axw, can you remind me what causes juju to try and release twice when bootstrap goes awry? [00:36] menn0_: I think that one is a bug that will bite a few more times [00:36] charm.v4, for all of the promise of gopkg.in [00:37] doesn't ensure we're running the same code [00:37] waigani: yeah, that's one that I catch while looking for transactions that touch the collection, but it's good to mention it explicitly [00:38] davecheney: 8b3cc836f54c2c78ce73d198100a30bb31d28392 [00:38] davecheney: I have godeps working fine [00:38] same [00:38] ok [00:38] scratch that [00:39] thumper: could you run [00:39] davecheney: if you have "export JUJU_MAKE_GODEPS=true" and use "make check" [00:39] thumper: i don't trust godeps [00:39] it makes sure your deps are up to date and runs the tests [00:39] i know how to run the command [00:39] menn0_: yeah, saves some brain power. the change.Id being passed in will always be a DocID but needs to be a localID. I misunderstood that during the transaction sweep. [00:39] i just don't trust it to work [00:39] thumper: can you please try [00:39] it fails for me when I have different origins and upstreams [00:39] env GOMAXPROCS=42 go test github.com/juju/juju/api/uniter [00:39] so I have to fetch upstream manually [00:39] davecheney: sure [00:41] davecheney: test passed here [00:44] waigani, thumper: i'm currently trying to figure out a regression with the relation/relationscopes branch. [00:44] menn0_: want to pair? [00:45] waigani: not at this stage thanks === menn0_ is now known as menn0 [00:45] waigani: I'm slowly making progress [00:46] menn0: cool. If you need a rubber duck let me know [00:46] waigani, thumper: when I destroy a service some relations aren't being deleted [00:46] waigani, thumper: i'll get there but it's painful [00:47] menn0: makes sense if it's looking for the wrong id right? [00:47] menn0: hmm... interesting [00:47] waigani: yes. i'm sure it's something like that but it's not obvious. it could be a watcher problem. [00:48] it only seems to affect subordinate relations AFAICT [00:48] menn0: checked the merge func? ;) [00:49] waigani: no but I think I just found it. missed a change in WatchRelations [00:49] nice [00:50] stupid prefix matching crap [00:50] ah right [00:58] * thumper heads off to see specialist [00:58] bbl [01:01] waigani: the unit tests didn't notice by fluke [01:01] waigani: with an _id like this "60d7b1b0-4fea-452f-8748-70c05cf489ec:wp0:db mysql:server" [01:02] waigani: WatchRelations was matching on _ids starting with "mysql:" or containing " mysql:" [01:02] bigjools: if bootstrap fails after acquiring, it'll release the bootstrap node; juju will then destroy the environment. whether it releases again depends on whether it's possible for MAAS to report that the bootstrap node still has the agent_name we gave it [01:02] ah fuck [01:02] bigjools: now that I write that out in full, it probably never happens [01:03] menn0: is it a regex match? [01:03] waigani: it just happened to always match with the "containing" case in the test but the prefix match is no longer correct [01:03] axw: heh, ok :) [01:03] waigani: no just a simple bit of string matching [01:03] waigani: I'll update the test and then fix [01:03] axw: it's just that we have observed this behaviour but now we can't make it happen again [01:03] waigani: it's a bit of a fluke that I found it [01:04] waigani: which is worrying [01:04] axw: in here: https://bugs.launchpad.net/maas/+bug/1386327 [01:04] Bug #1386327: Juju fails to destroy-environment with wipe enabled: Node cannot be released in its current state ('Disk erasing'). [01:04] menn0: might be worth while searching all the transactions [01:04] bigjools: if the first release failed, then it would attempt again (just the once) [01:04] menn0: or at least adding it to the doc for something to check [01:04] axw: right [01:04] waigani: yeah, I'll add it to the doc [01:04] MESS doc that is [01:05] menn0: you drew the short straw on that one [01:06] but good you spotted the problem, we can now keep an eye out for it [01:06] bigjools: is it possible for release to return before agent_name is removed from a node? [01:06] axw: yes [01:07] in the bug, it has to wipe the disk before it returns, so sits (internally) at a different state. The extra state is swallowed in the API to avoid confusing unaware API clients [01:07] menn0: how did you notice relations were left behind after a service was destroyed? [01:07] axw: so you end up seeing what looks like a node that didn't start releasing, but it really is [01:08] waigani: the service wasn't completely being removed [01:09] waigani: the units and machines were but the service was being left behind in a Dying state with one relation intact [01:10] waigani: I've just updated the manual test instructions to include testing of service removal [01:10] menn0: ah right, fair call [01:17] thumper: solved it [01:17] out of date names dependency [01:17] godeps didn't detect it [01:22] menn0: does mediawiki take ages to install for you? [01:22] it's been sitting on pending for well over 5min [01:23] waigani: I'm not sure exactly how long but that seems on the long side. also depends on you machine and how busy it is of course. [01:23] waigani: or, you've broken something :) [01:23] yeah... [01:24] might be time for a restart [01:29] ok, now I have some other failing tests [01:29] christ [01:43] :| gmail now adds a button "view issue" and "view pull request" on the email list for PRs [01:43] davecheney: want me to try? [01:47] davecheney: have you updated godeps recently? [01:50] nite ppl [01:51] thumper: btw, for that heat you wanted, near 30° its 22:51 and has been night for 3hs :p [01:52] perrito666: it is about 15 here [01:52] and has been day for more hours [01:54] waigani: I'm adding State.strictLocalID as part of this [01:54] it's like localID() but errors if the prefix isn't there [01:55] menn0: okay... [01:55] menn0: used when we assume the id passed in is a DocID [01:56] waigani: it's useful in cases where ids for multiple envs could be seen but you just want to deal with the ones for the State's environment [01:56] waigani: for example in the watchers [01:57] menn0: oh I see [01:57] waigani: i'll push this branch soon so you can see [02:14] arse [02:14] * thumper needs git help [02:14] menn0: around? [02:15] thumper: yep [02:15] thumper: what'd you break? [02:15] my branch [02:15] stop doing that :P [02:15] my branch was 5 commits behind master [02:15] and I did "git reset master" without "git merge master" [02:15] and now I have a bunch of changes I didn't make [02:16] was trying the other rebase options [02:16] forgot it only really works when you have all of the master revisions [02:16] bugger [02:16] so I have some dangling head somewhere [02:16] that I want back [02:16] as the tip of the branch [02:16] how do I find it? [02:16] * menn0 googles [02:17] oops [02:17] trying to think if you can get it without reflog'ing it [02:17] reset's not something I ever do except for git reset --hard HEAD when I want to really kill some work [02:18] thumper: highly recommend https://github.com/juju/juju-gui/blob/develop/HACKING.rst#syncing-your-feature-branch-with-develop-trunk for the whole 'sync with master to my feature branch' just s/develop/master for your use. [02:18] I blame menn0 [02:18] hah [02:18] good call, find a scape goat [02:18] I'm just going to revert everything I know isn't mine [02:19] i looks like it will be quicker [02:19] thumper: i'm not sure why I get the blame. I don't think I encouraged you to use reset did I? [02:19] menn0: if you had never mentioned reset, I'd still be happily rebasing :P [02:19] menn0: and you fit my need of a scapegoat [02:22] thumper: ok, to get back to where you were [02:22] thumper: git reflog | head [02:22] I'm there already [02:22] :) [02:22] thumper: ok [02:22] however... [02:22] for future me [02:22] what does that do? [02:22] thumper: shows you the history of what you've been doing [02:22] ok, I see the commit I care about [02:22] thumper: to recover you find the hash from when you last committed to the branch [02:22] thumper: and then git reset --hard [02:23] huh [02:23] thumper: I just tried it out to be sure [02:23] is that all? [02:23] thumper: yep [02:23] reflog ftw [02:23] ok, will remember that for next time I screw up [02:23] thumper: the thing is that dangling references will eventually get garbage collected by git [02:23] thumper: so you don't want to leave it too long [02:23] thumper: after the screwup to recover [02:24] thumper: this link is helpful: http://effectif.com/git/recovering-lost-git-commits [02:24] * thumper bookmarks [02:24] thumper: best quote: "it's like time travel, only cheaper" :) [02:42] thumper: no i have not updated godeps [02:42] i think the last time I did was when we were on that sprint in NZ [02:47] waigani, thumper: finally! http://reviews.vapour.ws/r/284/ [02:48] waigani: reviewing your charms branch now [02:48] menn0: thanks, I've got minUnits up too [02:48] menn0: let me nut out this problem then I'll get onto your branch [02:49] waigani: np [02:53] waigani: I think we need to a EnvUUID method to state to avoid repeating "st.EnvironTag().Id()" all over the place [02:53] menn0: agreed [02:57] menn0: when you get a chance, I'd love to get your feedback on http://reviews.vapour.ws/r/243. [02:58] ericsnow: will do. I had seen that review request - just hadn't gotten there yet. [02:58] menn0: no worries, I appreciate your time :) [03:05] can someone help please [03:05] http://paste.ubuntu.com/8727429/ [03:05] ^ this goes into stress.bash [03:06] cd $GOPATH/src/github.com/juju/juju/apiserver/client [03:06] bash stress.bash [03:06] * menn0 will run [03:06] ta [03:06] there is a logical race between uploading the charm and whatever the test is doing [03:10] davecheney: it's still running but I see a clientSuite.TestAddCharmConcurrently failure [03:10] davecheney: done now [03:10] http://paste.ubuntu.com/8727454/ [03:10] failing hiallriously [03:10] davecheney: that's what I see [03:10] i have anothe rfailure [03:11] davecheney: actually, on closer inspection my failure is different [03:12] davecheney: http://paste.ubuntu.com/8727496/ [03:14] https://bugs.launchpad.net/juju-core/+bug/1386968 [03:14] Bug #1386968: apiserver/client: clientSuite.TestAddCharmConcurrently failure do to local race [03:16] ugh [03:18] ericsnow: ping [03:19] menn0: hey [03:19] ericsnow: so doesn't option 2 involve using --oplog [03:19] ericsnow: ? [03:19] menn0: yeah [03:19] ericsnow: but it looks like it's not being used when specific dbs are backed up [03:19] menn0: correct [03:20] ericsnow: so what's the plan? [03:20] menn0: with option 2 we don't dump specific DBs [03:20] ericsnow: ok. so why leave the support in there for backing up individual dbs? [03:20] menn0: I'm hopeful that we will be able to stick to option 2 [03:21] menn0: in case option 2 doesn't pan out :) [03:21] menn0: right. got it. [03:22] menn0: started looking but Bella will be here in 10min, I'll be doing the dad thing [03:22] waigani: np [03:23] waigani: i have an errand to run shortly myself [03:23] menn0: btw cleanup docs has an id type of bson.ObjectId - which screws up our upgrade step assumtions [03:24] menn0: not a huge problem, but I'll have to do some rejigging tomorrow [03:24] owwie... [03:24] * thumper goes to lie down [03:24] shoulder is really starting to ache now [03:24] waigani: yeah, I've seen that. let's discuss tomorrow. there's a few ways we could tackle that. [03:24] go an injection in it this afternoon [03:24] hopefully should settle down in the next two weeks [03:24] here's hoping [03:25] menn0: sounds like a plan [03:26] ericsnow: are you sure this branch even compiles? [03:27] ericsnow: mongoDumper.Dump looks incomplete with both unused and undeclared variables [03:27] ericsnow: and mongoDumper.strip() has no code [03:27] menn0: yeah, I created the review request thinking I'd wrapped things up [03:27] menn0: I have it fixed up and will be updating momentarily [03:28] ericsnow: ok [03:33] menn0: shouldn't we always use strictLocalID unless we have a reason not to? [03:36] tada, data race http://paste.ubuntu.com/8727559/ [03:37] func (s *MockCharmStore) WithTestMode(testMode bool) charm.Repository { s.TestMode = testMode return s [03:37] } [03:37] man [03:37] we have _SO MANY_ of these races [03:38] :( [03:40] oh wow [03:40] that methods is pants on head retarded [03:40] it doens't return you a _new_ repository with a test flag set [03:41] it updates the repo that you passed to it ... [03:41] oh, and fantastic, the callers of that expect this behavior [03:43] fml, https://bugs.launchpad.net/juju-core/+bug/1386968/comments/2 [03:43] Bug #1386968: apiserver/client: clientSuite.TestAddCharmConcurrently failure do to local race [04:04] menn0: I've updated that patch [04:04] menn0: and am signing off [04:04] ericsnow: ok. i'm looking at your other one now [04:05] menn0: cool [04:05] ericsnow: I'll try to finish reviewing both [04:05] menn0: thanks again [04:05] ericsnow: np. [04:08] axw: i've found a problem with the recent work to validate credentials, and the use of Open vs Prepare - it does break validate-tools [04:09] you want to run validate-tools before deploying an environment [04:09] to see where the tools will come from [04:09] but that's not possible now, since the environment is not prepared, and hence is missing derived attrs [04:09] like control-bucket for ec2 [04:12] in any case, control-bucket should now be obsolete for deploying new environments [04:24] wallyworld_: yuck, that's a problem. I want to be able to change the manual provider so we can specify the bootstrap host in placement directives, but that involves restricting Prepare to bootstrap [04:26] menn0: if you read this and are not filled with dredd, http://reviews.vapour.ws/r/286/diff/# [04:26] let me know [04:26] axw: yeah. the environment is created in order to get parameters specific to the environment to use to search [04:27] davecheney: I have to stop for a bit soon so I won't be able to get to this for a few hours [04:27] axw: given the validation commands are now broken, short term to get alpha3 released, we'll need a fix that works with what we have [04:28] i have not raised a bug yet [04:28] menn0: that's ok [04:28] you don't need to review it [04:28] wallyworld_: revert to using Prepare for now I guess, will just need to change the metadata tests to set up a mock ec2 endpoint [04:28] axw: will that revert affect the credentials validation [04:29] nope [04:29] i can change to using Prepare in my current redo of the tools stream stuff [04:29] i found this issue by testing that work [04:30] wallyworld_: if it's not too much hassle. otherwise let me know and I'll fix it [04:30] it's no problem [04:30] i'm in the area anyway [04:31] davecheney: i've just had a quick look and although it's not pretty it does look reasonable [04:32] hey [04:32] it's much better [04:32] the caller doesn't need to remember to defer this magic cleanup function [04:32] that's something the test suite should do [04:32] and more improtantly [04:32] it highlights just how horrible that test is [04:32] updating a shared mutable package level variable [04:33] davecheney: you know that CleanupSuite (embedded in BaseSuite) already provides something very similar to what you've done with afterFunc? [04:33] davecheney: there's an AddCleanup method [04:33] menn0: cool [04:33] i was going to use that [04:34] but for some reason I thought it was only on the suites that had something to do with mongo [04:34] i'll fix the patch to use AddCleanup [04:34] davecheney: ok [04:34] davecheney: I'll be back on later but I need to go know [04:34] now [04:35] thanks for pointing me to the place it lives [05:05] axw: sadly, the metadata commands fail with an access credentials check error - i wonder if the verifyCredentials calls could be moved to Open() instead of Prepare(). I wonder if that's too late in the bootstrap process [05:22] wallyworld_: the bug (raised by mark) requested that we do this ASAP in the bootstrap process - and it is kinda necessary to ensure we get the error message early on [05:22] wallyworld_: what is going wrong with the metadata commands? [05:23] axw: yeah, i understand the context. the metadata commands can be run without env credentials, so that verify check fails [05:23] ah. [05:23] we could add a method to the BootstrapContext [05:23] wallyworld_: can we do this some other way? without opening an environment? cos really, we're not exactly opening a fully defined environment [05:24] right now, an env instance is needed to ask for things from [05:24] yes... can we change that. [05:24] not that easily, but yes [05:25] because creating an env manipulates config [05:26] i'll have a poke around, but there's potentially a fair bit to unpick [05:26] yeah I took a look, doesn't look like something we can do easily [05:27] axw: could add a method to BootstrapContext - ValidateCredentials() bool [05:27] would work for now [05:28] so that we can get alpha3 out [05:28] i also gotta make sync-tools backwards compatible, bit of a pita [05:29] wallyworld_: we can make it bootstrap only I suppose, would be nice not to but it'll do for now [05:29] yeah, agreed it's not nice :-( [05:30] i'll try it after fixing sync tools [05:30] lots of touch points there === urulama__ is now known as urulama [07:24] morning all [07:54] wallyworld_: FYI I've not sent a PR with my metadata changes yet, because I want to explore the usage of it a bit first. I would like to avoid churning packages [07:54] axw: +1 [07:55] axw: i'm almost finished the tools rework, including credentials verification fix. keep getting mongo replicaset errors testing sadly [07:55] timeouts mainly [08:04] okey dokey [08:04] gotta make dinner shortly, ping when it's ready and I'll take a look [08:07] wallyworld_, hey [08:07] hi [08:07] wallyworld_, so re that network-bridge-related bugs [08:07] wallyworld_, that guy probably misread or misunderstood what the code he pasted does [08:08] likely [08:08] i'm not 100% either [08:08] wallyworld_, but seeing lxcbr0 in there *at all* is wrong and should be fixed [08:09] dimitern: which bit? [08:09] wallyworld_, in fact I'm thinking of doing that today, as this is likely the 15th bug report related to that piece of code in the maas provider [08:09] ok, uniter tests passing again, popping out to shops, bbs [08:10] dimitern: that would be awesome [08:10] i don't know enough about networking [08:10] wallyworld_, there is some code in there that aims to "restore" /etc/network/interfaces the way they *should* look like in a stock trusty install, but that's really not the point - we shouldn't be using the bridge at all, but first we should respect the network-bridge setting [08:11] at least [08:11] sounds reasonable [08:12] morning [08:12] wallyworld_, cool, thanks [08:12] TheMue, morning [08:12] dimitern: no, thank you, i have no idea really [08:14] wallyworld_, well, I have to admit the simple solution of just using "network-bridge" in there as a temporary fix (so at least it's configurable) simply didn't occur to me earlier, and we can't just remove the bridge creation until we've implemented the static ip reservation api maas 1.7 gives us; so tl;dr - thanks for pointing that out :) [08:15] np, it was an educated guess :-) [08:51] morning all [09:01] morning all [09:03] mattyw, voidspace: heya [09:03] TheMue, morning morning [09:13] voidspace, would you be free to take a look at a couple of gsamfira's PRs? https://github.com/juju/juju/pull/782 https://github.com/juju/juju/pull/833 [09:13] voidspace, gsamfira: I should be able to get to them later, but I realised I broke something in the uniter and I'm focused on unbreaking it [09:14] fwereade: heh, no worries [09:14] and if anyone *else* is free -- jam, dimitern maybe? -- http://reviews.vapour.ws/r/288/diff/# is the code, on which I'd really a appreciate a logic/style pre-review, but I haven't yet fixed all the tests (sigh) [09:15] fwereade: looking [09:17] fwereade: is "State().Members" a map and you want the keys to be in the member name slice ? [09:17] (I wonder if memberName, _ := would be an obvious way to write that) [09:18] at least, I first read it as "Members is a slice" and we just want to copy the slice, but then iteration doesn't work like that :) [09:20] fwereade: I also thought: "contextRelations := map[int]*ContextRelation{}" was bad form because you end up with a nil map, you actually want make() there, don't you? [09:21] hmm. play.golang says I'm wrong [09:21] var x map[int]string is a nil map, but x := map[int]string{} is an empty map [09:24] jam: have you picked up both PRs or should I look at one? [09:25] voidspace: I'm looking at fwereade's not gsamfira's [09:25] jam: ah, cool [09:25] fwereade: sure, I'll look at the gsamfira reviews [09:26] jam, yeah, I've never understood the preference for make()ing maps over just using a literal [09:27] fwereade: well, because "var foo map" doesn't work [09:27] fwereade: so it would seem that the way to create a map is with "make" [09:29] jam, sure, but the literal syntax works fine, so I'm not sure why people prefer to avoid it [09:30] fwereade: in *my* case, not knowing (or expecting) it to wokr [09:30] work [09:30] jam, it's essentially just the same as slices [09:31] jam, var gives nil, type{} gives empty [09:31] fwereade: except empty slice lets you append as well as a nil slice [09:31] and call len [09:31] jam, indeed, *that*'s the reason people use var foo string{} :) [09:31] and iterate [09:31] etc [09:31] jam, len and range work on nil maps too [09:31]