diff --git a/pkg/api/resource_helpers.go b/pkg/api/resource_helpers.go index 0192eae028e..8a2948cf921 100644 --- a/pkg/api/resource_helpers.go +++ b/pkg/api/resource_helpers.go @@ -80,9 +80,9 @@ func IsPodReadyConditionTrue(status PodStatus) bool { // Extracts the pod ready condition from the given status and returns that. // Returns nil if the condition is not present. func GetPodReadyCondition(status PodStatus) *PodCondition { - for _, c := range status.Conditions { + for i, c := range status.Conditions { if c.Type == PodReady { - return &c + return &status.Conditions[i] } } return nil diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 851eb1a52e9..87c1b7a425a 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -2509,32 +2509,51 @@ func GetPhase(spec *api.PodSpec, info []api.ContainerStatus) api.PodPhase { } } -// Disabled LastProbeTime/LastTranitionTime for Pods to avoid constantly sending pod status -// update to the apiserver. See http://issues.k8s.io/14273. Functional revert of a PR #12894 +func readyPodCondition(isPodReady bool, reason, message string) []api.PodCondition { + condition := api.PodCondition{ + Type: api.PodReady, + } + if isPodReady { + condition.Status = api.ConditionTrue + } else { + condition.Status = api.ConditionFalse + } + condition.Reason = reason + condition.Message = message + return []api.PodCondition{condition} +} // getPodReadyCondition returns ready condition if all containers in a pod are ready, else it returns an unready condition. -func getPodReadyCondition(spec *api.PodSpec, containerStatuses []api.ContainerStatus, existingStatus *api.PodStatus) []api.PodCondition { - ready := []api.PodCondition{{ - Type: api.PodReady, - Status: api.ConditionTrue, - }} - notReady := []api.PodCondition{{ - Type: api.PodReady, - Status: api.ConditionFalse, - }} +func getPodReadyCondition(spec *api.PodSpec, containerStatuses []api.ContainerStatus) []api.PodCondition { + // Find if all containers are ready or not. if containerStatuses == nil { - return notReady + return readyPodCondition(false, "UnknownContainerStatuses", "") } + unknownContainers := []string{} + unreadyContainers := []string{} for _, container := range spec.Containers { if containerStatus, ok := api.GetContainerStatus(containerStatuses, container.Name); ok { if !containerStatus.Ready { - return notReady + unreadyContainers = append(unreadyContainers, container.Name) } } else { - return notReady + unknownContainers = append(unknownContainers, container.Name) } } - return ready + unreadyMessages := []string{} + if len(unknownContainers) > 0 { + unreadyMessages = append(unreadyMessages, fmt.Sprintf("containers with unknown status: %s", unknownContainers)) + } + if len(unreadyContainers) > 0 { + unreadyMessages = append(unreadyMessages, fmt.Sprintf("containers with unready status: %s", unreadyContainers)) + } + unreadyMessage := strings.Join(unreadyMessages, ", ") + if unreadyMessage != "" { + // return unready status. + return readyPodCondition(false, fmt.Sprint("ContainersNotReady"), unreadyMessage) + } + // return ready status. + return readyPodCondition(true, "", "") } // By passing the pod directly, this method avoids pod lookup, which requires @@ -2600,7 +2619,7 @@ func (kl *Kubelet) generatePodStatus(pod *api.Pod) (api.PodStatus, error) { } } } - podStatus.Conditions = append(podStatus.Conditions, getPodReadyCondition(spec, podStatus.ContainerStatuses, nil /* unused */)...) + podStatus.Conditions = append(podStatus.Conditions, getPodReadyCondition(spec, podStatus.ContainerStatuses)...) if !kl.standaloneMode { hostIP, err := kl.GetHostIP() diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 861285d25ed..11f16171d87 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -1755,44 +1755,30 @@ func getNotReadyStatus(cName string) api.ContainerStatus { Ready: false, } } -func getReadyCondition(status api.ConditionStatus, transitionTime unversioned.Time, reason, message string) []api.PodCondition { +func getReadyCondition(status api.ConditionStatus, reason, message string) []api.PodCondition { return []api.PodCondition{{ - Type: api.PodReady, - Status: status, + Type: api.PodReady, + Status: status, + Reason: reason, + Message: message, }} } func TestGetPodReadyCondition(t *testing.T) { - transitionTime := unversioned.Now() tests := []struct { spec *api.PodSpec containerStatuses []api.ContainerStatus - existingStatus *api.PodStatus expected []api.PodCondition - clearTimestamp bool }{ { spec: nil, containerStatuses: nil, - existingStatus: nil, - expected: getReadyCondition(api.ConditionFalse, transitionTime, "UnknownContainerStatuses", ""), - clearTimestamp: true, - }, - { - spec: nil, - containerStatuses: nil, - existingStatus: &api.PodStatus{ - Conditions: getReadyCondition(api.ConditionFalse, transitionTime, "", ""), - }, - expected: getReadyCondition(api.ConditionFalse, transitionTime, "UnknownContainerStatuses", ""), - clearTimestamp: false, + expected: getReadyCondition(api.ConditionFalse, "UnknownContainerStatuses", ""), }, { spec: &api.PodSpec{}, containerStatuses: []api.ContainerStatus{}, - existingStatus: nil, - expected: getReadyCondition(api.ConditionTrue, transitionTime, "", ""), - clearTimestamp: true, + expected: getReadyCondition(api.ConditionTrue, "", ""), }, { spec: &api.PodSpec{ @@ -1801,22 +1787,7 @@ func TestGetPodReadyCondition(t *testing.T) { }, }, containerStatuses: []api.ContainerStatus{}, - existingStatus: nil, - expected: getReadyCondition(api.ConditionFalse, transitionTime, "ContainersNotReady", "containers with unknown status: [1234]"), - clearTimestamp: true, - }, - { - spec: &api.PodSpec{ - Containers: []api.Container{ - {Name: "1234"}, - }, - }, - containerStatuses: []api.ContainerStatus{ - getReadyStatus("1234"), - }, - existingStatus: nil, - expected: getReadyCondition(api.ConditionTrue, transitionTime, "", ""), - clearTimestamp: true, + expected: getReadyCondition(api.ConditionFalse, "ContainersNotReady", "containers with unknown status: [1234]"), }, { spec: &api.PodSpec{ @@ -1829,9 +1800,7 @@ func TestGetPodReadyCondition(t *testing.T) { getReadyStatus("1234"), getReadyStatus("5678"), }, - existingStatus: nil, - expected: getReadyCondition(api.ConditionTrue, transitionTime, "", ""), - clearTimestamp: true, + expected: getReadyCondition(api.ConditionTrue, "", ""), }, { spec: &api.PodSpec{ @@ -1843,9 +1812,7 @@ func TestGetPodReadyCondition(t *testing.T) { containerStatuses: []api.ContainerStatus{ getReadyStatus("1234"), }, - existingStatus: nil, - expected: getReadyCondition(api.ConditionFalse, transitionTime, "ContainersNotReady", "containers with unknown status: [5678]"), - clearTimestamp: true, + expected: getReadyCondition(api.ConditionFalse, "ContainersNotReady", "containers with unknown status: [5678]"), }, { spec: &api.PodSpec{ @@ -1858,18 +1825,12 @@ func TestGetPodReadyCondition(t *testing.T) { getReadyStatus("1234"), getNotReadyStatus("5678"), }, - expected: getReadyCondition(api.ConditionFalse, transitionTime, "ContainersNotReady", "containers with unready status: [5678]"), - clearTimestamp: true, + expected: getReadyCondition(api.ConditionFalse, "ContainersNotReady", "containers with unready status: [5678]"), }, } for i, test := range tests { - condition := getPodReadyCondition(test.spec, test.containerStatuses, test.existingStatus) - if test.clearTimestamp { - condition[0].LastTransitionTime = transitionTime - test.expected[0].LastTransitionTime = transitionTime - } - condition[0].LastProbeTime = unversioned.Time{} + condition := getPodReadyCondition(test.spec, test.containerStatuses) if !reflect.DeepEqual(condition, test.expected) { t.Errorf("On test case %v, expected:\n%+v\ngot\n%+v\n", i, test.expected, condition) } diff --git a/pkg/kubelet/status/manager.go b/pkg/kubelet/status/manager.go index e174316f52c..6d3f3d75e6a 100644 --- a/pkg/kubelet/status/manager.go +++ b/pkg/kubelet/status/manager.go @@ -127,6 +127,21 @@ func (m *manager) SetPodStatus(pod *api.Pod, status api.PodStatus) { status.StartTime = oldStatus.StartTime } + // Set ReadyCondition.LastTransitionTime. + // Note we cannot do this while generating the status since we do not have oldStatus + // at that time for mirror pods. + if readyCondition := api.GetPodReadyCondition(status); readyCondition != nil { + // Need to set LastTransitionTime. + lastTransitionTime := unversioned.Now() + if found { + oldReadyCondition := api.GetPodReadyCondition(oldStatus) + if oldReadyCondition != nil && readyCondition.Status == oldReadyCondition.Status { + lastTransitionTime = oldReadyCondition.LastTransitionTime + } + } + readyCondition.LastTransitionTime = lastTransitionTime + } + // if the status has no start time, we need to set an initial time // TODO(yujuhong): Consider setting StartTime when generating the pod // status instead, which would allow manager to become a simple cache diff --git a/pkg/kubelet/status/manager_test.go b/pkg/kubelet/status/manager_test.go index 8de4b796146..c1b75d866a1 100644 --- a/pkg/kubelet/status/manager_test.go +++ b/pkg/kubelet/status/manager_test.go @@ -120,6 +120,37 @@ func TestNewStatusPreservesPodStartTime(t *testing.T) { } } +func getReadyPodStatus() api.PodStatus { + return api.PodStatus{ + Conditions: []api.PodCondition{ + { + Type: api.PodReady, + Status: api.ConditionTrue, + }, + }, + } +} + +func TestNewStatusSetsReadyTransitionTime(t *testing.T) { + syncer := newTestManager() + podStatus := getReadyPodStatus() + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{ + UID: "12345678", + Name: "foo", + Namespace: "new", + }, + Status: api.PodStatus{}, + } + syncer.SetPodStatus(pod, podStatus) + verifyUpdates(t, syncer, 1) + status, _ := syncer.GetPodStatus(pod.UID) + readyCondition := api.GetPodReadyCondition(status) + if readyCondition.LastTransitionTime.IsZero() { + t.Errorf("Unexpected: last transition time not set") + } +} + func TestChangedStatus(t *testing.T) { syncer := newTestManager() syncer.SetPodStatus(testPod, getRandomPodStatus()) @@ -144,6 +175,36 @@ func TestChangedStatusKeepsStartTime(t *testing.T) { } } +func TestChangedStatusUpdatesLastTransitionTime(t *testing.T) { + syncer := newTestManager() + podStatus := getReadyPodStatus() + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{ + UID: "12345678", + Name: "foo", + Namespace: "new", + }, + Status: api.PodStatus{}, + } + syncer.SetPodStatus(pod, podStatus) + verifyUpdates(t, syncer, 1) + oldStatus, _ := syncer.GetPodStatus(pod.UID) + anotherStatus := getReadyPodStatus() + anotherStatus.Conditions[0].Status = api.ConditionFalse + syncer.SetPodStatus(pod, anotherStatus) + verifyUpdates(t, syncer, 1) + newStatus, _ := syncer.GetPodStatus(pod.UID) + + oldReadyCondition := api.GetPodReadyCondition(oldStatus) + newReadyCondition := api.GetPodReadyCondition(newStatus) + if newReadyCondition.LastTransitionTime.IsZero() { + t.Errorf("Unexpected: last transition time not set") + } + if !oldReadyCondition.LastTransitionTime.Before(newReadyCondition.LastTransitionTime) { + t.Errorf("Unexpected: new transition time %s, is not after old transition time %s", newReadyCondition.LastTransitionTime, oldReadyCondition.LastTransitionTime) + } +} + func TestUnchangedStatus(t *testing.T) { syncer := newTestManager() podStatus := getRandomPodStatus() @@ -152,6 +213,36 @@ func TestUnchangedStatus(t *testing.T) { verifyUpdates(t, syncer, 1) } +func TestUnchangedStatusPreservesLastTransitionTime(t *testing.T) { + syncer := newTestManager() + podStatus := getReadyPodStatus() + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{ + UID: "12345678", + Name: "foo", + Namespace: "new", + }, + Status: api.PodStatus{}, + } + syncer.SetPodStatus(pod, podStatus) + verifyUpdates(t, syncer, 1) + oldStatus, _ := syncer.GetPodStatus(pod.UID) + anotherStatus := getReadyPodStatus() + syncer.SetPodStatus(pod, anotherStatus) + // No update. + verifyUpdates(t, syncer, 0) + newStatus, _ := syncer.GetPodStatus(pod.UID) + + oldReadyCondition := api.GetPodReadyCondition(oldStatus) + newReadyCondition := api.GetPodReadyCondition(newStatus) + if newReadyCondition.LastTransitionTime.IsZero() { + t.Errorf("Unexpected: last transition time not set") + } + if !oldReadyCondition.LastTransitionTime.Equal(newReadyCondition.LastTransitionTime) { + t.Errorf("Unexpected: new transition time %s, is not equal to old transition time %s", newReadyCondition.LastTransitionTime, oldReadyCondition.LastTransitionTime) + } +} + func TestSyncBatchIgnoresNotFound(t *testing.T) { syncer := newTestManager() syncer.SetPodStatus(testPod, getRandomPodStatus())