[01:07] <thumper> axw_: ping
[01:07] <axw_> thumper: gah sorry, overlap with standup
[01:07] <axw_> thumper: can we make it in 5-10 please?
[01:08] <thumper> sure
[02:55] <axw_> wallyworld: last one: https://github.com/juju/juju/pull/8106
[03:55] <thumper> well... a great day of race conditions and deadlocks
[03:56] <thumper> https://github.com/juju/juju/pull/8107
[04:16] <babbageclunk> thumper: stink
[04:17] <thumper> yeah
[04:18] <babbageclunk> 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] <thumper> babbageclunk: the apiserver got stuck ratelimiting every agent
[04:19] <thumper> because the forwarder tried too often
[04:21] <babbageclunk> thumper: approved
[05:03] <thumper> laters peeps
[06:23] <jam> babbageclunk: I looked at 8108
[06:23] <jam> I don't think we want to refuse any downgrade
[06:24] <jam> but maybe restrict it within a major.minor series
[06:24] <jam> the most important thing is to get "juju upgrade-juju" to agree with what the actual Upgrade logic will do.
[07:12] <wallyworld> 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] <axw> wallyworld: ok. still working on provisioner stuff, will see how I go
[07:12] <wallyworld> no worries, it can wait
[12:13] <jam> axw: reviewed 8109
[20:00] <babbageclunk> wallyworld: ping?
[20:43] <wallyworld> babbageclunk: hey
[20:48] <babbageclunk> wallyworld: sorry chatting to thumper
[20:48] <wallyworld> ok, np
[21:18] <babbageclunk> wallyworld: hey, did you see the message from jam above?
[21:18] <babbageclunk> (off the phone now
[21:18] <babbageclunk> )
[21:18] <wallyworld> looking
[21:19] <babbageclunk> thumper points out that the problem with downgrading is that we don't have inverses for the upgrade steps
[21:20] <wallyworld> babbageclunk: i haven't looked at your PR yet, but what he says has sound logic
[21:21] <babbageclunk> 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] <babbageclunk> wallyworld: ^
[21:22] <wallyworld> i'll need to look at the code
[21:23] <wallyworld> but in general downgrading has been an issue due to not being able to revserse schema changes etc
[21:25] <wallyworld> babbageclunk: it looks like that state code refuses any doengrade, right?
[21:31] <babbageclunk> wallyworld: yeah, it's pretty clear, and it's been like that since the beginning.
[21:37] <babbageclunk> wallyworld: So that sounds like I probably can't allow downgrades even within the same major.minor.
[21:39] <babbageclunk> (by since the beginning, I mean, since upgrading was introduced in 2014.)
[21:47] <wallyworld> babbageclunk: yes, we have never supported downgrading IIANM
[21:49] <wallyworld> 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] <babbageclunk> wallyworld: makes sense
[21:50] <babbageclunk> 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] <wallyworld> babbageclunk: you mean the there are CLI tests? that assert we can do downgrades?
[21:52] <babbageclunk> wallyworld: yeah, one in the apiserver tests for model manager, one in featuretests
[21:53] <babbageclunk> wallyworld: just looking at them now. Maybe we need to distinguish between allowing downgrades for the controller and hosted models?
[21:53] <wallyworld> i'd need to look at those, perhaps we should allow minor
[21:53] <wallyworld> controller versions needs to be > hosted model version
[21:54] <babbageclunk> That's already checked. But would there be any benefit for downgrading hosted models?
[21:57] <wallyworld> that's up to the user of thse models
[21:58] <babbageclunk> 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] <babbageclunk> wallyworld: but the feature test is trying to do it.
[21:58] <wallyworld> and the feature test passes? how?
[21:58] <wallyworld> since there is that state check
[22:01] <babbageclunk> I mean, it did pass until I broke it.
[22:08] <wallyworld> but the feature test is meant to be full stack which mean the state check would have been run
[22:14] <babbageclunk> wallyworld: I presume that means that it wasn't checking that there weren't any errors.
[22:15] <wallyworld> maybe, not sure without looking
[22:22] <babbageclunk> 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] <babbageclunk> wallyworld: https://github.com/juju/juju/blob/develop/featuretests/upgrade_test.go#L157
[22:24] <wallyworld> makes sense
[22:24] <wallyworld> so we must bypass that state check then for that case
[22:24] <wallyworld> but that's internal, not a user driven thing
[22:26] <wallyworld> thumper: small one https://github.com/juju/bundlechanges/pull/33
[22:31] <babbageclunk> wallyworld: ok, I'll try to chase that code down...
[22:34] <thumper> wallyworld: approved
[22:35] <wallyworld> ta
[23:48] <wallyworld> 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