[00:15] thumper: menn0 axw https://github.com/juju/testing/pull/85 [00:15] ^ for skipping go 1.5 failing tests [00:19] Bug #1520380 opened: worker/provisioner: unit tests fail on xenial [00:21] davechen1y: kk [00:21] thumper: ship it [00:22] davechen1y: why use the testing change instead of just using the go version in the code? [00:22] davechen1y: what does it buy us? [00:23] thumper: same as https://github.com/juju/testing/pull/84 [00:24] we're probably going to need to do it in a bunch of places [00:24] using build constraints requires at least 5 files [00:24] better to put it in one place, than replicate those turds across every package we have to skip conditionally [00:24] ok [00:24] good enough for me [00:25] also, when we're done with this, it's one place to delete [00:25] * thumper nods [00:25] davechen1y: I know I said Ship it, but one annoyance with this scheme is that we have to keep adding files as new Go versions come out [00:25] menn0: yes, and no [00:25] Parsing the output of https://golang.org/pkg/runtime/#Version is another option [00:25] you only need to add a new file if you want to identify that version of go [00:25] go 1.6 will identify as go 1.5 under this scheme [00:25] and also, when/if that happens, it happens in one place [00:26] and when we're done with this aweful hack, we can delete it in one palce [00:26] and when we're done with this aweful hack, we can delete it in one place [00:27] davechen1y: i'm fine with the approach, just wondering if using runtime.Version would be slightly better. it's fine the way it is. [00:27] it would be messier [00:27] parsing, yeah easy enough [00:27] but we'd lose the constant nature of these constnts [00:27] you'd get a float64, and get shat on by precision [00:27] equals may not work [00:28] doing it this way gives us equality between ideal constants [00:28] if testing.GOVERSION == 1.5 is a constant expression [00:29] i suspect if we did it by parsing runtime.Version, we'd end up _also_ defining a bunch of constants then returning those constnat values [00:29] which would also be forced throught float64 coercion, blah blah [00:30] this is less worse doing it this way [00:30] and if it feels a bit icky, that's good [00:30] we should be skiping all this crap [00:34] https://github.com/juju/juju/pull/3839 [00:34] ^ trivial review [00:34] axw: mgz_ http://juju-ci.vapour.ws/job/run-unit-tests-xenial-amd64/129/consoleFull [00:34] ^ what job triggers this build, is it build-revisions ? [00:35] it's not triggered in jenkins, the comment at the top is what tells lp:ci-director to run it, as soon as build-revison happens [00:36] from last night though, if you want to re-run a job [00:36] just login as developer per creds in lp:cloud-city [00:37] "build now" and give it the build revision job number (which serves as an index for all tests from one run) [00:37] ok, so if I land a fix for the xenial test failure, i should wait for build-revision to pick it up [00:37] yeah, to run a new revison just wait for it to go through ci as normal [00:39] mgz_: I noticed that if I kill a -race job that is working on a branch which I know will never pass [00:39] ci just runs it again [00:39] is there any way to avoid this ? [00:39] it wastes 3 hours every time it chews on some branch that will never pass [00:40] davechen1y: change failure-threshhold to 1 [00:40] issue with that being... the tests do randomly fail sometimes [00:40] mgz_: meh [00:40] so we get curses which retesting will 'cure; [00:41] if they fail sometimes, i'll comment 'em out [00:41] davechen1y: what you can do for that manual thing [00:41] is change it to one then... hm.. actually, you'd need to change it back after the entire run finished, which would suck [00:42] davechen1y: the problem with the just0skip-it approach [00:42] hmm [00:42] it sounds like I should just live with it [00:42] this problem will solve itself soon enough when feature branches rebase [00:42] well, problem one is we've been historically bad at fixing unreliable tests, skipping them just makes the problem easier to ignore [00:43] mgz_: no argument there [00:43] davechen1y: we can chose to not run race vs feature branches... well, that's bad long term, but we could blacklist a few now [00:43] anyway, problem two with skipping tests is sometimes it's like, "this entire suite is flakey" [00:43] mgz_: nah, this problem will solve itself soon enough [00:43] losing *all* our uniter coverage say, is bad [00:44] mgz_: no arguemnt there [00:44] we've shipped bad code in the past because tests have been ignored [00:44] i'm stuck between a rock and a hard place [00:44] indeed [00:44] thumper: do you have any comment on this position ? [00:44] how far back do I need to read? [00:45] 2 minutes [00:45] 4 at most [00:45] mgz_: rightly points out that historically skipping a failing package takes the issue off the radar [00:45] thumper: I write concisely and with clarity, surely reading isn't that much of a chore for you ;_; [00:45] i don't disagree, but historically fixing flakey tests, especially races as been an uphill battle [00:45] meaning we have no race coverage, so nothing prevening new races coming in [00:46] I think your approach on making -race useful was good [00:46] hmm... [00:46] I understand the races one [00:46] as people kept adding them too frequently [00:46] the tests just took way too long and had too much random output to be good without culling a bunch [00:46] I'm guessing that the wily failures are more limited? [00:46] as in we know which ones fail often? [00:47] xenial has 1 failure [00:47] worker/provisioner [00:47] welll... there's a few obvious bugs, like lxd borked a few tests [00:47] skipping that for go 1.5 gets us a voting go 1.5 build [00:47] and there's a few new intermittent failures, which... could be a number of things [00:47] which is crucial for upgrading to go 1.5 [00:47] what's the difference between failures on xenial and wily? [00:47] no idea [00:47] haven't looked at wily [00:47] and we get, possibily just a different symptom of, mongodb related failure [00:47] they're much the same [00:48] the package set hasn't really diverged, and it's all go 1.5 [00:48] * davechen1y is not touching the mongodb debt clusterfuck [00:48] if it's just the mongodb panic we can probably contain that [00:48] run three times, hope, etc, as previously [00:49] davechen1y: how about just fixing the test rather than skipping it if it is just one? [00:49] my main concern is that with 1.5 [00:50] if we are skipping a lot of tests [00:50] and losing coverage [00:50] we are losing faith that the code works [00:50] thumper: this is worker/provisioner [00:50] its riddled with timing issues [00:50] there have been bugs open against that packge for near 2 years [00:51] so clearly there is no priority in fixing it [00:52] so my point stands, if go 1.5 is more likely to cause problems in the provisioner due to the way things run [00:52] I'd rather have a failing test than a skipped test [00:53] my stance is generally this: [00:53] skipping shitty tests is one thing [00:53] but skipping good tests that show bad code is wrong [00:53] davechen1y: aside, I just added some code to the -race job config to show skipping particilar feature branches you know will fail, feel free to fill in branch names [00:53] you're fine with skipping packages with known data races, but you're concerned about skipping a package with logical timing issues ?!? [00:53] mgz_: thanks! [00:54] davechen1y: well... [00:54] the tests pass.. FSVO pass [00:54] you cannot choose which is the lesser issue [00:54] perhaps it is just a false sense of security [00:54] you cannot choose which is the lesser evil [00:54] every package I skip is tracked with an LP issue [00:54] make them critical blockers if you like [00:55] davechen1y: do you have tags on them? [00:55] if not, please add some [00:55] well, I guess the real issue is how close we are to getting stuff actually fixed in races vs 1.5 - I think there's probably only two issues we need looked at for 1.5 to get at least a resonable rate of blue [00:55] thumper: nope, no tags [00:55] which tags would you like ? [00:56] skipped-test [00:56] is what I have used [00:56] bzzr [00:56] bzzt [00:56] no new tag [00:56] how about skipped-test + ci-failure + ci-test name? [00:56] we have so many tags for "critical", "omg critical", "exploding customer", etc [00:56] adding another tag is a net negative [00:56] I think we should use milestones and LP status [00:56] a tag for skipped-test is fine [00:58] i disagree [00:58] historically tags have proved bad at communicating severity or priority [00:58] i think we should use LP severity [00:58] I agree with that, but it's not really a tag thing [00:58] which will automatically fit into the work that cherylj is doing with her dashboards [00:59] for example, we already have several tags for data-race [00:59] the main issue is no one looks at bugs as part of their daily workflow (well, except curtis, and we made him stop) [00:59] none have proven to motivate people to fix them [00:59] mgz_: no argument here [00:59] mgz_: no argument there [01:00] so, no one was taking bugs off the list of new issues and working on them unless instructed, either by hammer-on-head or by ci going into sirens-mode [01:01] mgz_: yup, so here is my proposition [01:01] skip the tests, get the jobs voting, fix the skipped tests [01:01] we've tried to do it in another order for some time now [01:01] without success [01:01] time to try something else [01:01] note: tests are skiped only in specific conditions, which are only true for a subset of CI builds [01:02] I am not unilaterally disabling the tests of a packge [01:02] mgz_: thumper for example, https://github.com/juju/juju/pull/3840/files [01:05] https://bugs.launchpad.net/juju-core/+bug/1520380 [01:05] Bug #1520380: worker/provisioner: unit tests fail on xenial [01:24] davechen1y: I think I have a fix for that test [01:25] found another bug [01:26] when status should indicate that an agent is down [01:26] it only sets the old status [01:28] ah this is so f'ed up [01:31] thumper: where was the log and return nil ? [01:32] instance poller [01:32] it should be setting the instance status to missing / unknown / down whatever [01:33] thumper: urgh [01:33] do you want me to take a pass at fixing that, or are you on it ? [01:34] axw: sure, if you reckon you have a fix then we can put my PR on the shelf [01:34] davechen1y: just running all tests now, should have a PR up shortly [01:38] axw: thanks! [01:44] wallyworld: what is up with your hangouts today? [01:44] nfi [01:44] i'm here [01:45] I can't hear you on the hangout [01:45] thumper: reconnecting, google has been bad today for me [01:45] docs as well [01:48] davechen1y: FYI, http://reviews.vapour.ws/r/3260/ [01:54] axw: hmm [01:54] that isn't the same issue [01:54] https://launchpad.net/bugs/1520380 [01:54] Bug #1520380: worker/provisioner: unit tests fail on xenial [01:55] wallyworld: sorry, giving up on the hangout [01:55] thumper: yeah, sorry [01:55] nfi [01:56] google is shite today [01:56] wallyworld: I'm going to write up a small spec re presense properly [01:56] ok [01:56] davechen1y: it is, grep for "unknown container type: lxd" [01:56] fair enough [01:56] lets commit it and see what happens [01:58] axw: do you have a few minutes? wallyworld said you have thought quite a bit about presense already [01:58] and I'm right in that head space [01:58] axw: no worries if you don't have time [01:58] thumper: can do, but meant to have a 1:1 with wallyworld in 2mins [01:58] axw: we can delay our 1:1 [01:58] ok [01:58] ping me when free [01:58] thumper: sure, name a place [01:58] wallyworld: okey dokey [01:59] axw: https://plus.google.com/hangouts/_/canonical.com/presence?authuser=1 [02:14] wallyworld: ready [02:16] menn0: I <3 u [02:16] ok [02:16] thumper: mwhudson https://groups.google.com/d/topic/golang-dev/y-mlM-XYysk/discussion [02:16] mgz_: do you want to share a room with him? [02:17] that may now be dangerous [02:17] * menn0 is glad we have our own rooms in Oakland [02:17] davechen1y: nice :) [02:17] davechen1y: oh hooray [02:18] i'm gobsmacked they did it in secrecy [02:18] yeah, seems insane [02:18] glad they're talking about it [02:18] still no link to the source though... [02:19] mgz_: so as well as 1.23 being a plague release, we know that 1.21 is dangerous too (well, only if any upgrade step fails) [02:19] mwhudson: i do hope they based their work of Go 1.5 or later [02:19] yeah, we should have apparently just kept the whol odd-numbers-are-bad thing [02:19] if they used 1.4 [02:19] davechen1y: ah uh yes [02:19] that will be, complicated [02:19] mgz_: menn0 <3s scotch (probably safer than sharing a room...) [02:21] mgz_: and before you get too excited... the bug I found was created by me :( [02:21] * menn0 hangs his head in shame [02:21] ehehe, that just makes it more amusing [02:21] double the scotch then \o/ [02:22] i'm glad it only exists in 1.21.N releases [02:30] davechen1y: nice [02:31] Bug #1520314 changed: Environment not usable after upgrade from 1.21.3 -> 1.25.0 fails with '"cannot retrieve meter status for unit xxx/0: not found"'

[02:52] axw: sorry, your change did not fix the problem I am seeing [02:53] mgz_: the build-revision job appears to be disabled [02:53] is that how it works, or is that a mistake ? [02:53] axw: http://paste.ubuntu.com/13522160/ [02:59] davechen1y: is that from juju-ci, or your machine? [02:59] my machine [02:59] but matches exaclty what I see for the xenial build [03:00] davechen1y: gah, I must have missed something. I'm getting it too [03:01] looking now [03:03] ta [03:03] davechen1y: no, sorry, I just hadn't pulled master. works on my machine ... :/ [03:03] davechen1y: can you confirm that "instance/container_go13.go" is not in your tree? [03:04] lucky(~/src/github.com/juju/juju/worker/provisioner) % ls [03:04] container_initialisation.go export_test.go kvm-broker_test.go logging_test.go lxc-broker_test.go provisioner.go provisioner_test.go [03:04] container_initialisation_test.go kvm-broker.go logging.go lxc-broker.go package_test.go provisioner_task.go [03:05] hmm, are we looking at the same thing ? [03:05] afk for a bit [03:05] davechen1y: what's in github.com/juju/juju/instance ? [03:05] ok [03:18] ok, crisis averted [03:29] axw: lucky(~/src/github.com/juju/juju/instance) % ls [03:29] container.go container_test.go instance.go instance_test.go placement.go placement_test.go testing [03:29] davechen1y: welp, I dunno then. I kicked off a build-revision job anyway [03:57] hmm... [03:57] wallyworld: I have a few questions about unit agent status [03:57] ok [04:01] davechen1y: http://juju-ci.vapour.ws:8080/job/run-unit-tests-xenial-amd64/133/console [04:01] davechen1y: Finished: SUCCESS [04:02] weird, it still fails 100% for me [04:03] davechen1y: only thing I can think of is that the juju/instance package didn't rebuild [04:07] axw: i have a change coming [04:08] its not a logic error [04:24] laters folks [04:50] wallyworld or axw: http://reviews.vapour.ws/r/3262/ please [04:50] sure [04:50] wallyworld or axw: no major rush as I'm done for the week [04:51] wallyworld: thanks [04:51] menn0: first glance, looks like a lot of moving of code? [04:51] wallyworld: yes, and a lot of changes to the moved code [04:51] \o/ [04:52] wallyworld: the PR and separate commits give more detail [04:52] ok, will look a little later [04:52] wallyworld: maybe it's easier to look at the individual commits on Github? [04:52] yeah === akhavr1 is now known as akhavr [09:03] dimitern: ping - if you have a minute [09:04] voidspace, sure [09:04] dimitern: so maas does support ipv6 subnets (max of one per interface) [09:04] voidspace, I didn't know it's just one [09:04] dimitern: my PR is done, most of your suggestions are in (tests refactoring and reformatting etc) [09:04] dimitern: according to their ipv6 docs [09:04] voidspace, cheers [09:05] voidspace, I'll have another look before standup [09:05] dimitern: however my code as written won't work with ipv6 - it assumes ipv4 for working out the allocatable range [09:05] dimitern: there's a TODO in there as you suggested [09:05] dimitern: should I fix it (without manual tests as I don't have an ipv6 setup) [09:05] dimitern: or ok to leave it as a TODO for now? [09:06] voidspace, as long as we keep it in mind, a TODO is fine for now I think [09:06] dimitern: cool [09:06] dimitern: I'll let you have another look before I land it then [09:06] dimitern: the only thing I didn't do was the extra work in the dummy provider [09:06] dimitern: I figure we can do that when we actually need it [09:06] it would be unused code until then [09:07] and I have ethical objections to unused code [09:08] voidspace, :) agreed - we can add it later, leaving a TODO there with some of the things we need would be good though - I'll comment [09:33] voidspace, reviewed [09:35] dimitern: subnetIdSet is a map, so ranging over it gets the keys and values not an index and value [09:35] dimitern: and it is a map because the bool value has semantic meaning [09:35] dimitern: if you read the code notFound is *not* the same length as subnetIdSet [09:36] dimitern: we only put in it the not found ones [09:36] dimitern, voidspace: why are we treating IPv4 and IPv6 addresses differently at all? They can both be represented as uint64. [09:36] dimitern, voidspace: subnet masks the same [09:36] dimitern: that's true for both your first two comments [09:37] voidspace, ah, I was too quick then :) I'll drop that issue [09:38] voidspace, we should use sets when possible though [09:38] in cases like this [09:38] dooferlad: because the code as written only works with IPv4 and would require rewriting [09:38] dimitern: did you read what I wrote... [09:38] dimitern: about why it is a map [09:38] voidspace, still, the second comment about using a helper for extracting notFound from subnetIdsSet holds [09:39] voidspace, yeah [09:39] dimitern: it needs to be a map [09:39] voidspace, yeah [09:39] dimitern: ok, I can do that [09:41] dooferlad: plus it would need some code to track if it's ipv4 or ipv6 addresses we're using [09:41] dooferlad: as addressing need converting back to the correct format [09:42] dooferlad: it's not hard it's just not zero work [09:44] voidspace: I mostly bring it up because it isn't hard and by landing this we introduce tech debt. [09:45] voidspace: that said, I am happy to write a followup. [09:45] voidspace: (as I am sure you would be) [09:45] dooferlad: cool [09:47] dooferlad: yep, there's a couple of places that use that conversion code that would need changing [09:47] dooferlad: definitely worth doing [09:47] dimitern: that trailing bracket was an oversight :-p [09:47] dimitern: thanks for catching it [09:47] voidspace: thankfully IPv4ToDecimal is used in four places before your change (other than tests), so killing it with fire is easy [09:48] voidspace, any time :) [09:51] dimitern: changes pushed [09:51] dimitern: it actually needs dooferlad's changes to gomaasapi to land first [09:51] dooferlad: did you get a review for your branch? [09:51] dooferlad: if not I'll look at it [09:51] voidspace: no, needs a review [09:51] voidspace, cheers - let's land it [09:52] cool [09:54] dooferlad: 1400 lines! [09:55] dooferlad: I'm concerned that the struct approach to serialising sends empty (uninitialised) arrays as null [09:55] dooferlad: which doesn't match maas and will break code that does GetArray() [09:59] voidspace: happy to run through it with you after the hangout [10:02] frobware: you around? [10:02] fwereade, jam, standup? [10:04] dimitern, strike 3 [10:04] dimitern: jam is never around on Friday :-) [10:32] dooferlad: will sync up with you shortly [10:32] voidspace: cool === beisner- is now known as beisner === cppforlife__ is now known as cppforlife_ [11:31] straw poll: WaitExpired, WaitForExpiry, WaitUntilExpired? [11:31] mgz, dimitern, frobware, voidspace? ^ [11:32] fwereade, last one [11:33] dimitern, cheers [11:33] fwereade, hmm. WaitForExpiry [11:33] fwereade, though I can do the last as well [11:34] fwereade, the For makes it sounds more like present tense [11:34] that works just as well for me :) [11:34] dimitern: WaitForExpiry [11:41] voidspace, was your "you around?" for the standup or something else? [12:10] frobware: yeah, just for standup [12:10] dooferlad appears not to be here === Makyo is now known as Guest61971 [12:25] dimitern, do you know if a runcmd (for cloud init & from juju) always needs to be a shell script? [12:26] frobware: I think cloud-init runs a shell script [12:26] not entirely sure though :-) [12:27] frobware, it needs to be executable AFAIK [12:28] frobware, have a look at https://cloudinit.readthedocs.org/en/latest/topics/examples.html#run-commands-on-first-boot [12:28] thx [12:29] frobware, but what I *think* you're asking is why adding a script generates a shell script wrapper? :) [12:30] dimitern, actually not sure what I was asking... just wondering how I know get my python-only /e/n/i renderer to work/be added. [12:30] dimitern, it's not quite 100% python but I was wondering if I still needed some sh wrapper [12:31] frobware, using AddScripts() creates a wrapper for you [12:31] frobware, it will render something like `#!/bin/bash\nscript1\nscript2...` [12:32] frobware, have a look around the tests in cloudconfig [13:24] Bug #1520571 opened: Juju destroy-environment stacktraces on local provider.

[13:27] Bug #1520571 changed: Juju destroy-environment stacktraces on local provider.

[13:30] Bug #1520571 opened: Juju destroy-environment stacktraces on local provider.

=== akhavr1 is now known as akhavr [14:18] fwereade, voidspace, dooferlad, frobware, please have a look - http://reviews.vapour.ws/r/3223/ I think the bindings stuff should be good to land [14:20] especially fwereade :) [14:21] voidspace, btw your branch looks like it needs rebasing before trying to land it [14:23] dimitern, ack [14:24] fwereade, cheers :) [14:30] dimitern, reopened one issue, looking at diff now [14:32] fwereade, I thought it was safer to do asserts like that [14:32] fwereade, but I'm happy to drop it if you think it's better [14:35] dimitern, I think it's mildly harmful -- when, e.g. two components are fighting over the doc, I'd much rather one just won the race, rather than risk them getting into edit wars with each other [14:36] dimitern, and also a txn is many times more expensive than a read, and doesn't actually give you any better certainty, someone could always overwrite just after your final confirmatory read [14:36] fwereade, right, sounds sensible [14:36] dimitern, cool [14:57] dimitern, reviewed [14:59]