kube-state-metrics should not use CreatedByAnnotation

[crimsonfaith91]

Based on this announcement, CreatedByAnnotation will be deprecated in 1.8 in favor of ControllerRef. However, the annotation is still used in this repo here.

As CreatedByAnnotation will be deprecated, we may have to remove this gauge or set it to <none>. I wish to ask for feedback regarding removal of the gauge.

[zouyee]

according to the doc, Kubernetes automatically sets the value of ownerReference for objects created or adopted by ReplicationController, ReplicaSet, StatefulSet, DaemonSet, and Deployment., maybe it is time to migrate from Annotation. @brancz

[brancz]

ControllerRef is already implemented, so the labels that we have will just be populated with <none>, which is essentially free in Prometheus (2.0) as strings are symbolized. This is an easier case, as we can just keep the label for compatibility, but I can imagine some day a field will disappear that will have a broader effect. We should be prepared for those situations. As we so heavily depend on kubernetes/kubernetes changes for this I'd suggest our compatibility guarantees go along with the client-go compatibility. So once we upgrade client-go 1.8 we remove these labels.

Wdyt @fabxc @andyxning ?

[zouyee]

I think api will be used, OwnerReferences related function has been migrated.

[brancz]

Yes, we're already using that.

[zouyee]

@brancz my problem， I mean kubernetes/api will be added, because client-go structure has beed refactored if we decide to using client-go 1.8 or master

[andyxning]

As we so heavily depend on kubernetes/kubernetes changes for this I'd suggest our compatibility guarantees go along with the client-go compatibility. So once we upgrade client-go 1.8 we remove these labels.

As described in readme about the supported Kubernetes version, kube-state-metrics has only one dependency, client-go, for determining the supported Kubernetes. So, all functionalities supported by current client-go version can also be supported by kube-state-metrics.

kubernetes.io/created-by will still be available before Kubernetes 1.8. So, keep using it should be fine. When Kubernetes 1.8 is released and we bump client-go to support kubernetes 1.8, we will and should deprecate the usage of CreatedByAnnotation as otherwise kube-state-metrics can not be complied successfully.

Btw, i think we can get the created_by_kind or created_by_name label values from ControllerRef as it is a replacement for CreatedByAnnotation. It should provide all the functionalities provided by CreatedByAnnotation. As for pods created by replicationset, the Controller value should be true for one of the OwnerReference.

What i am concern is that as Kubernetes will deprecate OwnerReference,

• should kube-state-metrics delete the created_by_kind and created_by_name labels in kube_pod_info because we already have all owner reference owner_kind and owner_name info in kube_pod_owner metrics
• should we just fill created_by_kind and created_by_name labels with <none> as supposed by @crimsonfaith91
• should we extract created_by_kind and created_by_name labels from ControllerRef.

[crimsonfaith91]

@andyxning I lean towards extracting created_by_kind and created_by_name labels from ControllerRef, which has details of current controller owner(s).

[crimsonfaith91]

@andyxning No, OwnerReference will not be deprecated. Kubernetes will only deprecate CreatedByAnnotation.

To clarify, OwnerReference is the more general feature that ControllerRef is built on. A ControllerRef is just an OwnerReference with the Controller field set to true. There can be only one OwnerReference that is a ControllerRef on a given object.

[brancz]

@crimsonfaith91 yep that's how I understood everything as well.

[zouyee]

Kubernetes sets the value of ownerReference automatically. For example, when you create a ReplicaSet, Kubernetes automatically sets the ownerReference field of each Pod in the ReplicaSet. In 1.6, Kubernetes automatically sets the value of ownerReference for objects created or adopted by ReplicationController, ReplicaSet, StatefulSet, DaemonSet, and Deployment.
You can also specify relationships between owners and dependents by manually setting the ownerReference field.

[crimsonfaith91]

Once we reach consensus on how to handle created_by_kind and created_by_name labels, I can send out a PR addressing this.

[crimsonfaith91]

After having a closer look to the codebase, we may delete the created_by_kind and created_by_name labels as there is another gauge accessing OwnerReference.

[andyxning]

@crimsonfaith91 @zouyee Thanks for the clarification.

Btw, i am in favor of the third alternative, i.e., extracting created_by_kind and created_by_name labels from ControllerRef. This keep backward compatibility.

And this apparently can be done right now. :)

[brancz]

As we already fully support everything the owner reference gives us in terms of information I don't see anything we need to do here. The created_by_kind and created_by_name labels can continue to exist but will simply be filled with <none> values, this doesn't do any harm. We can explicitly deprecate them and remove them in a future release.

[andyxning]

@brancz OK, either way is fine to me. :)

If we choose to make created_by_kind and created_by_name to <none>, then we should change this in a future release when we bump client-go to support Kubernetes 1.8. @crimsonfaith91

[crimsonfaith91]

@brancz @andyxning Sound good! I will send out a PR deprecate and remove created_by_kind and created_by_name. Thanks!

[k8s-ci-robot]

@zouyee: you can't close an issue unless you authored it or you are assigned to it, Only kubernetes org members may be assigned issues..

In response to this:

/close

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.

[crimsonfaith91]

@zouyee We should keep this issue opens as it is not resolved yet.

[brancz]

Yes, let's keep it open, until we actually remove them. Thanks for your efforts @crimsonfaith91 !

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

[andyxning]

As Kubernetes 1.9.0 has finally removed the support for CreatedByAnnotation and we have update client-go to v6.0.0 in #328 which does not support CreatedByAnnotation any more, kube-state-metrics does not use CreatedByAnnotation annotation any more. So, closing this. :)