kubectl drain ignores terminating pods when reboot master node

[LuckyChen666]

What happened:
When I evict and update one of the worker nodes, a master node is also rebooting at this time. At this time, the evicted pod needs to wait for terminationGracePeriodSeconds before the eviction is successful.
Then something strange happened, the eviction was successful, and the currently terminating pod was ignored.

kubectl/pkg/drain/drain.go

Line 420 in b359351

if apierrors.IsNotFound(err) || (p != nil && p.ObjectMeta.UID != pod.ObjectMeta.UID) {

I found that because the master node was rebooting at this time, obtaining the pod's interface failed. At this time, the above judgment p != nil && p.ObjectMeta.UID != pod.ObjectMeta.UID was met, so the current pod was ignored.

What you expected to happen:
Judgment here should be strictly controlled，such as：

if apierrors.IsNotFound(err)  {
  if (p != nil && p.ObjectMeta.UID != pod.ObjectMeta.UID) {
     ...
  }else{
   ...
  }
} else {
...
}

Maybe there is a better way to fix it.
How to reproduce it (as minimally and precisely as possible):

You can try to drain a worker node which owns terminating pod for a long time. Then reboot master node.

[eddiezane]

I am assuming that if the client-go Pod Get function is returning an err that p is nil by Go convention. If that is the case then I believe the current logic is correct?

@LuckyChen666 thoughts?

[eddiezane]

Actually no that assumption is incorrect. client go's getters use named returns and do alloc a struct.

So in this case p is never nil so our current logic is incorrect.

https://github.com/kubernetes/kubernetes/blob/cacdf6c70728086bfcab387b901d9765cbcbfb6b/staging/src/k8s.io/client-go/kubernetes/typed/core/v1/pod.go#L76

/triage accepted

[eddiezane]

/assign @brianpursley