From 19a1bd8b99d1dd9ec34d1214f5ae2dc843c4c6c4 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Wed, 21 Mar 2018 23:33:44 +0000 Subject: [PATCH] Fix `PodScheduled` bug for static pod. Signed-off-by: Lantao Liu --- pkg/kubelet/kubelet_pods.go | 11 ++-- pkg/kubelet/status/status_manager.go | 38 +++++++------- pkg/kubelet/status/status_manager_test.go | 61 +++++++++++++++++++++++ 3 files changed, 85 insertions(+), 25 deletions(-) diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 2712448cf81..91565c21962 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -1394,13 +1394,10 @@ func (kl *Kubelet) generateAPIPodStatus(pod *v1.Pod, podStatus *kubecontainer.Po kl.probeManager.UpdatePodStatus(pod.UID, s) s.Conditions = append(s.Conditions, status.GeneratePodInitializedCondition(spec, s.InitContainerStatuses, s.Phase)) s.Conditions = append(s.Conditions, status.GeneratePodReadyCondition(spec, s.ContainerStatuses, s.Phase)) - // s (the PodStatus we are creating) will not have a PodScheduled condition yet, because converStatusToAPIStatus() - // does not create one. If the existing PodStatus has a PodScheduled condition, then copy it into s and make sure - // it is set to true. If the existing PodStatus does not have a PodScheduled condition, then create one that is set to true. - if _, oldPodScheduled := podutil.GetPodCondition(&pod.Status, v1.PodScheduled); oldPodScheduled != nil { - s.Conditions = append(s.Conditions, *oldPodScheduled) - } - podutil.UpdatePodCondition(s, &v1.PodCondition{ + // Status manager will take care of the LastTransitionTimestamp, either preserve + // the timestamp from apiserver, or set a new one. When kubelet sees the pod, + // `PodScheduled` condition must be true. + s.Conditions = append(s.Conditions, v1.PodCondition{ Type: v1.PodScheduled, Status: v1.ConditionTrue, }) diff --git a/pkg/kubelet/status/status_manager.go b/pkg/kubelet/status/status_manager.go index d9a027d544e..eaf5b9a0512 100644 --- a/pkg/kubelet/status/status_manager.go +++ b/pkg/kubelet/status/status_manager.go @@ -316,26 +316,13 @@ func (m *manager) updateStatusInternal(pod *v1.Pod, status v1.PodStatus, forceUp } // Set ReadyCondition.LastTransitionTime. - if _, readyCondition := podutil.GetPodCondition(&status, v1.PodReady); readyCondition != nil { - // Need to set LastTransitionTime. - lastTransitionTime := metav1.Now() - _, oldReadyCondition := podutil.GetPodCondition(&oldStatus, v1.PodReady) - if oldReadyCondition != nil && readyCondition.Status == oldReadyCondition.Status { - lastTransitionTime = oldReadyCondition.LastTransitionTime - } - readyCondition.LastTransitionTime = lastTransitionTime - } + updateLastTransitionTime(&status, &oldStatus, v1.PodReady) // Set InitializedCondition.LastTransitionTime. - if _, initCondition := podutil.GetPodCondition(&status, v1.PodInitialized); initCondition != nil { - // Need to set LastTransitionTime. - lastTransitionTime := metav1.Now() - _, oldInitCondition := podutil.GetPodCondition(&oldStatus, v1.PodInitialized) - if oldInitCondition != nil && initCondition.Status == oldInitCondition.Status { - lastTransitionTime = oldInitCondition.LastTransitionTime - } - initCondition.LastTransitionTime = lastTransitionTime - } + updateLastTransitionTime(&status, &oldStatus, v1.PodInitialized) + + // Set PodScheduledCondition.LastTransitionTime. + updateLastTransitionTime(&status, &oldStatus, v1.PodScheduled) // ensure that the start time does not change across updates. if oldStatus.StartTime != nil && !oldStatus.StartTime.IsZero() { @@ -376,6 +363,21 @@ func (m *manager) updateStatusInternal(pod *v1.Pod, status v1.PodStatus, forceUp } } +// updateLastTransitionTime updates the LastTransitionTime of a pod condition. +func updateLastTransitionTime(status, oldStatus *v1.PodStatus, conditionType v1.PodConditionType) { + _, condition := podutil.GetPodCondition(status, conditionType) + if condition == nil { + return + } + // Need to set LastTransitionTime. + lastTransitionTime := metav1.Now() + _, oldCondition := podutil.GetPodCondition(oldStatus, conditionType) + if oldCondition != nil && condition.Status == oldCondition.Status { + lastTransitionTime = oldCondition.LastTransitionTime + } + condition.LastTransitionTime = lastTransitionTime +} + // deletePodStatus simply removes the given pod from the status cache. func (m *manager) deletePodStatus(uid types.UID) { m.podStatusesLock.Lock() diff --git a/pkg/kubelet/status/status_manager_test.go b/pkg/kubelet/status/status_manager_test.go index 1d332b221fc..6e68b20d5ed 100644 --- a/pkg/kubelet/status/status_manager_test.go +++ b/pkg/kubelet/status/status_manager_test.go @@ -798,6 +798,67 @@ func TestDoNotDeleteMirrorPods(t *testing.T) { verifyActions(t, m, []core.Action{getAction(), updateAction()}) } +func TestUpdateLastTransitionTime(t *testing.T) { + old := metav1.Now() + for desc, test := range map[string]struct { + condition *v1.PodCondition + oldCondition *v1.PodCondition + expectUpdate bool + }{ + "should do nothing if no corresponding condition": { + expectUpdate: false, + }, + "should update last transition time if no old condition": { + condition: &v1.PodCondition{ + Type: "test-type", + Status: v1.ConditionTrue, + }, + oldCondition: nil, + expectUpdate: true, + }, + "should update last transition time if condition is changed": { + condition: &v1.PodCondition{ + Type: "test-type", + Status: v1.ConditionTrue, + }, + oldCondition: &v1.PodCondition{ + Type: "test-type", + Status: v1.ConditionFalse, + LastTransitionTime: old, + }, + expectUpdate: true, + }, + "should keep last transition time if condition is not changed": { + condition: &v1.PodCondition{ + Type: "test-type", + Status: v1.ConditionFalse, + }, + oldCondition: &v1.PodCondition{ + Type: "test-type", + Status: v1.ConditionFalse, + LastTransitionTime: old, + }, + expectUpdate: false, + }, + } { + t.Logf("TestCase %q", desc) + status := &v1.PodStatus{} + oldStatus := &v1.PodStatus{} + if test.condition != nil { + status.Conditions = []v1.PodCondition{*test.condition} + } + if test.oldCondition != nil { + oldStatus.Conditions = []v1.PodCondition{*test.oldCondition} + } + updateLastTransitionTime(status, oldStatus, "test-type") + if test.expectUpdate { + assert.True(t, status.Conditions[0].LastTransitionTime.After(old.Time)) + } else if test.condition != nil { + assert.Equal(t, old, status.Conditions[0].LastTransitionTime) + } + } +} + func getAction() core.GetAction { return core.GetActionImpl{ActionImpl: core.ActionImpl{Verb: "get", Resource: schema.GroupVersionResource{Resource: "pods"}}} }