/srv/irclogs.ubuntu.com/2014/03/28/#juju-dev.txt

davecheneywallyworld: it sounds like you have a lot on your plate00:00
davecheneyi'll look at testing ec2 and hp deployments today now that RT is closed00:00
wallyworldido00:00
wallyworldawesome :-)00:00
davecheneywallyworld: fair warning, there are some remaining tools selectino bugs on ppc00:00
wallyworldok00:00
davecheneymainly because precise/ppc64el is not a thing00:00
davecheneyi'll do what I can to resolve them myself00:00
wallyworlddavecheney: i'm happy to fix also00:01
davecheneywallyworld: lemmie figure out what they are first00:01
wallyworldok00:01
davecheneyall I can see at the moment is tests pass with gccgo/amd6400:01
davecheneyand fail with gccgo/ppc64el whinging that no tools are available00:01
wallyworlddavecheney: is there simplestreams metadata for ppc64el?00:13
wallyworldthat would be a likely cause00:13
davecheneywallyworld: in the test fixures ?00:34
davecheneysinzui: https://bugs.launchpad.net/juju-core/+bug/129870800:35
_mup_Bug #1298708: bzr: bzr unit tests are not properly isolated <ppc64el> <juju-core:Triaged> <https://launchpad.net/bugs/1298708>00:35
davecheneythis is sort of a big failure00:35
davecheneyhow does this pass in CI ?00:35
wallyworlddavecheney: i think so. what tests are failing?00:35
davecheneywallyworld: hang on00:35
davecheneytalking at cross purpises00:35
davecheneywhat are you talking about00:35
wallyworldthe missing ppc64el tools metadata00:36
davecheneywallyworld: in that case I agree00:36
davecheneylet me check quickly00:36
* davecheney flails around a bit00:39
davecheneywallyworld: where in the source are the test fixtures ?00:40
wallyworlddepends on what tests are filing00:40
davecheneywallyworld: you mentioned that there was sample simples streams data00:41
davecheneywhere in the source are those fixtures kept ?00:41
wallyworldthere's several tests with different sources00:41
davecheneysure00:42
wallyworldthere's some in environs/simplestreams/testing00:42
wallyworldbut each provider also sets up its own data00:42
davecheneyubuntu@winton-02:~/src/launchpad.net/juju-core/environs/simplestreams/testing$ ls00:42
davecheneytesting.go00:42
davecheneyahh, rigth00:42
davecheneynow I understand00:42
davecheneyafk for a bit00:44
perrito666eu1Ahloo01:36
perrito666eu01:36
perrito666ouch01:37
perrito666:p01:37
sarnoldnot a bad password, as passwords go..01:37
perrito666sarnold: nah, altough I think I should definitely not use it for something else than my laptop :) since apparently I can have the screen locker on but the focus on the chat01:39
sarnoldperrito666: ah :) https://bugs.launchpad.net/ubuntu/+source/unity/+bug/129221701:41
_mup_Bug #1292217: lightdm screen lock has triggered but keyboard is still connected to the main session <amd64> <apport-bug> <lockscreen> <third-party-packages> <trusty> <unity (Ubuntu):Confirmed> <https://launchpad.net/bugs/1292217>01:41
perrito666sarnold: lol01:41
=== vladk|offline is now known as vladk
mattywwhen using goyaml and json is there a way I can tell struct tags to marshal/unmarshal the same for both systems - the syntax in my head is something like `json,yaml:foo-bar`03:08
davecheneymattyw: yes03:18
davecheneythe syntax is slightly different03:18
davecheneyfrom memory `yaml:foo-bar,json:foo-bar`03:18
davecheneythere might even be some "'s in there as well03:18
mattywdavecheney, I found that `json:"foo" yaml:"foo"` works - but it would be nice not to have to repeat the key03:18
mattywit would be nice to not need to repeat the key - but I guess that isn't the way the world works03:19
davecheneymattyw: well puyt03:19
=== vladk is now known as vladk|offline
mattywdavecheney, thanks dave - I'll let you get back to implementing generics03:20
davecheneymattyw: you're welcome03:20
davecheneymattyw: you're welcome03:20
davecheneyaxw: wallyworld i am looking at this bug03:38
davecheneyhttp://paste.ubuntu.com/7165797/03:38
davecheneyand I am really concerned that public and private addresses are being mixed up03:38
davecheneylooking at the code, so far I see they are handled separately03:38
davecheneybut I woner if at some point they are mushed into a map, then someone is grabbing the first key from the map and assuming that is the first item pushed into the map03:39
wallyworldcould be03:39
davecheneywallyworld: it looks liek they are stored in separate fields in the machine/unit document03:39
davecheneysorry it would be the machine document03:39
wallyworldi think so yes03:39
axwdavecheney: they are indeed put into a map03:39
davecheneyaxw: ORLY03:40
axwdavecheney: to remove dupes. does order matter here? SelectPublic/SelectInternal should still do the right thing...?03:40
davecheneyaxw: is that in state, or in instance ?03:40
wallyworldit looks just like another map orderin fting03:40
wallyworldthing03:41
davecheneywallyworld: the first bug is03:41
wallyworldin the test only03:41
davecheneythe seconds are a little weirder03:41
axwdavecheney: they're merged in state.Machine.Addresses03:41
axwstate/machine.go03:41
davecheney        addresses := []instance.Address{03:41
davecheney                instance.NewAddress("127.0.0.1"),03:41
davecheney                instance.NewAddress("8.8.8.8"),03:41
davecheney        }03:41
davecheney        err = machine.SetAddresses(addresses)03:41
davecheney        c.Assert(err, gc.IsNil)03:41
davecheney        address, ok = s.unit.PublicAddress()03:41
davecheney        c.Check(address, gc.Equals, "8.8.8.8")03:42
davecheneythis is weird03:42
davecheneywhat defined the public and private addresses ?03:42
davecheneyit feels like the order in which the are added03:42
davecheneythe first is always private03:42
davecheneythe rest are public03:42
wallyworldthere's an lgorithm03:42
wallyworldinternalAddressIndex03:43
axwdavecheney: addresses have a scope which says whether they're public/private03:43
wallyworldi think it's just the test03:43
davecheneywallyworld: the test is failing because s.unit.PublicAddress() returns the priovate address03:43
axwdavecheney: see NetworkScope in instance/address.go03:43
wallyworldthe test is inserting 2 addresses into a map without enough info for one to be considered private over another03:43
wallyworldthe test needs improving i think03:43
davecheneywallyworld: there aren't any maps on that code sample I showed03:43
davecheneyi'm concerned that this is a bug in the code03:43
davecheneynot the expected in the test03:44
wallyworlddavecheney: that's not the point03:44
wallyworldthe code picks an address based on the internalAddressIndex algorithm03:44
wallyworldand the addresses in the test don't appear to have enough info for one to be picked over another03:44
wallyworldso an arbitary one is chosen03:44
davecheneywallyworld: right, so that is a bug in unit.PublicAddress03:45
axwif they're all public or private, I think order matters03:45
davecheneynot the test03:45
wallyworldthat's based on a very quick look at the code03:45
axwbut that's arbitrary03:45
wallyworldit's not a bug03:45
wallyworldif more than 1 address matches, it just picks one03:45
wallyworldso the test needs to insert addresses with more info03:45
axwdavecheney: instance.NewAddress just gives you an address of "unknown" type03:46
wallyworldso that 1 is definitely private vs public03:46
wallyworldthe address type needs to be set03:46
axwso neither 127.0.0.1 nor 8.8.8.8 in that test are public03:46
axwit just happened to be picking 8.8.8.8 before because it was the last in the list03:46
davecheneyaxw: right, thanks03:46
davecheneyaxw: wallyworld thanks03:46
davecheneynow I understand03:46
davecheneyi'll fix it03:46
* axw hopes nothing actually relies on that03:47
wallyworlddavecheney: sorry about bad explanation, test is shit03:47
davecheneyaxw: you'll be surprised :)03:47
davecheneySpot the bug, Holiday04:11
davecheneyMay 201407/05/2014 - 07/03/2014Awaiting Sign Off04:11
davecheneyaxw: wallyworld instance.NewAddresses would appear to be a footgun04:21
axwdavecheney: yeah, it should probably be in a testing package, if that04:22
wallyworldit doesn't really set the scope does it04:22
axwit's only used by tests04:22
davecheneyaxw: should I delete it ?04:24
davecheneyit never sets the scope04:24
davecheneyso its value would be ... dubiois04:25
davecheneyinstance.Address{Value: "x.y.z.q"} does the same thing04:25
davecheneyhmm, NewAddress does do more that I just said04:29
davecheneybut it always sets the scope to unknown04:29
axwdavecheney: sorry, was making lunch. I don't know, depends on how it makes the tests look. I'd opt for just moving it, so it's not accidentally used in a place where it matters04:36
davecheneyaxw: moving it out of the instance API and as a helper04:46
davecheneythat would certainly reduce it's lethality04:46
davecheneylet's see what breaks04:49
davecheneyaxw: http://paste.ubuntu.com/7166620/ line 17405:04
davecheneyi wonder if this related to the last issue05:04
* axw looks05:10
axwdavecheney: broken test; it's setting the wrong scope on the "public" address05:11
davecheneyaxw: yet it passed up until now ....05:18
axwdavecheney: yes, by implicitly relying on map ordering05:18
axwit just happened to pick the internal address previously05:19
davecheneywaaaaaaaaaaaaaah05:19
davecheneyok, will fix05:19
davecheneywallyworld: i don't think i'm going to get to adding simple streams data for ppc64el05:20
davecheneyi'll log a but05:20
davecheneybug05:20
davecheneybut I know you have better things to do05:20
wallyworldok05:20
wallyworldi'm finishing the joyent provider05:20
wallyworldi'll try and get to it real soon now05:20
davecheneywallyworld: no no05:21
davecheneythat is what I am saying05:21
davecheneyi need to log a bug for my status report05:21
davecheneybut don't think you need to do it05:22
wallyworldok05:22
davecheneyaxw: OMG05:41
davecheney        cloudLocalAddress := instance.NewAddress("cloudlocal")05:41
davecheney        cloudLocalAddress.NetworkScope = instance.NetworkCloudLocal05:41
davecheney^ even other callers of NewAddress don't trust it05:41
axw:)05:45
axwit's a bit pointless there isn't it05:45
axwdavecheney: if you do move it, maybe rename it too. NewUnknownAddress05:45
davecheneyaxw: i'm thinking instead adding a manditory scope parameter instead05:51
davecheneythen it can't be misused05:51
axwdavecheney: sounds sensible05:51
davecheneyfamous last words05:51
davecheneycummon and land my change you mongrel06:02
davecheneyjam1: has the bot crapped itself ?06:21
davecheneyor is it just reeeeeeeeeeely slow ?06:21
davecheneyaxw: instance.NewAddress is barely used, only one case in the whole codebase06:25
axwdavecheney: apart from tests? yeah, that one can just be changed to create an address struct literal06:26
davecheneyyeah, quite a few tests06:30
davecheneyi'm going to go with the plan of adding an explict scope argument06:30
davecheneythis will probably make people questino the values passed to NewAddress06:30
wallyworldaxw: git 101 question - i've forked a github branch, cloned locally into the src tree i use for juju-core development, comitted and push some changes. How do i now revert  the branch on disk to the original from github?06:47
wallyworldi'd use bzr switch06:48
wallyworldwhat's the git equivalent?06:48
davecheneywallyworld: git checkout $BRANCH ?06:50
wallyworlddavecheney: ok, ta. i'm a total git noob.06:50
davecheneywallyworld: i *may* not have told you the right thing06:50
davecheneythere is also git reset --hard06:50
axwwallyworld: depends on whether you branched to start with06:51
davecheneybut that might be a hammer to crack an egg06:51
wallyworldi must say, having created a couple of pull requests on github, bzr + laucnhapd is MUCH nicer :-(06:51
wallyworldaxw: i just did a go get -u originally06:51
wallyworldi guess i could rerun that?06:52
axwwallyworld: so, I think what you probably should've done is branched, and pushed the branch to GitHub06:52
axwbut it's a bit late06:52
axwno, your push would've pushed to the master branch06:52
wallyworldaxw: i forked06:52
wallyworldand then cloned06:52
wallyworldthat's what the online help said to do06:53
axwwallyworld: right, but forking on github creates a repo + branch06:53
axwbranches06:53
axwso you've modified the master branch in your fork06:53
wallyworldmy push went to my copy of the branch06:53
wallyworldyes06:53
axwyep... now what do you want to do exactly?06:53
axwwallyworld: bear in mind I'm a git noob too :)06:54
wallyworldrevert, on disk, back to what was there before i deleted the go get version and clone my version06:54
wallyworldah, i thought you were a git master :-)06:54
wallyworldi used git *for the very first time* just 10 minute ago06:54
axwwallyworld: probably better just to create a new branch off the old commit06:54
wallyworldwouldn't checkout/clone be better? i wonder what go get does06:55
wallyworldi could just rm the dir and run go get again06:55
axwyou could, but you'll get the same stuff back06:56
wallyworldthat's what i want for now06:56
axwwallyworld: you want the same thing you pushed?06:56
axwthat's what you'll get back06:56
wallyworldi'm just trying to figure out how the do the equivalent of "bzr switch"06:56
axw"git checkout <branch>"06:57
wallyworldand that will overwrite what's there?06:57
wallyworldta, will try that06:57
axwthat'll change your working tree06:57
wallyworldcoolio, that's what i want06:57
wallyworldit's going to be "fun" switching over06:58
axwit's not so bad, just a bit different06:59
wallyworldi've read and heard that bzr's model and workflow is *much* nicer, so it will be a pain i think06:59
davecheneywallyworld: jam1 axw, https://code.launchpad.net/~dave-cheney/juju-core/104-fix-lp-1298703/+merge/21319507:19
davecheneystill hasn't landed07:19
davecheneycan someone kick the bot ?07:19
wallyworldsure07:19
davecheneyta much07:20
wallyworldPreparing to merge https://code.launchpad.net/~dave-cheney/juju-core/105-fix-lp-129878907:23
wallyworldtests running07:23
davecheneywallyworld: right07:24
davecheneythanks for checking07:24
wallyworldnp07:24
davecheneytests take 30 mins on my laptop with one cpu07:25
davecheneyi can imagine they are even slower on canonistack07:25
wallyworldoh yes07:25
rogpeppemornin' all07:32
davecheneyrogpeppe: o/07:33
davecheneywhile we're waiting for the bot, https://codereview.appspot.com/8133004407:33
rogpeppedavecheney: good call - i've been meaning to do that for a while07:34
davecheneyrogpeppe: and then I can change SetAddresses to be var arg07:34
davecheneyand remove all the use of NewAddresses07:34
rogpeppedavecheney: that doesn't seem quite right - addresses have more info than is just in the string07:35
davecheneyrogpeppe: indeed, that was the subject of todays bug07:36
davecheneyNewAddress always creates an address with scope NetworkUnknown07:36
davecheneyhttps://code.launchpad.net/~dave-cheney/juju-core/105-fix-lp-1298789/+merge/21319807:36
rogpeppedavecheney: are we using NewAddress in production code?07:36
davecheneyhttps://code.launchpad.net/~dave-cheney/juju-core/104-fix-lp-1298703/+merge/21319507:36
rogpeppedavecheney: we shouldn't really.07:36
davecheneyrogpeppe: i'm going to do two things07:36
davecheney1. make the caller always specify the scope07:36
davecheney2. make sure we're only using it in tests07:37
davecheneyrogpeppe: actually, if we do 1 then 2 may not be necssary because it will become less dangerous to call that function07:38
rogpeppedavecheney: that seems ok to me, except that i always presumed that Unknown was there for when a provider genuinely doesn't know the scope of an address07:38
davecheneyrogpeppe: unknown can be returned for both a public and private address07:39
davecheneythat seems wrong07:39
rogpeppedavecheney: i'm not sure07:39
rogpeppedavecheney: we should talk to mgz about this - it's his scheme07:39
rogpeppedavecheney: the other thing i'd like to do with addresses is drop the explicit type field07:40
davecheneyrogpeppe: have a look at the bug attached to this one, https://code.launchpad.net/~dave-cheney/juju-core/104-fix-lp-1298703/+merge/21319507:40
rogpeppedavecheney: that's not a production code bug, is it?07:41
=== vladk|offline is now known as vladk
vladkgoog morning07:42
vladkgood morning07:42
rogpeppevladk: hiya07:44
rogpeppeaxw: you've got a review: https://codereview.appspot.com/81700043/07:54
axwthanks07:55
vladkdimitern: morning07:58
axwrogpeppe: for client caching addrs, is it reasonable to only do it when using the configstore? and assume that Environ.StateInfo will get updated addresses too?07:59
dimiternmorning07:59
rogpeppedimitern: hiya07:59
axwmorning vladk, dimitern07:59
rogpeppedimitern: i'd appreciate your thoughts on this: https://codereview.appspot.com/81570043/08:00
dimiternrogpeppe, looking08:00
rogpeppedimitern: (particularly after our chat yesterday...)08:00
rogpeppeaxw: for the former, i think so08:01
rogpeppeaxw: without the configstore, i don't there *is* any way to cache addresses08:01
davecheneyaxw: didn't you fix this ? http://paste.ubuntu.com/7167180/08:01
axwrogpeppe: hah, good point :)08:01
rogpeppeaxw: i'm not sure what you mean by the latter08:01
axwrogpeppe: well actually...08:01
axwrogpeppe: we may have the configstore, but it may have out of date info08:02
axwrogpeppe: so we try using StateInfo to get addrs from external storage08:02
axwdavecheney: looking08:02
rogpeppeaxw: nothing should be using StateInfo08:02
davecheneyaxw: i saw you proposed a fix yesterday08:02
davecheneymaybe that hasn't laneded08:02
davecheneyi'm pretty sure i'm up to date08:02
axwdavecheney: I thought so too, but maybe not :(08:02
rogpeppeaxw: because nothing connects directly to mongo08:02
axwrogpeppe: I'm talking about environs.Environ.StateInfo, which gets provider-state from storage08:03
axwyou know, the one we chatted about the other day08:03
* rogpeppe feels bad about tests depending on map ordering.08:03
davecheneyrogpeppe: go 1.3 for the win08:03
rogpeppeaxw: ah yes, i think it's reasonable to assume that the info from StateInfo (actually we'll rename it to APIInfo soon) is maintained08:04
* rogpeppe will try running tests on tip08:05
axwrogpeppe: excellente, thanks08:07
axwrogpeppe: you may not have seen my email yet, so: is the APIHostPorts client API still necessary if we return them via Login?08:08
axwI would prefer to do it via Login08:08
rogpeppeaxw: i think it does no harm - we can leave it in08:08
axwok08:08
rogpeppeaxw: but i think we should return the results from Login anyway08:08
axwrogpeppe: yeah, that's what I am working on right now08:09
axwrogpeppe: simpler and more efficient08:09
rogpeppeaxw: <308:09
fwereadeguys, today may be challenging, cath is away and laura is a bit sick08:20
fwereadeupside: she's fairly subdued08:21
fwereadedownside: I will be off and on at complete random today08:21
fwereadejust flag me in here with reviews you need and I'll get to them as I can08:21
dimiternfwereade, take a look at this https://codereview.appspot.com/81570043/ i'd like to know your thoughts as well08:25
rogpeppedimitern: fwereade has already reviewed it (although perhaps not completely)08:28
dimiternrogpeppe, ah, sorry, haven't reached the end08:29
dimiternrogpeppe, i don't really get why do you have a clone method on the configsetteronly?08:39
dimiternrogpeppe, imo once you have configsetteronly, you should've got it from a clone of an existing config08:40
dimiternrogpeppe, what if you forget to call clone?08:40
rogpeppedimitern: there's not really any point on having it on the Config, because when you've got a Config, it should be immutable08:40
rogpeppedimitern: if you've got a mutable config (ConfigSetter or ConfigSetterWriter) you are responsible for cloning the config to hand out to potentially concurrent clients08:41
dimiternrogpeppe, and you're handing out a clone with a clone method itself?08:42
rogpeppedimitern: that's reasonable, i think, because that's the case in only a very small amount of code08:42
rogpeppedimitern: ?08:42
rogpeppedimitern: Clone returns Config, which doesn't have a clone method08:42
dimiternrogpeppe, oh ok08:43
dimiternrogpeppe, reviewed08:59
rogpeppedimitern: thanks a lot09:10
rogpeppedimitern: responded09:11
dimiternrogpeppe, will look in a moment09:13
dimiternrogpeppe, i'm confused about upgrades + migrate config09:15
dimiternrogpeppe, istm now we're migrating the config in-memory only and then what?09:15
rogpeppedimitern: the caller is responsible for actually writing the config09:16
dimiternrogpeppe, i'm asking about that particular caller09:16
dimiternrogpeppe, the upgrade step09:16
rogpeppedimitern: the upgrade step doesn't need to write the config09:17
dimiternrogpeppe, you've changed migrate not to write, but not changed agentconfig upgrade step calling migrate to write after calling migrate09:17
dimiternrogpeppe, that's its only purpose in life09:17
rogpeppedimitern: the write happens outside of PerformUpgrade09:17
dimiternrogpeppe, migrate and write the updated config09:17
rogpeppedimitern: which is why PerformUpgrade is called in a ChangeConfig-called function09:18
dimiternrogpeppe, have you tested this with an actual live test upgrading 1.16 to 1.18?09:18
rogpeppedimitern: not yet09:18
dimiternrogpeppe, please do :)09:19
rogpeppedimitern: will do09:19
rogpeppedimitern: is there a particular reason you can see why this scheme won't work?09:19
dimiternrogpeppe, i'm not saying it won't work, i'm just concerned not to introduce regressions09:20
rogpeppedimitern: ok, that's cool09:20
axwrogpeppe: last one, https://codereview.appspot.com/8178004309:25
axwrogpeppe: I'll look at parallel connection in the API client on Monday, unless you have something else more important?09:26
rogpeppeaxw: thankjs09:26
rogpeppeaxw: i'll have a think09:26
axwnps09:26
dimiternrogpeppe, lgtm09:29
wwitzel3hello09:36
davecheneystupid bot, how long is this going to take, https://code.launchpad.net/~dave-cheney/juju-core/104-fix-lp-1298703/+merge/21319509:40
rogpeppedavecheney: looks like the bot is hung09:42
rogpeppedavecheney: i'll kick it09:42
davecheneythanks09:43
davecheneyotherwise i'll check it tomorrow09:43
dimiterndavecheney, hey09:43
davecheneydimitern: hello09:43
davecheneyi'm not really here09:43
dimiterndavecheney, how do you run juju on ppc btw?09:43
davecheneyi have a scotch downstairs which is getting impatient09:43
davecheneydimitern: how do I run it ?09:43
davecheneygo build launchpad.net/juju-core/...09:43
davecheneymayube I don't understand the question09:44
rogpeppeds09:44
voidspacemorning all09:44
rogpeppedavecheney: bot now going again09:44
davecheneythanks09:44
davecheneythis is the changing of the guard09:44
rogpeppedavecheney: it's got to axw's branch first though, i'm afraid09:44
davecheneyrogpeppe: no worries09:44
davecheneynot much hurry now09:44
davecheneyme -> EOW09:44
rogpeppevoidspace: hiya09:44
dimiterndavecheney, I mean on a vm or what machine09:44
davecheneydimitern: we have ppc hardware available09:45
dimiterndavecheney, nice!09:45
davecheneydimitern: do you want a machine09:45
davecheneyor are you just curious09:45
davecheneyyou can have a machine09:45
davecheneythey are for the juju team09:45
dimiterndavecheney, nah, just curious :)09:45
davecheneybut it is a non zero amout of work09:45
dimiterndavecheney, i have enough on my plate already09:45
davecheneydimitern: right, yes, we have some ppc machines which are cut up into vms so we can each work without having to negotiate for port 17017 and things09:46
dimiterndavecheney, good to know, i assume arm vms as well?09:46
davecheneydimitern: not so much09:46
davecheneythese ppc machines are kvm vms09:46
davecheneyso they are mostly wire speed09:46
davecheneythe arm stuff is all inside the arm fast model simulator09:46
davecheneyit uses a lot more cpu power, and produce much less impressive results09:47
davecheneyfor 32 bit arm ubuntu has an AMI which runs userland inside QEMU which is ok09:47
davecheneybut still very slow09:47
davecheneythat is what sinzui uses for testing and releasing the current arm builds09:47
davecheneyadieu09:49
fwereaderogpeppe, 530-lose-hardware-characteristics10:09
rogpeppefwereade: oh darn, it failed10:11
rogpeppefwereade: will fix10:11
fwereaderogpeppe, cheers10:11
rogpeppefwereade: re-approved10:15
rogpeppeafk10:17
rogpeppeback10:23
perrito666aghh, test, why wont you fail10:39
natefinchperrito666: not a problem I often have, sadly10:40
perrito666natefinch: I am really having problems replicating it :p10:41
natefinchmgz, fwereade: standup10:47
mgzgah10:52
perrito666mgz: aghh our ticket is still on todo :p10:55
mgzwell, we're not really doing what that ticket says :)10:56
perrito666haha10:56
perrito666mgz: did you get my full of not wonderful news mail?10:57
mgzI did, sorry, should have replied10:59
mgzI think the subcommand thing should just work though?10:59
perrito666ill re read it, i already forgot what I wrote in :p11:00
perrito666it was late11:00
voidspacenew rule - we're not allowed to create any more types called "State"11:07
natefinchno new files called cloudinit.go or bootstrap.go11:11
wwitzel3so just for myself, I started fixing go vet warnings, mainly as a way to force myself to look them up and learn them and explore different types user types in juju-core. My question is, do we ever merge cleanup like that? should I propse this or should I just toss it?11:20
mgzmergeit11:22
natefinchdefinitely.  Go vet is almost always complaining about something useful that might bite us later (or might even be biting us now and we don't realize it)11:23
natefinchI thought lbox ran go vet?11:24
wwitzel3I was of that thought as well, but when I went to run go vet manually I didn't even have it installed and lbox never complained I didn't have it.11:25
wwitzel3natefinch: so it might only run it if you have it?11:25
fwereadenatefinch, rogpeppe: how does MA-HA fit in with that agent-config branch of roger's?11:28
rogpeppefwereade: it's gonna need some work11:28
natefinchfwereade: rogpeppe said it breaks some stuff in our branch11:28
fwereadenatefinch, rogpeppe: yeah, I'm getting conflict twitches as I read it11:29
rogpeppefwereade: i think i know what to do - i plan on working on it next11:29
rogpeppefwereade: in conjunction with nate'n'wayne11:29
rogpeppefwereade: the MA-HA branch as is has potential problems with overwriting previous config settings, which was why i did what i did yesterday as a priority11:30
natefinchrogpeppe: was there stuff that was broken in our branch?  I really wanted to get it in ASAP and adding more work to it is disappointing11:30
rogpeppenatefinch: i believe there was11:30
fwereaderogpeppe, natefinch, wwitzel3: do you guys all have something productive to work on in the face of this?11:31
natefinchfwereade: I have to tweak the local provider's mongo name, from a review comment from andrew, that should be orthogonal to roger's stuff11:32
rogpeppenatefinch, wwitzel3: if you find yourself at a loose end, there are a good few things that are independently workable on11:32
voidspacesimple review for someone: https://codereview.appspot.com/8185004311:32
rogpeppenatefinch, wwitzel3: i'm just writing a final test for my config branch, then i'll be with you11:32
wwitzel3rogpeppe, fwereade : ok, I was planning on doing a review of the agent config stuff this morning and then after that I can work on what ever is needed or pair.11:33
natefinchrogpeppe: ok11:33
rogpeppewwitzel3: more eyes always appreciated :-)11:33
wwitzel3PTAL https://codereview.appspot.com/81570045/ (non-priority cleanup branch)11:34
rogpeppevoidspace: LGTM11:36
fwereaderogpeppe, natefinch: so, shall I WIP MA-HA?11:41
rogpeppefwereade: i think that's fair11:41
natefinchyep11:42
voidspacerogpeppe: thanks11:53
voidspacerogpeppe: but if I remove that error path then my branch is even more trivial!11:54
voidspacerogpeppe: but yeah, true enough... :-)11:54
rogpeppevoidspace: trivial branches are good :)11:54
voidspacerogpeppe: I can hear the flaming lips...11:54
rogpeppevoidspace: you can11:54
rogpeppevoidspace: through my headphones, or is there some bleedthrough to the computer?11:54
voidspacevoidspace: from the headphones I assume11:55
rogpeppevoidspace: i'll mute11:55
voidspacerogpeppe: hah, I wasn't complaining :-)11:55
voidspacebut fine11:55
voidspacerogpeppe: I'm making both changes you suggest and then I'll approve11:55
rogpeppevoidspace: great11:55
=== vladk is now known as vladk|lunch
axwrogpeppe: re APIHostPorts failing in Login, and it being a warning vs. error ...12:16
rogpeppeaxw: oh yes?12:16
axwrogpeppe: say the state method had a bug in it. how would we fix it?12:16
axwupgrading requires that we connect to the API12:16
axwkind of a problem in general...12:16
rogpeppeaxw: interesting point12:17
rogpeppeaxw: login also requires interacting with the state12:17
rogpeppeaxw: i think at some point we have to rely on our infrastructure working12:17
axwyeah, I'm just thinking about minimising the surface area for defects. but this isn't really increasing it much12:18
axwI'll make it fatal12:18
rogpeppeaxw: as a last resort, it would be possible to directly connect to mongo to change the agent version12:18
axwyep12:19
rogpeppeaxw: thanks. i think it's more useful as a fatal error than potentially returning empty API addresses12:19
rogpeppefwereade: am adding a test for PerformUpgrades returning an error. unfortunately i'm pretty sure that a substantial proportion of jujud UpgradeSuite is crackful12:20
rogpeppefwereade: it seems to rely on knowing what upgrades will be performed, but that's information that will change over time, and is really private to the upgrades package12:21
fwereaderogpeppe, ha, only testable by side-effect again? goddamn jujud12:21
rogpeppefwereade: no, it's easy enough to test properly12:21
rogpeppefwereade: (by mocking PerformUpgrades)12:22
rogpeppefwereade: i've already done that, but i'm just checking that you agree the upgrade-specific tests shouldn't be there12:22
rogpeppefwereade: i'm just trying to verify that the upgrades package itself has equivalent tests12:24
fwereaderogpeppe, I agree, they should only be tested enough in jujud to reassure us that jujud does run upgrades12:30
rogpeppefwereade: thanks12:30
perrito666man I must be the first person ever to be frustrated that a test is passing12:31
axwheh, intermittent failure? :)12:32
perrito666axw: sort of, only fails on CI and sinzui's machine12:33
perrito666I even created a stream to test this hoping to make it fail... yet it works12:33
axwoh environmental, even better12:33
perrito666altough it is fun, makes suspense, I have to wait a decent amount of time for the test to finish, so it builds suspense a lot while I do other things12:34
wwitzel3rogpeppe: for what it's worth, review for the agent stuff is up12:35
rogpeppewwitzel3: thanks12:41
rogpeppefwereade: if it's ok, i'd like to leave adding that test for another CL - it's a substantial refactor of UpgradeSuite, and pretty much orthogonal to the rest of that branch (and it's a preexisting thing too)12:43
=== vladk|lunch is now known as vladk
fwereaderogpeppe, I has a bit of a sad about removing the old tests and not replacing them13:09
rogpeppefwereade: i don't *think* i've removed any tests13:10
fwereaderogpeppe, ah, ok then, I got the impression that there were some in jujud that you were getting rid of13:10
rogpeppefwereade: not right now, but i intend to13:11
rogpeppefwereade: because they're inappropriate13:11
fwereaderogpeppe, that's great, cheers, sorry misunderstanding13:11
rogpeppefwereade: (the tests for specific upgrade behaviour)13:11
bodie_morning all13:11
rogpeppefwereade: i just don't want to burden this CL with the UpgradeSuite revamp13:11
rogpeppebodie_: hiya13:12
fwereadebodie_, heyhey13:12
fwereaderogpeppe, +113:12
rogpeppefwereade: does it LGTY then, given that?13:12
bodie_:)13:12
fwereaderogpeppe, it's had 2 LGTMs and I liked the direction, I can rereview if there's something in particular?13:13
rogpeppefwereade: no, that's fine13:14
fwereaderogpeppe, cool13:14
rogpeppefwereade: i always like it if everyone explicitly LGTMs :-)13:14
natefinchwwitzel3, rogpeppe: do you guys have a hangout I should join?  I'm just working on undoing my changes to local provider mongo service name13:37
dimiternrogpeppe, fwereade, mgz, vladk, quick review  https://codereview.appspot.com/81550046 ?13:38
rogpeppenatefinch: i'm in a hangout with michael currently, but i can join you two13:38
rogpeppenatefinch: (i'm just testing environment upgrades)13:38
dimiternfwereade, that's what you wanted ^^13:40
fwereadedimitern, looking13:40
natefinchrogpeppe: not necessary right now.  Once we need to figure out what changes to make for the config stuff you did13:40
rogpeppenatefinch: ok13:41
rogpeppenatefinch: i'll start working on that, because i think i know what needs to happen13:41
rogpeppenatefinch: unless you want to do it, of course13:41
natefinchrogpeppe: I think you're in a much better position to do it, since you made the config changes13:42
rogpeppenatefinch: yeah13:42
fwereadedimitern, LGTM13:43
dimiternfwereade, thanks!13:44
rogpeppehmm13:51
rogpeppe2014-03-28 13:42:07 INFO juju.worker.upgrader error.go:32 upgraded from 1.17.7.1-precise-amd64 to 1.16.6-precise-amd64 ("https://juju-dist.s3.amazonaws.com/tools/releases/juju-1.16.6-precise-amd64.tgz")13:51
natefinchummmm13:53
mattywmgz, dimitern any idea if the landing bot is on holiday today?13:55
rogpeppemattyw: i'll have a look13:55
rogpeppemattyw: yeah, it's stuck13:56
rogpeppemattyw: i'll kick it13:57
* rogpeppe wished he knew why we get mongods left over from tests13:57
rogpeppemattyw: it's going again13:58
rogpeppethis is worryingly weird. here's an excerpt from the relevant machine-0 log: http://paste.ubuntu.com/7168627/14:20
rogpeppecan anyone think of some way that that sequence of events is possible?14:20
rogpeppefwereade, dimitern: ^14:20
rogpeppedimitern: FWIW, apart from this issue the upgrade worked fine14:21
natefinchhow do I edit my last commit message (presuming I haven't pushed yet)?14:25
wwitzel3natefinch: you can use uncommit14:26
wwitzel3natefinch: and then just do another commit14:26
natefinchwwitzel3: thanks, I saw uncommit moments after I asked.14:27
wwitzel3natefinch: no shame in teddy bear questions14:27
dimiternrogpeppe, that's moot14:27
natefinchwwitzel3: yep14:28
rogpeppedimitern: how so?14:28
dimiternrogpeppe, you'll need to change the version from 1.17.7 to 1.18.0 before the upgrade (and rebuild)14:28
natefinchwwitzel3, rogpeppe: btw I merged trunk into my branch again.... trying to keep up to date so it won't bitrot14:28
rogpeppedimitern: upgrade steps don't trigger upgrading to 1.17?14:28
rogpeppedimitern: ok, i'll start again14:29
rogpeppedimitern: it's still a *really* weird issue14:29
rogpeppedimitern: and it worries me14:29
dimiternrogpeppe, no, because 1.18 is the target14:29
rogpeppedimitern: right, ok14:29
dimiternrogpeppe, what's the weird thing about that log excerpt?14:30
rogpeppedimitern: i actually think that upgrade steps should be targetted to minor versions too14:30
rogpeppedimitern: the weird thing is that the environ config returned with an agent-version of 1.17; but then a subsequent call to DesiredAgentVersion returned 1.1614:31
rogpeppedimitern: which caused a unit agent to try to downgrade14:31
rogpeppedimitern: which caused it to break14:31
rogpeppedimitern: rather, it *did* downgrade, but the 1.16 logic then assumed that the config file was 1.12 because there was no version file, and 1.16 config isn't valid 1.12 config.14:32
dimiternrogpeppe, hmm.. well, 1.17 is not a desired version to upgrade to from 1.16 for sure14:32
rogpeppedimitern: yes it is, if you've done upgrade-juju --upload-tools14:33
dimiternrogpeppe, it still isn't14:33
dimiternrogpeppe, it's a dev version14:34
rogpeppedimitern: really? you're not allowed to upgrade to a dev version, even if you ask to?14:34
dimiternrogpeppe, so upgrade-juju won't try to upgrade you to it automatically14:34
dimiternrogpeppe, that's not the way to ask14:34
dimiternrogpeppe, the way to ask is --version= (i.e. force it)14:34
rogpeppedimitern: i thought --upload-tools asked14:34
rogpeppedimitern: otherwise upgrade-juju --upload-tools is useless14:35
dimiternrogpeppe, no, it just uploads doesn't change the logic14:35
rogpeppedimitern: because you don't know the version that it will choose14:35
mattywdoes anyone else see this error in cmd/juju? http://paste.ubuntu.com/7168714/14:35
rogpeppedimitern: (anyway, it demonstrably *does* upgrade, even if it perhaps should not)14:35
dimiternrogpeppe, --upload-tools --version=x.y.z does what you'd expect, but for the upgrade steps to trigger, change version to 1.18.0, then you just need to rebuild and use --upload-tools only, no --version14:36
rogpeppedimitern: so why did my environment upgrade to 1.17 when i didn't specify --version? would you consider that a bug?14:38
rogpeppedimitern: i still think that if you use --upload-tools, it should use that version, otherwise why would you use that flag?14:39
dimiternrogpeppe, read the help14:39
dimiternrogpeppe, of upgrade-juju - it's all explained14:39
dimiternrogpeppe, you get the default behavior when running it without --version and --upload-tools14:40
dimiternrogpeppe, otherwise you'd get what you'd get :)14:40
rogpeppedimitern: so do you think there's a bug currently?14:40
dimiternrogpeppe, because that's the way to upload tools so they can be among the ones to be considered?14:41
dimiternrogpeppe, no14:41
dimiternrogpeppe, it's as designed14:41
rogpeppedimitern: i thought you said that using --upload-tools shouldn't  consider the uploaded tools if you're currently on a non-dev version14:41
rogpeppedimitern: but in my case, it definitely did14:42
* rogpeppe tries to understand the upgrade-juju help14:42
dimiternrogpeppe, look at the code as well14:42
rogpeppe[14:32:56] <dimitern> rogpeppe, hmm.. well, 1.17 is not a desired version to upgrade to from 1.16 for sure14:42
rogpeppedimitern: doesn't that mean there's a bug?14:43
dimiternrogpeppe, upload-tools does not change the behavior of what's considered desired14:43
rogpeppedimitern: right, so since i didn't specify --version, it should not have chosen 1.1714:44
rogpeppedimitern: because the current version was 1.1614:44
dimiternrogpeppe, the logic is still the same, it just uploads or not; --version changes what gets considered14:44
rogpeppedimitern: but since it *did* choose 1.17, i think that counts as a bug14:44
dimiternrogpeppe, the current version was "juju version"14:44
dimiternrogpeppe, i.e. cli's current version14:44
rogpeppedimitern: oh really?14:44
rogpeppedimitern: that's too bizarre for words14:45
rogpeppedimitern: the "current agent version" is the command line tool's version?14:45
dimiternrogpeppe, look at initVersions - that's where most of the magic happens14:46
dimiternrogpeppe, I'm too maas-distracted to be a credible source without switching too much context14:46
rogpeppedimitern: FWIW i think it's initVersions1dot16 that's relevant here14:47
dimiternrogpeppe, that's correct14:49
rogpeppenatefinch: am currently resolving conflicts, FYI15:11
natefinchrogpeppe: coool15:12
rogpeppenatefinch: well, the conflicts are resolved. now for the meat of it...15:18
natefinchrogpeppe: heh15:19
wwitzel3rogpeppe, natefinch: is it about time for a hangout session then?15:20
rogpeppewwitzel3: i'd be happy to15:20
rogpeppewwitzel3: https://plus.google.com/hangouts/_/canonical.com/juju-core?authuser=1 ?15:21
rogpeppenatefinch: https://plus.google.com/hangouts/_/7ecpj5hnavq7rdcfes1ctdva0s?authuser=1&hl=en15:22
=== vladk is now known as vladk|offline
=== Ursinha is now known as Ursinha-afk
=== Ursinha-afk is now known as Ursinha
rogpeppevoidspace: ping16:10
voidspacerogpeppe: pong :-)16:25
dimiternfwereade, can I get an LGTM on https://codereview.appspot.com/80600044/ so I can integrate it with supportnetworks() and land it later?16:35
wwitzel3rogpeppe: you mentioned there were some HA tasks that could be done that you wanted to discuss?16:36
wwitzel3rogpeppe: you ready to talk about those yet?16:36
rogpeppewwitzel3: yeah, gimme a minute or too16:53
rogpeppetwo16:53
natefinchrogpeppe: he's getting lunch, so no rush16:53
=== hatch__ is now known as hatch
voidspacerogpeppe: ping17:34
rogpeppevoidspace: pong17:34
voidspacerogpeppe: we need to handle the case where we're unmarshaling and marshaling data with an address instead of statePort17:35
voidspacerogpeppe: what format are the stateDetails addresses?17:35
dimiternrogpeppe, can you review https://codereview.appspot.com/80600044 by any chance?17:35
voidspacerogpeppe: and as it's a slice of addresses, which should I use?17:35
voidspacerogpeppe: is it the standard foo:port17:35
voidspacerogpeppe: and can I use a naive split to get it - or should I find a proper address parsing function somewhere?17:36
voidspacerogpeppe: connectionDetails is defined in agent.go (at the top)17:36
rogpeppedimitern: currently overloaded. will do in a bit.17:36
voidspaceheh, sorry to add to your burden17:36
voidspaceI can dig into this myself if you prefer?17:36
dimiternrogpeppe, cheers, no rush17:37
* dimitern is away for a while17:40
rogpeppevoidspace: i'll join you17:40
voidspacekk17:40
voidspaceso it is populated from MgoServer.Addr()17:41
wwitzel3rogpeppe, natefinch https://codereview.appspot.com/81550047 replicaset MasterHostPort18:19
voidspacerogpeppe: if SplitHostPort(format.StateAddresses[0]) errors should we ignore it or should the unmarshal function return an error?18:26
voidspacerogpeppe: this would mean returning an error for data that we *used* to be able to unmarshal fine18:27
voidspacerogpeppe: on the other hand it would leave us without a StatePort which is probably an error...18:27
voidspacerogpeppe: I'm going to turn it into an error18:27
rogpeppevoidspace: yeah we should definitely error18:28
voidspacerogpeppe: cool18:28
voidspacerogpeppe: actually, we have tests to ensure that we can't write/read an invalid address18:33
voidspacerogpeppe: so it shouldn't be an issue18:33
rogpeppevoidspace: cool18:33
voidspaceright, going jogging - back later18:37
=== Ursinha is now known as Ursinha-afk
=== Ursinha-afk is now known as Ursinha
* rogpeppe is done for the day19:56
rogpeppeg'night all19:56
rogpeppehappy weekends to all too19:56
wwitzel3EOD for me as well19:57
natefinchsee ya wwitzel320:00
natefinch& rogpeppe20:00
sarnoldwhy does provider/local/environ.go Destroy() execute sudo itself?21:20
perrito666sarnold: bc you cannot operate with lxc without sudo (perhaps I missed where was your question going)21:27
sarnoldperrito666: I'm just surprised it doesn't just fail. it seems odd to me to hard-code an authentication mechanism in the source code.21:28
perrito666sarnold: well, I guess since it cannot be run without sudo it makes sense, it is better to run sudo for a particular part than for the whole program (just my opinion)21:29
sarnoldperrito666: heh, could be, but it appears to just be recursively calling juju destroy again :)21:30
perrito666sarnold: ah, that is not nice21:32
=== hatch__ is now known as hatch
=== Ursinha is now known as Ursinha-afk
=== Ursinha-afk is now known as Ursinha
=== arosales_ is now known as arosales

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