jamaxw: I sent a first set of review comments on your PR. I'm going to try and finish reviewing it today, but i realize that it would have been useful to submit the feedback I had earlier so you could respond05:00
axwjam: thanks05:02
=== frankban|afk is now known as frankban
axwjam: in one of your comments, it sounds like you're leaning back towards the original approach of mapping IDs to addresses not stored in the raft config, is that right?09:56
jamaxw: so I wanted to discuss it at least. I thought you were doing separate mapping on our side, not part of the raft config09:57
jamyou were saying that you needed to publish the alternative config and track it somewhere09:57
jamwhich was my big concern09:57
axwjam: originally I had (in the raft config) the server ID and address both equal to machine-tag09:58
axwjam: then the transport code would map the "address" (machine tag) to the real API server hostport09:58
jamaxw: ah, that's probably the part I didn't want. I'd prefer not to have the "address" be a tag09:58
axwjam: it eliminates the split-brain issue, since you have a stable address in the raft config (no need to remove/add). what's your concern exactly? about consistency?09:59
axwjam: consistency/ordering isn't ideal for name resolution, you always want what's current?10:00
jamaxw: my main concern is that the raft config is no longer self contained. you need to look up somewhere else that you then need to keep consistent10:04
axwjam: ok. I agree that it would be nice to have it self-contained. it's unfortunate that there's no atomic config change op10:05
jamaxw: so it separates the idea of what IP address a given node has, but it doesn't let you change the address of the node without removing it?10:05
jamaxw: that seems weird given the separation10:05
axwjam: yup10:05
axwjam: unless docs lie...10:07
axwI see code that looks like it *might* allow an address change, which is not what the docs say10:08
jamaxw: looking at the code in configuration.go10:08
axwyep, same10:08
jamlooks like you can just AddVoter(existingId, newAddress)10:08
jamand it will overried10:08
jamif server.Sufferage == Voter ; Servers[i].Address = serverAddress10:09
jamaxw: ^^10:09
axwjam: yep, I just saw the same. I'll test it now. yay for misleading doc comments10:09
axw"If the server is already in the cluster as a voter, this does nothing."10:09
jamaxw: yeah, there definitely is a discrepency between some of the code comments and the "design" vs the "implementation"10:11
jamthings like "AddVoter" actually adds it as staging, but promotion actually does nothing which means it actually adds it as voter10:11
axwjam: ok, it works fine - I'll update the code to get rid of the intermediate remove op, and I'll add in a check to stop it from removing a server if it could lead to split-brain10:22
jamwpk: I looked at PR 832310:23
jamaxw: that changes your code also to handle the "localhost" to "public IP" case, doesn't it?10:28
axwjam: sorry, don't follow - which case is that? you mean changing the bootstrap address (juju.localhost)?10:28
jamaxw: you have comments around "if len(X) < 3)" and some comments in the test suite about handling when we have 1 entry vs 3 entries10:29
jamaxw: can we simplify that code/10:29
axwjam: yes, as we speak :)   I'll just make sure that it doesn't remove any servers if that would lead to an even-sized configuration10:30
axwadd or remove...10:31
jamaxw: i'm less concerned about an even-sized config, because often it is necessary to go via that to get to what you want. I'd like to move our existing ensure-ha to not mandate even-always10:33
axwjam: OK, I'll leave that for now then10:33
axwI'll just drop the length check10:33
jamaxw: sgtm. (specifically, if there is a breakage on one of your machines, you'd really like to just kill that machine, and then bring it back into rotation, and our code around "protect you from yourself" usually gets in the way of people actually fixing stuff)10:34
* axw nods10:34
jamaxw: I updated my comments on 830810:37
axwjam: I've pushed updates10:46
axwjam: I have no strong feeling regarding Internal vs. HA, so let me know if you'd like me to change it. also please see my reply regarding Debug vs. Info for cluster config updates11:01
=== frankban is now known as frankban|afk
balloonshow is the ami chosen when creating a unit?19:07
balloonswallyworld, ^^?19:21

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