=== _thumper_ is now known as thumper | ||
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:07 |
thumper | sure | 01:08 |
axw_ | wallyworld: last one: https://github.com/juju/juju/pull/8106 | 02:55 |
=== axw_ is now known as axw | ||
thumper | well... a great day of race conditions and deadlocks | 03:55 |
thumper | https://github.com/juju/juju/pull/8107 | 03:56 |
babbageclunk | thumper: stink | 04:16 |
thumper | yeah | 04:17 |
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:18 |
thumper | because the forwarder tried too often | 04:19 |
babbageclunk | thumper: approved | 04:21 |
thumper | laters peeps | 05:03 |
jam | babbageclunk: I looked at 8108 | 06:23 |
jam | I don't think we want to refuse any downgrade | 06:23 |
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. | 06:24 |
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 | 07:12 |
=== mup_ is now known as mup | ||
jam | axw: reviewed 8109 | 12:13 |
=== petevg_afk is now known as petevg | ||
babbageclunk | wallyworld: ping? | 20:00 |
wallyworld | babbageclunk: hey | 20:43 |
babbageclunk | wallyworld: sorry chatting to thumper | 20:48 |
wallyworld | ok, np | 20:48 |
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:18 |
babbageclunk | thumper points out that the problem with downgrading is that we don't have inverses for the upgrade steps | 21:19 |
wallyworld | babbageclunk: i haven't looked at your PR yet, but what he says has sound logic | 21:20 |
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:21 |
wallyworld | i'll need to look at the code | 21:22 |
wallyworld | but in general downgrading has been an issue due to not being able to revserse schema changes etc | 21:23 |
wallyworld | babbageclunk: it looks like that state code refuses any doengrade, right? | 21:25 |
babbageclunk | wallyworld: yeah, it's pretty clear, and it's been like that since the beginning. | 21:31 |
babbageclunk | wallyworld: So that sounds like I probably can't allow downgrades even within the same major.minor. | 21:37 |
babbageclunk | (by since the beginning, I mean, since upgrading was introduced in 2014.) | 21:39 |
wallyworld | babbageclunk: yes, we have never supported downgrading IIANM | 21:47 |
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:49 |
=== _thumper_ is now known as thumper | ||
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:50 |
wallyworld | babbageclunk: you mean the there are CLI tests? that assert we can do downgrades? | 21:51 |
babbageclunk | wallyworld: yeah, one in the apiserver tests for model manager, one in featuretests | 21:52 |
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:53 |
babbageclunk | That's already checked. But would there be any benefit for downgrading hosted models? | 21:54 |
wallyworld | that's up to the user of thse models | 21:57 |
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 | 21:58 |
babbageclunk | I mean, it did pass until I broke it. | 22:01 |
wallyworld | but the feature test is meant to be full stack which mean the state check would have been run | 22:08 |
babbageclunk | wallyworld: I presume that means that it wasn't checking that there weren't any errors. | 22:14 |
wallyworld | maybe, not sure without looking | 22:15 |
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:22 |
babbageclunk | wallyworld: https://github.com/juju/juju/blob/develop/featuretests/upgrade_test.go#L157 | 22:23 |
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:24 |
wallyworld | thumper: small one https://github.com/juju/bundlechanges/pull/33 | 22:26 |
babbageclunk | wallyworld: ok, I'll try to chase that code down... | 22:31 |
thumper | wallyworld: approved | 22:34 |
wallyworld | ta | 22:35 |
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 | 23:48 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!