Add KEP 1659 - standard topology labels #1660

thockin · 2020-04-01T05:08:09Z

Kubernetes has always taken the position that "topology is arbitrary", and
designs dealing with topology have had to take that into account. Even so, the
project has two commonly assumed labels - topology.kubernetes.io/region and
topology.kubernetes.io/zone - which are used in many components, generally
hard-coded and not extensible. Those labels have relatively well understood
meanings, and (so far) have been sufficient to represent what most people need.

This KEP proposes to declare those labels, and possibly one more, as "standard"
and give them more well-defined meanings and semantics. APIs that handle
topology can still handle arbitrary topology keys, but these common ones may be
handled automatically.

/cc @robscott
/cc @freehan
/cc @JeremyOT
/cc @andrewsykim
/cc @dcbw
/cc @danwinship
/cc @ahg-g

k8s-ci-robot · 2020-04-01T05:08:14Z

@thockin: GitHub didn't allow me to request PR reviews from the following users: jeremyot.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Kubernetes has always taken the position that "topology is arbitrary", and
designs dealing with topology have had to take that into account. Even so, the
project has two commonly assumed labels - topology.kubernetes.io/region and
topology.kubernetes.io/zone - which are used in many components, generally
hard-coded and not extensible. Those labels have relatively well understood
meanings, and (so far) have been sufficient to represent what most people need.

This KEP proposes to declare those labels, and possibly one more, as "standard"
and give them more well-defined meanings and semantics. APIs that handle
topology can still handle arbitrary topology keys, but these common ones may be
handled automatically.

/cc @robscott
/cc @freehan
/cc @JeremyOT
/cc @andrewsykim
/cc @dcbw
/cc @danwinship
/cc @ahg-g

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

alculquicondor · 2020-04-02T18:45:45Z

#1127 for reference to previous discussions

keps/sig-architecture/1659-standard-topology-labels/README.md

thockin · 2020-04-06T23:58:23Z

Hello assignees. I am looking for a volunteer or two to be approvers of this.

andrewsykim

/approve
/lgtm

Just had some minor comments/questions about kep.yaml.

andrewsykim · 2020-04-07T15:41:41Z

keps/sig-architecture/1659-standard-topology-labels/kep.yaml

@@ -0,0 +1,17 @@
+title: Standard Topology Labels
+kep-number: NNNN #FIXME


andrewsykim · 2020-04-07T15:43:07Z

keps/sig-architecture/1659-standard-topology-labels/kep.yaml

+  - sig-cloud-provider
+  - sig-network
+  - sig-scheduling
+status: provisional


I think this should just be implemented since we're just documenting what already exists?

I think "implemented" means docs are done.

andrewsykim · 2020-04-07T15:43:48Z

keps/sig-architecture/1659-standard-topology-labels/README.md

+by downstream consumers of topology.
+
+<<[UNRESOLVED]>>
+Should we also try to standardize "kubernetes.io/hostname" as "topology.kubernetes.io/node" ?


This would be really nice for consistency, but not sure it's worth the trouble. Migrating from the old beta labels for zone/region was already a lot of work :(

Devil's advocate: we could add it and just support both, but call the old one deprecated.

See my comment above. I don't actually think limiting the application of these labels to the Node object is beneficial.

In this case, the label specifically means that the labelled item is strongly linked to a given k8s Node. The labelled item might be a pod or a disk or something else.

thockin · 2020-04-08T22:05:35Z

Nits fixed. I added you as a reviewer. Do you want to be an approver? I want to get broad eyes on this, so I tagged it sig-arch.

@smarterclayton or @dcbw or @danwinship - do you guys have thoughts here? Who else should I ping? :) Other cloud-provider people?

@ahg-g for scheduling

andrewsykim · 2020-04-08T22:07:50Z

ping @cheftako @nckturner for SIG cloud provider

thockin · 2020-05-09T06:19:51Z

@cheftako @nckturner @ahg-g @smarterclayton

This KEP has been open for a while. I'm still looking for an approver (unless you want to be it @andrewsykim ?) and more feedback.

ahg-g · 2020-05-09T13:03:32Z

@cheftako @nckturner @ahg-g @smarterclayton

This KEP has been open for a while. I'm still looking for an approver (unless you want to be it @andrewsykim ?) and more feedback.

This make a lot of sense for the scheduler, in the fact the scheduler already uses the zone topology label to spread pods by default across zones. The scheduler also special cases kubernetes.io/hostname topology in spreading/affinity computations to boost performance.

ahg-g

/approve
/lgtm

ahg-g · 2020-05-09T13:05:36Z

keps/sig-architecture/1659-standard-topology-labels/kep.yaml

+creation-date: 2020-03-31
+reviewers:
+  - "@andrewsykim"
+approvers:


you can list me as an approver.

M00nF1sh · 2020-05-09T17:26:17Z

/approve
/lgtm
:D

jaypipes · 2020-05-11T16:08:35Z

keps/sig-architecture/1659-standard-topology-labels/README.md

+Label prefixes allow us to group labels on common origin and meaning.  We
+propose to document somewhere (TBD) that the prefix "topology.kuberntes.io" is
+explicitly reserved for use in defining metadata about the physical or logical
+connectivity and grouping of Kubernetes nodes, and the associated behavioral


I recognize that the topology.kubernetes.io/region and topology.kubernetes.io/zone labels are currently mostly used for Kubernetes Node objects, but I think explicitly limiting the application of these labels to Node objects might be unnecessarily constraining. I've seen other cloud/virtual objects -- elastic load balancers, VPC subnets, etc -- as well as objects that represent physical inventory -- racks, TORs, etc -- be decorated with these labels and the desired meaning of the label is identical even if it doesn't apply to a Node object.

jaypipes · 2020-05-11T16:13:39Z

keps/sig-architecture/1659-standard-topology-labels/README.md

+
+This will also define that, while labels are generally mutable, the topology
+labels should be assumed immutable and that any changes to them may be ignored
+by downstream consumers of topology.


Would "region" have some explicit geographic denotation? Currently, "region" is almost always (AFAICT) assumed to be related to a geographic location. I would support an explicit callout that a "region" should actually NOT be assumed to be a specific geographic location in order to dispel this particularly constraining assumption. :)

I agree, but I think that's a comment for a PR to actually write the words :)

jaypipes · 2020-05-11T16:17:24Z

keps/sig-architecture/1659-standard-topology-labels/README.md

+by downstream consumers of topology.
+
+<<[UNRESOLVED]>>
+Should we also try to standardize "kubernetes.io/hostname" as "topology.kubernetes.io/node" ?


See my comment above. I don't actually think limiting the application of these labels to the Node object is beneficial.

jaypipes · 2020-05-11T23:39:20Z

keps/sig-architecture/1659-standard-topology-labels/README.md

+This KEP proposes to define the meaning and semantics of the following labels:
+
+    * topology.kubernetes.io/region
+    * topology.kubernetes.io/zone


Above, you say the definition of "region" and "zone" needs to address "the associated behavioral and failure properties of those groups". I think I agree. I'm interested in seeing the proposed definitions from folks in this area.

In particular, I'm curious how a single key-value label will be enough to describe the behaviours and failure properties. Would one alternative to this KEP (albeit a more invasive one) be to have actual Region and Zone objects in the API that could have additional properties associated with them?

I don't see that as an alternative to, but rather a further extension. If we assert that topology is region/zone/host then we may want to expose additional information about the possible values for those labels (network cost metric, etc)

szuecs · 2020-06-05T08:54:50Z

@thockin I just had an idea about hybrid cloud model could also be improved, if you could specify "use DC first" (maybe more relevant to multi cluster scheduling).

TBH: I don't have a hybrid cloud, so take it maybe as random idea with no priority.

thockin · 2020-06-19T21:36:02Z

@szuecs I am trying to bite off a really small nibble here - just documenting the current assumptions and behavior. :) How this is used is a different KEP (or several)

thockin · 2020-06-19T21:36:49Z

@ahg-g PTAL

ahg-g

/lgtm
/approve

ahg-g · 2020-06-20T13:09:21Z

keps/sig-architecture/1659-standard-topology-labels/kep.yaml

+replaces: []
+
+prr-approvers:
+  - "@jbelamaric"


verify is complaining, use johnbelamaric

freehan

LGTM. just one nit

keps/sig-architecture/1659-standard-topology-labels/README.md

thockin · 2020-06-23T15:59:55Z

Pushed with nits fixed

thockin · 2020-06-25T20:44:05Z

This KEP still needs another LGTM and approve :)

ahg-g · 2020-07-16T02:03:33Z

/lgtm

k8s-ci-robot · 2020-07-16T14:52:46Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: ahg-g, andrewsykim, M00nF1sh, 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:

keps/sig-architecture/OWNERS

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

k8s-ci-robot requested review from andrewsykim, dcbw and danwinship April 1, 2020 05:08

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 1, 2020

k8s-ci-robot requested review from ahg-g, robscott and freehan April 1, 2020 05:08

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. labels Apr 1, 2020

thockin force-pushed the standard-topology-labels branch 2 times, most recently from 35db052 to ed25c3f Compare April 1, 2020 18:23

andrewsykim reviewed Apr 2, 2020

View reviewed changes

keps/sig-architecture/1659-standard-topology-labels/README.md Show resolved Hide resolved

thockin force-pushed the standard-topology-labels branch from ed25c3f to b21c6a2 Compare April 6, 2020 23:55

thockin assigned dcbw, andrewsykim and ahg-g Apr 6, 2020

thockin force-pushed the standard-topology-labels branch from b21c6a2 to d48d06f Compare April 6, 2020 23:57

andrewsykim reviewed Apr 7, 2020

View reviewed changes

thockin force-pushed the standard-topology-labels branch from d48d06f to 122164c Compare April 8, 2020 22:03

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 8, 2020

thockin added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 8, 2020

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

ahg-g reviewed May 9, 2020

View reviewed changes

k8s-ci-robot assigned M00nF1sh May 9, 2020

jaypipes reviewed May 11, 2020

View reviewed changes

thockin force-pushed the standard-topology-labels branch from 122164c to f2a4f0f Compare June 19, 2020 21:34

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 19, 2020

thockin force-pushed the standard-topology-labels branch from f2a4f0f to 76e2171 Compare June 19, 2020 21:36

ahg-g reviewed Jun 20, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 20, 2020

thockin force-pushed the standard-topology-labels branch from 76e2171 to 9dd59dd Compare June 22, 2020 23:32

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 22, 2020

freehan reviewed Jun 22, 2020

View reviewed changes

keps/sig-architecture/1659-standard-topology-labels/README.md Outdated Show resolved Hide resolved

Add KEP 1659 - standard topology labels

0d40180

thockin force-pushed the standard-topology-labels branch from 9dd59dd to 0d40180 Compare June 23, 2020 00:09

thockin removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 23, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 16, 2020

thockin added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 16, 2020

k8s-ci-robot merged commit b68b9d7 into kubernetes:master Jul 16, 2020

k8s-ci-robot added this to the v1.19 milestone Jul 16, 2020

thockin mentioned this pull request Oct 29, 2020

Better docs for standard topology labels kubernetes/website#24783

Merged

		@@ -0,0 +1,17 @@
		title: Standard Topology Labels
		kep-number: NNNN #FIXME

Add KEP 1659 - standard topology labels #1660

Add KEP 1659 - standard topology labels #1660

Conversation

thockin commented Apr 1, 2020

k8s-ci-robot commented Apr 1, 2020

alculquicondor commented Apr 2, 2020

thockin commented Apr 6, 2020

andrewsykim left a 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

Choose a reason for hiding this comment

thockin commented Apr 8, 2020

andrewsykim commented Apr 8, 2020

thockin commented May 9, 2020

ahg-g commented May 9, 2020 • edited Loading

ahg-g left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

M00nF1sh commented May 9, 2020

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

szuecs commented Jun 5, 2020

thockin commented Jun 19, 2020

thockin commented Jun 19, 2020

ahg-g left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freehan left a comment

Choose a reason for hiding this comment

thockin commented Jun 23, 2020

thockin commented Jun 25, 2020

ahg-g commented Jul 16, 2020

k8s-ci-robot commented Jul 16, 2020

ahg-g commented May 9, 2020 •

edited

Loading