Fix kubectl drain error handling bug.

Fixed a bug where kubectl drain would consider a pod as having been deleted if an error occurs while calling the API.
This commit is contained in:
Brian Pursley 2024-01-04 08:53:57 -05:00
parent cacdf6c707
commit 4aecb151b8
2 changed files with 27 additions and 8 deletions

View File

@ -417,7 +417,9 @@ func waitForDelete(params waitForDeleteParams) ([]corev1.Pod, error) {
pendingPods := []corev1.Pod{}
for i, pod := range pods {
p, err := params.getPodFn(pod.Namespace, pod.Name)
if apierrors.IsNotFound(err) || (p != nil && p.ObjectMeta.UID != pod.ObjectMeta.UID) {
// The implementation of getPodFn that uses client-go returns an empty Pod struct when there is an error,
// so we need to check that err == nil and p != nil to know that a pod was found successfully.
if apierrors.IsNotFound(err) || (err == nil && p != nil && p.ObjectMeta.UID != pod.ObjectMeta.UID) {
if params.onFinishFn != nil {
params.onFinishFn(&pod, params.usingEviction, nil)
} else if params.onDoneFn != nil {

View File

@ -72,10 +72,27 @@ func TestDeletePods(t *testing.T) {
newPod := newPodMap[name]
return &newPod, nil
}
return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "pods"}, name)
return &corev1.Pod{}, apierrors.NewNotFound(schema.GroupResource{Resource: "pods"}, name)
}
return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "pods"}, name)
return &corev1.Pod{}, apierrors.NewNotFound(schema.GroupResource{Resource: "pods"}, name)
},
},
{
description: "Pod found with same name but different UID",
interval: 100 * time.Millisecond,
timeout: 10 * time.Second,
expectPendingPods: false,
expectError: false,
expectedError: nil,
getPodFn: func(namespace, name string) (*corev1.Pod, error) {
// Return a pod with the same name, but different UID
return &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Namespace: namespace,
Name: name,
UID: "SOME_OTHER_UID",
},
}, nil
},
},
{
@ -90,7 +107,7 @@ func TestDeletePods(t *testing.T) {
if oldPod, found := oldPodMap[name]; found {
return &oldPod, nil
}
return nil, fmt.Errorf("%q: not found", name)
return &corev1.Pod{}, fmt.Errorf("%q: not found", name)
},
},
{
@ -106,7 +123,7 @@ func TestDeletePods(t *testing.T) {
if oldPod, found := oldPodMap[name]; found {
return &oldPod, nil
}
return nil, fmt.Errorf("%q: not found", name)
return &corev1.Pod{}, fmt.Errorf("%q: not found", name)
},
},
{
@ -123,7 +140,7 @@ func TestDeletePods(t *testing.T) {
oldPod.ObjectMeta.SetDeletionTimestamp(dTime)
return &oldPod, nil
}
return nil, fmt.Errorf("%q: not found", name)
return &corev1.Pod{}, fmt.Errorf("%q: not found", name)
},
},
{
@ -134,7 +151,7 @@ func TestDeletePods(t *testing.T) {
expectError: true,
expectedError: nil,
getPodFn: func(namespace, name string) (*corev1.Pod, error) {
return nil, errors.New("This is a random error for testing")
return &corev1.Pod{}, errors.New("This is a random error for testing")
},
},
}