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

menn0thumper: here's the logsender worker (not wired up) http://reviews.vapour.ws/r/1244/00:01
mupBug #1434680 changed: Certificate generated by certupdater worker cannot be used by MongoDB <ci> <regression> <upgrade-juju> <juju-core:Fix Released by wallyworld> <juju-core 1.22:Fix Released by menno.smits> <juju-core 1.23:Fix Released by wallyworld> <https://launchpad.net/bugs/1434680>00:27
thumpermenn0: ack00:28
menn0thumper: i'm currently pushing branches and writing you a long email00:33
* thumper nods00:33
thumpersounds good00:33
axw_wallyworld: sorry I didn't make the standup, I didn't get much sleep and wasn't feeling well00:43
wallyworldaxw_: np at all, hope you're ok, take plenty of time to fell well again today if you need to00:44
axw_wallyworld: last night I got the EBS provider working; it informs the provisioner that it needs a volume before it can proceed to CreateVolumes, and it sets DeleteOnTermination for the attachment if non-persistent00:44
axw_wallyworld: just need to write lots of tests now00:44
wallyworldyay, awesome00:45
axw_that will involve the new method in goamz too00:45
wallyworldok00:45
=== kadams54 is now known as kadams54-away
jw4ocr ptal : forward port of previously reviewed changes into master: http://reviews.vapour.ws/r/1243/01:00
axw_wallyworld: is trunk still blocked?01:06
wallyworldaxw_: no, i unblocked just before01:06
axw_sweet, thanks01:06
wallyworldjoin the queue01:06
axw_wallyworld: if you have time, http://reviews.vapour.ws/r/1227/ needs reviewing01:07
wallyworldaxw_: sure, just need to do a 1.22.1 fix, review is next on my todol list01:07
axw_nps, thanks01:07
axw_anastasiamac: do you still want me to review your branch? I see wallyworld has +1d01:11
=== axw_ is now known as axw
anastasiamacaxw_: sure :D was trying to land but had a failure... if u could review, would be gr8 :D01:12
axwanastasiamac: don't wait for me, I'll just have a look and if there's anything just do it in a followup01:12
anastasiamacaxw: sounds gr8! thnx :D01:13
axwanastasiamac: your latest changes look fine, thanks01:17
anastasiamacaxw: tyvm :D have fix for failed tests :)) will try to land again!01:18
wallyworldaxw: a small one https://github.com/juju/juju/pull/192201:47
axwwallyworld: looking01:53
wallyworldty01:54
axwwallyworld: done01:56
wallyworldty01:57
wallyworldaxw: i've also added a quick drive by fix of an intermittent failure i introduced last night fixing the bug blocking trunk01:57
axwok, looking01:58
axwwallyworld: why does that change make a difference?01:59
wallyworldaxw: because it slightly delays the "now" time from which the expiry is calulated01:59
wallyworldto better reflect what is done when generating the cert02:00
wallyworldthere's 2 expity times - one for ca cert, one for server cert02:00
axwwallyworld: and the cert time needs to be before "now" ?02:01
wallyworldthe expiry tine needs to be before now+1002:01
wallyworldand if now is slightly earlier, the expiry time is slightly after now+1002:01
wallyworldie don't determine"now" and then do stuff and then generate the cert02:02
wallyworldas the doing stuff delay is enough to cause the test to sometimes fail02:02
axwok02:02
axwwallyworld: shipit02:02
wallyworldta02:02
wallyworldthis was a refactored test02:03
wallyworldthe original used the same expiry time for ca cert and server cert02:03
ericsnowthumper: FYI, I believe I have that RB webhook unicode bug fixed02:28
anastasiamacericsnow: \o/02:33
thumperericsnow: cool02:38
natefinchwallyworld_, thumper, anyone else: I need to change the "stateaddress" value in the machine's agent.conf (since I'm upgrading it to a state server, it needs to change to localhost).  However, I can't see any code that can actually serialize a value out to that.02:53
wallyworld_natefinch: hey, there's formatter.go files somewhere02:55
wallyworld_let me have a look02:56
natefinchwallyworld_: I see it - juju/agent/format-1.18.go02:58
wallyworld_natefinch: there's a format-1.18.go plus, if you have already loaded the config, you can use a ChangeConfig() callback to update02:58
wallyworld_you don't then need to serialise the whole lot02:59
natefinchwallyworld_: ok, yeah, I did that,  must have done something wrong, though. But it's good to know how the magic was happening.02:59
wallyworld_natefinch: you need to use an AGentConfigWriter to invoke the Change callback03:00
wallyworld_if you use an AgentConfig, it won't work03:00
ericsnowmenn0: CI failure due to your upgrade fix?  http://data.vapour.ws/juju-ci/products/version-2473/run-unit-tests-utopic-amd64/build-1747/consoleText03:00
wallyworld_natefinch: i found that out the hard way last night doing a patch to unblock trunk03:01
wallyworld_natefinch: if you look at the commit, you'll see how i did it03:01
wallyworld_https://github.com/juju/juju/commit/97028ee0bc998026f0e1ef758ef6afadb1734ac403:01
menn0ericsnow: that's actually wallyworld_'s03:01
wallyworld_yep03:01
ericsnowmenn0: ah03:01
wallyworld_there's a fix proposed03:02
wallyworld_intermittent03:02
ericsnowwallyworld_: cool :)03:02
wallyworld_landing in 1.22, then 123, then master03:02
wallyworld_sigh03:02
* ericsnow hopes we can preempt another blocker bug in the morning :)03:02
wallyworld_oh yes, i will ensure it03:03
ericsnowwallyworld_: :)03:03
natefinchwallyworld_: awesome, thanks03:04
wallyworld_natefinch: sure, let me know if you get stuck etc03:04
ericsnowthumper: about restore->recover...03:04
ericsnowthumper: "restore" gives the wrong idea about what it is for03:05
ericsnowthumper: "recover" seems to describe it really well though03:05
thumperI disagree03:05
ericsnowthumper: go on03:05
thumperrebuild?03:05
thumperbackup / restore seems more natural03:05
thumperare you concerned that you don't restore an environment03:06
thumperbut you recreate one?03:06
ericsnowthumper: restore is only meant to me used in the case that your environment blows up and you no longer have a state server03:06
ericsnowthumper: the way it is now at least03:06
natefinchwallyworld_: yeah, I was just using agent.ChangeCOnfig, not agent.AgentConfigWriter.ChangeConfig03:07
ericsnowthumper: so it covers only one part of what the name "restore" seems to promise03:07
thumperericsnow: however, recover doesn't work either03:07
menn0thumper: PTAL http://reviews.vapour.ws/r/1244/03:07
wallyworld_natefinch: that will work, but i think it uses a go routine or something to do it async03:07
wallyworld_natefinch: not the right thing to use if you just want it done ad hoc on startup or something03:07
ericsnowthumper: sure, I'm not married to "recover" as the new name03:07
thumperin which case, I think we need some more bikeshedding :-)03:08
ericsnowthumper: no worries03:08
natefinchwallyworld_: actually, heh, I hadn't actually saved the changes in my buffer yet.  That would probably help :003:08
ericsnowthumper: I opened the bug and the pull request at the very least to get things rolling03:08
thumperwell... some take the pull request to be a slightly more agressive stance on the idea03:09
ericsnowthumper: wasn't planning on landing anything until there was more feedback03:09
thumpernormally a PR is "this is they way I think it should be"03:09
thumperericsnow: part of the problem is that people take things differently than they way you intend03:09
thumperit isn't obvious that it is a discussion point03:10
thumperit is a "here's the fix"03:10
ericsnowthumper: ah, makes sense03:10
ericsnowthumper: I'll discard the PR for now then03:11
thumperok03:11
thumpercheers03:11
mupBug #1435644 was opened: private cloud:( environment is openstack )index file has no data for cloud <juju-core:New> <https://launchpad.net/bugs/1435644>03:16
ericsnowanastasiamac: thanks for those reviews03:16
anastasiamacericsnow: the least i could after u did an amazing review for me :D03:17
ericsnowanastasiamac: ha, thanks :)03:18
anastasiamacericsnow: done :D but of course my shipit is not loaded with much power :D03:22
ericsnowwallyworld_: FYI, I think we're going to be okay leaving things alone in the simplestreams code: https://github.com/Altoros/juju-vmware/blob/master/provider/vmware/image_metadata.go03:22
ericsnowanastasiamac: thanks03:23
wallyworld_ericsnow: looks reasonable - those are the standard extension points03:25
wallyworld_axw: finally reviewed, sorry about delay03:25
ericsnowwallyworld_: yeah, that's what I came to realize03:25
axwwallyworld_: nps, thanks03:30
ericsnowthumper: finally got a nice big round tuit so I've set up the webhook on each of the repos that RB knows about (except for docs)03:36
wallyworld_ericsnow: jorge's --journal backport to 1.22 also needs to get into 1.2303:36
ericsnowthumper: I'll keep an eye on those to make sure they're working03:36
ericsnowwallyworld_: I thought he landed the original patch before the 1.23 branch was made03:37
* ericsnow checks03:37
wallyworld_ericsnow: i could be wrong, but it only seemed to land 5 days ago and when i compared master to 1.23 branch it was missing03:37
wallyworld_happy to be wrong though03:37
ericsnowwallyworld_: no, you're right, it isn't there03:39
wallyworld_just need to cherrypick the commit03:39
ericsnowwallyworld_: I'll make sure that gets in03:39
wallyworld_ty03:39
wallyworld_i added the bug to the series and milestone03:40
ericsnowwallyworld_: thanks03:42
ericsnowwallyworld: FYI, that 1.23 backport patch is now waiting its turn (https://github.com/juju/juju/pull/1925)04:01
wallyworld\o/ ty04:01
ericsnowwallyworld: wish they were all that easy :)04:02
=== kadams54 is now known as kadams54-away
wallyworldindeed04:02
thumperjam: ping04:24
thumperjam: we need to chat about some spec topics04:25
thumperjam: I'm half preparing dinner, so I'll check back in shortly04:25
=== thumper is now known as thumper-dinner
jamthumper-dinner: k04:25
thumper-dinnerjam: ah... you are there04:26
thumper-dinnerjam: when is good?04:26
jamthumper-dinner: I'm around, so whenever is good for you04:26
thumper-dinnernow?04:26
thumper-dinnerjam: https://plus.google.com/hangouts/_/canonical.com/spec-chat04:26
wallyworldaxw: have you ever had a git stash become corrupt? mine just did and I had to try and piece together commits from git fsck --unreachable, and even them i lost a bunch of work :-(04:49
axwwallyworld: nope, sorry :(04:50
axwwallyworld: that's why I don't ues stash so much... I create branches and push WIP to GH when it gets big enough for me to worry about loss of work04:50
wallyworldyeah, i'll have to consider that04:51
wallyworldi still have one more big branch to sort out, sigh04:52
axwdimitern: I'm adding ModifyInstanceAttribute, but having trouble writing a test as it involves a VPC knowledge that I don't have and don't really have time to learn right now. Would you prefer that I leave out support for SourceDestCheck, or that I put it in without a test for that attribute?06:43
dimiternaxw, so what's the problem - it's just a flag set on a ec2test instance06:44
jamdimitern: just to check, request-address is part of the phase 1 networking stuff, right?06:45
axwdimitern: you can't set it on non-VPC instances. instances in the test double seem to be getting a VPC by default06:45
dimiternaxw, yes, that's true - it wasn't even available pre-vpc api IIRC06:45
dimiternjam, yes, it is06:45
jamk, it came up as some other stuff06:46
jamis request-iface ?06:46
axwdimitern: so I can test the error path on my account, but I suspect the test will fail if there's a default VPC in the region06:46
jam(thinking about kvm containers, they could create their own iface if they had an IP to assign it, right?06:46
jam)06:46
dimiternaxw, how about a simple test like this: set sourcedestcheck, followed by Instances() checking the flag is set on the instance?06:46
axwdimitern: that's what I was trying to do, and got errors about not being able to create VPC or subnet or something in AZs06:47
dimiternjam, request-iface is on the roadmap as well (but lower in priority than the rest I guess)06:47
jamdimitern: sure, just checking if we actually need it for charms06:47
jamdo they *need* juju to provide it, or could they do it with their own work after requesting ip06:47
dimiternjam, for *certain* very specific charms06:47
dimiternaxw, are you using the shared aws account? this is a live test I guess06:48
axwdimitern: my own06:48
axwyes, live06:48
dimiternaxw, well, if you don't mind adding the test and sending me a link, I can pull you branch and make sure the test pass, how about this?06:48
axwdimitern: sounds good, thanks06:49
dimiternjam, for charms that need a dedicated interface, e.g. like in the kvm appliance case, having native support in juju will definitely help06:49
wallyworldaxw: i haven't looked really hard (not much work on storage today sadly), but kat has added some more to the goose cinder branch, hopefully can be landed when she comes on tomorrow https://github.com/go-goose/goose/pull/306:50
axwwallyworld: cool, will take a look06:50
wallyworldta, have to head to soccer for a bit06:50
axwwallyworld: btw, should be able to deploy with tmpfs and rootfs on master now06:51
axwsure, ttyl06:51
TheMuemorning o/08:20
dimiternTheMue, hey, welcome back!08:21
dimiternTheMue, how was CeBIT?08:21
TheMuedimitern: a bit lame, the booth has been more like a recreation area, so we got only few audience08:22
TheMuedimitern: but used the time to visit other cloud providers and to tell them about the benefits of juju08:22
dimiternTheMue, ah :)08:22
TheMuedimitern: e.g. a guy of amazon ;)08:22
dimiternTheMue, nice! did you get to play with some power machines?08:22
TheMuedimitern: sadly not, those few which are installed by IBM are directly used for individual software products08:24
TheMuedimitern: those have been the types of booths we should have08:24
dimiternTheMue, yeah, I see08:24
TheMuedimitern: and also there has been a larger open-source area with many smaller booths08:25
TheMuedimitern: here we would have matched too08:25
TheMuedimitern: will this morning write the report for John, together with some photos of the IBM OpenPOWER area08:25
dimiternTheMue, so how many people where visiting appox ?08:25
dimiternTheMue, ah, nice! I'd appreciate if you share this with us as well :)08:26
TheMuedimitern: actively? one! for the rest I had to catch the people. the guy of redislabls had the same problem08:26
TheMuedimitern: yes, will sent you and Alexis a copy08:26
dimiternTheMue, ok :)08:27
TheMuedimitern: at least we've got a partner here in Germany, selling Ubuntu and OpenStack services. he's interested in doing more with Juju08:27
* TheMue also the next days has to create a new slide deck for his Juju/Go talk. thankfully already have a lot of material.08:29
TheMuedimitern: seen many usual reviewing and mergin mails during these days. anything else important happened?08:30
dimiternTheMue, great!08:31
dimiternTheMue, well, we had 2 releases :)08:32
dimitern1.22.0 and 1.23-beta108:32
TheMuedimitern: hehe, yes, I've seen. fantastic!08:32
axwdimitern: I figured out why my test wasn't working; my AZs are different, so I had to (temporarily) change the default AZ in ec2t_test.go08:52
dimiternaxw, hmm - ok, just beware this might lead to some failures08:53
axwdimitern: it's the same with the existing tests08:53
dimiternaxw, e.g. t1.micro not being available in some AZs, but used in a few live tests08:54
axwdimitern: there's a flawed assumption in the live tests. AZs are account-specific08:54
dimiternaxw, huh?08:55
dimiternaxw, you mean there might be less in some accounts08:55
axwdimitern: yes, and they might be different08:55
axwdimitern: http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#using-regions-availability-zones-describe08:56
axwsearch for "might be different"08:56
axwerr sorry08:56
axw"might not be the same"08:56
axwdimitern: I'd appreciate a review of https://github.com/go-amz/amz/pull/44 when you have a moment08:58
dimiternaxw, well, I learned something today :)08:59
dimiternaxw, sure, will have a look in a bit08:59
wallyworldfwereade: no idea how busy you are, i have a status-get PR that needs reviewing. as soon as this lands, I have a status-set one ready to propose. if you have time, awesome, if not fine too. both status-get and status-set tested live with a custom charm. http://reviews.vapour.ws/r/1252/09:11
fwereadewallyworld, awesome, I will take a look in a mo; in the meantime, I am a little bit baffled by cmd/jujud/unit.go:152 which was apparently yours (a long time ago...) in commit ead4f3ee09:13
wallyworldlet me look09:13
fwereadewallyworld, is the unit agent starting a container provisioner? if so, why? if not, why are we doing that there?09:13
wallyworldfwereade: just got called for dinner, i'll get back to you09:14
fwereadewallyworld, np09:14
dooferladdimitern: Morning o/09:15
dimiterndooferlad, morning o/09:16
dooferladdimitern: Just proposed that names change: http://reviews.vapour.ws/r/1253/09:16
dimiterndooferlad, cheers, will have a look in a bit09:18
axwdooferlad: what is a "space"?09:51
voidspaceaxw: spaces are a juju specific networking term09:51
dooferladaxw: A space is a security subdivision of a network.09:51
voidspaceaxw: it means a named group of subnets09:51
axwok09:51
voidspaceaxw: so you can have a space that spans several availability zones (for example)09:52
natefinchspace seems like a terrible name09:55
natefinchif it means a group, why not call it a group?09:56
voidspacenatefinch: I believe Mark picked it09:56
dimiternaxw, I'll pull your branch and test it locally before sending the review09:57
natefinchvoidspace: :/09:57
voidspacenatefinch: conceptually it forms a single network space though, not a "group of subnets"09:57
axwdimitern: cheers09:57
voidspacenatefinch: the point is to not [have to] think of it as a group09:57
axwgotta go out, will check back later09:57
voidspacebut a group is what it is...09:58
axwjust a bike shed, but something with "network" in the name would be more helpful. netspace perhaps. but sounds like it's already decided09:59
wallyworldfwereade: that diff shows the machine and unit agents informing the upgrader what tools version they are running. not sure what you mean about starting a container provisioner. the container provisioner is in worker/provisioner and is run by the machine agent10:04
dimiterndooferlad, standup?10:04
fwereadewallyworld, oh, I see, you took it out of upgrader but didn't move the api method, I didn't look at the diff properly and thought you just hacked an extra call in10:08
fwereadewallyworld, why does container initialisation depend on what tools the agent has reported to the state server though>#10:09
wallyworldit doesn't does it? i'd have to look at the code again10:09
wallyworldfwereade: the container init stuff in the diff is just a drive by logging change it seems10:10
wallyworldfrom trace -> info10:10
fwereadewallyworld, the commit claims it's fixing a bug in which container initialisation fails because the toools haven't been set10:10
fwereadewallyworld, damn sorry bbs10:11
wallyworldsure10:11
wallyworldfwereade: so, container_initialisation.go, that has NewContainerSetupHandler(), this does indeed create a Broker to start containers, it needs tools to do that (of the right version). all this is set up in the machine agent, not the unit agent10:14
wallyworldmachine agent has ths code handler := provisioner.NewContainerSetupHandler(params)10:14
wallyworldso if tools haven't been set, then when machine agent starts the container handler, the handler can't work properly10:15
fwereadewallyworld, I don't see why a provisioner ever needs to ask the api server for its own tools though10:25
fwereadewallyworld, it *knows* its tools10:25
fwereadewallyworld, the only tools it's guaranteed to be capable of using to provision another machine are the ones it's running already10:26
fwereadewallyworld, independent of what it might have told anyone else10:26
wallyworldfwereade: to start a container, it needs the tools tarball10:27
fwereadewallyworld, right, but all it needs to know is the version it's running -- not the version a separate worker told some other dude it was running10:28
fwereadewallyworld, it can ask the apiserver for the tools tarball corresponding to version.Current.Number for the target arch10:29
fwereadewallyworld, in fact it *must*, lest bugs like that one10:29
wallyworldfwereade:looking at the code now, the container initialiser gets a ToolsFInder interface which does what you say above i think10:30
wallyworldthe ToolsFinder method is on provisioner.State10:32
wallyworldi think but am not sure that version.Current is passed in, nd a tools list is returned10:32
fwereadewallyworld, yeah, I'm trying to follow those same threads :)10:33
fwereadewallyworld, so then I *think* we should be ok to revert the agent changes and put the SetTools back in Upgrader?10:34
wallyworldfwereade: i have no idea, i haven't enough context10:34
fwereadewallyworld, fair enough :)10:35
wallyworldthe change in that commit is from a while ago isn't it10:35
wallyworldfwereade: is this for a particular new bug?10:37
fwereadewallyworld, this is just because I'm trying to do something saner with the unit agent and I noticed that call (which I expected to be in Upgrader) squatting in the middle of APIWorkers()10:49
wallyworldfwereade: ah i see, it was maybe expedient at the time, i honestly can't recall. sorry :-(10:50
fwereadewallyworld, no worries at all, I see it was a copy from a release branch ;)10:50
fwereadevoidspace, hey, iirc you were involved with the proxy-settings weirdness10:52
voidspacefwereade: right10:52
voidspacefwereade: for my sins, which are many...10:53
mgzdooferlad: want to be a guinea pig?10:53
mgzI'm going to gate the juju/names package in a sec10:53
fwereadevoidspace, so, AIUI, we encounter problems if proxy settings get set too late10:53
voidspacefwereade: correct10:53
voidspacefwereade: anything that needs network access shouldn't run until after the proxy settings are made10:54
fwereadevoidspace, (1) why exactly is that and (2) does that mean we restart the whole agent when we get new proxy settings?10:54
fwereadevoidspace, (and if not why not)10:54
voidspacefwereade: so it was contacting the charm store specifically that failed10:54
voidspacefwereade: so the charmrevisionworker was one thing that had problems10:54
voidspacefwereade: I didn't look at what happens when proxy settings change so I can't answer 210:55
fwereadevoidspace, ok, I am slightly worried that all we've done is mask the problem then10:55
voidspacefwereade: deploying charms failing was the specific bug that caused me to look at the problem10:55
voidspacefwereade: we don't block on waiting for the proxy settings to be set10:56
voidspacefwereade: so we knew we had only made a partial fix, and there's a bug for *that*10:56
fwereadevoidspace, and that's specifically in the unit agent trying to download them then?10:56
voidspacefwereade: I believe so10:56
fwereadevoidspace, I feel like that ought to be a self-healing problem10:57
voidspacefwereade: the *only* fix I made was to move starting the proxyupdater to be one of the first workers started10:57
fwereadevoidspace, uniter tries to download a charm, fails out, gets restarted in 3s, by which time the proxysettingsupdater has run10:57
voidspaceso definitely a band-aid rather than a fix10:57
voidspacefwereade: it was failing hard10:57
dooferladmgz: sounds good10:57
fwereadevoidspace, *unless* the proxysettingsupdater doesn't actually work10:57
voidspacefwereade: moving it solved *that* problem10:58
voidspaceit's possible there's a bug around charm download retrying (worker doesn't die properly - or more likely reports a failed deploy causing the deploy to be cancelled and not retried?)10:59
fwereadevoidspace, ok, I think I'll just try to preserve your approach and comment it11:00
voidspacecool11:00
fwereadevoidspace, (and, hmm, I don't *think* that'd happen -- now or before -- I'm pretty sure the only special error is ErrConflict, and that only happens well after the charm is downloaded and verified)11:04
fwereadevoidspace, (while I remain deeply suspicious about what *actually* happens to the http module when the magic env vars are changed from some random goroutine...)11:06
dimiternvoidspace, hey11:14
dimiternvoidspace, just about finishing the review on the addresser11:14
dimiternvoidspace, considering you're already using interfaces for both the state and environ methods you need, why not just mock those in the test?11:15
voidspacedimitern: I did initially, but it's lot more work than just using the dummy provider11:16
voidspacedimitern: for no particular gain11:16
dimiternvoidspace, really?11:16
dimiternvoidspace, how so?11:16
voidspacedimitern: well I had an implementation of a mockReleaser and types for storing the calls11:17
voidspacedimitern: and then you call NewWorkerWithReleaser11:17
voidspacedimitern: instead of just breaking the environ method and listening on a channel to get the call11:17
dimiternvoidspace, yeah11:18
voidspacedimitern: so I deleted my mockReleaser and ReleaseCall types - they just seemed unnecessary11:18
mgzdooferlad: okay, land with $$merge$$ in the mp when ready11:19
dimiternvoidspace, there's juju/testing stub already there to support calling a type and inspecting the received calls11:19
mgzdooferlad: it's a more complex project, so may break first try, we'll see11:19
voidspacefwereade: the default Http transport we use specifically checks the environment variables11:19
voidspacedimitern: ok, interesting - I *am* having a problem right now with listening on the dummy channel11:20
voidspacedimitern: I seem to get the same call twice instead of the two different calls I expect11:20
voidspacedimitern: so maybe I should switch back11:20
dimiternvoidspace, it does seem a bit more complicated than it needs to be11:20
dimiternvoidspace, I'll add some comments with pointers11:21
voidspacedimitern: adding a mockReleaser won't make it less complicated11:21
voidspacedimitern: if you look at the latest diff the tests are quite clean now11:21
voidspacenot that they're all passing...11:22
voidspaceTestWorkerReleasesAlreadyDead doesn't get the correct release ops...11:22
voidspacedimitern: but thanks, I will appreciate comments!11:23
dimiternvoidspace, ok, I really need to go out and catch the DMV before lunch break now11:23
voidspacedimitern: cool11:23
voidspacedimitern: I got your email - it looks fine by the way11:23
dimiternvoidspace, I'll send the review I have so far and continue when I'm back - mostly tests remain11:23
voidspacedimitern: but enjoy the DMV...11:23
voidspacedimitern: ok11:24
dimitern:D yeah\11:24
dooferladmgz: waiting for bot...11:24
dooferladmgz: library problem in Jenkins run11:25
mgzdooferlad: ta, checking11:25
mgzhm, go get golang.org/x/crypto and go get golang.org/x/crypto/... both exit non-zero for me11:30
mgzthat's a bit of an issue11:30
mgzwhat's names actually using...11:30
mgztesting, utils, and mgo.v2 use various bits11:33
natefinchwwitzel3: I got ensure-availability --to 1,2 working, although it requires a manual reset of jujud... I think mongo just didn't have time to initialize before we restarted.... not sure why it's different in this case from a normal state server creation.11:33
mgzbut only utils needs the custom one, and only pbkdf2 and ssh11:35
mgzdooferlad: landed11:37
mgzthe log is a bit annoyingly out of order, I probably need to throw in flushes11:37
dooferladmgz: cool. Thanks!11:37
fwereadevoidspace, https://bugs.launchpad.net/juju-core/+bug/140322511:44
mupBug #1403225: charm download behind the enterprise proxy fails <cloud-installer> <deploy> <proxy> <sync-tools> <cloud-installer:Fix Released by adam-stokes> <juju-core:Fix Released by mfoord> <juju-core 1.21:Won't Fix> <juju-core 1.22:Fix Released by mfoord> <https://launchpad.net/bugs/1403225>11:44
fwereadevoidspace, comments 20-21 seem to indicate that unconditionally setting the env vars was sufficient to get a unit deploying11:45
fwereadevoidspace, and charmrevisionworker is only needed on state servers iirc11:46
fwereadevoidspace, was there another motivation for starting it early in the unit agent?11:47
voidspacefwereade: I believe I was mistaken about unconditionally setting the env vars12:00
voidspacefwereade: as once I'd started the proxyupdater early unconditionally setting env vars was not needed, but was considered harmless12:01
voidspacefwereade: my final conclusion was that the existing logic around setting env vars was correct, just started too late12:01
voidspacefwereade: and I didn't move charmrevisionworker - I moved proxyupdater12:01
fwereadevoidspace, isn't it necessary? I strongly suspect it's possible for proxy settings to change12:01
voidspacefwereade: it is possible, but the logic that determines that was correct12:02
fwereadevoidspace, right, but cmd/jujud/unit.go:15812:02
fwereadevoidspace, the unit agent doesn't run a crw12:02
voidspacefwereade: there was a suspicion that the logic around *first setting* was faulty - but that proved to be incorrect12:02
voidspacefwereade: but *anything* that needs external network access will need the proxy settings in place12:03
voidspacefwereade: so if we have proxy settings it seems like they ought to be put in place before we do anything else12:03
fwereadevoidspace, but anything that doesn't have the proper network settings in place should just be exiting with an error and coming back up in a bit12:03
fwereadevoidspace, and that will be everything that needs network access if the proxy settings change while an agent's running12:04
voidspacefwereade: if the agent is in process then it won't be a problem - the proxyupdater handles change to env vars without needing to restart12:04
fwereadevoidspace, right, and so it's similarly not a problem if we start up with the wrong proxy settings12:05
fwereadevoidspace, it's inelegant to bounce up and down drunkenly before settling, true12:05
voidspacefwereade: however in practise it *was a problem* (and is now less of a problem)12:05
voidspacefwereade: you're probably correct that moving the proxyupdater was papering over some other problem12:05
voidspace(why didn't charm download get retried)12:05
fwereadevoidspace, there are two cases though12:05
voidspacehowever it did solve the immediate problem (which was specifically - demo in 3 days need a fix released)12:06
voidspacefwereade: I think charm revision worker was set to only retry once a day12:06
voidspacefwereade: and to *not* die on error12:07
fwereadevoidspace, (I am not trying to rag on what you've done, just trying to work out what I can do that is not going to break stuff)12:07
voidspacebut that's only on state server12:07
voidspacesure, no problem12:07
voidspaceI'm just explaining *why* we went with that fix :-)12:07
voidspace"because it worked", essentially12:07
fwereadevoidspace, yay empiricism :)12:08
voidspacebut also worked for a sensible reason12:08
fwereadevoidspace, I am comfortable with the machine agent fix12:08
voidspacefwereade: take the fix out on the unit agent, and try charm deploy behind a proxy12:09
voidspacefwereade: it maybe that the problem was charm download on the state server and the unit agent fix wasn't needed at all12:09
fwereadevoidspace, that's the story I'm hesitantly inferring from the discussion on the bug12:10
fwereadevoidspace, I'll look into it, thanks12:10
voidspaceyou may well be right12:10
voidspacefwereade: I wrote up reasonably simple instructions for creating a restricted environment to deploy juju to with kvm and ufw12:10
voidspaceand the manual provider12:10
fwereadevoidspace, hey actually I suddenly feel more sure of myself -- hasn't the unit agent always downloaded its charms from either the state server or environment storage?12:11
fwereadevoidspace, I suppose it's not impossible that environment storage might go via proxy12:12
fwereadevoidspace, but now we're always getting them from the state server I think12:12
voidspacefwereade: however, if the problem was proxy settings on the state server12:12
voidspacefwereade: the deploy command can only have been issued after the state server was running12:12
voidspacefwereade: so the "order we start workers on the state server" cannot have been the problem12:13
voidspacemaybe it was environment storage access12:14
voidspacebrb12:14
* TheMue is at lunch12:21
voidspacegoing on lunch13:01
=== Spads_ is now known as Spads
* dimitern is back13:21
mattywdimitern, just in time, got a moment?13:24
dimiternmattyw, hey, yeah13:24
alexisbmorning/evening all13:25
dimiternalexisb, morning :)13:33
dooferladalexisb: o/13:33
mupBug #1435857 was opened: panic: osVersion reported an error: Could not determine series <ci> <packaging> <regression> <vivid> <juju-core:Triaged> <juju-core 1.22:Triaged> <juju-core 1.23:Triaged> <https://launchpad.net/bugs/1435857>13:44
mupBug #1435860 was opened: certSuite.TestNewDefaultServer failure <ci> <regression> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1435860>13:44
sinzuidimitern, natefinch : we need some to do many fixes or many rollbacks to address bug 143585713:45
mupBug #1435857: panic: osVersion reported an error: Could not determine series <ci> <packaging> <regression> <vivid> <juju-core:Triaged> <juju-core 1.22:Triaged> <juju-core 1.23:Triaged> <https://launchpad.net/bugs/1435857>13:45
natefinchalexisb: btw, made more progress on the HA feature.  Works other than requiring me to kill jujud to make it restart... I am restarting it in code, but evidently it's not waiting for mongo to be ready or something similar.  So just have to figure that out.13:45
dimiternsinzui, oh boy, let me have a look13:45
alexisbnatefinch, awesome, thank you13:45
dimiterndooferlad, you have a review on the net cli PR13:45
dooferladdimitern: thanks!13:46
natefinchdimitern, sinzui:  gah, these stupid series bugs kill me13:47
dimiternnatefinch, and it's always before a release, right? :)13:47
sinzuinatefinch, but the irony is the commit it to ensure we can register the next series. Life is cruel13:47
natefinchdimitern: heh yep13:47
=== Spads_ is now known as Spads
tasdomasdimitern, ping?13:55
dimiterntasdomas, pong13:58
tasdomasdimitern, hi - when you have a sec, could you take a look at http://reviews.vapour.ws/r/1070/ ?13:59
dimiterntasdomas, hey, sure, I'll have a look shortly13:59
dooferladdimitern, TheMue, voidspace: networking interlock hangout time14:01
TheMuedooferlad: oh, thx14:01
=== kadams54 is now known as kadams54-away
=== Spads_ is now known as Spads
voidspacedooferlad: dammit, sorry14:19
dimiternvoidspace, you haven't missed anything important - check the agenda/minutes doc from the meeting if you like14:28
voidspacedimitern: I will do, thanks14:30
voidspacedimitern: and thanks for the review14:30
voidspaceworking on it14:30
dimiternvoidspace, np, I'm trying out a few suggestions around the tests locally before commenting14:31
voidspaceok14:33
voidspacedimitern: when you changed network.NewAddress you didn't change the ones in my PR14:41
voidspacedammit14:41
dimiternvoidspace, I've only changed those in master14:42
voidspacedimitern: that's terrible14:42
dimiternvoidspace, :)14:42
voidspaceyou're creating extra work!14:42
dimiternvoidspace, ok, my bad then14:42
voidspace:-)14:43
dimiternvoidspace, if it's so bad it deserves compensation, I'll give you one of the beers I promised in nuremberg :)14:43
voidspacehaha14:43
dimiterns/I promised/I'm promised/14:43
voidspaceso I owe you one less then...14:44
dimiternI need to open a spreadsheet and tally them up :)14:44
mgzyou need to not get alchohol poisoning :P14:45
voidspacedimitern: you want me to test that a no-op does nothing! (Test calling EnsureDead doesn't trigger a watcher...)14:46
voidspacedimitern: EnsureDead has an early return - which is tested14:46
dimiternvoidspace, hmm14:46
voidspacedimitern: I don't think you're testing any code in the worker there14:46
dimiternvoidspace, well, ok - change something else then14:46
dimiternvoidspace, e.g. call SetState on a new alive address14:47
voidspacedimitern: but that's testing the watcher, which is a lifecycle watcher14:47
voidspacedimitern: so that's testing the lifecycle watcher not the worker14:47
voidspacedimitern: maybe change it to Dying?14:47
voidspacealthough there's no way to do that14:47
dimiternvoidspace, hmm - exactly14:47
voidspaceso, maybe there's no need to test it then14:47
dimiternvoidspace, that test looked like it's testing the watcher14:48
voidspacedimitern: it's testing that the worker does something when the watcher is triggered14:48
voidspacedimitern: maybe I can create a new addres14:48
voidspacedimitern: which will have a life of Alive14:48
voidspacedimitern: and test that the worker doesn't kill it14:48
voidspacewhich I don't think is tested already14:48
voidspacethe watcher will be triggered and there's code in the worker to check the Life, so that's a good test that's missing14:49
dimiternvoidspace, ok, I'll have another look in case I've missed something14:50
=== kadams54-away is now known as kadams54
dimiterntasdomas, you've got a review15:01
tasdomasdimitern, much appreciated15:02
dimiterndooferlad, yours is also good to go15:06
ericsnowsinzui: I'm not sure what to make of it yet, but lxc-start just worked for me on vivid-slave-b15:06
sinzuiericsnow, updates arrived?15:07
dimiternvoidspace, ah, you know what - I won't manage to finish that review with the mocked tests15:07
ericsnowsinzui: I expect so (but don't know)15:07
dimiternvoidspace, just make sure it works reliably as-is, and let's go on15:07
sinzuiericsnow, I see more updates are available15:07
voidspacedimitern: ok, the tests are still a bit of a WIP anyway15:08
voidspacedone today though15:08
voidspacereversing a string in umpteen languages. Go is amongst the longest.15:08
voidspacehttp://rosettacode.org/wiki/Reverse_a_string15:08
dimiternvoidspace, it's fine, and I'm sure we'll get better very soon at writing tests with the stubs, esp. around the CLI work15:08
voidspacedimitern: cool15:08
dimitern:)15:08
voidspacedimitern: I did a version with stubs15:08
voidspacedimitern: but I wanted some actually hitting the provider as well - and they actually looked nicer15:09
ericsnowsinzui: and I just noticed that local-deploy-vivid-amd64 passed in 2473!15:09
dimiternvoidspace, yeah, fair point15:09
sinzuiericsnow, \o/15:09
dimiterndooferlad, sadly nobody will hear your $$merge$$ on that branch15:10
sinzuiericsnow, I am retesting the rev right now in CI ti give it another change to pass15:10
dooferladdimitern: darn it!15:10
ericsnowsinzui: k15:10
dimiterndooferlad, or maybe I'm wrong?15:10
dooferladdimitern: indeed15:10
* ericsnow struggles to hold back the tears15:10
marcoceppi_hey dooferlad dimitern `juju space` is a terrible cmd name15:11
sinzuiericsnow, yep, I was so focused on the failures, I didn't see the pass in my open tab: http://reports.vapour.ws/releases/247315:11
sinzuiI hope the precise failures were bad luck15:11
ericsnowsinzui: I so wish I had thought to check the LXC container log weeks ago!15:11
ericsnowsinzui: same here15:12
dooferladmarcoceppi_: i am following a spec for that. I am sure dimitern is happy to hear suggestions.15:12
sinzuiericsnow, we will see it pass again in about an hour.15:12
marcoceppi_dooferlad dimitern why not `juju net` or `juju network` juju space is vague and will be hard to convey to people when talking15:12
marcoceppi_dimitern: link to spec? I was about to comment but I missed it by 2 minutes :(15:13
ericsnowsinzui: awesome15:13
sinzuiericsnow, I think the bug is fix-released15:13
dimiternmarcoceppi_, we have a verbal agreement to change it to "net-space" or something like that if it's too confusing15:13
marcoceppi_but what even is "space" reading the code I had a hard time figuring it out15:13
dimiternmarcoceppi_, pm you a link15:13
dooferladmarcoceppi_: https://docs.google.com/a/canonical.com/document/d/1Z7pWB-MT5UboTpeDCZWZtcp1blbrM0xiuqik-CWEaZ8/edit#15:14
* dimitern is afk for a bit15:14
dooferladdimitern: IRC race condition!15:14
mupBug #1432421 changed: vivid local failed to retrieve the template to clone <deploy> <local-provider> <vivid> <juju-core:Fix Released by ericsnowcurrently> <juju-core 1.23:Fix Released by ericsnowcurrently> <https://launchpad.net/bugs/1432421>15:14
marcoceppi_dimitern: I'm a strong +1 for net-space, space is just too vague of a name in my opinion (imagine telling someone to type a command 'just type juju space..."space")15:16
natefinch_marcoceppi_: good point on confusing space w/ spacebar-space15:23
voidspaceI'm thinking of changing my email address to dot-space.dash@voidspace.org.uk15:23
natefinch_voidspace: lol awesome15:24
jw4voidspace: hilarious15:24
marcoceppi_haha15:24
voidspacenot my joke, but still funny15:24
natefinch_voidspace: what you really need to do is change *someone else's* email to that15:24
voidspacenatefinch_: heh, like my parents...15:24
=== natefinch_ is now known as natefinch
natefinchcmars: why do we care that v5 is unstable?  stability only matters to third parties and past releases.15:26
cmarsnatefinch, the unstable suffix is a gopkg convention for better or worse15:29
natefinchcmars: yes but, you're choosing to use it, and I don't understand why15:30
cmarsnatefinch, API versioning is used in a very strict sense here. if we branch something as "v5" then there are very specific rules about what can be changed in that branch and still be called v515:30
cmarsnatefinch, see http://labix.org/gopkg.in, heading "When to change the version"15:31
natefinchcmars: in theory, yes.  But it's all convention.  There nothing actually enforcing it15:31
rick_h_natefinch: what's the harm? we've been bitten in the past with this where the api changed out from under things without a naming update.15:31
cmarsnatefinch, lying about API versions isn't nice15:32
natefinchrick_h_: it's just unnecessary churn.  We already use godeps, so nothing can get broken from changes to the v5 repo... and then all you have to do is update the line in dependencies.tsv to point at the new revision15:32
rick_h_natefinch: right, but when that update to the dep.tsv file is done it seems reasonable that a sanity check would be to find all use of something and make sure nothing between those revisions changed and would break the code?15:33
rick_h_natefinch: or do you all just rely on CI/tests to catch it all?15:33
cmarsnatefinch, its not unnecessary. its explicit. by importing a v5-unstable, you're saying, "yes, I know this package may change its API contract, and I'm ok with that"15:33
cmarsnatefinch, by changing that to a "v5" proper, you're saying, I depend on this v5 API contract. its kind of similar to what we do for facade versions15:34
cmarsnatefinch, its also a nice indicator that adds pressure in the right places. if you see an unstable API import, it should make us all think about what it takes to stabilize the APIs we import.15:36
natefinchcmars, rick_h_:  ok ok :)   I guess if we can't just land all the changes for v5 in core at once, it's a useful marker to say "hey, this is not safe to release as-is"15:39
mattywI'm staying out of this - but it's worth a simple reminder that core isn't the only "large" project that uses the charm package15:40
rick_h_mattyw: lol, sucessfully dodged there :P15:40
natefinchmattyw: sure.  but it's still all "us"15:41
cmarsnatefinch, thanks. yes, we want to release stable APIs. definitely.15:41
dimiterndooferlad, that test failure is genuine btw15:42
natefinchcmars: it sort of feels like landing broken code in master, is all.  If we're intentionally landing code we know we can't release15:42
dooferladdimitern: I know15:43
dooferladdimitern: I missed the dependencies.tsv update15:43
dimiterndooferlad, ah, good point15:43
cmarsnatefinch, unstable here does not mean broken. it has a very precise meaning: "the package API is subject to change"15:43
dimiterndooferlad, but how about the main tests in cmd/juju?15:43
natefinchcmars: I said "sort of" :) .... it's code that means we can't release until it's fixed.15:44
dooferladdimitern: just running them. Will see.15:45
urulamanatefinch, cmars: out of curiosity, this v5-unstable change does not influence the 1.23 code, right?15:47
urulamanatefinch, cmars: it should only be included for 1.24, for which we have enough time to make it into proper "v5"15:48
natefinchI promise I'm not trying to be a pain in the ass... It's just that this will become precedent, and we'll have to start doing it with every versioned package, so I want to make sure the process is well thought out.15:49
cmarsurulama, right, its only for master15:49
dooferladdimitern: you were right, looks like I missed adding "space" to a big array of strings.15:49
dimiterncmars, natefinch, urulama, btw, let's not release a version depending on anything -unstable, as I commented on that thread15:50
urulamadimitern: yes, 100% agree on that15:50
dimiterndooferlad, yep, that's it15:50
dimiternurulama, cool15:51
dimiternas a temporary step towards v5 it fine though IMO15:52
dimiternit's15:52
natefinchI think it's fine, or at least, close enough to fine that I don't need to argue about it any more. :)15:53
urulamanatefinch: :) next time we introduce something "-unstable", we'll do proper announcement to avoid any "wth is this thing doing here" mild-hear-attacks :)15:55
dimiternurulama, it doesn't count unless there's also special landing page about it on ubuntu.com :D15:58
urulamadimitern: what about on jujucharms.com, we can do that :)15:59
dimiternurulama, tempting :)15:59
dimiterndooferlad, it looks like the bot doesn't love you today16:01
dooferladdimitern: well, I haven't pushed the last fix yet. Thought at this rate I should tests on my machine(s) before trying again.16:02
dimiterndooferlad, :)16:03
dimiternok, I think I should call it a day16:04
dooferladdimitern: have a good evening!16:04
dimiterndooferlad, cheers, same to you!16:04
natefinchfinally... disabled the stupid ctrl+q and ctrl+w so I don't accidentally close windows when I typo ctrl-s or ctrl-a16:12
natefinchericsnow: btw, that code to restart the jujud service works like a charm16:24
ericsnownatefinch: awesome16:25
ericsnownatefinch: are you going to fix restore too?16:25
natefinchericsnow: yeah, I'll do it as a separate patch after16:25
ericsnowcool16:25
natefinchericsnow: it unfortunately (and totally unsurprisingly) does nothing to help make what happens after the restart work better.16:27
cmarssinzui, how is LP: #1435857 different from LP: #1427879 ?16:27
mupBug #1435857: panic: osVersion reported an error: Could not determine series <ci> <packaging> <regression> <vivid> <juju-core:Triaged> <juju-core 1.22:Triaged> <juju-core 1.23:Triaged> <https://launchpad.net/bugs/1435857>16:27
mupBug #1427879: juju cannot deploy when distro-info-data gets a new series <deploy> <series> <juju-core:Fix Committed by wallyworld> <juju-core 1.22:Fix Committed by wallyworld> <juju-core 1.23:Fix Committed by wallyworld> <https://launchpad.net/bugs/1427879>16:27
cmarssinzui, and does master need to be blocked on these?16:28
sinzuicmars, we cannot 80% of the tests because of that bug16:29
sinzuicmars, merging a an unproven change into every branch was risky16:29
sinzuicmars, master can be made unaffected by rolling the change out16:30
sinzuicmars, that bug is so massive that few tests can be run http://reports.vapour.ws/releases/247716:30
cmarssinzui, ok, thanks16:31
ericsnowsinzui: generate-docs.py (indirectly) runs juju.  In the publish-revision job, which juju is it running?16:49
ericsnowsinzui: I would expect it to be the one that is being built, but it looks like it just gets juju from $PATH16:50
sinzuiericsnow, we are retesting an older rev of master hoping to find a bless16:50
sinzuiericsnow, I imaging you can reproduce the problem by running juju version using tip of any branch. Append a series to /usr/share/distro-info/ubuntu.csv16:51
ericsnowsinzui: I did that and it worked fine for me (added the Angsty one from the bug)16:52
sinzuiericsnow, That command was run in chroot building a package16:52
ericsnowsinzui: I should clarify, generate-doc.py finished successfully16:53
sinzuiericsnow, sorry, in meeting.17:05
ericsnowsinzui: np17:05
sinzuiericsnow, since packaging find this, and packaging is in a chroot, I think the angsty issue may not be in play. The only juju that can be used it the juju that was just built17:05
mupBug #1435974 was opened: Copyright information is not available for some files <juju-core:New> <https://launchpad.net/bugs/1435974>17:09
sinzuiericsnow, when I am out of a meeting I will find the build script any of us can run locally with atarball17:13
ericsnowsinzui: k, thanks17:13
=== kadams54 is now known as kadams54-away
fwereadegaaaaaaaah import cycles17:27
=== kadams54-away is now known as kadams54
sinzuiericsnow, yuck. Ci still doesn't save the successful source packages. I need to write up the steps that went from release tarball to build-package-with-dsc. I now suspect that since a chroot was involved, that NO the systems didn't have distro-info-data at all.18:11
ericsnowsinzui: I just updated the bug with what I've found (sans solution)18:13
ericsnowsinzui: the only way we would see the panic we do is if /usr/share/distro-info/ubuntu.csv was there and only slightly invalid (with respect to juju's expectations)18:14
sinzuiericsnow, I don't this vivid is an issue. publish-revision fails the first time it tries to make binary packages. Vivid just happens to be the first source package it is trying to make18:15
ericsnowsinzui: juju expects lines in that file to look like: "12.04 LTS,Precise Pangolin,precise,2011-10-13,2012-04-26,2017-04-26"18:15
ericsnowsinzui: from the logs I looked at utopic passed before it failed with vivid18:15
ericsnowsinzui: build-162518:16
sinzuioh, I missed that18:16
sinzuiericsnow, I see18:17
ericsnowsinzui: either that CSV file is busted on the slave on which we're building, they've released a busted file, or they've changed the format18:17
sinzuiuhg18:17
* sinzui thinks18:17
ericsnowsinzui: it's on vivid-slave18:18
ericsnowsinzui: would it be the /usr/share/distro-info/ubuntu.csv that is still there?18:19
ericsnowsinzui: if so we can take a look at the file and see what's going on18:19
sinzuiyes, the same slave that unit-tests run on. run-unit-test added the angsty line to ensure juju doesn't assume the series in ubuntu.csv18:19
mupBug #1435999 was opened: juju run should have an --all-units option, --all is ambiguous and confusing <juju-core:New> <https://launchpad.net/bugs/1435999>18:21
wwitzel3is there docs for the dev flow if you need to update a /juju package, that we import using gopkg.in? In this case, I have some changes to juju/charm18:26
wwitzel3do I just change the import and then ensure the landing order is such that I update the import after the dep lands and have a new gopkg.in version?18:28
katcowwitzel3: i can't recall having to do that; what you suggest certainly sounds reasonable.18:33
katcowwitzel3: thumper would be a good person to ask; i think he's done a lot of work with juju/testing18:34
sinzuiericsnow, these are the two bugs I reported about vivid unit test https://bugs.launchpad.net/juju-core/+bug/1433577 and https://bugs.launchpad.net/juju-core/+bug/140845918:38
mupBug #1433577: Vivid unit tests need to pass <ci> <test-failure> <vivid> <juju-core:Triaged> <https://launchpad.net/bugs/1433577>18:38
mupBug #1408459: pingerSuite tests consistenly fail on trusty+386 and vivid+amd64 <ci> <i386> <intermittent-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1408459>18:38
ericsnowsinzui: I'll take a look at the failing tests in the most recent runs18:42
sinzuiericsnow, CI completed the build on vivid. I need to see a few more machines complete though. I fear more than one ubuntu.csv was tampered with18:45
ericsnowsinzui: okay18:46
ericsnowsinzui: looking at the most recent vivid unit test failures, there's a possibility that they could pass on the next build on master18:50
sinzui:)18:50
ericsnowsinzui: if not, they're pretty close to passing and I'll work on getting that resolved18:50
ericsnownatefinch: were you going to be able to review my two patches?18:54
natefinchericsnow: yes, sorry, been heads down on the HA stuff, but I shouldn't shirk my reviewing responsibilities either18:56
natefinchericsnow: I'll look now18:56
ericsnownatefinch: thanks18:56
natefinchkatco: just saw your comment here about AgentConf doing too much.... what, passing effectively four copies of it to a single function is too much? :)  https://github.com/juju/juju/blob/master/cmd/jujud/main.go#L12619:13
katconatefinch: haha19:13
katconatefinch: you see why i did that though right? does the comment make sense?19:14
natefinchkatco: not to mention that the line there is like embedding three factories into each other19:14
natefinchkatco: oh yeah, totally19:15
natefinchkatco: it makes it very clear that agentConf is doing too much, and in theory makes it easy to break out part of the functionality into separate bundles19:15
katconatefinch: ok good. at first glance i was like, "what the hell kind of code did i write?"19:15
natefinchI do like that the line is effectively19:16
natefinchjujud.Register(agentcmd.NewMachineAgentCmd(agentcmd.MachineAgentFactoryFn(&agentConf, &agentConf), &agentConf, &agentConf)19:16
katcohehe19:16
natefinchand by like... I mean it's atrocious, but I know that's not your fault :)19:16
katconatefinch: that is a great trick for breaking up large things: define finer-grained interfaces -- all of which the large thing implements. then you can peel off functionality19:16
natefinchkatco: this is my favorite use of interfaces.... lets the function only call out what it needs, then you can replace what you pass it with impunity.19:17
katcothe definition of an interface :)19:17
* katco back to reading a spec full-screen. ping me if you need my attention.19:19
natefinch:)19:19
natefinchsinzui: seen this? https://wiki.jenkins-ci.org/display/SECURITY/Jenkins+Security+Advisory+2015-03-2319:24
sinzuinatefinch, I had not, but that is exactly why Ubuntu removed it from its packages19:26
mupBug #1435857 changed: panic: osVersion reported an error: Could not determine series <ci> <packaging> <regression> <vivid> <juju-ci-tools:Fix Released> <juju-core:Invalid> <juju-core 1.22:Invalid> <juju-core 1.23:Invalid> <https://launchpad.net/bugs/1435857>19:27
ericsnownatefinch: of my 2 patches, could you take a look at http://reviews.vapour.ws/r/1233/ first?19:32
natefinchericsnow: looking19:33
ericsnownatefinch: thanks19:34
mupBug #1435857 was opened: panic: osVersion reported an error: Could not determine series <ci> <packaging> <regression> <vivid> <juju-ci-tools:Fix Released> <juju-core:Invalid> <juju-core 1.22:Invalid> <juju-core 1.23:Invalid> <https://launchpad.net/bugs/1435857>19:36
=== kadams54 is now known as kadams54-away
mupBug #1435857 changed: panic: osVersion reported an error: Could not determine series <ci> <packaging> <regression> <vivid> <juju-ci-tools:Fix Released> <juju-core:Invalid> <juju-core 1.22:Invalid> <juju-core 1.23:Invalid> <https://launchpad.net/bugs/1435857>19:48
natefinch3 hours sleep makes me a much slower reviewer, I've figured out19:52
=== kadams54-away is now known as kadams54
natefinchthumper: I know you have a stand up now, but afterward, I'd like to ask about why workers can all shutdown like this, and not cause jujud restart: http://paste.ubuntu.com/10672321/20:00
katconatefinch: i know your pain. i *know* it...20:00
katconatefinch: re: 3h of sleep20:01
katconatefinch: i was sick all last weekend, and my daughter decided this was the perfect time to stay up all night, every night, growing teeth.20:01
natefinchkatco: heh.. yeah, the baby normally sleeps great, but last night he decided it was time to wake up at 3am.. tried for an hour and a half to get him to go back to sleep to no avail.  So I got up with him, made some coffee... and of course then he fell asleep.  Since I have been getting up at 5am, I decided to just stay up20:01
natefinchkatco: ouch20:02
* katco nods20:02
katcothe only positive was that it was the weekend20:02
katcobut was up at 4:30 this morning o.020:02
natefinchkatco: I feel ya20:03
natefinchericsnow: https://github.com/juju/utils/blob/master/shell/renderer.go#L41  passing "" to indicate runtime.GOOS is very non-intuitive, IMO.20:04
* natefinch loves that github has blame just a click away ;)20:05
ericsnownatefinch: :)20:05
ericsnownatefinch: it's meant to be a convenience so you don't have to import runtime for what should be the common case20:05
natefinchericsnow: I'd prefer to split it out into a separate function.   Actually... the fact that you pass in a bunch of magic strings into this function is suboptimal in general.20:06
ericsnownatefinch: if it is confusing then it would probably be better to not bother20:07
natefinchericsnow: renderer.NewOSRenderer()20:07
ericsnownatefinch: ah, that would work20:07
natefinchericsnow: actually... these are all publicly exposed values it's returning... why does this method need to exist?20:08
ericsnownatefinch: the magic string thing corresponds to what runtime.GOOS might give you20:08
katconatefinch, ericsnow : fwiw, i like ctors because they document the minimum needed parameters20:09
natefinchericsnow: I mean, doing it based on runtime.GOOS is fine... but when are you going to pass "powershell" and not just instead do &renderer.PowershellRenderer{}20:09
natefinchkatco: but if the types are exposed, then I have to figure out when I need the constructor and when I can just use the type20:09
ericsnownatefinch: yeah, that is probably a bit premature :)20:10
katconatefinch: wouldn't you just assume to use the ctor if it's present?20:10
ericsnownatefinch: it probably would be better to just yank all that aliasing stuff20:10
katconatefinch: i mean if not, i guess you can use the raw type if you like. but the author seems to be saying: "here's how you allocate me" with ctors20:11
ericsnow(for "shell names")20:11
ericsnownatefinch: NewRenderer is still useful in the case of targeting linux from windows or vice versa20:12
=== kadams54 is now known as kadams54-away
thumperrick_h_: hey there20:17
thumperrick_h_: can we book a chat sometime soon?20:17
rick_h_thumper: sure thing, I'm actually out until next week after tomorrow20:18
thumperrick_h_: so soon then...20:18
rick_h_thumper: have 45min now or can do tomorrow evening20:19
rick_h_thumper: or can do next wed (your thurs morning?)20:19
=== kadams54-away is now known as kadams54
thumperrick_h_: on a call now20:23
rick_h_thumper: rgr20:23
thumperrick_h_: wanting to talk about shared delta state20:23
rick_h_thumper: yea, talked (well emailed) with jam and he noted we should touch base20:24
natefinchericsnow: at the very least, the magic (non-OS) strings should be factored out into an enum of some sort.  There's no way, other than looking at the implementation, that anyone would know to pass in "bat" right now.20:31
ericsnownatefinch: there's no need for them so I'm dropping them20:32
ericsnownatefinch: but that's not happening in the commit you're reviewing :)20:32
natefinchericsnow: right20:32
ericsnownatefinch: I'll put something up in a bit20:33
sinzuiCan someone help triage https://bugs.launchpad.net/juju-core/+bug/1431286 ? comment 6 has a log that shows everything looking good until the moment the jujud is stated and it just exits because of "no instances found"20:33
mupBug #1431286: juju bootstrap fails when http_proxy is set in environments.yaml <ubuntu-openstack> <juju-core:Incomplete> <https://launchpad.net/bugs/1431286>20:33
natefinchkatco: I guess I always try to make sure there's only one way to do it, otherwise someone is bound to do it the Wrong Way™20:34
natefinchkatco: yes, in theory, if the constructor is there, you should use it, but it's not always clear that you shouldn't just create the types, too.... it's tricky.20:34
katconatefinch: i agree. i prefer ctors, so i guess i look for them.20:35
katconatefinch: actually in go, they're more like factories aren't they?20:35
katconatefinch: because they're only tied to the type in that they return an instance20:36
natefinchkatco: yep.  I prefer just being able to create a type, but that's not always possible with complicated values20:36
katconatefinch: i like knowing which fields i have to populate20:36
natefinchkatco: absolutely.  If the default values aren't valid, then a constructor is the way to go. But I hate having confusion about public types that look like you can construct them manually.  Like I said, it's tricky. Sometimes a constructor function is absolutely the right thing.20:38
natefinchericsnow: what's the second one you need reviewed?20:41
ericsnownatefinch: http://reviews.vapour.ws/r/1234/20:41
ericsnownatefinch: http://reviews.vapour.ws/r/1256/ too, now20:42
natefinchericsnow:  I think you can write code faster than I can review it ;)20:42
ericsnownatefinch: maybe on a day when you are having trouble keeping your eyes open :)20:43
natefinchericsnow: holy crap that's a lot of commits20:47
ericsnownatefinch: yeah, but they're all little20:47
natefinchericsnow: yeah, just surprised me20:47
ericsnownatefinch: :)20:47
natefinchericsnow: I can't do this review justice right now, with the time I have left.  I gotta go.  Sorry..20:48
ericsnow(╯°□°)╯︵ ┻━┻20:48
jw4coreycb: is it possible for you to add the environment config file for your bug 1431286 (scrubbed of any private info of course)20:51
mupBug #1431286: juju bootstrap fails when http_proxy is set in environments.yaml <ubuntu-openstack> <juju-core:Incomplete> <https://launchpad.net/bugs/1431286>20:51
coreycbjw4, should already be in there I think20:52
jw4coreycb: is it in that proxy logs archive?20:52
jw4coreycb: hmm not seeing it there, I must just be missing it20:53
coreycbjw4, sorry terrible name, "console log"20:53
jw4coreycb: lol20:53
jw4okay thanks20:53
coreycbjw4, thank you20:54
jw4coreycb: for my education; does the bootstrap work when http-proxy is commented out? i.e. can the juju command "see" the vm's without the proxy?20:56
coreycbjw4, yes it works fine without http-proxy or https-proxy20:56
coreycbjw4, I'm currently hard-coding http(s)_proxy into the charm code as a work around20:56
jw4coreycb: okay, and the proxy (http://squid.internal:3128) does normally successfully relay openstack http/https requests other than from juju?20:57
coreycbjw4, everything openstack-related should be behind the proxy21:02
jw4coreycb: got it21:02
thumperis cmars around?21:06
cmarsthumper, hey21:06
thumperhey, you busy, or want to chat with our 1:1?21:06
ericsnowwallyworld_: thanks for the review; I've updated the patch22:12
wallyworld_ericsnow: ty, looking22:13
wallyworld_ericsnow: that patched newTimer now appears to not change anything22:15
ericsnowwallyworld_: it allows us to call Reset on it before the expected failure22:16
wallyworld_oh, i see22:16
wallyworld_was that the area where the test was flakey then?22:17
ericsnowwallyworld_: it's that we were using a 50 ms timeout and on some hosts that isn't long enough22:18
wallyworld_ok22:18
ericsnowwallyworld_: my patch takes a more direct approach by manually controlling the timeout22:18
redelmannhi everyone!22:32
redelmanntodaby i notice im using juju 1.23-beta1, but all my agents are 1.22.022:33
redelmanntoday <= todaby22:33
redelmannits that ok?22:33
ericsnowwallyworld_: I don't think my patch is testing what needs to be tested (that a Ping resets the timeout)22:38
ericsnowwallyworld_: I'll have an update shortly22:38
wallyworld_ok22:38
wallyworld_redelmann: that's ok, 1.23 CLI can talk to 1.22 agents just fine22:39
redelmannwallyworld__ Thank, any online howto to upgrade agents?22:41
wallyworld_"juju upgrade-juju"22:41
ericsnowwallyworld_: I've updated the patch with a check that Ping does reset the timeout22:58
wallyworld_ok22:58
ericsnowwaigani: thanks for the review :)22:59
wallyworld_ericsnow: i think it looks good to go, reset count was a nice addition23:01
ericsnowwallyworld_: oh good, ideally we could have left that test alone but the whole timeout/sleep-induced failures makes the situation messier23:02
wallyworld_yeah23:02
axwkatco: when I replied to your G+ post, I didn't realise I was commenting on YouTube ;(23:35
axwI try to keep out of that comment cess pit23:35
katcoaxw: yeah, they're linked now =/23:45
katcoaxw: asynch. convo (have to draw bath for daughter). had you heard of her there?23:46
katco(in aus)23:46
axwkatco: I have not, but I don't listen to the radio much or pay close attention to local-ish music23:46
axwhaven't been listening to this type of music for a while now either, really23:47
katcofinally get cinder support into goose. wallyworld_ gets the authored credit. way to be a manager wallyworld_! ;)23:56
wallyworld_katco: noooooo :-(23:57
katcohaha23:57
katcohey what is the last number in the dependencies.tsv file for?23:59
katcoit looks like it's probably the commit date/time?23:59

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