| davecheney | help, how do I fix this ? | 00:48 |
|---|---|---|
| davecheney | Error details: | 00:48 |
| davecheney | empty tools-url in environment configuration | 00:48 |
| davecheney | well, fuck | 00:50 |
| davecheney | % juju destroy-environment -y ap-southeast-2 | 00:50 |
| davecheney | ERROR empty tools-url in environment configuration | 00:50 |
| wallyworld_ | davecheney: um, not sure. is there a tools-url in the yaml? | 01:34 |
| davecheney | wallyworld_: i fixed it | 01:50 |
| davecheney | my working copy was old | 01:50 |
| wallyworld_ | ah ok | 01:50 |
| davecheney | i got the thing I needed | 01:53 |
| davecheney | https://bugs.launchpad.net/juju-core/+bug/1227952 | 01:53 |
| _mup_ | Bug #1227952: juju get give a "panic: index out of range" error <regression> <goyaml:New> <juju-core:Confirmed for dave-cheney> <https://launchpad.net/bugs/1227952> | 01:53 |
| davecheney | yay yaml | 01:53 |
| wallyworld_ | yay indeed | 01:54 |
| davecheney | ** Bug watch added: Go Issue Tracker #6761 http://code.google.com/p/go/issues/detail?id=6761 | 01:56 |
| davecheney | ^ I didn't know LP did this | 01:56 |
| === gary_poster is now known as gary_poster|away | ||
| davecheney | wallyworld_: http://play.golang.org/p/hedM5ozbo2 | 02:02 |
| davecheney | simple repro | 02:02 |
| wallyworld_ | can't find import: "launchpad.net/goyaml" | 02:03 |
| davecheney | yeah, won't work in the playground | 02:03 |
| davecheney | copy and paste it into a file | 02:03 |
| wallyworld_ | well, that error does indeed suck | 02:04 |
| wallyworld_ | at least it can be produced trivially | 02:05 |
| davecheney | is | 02:05 |
| davecheney | value: - | 02:05 |
| davecheney | valid yaml ? | 02:05 |
| davecheney | wallyworld_: could you try doing the same in python ? | 02:05 |
| wallyworld_ | not sure | 02:05 |
| wallyworld_ | ok | 02:05 |
| davecheney | ta | 02:05 |
| davecheney | wallyworld_: i fixed the bug, tests all pass | 02:11 |
| davecheney | by deleting code | 02:11 |
| davecheney | i'm not sure how gustavo will like that :) | 02:11 |
| wallyworld_ | davecheney: ah, ok. good luck :-) | 02:13 |
| wallyworld_ | davecheney: after i apt get installed a python yaml lib, python does seem to think it is valid fwiw | 02:16 |
| sinzui | davecheney, wallyworld_ We updated charm-tools proof a few weeks ago to check for valid yaml. | 02:16 |
| davecheney | sinzui: do you know if - has to be escaped in yaml ? | 02:17 |
| davecheney | ie | 02:17 |
| davecheney | output-file: - | 02:17 |
| sinzui | davecheney, It must be quoted when you don't mean list item | 02:18 |
| davecheney | right, so the charm is invalid ? | 02:19 |
| davecheney | g: Yes # a boolean True | 02:20 |
| davecheney | ^ oh for fucks sake yaml | 02:20 |
| davecheney | sinzui: i cannot find any think to say that values must be quited | 02:21 |
| davecheney | quoted | 02:21 |
| sinzui | Values do need to be quoted ":" must be in normal string values. dash is accepted at http://yaml-online-parser.appspot.com/ | 02:25 |
| davecheney | sinzui: orly | 02:26 |
| davecheney | i used that validator and it said | 02:26 |
| davecheney | value: - | 02:26 |
| davecheney | sequence entries are not allowed here | 02:26 |
| davecheney | in "<unicode string>", line 1, column 8: | 02:26 |
| davecheney | value: - | 02:26 |
| davecheney | in short | 02:26 |
| davecheney | gotta quote it | 02:26 |
| sinzui | ah | 02:27 |
| sinzui | but it you type | 02:27 |
| sinzui | value: this -that | 02:27 |
| sinzui | I think it is accepted | 02:27 |
| davecheney | yeah | 02:27 |
| davecheney | it's the bare hyphen that needs quoting | 02:27 |
| sinzui | yep | 02:28 |
| sinzui | value: nil | 02:28 |
| sinzui | is a problem not in yaml but is in our goyaml | 02:28 |
| davecheney | wsgi_log_file: | 02:30 |
| davecheney | type: string | 02:30 |
| davecheney | default: "-" | 02:30 |
| davecheney | description: "The log file to write to. If empty the logs would be handle by upstart." | 02:30 |
| davecheney | ^ the original yaml | 02:30 |
| davecheney | ah ha! | 02:30 |
| davecheney | ja'cuse | 02:30 |
| davecheney | this is correct | 02:30 |
| davecheney | when we format the data from juju get $SERVICE | 02:30 |
| davecheney | goyaml cocks it up | 02:30 |
| sinzui | davecheney, I see that wsgi_log_file: "-" is quoted in the current config.yaml | 02:30 |
| sinzui | ah we saw the same thig | 02:31 |
| davecheney | right, fixed | 02:36 |
| davecheney | MP incoming | 02:36 |
| davecheney | https://codereview.appspot.com/26430043 | 02:41 |
| davecheney | oh crap | 02:41 |
| davecheney | it's nearly 2 | 02:41 |
| davecheney | time for lunch | 02:41 |
| bigjools | if I know my charm needs certain constraints, can that be hard-coded in the charm such that juju will pick it up? | 05:31 |
| davecheney | bigjools: nope | 05:32 |
| bigjools | davecheney: bugger. worth a bug? | 05:32 |
| davecheney | sure | 05:32 |
| bigjools | ok ta | 05:33 |
| davecheney | i'm pretty sure this is by design | 05:33 |
| bigjools | oh? | 05:33 |
| bigjools | seems odd to me | 05:33 |
| davecheney | i didn't say it was a sensible design | 05:33 |
| bigjools | heh | 05:33 |
| davecheney | jam: ping | 05:33 |
| jam | davecheney: pong | 05:33 |
| davecheney | jam: you are an owner on goyaml, could I get a review https://codereview.appspot.com/26430043 | 05:33 |
| davecheney | pls | 05:33 |
| jam | 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 |
| bigjools | jam: ok, well I shall file a bug anyway for book-keeping | 05:34 |
| davecheney | jolly good | 05:35 |
| jam | davecheney: so the actual change is just len() ? | 05:35 |
| davecheney | jam: yup, the error was encode was detecting - as the start of a --- documented seperator | 05:36 |
| davecheney | and panicing when it ran off the end of the slice | 05:36 |
| davecheney | an altertive solution would be to delete line 1015-1017 | 05:37 |
| jam | davecheney: so LGTM though if you have a bug related it would be good to associate it. | 05:37 |
| davecheney | bug is linked to the MP | 05:37 |
| jam | ah, it wasn't linked in the codereview which you linked me :) | 05:37 |
| davecheney | sorry | 05:37 |
| davecheney | lbox failed me | 05:37 |
| jam | joys of having double systems | 05:37 |
| davecheney | jam: can you please commit that for me | 05:37 |
| davecheney | i do not have permission | 05:38 |
| davecheney | jam: thanks for committing that | 05:45 |
| jam | davecheney: np, I'm glad I was able to submit it correctly for you :) | 05:47 |
| davecheney | 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 <regression> <goyaml:Fix Committed by dave-cheney> <juju-core:Fix Committed by dave-cheney> <https://launchpad.net/bugs/1227952> | 05:48 |
| jam | There should be a "Target to series" underneath the existing bug targets | 05:48 |
| davecheney | I only get project and distro | 05:49 |
| davecheney | oh no wait | 05:49 |
| davecheney | reloading the page | 05:49 |
| davecheney | does somethine selse | 05:49 |
| jam | so going to the page probalby brought you to the goyaml version | 05:49 |
| davecheney | aaah | 05:50 |
| jam | you want the juju-core version at: https://bugs.launchpad.net/juju-core/+bug/1227952 | 05:50 |
| jam | I think | 05:50 |
| _mup_ | Bug #1227952: juju get give a "panic: index out of range" error <regression> <goyaml:Fix Committed by dave-cheney> <juju-core:Fix Committed by dave-cheney> <juju-core 1.16:Fix Committed> <https://launchpad.net/bugs/1227952> | 05:50 |
| jam | davecheney: so, is an update to the goyaml dependency in the 1.16 branch? | 05:50 |
| jam | (or at least sinzui knows about it?) | 05:50 |
| jam | otherwise I'd mark it "In Progress" | 05:50 |
| davecheney | kk | 05:51 |
| davecheney | yeah, i nede to make sure sinzui knows that deps.tsv needs to be updated | 05:51 |
| davecheney | rogpeppe1: ping, need help with godeps | 05:52 |
| wallyworld_ | 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 |
| wallyworld_ | i didn't think that would happen, but maye it does | 05:54 |
| jam | wallyworld_: AFAIK when we upgrade nothing changes unless you put that code into the new version | 05:55 |
| wallyworld_ | 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 |
| axw | jam: there's a bug (?) in state that means no config attributes ever get removed from state | 05:57 |
| axw | err sorry, wallyworld_ | 05:57 |
| wallyworld_ | that's what i thought | 05:58 |
| wallyworld_ | 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 | 05:58 |
| jam | 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 |
| jam | You probably need a "juju set-environment tools-url=XYZ; juju upgrade-juju" | 06:03 |
| axw | jam, wallyworld_: now that I think of it, this was one of the feature requests from IS at SFO | 06:06 |
| axw | atomic upgrade + set-env | 06:06 |
| jam | axw: I'm pretty sure they wanted it for charms, not for juju as a whole | 06:11 |
| axw | jam: ah yes, sorry, right you are | 06:12 |
| davecheney | https://code.launchpad.net/~dave-cheney/juju-core/161-update-goyaml-godep/+merge/195173 | 06:17 |
| jam | davecheney: I just approved, but you seem to have a debug "fmt.Printf" in there | 06:18 |
| davecheney | jam: good catch | 06:24 |
| davecheney | that diff is dirty | 06:24 |
| davecheney | it shold only be the deps.tsv | 06:24 |
| davecheney | TIL, don't remove files from the bzr commit message, it doesn't care | 06:24 |
| jam | davecheney: no, it doesn't do anything. You have to specify it on the command line | 06:25 |
| jam | the listing in the message is just for you as the writer of the message | 06:25 |
| jam | I can see the draw, though. | 06:25 |
| jam | 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 <ci> <regression> <upgrade-juju> <juju-core:In Progress by wallyworld> <https://launchpad.net/bugs/1250974> | 07:36 |
| jam | 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 |
| jam | If you have an omitted entry, it *doesn't* touch it. (as noted by axw) | 07:37 |
| jam | so when you did patch r2053 | 07:37 |
| jam | you changed tools-url:null into tools-url: "" | 07:37 |
| jam | which means that SetEnvironConfig({stuff, tools-url:null}) would not have touched that field | 07:37 |
| jam | well, I should say | 07:37 |
| jam | SetEnvironConfig({stuff, agent-version:"1.17.0", tools-url:null) | 07:38 |
| jam | but SetEnvironConfig({stuff, agent-version:"1.17.0", tools-url:""}) *does* wipe out the tools-url field. | 07:38 |
| jam | And note, upgrade-juju *does* read the remote environ config | 07:39 |
| jam | with conn.State.EnvironConfig() | 07:39 |
| jam | but that yaml is parsed by the local code | 07:40 |
| jam | so it strips out stuff it doesn't understand | 07:40 |
| jam | 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. | 07:40 |
| dimitern | fwereade, if you're around - https://codereview.appspot.com/25080043/ from yesterday, when you can please | 08:00 |
| wallyworld_ | jam: cause i got errors in some live tests | 08:03 |
| wallyworld_ | oh bugger. dinner ready | 08:04 |
| wallyworld_ | back soon | 08:04 |
| === jtv2 is now known as jtv | ||
| wallyworld_ | 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:14 |
| wallyworld_ | i will test again with that line back in | 08:15 |
| wallyworld_ | 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:16 |
| jam | wallyworld_: yeah. so "set agent-version: NEWVERSION" is "read the config from remote, change that line, and write it back" | 08:17 |
| jam | but the "read from remote" parses it with the local structure | 08:17 |
| jam | write it back only update fields that exist in the thing we are writing | 08:17 |
| wallyworld_ | ok, so the code in trunk should work then | 08:17 |
| jam | well, I could be wrong with the direct DB access | 08:17 |
| jam | well 2053 doesn't | 08:17 |
| jam | wallyworld_: is there a follow up later? | 08:17 |
| wallyworld_ | cause i remove tools-url from the map | 08:17 |
| wallyworld_ | so it should not overwrite the value in env sate | 08:18 |
| jam | wallyworld_: so *if* my understanding of SetEnvironConfig is true, then you're right | 08:18 |
| wallyworld_ | and yet, if i remove the delet(attrs, "tools-url") line it works | 08:18 |
| wallyworld_ | that's the only change i made from trunk before i tested | 08:19 |
| wallyworld_ | i'll test again with trunk | 08:19 |
| wallyworld_ | jam: to answer question above - no follow up yet to 2053 | 08:20 |
| jam | 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:39 |
| wallyworld_ | i'm running another test, will look in more detail at that code in a sec | 08:40 |
| jam | 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:41 |
| wallyworld_ | makes sense, would be more robust | 08:42 |
| fwereade | dimitern, reviewed, basically LGTM, one substantial question | 08:46 |
| fwereade | wallyworld_, jam: is it time to fix SetEnvironConfig? :/ | 08:47 |
| wallyworld_ | perhaps | 08:47 |
| wallyworld_ | just confirming my theory about ause | 08:47 |
| jam | fwereade: well Dimiter is at least fixing Upgrade by going via the API and we can sort of do something better there. | 08:47 |
| wallyworld_ | cause | 08:47 |
| fwereade | jam, yeah, there's SetEnvironAgentVersion already | 08:48 |
| jam | SetEnvironConfig is *almost* just taking a delta and applying it, so we could just make that explicit "UpdateEnvironConfig" and go from there. | 08:48 |
| fwereade | jam, but SetEnvironConfig itself is kinda broken by not-even-design | 08:48 |
| fwereade | jam, there's no way to remove fields but maybe that's not a serious problem? | 08:48 |
| dimitern | fwereade, thanks | 08:48 |
| jam | 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:50 |
| fwereade | jam, yeah, that (probably the former) sounds sane | 08:51 |
| fwereade | jam, there is still the validity problem | 08:51 |
| dimitern | fwereade, I thought you suggested trying first the newest current and then next stable? | 08:55 |
| fwereade | dimitern, I don't *think* I did, but maybe -- did I say why? | 08:56 |
| fwereade | 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 |
| dimitern | fwereade, something about "we should be always able to upgrade to a more recent current version" | 08:57 |
| dimitern | fwereade, ah, ok then - will change it to prefer next stable over current | 08:57 |
| fwereade | 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 |
| dimitern | fwereade, although it kinda makes sense to prefer current as it is | 08:57 |
| fwereade | dimitern, both do, don't they | 08:58 |
| dimitern | 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:58 |
| dimitern | fwereade, seems less intrusive | 08:59 |
| fwereade | 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 |
| fwereade | 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 | 08:59 | |
| dimitern | fwereade, ok | 09:00 |
| fwereade | dimitern, cheers | 09:00 |
| jam | fwereade: arguably what we want is an index on something.canonical.com that says "if you're at X go to Y" | 09:00 |
| jam | and then we can make the jumps whatever we've tested :) | 09:00 |
| fwereade | jam, that'd be a nice argument to have, but I think it's out of scope here | 09:01 |
| jam | 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:02 |
| wallyworld_ | axw: do you have the bug number handy for the config attrs not being deleted? | 09:22 |
| axw | wallyworld_: nope, I'll do a quick search | 09:23 |
| wallyworld_ | or i can, was just being lazy :-) | 09:23 |
| axw | wallyworld_: #1248809 | 09:23 |
| _mup_ | Bug #1248809: state.State.SetEnvironConfig never forgets <state-server> <juju-core:Triaged> <https://launchpad.net/bugs/1248809> | 09:23 |
| wallyworld_ | ta | 09:23 |
| axw | fwereade: no worries, we can chat about the bootstrap stuff tomorrow. I'll have to refresh my memory on what I did ;) | 09:33 |
| fwereade | axw, yeah, that was my problem with reviewing it, it demands an awful lot of context | 09:33 |
| fwereade | axw, I've been trying to think of how to split it, though,and it's not easy | 09:34 |
| axw | yeah, it's all related :( | 09:34 |
| axw | not the monolith I wanted | 09:34 |
| fwereade | 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 |
| fwereade | axw, haha | 09:34 |
| dimitern | fwereade, looking | 09:36 |
| * TheMue grumbles, has latest changes dislike him | 09:44 | |
| axw | fwereade: if you didn't see, I simplified (I hope) the destroy-env CL: https://codereview.appspot.com/22870045/ | 09:48 |
| axw | it now rejects an attempt to destroy-env while there are manual non-manager machines | 09:49 |
| rogpeppe1 | 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:00 |
| fwereade | axw, ah, thanks, I'll check it out | 10:01 |
| jam | fwereade: team meeting | 10:02 |
| jam | mgz^^ | 10:02 |
| jam | https://plus.google.com/hangouts/_/calendar/bWFyay5yYW1tLWNocmlzdGVuc2VuQGNhbm9uaWNhbC5jb20.09gvki7lhmlucq76s2d0lns804 | 10:02 |
| wallyworld_ | fwereade: jam: if you guys could take another look at my updated merge proposals from yesterday that would be muchly appreciated | 10:34 |
| jam | (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 |
| jam | (14:42:01) jam: axw: if you're around for a bit, I'm interested in chatting about the manual teardown stuff | 10:42 |
| jam | (I meant them in this channel) | 10:42 |
| axw | sure | 10:42 |
| jam | 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 |
| jam | axw: even once we have this, what happens if we upgrade 1.17 => 1.18 | 10:43 |
| jam | could the teardown change? | 10:43 |
| axw | jam: ah, right. hmm... not really, because it's an agent-level thing? | 10:44 |
| axw | sorry, just a minute | 10:45 |
| dimitern | blast | 10:46 |
| axw | 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 |
| dimitern | what happened to the weekly meeting? | 10:47 |
| jam | dimitern: we did it | 10:47 |
| jam | Daylightsavings time and all | 10:47 |
| dimitern | ah, sorry | 10:47 |
| jam | it was at 10:00 UTC | 10:47 |
| jam | we went pretty fast | 10:47 |
| dimitern | jam, did I miss anything important? | 10:48 |
| jam | dimitern: we're making you do all the work this week for missing the meeting :) | 10:48 |
| dimitern | jam, sweet :) | 10:49 |
| jam | dimitern: https://docs.google.com/a/canonical.com/document/d/1eeHzbtyt_4dlKQMof-vRfplMWMrClBx32k6BFI-77MI not a lot. Mostly talked about release planning | 10:49 |
| dimitern | jam, thanks! | 10:49 |
| axw | jam: sorry, family's home - I need to help with the kids | 10:51 |
| axw | mind if we chat about it tomorrow? | 10:51 |
| jam | axw: I won't be around tomorrow, but you can chat with a different reviewer then :) | 10:51 |
| jam | enjoy your family time | 10:52 |
| axw | ok | 10:52 |
| axw | thanks | 10:52 |
| axw | have a nice weekend | 10:52 |
| === gary_poster|away is now known as gary_poster | ||
| wallyworld_ | jam: https://codereview.appspot.com/26550043/ | 11:06 |
| wallyworld_ | fix for tools-url | 11:06 |
| jam | 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:07 |
| wallyworld_ | yeah, the original code did migrate it | 11:08 |
| wallyworld_ | 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:09 |
| jam | wallyworld_: did you do live testing ? | 11:10 |
| wallyworld_ | yep | 11:10 |
| wallyworld_ | before i did the patch :-) | 11:10 |
| wallyworld_ | well, before i did the tests | 11:10 |
| jam | wallyworld_: I would hope that your live testing was of the patch :) | 11:10 |
| wallyworld_ | yeah, i meant i made sure it worked before doing the unit tests | 11:10 |
| jam | wallyworld_: if you didn't see the email LGTM | 11:25 |
| wallyworld_ | thanks jam | 11:25 |
| wallyworld_ | doing one final test | 11:25 |
| wallyworld_ | 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 |
| wallyworld_ | the juju client doesn't complain though when firing off the upgrade | 11:36 |
| wallyworld_ | which it would if the client could not find the 1.17 tools | 11:36 |
| jam | 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 |
| jam | wallyworld_: are you sure? | 11:37 |
| wallyworld_ | i am pretty sure | 11:37 |
| jam | the code I see in "cmd/juju/upgradejuju.go" first uses conn.State.EnvironConfig() | 11:37 |
| jam | it doesn't use the local EnvConfig | 11:37 |
| wallyworld_ | but i may have made a mistake testing | 11:37 |
| wallyworld_ | ffs, i've cleared my scrollback buffer. so can't check for sure without re-testing. will do it tomorrow | 11:39 |
| wallyworld_ | at least curtis' issue is fixed | 11:39 |
| === TheRealMue is now known as TheMue | ||
| hazmat | do we have godoc output for juju-core available/extant anywhere? | 14:17 |
| mgz | only insofaras you can build/serve it out of the branch I think, hazmat | 14:19 |
| mgz | not seperately hosted anywhere | 14:19 |
| hazmat | hmm.. hard to ref the api docs for third parties if we don't publish our docs. | 14:19 |
| sinzui | wallyworld_, jam: did we intentionally change dependencies.tsv to be space delimited, not tab? | 14:21 |
| mgz | it would be nice to bundle it into a build process for the juju.ubuntu.com docs | 14:21 |
| mgz | sinzui: looks like it's still tabs to me... | 14:22 |
| sinzui | mgz, wallyworld_'s goyaml line uses spaces! | 14:23 |
| mgz | ah, that's likely an error from that update then | 14:23 |
| sinzui | CI is down. I think godeps doesn't support spaces either | 14:23 |
| mgz | it does not. | 14:24 |
| mgz | I can propose or lgtm a fix for that if it'd help curtis | 14:25 |
| sinzui | mgz, I can propose the fix if you can review it | 14:27 |
| rogpeppe1 | 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 |
| rogpeppe1 | fwereade: http://paste.ubuntu.com/6416040/ | 14:27 |
| TheMue | hazmat: http://godoc.org/launchpad.net/juju-core | 14:29 |
| mgz | TheMue: oo, lookit dat | 14:30 |
| hazmat | TheMue, thanks | 14:31 |
| mgz | TheMue: that seems to be lacking lots of doc things though | 14:32 |
| TheMue | mgz: indeed, we don't have many package docs | 14:33 |
| mgz | or ah, I see, without enough magic it just omits things. stuff like instance/address.go is actually quite well covered | 14:34 |
| natefinch | mgz: we aren't following convention in a bunxch of things. we should fix that. Godoc is the Go standard | 14:35 |
| rogpeppe1 | mgz: what specific things are you thinking we're missing? | 14:38 |
| rogpeppe1 | mgz: (doc-wise, that is) | 14:38 |
| * rogpeppe1 goes for some lunch | 14:40 | |
| natefinch | rogpeppe1, mgz: this is kind of unconscionable: http://godoc.org/launchpad.net/juju-core/cmd/juju | 14:46 |
| mgz | rogpeppe1: I was just looking at some random packages. the provider ones are pretty sparse | 14:47 |
| TheMue | natefinch, mgz: displaying the import chars is sometimes wild | 14:52 |
| TheMue | eh, s/chars/graphs/ | 14:53 |
| * TheMue => lunch | 14:58 | |
| natefinch | mgz, TheMue, rogpeppe1: how to do it right: http://godoc.org/github.com/natefinch/gocog :) | 15:00 |
| sinzui | mgz, do you have time to review https://codereview.appspot.com/26370044 | 15:15 |
| TheMue | natefinch: or http://godoc.org/git.tideland.biz/gocn or http://godoc.org/git.tideland.biz/godm etc | 15:29 |
| TheMue | natefinch: ;) | 15:29 |
| natefinch | 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:32 |
| TheMue | natefinch: +1 | 15:35 |
| rogpeppe1 | natefinch: agreed. we should probably have some preprocessor that generates godoc output from the standard help commands. | 15:38 |
| rogpeppe1 | natefinch: and also there's the fact that packages with a package comment in the right style get rated higher in godoc | 15:39 |
| rogpeppe1 | natefinch: which might well be good for our general visibility | 15:39 |
| mgz | sinzui: approved | 15:39 |
| sinzui | thank you mgz | 15:39 |
| rogpeppe1 | fwereade: ping | 15:45 |
| sinzui | 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 <cts-cloud-review> <maas-provider> <Go MAAS API Library:Fix Committed> <juju-core:Fix Committed by thumper> <https://launchpad.net/bugs/1222671> | 16:09 |
| rogpeppe1 | sinzui: looking | 16:09 |
| rogpeppe1 | sinzui: do you have a reference for the merge proposal that fixed it? | 16:10 |
| fwereade | rogpeppe1, pong, sorry, happened while I was joining a meeting | 16:10 |
| rogpeppe1 | fwereade: np | 16:10 |
| sinzui | rogpeppe1, I don't :( | 16:10 |
| rogpeppe1 | sinzui: i'm pretty sure it's a maas-only bug | 16:11 |
| rogpeppe1 | 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:11 |
| * fwereade looks | 16:12 | |
| rogpeppe1 | fwereade: that's my take at a complete description. obviously we won't implement it all to start with. | 16:13 |
| fwereade | 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 |
| rogpeppe1 | fwereade: how do you propose we do that? | 16:16 |
| fwereade | rogpeppe1, well, it does irritatingly involve watching jobs -- but if we have prefer-machines we need that anyway | 16:17 |
| fwereade | rogpeppe1, do you mean inside state? | 16:17 |
| fwereade | 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:17 |
| rogpeppe1 | fwereade: that seems possible, but does mean we have to do dynamic jobs in the machine agent now rather than later | 16:18 |
| rogpeppe1 | 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 |
| fwereade | 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 |
| fwereade | rogpeppe1, indeed, this is the source of my resistance to the *automatic* failover of state | 16:19 |
| fwereade | rogpeppe1, I expect some degree of human judgment to be applied | 16:20 |
| rogpeppe1 | 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:20 |
| fwereade | rogpeppe1, yes | 16:22 |
| rogpeppe1 | fwereade: there's another question: when do we actually add and remove machines to/from the set of mongo peers? | 16:22 |
| natefinch | 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 |
| fwereade | rogpeppe1, as soon as we can, I think? | 16:24 |
| fwereade | rogpeppe1, natefinch: yeah, I used the word "failover" intemperately above | 16:24 |
| rogpeppe1 | fwereade: we need an agent to do it, because we can't necessarily do it until the machines come up | 16:24 |
| natefinch | fwereade: ok, I thought so, but just wanted to make sure I wasn't misunderstanding | 16:25 |
| rogpeppe1 | 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 |
| rogpeppe1 | fwereade: the state machines in state are only an aim - they don't necessarily reflect reality | 16:26 |
| fwereade | rogpeppe1, sure -- it's that target set of peers that I want hard guarantees about | 16:27 |
| rogpeppe1 | fwereade: right | 16:27 |
| fwereade | rogpeppe1, the fact that the rest of reality has to converge is inescapable | 16:27 |
| rogpeppe1 | fwereade: well actually, i think we want guarantees about the *actual* set of peers too, as much as possible | 16:27 |
| rogpeppe1 | fwereade: so, for instance, if you start two new state servers and only one comes up, what should we do then? | 16:28 |
| rogpeppe1 | 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:28 |
| fwereade | rogpeppe1, yeah, the agent's job has subtleties -- there surely is a meaningful distinction between never-came-up and went-down-unexpectedly | 16:30 |
| rogpeppe1 | fwereade: indeed, and perhaps that's something that needs to be stored in state | 16:32 |
| rogpeppe1 | fwereade: i'm wondering if the agent should announce its state in its machine somehow, so we have a permanent record that it actually came up | 16:33 |
| rogpeppe1 | fwereade: BTW i've discovered that each mongo peer caches the addresses of its peers | 16:33 |
| fwereade | rogpeppe1, that might be sensible, I don;t have a strong opinion at the moment | 16:34 |
| rogpeppe1 | fwereade: which makes life a little easier for us, although we'll still need to cache the API addresses. | 16:34 |
| fwereade | rogpeppe1, ah yes :) | 16:35 |
| rogpeppe1 | fwereade: one possibility is that the machine agent can store (perhaps amongst other info) the port that its mongo server is listening on. that gives us the potential freedom at some point in the future, to host state servers where not all the servers in an environment are listening on the same port. | 16:36 |
| fwereade | rogpeppe1, I'm not sure I see the use case there tbh | 16:37 |
| rogpeppe1 | fwereade: it means that you can host state servers on machines that are hosting other state servers, without having to try to choose an arbitrary port that noone is already using. | 16:38 |
| rogpeppe1 | fwereade: it also means that if someone happens to have a service that's using the juju port, and they want to host a state server on that node, that they can do that | 16:39 |
| rogpeppe1 | fwereade: it seems like a potentially useful piece of flexibility - it's easy to add right now, but perhaps not so easy in the future. | 16:40 |
| rogpeppe1 | fwereade: it's also potentially useful for testing | 16:41 |
| fwereade | rogpeppe1, multiple state servers per machine does not really seem like something we want to encourage... except, hm, indeed, in a testing context | 16:41 |
| rogpeppe1 | fwereade: i don't see why multiple state servers (for different environments) on a single machine should be a particular problem | 16:42 |
| rogpeppe1 | fwereade: are you thinking just from a load perspective? | 16:42 |
| fwereade | rogpeppe1, I'm thinking from an "isn't that just a parallel multitenant implementation" perspective | 16:43 |
| rogpeppe1 | fwereade: well, kinda, but there are various reasons that you might not want them as peers inside the same state server | 16:43 |
| fwereade | rogpeppe1, and that'd be allowed by the env setting anyway, right? | 16:45 |
| rogpeppe1 | fwereade: what would, sorry? | 16:45 |
| fwereade | rogpeppe1, mutiple envs' state servers on the same hardware if people *really* wanted to do that | 16:46 |
| * fwereade quick ciggie before next meeting, response rate will slow down a bit | 16:47 | |
| === rogpeppe1 is now known as rogpeppe | ||
| TheMue | fwereade: thx for your review, I think I've got now covered most of it | 17:53 |
| TheMue | fwereade: next step is live testing | 17:53 |
| rogpeppe | natefinch: here's a collection of slightly more thought-through details on the ensure-ha stuff - just the parts that are directly concerned with starting and stopping state servers. http://paste.ubuntu.com/6417151/ | 18:29 |
| rogpeppe | fwereade: you might want to take a look at the above for sanity checking | 18:29 |
| rogpeppe | and with that i must heed the call from downstairs | 18:30 |
| rogpeppe | g'night all | 18:30 |
| natefinch | rogpeppe: where did prefer-machines come from? That seems too... squishy. Like "Hey, you know, if you feel like, try to get the state servers over there, if you don't mind." | 18:30 |
| natefinch | rogpeppe: night, I'll read up | 18:30 |
| rogpeppe | natefinch: it was fwereade's suggestion | 18:30 |
| natefinch | rogpeppe: I'll complain to him then ;) | 18:30 |
| rogpeppe | natefinch: i'm trying very hard to be non-controversial | 18:30 |
| rogpeppe | natefinch: yes | 18:30 |
| rogpeppe | natefinch: g'night | 18:31 |
| sinzui | natefinch, do you have a few minutes to review https://codereview.appspot.com/24280044 | 18:50 |
| natefinch | sinzui: sure thing. I like the easy ones | 18:52 |
| natefinch | sinzui: looks kinda buggy, but I guess I can give it the LGTM | 18:53 |
| sinzui | it is? | 18:53 |
| natefinch | sinzui: a joke. It's two characters, changing a 3 to 4 :) | 18:53 |
| sinzui | natefinch, I am humour challenged today. I saw CI make a 1.16.3 release thought, oops, they better not ever get out into the wild | 18:54 |
| natefinch | sinzui: ahh, apologies. Glad it got caught. | 18:58 |
| === gary_poster is now known as gary_poster|away | ||
| davecheney | rogpeppe1: good news! | 22:38 |
| davecheney | the reflect issue in gccgo isn't as bad as everyone things | 22:38 |
| rogpeppe1 | davecheney: i heard that | 22:39 |
| rogpeppe1 | davecheney: only struct{} | 22:40 |
| davecheney | yup | 22:41 |
| davecheney | it will be straight forward to fix | 22:41 |
| axw__ | fwereade: a bit late your time? do you want to chat your morning instead? | 22:41 |
| === axw__ is now known as axw | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!