Skip to content
Permalink
Browse files

Merge pull request #81183 from RobertKrawitz/automated-cherry-pick-of…

…-#79148-upstream-release-1.14

Automated cherry pick of #79148: Do not delete an incorrect pod when replacing a mirror pod
  • Loading branch information...
k8s-ci-robot committed Sep 14, 2019
2 parents 3db80af + beedb2f commit 8fca2ec50a6133511b771a11559e24191b1aa2b4
@@ -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
}
}
}
@@ -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
}

func IsStaticPod(pod *v1.Pod) bool {
@@ -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)
}
}

@@ -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 {
@@ -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:

0 comments on commit 8fca2ec

Please sign in to comment.
You can’t perform that action at this time.