/srv/irclogs.ubuntu.com/2015/04/01/#juju-dev.txt

redelmann_thumper, https://bugs.launchpad.net/juju-core/+bug/143895100:06
mupBug #1438951: destroy-enviroment --force destroy all aws instances <juju-core:New> <https://launchpad.net/bugs/1438951>00:06
redelmann_thumper, y cant give more info about this, and i cant test it again in current aws00:07
thumperredelmann_: thanks00:07
ericsnowwallyworld_: regarding IsLocal, the key thing is that we are checking if the init system corresponding to the package is the one running on the *local* host00:11
wallyworld_IsLocal() doesn't imply that00:12
ericsnowwallyworld_: in code it won't be "IsLocal()", it will be "systemd.IsLocal()", etc.00:12
ericsnowwallyworld_: k00:12
wallyworld_hmmm00:12
wallyworld_IsUsed() perhaps00:12
wallyworld_systemd.IsUsed()00:12
wallyworld_wadda ya reckon?00:13
ericsnowwallyworld_: IsCurrentlyTheOneRunningOnTheLocalHost? :)00:13
wallyworld_always one smarty pants00:14
wallyworld_systemd.IsRunning()00:14
wallyworld_what happens if both files are there though?00:14
ericsnowwallyworld_: IsRunning is good00:14
wallyworld_can a host have both binaries installed?00:14
ericsnowwallyworld_: the Ubuntu guys promised me that that will *never* be a problem00:14
ericsnow:)00:15
wallyworld_promises, yeah right00:15
ericsnowwallyworld_: how about IsRunningLocally?  too long?  I was hoping that it would be super clear that we're only checking the local host00:16
ericsnowwallyworld_: BTW, thanks for the review :)00:16
wallyworld_how is IsRunning() not implying that?00:16
wallyworld_sure :-)00:17
wallyworld_wouldn't it be assumed that it pertains to the host on which it is run?00:17
ericsnowwallyworld_: yeah, you're right; the lack of args limits the possible interpretations :)00:17
wallyworld_ericsnow: i started reviewing the other one. i don't understand why ioutil.TempFIle is not used00:21
wallyworld_wasn't the issue that we were using /tmp/somestaticfilename00:21
wallyworld_so /tmp/somerandomfilename would be fine00:22
ericsnowwallyworld_: right00:22
ericsnowwallyworld_: but I want to be consistent between the shell script and the Go code00:22
wallyworld_what do you mean?00:22
ericsnowwallyworld_: the patch fixes both the discovery script (written in bash for remote use) and the Go code that does the same thing00:23
wallyworld_both can still use a random tmp dir00:24
ericsnowwallyworld_: I looked at using mktemp on the shell side to do that, but doing it made the code (not insignificantly) more complicated00:25
wallyworld_hmmm00:26
ericsnowwallyworld_: using a subdirectory of the juju data dir helped it stay simple (er)00:26
wallyworld_i'm not across the implementation, why is using mktemp in the shell script hard?00:27
ericsnowwallyworld_: because subsequent commands must be written in relation to where that script gets written00:29
wallyworld_what generates the shell script? juju? why can't the dir be a script arg?00:30
ericsnowwallyworld_: juju does (it's all in service/discover.go + service.go)00:31
mupBug #1438951 was opened: destroy-enviroment --force destroy all aws instances <juju-core:New> <https://launchpad.net/bugs/1438951>00:35
ericsnowyikes, mup!00:36
wallyworld_ericsnow: so, i'm missing something maybe. in ListServicesScript(), we have const filename = "/tmp/discover_init_system.sh, and that is passed to the script generation commands. why not just generate a random file and pass that00:37
ericsnowwallyworld_: so just generate a random filename under /tmp and use that on the remote host?00:40
wallyworld_oh you are generating the scripts locally and then running remotely00:40
ericsnowwallyworld_: correct00:41
ericsnowwallyworld_: currently all for the sake of manual provider :)  though I have considered using some of this for the cloudinit script00:42
wallyworld_can init_system=$(blah) run the script inline then?00:42
wallyworld_rather than from a filename00:42
wallyworld_then the problem goes away00:42
ericsnowwallyworld_: perhaps00:43
wallyworld_this is a one off bit of bash I think?, so no need to write it out i don't think00:44
ericsnowwallyworld_: originally that is effectively what I was doing, but I had to add more complicated logic to the script, I switched to writing the script out to a separate file00:44
wallyworld_bash doesn't care about how complicated it is when it runs it00:45
ericsnowwallyworld_: having a separate script certainly requires much less knowledge of bash :)00:45
ericsnowwallyworld_: it's more for my and our sakes than for bash's :)00:45
ericsnowwallyworld_: biab00:46
wallyworld_ok00:46
ericsnowwallyworld_: back01:00
ericsnowwallyworld_: did I mention that bash is the devil?01:01
wallyworld_yeah01:01
wallyworld_i like tcsh01:01
ericsnowwallyworld_: hey, isn't Python system-installed everywhere nowadays...01:02
wallyworld_so ioutil.TempFile is nice and portable, and simple to use, none of this data dr and series stuff andeep juju's dir clean01:02
wallyworld_can't count on it maybe? not sure?01:02
wallyworld_the script is only 20 lines or so, so surely it can be run inlien and all this just goes away01:04
ericsnowwallyworld_: I'll work up a different approach in that patch01:04
wallyworld_ok, sorry to be a party pooper, it's just my opinion, maybe a different reviewer would feel differently01:05
ericsnowwallyworld_: after that other patch you reviewed the script gets a lot shorter and simpler01:05
wallyworld_yeah01:05
ericsnowwallyworld_: I'll get that landed and see where things are at01:05
wallyworld_i was having a big problem with the various g=hacks also01:05
ericsnowwallyworld_: and hey, I appreciate you taking a stand :)01:05
wallyworld_and this will remove those01:05
axwwallyworld_: just gave you a shipit01:59
wallyworld_axw: awesome, ty02:00
ericsnowwallyworld_: I think you'll like this better: http://reviews.vapour.ws/r/1351/02:16
wallyworld_ok02:16
wallyworld_ericsnow: yay, looks much better02:20
wallyworld_it works?02:20
ericsnowwallyworld_: thanks for sticking to your guns :) I like it better too02:20
ericsnowwallyworld_: yep02:20
wallyworld_\o/02:20
ericsnowwallyworld_: there's a unit test that checks02:20
wallyworld_yeah, much cleaner02:20
ericsnowwallyworld_: thanks for those reviews02:26
wallyworld_np :-)02:27
ericsnowwallyworld_: I'll land those for 1.23 now-ish and then vanish into the night02:28
wallyworld_sure02:28
wallyworld_i'll re-merge if there's a spurious failure02:28
wallyworld_ty02:28
thumperwallyworld_: got a few minutes?03:30
thumperwallyworld_: I need an insanity check03:30
wallyworld_ok, give me 2 mins03:31
thumperkk03:31
wallyworld_axw: would be awesome if you got to look at this sometime if you are free http://reviews.vapour.ws/r/1354/04:34
axwwallyworld_: sure, will look in a little while04:35
wallyworld_np ty, i have to duck out for an hour or so anyway04:35
wallyworld_soon04:35
jogwallyworld_, still around?05:11
jamwallyworld_: ping05:49
axwwallyworld_: I'll have to read the spec before I can review, so probably won't get to it until I'm OCR tomorrow sorry06:10
* dimitern steps out - back in a few hours06:37
wallyworld_jam: hi, i was out, about to have dinner, can i ping you soon?07:29
jamwallyworld_: np07:30
jamI was just checking if there was anything you want me to bring up to Mark.07:30
jamI know you had the Storage "hints" vs "properties"07:31
jamwallyworld_: but any other information about things you want clarity on for Storage or Health, I'm on the call with him now07:31
TheMuemorning o/07:45
TheMuedimitern: I'll be working in an interrupted way today. coordinating cleaning of the ground, expecting the reviewer, calling the garage to pick the car etc07:46
voidspaceTheMue: sorry about your car!07:46
voidspacedimitern: I've made those tweaks07:46
voidspacedimitern: I'll land the branch so I can fix the issue with upgrading and then forward port the fix to trunk07:47
TheMuedimitern: regarding the fix imho our tests showed that it isn't the primary interface. so now I'll focus on the empty interfaces, because they are the only difference returned from the lshw parsing function07:48
TheMuedimitern: so I'll compare the values and follow their usage bottom-up to see, how they influence the further process07:49
TheMuevoidspace: thanks, yes. thankfully first feedback is, that it likely is full covered07:50
voidspaceTheMue: *phew*, what a hassle :-/07:50
wallyworld_jam: with "juju status-history <unit>", I was going to default to say 10 recent status changes with an option -n arg. Also you would see agent and workload status interleaved07:51
TheMuevoidspace: now it's "only" all that organizational stuff as well as looking/waiting for a new one07:51
jamwallyworld_: mark likes "hints"07:51
wallyworld_ok07:51
wallyworld_jam: with the legacy agent state, I added Stopped (so now we have Pending, Error, Started and Stopped07:52
wallyworld_jam: the status interleaving is so you could see that an agent ran X hook, which caused the unit to report Y status07:52
wallyworld_jam: we also firmly believe that when a hook fails, it should be the agent that is in error, not the workload (as the workload is most very likely still fine). Nonetheless, I followed the spec and we can discuss again if users complain07:54
* dimitern is back08:16
dimiternTheMue, voidspace, thanks guys, sounds good - keep me posted :)08:17
voidspacedimitern: fix for upgrade on 1.23 nearly ready for review - just pushing it now08:18
dimiternvoidspace, awesome!08:19
jamwallyworld_: "juju status-history" sounds good to me, can you put it into the doc as you've developed it?08:28
wallyworld_jam: haven't put fingers to keyboard yet; that's how i propose to do it, will update doc08:28
jamwallyworld_: for the workload, we're certainly in a situation where "we have no idea what is going on" once we're in an error state. So there are at least questions about what is the smallest lie08:28
wallyworld_jam: agreed to an extent. we can see how it unfolds once in the hands of users. william has very strong views08:29
dimiternvoidspace, I assume your latest PR includes both the worker and the fix?08:32
voidspacedimitern: ah yes08:37
voidspacedimitern: yeah, to fix on 1.23 I needed to base it off the release-address branch which has the worker08:37
voidspacedimitern: once that branch lands the diff will be smaller...08:37
dimiternvoidspace, right, got it08:37
dimiternvoidspace, reviewed08:38
voidspacedimitern: thanks08:39
voidspacedimitern: they're good comments - except for the very last one08:39
dimiternvoidspace, oh yeah? :)08:39
voidspacedimitern: what do you mean by an extra test for a removed machine?08:40
voidspacedimitern: we have a test for a machine that doesn't exist08:40
voidspacedimitern: what's the difference between that and a removed machine?08:40
dimiternvoidspace, no, that's fine then08:40
dimiternvoidspace, I must've missed it08:40
voidspacedimitern: the SetUp code that creates dead addresses allocates them to a non-existent machine08:40
dimiternvoidspace, right, all good then :)08:41
voidspacewhich simulates the situation we're guarding against08:41
voidspaceI'll fix the other issues you pointed out, thanks08:41
dimiternvoidspace, cheers08:41
dimiterndooferlad, hey08:41
dooferladdimitern: hi08:41
dimiterndooferlad, I've realized while reviewing your update command that some bits of the spec are contradictory08:42
dooferladdimitern: ah08:42
dimiterndooferlad, we shouldn't allow a space with no subnets08:42
dimiterndooferlad, and create has to be changed slightly to accept at least one subnet08:42
dooferladdimitern: I was pondering this yesterday, but after reading the spaces and subnets sections of the spec several times it seemed that just implementing what it said was the way to go for now.08:43
dimiterndooferlad, that's due to the way I plan to implement spaces on AWS at least - using tags on subnets08:43
dooferladdimitern: I agree that a space without subnets doesn't make any sense08:44
voidspacedimitern:  logger.Debugf("requested ReleaseAddress of address %q for unknown instance, ignoring", addr.Value)08:44
dimiterndooferlad, I agree - with nothing better to go with :) sure08:44
voidspacedimitern: will change to (ignoring) - I think that's your preferred style08:44
dimiternvoidspace, I'd go with "release address for an unknown instance ID is a no-op, ignoring" ?08:45
dimiternvoidspace, or (ignoring) at the end - both are fine08:45
voidspacedimitern: and not log the actual address?08:46
dimiternvoidspace, ah, good point - please do :)08:46
dimiternrelease address %q ...08:47
voidspacedone08:47
dimiternTheMue, voidspace, standup?09:01
voidspacedimitern: oops, omw09:04
voidspacedimitern: the forward port to trunk: http://reviews.vapour.ws/r/1358/diff/#09:17
dimiternvoidspace, LGTM09:19
=== natefinch-dinner is now known as natefinch
natefinchdimitern: saw your review on my WIP branch for ensure-availability converting machines... after looking at the code, I think I understand the problems with the API stuff, but want to make sure I'm going down the right path.09:59
dimiternnatefinch, great, do you think we need to have a chat or?10:00
dimiternnatefinch, I'm glad to help if you have things to ask as well10:01
natefinchdimitern: yeah, but I have a lightly sleeping baby next to me, so it'll need to be text. :)10:01
dimiternnatefinch, sure :)10:01
natefinchdimitern: So, I presume I should be using UnitsWatcher to watch a machine, instead of rolling my own, correct?10:02
natefinchdimitern: wayne wrote the watcher code for this, and I think didn't realize UnitsWatcher was also for watching machines (it took me a while to figure that out too... actually just got lucky seeing a comment about how it also worked for machines)10:02
dimiternnatefinch, hmm UnitsWatcher IIRC reports changes to a service's units10:03
dimiternnatefinch, i might be missing some context10:04
natefinchdimitern: it seems to work on machines and services in practice, plus the code just takes a tag10:04
* dimitern revisits the PR10:04
natefinchdimitern: you were pretty vague about what was wron with the code, so maybe you should start by explaining what you meant: http://reviews.vapour.ws/r/1299/10:05
natefinch:)10:05
dimiternnatefinch, right, sorry - I tried to explain 3 times, but each time my connection dropped or something else was wrong, so I lost 3 separate summaries (growing shorter with frustration)10:06
natefinchha, I know how it is.10:06
dimiternnatefinch, yeah, so let me have another look now and will explain10:06
natefinchdimitern: thanks10:07
natefinchdimitern: I think I have an idea, after looking at the other API endpoints, but want to make sure I'm on the right track.10:07
dimiternnatefinch, ok, so one thing is definitely about using the error result for "fatal" errors, while using the Error field of the result structs for per-tag issues (e.g. failed to start watching machine X's units)10:08
dimiternnatefinch, that's in the client-side api methods10:08
dimiternnatefinch, and checking that if you asked for 1 tag you got 1 result and there wasn't an error in the result's Error field10:09
dimiternnatefinch, this applies to both client methods10:09
natefinchdimitern: ok, yeah, trhat's what I was thinking10:11
dimiternnatefinch, on the server-side, it seems you are taking a bunch of args (tags), but in some cases returning on error with one of them (e.g. ParseMachineTag failed), rather than continuing10:11
mattywdimitern, ping when you have a moment10:11
dimiternmattyw, sure, in a little bit10:11
mattywdimitern, sure, I'll ping you some stuff for you to take a look at at your convenience if that's ok?10:11
dimiternmattyw, no worries10:11
dimiternnatefinch, also server-side - it's more common to have a getAuthFunc which returns an AuthFunc(tag) and checks permissions, rather than using the authorizer directly10:12
dimiternnatefinch, the JobsResult struct should have a Error field like other result structs10:13
natefinchdimitern: yep, ok.  This is all exactly what I just fixed up yesterday afternoon... I had hoped that's what you meant.  Of course now that I've fixed it all, it doesn't work anymore, but I'm sure there's just a dumb bug somewhere :)10:14
dimiternnatefinch, finally, please use names.MachineTag rather than string client-side; the rest are trivial formatting things10:15
natefinchdimitern: yep, cool.10:15
dimiternnatefinch, oh, too bad - have you found out why it doesn't work?10:15
natefinchdimitern: not yet, need to put in some logging to see... the handler fires, so that much is working, maybe I mucked up the code that returns the jobs10:17
dimiternnatefinch, I see, well ping me if I can help with something else10:18
natefinchdimitern: thanks10:18
TheMuedimitern: you yesterday wanted to take a look into the cloud-init.log. I still cannot reach it on a failing node, but I'm now on a working one and try to find aspects that are matching to the information generated based on the lshw results10:25
TheMuedimitern: do you had something special in mind to look at or only wanted to find some kind of errors or strange looking messages?10:26
dimiternTheMue, well, it's hard to understand what's going on without being familiar with cloud-init source - there's a lot of stuff in that log10:26
* TheMue btw should create another vmaas environment for overlapping tests :/10:26
TheMuedimitern: it is, indeed10:27
dimiternTheMue, more useful usually is cloud-init-output.log10:27
TheMuedimitern: ah, thx for that hint, will switch10:27
dimiternTheMue, there could be some errors or warnings in cloud-init.log, but most of them are ok - unless it says something serious (e.g. "cannot start" or "failed ..")10:27
TheMuedimitern: ok10:28
TheMuedimitern: I'm currently adding debugs all the call stack upwards to see how the data influences the processing10:29
dimiternTheMue, +110:29
TheMuedimitern: sometimes I wish my old smalltalk back, breaking the execution with live editing of the code and manipulation of the data10:30
dimiternTheMue, or even a proper debugger with breakpoints and patching the code on the fly? :)10:31
TheMuedimitern: so far no debugger for static languages I've seen during my travel through programming languages had the same simplicity and power of a Smalltalk environment10:34
dimiternTheMue, I haven't used it, but I believe you :)10:37
TheMuedimitern: it's a wonderful language as well as environment (everything is in one so called image). but sadly it doesn't scale on multicore, everything is single-process :(10:38
dimiternTheMue, I see - well, multicore CPUs where not that common at the time I guess10:39
TheMuedimitern: exactly, but multicore isn't so young anymore too, so one could at least have added this as a feature to the VM and then the according code. maybe I should port it to the Erlang VM, like LFE is doing it for LISP :D10:43
dimitern:)10:43
TheMuebut then I would need a time compressor before, putting 48h into a day10:44
tasdomasI'm seeing test failures like this when I run tests in juju master:10:51
tasdomashttp://paste.ubuntu.com/10717600/10:51
tasdomasthe weird part is that I'm getting these with go1.4, go1.3 and go1.2.210:51
tasdomasthey seem to be an ordering issue10:52
tasdomasbut as far as I can tell, nobody can reproduce them10:52
tasdomascould I have messed something up in my environment?10:52
natefinchfirst off, only use 1.2.2 .. we don't officially support newer versions (and I know there's a problem at least with 1.4)10:55
mupBug #1439112 was opened: cmd/juju DeploySuite.TestUpgradeCharmDir fails on windows <test-failure> <windows> <juju-core:Triaged> <https://launchpad.net/bugs/1439112>10:55
dimiterntasdomas, is than on master?10:55
tasdomasdimitern, yes10:56
tasdomasdimitern, I've been able to reproduce that with a clean checkout of juju (go get github.com/juju/juju) in an empty GOPATH10:56
dimiterntasdomas, and is it random or you can repro it reliably?10:56
tasdomasdimitern, can reproduce it every single time10:57
dimiterntasdomas, ok, that's good - we had reports about those tests but they were unreliably reproducible10:58
dimiterntasdomas, it's not an ordering issue as there are not maps involved there at all, something else is wrong10:59
natefinchdimitern: it could still be an ordering issue :)11:00
tasdomasdimitern, well, if I can help with solving this let me know11:00
natefinchdimitern: it's a slice :)11:00
dimiternnatefinch, with slices?11:00
dimiternnatefinch, :) ah, right11:00
dimiterntasdomas, can you replace that gc.DeepEquals with jc.DeepEquals, run the tests a few times and paste the result please?11:02
tasdomasdimitern, sure11:03
tasdomasdimitern, still a failure11:05
tasdomashttp://paste.ubuntu.com/10717644/11:05
dimiterntasdomas, ok, thanks - just to scratch off the easy solution - try one more time with jc.SameContents instead of jc.DeepEquals?11:06
tasdomasdimitern, sure11:07
natefinchI think the lists are just different11:07
dimiternnatefinch, they are the same length, so ISTM the order is the only issue11:08
dimiterntasdomas, could it be you're using a more recent mongodb?11:09
tasdomasdimitern, 2.6.311:10
tasdomasdimitern, SameContents panics11:10
dimiterntasdomas, hmm I thought it could be mongo related; oh - is the panic about the map field ExtraConfig?11:11
tasdomasdimitern, not as far as I can tell: ➜  juju  (git)-[master]-% go test github.com/juju/juju/api/networker11:11
tasdomas----------------------------------------------------------------------11:11
tasdomasPANIC: networker_test.go:204: networkerSuite.TestMachineNetworkConfig11:11
tasdomas[LOG] 0:00.011 DEBUG juju.environs.configstore Made dir /tmp/check-8674665223082153551/2/home/ubuntu/.juju/environments11:11
tasdomas[LOG] 0:00.099 DEBUG juju.environs.configstore writing jenv file11:11
tasdomas[LOG] 0:00.099 DEBUG juju.environs.configstore writing jenv file to /tmp/check-8674665223082153551/2/home/ubuntu/.juju/environments/dummyenv.jenv11:11
tasdomas[LOG] 0:00.099 DEBUG juju.environs.tools reading v1.* tools11:11
tasdomas[LOG] 0:00.099 INFO juju.environs.testing uploading FAKE tools 1.24-alpha1-trusty-amd6411:11
tasdomas[LOG] 0:00.101 INFO juju.environs.testing uploading FAKE tools 1.24-alpha1-precise-amd6411:11
tasdomas[LOG] 0:00.101 INFO juju.environs.testing uploading FAKE tools 1.24-alpha1-utopic-amd6411:11
tasdomas[LOG] 0:00.102 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any11:11
tasdomas[LOG] 0:00.102 DEBUG juju.environs.tools no series specified when finding tools, looking for any11:12
tasdomas[LOG] 0:00.103 DEBUG juju.environs.simplestreams searching for metadata in datasource "existing metadata"11:12
tasdomas[LOG] 0:00.103 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/check-8674665223082153551/5/tools/streams/v1/index2.sjson": stat /tmp/check-8674665223082153551/5/tools/streams/v1/index2.sjson: no such file or directory11:12
tasdomas[LOG] 0:00.103 DEBUG juju.environs.simplestreams streams/v1/index2.sjson not found, trying legacy index file11:12
tasdomas[LOG] 0:00.103 DEBUG juju.environs.simplestreams fetchData failed for "file:///tmp/check-8674665223082153551/5/tools/streams/v1/index.sjson": stat /tmp/check-8674665223082153551/5/tools/streams/v1/index.sjson: no such file or directory11:12
tasdomas[LOG] 0:00.103 DEBUG juju.environs.simplestreams cannot load index "file:///tmp/check-8674665223082153551/5/tools/streams/v1/index.sjson": invalid URL "file:///tmp/check-8674665223082153551/5/tools/streams/v1/index.sjson" not found11:12
tasdomas[LOG] 0:00.103 DEBUG ju11:12
tasdomasdimitern, sorry - wrong buffer11:12
tasdomashttp://paste.ubuntu.com/10717662/11:12
dimiterntasdomas, right - yeah it's not obvious, but it is because the map field makes the struct unhashable11:13
dimiterntasdomas, ok, so I think we should definitely file a bug about this and tackle it at some point, but because it only happens with a more recent mongod, it's not a serious issue (I mean of the "fix asap" kind)11:14
tasdomasdimitern, understood11:14
tasdomasdimitern, I'll file the bug11:14
tasdomasdimitern, what version of mongo is preferrable ?11:15
dimiterntasdomas, great, thanks for your help11:15
dimiterntasdomas, the one we're using is 2.4.9 from trusty11:15
dimiterntasdomas, there are a few known issues with 2.611:15
dimitern(more juju incompatibilities than mongo issues)11:16
=== gnuoy` is now known as gnuoy
dimiterndooferlad, I've updated the network spec to clarify a few bits around managing spaces, including the changes we discussed around update (added rename as a separate command)11:42
jamwallyworld: are you still around?11:49
wallyworldyeah11:50
jamwallyworld: well, I'm half tempted to just say "go to bed" :)11:52
jamwallyworld: but the other half of me has a question about the simplestreams tuff11:52
jamstuff11:52
jamwallyworld: specifically, in environs/simplestreams/simplestreams.go GetProductsPath11:53
jamwe filter by datatype11:53
jamand then we return "errors.NotFoundF" if there are no entries that have the same DataType11:53
jamwe alsor return NotFoundf if the cloud spec doesn't match11:53
jambut we return NoMatchingProductsError11:54
jamfor the other circumstances11:54
jam(if the only thing that doesn't match is ProductId)11:54
wallyworldjam: ?11:54
jamwallyworld: so I'm curious why finding an index file but not finding cloudspec or datatype is considered NotFound11:55
jambut finding one that just doesn't have the product is considered NoMatchingProducts11:55
jam*specifically* because we special case NotFound in how we log errors11:55
wallyworldno particular reason, except maybe bad implementation. maybe we should have used NoMatchingClouds and NoMatchingDataTypes errors11:56
wallyworldwe could or should fix it i guess11:57
jamwallyworld: well if we get noMatchingProductsError, then we do something weird and log it, but then fall through11:57
jamso you still get an items list instead of nil11:57
wallyworldfrom memory, that could be because the products might still be found in a different products file11:58
wallyworldbut i'd have to look at the code, it's been a while11:58
jamwallyworld: sure, I have the feeling things have evolved, because the next higher loop keeps searching on any error11:58
wallyworldthe implementation has grown organically to match what's served by cloudimages11:59
wallyworldit needs to be looked at fresh and a proper V2 refactoring done11:59
wallyworldmirrors were also add after V12:00
wallyworld112:00
wallyworldso it got very messy12:00
jamsure12:00
wallyworldi do wish it were rewritten12:01
jamwallyworld: http://reviews.vapour.ws/r/1362/12:07
wallyworldlooking12:09
wallyworldjam: \o/12:12
tasdomasdimitern, ping?13:06
dimiterntasdomas, pong13:09
tasdomasdimitern, a uniter API question, if I may13:09
dimiterntasdomas, sure13:10
tasdomasdimitern, so the uniter uses only the uniter API client, I need to modify the way metrics are added to state (from the unit)13:10
tasdomasshould I override the existing method in the uniter API (and APIserver) or should I introduce a different facade?13:11
tasdomass/different/new13:11
dimiterntasdomas, it depends on how compatible the api changes is - if it's adding a new method or adding args to an existing one, it's fine to do it directly in the current uniter api facade13:11
tasdomasdimitern, what if it's changing the params of an existing one?13:12
dimiterntasdomas, if it changes behavior or removes args/methods in a backward-incompatible way - a new facade version of the uniter api is needed13:12
tasdomasdimitern, got it - thank you13:12
dimiterntasdomas, np - it will help to have a look at the change, when ready13:13
tasdomasdimitern, it's related to api/metricstorage (and apiserver/metricstorage) - I introduced this as a new facade, but now see the error of my ways and will move the functionality to the uniter api client13:14
dimiterntasdomas, ok, ISTM it will be fine to add the new methods to the existing uniter api facade (v1 IIRC)13:15
dimiterntasdomas, however, keep in mind when implementing the client-side methods that the apiserver might be older and can return CodeNotImplemented error for the new methods13:16
tasdomasdimitern, thanks13:19
tasdomasdimitern, now I remember why I added this as a new facade - fwereade suggested I do this to avoid incrementing the version of the uniter facade (which is largish) due to this small change13:20
dimiterntasdomas, I see, there's no need to increment the version if only new (and not critical) methods are added (i.e. if the client code is prepared to handle gracefully the case when the new methods are missing)13:22
* dimitern steps out for a bit13:31
mupBug #1439204 was opened: environs/tools does not cleanup in the right order <test-failure> <windows> <juju-core:Triaged> <https://launchpad.net/bugs/1439204>14:01
* dimitern is back14:25
rogpeppedimitern: what's the current standard way of creating a juju review request?14:28
jamkatco: fairly trivial review: http://reviews.vapour.ws/r/1363/diff/#14:29
katcojam: k i'll take a look14:29
dimiternrogpeppe, just propose a branch against one of the branches in juju/juju - a RB diff should be created automatically14:29
jamrogpeppe: generally propose a github merge14:29
rogpeppedimitern: ah, i'll just make a PR then, thanks14:29
jamrogpeppe: rb notices and then updates reviews.vapour.ws and updates github with a link to it.14:30
katcojam: wow you weren't kidding :)14:30
rogpeppejam: it would be nice if the README had contribution guidelines, i guess14:30
katcojam: +114:30
jamrogpeppe: probably needs to be updated, I agree.14:30
jamI thought we had CONTRIBUTING14:30
natefinchwe do... the code review section is kind of a wall of text, though14:31
natefinchand links to https://github.com/juju/juju/blob/master/doc/contributions/reviewboard.md14:32
natefinchwhich has more specifics14:32
rogpeppejam: thanks - i hadn't seen that14:33
rogpeppejam: the README should probably link to it14:33
rogpeppejam: this particular PR is interesting from the p.o.v. of the recent discussion on juju-dev14:35
rogpeppejam: as it uses an external type in the API serialisation14:36
rogpeppejam: i'm not convinced that it's wrong here though14:36
rogpeppejam: i'd be interested in your thoughts14:36
* katco does happy dance14:37
katcoi'm feeling good today :)14:37
* rogpeppe is pleased for katco14:38
jamrogpeppe: I'd say I generally prefer being more explicit on API stuff because it is far too easy to stay compatible with yourself and break everyone else in the world14:38
katcorogpeppe: i've had a sinus infection for the past few weeks. haven't been able to do much of anything =/14:38
rogpeppekatco: ew14:39
rogpeppekatco: i also had a horrible flu thing last week, still feeling pretty knacked14:39
katcorogpeppe: seems everyone's been sick14:39
rogpeppekatco: yeah14:39
katcorogpeppe: and we'll bring it all to nuremberg! :)14:39
rogpeppekatco: i hadn't had something like this in decades14:39
rogpeppekatco: joy!14:39
rogpeppekatco: sinus thing sounds 'orrible14:40
katcorogpeppe: yeah it's been a few years since i've been knocked on my butt like this14:40
katcorogpeppe: i suppose it's a good reminder to appreciate the good times :)14:40
rogpeppekatco: well, continue to take it easy! yeah.14:40
katcorogpeppe: you too sir! see you in nuremberg i hope14:40
rogpeppekatco: deffo14:40
rogpeppejam: the issue in this particular case is that we want to transfer a macaroon over the wire, and the macaroon wire format is well defined, and implemented inside the macaroon package14:41
rogpeppejam: there's actually no easy way to use a different format except by dependending on precisely the format we're trying to avoid depending on14:42
jamrogpeppe: isn't a serialized macaroon just a series of lines with a final hash?14:42
rogpeppejam: no14:42
rogpeppejam: and even if it was, i wouldn't want to duplicate all the (very well tested) serialisation code that's in the macaroon package14:43
jamrogpeppe: so I'm saying that the API can be "you get a string" which you then parse as a macaroon. not "you get a Macaroon object"14:43
rogpeppejam: how is that any better?14:44
jamrogpeppe: fwiw I was confusing M.serialize() with M.inspect(). The latter being the line-by-line with a final sig14:44
rogpeppejam: there's a JSON serialisation format too14:44
rogpeppejam: (which is what we use in all the other APIs where we're using macaroons)14:45
jamrogpeppe: interestingly JSON serialization isn't supported by the android lib14:47
rogpeppejam: which android lib?14:47
jamrogpeppe: https://android-arsenal.com/details/1/91414:48
jamrogpeppe: I was searching for "macaroon JSON serialization" and one of the first hits was "JSON not supported" :)14:48
rogpeppejam: i'm not that surprised. the libmacaroons guy doesn't like the JSON serialisation much, although i took the format from his code.14:48
jamrogpeppe: so for something like this where there is an official format, it does feel like you should use it. It is different when it is one of *our* formats14:48
jamrogpeppe: the short text from google references you14:49
jamMerge branch '002-fix-json-serialization' of https://github.com/rogpe14:49
rogpeppejam: yeah, i fixed libmacaroons so it actually worked14:50
jamrogpeppe: so is http://godoc.org/gopkg.in/macaroon.v1 just a wrapper around lib or you were just looking at interop ?14:51
rogpeppejam: i was just looking at interop14:51
rogpeppejam: (i wrote it independently to start with, but then converged the serialisation formats)14:51
rogpeppejam, dimitern, katco, anyone else: anyway, a review would be appreciated if you have a few moments to look at it: http://reviews.vapour.ws/r/1364/diff/#14:54
natefinchthe macaroon stuff is versioned with gopkg.in, right?  So in theory, it should never break compatibility unless juju updates to including a new version14:54
ericsnowfwereade: ping14:54
jamnatefinch: there is code compat, and there is wire-protocol compat14:54
jamtechnically gopkg.in is only code compat I thought14:54
katcorogpeppe: i'll get to it in a few, working down the queue (but i'll skip to yours next)14:55
natefinchjam: I would hope that would be part of the contract14:55
rogpeppekatco: ta!14:55
jamcertainly that was the discussion from something else, though the thought was that code compat would iterate faster than wire compat14:55
natefinchjam: I could see how v1 and v2 could share wire compatibility... but I can't see how v1 and v1.1 could not... at least, I think most people would assume they'd have compatible wire formats14:56
jamrogpeppe: so one problem with your proposal is that you are changing what is supported by the API without changing the version numbers. So new juju clients can't tell that the server doesn't support it, right?14:57
jamrogpeppe: you really do need a version bump14:57
dimiternrogpeppe, why macaroon-bakery.v0 and not v1? I thought v0 was only used as a fallback when no versions exist14:58
jamso that juju 1.25 can say "oh,I'm talking to Client v0, which doesn't support macaroon based conversations"14:58
rogpeppejam: that seems right14:58
rogpeppejam: where do i bump the version?14:58
katcorogpeppe: jam: natefinch: http://semver.org/14:58
rogpeppedimitern: because we haven't decided that the bakery API is stable yet14:58
rogpeppedimitern: i think it's getting close though14:59
dimiternrogpeppe, so imports will use v1 for the release of 1.23 then?14:59
rogpeppedimitern: there is no v1 yet14:59
rogpeppedimitern: i have one moderately big change in the pipeline and then we might consider bumping to v114:59
dimiternrogpeppe, please do, as releasing juju depending on unstable imports is a no-no15:00
dimiternrogpeppe, apart from that LGTM I think15:00
rogpeppedimitern: most of juju's imports are unstable...15:01
dimiternrogpeppe, none of the versioned ones are, except charm.v5-unstable, but we agreed to bump that to v5 before the release15:01
rogpeppedimitern: most of juju's imports aren't versioned15:02
dimiternrogpeppe, I believe you got my point, come on :)15:02
rogpeppedimitern: not really15:02
rogpeppedimitern: i don't see there's a difference between an unversioned import and an import with a v0 version15:03
rogpeppedimitern: and i don't think you'd be complaining if the import path was github.com/juju/bakery15:03
dimiternrogpeppe, the difference is simple: the unversioned import is not yet versioned, the unstable but versioned one is imported as unstable as a conscious choice15:04
rogpeppedimitern: they're exactly the same. neither is stable.15:04
rogpeppedimitern: i used .v0 just so it would be trivial to update with govers when the time comes to bump the version15:05
dimiternrogpeppe, ok, that's fine, I'm just saying we'll need to bump it before releasing a stable 1.2315:05
rogpeppedimitern: i don't think that should be a necessary prereq15:06
rogpeppedimitern: although i'd like to do it anyway15:06
dimiternrogpeppe, cool15:08
natefinchtechnically, with godeps, we don't really need versioning.  The version is just a nice human-readably suggestion.  For example... 1.23 says it's importing charm.v5-unstable, but it's really importing a commit from the 1.23 branch15:12
natefinchnote that I do still think versioning is good and that we should continue to use it.15:13
natefinchwe need juju status -f15:17
natefinchso it'll update the status whenever there's a change, and I don't have to spam juju status constantly15:17
benji"watch juju status" can be a reasonable substitute15:31
natefinchbenji: yeah that works :)15:33
voidspacedimitern: so, there's a comment in extractInterfaces about skipping disabled network interfaces16:06
voidspacedimitern: but it doesn't skip them16:06
voidspacedimitern: if it did, life might be simpler...16:06
dimiternvoidspace, oh, yeah - I noticed that one as well and glared at it :)16:08
voidspacedimitern: do we *need* to know the disabled interfaces?16:08
dimiternvoidspace, we need to not try to use them as primary interfaces at least16:11
dimiternvoidspace, that's coming from another bug around that code16:11
voidspacedimitern: well, if extract interfaces didn't return them there's a lot less chance we'd try and use them16:11
voidspacedimitern: although finding the actual bug would be good - maybe *as well*?16:11
dimiternvoidspace, no, I mean we need to know whether they are disabled or not16:11
dimiternif possible16:12
voidspacedimitern: digging in still16:12
dimiterndooferlad, voidspace, PTAL http://reviews.vapour.ws/r/1365/ when you have a moment - juju subnet add cmd16:15
dooferladdimitern: *click*16:15
voidspacealso looking16:16
dimiternthanks guys16:16
dooferladdimitern: I like CheckNumArgs - nice!16:25
dimiterndooferlad, it came along nicely yeah :)16:27
dimiterndooferlad, that helper is a candidate for factoring out in a common place later16:28
dimiterndooferlad, if you like, use it for spaces as well (just copying it for now I guess - for simplicity, then we'll move it somewhere)16:29
dooferladdimitern: Yea, I was thinking of making use of it, though I was thinking of something like ParseArg(arg string, *parseFunction, *targetVariable) err {}16:36
dooferladdimitern: Not sure if combining the two ideas just gets messy!16:36
dooferladdimitern: also, it gets to a point where you are just writing code because it is a cool idea, rather than making progress. I think I am approaching that point :-)16:38
dimiterndooferlad, hah I know the feeling16:39
dimiterndooferlad, voidspace, and a final PR for today - juju subnet remove - https://github.com/juju/juju/pull/2014/ (the diff includes the previous PR - please ignore that)16:53
TheMuedimitern: what does " install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'" of the userdata in cloudinit do?16:54
dooferladTheMue: I believe that file is required for the machine to boot, but it doesn't need to contain anything. I ran into it a while ago.16:55
dimiternTheMue, it installs the nonce file for the machine agent16:55
dimiterndooferlad, TheMue, the nonce is used to verify the machine agent trying to login as "machine-X" is indeed allowed to do so (the provisioner generates the nonce when it starts an instance for machine-X)16:56
dimiternok folks - time for me to go16:56
TheMuedimitern: I looked the whole bootstap up and down16:57
TheMuedimitern: and everything really looks similar, so I'll now check the content of the nonce16:57
dooferladTheMue: There are plenty of people having problems because that file is missing (https://www.google.co.uk/search?q=var%2Flib%2Fjuju%2Fnonce.txt%2520missing&gws_rd=ssl#q=var%2Flib%2Fjuju%2Fnonce.txt+missing)16:57
dimiternTheMue, I'd appreciate if you summarize your findings in a mail please, and let's discuss it tomorrow?16:58
TheMuedimitern: yep, will do16:58
dimiternTheMue, thanks!16:58
alexisbkatco, you around?17:14
katcoalexisb: i am!17:15
alexisbdooferlad, voidspace if you all have updates on the 1.23 networking bugs please provide them to me before you eod17:20
alexisbwe need to make a call on releasing 1.23 by end of my day17:20
jw4alexisb: there is a high priority bug in upgrades that I may have a fix for today17:21
jw4(1.23)17:21
jw4143848917:21
jw4bug 1438489 (sorry for the stutter)17:22
mupBug #1438489: juju stop responding after juju-upgrade <upgrade-juju> <juju-core:Triaged by johnweldon4> <https://launchpad.net/bugs/1438489>17:22
alexisbjw4, nice17:24
alexisbif we want to included it for beta3 it needs to land today17:24
alexisbotherwise it will go in a point release17:24
jw4alexisb: hopefully yep, although there seems to be another bug right behind it if this one clears17:24
jw4alexisb: in a different part of code but also to do with upgrades17:25
mupBug #1439204 changed: environs/tools does not cleanup in the right order <test-failure> <windows> <juju-core:Fix Released by jameinel> <https://launchpad.net/bugs/1439204>17:38
voidspacealexisb: two bugs of the three I was working on done and fix committed18:13
voidspacealexisb: third in progress, partial fix but not the main issue, done - still hopeful to land tomorrow18:13
voidspacealexisb: can go into the next release though - or I can try and land the simple (partial) fix we have immediately18:15
alexisbvoidspace, ack, thank you!18:22
natefinchericsnow, wwitzel3: either of you still working on 1.23 bugs?18:28
mupBug #1435413 changed: rename "juju backups restore" to "juju backups recover" <backup-restore> <juju-core:Won't Fix by ericsnowcurrently> <juju-core 1.23:Won't Fix by ericsnowcurrently> <https://launchpad.net/bugs/1435413>18:32
ericsnownatefinch: wrapping up some vmware stuff and there's still the 1 GCE bug that needs attention18:52
natefinchericsnow: sounds like they're trying to get 1.23 cut tonight.   vmware is still a ways off, right?  Seems like GCE should be the priority18:53
ericsnownatefinch: yep18:56
voidspaceg'night all19:15
natefinch...and now that my HA stuff is ready, there's no graduated reviewers around19:33
natefinchle sigh19:33
katconatefinch: o/19:33
katconatefinch: finishing up a review and i'll TAL19:33
natefinchoh, shit,  I forgot some of the "newbies" are graduated :)19:33
katconatefinch: haha yeah :)19:34
natefinchkatco: much appreciated - http://reviews.vapour.ws/r/1299/19:34
katcoboy reviews are like laundry.... just when you think you might be done and you can work on something else :p19:34
natefinchkatco: haha, yep19:40
natefinchkatco: ahh crud, I have to fix some of the comments... I went from one type to another and back to the first, but I forgot to put the comments back to the first type19:41
katconatefinch: if you want to return the favor, fairly small: http://reviews.vapour.ws/r/1367/19:41
katconatefinch: ok np, lmk when it's ready19:41
natefinchkatco: ok, fixed19:42
katcok looking19:43
natefinchkatco: looking at your review19:43
katconatefinch: ty19:43
natefinchkatco: The file 'featuretests/package_gccgo_test.go' (r3b6a6c8) could not be found in the repository19:43
katconatefinch: right i deleted it :)19:43
katconatefinch: i don't know why RB has problems with that19:44
natefinchkatco: heh ok19:44
katconatefinch: https://github.com/juju/juju/pull/2015/files#diff-765f9c55678c118cef1ce099ec147738L119:44
katconatefinch: that's what it was19:44
ericsnowany networking folks around?19:49
ericsnowI'm seeing the addressworker in a restart loop around this log entry:  2015-04-01 19:47:52 ERROR juju.worker runner.go:219 exited "addresserworker": environment does not support networking19:49
natefinchkatco: ship it19:51
katconatefinch: woot! ty!19:51
natefinchkatco: welcome :)19:51
natefinchI swear HA used to work on local, but it totally doesn't now.19:52
mupBug #1439364 was opened: error in logs: environment does not support networking <juju-core:Triaged> <juju-core 1.23:Triaged> <https://launchpad.net/bugs/1439364>19:56
mupBug #1439364 changed: error in logs: environment does not support networking <juju-core:Triaged> <juju-core 1.23:Triaged> <https://launchpad.net/bugs/1439364>19:59
mupBug #1439364 was opened: error in logs: environment does not support networking <juju-core:Triaged> <juju-core 1.23:Triaged> <https://launchpad.net/bugs/1439364>20:08
natefinchkatco: how's the review coming?20:27
katconatefinch: good. my wife came home so i got interrupted for a few20:27
natefinchkatco: no problem. Sorry to pester you, just need to get it checked in today (if possible), and I'm running out of day.20:28
katconatefinch: np, reviewing as fast as i can :)20:28
natefinchkatco: thank you, I appreciate it.20:32
mupBug #1439375 was opened: logger in worker/instancepoller is named "juju.worker.instanceupdater" <juju-core:New> <https://launchpad.net/bugs/1439375>20:35
katconatefinch: i forget, do mgo update operations need an exists assert?20:41
natefinchkatco: shouldn't... mongo is update == upsert  but I don't know if mgo somehow separates the two20:42
katcok20:42
katconatefinch: i have an initial review posted20:43
katconatefinch: i need to look at the api stuff again... that stuff is so tedious20:43
natefinchkatco: yeah, the API stuff is super annoying.  totally needs to be either generated or somehow refactored to take out all the boilerplate20:45
katconatefinch: definitely20:45
katconatefinch: i see dimiter already commented on the api stuff, do you feel like he gave that a good once-over?20:47
katconatefinch: b/c the logic looks fine to me, it's the nuances of our RPC implementation that i'd really have to go over20:47
natefinchkatco: yes, but I had to make changes to it in response to what he asked for, so really, he sort of needs to approve my changes. But of course, they weren't done until he went offline. le sigh.  Maybe I can get some of the NZ/AUS guys to review the API stuff and get it merged in later tonight.20:48
katconatefinch: that might be best... i don't want to just rubber stamp it.20:49
katconatefinch: the logic looks fine though20:49
katconatefinch: axw is the next OCR20:49
natefinchkatco: no problem.  I appreciate the time you put into it.20:50
katconatefinch: today, it's my job ;)20:50
natefinchaxw: when you get on, if you could review http://reviews.vapour.ws/r/1299/ I would really appreciate it, especially the API stuff.21:01
=== natefinch is now known as natefinch-dinner
katconatefinch-dinner: i will ask him in our standup as well21:01
natefinch-dinnerkatco: thanbks21:01
ericsnowwwitzel3: I think you may be right21:01
katconatefinch-dinner: enjoy dinner21:02
cheryljericsnow, thumper:  Could I get a quick license review?  https://github.com/juju/jujusvg/pull/2421:35
cheryljcmars ^^21:35
mupBug #1439398 was opened: GCE low-level RemoveInstances fails if firewalls are not found <juju-core:Triaged> <juju-core 1.23:Triaged> <https://launchpad.net/bugs/1439398>21:41
ericsnowcherylj: that license exception is the same one we have everywhere else, right?21:46
cheryljericsnow: yes, I copied it from another repo21:46
ericsnowcherylj: cool, LGTM21:46
cheryljthanks!21:46
jw4I'm pulling my hair out with this local upgrade test... it seems to not be picking up my updated code on my 1.23 branch... any tips for iterating on upgrade testing?22:15
jw4ah, strike that, my code is getting pulled in... there was a problem in my assumptions about upgrades22:17
jw4it looks like my upgrade steps have to manually iterate units on the machine, they don't get passed in as part of the upgrade operation22:18
ericsnowjust got bit by the fact that the pointer to a loop var does not change across iterations (which makes sense *now*)22:35
ericsnowcould I get a review on http://reviews.vapour.ws/r/1370/?22:42
ericsnowkatco: ^^^22:42
ericsnowaxw: ping22:55
ericsnowaxw: I guess you won't be one quite yet :)22:56
ericsnows/one/on/22:56
ericsnowwallyworld: perhaps you could takea quick look at http://reviews.vapour.ws/r/1370/22:57
wallyworldericsnow: in a meeting, will look soon22:57
ericsnowwallyworld: thanks!22:57
axwericsnow: LGTM23:06
ericsnowaxw: thanks23:06
axwnatefinch-dinner: will take a look a bit later on23:06
axwnp23:07
ericsnowaxw: it was such a dumb bug :(23:07
axwericsnow: yeah, that's an easy one to fall for23:07
mupBug #1439447 was opened: tools download in cloud-init should not go through http[s]_proxy <cloud-installer> <landscape> <juju-core:New> <https://launchpad.net/bugs/1439447>23:38

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