remove created-by annotation #54445

crimsonfaith91 · 2017-10-24T00:57:24Z

What this PR does / why we need it:
This PR removes CreatedByAnnotation.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #50720

Release note:

The `kubernetes.io/created-by` annotation is no longer added to controller-created objects. Use the  `metadata.ownerReferences` item that has `controller` set to `true` to determine which controller, if any, owns an object.

kow3ns · 2017-10-26T17:13:45Z

/approve

janetkuo · 2017-10-27T17:57:11Z

pkg/controller/controller_utils.go

@@ -474,29 +472,12 @@ func getPodsFinalizers(template *v1.PodTemplateSpec) []string {
 	return desiredFinalizers
 }

-func getPodsAnnotationSet(template *v1.PodTemplateSpec, object runtime.Object) (labels.Set, error) {
+func getPodsAnnotationSet(template *v1.PodTemplateSpec, object runtime.Object) labels.Set {


object is no longer needed

janetkuo · 2017-10-27T18:11:04Z

pkg/kubectl/cmd/drain_test.go

 	orphaned_ds_pod := corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:              "bar",
 			Namespace:         "default",
 			CreationTimestamp: metav1.Time{Time: time.Now()},
 			Labels:            labels,
 			SelfLink:          testapi.Default.SelfLink("pods", "bar"),
-			Annotations:       missing_ds_anno,
 			OwnerReferences: []metav1.OwnerReference{


This test setup isn't correct. An "orphaned" pod does not have any controller (controllerRef == nil). "Orphan" simply means it's not controlled by anyone, and GC controller "orphans" a resource by removing its controllerRef.

I am aware of it, but i didn't change it as it is not relevant to created-by annotation.

It's actually related, because this controllerRef was mistakenly added here when replacing created-by with controllerRef d76b08d#diff-db0998ca74ee87d106ee23d584f337a5R399

I verified it was me who added the owner reference to the test. I did not know about orphaning at that moment (it was my first PR for k8s main repo). Thanks for catching it!

janetkuo · 2017-10-27T18:34:30Z

hack/make-rules/test-cmd-util.sh

@@ -1253,7 +1253,7 @@ run_kubectl_run_tests() {
  # Post-Condition: Job "pi" is created
  kube::test::get_object_assert jobs "{{range.items}}{{$id_field}}:{{end}}" 'pi:'
  # Describe command (resource only) should print detailed information
-  kube::test::describe_resource_assert pods "Name:" "Image:" "Node:" "Labels:" "Status:" "Created By"
+  kube::test::describe_resource_assert pods "Name:" "Image:" "Node:" "Labels:" "Status:"


For every kube::test::describe_resource_assert call to a resource that's controlled by something, verify that "Controlled By" is there too

Does "Controlled By" apply to these?
https://github.com/kubernetes/kubernetes/blob/master/hack/make-rules/test-cmd-util.sh#L520
https://github.com/kubernetes/kubernetes/blob/master/hack/make-rules/test-cmd-util.sh#L3983

As synced by person, the above two tests should not be added with "Controlled By" print attribute. For the first test, we are not interested with it. For the second one, although a node is managed by node controller, "Controlled By" print attribute will look at controllerRef field, which is not available for nodes.

I also did a thorough search, and added "Controlled By" to only the tests that manage to pass with it.

crimsonfaith91 · 2017-10-27T21:58:31Z

For future reference, Controlled By will not be available for printing if the resource is created directly from a YAML file. A resource needs to be created by a parent resource (e.g., a replicaset is created by a deployment) for it to have Controlled By info at the beginning. This is because Controlled By info depends on controllerRef field under the hood, which is not set if there is no parent resource.

The resource created directly from a YAML file can later be adopted and controlled by a parent resource, in which the resource's controllerRef points to the parent resource.

Note that the controllerRef field is populated when an ownerReference has controller field set to true. Each resource can have only one ownerReference with controller field as true.

It is possible for a node or a service to have controllerRef field pointing to a parent resource. A user can create a custom resource (not k8s core resource) that controls the node or service.

janetkuo · 2017-10-30T18:37:52Z

For future reference, Controlled By will not be available for printing if the resource is created directly from a YAML file. A resource needs to be created by a parent resource (e.g., a replicaset is created by a deployment) for it to have Controlled By info. This is because Controlled By info depends on controllerRef field under the hood.

To clarify, node and service do not have controllerRef field, so Controlled By info is not applicable too.

When we say controllerRef we actually mean the .metadata.ownerReferences field and specifically the one with .metadata.ownerReference[].controller == true. Every resource has the ownerReferences field (as long as it has metadata).

A resource can be adopted by other resources dynamically. For example, an RS can be created from a yaml file, and it does not have controllerRef at creation time (because no one controls its lifecycle); however, it can then be adopted (and controlled) by a deployment, and have its controllerRef points to the deployment.

Theoretically, a user can create a custom resource (not Kubernetes core resource) that controls a node or service, and have their controllerRef point to the custom resource.

crimsonfaith91 · 2017-10-30T19:12:33Z

@janetkuo Thanks for pointing out the custom resource - didn't know about that. Is the custom resource named TPR or CRD? Updated my summary comment too.

If everything looks good to you, I will squash.

janetkuo · 2017-10-30T19:45:00Z

LGTM, feel free to squash.

Is the custom resource named TPR or CRD

Custom resource can be defined with CRD. CRD supersedes TPR.

janetkuo · 2017-10-31T00:51:47Z

/lgtm

crimsonfaith91 · 2017-10-31T01:02:42Z

/assign @lavalamp @liggitt
This PR removes created-by annotation. It needs your approval for merging. The annotation has been deprecated in 1.8, and is scheduled for removal in 1.9.

liggitt · 2017-10-31T17:10:00Z

noted deprecated in https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG-1.8.md#deprecations

updated release note with more detailed info

/lgtm

crimsonfaith91 · 2017-10-31T17:18:50Z

@liggitt Thanks for updating the release note with more detailed info! I will make changes to CHANGELOG-1.8.md once this PR merges.

fejta · 2017-10-31T22:56:46Z

/approve

k8s-github-robot · 2017-10-31T22:57:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crimsonfaith91, fejta, janetkuo, kow3ns, liggitt

Associated issue: 50720

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

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

~~hack/OWNERS~~ [fejta]
~~pkg/api/OWNERS~~ [liggitt]
~~pkg/controller/OWNERS~~ [janetkuo]
~~pkg/kubectl/OWNERS~~ [janetkuo,liggitt]
~~pkg/printers/OWNERS~~ [janetkuo,liggitt]
~~staging/src/k8s.io/api/OWNERS~~ [liggitt]
~~test/e2e/kubectl/OWNERS~~ [fejta,janetkuo,kow3ns,liggitt]

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-01T03:10:20Z

Automatic merge from submit-queue (batch tested with PRs 53190, 54790, 54445, 52607, 54801). If you want to cherry-pick this change to another branch, please follow the instructions here.

Automatic merge from submit-queue (batch tested with PRs 53190, 54790, 54445, 52607, 54801). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. remove created-by annotation **What this PR does / why we need it**: This PR removes `CreatedByAnnotation`. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes kubernetes#50720 **Release note**: ```release-note The `kubernetes.io/created-by` annotation is no longer added to controller-created objects. Use the `metadata.ownerReferences` item that has `controller` set to `true` to determine which controller, if any, owns an object. ```

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 24, 2017

k8s-ci-robot assigned crimsonfaith91 Oct 24, 2017

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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 Oct 24, 2017

crimsonfaith91 force-pushed the rem branch 3 times, most recently from 4b226ed to d6a748c Compare October 24, 2017 19:10

k8s-ci-robot requested a review from kow3ns October 24, 2017 20:10

k8s-ci-robot assigned crimsonfaith91 and unassigned crimsonfaith91 Oct 24, 2017

crimsonfaith91 force-pushed the rem branch from d6a748c to 34dfa28 Compare October 24, 2017 23:19

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 26, 2017

kow3ns approved these changes Oct 26, 2017

View reviewed changes

crimsonfaith91 force-pushed the rem branch from 34dfa28 to c805bda Compare October 26, 2017 17:23

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 26, 2017

janetkuo assigned janetkuo and unassigned crimsonfaith91 Oct 27, 2017

janetkuo reviewed Oct 27, 2017

View reviewed changes

janetkuo approved these changes Oct 30, 2017

View reviewed changes

remove created-by annotation

efbfead

crimsonfaith91 force-pushed the rem branch from d65a1c9 to efbfead Compare October 30, 2017 19:50

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

k8s-ci-robot assigned lavalamp and liggitt Oct 31, 2017

liggitt added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Oct 31, 2017

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. labels Oct 31, 2017

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

k8s-github-robot merged commit ed00d9c into kubernetes:master Nov 1, 2017

crimsonfaith91 mentioned this pull request Nov 2, 2017

Update created-by annotation details #54951

Closed

This was referenced Nov 29, 2017

[e2e failure] [sig-cli] Kubectl client [k8s.io] Kubectl describe should check if kubectl describe prints relevant information for rc and pods [Conformance] #56414

Closed

adjust the expected output based kubectl verison #56508

Merged

chino mentioned this pull request Jan 10, 2018

CreatedByAnnotation is deprecated in 1.9 coreos/container-linux-update-operator#163

Closed

andor44 mentioned this pull request Jan 24, 2018

Kubernetes 1.9 breaks pod owner lookup DataDog/dd-agent#3642

Closed

antoinepouille mentioned this pull request Feb 9, 2018

[kubernetes] fix logic to collect ownerreference for k8 1.9 DataDog/dd-agent#3667

Merged

yilmi mentioned this pull request Aug 9, 2022

Removing created-by label deprecated since 1.9 kubernetes/website#35810

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove created-by annotation #54445

remove created-by annotation #54445

crimsonfaith91 commented Oct 24, 2017 •

edited by liggitt

kow3ns commented Oct 26, 2017

janetkuo Oct 27, 2017

janetkuo Oct 27, 2017 •

edited

crimsonfaith91 Oct 27, 2017

janetkuo Oct 27, 2017

crimsonfaith91 Oct 27, 2017 •

edited

janetkuo Oct 27, 2017

crimsonfaith91 Oct 27, 2017 •

edited

crimsonfaith91 Oct 27, 2017 •

edited

crimsonfaith91 commented Oct 27, 2017 •

edited

janetkuo commented Oct 30, 2017

crimsonfaith91 commented Oct 30, 2017 •

edited

janetkuo commented Oct 30, 2017

janetkuo commented Oct 31, 2017

crimsonfaith91 commented Oct 31, 2017 •

edited

liggitt commented Oct 31, 2017

crimsonfaith91 commented Oct 31, 2017

fejta commented Oct 31, 2017

k8s-github-robot commented Oct 31, 2017

k8s-github-robot commented Nov 1, 2017

Navigation Menu

remove created-by annotation #54445

remove created-by annotation #54445

Conversation

crimsonfaith91 commented Oct 24, 2017 • edited by liggitt

kow3ns commented Oct 26, 2017

janetkuo Oct 27, 2017

Choose a reason for hiding this comment

janetkuo Oct 27, 2017 • edited

Choose a reason for hiding this comment

crimsonfaith91 Oct 27, 2017

Choose a reason for hiding this comment

janetkuo Oct 27, 2017

Choose a reason for hiding this comment

crimsonfaith91 Oct 27, 2017 • edited

Choose a reason for hiding this comment

janetkuo Oct 27, 2017

Choose a reason for hiding this comment

crimsonfaith91 Oct 27, 2017 • edited

Choose a reason for hiding this comment

crimsonfaith91 Oct 27, 2017 • edited

Choose a reason for hiding this comment

crimsonfaith91 commented Oct 27, 2017 • edited

janetkuo commented Oct 30, 2017

crimsonfaith91 commented Oct 30, 2017 • edited

janetkuo commented Oct 30, 2017

janetkuo commented Oct 31, 2017

crimsonfaith91 commented Oct 31, 2017 • edited

liggitt commented Oct 31, 2017

crimsonfaith91 commented Oct 31, 2017

fejta commented Oct 31, 2017

k8s-github-robot commented Oct 31, 2017

k8s-github-robot commented Nov 1, 2017

crimsonfaith91 commented Oct 24, 2017 •

edited by liggitt

janetkuo Oct 27, 2017 •

edited

crimsonfaith91 Oct 27, 2017 •

edited

crimsonfaith91 Oct 27, 2017 •

edited

crimsonfaith91 Oct 27, 2017 •

edited

crimsonfaith91 commented Oct 27, 2017 •

edited

crimsonfaith91 commented Oct 30, 2017 •

edited

crimsonfaith91 commented Oct 31, 2017 •

edited