davecheneywallyworld_: https://code.launchpad.net/~dave-cheney/juju-core/134-tag-release-1.11.2/+merge/17408700:57
davecheney^ tagging, mark II00:57
* wallyworld_ takes a look00:58
davecheneythumper still got man flu ?00:58
=== Guest18637 is now known as DarrenS
wallyworld_davecheney: out of curiosity, i listed the current tags on trunk and there's some weird ones there. nothing resembling a juju release01:08
davecheneythey look like old juju 0.6/7 tags01:11
davecheneyblame gustavo01:11
wallyworld_ok. and we haven't tagged 1.10 etc?01:12
wallyworld_seems kinda strange not to have tagged the releases01:13
davecheneywallyworld_: yes, this is suckful01:13
davecheneyi'm trying to redeem myself01:14
wallyworld_you don't need to try too hard. you are always pushing to get proper processes in place which i fully agree with01:14
* davecheney blushes01:15
wallyworld_davecheney: sorry for making your pocket all wet01:15
davecheneyoh, i thought I had a drinking problem01:15
wallyworld_that's not mutually exclusive :-)01:16
davecheneywallyworld_: can I try submitting this tag change ?01:21
davecheneysee if it works this time ?01:21
wallyworld_sure. i've not used bzr to create tags before. i think it will work01:22
davecheneywallyworld_: related, https://docs.google.com/a/canonical.com/document/d/1eeHzbtyt_4dlKQMof-vRfplMWMrClBx32k6BFI-77MI/edit01:24
davecheneywallyworld_: https://code.launchpad.net/~dave-cheney/juju-core/134-tag-release-1.11.2/+merge/17408701:43
davecheneymerged fine, but not tag :(01:43
wallyworld_bigjools: any hints? ^^^^^^01:44
bigjoolswallyworld_: wat?02:14
wallyworld_see the backscroll. davecheney wants to tag a rev for the 11.2 release. he added a tag locally, pushed up a mp, merged, but the tags didn't get copied in02:14
* bigjools looks02:15
bigjoolsit's not merged02:20
wallyworld_bigjools: davecheney: the mp says merged though, doesn't it?02:21
bigjoolsit does, but if you look at the branch history, it's not02:22
wallyworld_so what would cause that?02:22
bigjoolsbecause not all the revisions were copied - the tag was not on the top revision02:23
bigjoolsmore to the point there's no tag on r1415 but there is one on r1414 which is dimiter's rev02:23
bigjoolsie the tag was put on a revno that is already merged02:24
bigjoolsrather than the empty commit02:24
davecheneybigjools: how would you do it ?02:24
davecheneyi used02:24
davecheneybzr tag ; bzr commit --unchanged02:24
wallyworld_i assume the command run locally was bzr tag -r 1214 blah?02:24
bigjoolscommit first then tag02:24
bigjoolstag works on committed revnos02:25
bigjoolsyou can see what you did on here: http://paste.ubuntu.com/5863584/02:26
bigjoolscan you bzr tag lp:juju-core directly?02:26
bigjoolsjust tag the revision you just landed02:27
davecheneybigjools: i don't think we can, Go bot owns everything02:27
wallyworld_davecheney: didn't you want to tag rev 1214?02:27
davecheneywallyworld_: no02:27
wallyworld_davecheney: if your ssh keys are registered, you can commit directly to juju-core02:28
bigjoolsdavecheney: should work though, your key is on it02:28
davecheneywallyworld_: ok, i'll do that02:28
bigjoolsdon't commit again02:28
bigjoolsjust tag02:28
bigjoolsbzr tag -d lp:juju-core02:29
bigjoolswith a -r of course02:29
davecheneybigjools: right-o, thanks02:31
bigjoolsnae prob02:31
fwereadedavecheney, oh hell, I thought mgz was handling 1.11.2... there are a couple of open issues aimed for that milestone03:12
fwereadewallyworld_, sorry I fell asleep, I'll do your reviews now03:12
davecheneyfwereade: ok, how should I fix this ?03:13
wallyworld_fwereade: no hurry. it's the middle of the night for you. just do them later!03:13
wallyworld_fwereade: btw, will have add-unit with force machines up for review soon03:14
fwereadedavecheney, depends how far the release has progressed -- if it's done, but just includes bugs we hoped it didn't, I'm fine doing another for 1.11.4 tomorrow I guess?03:15
fwereadewallyworld_, yeah, but I've just woken up, may as well do something useful03:15
davecheneyfwereade: i've beent trying to release this fucker for two weeks03:15
davecheneywe can always do another release03:15
davecheneywe can do as many as we like03:15
fwereadedavecheney, yeah, indeed03:16
davecheneythe next release number is 1.11.303:16
davecheneybut see https://docs.google.com/a/canonical.com/document/d/1eeHzbtyt_4dlKQMof-vRfplMWMrClBx32k6BFI-77MI/edit#03:16
fwereadewallyworld_, crap, what "containertype" is set on a new environ machine?03:32
wallyworld_fwereade: ""03:32
fwereadewallyworld_, ah ok, that does make sense, I expected it to be "none" for a moment03:32
wallyworld_all new machines which are not containers get ""03:32
wallyworld_none is only for constraints03:33
wallyworld_add-unit force machine is up https://codereview.appspot.com/1096204603:33
fwereadewallyworld_, cool, I have a review of https://codereview.appspot.com/11019044/ up, I may have misunderstood some things03:45
* wallyworld_ looks03:45
wallyworld_fwereade: afaik, deployment constraints for stuff like mem are not yet supported - there is a todo in the deploy command03:48
wallyworld_hence there's no work done with those in th assignment policies right now03:48
fwereadewallyworld_, please, what is the difference between a "deployment" constraint and a "provisioning" constraint?03:49
fwereadewallyworld_, you keep talking as if there's a distinction but I don't see one03:49
wallyworld_sorry, i might be confused. isn't provisioned related to starting a machine?03:49
wallyworld_and deployment related to deploying a charm03:49
fwereadewallyworld_, sure -- but what's the distinction from the POV of the constraints?03:50
fwereadewallyworld_, (deployemtn is a very hairy and overloaded word in juju land :/)03:50
fwereadewallyworld_, constraints are still constraints and still expected to apply, whether to a new machine or an existing one03:51
wallyworld_not sure what is meant by POV of constraints. right now, when a unit is deployed, constrints are ignored03:51
fwereadewallyworld_, sure, but that's just a bug, right?03:51
fwereadewallyworld_, not a fundamental distinction03:51
wallyworld_but this mp is not aout fixing that bug03:51
wallyworld_since when it was started, not hardware characteristics were there03:52
wallyworld_or so i recall03:52
wallyworld_the rest of the constraints is coming03:52
wallyworld_this mp is just for th contsainer stuff03:52
fwereadewallyworld_, yeah, I see... I worry that this puts the cart before the horse a little03:53
wallyworld_only for a short time, and it is not advertised functionality - it's an interim step that is useful to us as developers03:53
wallyworld_helps greatly to progress the container work03:53
wallyworld_as soon as the hardware for bootstrap machine is done, i'm onto this next step03:54
wallyworld_it allows us to more easily sort out the addressability work03:55
wallyworld_fwereade: with the mongo query you question - you suggested an alternative but i did reply saying i didn't think it would work. the machineDoc doesn't have any "children" attribute and mongo doesn't support cross document queries03:59
wallyworld_hence i need to read in the containers and do a $nin03:59
wallyworld_it sucks balls but that's mongo for you04:00
fwereadewallyworld_, ha, that would make sense then04:00
wallyworld_sadly it appears so :-(04:01
wallyworld_fwereade: replied to your comments04:06
wallyworld_also, the AddMachineParams is used outside of constraints for add-machine etc04:06
wallyworld_so it's ContainerType attribute makes sense04:07
fwereadewallyworld_, ok, that's seeming clearer now04:16
fwereadewallyworld_, I have a couple of comments on https://codereview.appspot.com/10777044/ -- thoughts?04:16
* wallyworld_ looks04:16
fwereadewallyworld_, (there probably is a good reason, I just can't see it, and because I can't see it the drawbacks seem stark)04:17
wallyworld_fwereade: the "--force-machine lxc" syntax is analogous to "add-machine lxc". it means to create a new container on a new instance04:20
wallyworld_note i got rid of the leading "/" for add-machine lxc04:20
fwereadewallyworld_, yeah, I think the problem is in the implicit creation via these paths, not just the syntax xhosen to do it04:21
wallyworld_but we want to support "--force-machine 25/lxc" ?04:21
wallyworld_which creates a new container on machine 2504:22
fwereadewallyworld_, and ctype is ignored if mid == "" in AddUnits anyway04:22
fwereadewallyworld_, yeah, I think so04:22
wallyworld_ok, i'll fix that. sorry, i've got so many branches on the go with tweaks in each i may have lost track a bit04:23
fwereadewallyworld_, I know the feeling :)04:23
fwereadewallyworld_, sorry, though, I'm not quite clear which bit you'll be fixing04:24
wallyworld_1st branch (clean assignment policy): pass in entire constraints struct and for now just use container type04:24
wallyworld_instead of just passing in container type. less churn later04:25
wallyworld_3rd branch (add unit force machine): disallow "lxc" and defer splitting up of force machine spec04:25
fwereadewallyworld_, I think that's the second branch04:26
fwereadewallyworld_, but that sounds perfect :)04:26
wallyworld_i think its the 3rd04:27
wallyworld_let me check, just gotta switch back to it04:27
wallyworld_pipeline is04:27
wallyworld_   clean-policy-container04:27
wallyworld_   force-machine-containers04:27
wallyworld_*  add-unit-force-machine04:27
fwereadewallyworld_, it's force-machine-containers; I haven't looked at the 3rd branch yet but so long as it follows what we just discussed I'll be happy :)04:27
wallyworld_ah sorry, was getting ahead of myself04:28
wallyworld_3rd one adds force machine to add-unit04:28
wallyworld_similar to 2nd04:28
fwereadewallyworld_, cool04:28
fwereadewallyworld_, if you don't mind I will not look at that one right away because the most obvious comment will be "make it like the second one"04:29
wallyworld_sure, np. thanks for the input04:29
fwereadewallyworld_, cheers04:29
* fwereade is just glad he's ~up to date with your reviews before your eod, roughly04:30
* fwereade will be back a bit later04:32
=== ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: dimitern | Bugs: 6 Critical, 79 High - https://bugs.launchpad.net/juju-core/
jamfwereade: when do you sleep? :)06:47
jamdimitern: care to chat about when the password stuff triggers? I'm trying to sort out why the api is spinning after an upgrade. I have a thought, but I don't have as much depth here.06:47
dimiternjam: sure, as far as I can help06:48
jamdimitern: so we have a field OldAPIPassword, but that field is not referenced anywhere in the code. Can you confirm it has never been used?06:49
jamI think there was a discussion as to whether we have 2 passwords or just 1 for API vs direct State access06:49
jamI don't quite know what the current state is.06:49
jamor what is was in 1.1006:50
dimiternjam: if it's not used now that's new to me - it used to be used for the inital connection / password changing dance06:50
jamdimitern: agent/agent.go line 227 has "info.Password = info.OldPassword"06:50
jamNothing in there touches APIPassword or OldAPIPassword06:50
dimiternjam: it's passed in agent conf on bootstrap i think06:51
dimiternjam: and then just changed06:51
jamdimitern: so, a small bit of confusion before clarity. There is agent.Conf which has OldAPIPassword in it, which is never used (that I can see)06:52
jamthere is also an *optional* APIInfo pointer06:52
jamwhich can contain something which holds Password and OldPassword06:52
dimiternjam: if you take a look at machine.SetPassword - roger's recent changes made it call SetMongoPassword as needed06:53
jamapi.Info doesn't have a spot for "OldPassword"06:53
dimiternjam: because at bootstrap time there's no api yet06:54
jamdimitern: so on fresh install of trunk, in agent.conf, I have: oldpassword: "", stateinfo: password: XXX, apiinfo:password: XXX where XXX is the same06:55
jamdimitern: on an upgrade, I have: oldpassword: Something, stateinfo: password: somethingdifferent, apinfo:password: ""06:56
jamI have the feeling we need to use stateinfo.password for the apinfo.password06:56
jambecause the code is looping trying to connect with oldpassword and failing.06:56
dimiternjam: seems correct yes - api and state passwords should match06:56
jamdimitern: so what I *think* happened, in 1.10 we would bootstrap, create the agent.conf without an api password. The State connection would connect, and then immediately change the password, but it doesn't record that fact in the API password, only the stateinfo.password06:59
dimiternjam: seems likely, because iirc at that time api password changing was not working correctly07:00
fwereadejam, dimitern: as far as I'm aware stateinfo and apiinfo duplicate the same information, apart from the addresses07:01
dimiternfwereade: yes, or they should at lest07:01
jamfwereade: as in, we should just ignore APIInfo.Password and always use StateInfo.Password ?07:01
fwereadedimitern, yeah, this is why I bitch and moan about these sorts of OAOO violations07:01
fwereadejam, i suspect that's actually worth a try... but probably not sane long-term07:02
jamwhat about the vestigial Conf.OldAPIPassword07:02
dimiternoaoo ?07:02
fwereadedimitern, Once And Only Once07:02
fwereadejam, on balance I think I would not have a problem with a tweak to agent.Conf that effectively dropped all of apiinfo apart from the addresses07:04
jamfwereade: is the thought that eventually we don't want to allow direct state access, so we actually want to drop the StateInfo side of that file?07:05
fwereadejam, but agent.Conf itself is kinda weird, what with how it serializes itself and all07:05
fwereadejam, yeah, basically07:05
fwereadejam, stateinfo is now authoritative but will one day be very infrequently used07:06
fwereadejam, using stateinfo solves a problem today but we'd need to do something like always read from stateinfo, and write to both, and still apply liberally fancy footwork to dance through the next upgrade07:07
fwereadejam, so... I dunno, I said yes above but probably not for pure or defensible reasons07:08
fwereadejam, some of this is academic because apparently dave did a release last night07:08
fwereadejam, I think we should at least be aiming for another one, imminently, that *can upgrade* from 1.10 without devastation07:09
fwereadejam, and consider that one for promotion to 1.1207:09
jamfwereade: so it isn't hard to change agent.OpenAPI to notice that Conf.APIInfo.Password isn't set but Conf.StateInfo.Password is set07:10
jamthough my immediate result is that it may not be enough07:10
fwereadejam, yeah, I feel like there is probably no shortage of edge cases in play07:10
jamfwereade: is there a way to get juju to re-read its agent.conf without killing it?07:11
fwereadejam, heh, it doesn't do that anyway? well that sucks07:11
jamfwereade: afaict I can change it, and it keeps trying with the old content07:12
fwereadejam, oh, ffs07:12
fwereadejam, the original plan was that we store the separate bits in separate files, and make agent.Conf an accessor for those files, precisely so that agents could update parts of their own conf without falling over07:13
fwereadejam, eg so that we can write out state info gleaned from the api and then have our state worker come up because, yay, valid information just appeared07:14
jamfwereade: so ATM, the only place that reads the agent conf is at the beginning of cmd/jujud/machine.go: MachineAgent.Run()07:15
jamso if there is something that restarts the Agent.Run() it will reread it07:15
jamI *think* restarting the API is being done by the runner07:16
fwereadejam, IIRC the justification for not doing that was that the possible errors from reading the conf repeatedly were a hassle and dirtied up the api or something07:16
fwereadejam, yeah, if the conf is just read once in Run() it's useless for that purpose07:16
* fwereade has already used up his daily quota of heavy sighs07:18
dimiternfwereade: can lend you some, if you need ;)07:19
fwereadehaha :)07:19
fwereadeok, it could be worse; there could be snakes in here with me07:20
dimitern:D - with that weather?07:20
fwereadedimitern, I seems to have had the uk's two weeks of nice weather since I arrived07:21
fwereadejam, so, ok, some sort of change to agent conf has to be in the offing07:21
dimiternfwereade: ah, cool - summer time then07:21
fwereadejam, because sooner or later we *will* want to rewrite parts of it07:21
jamfwereade: I know that today it has a fair amount of cruft in it. But as you say, lets get something that *can* upgrade from 1.10 :)07:22
fwereadejam, and when we do that we either have to mess around locking access to it, or we need to break it up into parts that can be read independently07:22
fwereadejam, so I leave it up to your judgment what to actually do about it today07:23
fwereadejam, dimitern, tyvm for reviews07:34
dimiternfwereade: yw07:34
jamfwereade: happy to, you always return the favor.07:36
fwereadejam, the reason for the repeated startsyncs is the separate state objects... the agentState.Sync ensures that the appropriate event has been delivered to the apiserver's watcher, and at some point after that it will change in the local state07:36
jamfwereade: I *really* wish we encoded agent cert differently in the file, it is so hard to read manually.07:36
fwereadejam, tell me about it07:36
jamfwereade: a simple base64 into a string07:36
jamso nice07:36
* fwereade knows, he really does07:37
fwereadejam, you know it'd be even nicer if we just put the cert and key files up directly and manipulated them on the instance07:38
fwereadejam, we put that same data up in far too many ways07:38
fwereadejam, I think there's another copy with the cert and key concatenated for use by the mongodb server itself07:38
jamfwereade, dimitern: so without touching juju-1.11* I tried just editing agent.conf so that it had the same password for agentinfo.password as stateinfo.password. I can see it trying to login, fail, fall back to oldpassword, also fails.07:45
jamIs there a reason to believe that StateInfo.password should be different from APIInfo.password ?07:45
jamwould there be some other field that didn't get set properly?07:46
dimiternjam: i'd look in the agent conf rendering in cloud init07:46
fwereadejam, last time I asked roger (because I get nervous about this stuff periodically) he said they should be the same07:46
jamfwereade: on a fresh trunk install, they are the same07:46
jamand it works07:46
jamI'm trying to figure out if that password is stored somewhere07:46
jamdifferently in the db, for example07:47
fwereadejam, but, upgraded: the state one works, the api one is missing, and the state password doesn't work for the api?07:47
jaminternally we check entity.PasswordValid which compares utils.PasswordHash(password) to entity.PasswordHash07:47
jamfwereade: correct07:47
jamfwereade: so I'm thinking that the "I'm connecting to the DB" password is stored in a mongo config, vs the "I'm a Machine-0 agent" is stored inside the DB07:48
fwereadejam, that should indeed be the case07:48
jamand 1.10 is setting the Mongo password, but not the contents of the entity in the DB07:48
fwereadejam, perhaps we were at one stage setting only the mongo password?07:48
jamso, essentially, the entity *has* no password in the db.07:48
jaminside the db07:48
fwereadejam, that sounds very plausible07:49
jamand we don't know what the password could be, because we only store password hashes.07:49
jamfwereade: so how do we unbreak it?....07:49
jamif entity.PasswordHash == "" any password is valid?07:49
jamfwereade: as an aside, did we sort out how to install lxc on upgrade?07:50
jamas the other 'we can't upgrade' bug07:50
fwereadejam, that strikes me as insanity07:50
fwereadejam, we did not, because I fell asleep yesterday before talking to thumper07:50
jamfwereade: and he's been sick anyway, I believe. Maybe we'll catch him at the standup.07:51
fwereadejam, ah ok07:51
jamfwereade: here are some issues: We can't change the 1.10 upgrade code. So even if we said "while you upgrade, set these values" it only works when upgrading from trunk, not too trunk.07:51
jamSo we need a hook at first run of a new binary?07:51
fwereadejam, I'm having a ciggie before the standup, brb07:51
jammostly just thinking outloud07:52
jamfwereade: confirmed. m.doc.PasswordHash == ""07:58
fwereadejam, balls07:58
fwereadejam, ok we can fix that, we have a state connection07:58
jamfwereade: where does that happen?07:58
jamMachineAgent.Run() calls MachineAgent.FixAllTheUpgradeBreaks() ?07:59
fwereadejam, basically yeah :/07:59
jammgz: team meeting? https://plus.google.com/hangouts/_/bf3f4cfe715017bf60521d59b0628e5873f2a1d308:05
wallyworld_fwereade: so if you are able to +1 the current 3 branches (or indicate what needs fixing), i'll get tim to do another +1 tomorrow so i can land08:47
fwereadewallyworld_, I think what we agreed this morning is fine, I'll add to those CLs08:49
wallyworld_fwereade: ty. i've updated the first 2. i need to port the changes forward to the 3rd08:50
fwereadewallyworld_, awesome, tyvm09:01
mgzyeay, we have a whole bunch of new public ips for canonistack!09:22
mgzit breaks a few things in juju, but hey09:22
dimiternfwereade, jam, TheMue: https://codereview.appspot.com/10949046 - deployer facade (done properly this time, i hope)09:50
TheMuefwereade: I added the race detection report to the issue. to me it looks like the calling of MgoSuite.SetUpTest() and MgoSuite.TearDownTest() dislike the working in parallel.09:51
jamfwereade: just to get your pre-implementation approval before I spend a lot of time on it. We want to add a cmd/jujud/agent.go function which is called something like EnsureValidConfiguration(agent, st *state.State) and that gets called at the beginning of MachineAgent.Run() and ?UnitAgent?.Run()09:51
TheMuedimitern: you've got a review10:03
dimiternTheMue: thanks10:03
TheMuedimitern: yw10:03
jamdimitern: reviewing, I'll have some comments.10:04
dimiternjam: sure10:06
dimiternfwereade: ping10:13
jtv1Is there an easy way to verify in gocheck that a multi-line text contains a particular substring?  I tried Matches in multi-line mode but it only seems to work if my pattern occurs in the *first* line of the text.10:17
=== jtv1 is now known as jtv
dimiternjtv: prepend '(*|\n)' before the regexp10:18
* jtv tries10:18
jtvdimitern: that fails to parse...missing argument to repetition operator: `*`10:19
dimiternjtv: sorry, see this: c.Assert(err, ErrorMatches, "(.|\n)*cannot allocate a public IP as needed(.|\n)*")10:20
dimiternjtv: ErrorMatches (and Matches I think) prepend '^' and append '$' to the regexp, so this is a way around it10:20
jtvOh ffs...10:21
jtvThanks for pointing that one out...  pretty horrible to leave that kind of subtlety undocumented!10:22
dimiternjtv: tell me about it!10:22
dimiternjtv: spend half a day to discover that at the time10:22
jtvWell then I'm very, very glad that I ran into you.  :-)10:23
dimitern;) np10:23
jamdimitern: I'm going to publish my comments, though I want to chat with you about them because I realize I'm *slightly* off in my statements.10:26
dimiternjam: ok10:26
jamdimitern: so the fundamental thing, is you are generating the set-of-all-units at construction of DeployerAPI time.10:27
jamwhich I think is too early.10:27
jamA MachineAgent will be running a long time.10:27
jamand it will get units added and removed as time goes on10:28
dimiternjam: so getAuthFunc gets called every time an entity is about to be accessed, so we get a fresh list of units that are relevent10:28
jamit won't be re-connecting to the API during that time.10:28
jamdimitern: your getAuthFunc only checks the set I think10:28
jamah maybe you're right. It looked like it was canned, but I guess the the outer function returns the inner.10:28
dimiternjam: no the function it returns only checks the set, but the set is constructed in the beginning of each Remove, SetPasswords or Life calls10:29
jamdimitern: so... can we add some tests in deployer_test about the specifics of removing10:29
jamso that we know we got our getAuthFunc right?10:29
dimiternjam: sounds reasonable, will do10:29
dimiternjam: and no, probably WatchUnits is not the best way to get them, but it certainly is the easiest10:30
dimiternjam: otherwise, i have to get all principals, for each one get all subs, and then construct the list10:30
dimiternjam: william suggested using the watcher as a simpler way to do the same10:31
jamdimitern: k. It still feels a bit strange to me. I think the timeout comment still applies.10:32
dimiternjam: i can try relying on the initial changes set and not timeout, but blocking seems even worse10:32
dimiternjam: in production code especially10:32
jamdimitern: (a) timeout of 50ms is way to slow in a real deployment, (b) Given our assumptions about initial event on watchers blocking doesn't seem that bad.10:33
jamIf it actually blocks, we've got code level problems, right?10:33
jamnot "oh the database is busy now".10:33
dimiternjam: not sure, that's why i'd like fwereade opinion on that10:34
dimiternjam: i can change it to block, no problem10:34
fwereadedimitern, sorry, breakfasting10:35
fwereadedimitern, reading back10:35
jamfwereade: you know that you're not allowed regular body functions like eating sleeping or going to the bathroom, right? You're barely allowed to smoke :)10:35
jamdimitern: so if the initial Changes event blocks (hard, as in never completes), it would be because our assumption about changes works is wrong10:36
jamdimitern: If it just takes a while (2s, 5min) it is because the database is heavily loaded.10:36
dimiternjam: so you propose to use LongWait if we need timeouts?10:37
fwereadejam, dimitern: ...and so if that's blocking hard, probably so would an explicit get-and-collect10:37
jamI could argue for some "unreasonably long" timeout, but that would probably be minutes.10:37
jamnot even 500ms10:37
jamfwereade: I would expect so, yes.10:37
jamfwereade: I'm roughly ok using changes as a shortcut for the get+collect because we know changes already has that logic.10:38
fwereadejam, dimitern: do you feel it's unreasonable to depend on a guaranteed first event (which may ofc be a channel close in case of error...)?10:38
dimiternfwereade: so what's gonna be - blocking read or longer timeout?10:38
jamI just think we shouldn't timeout, or should take *much* longer to timeout.10:38
fwereadejam, dimitern: yeah, I would prefer to trust the underlyng watcher to do what it should10:38
jamdimitern: I slightly prefer blocking read in a "less chance we got this tunable parameter wrong"10:38
dimiternfwereade, jam: ok, will change it to blocking then10:38
jamdimitern: I hope I'm getting the concept of when to timeout across. Timeout is useful for detecting when you have bad *code*. That means you want to do it in testing. In production you should have already caught the bad code and fixed it, so you can just trust the thing is written correctly (And if it isn't write more tests :)10:40
jamdimitern: and then on things that are genuinely uncertain10:40
jamlike network access10:40
jamdoes that make sense?10:41
jamfwereade: you're welcome to correct my thought if you feel differently.10:41
jam(I'm certainly willing to debate it)10:41
dimiternjam: the worst thing that can happen is, we'll just return late from the method calling getAuthFunc, with an error10:42
fwereadejam, that makes sense to me at least in this case10:42
jam(13:51:49) jam: fwereade: just to get your pre-implementation approval before I spend a lot of time on it. We want to add a cmd/jujud/agent.go function which is called something like EnsureValidConfiguration(agent, st *state.State) and that gets called at the beginning of MachineAgent.Run() and ?UnitAgent?.Run()10:42
jamsince you were out10:42
fwereadejam, ah sorry10:42
fwereadejam, that sounds plausible to me though10:42
jamfwereade: manager meeting in 15, right?10:43
jamCan I get anything productive done in that time...10:43
fwereadejam, oh, god, already, bah10:43
dimiternguys, since I'm the OCR today, I'll take a look at the review queue after I land this branch of mine10:46
jtvdimitern: I've got some fun ones lined up for you.  :)10:59
dimiternjtv: cool :) keep 'em coming10:59
jamjtv: do we need to update gwacl on go-bot for your patch to land?11:06
rvba`jam: yes11:11
=== rvba` is now known as rvba
rvbajam: but don't update right now, you need the fix in jtv's branch to cope with the changes in gwacl.11:20
jtvrvba: I figured it'd be either yours or mine, doesn't matter which lands first.  :)11:23
jamrvba: do they need to land in lockstep?11:24
rvbajtv: it will be yours.11:24
jtvjam: rvba's branch and mine?  No, they're decoupled otherwise.11:24
jam(the branch can't land without the update but the update can't be done without the branch)11:24
jamjtv: gwacl + juju-core11:24
jtvBut whichever comes first will also necessitate a gwacl update.11:24
rvbajam: that's right.11:25
jtvSo yes, as soon as mine lands, we'll need the gwacl update.11:25
jamjtv: actually, as soon as yours is scheduled for landing11:25
jtvThat works for me too.  :)11:26
jamjtv: which means I (or someone likes me) needs to be pinged when you are ready to mark it APproved11:27
jamjtv: that's my question about lock-step11:27
jamif you can land the code, and then update gwacl, or the reverse it is much easier to do11:27
jtvYes...  unfortunately gwacl upstream is already updatd.11:28
jamjtv: dependency changes are rougly equivalent to db schema changes in LP11:28
jtvYes I understand.11:28
jtvdimitern: allenap` had a better way to get multi-line regex matching...  The "s" flag.  It makes newlines match "."  For example, "(?s).*foo.*" looks for any instance of "foo" in the input.11:37
=== allenap` is now known as allenap
dimiternjtv: if it works, great11:44
dimiternjtv: with the ^$ already in plce11:44
jamfwereade, dimitern: Beta1 freeze is August 29th, so 4 months into the cycle.11:57
jamAlpha 2 will be July 25th.11:57
dimiterncool, soon then11:58
dimiternfwereade: ping13:05
dimiternabentley: hey, after the changes rvsubmit works fine, just the approvers list could still be duplicated13:10
abentleydimitern: I don't understand that.  The approvers are represented as a set, so they can't have duplicates.13:10
fwereadedimitern, pong13:11
abentleydimitern: Could you point me at the case where this happened?13:11
dimiternabentley: I don't know how, but my previous MP had that - 2 LGTMs from 2 people and the list in the commit message contained twice each reviewer name13:11
dimiternfwereade: can you take a look please? https://codereview.appspot.com/10949046/13:11
abentleydimitern: Can you please give me a link so I can check them out?13:12
dimiternabentley: http://bazaar.launchpad.net/~go-bot/juju-core/trunk/revision/141713:12
abentleydimitern: Please give me a link to the merge proposal or the reitveldt code review, not to the branch.13:13
dimiternabentley: it's in the commit message, sorry: https://codereview.appspot.com/10944044/13:14
abentleydimitern: When I run "bzr rv-submit https://code.launchpad.net/~dimitern/juju-core/066-apiserver-stringswatcher/+merge/173896", it wants to append R=fwereade, jameinel to to bottom, which doesn't give any duplicates.  Did you maybe run rv-submit against this branch before I fixed the duplicate problem?13:20
dimiternabentley: ah, sorry - this was before, haven't tested for duplicates after the fix, just ensured the approved revision is set and the bot is happy13:21
dimiternabentley: will try it out by adding an extra lgtm next time i land13:21
abentleydimitern: Sorry, what?  Were you saying the bug still exists without testing whether the bug exists?13:22
abentleydimitern: I fixed the duplicates bug yesterday because you pointed it out.13:23
dimiternabentley: no, i looked at the patch and it seemed so, sorry13:23
abentleydimitern: Okay.13:24
dimiternabentley: didn't claim i've seen the bug, was just asking if it was fixed as well13:24
dimiternabentley: thanks again for the quick fix13:25
abentleydimitern: I thought you were saying the bug still existed when you said "the approvers list could still be duplicated".13:25
dimiternabentley: bad phrasing on my part13:26
abentleydimitern: Okay.13:27
fwereadedimitern, reviewed, let me know your thougts13:45
dimiternfwereade: cheers13:46
dimiternfwereade: i don't get why block the removal of alive units?13:48
fwereadedimitern, because the deployer gosh-darn-well ought to be deploying them, not trashing them13:49
fwereadedimitern, and the provisioner should be doing the same thing in the same situation13:49
dimiternfwereade: and if we want to remove something that's stuck?13:50
fwereadedimitern, it's not the deployer's responsibility to make a unit dying, but it's the deployer's responsibility to lead it to the grave once it is13:50
jamdimitern: fwereade: I'm proposing a patch now which fixes bug #1199915 . It isn't perfectly tested, but I did do a live upgrade test, and it does fix the existing bug13:50
_mup_Bug #1199915: api cannot connect, fills deployed machine log with spam <juju-core:Triaged by jameinel> <https://launchpad.net/bugs/1199915>13:50
fwereadedimitern, we tell the system we want to remove it by making it dying, not alive13:50
jamugh, wrong one13:51
jamI fixed the LXC bug13:51
jambad me13:51
jambug #119991313:51
_mup_Bug #1199913: upgrade from 1.10 to trunk cannot deploy containers <juju-core:Triaged> <https://launchpad.net/bugs/1199913>13:51
fwereadedimitern, stuckness is I think orthogonal here13:51
dimiternfwereade: ok, fair enough13:51
dimiternfwereade: i'd prefer not to do the AssignedUnitNames in this CL though13:52
dimiternfwereade: (and I though you said you won't mind the watcher approach ;)13:52
jamfwereade, dimitern: https://codereview.appspot.com/10986044/13:53
fwereadedimitern, yeah, I think I reiterated there that I don't *mind* it but it would probably still be better as a direct state method; twas a rumination not really a request13:58
dimiternfwereade: ok ;)13:58
fwereadejam, reviewed14:03
dimiternjam: reviewed as well14:08
rvbadimitern: Hi! May I add another review to your queue? https://codereview.appspot.com/1116604314:28
dimiternrvba: sure, will get to it a bit later14:29
rvbadimitern: ta!14:29
jamfwereade: so there is fslock, but the actual path to lock is pretty hard to get at. Thoughts?14:30
jamfwereade: and yes, MachineAgent will be broken on non-debian-based platforms, but it already is today.14:30
jamAnd whatever we do to make Container a bit happier there *should* give us a guide for apt-get install.14:31
jam(I'm hoping )14:31
jamSure RH will be different14:31
jambut until we actually support RH.14:31
jammgz mentioned trying to have cloud-init do the work, but we looked at it, and it isn't really designed around calling as a script14:31
fwereadejam, the path isn't that hard is it? it's relative to dataDir I thought14:33
jamfwereade: "what" dataDir?14:35
fwereadejam, *the* dataDir :)14:36
fwereadejam, it's a global lock for all units14:36
fwereadejam, they all get to it relative to dataDir, ie, usually /var/lib/juju14:37
fwereadejam, this may be messy actually :/ we do *not* want the machine agent to go down and wait on that lock while a unit finishes running a long hook. blast14:41
dimiternfwereade: should I change the Remover to refuse removing alive entities?14:43
fwereadedimitern, yes please, it should just be an additional check once you've got the unit14:44
dimiternfwereade: I never have a unit - I need to amend the Remover to have Life() as well14:44
dimiternfwereade: (the interface)14:44
fwereadedimitern, ha, good point14:46
dimiternfwereade: i'm embedding a lifer into the remover14:47
fwereadedimitern, I don't think that's too much of an issue in practice? perfectc14:47
dimiternfwereade: but since both embed tagger, would that be an issue?14:47
fwereadedimitern, just embed lifer instead of tagger14:48
fwereadedimitern, it's coherent14:48
dimiternfwereade: ah! yes14:48
fwereadedimitern, remove only currently makes sense for lifecycle entities anyway14:48
jamfwereade: we could use it in a strictly advisory way.14:48
jamTry to grab it14:48
jamif we fail, still try to install lxc14:48
jamworst case, the apt-get install fails14:49
jamand we are in the same boat14:49
jambest case, we prevent a race14:49
jamso try for eg 1 minute14:49
fwereadejam, I forget how fslock works, is it plausible that we could grab it before restarting?14:49
fwereadejam, and release it after handling upgrade changes on startup?14:49
jamfwereade: no, because we don't control the old code.14:49
jamfwereade: remember 1.10 is the thing triggering the upgrade14:50
jamfwereade: if we had hooks during upgrade, we could tell it to install lxc before restarting :)14:50
fwereadejam, ok, sgtm14:50
fwereadejam, indeed :)14:50
jamfwereade: I updated the CL: https://codereview.appspot.com/10986044/  I'm testing it now, but since I'm "not working in the evening" I figured I would solicit feedback before I finish.14:58
jamdimitern: ^^14:59
dimiternjam: will look soon15:01
jamdimitern, fwereade: confirmed that it solves bug #119991315:04
_mup_Bug #1199913: upgrade from 1.10 to trunk cannot deploy containers <juju-core:Triaged> <https://launchpad.net/bugs/1199913>15:04
dimiternjam: great!15:04
jamI'm off again (for dinner), I will definitely try to check back in tonight. (I only have 3 more working days before I go on vacation, so I'll try to put in some overtime :)15:06
=== arosales_ is now known as arosales
fwereadejam, LGTM15:08
dimiternjam: LGTM with 1 comment15:09
=== tasdomas` is now known as tasdomas_afk
dimiternfwereade, jam: updated https://codereview.appspot.com/1094904615:14
fwereadedimitern, need to take a break for a bit I'm afraid15:42
dimiternfwereade: sure, when you can; i can use a break myself15:42
teknicomarcoceppi: hi, some improvements (hopefully :-) ) to your discourse charm: https://code.launchpad.net/~teknico/charms/precise/discourse/add-cheetah-pkg-misc-fixes/+merge/17424816:57
marcoceppiteknico: thanks! I've been hoarding progress on the charm, so this is a good excuse to update the remote branch17:05
teknicomarcoceppi: you're welcome!17:06
teknicomarcoceppi: if you don't mind me asking, any specific reason the charm is written in shell script, rather than something saner? :-)17:07
marcoceppiteknico: I hack faster in shell :)17:08
teknicomarcoceppi: oh, I see. It must be painful ;-)17:09
hazmatmramm, so re current state of containers17:37
mrammhazmat: sure...17:37
hazmatmramm, we can create them with add-machine 0/0 but how do we deploy to them or is that post local provider?17:38
hazmatis that just going to be a constraint for containerized deploys17:38
mrammjuju deploy wordpress --force-machine17:39
mrammwith syntax like 0/lxc117:39
mrammthe spelling of all that is up for debate with william and mark tomorrow morning17:39
=== JoseeAntonioR is now known as jose
=== ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: - | Bugs: 6 Critical, 80 High - https://bugs.launchpad.net/juju-core/
fwereademgz, ping20:37
fwereademgz, if you're around, do you know offhand how we push tools to hp cloud on release?20:38
=== _thumper_ is now known as thumper
thumpermramm: hi there, I'm semi-functional21:36
thumpermramm: so working, but not up to full speed21:36
mrammglad to see you are feeling better!21:37
* thumper sighs21:45
thumperfwereade: ping21:45
thumperI guess I wasn't able to articulate why I didn't want the agent stuff merged with tools21:45
thumperwhile it is useful in some way21:45
thumperit again brought in all the state and api dependencies21:45
thumperwhich we were trying to get away from21:46
thumperthe whole point of tools.Tools was to be able to use it *without* importing state21:46
* thumper thinks on how to untangle21:47
* thumper wonders how to defer this horrible problem21:48
thumperwe suck as a team at dependency management21:49
thumperthe whole interface segregation principle is violated throughout the code21:49
thumperwith willful abandon21:49
* thumper has import loops again...22:07
thumperdue to this lack of segregation22:07
* thumper thinks22:07
* thumper gives up on this22:09
* fwereade hears thumper loud and clear22:24
fwereadethumper, hi, btw, hope you're on the mend22:27
thumperI think I am22:27
thumperjust not fully there yet22:27
fwereadethumper, glad to hear it, we've been missing you22:27
thumperfuzzy head and all that22:27
fwereadethumper, don't overdo it :)22:27
* thumper smiles22:27
thumperI'm going to try to work out how to get local tools synced for the local provider without having to build them22:27
thumperI have a plan22:28
fwereadethumper, frank is not currently working on auto-sync-tools but may do very soon, so please let him know if you start work on it22:28
fwereadethumper, sounds like it may be at risk of collision22:29
m_3_does anybody know what happened to update-alternatives in juju-core-1.11.2-3~1414~raring122:36
thumperm_3_: nope, perhaps dave does, he should be around soon23:36
m_3_ack... thanks23:38

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