[00:16] davechen1y: the second one was intended to be a fix it and ship it but rb disagreed with me [00:17] davechen1y: http://reviews.vapour.ws/r/2880/ the first issue is more a question of personal curiosity than an issue [01:24] * thumper headdesks [01:24] * thumper takes a breath [01:24] FFS [01:25] thumper: ? [01:25] just something that is SO broken [01:25] * thumper submits a reversal [01:25] thumper: is ur desk titanium? :D [01:25] or better yet - foam? [01:27] alright git folks [01:27] ... [01:27] * thumper works it out [01:27] thumper: git 42 ./...? [01:34] reviewer: http://reviews.vapour.ws/r/2886/ [01:34] plz [01:37] davechen1y: ^^? [01:43] wallyworld: ping [01:43] yo [01:45] wallyworld: I need a sanity check [01:45] quick call? [01:45] sure [01:45] 1:1 [01:50] Bug #1505866 opened: TestRenderNetworkInterfacesScript* tests fail on windows

[01:52] thumper: this is just a revert? [01:53] perrito666: yes [01:53] thumper: does it require juju-core fixes to go along? [01:53] Bug #1505866 changed: TestRenderNetworkInterfacesScript* tests fail on windows

[01:53] perrito666: no [01:53] thumper: then ship it [01:55] your comment seems to be on par with the code for what I can see [01:56] Bug #1505866 opened: TestRenderNetworkInterfacesScript* tests fail on windows

[02:07] anyone else needs a review before I go to sleep and you are out of reviewer until its the morning wherever katco lives? [02:13] k timeout [02:13] bye [03:16] wallyworld: here's one http://reviews.vapour.ws/r/2887/ [03:16] ok [03:21] wallyworld: just pushing a branch based on what we talked about [03:21] ok [03:24] wallyworld: http://reviews.vapour.ws/r/2889/ [03:25] ok [03:30] wallyworld: :-( the check means passing the gc.C object all the way through [03:30] * thumper hacks [03:31] thumper: oh. was a suggestion, but i think worthwhile, if you agree [03:31] I'm just trying it [03:33] wallyworld: I think this is what you are after, good enough? [03:33] looking [03:34] wallyworld: most the attempt strategies are in tests right now [03:34] and it makes the code less clear [03:34] IMO [03:34] also [03:34] fastclock change looks ok [03:35] the duplication of the unlock is so we have decent logging [03:35] no point saying we are retrying and then not [03:35] i realise that, but you log after each failed attemot [03:35] by having the unlock first, then retry loops, better information logging [03:35] but then you have to check if you are the last one [03:35] and not log that one [03:35] I went for this as I thought it was more clear [03:35] ok [03:36] feel free to drop any [03:36] ok, I'll try to be nice [04:03] wallyworld: in the end I did everything you wanted [04:03] just like always [04:03] :) [04:04] thumper: aw, shucks [04:04] but you didn't have to [04:04] is there anyone looking at the 1.25 block? [04:04] * thumper shrugs [04:04] it's fine [04:04] eg the attempt strategy [04:04] and you made a good point abput the loop [04:04] Attempt has a HasNext [04:04] just used that [04:04] ok [04:04] internally it is overkill for what is needed [04:04] will be good to se ehow this improves it [04:05] but as a UI, it is fine [04:26] ugh [04:27] bug 1505866 [04:27] Bug #1505866: TestRenderNetworkInterfacesScript* tests fail on windows

[05:00] * thumper EOD [05:00] laters [05:39] Bug #1505902 opened: MAAS 1.9: "storage-default-block-source" is not allowed in bootstrap

[06:58] morning all [07:47] dimitern, hey - can we catch up after the sprint? [07:47] dimitern, s/sprint/standup/ [08:05] frobware, hey, yes - I've just accepted it [08:06] frobware, upgrading my hardware maas here to 1.9 to test voidspace's fix and mine [08:09] dimitern: cool, I'm porting mine to master [08:09] dimitern: you're doing 1.24 differently apparently? [08:09] dimitern: mine's landed on 1.25 [08:10] dimitern: including a fix for interface alias mangling [08:10] voidspace, I had some comments yes - mostly about taking a simpler approach with the script [08:10] voidspace, that last one I couldn't quite get [08:10] dimitern: you mean understand? [08:10] voidspace, yep [08:10] so maas creates aliases like "eth0:1" [08:11] dimitern: and if the primary interface is "eth0" then some of the sed manipulations that change "eth0" to "juju-br0" [08:11] voidspace, ah I see, so the script needs to replace the whole thing incl. :1 with juju-br09 [08:11] juju-br0 even [08:11] will change some references to "eth0:1" to "juju-br0:1" [08:11] yep [08:11] which broke mongo [08:11] voidspace, hmm good catch though! [08:12] so I fixed that by improving the sed regex to check for "${PRIMARY_IFACE}\s*$ [08:12] voidspace, so did you manage to reproduce the issue on 1.9 before your fix? [08:12] dimitern: yep [08:12] voidspace, I'm not sure that thing with the aliases was part of the original issue - as you perhaps added 2 IPs to the eth0 NIC, and maas renders those as aliases [08:13] dimitern: I didn't change the aliases - I left them intact. I changed the script to not touch them. [08:13] dimitern: I don't think it was part of the original issue [08:13] dimitern: but it showed up when testing - so I had to work out why it was happening [08:13] somehow I'd got an alias for a nic on one node [08:13] dimitern: you might be interested in http://reviews.vapour.ws/r/2892/ (support service constraints in bundle deployment). if you could take a look it would be great [08:13] not sure how [08:14] frankban, awesome! will have a look, thanks! [08:14] dimitern: ty [08:14] dimitern: I don't know if you saw - my PR splits the part of the script that does text manipulation on /e/n/i into a separate script [08:15] dimitern: so it can be unit tested [08:15] * dimitern has a shiny new hw maas with 1.9.0alpha4 now :) [08:15] dimitern: so adding new tests / changing the script is easy [08:15] voidspace, sounds good [08:16] voidspace, my alternative solution was to do something like sed -i s/${PRIMARY}/{{.Bridge}}/;/iface ${PRIMARY} inet/a\ bridge_ports: ${PRIMARY}" && append "iface ${PRIMARY} inet manual" [08:19] frankban, is guibundles branched off of master? [08:20] dimitern: that looks pretty much equivalent yeah [08:20] dimitern: yes, last merged master was a blessd one from the week before seattle. will have to merge it again soon [08:21] voidspace, it's shorter but perhaps more dangerous, as it does replace all "eth0" with "juju-br0" indiscriminately [08:21] dimitern: yep, it would need fixing to not mangle aliases [08:21] frankban, ok, I've LGTM-ed it - will you backport the same to 1.25? [08:21] dimitern: you can run the tests I wrote against it [08:22] voidspace, yeah, I'll now test first your fix then mine [08:22] dimitern: I was going to ask about what should I do to include this work in 1.26 [08:22] dimitern: so I'll do whatever required [08:23] dimitern: oh, and thanks for the review! [08:23] frankban, well, master (1.26) is not quite up-to-date with 1.25 as I've realized last week, there are a few fixes needed in provider/ec2, but I'll take care of that [08:23] dimitern: I'm porting mine to master - if you have a simplification we could do that as a patch on top of what I've already done [08:23] voidspace, sounds good - we can have a simpler solution for 1.26 and one that works for 1.24 and 1.25 (even if they're different - shouldn't be a big deal) [08:23] dimitern: if you go to an individual node view in maas 1.9 you can add an interface alias and check your fix works with that [08:25] voidspace, I'm doing just that now - added 2 aliases and set eth0 (the only NIC the NUC it has unfortunately) to static, 1 alias to auto assign, and 1 to static [08:27] voidspace: bug 1505949 [08:27] Bug #1505949: TestRenderNetworkInterfacesScript* fail on windows

[08:28] mgz, d'oh :) voidspace - it should be skipped in windows [08:28] mgz: pthpthpthpth [08:28] it's easy to fix at least :) [08:28] that looked like an interface name with biosdevnames=1 :) [08:29] mgz: dimitern: ok, on it [08:29] voidspace, cheers - please add a card for it etc. [08:29] mgz: it's not in master yet - so I'll land a fix for 1.25 and make sure the fix is in the port to master [08:29] dimitern: yep [08:31] voidspace: also not in 1.24 it seems, a different version of the maas fix landed there? [08:31] mgz: not yet [08:31] mgz: shortly... [08:31] okay, thanks! [08:33] mgz, regarding http://reviews.vapour.ws/r/2883. I guess your comment is in response to my use of return err in the UnmarshalYaml function? [08:36] mattyw: yeah, as we're now returning err rather than false from the marshalling funcs, I just wondered if the errors are clearer with trace [08:37] mgz, I'll look again, but I think you're right, I think trace would be better, thanks for pointing it out [08:38] or something like `spaces, err = parseYamlStrings("spaces", val); if err != nil { return errors.Annotate("failed to parse spaces", err) }` type thing [08:39] don't think we need to go overboard, but may be nice for the complex functions [08:41] mgz, it's useful in contraints becuase there are a few steps to that [08:41] mgz, but in version it's best to leave it as it is I think [08:41] mattyw: yes, that makes sense to me [08:42] mgz, just pushing those changes, do you want to take another look? [08:48] mgz: dimitern: http://reviews.vapour.ws/r/2893/ [08:52] mattyw: reshipit [08:53] mgz, it's going in, thanks very much [08:53] voidspace: looks fine [08:54] voidspace, reviewed [08:54] dimitern: I'm not sure about your comment [08:54] really the skipifbug is for stuff we're skipping until we fix the bug [08:54] dimitern: it's not a windows bug [08:54] I don't intent to implement /bin/sh on windows [08:55] voidspace, mgz, I see - well, then ignore my comment :) [08:55] heh :-) [09:02] fwereade, standup? [09:56] mgz, anything more I can do to help with yaml.v2 stuff? [09:56] mattyw: can you look at my charm branch change? [09:57] rog suggested expanding test coverage on it, so suggestions on that would be helpful [09:59] mgz, looking now, there seems to be some controversy about using the latest juju/utils, any idea what's going on there? [09:59] mgz, you seem to change it in that pr [09:59] mattyw: it's fine if we rebump to now latest [09:59] and it's not code that charm uses [10:00] mgz, any idea what that controversy is [10:00] mgz, also, any idea what kind of expanded test coverage rog had in mind, it seems ok to me, worrying that I'm missing something [10:01] the see github.com/juju/utils/pull {159,162,163} [10:02] mattyw: I think he had in mind that the YAMLMarshal tests didn't tell anyone that storage wasn't being serialised [10:02] and don't say if the old redundant vars should be [10:04] hm, and this test just failed, did it do that for me last time... aha, no, it's after I rebased on latest [10:04] well, that's a starting point [10:04] I'll bump utils and do this [10:05] ooo, I'm silly [10:06] I did the rebase wrong, someone had added json attrs to Meta, not yaml ones [10:06] ...three lots of these is going to be painful [10:08] mattyw: so, my basic issue is I'm not quite sure what the intended non-bson serialisation of Meta is exactly [10:11] mgz, we can get rid of the meta.SetYaml because yaml.v2 does it all for us right?... [10:11] mgz, but type Meta is missing the yaml tags, which I think would be useful [10:11] mattyw: right, it needs them [10:11] mattyw: I had added them but the rebase conflicted and I dropped my changes [10:11] mgz, ah ok [10:12] mattyw: but I don't quite understan https://github.com/juju/charm/pull/158 [10:13] mattyw: do you know what is using json serialisation of Meta? there are no tests for that in meta_test.go [10:14] or was that just a mistake and yaml was intended? [10:15] (it's what I assumed on the conflict...) [10:16] mgz, hmm, I'm looking through master for changes to see if it's used there [10:16] mgz, it's also possible it's used in one of the feature branches? [10:16] mgz, or a mistake? [10:16] I think it's just a mistake that happened to work [10:18] because what mattered was the field rename back, and the serialisation tags are ignored... and GetYAML actually did the serialisation [10:19] ...hm, comment in that branch makes it sound intentional [10:19] but the pr discussion then makes no sense [10:22] mattyw: I'm just going to make the yaml serialisation do exactly what it did before [10:22] and let someone else deal with this confusion [10:22] mgz: the windows bug was never committed to master [10:22] mgz: can I delete juju-core from that issue [10:22] dimitern: http://reviews.vapour.ws/r/2894/ [10:22] mgz, I'd say that's a wise choice [10:23] voidspace: I prefer if it's just Invalid and present, otherwise makes lp search even more confused [10:23] mgz: the branch has landed on 1.25 (thanks for kicking the PR again) and I've marked it as fix committed [10:23] mgz: ok [10:23] mgz: I have a branch for master that incorporates the windows skipping [10:24] dimitern: shall I backport mine to 1.24, or would you rather land yours there? [10:26] voidspace, do yours [10:26] voidspace, I'd like to simplify the master [10:27] s/master/fix for master/ [10:28] Bug #1505866 changed: TestRenderNetworkInterfacesScript* tests fail on windows

[10:33] dimitern: ok [10:34] mattyw: okay, this is terrifying [10:34] three different serialisations with slightly different behaviours [10:35] like, is it intentional or a tyop that Tags is json:"tag" [10:35] mgz, do we have tests in place for the three different behaviours? [10:35] I see nothing anywhere using json [10:37] Bug #1505866 opened: TestRenderNetworkInterfacesScript* tests fail on windows

[10:38] mattyw: I run `:%s/ json:$[^ ]\+$//g` and no tests fail [10:38] (seriously vim, I always forget which regexp chars need \) [10:39] mgz, so, luckily for me I'll be buggering off to lunch soon ;) [10:39] mgz, but [10:40] mgz, I'd try to break the behvaiour and see if any tests in charm or core fail [10:40] mgz, and then try to get at least simple test coverage for the 3 behaviours in the charm package [10:40] mgz, and then cover it in todo's and bug numbers to sort it out [10:40] (if you want my opinion) [10:40] mattyw: I have fixed by branch, pushing now [10:41] I'll send a message to list about whether the json stuff was actually intended [10:41] dimitern: so for 1.24 I'll just land it [10:41] dimitern: for master I'll wait for your review with a suggested simpler approach [10:44] voidspace, sgtm, sorry - still in a call [10:44] dimitern: np [10:49] Bug #1505866 changed: TestRenderNetworkInterfacesScript* tests fail on windows

[10:53] mgz, taking a long lunch, I'll look at the charm pr when I get back [10:59] mgz: to get a "bug fix" PR past the bot, where should the "fixes-xxxx" thing go? [10:59] mgz: in the subject of the PR or the body? [10:59] mgz: and are the square brackets and quotes mandator? [10:59] y [11:01] fixes-xx in comment on github, no other characters required [11:07] mgz: ah, in a comment! [11:08] mgz: i tried subject and in the body in 3 forms! [11:08] rogpeppe: unrelated, I would like to land https://github.com/juju/charm/pull/164 even though the serialisation is insane, and fix the test coverage/intetions in a follow up. the v2 changes to yaml have the exact behaviour as before, it's just everything else is nuts [11:08] rogpeppe: it should probably check the body of the commit for bug mentions too, it's nice to have them there anyway [11:10] mgz: i'd really like it if you added some serialization tests in a previous branch (without mgo.v2) then we can see how the behaviour changes [11:10] mgz: otherwise we have no way of knowing what it has changed [11:12] rogpeppe: so, what I'm changing (the yaml) is covered by testing [11:12] rogpeppe: what's not covered (the new stuff, storage etc) I've now omitted from serialisation, which is what the codew was doing before [11:13] and thelack of tests of any json serialisation is another thing all together [11:14] rogpeppe: so, what I anticipate, is a follow branch that adds Sotrage and PayloadClass and testing for those [11:15] mgz: it would be nice to add some tests that have some omitted storage serialisation [11:15] rogpeppe: and another folloup that either deletes the json or actually tests it [11:15] mgz: so that it's easy to see that the followup fixes that [11:15] mgz: i don't think you can delete the json. [11:15] okay, will throw one in for that [11:15] mgz: thanks [11:16] mgz: at least one API already serialises the charm metadata as JSON [11:19] well, in the last 16 days it changed from serialising Tags as "tag" rather than "tags" [11:22] rogpeppe: I give up, just going to have to fix this ;_; [11:22] oh really? that's bad. [11:22] I prefer when touching code doesn't mean you have to sort out everyone before you's bugs [11:22] mgz: me too [12:05] voidspace, I'm having trouble with your fix for 1.25 [12:06] voidspace, here's what /e/n/i looks like after the juju-br0 script have run: http://paste.ubuntu.com/12780287/ [12:07] voidspace, here's how it looked like when deploying the node via MAAS (no juju involved): http://paste.ubuntu.com/12780294/ [12:08] voidspace, notice the aliases are not handled ok and there's also no lo device [12:17] voidspace, sorry, just to be sure I've deployed another node from maas, configured the same way and lo is missing, but the rest is as expected: http://paste.ubuntu.com/12780344/ [12:24] voidspace: so I have maybe-bad news [12:25] man we really need to find a prettier way to include bash into our code [12:25] voidspace: the networking for lxc on our wily slave seems to be screwed, not sure why yet [12:26] voidspace: but could be a leak from running local-deploy on that machine with the 1.24 version of the maas networking change [12:27] or manual deploy [12:28] 1.25 did not have this problem [12:30] voidspace: is it possible our real /etc/network/interfaces got stomped? [12:36] mgz, what do you see? [12:36] mgz, I think voidspace's fix does not do the right thing with /e/n/i [12:36] just no interfaces other than loopback visible to a non-juju managed lxc container [12:37] and whoops, I think I just borked networking to the box entirely when poking things ;_; [12:37] mgz: ouch. borking networking is definitely something that hurts, as you use it to fix the thing [12:38] dimitern, voidspace: I also see the lack of loopback [12:38] mgz, hmm that sounds like an issue with /var/lib/lxc//config [12:38] frobware, yeah, I can confirm this on both vivid and trusty, but this is how maas (curtin?) renders /e/n/i [12:40] dimitern, you mentioned this morning about cloud-init not getting an interface with just a deploy from MAAS... I think? [12:41] frobware, on one of the machines, yes - which has a single eth0 configured in maas to DHCP [12:41] dimitern, so I think I see the same. eth0 is static, with static aliases for eth0:1 and eth0:2 [12:41] dimitern, cloud-init eventually completes post its 120s timeout [12:43] Bug #1506044 opened: juju.worker.diskmanager goes into ERROR loop inside a container

[12:44] frobware, and no lo to defined at all? [12:44] dimitern, and although there is no 'auto lo - iface lo inet loopback' entry rebooting the machine I do see the loopback device configured. [12:44] frobware, ok, so then the lack of loopback perhaps is not an issue? [12:46] dimitern, I can certainly `telnet 127.0.0.1 22' and it connects so there's something bound and listening... [12:52] frobware, right [12:53] frobware, I'm experimenting with the ways to deal with juju-br0 where there are aliases [12:53] dimitern, I'm just wondering if the whole thing could be simpler. [12:53] dimitern, the aliases seems to work OK, no? [12:54] frobware, I managed to get both s/eth0/juju-br0/ + add iface eth0 inet manual and bridge_ports eth0 to juju-br0's config; AND just replacing the "eth0" -> "juju-br0", leaving "eth0:0" and "eth0:1" intact [12:54] frobware, the only issue is in the second case I end up having duplicate routes: 192.168.1.0/24 dev eth0 and juju-br0 [12:56] frobware, voidspace, ok, I'm convinced now the 1.9 fix is not working for me, I'm going back to my original simpler fix and tweaking it to make it work [12:57] dimitern, there's also the possibility of introducing 'ip link add name juju-br0 type bridge' [12:57] frobware, however.. I've just realized we might be having a lot bigger issue here [12:57] frobware, yeah, I'll check this too [12:58] frobware, the bigger issue is simply, juju-br0 is *supposed* to get its address via DHCP [12:58] frobware, and hence the containers on it will do the same [12:58] frobware, when we have a statically configured eth0 (or juju-br0) we won't be able to just do dhcp on the container side [12:59] frobware, in fact this seems so serious to me that we need to have a chat with the maas team how to resolve this [12:59] it won't just work [12:59] without dhcp [13:00] it might work ok if we had a solid story around addressable containers (with static IPs) [13:00] dimitern, there's nothing preventing the containers getting DHCP [13:00] dimitern, MAAS still runs DHCP, no? [13:01] frobware, I've experimented with a vivid vbox vm so far, but the case is the same more or less - single eth0 on the vm, configured as bridged to the host's eth0 (which is statically configured, but can also dhcp to the router and get an IP) [13:06] dimitern, voidspace: so I think this line is broken in that script: " sed -i "s/auto ${PRIMARY_IFACE}//" {{.Config}}" [13:06] dimitern, voidspace: this is why you get ":1", ":2" entries [13:07] frobware, yeah [13:07] dimitern, voidspace: this might be better expressed as s/auto ${PRIMARY_IFACE}[^:]// [13:07] hmmm [13:08] dimitern: frobware: just been on lunch, reading scrollback [13:08] doesn't sound good [13:14] mgz: ouch [13:15] mgz: I only touched maas networking not local [13:16]