search

coreos / container-linux-update-operator

A Kubernetes operator to manage updates of Container Linux by CoreOS
Apache License 2.0
209 stars 49 forks source link

CLUO uses pod name during deletion checks #149

Closed diegs closed 7 years ago

diegs commented 7 years ago

The CLUO only checks the pod name for eviction checks. This is unsound, as pod names are not guaranteed to be unique or tied to a specific node.

https://github.com/coreos/container-linux-update-operator/blob/master/pkg/agent/agent.go#L362

It also serializes these checks for all pods where really they should be concurrent.

euank commented 7 years ago

Note that logic was taken from kubectl drain: https://github.com/kubernetes/kubernetes/blob/v1.8.1/pkg/kubectl/cmd/drain.go#L565-L585 (and https://github.com/kubernetes/kubernetes/blob/v1.8.1/pkg/kubectl/cmd/drain.go#L475-L477)

I believe kubectl drain will have a similar problem.

Yet more evidence drain should be a server-side operation IMO.

dghubble commented 7 years ago

Context: Specifically this is to address etcd-operator and prometheus-operator which always create pods with the same name, even if CLUO deletes them.

diegs commented 7 years ago

This is generally allowed though, as the Kubernetes documentation states[1] (my emphasis added):

A given pod (as defined by a UID) is not “rescheduled” to a new node; instead, it can be replaced by an identical pod, with even the same name if desired, but with a new UID (see replication controller for more details).

  1. https://kubernetes.io/docs/concepts/workloads/pods/pod/#what-is-a-pod
euank commented 7 years ago

You should also check if there's a bug upstream already and, if not, file one.

Edit, nvm, I missed that upstream is fine here. Sorry for the noise!