davecheneywaigani_: menn0 thumper https://github.com/juju/juju/pull/54800:19
davecheneyhere is an easy one to warm up with this morning00:19
hazmatanyone know instance distributor provider code?00:35
hazmatjust curious how it knows service name00:35
menn0thumper: got a sec?01:28
thumpermenn0: aye01:28
menn0thumper: hangout?01:28
menn0thumper: calling you now01:29
menn0thumper: https://plus.google.com/hangouts/_/g4pl2c7bq3jqz2jy3rsjdtvakea?hl=en-GB01:29
menn0thumper: grrr01:29
menn0let's use the standup hangout01:30
davecheneywow, my branch from a month ago merged mostly cleanly with master01:38
davecheneynot bad01:38
axwcan I please get a review on https://github.com/juju/juju/pull/552 to unblock CI?02:14
axwwallyworld_: ^^  very small one02:16
davecheneyOOPS: 34 passed, 6 FAILED03:15
davecheney--- FAIL: Test (18.44s)03:15
davecheneyexit status 103:15
davecheneyFAIL    github.com/juju/juju/state/apiserver/provisioner        18.541s03:15
davecheneywell, at least it's only 3 of the apiserver packages03:16
davecheneywhich is better than before03:16
davecheneybut still a lot of tests to fix03:16
thumperwaigani: I'm about to drop maia down at hockey03:17
thumperwaigani: how about we hangout when I get back and talk about the testing ?03:17
waiganithumper: that would be great, thanks03:17
davecheney... obtained params.ErrorResults = params.ErrorResults{Results:[]params.ErrorResult{params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:&params.Error{"not found", "machine 42 not found"}}, params.ErrorResult{Error:&params.Error{"not found", "unit \"foo/0\" not found"}}, params.ErrorResult{Error:&params.Error{"not found", "service \"bar\" not found"}}}}03:20
davecheney... expected params.ErrorResults = params.ErrorResults{Results:[]params.ErrorResult{params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:<nil>}, params.ErrorResult{Error:&params.Error{"not found", "machine 42 not found"}}, params.ErrorResult{Error:&params.Error{"unauthorized access", "permission denied"}}, params.ErrorResult{Error:&params.Error{"unauthorized access", "permission denied"}}}}03:20
davecheneywelcome to my day03:20
davecheneylooking at long likes of quasi json and spotting the n differences03:20
thumperwaigani: back03:46
waiganithumper: https://github.com/juju/juju/pull/55303:46
thumperwaigani: looking now, tests work ok?03:47
wwitzel3waigani: I added the comments for the public methods on https://github.com/juju/juju/pull/51103:48
waiganithumper: all tests passed except state/envuser_test.go which I fixed and tested individually03:49
waiganithumper: I'll do one more make test to be sure03:49
waiganiwwitzel3: great thanks :)03:50
waiganiwwitzel3, davecheney is my review mentor so you'll have to poke him for an LGTM03:52
wwitzel3thanks :)03:52
davecheneytwo secs03:54
thumperwaigani: you are missing the new files...03:55
thumperwaigani: state/envuser.go + test are not there03:55
thumperwwitzel3: o/03:55
waiganithumper: fuk03:55
wwitzel3thumper: heya03:56
waiganithumper: state/envuser.go is already in master?04:00
thumperwaigani: is it?04:00
* thumper looks04:00
waiganithumper: is for me?04:00
thumperwaigani: umm... no. don't think so04:01
waiganithumper: okay my local master is stuffed up04:01
waiganithumper: let me clean this up and try again04:02
menn0thumper: that change you helped me with: https://github.com/juju/juju/pull/554/04:20
menn0thumper: do you have time to review?04:20
menn0thumper: i've already made sure this solves the problem of downgrades in local/manual envs04:21
menn0so once this is in04:21
menn0the other change can be proposed straight away04:21
waiganithumper: cleaned up, I'll address your initial comments before pushing again.04:23
waiganithumper: user take does not have a Name() method?04:23
thumperyes it does04:24
thumperare your deps up to date?04:24
waiganithumper: I just ran the godeps tool04:24
thumperbut your branch probably isn't up to date with trunk04:24
thumpermenn0: only one real comment04:26
menn0thumper: I don't think what you've suggested is necessary04:29
menn0the test intentionally has no tools in env storage04:29
menn0so if a download was attempted it would fail04:29
menn0this work was done test first, and it failed because of this04:30
waiganithumper: dad duty just started, I'll get it back up later tonight.04:31
thumperwaigani: ack04:31
menn0thumper: does that make sense?04:31
thumpermenn0: in which case, please add a comment :-) as it wasn't obvious to me04:31
menn0thumper: will do04:31
menn0thumper: added comment and rearranged the test a little to make things clearer04:37
menn0how's this? https://github.com/juju/juju/pull/554/files04:37
thumpermenn0: cheers04:37
davecheneywwitzel3: you've got a LGTM on that change04:38
davecheneythere is nothing to be gained from another round of reviews04:38
davecheneydo what you think is reasonable then submit that sukka before we get locked down again04:38
thumpermenn0: a slightly premature merge :)04:40
menn0thumper: sorry. I wasn't sure if you were going to say anything else04:40
thumpermenn0: that's fine :)04:40
menn0thumper: is there a problem04:40
menn0just saw your LGTM :)04:40
wwitzel3davecheney: thanks04:41
menn0potentially dumb question: what does this do? go test -p 2 ./...04:43
menn0and why does the build do it?04:43
menn0wallyworld_'s last merge got aborted because a test took too long and then the tests were run again as above and passed04:44
wallyworld_menn0: it runs the tests with reduced parallelism04:44
wallyworld_because our tests sucks04:45
menn0oh right04:45
wallyworld_and often fail spuriously04:45
menn0I didn't realise we were running tests in parallel at all!04:45
wallyworld_but -p 2 works more often than not but is slower04:45
wallyworld_menn0: that's what Go does out of the box04:45
menn0well I have learned something today04:45
menn0time to put my feet up04:46
davecheneymenn0: you and andrew gerrand05:01
davecheneyhe didn't know -p $NUMCPU was the default either05:01
menn0davecheney: well I don't feel so bad then :)05:01
davecheneypossibly he doesn't spend as long as you and I do waiting for tests to ru05:02
* thumper goes to make dinner05:02
wwitzel3I'm off, be back in the morning o/05:09
=== uru-afk is now known as urulama
davecheneywallyworld_: ping06:26
wallyworld_davecheney: hey06:55
davecheneywallyworld_: i have a question about the keymanager06:57
davecheneywhen we pass user: juju-system-key06:57
davecheneyto the keymanager06:57
davecheneyis that a tag ?06:57
wallyworld_i didn't add that bit, but let me look at the code06:57
davecheneyi've had to put a horrible hack in to make juju accept that as a tag06:57
davecheneyok, ifyou didnd't writ it06:57
davecheneythat is ok06:57
davecheneyyou have TODOs all over it06:58
wallyworld_i don't think i did - i wrote the key manager itself06:58
davecheneyi have exactly one test to fix06:58
davecheneythen this bloody branch is done06:58
wallyworld_i think someone came along and added support for a system-key06:58
davecheneywell, they cocked it up06:58
wallyworld_let me look briefly06:58
wallyworld_i could have been me, but i can't recall06:58
wallyworld_davecheney: looks like it was me. a key can be for a user eg fred, or is the juju system key07:02
davecheneyright, so they aren't tags07:02
wallyworld_i guess that was done back before tags07:02
davecheneywhich makes it a bit of a hack to use common.AuthFunc07:03
davecheneyi've done a gross hack07:03
davecheneyit'll hold07:03
wallyworld_yuk, sorry07:03
davecheneywe'll live07:03
davecheneyit can't be fixed now because of BACKWARDS COMPATABILITY !!1!07:03
wallyworld_yep :-(07:04
davecheneywallyworld_: https://github.com/juju/juju/pull/55607:07
davecheneyit's only taken a month07:07
davecheneyi'm off to the pub07:07
wallyworld_have a drink for me :-)07:07
voidspaceperrito666: let me know when you arrive :-)07:09
jamwallyworld_: I'm pretty sure you'll be having a drink for yourself, too :)08:14
wallyworld_jam: yes i will :-)08:18
jammorning TheMue08:18
TheMuedimitern: ping08:22
dimiternTheMue, hey08:22
TheMuedimitern: I changed my approach a bit, but I still hesitate to use the Entity approach.08:23
TheMuedimitern: see https://github.com/TheMue/juju/compare/capability-detection-for-networker08:23
dimiternTheMue, looking08:23
TheMuedimitern: Entity is really generic, for all entities with a tag. But IsManual is a flag for machines.08:24
TheMuedimitern: I now read it only once when creating a local Machine instance, so it is cached and will never bee refreshed, because it won’t change.08:25
jamTheMue: AIUI Jobs is also specific to Machines, but the "thing" is Entity which is more generic08:25
dimiternTheMue, no, please let's not make 2 calls for every life request08:25
TheMuejam: Entity doesn’t know about jobs08:26
dimiternTheMue, yes jam's right - there's already a precedent with Jobs, adding another flag is not a big deal I think08:26
jamdimitern: themue seems to think I've got the wrong object, I could be wrong there.08:27
jamBut I thought I remembered seeing other stuff that was machine specific on the generic class08:27
TheMueAaargh, I see, we’re talking about AgentGetEntitiesResult08:28
dimiternyes! :)08:29
TheMueBut I’m still not happy with overloading that struct more and more08:29
dimiternTheMue, that's what versioning is for, no?08:29
TheMuedimitern: I don’t talk about differences, I talk about common and specific fields, separation of concerns08:30
dimiternTheMue, the agents need this data to do their jobs (efficiently), and it's also an internal api08:30
TheMuedimitern: So I would be more happy to let the machine fetch a doc especially for a machine08:30
TheMuedimitern: still not clean and optimal for maintainability, you see how easy one can move into the wrong direction08:31
dimiternTheMue, I don't think this is us moving in the wrong direction08:31
TheMuedimitern: IMHO the Machiner would be responsible for Machine operations and the Machine in the should know what it has to know about machines08:32
dimiternTheMue, fwiw, we probably need to have 2 separate unit and machine agent facades, rather than pretending we can treat both and their entities the same08:32
dimiternTheMue, and if you look at AgentGetEntitiesResults, only Life is common between machines and units, ContainerType and Jobs only apply to machines08:34
TheMuedimitern: yeah, maybe. so to come to a quick solution I will do the AgentGetEntitiesResult change. and I’ll file an issue for refactoring with low priority08:35
dimiternTheMue, that sounds good to me!08:37
voidspaceSo "juju restore" bootstraps again (which it needs to)08:52
voidspace but if it fails it leaves the instance created - but with no running agent.08:52
voidspaceSo "juju status" (etc) fail and you have to manually destroy the instance (or accidentally leave it running because you don't realise)...08:53
voidspacejam: your hacked-up-backup script (for local provider)08:53
voidspacejam: it uses $JUJU_ENV08:53
voidspacethis isn't set when I'm running the script directly, so I have to set it myself I guess08:53
voidspaceor would you expect it to be set?08:53
jamvoidspace: I believe it is set if you run "juju restore" but I just use "JUJU_ENV=local ./juju-restore"08:54
voidspacejam: right, cool08:54
jamvoidspace: if you just run "juju-backup" it fails too, right?08:54
jamMy understanding is that the plugin infrastructure sets that variable08:54
voidspacejam: it fails for publiic-key08:54
voidspacejam: right08:54
voidspacejam: that's what I *thought*, but I as just checking my system wasn't screwed in new and exciting ways08:54
jam voidspace: did you do any more hacking on it, or shall I see if I can get it through to completion?09:01
voidspacejam: I was trying (and failing) to get amazon working09:01
voidspacejam: I've now switched to the local script09:01
voidspacebackup seems to work fine :-)09:01
dimiternjam, got a minute?09:04
jamvoidspace: well local backup "works" in that it generates a tarball, but all the paths match exactly the local paths, which means they *don't* match the "official" paths.09:04
dimiternjam, I'd like to pass an idea by you09:04
jamwell, what *I* would consider official, at least (/var/lib vs /home/$USER/.juju/etc)09:04
voidspacejam: right09:04
jamdimitern: G+ or just in IRC?09:05
dimiternjam, just IRC should be fine09:05
jamvoidspace: I think the thing to do there would actually be to rewrite them, but I don't expect backup of Amazon to be able to restore into local09:05
voidspacejust local to local will be a good start09:05
voidspacelocal to amazon might be useful :-)09:06
dimiternjam, so after experimenting with the net package's Interfaces() and InterfaceAddrs() on precise, trusty and utopic daily image, I realized we can get all the information we need about the network interfaces *without* ever bothering to read /etc/network/interfaces or /interfaces.d/*.cfg09:06
voidspacecreate your system locally, then "restore" to your production systems09:06
jamdimitern: what is it that we wanted but don't have access to?09:06
dimiternjam, like what?09:07
jam"we can't get all the information we need" such as?09:07
jamlooking here: http://golang.org/pkg/net/#Interface09:07
jamit looks like we don't have anything about bridging, etc.09:07
dimiternjam, we can discover all interfaces, bridges included, their flags (up, down, broadcast, loopback, multicast), and their index and names, also any addresses assigned09:08
dimiternjam, I'm proposing to just rewrite the interfaces config as needed for ifup/down to work at boot, and as the networker starts it will read all configs and check if there are any changes between what we have in state and what's configured on the machine, and rewrite the configs as needed09:09
jamdimitern: ok, I read what you said wrong. I read it as "we cannot get all the information", if we *can* that's great, though I have my doubts. I'm also wondering where "net" gets its information from, is it just the live state, or is it the state that will be started on restart09:10
dimiternjam, the net package gets the info using syscalls and some obscure netlink packets09:11
jamdimitern: k, so it would be the "live" state, vs the saved state09:11
dimiternjam, it is more or less equivalent to what we can see using the "ip" command (i.e. real-time)09:11
jamdimitern: right, it is a kernel sort of interface09:11
jam"ifup" and /etc/network/interfaces are a debian/ubuntu-ism09:11
dimiternjam, yes, and it works just as well in precise as in utopic09:11
jamIIRC, Fedora/Redhat use a different saved state09:11
dimiternjam, well, there is support only for linux to discover the interfaces like that, but I've yet to try centos and fedora09:12
dimiternjam, that's true, we'll have to render different stored config for other distros, but let's worry about this as we get there :)09:13
dimiternjam, another up-side of using the net package is we don't need to rely on /etc/network/interfaces being there to decide whether to start the networker or not (more so now, as we have the "safe mode" to disable it selectively)09:15
jamdimitern: sorry about the delay, emergency take-the-dog-out action.09:38
jamdimitern: so I like the idea a bit (no safe mode would be very nice)09:38
jamI wonder if we really don't want to save our configuration, which means writing to /etc/network/*09:38
dimiternjam, no, no, I'm not saying that :)09:39
jamdimitern: perhaps we could only rewrite things when we've actually decided we need to change things09:39
dimiternjam, we do write the network config in /etc, but we don't read it - we use the net package to discover what nics are there09:39
jamdimitern: but we shouldn't write it unless we think there is a change from what's written there, should we?09:40
jamin which case, we have to read what is there to write anythnig09:40
dimiternjam, we need to write the config, otherwise without a working juju machine agent + networker, the networking won't be setup correctly after system boot09:40
dimiternjam, yes, we get all NICs, read what's there and render and write "managed" config for each nic that we need to setup, but only if it's pre-existing config on disk is different from what the networker would generate09:41
dimiternjam, am I making more sense? :)09:42
jamdimitern: so I can understand that we may not need to parse them, I'm not 100% sure that our method of comparison as byte-strings is quite what we want (what about comments in the files, etc).09:43
dimiternjam, ok, so let's backtrack just a bit09:43
dimiternjam, the user trusts (or we hope they do) juju to do the right thing with the machines juju manages, right? the same should be true wrt the networker taking control over the machine's networking config and managing it, without expecting the user to interfere?09:44
jamdimitern: the whole point of "Safe" is because we don't trust us on Manual or Local09:45
dimiternjam, we make this obvious by using the comment header "# Managed by Juju, please do not change." in each config file the networker generates09:45
dimiternjam, right, so for "safe mode", don't write changes to the config or run any ifup/down commands, effectively just sitting there and logging what we discovered09:46
dimiternjam, I'm hoping we can get rid of safe mode for manual machines once we have a way for the user to explicitly tell us how to setup networking on them, or we're able to discover it somehow, smartly09:50
jamdimitern: TheMue: voidspace: standup?10:45
voidspacejam: omw10:45
voidspaceperrito666: ping10:50
jamvoidspace: are you still there?10:52
jammarcoceppi: poke about charm tools: https://bugs.launchpad.net/charm-tools/+bug/135917011:34
mupBug #1359170: juju charm create fails in an unhelpful fashion with juju-1.21 in PATH <Juju Charm Tools:New> <https://launchpad.net/bugs/1359170>11:34
jamAlso, when I look at: https://juju.ubuntu.com/docs/authors-charm-writing.html11:34
jamIt has the lines:11:34
jamjuju charm create vanilla11:34
jambut in my browser, those are wrapped onto 2 lines11:34
jamso it looks like 2 commands11:34
jamCtrl Refresh fixed it for me.... very weird11:35
perrito666morning voidspace11:41
voidspaceperrito666: morning11:41
voidspaceperrito666: solved my horrendous backup / restore problems11:41
voidspaceperrito666: well, more or less11:42
perrito666voidspace: heh, all backup and restore problems are horrendous11:42
voidspaceperrito666: backup wasn't working *at all*11:42
perrito666voidspace: odd, since when?11:43
voidspaceperrito666: but jam finally sussed it - the juju 1.18 juju-backup is in the path and the dev one isn't11:43
voidspaceperrito666: only for me on trunk11:43
voidspaceperrito666: so I have to run the backup script directly to try it11:43
perrito666sweet, is there a PR already?11:46
voidspaceperrito666: not a PR yet11:46
voidspaceperrito666: I'll show you what I have (complete now I think)11:47
jamperrito666: the issue is that the 1.18 "juju-backup" shell script didn't know about /usr/lib/juju/bin/mongodump which you fixed for 1.20, but 1.18 is the one in /usr/bin/juju11:47
voidspaceperrito666: although I need to test it, hence the problem with backup not working11:47
voidspaceperrito666: https://github.com/voidspace/juju/compare/backup-ssh11:47
jamand just running "juju backup" searches $PATH for "juju--backup" so even though juju itself is 1.21, "juju backup" was running the 1.18 version.11:48
voidspacejam: so backup and restore from trunk worked fine for me - which is a first11:48
voidspacenow to try it with my code...11:48
perrito666ah very true, my usual test uses absolute path11:48
jamperrito666: you mean /home/user/path/to/juju/cmd/plugins/juju-backup/juju-backup ?11:50
perrito666jam: yup, since I use the automated test from qa I added my paths to make sure it alwas runs the same11:52
jamperrito666: and then presumably you just set JUJU_ENV to test the right environment?11:52
jamTheMue: didn't you land a patch to allow "@" syntax for "juju set" ?11:53
perrito666jam: the CI test requires it as a param actually (test_recovery.py)11:54
jamperrito666: I don't see a test_recovery.py in lp:juju-ci-tools11:56
jamtest_assess_recovery.py ?11:56
perrito666jam: it was test_recovery.py until friday11:57
* perrito666 pulls11:57
jamperrito666: perhaps it is a different source code, because test_assess_recovery.py imports "test_recovery" at the top.11:57
perrito666RM  test_recovery.py => assess_recovery.py11:58
jamperrito666: looks like test_assess_recovery.py didn't get fixe11:58
jamperrito666: and it looks like CI does, indeed, run "juju-backup" and not "juju backup" which might be something we should think carefully about, since *users* are going to be doing the latter.11:59
jambut we are testing the former11:59
perrito666I must have local changes to ensure that, ah I see, those tests ship their own version of backup11:59
* perrito666 wonders why12:00
voidspaceyeah, seems odd12:00
perrito666jam: good point Ill nag sinzui about it when he shows12:00
voidspaceanyway, about to test my restore12:00
voidspacejust killed the state server12:00
voidspacejam: looks like restore does fetch the tools from the correct storage location when used in conjunction with upload-tools (just btw)12:04
jamvoidspace: good to hear. I expected as much, since we are using the control-bucket as defined in the .jenv12:04
jamI almost wonder if backup should be saving the .jenv as well.12:05
jamsince you really need both (I believe)12:05
voidspacejam: so my restore failed12:08
jamvoidspace: did stock juju core succeed?12:08
voidspacejam: it looks like it opened the apiState fine, but then the Client failed to get an address for "machine-1" (my mysql unit - so the call to client.PublicAddress("machine-1") failed12:08
voidspacejam: yes12:08
voidspacejam: heh, although to be fair I didnt try with a unit with trunk as I wanted a quick test12:09
voidspacebut this seems like a plausible failure12:09
voidspaceI'm going to make coffee and then dig in12:09
voidspaceno public address found: unknown unit or machine "machine-112:09
jamvoidspace: the nice thing about backup and restore is that once you have the backup, you don't have to backup again, just try restore12:10
voidspaceyep :-)12:10
jamvoidspace: that sounds like a "tag" vs an "id" issue12:10
jamas in, it might be expecting "1" for "machine-1"12:10
voidspaceah right, just "1" maybe12:10
voidspacewill try that12:10
voidspaceeven before coffee :-)12:10
voidspacealthough if restore fails you have to manually kill the ec2 instance12:12
voidspaceas a failed restore leaves the instance it created running12:13
TheMuejam: sorry, just seen your question. have been to lunch.12:15
voidspaceok, restore underway again12:16
voidspacecoffee time12:16
TheMuejam: have to see what it has been exactly, but yes, IMHO it has been the @ to lead from files12:16
TheMuejam: https://bugs.launchpad.net/juju-core/+bug/121696712:18
mupBug #1216967: Missing @ syntax for reading config setting from file content <compat> <docs> <hours> <improvement> <juju-core:Fix Committed by themue> <https://launchpad.net/bugs/1216967>12:18
jamTheMue: k, I thought it would show up in "juju help set" but I didn't see it there12:18
TheMuejam: interesting, afair it should12:19
TheMuejam: the set doc has a hint. three paragraphs, the second one. which version do you use?12:21
jamTheMue: http://paste.ubuntu.com/8097539/12:23
jamis all I see for "juju help set12:23
TheMuejam: see line 28 in juju/cmd/set.go, it’s more. but it’s not yet released12:25
TheMuejam: so if you don’t use your own build you won’t see it.12:25
jamTheMue: $ ~12:26
jam~/dev/go/bin/juju set -h12:26
jamI'm using trunk12:26
jam~/dev/go/bin/juju --version12:26
jamTheMue: you have "setDoc" set to the correct thing12:26
jambut line 4812:26
jamhas "Doc: "set one or more ..."12:27
jamnot Doc: setDoc12:27
jamTheMue: care to submit a bug and a patch for it?12:27
TheMuejam: aaargh, I found setDoc before and expanded it. never seen that it isn’t used.12:27
TheMuejam: yep, will quickly do it.12:27
voidspacejam: perrito666: looks to me like restore worked fine!12:46
perrito666voidspace: sweet12:46
voidspacejam: perrito666: will double check the code and create PR12:46
voidspaceand then *re-PR* the database shutoff12:47
jamvoidspace: greate. why didn't it work the first time? (did you fix something?) was it just the "machine-1" vs "1" issue?12:47
voidspacejam: yep12:47
voidspacejam: switching to id instead of tag fixed it12:48
jamvoidspace: I wish our code was more consistent here, but that is what API versioning could get us to :)12:48
jamvoidspace: can you file a bug tagged "API" about the inconsistency?12:48
jamand tech-debt12:48
voidspacejam: yep, and I'm adding a note to the PublicAddress docstring that "target" is an id not a tag.12:49
perrito666aghh the convoluted settings of networkmanager/dnsmasq in 14.04 drive me crazy12:51
jamperrito666: it looks like we broke plugins in 1.21 as well, which would have been caught if we were doing "juju restore" instead of "juju-restore", not that we should have used that to actually catch the bug12:52
perrito666jam: thats huge, dont we have a test to make sure plugins actually works?12:53
jamperrito666: apparently not https://bugs.launchpad.net/juju-core/+bug/135917012:53
mupBug #1359170: juju charm create fails in an unhelpful fashion with juju-1.21 in PATH <Juju Charm Tools:New> <juju-core:Triaged> <https://launchpad.net/bugs/1359170>12:53
jamI tried to run the charm-tools plugin12:53
jamand apparently we stopped passing arguments to plugins in 1.2112:54
jamI'm testing 1.20 now12:54
jamit seems broken there, too12:54
jamcurtis won't be happy12:54
jamwallyworld_: can I point you to bug #1359170 when you're around?12:56
mupBug #1359170: arguments no longer passed to plugins <Juju Charm Tools:Invalid> <juju-core:Triaged> <juju-core 1.20:Triaged> <https://launchpad.net/bugs/1359170>12:56
jamI'll try to pick it up since I discovered it12:56
jamCI isn't failing, so I'm not sure if it should be "CI regression" and block trunk, but it is a *serious* f-up on our part.12:58
jamI can't quite see why it is failing yet, though.12:59
jamsinzui: you'll certainly be happy to hear that I just opened a critical bug against 1.20...13:02
jamsinzui: bug #135917013:03
mupBug #1359170: arguments no longer passed to plugins <regression> <Juju Charm Tools:Invalid> <juju-core:Triaged by jameinel> <juju-core 1.20:Triaged by jameinel> <https://launchpad.net/bugs/1359170>13:03
sinzuijam, That is nicer that something breaking the entire packaging and publication step. I think you know what is wrong in you bug. at this moment CI is dead and I don't know why13:04
TheMueHmm, it’s more intelligent to push branches when planing to create a PR.13:04
sinzuiabentley, mgz: Looks like by stress from yesterday was just waiting for me to return to the job this morning. My top priority is to backport some ubuntu packaging rules into our source package branch. We want 1.20.5 accepted into utopic by feature freeze this Friday13:07
abentleysinzui: Hoo boy.  Have fun.13:08
sinzuiLooks like one of the packaging changes broke CI. I will revert, but I think we need to rerun 1.20 each time I want to verify my changes to the source package branch13:09
TheMuejam: https://github.com/juju/juju/pull/559, extreme trivial :D13:09
abentleysinzui: Is "/home/ubuntu/juju-build/*.deb" right?  Should it be in the jenkins home dir?13:10
jamTheMue: I'd like to have a Launchpad bug to reference for this fix, also, you can add a test case in test_main.go which asserts the output of "juju set -h" matches setDoc.13:10
jamTheMue: otherwise LGTM13:11
sinzuiabentley, no, because the script was written to work on a remote instance.13:11
TheMuejam: OK, the bug already exists, will put it in the description. And will add a test. Do we test the doc output in other commands too? Otherwise we should add tests there too.13:12
sinzuiabentley, I added a hack to the script to run on local host to use the jenkin's workspace...but we haven't dismantled publish-revision to build on separate machines.13:12
jamTheMue: I've written tests in "main_test.go" before in order to assert that things were actually registered13:12
jamTheMue: for example "juju sync-tools --help" is in there.13:13
jamTheMue: *I* did it, because it was the most obvious place to check the lines in main.go where we have "r.Register(&Command)"13:13
jamTheMue: (test that the command exists because we get proper help text for it, rather than trying to say run the command and have unwanted side effects)13:14
=== ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: 1359170
TheMuejam: fine, so should look if this is done for all commands (but in an extra change)13:15
jamah ffs, things work *if* you pass an environment name in "-e"....13:16
sinzuijam: I would like your advise. backup restore doesn't work unless mongodb-clients is not installed on the client machine. I think this is a " Depends" relationship in packaging. It was suggested to me that I use "recommends" because it is not needed by everyone.13:18
jamsinzui: if having something installed/not installed on the *client* changes whether backup + restore works then we just have a bug.13:20
jamwe should not depend on local stuff in order for us to run stuff on the server.13:20
jamsinzui: it sounds like we are doing some sort of "does X exist" check on the client, when it needs to be run on the server.13:21
perrito666sinzui: mongodb-clients isrequired on the server not the client13:21
sinzuiperrito666, oh!13:22
perrito666sinzui: and it is only required for restore13:22
perrito666and restore installs it if its not there13:22
sinzuiperrito666, fab. Thank you very much perrito666 , so I am going to mark the bug as fixed in 1.20.x by you13:23
perrito666sinzui: indeed, the bug you just marked as fixed has the wrong diagnose, but it has been fixed in 1.2013:27
sinzuiperrito666, Yeah. I elected to show it fixed in the latest to encourage people to seek it out13:28
perrito666sinzui: jam talked about it earlier, its because backup in 1.18 does not do a proper lookup for the tools which are in a juju folder13:28
jamdimitern: you're OCR today, so https://github.com/juju/juju/pull/560 if you want to give it a look. It needs a test that we operate properly under the circumstances I described in the bug.13:34
jamdimitern: I'd also like to run it past thumper, since I'm pretty sure he wrote the environmentCommandWrapper stuff.13:35
* sinzui restarts CI from the publish-revision step for master13:35
dimiternjam, sure, will have a look in a bit13:40
=== gsamfira1 is now known as gsamfira
jamperrito666: so it turns out we wouldn't have discovered my bug by running "juju restore", because it only breaks Args when you don't have an Environment, and "juju-restore" must have an environment set anyway.13:48
perrito666jam: you do make an interesting case for a test anyway13:48
jamperrito666: so we might have a test, but it either has a default environment, or what it is testing requires one.13:48
jamperrito666: I may be unique in how I do my environment management, and if I'm unique enough, maybe we should just deprecate it and not actually support it13:49
jameither that, or we need to be doing more testing that my way is actually supported.13:49
jambash autocompletion also was (is?) buggy for me because it assumed you'd have an environment set13:49
perrito666jam: well, if we support it, we need to test it13:49
alexisbnatefinch, wwitzel3: TOSCA call?14:02
natefinchalexisb: coming14:02
wwitzel3alexisb: I thought I was already in it ..14:02
wwitzel3alexisb: restarting the hangout :)14:02
dimiternjam, reviewed; just one comment - sorry it took so long :/14:05
ericsnowdimitern: you have a minute to take a look at a PR? https://github.com/juju/juju/pull/53014:30
alexisbTheMue, on my way, but will be a few minutes late15:01
TheMuealexisb: ok15:01
ericsnowperrito666, natefinch, wwitzel3: standup?15:06
perrito666ericsnow: going, just fighting with my hangut15:07
=== urulama is now known as uru-afk
natefinchemail I just got: "This is to notify you that the sum of 800,000 Britain Currency has been Donated to you by Colin Chris Weir Donation Scheme. for claims provide the below details"15:39
gsamfiraahh, they toned it down a bit. Used to be 10X that :P16:09
natefinchI just though "britain currency" was cute16:19
natefinchAlso, funny they call it a "donation scheme"16:19
natefinchfwereade: at some point I'd like to pick your brain about charm feature flags.  I was hoping to work on it this week, but I'm less certain now, after talking to rick_h__ about it.16:25
natefinchsinzui: did you make progress on getting godeps in the tarball?  No big deal if it's not in yet, just checking.16:25
fwereadenatefinch, dammit, I wanted to talk to you today, but the babysitter's just arrived and I'm trying to finish an email16:25
fwereadenatefinch, what time is it for you now?16:25
natefinchfwereade: 12:3016:25
fwereadenatefinch, cool, I hope I can get back to you in the next 5.5h16:26
natefinchfwereade: cool, ping me when you have time.  I appreciate you making time for it in your evening.16:26
sinzuinatefinch, I failed. I cannot make it run run in unittests. But this morning I realised I want to run that as a precheck before the tarball is created. I hope to have that sorted out this week. Sorry, it wasn't an easy change, and I need to focus on putting 1.20.5 into utopicbefore Friday16:28
alexisbjam, you doing the cloudsigma call today16:31
natefinchsinzui: no problem.  It's not pressing.  It's a nice to have.  What I can do is make the test skip itself for now if godeps doesn't exist, that way it can still help developers (who should have godeps).16:39
sinzuinatefinch, thank you that works for me because godeps does exist when we make the tarball.16:46
rogpeppe1natefinch: did you consider making that godeps test part of CI rather than an actual test in the suite?16:49
natefinchrogpeppe1: it's at least as important for developers as CI..... since innumerable times I've seen people ask why something was failing, only to figure out later that their dependencies were out of date16:49
natefinch(which has happened to me, as well)16:50
natefinchsinzui: I would like to make it mandatory at some point.  If you're developing on juju, you *need* godeps... I don't want someone to start hacking on juju and wonder why it's broken, just because they haven't run godeps.16:51
sinzuiericsnow, I am indeed very busy today I will reply before I EOD. Hp is my preferred could this week, and the the juju-core-slave machine might be ideal for setting up review board there. I need to think about your questions in more details though16:52
ericsnowsinzui: no worries :)16:52
rogpeppe1natefinch: ok. BTW, in your test, why not just use diff?16:54
natefinchrogpeppe1: what diff?16:57
rogpeppe1natefinch: diff(1)16:57
natefinchrogpeppe1: windows?16:57
rogpeppe1natefinch: no diff on windows?16:58
natefinchrogpeppe1: lol, no :)16:58
gsamfiragit diff works :P16:58
gsamfiraor were you referring to the manual? :P16:58
gsamfirain which case...no :D16:59
rogpeppe1gsamfira: i was referring to a command-line diff tool. i guess git has access to one, but perhaps it's not available for general use16:59
gsamfirait is available17:00
rogpeppe1natefinch: ^17:00
gsamfiraif you have Git installed its in C:\Program Files (x86)\Git\bin\diff.exe17:01
gsamfiraalong with less, grep, etc17:01
gsamfiraa small slice of home when having to work on windows17:01
rogpeppe1gsamfira: hmm, but i guess that won't be in the path, so you'd need some custom logic to work out how to find it (perhaps it's not in C:)17:01
gsamfirasetx PATH "%PATH%;C:\Program Files (x86)\Git\bin"17:02
gsamfiraand there ya go17:02
natefinchthe last thing I want is yet another external dependency17:02
gsamfiraif you set up your own system17:02
gsamfirayou know where it is :)17:02
rogpeppe1natefinch: out of interest, what proportion of our tests pass under windows?17:02
gsamfirawell, yes...an external dependency sucks :)17:02
rogpeppe1natefinch: you're probably right. but you could reduce your "monstrosity" by factoring out a "diff" function (diff(s, t string) []string, or something17:03
natefinchrogpeppe1:  a bunch, and that's next on our list of things to do - get tests passing on windows (except those we deem we can skip on windows because we just can't support them, like containers)17:03
natefinchrogpeppe1: yep17:03
gsamfiramost tests on Windows fail because of the same reasons: unsanitized paths, short paths vs long paths, directory names containing illegal characters on windows (like ":")17:05
gsamfirathere will be a few pull requests this week to fix some of them17:07
gsamfirathe new github.com/juju/juju/wrench package also breaks tests on windows17:07
natefinchwwitzel3: want to do our 1:1?17:19
wwitzel3natefinch: yep17:19
=== uru-afk is now known as urulama
natefinchheh, I had to fix dependencies.tsv on my PR that tests dependencies.tsv, because it was wrong.18:07
=== ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: 1359170, 1359333
sinzuinatefinch, can you find an engineer to look into bug 135933318:58
mupBug #1359333: win client thinks the cloud-init log is on the c drive <ci> <regression> <windows> <juju-core:Triaged> <https://launchpad.net/bugs/1359333>18:58
natefinchsinzui: crud. Yep.18:59
natefinchfwereade: forgot about the all team meeting tonight, I'm going to have to run to get some stuff done before then. Hopefully we can talk in the morning.20:06
thumper(╯°□°)╯︵ ┻━┻21:51
thumpermenn0: still need that review?21:59
thumpercan do it while all tests running21:59
menn0thumper, yes pleased21:59
menn0please even21:59
menn0(urgh... I have had just over 2 hours sleep - sick kids suck)21:59
thumpermenn0: trade you https://github.com/juju/juju/pull/56322:02
menn0thumper, done22:02
thumpermenn0: that was quick, did you read it ?22:03
* thumper pokes22:03
menn0thumper, no I meant "yes, that trade sounds fair"22:03
ericsnowmenn0: when you get a chance could you take a look at https://github.com/juju/juju/pull/53022:04
menn0the fun thing about me reviewing your change is that you then have to review my review :)22:04
alexisbmenn0, heh22:04
menn0thumper, maybe we ask dave or someone else to do that for this situation22:05
menn0ericsnow, will do22:05
ericsnowmenn0: thanks!22:06
menn0let me get this review done and another small PR up first22:06
menn0thumper, regarding your PR... is it safe to assume that people won't/can't upgrade directly from pre 1.18 to post 1.20?22:08
menn0thumper, they have to go through an intermediate 1.18 release first?22:08
thumpermenn0: yes22:08
menn0actually 1.18 and 1.2022:08
thumperthat has always been the approach22:09
thumperthe idea with the upgrade steps is that now it is *possible* to jump22:09
thumperbut we don't support it yet22:09
menn0thumper, is that enforced?22:09
menn0thumper, found the bit that enforces it: validate() in upgradejuju.go22:12
thumpernot explicitly I think22:12
menn0shame that it's enforced on the client side22:12
thumperoh, good :)22:12
menn0thumper, with the new release number scheme it's not quite right either...22:13
menn0thumper, the logic tries to find the next stable version by bumping minor by 1 if the current version is a dev release and by 2 otherwise22:15
menn0thumper, that's no longer right is it?22:15
thumperplease file a bug22:15
menn0thumper, already doing it :)22:16
menn0thumper, when does the new scheme start? 1.20? i.e. will the next stable release be 1.21?22:17
thumpermenn0: yes22:17
thumper1.21 will be a stable release22:17
waiganithumper:  talking of reviews: https://github.com/juju/juju/pull/55322:18
* thumper nods22:18
menn0thumper, please check the milestones and priorities for bug 135943522:24
mupBug #1359435: Next version selection for upgrades is no longer correct <juju-core:New> <juju-core 1.20:New> <https://launchpad.net/bugs/1359435>22:24
menn0thumper, review done.22:28
* thumper does a bootstrap 22:28
menn0thumper, so the stateservinginfo doc was only being created via the functionality for dealing with legacy envs? nice.22:29
thumperfound that by failing tests when I removed it22:29
thumperlocal bootstraps fine22:30
menn0thumper, cool22:31
menn0thumper, I've replied to your review comments22:39
* thumper nods22:39
thumpermenn0: that comment makes no sense..22:43
thumperThis is capturing the upgradedToVersion field from the agent's config as the agent starts up - i.e. the version we've just upgraded from22:43
thumperI read that as:22:43
thumper'upgradedToVersion' is the version we have just upgraded from22:43
thumperwhich seems arse backwards22:43
menn0it is22:43
menn0upgradeToVersion is the version for which upgrades have successfully run22:44
menn0it gets updated to version.Current once upgrade steps have successfully run22:44
menn0but when a machine agent starts up, before the upgrade steps have run22:45
menn0it'll be set to the previous agent version22:45
thumperok, that kinda makes sense22:45
menn0I didn't name this stuff22:45
menn0it's before my time22:45
menn0thumper, I'm so used to this part of the code now that it doesn't even seem strange to me anymore22:47
menn0but I do remember finding it confusing initially22:47
* thumper nods22:47
thumperwallyworld: I took a very quick look at bug 1359333 (blocking landing)22:59
mupBug #1359333: win client thinks the cloud-init log is on the c drive <ci> <regression> <windows> <juju-core:Triaged> <https://launchpad.net/bugs/1359333>22:59
thumperbut I can't see how the diffs cause the failure22:59
wallyworldthumper: ok, i'm about to go into a meeting, but can look after that22:59
* thumper has standup now too23:00
davecheneywallyworld: https://github.com/juju/juju/commit/3a8f263f8c2977241fc39450e9fb47b4d56fc97523:24
davecheneythis was the commit that broke CI23:24
davecheneyplease revert it23:25
mupBug #1359333: win client thinks the cloud-init log is on the c drive <ci> <regression> <windows> <juju-core:Triaged> <https://launchpad.net/bugs/1359333>23:25
wallyworlddavecheney: hi, just finished a call, looking now23:31
davecheneywallyworld: the bug is it uses paths.DataDir which uses version.Current23:32
davecheneyand version.Current, when you're using windows tools, will using windows tools23:32
davecheneythis is reason number 9001 why version.Current is lethal23:32
wallyworlddavecheney: do these guys doing windows develppment event test their stuff on windows before landing?23:32
davecheneywallyworld: that's between them and the person that reviewed their code23:33
wallyworldwell i can understand how stuff gets through a review, but not to test on the platform you are developing for....23:34
thumperholy cow: 44 changed files with 1,954 additions and 360 deletions.23:38
thumperwallyworld: I don't think they are really testing it manually23:38
thumperthat should really be raised as a concern23:38
thumperalexisb: ^^23:38
wallyworldthumper: we'll discuss tonight at team leads' meeting23:39
wallyworldthumper: davecheney: so are we 100% sure that https://github.com/juju/juju/pull/189 is the bad one? i'll just hit the Revert button if so23:40
davecheneywallyworld: not 100% sure23:40
davecheneywhat's the worse that can happen ?23:40
thumperwallyworld: can't say 100%, but indications are high23:40
davecheneywe just re apply iy23:40
davecheneyman version.Current is a timb bomb23:42
alexisbthumper, wallyworld, davecheney: sorry I have to run, but I added a note about the windows workload reviews to the leads agenda so we wont forget23:42
wallyworld+1, and not just reviews, actual proper testing and due dilligence prior to landing23:42
wallyworlddavecheney: thumper: reverted, let's now wait to see if CI is happy again23:46
thumperdavecheney: did you mark it as fixes the bug so ci is unblocked?23:47
thumpersorry davecheney23:47
thumperwallyworld: ??^^23:47
davecheneythumper: thats ok thumper23:47
davecheneywallyworld: how long do we have to wait ?23:47
wallyworldthumper: i didn't because the revert didn;t go through ci23:48
wallyworldbut i can mark the bug as resolved23:48
wallyworldpending CI running it's tests23:48
wallyworldwe can reopen if needed23:48
* thumper wonders if it should have...23:48
davecheney\o/, one down, one to go23:49
wallyworldthumper: maybe it should have, but i'll pull trunk now and run the tests23:50
wallyworldFAIL: dependencies_test.go:42: dependenciesTest.TestGodepsIsRight23:54
wallyworld    ...23:54
wallyworld    c.Fatal(string(out))23:54
wallyworld... Error: godeps: no version control system found for "/usr/lib/go/src/pkg/bufio"23:54
wallyworldgodeps: no version control system found for "/usr/lib/go/src/pkg/bytes"23:54
wallyworldthumper: davecheney: you guys seen ^^^^ as a result of PR 495?23:55
thumperhuh? nope23:56
thumperI do think that a test for godeps isn't right23:57
thumperand the make rules I added were better23:57
thumperbut ..23:57
* thumper shrugs23:57
* wallyworld will reply to list23:57

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