Code review comment for lp:~julian-edwards/maas/consider-static-range

Thank you for the review.

On Thursday 14 Aug 2014 09:41:01 you wrote:
> Review: Approve
>
> "Everything is closely tied" is not a reason not to break things up; it's a
> reason to put more effort into breaking them up! Such irredeemable patches
> of code tend to spread like oil slicks. There's clearly stuff here that
> could be extracted, and make the code cleaner for it. I see loops that
> could do with a name, a docstring, or at least a comment.

Oh I do agree, believe me. What I meant by closely tied is that I want to
namespace it. I *really* dislike interdependent functions scattered around a
module. But right now I don't have the headspace to tackle this in a way that
I would like. It's not fun having an illness that makes you feel stupid :(

> One question about the design of this branch: why would you look just at the
> static range, and not at the network's full address range? Does it matter
> for the purpose of linking a node's network interface to a subnet whether
> the address is in the static range? If I reduced the static range after a
> node had already acquired a static address, so that the address is now
> outside the static range, should that affect whether this could makes the
> link? Wouldn't be a check against the network be more robust, as well as

The existing ranges are definitive in terms of what we are serving on this
cluster. You cannot reduce a range and leave an existing address outside that
range; it is checked for and disallowed.

The point is that we can (and will) have machines appearing on this network
that we're not meant to service, so a tighter check is warranted.

> faster (because IPRange is so awful about “in”) and simpler?

Actually IPRange is ok, it's IPSet that's slow.