[00:00] natefinch: see tim's email thread from last year about the proposed design [00:00] sorry i cannot link to it [00:01] i dunno how to link to messages on that ML [00:01] davecheney: search for diaf is surprisingly useful [00:01] :) [00:04] Bug #1582841 changed: remove check for lxdbr0 [00:08] davecheney: I don't see anything in that thread that says that flock won't work. It seems to say flock won't release on process end, but that's incorrect from my reading and experience. go closes the file handle and the lock is released on process exit. [00:09] er lock is released when the file descriptor is closed, which happens automatically on exit [00:10] davecheney: if you just want to avoid futzing with files on disk, that's valid, too [00:11] anastasiamac: FWIW I only found these 3 https://goo.gl/JG8NXL none of which would have been fixed by the updates to aggregator AFAICT. [00:25] natefinch: please, no flock, use unix domain sockets as we discussed [00:25] davecheney: ok [00:25] flock requires a file on disk [00:25] if I delete that file I can effeictly release that lock [00:25] and we're back to square one [00:26] can you do that in windows? (without a signifficant effort?) [00:26] nope [00:26] windows is a real lock [00:26] I know flock is only advisdory [00:27] it's not that flock is advisory [00:27] but the thing that is being locked can be moved or deleted which breaks the lock invariant [00:29] flock also leaves with the 'oh i crashed and there is a file on disk' problem [00:29] man page says "Apply or remove an advisory lock on the open file specified by fd." maybe the docs are misleading, but regardless.. yes, you can break it trivially [00:29] the file on disk doesn't matter [00:30] yes it does [00:30] you need to have a file to apply a lock to [00:30] the existence of the file has no meaning [00:30] you cannot flock a non existant file [00:30] so you have to create that file [00:30] well, yes [00:30] so now you have two lockers racing to create the file [00:30] if you fail to create the file because someone else was racing with does taht mean you cannot lock it, etc [00:30] k people, EOD, see you all on thu [00:30] please just use unix domain sockets [00:31] they solve 100% of these problems [00:31] it's really not a race, but that's fine [00:31] net.Dial("unix", "@/juju/$SOMEHASH") [00:32] I open with O_create... one proc will create first, the other will open [00:33] good point [00:33] but please [00:33] no files on disk [00:34] I agree that files on disk are a liability. I was just doing it that way to minimize changes to current code. but we can do socket on linux and mutex on windows [00:38] fwiw I seem to be hitting bug 1537585 a lot more with juju 2 and maas 2 [00:38] Bug #1537585: machine agent failed to register IP addresses, borks agent <2.0-count>

=== redir is now known as redir_afk [00:59] thumper: what tha ? [00:59] lucky(~/src/github.com/juju/juju/cmd/jujud/agent) % go test -i -v . && go test . [00:59] ok github.com/juju/juju/cmd/jujud/agent 122.841s [01:00] lucky(~/src/github.com/juju/juju/cmd/jujud/agent) % go test -race -i -v . && go test -race . [01:00] ok github.com/juju/juju/cmd/jujud/agent 1.168s [01:00] ^ this test runs 100x faster under the race detector ... [01:00] lol, we should use the race detector during production, evidently [01:02] uh oh [01:02] i have a terrible feeling about this [01:02] I have a great feeling that this is going to be an awesome bug [01:02] lucky(~/src/github.com/juju/juju/cmd/jujud/agent) % pt -i race [01:02] package_test.go: [01:02] 15: if testing.RaceEnabled { [01:02] 16: t.Skip("skipping package under -race, see LP 1519133, 1519097") [01:02] lol [01:02] nice [01:02] bug 1519133 [01:02] Bug #1519133: cmd/jujud/agent: data race <2.0-count> [01:03] bug 1519097 [01:03] Bug #1519097: juju/utils/fslock: data race caused by createAliveFile running twice <2.0-count>

[01:03] OH MY FUCK [01:03] this god damn fslock nonsense [01:03] what, it's fix released [01:03] no more race [01:03] let's remove that check and see what breaks [01:05] well, the fslock one we are working on [01:10] Bug #1585424 opened: all: data race's in tests are surpressed [01:11] thumper: https://bugs.launchpad.net/juju-core/+bug/1585424 [01:11] please see email [01:11] Bug #1585424: all: data race's in tests are surpressed [01:11] and let me know your decision [01:19] Bug #1585424 changed: all: data race's in tests are surpressed [01:20] bug 1519095 [01:20] Bug #1519095: state: tests to not pass with -race under Go 1.2 <2.0-count> [01:20] oh wow [01:21] welp that's what happens when a bug isn't a blocker [01:21] it goes straight to the bottom of the pool [01:31] Bug #1585424 opened: all: data races in tests are surpressed [01:36] davechen1y: IIRC the idea was that at least we wouldn't get any *new* data races in core if we're running the race detector. The problem being that we never went back and actually fixed the ones that got skipped [01:38] yeah, that's what I remembmer now [01:38] i remember it was an uphill battle to stop new races being added [01:38] it's good to know that we can prdict the future [01:40] http://reviews.vapour.ws/r/3204/ [01:40] :( [01:41] *sad trombone* [01:42] * davechen1y considers replacing paul the octopus as a fortune teller [02:10] Bug #1585430 opened: Cloud-init failed on windows

[02:19] if you thought the state tests were slow without -race, just wait til you see them with -race [02:19] heh yeah, -race and -cover are always interesting additions [02:33] thumper: cherylj https://github.com/juju/juju/pull/5452 [02:33] please read the description closely [02:33] I think our timeout for race is like 60 minutes [02:33] * thumper double checks [02:34] that's probably _just_ enough [02:36] 30 minutes [02:36] go test -race -test.timeout=1800s ./... [02:36] it'll be tight [02:36] this passed in 1500s on my machine [02:37] without any other tests running [02:37] it'll probably be ok [02:37] at least we know which change to revert [02:37] right [02:37] I say push it in [02:39] jolly good [03:11] Bug #1583893 changed: 1.25.5: goroutine panic launching container on xenial

[03:50] Bug #1583893 opened: 1.25.5: goroutine panic launching container on xenial

[03:59] cherylj: anastasiamac https://bugs.launchpad.net/juju-core/+bug/1518806 [03:59] Bug #1518806: apiserver: tests to not pass with -race under Go 1.2 <2.0-count> [03:59] i think there is a still a race here [03:59] what testing did you do and why do you think the race is not present ? [04:00] anastasiamac: cherylj for reference the race I see is https://bugs.launchpad.net/juju-core/+bug/1519133/comments/2 [04:00] Bug #1519133: cmd/jujud/agent: data race <2.0-count> [04:00] davechen1y: I ran the race test locally [04:00] specifically for apiserver/ [04:02] davechen1y: did u ran only apiserver package with race detector (in isolation) or started at the root, i.e. from top of juju/juju? [04:02] anastasiamac: did you remove the skip ? [04:02] that package knows when it's run under the race detector and skips all the tests [04:02] it does log it [04:02] davechen1y: i have not run race detector here as I have not been in this package yet [04:02] but the log is not printed anywhere [04:02] :( [04:03] righto, i guess it's not fixed then [04:03] if u r seen it, then it does not sound fixed ;-P [04:03] it could have also been re-introduced?.. [04:03] I ran the test locally removing the skip and didn't see a race in apiserver/, but that doesn't mean that there isn't one [04:05] at some stage, we should really unskip tests that r related to bugs that are considered fixed... [04:18] cherylj: i think it's not well exercised by the race [04:18] the tests in cmd/jujud/agent agetate the cert updater so that's why it shows up there [04:18] but ht race is definitely in the apiserver code [04:19] i'll try to write a test that agrevates the problem [04:22] davechen1y: when you said use unix domain sockets and net.Dial(....) did you mean net.listen? can you dial a domain socket without anyone listening? [04:25] yes, i'm sorry, i meand net.ListenUnix [04:33] EOD++ for me. night all. [04:37] axw: wallyworld: this is permissions check that i was talking about at standup - http://reviews.vapour.ws/r/4895/ [04:38] ok, taking a look [04:39] anastasiamac: but [04:39] // password, so that we don't allow unauthenticated users to find [04:39] // information about existing entities. [04:39] that information hiding is now lost [04:40] it's like when you enter user/pass into a banking website [04:40] it just says bad credentials or something [04:40] it doesn't tell you what was invalid [04:40] it's not lost, we return permission denied without saying that entity does not exist [04:41] when u go to the bank website, they say the same thing [04:41] right, and then when the correct entity is passed, we then return BadCreds right? [04:41] so 2 different errors depending if the user is valid or not [04:42] we have a reaction to bad credentials error - we restart stuff... so we need to ensure that we only return bad credentials when we intent to resat agent, for eg.. [04:42] yes, if user is valid but does not have permission - deny acces [04:42] but the error code becomes different so we leak info [04:42] if user/pwd is not valid, bad credentials - do whatever needs to be done like restart... [04:43] we do not leak info - we do not say 'not found" [04:43] the point is - we need to return the same error if user/pass together are not valid, regardless of cause [04:43] we say - u have no permissions to do what u r trying to do [04:43] you can infer the not found [04:44] by looking at the return error [04:45] if we return bad credentials error, for example, we terminate api... we do not want to that when the login entity has valid creds but not permissions [04:47] sure, that's an upstream problem though [04:47] we cannot solve that issue by leaking information [04:47] ? [04:47] the comment that was deleted in the PR explained why it was done that way [04:47] this is the reason we keep restarting agents [04:48] permission error doe s not imply something is not found... [04:48] it does in this context as it is different to what's returned when the entity is found [04:49] the question to ask is - why is the api being called with a bad user? [04:49] isn't that the root cause issue? [04:51] the root cause is in several bug, the prime one is refered to in PR. [04:51] essentially, we (juju) reacts differently to bad creds [04:51] so we need to be careful about when we throw it [04:52] sure, but we can't fix the issue by leaking information [04:52] otherwsie, we;'ll have a reaction that is too drastic, is uncalled for and is not helping [04:52] \o/ i have difficulty seeing info leak [04:53] if i'm a hacker, i throw different user/pass at the api. if i get an errperm vs an badcreds, i can deduce if the user exists or not [04:53] that's an information leak [04:53] once i know a user exists vs not exists, the cracking problem becomes much easier [04:53] i now just need to guess the passowrd [04:54] sure, but if "entity" is not found, u really do not want to restart everything either [04:54] correct [04:54] so we need a different solution [04:54] that doesn't leak info [04:54] wel, we need an error that is not bad creds (coz we have a reaction to that) [04:54] the only other one that we have is PermErr [04:54] we need to understand the upstream root cause [04:54] whcih is actually what we are after - permission is denied \o/ [04:54] why is the caller passing a bad entity name? [04:57] this func is not just used by users, if u look for dependent calls [04:58] also i think that this conversation should be taken off-line and have Will and Andrew involved :D [04:58] m happy to have it anytime and m happy with any solution that involves not to restart for every failure [05:01] sure, we can discuss. but it's an information leak, and there was a comment that explained why. if it is decided we don't care about the information leak, then fine. but it's risky to leak information without a proper discussion [05:03] true information leask is a concern - m not convinced this si the case here.... another concern is the restart that is caused by bad creds... solution needs to ensure that neither happen :D [05:04] disclosure of whether an user protected by a password exists or not is an attack vector [05:04] the information is whether the user exists and it is leaked [05:05] if different errors are returned. it's why banks for example always just return a generic "invalid user password combination" error [05:05] they don't say *which* of the user or password is wrong [05:10] in this instance, find entity determines if the call is made by machine/unit/user/service/etc... r u saying that if an unknown to us machine is making a call, it is the correct call for us to restart the api? :D [05:17] anastasiamac wallyworld: what has the server's behaviour got to do with the client's behaviour around restarting the API? that's a client policy. you should not be leaking information (yes there *is* leakage) from the server to control the client's behaviour [05:18] +1 [05:18] that's my point [05:18] anastasiamac: like wallyworld says, if you can infer that the username is valid because "not found", then you're leaking info, and htat's a security concern [05:18] that reduces the cost of a brute foce attack to focusing mostly on the password [05:18] force* [05:19] yep [05:19] sure and i agree [05:19] but throwing bad creds here if the entity is not a user, is what is causing issues [05:20] anastasiamac: so the issue is related to the unhandled tag kind? [05:24] axw: the issue is thatwe use given tag to find entity. if entity is not found we throw bad creds which we react to.. i think we should throw something else here [05:24] m happy to revert this badcreds instance for now and only keep the other instances (that are more cleanly defined) in this PR [05:25] axw: this fix is surprisingly simple, can I get a sanity check https://github.com/juju/juju/pull/5455 [05:26] thnks! [05:31] anastasiamac: I don't understand how can say "sure and i agree" and then what you just said. either we return ErrBadCreds, or we leak information [05:31] davechen1y: looking [05:34] davechen1y: LGTM [05:36] axw: I agree about obfuscating and not leaking info :D i am not convinced that badcreds is always the right answer here.. maybe only for user tags...either way, i'll come back to this instance later but will adress other comments on PR after school/kids pick-up [05:43] who has the power to unblock a message to juju-dev ? [06:07] axw: "application-wordpress" or "app-wordpress" ? [06:07] wallyworld: "app-" please [06:07] ok, i kinda like application- :-) [06:08] app- is ok i guess [06:08] wallyworld: feel free to get a second opinion, I just think application- is tedious [06:08] yeah it is, but app- sorta grates for some reason. maybe just me [06:25] axw: if you have a moment, first of a few sigh http://reviews.vapour.ws/r/4897/ [06:27] wallyworld: hrm, app- looks grating to me too now. maybe bring it up in the meeting later [06:27] axw: i do think application works better [06:28] it's not much longer than service [06:28] i have a charm.v6 change queued up, so i'll change to application- [06:28] wallyworld: did you intend to change the package paths to gopkg.in? [06:28] yeah [06:28] so we can rev up the version [06:29] to names.v2 [06:29] right, ok [06:29] so we don't buuger up 1.25 etc [06:29] there'll be a lot of churn in all this sadly [06:29] Bug #1584059 changed: Deployment of swift-storage charms fails with Juju 2.0 - swift-storage-relation-joined KeyError: 'JUJU_ENV_UUID'

[06:49] axw: next one https://github.com/juju/charm/pull/210 [06:49] gotta duck out for school pickup, will look at yuors in a bit [07:22] axw: the yaml thing - we had the relations data unmarshalled with the v2 Unmarshall, and I think the service data with GetYaml() from v1, it was a bit of a mess [07:23] i had a quick look at the v2 release notes and didn't see anything that jumped out [07:23] multi-line strings have been improved, but i don't think that affects charms [07:24] wallyworld: non capisco. is anything feeding charm metadata into yaml.v1? not sure which things use it TBH. charmstore possibly? new charm tool? [07:25] no argument that it's a mess, just not sure if we can fix it without breaking others [07:25] given we had a mix of v1 and v2 before, it seems unlikely we'd be relying on v2 specific behaviour [07:26] the charm metadata is fairly simple yaml i guess? [07:26] nothing too esoteric [07:27] wallyworld: my point is that (AFAIK) the MarshalYAML methods won't be called if the structs are passed to yaml.v1 [07:27] wallyworld: so the custom behaviour that was in GetYAML won't be triggered [07:27] yeah, that's correct [07:28] i can't fathom why we'd need both [07:28] wallyworld: only because some clients might not have been updated [07:28] now is the time ot make this change i guess [07:29] it is charm.v6-unstable, clients out there would be using .v5 [07:29] ok [07:29] that's my self justification [07:29] if i say it often enough it makes sense [07:30] wallyworld: :) [07:30] wallyworld: seems fair, I'm just not sure what the impact is [07:30] yeah, i'm not 1000% sure [07:31] we can fix anything that comes up easily enough [07:31] wallyworld: yep ok, consider the question dropped [07:31] ok [07:33] wallyworld, in hangout [07:39] anyone know what the deal with br-bond1 is? we have 2 bonds on these machines, and juju creates br-bondx for both of them, but only one of them is put in the interfaces file [07:39] this is with juju-2 [07:57] bradm: dimitern or frobware would know [07:58] bradm: can you PB the original /e/n/i file and Juju's morphed version. [07:59] frobware: thats the point, there is no details about br-bond1 in /e/n/i [07:59] bradm: MAAS? [07:59] frobware: yup [07:59] frobware: in this case, maas 2.0 with juju 2.0 [08:00] bradm: ah. haven't really tried MAAS 2.0 that hard. hmm. [08:00] bradm: frobware: maybe voidspace then [08:00] I have to run to take the boy to cubs, will check back in later [08:01] bradm: but there should be the original /e/n/i available [08:01] frobware, bradm: hey, what's the issue? [08:01] frobware: ayup, there is - it adds br-bond0 to /e/n/i, but not br-bond1 [08:01] bradm: it has the extension -before-juju-add-bridge.py [08:02] got to run, didn't realise the time [08:02] bradm: ack [08:02] perhaps is br-bond1 unconfigured and has no address, but it has configured 'children'? === Guest6887 is now known as BrunoR [09:02] can someone with mod rights on juju-dev please let my message out of moderation [09:02] ta! === urulama is now known as urulama|swap [10:29] anyone know anything about the reasoning behind https://github.com/juju/juju/pull/5100 ? [10:29] for example, was it deliberate to remove "juju help plugins" ? [10:30] fwereade, davechen1y, axw: ^ [10:31] frobware, dimitern: yes, our bond1 is unconfigured, we use it for neutron-gateway lxcs [10:32] bradm: so juju should not create a bridge for it, but it should still be present in /e/n/i [10:32] bradm: so you have bond1 unconfigured but up and e.g. bond1.42 also unconfigured and up? [10:33] frobware: right, that would do us - we create a bridge ourselves in the preseed [10:33] oh.. [10:33] dimitern: in maas we create teh bond and leave it unconfigured, we add it to the lxcs as an extra interface, so we do need a bridge for it [10:34] which is fine if you guys do it, we can use whichever bond is there [10:34] er, bridge [10:34] just having 2 bridges on the interface doesn't work so well :) [10:34] bradm: well, as long as the bridge you create in the preseed is called 'br-bond1' and e.g. 'br-bond1.42' for the neutron-ext vlan, the bridge script shouldn't mess with it [10:35] bradm: can you PB the original /e/n/i file - it should be /etc/network/interfaces-before-juju-add-bridge.py [10:36] bradm: are there any VLANs configured on the bond? [10:37] frobware: https://pastebin.canonical.com/157211/ - nope, very plain [10:37] I can try creating it as br-bond1 tomorrow to see if that fixes it [10:38] bradm: your dns-nameserver entry (x.y.z.5) - is that right? [10:38] yeah, I just hid our ip range, out of habit [10:39] frobware: thats the IP of the maas node. [10:39] bradm: ok, just checking... :) [10:40] bradm: so I don't see juju messing with bond1 at all [10:40] trying to do as much as we can in maas, and not hacking up preseeds for it [10:40] * frobware tries to recall what the issue was/is ... [10:40] frobware: right, yet there's a br-bond1 up [10:40] frobware: there's commands in the cloud-init-output.log that configure it [10:41] frobware: https://pastebin.canonical.com/157212/ [10:41] bradm: so juju will create br-bond0 based on your original eni - http://pastebin.ubuntu.com/16676634/ [10:42] actually, there's a br-ethx for all of the eth devices too [10:43] bradm: ah... let me check something... [10:43] but only br-bond0 gets written out to /e/n/i [10:43] maybe because none of the others have IPs? dunno [10:44] bradm: br-bond0 get written to eni because its parent device is 'static' [10:44] bradm: bond1 is manual. [10:44] bradm: so does not get bridged [10:44] frobware: ah, but it does! it creates a br-bond1 [10:44] frobware: its just not in /e/n/i [10:45] bradm: right. that's my "aha" - just checking something... [10:45] frobware: like I said, all the interfaces have a bridge [10:45] well, except for lo [10:45] bradm: I would say, based on the input, the /e/n/i as written looks correct, just checking on something that has changed in the bridge script quite recently. [10:46] frobware: yup, /e/n/i is what we want. its all the extra interfaces that are up we don't want :) [10:46] bradm: agreed. state of interfaces should reflect what's in /e/n/i [10:47] frobware: https://pastebin.canonical.com/157215/ <- those are the extra interfaces [10:49] we just create our br1 in the maas preseed, which is fine [10:50] dooferlad: ^^ [10:50] I can see in the cloud-init-output log where it does it all [10:50] dooferlad: I have a sneaky suspicion your recent changes to the bridge script may be bringing interfaces up where it shouldn't... [10:51] honestly, either way works - if it wants to create a br-bond1 because the interface is there, thats fine, we can use it [10:51] just need it in /e/n/i [10:51] bradm: plz clarify - r u using juju2.beta7 or master tip? [10:52] anastasiamac: juju2 beta 7 with maas 2.0 beta 5 [10:52] frobware: that is horrid. Yea, maybe that is happening. [10:52] dooferlad: bridge_now() should consider is_active - we are creating bridges where we should not. [10:52] dooferlad: see https://pastebin.canonical.com/157215/ [10:53] bradm: this is a recent regression [10:53] bradm: I'm guessing that if you were to reboot the machine it would come up as you expect [10:53] frobware: ayup [10:54] frobware: well, except the lxcs are cranky because they couldn't access anything in our public IP range, so neutron is a little upset [10:55] but, cool, happy to see its not me totally misunderstanding something about the new stuff, these things happen. [10:55] bradm: I think we just need to get a fix and a binary to you. Would that work? [10:56] frobware: sure, thats fine, this is for an internal stack right now [10:56] frobware: and I'm not at work for another 12 hours anyway. :) [10:56] bradm: you happy with some unofficial build from some bloke off the net? [10:56] frobware: ideally we want it fixed in upstream :) [10:57] bradm: of, of course. [10:57] bradm: I was merely trying to find a way to unblock you. [11:05] dooferlad, bradm: https://bugs.launchpad.net/juju-core/+bug/1585582 [11:05] Bug #1585582: MAAS bridge script bridges inactive interfaces

[11:06] Bug #1585582 opened: MAAS bridge script bridges inactive interfaces