/srv/irclogs.ubuntu.com/2016/05/25/#juju-dev.txt

davecheneynatefinch: see tim's email thread from last year about the proposed design00:00
davecheneysorry i cannot link to it00:00
davecheneyi dunno how to link to messages on that ML00:01
natefinchdavecheney: search for diaf is surprisingly useful00:01
davecheney:)00:01
mupBug #1582841 changed: remove check for lxdbr0 <juju-core:Invalid> <https://launchpad.net/bugs/1582841>00:04
natefinchdavecheney: 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:08
natefincher lock is released when the file descriptor is closed, which happens automatically on exit00:09
natefinchdavecheney: if you just want to avoid futzing with files on disk, that's valid, too00:10
rediranastasiamac: FWIW I only found these 3 https://goo.gl/JG8NXL none of which would have been fixed by the updates to aggregator AFAICT.00:11
davecheneynatefinch: please, no flock, use unix domain sockets as we discussed00:25
natefinchdavecheney: ok00:25
davecheneyflock requires a file on disk00:25
davecheneyif I delete that file I can effeictly release that lock00:25
davecheneyand we're back to square one00:25
perrito666can you do that in windows? (without a signifficant effort?)00:26
natefinchnope00:26
natefinchwindows is a real lock00:26
natefinchI know flock is only advisdory00:26
davecheneyit's not that flock is advisory00:27
davecheneybut the thing that is being locked can be moved or deleted which breaks the lock invariant00:27
davecheneyflock also leaves with the 'oh i crashed and there is a file on disk' problem00:29
natefinchman 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 trivially00:29
natefinchthe file on disk doesn't matter00:29
davecheneyyes it does00:30
davecheneyyou need to have a file to apply a lock to00:30
natefinchthe existence of the file has no meaning00:30
davecheneyyou cannot flock a non existant file00:30
davecheneyso you have to create that file00:30
natefinchwell, yes00:30
davecheneyso now you have two lockers racing to create the file00:30
davecheneyif you fail to create the file because someone else was racing with does taht mean you cannot lock it, etc00:30
perrito666k people, EOD, see you all on thu00:30
davecheneyplease just use unix domain sockets00:30
davecheneythey solve 100% of these problems00:31
natefinchit's really not a race, but that's fine00:31
davecheneynet.Dial("unix", "@/juju/$SOMEHASH")00:31
natefinchI open with O_create... one proc will create first, the other will open00:32
davecheneygood point00:33
davecheneybut please00:33
davecheneyno files on disk00:33
natefinchI 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 windows00:34
bradmfwiw I seem to be hitting bug 1537585 a lot more with juju 2 and maas 200:38
mupBug #1537585: machine agent failed to register IP addresses, borks agent <2.0-count> <blocker> <landscape> <network> <juju-core:Triaged> <juju-core 1.25:Triaged> <https://launchpad.net/bugs/1537585>00:38
=== redir is now known as redir_afk
davechen1ythumper: what tha ?00:59
davechen1ylucky(~/src/github.com/juju/juju/cmd/jujud/agent) % go test -i -v . && go test .00:59
davechen1yok      github.com/juju/juju/cmd/jujud/agent    122.841s00:59
davechen1ylucky(~/src/github.com/juju/juju/cmd/jujud/agent) % go test -race -i -v . && go test -race .01:00
davechen1yok      github.com/juju/juju/cmd/jujud/agent    1.168s01:00
davechen1y^ this test runs 100x faster under the race detector ...01:00
natefinchlol, we should use the race detector during production, evidently01:00
davechen1yuh oh01:02
davechen1yi have a terrible feeling about this01:02
natefinchI have a great feeling that this is going to be an awesome bug01:02
davechen1ylucky(~/src/github.com/juju/juju/cmd/jujud/agent) % pt -i race01:02
davechen1ypackage_test.go:01:02
davechen1y15:     if testing.RaceEnabled {01:02
davechen1y16:             t.Skip("skipping package under -race, see LP 1519133, 1519097")01:02
natefinchlol01:02
natefinchnice01:02
natefinchbug 151913301:02
mupBug #1519133: cmd/jujud/agent: data race <2.0-count> <juju-core:Triaged> <https://launchpad.net/bugs/1519133>01:02
natefinchbug 151909701:03
mupBug #1519097: juju/utils/fslock: data race caused by createAliveFile running twice <2.0-count> <race-condition> <tech-debt> <juju-core:Fix Released by dooferlad> <https://launchpad.net/bugs/1519097>01:03
davechen1yOH MY FUCK01:03
davechen1ythis god damn fslock nonsense01:03
natefinchwhat, it's fix released01:03
natefinchno more race01:03
davechen1ylet's remove that check and see what breaks01:03
natefinchwell, the fslock one we are working on01:05
mupBug #1585424 opened: all: data race's in tests are surpressed <juju-core:New> <https://launchpad.net/bugs/1585424>01:10
davechen1ythumper: https://bugs.launchpad.net/juju-core/+bug/158542401:11
davechen1yplease see email01:11
mupBug #1585424: all: data race's in tests are surpressed <juju-core:New> <https://launchpad.net/bugs/1585424>01:11
davechen1yand let me know your decision01:11
mupBug #1585424 changed: all: data race's in tests are surpressed <juju-core:New> <https://launchpad.net/bugs/1585424>01:19
davechen1ybug 151909501:20
mupBug #1519095: state: tests to not pass with -race under Go 1.2 <2.0-count> <juju-core:Triaged> <https://launchpad.net/bugs/1519095>01:20
davechen1yoh wow01:20
davechen1ywelp that's what happens when a bug isn't a blocker01:21
davechen1yit goes straight to the bottom of the pool01:21
mupBug #1585424 opened: all: data races in tests are surpressed <juju-core:New> <https://launchpad.net/bugs/1585424>01:31
natefinchdavechen1y: 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 skipped01:36
davechen1yyeah, that's what I remembmer now01:38
davechen1yi remember it was an uphill battle to stop new races being added01:38
davechen1yit's good to know that we can prdict the future01:38
davechen1yhttp://reviews.vapour.ws/r/3204/01:40
davechen1y:(01:40
natefinch*sad trombone*01:41
* davechen1y considers replacing paul the octopus as a fortune teller01:42
mupBug #1585430 opened: Cloud-init failed on windows <ci> <cloud-init> <regression> <windows> <juju-core:Triaged> <https://launchpad.net/bugs/1585430>02:10
davechen1yif you thought the state tests were slow without -race, just wait til you see them with -race02:19
natefinchheh yeah, -race and -cover are always interesting additions02:19
davechen1ythumper: cherylj https://github.com/juju/juju/pull/545202:33
davechen1yplease read the description closely02:33
thumperI think our timeout for race is like 60 minutes02:33
* thumper double checks02:33
davechen1ythat's probably _just_ enough02:34
thumper30 minutes02:36
thumpergo test -race -test.timeout=1800s ./...02:36
davechen1yit'll be tight02:36
davechen1ythis passed in 1500s on my machine02:36
davechen1ywithout any other tests running02:37
davechen1yit'll probably be ok02:37
davechen1yat least we know which change to revert02:37
thumperright02:37
thumperI say push it in02:37
davechen1yjolly good02:39
mupBug #1583893 changed: 1.25.5: goroutine panic launching container on xenial <landscape> <juju-core:Fix Released> <juju-core 1.25:Triaged> <https://launchpad.net/bugs/1583893>03:11
mupBug #1583893 opened: 1.25.5: goroutine panic launching container on xenial <landscape> <juju-core:Triaged> <juju-core 1.25:Triaged> <https://launchpad.net/bugs/1583893>03:50
davechen1ycherylj: anastasiamac https://bugs.launchpad.net/juju-core/+bug/151880603:59
mupBug #1518806: apiserver: tests to not pass with -race under Go 1.2 <2.0-count> <juju-core:Triaged by dave-cheney> <https://launchpad.net/bugs/1518806>03:59
davechen1yi think there is a still a race here03:59
davechen1ywhat testing did you do and why do you think the race is not present ?03:59
davechen1yanastasiamac: cherylj for reference the race I see is https://bugs.launchpad.net/juju-core/+bug/1519133/comments/204:00
mupBug #1519133: cmd/jujud/agent: data race <2.0-count> <juju-core:Triaged by dave-cheney> <https://launchpad.net/bugs/1519133>04:00
cheryljdavechen1y: I ran the race test locally04:00
cheryljspecifically for apiserver/04:00
anastasiamacdavechen1y: 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
davechen1yanastasiamac: did you remove the skip ?04:02
davechen1ythat package knows when it's run under the race detector and skips all the tests04:02
davechen1yit does log it04:02
anastasiamacdavechen1y: i have not run race detector here as I have not been in this package yet04:02
davechen1ybut the log is not printed anywhere04:02
davechen1y:(04:02
davechen1yrighto, i guess it's not fixed then04:03
anastasiamacif u r seen it, then it does not sound fixed ;-P04:03
anastasiamacit could have also been re-introduced?..04:03
cheryljI ran the test locally removing the skip and didn't see a race in apiserver/, but that doesn't mean that there isn't one04:03
anastasiamacat some stage, we should really unskip tests that r related to bugs that are considered fixed...04:05
davechen1ycherylj: i think it's not well exercised by the race04:18
davechen1ythe tests in cmd/jujud/agent agetate the cert updater so that's why it shows up there04:18
davechen1ybut ht race is definitely in the apiserver code04:18
davechen1yi'll try to write a test that agrevates the problem04:19
natefinchdavechen1y: 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:22
davechen1yyes, i'm sorry, i meand net.ListenUnix04:25
natefinchEOD++ for me.  night all.04:33
anastasiamacaxw: wallyworld: this is permissions check that i was talking about at standup - http://reviews.vapour.ws/r/4895/04:37
axwok, taking a look04:38
wallyworldanastasiamac: but04:39
wallyworld// password, so that we don't allow unauthenticated users to find04:39
wallyworld// information about existing entities.04:39
wallyworldthat information hiding is now lost04:39
wallyworldit's like when you enter user/pass into a banking website04:40
wallyworldit just says bad credentials or something04:40
wallyworldit doesn't tell you what was invalid04:40
anastasiamacit's not lost, we return permission denied without saying that entity does not exist04:40
anastasiamacwhen u go to the bank website, they say the same thing04:41
wallyworldright, and then when the correct entity is passed, we then return BadCreds right?04:41
wallyworldso 2 different errors depending if the user is valid or not04:41
anastasiamacwe 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
anastasiamacyes, if user is valid but does not have permission - deny acces04:42
wallyworldbut the error code becomes different so we leak info04:42
anastasiamacif user/pwd is not valid, bad credentials - do whatever needs to be done like restart...04:42
anastasiamacwe do not leak info - we do not say 'not found"04:43
wallyworldthe point is - we need to return the same error if user/pass together are not valid, regardless of cause04:43
anastasiamacwe say - u have no permissions to do what u r trying to do04:43
wallyworldyou can infer the not found04:43
wallyworldby looking at the return error04:44
anastasiamacif 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 permissions04:45
wallyworldsure, that's an upstream problem though04:47
wallyworldwe cannot solve that issue by leaking information04:47
anastasiamac?04:47
wallyworldthe comment that was deleted in the PR explained why it was done that way04:47
anastasiamacthis is the reason we keep restarting agents04:47
anastasiamacpermission error doe s not imply something is not found...04:48
wallyworldit does in this context as it is different to what's returned when the entity is found04:48
wallyworldthe question to ask is - why is the api being called with a bad user?04:49
wallyworldisn't that the root cause issue?04:49
anastasiamacthe root cause is in several bug, the prime one is refered to in PR.04:51
anastasiamacessentially, we (juju) reacts differently to bad creds04:51
anastasiamacso we  need to be careful about when we throw it04:51
wallyworldsure, but we can't fix the issue by leaking information04:52
anastasiamacotherwsie, we;'ll have a reaction that is too drastic, is uncalled for and is not helping04:52
anastasiamac\o/ i have difficulty seeing info leak04:52
wallyworldif 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 not04:53
wallyworldthat's an information leak04:53
wallyworldonce i know a user exists vs not exists, the cracking problem becomes much easier04:53
wallyworldi now just need to guess the passowrd04:53
anastasiamacsure, but if "entity" is not found, u really do not want to restart everything either04:54
wallyworldcorrect04:54
wallyworldso we need a different solution04:54
wallyworldthat doesn't leak info04:54
anastasiamacwel, we need an error that is not bad creds (coz we have a reaction to that)04:54
anastasiamacthe only other one that we have is PermErr04:54
wallyworldwe need to understand the upstream root cause04:54
anastasiamacwhcih is actually what we are after - permission is denied \o/04:54
wallyworldwhy is the caller passing a bad entity name?04:54
anastasiamacthis func is not just used by users, if u look for dependent calls04:57
anastasiamacalso i think that this conversation should be taken off-line and have Will and Andrew involved :D04:58
anastasiamacm happy to have it anytime and m happy with any solution that involves not to restart for every failure04:58
wallyworldsure, 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 discussion05:01
anastasiamactrue 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 :D05:03
wallyworlddisclosure of whether an user protected by a password exists or not is an attack vector05:04
wallyworldthe information is whether the user exists and it is leaked05:04
wallyworldif different errors are returned. it's why banks for example always just return a generic "invalid user password combination" error05:05
wallyworldthey don't say *which* of the user or password is wrong05:05
anastasiamacin 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? :D05:10
axwanastasiamac 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 behaviour05:17
wallyworld+105:18
wallyworldthat's my point05:18
axwanastasiamac: 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 concern05:18
axwthat reduces the cost of a brute foce attack to focusing mostly on the password05:18
axwforce*05:18
wallyworldyep05:19
anastasiamacsure and i agree05:19
anastasiamacbut throwing bad creds here if the entity is not a user, is what is causing issues05:19
axwanastasiamac: so the issue is related to the unhandled tag kind?05:20
anastasiamacaxw: 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 here05:24
anastasiamacm happy to revert this badcreds instance for now and only keep the other instances (that are more cleanly defined) in this PR05:24
davechen1yaxw: this fix is surprisingly simple, can I get a sanity check https://github.com/juju/juju/pull/545505:25
davechen1ythnks!05:26
axwanastasiamac: I don't understand how can say "sure and i agree" and then what you just said. either we return ErrBadCreds, or we leak information05:31
axwdavechen1y: looking05:31
axwdavechen1y: LGTM05:34
anastasiamacaxw: 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-up05:36
davechen1ywho has the power to unblock a message to juju-dev ?05:43
wallyworldaxw: "application-wordpress" or "app-wordpress" ?06:07
axwwallyworld: "app-" please06:07
wallyworldok, i kinda like application- :-)06:07
wallyworldapp- is ok i guess06:08
axwwallyworld: feel free to get a second opinion, I just think application- is tedious06:08
wallyworldyeah it is, but app- sorta grates for some reason. maybe just me06:08
wallyworldaxw: if you have a moment, first of a few sigh http://reviews.vapour.ws/r/4897/06:25
axwwallyworld: hrm, app- looks grating to me too now. maybe bring it up in the meeting later06:27
wallyworldaxw: i do think application works better06:27
wallyworldit's not much longer than service06:28
wallyworldi have a charm.v6 change queued up, so i'll change to application-06:28
axwwallyworld: did you intend to change the package paths to gopkg.in?06:28
wallyworldyeah06:28
wallyworldso we can rev up the version06:28
wallyworldto names.v206:29
axwright, ok06:29
wallyworldso we don't buuger up 1.25 etc06:29
wallyworldthere'll be a lot of churn in all this sadly06:29
mupBug #1584059 changed: Deployment of swift-storage charms fails with Juju 2.0 - swift-storage-relation-joined KeyError: 'JUJU_ENV_UUID' <oil> <juju-core:Triaged> <swift-storage (Juju Charms Collection):New> <https://launchpad.net/bugs/1584059>06:29
wallyworldaxw: next one https://github.com/juju/charm/pull/21006:49
wallyworldgotta duck out for school pickup, will look at yuors in a bit06:49
wallyworldaxw: 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 mess07:22
wallyworldi had a quick look at the v2 release notes and didn't see anything that jumped out07:23
wallyworldmulti-line strings have been improved, but i don't think that affects charms07:23
axwwallyworld: non capisco. is anything feeding charm metadata into yaml.v1? not sure which things use it TBH. charmstore possibly? new charm tool?07:24
axwno argument that it's a mess, just not sure if we can fix it without breaking others07:25
wallyworldgiven we had a mix of v1 and v2 before, it seems unlikely we'd be relying on v2 specific behaviour07:25
wallyworldthe charm metadata is fairly simple yaml i guess?07:26
wallyworldnothing too esoteric07:26
axwwallyworld: my point is that (AFAIK) the MarshalYAML methods won't be called if the structs are passed to yaml.v107:27
axwwallyworld: so the custom behaviour that was in GetYAML won't be triggered07:27
wallyworldyeah, that's correct07:27
wallyworldi can't fathom why we'd need both07:28
axwwallyworld: only because some clients might not have been updated07:28
wallyworldnow is the time ot make this change i guess07:28
wallyworldit is charm.v6-unstable, clients out there would be using .v507:29
axwok07:29
wallyworldthat's my self justification07:29
wallyworldif i say it often enough it makes sense07:29
axwwallyworld: :)07:30
axwwallyworld: seems fair, I'm just not sure what the impact is07:30
wallyworldyeah, i'm not 1000% sure07:30
wallyworldwe can fix anything that comes up easily enough07:31
axwwallyworld: yep ok, consider the question dropped07:31
wallyworldok07:31
fwereadewallyworld, in hangout07:33
bradmanyone 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 file07:39
bradmthis is with juju-207:39
wallyworldbradm: dimitern or frobware would know07:57
frobwarebradm: can you PB the original /e/n/i file and Juju's morphed version.07:58
bradmfrobware: thats the point, there is no details about br-bond1 in /e/n/i07:59
frobwarebradm: MAAS?07:59
bradmfrobware: yup07:59
bradmfrobware: in this case, maas 2.0 with juju 2.007:59
frobwarebradm: ah. haven't really tried MAAS 2.0 that hard. hmm.08:00
wallyworldbradm: frobware: maybe voidspace then08:00
bradmI have to run to take the boy to cubs, will check back in later08:00
frobwarebradm: but there should be the original /e/n/i available08:01
dimiternfrobware, bradm: hey, what's the issue?08:01
bradmfrobware: ayup, there is - it adds br-bond0 to /e/n/i, but not br-bond108:01
frobwarebradm: it has the extension -before-juju-add-bridge.py08:01
bradmgot to run, didn't realise the time08:02
frobwarebradm: ack08:02
dimiternperhaps is br-bond1 unconfigured and has no address, but it has configured 'children'?08:02
=== Guest6887 is now known as BrunoR
davechen1ycan someone with mod rights on juju-dev please let my message out of moderation09:02
davechen1yta!09:02
=== urulama is now known as urulama|swap
rogpeppeanyone know anything about the reasoning behind https://github.com/juju/juju/pull/5100 ?10:29
rogpeppefor example, was it deliberate to remove "juju help plugins" ?10:29
rogpeppefwereade, davechen1y, axw: ^10:30
bradmfrobware, dimitern: yes, our bond1 is unconfigured, we use it for neutron-gateway lxcs10:31
frobwarebradm: so juju should not create a bridge for it, but it should still be present in /e/n/i10:32
dimiternbradm: so you have bond1 unconfigured but up and e.g. bond1.42 also unconfigured and up?10:32
bradmfrobware: right, that would do us - we create a bridge ourselves in the preseed10:33
dimiternoh..10:33
bradmdimitern: 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 it10:33
bradmwhich is fine if you guys do it, we can use whichever bond is there10:34
bradmer, bridge10:34
bradmjust having 2 bridges on the interface doesn't work so well :)10:34
dimiternbradm: 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 it10:34
frobwarebradm: can you PB the original /e/n/i file - it should be /etc/network/interfaces-before-juju-add-bridge.py10:35
frobwarebradm: are there any VLANs configured on the bond?10:36
bradmfrobware: https://pastebin.canonical.com/157211/ - nope, very plain10:37
bradmI can try creating it as br-bond1 tomorrow to see if that fixes it10:37
frobwarebradm: your dns-nameserver entry (x.y.z.5) - is that right?10:38
bradmyeah, I just hid our ip range, out of habit10:38
bradmfrobware: thats the IP of the maas node.10:39
frobwarebradm: ok, just checking... :)10:39
frobwarebradm: so I don't see juju messing with bond1 at all10:40
bradmtrying to do as much as we can in maas, and not hacking up preseeds for it10:40
* frobware tries to recall what the issue was/is ...10:40
bradmfrobware: right, yet there's a br-bond1 up10:40
bradmfrobware: there's commands in the cloud-init-output.log that configure it10:40
bradmfrobware: https://pastebin.canonical.com/157212/10:41
frobwarebradm: so juju will create br-bond0 based on your original eni - http://pastebin.ubuntu.com/16676634/10:41
bradmactually, there's a br-ethx for all of the eth devices too10:42
frobwarebradm: ah... let me check something...10:43
bradmbut only br-bond0 gets written out to /e/n/i10:43
bradmmaybe because none of the others have IPs?  dunno10:43
frobwarebradm: br-bond0 get written to eni because its parent device is 'static'10:44
frobwarebradm: bond1 is manual.10:44
frobwarebradm: so does not get bridged10:44
bradmfrobware: ah, but it does!  it creates a br-bond110:44
bradmfrobware: its just not in /e/n/i10:44
frobwarebradm: right. that's my "aha" - just checking something...10:45
bradmfrobware: like I said, all the interfaces have a bridge10:45
bradmwell, except for lo10:45
frobwarebradm: 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:45
bradmfrobware: yup, /e/n/i is what we want.  its all the extra interfaces that are up we don't want :)10:46
frobwarebradm: agreed. state of interfaces should reflect what's in /e/n/i10:46
bradmfrobware: https://pastebin.canonical.com/157215/ <- those are the extra interfaces10:47
bradmwe just create our br1 in the maas preseed, which is fine10:49
frobwaredooferlad: ^^10:50
bradmI can see in the cloud-init-output log where it does it all10:50
frobwaredooferlad: I have a sneaky suspicion your recent changes to the bridge script may be bringing interfaces up where it shouldn't...10:50
bradmhonestly, either way works - if it wants to create a br-bond1 because the interface is there, thats fine, we can use it10:51
bradmjust need it in /e/n/i10:51
anastasiamacbradm: plz clarify - r u using juju2.beta7 or master tip?10:51
bradmanastasiamac: juju2 beta 7 with maas 2.0 beta 510:52
dooferladfrobware: that is horrid. Yea, maybe that is happening.10:52
frobwaredooferlad: bridge_now() should consider is_active - we are creating bridges where we should not.10:52
frobwaredooferlad: see https://pastebin.canonical.com/157215/10:52
frobwarebradm: this is a recent regression10:53
frobwarebradm: I'm guessing that if you were to reboot the machine it would come up as you expect10:53
bradmfrobware: ayup10:53
bradmfrobware: well, except the lxcs are cranky because they couldn't access anything in our public IP range, so neutron is a little upset10:54
bradmbut, cool, happy to see its not me totally misunderstanding something about the new stuff, these things happen.10:55
frobwarebradm: I think we just need to get a fix and a binary to you. Would that work?10:55
bradmfrobware: sure, thats fine, this is for an internal stack right now10:56
bradmfrobware: and I'm not at work for another 12 hours anyway. :)10:56
frobwarebradm: you happy with some unofficial build from some bloke off the net?10:56
bradmfrobware: ideally we want it fixed in upstream :)10:56
frobwarebradm: of, of course.10:57
frobwarebradm: I was merely trying to find a way to unblock you.10:57
frobware dooferlad, bradm: https://bugs.launchpad.net/juju-core/+bug/158558211:05
mupBug #1585582: MAAS bridge script bridges inactive interfaces <network> <juju-core:New for dooferlad> <https://launchpad.net/bugs/1585582>11:05
mupBug #1585582 opened: MAAS bridge script bridges inactive interfaces <network> <juju-core:New for dooferlad> <https://launchpad.net/bugs/1585582>11:06
bradmfrobware: perfect.11:07
fwereadeericsnow, katco: I can add a payload with ID "xyz" (and class type/name docker/payloadA), but when I list the unit payloads it's reported as a Result with ID "payloadA"11:20
fwereadeericsnow, katco: I presume this is expected behaviour, but I'm not sure what it's for11:21
fwereadeericsnow, katco: can you enlighten me?11:21
voidspacedimitern: ping11:41
dimiternvoidspace: pong11:42
voidspacedimitern: subnets in model migration11:42
voidspacedimitern: would you consider spacename and availabilityzone fields of the subnet to be "optional"?11:43
voidspacedimitern: (i.e. should they be omitempty in the yaml)11:43
voidspacedimitern: what do you think?11:43
dimiternvoidspace: I think we should make best effort to set them, but failing that it's better to have them stored empty (i.e. not omitempty)11:45
voidspacedimitern: ok, cool - thanks11:45
voidspaceno default on the import schema either then11:45
dimiternvoidspace: yeah, I think so11:47
dimiternvoidspace: on a side-note: the subnetDoc should support storing multiple AZs per subnet11:47
voidspacedimitern: yeah, it should11:47
voidspacefwereade: ping12:01
voidspacedimitern: ping12:03
dimiternvoidspace: pong12:03
voidspacedimitern: fwereade: the State.AddSubnet method has a checkModelActive in the transaction12:04
dimiternvoidspace: yeah?12:04
voidspacethis causes adding a subnet during a model migration import to fail because the model is not active12:04
voidspacethe obvious solution is to remove the check...12:04
voidspaceis that ok, or is there a "right way" to do this12:05
dimiternvoidspace: I don't think that's ok12:05
dimiternvoidspace: removing the assert I mean12:06
dimiternvoidspace: why won't the model be alive ?12:06
voidspacethis test fails: https://github.com/juju/juju/compare/master...voidspace:model-migrations-subnets#diff-62dc0c2f793cbec94ae9fcbdf9c9ab84R50712:06
voidspaceon that assert12:06
voidspaceit fails with: github.com/juju/juju/state/model.go:878: model "new" is being migrated12:07
voidspacedimitern: a model that is being migrated is not active12:07
voidspacedimitern: the checkModelActive assert actively checks for this12:07
voidspacedimitern: see the definition of checkModelActive12:08
dimiternvoidspace: well, the checkModelActive is pretty common - how is that working for other types?12:08
voidspacedimitern: AddSpace doesn't do the check12:09
dimiternvoidspace: it should btw; but how about e.g. machines, units, ..12:10
dimiternvoidspace: maybe it's ok to skip the checkModelActive check *only* during migration?12:10
babbageclunkWeird - if I cd '$GOROOT/src/github.com/juju/juju/mongo; go test -v -check.v' the tests all run twice. Does anyone know why that is?12:12
babbageclunkDoes anyone else see that?12:13
dimiternbabbageclunk: perhaps due to how '-v' and '-check.v' are handled?12:14
babbageclunkOops, meant $GOPATH12:14
dimiternbabbageclunk: check `go help testflag`12:14
voidspacedimitern: machine import has it's own transaction and manually inserts the machine into state12:14
babbageclunkWell, without -v you don't see the output from -check.v12:14
dimiternvoidspace: so then it sounds like AddSubnet's ops cannot be used literally for import12:15
voidspacedimitern: copying and pasting them sounds worse12:15
dimiternbabbageclunk: try '-check.v -test.v' in that order?12:15
dimiternvoidspace: they are likely to be quite different anyway, aren't they?12:16
voidspacedimitern: identical minus that check12:16
voidspaceI think, let me confirm12:16
dimiternvoidspace: assuming they are called in a certain order - e.g. if spaces weren't yet imported..12:17
babbageclunkdimitern: oh no - taking off the -v I still see the (duplicate) output12:17
voidspacedimitern: we only store SpaceName on subnet12:17
voidspacedimitern: so order doesn't matter12:17
voidspaceAddSubnet is quite short - I can copy it into migration_import and remove the assert12:18
dimiternbabbageclunk: usually, mixing '-check.v' and '-test.v' is not a good idea - as -check.v works for the current package, whereas '-test.v' can work on a root import path recursively - e.g. go test -v github.com/juju/juju/...12:19
voidspacenot so terrible I guess12:19
babbageclunkdimitern: Could you try 'cd $GOPATH/src/github.com/juju/juju/mongo; go test -check.v -check.f oplogSuite.TestRestartsOnError' for me and see if you see the test run twice?12:19
dimiternvoidspace: or perhaps better, make a helper that returns addSubnetOps which does not add checkModelActive and use it for both import and AddSubnet, but in the last case append the check before running?12:20
voidspacedimitern: heh, yeah - better12:21
dimiternbabbageclunk: just a sec..12:21
voidspacedimitern: ta12:21
babbageclunkdimitern: Thanks! I think the -check.v flag is a red-herring - I'm trying to understand why the tests run twice.12:21
dimiternbabbageclunk: yeah, it does run the test twice12:22
dimiternbabbageclunk: so not related to -check.v / -test.v12:22
babbageclunkdimitern: What's up with that?12:23
dimiternbabbageclunk: usually it's because a 'baseSuite' or something is embedded in 2 other, separately registered suites (i.e. var _ = gc.Suite(&SuiteEmbeddingBase{})12:25
dimiternbabbageclunk: due to the embedding, any baseSuite.TestFoo() will be called once for each SuiteEmbeddingBase like that12:26
dimiternas TestFoo is 'inherited'12:26
babbageclunkdimitern: I looked to see if there was a suite embedding this one, but no dice. Also this one just embeds BaseSuite.12:26
dimiternbabbageclunk: indeed, might be something else..12:28
dimiternbabbageclunk: always try '-check.vv' in such cases to see which setup/teardown methods where called12:29
alexisbcherylj, are you joining us this morning12:32
alexisbfwereade, ping12:32
babbageclunkdimitern: ok, I'll try that too, thanks.12:32
dimiternbabbageclunk: ah! got it.. nasty indeed - internal_test.go:3112:33
dimiternbabbageclunk: calling gc.TestingT twice in the same package is *infinitely* more sinister than embedding a suite12:34
babbageclunkdimitern: Ooooh. Nasty!12:35
dimiternbabbageclunk: (it's already called in package_test.go as well)12:35
dimiternbabbageclunk: and gc.TestingT is the entry point gc uses to hook into 'go test' machinery12:36
babbageclunkdimitern: Right. So is there any reason not to just delete the internal_test one?12:36
dimiternwhile gc.Suite is registering a suite (not fixture - it needs TestXX() methods to be a suite) into gc to run12:36
dimiternbabbageclunk: please do, and I'm sure fwereade might like to add this to the guidelines :) never run gc.TestingT more than once per package12:37
babbageclunkdimitern: Awesome - thanks for the help!12:38
dimiternnp :)12:38
hoenircould anyone review my PR?12:56
hoenirhttp://reviews.vapour.ws/r/4900/12:56
voidspacedimitern: babbageclunk: http://reviews.vapour.ws/r/4901/13:07
dimiternvoidspace: looking in a sec13:07
* voidspace lunches13:07
sinzuiabentley: can you review https://code.launchpad.net/~sinzui/juju-ci-tools/controller-model/+merge/29565713:30
abentleysinzui: sure.13:31
babbageclunkperrito666: ping?13:31
babbageclunkHe's probably off doing fun birthday things.13:45
hoenircould someone review my PR http://reviews.vapour.ws/r/4900/13:47
hoenir?13:47
hoenirmy PR fixes this https://bugs.launchpad.net/juju-core/+bug/158543013:47
mupBug #1585430: Cloud-init failed on windows <ci> <cloud-init> <regression> <windows> <juju-core:Triaged> <https://launchpad.net/bugs/1585430>13:47
dimiternvoidspace: reviewed14:00
natefinchkatco: +1 on the comment about aliases. Feels like two people disagreed so we decided to just do both14:23
katconatefinch: yeah... death by options14:23
frobwaredimitern: please could you take a look at http://reviews.vapour.ws/r/4903/14:39
frobwaredimitern: we looked at this a while ago, was just getting back to it14:39
dimiternfrobware: sure, looking14:39
natefinchfwereade: when you change a model config in a running model, what executes those changes?14:39
fwereadenatefinch, depends -- what changes are you thinking of? substrate config will be watched for and applied by several things that have their own environs14:40
fwereadenatefinch, different bits of config will be watched for by other things, e.g. logging-config14:40
natefinchfwereade: working on the syslog forwarding stuff... didn't know if there was a generic watcher for it, or if I should write a new one for the syslog stuff explicitly14:41
fwereadenatefinch, you need to watch model config under the hood whatever you do14:43
fwereadenatefinch, so at least it's easy to create the watcher14:43
fwereadenatefinch, just don't let on what it's really watching, call it WatchRsyslogConfig or something14:44
natefinchfwereade: yep, ok14:44
fwereadenatefinch, (note: do not attempt to filter out changes that don't apply to your fields: it is not possible to do so reliably)14:44
natefinchfwereade:  good to know.14:45
natefinchahh, so there's ModelWatcher.WatchForModelConfigChanges14:47
fwereadenatefinch, yeah, that's the one14:48
fwereadenatefinch, ...although, looking at it, there's a bit of WTF there14:52
mupBug #1585674 opened: "Waiting for agent initialization to finish" needs to be more detailed. <juju-core:New> <https://launchpad.net/bugs/1585674>14:52
frobwaredooferlad, voidspace: would appreciate a review of http://reviews.vapour.ws/r/4903/14:53
fwereadenatefinch, non-bulk, doesn't identify model, returns an error indicating OMG INFRASTRUCTURE BORKED instead of "this request didn't work"14:53
mupBug #1585674 changed: "Waiting for agent initialization to finish" needs to be more detailed. <juju-core:New> <https://launchpad.net/bugs/1585674>15:01
mupBug #1585674 opened: "Waiting for agent initialization to finish" needs to be more detailed. <juju-core:New> <https://launchpad.net/bugs/1585674>15:07
mupBug #1585674 changed: "Waiting for agent initialization to finish" needs to be more detailed. <juju-core:New> <https://launchpad.net/bugs/1585674>15:10
mupBug #1585674 opened: "Waiting for agent initialization to finish" needs to be more detailed. <juju-core:New> <https://launchpad.net/bugs/1585674>15:13
voidspacedimitern: ping15:28
katconatefinch: lmk when you're done with the 1-pager; need to send an email out about the bug15:29
natefinchkatco: ok, working on it now15:29
dimiternvoidspace: pong15:29
katconatefinch: ta15:29
voidspacedimitern: you suggest abstracting newSubnetFromArgs out from addSubnetOps15:29
voidspacedimitern: but addSubnetOps still needs to construct a subnetDoc - so that at least would be duplicated15:30
voidspacedimitern: not a big deal, but that's why I didn't do it15:30
voidspacedimitern: if you still think it's worth it I'll make the change15:31
dimiternvoidspace: sorry, otp - will get back to you shortly15:31
dimiternvoidspace: well, addSubnetOps can take a prepopulated subnetDoc15:34
dimiternvoidspace: it doesn't have to also create it15:34
voidspacedimitern: still leaving creation and validation of the Subnet to be duplicated, plus the subnetDoc is a bit of an "internal type" so it feels a bit yucky15:40
voidspaceI'll duplicate creation of the subnetDoc15:40
dimiternvoidspace: I'll leave it up to you15:47
bogdanteleaganatefinch, you could look at the retrystrategy stack, it's essentially watching a model config variable and acting on changes15:50
alexisbcan someone please review this commit: https://github.com/juju/juju/pull/545716:02
alexisbkatco, ^^16:02
katcoalexisb: tal16:02
alexisbbtw perrito666, happy bday!16:02
katcoperrito666: happy birthday :) hope you're feeling better16:03
alexisbthanks katco16:03
natefinchbogdanteleaga: awesome, thanks16:04
katcoalexisb: looks like that already has a review: http://reviews.vapour.ws/r/4900/16:04
alexisbdoes the code author use review board?16:06
katcoalexisb: i don't know, but the project does :) the link is automatically injected into the PR. i'll leave a note asking them to review the review16:07
alexisbkatco, thank you16:08
bogdanteleagaalexisb, katco, yes he does, he recently joined our juju team :)16:13
alexisbbogdanteleaga, aaaah, ok that makes sense16:14
katcobogdanteleaga: grats on the add :)16:15
mbruzekkatco: https://bugs.launchpad.net/juju-core/+bug/158570116:17
mupBug #1585701: juju resources needs to support extensions other than ".tgz" <juju-core:New> <https://launchpad.net/bugs/1585701>16:17
katcombruzek: tal16:17
katcombruzek: replied on bug16:18
babbageclunkperrito666: ping?16:18
bogdanteleagakatco, cheers :)16:19
babbageclunkbogdanteleaga: I've reviewed it as well (although I'm technically only a junior reviewer).16:21
mbruzekkatco: Thanks for pointing that out. My bad.16:21
katcombruzek: no worries; new feature16:22
bogdanteleagababbageclunk, you've basically expressed my exact thoughts on it :P16:22
katcombruzek: fwiw we opposed checking the file extension because it's just a blob of data16:22
bogdanteleagaI guess we should start reviewing on rbt as well16:23
katcobogdanteleaga: i didn't see the need at first, but after using it for awhile it's hard when i have to review something on GH16:24
babbageclunkbogdanteleaga: ok great - I thought maybe I was missing something.16:24
natefinchsinzui: what's with the multiple posts from the bot on this PR? https://github.com/juju/juju/pull/541916:26
natefinchsinzui: (at the bottom it says merge request accepted 4 times in response to a single $$merge$$_16:27
sinzuinatefinch: I have never seen that16:28
sinzuiand I am looking at it now16:28
natefinchsinzui: k, just figured I'd bring it to your attention in case it indicates a problem16:28
=== frankban is now known as frankban|afk
katconatefinch: ding! your hour on the 1-pager is up :) are you about wrapped up?16:32
natefinchkatco: forgot and got lunch in the middle, but still almost done. It's actually fairly simple.  5 minutes.16:34
katconatefinch: kk16:34
voidspacedimitern: why does the subnetDoc have an IsPublic field?16:39
natefinchkatco: last two pages here: https://docs.google.com/document/d/1x60GL9zckzXNfHw_yyY_syxF1WSL5JkcWoWKJtwT_Ww/edit#16:42
dimiternvoidspace: because subnets can be public or private (i.e. with and without shadow addresses support)16:42
dimitern(by spec, not as implemented currently except by mocking)16:42
katconatefinch: ta; can you send an email to ian and cc eric, me stating as such?16:42
natefinchkatco: sure thing16:42
katconatefinch: and then you're rolling back onto the bug now?16:43
natefinchkatco: yep16:43
katconatefinch: cool, reassign yourself plx :)16:43
voidspacedimitern: then why is that field not *used*16:45
voidspacedimitern: it's not on SubnetInfo16:45
voidspacedimitern: and it's not exposed on Subnet16:45
voidspacedimitern: so it's never set or changed16:46
=== redir_afk is now known as redir
dimiternvoidspace: because we haven't quite got there yet16:51
voidspacedimitern: unused code for imaginery future use cases... :-p16:51
dimiternvoidspace: not imaginary, just not done yet16:52
voidspacedimitern: I've added it as an ignored field in the migration_internal_test - when we do start using it we'll have to remember to migrate it16:52
dimiternvoidspace: if you read the network model spec, private/public subnets and spaces are part of it16:52
dimiternvoidspace: sounds good, +116:53
voidspacedimitern: that doesn't change my opinion - specs are always imaginery until they're actually implemented16:53
dimiternvoidspace: of course - I'd leave a TODO on the field at least - to keep it in mind; there's even a bug for it16:55
voidspacedimitern: good idea, adding comment16:56
dimiternvoidspace: thanks!16:57
rogpeppea simple addition to the juju/cmd package. anyone up for a little review? https://github.com/juju/cmd/pull/3717:13
rogpeppevoidspace, dimitern: ^17:13
rogpeppenatefinch: ^17:14
rogpeppefwereade: ^17:14
dimiternrogpeppe: LGTM17:21
rogpeppedimitern: you angel, thanks!17:21
natefinchrogpeppe: I'm sort of surprised that this wouldn't just be something we add to the help command itself17:22
rogpeppenatefinch: how would we do that?17:22
rogpeppenatefinch: the help command is built into SuperCommand17:22
rogpeppenatefinch: FWIW we considered about 3 dozen options here :)17:22
rogpeppenatefinch: this was the simplest and best tailored to what we actually needed17:23
natefinchrogpeppe: oh yeah, I forgot the help command is built in....17:23
rogpeppenatefinch: this is what it enabled: https://github.com/juju/charmstore-client/pull/6617:23
natefinchrogpeppe: very nice17:24
rogpeppenatefinch: yup, always happy to delete code :)17:25
redirreboot brb17:37
mupBug #1585750 opened: Destroying a service in error gives no feedback <juju-core:New> <https://launchpad.net/bugs/1585750>19:11
=== mramm_ is now known as mramm
=== natefinch is now known as natefinch-afk
katconatefinch-afk: you never reassigned yourself to bug 1537585 :(21:38
mupBug #1537585: machine agent failed to register IP addresses, borks agent <2.0-count> <blocker> <landscape> <network> <juju-core:Triaged> <juju-core 1.25:Triaged> <https://launchpad.net/bugs/1537585>21:38
axwrogpeppe: not sure why help plugins was removed.21:51
axwnor the rest really21:51
cheryljaxw: the help topics were removed to avoid having to keep two places up-to-date: (online docs and cli help)22:11
axwcherylj: ok, fair enough. plugins tho? that might have been accidental?22:12
cheryljoooh, yeah I forgot that help plugins wasn't just a topic22:13
cheryljyeah, that might've been an oversight22:13
wallyworldaxw: anastasiamac: have standup without me today, in another meeting23:09
axwok23:09
axwanastasiamac: standup?23:16
axwperrito666: ^23:16
mupBug #1585825 opened: Takes too long to download a resource from a controller to unit <ci> <resources> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1585825>23:39

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!