[00:06] cmars: done with the review [00:06] menn0, thanks! [00:09] davecheney: you have a ship it from me although I see you already had Tim's approval on GH === Spads_ is now known as Spads === kadams54 is now known as kadams54-away [00:39] davecheney: I edited that message before it emailed... [00:39] I missed out a word [00:40] davecheney: what I mean is "all good, land it!" [00:46] thumper: cool [00:46] i'll have to do a big branch to fix the callers of this type [00:46] there are a lot [00:46] and a lot of tests panic [00:46] now [00:46] that doesn't mean they are wrong [00:47] but they could be wrong [00:47] it depens on the state of the set passed in [00:47] for the record, this showed up doing a code review with ericsnow [00:47] i suspect he'd figured this out [00:47] and had worked around the problem [00:47] which prompted me to look a little closer [00:54] * thumper nods === fuzzy_ is now known as Ponyo [01:17] menn0: https://github.com/juju/juju/pull/1108 [01:17] ^ this is the kind of cleanup that suggests this refactor is on the right track [01:18] davecheney: just finishing another review [01:18] s'ok [01:18] no hurry [01:18] this will be the last one I do on this today [01:18] need to do cleanup from the set.Strings change [01:35] anastasiamac_: just reviewed 407 for you [01:36] menn0: thx!!! [01:37] Hello :D [01:44] davecheney: that change looks good and paves the way for allowing the watcher to work in a multi-env state server [01:44] davecheney: how do we feel though about types defined in state being used as part of the API? [01:46] davecheney: was the top level juju file there just to allow moving things around? [01:46] davecheney: my first thought was "WTH is that doing there?" [01:46] * thumper is +1 on the fix [01:47] davecheney: with your changes, Delta is defined in state/multiwatcher and is used by the multiwatcher internally, but is also used for the allwatcher API [01:48] davecheney: ultimately there might need to be a related struct defined in the API that returns Deltas [01:49] davecheney: that might happen when we do the multi-env support for the multiwatcher but I think we might want to change state/megawatcher/Delta anyway [01:56] menn0: api/ or apiserver/ ? [01:56] apiserver, i'm fine with [01:57] api, not so much [01:57] that's at the end of the list [01:57] it will mean params has to translate between state/multiwatcher types and apiserver/params [01:58] well right now Delta is used in api/ [01:58] yeah [01:58] it's not awesome [01:58] api/allwatcher.go [01:58] but it is what it is [01:58] I think ultimately we want apiserver to have its own type for this with some translation done there [01:59] yup [01:59] it's on the list [01:59] especially if Delta in allwatcher is going to grow some extra (internal-only) bits, which i suspect it will with multi-env suport [01:59] ok, as long as it's on the roadmap [02:00] davecheney: I think you could have easily moved MachineInfo, ServiceInfo etc etc in this PR as well [02:01] ugh [02:01] * thumper spotted shitty code he wrote some time ago [02:04] oh FFS, will you people stop getting shit done :) [02:04] more things to review === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 === kadams54 is now known as kadams54-away [02:13] menn0: talking of which http://reviews.vapour.ws/r/409/ ;) [02:15] waigani: that's what prompted my outburst :) [02:15] menn0: LOL, your welcome :) [02:15] waigani: I have been reviewing non-stop all day [02:17] menn0: can you see the matrix yet? [02:39] menn0: https://github.com/juju/testing/pull/37/files [02:39] :) [02:45] thumper: looking in a sec [02:46] waigani: any upgrade steps should now be targeted to 1.22. I just realised you've put some against 1.21. That needs to get fixed. [02:47] menn0: ah yeah right [02:47] waigani: have any changed merged already where this isn't right? [02:48] menn0: so settings was the last upgrade step I merged, that went in on Monday [02:50] yeah, they need to be fixed too [02:50] that was after the 1.21 cut [02:50] settings and settingRefs [02:50] waigani: ^ [02:50] menn0: yep, I'll move them [02:55] thumper: done. [02:56] ta [02:56] geez [02:56] nit pick much [02:57] but... [02:57] good review [02:57] * thumper will tweak [02:58] menn0: here is another https://github.com/juju/errors/pull/12 [03:10] thumper: that errors change looks fine although it appears to be used in a number of places in Juju. Are you going to change those? [03:10] menn0: yup [03:10] right now [03:10] thumper: "it" being LoggedErrorf of course [03:10] yeah, I knew what you ment [03:10] meant [03:10] which rhymes with bent [03:10] got I hate the english language [03:12] menn0: done. [03:14] menn0: although I can't change juju until the branch has landed (well, make it work anyway) [03:14] thumper: doesn't it make slightly more sense to change juju first? [03:15] menn0: umm... sure [03:56] wallyworld_: do we need to poke someone to get either https://github.com/juju/juju/pull/993 or https://github.com/juju/juju/pull/689 in? [03:56] wallyworld_: seems like an important fix to get in that hasn't due to poor handling of the review process [03:57] menn0: oops, yeah, i'll take a look [03:59] wallyworld_: cheers [04:02] menn0: only trouble is that there's a dependent goose mp on lp that needs fixing, so i'll have to follow up with the author of that mp [04:04] wallyworld_: right. just wanted to point the PRs out since they didn't seem to be moving. [04:04] yep, np, thanks, they had been forgotten about for sure [04:16] fuckity fuck fuck [04:18] * thumper is fixing an intermittent error in the actions time checking [04:47] menn0: https://github.com/juju/juju/pull/1110 [04:49] which is http://reviews.vapour.ws/r/410/ === liam_ is now known as Guest46061 === urulama___ is now known as urulama === Spads_ is now known as Spads [09:00] morning all [10:02] TheMue: standup ? [10:06] morning-ish [10:39] jam: it occurs to me that 404 might be different [10:39] k [10:39] voidspace: how so [10:39] jam: I think if we get a 404 maas might bomb out and not stop the other instances [10:39] jam: with a 409 it collects all the errors [10:39] jam: let me check [10:40] voidspace: so if you ask to stop 10 things, and *one* is a 404, it bombs out early.... :( [10:40] jam: yep, confirmed [10:40] jam: damn, my branch was ready [10:40] https://github.com/voidspace/juju/compare/maas-404 [10:40] :-( [10:41] but in the release bulk call it first checks that the number of found nodes matches the number requested [10:41] voidspace: k... do we have a way to know which item was causing the 404 ? [10:41] and raises a PermissionDenied [10:41] jam: parsing the message... [10:41] voidspace: my concern is that if you got your environment into that situation, how do you get out of it? [10:41] voidspace: I guess if this is "destroy-environment --force" then its ok because we're only listng the MaaS server anyway [10:42] jam: http://pastebin.ubuntu.com/8961592/ [10:42] and it won't have it [10:42] voidspace: so is that 404 or is that PermissionDenied ? [10:42] jam: what do you mean by "only listing MaaS server anyway" [10:42] maybe they translate PermDenied to 404 [10:43] maybe, but any missing for any reason will show up like that [10:43] voidspace: it *should* be 403 Perm [10:43] I think in that code they're assuming that if a node is not found it's because of permission [10:43] voidspace: its a fair point to not distiguish from 403 and 404 [10:43] the release single call (api) does an object_or_404 [10:43] since otherwise you can leak the existence (or not) of something [10:43] that you don't otherwise have permission to see [10:43] right [10:44] it's an incompatibility between those two calls though [10:44] so handling 404 in StopInstances is pointless [10:44] so there is still a small concern on the "destroy-environment --no-force" case [10:45] The single call /api/node//op=release can 404 [10:45] which is that the API server still wants to stop an instance that it knew about [10:45] if one node has been deleted but the rest are present, then the call to StopInstances will fail without releasing any [10:46] voidspace: it sounds bug worthy that "single node release == 404" but "bulk node release == 403" [10:46] jam: I'll raise it [10:47] so, for a 403 we could loop over the system ids and stop the nodes individually - or we could parse the error message and retry without the missing nodes [10:47] parsing the error message sounds fragile [10:48] voidspace: it does... my concern is that we are asking the API server to tear down nodes it knows about, and not giving any way for it to handle nodes that are already gone. [10:49] jam: for a 403 I can retry removing the instances individually and ignore 403/404/409 [10:50] just logging as a Warning [10:51] voidspace: so, not being able to "destroy-environment" because a machine was decommisioned in maas does sound like a poor user experiience failure mode, but it could be considered a different bug [10:51] so either fix it now, or right this discussion up as a new bug [10:51] it doesn't seem to have hit anyone yet [10:51] and it's easy enough to fix if we need to [10:52] the sympton will be destroy environment failing with a 403 error and not releasing any nodes [10:52] voidspace: I feel like "we know this is an easy to fix bug, but we won't address it now" isn't great. I'm wondering if we need to involve more people or if we've discussed it sufficiently to be confident in the fix [10:54] jam: well, I'm pretty confident of my reading of the maas code - it seems straightforward [10:54] ah, wait... [10:54] hah [10:54] jam: in fact... there's a call to self._check_system_ids_exist first [10:54] jam: that raises MAASAPIBadRequest if nodes don't exist [10:55] jam: so the PermissionDenied really does mean that the nodes failed to be found because of the permissions problem [10:55] voidspace: right, but we still need to handle MAASAPIBadRequest [10:55] and BadRequest isn't a 404 either [10:55] 400 I assume [10:56] so same problem, different number [10:56] and should we handle 403 [10:56] the node was released and given to someone else [10:56] voidspace: it sounds like we should test to see if we can actually make it happen, and then do as you say "kill in bulk, then go 1-by-1" [10:56] same problem as with a missing node - now we can't destroy environment [10:57] jam: ok, I'll try deleting the node and check that we fail to destroy environment with a 400 [10:57] and treat 400/403/404 in the same way [10:57] voidspace: note that I would expect "juju destroy-environment" to maybe complain on the API server side, but the client-side cleanup to be fine because it just lists the maas provider [10:57] (though there is still potentially a race) [10:58] jam: so you would want the --no-force to fail - or just warn? [10:58] voidspace: if the failure is that the node is already gone, destroy-environment should succeed [10:58] right [10:59] so what do you mean by "complain"? [10:59] voidspace: I mean that today [10:59] the "soft shutdown" is likely to fail [10:59] ah, I see [10:59] right [10:59] it did for 409 [10:59] but the hard shutdown doesn't know anything about the missing node [10:59] hence the bug - and even with today's code it should die with a 400 [11:00] right, missing nodes shouldn't be a problem for --force [11:00] understood [11:00] ok, I'll try it and see what happens [11:16] morning and sorry for missing standup, had to wait longer at the dentist as planned. thankfully e'thing OK as usual. [11:16] but at least my new setup maas controller used this time to download 135 boot images *phew* [11:19] dimitern: the described configuration of the interfaces worked btw [11:20] TheMue, great, so your VM networking is all set up correctly now? [11:21] dimitern: it looks so. I will now create the second instance for PXE on the private network of the controller [11:31] TheMue: 135?... [11:32] jam: yeah, impressive number, even if the size of my virtual disk lets expect a lower number [11:33] jam: but thats what my list her on the screen shows [11:34] jam: they are always with the purposes "comissioning", "install", "xinstall". so maybe they share a lot of resources? [11:34] and now, a bit more awake, hello everybody [11:34] perrito666: hello [11:35] jam: also several sub-architectures per arch [11:35] Hi, I'm trying to build juju on windows. "go get -d -v github.com/juju/juju/..." seems happy enough but "go install -v github.com/juju/juju/..." results in ... watcher.go:121: undefined: "github.com/juju/errors".LoggedErrorf. Is this an issue with missing dependancies? [11:42] gnuoy: take a look at GOPATH/src/github.com/juju/errors/errortypes.go [11:42] that should be defined there [11:42] mm or try running godeps -u dependencies.tsv on the juju/juju folder [11:43] wallyworld, hey, don't suppose you're around? wanted to chat quickly about the changes we need to make to distinguish between waiting/blocked [11:43] fwereade: hey [11:43] hangout? [11:43] perrito666, this godeps you speak of, should it be bundled with go for windows ? [11:44] gnuoy: nope, go get launchpad.net/godeps/... [11:45] wallyworld, joining ian-william [11:46] perrito666, fantastic, I think there may be light at the end of the tunnel [11:46] :D [11:47] jam: digged into /var/lib/maas/boot-resources, the files in the subarchs are hardlinked [12:47] how are we merging to 1.21 - git cherry-pick? [14:21] dimitern, would you pop into #jujuskunkworks please? [14:25] fwereade, ok, just a sec [14:28] fwereade: ping, need to speak to you about charm level constraints at some point [14:28] rick_h_: ping ^ [14:28] wwitzel3: howdy [14:29] wwitzel3: what's up? [14:29] rick_h_: hey, I'm going to be doing the work around min juju version [14:29] rick_h_: was told you might be a good person to sync up with before I start [14:29] wwitzel3: ok, what's the latest plan? [14:29] rick_h_: well .. I think I'm making right now ;) [14:30] wwitzel3: heh, I'm a chief complainer so happy to help :) [14:30] wwitzel3: if you can deal with coffee shop background I'm free to chat for the next hour [14:30] wwitzel3: or if you've got a doc/notes you'd like looked over I'm happy to [14:31] rick_h_: awesome, lets do it .. nope, no notes yet, some might exist, but I haven't seen any yet. [14:31] rick_h_: https://plus.google.com/hangouts/_/canonical.com/moonstone?authuser=1 [14:32] wwitzel3: rgr [14:32] wwitzel3: in there [14:33] fwereade: ping [14:34] natefinch, sorry, omw [14:53] hey, does anyone know what are the specs of the nucs marco used for this? http://marcoceppi.com/2014/06/deploying-openstack-with-just-two-machines/ [14:54] perrito666: just ordered two more the other day to expand our maas, I'll get you a link [14:54] perrito666: http://www.newegg.com/Product/Product.aspx?Item=N82E16856102035 [14:55] rick_h_: tx, I am trying to make a very similar experiment by mounting 4 physical nodes with 600U$D or less (obviously i will not be using nucs) [14:56] perrito666: gotcha, yea we setup http://maas.jujugui.org/MAAS using 3 (will be 5 now later today) nucs on a switch and such. [14:56] perrito666: look forward to seeing what you find to use. [14:56] rick_h_: hoy much ram are you actually packing into those? says up to 16 but I am not sure you are actually putting all that in [14:56] I used 8 and 128gb HD [14:56] because we lxc/container with them [14:56] http://www.newegg.com/Product/Product.aspx?Item=N82E16820191523 and http://www.newegg.com/Product/Product.aspx?Item=N82E16820148697 [14:57] perrito666: I think the orange boxes use the full 16 [14:57] yeeehaw, found the correct config to boot my vms in my vmaas [14:57] perrito666: but for our needs the 8's been ok mostly QA'ing stuff and doing some small testing. [14:58] rick_h_: I am trying to go with http://www.asrock.com/mb/Intel/H61M-VG3/ + http://ark.intel.com/products/71073/Intel-Celeron-Processor-G1620-2M-Cache-2_70-GHz [14:58] perrito666: does it have any sort of ipmi/amt control? [14:59] rick_h_: I am trying to figure out that :) I am pretty sure one of the boards of that family support ipmi [14:59] perrito666: you will hate life the moment you try to put any load on that Celeron [14:59] wwitzel3: I hate life in general [14:59] lol [15:00] wwitzel3: what kind of load are you talking about? [15:00] perrito666: good luck, I'm not seeing any sort of power control on that one but yea might be others. [15:00] rick_h_: true, that is the first model available here, but I am told I can get pretty much any model from that company [15:00] and they have decent ones [15:01] wwitzel3: I was honestly thinking on nothing more than have a maas with 4 of those and use it to test my juju on maas [15:01] locally [15:01] perrito666: gotcha, cool. Well if you get something let me know. We're heading to 4 nodes in our maas but I'd like to have more down the road [15:01] and if I could get them for less than $500 each that'd be nice [15:02] rick_h_: well I will most likely get a howto of this, the only thing stopping me is that hardware here has, from scratch a 50% import tax :p which kills my under 600 mark pretty easily [15:02] perrito666: ah time for a US sprint :) [15:02] rick_h_: well 600/4 is a good price :p [15:03] perrito666: definitely [15:03] rick_h_: I am pretty sure that if I pack 4 pcs with me I will have hard time convincing customs that it is for personal use :p [15:03] psh, you should see the stuff these guys travel around with. [15:04] I know one guy went to a sprint with 5 laptops for folks he got them for [15:04] natefinch, perrito666, wwitzel3: standup? [15:04] ericsnow: tosca call atm [15:04] oh my cal says its in one hour [15:04] natefinch, perrito666, wwitzel3: duh, see ya [15:04] rick_h_: our customs people are a bit sensitive these days :p [15:06] rick_h_: http://www.newegg.com/Product/Product.aspx?Item=N82E16813157419 [15:06] beats the nuc by quite far [15:06] :p [15:07] perrito666: so $130 and needs a cpu [15:07] perrito666: so not sure on 'quite far' not sure how much a cpu/cooler for that goes for [15:07] perrito666: but cool [15:07] I understood cpu was included [15:08] rick_h_: anyway yest, that is the server line :) I am looking for one in the consumer end [15:09] oh is it? cool guess it is http://ark.intel.com/products/77982/Intel-Atom-Processor-C2550-2M-Cache-2_40-GHz [15:10] perrito666: cool, if maas supports that ipmi tempted to try it out. [15:10] rick_h_: yup, cpu included supports a lot of ram, packed with 3 eths, it is quite production ready for the price [15:11] also 8 cores, pretty nice machine [15:11] perrito666: yea, might be worth an email to the maas list to see if anyone knows anything about it [15:11] rick_h_: yup, at least for testing purposes they seem quite useful given the price tag [15:23] meh, they have a lot of ipmi enabled motherboards, none consumer level [15:55] katco`: ping [15:56] anyone want an easy-ish review for 1.21? [15:56] http://reviews.vapour.ws/r/413/ [15:56] natefinch: ericsnow wwitzel3 can we push the standup an hour more, I need to rush to look for my wife that is a bit ill at her job [15:57] fwereade, dimitern, thumper, et. al. state.nowToTheSecond() is used in a few places to reduce the precision of time.Now() to the level of Seconds [15:57] perrito666: fine with me [15:57] perrito666: sure, hope she feels better [15:57] however that function uses time.Round(Second), and I think it would be more reliable to use time.Truncate(Second) [15:58] jw4, +1 [15:58] fwereade: ta [15:59] perrito666: sure thing [16:00] perrito666, ericsnow, wwitzel3: I'm going to push it 2 hours because 1 hour is no good for me. [16:00] natefinch: k [16:04] hey ericsnow, I'm trying to understand some of the cmd/juju/backups code, since I'm also implementing a new supercommand [16:04] bodie_: sure [16:04] ericsnow, so I'm getting "client.Close undefined (type APIClient has no field or method Close)" [16:05] bodie_: FWIW, I mostly followed the example of the new User super command :) [16:05] hmm, okay [16:05] I think my Client type actually is composed from two interfaces which both require Close() [16:05] so that might be the issue [16:05] bodie_: what is APIClient in this case? [16:05] I'll take a look at the cmd/juju/user code [16:06] bodie_: backups has its own API client type [16:06] ericsnow, it's like your backups APIClient interface requiring the methods from the api type [16:06] yeah, we have an Actions api client type [16:06] bodie_: nice [16:06] (api/actions/client.go) [16:07] the implementation is a little different though, since we don't require SendHTTPRequest [16:07] sinzui: I'm one of the culprits, sorry [16:07] currently waiting for a review of the backport of the fix [16:07] sinzui: (marking bugs as fix committed but not backporting I mean) [16:07] dimitern: you still around? [16:08] dimitern: this is a 1.21 bug fix that could do with a review: http://reviews.vapour.ws/r/413/ [16:08] bodie_: I don't see a Close method on the actions Client [16:08] bodie_: backups gets it from api.State [16:08] voidspace, looking [16:08] bodie_: so I expect you don't need to call Close on your client [16:09] dimitern: thanks [16:10] voidspace, all is well when we spot the problem early [16:10] good [16:10] ericsnow, I think that api.State bit is the missing link I'm not seeing [16:10] either way, I think the user cmd matches what we need a bit better, thanks! [16:11] bodie_: yeah, backups keeps it around for the direct HTTP requests so it has to close it later rather than in the New func [16:11] bodie_: cool [16:13] marcoceppi: can you try to look at this soon? https://github.com/marcoceppi/amulet/pull/48 [16:17] voidspace, LGTM +small comment [16:17] dimitern: where's the comment? [16:17] dimitern: did you publish it? [16:18] heh, reviewboard can't display the diff [16:18] ericsnow: http://reviews.vapour.ws/r/413/diff/# [16:19] * ericsnow checks [16:21] voidspace, yeah, it's on the PR itself [16:22] dimitern: ah, cool - thanks [16:22] dimitern: I can't quite - as a deleted node will still fail (400 error) [16:22] dimitern: I'm working on that now [16:22] dimitern: (close that bug you suggest) [16:23] voidspace: I expect the webhook code is trying to apply the diff to master rather than 1.21 [16:23] voidspace: I'll fix that [16:24] ericsnow: thanks old boy [16:24] :) [16:24] * ericsnow doffs hat [16:25] tvansteenburgh: can you look at an amulet pull request for me? [16:26] voidspace, sure, thanks [16:26] bac, sure [16:26] tvansteenburgh: https://github.com/marcoceppi/amulet/pull/48 [16:26] thanks [16:27] dimitern: do you know off hand if the errors package has a way to wrap multiple errors as a single error? [16:27] * voidspace is looking now [16:28] voidspace, I don't believe so, but it should be relatively easy to add a wrapper-for-two [16:28] dimitern: no it doesn't [16:29] I'd like a MultiError [16:29] dimitern: actually, philosophical question [16:29] dimitern: if we get an error 400/403/404 from maas release operation then at least one of the nodes is already gone and *none* will have been released [16:30] dimitern: so I'm going to loop and release them individually - ignoring ones with 400/403/404 errors [16:30] dimitern: so the question is, if in the loop one fails with a *different error* [16:30] dimitern: should I stop - or should I complete the loop and collect (and return) all the errors [16:33] voidspace, I think the loop should make the best effort to release all nodes, which means probably logging any errors and continuing [16:33] dimitern: ok [16:33] dimitern: we *should* return an error if there are unrecognised errors [16:33] dimitern: so I'll collect errors [16:34] dimitern: MultiError would be a pain, because it would be in compatible with Cause [16:34] dimitern: I'll construct a compound error [16:35] voidspace, there are a few cases around connecting to the api server code which strike me as very similar [16:35] voidspace, with the "moreImportant" filtering of encapsulated errors [16:35] dimitern: "moreImportant"? [16:36] dimitern: ah, interesting [16:37] voidspace, yeah, it might be what you need [16:38] bac, i can't merge into marcoceppi [16:38] 's repo, but i left a LGTM [16:40] bac: tvansteenburgh merged [16:41] marcoceppi: thanks! [16:41] thanks tvansteenburgh and marcoceppi [16:41] * marcoceppi is in conference lag mode [16:42] dimitern: moreImportant drops errors [16:42] dimitern: I don't think we want to do that [16:42] dimitern: unless we log all the errors and just return the last one [16:43] dimitern: the actual errors will be in the log [16:43] voidspace, I was thinking something along those lines, yes [16:43] dimitern: that's easy enough :-) [16:43] voidspace, we definitely need to log them, but only return unexpected ones perhaps? [16:43] dimitern: the question is what to do about multiple unexpected ones [16:44] dimitern: the only "expected" ones we're just ignoring [16:44] so I'll return the last unexpected error and log *all* errors - expected or not [16:44] voidspace, the last? [16:44] voidspace, yeah, ok [16:45] dimitern: well, I'm looping setting err as I go [16:45] dimitern: so the easiest implementation leaves err set to the last one... [16:45] well, I'll have to copy it actually [16:45] but you get the idea [16:45] voidspace, but for expected errors let's use Debugf or something (maybe Warningf with a bit more context?) [16:45] dimitern: I think Warning [16:45] voidspace, I mean they logged errors shouldn't look like something bad happened and we didn't handle it [16:46] dimitern: something unexpected has happened, it just isn't a problem (we couldn't stop the instance because it's already dead) [16:46] dimitern: ok [16:46] voidspace, +1, cheers [16:51] is there an easy way of finding the reviewboard review of a PR from the PR on github? [16:52] dimitern: ^ [16:52] in theory, whoever makes the PR is supposed to paste a link to the review on github [16:52] it seems the search capability of reviewboard is either exceedingly terrible, or simply not enabled [16:53] the CONTRIBUTING page doesn't seem to have a link to the reviewboard page [16:53]