IPAddress allocator #115075

aojea · 2023-01-14T16:51:56Z

/kind bug
/kind feature

What this PR does / why we need it:

This is the first step towards the implementation of KEP 1880, it introduces a new API object IPAddress and a feature gate MultiCIDRServiceAllocator

The feature gate enables a new ClusterIP Allocator that uses the IPAddress object instead of the the actual bitmap allocator, removing the current service cidr size limitations (ipv4 is a /12 and ipv6 /108) and setting the maximum size of a service CIDR in /64

The new IPAllocator uses IPAddress objects that reference the Service to store the ClusterIP, creating a 1-to-1 relation.
This relation is maintained by a repair controller that runs in the apiserver.

- Added a new IPAddress object kind
- Added a new ClusterIP allocator. The new allocator removes previous Service CIDR block size limitations for IPv4, and limits IPv6 size to a /64

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Please use the following format for linking documentation:

- [KEP]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-network/1880-multiple-service-cidrs

cici37 · 2023-01-17T21:18:54Z

/remove-sig api-machinery

pkg/apis/networking/validation/validation.go

pkg/features/kube_features.go

Change-Id: I19e12ca05d977dca63043cb07ecf8a90e0e525c5

aojea · 2023-03-14T22:59:39Z

PR needs rebase.

rebased

@thockin we need sudo 😄

/lgtm
/approve

aojea · 2023-03-14T23:36:22Z

Kubernetes e2e suite: [It] [sig-cli] Kubectl client kubectl subresource flag GET on status subresource of built-in type (node) returns identical info as GET on the built-in type expand_less

known flake

/test pull-kubernetes-e2e-kind
#116619

thockin

On behalf of the review done by others and the fact that it is alpha

/lgtm
/approve

pkg/apis/networking/types.go

pkg/apis/networking/validation/validation.go

thockin · 2023-03-14T23:57:28Z

staging/src/k8s.io/api/networking/v1alpha1/well_known_labels.go

+	// LabelIPAddressFamily is used to indicate the IP family of a Kubernetes IPAddress.
+	// This label simplify dual-stack client operations allowing to obtain the list of
+	// IP addresses filtered by family.
+	LabelIPAddressFamily = "ipaddress.kubernetes.io/ip-family"


Can field selectors be used in watch? I have never done that...

We can revisit this and either make it a field selector or a "magic" label or something. I agree we need to make some guarantees

thockin · 2023-03-14T23:57:51Z

staging/src/k8s.io/api/networking/v1alpha1/well_known_labels.go

+	// objects to be managed by different controllers or entities within the
+	// same cluster. It is highly recommended to configure this label for all
+	// IPAddress objects.
+	LabelManagedBy = "ipaddress.kubernetes.io/managed-by"


We need to fix owner ref. Grr....

k8s-ci-robot · 2023-03-14T23:58:32Z

LGTM label has been added.

Git tree hash: 6bd9bd2d25dd4855c9f7941cc91f3746c1f94c08

thockin · 2023-03-15T00:00:16Z

thockin · 2023-03-15T00:00:28Z

/retest

sftim · 2023-03-15T00:13:28Z

Changelog suggestion

- Added a new IPAddress object kind
- Added a new ClusterIP allocator. The new allocator removes previous Service CIDR block size limitations for IPv4, and limits IPv6 size to a /64

something like that?

sftim · 2023-03-15T00:16:37Z

staging/src/k8s.io/api/networking/v1alpha1/well_known_labels.go

+	// LabelManagedBy is used to indicate the controller or entity that manages
+	// an IPAddress. This label aims to enable different IPAddress
+	// objects to be managed by different controllers or entities within the
+	// same cluster. It is highly recommended to configure this label for all
+	// IPAddress objects.


We should register this - see https://kubernetes.io/docs/reference/labels-annotations-taints/

sftim · 2023-03-15T00:16:36Z

staging/src/k8s.io/api/networking/v1alpha1/well_known_labels.go

+	// objects to be managed by different controllers or entities within the
+	// same cluster. It is highly recommended to configure this label for all
+	// IPAddress objects.
+	LabelManagedBy = "ipaddress.kubernetes.io/managed-by"


If we can't fix owner reference, or find an equivalent mechanism that doesn't use labels, could the label key be kubernetes.io/managed-by? I'd like to just have one notion of things managing things rather than n of them.

sftim · 2023-03-15T00:17:24Z

staging/src/k8s.io/api/networking/v1alpha1/well_known_labels.go

+	// TODO: Use IPFamily as field with a field selector,And the value is set based on
+	// the name at create time and immutable.
+	// LabelIPAddressFamily is used to indicate the IP family of a Kubernetes IPAddress.
+	// This label simplify dual-stack client operations allowing to obtain the list of
+	// IP addresses filtered by family.


If we need a label, we should register this - see https://kubernetes.io/docs/reference/labels-annotations-taints/

I'd go for network.kubernetes.io/ip-address-family. Network is a nice general concept whereas IP address feels very specific. And, as a project, we control the namespace fairly effectively.

yes, there is also a comment to move it to a field, once we settle we should update all the docs accordingly

aojea · 2023-03-15T00:31:19Z

/milestone v1.27

approved before code freeze
#115075 (comment)

aojea · 2023-03-15T00:34:45Z

On behalf of the review done by others and the fact that it is alpha

/lgtm /approve

@thockin it seems it didn't pick the approve label correctly, can you retag?

thockin · 2023-03-15T00:38:01Z

/approve

k8s-ci-robot · 2023-03-15T00:38:30Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

~~api/OWNERS~~ [thockin]
~~cmd/kube-apiserver/OWNERS~~ [thockin]
~~hack/OWNERS~~ [thockin]
~~pkg/apis/networking/OWNERS~~ [thockin]
~~pkg/controlplane/OWNERS~~ [thockin]
~~pkg/features/OWNERS~~ [thockin]
~~pkg/generated/openapi/OWNERS~~ [thockin]
~~pkg/kubeapiserver/OWNERS~~ [thockin]
~~pkg/printers/OWNERS~~ [thockin]
~~pkg/registry/OWNERS~~ [thockin]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]
~~staging/src/k8s.io/client-go/OWNERS~~ [thockin]
~~staging/src/k8s.io/kubectl/OWNERS~~ [thockin]
~~test/OWNERS~~ [aojea,thockin]

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

BenTheElder · 2023-03-15T04:30:47Z

Congrats @aojea :-)

aojea · 2023-03-15T11:20:42Z

Summarizing issues from this PR to not forget, please add the things you find missing, so I can start incorporating them to the follow up in #116516

Document upgrades and rollbacks, specially for those clusters with size that are now allowed that were not allowed before
Owner references, namespaces -> non-namespaces, it seems we should keep using label IPAddress allocator #115075 (comment)
Field-selector vs Label-selector, the flexibility of labels makes it simpler to create controllers that handle IPAddresses and need to be dual-stack IPAddress allocator #115075 (comment), I prefer to keep using labels why podLister do not support FieldSelector? client-go#604 since I expect most of the consumers will need to do a lot of filtering client side
Drop ParentReference UID
If we use labels, document them properly

cici37 · 2023-03-16T16:53:14Z

/triage accepted

k8s-ci-robot requested review from andrewsykim and apelisse January 14, 2023 16:52

kubernetes deleted a comment from diadj Jan 15, 2023

k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jan 17, 2023

aojea commented Jan 23, 2023

View reviewed changes

pkg/apis/networking/validation/validation.go Outdated Show resolved Hide resolved

aojea commented Jan 23, 2023

View reviewed changes

pkg/features/kube_features.go Outdated Show resolved Hide resolved

aojea force-pushed the ipaddress branch from 6f4b60d to d50cfb8 Compare January 23, 2023 13:35

k8s-ci-robot added the area/kubeadm label Jan 23, 2023

make update

ba42ed9

Change-Id: I19e12ca05d977dca63043cb07ecf8a90e0e525c5

aojea force-pushed the ipaddress branch from beec768 to ba42ed9 Compare March 14, 2023 22:58

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 14, 2023

k8s-ci-robot requested review from BenTheElder, khenidak and thockin March 14, 2023 22:58

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Mar 14, 2023

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 14, 2023

thockin reviewed Mar 14, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 14, 2023

aojea mentioned this pull request Mar 15, 2023

KEP: remove IPv6 allocation limitations kubernetes/enhancements#1534

Closed

sftim reviewed Mar 15, 2023

View reviewed changes

k8s-ci-robot added this to the v1.27 milestone Mar 15, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 15, 2023

k8s-ci-robot merged commit f44d561 into kubernetes:master Mar 15, 2023

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IPAddress allocator #115075

IPAddress allocator #115075

aojea commented Jan 14, 2023 •

edited

cici37 commented Jan 17, 2023

aojea commented Mar 14, 2023

aojea commented Mar 14, 2023

thockin left a comment

thockin Mar 14, 2023

thockin Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

thockin commented Mar 15, 2023 •

edited

thockin commented Mar 15, 2023

sftim commented Mar 15, 2023

sftim Mar 15, 2023

sftim Mar 15, 2023

sftim Mar 15, 2023

aojea Mar 15, 2023

aojea commented Mar 15, 2023

aojea commented Mar 15, 2023

thockin commented Mar 15, 2023

k8s-ci-robot commented Mar 15, 2023

BenTheElder commented Mar 15, 2023

aojea commented Mar 15, 2023

cici37 commented Mar 16, 2023

IPAddress allocator #115075

IPAddress allocator #115075

Conversation

aojea commented Jan 14, 2023 • edited

What this PR does / why we need it:

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

cici37 commented Jan 17, 2023

aojea commented Mar 14, 2023

aojea commented Mar 14, 2023

thockin left a comment

Choose a reason for hiding this comment

thockin Mar 14, 2023

Choose a reason for hiding this comment

thockin Mar 14, 2023

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 14, 2023

thockin commented Mar 15, 2023 • edited

thockin commented Mar 15, 2023

sftim commented Mar 15, 2023

sftim Mar 15, 2023

Choose a reason for hiding this comment

sftim Mar 15, 2023

Choose a reason for hiding this comment

sftim Mar 15, 2023

Choose a reason for hiding this comment

aojea Mar 15, 2023

Choose a reason for hiding this comment

aojea commented Mar 15, 2023

aojea commented Mar 15, 2023

thockin commented Mar 15, 2023

k8s-ci-robot commented Mar 15, 2023

BenTheElder commented Mar 15, 2023

aojea commented Mar 15, 2023

cici37 commented Mar 16, 2023

aojea commented Jan 14, 2023 •

edited

thockin commented Mar 15, 2023 •

edited