Merge pull request #14767 from nikhiljindal/podConditionTransitionTime

Auto commit by PR queue bot
pull/6/head
k8s-merge-robot 2015-10-01 13:01:45 -07:00
commit b19837f4e2
5 changed files with 155 additions and 69 deletions

View File

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

View File

@ -2512,32 +2512,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
@ -2603,7 +2622,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()

View File

@ -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)
}

View File

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

View File

@ -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())