Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kube-proxy show up as single pod #2931

Closed
rade opened this issue Nov 11, 2017 · 9 comments
Closed

kube-proxy show up as single pod #2931

rade opened this issue Nov 11, 2017 · 9 comments
Labels
accuracy Incorrect information is being shown to the user; usually a bug bug Broken end user or developer functionality; not working as the developers intended it k8s Pertains to integration with Kubernetes

Comments

@rade
Copy link
Member

rade commented Nov 11, 2017

The kube-proxy shows up as a single pod
screenshot from 2017-11-11 08 34 10
even though there is one running on each host

# kubectl get pods -n kube-system
NAME                                                  READY     STATUS    RESTARTS   AGE
...
kube-proxy-gke-sock-shop-default-pool-1cc5ad4b-6cf5   1/1       Running   0          1d
kube-proxy-gke-sock-shop-default-pool-1cc5ad4b-d4m6   1/1       Running   0          1d
kube-proxy-gke-sock-shop-default-pool-1cc5ad4b-wvh0   1/1       Running   0          1d
...

On closer inspection, this issue also shows for etcd, kube-scheduler, kube-api-server, kube-controller-manager.

The underlying problem is that we identify certain system pods by their kubernetes.io/config.hash annotation, which turns out to be the same on all hosts, and hence scope will merge all the information into a single node (hence we see multiple kube-proxy containers and processes in the above).

We use that value instead of the usual metadata.uid field because for these system pods the io.kubernetes.pod.uid container label we use for determining which pod a container belongs to, contains that value. See #1412 for when this special case was introduced.

@rade rade added bug Broken end user or developer functionality; not working as the developers intended it k8s Pertains to integration with Kubernetes labels Nov 11, 2017
@rade
Copy link
Member Author

rade commented Nov 11, 2017

There is a related problem here: We may pay a (small, in the grand scheme of things) performance penalty because under some circumstance the probes will be unable to determine which host these system pods run on, which leads these pods to be reported by all probes. That's because one path through that logic identifies local pods by their metadata.uid as returned by the local kubelet.

@rade
Copy link
Member Author

rade commented Nov 11, 2017

There is just one place in the kubernetes code base which sets the kubernetes.io/config.hash annotation. When the config comes from a file, the node name is mixed into the hash, yielding unique IDs, but when it comes from a URL it isn't, yielding IDs that are the same on all nodes.

That looks like a bug.

What I haven't worked out yet is what subsequently replaces the pod's UID, since by the time we get hold of it from the apiserver, it has a regular UID, which might explain why the problem has gone unnoticed for so long, since it only manifest in places where the originally assigned ID is used, such as the io.kubernetes.pod.uid container label pertaining to our issue here.

@rade
Copy link
Member Author

rade commented Nov 11, 2017

When the config comes from a file, the node name is mixed into the hash, yielding unique IDs, but when it comes from a URL it isn't, yielding IDs that are the same on all nodes.

The implication of us seeing the problem across the three cluster's I've looked at, is that kubelet on these clusters obtained the manifest from a URL rather than a file. But that's not the case - I see a --pod-manifest-path argument and no --manifest-url argument.

@rade
Copy link
Member Author

rade commented Nov 11, 2017

Found it. The hash is created as follows:

hasher := md5.New()
if isFile {
	fmt.Fprintf(hasher, "host:%s", nodeName)
	fmt.Fprintf(hasher, "file:%s", source)
} else {
	fmt.Fprintf(hasher, "url:%s", source)
}
hash.DeepHashObject(hasher, pod)
pod.UID = types.UID(hex.EncodeToString(hasher.Sum(nil)[0:]))

Alas, hash.DeepHashObject() calls hasher.Reset, and consequently only pod contributes to the hash; nodeName and source are ignored.

@rade
Copy link
Member Author

rade commented Nov 11, 2017

There is one other call site to DeepHashObject that is affected by this; the others all operate on fresh hashers.

Seems to me that the the Reset() should be removed. It was introduced in kubernetes/kubernetes#5952 for unknown reasons.

And the config hash code should be updated to always mix in the node name.

@rade rade added the accuracy Incorrect information is being shown to the user; usually a bug label Nov 12, 2017
@xiangpengzhao
Copy link

@rade thanks for reporting and digging into this! I send a PR kubernetes/kubernetes#57135 to the upstream. Please take a look :)

@xiangpengzhao
Copy link

I'm not sure why Reset() was added, so I don't remove it in my PR for safety concern.
@thockin I see you sent kubernetes/kubernetes#5952 to add Reset(). Can you explain a bit here? Thanks!

@rade
Copy link
Member Author

rade commented Dec 13, 2017

@xiangpengzhao Thanks. PR looks good with the caveat you noted, i.e. it's preserving a somewhat questionable API.

@bboreham
Copy link
Collaborator

bboreham commented Dec 3, 2020

The fix has at last merged in Kubernetes kubernetes/kubernetes#87461

(Nowadays kube-proxy is typically installed as a DaemonSet rather than static pods, which means the original symptom is rarely seen)

@bboreham bboreham closed this as completed Dec 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accuracy Incorrect information is being shown to the user; usually a bug bug Broken end user or developer functionality; not working as the developers intended it k8s Pertains to integration with Kubernetes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants