[00:48] help, how do I fix this ? [00:48] Error details: [00:48] empty tools-url in environment configuration [00:50] well, fuck [00:50] % juju destroy-environment -y ap-southeast-2 [00:50] ERROR empty tools-url in environment configuration [01:34] davecheney: um, not sure. is there a tools-url in the yaml? [01:50] wallyworld_: i fixed it [01:50] my working copy was old [01:50] ah ok [01:53] i got the thing I needed [01:53] https://bugs.launchpad.net/juju-core/+bug/1227952 [01:53] <_mup_> Bug #1227952: juju get give a "panic: index out of range" error

[01:53] yay yaml [01:54] yay indeed [01:56] ** Bug watch added: Go Issue Tracker #6761 http://code.google.com/p/go/issues/detail?id=6761 [01:56] ^ I didn't know LP did this === gary_poster is now known as gary_poster|away [02:02] wallyworld_: http://play.golang.org/p/hedM5ozbo2 [02:02] simple repro [02:03] can't find import: "launchpad.net/goyaml" [02:03] yeah, won't work in the playground [02:03] copy and paste it into a file [02:04] well, that error does indeed suck [02:05] at least it can be produced trivially [02:05] is [02:05] value: - [02:05] valid yaml ? [02:05] wallyworld_: could you try doing the same in python ? [02:05] not sure [02:05] ok [02:05] ta [02:11] wallyworld_: i fixed the bug, tests all pass [02:11] by deleting code [02:11] i'm not sure how gustavo will like that :) [02:13] davecheney: ah, ok. good luck :-) [02:16] davecheney: after i apt get installed a python yaml lib, python does seem to think it is valid fwiw [02:16] davecheney, wallyworld_ We updated charm-tools proof a few weeks ago to check for valid yaml. [02:17] sinzui: do you know if - has to be escaped in yaml ? [02:17] ie [02:17] output-file: - [02:18] davecheney, It must be quoted when you don't mean list item [02:19] right, so the charm is invalid ? [02:20] g: Yes # a boolean True [02:20] ^ oh for fucks sake yaml [02:21] sinzui: i cannot find any think to say that values must be quited [02:21] quoted [02:25] Values do need to be quoted ":" must be in normal string values. dash is accepted at http://yaml-online-parser.appspot.com/ [02:26] sinzui: orly [02:26] i used that validator and it said [02:26] value: - [02:26] sequence entries are not allowed here [02:26] in "", line 1, column 8: [02:26] value: - [02:26] in short [02:26] gotta quote it [02:27] ah [02:27] but it you type [02:27] value: this -that [02:27] I think it is accepted [02:27] yeah [02:27] it's the bare hyphen that needs quoting [02:28] yep [02:28] value: nil [02:28] is a problem not in yaml but is in our goyaml [02:30] wsgi_log_file: [02:30] type: string [02:30] default: "-" [02:30] description: "The log file to write to. If empty the logs would be handle by upstart." [02:30] ^ the original yaml [02:30] ah ha! [02:30] ja'cuse [02:30] this is correct [02:30] when we format the data from juju get $SERVICE [02:30] goyaml cocks it up [02:30] davecheney, I see that wsgi_log_file: "-" is quoted in the current config.yaml [02:31] ah we saw the same thig [02:36] right, fixed [02:36] MP incoming [02:41] https://codereview.appspot.com/26430043 [02:41] oh crap [02:41] it's nearly 2 [02:41] time for lunch [05:31] if I know my charm needs certain constraints, can that be hard-coded in the charm such that juju will pick it up? [05:32] bigjools: nope [05:32] davecheney: bugger. worth a bug? [05:32] sure [05:33] ok ta [05:33] i'm pretty sure this is by design [05:33] oh? [05:33] seems odd to me [05:33] i didn't say it was a sensible design [05:33] heh [05:33] jam: ping [05:33] davecheney: pong [05:33] jam: you are an owner on goyaml, could I get a review https://codereview.appspot.com/26430043 [05:33] pls [05:34] bigjools: we've talked about adding charm defined constraints in SFO, I don't know if/where it ended up on the schedule [05:34] jam: ok, well I shall file a bug anyway for book-keeping [05:35] jolly good [05:35] davecheney: so the actual change is just len() ? [05:36] jam: yup, the error was encode was detecting - as the start of a --- documented seperator [05:36] and panicing when it ran off the end of the slice [05:37] an altertive solution would be to delete line 1015-1017 [05:37] davecheney: so LGTM though if you have a bug related it would be good to associate it. [05:37] bug is linked to the MP [05:37] ah, it wasn't linked in the codereview which you linked me :) [05:37] sorry [05:37] lbox failed me [05:37] joys of having double systems [05:37] jam: can you please commit that for me [05:38] i do not have permission [05:45] jam: thanks for committing that [05:47] davecheney: np, I'm glad I was able to submit it correctly for you :) [05:48] jam: how can I target this bug to 1.16 stable ? https://bugs.launchpad.net/goyaml/+bug/1227952 [05:48] <_mup_> Bug #1227952: juju get give a "panic: index out of range" error

[05:48] There should be a "Target to series" underneath the existing bug targets [05:49] I only get project and distro [05:49] oh no wait [05:49] reloading the page [05:49] does somethine selse [05:49] so going to the page probalby brought you to the goyaml version [05:50] aaah [05:50] you want the juju-core version at: https://bugs.launchpad.net/juju-core/+bug/1227952 [05:50] I think [05:50] <_mup_> Bug #1227952: juju get give a "panic: index out of range" error

[05:50] davecheney: so, is an update to the goyaml dependency in the 1.16 branch? [05:50] (or at least sinzui knows about it?) [05:50] otherwise I'd mark it "In Progress" [05:51] kk [05:51] yeah, i nede to make sure sinzui knows that deps.tsv needs to be updated [05:52] rogpeppe1: ping, need help with godeps [05:54] jam: when we upgrade, and a config.New() is done by the juju upgrade command (client), do those config values get pushed to the server environment? in particular, if i delete a config value, will that value then be made to disappear from the server env config on the (old version) state server before the server side upgrade process starts? [05:54] i didn't think that would happen, but maye it does [05:55] wallyworld_: AFAIK when we upgrade nothing changes unless you put that code into the new version [05:57] jam: that's what i thought, and yet it seems that if i delete tools-url from the config (in 1.17), then 1.16 servers complain they can't find it [05:57] jam: there's a bug (?) in state that means no config attributes ever get removed from state [05:57] err sorry, wallyworld_ [05:58] that's what i thought [05:58] but there's an upgrade bug that seems to be caused by tools-url not being available when the 1.16 servers run the upgrade process. i'll test a bit more. gotta duck out to take my kid to the doctor. back later [06:03] wallyworld_: my guess is that the original bootstrap *didn't* have a tools-url because it was using stock tools and so it didn't need it. Then "upgrade-juju" is running from an environment which has been edited to *add* tools-url but only locally. [06:03] You probably need a "juju set-environment tools-url=XYZ; juju upgrade-juju" [06:06] jam, wallyworld_: now that I think of it, this was one of the feature requests from IS at SFO [06:06] atomic upgrade + set-env [06:11] axw: I'm pretty sure they wanted it for charms, not for juju as a whole [06:12] jam: ah yes, sorry, right you are [06:17] https://code.launchpad.net/~dave-cheney/juju-core/161-update-goyaml-godep/+merge/195173 [06:18] davecheney: I just approved, but you seem to have a debug "fmt.Printf" in there [06:24] jam: good catch [06:24] that diff is dirty [06:24] it shold only be the deps.tsv [06:24] TIL, don't remove files from the bzr commit message, it doesn't care [06:25] davecheney: no, it doesn't do anything. You have to specify it on the command line [06:25] the listing in the message is just for you as the writer of the message [06:25] I can see the draw, though. [07:36] wallyworld_: when you're back, I've sorted through enough logs to feel like I have a handle on bug #1250974 but I need some feedback from you [07:36] <_mup_> Bug #1250974: upgrade to 1.17.0 fails

[07:37] the short summary: When you call "upgrade-juju" it marks the AgentVersion field by writing the entire EnvironConfig. However, our "write the config" code is just "add/update these fields to the dict". [07:37] If you have an omitted entry, it *doesn't* touch it. (as noted by axw) [07:37] so when you did patch r2053 [07:37] you changed tools-url:null into tools-url: "" [07:37] which means that SetEnvironConfig({stuff, tools-url:null}) would not have touched that field [07:37] well, I should say [07:38] SetEnvironConfig({stuff, agent-version:"1.17.0", tools-url:null) [07:38] but SetEnvironConfig({stuff, agent-version:"1.17.0", tools-url:""}) *does* wipe out the tools-url field. [07:39] And note, upgrade-juju *does* read the remote environ config [07:39] with conn.State.EnvironConfig() [07:40] but that yaml is parsed by the local code [07:40] so it strips out stuff it doesn't understand [07:40] wallyworld_: so what I need to understand is why in r2053 did you need to switch from Omit (which would have it locally just ignored) to a default of "" which means we cause it to get overwritten. [08:00] fwereade, if you're around - https://codereview.appspot.com/25080043/ from yesterday, when you can please [08:03] jam: cause i got errors in some live tests [08:04] oh bugger. dinner ready [08:04] back soon === jtv2 is now known as jtv [08:14] jam: i did a live test on ec2, upgrade from 1.16 to 1.17, it worked ok, but i did it with the line which deletes tools-url from the config attrs map commented out [08:15] i will test again with that line back in [08:16] jam: what i was asking before was about whether config created on the new 1.17 client would overwrite config in the env state on the 1.16 node - you seem to imply it will overwrite? [08:17] wallyworld_: yeah. so "set agent-version: NEWVERSION" is "read the config from remote, change that line, and write it back" [08:17] but the "read from remote" parses it with the local structure [08:17] write it back only update fields that exist in the thing we are writing [08:17] ok, so the code in trunk should work then [08:17] well, I could be wrong with the direct DB access [08:17] well 2053 doesn't [08:17] wallyworld_: is there a follow up later? [08:17] cause i remove tools-url from the map [08:18] so it should not overwrite the value in env sate [08:18] wallyworld_: so *if* my understanding of SetEnvironConfig is true, then you're right [08:18] and yet, if i remove the delet(attrs, "tools-url") line it works [08:19] that's the only change i made from trunk before i tested [08:19] i'll test again with trunk [08:20] jam: to answer question above - no follow up yet to 2053 [08:39] wallyworld_: so looking at state/state.go SetEnvironConfig. It expects to have a config.Config that at least has an AgentVersion in it, but that appears to be the only thing it needs, and it only calls old.Update(new) [08:40] i'm running another test, will look in more detail at that code in a sec [08:41] wallyworld_: k. I'm thinking we *might* be able to get away with just creating a new Config that just has the fields we actually want to update, to sort of protect us in the future from other incompatible Config things. [08:42] makes sense, would be more robust [08:46] dimitern, reviewed, basically LGTM, one substantial question [08:47] wallyworld_, jam: is it time to fix SetEnvironConfig? :/ [08:47] perhaps [08:47] just confirming my theory about ause [08:47] fwereade: well Dimiter is at least fixing Upgrade by going via the API and we can sort of do something better there. [08:47] cause [08:48] jam, yeah, there's SetEnvironAgentVersion already [08:48] SetEnvironConfig is *almost* just taking a delta and applying it, so we could just make that explicit "UpdateEnvironConfig" and go from there. [08:48] jam, but SetEnvironConfig itself is kinda broken by not-even-design [08:48] jam, there's no way to remove fields but maybe that's not a serious problem? [08:48] fwereade, thanks [08:50] fwereade: well I mean write an UpdateEnvironConfig that would take an actual delta (maybe 1 map to add and a list to remove? Or 2 maps and we assert old and new values?) [08:51] jam, yeah, that (probably the former) sounds sane [08:51] jam, there is still the validity problem [08:55] fwereade, I thought you suggested trying first the newest current and then next stable? [08:56] dimitern, I don't *think* I did, but maybe -- did I say why? [08:57] dimitern, I thought the logic was the other way round before, and I was *trying* to suggest just reversing the test but keeping the logic the same, so the preferred branch came first [08:57] fwereade, something about "we should be always able to upgrade to a more recent current version" [08:57] fwereade, ah, ok then - will change it to prefer next stable over current [08:57] dimitern, that should always be a plausible fallback, but I *think* it makes most sense to go as far as we know we can [08:57] fwereade, although it kinda makes sense to prefer current as it is [08:58] dimitern, both do, don't they [08:58] fwereade, that way we can upgrade as far as possible on the current one, and once there's no more recent current we'll automatically choose next stable [08:59] fwereade, seems less intrusive [08:59] dimitern, if you're on 1.17.3.4 and you use "--version 1.17", that'll keep you where you want to be, right? [08:59] dimitern, otherwise I think we want people on the latest stable by default [08:59] * jam goes to get some lunch, back in a bit [09:00] fwereade, ok [09:00] dimitern, cheers [09:00] fwereade: arguably what we want is an index on something.canonical.com that says "if you're at X go to Y" [09:00] and then we can make the jumps whatever we've tested :) [09:01] jam, that'd be a nice argument to have, but I think it's out of scope here [09:02] fwereade: that way if we find a bug in 1.16.2 that prevents upgrading to 1.18.0, we can point you at 1.16.4 before you go to 1.18. But yeah, ESCOPE [09:22] axw: do you have the bug number handy for the config attrs not being deleted? [09:23] wallyworld_: nope, I'll do a quick search [09:23] or i can, was just being lazy :-) [09:23] wallyworld_: #1248809 [09:23] <_mup_> Bug #1248809: state.State.SetEnvironConfig never forgets

[09:23] ta [09:33] fwereade: no worries, we can chat about the bootstrap stuff tomorrow. I'll have to refresh my memory on what I did ;) [09:33] axw, yeah, that was my problem with reviewing it, it demands an awful lot of context [09:34] axw, I've been trying to think of how to split it, though,and it's not easy [09:34] yeah, it's all related :( [09:34] not the monolith I wanted [09:34] dimitern, would you also take a look at https://codereview.appspot.com/14433058/ please? it's related to what you've been doing [09:34] axw, haha [09:36] fwereade, looking [09:44] * TheMue grumbles, has latest changes dislike him [09:48] fwereade: if you didn't see, I simplified (I hope) the destroy-env CL: https://codereview.appspot.com/22870045/ [09:49] it now rejects an attempt to destroy-env while there are manual non-manager machines [10:00] jam: i just posted a query to https://codereview.appspot.com/26430043/ - it looks slightly questionable to me, but that's only from a naive p.o.v. [10:01] axw, ah, thanks, I'll check it out [10:02] fwereade: team meeting [10:02] mgz^^ [10:02] https://plus.google.com/hangouts/_/calendar/bWFyay5yYW1tLWNocmlzdGVuc2VuQGNhbm9uaWNhbC5jb20.09gvki7lhmlucq76s2d0lns804 [10:34] fwereade: jam: if you guys could take another look at my updated merge proposals from yesterday that would be muchly appreciated [10:42] (14:41:31) jam: fwereade: good job sorting out the backporting of destroy-machine --force, your coverletter shows a lot of convoluted steps [10:42] (14:42:01) jam: axw: if you're around for a bit, I'm interested in chatting about the manual teardown stuff [10:42] (I meant them in this channel) [10:42] sure [10:43] axw: so. the question I had was is it possible to have the jujud/juju client generate the teardown when it is time to tear down, rather than at the time we create it [10:43] axw: even once we have this, what happens if we upgrade 1.17 => 1.18 [10:43] could the teardown change? [10:44] jam: ah, right. hmm... not really, because it's an agent-level thing? [10:45] sorry, just a minute [10:46] blast [10:47] jam: I think I misunderstood, I thought you meant from the client side. one of the reasons why it's difficult to do at runtime is because, for example, the juju-db job isn't known to the machine-agent [10:47] what happened to the weekly meeting? [10:47] dimitern: we did it [10:47] Daylightsavings time and all [10:47] ah, sorry [10:47] it was at 10:00 UTC [10:47] we went pretty fast [10:48] jam, did I miss anything important? [10:48] dimitern: we're making you do all the work this week for missing the meeting :) [10:49] jam, sweet :) [10:49] dimitern: https://docs.google.com/a/canonical.com/document/d/1eeHzbtyt_4dlKQMof-vRfplMWMrClBx32k6BFI-77MI not a lot. Mostly talked about release planning [10:49] jam, thanks! [10:51] jam: sorry, family's home - I need to help with the kids [10:51] mind if we chat about it tomorrow? [10:51] axw: I won't be around tomorrow, but you can chat with a different reviewer then :) [10:52] enjoy your family time [10:52] ok [10:52] thanks [10:52] have a nice weekend === gary_poster|away is now known as gary_poster [11:06] jam: https://codereview.appspot.com/26550043/ [11:06] fix for tools-url [11:07] wallyworld_: yeah, I was thinking about the reverse problem, someone used an old 1.16 to bootstrap, and then tries to upgrade to 1.17. They won't have tools-metadata-url set (though I'm guessing your compat code would have migrated it anyway?) [11:08] yeah, the original code did migrate it [11:09] jam: and the previous public-bucket-url deprecation - the bucket value could be removed from config because the tools url that was set up overrode it. but this case is subtley different [11:10] wallyworld_: did you do live testing ? [11:10] yep [11:10] before i did the patch :-) [11:10] well, before i did the tests [11:10] wallyworld_: I would hope that your live testing was of the patch :) [11:10] yeah, i meant i made sure it worked before doing the unit tests [11:25] wallyworld_: if you didn't see the email LGTM [11:25] thanks jam [11:25] doing one final test [11:36] jam: there's a separate issue. if you start a 1.16 system without *any* tools url, then add one to your config (cause 1.17 tools are elsewhere) and try to upgrade, the server side does not find any 1.17 tools. but that's more a separate corner case, not a blocker [11:36] the juju client doesn't complain though when firing off the upgrade [11:36] which it would if the client could not find the 1.17 tools [11:37] wallyworld_: from what I've seen it won't try to find the tools, because it *is* smart enough to ignore your local tools-url and read the remote one. [11:37] wallyworld_: are you sure? [11:37] i am pretty sure [11:37] the code I see in "cmd/juju/upgradejuju.go" first uses conn.State.EnvironConfig() [11:37] it doesn't use the local EnvConfig [11:37] but i may have made a mistake testing [11:39] ffs, i've cleared my scrollback buffer. so can't check for sure without re-testing. will do it tomorrow [11:39] at least curtis' issue is fixed === TheRealMue is now known as TheMue [14:17] do we have godoc output for juju-core available/extant anywhere? [14:19] only insofaras you can build/serve it out of the branch I think, hazmat [14:19] not seperately hosted anywhere [14:19] hmm.. hard to ref the api docs for third parties if we don't publish our docs. [14:21] wallyworld_, jam: did we intentionally change dependencies.tsv to be space delimited, not tab? [14:21] it would be nice to bundle it into a build process for the juju.ubuntu.com docs [14:22] sinzui: looks like it's still tabs to me... [14:23] mgz, wallyworld_'s goyaml line uses spaces! [14:23] ah, that's likely an error from that update then [14:23] CI is down. I think godeps doesn't support spaces either [14:24] it does not. [14:25] I can propose or lgtm a fix for that if it'd help curtis [14:27] mgz, I can propose the fix if you can review it [14:27] fwereade: is this close to your wishes regarding ensure-ha? i've written the docs as if it was completely done, so we have an idea where we're going, although we won't implement all of it to start with (e.g. prefer-machines, no-destroy) [14:27] fwereade: http://paste.ubuntu.com/6416040/ [14:29] hazmat: http://godoc.org/launchpad.net/juju-core [14:30] TheMue: oo, lookit dat [14:31] TheMue, thanks [14:32] TheMue: that seems to be lacking lots of doc things though [14:33] mgz: indeed, we don't have many package docs [14:34] or ah, I see, without enough magic it just omits things. stuff like instance/address.go is actually quite well covered [14:35] mgz: we aren't following convention in a bunxch of things. we should fix that. Godoc is the Go standard [14:38] mgz: what specific things are you thinking we're missing? [14:38] mgz: (doc-wise, that is) [14:40] * rogpeppe1 goes for some lunch [14:46] rogpeppe1, mgz: this is kind of unconscionable: http://godoc.org/launchpad.net/juju-core/cmd/juju [14:47] rogpeppe1: I was just looking at some random packages. the provider ones are pretty sparse [14:52] natefinch, mgz: displaying the import chars is sometimes wild [14:53] eh, s/chars/graphs/ [14:58] * TheMue => lunch [15:00] mgz, TheMue, rogpeppe1: how to do it right: http://godoc.org/github.com/natefinch/gocog :) [15:15] mgz, do you have time to review https://codereview.appspot.com/26370044 [15:29] natefinch: or http://godoc.org/git.tideland.biz/gocn or http://godoc.org/git.tideland.biz/godm etc [15:29] natefinch: ;) [15:32] TheMue: yeah... my specific point was that our command (the juju client executable) doesn't have any godocs. But yes, nicely done. I really like that it's so easy to write docs for Go, and how standard they are, that when they're missing, it's a glaring deficiency. [15:35] natefinch: +1 [15:38] natefinch: agreed. we should probably have some preprocessor that generates godoc output from the standard help commands. [15:39] natefinch: and also there's the fact that packages with a package comment in the right style get rated higher in godoc [15:39] natefinch: which might well be good for our general visibility [15:39] sinzui: approved [15:39] thank you mgz [15:45] fwereade: ping [16:09] rogpeppe1, was ths fix for this bug just to gomass? Could I update the 1.16 branch to use that dep to release a fix? https://bugs.launchpad.net/gomaasapi/+bug/1222671 [16:09] <_mup_> Bug #1222671: Using the same maas user in different juju environments causes them to clash

[16:09] sinzui: looking [16:10] sinzui: do you have a reference for the merge proposal that fixed it? [16:10] rogpeppe1, pong, sorry, happened while I was joining a meeting [16:10] fwereade: np [16:10] rogpeppe1, I don't :( [16:11] sinzui: i'm pretty sure it's a maas-only bug [16:11] fwereade: i wonder if you could pass your eyes over this for sanity checking. i'm trying to stick closely to what you have been suggesting. http://paste.ubuntu.com/6416040/ [16:12] * fwereade looks [16:13] fwereade: that's my take at a complete description. obviously we won't implement it all to start with. [16:16] rogpeppe1, for a complete spec, I think you convinced me that we do want to be able to demote rather than destroy a non-functioning manager [16:16] fwereade: how do you propose we do that? [16:17] rogpeppe1, well, it does irritatingly involve watching jobs -- but if we have prefer-machines we need that anyway [16:17] rogpeppe1, do you mean inside state? [16:17] rogpeppe1, we remove the appropriate jobs from the problematic machine in the same transaction we bring up the new one -- am I being obtuse? [16:18] fwereade: that seems possible, but does mean we have to do dynamic jobs in the machine agent now rather than later [16:19] fwereade: also, it's not clear to me that just because a state server's network connection has been down for more than 3 minutes that we should take it out of action indefinitely [16:19] rogpeppe1, or, at least, the job that maintains mongo needs to keep an eye on whether it still should and exit without error if it doesn't [16:19] rogpeppe1, indeed, this is the source of my resistance to the *automatic* failover of state [16:20] rogpeppe1, I expect some degree of human judgment to be applied [16:20] fwereade: so if you happen to run ensure-ha and a server happens to have been down for more than 3 minutes, then the logic triggers [16:22] rogpeppe1, yes [16:22] fwereade: there's another question: when do we actually add and remove machines to/from the set of mongo peers? [16:24] fwereade, rogpeppe1: it seems like there's a difference between automatic failover and automatic replacement of machines. Failover has to be automatic, otherwise it's useless. But maybe that's just me nitpicking semantics. [16:24] rogpeppe1, as soon as we can, I think? [16:24] rogpeppe1, natefinch: yeah, I used the word "failover" intemperately above [16:24] fwereade: we need an agent to do it, because we can't necessarily do it until the machines come up [16:25] fwereade: ok, I thought so, but just wanted to make sure I wasn't misunderstanding [16:26] fwereade: i'm thinking that it's that agent that can be responsible for being the last word in making sure that mongo has an odd number of peers [16:26] fwereade: the state machines in state are only an aim - they don't necessarily reflect reality [16:27] rogpeppe1, sure -- it's that target set of peers that I want hard guarantees about [16:27] fwereade: right [16:27] rogpeppe1, the fact that the rest of reality has to converge is inescapable [16:27] fwereade: well actually, i think we want guarantees about the *actual* set of peers too, as much as possible [16:28] fwereade: so, for instance, if you start two new state servers and only one comes up, what should we do then? [16:28] fwereade: i'm thinking that we don't add either of them as peers until they both come up (or we might possibly consider adding the first one as non-voting peer) [16:30] rogpeppe1, yeah, the agent's job has subtleties -- there surely is a meaningful distinction between never-came-up and went-down-unexpectedly [16:32]