=== kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 === kadams54 is now known as kadams54-away [01:49] ericsnow, ping [01:49] ericsnow, http://reviews.vapour.ws/r/1088/ , re-submitted, thanks ! [02:14] katco: just wondering if you got to the LeaderSettings watcher registration issue or whether I should be picking it up [02:20] jam: hey i'm working on that right now [02:20] katco: k [02:20] jam: it turned out to be a bit trickier to diagnose; the code was registering the worker correctly, but the worker itself didn't adhere to the state.NotifyWatcher interface [02:21] jam: wallyworld figured that one out [02:21] jam: so there's a few issues; one question is why is the type assertion failure not being bubbled up [02:21] jam: but at any rate, i'll have a patch momentarily. had to eat dinner. [02:22] katco: when you EOD can you make sure to push it up so I can pick it up if it isn't done? and a ping to me to look at it would be good [02:22] jam: it's 21:22 here, so i'll get a PR in before i stop :) [02:39] jam: http://reviews.vapour.ws/r/1141/ i will stay on for questions/fixes [02:39] katco: looking now [02:40] jam: wallyworld made an excellent suggestion for making the test better, but i don't have the time tonight to grok it; i can shore that up later: http://pastebin.ubuntu.com/10582820/ [02:41] katco: right. I was about to suggest using the NotifyAsserter directly on the Changes rather than a secondary stream [02:45] katco: reviewed [02:45] jam: ty looking [02:46] * katco smacks head [02:47] jam: so, you're absolutely right [02:47] jam: do you think i should just implement a full watcher instead of attempting to piggy-back off of settingsWatcher? [02:47] jam: or do you think a simple for loop wrapping the settings-watcher's change is sufficient? [02:48] katco: if all it needs to do is proxy another one, thats fine (I think). Is there a reason you can't just hand back the settings watcher itself? [02:48] jam: yeah, the state.NotifyWatcher signature demands a signature of type Changes() <-chan struct{} [02:49] katco: if you are embedding *settingsWatcher, doesn't that give you *its* Changes channel? [02:49] jam: and that's the bit preventing it from being registered [02:49] ah, so we have changes but its not a struct{} [02:49] k [02:49] jam: the settings watcher is not of type NotifyWatcher; it's signature is <-chan Setting something or other [02:49] katco: from being registered? or from being instantiated ? [02:49] jam: i believe from being registered [02:49] jam: we do some reflection from what wallyworld found [02:50] jam: and anything stuffed into resources which does not conform to NotifyWatcher will not be utilized [02:50] katco: right but that's not *registration* thats *extraction* [02:50] katco: because resources can be whatever [02:50] jam: right sorry; extraction [02:51] but when you go to look them up it checks that the thing that's there matches. [02:51] right [02:51] probably we should have good logging as well [02:51] something that says "I found something, but its not what you're looking for" [02:51] yeah we discussed that; it's a bit odd that this didn't trigger some kind of error/log, *something* [02:51] we generally don't send that as an error to the caller (though I think we could here) because of Permission/Priority concerns. [02:52] we also discussed maybe having separate register methods for different types to force this into a compile-time error [02:52] anyway, I'll be back in 10 [02:52] k, i'll implement the for loop [02:55] jam: i'm a bit concerned about what happens when the embedded settingsWatcher is told to stop; what would happen to the LeadershipSettingsWatcher's Changes loop? [03:01] katco: coming late to the party, but in such cases you'd implement Stop() on the outer struct and call the embedded Stop() as well as cleaning up anything else [03:02] axw: i've made a few comments on the gh pr [03:02] wallyworld: yeah the thought crossed my mind; but that means i need to probably implement a tomb of my own [03:03] wallyworld: thanks. I've gotta head out shortly, will respond when I get back [03:03] sure [03:05] katco: yeah maybe, i'd need to check the code in more detail. the reason for not using settings watcher directly is the changes method returns a settings delta, not just an event? and we want notify watcher samantics? [03:05] wallyworld: yeah [03:05] hmmmm [03:06] katco: could we just add a Changes() <-chan struct{} to settings watcher [03:06] to make it also a notify watcher [03:06] i don't think go allows overloading like that [03:06] naming conflict [03:06] it doesn't take into account the entire signature [03:06] ah, yeah [03:06] not i remember why i hate Go often [03:06] now [03:07] lol [03:07] that and no exceptions and no generics [03:07] it forces you to stay simple... which can be an immensely good thing :) [03:08] katco: so we could extract a base settings watcher and embed that in 2 variations - what we have now and a new notify variant [03:08] the 2 new versions of the containing struct would have the different Changes() signatures [03:08] wallyworld: hm. i'm kind of liking that idea. i do worry about unforeseen consequences [03:09] that's what tests are for :-D [03:09] i think we have pretty ok coverage in that area [03:10] maybe earlier today i would have agreed with that [03:10] :) [03:10] lol [03:10] i think i'll give that way a go though [03:11] good idea [03:11] good luck :-) [03:11] i think it will work fine, and solves the issues encountered [03:15] wallyworld: mmm... i think that just makes the issue worse. because i'd then have the same problem in 2 places. the base watcher would still have to notify on a <-chan settings [03:17] couldn't the base watcher use an embedded interface to call when it needs to notify? [03:17] i'd need to look at the code to map it out [03:18] wallyworld: hm. no that may work. [03:19] the one with the notify semantics would just discard the info given and shove struct{} onto a channel [03:20] wallyworld: right, but you're still talking to a channel; you lose the magic of select [03:21] so the way this would work is the i'm in the base type's select, specifically in the case statement that received a change [03:21] i then call the notify function that was passed to me [03:21] that notify function has to send on a channel [03:21] except that now if stop is called, it will block [03:22] as it's implemented now, the select would select the stop even if it was waiting on sending the notification [03:22] because it's in its select [03:22] hmmm [03:22] now mind you [03:22] my brain is revolting a bit at this time of night [03:23] wallyworld: do you have a link to the CI jobs on jenkins? [03:23] this is where generics would be wonderful [03:23] url hacking is failing me [03:23] thumper: o/ [03:23] hey katco [03:23] thumper: http://juju-ci.vapour.ws:8080/job/github-merge-juju/ ? [03:23] or http://juju-ci.vapour.ws:8080/ [03:24] the last one is the dashboard for all the jobs [03:24] that doesn't have the CI jobs on it [03:24] thumper: you need to be logged in [03:24] as who? [03:25] I don't have any creds that AFAIK [03:25] thumper: everything got locked down due to the creds leakage issue raised last week [03:26] hey rick_h_ [03:26] katco: wanna leave it till tomorrow? I'll see if i can hack it a bit today [03:26] wallyworld: nah i'm close [03:27] wallyworld: i don't think there's a way around it. i have to do a copy/paste job. it needs its own tomb setup [03:27] yeah, could be right [03:27] thumper: howdy [03:27] this is when i think go's simplicity works against it [03:28] exactly [03:28] and i run into that *all* the time [03:28] i think they are trying to solve this still [03:28] for me, it's too simple [03:28] but code duplication of this complexity is annoying [03:28] leads to *lots* ot cut and paste bilerplate [03:29] code duplication of any complexity can get annoying - if i never see another cut and paste arraycontainsstring() it will be too soon [03:32] FYI: both CI blockers have now been cleared [03:32] verified that the fix that landed earlier fixed the upgrade issue [03:32] and the power test isn't timing out on cmd/jujud/agent either [03:33] pretty sure those two things were related [03:38] wallyworld: jam: helper method for registering watcher: RegisterWatcher(...) or RegisterNotifyWatcher(...)? [03:39] Notify imo [03:39] as there are idfferent kinds [03:39] eg StringsWatcher [03:39] Bug #1430791 changed: Upgrade-juju is broken on most/all substrates

[03:39] Bug #1430898 changed: run-unit-tests ppc64el timeout

[03:39] Bug #1431130 was opened: make kvm containers addressable (esp. on MAAS)

[03:40] wallyworld: gosh... it makes apiserver/common depend on state [03:40] i don't know if it's worth that. [03:40] no [03:40] let's not do that [03:42] pushing up changes with a little piece of my rotted heart. [03:43] lol [03:43] http://reviews.vapour.ws/r/1141/ [03:43] i think we could probably do something better by passing in a channel into a base type [03:43] but i'm not sharp enough right now to conceive of that [03:48] katco: i know it's late for you, and i'm happy to do it, but i do think the test needs to use something like the pastebin i offerred earlier [03:48] wallyworld: i was going to land that separately... if you want it in i'll have to hand off [03:48] wallyworld: otherwise i won't be fresh for tomorrow. b/c babies don't have a snooze ;) [03:49] katco: ok, so long as you land first thing tomorrow before we branch 1.23 :-) [03:49] wallyworld: land the separate branch? or land changes you will make? [03:49] the followup branch with revised tests [03:50] this branch at least will fix the issue itself [03:50] wallyworld: keep in mind i also have to do the amz fix [03:50] i can do the tests one [03:50] wallyworld: it would be appreciated... so does this one have a ship it then? [03:51] katco: just eyeballing changes - they are a cut and paste right? [03:51] wallyworld: yup [03:51] wallyworld: from settingsWatcher [03:52] katco: ok, tyvm for perservering with this, +1 [04:06] * katco teuche, Jenkins. it is i who waits on you. [04:07] touche even [04:09] Bug #1430898 was opened: run-unit-tests ppc64el timeout

[04:09] Bug #1431134 was opened: fix container addressability issues with cloud-init, precise, when lxc-clone is true

[04:09] wallyworld: I just realised there's more to do, I hadn't updated the apiserver/storageprovisioner code to use the new watchers [04:10] doing so now [04:11] gah [04:11] between the time i submitted and the job queued up that bug hit [04:11] and now it rejected the build [04:14] can someone land this when trunk opens up? https://github.com/juju/juju/pull/1813 [04:14] i need to go to bed [04:15] wallyworld: axw: anastasiamac ^^ [04:16] katco: ok, will keep an eye out [04:16] good night [04:16] axw: ty sir. have a good day [04:41] wallyworld: katco: so I looked at the change, and we need to tweak it a bit for not actually being a settings watcher, also it is completely untested as is. But if you want to hand it off to me, since I actually need it for my patch, I can pick it up. [04:41] wallyworld: would you rather just hand it off to me? [04:42] jam: if you want, i realise the tests need updating, but it was late for katco so i was going to follow up with a branch, but if you want to do it for your patch that wuld be fine with me [04:42] wallyworld: well both that we now have a lot of untested code in LeadershipSettingsWatcher, and the test needs tweaking for featuretests [04:43] jam: agreed, hence the tests were to be updated straight away [04:45] jam: i was looking to use something like statetesting.NewNotifyWatcherC as is used elsewhere [04:45] was that ow you wre going to redo the tests? [04:47] wallyworld: yeah, I'm grabbing your paste as a reference [04:47] oh, that paste was just an initial brain dum [04:47] p [04:47] wallyworld: I had forgotten that NotifyWatcherC was the original that I used to create the NotifyAsserterC channel stuff [04:47] but should be close [04:47] wallyworld: yeah, I'm tweaking it, but it gives a good reference [04:47] like, we have to import statetesting :) [04:47] of course :-) [04:48] i didn't compile it, just dumpt the text into pastebin [04:49] jam: also for later, did you talk with mark about Properties vs Hints? [04:49] no, mark was out of town last week [04:50] ok, np [04:50] wallyworld: do you know why featuretests starts mongo in a different way? [04:51] no [04:51] i either didn't realise or forgot that it did [04:51] jam: but isn't this standard? coretesting.MgoTestPackage(t) [04:51] hmm… maybe it doesn't. I was running into problems with a —journal flag but only for the featuretests directory [04:51] in package_test [04:52] but they seem to be running now [04:52] maybe it was something else [04:52] symlink issue :-P [04:53] wallyworld: from what I've seen could be [04:53] oh, i was joking :-) [04:53] nope, still happening: [LOG] 0:03.670 ERROR juju.worker exited "state": cannot create database index: cannot use 'j' option when a host does not have journaling enabled [04:54] my state worker spins endlessly in those tests [04:54] hmmmm, not seen that before === beisner- is now known as beisner [05:40] wallyworld: can you please check my latest diff in http://reviews.vapour.ws/r/1128/ [05:40] wallyworld: I've updated storageprovisioner to match the scope changes [05:41] sure [07:20] dimitern: do you know why the blocking bug fails on ppc64 only? [07:21] wallyworld, I don't know that for sure - it might be gccgo-specific [07:22] dimitern: it's hard to tell from the console log - there's so many go routines, most of which are blocked on a select; where did you see an concrete error info? [07:24] wallyworld, search for the test I mentioned in the log - it's there, about the newDummyWorker (or whatever, the second thing) - towards the end of the panic dump you can see upgrade_test.go:1779 (or was it machine_test.go) [07:24] dimitern: thank you, my eyes weren't working properly [07:27] wallyworld, oh, np - it's hard to see anything useful in most of these panics [08:00] wallyworld: we're going to need to expand the storage provider and provisioner. if a machine reboots, we'll have to recreate tmpfs/loop [08:22] morning o/ [08:27] TheMue, o/ [08:29] axw: unless they say they want transient right? [08:29] wallyworld: transient volumes should still be attached after restart, they just may be empty [08:29] wallyworld: at least, in the ec2 definition [08:29] yes, that is true [08:30] wallyworld: I'm thinking of folding the filesystem mounting into the storage provider/provisioner [08:30] wallyworld: and how to deal with remounting after restart. then I realised we need to do the same for tmpfs/loop [08:30] * TheMue just read the State discussion. yeah, there are always repeating names in many projects, e.g. also Context. [08:31] TheMue: it occurred to me as I used the word "context" a lot in that email ;) [08:31] ..ooOO( Data, Dump, Thing, Stuff, ... ) [08:31] axw: yeah, as "context of a state" [08:31] wallyworld: so, my current thinking is that we require Attach{Volume,Filesystem} to be idempotent [08:31] axw: i think it makes sense for the provisioner worker to have that responsibility of dealing with restart/remounting [08:32] yes [08:32] wallyworld: and then the first time we see an already provisioned storage, we request a re-attach [08:32] like unit hooks [08:32] which may be a no-op [08:32] yep [08:33] wallyworld: also, we don't need to think about htis too much now, but I think we're going to want to run Attach on both env + machine for env-scoped attachments [08:34] wallyworld: e.g. for Ceph where we'd want to prepare the resource in the env storage provisioner, then attach/mount in the machine storage provisioner [08:34] yeah, i haven't thought about that [08:34] not enough brain space [08:34] that can come later, I'm pretty confident we can work it in [08:35] wallyworld: anyway, I think I'll work on provisioning filesystems/filesystem attachments in the storage provisioner now [08:35] I won't worry about loop/tmpfs for the moment, will come back to them later [08:36] you mean the restart behaviout for tmpfs/loop? [08:37] wallyworld: we don't watch volume attachments in the storage provisioner yet, so we can't handle them until we do [08:37] wallyworld: if you prefer I can do it for volumes first, I was going to leave it so you could continue on it in parallel [08:38] axw: ok, start on fs, i have some health stuff to do but can then pick up volumes [08:38] though TBH, it's kinda much the same... I might see if I can abstract some stuff to make the core logic common [08:39] ok sounds good [09:03] ugh sleepless night in our household o.0 [09:04] going to work on goamz while i'm up [09:07] bodie_, here now. I'm on UK time this week and back to US eastern time next week. [09:09] katco: is that you or a bot? [09:10] beep boop [09:10] R2D2? [09:11] lol [09:11] i'm going to drink an r2d2 sized coffee [09:11] in a few hours [09:11] double shot [09:12] we think she is teething again [09:19] dimitern: if you get a chance [09:19] dimitern: http://reviews.vapour.ws/r/1138/ [09:20] voidspace, cheers, will look shortly === ashipika1 is now known as ashipika === perrito667 is now known as perrito666 [09:45] voidspace, dooferlad, TheMue, please take a look at this http://reviews.vapour.ws/r/1144/ [09:45] dimitern: *click* [09:51] dimitern: s/live tests on MASS/live tests on MAAS/ ? [09:51] dimitern: in PR description [09:52] dimitern: ah, dhcp leases... :-) [09:52] dimitern: sounds like you've been having fun working all this out [09:52] voidspace, :) oh yeah - ofc [09:53] voidspace, it was a fun week, yeah :D [09:53] TheMue, thanks! [09:53] voidspace: MASS is ok, Metal as Super-Service [09:53] * dimitern has bootstrapped a maas environment at least a 100 times in the past 4 days [09:53] ouch [09:59] voidspace, you've got a review [09:59] dimitern: thanks [10:34] dimitern: PR updated with extended test [10:34] voidspace, great! I'm having another look [10:39] voidspace, looks good to land [10:41] dimitern: cool, thanks [10:52] axw: ping [11:10] katco: axw is here now, if you ping him again, or at least rub his lamp, he may appear in a puff of smoke [11:10] or he was [11:10] lol [11:10] i think i figured it out, i just wanted to pick his brain [11:10] Bug #1431286 was opened: juju bootstrap fails when http_proxy is set in environments.yaml [11:18] wallyworld: got time for a review of goamz? [11:21] wallyworld: dimitern: https://github.com/go-amz/amz/pull/37 [11:25] katco, will have a look shortly [11:27] dimitern: thx [11:37] dimitern: ready with a first pass and like it so far. but would like a 2nd review [11:37] TheMue, sure, thanks so far :) [11:43] * TheMue steps out for lunch [11:44] * fwereade has confused his sleeping patterns again and would like to beg for someone to implement a trivial branch for him while he goes and has breakfast and stuff [11:44] katco: sorry, was afk. looking [11:44] katco: pong [11:44] wallyworld: np at all [11:44] * axw_ emerges in a puff of smoke [11:44] lol === axw_ is now known as axw [11:44] axw_: hey, was mostly going to pick your brain re: io.Copy moving the stream pointer of a request body to the end [11:44] * fwereade needs "leader-elected", "leader-deposed", and "leader-settings-changed" hooks added to charm/hooks [11:44] ah, awx has already reviewed [11:45] wallyworld: ah cool [11:45] katco: I just commentd on the PR [11:45] came on here to check if you were around :) [11:45] axw, jam, I have a couple of concerns re http://reviews.vapour.ws/r/959/ -- possibly unfounded? please consider [11:46] fwereade: okey dokey [11:47] axw: so i don't think making a type assertion for ReadSeeker will work, because the body of http.NewRequest wraps the incoming body in a ioutil.NopCloser [11:47] fwereade: I agree with you about axw's comment. I had one written myself, but forgot that publish button [11:47] why is it at the top....! [11:47] axw, actually I'm not sure I expressed them very well -- I just have a bit of a nervous about not coalescing [11:48] axw, it was an uphill struggle to convince myself it was ok in the client watcher code [11:48] katco: rats [11:48] axw: i know, i tried that at first =/ [11:48] fwereade: so the tick-tock stuff was just preserving what we had and going forward with it. I also wasn't entirely happy as it seems very different from our standard behavior. [11:48] axw: i also realize there is an API break; but we *just* cut v3 [11:49] axw: not really that sure what our stance is on that [11:49] katco: fair enough [11:50] axw: i suppose i'll let dimitern weigh in on that one [11:50] dimitern: Could you take a look at https://github.com/juju/testing/pull/54/files [11:50] fwereade: I don't really understand about the watcher ownership. if the source Stop()s it cleanly, why wouldn't its Err() be nil? [11:51] axw, IMO the source shouldn't stop its watcher until it's exiting its goroutine [11:51] katco: official versions is "what was in a stable release" IMO [11:51] dimitern: I tried running the .bat version and it doesn't work inside "wine cmd" so I am guessing it was broken anyway. I can test in Windows in a moment. [11:52] jam: stable release of juju? or of goamz? [11:52] jam: how did the leader settings work out btw. sorry i had to (try) and go to bed last night [11:52] katco: well goamz is trickier, as its a dependency. I thought it was an API version. [11:52] but if things are just broken in v3 seems like people can't use it [11:53] katco: working on the patch to tests, been very distracted this morning, but I can build on what you landed. [11:53] jam: well, not so much broken. v4 signing just uses more memory than it should. [11:53] axw, "just one deferred Stop, guaranteed to happen after the loop exits" feels much cleaner -- and thus indicates that any close of that channel inside the loop implies that *if* there's no error returned *someone else* has interfered with us, and we should complain [11:53] fwereade: I'm sure I'm being slow, but I still don't see how that affects the error state of the watcher. One way or another the source stops the watcher; if it's *asking* to stop it, why would the watcher stop with an error? [11:53] katco: does fixing the memory issue actually cause a change to the API? [11:53] jam: distracted? people on juju core? no! ;) [11:54] axw: if someone stops something you depend on, isnt that an error? [11:54] axw: (if I pull the handbrake while you're driving, I don't think you'd exit cleanly :) [11:54] jam: it does, because it necessitates some signature changes to take in io.ReadSeeker and not io.Reader [11:54] jam: we don't know how awesome axw is at drifting... [11:55] fwereade: so liveSource.Stop() should always error? [11:55] jam: if you're interested, here is one of the API breakers: https://github.com/go-amz/amz/pull/37/files#diff-7db3cb93944d57b2ffc803281c906018R209 [11:56] * axw is confused [11:56] axw, I think what would happen is [11:56] katco: so if you're really concerned, you could just add a new function PutReadSeeker [11:56] axw, liveSource.Stop calls q.tomb.Kill(nil); return q.tomb.Wait() [11:56] not an API break by most people's definition [11:56] axw, the watcher lives on happily in the background [11:56] I guess if the underlying code *requires* read seaker [11:56] jam: except the old function could not exist as is [11:56] seeker [11:57] jam: yeah exactly [11:57] katco: well you could always read and buffer [11:57] katco, dooferlad, ok, I'm back and responding in order :) [11:57] jam: well, that's what causes the memory concern. that's what we were doing prior [11:57] axw, the main loop observes that <-q.tomb.Dying(), and exits with ErrDying, whithout touching the watcher [11:57] fwereade: ahh, I see. it makes sense when taken with your other comment [11:57] dimitern: ty! [11:57] axw, cool :) [11:59] axw, and fwiw I don't think you're being slow, these types with internal goroutines are subtle and quick to anger [11:59] daughter is awake (at the appropriate time this time)... afk for a bit [11:59] dimitern: no rush - was about to get lunch anyway [12:00] dooferlad, cheers [12:04] katco, reviewed [12:04] katco, couldn't help much I'm afraid [12:06] jam: if you come up with something better than the tick/tock, I'm happy to change the storage source. I'll fix the EnsureErr/watcher.Stop bit tomorrow [12:06] axw: well that patch hasn't landed has it? [12:06] jam: which patch? [12:07] jam: I mean, I'll change the storage source to match whatever you do for the relation one [12:07] axw: ah [12:07] katco: so you can add a new memory-efficient API, and leave the old inneficient one around for compatibility [12:09] jam, just posted http://reviews.vapour.ws/r/1145/, please let me know if you spot any impedance mismatches with what you're doing with Filter [12:09] fwereade: if it is run-leader-elected, I just merged it, the API fits [12:09] jam, will be writing proper tests for it today once I'm ~functional again [12:09] jam, cool [12:09] jam, pushed an update or two since I last spoke to you [12:09] jam, but I don't think I touched leader-settingschanged [12:11] jam, would also appreciate a second opinion on http://reviews.vapour.ws/r/1134/ [12:13] jam, and if you run your merged branch and find that it looks like it roughly works, would you send jamespage a link to the code? [12:14] jam, l-s-c is the important bit for him AIUI, l-e is just a bonus, but giving the structure a bit of a kick around would be good all the same [12:21]