[00:35] thumper: I was going to add a test to make sure the machine unit watcher doesn't show things from other environments but that's opened a whole can of worms in terms of filtering work we need to do and required test infrastructure [00:36] thumper: do you want me to press on or do the minimal work required to get the units env UUID work landed? [00:36] thumper: I'm aware that the identity work needs to get done too === mup_ is now known as mup [01:03] how do you get an agent.Config from state? [01:07] ericsnow: you don't. agent.Config is read from disk, and only from disk [01:08] menn0: how big is that can of worms? [01:08] thumper: at least the rest of the day [01:09] * thumper hears "and Friday" [01:09] axw: my real question is where do I look up the environment's data dir [01:09] thumper: likely [01:09] axw: from what I could tell environs.Config doesn't give it to you [01:09] menn0: well, we know we will need a bunch of extra tests later anyway [01:09] ericsnow: that'd be encoded in the upstart job [01:09] menn0: I say punt on it for now, but leave notes in the code [01:09] thumper: and actually, since writing to you last i've realised that we can't really do this until at machiens have been migrated too [01:10] ericsnow: the agent is told where it is as a command line arg [01:10] menn0: that makes sense [01:10] ericsnow: what are you trying to do? [01:10] menn0: because they all have dependencies on each other [01:10] axw: that's what I gathered [01:10] axw: for backups we currently have a bunch of paths hard-coded [01:10] thumper: I was trying to set up a second env it's own machine, services and units to test with and kept running into collisions with the initial env [01:10] heh [01:10] * thumper nods [01:10] thumper: I will add some TODOs [01:11] axw: I'm looking at how to pull those from elsewhere [01:11] ericsnow: IIRC, datadir is passed into the apiserver as a "resource" ... one sec [01:11] axw: you're right [01:12] thumper: I have a fair idea of how the test infrastructure for this should look (i.e. a new method on ConnSuite to create a new env and an easy way to get a Factory for that) [01:12] * thumper nods [01:12] ericsnow: can you not take it from there then? [01:12] axw: CI blocker: http://reviews.vapour.ws/r/104/diff/ ? [01:13] axw: yeah, I guess I was hoping for a single-source-of-truth for paths [01:13] thumper: looking [01:14] ericsnow: are you guys looking at https://bugs.launchpad.net/juju-core/+bug/1373611 [01:14] Bug #1373611: cannot restore a HA state-server

[01:14] perrito666: is this your fix from before? [01:14] thumper: no [01:15] thumper: bugger, I thought our compatibility was only the other way around... [01:15] axw: I pasted back in the code I deleted earlier [01:16] thumper: http://reviews.vapour.ws/r/101/ has a fix [01:16] thumper: yep. LGTM [01:16] axw: yeah... see the bug description https://bugs.launchpad.net/juju-core/+bug/1373424 [01:16] Bug #1373424: method Client.AddMachinesV2 is not implemented

[01:16] thumper: for #1373611 [01:16] Bug #1373611: cannot restore a HA state-server

[01:16] ericsnow: I was wondering if that was it [01:16] ericsnow: has it been submitted? [01:17] thumper: don't think so: https://github.com/juju/juju/pull/836 [01:21] * thumper submits it [01:32] does anyone know where in the API call steps we unmarshall values? [01:38] wwitzel3: in the magic bit of the code [01:38] wwitzel3: juju/rpc maybe? [01:38] thumper: well, I'm getting ERROR json: cannot unmarshal object into Go value of type names.Tag [01:38] yeah... [01:38] but the logs have no information on where the is being generated from at all [01:39] the wire protocol needs to use strings [01:39] until we fix the wire protocol [01:39] wwitzel3: we could fix it... [01:39] wwitzel3: you just need to provide the json serialization functions for all the concrete types [01:40] wwitzel3: the error is being raised by the json serialization code [01:40] thumper: where is that code? apiserver? the jsoncodec call? [01:40] wwitzel3: the json code? inside the standard library [01:41] json.Marshal [01:41] thumper: I mean within our code, where is the unmarshalling happening? [01:41] wwitzel3: I'm guessing, but probably in juju/rpc where it puts the code on the wire [01:41] wwitzel3: api.client unmarshals actually [01:43] thumper: if I could fucking spell, I'd probably have less questions [01:43] acking for unmarshall exact match wasn't ever going to be helpful [01:44] I'm a moron [01:44] thank you [01:44] Umarshall that is [01:44] I at least had the puncuation right [01:45] thumper: so, is the solution to switch back to strings? or to define how to unmarshal names.Tag? [01:45] wwitzel3: yes... [01:46] wwitzel3: for assistance, version.go:117 [01:46] not sure if you want to go that way or not just yet [01:46] thumper: a Tag is pretty easy type to implement that for [01:47] wwitzel3: yep [01:47] thumper: and it is still that something based on string isn't easily serializable to JSON anyway [01:47] wwitzel3: because we even have the magic parseFooTag mthods [01:47] s/still/silly [01:47] ok, I'll go that route since I think it makes things better [01:47] good luck [01:48] it's just typing [01:48] and tests :) [01:48] yep [01:59] can someone help me [01:59] for some reason I can no longer see both the comments and the diff on the same screen [01:59] davecheney: maybe [02:00] something has happened and reviewboard does not show me the comments that others have made on a review [02:00] unless I go to the "view review" screen [02:00] but then it doesn't show me the code [02:00] and I need to go to the "view diff" screen [02:06] davecheney: I've had to go to view diff screen too [02:06] I thought it was just me getting used to the tools [02:07] thumper: did you merge my fix? [02:07] perrito666: aye [02:07] thumper: thank you sir [02:08] thumper: this worked a few weeks ago [02:08] ie, on the diff screen you would see inline comments [02:08] now they are all hidden on another screen [02:08] btw axw you might have inadvertedly fixed a problem in environs.Environ StateServerInstances congrats, you now subconsciously rock [02:08] how the hell do I change the topic [02:09] perrito666: heh, what was that? [02:09] massive storm in sydney atm [02:09] if i go offline [02:09] please sent paramedics [02:09] heh [02:09] axw: well the bug I fixed should have been triggered long ago but apparently StateServerInstances was returning only one instance :p and after your commit it now returns all the state servers [02:10] ugh [02:10] perrito666: ah :) that only happens for new environments tho [02:10] the bot still things there are ci blockes [02:10] davecheney: dont you prefer us to send an internet technician? I mean I dont see paramedics getting you back online [02:10] but they are both fix committed now [02:10] thumper: takes a bit I think [02:10] perrito666: it was an intentional improvement, didn't know it fixed anything in particular tho :p [02:11] * thumper taps his fingers impatiently [02:12] axw: neither did I, in fact none of us knew it was broken [02:12] axw: the test you added does the same restore did that is why I realized [02:12] thumper: http://reviews.vapour.ws/r/79/diff/# [02:13] can you go to line 100 in filestorage/wrapper.go [02:13] and tell me if the line is properly gofmt'd on your screen [02:14] i'd like you to the line if that was something that RB could do [02:17] * thumper looks [02:18] or this one http://reviews.vapour.ws/r/103/diff/# [02:18] api/backups/list_test.go lines 27 onwards [02:18] it looks liek RB gives up after three levels of indentation [02:18] davecheney: looks properly formatted to me [02:19] davecheney: list_test looks fine to me too [02:20] hang on, screenshotting [02:21] thumper: check your mail [02:21] davecheney: yeah, yours looks different to mine [02:21] davecheney: I see proper formatting [02:22] davecheney: may I suggest it is your shitty browser? [02:22] :) [02:22] chrome, just like everyone else [02:22] hmm... [02:22] it really does look fine here [02:23] chromium here [02:23] fine, i'll just ignore any formatting issues [02:23] the bot will catch them for me [02:55] gah... why is the bot not working out that we have fixed the blockers? [02:58] thumper: http://reviews.vapour.ws/r/93/diff/ PTAL [02:58] menn0: ok [03:01] thumper, i told the buildbot that those blockers were fixed. it seemed to accept builds now [03:01] cmars: how? [03:02] cmars: I thought it looked at LP [03:02] you put $$fixes-$$ in the merge comment [03:05] $$__JFDI__$$ [03:16] ah, crap. buildbot is not remembering the fixes-NNN. i suppose that was already tried.. [03:18] thumper: are you still looking at that PR? [03:19] yup [03:19] thumper: ok [03:19] cmars: I did that [03:19] thumper: just making sure you didn't forget to hit publish [03:19] cmars: that is how it landed the fixes [03:19] cmars, CI is blocked by the two critical bugs in the topic. a fix for 1 of them is being test now [03:20] sinzui: I thought the blockage got removed when we committed them? [03:20] no [03:20] sinzui: are they removed later now? [03:20] ah [03:20] CI was being reopen but the tests dfailed [03:20] we not wait for someone to make the bug fix released when the test is passed [03:24] sinzui: ack [03:24] oh man, so apparently if you implement a method on an interface with a pointer receiver .. every single place in the world you've ever used that break [03:24] wwitzel3: you can't implement a method on an interface [03:24] at least I didn't think so... [03:25] davecheney: ? [03:25] wwitzel3: yeah, the more I think about it, the more I think that you can't do that, it makes no sense [03:26] menn0: published response [03:28] thumper: no, I mean on the concreate implementation [03:28] thumper: sorry, it's late, shitty wording [03:29] wwitzel3: ok [03:29] why does it break? [03:30] thumper: I added the UnmarshalJSON method to the tag interface and then added an implementation on MachineTag. Now I have several dozen errors for does not implement Tag (UnmarshalJSON method has pointer receiver). Every where we reflect tag.(MachineTag) [03:30] which makes sense [03:30] I would need to pass in a pointer [03:30] but there are a ton of touch points in the test :/ [03:31] wwitzel3: you don't need to add the marshal and unmarshal methods to the tag interface... [03:31] wwitzel3: or at least, not yet [03:31] but the params we are sending over the wire are []names.Tag [03:31] wwitzel3: yes, the unmarshal needs a pointer [03:31] wwitzel3: as it changes the instance [03:31] wwitzel3: and not a copy of it [03:32] wwitzel3: hmm... [03:32] maybe you do need it ? [03:33] if I don't add them to the interface, it complains it doesn't know how to Unmarshal a Tag, which makes sense [03:34] I'll figure it out eventually [03:35] tag.(*MachineTag) .. is probably want I want [03:36] thumper: thanks [03:36] thumper: I did make the change in service.go! [03:37] menn0: did you [03:37] thumper: the tests wouldn't be passing without it [03:37] must have missed it [03:37] thumper: but you're right that a migration step would also be needed so I might just undo that [03:37] menn0: ah, so you did [03:37] yeah, it makes me a little sad, but probably faster for now [03:40] thumper: or *evil grin* could we just strip a trailing "/" off the prefix if it's there? [03:40] haha [03:40] thumper: I'm half-serious [03:40] do it [03:41] thumper: alrighty [03:46] that's my favorite exchange of the night [04:04] thumper: http://reviews.vapour.ws/r/93/diff/ [04:05] menn0: do your upgrade test while I have a coffee [04:05] that sounds a bit weird [04:05] but Rachel just got home [04:06] * thumper cracks the whip [04:06] thumper: doing it already :) [04:06] thumper: what's a quick and easy subordinate charm I can add to my test environment? [04:11] menn0: nagios-nrpe [04:12] davecheney: thanks === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: 1373611 [04:30] thumper: well that didn't go so well... [04:30] thumper: some services have disappeared [04:30] thumper: I'm going to check the DB [04:40] oh... [04:43] jam: hey there [04:51] menn0: so... how is the db looking? [04:51] menn0: did you want to talk it through? [04:52] thumper: the db looked as I expected it but obviously something wasn't right [04:52] so the Marshal methods have to be part of the interface definition for the generic names.Tag to support json encoding, and eventually I got it all working on the names package, but there are just too many places where we do something like []names.Tag{NewUnitTag()} .. which no longer works the momemnt you have a pointer receiver [04:52] thumper: I'm just testing the same env with master so that I can be sure it was the units change [04:52] menn0: ok, [04:54] thumper: ok master is fine [04:54] thumper: going to rebase my branch to upstream and recreate the problem again [04:54] kk [04:56] * thumper notices that st.AllServices() definitely leaks services across environments... [04:57] thumper: there's tons and tons of stuff that leaks across envrionments [04:57] thumper: once we have the base DocID changes done we'll need to circle back and deal with all that, writing tests as we go [04:57] agreed [04:58] thumper: I was thinking it might be a good idea to set up an alternate environment in the setup of ConnSuite that doesn't get directly referred to in tests but will expose places that leak [04:59] thumper: unexpected data should show up all over the place and cause tests to fail [05:01] thumper: this canary env should be fairly well set up with several machines, services, subordinate units etc to maximise the stuff it'll expose [05:02] * thumper nods [05:03] the mediawiki charm just won't install at the moment [05:03] that seems reasonable, but potential overkill [05:03] I keep having to ssh to the box and do a "apt-get update" and then run "juju resolved --retry mediawiki/0 to get it to install [05:03] is that expected? [05:03] this is with 1.20.7 [05:04] menn0: local? [05:04] thumper: yep [05:04] menn0: could be due to the apt upgrade / update thing [05:04] your template is out of date [05:04] thumper: right [05:04] thumper: very likely [05:05] thumper: delete the template machine or is there a faster way? (start the template and update it?) [05:05] yeah [05:05] yes to starting the template [05:06] and logging in, and doing an apt-get update/upgrade [05:06] thumper: ok, i'll do that [05:06] then shutting it down again [05:08] (╯°□°)╯︵ ┻━┻ [05:09] ┬─┬ ノ( ゜-゜ノ) [05:09] hey, do we have a stable 1.20.8 yet? wondering when I should expect to need to change to it [05:09] thumper: http://paste.ubuntu.com/8423049/ :( [05:10] thumper: the exact final result doesn't appear to be deterministic either [05:10] menn0: oh crud [05:10] menn0: is it the status collection? [05:10] menn0: units have status [05:11] * menn0 checks [05:11] the status command would be looking there, right? [05:11] yeah but we haven't changed the statuses collection keys yet. [05:11] maybe that's the problem. [05:11] it didn't break services... [05:11] * thumper nods [05:12] menn0: I think... [05:12] menn0: that status should still keep local ids [05:12] but need to check [05:12] it is how it gets the underlying entity [05:12] I'm being called for dinner [05:12] * thumper will be back for team-lead meeting later === thumper is now known as thumper-afk [05:24] morning all [05:26] thumper-afk: the migration steps for units and services have mixed everything up. [05:27] thumper-afk: for example, all services now have a name of "rsyslog-forwarder-ha" I can only tell them apart from the _id. [05:27] thumper-afk: must be something with the generic upgrade function. I'll take a look [06:03] davecheney, hey [06:03] dimitern: o/ [06:04] davecheney, due to the way names.NewEnvironTag() works, you *can* create an invalid tag without panicking, i.e. tag := names.NewEnvironTag(""), then tag.Id() == "" is equivalent to tag == nil if tag used to be names.Tag [06:04] dimitern: thanks for confimring [06:04] i think it is still a bug [06:05] i think the problem is NewEnvironTag("") should fail [06:05] davecheney, I agree [06:05] davecheney, but environ tags are relatively recent, and we still have to support the case when it's not set [06:06] dimitern: i wish thumper was here [06:06] environ tags are basically manditory for mees [06:06] mess [06:06] whatever [06:06] we can't keep continuing to say, oh well, if you don't have an env tag we can just overlook it this time [06:07] davecheney, since I saw your comments too late, I'll do a follow-up to do some of your suggestions and reply about NewEnvironTag [06:08] ok, thanjs [06:08] thanks [06:11] thumper-afk: I've figured out the garbled data following the migration. if you read data into a map using mgo and then save that map in a slice you need to recreate that map before starting the next loop. setting it to nil was sufficient. [06:13] thumper-afk: there's still what looks like a presence issue. after the upgrade the units are marked as down. the agents are started (in the DB too) but the presence check is failing. [06:13] thumper-afk: I'll continue tomorrow. I need to EOD. === menn0 is now known as menn0-eod [06:27] davecheney, wanna have a look? http://reviews.vapour.ws/r/105/ [06:30] looking [06:32] dimitern: looks good, two issues [06:33] davecheney, cheers [06:35] davecheney, I don't think returning an error there is sensible, due to the possibility of not having environ UUID at the endpoint level [06:36] davecheney, and the login *will work* without it, for the same reason [06:36] sure [06:36] davecheney, but maybe adding a warning and continuing will be useful [06:36] var e names.EnvironTag [06:36] e.String() [06:36] no [06:36] e.Id() => "" [06:37] so there is no way to distinguish between presenting an environ id that is "", and the environ id being invalid and passing "" as a fallback [06:37] simply put, there is no point validating if we're not going to reject values which are not valid [06:38] davecheney, given a names.EnvironTag already, Id() == "" is the same as it being invalid === uru_ is now known as urulama [06:38] davecheney, but names.IsValidEnvironment(uuid-or-tag.Id()) works [06:38] crap [06:39] davecheney, the point of that code there is to only use the uuid if it's valid (an improvement from before when we didn't even validate the uuid) [06:40] davecheney, that's why I suggest to do a logger.Warningf("API endpoint has invalid environment UUID %v", uuid) when it's not valid, but still let it through [06:40] i guess that is all we can do [06:40] this will end up being a buig [06:40] i'm sure of this [06:40] but what else can we do [06:40] or maybe "ignoring invalid API endpoint environment UUID %v" [06:41] sure [06:41] yeah, we can incrementally make it better :) [06:48] davecheney, updated - http://reviews.vapour.ws/r/105/diff/1-2/ === uru_ is now known as urulama [06:53] davecheney, is it good to land like this? [06:55] jam: contributions to things like gwacl still need CLA signing, right? [06:56] axw: I'm pretty sure all our stuff is (c) Canonical under the same CLA [06:56] so they don't need to sign it separately [06:56] but it does need to be signed [06:56] jam: ok - someone sent in an MP for gwacl specifically [06:57] if they've signed, it shows up on their LP ~ page right? [06:57] axw: I see "Canonical Contributer Agreement" as one of my groups on https://launchpad.net/~jameinel/+participation [06:57] so I think so [06:58] thanks [06:58] though I don't see it for https://launchpad.net/~axwalk/+participation [06:58] jam, hey, can you have a look at http://reviews.vapour.ws/r/105/diff/ please? [06:58] I'm not sure that everyone in "Canonical" is also in CCA [06:59] dimitern: can you actually compare Tag objects? I thought it was an interface, and you don't want to compare exact internals [06:59] if you can, those changes seem fine [07:00] jam: AFAIK, only required if you don't work for Canonical; employment contract covers employees [07:00] I'm just concerned if doing "t1 := names.Tag("foo"); t2 := names.Tag("foo"); t1 != t2" [07:00] axw: right, you don't need to sign it, I just mean you need to look for one or the other [07:00] ah yep [07:01] jam, only names.Tag is an interface, the others are simple structs implementing it [07:01] dimitern: axw: https://launchpad.net/~not-canonical :) [07:01] hehe [07:02] I got there from jelmer, who used to work at canonical [07:02] Trying to find someone who isn't me or at canonical who should have signed the agreement to confirm participation [07:02] jam, yep that's the "former employees and people often mistaken as employees" list :) [07:03] dimitern: is there a simple way to link part of an RB diff? [07:03] I'm looking at one that is very-much comparing a names.Tag to a names.Tag [07:03] jam, you can link a X-Y diff, like this http://reviews.vapour.ws/r/105/diff/1-2/ [07:04] dimitern: yeah, I want to link a line in the review: http://reviews.vapour.ws/r/105/diff/#1 [07:04] one of those is *definitely* a names.Tag [07:04] jam, apistate.EnvironTag() (names.EnvironTag, error) [07:05] jam, and environ.EnvironTag() (names.EnvironTag) [07:05] jam, confusingly enough, environ has at least 4 more Tag methods - ServerTag, Tag, etc. [07:05] func(tag names.Tag) bool { [07:06] dimitern: ^^ [07:06] from 3 lines above that [07:06] jam, aah, this one [07:07] jam, yeah, but it really is a names.EnvironTag [07:07] jam, because we do tag, err := ParseEnvironTag(strTag) and then call canAccess(tag) [07:08] jam, it's just wrapped inside a names.Tag due to the way AuthFunc is defined [07:09] dimitern: http://play.golang.org/p/wbU4gcGnWe seems to work [07:10] the interface isn't the same, but it seems to compare the underlying object [07:11] jam, http://play.golang.org/p/DzZ6RmhiE3 [07:12] jam, :) yep [07:12] dimitern: in your example you're using concrete types, Tag isn't doing anything [07:13] ah, I guess 'u' was wrapped in a Tag [07:13] jam, exactly [07:14] jam, if I had func(tag Tag) bool { return tag == internalTag }, I could call it like f(Tag(u)) [07:14] oops, I meant just f(u) [07:14] dimitern: so what I was more concerned about was the e1 == e2 [07:14] because the interface comparison *could* compare the underlying pointers [07:14] rather than the objects that those pointers represent [07:15] dimitern: but you can use all kinds of wrappers, "==" seems to just compare the underlying structs [07:16] jam, yeah, but if that's the case st.EnvironTag() will return some different pointer than the tag arg points to [07:16] and they'll never be == [07:17] yeah, I seem to recall some doc or article about golang that comparisons work like that [07:18] jam, so, is it good to land? [07:18] jam, I have at least 2 follow-ups queued :) [07:20] dimitern: this seems strange: if names.IsValidEnvironment(apiInfo.EnvironTag.Id()) { [07:20] convert it from being a tag, just to check if it was a valid tag [07:20] not saying there is a better way [07:20] LGTM [07:20] jam, if you follow the discussion we had earlier with davecheney [07:21] jam, NewEnvironTag does not panic or validate at all what you give it [07:21] it's a bug, but kinda intentional wrt backwards-compatibility for envs without uuid [07:21] jam, thanks! [07:21] dimitern: sure, it just feels like something that should be "tag.IsValid()" [07:22] jam: no [07:22] i disagree [07:22] you should never have to ask tag.IsValid [07:22] there are two possiblitues [07:22]