=== _thumper_ is now known as thumper [01:07] axw_: ping [01:07] thumper: gah sorry, overlap with standup [01:07] thumper: can we make it in 5-10 please? [01:08] sure [02:55] wallyworld: last one: https://github.com/juju/juju/pull/8106 === axw_ is now known as axw [03:55] well... a great day of race conditions and deadlocks [03:56] https://github.com/juju/juju/pull/8107 [04:16] thumper: stink [04:17] yeah [04:18] thumper: oh nasty - so the rate-limiting would start causing the agents to timeout and then what - they go into a never-ending loop trying to connect and timing out? [04:18] babbageclunk: the apiserver got stuck ratelimiting every agent [04:19] because the forwarder tried too often [04:21] thumper: approved [05:03] laters peeps [06:23] babbageclunk: I looked at 8108 [06:23] I don't think we want to refuse any downgrade [06:24] but maybe restrict it within a major.minor series [06:24] the most important thing is to get "juju upgrade-juju" to agree with what the actual Upgrade logic will do. [07:12] axw: if you get a chance later, could you look at https://github.com/juju/juju/pull/8104. the jujud operator branch depends on this one. i've got other stuff to do so no huge rush [07:12] wallyworld: ok. still working on provisioner stuff, will see how I go [07:12] no worries, it can wait === mup_ is now known as mup [12:13] axw: reviewed 8109 === petevg_afk is now known as petevg [20:00] wallyworld: ping? [20:43] babbageclunk: hey [20:48] wallyworld: sorry chatting to thumper [20:48] ok, np [21:18] wallyworld: hey, did you see the message from jam above? [21:18] (off the phone now [21:18] ) [21:18] looking [21:19] thumper points out that the problem with downgrading is that we don't have inverses for the upgrade steps [21:20] babbageclunk: i haven't looked at your PR yet, but what he says has sound logic [21:21] Yeah, and it matched a bit with some of the code in the upgrade-juju command. But I think the check in state.checkUpgradeInfoSanity is there because downgrading would require undoing upgrade steps. [21:21] wallyworld: ^ [21:22] i'll need to look at the code [21:23] but in general downgrading has been an issue due to not being able to revserse schema changes etc [21:25] babbageclunk: it looks like that state code refuses any doengrade, right? [21:31] wallyworld: yeah, it's pretty clear, and it's been like that since the beginning. [21:37] wallyworld: So that sounds like I probably can't allow downgrades even within the same major.minor. [21:39] (by since the beginning, I mean, since upgrading was introduced in 2014.) [21:47] babbageclunk: yes, we have never supported downgrading IIANM [21:49] babbageclunk: since that's the case, making the CLI match what the server will do is to me the most important bit, and then we can discuss whether to allow downgrades separately [21:49] wallyworld: makes sense === _thumper_ is now known as thumper [21:50] wallyworld: there are a couple of test failures about trying to do downgrades - it's kind of surprising that they were considered to pass when the sanity check would still fail. [21:51] babbageclunk: you mean the there are CLI tests? that assert we can do downgrades? [21:52] wallyworld: yeah, one in the apiserver tests for model manager, one in featuretests [21:53] wallyworld: just looking at them now. Maybe we need to distinguish between allowing downgrades for the controller and hosted models? [21:53] i'd need to look at those, perhaps we should allow minor [21:53] controller versions needs to be > hosted model version [21:54] That's already checked. But would there be any benefit for downgrading hosted models? [21:57] that's up to the user of thse models [21:58] wallyworld: it doesn't look like the modelmanager test is actually trying to downgrade - just happens to trip that in setting up the test. [21:58] wallyworld: but the feature test is trying to do it. [21:58] and the feature test passes? how? [21:58] since there is that state check [22:01] I mean, it did pass until I broke it. [22:08] but the feature test is meant to be full stack which mean the state check would have been run [22:14] wallyworld: I presume that means that it wasn't checking that there weren't any errors. [22:15] maybe, not sure without looking [22:22] wallyworld: hmm, the test is about HA - there's a need for the leader to be able to downgrade if one of the followers can't upgrade? [22:23] wallyworld: https://github.com/juju/juju/blob/develop/featuretests/upgrade_test.go#L157 [22:24] makes sense [22:24] so we must bypass that state check then for that case [22:24] but that's internal, not a user driven thing [22:26] thumper: small one https://github.com/juju/bundlechanges/pull/33 [22:31] wallyworld: ok, I'll try to chase that code down... [22:34] wallyworld: approved [22:35] ta [23:48] thumper: a larger one https://github.com/juju/juju/pull/8111. sadly we still need to use charmstore.v5-unstable, but at least that's only in tests