[00:20] wallyworld: btw I began using quassel, it sucks that they dont support ubuntu indicator for messages [00:20] yeah, maybe htere's a plugin, not sure [00:21] doesnt seem to implement plugins [00:22] but having the server in amazon works better than bip [00:29] wallyworld: re bug 1236471 [00:29] Bug #1236471: Sporadic test failure w/ bot inside Uniter: FilterSuite.TestUnitRemoval

[00:29] wallyworld: it seems to be just utopic [00:29] is there someone running utopic that could test it? [00:29] oh joy [00:29] i can [00:29] works fine here [00:29] sigh [00:29] i'll try it now [00:30] wallyworld: although a key indicator to the problem could be the comment on line 111 [00:31] thumper: works fine for me on utopic [00:31] wallyworld: reading the code... [00:31] wallyworld: it seems that sometimes destroy removes the unit, and sometimes it just sets it to dying [00:32] smells like a race condition [00:32] the filter that we check dies seems to not find the unit [00:32] yeah, sadly [00:32] so it feels like in that case, the doc was removed [00:32] as opposed to just set to dying [00:33] wallyworld: reading the comment in state/unit.go Destroy method [00:33] that makes sense having now read the code [00:33] wallyworld: it seems that once destroy has been called, we can't depend on it being there [00:33] so the test looks fucked [00:33] or the filter is screwed [00:33] as it expects it to be there [00:33] and it has goine [00:33] perhaps the test should handle the missing unit error [00:34] I bet what is happening [00:34] is that the remove is being executed before the filter go routine starts [00:34] thumper: this comment [00:34] / Ensure we get a signal on f.Dead() [00:34] seems to imply the test expects the unit to be Dead [00:35] but it's either still dying or already removed as you say [00:35] seems like the test needs to ensure that the filter go routine has started [00:35] and it probably doesn't [00:35] nope [00:35] it doesn't [00:35] there is the race [00:37] filter.go line 281 will be where it errors [00:38] * thumper grunts [00:38] wallyworld: perhaps the best way to fix this is to have NewFilter not return until the goroutine for the loop has hit a ready state? [00:38] wallyworld: thoughts? [00:39] thumper: i'm just reading the filter code and in various places there's this: IsCodeNotFoundOrCodeUnauthorized [00:39] so it seems that the filter can expect the unit to not be there [00:39] and in that case, it returns an ErrTerminateAGent which is what the test wants [00:40] do you see what i mean? [00:40] in the loop() [00:41] oh, i just read what you posted above [00:41] let me look at that [00:42] wallyworld, thumper: fyi I have made little progress with bug 1409827 [00:42] Bug #1409827: TestSetMembersErrorIsNotFatal fails

[00:42] wallyworld: but you can see from the bug, the error is a "not found" one [00:42] wallyworld, thumper: mainly due to distractions at home [00:42] menn0: i hate those tests [00:43] wallyworld: because it is a params.Error not an rpc.Error [00:43] wallyworld, thumper: I am questioning why it's a CI blocker though. it fails far less often than the other ones you're currently looking at [00:43] wallyworld, thumper: I can only find one recent failure [00:43] menn0: maybe sinzui can answer [00:45] wallyworld, thumper, sinzui: it obviously needs to be dealt with, I just don't know if it's CI blocker material [00:45] wallyworld, thumper, sinzui: continuing with it now anyway [00:46] ty [00:47] wallyworld: I think I know why it isn't being caught [00:47] thumper: so the test log shows the CharmURL() api call failing with a params.NotFound error but this error should have been caught in the defer and changed to ErrTerminateAgent [00:47] wallyworld: because if any of the errors are traced or wrapped, then they don't match the call [00:48] maybe... [00:48] thumper: because ? [00:48] if err, _ := err.(rpc.ErrorCoder); err != nil { [00:48] err could be wrapped? [00:49] I'm thinking it is a possibility [00:49] however... [00:49] no... [00:49] because the resulting error is a params.Error [00:51] wallyworld: I don't see how this could fail... :( [00:51] sigh, me either, but we might me missing something subtle not being familiar with the code [00:59] thumper: if i comment out the deferred error handling to convert a not found to a errterminateagent, i can get it to fail just like in the test [01:00] so that does suggest that that error handling being used is critical to making the test pass, and i can't see how the error could be escaping [01:01] looking at the bug test log [01:02] [LOG] 67.77366 DEBUG juju.rpc.jsoncodec -> {"RequestId":12,"Response": [01:02] indicates that line 320 is the return statement in question [01:02] but I agree, can't see why it wasn't chagned [01:02] thumper: sill, i do think we need an errors.Cause() instead of a straight cast tp rpc.Error in the params error code stuff [01:03] rpc.ErrorCoder i mean [01:04] while I generally agree [01:04] I'm trying to work out this failure [01:04] and this isn't it... [01:04] api/uniter/unit.go line 446 [01:05] * thumper thinks... [01:05] hang on... [01:05] sure, that was a general cooment [01:06] not for this fix [01:06] fucker [01:06] ... [01:06] damn it [01:06] * thumper looks deeps [01:07] result.Error won't be caught maybe [01:07] i saw that line before and assumed it would be caught [01:08] oh wait, yes it will [01:08] wallyworld: I need a reference to a recent failure [01:09] result.Error is a pointer [01:09] and the *params.Error does match the interface [01:09] we need to see the recent failure [01:09] looking at modern code for an old failure is a waste of time [01:09] too much can change [01:09] thumper: attached to the bug [01:09] https://launchpadlibrarian.net/190620335/filter-failure.log [01:10] wallyworld: the 19th of November isn't recent [01:10] thumper: looks like a wrapping issue [01:10] ah, that one is a wrapping issue [01:10] obtained *errors.Err = &errors.Err [01:11] haha [01:11] yeah, that is one [01:11] so maybe we should do my previous suggestion [01:11] I'll fix this [01:11] awesome [01:15] wallyworld, thumper: I think I see a potential data race relating to bug 1409827 [01:15] Bug #1409827: TestSetMembersErrorIsNotFatal fails

[01:16] mustNext in worker/peergrouper/worker_test.go [01:16] the return value is set in a separate goroutine [01:17] isn't that a no-no? [01:17] thumper: ^ [01:18] might not be the source of the test failures but looks fishy [01:18] menn0: just in meeting, will look soon [01:19] katco: standup? [01:20] axw: shoot sorry [01:21] menn0: /me looks [01:21] menn0: which line? [01:22] thumper: from 513 onwards [01:23] thumper: the return value "val" is assigned directly from the goroutine [01:23] thumper: the more I think about it the more I don't think this is the test failure (because if nil was returned we'd see a different kind of failure) [01:23] thumper: but i'm clearing it up anyway [01:24] you could be right [01:24] but we aren't seeing "timeout waiting" are we? [01:24] menn0: it is wrong, but not the source I think [01:25] wallyworld: this bug isn't a critical blocker, but done anyway... [01:26] thumper: in meeting, will look in a sec [01:27] wallyworld: https://bugs.launchpad.net/juju-core/+bug/1236471 [01:27] Bug #1236471: Sporadic test failure w/ bot inside Uniter: FilterSuite.TestUnitRemoval

[01:27] gah, wrong paste: http://reviews.vapour.ws/r/723/diff/# [01:36] axw: test added [01:36] thumper: thanks [01:37] * thumper wonders if the bot will pick this up [01:47] thumper: yay, bot got your change. so i think the SetMembers test is now the only remaining blocker [01:47] menn0: I took a look at the test yesterday, it just looked like time sensitivity to me [01:48] menn0: if you double that last sleep, it'll fail each time [01:49] the sleep durations do not leave a lot of margin for error/jitter/whatever [01:53] axw: totally agree. i'm figuring out how to rewrite it to not be so fragile. [01:55] axw: in fact, checking that retries happen with exponential backup seems somewhat unnecessary [01:55] axw: what do you think about just ensuring that retries are occurring and leaving it at that? [01:57] wallyworld: branch landed, bug updated to fix released [01:58] wallyworld: as it was just a test failure bug [01:58] thumper: awesome, thanks [01:58] menn0: seems fine to me [01:58] menn0: it's a bit of an overkill test [02:14] wwitzel3: the patch to fix PortSet was pretty simple [02:14] wwitzel3: not that it affects us, but Intersection had the same problem [02:14] wwitzel3: anyway, I'm EOD :) [02:14] axw: I have a fix but thought I'd run one other possibility by you [02:15] axw: you don't think there's a possibility that the issue is to do with the way the count variable is being handled? [02:15] axw: it's being updated in another goroutine [02:15] menn0: moment [02:16] axw: davecheney tells me that there's no guarantees about how updates will be seen by other goroutines [02:16] menn0: yeah, I think that's wrong. it should just return the value on the channel... [02:16] menn0: I don't think that's the cause of the failure tho [02:17] well, it could be but I think the time sensitivity is more likely [02:17] axw: so do i but i thought i'd run it past you [02:17] axw: anyway, i've removed all the fragile timing checks... i'll have that up for review shortly [02:18] cool [02:18] menn0: maybe just change the chan bool to a chan struct{bool, interface{}} while you're there? :) [02:19] axw: oh i've done that [02:19] great [02:19] axw: but what I meant is the function passed to setErrorFuncFor in TestSetMembersErrorIsNotFatal [02:19] axw: it updates the voyeur with a integer [02:19] menn0: oh.. looking [02:19] axw: actually... never mind [02:20] axw: that's fine [02:20] axw: i'm looking too closely [02:20] axw: the variable is only used and updated from one goroutine so there's no issue [02:20] yup [02:26] axw: http://reviews.vapour.ws/r/725/ [02:26] looking [02:26] axw: the channel change is a separate PR [02:30] axw: here's the other one: http://reviews.vapour.ws/r/726/ [02:34] axw: all ok? [02:35] menn0: reviewed, I'd prefer if we got rid of any explicit sleeps [02:35] lemme know what you think - maybe I'm being too pedantic [02:37] menn0: alternatively just get rid of the sleep in the test, since mustNext will wait up to LongWait anyway [02:39] axw: I guess I was wanting to see multiple retries [02:39] axw: but that can be done with multiple mustNext calls [02:39] so i'll do that [02:40] menn0: hence the loop in my code, but yes, multiple mustNexts will do that too [02:44] ericsnow: is http://reviews.vapour.ws/r/724 really that big (about 40 files, 5000 lines changed) or has RB gotten confused? [02:44] * _thumper_ headdesks === _thumper_ is now known as thumper [02:44] * thumper headdesks [02:44] waigani: oops, no it's like 15 lines :) [02:44] yay string constants [02:45] go on, search the codebase for "90168e4c-2f10-4e9c-83c2-feedfacee5a9" [02:45] * thumper fixes [02:45] ericsnow: few. I thought, "there goes my afternoon..." [02:46] whoha, that's a lot of feedface [02:46] waigani: what you saw is the GCE provider patch (minus +/- 1500 lines of tests we're still writing) [02:47] waigani: we'll be splitting that up into multiple review requests though :) [02:47] axw: http://reviews.vapour.ws/r/725/ updated [02:47] ericsnow: that would be good, otherwise it's a hell of a patch to review! [02:48] waigani: :) [02:48] menn0: lgtm, thanks [02:48] axw: sweet. thanks. [02:49] thumper, wallyworld : fix for bug 1409827 merging. is the policy that I can mark the ticket as Fix Released once it's in b/c it's a test only fix? [02:49] Bug #1409827: TestSetMembersErrorIsNotFatal fails

[02:50] menn0: I think so [02:50] menn0: not sure, what tim said [02:52] wallyworld: good answer :-p [02:56] yep :-) [03:02] waigani: do you remember where we generate the uuid for new environments? [03:02] waigani: I remember we moved it... [03:02] but can't remember where [03:03] thumper: I think it's actually in util? [03:03] waigani: well, where we assign it into the environ config [03:03] thumper: will look in a sec, hangon [03:05] environs.ensureUUID [03:05] which is in environs.prepare [03:07] hmm... [03:07] ok [03:09] thumper: sorry, did you want me to hunt for anything else now? [03:09] waigani: nope [03:09] got it now [03:10] cool [03:23] damn... how long do these tests take to run? [03:23] geez [03:24] waigani: wallyworld: had to change http://reviews.vapour.ws/r/722/ [03:24] waigani: wallyworld: could u PTAL again? [03:24] * thumper taps his fingers [03:25] waigani: wallyworld: Get at apiserver now takes params.Entities [03:25] waigani: wallyworld: everything else should be the same [03:25] waigani: wallyworld: in fact, it's a revert to oirginal rather than a change... [03:25] anastasiamac: just read review, yep that makes sense to me [03:26] waigani: awesome - so I'll keep ur shipit :) [03:26] anastasiamac: sure :) [03:26] anastasiamac: +1 [03:27] can someone tell me how long the tests should take in worker/provisioner plz? [03:27] thumper: my last run [03:27] thumper: github.com/juju/juju/worker/provisioner 56.706s [03:27] anastasiamac: ta [03:28] wallyworld: thnx!! [03:30] merges are unblocked people [03:31] menn0: yes there r already changes qued up in jenkins :) [03:32] menn0: axw: thnx for unblocking it :) [03:32] anastasiamac: all those queued merges may or may not be mine [03:32] * menn0 ducks [03:33] menn0: and here i was thinking to start the collection for a case of scotch... [03:33] ha === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 [03:44] fark [03:44] fark fark fark [03:44] bitten by this same fucking issue again [03:45] FYI, machine config only has APIInfo structure set for state server machines [03:46] hmm... [03:46] no [03:46] damn [04:13] thumper: i'm trying to get my head around how an API connection for a new environment will be opened [04:13] thumper: the password will always be the same right? it's stored on the user, not the envuser. === kadams54 is now known as kadams54-away [04:14] thumper: actually... how do passwords for machines work? [04:14] yes, machines have passwords [04:14] menn0: but that bit isn't working right now [04:15] thumper: fixes in your branch? [04:15] menn0: I've opened a huge pile of worms with this env uuid in the agent config stuff [04:15] broken heaps [04:15] and slowly untangling [04:15] but I'm being called away [04:15] to walk the dog [04:15] so I'm done [04:15] for today [04:15] that sucks [04:15] ok [04:15] i will ignore that part for now === kadams54-away is now known as kadams54 [04:21] anastasiamac: looks like your branch failed on the bot, but I think it might be an infrastructure issue [04:21] take a look, you can probably just retry it [04:27] axw: thnx will look === kadams54 is now known as kadams54-away [05:18] is there a comment I can send to bot to not try to $$merge$$? [05:18] like cancel a merge? [05:20] anastasiamac: you need to have credentials, i can cancel an in progress landing if you want [05:21] wallyworld: ic [05:21] axw: if have have a few minutes, i'd appreciate a review of http://reviews.vapour.ws/r/727/ [05:21] wallyworld: no big deal i have qd couple of branches and noticed too late that one of them has unresolved conflict [05:21] wallyworld: looking [05:22] wallyworld: it'll just fail.. but thnx :) [05:22] ok [05:22] anastasiamac: annotations-tags? [05:23] wallyworld: no sync-tools [05:23] wallyworld: annotations tags are about to be backported to 1.22 ;) [05:23] wallyworld: since they've merged well :P [05:24] anastasiamac: wallyworld: aren't we in feature freeze for 1.22? [05:24] jam: this is not a feature that's new to 1.22 [05:25] jam: it's kind of a bug... that needs to be fixed in 1.22 [05:25] jam: trying to get ckient signature right to avoid conflicts later [05:25] jam: client* [05:26] jam: s/conflicts/headaches [05:26] k [05:27] jam: thnx for checking :) [05:36] wallyworld: reviewed [05:36] ty looking [05:37] grrrrrr, shitty tests [05:39] wallyworld: cherry picked annotations change http://reviews.vapour.ws/r/728/ [05:42] axw: i introduced a new bootstrap method to avoid churn on the other providers. i can quite see how to wrap the finaliser though such that the instance id is available to it, since it's called from bootstrap/bootstrap.go with a machine cfg without the id and is only filled in inside the closure [05:44] s/can/can't [05:45] wallyworld: maybe I'm wrong, gimme a sec [05:45] wallyworld: yep, you're right, sorry [05:46] axw: sure, np. you happy with the sig change to avoid churn? [05:46] wallyworld: yes that'd be good, thanks [05:47] axw: to be clear, I made the sig change to avoid updating all the other providers. but you didn;t like it [05:47] also, the way i have it avoids the boilerplate error checking [05:47] that would otherwise have to be introduced [05:47] wallyworld: where would there be extra boilerplate? [05:48] axw: what's inside the new Bootstrap() func - those 5 lines or so [05:49] wallyworld: I don't follow. the existing code hasn't changed much, and the callers of Bootstrap still need to check an error [05:49] since the environs.Bootstrap interface method i would think we'd want to retain [05:49] they just return common.Bootstrap() directly since the signature matches that of environs.Bootstrap [05:50] wallyworld: ah yeah, that would need to change [05:50] ok [05:50] forget it [05:50] ok, i'll make the err change though [05:50] wallyworld: keep it as is. I'll comment on the branch [05:50] ok, ta [05:52] axw: also, i started looking at bug 1384259. it seems cloud init is directly running the various apt commands it is configured with , and something else on the machine locks apt and then cloud init is sad. but i haven't dug any deeper. not sure if you have any ideas [05:52] Bug #1384259: race condition running apt in bootstrap

[05:52] not sure if we want to wrap the cloud init apt commands with a retry [05:54] wallyworld: apt is called from the ssh script, not cloud-init (on bootstrap only) [05:54] ah doh, yeah. i saw cloud init text in the log on the bug [05:54] we could lock... I don't *think* cloud-init does anything like that though [05:55] not sure off hand how to solve this one, need to dig into it some more. any suggestions welcome. [06:01] wallyworld: only thing I can think of is to add a script that waits for any apt or dpkg processes to stop running before we do anything [06:02] yuk, but may have no choice :-( [06:02] i've updated the pr too [06:02] wallyworld: it'd be good to know what it's conflicting with, that might give us a better approach [06:03] i'll ask on the bug [06:04] wallyworld: lgtm [06:04] ty [06:06] axw: in doing that branch i lost so much time due to not realising maas gave back instance ids that were different from the system ids to be passed over the api. sigh. i now know [06:08] wallyworld: :( any idea why we're using the resource_uri instead? [06:08] nope :-( [06:08] i found a helper function someone wrote to convert [06:08] so it must have been a deliberate decision [06:13] whee, finally. you can now provision ec2 instances with volumes [06:30] axw: whoot! fantastic [07:12] wallyworld: I forgot to ask before: is there a way we can flag some manual testing as being required for the next release? [07:12] (e.g. ensuring MAAS 1.7 deployments work well, for non-bootstrap machines) [07:19] axw: yes, i plan on raising this with curtis tomorrow [07:20] wallyworld: ok, cool [07:20] wallyworld: but in general, should we be using launchpad bugs or what...? [07:21] for recording testing notes? using lp bugs seems reasonable [07:21] this close to release, i wanted to do it more explicitly [07:21] wallyworld: not so much a note, as "we should not release unless we know this thing has been tested" [07:22] yeah that, sorry, was using the term generically [07:22] ok [07:22] but we don't have a documented process AFAIK to flag critical testing issues [07:23] wallyworld: in my previous job we used to create tasks for every new feature and major bug fix that would block a release. they'd generally need to be done by someone other than the implementer. we had the luxury of having big, dedicated testing teams though :) [07:24] axw: yeah, we had something similar previously for me also [07:25] we just need to make sure that wes the release manager and the QA team are informed, and that other stakeholders are brought in as needed to help test [08:33] axw, hey, are you still around? [08:34] dimitern: heya, yes I am [08:34] morning *yawn* [08:34] morning [08:35] axw, a quick storage question: do we plan to mount devices in lxc containers? [08:35] morning TheMue [08:36] dimitern: we want to be able to, yes. it's going to require some changes to lxc templates to allow mounting and so on [08:36] dimitern: why do you ask? [08:37] axw, because due to the networking work I plan to make lxc config file templates more flexible [08:37] axw, and this should also help for storage [08:38] dimitern: I see, yes, that will be helpful [08:38] axw, sweet, I'll let you know when my changes are in then [08:39] dimitern: thanks very much. probably won't be getting to lxc for a little while yet, but that'll be much appreciated [08:39] wallyworld: ^^ dimitern is helping with storage now ;) [08:39] wallyworld, axw, :D more like side-effecting it [09:31] wallyworld, axw, can any of you have a look at a small goamz PR? https://github.com/go-amz/amz/pull/16 thanks! [09:43] dimitern: ping [09:43] dimitern: cannot use parent (type names.Tag) as type names.MachineTag in function argument: need type assertion [09:44] voidspace, hmm [09:44]