KEP-3070: Reserve Service IP Ranges For Dynamic and Static IP Allocation

[aojea]

• One-line PR description: KEP-3070: adding KEP

• Issue link: Reserve Service IP Ranges For Dynamic and Static IP Allocation #3070

• Other comments: Implementation KEP-3070: Reserve Service IP Ranges For Dynamic and Static IP Allocation kubernetes#106792

[aojea]

/assign @thockin @danwinship
/sig network

[danwinship]

That seems weird. I would expect that if there was a reserved range, that it would be reserved unconditionally, not "reserved until we run out of unreserved addresses".

Also, it's weird to hardcode the reserved range to 32 addresses regardless of the size of the service CIDR...

[aojea]

That seems weird. I would expect that if there was a reserved range, that it would be reserved unconditionally, not "reserved until we run out of unreserved addresses".

this keeps the behavior backwards compatible, you can create services as long as there are free IPs, if we keep the lower range reserved only for static IPs we are changing the behavior

Also, it's weird to hardcode the reserved range to 32 addresses regardless of the size of the service CIDR...

@thockin I knew we should have a history about why 32 😄

[thockin]

Heh. We went back and forth on this a lot in discussing.

If your service CIDR is /27, you only get 32 service IPs (IIRC, it's 30 actually, we avoid the first and last). At that point we can debate whether it's OK to have so few and if, at that point, we can make many guarantees.

If your CIDR is bigger than that, do we need to reserve more than 32 IPs? If so, do we need a "cascade" of blocks?

e.g. something like min(32, cidrSize/2) is attractive, but only useful if we think there are a lot of users with really small ranges.

Something more dynamic like "always prefer the upper half unless it is full" is more general, I guess. Is it worth the extra effort?

E.g. assume a /22 service range (1024 IPs). First look at [512]-[1023]. If that's full, look at [256-511]. If that's full, look at [128-255]. It seems like that gives the caller influence where it isn't needed, which could be a vector for some sort of attack.

min(max(16, cidrSize/16), 256) ? At /28 it would be 16 (full range). At /27 - /24 it would be 16 (part of the range). At /23 it would be 32. /22 -> 64. /21 ->128. /20 and bigger -> 256.

Again, worth the effort?

[aojea]

what about making it a fixed percentage? we subdivide in two ranges,

           ┌─────────────┬─────────────────────────────────────────────┐
           │             │                                             │
           │             │                                             │
           │             │                                             │
           │   static    │                    dynamic                  │
           │             │                                             │
           │             │                                             │
           │             │                                             │
           └─────────────┴─────────────────────────────────────────────┘

           ◄────────────► ◄────────────────────────────────────────────►

                15%                              85%

[thockin]

"15%" must be a typo, since it is not a power of two. I think you meant to type "12.5%" :)

Seriously though, I think percents are OK for humans to get a big-picture, but harder to be precise about.

min(max($min, cidrSize/$step), $max) is easy to describe as "never less than $min or more than $max, with a graduated step function between them. We can argue about values for those variables (or even tweak them over time if we get feedback), but the logic still seems appropriate?

I proposed:

$min = 16
$max = 256
$step = 16

But I am fine to haggle :)

[aojea]

I'm fine with anything meanwhile users can understand it

[aojea]

updated with the suggested formula

[danwinship]

FTR I meant that 32 seemed like too many reserved IPs for small service CIDRs, not that I thought we should reserve more for large CIDRs.

But I guess it's reasonable to assume that users with larger clusters are likely to have more use cases for fixed-IP services, so I guess giving them a larger range makes sense. And if we want the reserved size to (a) always be a power of 2, and (b) always include the .10 address, then I guess 16 has to be the minimum value. And given that the "reserved" range is more like a "preferred" range, then even 16 isn't too large for a /27

[thockin]

Heh. We went back and forth on this a lot in discussing.

If your service CIDR is /27, you only get 32 service IPs (IIRC, it's 30 actually, we avoid the first and last). At that point we can debate whether it's OK to have so few and if, at that point, we can make many guarantees.

If your CIDR is bigger than that, do we need to reserve more than 32 IPs? If so, do we need a "cascade" of blocks?

e.g. something like min(32, cidrSize/2) is attractive, but only useful if we think there are a lot of users with really small ranges.

Something more dynamic like "always prefer the upper half unless it is full" is more general, I guess. Is it worth the extra effort?

E.g. assume a /22 service range (1024 IPs). First look at [512]-[1023]. If that's full, look at [256-511]. If that's full, look at [128-255]. It seems like that gives the caller influence where it isn't needed, which could be a vector for some sort of attack.

min(max(16, cidrSize/16), 256) ? At /28 it would be 16 (full range). At /27 - /24 it would be 16 (part of the range). At /23 it would be 32. /22 -> 64. /21 ->128. /20 and bigger -> 256.

Again, worth the effort?

[thockin]

After discussion a few hours ago - should we cover node ports in the same way?

[aojea]

After discussion a few hours ago - should we cover node ports in the same way?

implementation wise is simpler if we want to use the same reserved ranges sizes for each

[aojea]

however, the maths we are doing here doesn't apply, we are always assuming binary arithmetic for the range sizes, not a big deal if we assume this will always remain as is.
Now that I write it, I have the feeling that for the future it is better if we decouple NodePorts from ClusterIPs and break this dependency now that we can.

[thockin]

OK. In the future we can do a KEP for NodePorts, if needed. I bet the same formula applies, just with different coefficients.

This LGTM - @danwinship do you want a word on it? I'll approve but hold for now.

/approve
/lgtm
/hold

[thockin]

oh, needs PRR anyway. I did not read for that. Will do

[thockin]

What does this mean? That 2 providers have switched to this mode as the default? That's unlikely while it is beta. Let's get specific here. I think time and lack of trouble is the only real signal.

When do we make this mode the default? Is there a reason not to do that?

[aojea]

It is interest in enabling it in Openshift by default to solve some races at installation time, I was expecting more installers will face similar issue, but as you say this is just more a matter of time and signal.

[thockin]

Do we want new metrics to indicate "forced to use reserved range" ?

[aojea]

I've tried to add new metrics but its complicated, since this behavior is deep in the bitmap allocation and the metrics are one layer above on the ip allocator .... inheritance problems 🤷

I'll add a comment explaining it

[wojtek-t]

I've tried to add new metrics but its complicated, since this behavior is deep in the bitmap allocation and the metrics are one layer above on the ip allocator .... inheritance problems

Hmm - can't we report them somewhat post-factum (when we're returning the actual result we already know what IP we allocated and if it was statically requested or not)?

[aojea]

do you mean something like randomly_allocated_ips gauge and randomly_allocation_total counter ?
that is easy to do, will that work for you?

[wojtek-t]

do you mean something like randomly_allocated_ips gauge and randomly_allocation_total counter ?
that is easy to do, will that work for you?

Can you clarify? I think these can be interpreted differently.

[aojea]

we have these metrics now

  - allocated_ips
  - available_ips
  - allocation_total
  - allocation_errors_total

we can add two new ones

randomly_allocated_ips
randomly_allocation_total

randomly_allocated_ips reports the number of IPs in the allocator that are allocated "randomly" i.e. with ClusterIP empty, it reports current value.
randomly_allocation_total is a counter, every time that you allocate a ClusterIP randomly it increases, the user can play with the current metrics and these new ones to obtain the IPs allocated statically

[wojtek-t]

Thanks for clarifiction.

Yes - those metric will be very useful.

What I had on my mind (please let me know if this doesn't make any sense because of some reason) is actually an extension of that by:

• adding a label to that metric (like "from-static" or sth) to show if those random IPs were allocated from static range

Does that make sense?

[aojea]

yeah, that is much simpler and easier, I will update the kep with that

[thockin]

/lgtm

[danwinship]

lgtm except for the "reserved block" comment

I'd like to avoid using "Reserved" in the KEP title since neither range is actually reserved for a single use, but I can't think of any way to describe the behavior well in few enough words...

[danwinship]

(Well, no, the best example is the apiserver...)

[aojea]

is it possible to race with the apiserver .1 address?
The loop to create the kubernetes.default runs at the beginning, I honestly don't know if at that point clients are able to create services, but is worth adding it

[danwinship]

"to allocate first from the unreserved block", right? (You're referring to both ranges as "the reserved block" here.)

and "(it will wrap around back to the start of the unreserved subrange if necessary)"

[aojea]

I will word it as reserved for static and reserved for dynamic allocation, it sounds less ambiguous

[aojea]

/hold cancel

comments addressed, ready for final review

[aojea]

@wojtek-t these 2 metrics can not track the information of the type of allocation, because when an ip is released we don't have that information.
however, the counters can have it, because are only incremented

  - allocation_total
  - allocation_errors_total

[wojtek-t]

Fair - that sounds good enough to me.

Can you clarify that here?

[wojtek-t]

Couple nits, but I'm overall fine with it. Will approve once addressed.

[wojtek-t]

Fair - that sounds good enough to me.

Can you clarify that here?

[wojtek-t]

@aojea - can you please squash the commits?

[wojtek-t]

This is more than enough for Alpha from PRR perspective - thanks!

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, thockin, wojtek-t

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• keps/prod-readiness/OWNERS [wojtek-t]
• keps/sig-network/OWNERS [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[wojtek-t]

@aojea @thockin - requires adding to the tracking spreadsheet:
https://docs.google.com/spreadsheets/d/1T21mUTvPh70NB2eseHjCyD4LgRjyxWI9Bd1SoP8zAwA/edit#gid=0