diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index f63915a0872..293fe1a4f96 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1621,11 +1621,13 @@ func (kl *Kubelet) syncPod(o syncPodOptions) error { if mirrorPod.DeletionTimestamp != nil || !kl.podManager.IsMirrorPodOf(mirrorPod, pod) { // The mirror pod is semantically different from the static pod. Remove // it. The mirror pod will get recreated later. - klog.Warningf("Deleting mirror pod %q because it is outdated", format.Pod(mirrorPod)) - if err := kl.podManager.DeleteMirrorPod(podFullName); err != nil { + klog.Infof("Trying to delete pod %s %v", podFullName, mirrorPod.ObjectMeta.UID) + var err error + deleted, err = kl.podManager.DeleteMirrorPod(podFullName, &mirrorPod.ObjectMeta.UID) + if deleted { + klog.Warningf("Deleted mirror pod %q because it is outdated", format.Pod(mirrorPod)) + } else if err != nil { klog.Errorf("Failed deleting mirror pod %q: %v", format.Pod(mirrorPod), err) - } else { - deleted = true } } } diff --git a/pkg/kubelet/pod/mirror_client.go b/pkg/kubelet/pod/mirror_client.go index b90254b6a25..79d8cecd01c 100644 --- a/pkg/kubelet/pod/mirror_client.go +++ b/pkg/kubelet/pod/mirror_client.go @@ -20,6 +20,7 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" clientset "k8s.io/client-go/kubernetes" "k8s.io/klog" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" @@ -35,7 +36,7 @@ type MirrorClient interface { CreateMirrorPod(pod *v1.Pod) error // DeleteMirrorPod deletes the mirror pod with the given full name from // the API server or returns an error. - DeleteMirrorPod(podFullName string) error + DeleteMirrorPod(podFullName string, uid *types.UID) (bool, error) } // basicMirrorClient is a functional MirrorClient. Mirror pods are stored in @@ -73,21 +74,35 @@ func (mc *basicMirrorClient) CreateMirrorPod(pod *v1.Pod) error { return err } -func (mc *basicMirrorClient) DeleteMirrorPod(podFullName string) error { +// DeleteMirrorPod deletes a mirror pod. +// It takes the full name of the pod and optionally a UID. If the UID +// is non-nil, the pod is deleted only if its UID matches the supplied UID. +// It returns whether the pod was actually deleted, and any error returned +// while parsing the name of the pod. +// Non-existence of the pod or UID mismatch is not treated as an error; the +// routine simply returns false in that case. +func (mc *basicMirrorClient) DeleteMirrorPod(podFullName string, uid *types.UID) (bool, error) { if mc.apiserverClient == nil { - return nil + return false, nil } name, namespace, err := kubecontainer.ParsePodFullName(podFullName) if err != nil { klog.Errorf("Failed to parse a pod full name %q", podFullName) - return err + return false, err } - klog.V(2).Infof("Deleting a mirror pod %q", podFullName) - // TODO(random-liu): Delete the mirror pod with uid precondition in mirror pod manager - if err := mc.apiserverClient.CoreV1().Pods(namespace).Delete(name, metav1.NewDeleteOptions(0)); err != nil && !errors.IsNotFound(err) { - klog.Errorf("Failed deleting a mirror pod %q: %v", podFullName, err) + klog.V(2).Infof("Deleting a mirror pod %q (uid %#v)", podFullName, uid) + var GracePeriodSeconds int64 + GracePeriodSeconds = 0 + if err := mc.apiserverClient.CoreV1().Pods(namespace).Delete(name, &metav1.DeleteOptions{GracePeriodSeconds: &GracePeriodSeconds, Preconditions: &metav1.Preconditions{UID: uid}}); err != nil { + // Unfortunately, there's no generic error for failing a precondition + if !(errors.IsNotFound(err) || errors.IsConflict(err)) { + // We should return the error here, but historically this routine does + // not return an error unless it can't parse the pod name + klog.Errorf("Failed deleting a mirror pod %q: %v", podFullName, err) + } + return false, nil } - return nil + return true, nil } // IsStaticPod returns true if the pod is a static pod. diff --git a/pkg/kubelet/pod/pod_manager.go b/pkg/kubelet/pod/pod_manager.go index 17f54184b16..9a67f0ff2c5 100644 --- a/pkg/kubelet/pod/pod_manager.go +++ b/pkg/kubelet/pod/pod_manager.go @@ -338,7 +338,7 @@ func (pm *basicManager) getOrphanedMirrorPodNames() []string { func (pm *basicManager) DeleteOrphanedMirrorPods() { podFullNames := pm.getOrphanedMirrorPodNames() for _, podFullName := range podFullNames { - pm.MirrorClient.DeleteMirrorPod(podFullName) + pm.MirrorClient.DeleteMirrorPod(podFullName, nil) } } diff --git a/pkg/kubelet/pod/testing/fake_mirror_client.go b/pkg/kubelet/pod/testing/fake_mirror_client.go index a36a80a2c94..e9515e6aa55 100644 --- a/pkg/kubelet/pod/testing/fake_mirror_client.go +++ b/pkg/kubelet/pod/testing/fake_mirror_client.go @@ -20,6 +20,7 @@ import ( "sync" "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" cp "k8s.io/kubernetes/pkg/kubelet/checkpoint" "k8s.io/kubernetes/pkg/kubelet/checkpointmanager" @@ -52,12 +53,13 @@ func (fmc *FakeMirrorClient) CreateMirrorPod(pod *v1.Pod) error { return nil } -func (fmc *FakeMirrorClient) DeleteMirrorPod(podFullName string) error { +// TODO (Robert Krawitz): Implement UID checking +func (fmc *FakeMirrorClient) DeleteMirrorPod(podFullName string, _ *types.UID) (bool, error) { fmc.mirrorPodLock.Lock() defer fmc.mirrorPodLock.Unlock() fmc.mirrorPods.Delete(podFullName) fmc.deleteCounts[podFullName]++ - return nil + return true, nil } func (fmc *FakeMirrorClient) HasPod(podFullName string) bool { diff --git a/pkg/kubelet/pod/testing/mock_manager.go b/pkg/kubelet/pod/testing/mock_manager.go index f15845b79ff..32051659410 100644 --- a/pkg/kubelet/pod/testing/mock_manager.go +++ b/pkg/kubelet/pod/testing/mock_manager.go @@ -48,17 +48,17 @@ func (_m *MockManager) CreateMirrorPod(_a0 *v1.Pod) error { } // DeleteMirrorPod provides a mock function with given fields: podFullName -func (_m *MockManager) DeleteMirrorPod(podFullName string) error { +func (_m *MockManager) DeleteMirrorPod(podFullName string, _ *types.UID) (bool, error) { ret := _m.Called(podFullName) var r0 error if rf, ok := ret.Get(0).(func(string) error); ok { - r0 = rf(podFullName) + return true, rf(podFullName) } else { r0 = ret.Error(0) } - return r0 + return false, r0 } // DeleteOrphanedMirrorPods provides a mock function with given fields: