| thumper-afk | dpb1: yes, you have the wrong mongodb | 00:13 |
|---|---|---|
| === thumper-afk is now known as thumper | ||
| thumper | sinzui: yay | 00:14 |
| thumper | sinzui: I wonder if the hp upgrade failure was to do with the security group changes | 00:14 |
| thumper | sinzui: it is the only thing that has changed and is different from azure/ec2 | 00:14 |
| thumper | sinzui: this occurred to me on the drive up the hiill | 00:14 |
| thumper | dpb1: can you do apt-cache policy mongodb? | 00:17 |
| * thumper -> fud | 00:29 | |
| davecheney | dpb1: you do not have the correct versino of mongo installed | 00:34 |
| davecheney | did ppa:juju/stable get added during cloud init | 00:34 |
| davecheney | check | 00:34 |
| davecheney | /var/log/cloudinit-output.log | 00:35 |
| davecheney | please | 00:35 |
| fwereade | aw ffs how is it 2:45 | 00:42 |
| axw | whoa, you're still awake :) | 00:43 |
| fwereade | axw, didn't really intend to be, but I started programming | 00:43 |
| * axw knows how that can be | 00:44 | |
| fwereade | axw, so I still haven't done your reviews, you may have to ask thumper | 00:44 |
| axw | fwereade: okey dokey | 00:44 |
| axw | fwereade: if I'm stuck, I'll start looking at the environ updater thing | 00:45 |
| fwereade | axw, please quickly check with thumper & sinzui if there's anything we need to do with azure simplestreams fallbacks -- but I see an "azure upgrade PASS" above so you might not need to | 00:46 |
| axw | fwereade: ok | 00:46 |
| sinzui | fwereade, azw: azure is 100% happy when the image-metadata-url is set | 00:47 |
| fwereade | sinzui, ah ok -- but still screwed if not? | 00:47 |
| * thumper has some dots in the middle of his vision... | 00:48 | |
| sinzui | thats right | 00:48 |
| * thumper tries to read around the floating dots | 00:48 | |
| thumper | I'm just looking at fixing the file permissions on the $(JUJU_HOME)/environments dir and files | 00:49 |
| fwereade | axw, ok, I think unfucking the fallbacks is more important | 00:49 |
| thumper | then I can look at the image-metadata-url | 00:49 |
| fwereade | axw, but I actually must sleep | 00:49 |
| axw | fwereade: no worries, I'll have a look | 00:49 |
| fwereade | axw, thumper has context :) | 00:50 |
| davecheney | dpb1: ping | 00:50 |
| fwereade | axw, thumper: and I'd love a review of https://codereview.appspot.com/14254043 if you get a moment | 00:51 |
| * thumper nods | 00:51 | |
| axw | fwereade: sure, will have a look later today | 00:51 |
| * sinzui reboots to purge sshuttle non-sense | 00:52 | |
| === hazmat` is now known as hazmat | ||
| hazmat | sinzui, iptables -F -X and --policy to reset | 01:08 |
| sinzui | super. Thanks! | 01:08 |
| hazmat | sinzui, http://ubuntuforums.org/showthread.php?t=1381516 | 01:08 |
| axw | sinzui: is there a bug for this image-metadata-url thing? I can't find anything in LP | 01:14 |
| sinzui | axw: no. my bad. | 01:15 |
| sinzui | axw I can report it now | 01:15 |
| axw | sinzui: if you don't mind, I don't know what the issue is that's all | 01:15 |
| axw | sinzui: I'm getting "The affinity group name is empty or was not specified. (http code 400: Bad Request)" when I try to bootstrap | 01:16 |
| axw | is that the issue, or something new? :) | 01:16 |
| sinzui | different | 01:16 |
| sinzui | axw, When the azure provider goes looking for the os image, it bails early wne it does not find a match in the "daily" stream set | 01:17 |
| axw | ok | 01:17 |
| sinzui | axw: this is all the info I have https://bugs.launchpad.net/juju/+bug/1233924 | 01:26 |
| _mup_ | Bug #1233924: cannot bootstrap azure because no OS image found <azure> <bootstrap> <juju-1.15.0> <juju:Triaged> <https://launchpad.net/bugs/1233924> | 01:26 |
| axw | sinzui: thanks! | 01:26 |
| sinzui | axw, one moment, I need to move that bug to juju-core. | 01:27 |
| axw | sinzui: I'm having trouble getting *anything* to work on azure at the moment, so may take me a little while to get to it... | 01:27 |
| sinzui | axw: my setup is just this https://juju.ubuntu.com/docs/config-azure.html with the addition on tools-url: https://jujutools.blob.core.windows.net/juju-tools/tools | 01:30 |
| axw | sinzui: likewise, tho I am using the Southeast Asia location | 01:32 |
| axw | hmm maybe if I set tools-url.. | 01:32 |
| axw | nope | 01:33 |
| sinzui | axw, I think you should try West US. The other bug you found is about not being able to deploy from any other region/affinity | 01:33 |
| axw | sinzui: this was working before | 01:34 |
| sinzui | Then I hope it will work again | 01:34 |
| dpb1 | davecheney: | 01:40 |
| dpb1 | sup? | 01:40 |
| axw | sinzui: ok, reproduced the 1233924 on West US. I'll log a bug for the other one later, if I can reproduce it in a fresh env | 01:45 |
| sinzui | fab | 01:45 |
| davecheney | dpb1: you had questions ? | 01:49 |
| davecheney | thumper: what the balls ?! | 02:08 |
| thumper | what balls? | 02:08 |
| davecheney | ip-10-240-27-224:2013-10-02 02:07:08 INFO juju.worker.uniter uniter.go:105 unit "w1/0" shutting down: tomb: dying | 02:09 |
| davecheney | ip-10-240-27-224:2013-10-02 02:07:08 ERROR juju runner.go:211 worker: exited "uniter": permission denied | 02:09 |
| davecheney | ip-10-240-27-224:2013-10-02 02:07:08 INFO juju runner.go:245 worker: restarting "uniter" in 3s | 02:09 |
| davecheney | what the hell happened here | 02:09 |
| davecheney | all I did was remove the w1 m1 relation | 02:09 |
| * thumper shrugs | 02:13 | |
| davecheney | https://bugs.launchpad.net/juju-core/+bug/1233936 | 02:15 |
| _mup_ | Bug #1233936: worker/uniter: uniter restarts when relation removed <juju-core:New> <https://launchpad.net/bugs/1233936> | 02:15 |
| davecheney | https://bugs.launchpad.net/juju-core/+bug/1233938 | 02:17 |
| _mup_ | Bug #1233938: cmd/juju: debug-log lines appear out of order <juju-core:Triaged> <https://launchpad.net/bugs/1233938> | 02:17 |
| davecheney | aaand that is enough bugs for the morning | 02:17 |
| thumper | davecheney: dude, rsyslog interleving messages from remote clients may well not have them in strict time order | 02:20 |
| thumper | davecheney: and, seriously, not critical | 02:22 |
| thumper | critical means "down tools and fix right now" | 02:22 |
| thumper | which this certainly isbn't | 02:22 |
| thumper | axw: https://codereview.appspot.com/14258043/ | 02:32 |
| thumper | axw: and lets skip the package review | 02:33 |
| axw | thumper: sgtm | 02:33 |
| thumper | axw: did you want to chat through the fallback mechanism? | 02:33 |
| axw | thumper: I've got a fix for the azure thing, just writing a test | 02:33 |
| thumper | axw: is the the "to maintain existing behaviour, don't fall back" ? | 02:33 |
| axw | thumper: uhh maybe we should chat, not sure what you mean | 02:34 |
| thumper | :) | 02:34 |
| axw | thumper: https://plus.google.com/hangouts/_/e2ed2f3559f1cd0314405f2303d3c222e83803c4?authuser=1&hl=en-GB | 02:35 |
| * thumper -> coffee time | 02:43 | |
| axw | axw -> hammer time | 02:43 |
| dpb1 | error command line: unknown option sslOnNormalPorts | 03:10 |
| dpb1 | davecheney: getting that error when juju-db is starting on a maas node. Wrong version of mono being used? | 03:11 |
| dpb1 | why do I keep doing that. | 03:14 |
| dpb1 | *mongo | 03:14 |
| thumper | dpb1: yes, wrong mongo | 03:15 |
| thumper | <davecheney> dpb1: you do not have the correct versino of mongo installed | 03:15 |
| thumper | <davecheney> did ppa:juju/stable get added during cloud init | 03:15 |
| thumper | <davecheney> check | 03:15 |
| thumper | <davecheney> /var/log/cloudinit-output.log | 03:15 |
| thumper | <davecheney> please | 03:15 |
| davecheney | dpb1: can you provide /var/log/cloudinit-output.log | 03:15 |
| davecheney | that will answer all the questions | 03:16 |
| dpb1 | thumper: how is that happening? | 03:16 |
| dpb1 | ya sec, checking | 03:16 |
| davecheney | dpb1: apt-add-reposutory ppa:juju/stable is failing | 03:16 |
| davecheney | ^ best guess | 03:16 |
| dpb1 | d'oh, right at the top | 03:17 |
| dpb1 | 2013-10-01 16:26:17,159 - cc_apt_update_upgrade.py[WARNING]: Source Error: ppa:juju/stable:add-apt-repository failed | 03:17 |
| davecheney | dpb1: does this maas environment have access to the interwebs ? | 03:17 |
| dpb1 | kind of a sneaky little error there though | 03:17 |
| dpb1 | no... it's behind a firewall, this is for the OIL lab if you have heard of that effort | 03:17 |
| dpb1 | but I can selectively allow things through, and have. probably something with a keyserver. | 03:18 |
| davecheney | dpb1: you could try the apt-squid-proxy | 03:18 |
| davecheney | if you do that | 03:18 |
| davecheney | you need to configure it on the machine that initiates juju bootstrap | 03:18 |
| davecheney | (the client) | 03:18 |
| davecheney | which is automagically sniff the settings | 03:18 |
| dpb1 | davecheney: davecheney even keys? I think keyserver.ubuntu.com still needs to be faked | 03:19 |
| davecheney | dpb1: never had to do that | 03:19 |
| dpb1 | ok | 03:19 |
| dpb1 | I'll run it by is in the morning. thanks, that gives me what I need to go on next. :) | 03:20 |
| axw | thumper: https://codereview.appspot.com/14218044/ when you're free | 03:29 |
| thumper | axw: ack | 03:35 |
| thumper | axw: https://codereview.appspot.com/14260043, and I'll review yours before getting back to the other. | 03:49 |
| axw | thumper: okey dokey | 03:49 |
| axw | thumper: I don't suppose you'll have time to review null provider stuff today? | 03:49 |
| thumper | sure, after these two :) | 03:49 |
| thumper | I HATE that we match log messages | 03:50 |
| thumper | I think it is a FAIL on so many levels | 03:50 |
| axw | thumper: I was going for expediency, but I do agree | 03:50 |
| axw | I would prefer a dummy data-source that checked it was called | 03:50 |
| thumper | axw: I understand | 03:50 |
| thumper | not critical of this in particular, just that it exists | 03:50 |
| thumper | axw: what needs the most urgent review? | 04:11 |
| axw | thumper: https://code.launchpad.net/~axwalk/juju-core/null-provider-customsources/+merge/187978 | 04:12 |
| axw | thumper: then this: https://code.launchpad.net/~axwalk/juju-core/null-provider-storage-auth/+merge/187964 | 04:12 |
| thumper | ack | 04:12 |
| axw | lunch, bbs | 04:28 |
| thumper | axw: https://codereview.appspot.com/14258043/ | 04:30 |
| thumper | axw: looking at "Wire up authenticating httpstorage", if you are back, I'd like to chat, if not I'll continue with the review | 04:35 |
| axw | thumper: I'm here | 04:49 |
| thumper | axw: do we care that we are putting the auth token in the url? | 04:50 |
| axw | thumper: shouldn't do, it's HTTPS | 04:51 |
| thumper | always? | 04:51 |
| axw | thumper: should be, let me just double check... | 04:53 |
| axw | thumper: yep. only if you use ClientTLS, then it'll send an auth-key. and it'll only send it for modifying requests | 04:54 |
| axw | thumper: modifying requests always go via HTTPS | 04:54 |
| thumper | kk | 04:54 |
| axw | thumper: did you want to hangout, or was that all you wanted to ask? | 04:55 |
| thumper | I was going to ask about the boilerplate additions | 04:56 |
| thumper | those are being moved into this extra env.jenv files in $(JUJU_HOME)/environments | 04:57 |
| thumper | have you considered that? | 04:57 |
| thumper | or is it deferred | 04:57 |
| thumper | axw: also, for null provider, under what circumstances would the storage certs not be there? | 04:58 |
| axw | thumper: I'm not really abreast of what I need to do for env.jenv | 04:58 |
| thumper | axw: ok, we'll defer that bit | 04:59 |
| axw | thumper: the storage certs are the same as those used for mongo | 05:00 |
| axw | the CA cert is I mean | 05:00 |
| thumper | axw: so surely by the time we want to use them, if they aren't there, we should be erroring? | 05:01 |
| thumper | just thinking about the null provider Storage method | 05:01 |
| axw | thumper: yeah, except Storage() doesn't reutrn an error | 05:01 |
| thumper | and the just use http if the certs aren't there | 05:01 |
| axw | I could have it panic | 05:01 |
| thumper | is it an error for them to be not set? | 05:01 |
| axw | thumper: that's what it's doing now | 05:01 |
| thumper | I think it is by the time it gets there | 05:02 |
| * axw checks | 05:02 | |
| axw | thumper: it's not an error in config.Validate | 05:03 |
| axw | bootstrap ensures there's one tho | 05:03 |
| * thumper nods | 05:03 | |
| thumper | I think the validate bits are not errors for testing :) | 05:03 |
| axw | ugh, I hate that | 05:04 |
| axw | it's one thing to mock something out, but to change all behaviour to accommodate testing... | 05:04 |
| thumper | review done | 05:08 |
| * thumper called for dinner | 05:08 | |
| axw | thumper: thanks! | 05:08 |
| axw | if you're around later, I updated this: https://codereview.appspot.com/14218044/ | 05:08 |
| jam | axw: I think your fix is actually incorrect | 06:24 |
| jam | what we *want* is to use the released steram | 06:24 |
| jam | stream | 06:24 |
| jam | by default | 06:24 |
| jam | axw: so "provider/azure/environ.go" should be "http://cloud-images.ubuntu.com/released" | 06:25 |
| axw | jam: ah, ok | 06:25 |
| jam | axw: we might *also* need to do your fix | 06:25 |
| jam | for whatever reason | 06:25 |
| axw | well, either way my change makes the simplestreams code do what it's meant to do | 06:25 |
| axw | I can update it to use released first tho, if that's what should be done | 06:26 |
| jam | but the fix for bug #1233924 is that we should point to the released stream by default | 06:26 |
| _mup_ | Bug #1233924: cannot bootstrap azure because no OS image found <azure> <bootstrap> <juju-1.15.0> <juju-core:In Progress by axwalk> <https://launchpad.net/bugs/1233924> | 06:26 |
| jam | axw: I'm testing now, but I think this is fallout from before. We used to only have dailies for Azure | 06:26 |
| jam | and then you would specify "image-stream: daily" | 06:26 |
| jam | and it would use the daily cloud-url | 06:26 |
| jam | but I think we just want to ignore that dailies exist | 06:27 |
| jam | and if someone wants to try really hard they can set their imagemetadata-url to the daily stream | 06:27 |
| axw | jam: ok. that sounds sensible | 06:28 |
| jam | axw: it is a shame that they can't just set "image-stream: daily" and have it switch the metadata for you, but I think we can punt on that for now | 06:28 |
| axw | jam: there's two different issues tho, so I'll create a new MP that fixes this in the proper way | 06:28 |
| axw | jam: ah, you can't do that anymore? | 06:29 |
| jam | axw: so there are 2 options | 06:29 |
| jam | 1) we always only search cloud-images.ubuntu.com/released | 06:29 |
| jam | 2) we also add a baseURL for cloud-images.ubuntu.com/daily | 06:29 |
| jam | which 99% of the time won't have what you want | 06:29 |
| jam | but *if* you set "image-stream: daily" then /released doesn't have what you want but /daily does | 06:30 |
| jam | my above point is that (2) can be taken care of by a *user* setting imagemetada-url: *and* image-steram | 06:30 |
| jam | image-stream | 06:30 |
| jam | axw: does that make sense? | 06:30 |
| axw | jam: yep | 06:31 |
| jam | the reason they have to set 2 things, is that the files they need to *read* are in a different location, and the index key *in those files* is also different | 06:31 |
| jam | I'd like to unify it, but I think it is something users won't really use | 06:31 |
| jam | and not worth our time today | 06:31 |
| jam | axw: have you bootstrapped on azure before? | 06:31 |
| jam | I did just the 1-line fix to point at '/released' | 06:32 |
| jam | and it has gotten to "finding products at path" ... | 06:32 |
| jam | but it seems to be stuck | 06:32 |
| jam | ah ffs | 06:32 |
| jam | the URL is "/releases"b | 06:33 |
| jam | but the data inside the file is "released" | 06:33 |
| axw | jam: yes I have bootstrapped before | 06:33 |
| jam | http://cloud-images.ubuntu.com/releases/streams/v1/com.ubuntu.cloud:released:azure.json | 06:33 |
| jam | notice the "releases" at the beginning and the "released" at the end | 06:34 |
| axw | jam: I think you don't want to put "releases" on at all | 06:34 |
| axw | no prefix | 06:34 |
| axw | just .com/streams | 06:34 |
| jam | axw: maybe, but looking at http://cloud-images.ubuntu.com/daily/ | 06:34 |
| jam | to differentiate it from http://cloud-images.ubuntu.com/releases/ | 06:34 |
| jam | axw: DefaultBaseURL = "http://cloud-images.ubuntu.com/releases" | 06:35 |
| axw | hrm yeah... | 06:35 |
| jam | axw: ah you know what, you might be right | 06:35 |
| axw | I'm thinking of image-stream I think | 06:35 |
| jam | that azure is providing an optional location | 06:35 |
| jam | and we should be finding the real fallback for all clouds | 06:35 |
| jam | axw: in which case, we probably just need your fix | 06:36 |
| jam | as in, it is finding daily it appears configured correctly, but doesn't have the data we want, so we need to keep searching. | 06:36 |
| jam | I'm wondering if we just remove daily from baseURLs | 06:36 |
| axw | jam: well, my fix does actually work. but I kinda get your point about not wanting to go to daily all the time | 06:36 |
| axw | yeah that's what I was thinking | 06:36 |
| jam | axw: I think it is silly to look at daily for most people | 06:37 |
| jam | It isn't terribly harmful (with your fix :) but it isn't useful and adds roundtrips to bootstarp | 06:37 |
| jam | bootstrap | 06:37 |
| jam | axw: I'm trying to look at the logic just before what you change | 06:39 |
| jam | changed | 06:39 |
| jam | namely, it has "if err != nil && len(items) == 0 && ..." | 06:39 |
| jam | how would we have items and an error? | 06:39 |
| jam | that sure seems like it should be | 06:40 |
| jam | if (err != nil || len(items) == 0) | 06:40 |
| axw | jam: hmm yes, I think you're right | 06:40 |
| jam | axw: anyway, it doesn't really change what you need to do | 06:41 |
| jam | which is, simplestreams was expecting that it would get an error if nothing matchedb | 06:41 |
| jam | axw: actually, we are supposed to | 06:41 |
| jam | if len(matches) ==0 { return nil, newNoWatchingProductsError} | 06:41 |
| axw | jam: not even; the function directly below GetMetadata (which calls it) has a comment saying that err == nil if no matches | 06:41 |
| axw | that's internal stuff anyway | 06:42 |
| jam | axw: ah, I see, it traps that error and says the higher up will go to the next, but the higher up doesn't because it has a "if err != nil { break }" | 06:42 |
| axw | yup | 06:42 |
| jam | axw: interestingly if we *did* return the error | 06:42 |
| jam | then the higher up code would have continued properly | 06:43 |
| axw | heh yes :) | 06:43 |
| jam | axw: so I think it is a case of "ok, loop until we don't get an error, and the lowest level has an error if nothing matches" and forgetting that the intermediate step filters that error out | 06:43 |
| jam | axw: so offhand, I would be fine with either fix | 06:44 |
| jam | return the error | 06:44 |
| jam | or trap for empty item list | 06:44 |
| jam | I *think* we want to log that we didn't find anything, and just continue | 06:44 |
| jam | and returning it as an error might cause us to double log it | 06:44 |
| jam | but it might also give us a better final message when you don't run with "--debug" | 06:45 |
| jam | axw: can you try both and see if there is a clear win? | 06:45 |
| axw | jam: sure, I'll give it a shot | 06:45 |
| axw | jam: hmm, if GetMetadata is changed to not return the error, it's going to require a lot of changes in callers I think. | 06:47 |
| axw | seems reasonable for it to return an error if it doesn't find anything from any of the sources | 06:47 |
| axw | oh but... | 06:48 |
| axw | it's nil | 06:48 |
| axw | never mind :) | 06:48 |
| jam | axw: my point was, rather than putting "len(items) == 0" before breaking | 06:49 |
| jam | change the function it calls to *not* filter out the noMatchingProductsError | 06:49 |
| jam | axw: for context, rev 1878.1.1 was where Ian introduced GetMetadata | 06:50 |
| jam | and inverted the logic about how to handle noMatchingProductsError | 06:50 |
| jam | axw: it *used* to be that we would search across all data sources | 06:50 |
| jam | for signed data | 06:50 |
| jam | and then all data sources for unsigned data | 06:50 |
| jam | axw: and note that environs/tools/simplestreams.go Fetch() has: | 06:52 |
| jam | if (err != nil || len(items) == 0) && !onlySigned | 06:52 |
| jam | axw: which is why it used to work | 06:52 |
| jam | axw: have I succeeded in confusing you? Because I think I did make sense of it | 06:52 |
| axw | jam: slightly ;) | 06:53 |
| axw | so in other words, this logic used to be outside | 06:53 |
| axw | and it used to check len(items) as well as err | 06:53 |
| axw | when going to the next source | 06:53 |
| axw | ? | 06:53 |
| jam | axw: so it used to be we would return nil, [] so that we would try the unsigned metadata | 06:54 |
| jam | but the len(items) == 0 is still only used for the signed/unsigned check and not the rest | 06:54 |
| jam | axw: *however* the old code had the same bug | 06:55 |
| jam | because err.noMatchingProductsError would break out of the search loop | 06:55 |
| jam | so if it succeeded in reading any stream | 06:55 |
| jam | but just didn't find anything | 06:55 |
| jam | it would stop searching | 06:55 |
| jam | (bzr revert -r 1878; and inspect environs/simplestreams/simplestreams.go line 419) | 06:55 |
| axw | ta | 06:56 |
| jam | axw: and I might understand why | 06:56 |
| jam | axw: so imagine you put data into your private bucket | 06:56 |
| jam | and then ask to boostrap | 06:56 |
| jam | do you want it to read that data, and then say "oh, I didn't find what you requested in your personal store, I'll just go read cloud-images.ubuntu.com" | 06:57 |
| jam | or do you want it to fail right away with "your request couldn't be fulfilled with the data you provided" | 06:57 |
| jam | this is the sort of "either way could be valid" | 06:57 |
| axw | urgh, yeah I can see that | 06:57 |
| jam | Either could be a WTF for the user | 06:58 |
| jam | Why did you start something when I asked you to use my personal image | 06:58 |
| jam | vs | 06:58 |
| jam | Why is this failing to start. | 06:58 |
| jam | So I'm *tempted* to just nuke the daily stream :) | 06:58 |
| jam | And then talk about it in depth as a team | 06:58 |
| axw | jam: yeah that's fair enough, I hadn't considered private cloud metadata | 06:58 |
| jam | axw: does that sound reasonable to you? | 06:58 |
| jam | The person who thought about it the most is on vacation this week (Ian) | 06:59 |
| axw | heh yeah :) | 06:59 |
| jam | fwereade: are you around ? | 06:59 |
| jam | axw: so I know the reason we wanted the shortcut-to-failure was because originally we had a different bug because of the Signed first then Unsigned | 07:00 |
| jam | which is that users can only really upload unsigned metadata | 07:00 |
| jam | and if we read all signed metadata firs | 07:00 |
| jam | then it would have fallen back to cloud-images.ubuntu.com (the only source for signed data) and users could never override it | 07:00 |
| jam | we switched it to search each location signed then unsigned | 07:01 |
| axw | yeah that bit makes sense | 07:01 |
| jam | but we need to figure out what helps users the most | 07:01 |
| jam | falling back so that when you ask for an armhf but have only configured amd64 in your images | 07:01 |
| jam | we still give you the cloud-images instance | 07:01 |
| jam | or not falling back so that we don't accidentally give you the non-customized image if your constraint accidentally doesn't match | 07:02 |
| jam | axw: either way, current logic is just broken for Azure since it is injecting a source that won't ever have released data in it | 07:02 |
| axw | jam: I think we should just do what you said; take out daily, and people can put in the stream if they really do want it | 07:03 |
| axw | then we can revisit this when Ian comes back | 07:03 |
| jam | axw: yeah, I still think we need to decide whether we want to fallback or not. but we can address it separately that way | 07:03 |
| axw | I'll do that now | 07:03 |
| jam | fwereade: when you get in, I'd like to G+ with you about simplestreams fallback-or-not I can give you context when you get in | 07:03 |
| jam | axw: fwiw right now I think I'd rather fallback and give users something than just fail, so I'm happy you wrote the patch, but I want to make sure we understand it as a team | 07:04 |
| axw | jam: no worries. I'll leave that MP there, and do a new one that just touches azure | 07:05 |
| jam | axw: afaik Azure was the only one that ever actually supported the daily stream, and they only did because they had to (there were originally no full releases for azure) | 07:06 |
| jam | axw: and I'm pretty sure on it, because Azure is the only one that supports "image-stream:" and you can't *read* the daily cloud images without that (because they use a different simplestreams key). | 07:09 |
| jam | axw: and I'm pretty sure on it, because Azure is the only one that supports "image-stream:" and you can't *read* the daily cloud images without that (because they use a different simplestreams key). | 07:10 |
| jam | welcome back, btw, | 07:10 |
| axw | jam: my laptop just froze | 07:10 |
| axw | ta | 07:10 |
| axw | jam: last line in my log is from me, saying "jam: no worries..." | 07:11 |
| jam | axw: so I'm thinking we just remove all the daily support from the Azure provider | 07:12 |
| jam | none of the other ones ever supported it | 07:12 |
| jam | because you have to have "image-stream:" in order to actually read the daily streams | 07:12 |
| jam | and we *had* to do it back in the da | 07:12 |
| jam | day | 07:12 |
| jam | because there were only Daily Saucy images being published to Azure | 07:12 |
| axw | jam: sgtm, that was just while saucy was in development afaik | 07:12 |
| jam | now that we have official LTS releases | 07:12 |
| axw | *nods* | 07:12 |
| jam | we just treat Azure like the others | 07:12 |
| axw | er 12.04.03 I mean | 07:12 |
| axw | yep | 07:13 |
| axw | jam: so does that mean taking out image-stream altogether? | 07:13 |
| jam | axw: I think so, because it isn't a *juju* supported config | 07:13 |
| jam | so the overhead of making sure it works for just one provider | 07:14 |
| jam | seems a bit much | 07:14 |
| jam | I can see a point to having it, but if we wanted it, we should have it everywhere | 07:14 |
| axw | jam: can we defer that change? or are you concerned that people will rely on it in 1.16.0? | 07:15 |
| jam | axw: defer away, I'm just thinking out loud | 07:16 |
| jam | it is the natural follow on to removing daily from the search path | 07:16 |
| rogpeppe | mornin' all | 07:36 |
| rogpeppe | dimitern: i found out a little bit more about my dns name query problem BTW | 07:36 |
| axw | morning rogpeppe | 07:38 |
| rogpeppe | axw: hiya | 07:38 |
| rogpeppe | axw: how much do you know about dns? | 07:38 |
| rogpeppe | davecheney: ^ | 07:40 |
| * rogpeppe reboots | 07:46 | |
| axw | jam: https://codereview.appspot.com/14266043 | 07:55 |
| axw | rogpeppe> axw: how much do you know about dns? | 07:55 |
| axw | <axw> rogpeppe: not a heap, why? | 07:55 |
| jam | axw: he was just rebooting just before you got back | 07:57 |
| jam | he was saying something about having a dns name query problem | 07:57 |
| axw_ | grr, net keeps going up and down | 07:59 |
| dimitern | rogpeppe, hey | 08:01 |
| rogpeppe | dimitern: hiya | 08:01 |
| jam | axw_: reviewed | 08:01 |
| dimitern | rogpeppe, so what was it? | 08:01 |
| rogpeppe | dimitern: the dns client is doing two lookups, one for an A (ipv4) record, which returns immediately, but another for an AAAA (ipv6) record, which always times out | 08:02 |
| rogpeppe | dimitern: i'm not sure where the problem is - probably in my ISP's DNS server | 08:02 |
| rogpeppe | dimitern: i can't currently work out a way of disabling the ipv6 request | 08:03 |
| axw_ | jam: ta | 08:03 |
| === axw_ is now known as axw | ||
| dimitern | rogpeppe, I think you can tweak the sysctl.d or something to disable ipv6 on an interface | 08:03 |
| rogpeppe | dimitern: i've already done that - it doesn't seem to make a difference | 08:04 |
| jam | rogpeppe: http://askubuntu.com/questions/32298/prefer-a-ipv4-dns-lookups-before-aaaaipv6-lookups ? | 08:04 |
| jam | says you can tweak /etc/gai.conf | 08:04 |
| jam | rogpeppe: are you using v6 on your local network ? | 08:05 |
| rogpeppe | jam: no, i don't think so | 08:06 |
| rogpeppe | jam: afaic the gai.conf suggestion will only affect the order of the requests, not whether AAAA requests are issued | 08:06 |
| rogpeppe | afaics | 08:06 |
| jam | rogpeppe: the link above mentions adding "options single-request" to /etc/resolve.conf | 08:07 |
| jam | or disable ipv6 entirely (in a link from the one I gave) | 08:07 |
| rogpeppe | jam: that's similar - it means both requests will be issued in series rather than parallel, i think | 08:08 |
| rogpeppe | jam: and i've just disabled ipv6... | 08:08 |
| jam | rogpeppe: but if the first succeeds will it do a second ? | 08:08 |
| jam | anyway sure | 08:08 |
| rogpeppe | jam: yes, i think | 08:08 |
| rogpeppe | jam: i can see that the first is succeeding already (host -v prints an address almost immediately, then waits for 10-20s for the next request to time out) | 08:09 |
| jam | rogpeppe: host isn't getaddrinfo IIRC, though I'll note if I do "host google.com" it has a valid ipv6 address | 08:10 |
| jam | though I get v4 first | 08:10 |
| jam | host arbash-meinel.com doesn't have ipv6 though it seems to pause for only 1s or so, not 10-20 | 08:10 |
| TheMue | fwereade: happy birthday | 08:18 |
| dimitern | fwereade, yeah, dude! | 08:18 |
| allenap | Does juju-core work with ARM okay? See comment #3 on https://bugs.launchpad.net/ubuntu/+source/maas/+bug/1233831 | 08:26 |
| _mup_ | Bug #1233831: maas doesn't return zookeeper instances for newly provision environment <amd64> <apport-bug> <raring> <maas (Ubuntu):New> <https://launchpad.net/bugs/1233831> | 08:26 |
| rogpeppe | jam: i see 10s pause if i use 127.0.1.1, 6.64s if i use my provider's DNS server directly, and 2.9s if i use google's dns service (8.8.8.8) | 08:26 |
| === axw_ is now known as axw | ||
| jam | allenap: "zookeeper" would be python-juju | 08:27 |
| dimitern | or a really old juju-core | 08:28 |
| jam | allenap: though the log messages look a whole lot like juju-core | 08:28 |
| jam | so the person might just be reporting it poorly | 08:28 |
| dimitern | jam, juju-core used zookeeper for a while after the migration to go | 08:29 |
| jam | allenap: so digging through the bug more I see the follow up posts. So we might-ish support it, but I don't think we were building the binaries for arm as of last week | 08:30 |
| jam | someone was just requesting sinzui do an arm build for 1.15 | 08:30 |
| jam | so we might in the near future | 08:30 |
| jam | hmm... I see armhf for 1.14.0 | 08:31 |
| fwereade | TheMue, dimitern: cheers | 08:31 |
| fwereade | jam, heyhey | 08:31 |
| fwereade | sorry I'mlate, Iwas uprather late last night | 08:31 |
| allenap | jam: The message is: stick with PyJuju on ARM for now? | 08:32 |
| jam | tools/juju-1.14.1-saucy-armhf.tgz | 08:32 |
| jam | fwereade: happy birthday (to continue the trend) | 08:32 |
| jam | allenap: so we only have *saucy* builds with arm | 08:33 |
| fwereade | jam, so, I would like to say "drop the daily datasource in azure and have done with it" | 08:33 |
| jam | fwereade: right, we still have the open question about whether we want to fallback or not | 08:33 |
| fwereade | jam, but I don't know enough about how/if it will react with the image-stream config setting, and other associated snippets of code inside that provider | 08:34 |
| allenap | jam: I'll ask him to deploy saucy with 1.14.1 to see what happens. It may be he's okay with saucy in general. Thanks. | 08:34 |
| jam | allenap: so I think we want to support it, but only having saucy builds is going to make things harder to debug rather than easier, I imagine | 08:34 |
| jam | fwereade: so (1) we're going to just delete the azure daily stream lookup, no problem there we all agree | 08:34 |
| jam | fwereade: but (2) if you have some data that gets found, but without matching, should we go to fallbacks? | 08:35 |
| jam | eg, I upload a custom Precise image, set up the simplestreams for it, and then go "juju deploy cs:saucy/mongo" | 08:35 |
| jam | fwereade: current implementation just fails because it finds your private-bucket simplestreams data and goes no further | 08:36 |
| jam | fwereade: axw's patch would let us keep searching and find the Ubuntu stock images for saucy | 08:36 |
| fwereade | jam, one possibility that crossed my mind was to give datasources a bool method meaning "if there's an index, fall back no further" | 08:36 |
| jam | fwereade: which way seems less WTF for a users? | 08:36 |
| fwereade | jam, and so anything explicitly user-set will mask out everything else | 08:36 |
| fwereade | jam, ie image-metadata-url, tools-url, and synced tools | 08:37 |
| jam | fwereade: but is that better than just allowing them to override certain settings (augment cloud-images) rather than lose everything from there entierly | 08:37 |
| axw | fwereade: I had the same thought too (a flag to say whether or not to keep going). But I sorta think daily shouldn't be in there by default anyway. | 08:37 |
| jam | fwereade: also, I wouldn't mind chatting about the fact we *cannot* upgrade 1.14 => 1.15 | 08:38 |
| jam | fwereade: I saw you going WTF yesterday, so you might have a plan of attack | 08:38 |
| jam | though I have one that I'm willing to try | 08:38 |
| fwereade | jam, dimitern has been looking at the worst of the upgrade problems and I think has a plan of attack | 08:38 |
| jam | fwereade: so download-tools.txt vs download-url.txt comes to mind, but beyond that ? | 08:39 |
| fwereade | jam, there's a sort of nexus of suck around tools, setagenttools,and upgrader | 08:39 |
| dimitern | fwereade, jam, yeah - just filing a bug about it | 08:40 |
| jam | dimitern: bug #1233934 | 08:40 |
| _mup_ | Bug #1233934: upgrade from 1.14.1 to 1.15.0 fails <hp> <juju-1.15.0> <upgrade-juju> <juju-core:Triaged by jameinel> <https://launchpad.net/bugs/1233934> | 08:40 |
| fwereade | jam, so yeah, those text files are part of the problem | 08:40 |
| fwereade | jam, which I propose we handle by just never reading them | 08:41 |
| fwereade | jam, and never storing any of the associatedinformation in state | 08:41 |
| fwereade | jam, write them out as we unpack, sure, why not | 08:43 |
| jam | fwereade: well arguably we'd also like to cache the lookup in state, but that is pretty removed from the issue at hand. | 08:43 |
| jam | so we *do* want FindTools to report them (hopefully it always can, as it seems to be a hard requirement right now if we actually want to validate the binaries we've downloaded) | 08:43 |
| jam | But we could have a type FoundTools struct { Tools, SHA256, ...} | 08:44 |
| jam | ytpe | 08:44 |
| jam | type | 08:44 |
| jam | fwereade: as for reading downloaded-url.txt, that has been the indication of if we have done a download | 08:44 |
| fwereade | jam, but it's only there so ReadTools can hack together a *Tools | 08:44 |
| jam | rather than statting the directory | 08:44 |
| fwereade | jam, ok the fundamental thing is that SetAgentTools shouldnot beSetAgentTools | 08:45 |
| fwereade | jam, it should be SetAgentVersion | 08:45 |
| fwereade | jam, so the upgrader should not be dicking around with ReadTools at all, ever | 08:46 |
| dimitern | fwereade, jam, there it is - bug 1234035 | 08:46 |
| _mup_ | Bug #1234035: upgrading from 1.14 to 1.15 fails (tools not found, upgrader) <juju-core:Triaged by dimitern> <https://launchpad.net/bugs/1234035> | 08:46 |
| fwereade | jam, because the only useful piece of information is the version | 08:46 |
| jam | fwereade: we need the Binary information | 08:46 |
| jam | fwereade: so it has to be at least SetAgentBinary | 08:46 |
| fwereade | jam, all the rest is just N copies of redundant data, one per agent, that's never even looked at | 08:46 |
| jam | dimitern: isn't that just a dup of bug #1233934 | 08:46 |
| _mup_ | Bug #1233934: upgrade from 1.14.1 to 1.15.0 fails <hp> <juju-1.15.0> <upgrade-juju> <juju-core:Triaged by jameinel> <https://launchpad.net/bugs/1233934> | 08:46 |
| fwereade | jam, ehh, maybe, but really why bother denormalising it? | 08:47 |
| fwereade | jam, we know hardware characteristics (ie arch) and machine series *anyway* | 08:47 |
| jam | fwereade: we need the arch and series so that when you ask for "what tools do I need to upgrade to" we can point you to the right URL | 08:47 |
| jam | fwereade: from where? | 08:47 |
| jam | because we can search around for the machine the agent is running on ? | 08:47 |
| dimitern | jam, maybe that one is a duplicate of mine, because it has more context inside? | 08:48 |
| fwereade | jam, but yeah, it's convenient enough, I don't really have a quarrel with a version.Binary | 08:48 |
| fwereade | jam, er, version.Current? | 08:48 |
| fwereade | jam, the only other client of ReadTools is the lxc provisioner, which is a bit cracky itself, because we can get all the relevant information from the tools-for-this-machine API call | 08:49 |
| jam | fwereade: so we have Upgrader.Tools() as an API, which needs to return a URL to download from, and preferably a Version so that we can make sure it actually changes something | 08:49 |
| jam | we currently do the lookup just on Machine.Tag() | 08:49 |
| jam | because we already ha | 08:49 |
| jam | had the desired version in EnvironConfig | 08:50 |
| jam | but we needed the Arch and Series informaction | 08:50 |
| fwereade | jam, yeah, which we have | 08:50 |
| jam | We get that from whatever the agent previously recorded from an earlier SetTools call | 08:50 |
| jam | (agent always calls SetTools on start) | 08:50 |
| jam | before it asks if it should upgrade | 08:50 |
| fwereade | jam, I know all this... and none of it has any reason to ReadTools because *all* that info is in version.Current | 08:51 |
| jam | fwereade: right, except URL, which we don't need to pass around | 08:51 |
| jam | we need it *from* Upgrader.Tools | 08:52 |
| jam | to tell us what to download | 08:52 |
| fwereade | jam, and we could even just send version.Current.Number because we *do* actuall know series/arch, but that's a smentic change to a field so let's not go there | 08:52 |
| fwereade | jam, ok, agreed | 08:52 |
| fwereade | jam, so we've implemented this Tools method | 08:52 |
| fwereade | jam, and we can drop ReadTools *completely* if we just use it in provisioner instead of messing around grubbing up our own data instead of asking the authoritative source | 08:53 |
| jam | fwereade: so lxcBroker wants the full tools information because of ? It wants to give the instances identical tools with the same source? | 08:55 |
| jam | (I'm pretty sure you can run a Saucy machine and have a Precise LXC on i t) | 08:55 |
| jam | fwereade: so I'm not 100% sure why lxcBroker needs tools at all | 08:56 |
| fwereade | jam, well, at the moment, we're restricted to the same series | 08:56 |
| jam | fwereade: why ? | 08:56 |
| fwereade | jam, exactly because we had this readtools thing to mes about with, and *didn't* have an api method to talk to | 08:56 |
| jam | fwereade: k, but even there do we actually use the URL? | 08:57 |
| jam | if we are requiring the instances to use the file we have | 08:57 |
| jam | we have it right here on disk | 08:58 |
| jam | fwereade: anyway, sounds reasonable to change the SetAgentTools api | 09:00 |
| jam | and then get rid of having to read the URLs everywhere | 09:00 |
| fwereade | jam, right | 09:01 |
| fwereade | jam, but saying "download it from the same place" seemed like the easiest way to get them there at the time | 09:01 |
| fwereade | jam, fwiw "download from somewhere" will I think be the right answer in general anyway, once we have eg kvm machines that might not even be the same arch | 09:01 |
| fwereade | jam, doesn't look like we actually enforce the series restriction in state though | 09:02 |
| fwereade | jam, maybe that's ok, it'snot really state's concern | 09:02 |
| jam | fwereade: right, you *should* be able to switch series, even if you can't switch arch on containers (though I also know that you can run 32-bit LXC on 64-bit very easily and have done it many times) | 09:04 |
| jam | you probably can't run 64-bit lxc on 32-bit host, or arm on i386, etc. | 09:04 |
| fwereade | jam, yeah | 09:05 |
| jam | fwereade: anyway, this sounds like a great plan, and if dimitern is picking it up, I'm happy to let him do it and focus on other things. | 09:05 |
| fwereade | dimitern, you ok with that? | 09:05 |
| dimitern | fwereade, jam, cheers | 09:06 |
| dimitern | I'll deal with it | 09:06 |
| jam | fwereade: the nice effect is that it leaves us in a better place going forward, rather than "ok at least the upgrade works for 1.14 => 1.6" | 09:06 |
| jam | 1.16 | 09:06 |
| fwereade | jam, yeah, that's the hope, just strips away a little bit of unnecessary complexity | 09:07 |
| jam | certainly good to step back and say "why do I need this field" rather than adding another kludge to handle the previous one | 09:07 |
| fwereade | jam, the provisioner API will probably have to evolve a little as we progress, but that shouldn't hurt too much | 09:07 |
| fwereade | jam, so, I am also mildly freaking out about this maas business | 09:08 |
| jam | fwereade: which bit ? bootstrapping maas with juju to have juju on maas ? or some specific bug about juju and maas ? | 09:09 |
| fwereade | jam, https://bugs.launchpad.net/gomaasapi/+bug/1222671 | 09:10 |
| _mup_ | Bug #1222671: maas provider must only attempt to stop machines in the allocated state <cts-cloud-review> <Go MAAS API Library:Triaged> <https://launchpad.net/bugs/1222671> | 09:10 |
| jam | ah, can we just do the name thing and not try to get it perfect ? | 09:10 |
| fwereade | jam, so basically we have this stuff documented both in maas and juju, going back to python times | 09:13 |
| fwereade | jam, "issue yourself a fresh maas api key for each juju environment and everything will be fine" | 09:13 |
| fwereade | jam, apparently that is horsepoo | 09:13 |
| fwereade | jam, rvba informs us that it doesn't work and I'm getting a string impression that it never did | 09:13 |
| fwereade | jam, which kinda leaves me WTFing a bit hopelessly | 09:13 |
| fwereade | jam, but I guess it'snot directly a1.16 thing | 09:13 |
| fwereade | jam, so I should probably be off to review some things that *are* 1.16y | 09:13 |
| fwereade | jam, hell, I thought I'd published comments on https://codereview.appspot.com/13962043/ | 09:18 |
| fwereade | rogpeppe, can we land https://codereview.appspot.com/13968043/ ? | 09:22 |
| rogpeppe | fwereade: will do | 09:22 |
| rogpeppe | fwereade: i'm looking for a review of https://codereview.appspot.com/14207046/ BTW | 09:24 |
| fwereade | rogpeppe, and https://codereview.appspot.com/14123043/ looks like it was blocked by a known intermittent failure so that might be worth another shot? | 09:25 |
| fwereade | rogpeppe, looking at that now | 09:26 |
| rogpeppe | fwereade: thanks | 09:26 |
| fwereade | rogpeppe, LGTM essentially, bit of waffling in the CL, let me know your thoughts | 09:41 |
| rogpeppe | fwereade: thanks. will do. | 09:41 |
| jam | fwereade: I think you're right about FinishMachineConfig, I didn't really know it existed, and it feels a *bit* funny to have stuff not initialized until you "Finish" but it is absolutely a place that takes Config data and puts it into the MachineConfig (which New* don't) | 09:52 |
| rogpeppe | fwereade: the ErrPrepared thing is a good point. it *can* currently leak out to the user in at least one circumstance, i think, which is when the user hasn't bootstrapped, invokes a juju command that expects a prepared environment, and the provider needs extra attributes created by Prepare. | 09:52 |
| rogpeppe | fwereade: part of the problem is deciding on an appropriate error message in this case | 09:55 |
| jam | fwereade: provisioner_task calls environs.NewMachineConfig but does *not* call environs.FinishMachineConfig ? | 09:55 |
| fwereade | jam, FinishMachineConfig happens elsewhere, I think, just a mo... | 09:56 |
| jam | fwereade: inside StartInstance | 09:56 |
| jam | so... fairy nuff | 09:56 |
| fwereade | jam, heh, I see, by hand every time :/ | 09:56 |
| jam | fwereade: probably why I didn't think about it in the first plcae | 09:57 |
| jam | place | 09:57 |
| fwereade | rogpeppe, surely we can infer non-bootstrappedness from non-preparedness? | 09:58 |
| rogpeppe | fwereade: not really | 09:58 |
| jam | fwereade: you may not have prepared it on your machine, but someone else bootstrapped already | 09:58 |
| jam | I'm guessing | 09:58 |
| rogpeppe | fwereade: bootstrapped-ness is somewhat orthogonal (actually, totally orthogonal currently) | 09:58 |
| rogpeppe | fwereade: because you can be prepared without being bootstrapped | 09:59 |
| jam | rogpeppe: fwereade's point is that you can't be bootstrapped without being prepared | 09:59 |
| rogpeppe | fwereade: and currently, for backward compatibility, you can be bootstrapped without being prepared | 09:59 |
| jam | but I think that is only if you are running on one machine. | 09:59 |
| fwereade | rogpeppe, ok, got you | 09:59 |
| rogpeppe | fwereade: in the future, i want to make it so that if there's no environment then you'll get an "environment not found" error or something | 10:00 |
| fwereade | rogpeppe, ok, that one slightlynasty error messageshould not block it | 10:00 |
| rogpeppe | fwereade: ok, thanks. | 10:01 |
| rogpeppe | fwereade: i wonder if "incomplete environment configuration" might be a better message | 10:01 |
| fwereade | rogpeppe, not sure it helps a lot tbh | 10:02 |
| jam | fwereade: the other *really* nice thing about FinishMachineConfig is that I can actually test that setting the Config entry has an effect on the MachineConfig NewBootstrapConfig sort of stuff is not well isolated from actually starting an instance. | 10:03 |
| jam | fwereade: so thanks for the pointer | 10:04 |
| jam | makes me a lot happier there | 10:04 |
| fwereade | jam, cheers :) | 10:04 |
| dimitern | fwereade, fix done hopefully, live testing now | 10:10 |
| fwereade | dimitern, <3 | 10:10 |
| natefinch | jam: did we decide on a review target? I think you mentioned something, but I don't remember an email about it, and searching for code review in my inbox is not helpful ;) | 10:12 |
| jam | fwereade: https://codereview.appspot.com/13962043/ has been updated | 10:12 |
| jam | natefinch: I didn't | 10:12 |
| jam | with the release I decided lets focus on getting code done | 10:12 |
| natefinch | jam: ok, cool | 10:12 |
| fwereade | axw__, responded on https://codereview.appspot.com/14254043/ -- will be landing unless you still think it's wrong | 10:15 |
| * TheMue => short lunch break | 10:22 | |
| fwereade | dimitern, actually if you have 5s would you take a quick look at https://codereview.appspot.com/14254043/ and let me know if my comment's obviously insane | 10:22 |
| dimitern | fwereade, looking | 10:22 |
| dimitern | fwereade, as I get it - it's better not to kill the environment if we fail to stop the instances? | 10:25 |
| fwereade | dimitern, yeah, exactly, that's the original behaviour | 10:25 |
| dimitern | fwereade, sgtm | 10:25 |
| fwereade | dimitern, the env is not considered destroyed until its storage is trashed | 10:25 |
| dimitern | fwereade, yeah, sounds sane; and (*drumroll*) there it is https://codereview.appspot.com/14231044 | 10:26 |
| jam | fwereade: fwiw, I think we want to just punt about the bootstrap-state file, we didn't really follow through with that and now it gets us into "juju bootstrap" failing leaving us in a state where we thing it is bootstrapped and you have to destroy something that never existed. | 10:28 |
| jam | fwereade: I also agree that if we fail to stop something we don't delete the storage based on how the if/return works | 10:32 |
| jam | dimitern: don't we want SetAgentTools to actually strip the values? Or are you thinking to add a SetAgentVersion ? | 10:33 |
| fwereade | jam, expand on the first comment a little please -- punt on what exactly? | 10:33 |
| jam | fwereade: we added bootstrap-state so we could try to notice "you're running a python-juju env" | 10:33 |
| jam | and not muck it up | 10:33 |
| jam | fwereade: however, if you're running python-juju environment, you won't have a Cert file to talk to the state node anyway | 10:33 |
| jam | so we fail really early | 10:34 |
| jam | fwereade: so *I* think we should just nuke all the bootstrap-state code because it just complicates things for no gain | 10:34 |
| fwereade | jam, except that we have hard dependencies on it | 10:34 |
| fwereade | jam, LoadStateFromURL in jujud bootstrap | 10:34 |
| jam | fwereade: I think all of those could be adjusted. We have provider-state rather than bootstrap-verify | 10:35 |
| dimitern | jam, I don't plan on adding SetAgentVersion | 10:35 |
| jam | fwereade: but notice that LoadState *doesn't* read the file | 10:35 |
| jam | etc | 10:35 |
| jam | I may be wrong on the naming | 10:35 |
| jam | but we have 2 ways of "loading" and *one* checks the file, and the other *doesn't* | 10:35 |
| jam | dimitern: in which case, I would actually strip the fields that we don't actually want in the DB | 10:35 |
| jam | namely URL Size and SHA | 10:35 |
| fwereade | dimitern, I like what jam's saying | 10:36 |
| jam | we need Tools() to give the caller those values, but SetAgentTools doesn't need to record them. | 10:36 |
| dimitern | jam, fwereade, we do want them, don't we? | 10:36 |
| fwereade | dimitern, leave the API alone but make the SetAgentTools in state into SetAgentVersion? | 10:36 |
| fwereade | dimitern, no | 10:36 |
| dimitern | fwereade, why? | 10:36 |
| jam | fwereade: it was something danilo had been working on, but he left and it didn't realyl get that polished, and other changes mean it is just like your appendix | 10:36 |
| jam | cruft that is likely to get you in trouble :) | 10:36 |
| fwereade | dimitern, because we do not want any of that crapin state | 10:37 |
| fwereade | dimitern, full stop | 10:37 |
| dimitern | fwereade, which crap? | 10:37 |
| dimitern | fwereade, size, checksum or url? | 10:37 |
| jam | dimitern, fwereade: sha size, URL | 10:37 |
| fwereade | dimitern, anything except the agent'sbinary version | 10:37 |
| dimitern | fwereade, so no verification of what we're trying to install? | 10:38 |
| fwereade | dimitern, no verification of what we've *already* installed | 10:38 |
| fwereade | dimitern, it's verified perfectly well before the code that sets it even runs | 10:38 |
| fwereade | dimitern, if the verification worked, no need to set it | 10:38 |
| dimitern | fwereade, so are we doing pre-install verification? | 10:38 |
| dimitern | fwereade, we can't just remove the Tools method from the upgrader and change it to Version | 10:39 |
| fwereade | dimitern, I never mentioned Tools | 10:39 |
| fwereade | dimitern, we need to et that info in order to verify | 10:40 |
| dimitern | fwereade, so only SetAgentTools -> SetAgentVersion | 10:40 |
| dimitern | fwereade, in state and no changes in the API except for that? | 10:40 |
| fwereade | dimitern, yes, and that just in state, leave the API alone as much as possible -- just ignore/drop fields, don't change names or anything | 10:40 |
| dimitern | fwereade, I need to change the API to call SetAgentVersion instead of SetAgentTools at least | 10:41 |
| dimitern | fwereade, but that's internal, won't change the interface | 10:41 |
| fwereade | dimitern, yep, that's all good | 10:41 |
| dimitern | fwereade, and I suppose that's another live ec2 tests after that | 10:41 |
| fwereade | dimitern, probably, but I'll try to hit the rest of the review soon too | 10:42 |
| dimitern | fwereade, ok | 10:42 |
| fwereade | dimitern, ie I'll try to save you having to do more than one more round of live testsing ;p | 10:43 |
| dimitern | fwereade, ah well, that's nice :) | 10:43 |
| jam | dimitern: right, so Tools doesn't change because we need a URL and when we download it we need to validate the SHA, but SetAgent* doesn't need the URL or sha, etc. Arguably we should have different types | 10:45 |
| jam | but we can live with 1 type and just ignore bits of it | 10:45 |
| jam | fwereade: should we be adding "omitempty" flags to the Tools object ? | 10:45 |
| dimitern | jam, machine and unit will have SetAgentVersion(version.Binary) methods instead of SetAgentTools | 10:45 |
| dimitern | standup | 10:45 |
| fwereade | jam, what, are we storing Toolses directly in state? | 10:45 |
| dimitern | fwereade, we are storing coretools.Tools in state | 10:46 |
| fwereade | jam, dimitern: can't we just make it a one-field struct? | 10:46 |
| jam | mgz fwereade: rogpeppe:https://plus.google.com/hangouts/_/3b7ffbb7710f75a160f5ae900736d7276d6b241f | 10:46 |
| dimitern | fwereade, I'd rather not explode the complexity of this already not-so-small CL | 10:47 |
| fwereade | dimitern, jam: or, ehh, keep the stupidity, just construct a Toolswith just the version field | 10:47 |
| jam | fwereade: so the issue is that we've been using it in the API and writing it to State | 10:47 |
| jam | and they need to be a bit different now | 10:47 |
| jam | fwereade: but that means we've written empty values into the DB unless we have omitempty, right? | 10:48 |
| dimitern | it's ok if we just set the version | 10:48 |
| jam | fwereade: anyway, standup we can live chat this after | 10:48 |
| dimitern | we have omitempty only for the sha I think | 10:48 |
| dimitern | we can make all the others except version omitempty as well | 10:48 |
| jam | dimitern: +1 | 10:49 |
| fwereade | jam, I don't care what's in the DB, nobody has ever read anything from the db apart from the version | 10:50 |
| fwereade | jam, it's straight-up crack putting external objects directlyinto the db *anyway* | 10:50 |
| dimitern | fwereade, I did btw | 11:04 |
| mgz | gahhh... deliver th | 11:17 |
| mgz | e packets google | 11:17 |
| fwereade | mgz, I feel a bit better if it hates you too :/ | 11:22 |
| fwereade | dimitern, reviewed | 11:22 |
| dimitern | fwereade, thanks | 11:22 |
| fwereade | dimitern, I observe that ReadTools is now basically unused, but that can wait | 11:22 |
| fwereade | dimitern, I am fairly heavily weirded out by what you do in Upgrader though | 11:23 |
| jam | fwereade: going over https://launchpad.net/juju-core/+milestone/1.16.0 right now | 11:24 |
| jam | are we actually intending to do all of them | 11:24 |
| fwereade | dimitern, looks like you're getting the proposed tools and setting them as the current ones, this will mean you can't upgrade now | 11:24 |
| jam | like bug #1089291 | 11:24 |
| _mup_ | Bug #1089291: destroy-machine --force <juju-core:Triaged> <https://launchpad.net/bugs/1089291> | 11:24 |
| fwereade | jam, that one? not a chance | 11:25 |
| rogpeppe | fwereade: what do you think about the status os https://bugs.launchpad.net/juju-core/+bug/1089291? | 11:25 |
| dimitern | fwereade, ReadTools is used in a few places, so I didn't want to remove it just yet | 11:25 |
| _mup_ | Bug #1089291: destroy-machine --force <juju-core:Triaged> <https://launchpad.net/bugs/1089291> | 11:25 |
| rogpeppe | s/os/of/ | 11:25 |
| dimitern | fwereade, not sure what you mean for the upgrader | 11:25 |
| dimitern | fwereade, the upgrade works actually | 11:25 |
| fwereade | dimitern, yeah, but no future upgrade ever will | 11:25 |
| fwereade | dimitern, https://codereview.appspot.com/14231044/diff/1/worker/upgrader/upgrader.go#newcode101 | 11:26 |
| dimitern | fwereade, ok | 11:27 |
| fwereade | rogpeppe, jam: that bug in particular is at least a few day's work I think | 11:27 |
| rogpeppe | fwereade: thought it might be | 11:27 |
| fwereade | rogpeppe, jam: https://bugs.launchpad.net/juju-core/+bug/1217781 is smaller and more important though | 11:27 |
| rogpeppe | fwereade: we're going through https://launchpad.net/juju-core/+milestone/1.16.0 BTW | 11:27 |
| _mup_ | Bug #1217781: machine destruction depends on machine agents <cts> <cts-cloud-review> <juju-core:Triaged> <https://launchpad.net/bugs/1217781> | 11:27 |
| rogpeppe | fwereade: that's not targeted to the milestone | 11:28 |
| fwereade | rogpeppe, I think it's somewhat unrealistic regardless, but unquestionably a better candidate than 1089291 | 11:29 |
| fwereade | oh ffs | 11:36 |
| rogpeppe | fwereade: gone again? | 11:36 |
| fwereade | indeed | 11:37 |
| fwereade | everything is working just fiine, except for hangouts :/ | 11:37 |
| rogpeppe | gah | 11:37 |
| rogpeppe | fwereade: finished now | 11:39 |
| fwereade | heh | 11:39 |
| rogpeppe | i see sporadic test failures on provider/null | 11:39 |
| fwereade | rogpeppe, would you hand them over to axw_ for tonight or tomorrow or whatever "soon" is in his timezone please? | 11:41 |
| * fwereade is grabbing a quick lunch | 11:41 | |
| rogpeppe | fwereade: will do | 11:41 |
| jam | fwereade: what was the bug I was asking if we had (and didn't see) ? I can't remember now that I've gone through 10 other bugs. | 11:45 |
| jam | fwereade: for "provider.Tools()" this is a bug that might be serious | 11:57 |
| jam | fwereade: https://codereview.appspot.com/14231044/patch/1/1005 | 11:57 |
| jam | dimitern: ^^ | 11:57 |
| jam | specifically, for one of our releases Upgrader was calling Tools as well | 11:57 |
| jam | but we didn't have Provider credentials yet | 11:57 |
| jam | so it spins and keeps killing the API server. | 11:57 |
| fwereade | jam, cloud-tools-archive | 11:58 |
| jam | For Upgrader this was fixed with adding the DesiredVersion API call | 11:58 |
| jam | fwereade: thanks | 11:58 |
| fwereade | jam, surely that shouldn't be killing the whole api server though, just the failing task? | 11:59 |
| fwereade | jam, was that when we were still using allFatal perhaps? | 12:00 |
| jam | fwereade: my point being, "bouncing workers" seems like a bad thing to think is just-ok-to-do | 12:02 |
| jam | and asking Tools before first "juju status" means it will keep bouncing | 12:02 |
| * fwereade grabbing a bit more lunch | 12:02 | |
| jam | dimitern: ^^ Calling Tools requires us to have the Environment/Provider credentials enabled on the API server (aka, juju status has transferred the secret attrs). | 12:13 |
| jam | I *believe* we are ok because of the call for WaitForEnviron | 12:13 |
| jam | is that true? | 12:13 |
| jam | or are we going to see LXC Provisioners start bouncing once their up because they call Tools and it can't access the Storage to search for tools ? | 12:13 |
| dimitern | jam, we have an environ before we call Tools | 12:21 |
| dimitern | jam, that's WaitForEnviron's job | 12:21 |
| jam | dimitern: not in upgrader | 12:21 |
| jam | but yeah | 12:21 |
| jam | I tihnk Provisioner is ok | 12:21 |
| jam | dimitern: so if you make the change William suggests here: https://codereview.appspot.com/14231044/patch/1/1015 | 12:22 |
| dimitern | jam, the upgrader won't call Tools - i'm changing it to set the current tools, as fwereade suggested | 12:22 |
| jam | I think we're ok | 12:22 |
| fwereade | dimitern, well, it and the provisioner will still both call Tools | 12:22 |
| jam | dimitern: https://codereview.appspot.com/14231044/patch/1/1002 is still odd to me | 12:23 |
| jam | fwereade: so Upgrader will definitely only call Tools after calling SetTools which is just fine. And It won't call Tools until it has called DesiredVersion | 12:23 |
| jam | so we are generally ok | 12:23 |
| jam | (if someone asks for "juju upgrade-juju" they should have connected and passed the secrets) | 12:23 |
| jam | fwereade: there is a small race if Provisioner starts up first, sees we have a configured environment, and Upgrader hasn't finished calling SetTools first | 12:24 |
| jam | I think | 12:24 |
| fwereade | jam, ok, but provisioner *might* get in there -- indeed | 12:24 |
| dimitern | jam, I commented on it, and will be removed - rogpeppe already fixed it in trunk | 12:24 |
| fwereade | jam, imo that doesn't matter enough to invalidate the technique | 12:24 |
| fwereade | jam, that's almost the whole point of runner, so we can be resilient to this sort of thing | 12:25 |
| jam | fwereade: so I feel like the whole thing is a bit incorrect, though. | 12:25 |
| jam | We shouldn't be calling tools to *instantiate* the broker | 12:25 |
| jam | we should have the lxc broker calling Tools when it is asked for a new instance. | 12:25 |
| fwereade | jam, the broker should not be responsible for tools in the first place afaict | 12:25 |
| jam | fwereade: My faith in non-bouncing runners is quite low | 12:25 |
| jam | fwereade: Environs provide tools in the current layout | 12:25 |
| jam | and LXC Broker is environ-lite | 12:26 |
| jam | I guess | 12:26 |
| fwereade | jam, broker is clearly not responsible for tools, because it *takes* possibleTools and doesn't look them up itself | 12:26 |
| fwereade | jam, provisioner should be thinking "broker" not "environ" | 12:26 |
| jam | fwereade: provider.StartInstance looks up tools for the broker | 12:26 |
| fwereade | jam, afaict tools should be coming from the api in exactly the same way in both cases frankly | 12:27 |
| jam | and uses either "environ.Environ.FindTools" or broker.Tools if it has the attribute | 12:27 |
| fwereade | jam, that however is too big a change to make here and now | 12:27 |
| fwereade | jam, I know what it does | 12:27 |
| jam | fwereade: right, I wasn't going to block on it, but notice "this is really messed up" :) | 12:27 |
| fwereade | jam, I'm just saying it's crap :) | 12:27 |
| jam | fwereade: and has the advantage that we avoid a boot-time race condition | 12:27 |
| fwereade | jam, yeah, I think it's tolerable | 12:28 |
| * rogpeppe goes for lunch | 12:28 | |
| jam | fwereade: probably should be documented in a bug | 12:29 |
| fwereade | jam, I'm just not quite sure what the bug is -- "lxc provisioner might bounce once or twice on upgrade"? | 12:31 |
| jam | fwereade: lxc provisioner calls Tools before it actually tries to start an instance | 12:31 |
| fwereade | jam, or possibly "lxc broker pretends to know about tools" | 12:35 |
| fwereade | jam, hey, is there any reason to do *that* even? | 12:35 |
| jam | fwereade: given we can just search for tools again, I don't see why. | 12:36 |
| fwereade | jam, can we just drop lxc broker's Tools full stop? | 12:36 |
| jam | but I don't know why the LXCBroker is playing games with tools | 12:36 |
| jam | fwereade: so you do still need to find tools, and you don't have Env creds to search directly | 12:36 |
| jam | fwereade: can I get a final LGTM on https://codereview.appspot.com/13962043/ ? | 12:36 |
| jam | I think I addressed your comments | 12:36 |
| fwereade | jam behind api, though, so irrelevant? | 12:37 |
| fwereade | jam, looking | 12:37 |
| jam | fwereade: if we change it to call the API, +1 from me | 12:37 |
| fwereade | jam, that LGTM, thanks | 12:38 |
| === jtv2 is now known as jtv | ||
| dimitern | fwereade, so we're changing SetTools of the upgrader API to take only a version and ignore the others as well ? | 12:45 |
| dimitern | jam ^^ | 12:46 |
| jam | dimitern: version.Binary (aka with Arch and Series) | 12:47 |
| jam | so version.Current content | 12:47 |
| dimitern | jam, yeah, ok | 12:47 |
| jam | axw_: what are you doing working? :) | 12:48 |
| axw_ | jam: just culling my inbox :) | 12:49 |
| jam | axw_: and responding to IRC, and reviewing code, and ? :) | 12:49 |
| axw_ | hehe | 12:49 |
| axw_ | and now playing skyrim ;) | 12:49 |
| jam | axw_: very fun game | 12:50 |
| jam | PC? | 12:50 |
| axw_ | yup | 12:50 |
| axw_ | I don't play games all that often, but this one has me hooked for now | 12:50 |
| jam | axw_: Steam claims I played it for 153 hours.... | 12:51 |
| axw_ | jam: only 105 here ;) | 12:52 |
| natefinch | man I wish I still had time for games. Maybe once the kids are in school :) | 12:54 |
| mgz | or when they leave home | 12:55 |
| mgz | only a decade and a bit till freedom? | 12:55 |
| natefinch | my kids are 0 and 2, so, like 18 years :) Hey, at least graphics ought to be pretty awesome by then | 12:56 |
| TheRealMue | natefinch: be promised it will get better | 13:01 |
| fwereade | natefinch, I was playing a game just the other day! | 13:02 |
| natefinch | TheRealMue: thanks. I think it's pretty awesome, I just mourn for my free time, especially now that our 3 month old has decided 6am is a good time to get up (previously, I had been able to get up early to get some free time before the rest of the house woke up) | 13:02 |
| fwereade | natefinch, iirc it was "dora the explorer saves the crystal kingdom", but hopefully laura's tastes will mature | 13:02 |
| === TheRealMue is now known as TheMue | ||
| natefinch | fwereade: haha. Yeah, I'm hoping to encourage good tastes in games, both digital and tabletop, since I enjoy both | 13:03 |
| rogpeppe | fwereade: ha, i've discovered why the jujutest live tests are failing | 13:05 |
| fwereade | rogpeppe, oh yes? | 13:06 |
| rogpeppe | fwereade: the jujud that gets uploaded is a fake! (it's just a temp file containing the text "jujud contents 1.15.1-precise-amd64") | 13:06 |
| rogpeppe | s/temp file/text file/ | 13:06 |
| fwereade | rogpeppe, arrgh | 13:06 |
| rogpeppe | fwereade: it was pretty awkward to find out though, because it seems that there's no default key pair used for new instances | 13:07 |
| rogpeppe | fwereade: i used to be able to ssh into an instance even before cloudinit had finished | 13:07 |
| rogpeppe | fwereade: so i had to detach the instance's volume and attach it to another instance so i could see what was going on | 13:07 |
| fwereade | rogpeppe, that's odd, I didn't think we had any awareness of amazon keypairs | 13:09 |
| fwereade | ever | 13:09 |
| rogpeppe | fwereade: me neither. i wonder if an amazon default has changed. | 13:09 |
| rogpeppe | fwereade: ah, looks like the live tests were broken here: https://codereview.appspot.com/14031043/diff/7001/environs/jujutest/livetests.go | 13:14 |
| rogpeppe | fwereade: i *think* all those UploadFakeTools calls are spurious in a live test context, but i don't really understand why they were added. | 13:15 |
| dimitern | so ian goes to a holiday and all tools break loose :) | 13:15 |
| rogpeppe | dimitern: it's not easy to review 4700 line diffs... | 13:16 |
| fwereade | dimitern, ehh, less hell than I feared | 13:17 |
| dimitern | :) | 13:18 |
| dimitern | rogpeppe, any idea why I see this failing? http://paste.ubuntu.com/6183812/ | 13:19 |
| rogpeppe | dimitern: that test is new on me; just introduced in https://code.launchpad.net/~thumper/juju-core/environments-dir-permission/+merge/188754 | 13:23 |
| rogpeppe | dimitern: i'll have a look | 13:24 |
| rogpeppe | dimitern: that's pretty weird | 13:25 |
| rogpeppe | dimitern: what go version are you using? | 13:26 |
| dimitern | rogpeppe, the one from the archive | 13:26 |
| dimitern | rogpeppe, go version go1.1.1 linux/amd64 | 13:26 |
| rogpeppe | dimitern: from which archive? | 13:26 |
| rogpeppe | dimitern: i.e. is it from a PPA? it's quite an odd error to get, because Current *is* (or should be) implemented on linux/amd64 | 13:27 |
| dimitern | rogpeppe, there http://paste.ubuntu.com/6183836/ | 13:27 |
| dimitern | rogpeppe, it's not from a PPA | 13:28 |
| fwereade | rogpeppe, there may well be some opportunity to pull the tool-fiddling up to a higher level so we can upload real or fake as indicated by the, er, liveness of the test | 13:28 |
| dimitern | rogpeppe, there's a golang bug with might be relevant https://code.google.com/p/go/issues/detail?id=6376 | 13:28 |
| rogpeppe | dimitern: that *shouldn't* be relevant as it won't have been cross-compiler | 13:29 |
| rogpeppe | compiled | 13:29 |
| dimitern | jamespage, do you know how is the golang 1.1.1 package in main compiled in raring/amd64? is it possible for it to be cross-compiled? | 13:29 |
| rogpeppe | oh i hate it when apt-get tries to use the terminal | 13:32 |
| dimitern | rogpeppe, looking here https://launchpad.net/ubuntu/saucy/amd64/golang-go-linux-amd64/2:1.1.1-3ubuntu3 | 13:35 |
| dimitern | rogpeppe, from the description one might assume it's actually cross-compiled on i386? | 13:36 |
| rogpeppe | dimitern: BTW you've got two golang packages in the list you pasted - if i apt-get install golang, i get the second one, not the first | 13:37 |
| rogpeppe | dimitern: (i.e. 1.0.2 not 1.1) | 13:37 |
| rogpeppe | dimitern: if we are cross-compiling the standard go compiler, that's a potential problem | 13:38 |
| dimitern | rogpeppe, yeah, but I forced the version I think | 13:38 |
| rogpeppe | dimitern: ah, how do you force a version? | 13:38 |
| dimitern | rogpeppe, ah, no sorry | 13:38 |
| dimitern | rogpeppe, it seems i used jamespage's raring backports ppa: http://ppa.launchpad.net/james-page/golang-backports/ubuntu raring main | 13:39 |
| rogpeppe | dimitern: right, i wondered | 13:39 |
| jamespage | dimitern, please don't use that ppa | 13:39 |
| jamespage | ppa:juju/golang | 13:39 |
| jamespage | is the right one to use | 13:39 |
| dimitern | rogpeppe, you force a version by setting it explicitly golang=2:1.1.1-3ubuntu3 .. something | 13:40 |
| rogpeppe | dimitern: could you try what jamespage says and see if you still have a problem? | 13:40 |
| jamespage | the packages in my ppa won't work | 13:40 |
| dimitern | jamespage, rogpeppe, ok, will try | 13:40 |
| jamespage | they cross compile in a way that is broken | 13:40 |
| rogpeppe | jamespage: +1 | 13:40 |
| dimitern | aha! | 13:40 |
| jamespage | the packages in juju/golang ppa do it right | 13:40 |
| rogpeppe | jamespage: that looks like the issue dimitern was seeing | 13:40 |
| dimitern | rogpeppe, ok it's fine now - using go version go1.1.2 linux/amd64 and the tests pass | 13:43 |
| rogpeppe | dimitern: cool | 13:43 |
| dimitern | jam, fwereade, updated https://codereview.appspot.com/14231044/ - live testing on ec2 in the mean time | 13:43 |
| fwereade | dimitern, cheers | 13:45 |
| jam | dimitern: I don't think we want to change the API do we ? | 13:45 |
| jam | If you aren't changing the Tools struct, I don't see the point of changing the name of the API | 13:45 |
| dimitern | jam, what do you mean? | 13:45 |
| jam | change the state.SetAgentVersion but state/api/params/internal.go | 13:45 |
| jam | https://codereview.appspot.com/14231044/patch/10001/11005 | 13:45 |
| dimitern | jam, the name of the type doesn't matter | 13:46 |
| dimitern | jam, just the fields should be the same to be compatible | 13:46 |
| jam | dimitern: fair enough, but I wouldn't change the type name when it still has a Tools object, we can change the type when we change the content | 13:46 |
| dimitern | jam, that's why I put the DEPRECATE tag there | 13:46 |
| dimitern | jam, it's ok we've done this before | 13:47 |
| jam | dimitern: but *why* bother to change X when it does nothing when you have to change Y that actually does something and you can change X when you change Y | 13:47 |
| dimitern | jam, because I wanted to clean up all refs to SetAgentTools in the code | 13:47 |
| dimitern | jam, and I think it's properly documented, so it's not confusing | 13:48 |
| jam | dimitern: so I think we wanted to change "tools.Tools" to add omitempty entries | 13:50 |
| jam | and I'm still not sold on changing the name of the struct | 13:51 |
| jam | but the rest LGTM | 13:51 |
| dimitern | jam, I forgot the omitempty, will add that | 13:51 |
| jam | I'll let fwereade and you discuss the other bits, I'm off for now | 13:51 |
| dimitern | jam, thanks | 13:51 |
| dimitern | bugger.. another random test failure http://paste.ubuntu.com/6183904/ | 13:52 |
| fwereade | jam, dimitern: I'd just as soon define (in params) `type Tools struct {Version version.Binary}` and use that -- seems equivalent, right? | 13:52 |
| dimitern | that *evil* EnvironBootstrapStorager strikes again | 13:52 |
| dimitern | fwereade, not really | 13:53 |
| dimitern | fwereade, we do want to keep the logic with getting the url + size + sha from the environ, right? | 13:53 |
| fwereade | dimitern, ehh, right | 13:54 |
| fwereade | dimitern, type Version struct ;p | 13:54 |
| dimitern | fwereade, we can change all that in 1.18 as we deprecate that stuff | 13:54 |
| fwereade | dimitern, EntityVersion struct {Tag string, Tools Version} | 13:54 |
| fwereade | dimitern, ok, I guess I don't see why we have future deprecation considerations | 13:55 |
| fwereade | dimitern, all that any server requires today is Version, right? | 13:55 |
| dimitern | fwereade, but that way we'll change the type of an API type's field | 13:55 |
| dimitern | fwereade, I chose the most backwards-compatible approach I think | 13:55 |
| fwereade | dimitern, if it's just a struct with compatible fields, what's the problem? it's the field names that screw us,not the types themselves | 13:56 |
| dimitern | fwereade, but if an older client sends fields for that type which are missing? | 13:56 |
| fwereade | dimitern, dropped on the floor automatically, aren't they? | 13:56 |
| dimitern | fwereade, we'll just ignore them I guess | 13:56 |
| dimitern | fwereade, yeah, ok, will do | 13:56 |
| fwereade | dimitern, lovely, thanks | 13:57 |
| fwereade | rogpeppe, is this the one you saw? http://paste.ubuntu.com/6183904/ | 13:57 |
| rogpeppe | fwereade: yes | 13:57 |
| rogpeppe | fwereade: and i know why it happens - i raised a bug | 13:57 |
| rogpeppe | fwereade: https://bugs.launchpad.net/juju-core/+bug/1234125 | 13:57 |
| _mup_ | Bug #1234125: provider/null: sporadic test failure <juju-core:New for axwalk> <https://launchpad.net/bugs/1234125> | 13:57 |
| dimitern | rogpeppe, if it's sporadic, add an intermittent-failure tag (I did that now) | 13:58 |
| fwereade | rogpeppe, nice, thanks | 13:59 |
| dimitern | fwereade, EnvironVersion has to be type EnvironVersion version.Binary, right? | 14:01 |
| dimitern | fwereade, no Tag to be seen | 14:01 |
| dimitern | fwereade, and if it is, then I don't see why we need to even define a type - just use version.Binary | 14:02 |
| fwereade | dimitern, we need a type to be where the Tools struct was, surely | 14:03 |
| dimitern | fwereade, yes | 14:04 |
| dimitern | fwereade, so it we redefine Tools *version.Binary should be enough? | 14:04 |
| fwereade | dimitern, surelynot no | 14:04 |
| dimitern | fwereade, ok, I'm confused now, why not? | 14:04 |
| fwereade | dimitern, {Tag: "foo", Tools: {Version: "bar"}} != {Tag: "foo", Tools: "bar"} | 14:05 |
| dimitern | fwereade, ah | 14:05 |
| dimitern | fwereade, so we have type Version stuct { Version: version.Binary } and use that in SetAgentVersion struct | 14:06 |
| fwereade | dimitern, yeah | 14:07 |
| fwereade | dimitern, although it would be great if we didn't continue the practice of verbing sumb structs | 14:07 |
| fwereade | s/sumb/dumb/ | 14:07 |
| fwereade | dimitern, afaict that data there is an AgentVersion, or possibly an EntityVersion -- not a *Set* anything | 14:08 |
| dimitern | fwereade, there's already AgentVersionResult and Results | 14:09 |
| rogpeppe | fwereade: i'm looking at the changes in SetUpSuite in https://codereview.appspot.com/14031043/diff/7001/provider/ec2/live_test.go and they look like crack but i don't know what the current status w.r.t. public buckets is | 14:09 |
| dimitern | fwereade, don't you thing it would be confusing to have AgentVersionResult, AgentVersionResults, AgentVersion and AgentsVersion ? | 14:09 |
| rogpeppe | fwereade: the reason for the "verbed" structs in params is that params.X is supposed to mean "the parameters to the call X" | 14:10 |
| fwereade | dimitern, and AgentVersionResult doesn't actually have an agent reference anywhere in it, it's just a VersionResult, surely? | 14:10 |
| dimitern | fwereade, ok, will rename AgentVersionResult* to VersionResult* | 14:10 |
| rogpeppe | fwereade: if we want to change that convention, i think we should do it all at once rather than changing some things not others | 14:11 |
| fwereade | rogpeppe, I think that justification breaks down pretty quickly in practice, doesn't it? I haven't really spotted anything resembling consistency in params names thus far | 14:11 |
| dimitern | fwereade, how about AgentToolsResult(s) ? | 14:12 |
| fwereade | dimitern, ToolsResult? no agent mentioned in there... | 14:13 |
| dimitern | fwereade, ok | 14:13 |
| rogpeppe | fwereade: they might not be utterly consistent, but *most* of them abide by that convention AFAICS | 14:13 |
| fwereade | dimitern, rogpeppe: sorry I must go, I think laura is crying about me not coming to see the birthday cake she's decorated for me and I amfeeling like a Bad Person | 14:13 |
| fwereade | bbs | 14:13 |
| rogpeppe | fwereade: happy birthday, BTW! | 14:13 |
| fwereade | rogpeppe, cheers | 14:14 |
| fwereade | and cath has mollified laura so I have breathing space | 14:14 |
| * fwereade still a bad person | 14:14 | |
| fwereade | dimitern, ok, reviewed with a few more thoughts, bbs | 14:21 |
| dimitern | fwereade, cheers | 14:21 |
| rogpeppe | fwereade, jam, mgz: it's my understanding that an environment's PublicStorage is now entirely ignored when checking for existing tools - is that right? | 14:23 |
| jam | rogpeppe: PublicStorage (afaik) is completely ignored | 14:24 |
| rogpeppe | fwereade, jam, mgz: so that the old logic on line 94 of https://codereview.appspot.com/14031043/diff/7001/provider/ec2/live_test.go can't be replaced decently | 14:24 |
| rogpeppe | jam: the new code breaks any of the live tests that actually want to run the tools | 14:24 |
| jam | rogpeppe: you can create a storage bucket, write tools to it, and set tools-url: in config | 14:25 |
| abentley | sinzui: I believe we can disable auto-discovery in elasticsearch by setting discovery.zen.ping.multicast.enabled to False. We would then enable discovery.zen.ping.unicast and use a peer relation to determine discovery.zen.ping.unicast.hosts. | 14:26 |
| rogpeppe | jam: how does that help? what we need here is a fallback, not something that will be used if we've uploaded the actual tools | 14:26 |
| abentley | sinzui: http://www.elasticsearch.org/guide/reference/modules/discovery/zen/ | 14:26 |
| rogpeppe | jam: ah, i see, you mean tools-url points to the simple-streams thingy | 14:27 |
| rogpeppe | jam: presumably you'd need to generate simplestreams metadata too | 14:27 |
| jam | rogpeppe: sync-tools / upload-tools generate all that stuff | 14:28 |
| jam | so you need a bucket and then publish tools into it | 14:28 |
| rogpeppe | jam: and from within Go ? | 14:28 |
| TheMue | hmm, bzr tells me "This transport does not update the working tree of: bzr+ssh://bazaar.launchpad.net/~themue/juju-core/049-prepare-ec2/" but the branch can be seen in launchpad. anyone an idea? | 14:28 |
| jam | rogpeppe: upload tools from withing go does create simplestreams metadata for the tools it publishes | 14:28 |
| jam | TheMue: there shouldn't be a working tree for a bazaar.launchpad.net branch | 14:29 |
| jam | so there shouldn't be any user files there | 14:29 |
| rogpeppe | jam: ok, i'll have a look | 14:29 |
| rogpeppe | jam: thanks | 14:29 |
| TheMue | jam: ok, but why does bzr think i want to do so? | 14:30 |
| jam | TheMue: I don't know, I don't see one here: http://bazaar.launchpad.net/~themue/juju-core/049-prepare-ec2/.bzr/ | 14:30 |
| jam | TheMue: I could probably do a better diagnose if you had more of the .bzr.log file (found in $HOME) | 14:31 |
| jam | rogpeppe: sync.Upload | 14:31 |
| rogpeppe | jam: ah, i was looking at tools.WriteMetadata | 14:31 |
| mgz | TheMue: did you do something funny like `bzr pull -d REMOTE` rather than `bzr push REMOTE`? | 14:31 |
| TheMue | jam: thx, i'll take a look in there | 14:31 |
| jam | rogpeppe: sync.Upload does all the work for you, from what I can tell it is used by bootstsrap | 14:31 |
| jam | bootstrap | 14:31 |
| TheMue | mgz: no, a standard push --remember ... | 14:32 |
| rogpeppe | jam: in this case i don't want to build and upload the actual tools | 14:32 |
| TheMue | mgz: and it is visible at https://code.launchpad.net/~themue/juju-core/049-prepare-ec2 | 14:33 |
| rogpeppe | jam: hmm, looks like more work than i want to do currently - i'll just go with even slower live tests for the time being | 14:35 |
| jam | rogpeppe: can't you then do UploadFakeTools ? | 14:46 |
| jam | there is a GenerateFakeTools somewhere | 14:47 |
| jam | rogpeppe: provider/openstack/live_test.go | 14:49 |
| jam | does: | 14:49 |
| jam | t.metadataStorage = openstack.MetadataStorage(t.env) | 14:49 |
| jam | envtesting.UploadFakeTools(c, t.metadataStorage) | 14:49 |
| jam | rogpeppe: MetadataStorage is a testing thing that points at the location pointed to by the LiveTest config | 14:50 |
| rogpeppe | jam: yes, i could probably something like that, but i can't afford the hour or so it would take currently. | 14:50 |
| rogpeppe | jam: because testing it is so slow | 14:51 |
| rogpeppe | jam: and i'm hoping to push the addresser branch | 14:51 |
| rogpeppe | jam: and i already have *something* that makes the live tests work | 14:51 |
| rogpeppe | jam: i'll leave it as a todo | 14:51 |
| jam | rogpeppe: fine with me | 14:52 |
| mattyw | I'm a little confused by the new logging stuff, if I want to deploy a charm and have debug logging do I need to supply "--log-config=<root>=DEBUG" is that right? | 14:59 |
| rogpeppe | mattyw: i'm afraid i'm not sure - it's new to me too | 15:05 |
| rogpeppe | fwereade: would you be able to take another look at https://codereview.appspot.com/14038045/ please? | 15:06 |
| rogpeppe | fwereade: it addresses your concerns i think | 15:06 |
| rogpeppe | fwereade, jam: BTW i'm a little concerned about these changes: https://codereview.appspot.com/14258043/diff/6001/environs/configstore/disk.go | 15:08 |
| rogpeppe | fwereade, jam: ISTM that they might not work on windows | 15:08 |
| rogpeppe | natefinch: how much of our stuff is supposed to work under Windows? | 15:09 |
| fwereade | rogpeppe, hell, that sounds very plausible | 15:09 |
| rogpeppe | fwereade: i have a feeling that ensurePathOwnedByUser should be in an foo_unix.go file | 15:10 |
| rogpeppe | fwereade: with a fallback for non-unix | 15:11 |
| fwereade | rogpeppe, that sounds very likely to me to be correct | 15:14 |
| natefinch | rogpeppe: define stuff? The client builds under windows. I'm not sure about the tests for the packages the client uses, though. And definitely, a lot of the code outside the client is linux-specific | 15:16 |
| rogpeppe | natefinch: this is client stuff | 15:16 |
| rogpeppe | natefinch: i suppose this stuff *might* work under windows, because os.Chown will still be there | 15:16 |
| natefinch | rogpeppe: I'm taking a look at the code review | 15:17 |
| rogpeppe | natefinch: thanks | 15:17 |
| fwereade | natefinch, if you concur there's a probablem, would you kick it over to thumper and cc me please? | 15:17 |
| natefinch | fwereade: sure thing | 15:18 |
| rogpeppe | fwereade, natefinch: i think that it'll probably work actually. | 15:18 |
| rogpeppe | fwereade, natefinch: because SUDO_[GU]ID won't be set, so it won't ever try to do the os.Chown | 15:18 |
| natefinch | rogpeppe: interesting... true, but is that still the correct behavior? I'm not sure what uid == 0 means | 15:20 |
| natefinch | (in the linux case) | 15:20 |
| fwereade | natefinch, don't suppose you have a windows box handy to try it out on? | 15:20 |
| natefinch | fwereade: sure do. | 15:20 |
| fwereade | natefinch, great :) | 15:21 |
| rogpeppe | natefinch: it means SudoCallerIds didn't find SUDO_UID and SUDO_GID i think | 15:21 |
| dimitern | fwereade, updated https://codereview.appspot.com/14231044/ and the live upgrade from 1.14 to 1.15 passes | 15:21 |
| dimitern | fwereade, but I decided to try changing version to 1.16.0 and try to upgrade to that, just in case | 15:22 |
| dimitern | fwereade, but I'm running into issues, like phantom debug logs which are not where they say they are in the source and indeed nowhere to be seen | 15:22 |
| * fwereade does not like the sound of *that* -- they're not 1.14ones, are they? | 15:23 | |
| dimitern | fwereade, http://paste.ubuntu.com/6184278/ | 15:23 |
| dimitern | fwereade, no | 15:23 |
| dimitern | fwereade, in tools/list.go:107 there's indeed a loop to match tools in a slice/List but there's no log message there saying "trying to match tools..." | 15:24 |
| dimitern | fwereade, neither in trunk, nor in 1.14 | 15:24 |
| fwereade | what version do you have installed locally? | 15:25 |
| fwereade | because that "found existing jujud" is a bit scary to me | 15:25 |
| dimitern | fwereade, I have my branch | 15:25 |
| fwereade | dimitern, oh wait that's not so scary, I think, ok | 15:25 |
| dimitern | fwereade, and I did rebuild jujuc and jujud after changing the version to 1.16.0 | 15:25 |
| dimitern | fwereade, the problem is from line 29 on in that paste - before that all seems fine i think | 15:26 |
| fwereade | dimitern, delete pkg/linux_amd64 or whatever it is, maybe? | 15:26 |
| dimitern | fwereade, I'll try | 15:27 |
| rogpeppe | fwereade: any chance of a look at https://codereview.appspot.com/14038045/ please? it's a prereq if addressing is going to get done today | 15:28 |
| rogpeppe | fwereade: (and you have gone over it before, so shouldn't be too onerous) | 15:28 |
| dimitern | fwereade, ok, so the phantom message is gone, but I still get these on lines 30 and 31 | 15:29 |
| dimitern | fwereade, why is the filter empty i wonder? | 15:29 |
| rogpeppe | mgz: it'd be nice if you could look too, as it's our branch: https://codereview.appspot.com/14038045/ | 15:30 |
| natefinch | rogpeppe, fwereade: uh, hmmm... C:\Users\Nate>juju --debug bootstrap | 15:30 |
| natefinch | 2013-10-02 15:29:05 DEBUG juju.environs.configstore disk.go:77 Making C:\Users\Nate\AppData\Roaming\Juju\environments | 15:30 |
| natefinch | 2013-10-02 15:29:05 INFO juju.provider.ec2 ec2.go:215 preparing environment "amazon" | 15:30 |
| natefinch | 2013-10-02 15:29:05 INFO juju.provider.ec2 ec2.go:193 opening environment "amazon" | 15:30 |
| natefinch | 2013-10-02 15:29:05 ERROR juju supercommand.go:282 cannot create environment info "amazon": cannot rename new environment info file: rename C:\Users\Nate\AppDat | 15:30 |
| natefinch | a\Roaming\Juju\environments\098297807 C:\Users\Nate\AppData\Roaming\Juju\environments\amazon.jenv: The process cannot access the file because it is being used b | 15:30 |
| fwereade | dimitern, 1.16.0.*1* is dev | 15:30 |
| natefinch | y another process. | 15:30 |
| rogpeppe | natefinch: oh great | 15:30 |
| rogpeppe | natefinch: i think i know the fix | 15:31 |
| dimitern | fwereade, 1.16.0 is what I set - .1 is added by upload-tools | 15:31 |
| rogpeppe | natefinch: yeah, we need to avoid the defer tmpFile.Close | 15:32 |
| rogpeppe | natefinch: that's useful, thanks | 15:32 |
| natefinch | rogpeppe: I figured it was something like that | 15:32 |
| natefinch | rogpeppe: I don't think I actually tested the changes in that CL, though | 15:32 |
| dimitern | fwereade, i'm trying 1.17.1 now | 15:32 |
| dimitern | fwereade, now it works | 15:33 |
| rogpeppe | natefinch: actually i think you probably have | 15:33 |
| fwereade | dimitern, if you want 1.16.0 I think you'll need to generate tools metadata and sync *that*, won't you? | 15:33 |
| dimitern | fwereade, so we can't use --upload-tools with even minor versions | 15:33 |
| rogpeppe | natefinch: it would have failed earlier if they hadn't worked | 15:33 |
| fwereade | dimitern, we can, but juju knows damn well that uploaded tools arenot releasedones ;p | 15:33 |
| natefinch | rogpeppe: oh, ok. It wasn't clear to me if I'd gotten to that code or not. good. | 15:34 |
| dimitern | fwereade, whew.. ok, 1.17 works and upgrades ok | 15:34 |
| dimitern | fwereade, I'm not even trying to understand that whole complex process just now | 15:34 |
| dimitern | fwereade, maybe tomorrow, once I start on minor version upgraders | 15:35 |
| dimitern | upgrades | 15:35 |
| mgz | rogpeppe: sure, looking | 15:35 |
| mgz | rogpeppe: while you're between things, what's the correct way to run amazone live tests right now? | 15:36 |
| mgz | I get a panic when cdng into provider/ec2 and using -gocheck.f | 15:36 |
| rogpeppe | mgz: i run them with "go test -amazon -test.timeout 2h" | 15:36 |
| mgz | hm, probably can't do the cd | 15:36 |
| rogpeppe | mgz: some of the live tests don't work in isolation | 15:36 |
| rogpeppe | mgz: and many of the live tests are broken currently because they don't upload the tools correctly | 15:37 |
| mgz | hm, indeed, not singling out one test to run does things | 15:38 |
| rogpeppe | mgz: i have a fix that makes some of the live tests work, but breaks the normal tests. i want to do better, but i'm concentrating on addressupdater for the moment. | 15:38 |
| dimitern | fwereade, btw https://codereview.appspot.com/14231044/ still waits final approval, now after live tests passed | 15:38 |
| rogpeppe | mgz: which test were you trying to run and how did it fail? | 15:38 |
| mgz | okay, the test lacks a prepare... | 15:39 |
| mgz | panic on first line, which uses t.Env | 15:39 |
| mgz | okay, that's an easy fix | 15:40 |
| mgz | man, we realy should just isolate by default | 15:40 |
| mgz | sloppiness | 15:40 |
| rogpeppe | natefinch, fwereade: fix configstore under Windows: https://codereview.appspot.com/14285043 | 15:42 |
| rogpeppe | mgz: the live tests are deliberately not isolated because we don't want them to take 4 hours to run | 15:43 |
| rogpeppe | mgz: but i've been thinking we could do better | 15:43 |
| mgz | rogpeppe: point there is they should have code to share stuff, not need code to not-share stuff | 15:43 |
| natefinch | rogpeppe: looking | 15:43 |
| rogpeppe | mgz: we could have one suite that just bootstraps once for the whole suite, and no tests in it destroy the environment | 15:44 |
| rogpeppe | mgz: and another which isolates and lets the tests do whatever they like | 15:44 |
| fwereade | rogpeppe, addresspublisher LGTM | 15:45 |
| rogpeppe | fwereade: thanks | 15:45 |
| fwereade | dimitern, rereviewing | 15:45 |
| fwereade | natefinch, would you do rog's review please? | 15:45 |
| natefinch | fwereade: was already looking :) | 15:45 |
| fwereade | natefinch, have to do dimitern's and become coherent andprofessional before client call in 15 mins | 15:45 |
| fwereade | natefinch, <3 | 15:45 |
| natefinch | fwereade: good luck ;) | 15:46 |
| fwereade | dimitern, Idon't understand what's incompatible about the name change -- it's not the API, it's just code | 15:48 |
| dimitern | fwereade, you mean change only the client-side? | 15:49 |
| dimitern | fwereade, SetTools is an API call | 15:49 |
| fwereade | dimitern, it's just a method on an object as far at the provisioner knows | 15:50 |
| dimitern | fwereade, when it's client-side, yes | 15:50 |
| dimitern | fwereade, I though you wanted me to change the SetTools server-side and client-side API calls to SetVersion | 15:51 |
| dimitern | fwereade, to be compatible, we can just change the client-side | 15:51 |
| natefinch | rogpeppe: I see the same pattern in utils.WriteYaml, but that's only called by the uniter, so shouldn't get run on Windows. It's a good thing to keep in mind, though. | 15:51 |
| rogpeppe | natefinch: yeah | 15:51 |
| fwereade | dimitern, state object changes: good | 15:51 |
| fwereade | dimitern, wire protocol changes: bad (except when analyzed carefully) | 15:52 |
| fwereade | dimitern, state/api code changes for clarity: good, I think | 15:52 |
| fwereade | dimitern, and the state/api interface change has nothing to do with compatibility | 15:52 |
| fwereade | dimitern, just what wesend on the wire | 15:52 |
| dimitern | fwereade, yeah | 15:56 |
| dimitern | fwereade, ok I'll change the client-side to be SetVersion, and keep the server-side as SetTools | 15:56 |
| fwereade | dimitern, but, look, I'm not really bothered about the *actual* name of that method in state/api -- but essentially I think that everything marked DEPRECATE is just unnecessary | 15:56 |
| fwereade | dimitern, I don't want to change the wire protocol further unless there's a behaviour benefit | 15:57 |
| fwereade | dimitern, the suckiness of Version{Version: "foo"}} is not worth the churn IMO | 15:57 |
| dimitern | fwereade, how is it unnecessary? | 15:57 |
| dimitern | fwereade, if we change these right away we're making it incompatible | 15:57 |
| dimitern | fwereade, if we change it at a later release, it'll be ok | 15:58 |
| fwereade | dimitern, I'm telling you *not to change them* unless there's a reason involving something being broken | 15:58 |
| dimitern | fwereade, I'm not changing them | 15:58 |
| fwereade | dimitern, our updating and compatibility story is, yu may have observed, shit | 15:58 |
| fwereade | dimitern, you're apparently saying "I will change them later" | 15:59 |
| fwereade | dimitern, I don't think that's worth committing to | 15:59 |
| fwereade | dimitern, lots of places the API is inelegant | 15:59 |
| dimitern | fwereade, well it is, for clarity | 15:59 |
| dimitern | fwereade, having Tools field which is actually just a version | 15:59 |
| fwereade | dimitern, every such change carries a massive cost in compatibility | 15:59 |
| dimitern | fwereade, I want to commit to improving the api over time | 16:00 |
| dimitern | fwereade, without breaking anything now, hence the DEPRECATE tags | 16:00 |
| dimitern | fwereade, g+? | 16:00 |
| fwereade | dimitern, sorry, meeting started | 16:01 |
| dimitern | fwereade, ok | 16:01 |
| dimitern | fwereade, although please ping me when done | 16:02 |
| mgz | oh gawd, this is amusing | 16:09 |
| mgz | I've actually got the change needed for vpc down to one thing, as the real behaviour is quite a bit more liberal than docs and earlier experimentation implied | 16:09 |
| mramm | awesome! | 16:11 |
| natefinch | mgz: it's nice when things turn out easier than expected | 16:14 |
| hazmat | mgz, nice | 16:27 |
| hazmat | mgz, you mean the sec group id vs name distinction isn't black and white? | 16:28 |
| mgz | nope, it *only* matters for filters, not general params, the online docs state it should matter for everything | 16:30 |
| mgz | so, it's fewer extra apis calls to fix for now | 16:30 |
| mgz | the fun bit being we have zero test coverage for this inside juju-core... | 16:30 |
| mgz | the one security group focused live test actually passed on default vpc as it didn't exercise the problem case | 16:31 |
| mgz | ...or something else weird is going on with live tests | 16:33 |
| mgz | ah, ciute, I see | 16:34 |
| TheMue | so, will leave now until monday. tomorrow national holiday | 17:00 |
| TheMue | enjoy your evenings/weekend | 17:00 |
| fwereade_ | right, I might go and see my family for a bit -- does anyone need anything from me imminently? | 17:05 |
| natefinch | fwereade_: it's your birthday, stop working! :) | 17:10 |
| hazmat | adam_g, the problem with switching to lp:juju-deployer atm appears to be i don't have access to it. the branch needs to be owned by the group juju-deployers, atm its owned by you, ie for comparison darwin series branch is owned by the group | 17:42 |
| adam_g | hazmat, updated | 17:43 |
| hazmat | adam_g, thanks | 17:47 |
| hazmat | adam_g, not quite.. you have to push the branch to lp:~juju-deployers/juju-deployer/trunk and update the series branch.. branch is currently pointed to a personal branch. lp:~gandelman-a/juju-deployer/trunk - | 17:48 |
| adam_g | hazmat, one sec | 17:49 |
| hazmat | jamespage, deployer 0.25 uploaded to pypi fwiw, no support yet for to = [1,2,3] | 17:51 |
| adam_g | hazmat, look better? | 17:52 |
| hazmat | adam_g, looks good, thanks | 17:55 |
| hazmat | pushed latest darwin merges to that | 17:56 |
| hazmat | ahasenack, could you update your recipe to point to trunk instead of darwin when you have a chance. | 17:56 |
| ahasenack | hazmat: oh, sure, I must have forgotten | 17:57 |
| ahasenack | no, wait, that was something else | 17:57 |
| ahasenack | hazmat: so your "fork" was merged then | 17:58 |
| hazmat | ahasenack, yes | 17:58 |
| ahasenack | hazmat: is lp:juju-deployer enough, or do I need to specify the full path? Depends on how "development focus" is setup | 17:59 |
| natefinch | Anyone else think it's a little weird that the help from the juju client includes how to install the juju client? :/ | 17:59 |
| hazmat | ahasenack, lp:juju-deployer is enough | 18:00 |
| ahasenack | ok | 18:00 |
| hazmat | ahasenack, thanks.. also fwiw, the version is now at 0.2.5 | 18:01 |
| ahasenack | on it | 18:01 |
| jamespage | hazmat, thanks - uploaded to saucy | 18:42 |
| mgz | yak #1 of the day: https://codereview.appspot.com/14302043 | 19:18 |
| sinzui | I see this file in a place I don't think it belongs | 19:22 |
| sinzui | s3://juju-dist/tools/releases/juju-1.10.0-precise-amd64.tgz | 19:22 |
| sinzui | ^ I think it is a test | 19:22 |
| sinzui | ^ I want to delete it, or else I need to write a rule to never pull it down | 19:22 |
| jam | sinzui: delete it, it isn't referenced in the released:tools.json file | 19:24 |
| sinzui | jam, thank you | 19:24 |
| fwereade_ | jam, hey... we could actually delete all that public-bucket-related code now, couldn't we? | 19:32 |
| jam | fwereade_: I would need to actually audit for it, but I believe so | 19:32 |
| jam | fwereade_: I was looking around earlier and it seemed that way, but it is still mentioned in some tests, tec. | 19:33 |
| jam | etc | 19:33 |
| fwereade_ | jam, indeed | 19:33 |
| jam | fwereade_: hopefully my patch lands, I'm off to sleep | 19:34 |
| fwereade_ | jam, tyvm, sleep well | 19:34 |
| rogpeppe | mgz: i reviewed https://codereview.appspot.com/14302043/ | 20:00 |
| mgz | rogpeppe: thanks, rest incoming, was going to break up, but not *too* huge I think (though will take me a little longer to test out) | 20:06 |
| rogpeppe | mgz: i've done the rest of the address updater BTW, but still lack most of the tests | 20:07 |
| thumper | morning | 20:11 |
| natefinch | thumper: morning | 20:13 |
| thumper | natefinch: how are things going? | 20:14 |
| natefinch | thumper: good good. documenting stuff. | 20:14 |
| thumper | fwereade_: anything I need to be aware of? | 20:14 |
| thumper | natefinch: the maas tags? | 20:15 |
| natefinch | thumper: and azure provider (and cleaning up some of the docs on the other providers) | 20:15 |
| * thumper nods | 20:15 | |
| natefinch | thumper: tags are really pretty much already documented as a part of the constraints docs I did yesterday | 20:16 |
| natefinch | oh, I also bugged the web guys to put a menu item for Docs on juju.ubuntu.com | 20:16 |
| natefinch | well, I bugged Nick, and he bugged the web guys | 20:17 |
| thumper | :) | 20:18 |
| natefinch | thumper, mgz, rogpeppe, fwereade_: I noticed juju help aws is just about the only place we use the term aws, everywhere else we use ec2 or amazon.... in fact, I went through juju help amazon and juju help ec2 before going to juju help to find out what the right term was. | 20:19 |
| thumper | hmm... we really want topic aliases | 20:20 |
| rogpeppe | natefinch: ec2 would probably be a better term to use | 20:20 |
| thumper | agreed, since it is the type we use in the config | 20:20 |
| rogpeppe | yup | 20:20 |
| natefinch | yeah, I was thinking ec2 being the displayed topic, and then having aliases for amazon and aws | 20:20 |
| thumper | rogpeppe: do you know the current api only for non-bootstrap nodes is going? | 20:21 |
| rogpeppe | thumper: i'm having difficult parsing that question... | 20:22 |
| rogpeppe | ulty... | 20:22 |
| thumper | rogpeppe: we are wanting agents on non-bootstrap nodes to not access state directly | 20:22 |
| thumper | rogpeppe: do you know if this is done? | 20:22 |
| rogpeppe | thumper: i think it is, yes, although it's possible the final branch hasn't actually landed yet | 20:23 |
| thumper | rogpeppe: do you know the status of the CLI using the api? | 20:23 |
| rogpeppe | thumper: not done | 20:23 |
| rogpeppe | thumper: there's one command that uses the API | 20:23 |
| thumper | is it status? | 20:23 |
| rogpeppe | thumper: i wish | 20:23 |
| rogpeppe | thumper: no, it's get | 20:23 |
| thumper | ah... | 20:23 |
| thumper | I heard that status is particularly shit with low latency to the cloud | 20:24 |
| thumper | rogpeppe: have you started on the ha stuff? | 20:24 |
| rogpeppe | thumper: no | 20:24 |
| thumper | I'm looking at the address publisher and thinking that's what it is for | 20:25 |
| thumper | is it not? | 20:25 |
| rogpeppe | thumper: it's a necessary prereq, but it's also for the addressability stuff | 20:25 |
| * thumper nods | 20:25 | |
| thumper | ok | 20:25 |
| rogpeppe | thumper: and it's also for the API endpoint caching | 20:25 |
| fwereade_ | thumper, rogpeppe: fwiw, get and add-unit use the API | 20:45 |
| rogpeppe | fwereade_: ah, i'd forgotten about add-unit | 20:46 |
| mgz | well, this is a barrel of laughs | 21:34 |
| thumper | mgz: what's that? | 21:36 |
| arosales | thumper, whats your feeling on https://bugs.launchpad.net/ubuntu/+source/juju-core/+bug/1219879 | 21:37 |
| _mup_ | Bug #1219879: [FFe] juju-core 1.16 for Ubuntu 13.10 <juju-core (Ubuntu):Confirmed> <https://launchpad.net/bugs/1219879> | 21:37 |
| mgz | silly changes needed to goamz due to test server limitations/oddnesses with what the real stuff actually supports | 21:37 |
| arosales | for US Oct 3rd | 21:37 |
| thumper | arosales: I'm adding a comment | 21:38 |
| arosales | https://launchpad.net/juju-core/+milestone/1.16.0 isn't looking good either | 21:38 |
| thumper | arosales: heh, no | 21:39 |
| arosales | thumper, is your comment going to address what is going to land feature wise per the bug description? | 21:39 |
| thumper | arosales: https://launchpad.net/juju-core/+milestone/1.15.1 is better | 21:39 |
| thumper | arosales: well, with 1.14, the local provider is broken on saucy | 21:40 |
| thumper | arosales: 1.16 unfucks it | 21:40 |
| arosales | nice to see some fix committed | 21:40 |
| arosales | thumper, lol | 21:40 |
| arosales | thumper, please add that to the release notes | 21:40 |
| mgz | yak #2: https://codereview.appspot.com/14304043 | 21:42 |
| arosales | thumper, I can defer bug 1214178 to post 1.16 | 21:43 |
| _mup_ | Bug #1214178: Azure provider configuration is difficult to configure <azure> <papercut> <juju-core:Triaged> <https://launchpad.net/bugs/1214178> | 21:43 |
| arosales | thumper, I'll look for a couple others to retarget | 21:44 |
| thumper | arosales: I have a feeling that most of those on 1.16 will need to be pushed off | 21:44 |
| arosales | we'll need to hit some, but I agree most will have to be re-targetted | 21:45 |
| * arosales just wanted to defer the low hanging fruit | 21:45 | |
| natefinch | thumper, mgz, anyone else: https://codereview.appspot.com/14207048/ just some doc changes. Except for help_topics.go, most of it is just formatting changes, not content changes. | 21:51 |
| fwereade_ | rogpeppe, hey, what's the deal with verifyAllConfig in ec2? | 21:52 |
| * rogpeppe looks | 21:52 | |
| rogpeppe | fwereade_: ah, ok | 21:53 |
| fwereade_ | rogpeppe, looks like it's already gone through validation before it's called | 21:53 |
| rogpeppe | fwereade_: i'm coming to that conclusion | 21:54 |
| rogpeppe | fwereade_: i think what i was aiming towards is that no defaults come into play at that point | 21:56 |
| fwereade_ | rogpeppe, I think it's too late for that | 21:56 |
| rogpeppe | fwereade_: yes, we need a better SetConfig | 21:57 |
| rogpeppe | fwereade_: i think we should leave those changes for another day | 21:57 |
| fwereade_ | rogpeppe, sorry, which? picking control-bucket? | 21:58 |
| fwereade_ | rogpeppe, I'm not seeing the issue, expand please | 21:58 |
| rogpeppe | fwereade_: the issue i'm suggesting leaving alone for now is that providers should not pick default values except right at the beginning. | 21:58 |
| fwereade_ | rogpeppe, ah, ok, makes some sense; and rest easy, I have no intention of touching it :) | 21:59 |
| rogpeppe | fwereade_: cool. just throw validateAllConfig away. | 22:00 |
| mgz | geh, what's the idiom for returning an empty struct in an error case, without needing to type `return package.StructName{}, err` everytime? | 22:04 |
| rogpeppe | fwereade_, mgz: review would be much appreciated: https://codereview.appspot.com/14306043 | 22:13 |
| rogpeppe | mgz: var zero = package.StructName{} | 22:13 |
| mgz | rog, perfect timing, just started livetest run so otherwise idle | 22:13 |
| rogpeppe | mgz: lovely | 22:13 |
| mgz | rog, just at the top of the func? guess it makes sense as I use it twice | 22:14 |
| mgz | rogpeppe: while I read, see 14304043 for me :) | 22:14 |
| rogpeppe | mgz: looking | 22:15 |
| rogpeppe | mgz: reviewed | 22:19 |
| mgz | I'm about half way :) | 22:19 |
| rogpeppe | mgz: pretty good considering yours is about 1/16th the size :-) | 22:21 |
| rogpeppe | mgz, fwereade_: do you think addressupdater should be part of JobManageState or JobManageEnviron ? | 22:21 |
| fwereade_ | rogpeppe, feels a bit statey tome | 22:22 |
| fwereade_ | to me | 22:22 |
| rogpeppe | fwereade_: i started off with that, but just decided otherwise | 22:22 |
| fwereade_ | rogpeppe, but it's most closely connected with the provisioner, indeed | 22:22 |
| rogpeppe | fwereade_: currently everything in ManageState could theoretically live without a valid Environ | 22:23 |
| fwereade_ | rogpeppe, nice theory :) | 22:23 |
| rogpeppe | fwereade_: lol | 22:23 |
| rogpeppe | fwereade_: that's my aim, eventually anyway | 22:23 |
| fwereade_ | rogpeppe, I think it's safe to say that whatever we pick we'll find out why it was wrong soon enough | 22:24 |
| fwereade_ | rogpeppe, follow your heart :) | 22:24 |
| rogpeppe | fwereade_: ok will do... | 22:24 |
| fwereade_ | rogpeppe, (I take particular pleasure in that phrase as a simpsons echo... "hey, boss, should I shoot him gangland-style or execution-style?" "follow your heart" | 22:26 |
| fwereade_ | ) | 22:26 |
| rogpeppe | . | 22:26 |
| rogpeppe | :-) | 22:26 |
| hatch | hey all - in charm options are .'s a valid word delimeter? 'foo.bar.baz' vs 'foo-bar-baz' | 22:27 |
| fwereade_ | hatch, I would not personally stake money on .s working right | 22:28 |
| hatch | fwereade_ ok there is an issue with the Hadoop charm, and the GUI because it uses .'s | 22:29 |
| hatch | could we set a rule that they are invalid? :) | 22:29 |
| fwereade_ | hatch, hmm, is it just hadoop, do you know? | 22:29 |
| hatch | fwereade_: I haven't been able to find another | 22:30 |
| hatch | and I have been looking :) | 22:30 |
| fwereade_ | hatch, ok, then, I'm more than happy to call that invalid -- do you know what happens if you try to use those settings on the CLI? | 22:31 |
| hatch | rumor has it it works | 22:31 |
| hatch | I'm going to try it now | 22:31 |
| hatch | it'll take a bit for it to deploy | 22:32 |
| fwereade_ | hatch, ok, it would be great if it did break somewhere so I could declare it to be poor input validation with a 100% clear conscience | 22:32 |
| hatch | certainly - the fact it's an approved charm with them made me a little curious | 22:32 |
| hatch | so once it spins up I'll see if I can set a config option | 22:32 |
| fwereade_ | hatch, would you also debug-hooks it please, and see whether you can config-get it correctly too? | 22:33 |
| hatch | fwereade_: I've never done that before - would it be `juju debug-hooks hadoop/0 config-changed` ? | 22:35 |
| fwereade_ | hatch, that sounds right, but yu can skip the hook name to debug everything iirc | 22:35 |
| fwereade_ | hatch, (do it in a separate terminal or it will be frustrating) | 22:36 |
| hatch | will do | 22:36 |
| rogpeppe | mgz: " | 22:40 |
| rogpeppe | I assume this gets cleaned up at test end anyway, so doesn't need any of its | 22:40 |
| rogpeppe | own? | 22:40 |
| rogpeppe | " | 22:40 |
| rogpeppe | mgz: i don't get that remark | 22:40 |
| mgz | we're fiddling with something global-lloking that gets passed in | 22:43 |
| mgz | I'm just used to change-thi-thing-for-testing function coming with their own cleanup | 22:43 |
| mgz | rogpeppe: do you know any way of switching region with the goamz live tests? | 22:44 |
| rogpeppe | mgz: state is reset at test end, yeah | 22:44 |
| rogpeppe | mgz: not sure. | 22:44 |
| rogpeppe | mgz: i'm not sure where they take their region from | 22:44 |
| mgz | yeah, I couldn't see at a glance | 22:45 |
| mgz | I have a hack that works for the juju-core ones... | 22:45 |
| mgz | hm, the live TestBootstrapAndDeploy test is failing for me... and I think causing other fallout | 22:58 |
| mgz | guess I just propose without confidence of the live tests passing for now | 22:58 |
| hatch | fwereade_: I am not able to set config options via the CLI with .'s FYI | 23:01 |
| fwereade_ | hatch, awesome! sounds like an input-validation bug in juju-core, and bad-config bug in the hadoop charm :) | 23:02 |
| hatch | yup - ok thanks, I appreciate the help | 23:02 |
| fwereade_ | anyone free to review https://codereview.appspot.com/14307043 quickly? | 23:03 |
| mgz | fwereade_: looking | 23:09 |
| mgz | yaks shaved, payoff: https://codereview.appspot.com/14309043 | 23:10 |
| mgz | and day done | 23:20 |
| rogpeppe | mgz: well done | 23:26 |
| rogpeppe | mgz, fwereade_: this actually integrates the address updater: https://codereview.appspot.com/14251044 | 23:26 |
| rogpeppe | fwereade_: looking | 23:26 |
| rogpeppe | fwereade_: reviewed | 23:38 |
| rogpeppe | thumper: any chance of a review of https://codereview.appspot.com/14251044 ? | 23:39 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!