KEP-2149: bump clusterid to beta #3652

lauralorenz · 2022-11-09T17:43:01Z

One-line PR description: Bump clusterid to beta

Issue link: ClusterID for ClusterSet identification #2149

Other comments:

Alpha --> Beta graduation requirements:

Determine if an id.k8s.io ClusterProperty be strictly a valid DNS label, or is allowed to be a subdomain.
- KEP-2149: Rename id.k8s.io --> cluster.clusterset.k8s.io #3508 simplified the usages of what was previously id.k8s.io to be specific for ClusterSet usages, limiting the blast radius of this decision. Notes from CAPI and discussion with SIG-MC are here in terms of determining whether there was a compelling use case from the field of cloud providers' cluster naming conventions. As of 3/7 SIG-MC meeting we intend to expand this to allow N dots (considering starting with N=1) to support implementation-specific disambiguation for pod DNS (the concrete use case being a region due to regional cluster registries). Language is updated in KEP-1645, KEP-2149: Location-disambiguated addressing of Headless service pods #3918.
To CRD or not to CRD
- This was provisionally decided in favor or CRD in the pre-alpha stage in March 2021 (ref) and the position had not changed despite being discussed on 6/8/2021 and 8/20/2021 and the alpha proceeding with a CRD implementation. Updates to the language reflecting this is in 8e3011b.
Determine if CRD implementation should use CEL validation to limit byte length instead of code points; this would make it only compatible with 1.23+ where CEL validation is behind a feature gate for alpha.
- Updated in this PR the limitations of CEL validation and more rationale for the unicode code point length we decided in 91cc81c

k8s-ci-robot · 2022-11-09T17:43:03Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

k8s-triage-robot · 2023-02-07T17:44:06Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-ci-robot · 2023-03-21T00:03:47Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lauralorenz
Once this PR has been reviewed and has the lgtm label, please assign jeremyot for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

keps/sig-multicluster/OWNERS

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

lauralorenz · 2023-03-29T21:26:48Z

/remove-lifecycle stale

Waiting to finalize language in #3918 to cover graduation requirement 1

PRR merged in #3917

/retest

Signed-off-by: lauralorenz <lauralorenz@google.com>

lauralorenz · 2023-04-10T21:10:22Z

/label api-review
/cc thockin

Requesting API review as the well-known properties use the k8s.io suffix. This is a re-request of API review as the first round occurred pre-alpha in #3084.

Since then I direct your attention to the following API related changes:

The well-known property formerly required to be known by metadata.name as id.k8s.io was renamed to cluster.clusterset.k8s.io to more clearly reflect its restrictions when observed by its resource name out of context. See also KEP-2149: Rename id.k8s.io --> cluster.clusterset.k8s.io #3508 and the links in the OP there.
Rationale for not requiring CEL validation for the implementation of this CRD is added in this PR in 91cc81c.

JeremyOT · 2023-05-02T22:56:58Z

/lgtm

thockin · 2023-06-05T23:30:08Z

keps/sig-multicluster/2149-clusterid/README.md

-* Define any characteristics of the system that tracks cluster ids within a cluster (i.e. a cluster registry)
-* Solve any problems without specific, tangible use cases (though we will leave room for extension).
+* Define any characteristics of the system that tracks cluster ids within a
+  cluster (i.e. a cluster registry)


s/cluster/clusterset ?

thockin · 2023-06-05T23:40:55Z

keps/sig-multicluster/2149-clusterid/README.md

@@ -253,14 +274,28 @@ nitty-gritty.
 -->

 ### Overview


I think we need a bit of setup. Something like:

"""
This proposal presupposes that some abstract control plane exists which describes the concept of a ClusterSet. The details of that control plane are beyond this specification, and are intentionally very loosely defined, such that implementations may make their own design decisions. The only assumptions made by this specification are that:

Users must be able to add clusters to the clusterset

Users may be able to remove clusters from the clusterset

This abstract control plane may exist in one or more of the constituent clusters or may exist outside of them, or may be some sort of hybrid, with aspects of both.

This abstract control plane may use the constituent clusters as the primary source of truth for information on which it depends, or it may have its own notion of truth, or may be some sort of hybrid, with aspects of both.
"""

thockin · 2023-06-05T23:44:18Z

keps/sig-multicluster/2149-clusterid/README.md

@@ -253,14 +274,28 @@ nitty-gritty.
 -->

 ### Overview
-Each cluster in a ClusterSet will be assigned a unique identifier, that lives at least as long as that cluster is a member of the given ClusterSet, and is immutable for that same lifetime. This identifier will be stored in a new cluster-scoped `ClusterProperty` CR with the well known name `cluster.clusterset.k8s.io` that may be referenced by workloads within the cluster. The identifier must either:
+Each cluster in a ClusterSet will be assigned a unique identifier, that lives at


s/will/must/ ? I'm trying to clear up the concept of who enacts what ideas and what happens if this spec is violated. "will" suggests we own the act of assigning. "must" suggests this is the law, but the implementation gets to implement...

"Each cluster in a ClusterSet must be assigned a unique identifier" implies "...or else some capabilities which depend on that identifier will not function".

Yes, this must be must :)

thockin · 2023-06-05T23:46:09Z

keps/sig-multicluster/2149-clusterid/README.md

@@ -253,14 +274,28 @@ nitty-gritty.
 -->

 ### Overview
-Each cluster in a ClusterSet will be assigned a unique identifier, that lives at least as long as that cluster is a member of the given ClusterSet, and is immutable for that same lifetime. This identifier will be stored in a new cluster-scoped `ClusterProperty` CR with the well known name `cluster.clusterset.k8s.io` that may be referenced by workloads within the cluster. The identifier must either:
+Each cluster in a ClusterSet will be assigned a unique identifier, that lives at
+least as long as that cluster is a member of the given ClusterSet, and is


s/is immutable/must not be changed/

thockin · 2023-06-06T00:03:29Z

keps/sig-multicluster/2149-clusterid/README.md

-While a member of a ClusterSet, a cluster will also have an additional `clusterset.k8s.io ClusterProperty` which describes its current membership. This property must be present exactly as long as the cluster's membership in a ClusterSet lasts, and removed when the cluster is no longer a member.
-
-More detail and examples of the uniqueness, lifespan, immutability, and content requirements for both the `cluster.clusterset.k8s.io ClusterProperty` and `clusterset.k8s.io ClusterProperty` are described further below. The goal of these requirements are to provide to the MCS API a cluster id of viable usefulness to address known user stories without being too restrictive or prescriptive.
+- or be composed of two valid [RFC-1123](https://tools.ietf.org/html/rfc1123)


Contentious: Because we are no interacting with external control planes - is this sufficiently flexible? If it turned out not to be, could we extend it? DNS is more flexible than we give it credit for - underscores are not so uncommon as I once thought, for example!

I propose we start with the current proposal, then add flexibility in beta if needed as it would not be a backward incompatible change. It seems like the next logical step would be "any valid domain name" and I don't think we're ready or see sufficient need to impose that yet

How about we change it from "one or two labels" to something like:

"""
The identifier must be a valid RFC-1123 DNS subdomain and should be less than 128 characters in total. This identifier might be used to compose larger DNS names (e.g. in the case of multi-cluster services), so care should be take to ensure that the final names fit into the limit of 253 characters.
"""

thockin · 2023-06-06T00:31:05Z

keps/sig-multicluster/2149-clusterid/README.md



 ##### Contents

 *   The identifier **must** associate the cluster with a ClusterSet.
-*   The identifier **may** be either unique or shared by members of a ClusterSet.
+*   The identifier **may** be either unique or shared by members of a


Is this more simply stated as "The identifier may be any value" ? Do we include non-ASCII? We don't have a lot of concrete use for this yet, so it's hard to know if we are cutting off interesting exploration

I would also add * The identifier should be the same across members of a ClusterSet" ?

This was introduced to highlight two patterns:

where the identifier is the same for every member and directly identifies the clusterset

where the identifier is actually a reference to a relationship - i.e. points to a cluster's "membership"

This is slightly different than just "any value" as we steer away from a mixed case. May instead of should/must because we have no control and as you say, no concrete use. But it seems like making excess room for exploration here may also be making room for pain

Why would clusterset.k8s.io indicate the relationship? I may have missed something?

thockin · 2023-06-06T00:31:57Z

keps/sig-multicluster/2149-clusterid/README.md



 ##### Consumers

-*   **Must** be able to rely on the identifier existing, unmodified for the entire duration of its membership in a ClusterSet.
-*   **Should** watch the clusterset property to detect the span of a cluster’s membership in a ClusterSet.
+*   **Must** be able to rely on the identifier existing, unmodified for the


May rely on

thockin · 2023-06-06T00:32:57Z

keps/sig-multicluster/2149-clusterid/README.md

+following suffixes are reserved for Kubernetes and related projects: `.k8s.io`,
+`.kubernetes.io`. For example, an implementation may utilize the `Kind`
+`ClusterProperty` to store objects with the name
+`fingerprint.coolmcsimplementation.com` but not `fingerprint.k8s.io` and not


s/coolmcsimplementation/example/

thockin · 2023-06-06T00:33:36Z

keps/sig-multicluster/2149-clusterid/README.md

+`.kubernetes.io`. For example, an implementation may utilize the `Kind`
+`ClusterProperty` to store objects with the name
+`fingerprint.coolmcsimplementation.com` but not `fingerprint.k8s.io` and not
+simply `fingerprint`.


We usually reserve un-spaced names for cluster operators - any reason not to do so here?

thockin · 2023-06-06T00:37:16Z

keps/sig-multicluster/2149-clusterid/README.md

-2. When a cluster that does not have a `cluster.clusterset.k8s.io ClusterProperty` tries to join a ClusterSet.
-
-In situations like these, the admission controller will need to fail to add the invalid cluster to the ClusterSet by refusing to set its `clusterset.k8s.io ClusterProperty`, and surface an error that is actionable to make the property valid.
+A cluster in a ClusterSet is expected to be authoritatively associated with that


I think I would soften all of this to make less assumptions. A self-assembling (e.g. peer-to-peer discovered) clusterset should be a valid implementation

thockin · 2023-09-01T20:26:49Z

Why did this merge? It's not labelled approved?

JeremyOT · 2023-09-01T23:00:38Z

Why did this merge? It's not labelled approved?

It didn't, this one did: #4156

Since it's a superset of commits I guess github considers this merged as well? Interesting UX

thockin · 2023-09-01T23:02:47Z

Oh. Weird.

…

On Fri, Sep 1, 2023 at 4:00 PM Jeremy Olmsted-Thompson < ***@***.***> wrote: Why did this merge? It's not labelled approved? It didn't, this one did: #4156 <#4156> Since it's a superset of commits I guess github considers this merged as well? Interesting UX — Reply to this email directly, view it on GitHub <#3652 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKWAVEKJIEFTWBYBEK7BBLXYJSKFANCNFSM6AAAAAAR3V6MRU> . You are receiving this because you were assigned.Message ID: ***@***.***>

liggitt · 2023-10-13T12:59:37Z

Oh. Weird.

It didn't, this one did: #4156

Since it's a superset of commits I guess github considers this merged as well? Interesting UX

Why did this merge? It's not labelled approved?

I'm cleaning up closed api-review issues for 1.29, were there outstanding issues or were the API bits good?

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory labels Nov 9, 2022

k8s-ci-robot requested review from JeremyOT and jeremyrickard November 9, 2022 17:43

k8s-ci-robot added sig/multicluster Categorizes an issue or PR as relevant to SIG Multicluster. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 9, 2022

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 7, 2023

lauralorenz force-pushed the bump-clusterID-beta branch from 5d8b2d4 to 34de2cd Compare March 6, 2023 23:39

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 6, 2023

lauralorenz marked this pull request as ready for review March 7, 2023 00:08

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 7, 2023

k8s-ci-robot requested a review from kikisdeliveryservice March 7, 2023 00:08

lauralorenz force-pushed the bump-clusterID-beta branch from a4c8918 to f8d7cac Compare March 7, 2023 00:15

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 20, 2023

lauralorenz force-pushed the bump-clusterID-beta branch 2 times, most recently from 5f265f7 to 3719ab2 Compare March 21, 2023 00:03

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 21, 2023

lauralorenz mentioned this pull request Mar 24, 2023

KEP-2149: Adding best effort PRR and scalability Qs required for beta #3917

Merged

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 29, 2023

lauralorenz added 2 commits April 10, 2023 20:37

bump clusterid to beta

558d946

Signed-off-by: lauralorenz <lauralorenz@google.com>

Add extended rationale for value maxLength validation

91cc81c

Signed-off-by: lauralorenz <lauralorenz@google.com>

lauralorenz added 4 commits April 10, 2023 20:39

Update language in to crd or not to crd

8e3011b

Signed-off-by: lauralorenz <lauralorenz@google.com>

fix formatting in kep.yaml

c631d59

Signed-off-by: lauralorenz <lauralorenz@google.com>

fix toc, fix spelling

95ef1f6

Signed-off-by: lauralorenz <lauralorenz@google.com>

actually update toc

910597a

Signed-off-by: lauralorenz <lauralorenz@google.com>

lauralorenz force-pushed the bump-clusterID-beta branch from 1b224b5 to 910597a Compare April 10, 2023 20:40

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

rewrap everything but toc and release checklist

5585748

Signed-off-by: lauralorenz <lauralorenz@google.com>

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 10, 2023

missed an unhappy toc

d38a347

Signed-off-by: lauralorenz <lauralorenz@google.com>

k8s-ci-robot requested a review from thockin April 10, 2023 21:10

k8s-ci-robot added the api-review Categorizes an issue or PR as actively needing an API review. label Apr 10, 2023

lauralorenz mentioned this pull request Apr 26, 2023

Update CRD to beta maturity kubernetes-sigs/about-api#18

Closed

liggitt assigned thockin Apr 27, 2023

k8s-ci-robot assigned JeremyOT May 2, 2023

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

thockin reviewed Jun 6, 2023

View reviewed changes

JeremyOT mentioned this pull request Aug 20, 2023

bump clusterid to beta #4156

Merged

3 tasks

k8s-ci-robot merged commit d38a347 into kubernetes:master Sep 1, 2023

k8s-ci-robot added this to the v1.29 milestone Sep 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KEP-2149: bump clusterid to beta #3652

KEP-2149: bump clusterid to beta #3652

lauralorenz commented Nov 9, 2022 •

edited

Loading

k8s-ci-robot commented Nov 9, 2022

k8s-triage-robot commented Feb 7, 2023

k8s-ci-robot commented Mar 21, 2023

lauralorenz commented Mar 29, 2023

lauralorenz commented Apr 10, 2023

JeremyOT commented May 2, 2023

thockin Jun 5, 2023

thockin Jun 5, 2023

thockin Jun 5, 2023

JeremyOT Jul 6, 2023

thockin Jun 5, 2023

thockin Jun 6, 2023

JeremyOT Jul 6, 2023

thockin Jul 10, 2023 •

edited

Loading

thockin Jun 6, 2023

thockin Jun 6, 2023

JeremyOT Jul 6, 2023

thockin Jul 10, 2023

thockin Jun 6, 2023

thockin Jun 6, 2023

thockin Jun 6, 2023

thockin Jun 6, 2023

thockin commented Sep 1, 2023

JeremyOT commented Sep 1, 2023

thockin commented Sep 1, 2023 via email

liggitt commented Oct 13, 2023

KEP-2149: bump clusterid to beta #3652

KEP-2149: bump clusterid to beta #3652

Conversation

lauralorenz commented Nov 9, 2022 • edited Loading

k8s-ci-robot commented Nov 9, 2022

k8s-triage-robot commented Feb 7, 2023

k8s-ci-robot commented Mar 21, 2023

lauralorenz commented Mar 29, 2023

lauralorenz commented Apr 10, 2023

JeremyOT commented May 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin commented Sep 1, 2023

JeremyOT commented Sep 1, 2023

thockin commented Sep 1, 2023 via email

liggitt commented Oct 13, 2023

lauralorenz commented Nov 9, 2022 •

edited

Loading

thockin Jul 10, 2023 •

edited

Loading