diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index c41098b94a8..70be042df22 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -2001,19 +2001,22 @@ func (kl *Kubelet) syncLoopIteration(configCh <-chan kubetypes.PodUpdate, handle } // dispatchWork starts the asynchronous sync of the pod in a pod worker. -// If the pod is terminated, dispatchWork will perform no action. +// If the pod has completed termination, dispatchWork will perform no action. func (kl *Kubelet) dispatchWork(pod *v1.Pod, syncType kubetypes.SyncPodType, mirrorPod *v1.Pod, start time.Time) { - if kl.podIsTerminated(pod) { - klog.V(4).Infof("Pod %q is terminated, ignoring remaining sync work: %s", format.Pod(pod), syncType) - if pod.DeletionTimestamp != nil { - // If the pod is in a terminated state, there is no pod worker to - // handle the work item. Check if the DeletionTimestamp has been - // set, and force a status update to trigger a pod deletion request - // to the apiserver. - kl.statusManager.TerminatePod(pod) - } + // check whether we are ready to delete the pod from the API server (all status up to date) + containersTerminal, podWorkerTerminal := kl.podAndContainersAreTerminal(pod) + if pod.DeletionTimestamp != nil && containersTerminal { + klog.V(4).Infof("Pod %q has completed execution and should be deleted from the API server: %s", format.Pod(pod), syncType) + kl.statusManager.TerminatePod(pod) return } + + // optimization: avoid invoking the pod worker if no further changes are possible to the pod definition + if podWorkerTerminal { + klog.V(4).Infof("Pod %q has completed, ignoring remaining sync work: %s", format.Pod(pod), syncType) + return + } + // Run the sync in an async worker. kl.podWorkers.UpdatePod(&UpdatePodOptions{ Pod: pod, diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 778050ee25b..99e35f32db0 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -865,8 +865,9 @@ func (kl *Kubelet) getPullSecretsForPod(pod *v1.Pod) []v1.Secret { return pullSecrets } -// podIsTerminated returns true if pod is in the terminated state ("Failed" or "Succeeded"). -func (kl *Kubelet) podIsTerminated(pod *v1.Pod) bool { +// podStatusIsTerminal reports when the specified pod has no running containers or is no longer accepting +// spec changes. +func (kl *Kubelet) podAndContainersAreTerminal(pod *v1.Pod) (containersTerminal, podWorkerTerminal bool) { // Check the cached pod status which was set after the last sync. status, ok := kl.statusManager.GetPodStatus(pod.UID) if !ok { @@ -875,11 +876,28 @@ func (kl *Kubelet) podIsTerminated(pod *v1.Pod) bool { // restarted. status = pod.Status } - return status.Phase == v1.PodFailed || status.Phase == v1.PodSucceeded || (pod.DeletionTimestamp != nil && notRunning(status.ContainerStatuses)) + // A pod transitions into failed or succeeded from either container lifecycle (RestartNever container + // fails) or due to external events like deletion or eviction. A terminal pod *should* have no running + // containers, but to know that the pod has completed its lifecycle you must wait for containers to also + // be terminal. + containersTerminal = notRunning(status.ContainerStatuses) + // The kubelet must accept config changes from the pod spec until it has reached a point where changes would + // have no effect on any running container. + podWorkerTerminal = status.Phase == v1.PodFailed || status.Phase == v1.PodSucceeded || (pod.DeletionTimestamp != nil && containersTerminal) + return } -// IsPodTerminated returns true if the pod with the provided UID is in a terminated state ("Failed" or "Succeeded") -// or if the pod has been deleted or removed +// podIsTerminated returns true if the provided pod is in a terminal phase ("Failed", "Succeeded") or +// has been deleted and has no running containers. This corresponds to when a pod must accept changes to +// its pod spec (e.g. terminating containers allow grace period to be shortened). +func (kl *Kubelet) podIsTerminated(pod *v1.Pod) bool { + _, podWorkerTerminal := kl.podAndContainersAreTerminal(pod) + return podWorkerTerminal +} + +// IsPodTerminated returns true if the pod with the provided UID is in a terminal phase ("Failed", +// "Succeeded") or has been deleted and has no running containers. This corresponds to when a pod must +// accept changes to its pod spec (e.g. terminating containers allow grace period to be shortened) func (kl *Kubelet) IsPodTerminated(uid types.UID) bool { pod, podFound := kl.podManager.GetPodByUID(uid) if !podFound {