IP Allocation Management

[bboreham]

Implements the scheme described at https://github.com/weaveworks/weave/wiki/IP-allocation-design, with address ranges controlled by a CRDT gossiped between peers.

weave launch takes a new -alloc argument specifying the subnet to work within.

Commands updated to take advantage of IPAM: weave run, weave start, weave attach, weave expose

[tomwilkie]

I would appreciate if this was a mutli-author commit - the history of the bits I wrote was lost when this was squashed into one change.

[rade]

"We are all individuals!"

[bboreham]

@dpw would you kindly consider reviewing this, please?
We will squash the commits down into ~2 when all done.

[dpw]

@dpw would you kindly consider reviewing this, please?

Ok.

I would like to make sure I understand the context properly first though. I know this work has been going on a while, and has already had several team members involved as contributors and reviewers. So it seems reasonable to ask about the purpose of another review.

My guess is that there is some consensus that it is ready now, but there's also a wish for a fresh set of eyes to do what is hopefully a final pass.

Is that correct?

[rade]

Correct.

@bboreham @tomwilkie please make sure this is really ready. We are rapidly running out of people who can look at this with fresh eyes.

[bboreham]

This redesign (with the Ring) has not been reviewed in earnest by anyone outside its authors.

@tomwilkie and I think it's ready, and yes we are looking for a fresh set of eyes.

[dpw]

Ok, then I'd suggest doing any commit munging before I look at it, so that I am looking at it in a genuinely final state. I'm expecting to be ready to hit the merge button on this!

I'd also like to check another thing before I look at it. This may be something you've already taken care of, but just in case: I understand that there is a design document to explain how the algorithm works. Is that document needed to properly understand the code, or not? If it is, then it ought to be contained in its entirety in the git repo, so that in future it is clear what version of the doc corresponds to what version of the code. Also, please make sure that the code and the doc do correspond. And if the doc describes design alternatives not implemented by the code, these should be clearly marked as such. I bring it up because this is the kind of thing that is easy to overlook when one is immersed.

Conversely, if the document is not considered essential to understanding the code, please check that it really is possible to understand the algorithm from the code alone, without it seeming like an exercise in reverse engineering.

[tomwilkie]

Its worth saying we know of a few limitations (in the design). These should probably be included in release notes / docs. We do not intend to fix this in this PR, as we think what we have is 'good enough' for v1.

• our simple master election algorithm will give different answers on different sides of partition, leading to potentially issuing the same IP twice and an unmergeable ring. This will happen if you bring up a network in a partitioned state, for instance. The fix is to use a quorum based master election algorithm, like raft or paxos do.
• similarly, if a healthy ring is partitioned & then all state is lost of one side of the ring (for instance, due to a coordinate power event), you could issue IPs twice (related to above) and the rings will not be mergeable. The fix is to persist a list of peers, and not blow away the weave container each time - so its out of scope.
• if you use rmpeer & tombstones in a partition, this can sometimes lead to unmergable rings and issuing IP twice. This is hard to fix, and probably never will be.

[bboreham]

Design doc is at https://github.com/weaveworks/weave/wiki/IP-allocation-design
There is another, lower-level, doc in the repo at https://github.com/bboreham/weave/blob/22_ipam/ipam/ipam.md

I suspect I am too far immersed in the code to know what the document really ought to say.

I can move the design doc into the repo; the diagram will have to be converted.

[dpw]

Well, I'm hoping not look at any of this until I look at all of it, so I'm not going to follow those links. But it sounds a bit odd that there are two design docs, even if they have a different emphasis. Would it be unreasonable to suggest that they be folded together, moving some of the low-level exposition into comments in the code if appropriate?

And while I can understand that it might be convenient to author a document as a wiki page, I think a copy of it should go into the git repo, so that in the future, as things evolve, it's clear which version of the doc goes with which version of the code.

[rade]

I suggest adding an IPAM section to our How it Works docs.

[dpw]

I suggest adding an IPAM section to our How it Works docs.

Maybe, but I want to be clear about my expectations, as I sense that fatigue has set in on this sub-project: the IPAM design doc should match the code and say whatever needs to be said to understand the code, so that when I review it everything seems clear. Its location in the repo does not seem like a blocker for merging, as it can easily be changed later on, and how we handle developer docs vs outwards-facing technical docs vs user docs does not seem like something we have quite settled on yet.