[00:00] why? [01:50] Hey jam - I've replied to some of your comments. I think the problem is that the agents will reject any downgrade because we don't have a way to undo any upgrade steps that have run. [02:01] jam: thanks for the review. I've updated, PTAL [02:08] thumper: why we still need charmstore.v5-unstable? [02:08] because it pulls in deps incompatible with juju [02:08] yes [02:08] we need charmstore.v5 t be updated [02:08] it uses an old idmclient [02:08] what is incompatible? [02:09] and also pulls in bakery.v-unstable [02:09] I'd prefer not to update the code so we have both [02:09] v2 [02:09] juju uses v1 [02:09] so structs don't work together [02:09] * thumper sighs [02:09] and idmclient has api changes [02:09] I guess that is a no go then [02:09] we can keep v5-unstable for now [02:09] it's only used in tests [02:10] the pr as is can land - it takes charm and charmrepo stable branhces for prod code [02:12] thumper: the only place charmstore.v5-unstable is used is in a small number of tests for juju repo. all other code uses charm.v6 and charmrepo.v2 and other stable branches. [02:13] but I'm not happy about bringing multiple copyes if the bakery code either [02:13] it's not used by juju, just a transitive dep [02:13] juju itself only uses charm.v6, charmrepo.v2 [02:14] and some tests use charmstore.v5-unstable [02:14] that's it [02:15] you can see the import changes to juju in the PR - they are well contained to just charm.v6-unstable -> charm.v6 etc [02:18] ince we land this now, we can follow up in a point release with a charmstore.v5 update and just 5 test files will be updated [02:19] ok, fair enough [02:36] thumper: you able to +1 the PR? [02:38] done [02:40] ta [02:58] wallyworld: so, it's safe to remove the juju-upgrade-mongo plugin from develop? I'm trying to fix these login issues cleanly, and removing that would help a bit I think [02:58] yup [02:58] that and related code I mean... maybe I'll just remove the agent bits on develop [02:58] k [02:58] it ws only ever for in situ mongo upgrades from 2.4 -> 3.2 [04:00] jam: i just looked at the upgrade PR - the PR as written makes the CLI enforce what the backend (has always IIRC) done. we do allow downgrades in the HA case where a controller itself needs to roll back. but not user initiatd ones - i don't think we ever have, or at least not for a long time. i'm personally reluctant to allow the user a foot gun here. we now have your script to force downgrades if we really want to get someone unstuck down't we? [04:01] maybe if we ever implement downgrade steps [04:01] but it's not just schema changes to worry about [04:25] anyone around? [04:25] wondering if its possible to --force a charm (series issue) in a bundle [04:58] wallyworld: so he had to make a change to the State side of the code. I have the feeling we do a Major.Minor check but not a Major.Minor.Patch [04:58] which I think he's adding, and then also adding to the client as well. [04:59] jam: the state changes have been backed out IIANM [05:00] jam: I added a check to State.SetModelAgentVersion (to match the one in state.checkUpgradeInfoSanity), but I've backed it out now because it caused problems with a feature test. [05:01] babbageclunk: k. So ideally we would use the same logic front and back, but I'd also rather the version be validated server side and rejected before it gets set. [05:01] the particular bug was that what was allowed was ultimately rejected by the Upgrader, but so late in the process that everything got wedged. [05:01] babbageclunk: wallyworld: I think we'd get more traction on upgrades if we had a better downgrade story [05:02] I appreciate that its a bit hard [05:02] jam: agreed, but to do that a day before thr rc.... [05:02] when this has been the behaviour since forever [05:02] sure fix it, but in 2.3.x or 2.4 [05:02] wallyworld: pretty sure that .PATCH has been allowed for a long time [05:02] the code in state suggests otherwise? not sure, i'll have to read it in detail [05:03] I'll test in a sec [05:03] not hard to try 2.2.6 => 2.2.5 [05:03] jam: The version check in checkUpgradeInfoSanity has been there since 2014. [05:05] babbageclunk: so checkUpgradeInfoSanity certainly dates back to 2014 from William. But I know that CI has for a long time bootstrapped 2.X.Y and then targeted a rollback to 2.X.Y-? [05:05] babbageclunk: I'm trying to figure out if something used to be setting .Patch to 0 or something to make it work [05:56] jam: ah, good point - that's certainly possible [06:06] babbageclunk: so "juju upgrade-juju --debug -m controller --agent-version 2.2.5" is perfectly accepted, and never complains, but doesn't actually do the change [06:06] there is a line in the log file [06:06] but nothing on any machine, etc. [06:06] and you can't "juju upgrade-juju --agent-version 2.2.6" to fix anything because it complains that not all agents are version 2.2.5 yet [06:07] babbageclunk: so good enough on your patch, I'm not sure why it worked in the past, but at least it makes it consistent with current internals. [07:06] jam: can you PTAL at https://github.com/juju/juju/pull/8109? would like to land for rc2 [07:09] jam: ok, thanks for looking a closer look. [07:16] axw: looking [07:18] axw: did you check if we still get the right error stack out the far end by annotating the inner Err: if we are wrapping it with StartInstanceError ? [07:21] axw: reviewed [07:21] jam: error stack in what context? which "far end" are you talking about? [07:21] thanks [07:21] axw: the goal of a stack trace is to see where the originating error comes from [07:22] if we are wrapping, do we lose the ability to see the stack trace if we ever bubble the errors up to (say the DependencyEngine) [07:22] jam: of course. so just a general question - no I haven't checked that, I'll do so before landing [07:28] sgtm [07:48] axw: here's a fix for that file handle leak. i have to pop out for a bit for dinner but will do more than smoke testing when i get back https://github.com/juju/juju/pull/8112 [07:49] wallyworld: ok, will look after I'm done here [07:50] yup. no ruah [07:50] jam: my previous approach doesn't work with stacks. the definition of "cause" in errors seems a bit whacko, so I didn't quite understand how Mask could work before. I'm reworking to use that now [07:51] jam: I'll add a common function in provider/common and just fix up the source location from there [07:51] axw: thanks === frankban|afk is now known as frankban [08:36] jam: https://github.com/juju/juju/pull/8109 is updated with a function, do you want to take another quick look before I land? [08:36] helper function* [08:38] gotta go cook dinner, I'll bbl [08:38] looking [08:43] axw: 2 small tweaks and its good to land [08:44] jam: thanks (didn't get away yet - realised I missed ec2) [08:44] jam: errors.Wrap always returns *errors.Err [08:44] axw: k. I'm always scared of bare casts, I thought it was the underlying "err" that we were touching, but if its the Wrap return that's ok. [10:31] jam: if you have time, can you please take a look at https://github.com/juju/juju/pull/8113. I'll be back later [10:51] lgtm [11:33] anyone up for a trivial review: https://github.com/juju/juju/pull/8114 [11:54] jam: I'm not sure I understand your question about restoreStatus being set correctly. It's set by the restoreChanged method - does that answer it? [11:57] anyway, that bit should be fine - landing. I'll address concerns in a followup if I missed the point [12:10] axw: I saw a change that introduces a new way to read a variable, but didn't see where that variable was being set. But presumably its already been taken care of elsewhere. [12:10] jam: yeah. there's a small change in the PR, to an existing method. it's just a mutex protected write to the same var [12:11] commented on the PR if you're interested === frankban is now known as frankban|afk [21:20] thumper: re: containers MTU - I worked with ivoks on this today, we have confirmed that machiner is reporting correct MTU of the parent bridge (which should be used for the container device). Tomorrow I'll investigate further. [21:20] wpk: ok [21:20] wpk: is there someone else that could pick this up? [21:20] wpk: either that or we'll hold rc2 for it [21:22] * thumper quietly wishes we had someone that understood all the networking bits in NZ/AU [21:25] thumper: I'll put a comment on the ticket with what I've discovered so far and will be my next steps. [21:25] * wpk loudly wishes we had someone that understood all the networking bits. [21:26] (wpk: I thought that was you!) [21:27] wpk: thanks [21:35] babbageclunk: everybody seems to think that, I don't understand why... [21:36] btw, weirdest completely non-juju bug on Artful - I plugged ps4 pad to my laptop to charge, and my screen started to rotate with pad - if the pad is upside down the screen is right side up, when I rotate it 90deg the screen rotates, when the pad is right side up the screen is upside down.... [21:36] ha, amazing [21:36] and that's probably enough computers for me for today ;) [21:37] wpk: when you say pad do you mean controller? Or is this some other crazy peripheral? [21:37] babbageclunk: 1. pad has accelerometers 2. for some reason, unknown to me, Ubuntu decides that this accelerometer should dictate screen orientation [21:38] babbageclunk: https://gameidealist.com/wp-content/uploads/magma-red-dualshock4.jpg this exact model [21:38] ooh, fancy [21:39] I don't even know where to report this kind of bug [21:40] It reminds me of a 'bug' in my Peugeot 307 - if ashtray lightbulb is blown headlights leveling doesn't work. [21:51] wallyworld, thumper, veebers, I'm feeling well enough, let's do chat about upgrades in 10 [21:52] ok