Sts per Pod Name Label #55329

kow3ns · 2017-11-08T17:36:00Z

What this PR does / why we need it:
StatefulSet controller will add a label for each Pod in the StatefulSet. The label is of the form
statefulset.kubernetes.io/pod-name: <pod.Name>. This allows a unique service to be created for each Pod in the StatefulSet.

Fixes #44103, #28660

StatefulSet controller will create a label for each Pod in a StatefulSet. The label is named statefulset.kubernetes.io/pod-name and it is equal to the name of the Pod. This allows users to create a Service per Pod to expose a connection to individual Pods.

enisoc · 2017-11-08T18:16:29Z

pkg/controller/statefulset/stateful_set_utils.go

@@ -37,6 +37,10 @@ import (
 // maxUpdateRetries is the maximum number of retries used for update conflict resolution prior to failure
 const maxUpdateRetries = 10

+// StatefulSetPodNameLabel is label added to each Pod in a StatefulSet. Its purpose is to allow Services to select
+// individual Pods in the StatefulSet.
+const StatefulSetPodNameLabel = "statefulSetPodName"


Do we have conventions for machine-managed label names? For controller revision, we use controller.kubernetes.io/hash. For failure domains, we use failure-domain.beta.kubernetes.io/region. For Deployment, we use pod-template-hash.

Also, should we put this constant in types.go for a given API version like the others?

Also, should we put this constant in types.go for a given API version like the others?

The label is not version specific, and, by convention, the versioning seems to only apply to annotations. If we were to move it, I think the only other appropriate home would well_know_labels.go in the apis package.

I just want the name to be easily usable statefulset.kubernetes.io/podName is fine with me, but as you point out we don't have conventions. It would be good to start a trend though. Do you have thoughts on a better name?

enisoc · 2017-11-08T18:20:16Z

pkg/controller/statefulset/stateful_set_utils.go

@@ -112,7 +116,8 @@ func identityMatches(set *apps.StatefulSet, pod *v1.Pod) bool {
 	return ordinal >= 0 &&
 		set.Name == parent &&
 		pod.Name == getPodName(set, ordinal) &&
-		pod.Namespace == set.Namespace
+		pod.Namespace == set.Namespace &&
+		pod.Labels[StatefulSetPodNameLabel] == pod.Name


Do we lose anything by making the label just the ordinal, instead of the whole Pod name?

I think it's more flexible. For example, you could select the ordinal=0 Pod across multiple StatefulSets.

yes, the pod name is namespace unique. The ordinal is not.

Selecting the same Pod across multiple sts's defeats the purpose of the label.

I see. The ordinal label would be insufficient, even if combined with the other selector labels, because controllers can have overlapping selectors. Agreed that selecting across multiple StatefulSets doesn't meet any need we've ever heard of.

enisoc · 2017-11-08T18:23:59Z

pkg/controller/statefulset/stateful_set_utils.go

 func updateIdentity(set *apps.StatefulSet, pod *v1.Pod) {
 	pod.Name = getPodName(set, getOrdinal(pod))
 	pod.Namespace = set.Namespace
-
+	pod.Labels[StatefulSetPodNameLabel] = pod.Name


Is it possible that pod.Labels has become nil again at this point? Do we enforce anywhere that Pods must have at least one label?

it shouldn't but it doesn't hurt to check

kow3ns · 2017-11-09T00:40:40Z

/retest

enisoc · 2017-11-09T22:44:53Z

I found some written conventions that seem to apply here:

Inclusion of a system prefix in a label key is fairly hostile to UX. A prefix is only necessary in the case that the user cannot choose the label key, in order to avoid collisions with user-defined labels. However, I firmly believe that the user should always be allowed to select the label keys to use on their resources, so it should always be possible to override default label keys.

Therefore, resources supporting auto-generation of unique labels should have a uniqueLabelKey field, so that the user could specify the key if they wanted to, but if unspecified, it could be set by default, such as to the resource type, like job, deployment, or replicationController.

This indicates we should avoid adding a prefix for the label key, but we should allow the user to override the label key with a setting in StatefulSet.Spec.

For backward compatibility, maybe we should set the default key as "" (meaning don't add a label at all) for pre-v1 versions, so the behavior stays the same. For v1, we can set a non-empty default key.

Key names should be all lowercase, with words separated by dashes instead of camelCase

Given this I suggest pod-name as the default key for v1.

kow3ns · 2017-11-13T19:20:33Z

Therefore, resources supporting auto-generation of unique labels should have a uniqueLabelKey field, so that the user could specify the key if they wanted to, but if unspecified, it could be set by default, such as to the resource type, like job, deployment, or replicationController.

This is not applicable as it pertains to auto generated labels based on hashes and UUIDs that have no semantic meaning. e.g. Job controllers override.

For backward compatibility, maybe we should set the default key as "" (meaning don't add a label at all) for pre-v1 versions, so the behavior stays the same. For v1, we can set a non-empty default key.

I don't think we should allow name overriding without a more compelling reason. We should prefer simplicity over the potential to introduce misconfiguration unless there is an actual use case to support. If we find one, we can add an override switch in a backward compatible way.

Given this I suggest pod-name as the default key for v1.

SGTM

janetkuo · 2017-11-13T21:58:47Z

pkg/controller/statefulset/stateful_set_utils.go

@@ -37,6 +37,10 @@ import (
 // maxUpdateRetries is the maximum number of retries used for update conflict resolution prior to failure
 const maxUpdateRetries = 10

+// StatefulSetPodNameLabel is label added to each Pod in a StatefulSet. Its purpose is to allow Services to select
+// individual Pods in the StatefulSet.
+const StatefulSetPodNameLabel = "statefulset.kubernetes.io/podName"


We should probably put this in types.go, like where we placed pod-template-hash

janetkuo · 2017-11-13T22:06:13Z

pkg/controller/statefulset/stateful_set_utils.go

@@ -185,13 +190,18 @@ func initIdentity(set *apps.StatefulSet, pod *v1.Pod) {
 	// Set these immutable fields only on initial Pod creation, not updates.
 	pod.Spec.Hostname = pod.Name
 	pod.Spec.Subdomain = set.Spec.ServiceName
+


nit: remove this

janetkuo · 2017-11-13T22:42:32Z

pkg/controller/statefulset/stateful_set_utils.go

@@ -37,6 +37,10 @@ import (
 // maxUpdateRetries is the maximum number of retries used for update conflict resolution prior to failure
 const maxUpdateRetries = 10

+// StatefulSetPodNameLabel is label added to each Pod in a StatefulSet. Its purpose is to allow Services to select
+// individual Pods in the StatefulSet.
+const StatefulSetPodNameLabel = "statefulset.kubernetes.io/podName"


Most workload controllers use xxx-xxx-xxx format (instead of xxx.xxxx.xxx/xxxx) as label key (e.g. pod-template-hash, controller-revision-hash).

Deployment hash label was named pod-template-hash instead of something like deployment.kubernetes.io/podTemplateHash, because the latter is lengthy and more suitable as annotations -- we expect users to type label name (e.g. kubectl get name -l) more frequently than annotations.

This is not documented AFAIK. The convention may have changed.

janetkuo · 2017-11-13T22:43:47Z

Does this fix #28660?

enisoc · 2017-11-13T23:36:28Z

@kow3ns wrote:

This is not applicable as it pertains to auto generated labels based on hashes and UUIDs that have no semantic meaning. e.g. Job controllers override.

That particular paragraph may be specific to hash-style labels, but it's an application of the general convention proposed above it:

I firmly believe that the user should always be allowed to select the label keys to use on their resources, so it should always be possible to override default label keys.

Of course, it's only the author's opinion of what the convention should be. Also, it seems we have decided not to follow it for Job and Deployment, so I'm willing to chalk it up to out-of-date documentation.

However, I still think it's important that this behavior be controlled by a field that's off by default for pre-v1. Unlike annotations, labels are the user's domain, so adding a new label is a change in behavior. What do you think?

@janetkuo wrote:

Does this fix #28660?

I think it makes a workaround possible, but a full fix for that issue would include a feature that creates a Service per StatefulSet Pod for you.

kow3ns · 2017-11-16T19:27:34Z

@janetkuo I think it closes #28660 as will not fix. We will never materializes Services on behalf of the user. We will provide, as a part of the StatefulSet's Pods' identity, a label that allows Services to be attached to the Pods in order to allow users to configure the Pods' networking as appropriate to their use case.

@enisoc The label should be considered to be part of the Pods identity. Having a unique identity that is not selectable has become tantamount to a defect for many users. Service per Pod is the right way to route traffic to StatefulSet Pods where each Pod needs to advertise a unique network identity, and, without this label as part of the Pods identity, there is no viable way to do this. If adding the label was a destructive update I would definitely want an opt in switch, but the addition of the label to running Pods is a non-destructive update that should not adversely impact users.

enisoc · 2017-11-16T20:36:55Z

I still think that adding the label is new behavior according to the letter of the API conventions, but I'm willing to believe that it's non-destructive in all but the most idiosyncratic of setups. In addition, we are only "breaking" pre-v1 APIs.

@janetkuo Do you have an opinion on the above?

kow3ns · 2017-11-17T19:14:34Z

/retest

thockin · 2017-11-18T00:42:44Z

staging/src/k8s.io/api/apps/v1/types.go

@@ -28,6 +28,7 @@ const (
 	StatefulSetRevisionLabel       = ControllerRevisionHashLabelKey
 	DeprecatedRollbackTo           = "deprecated.deployment.rollback.to"
 	DeprecatedTemplateGeneration   = "deprecated.daemonset.template.generation"
+	StatefulSetPodNameLabel        = "statefulset-pod-name"


This should be prefixed. We have absolutlely no consistency across teh codebase but it should be either:

statefulset.kubernetes.io/pod-name

or

kubernetes.io/statefulset-pod-name

I lean towards the former.

revision hash should probably be, also, but it's harder to fix retroactively.

attach a Service to an individual Pod.

thockin · 2017-11-20T18:56:05Z

/approve

kow3ns · 2017-11-20T20:29:51Z

/retest

k8s-github-robot · 2017-11-21T00:24:37Z

[MILESTONENOTIFIER] Milestone Pull Request Needs Attention

@enisoc @janetkuo @kow3ns @thockin @kubernetes/sig-apps-misc

Action required: During code slush, pull requests in the milestone should be in progress.
If this pull request is not being actively worked on, please remove it from the milestone.
If it is being worked on, please add the status/in-progress label so it can be tracked with other in-flight pull requests.

Note: If this pull request is not resolved or labeled as priority/critical-urgent by Wed, Nov 22 it will be moved out of the v1.9 milestone.

Pull Request Labels

sig/apps: Pull Request will be escalated to these SIGs if needed.
priority/important-longterm: Escalate to the pull request owners; move out of the milestone after 1 attempt.
kind/feature: New functionality.

Help

janetkuo · 2017-11-21T21:02:10Z

/lgtm

k8s-github-robot · 2017-11-21T21:03:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: janetkuo, kow3ns, thockin

Associated issue: 44103

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

Needs approval from an approver in each of these OWNERS Files:

~~pkg/controller/statefulset/OWNERS~~ [janetkuo,kow3ns,thockin]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-11-22T02:56:30Z

Automatic merge from submit-queue (batch tested with PRs 55340, 55329, 56168, 56170, 56105). If you want to cherry-pick this change to another branch, please follow the instructions here.

StatefulSets have pod name labels since Kubernetes v1.9 [1] Minimum supportted version in the project is 1.17.2+ The sync pod-name hook is provided in the example repo [2] for older versions where Sts pod-name-labels are not set by default [1] kubernetes/kubernetes#55329 [2] https://github.com/GoogleCloudPlatform/metacontroller/tree/master/examples/service-per-pod

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 8, 2017

kow3ns requested a review from janetkuo November 8, 2017 17:36

k8s-github-robot assigned enisoc and janetkuo Nov 8, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 8, 2017

enisoc reviewed Nov 8, 2017

View reviewed changes

janetkuo reviewed Nov 13, 2017

View reviewed changes

kow3ns force-pushed the sts-name-label branch from ffa1624 to 4aab534 Compare November 16, 2017 19:00

k8s-github-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 16, 2017

kow3ns assigned thockin Nov 16, 2017

thockin reviewed Nov 18, 2017

View reviewed changes

foxish mentioned this pull request Nov 18, 2017

[mongodb-replicaset] WIP - Add name label to pods helm/charts#1743

Closed

Adds the statefulset.kubernetes.io/pod-name label allowing users to

1ce4ef8

attach a Service to an individual Pod.

kow3ns force-pushed the sts-name-label branch from d7d5fcc to 1ce4ef8 Compare November 20, 2017 17:14

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 20, 2017

enisoc added kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/apps Categorizes an issue or PR as relevant to SIG Apps. labels Nov 20, 2017

enisoc added the status/approved-for-milestone label Nov 20, 2017

enisoc added this to the v1.9 milestone Nov 20, 2017

k8s-github-robot added the milestone/needs-attention label Nov 21, 2017

enisoc added the status/in-review label Nov 21, 2017

dhilipkumars mentioned this pull request Nov 21, 2017

StatefulSet deletion leaves an orphan pod if the pod label has been changed #55559

Closed

janetkuo approved these changes Nov 21, 2017

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 21, 2017

k8s-github-robot merged commit 5ac4f17 into kubernetes:master Nov 22, 2017

tengqm mentioned this pull request Nov 25, 2017

Statefulset pod label not documented kubernetes/website#6448

Closed

0xmichalis mentioned this pull request Dec 5, 2017

public network identities for pets #28660

Closed

sander-su mentioned this pull request Dec 21, 2017

Sticky IPs for StatefulSet #28969

Closed

solsson mentioned this pull request Jan 8, 2018

Fix rbac to set useful labels on broker pods from init script Yolean/kubernetes-kafka#117

Merged

jpiper mentioned this pull request Aug 3, 2018

Datanodes would be left with stale DNS records if namenode pods restart. apache-spark-on-k8s/kubernetes-HDFS#48

Open

manquer mentioned this pull request May 17, 2021

Removes Sync Pod Name DecoratorController and hook hpi-schul-cloud/jitsi-deployment#40

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sts per Pod Name Label #55329

Sts per Pod Name Label #55329

kow3ns commented Nov 8, 2017 •

edited

enisoc Nov 8, 2017

kow3ns Nov 8, 2017 •

edited

enisoc Nov 8, 2017

kow3ns Nov 8, 2017

kow3ns Nov 8, 2017

enisoc Nov 9, 2017

enisoc Nov 8, 2017

kow3ns Nov 8, 2017

kow3ns commented Nov 9, 2017

enisoc commented Nov 9, 2017

kow3ns commented Nov 13, 2017 •

edited

janetkuo Nov 13, 2017

janetkuo Nov 13, 2017

janetkuo Nov 13, 2017 •

edited

janetkuo commented Nov 13, 2017

enisoc commented Nov 13, 2017

kow3ns commented Nov 16, 2017

enisoc commented Nov 16, 2017

kow3ns commented Nov 17, 2017

thockin Nov 18, 2017

thockin commented Nov 20, 2017

kow3ns commented Nov 20, 2017

k8s-github-robot commented Nov 21, 2017

janetkuo commented Nov 21, 2017

k8s-github-robot commented Nov 21, 2017

k8s-github-robot commented Nov 22, 2017

Sts per Pod Name Label #55329

Sts per Pod Name Label #55329

Conversation

kow3ns commented Nov 8, 2017 • edited

Choose a reason for hiding this comment

kow3ns Nov 8, 2017 • edited

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

kow3ns commented Nov 9, 2017

enisoc commented Nov 9, 2017

kow3ns commented Nov 13, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janetkuo Nov 13, 2017 • edited

Choose a reason for hiding this comment

janetkuo commented Nov 13, 2017

enisoc commented Nov 13, 2017

kow3ns commented Nov 16, 2017

enisoc commented Nov 16, 2017

kow3ns commented Nov 17, 2017

Choose a reason for hiding this comment

thockin commented Nov 20, 2017

kow3ns commented Nov 20, 2017

k8s-github-robot commented Nov 21, 2017

janetkuo commented Nov 21, 2017

k8s-github-robot commented Nov 21, 2017

k8s-github-robot commented Nov 22, 2017

kow3ns commented Nov 8, 2017 •

edited

kow3ns Nov 8, 2017 •

edited

kow3ns commented Nov 13, 2017 •

edited

janetkuo Nov 13, 2017 •

edited