From 3dc7c64ce9f948e51c0940ae7177f77b8dabb6ce Mon Sep 17 00:00:00 2001 From: Aaron Levy Date: Tue, 17 Nov 2015 13:05:53 -0800 Subject: [PATCH 1/2] kubelet: report NodeReady last in status list Addresses a version skew issue where the last condition status is always evaluated as the NodeReady status. As a workaround force the NodeReady condition to be the last in the list of node conditions. ref: https://github.com/kubernetes/kubernetes/issues/16961 --- pkg/kubelet/kubelet.go | 95 +++++++++++++++++---------------- pkg/kubelet/kubelet_test.go | 101 ++++++++++++++++++++++-------------- 2 files changed, 110 insertions(+), 86 deletions(-) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 61cb2a746ce..8013c5fe878 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -2773,52 +2773,6 @@ func (kl *Kubelet) setNodeStatus(node *api.Node) error { node.Status.DaemonEndpoints = *kl.daemonEndpoints currentTime := unversioned.Now() - var newNodeReadyCondition api.NodeCondition - var oldNodeReadyConditionStatus api.ConditionStatus - if rs := kl.runtimeState.errors(); len(rs) == 0 { - newNodeReadyCondition = api.NodeCondition{ - Type: api.NodeReady, - Status: api.ConditionTrue, - Reason: "KubeletReady", - Message: "kubelet is posting ready status", - LastHeartbeatTime: currentTime, - } - } else { - newNodeReadyCondition = api.NodeCondition{ - Type: api.NodeReady, - Status: api.ConditionFalse, - Reason: "KubeletNotReady", - Message: strings.Join(rs, ","), - LastHeartbeatTime: currentTime, - } - } - - updated := false - for i := range node.Status.Conditions { - if node.Status.Conditions[i].Type == api.NodeReady { - oldNodeReadyConditionStatus = node.Status.Conditions[i].Status - if oldNodeReadyConditionStatus == newNodeReadyCondition.Status { - newNodeReadyCondition.LastTransitionTime = node.Status.Conditions[i].LastTransitionTime - } else { - newNodeReadyCondition.LastTransitionTime = currentTime - } - node.Status.Conditions[i] = newNodeReadyCondition - updated = true - break - } - } - if !updated { - newNodeReadyCondition.LastTransitionTime = currentTime - node.Status.Conditions = append(node.Status.Conditions, newNodeReadyCondition) - } - if !updated || oldNodeReadyConditionStatus != newNodeReadyCondition.Status { - if newNodeReadyCondition.Status == api.ConditionTrue { - kl.recordNodeStatusEvent(api.EventTypeNormal, kubecontainer.NodeReady) - } else { - kl.recordNodeStatusEvent(api.EventTypeNormal, kubecontainer.NodeNotReady) - } - } - var nodeOODCondition *api.NodeCondition // Check if NodeOutOfDisk condition already exists and if it does, just pick it up for update. @@ -2873,6 +2827,55 @@ func (kl *Kubelet) setNodeStatus(node *api.Node) error { node.Status.Conditions = append(node.Status.Conditions, *nodeOODCondition) } + // NOTE(aaronlevy): NodeReady condition needs to be the last in the list of node conditions. + // This is due to an issue with version skewed kubelet and master components. + // ref: https://github.com/kubernetes/kubernetes/issues/16961 + var newNodeReadyCondition api.NodeCondition + var oldNodeReadyConditionStatus api.ConditionStatus + if rs := kl.runtimeState.errors(); len(rs) == 0 { + newNodeReadyCondition = api.NodeCondition{ + Type: api.NodeReady, + Status: api.ConditionTrue, + Reason: "KubeletReady", + Message: "kubelet is posting ready status", + LastHeartbeatTime: currentTime, + } + } else { + newNodeReadyCondition = api.NodeCondition{ + Type: api.NodeReady, + Status: api.ConditionFalse, + Reason: "KubeletNotReady", + Message: strings.Join(rs, ","), + LastHeartbeatTime: currentTime, + } + } + + updated := false + for i := range node.Status.Conditions { + if node.Status.Conditions[i].Type == api.NodeReady { + oldNodeReadyConditionStatus = node.Status.Conditions[i].Status + if oldNodeReadyConditionStatus == newNodeReadyCondition.Status { + newNodeReadyCondition.LastTransitionTime = node.Status.Conditions[i].LastTransitionTime + } else { + newNodeReadyCondition.LastTransitionTime = currentTime + } + node.Status.Conditions[i] = newNodeReadyCondition + updated = true + break + } + } + if !updated { + newNodeReadyCondition.LastTransitionTime = currentTime + node.Status.Conditions = append(node.Status.Conditions, newNodeReadyCondition) + } + if !updated || oldNodeReadyConditionStatus != newNodeReadyCondition.Status { + if newNodeReadyCondition.Status == api.ConditionTrue { + kl.recordNodeStatusEvent(api.EventTypeNormal, kubecontainer.NodeReady) + } else { + kl.recordNodeStatusEvent(api.EventTypeNormal, kubecontainer.NodeNotReady) + } + } + if oldNodeUnschedulable != node.Spec.Unschedulable { if node.Spec.Unschedulable { kl.recordNodeStatusEvent(api.EventTypeNormal, kubecontainer.NodeNotSchedulable) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 6095ba384d8..37d0165394c 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -2574,14 +2574,6 @@ func TestUpdateNewNodeStatus(t *testing.T) { Spec: api.NodeSpec{}, Status: api.NodeStatus{ Conditions: []api.NodeCondition{ - { - Type: api.NodeReady, - Status: api.ConditionTrue, - Reason: "KubeletReady", - Message: fmt.Sprintf("kubelet is posting ready status"), - LastHeartbeatTime: unversioned.Time{}, - LastTransitionTime: unversioned.Time{}, - }, { Type: api.NodeOutOfDisk, Status: api.ConditionFalse, @@ -2590,6 +2582,14 @@ func TestUpdateNewNodeStatus(t *testing.T) { LastHeartbeatTime: unversioned.Time{}, LastTransitionTime: unversioned.Time{}, }, + { + Type: api.NodeReady, + Status: api.ConditionTrue, + Reason: "KubeletReady", + Message: fmt.Sprintf("kubelet is posting ready status"), + LastHeartbeatTime: unversioned.Time{}, + LastTransitionTime: unversioned.Time{}, + }, }, NodeInfo: api.NodeSystemInfo{ MachineID: "123", @@ -2639,6 +2639,11 @@ func TestUpdateNewNodeStatus(t *testing.T) { updatedNode.Status.Conditions[i].LastTransitionTime = unversioned.Time{} } + // Version skew workaround. See: https://github.com/kubernetes/kubernetes/issues/16961 + if updatedNode.Status.Conditions[len(updatedNode.Status.Conditions)-1].Type != api.NodeReady { + t.Errorf("unexpected node condition order. NodeReady should be last.") + } + if !reflect.DeepEqual(expectedNode, updatedNode) { t.Errorf("unexpected objects: %s", util.ObjectDiff(expectedNode, updatedNode)) } @@ -2690,14 +2695,6 @@ func testDockerRuntimeVersion(t *testing.T) { Spec: api.NodeSpec{}, Status: api.NodeStatus{ Conditions: []api.NodeCondition{ - { - Type: api.NodeReady, - Status: api.ConditionTrue, - Reason: "KubeletReady", - Message: fmt.Sprintf("kubelet is posting ready status"), - LastHeartbeatTime: unversioned.Time{}, - LastTransitionTime: unversioned.Time{}, - }, { Type: api.NodeOutOfDisk, Status: api.ConditionFalse, @@ -2706,6 +2703,14 @@ func testDockerRuntimeVersion(t *testing.T) { LastHeartbeatTime: unversioned.Time{}, LastTransitionTime: unversioned.Time{}, }, + { + Type: api.NodeReady, + Status: api.ConditionTrue, + Reason: "KubeletReady", + Message: fmt.Sprintf("kubelet is posting ready status"), + LastHeartbeatTime: unversioned.Time{}, + LastTransitionTime: unversioned.Time{}, + }, }, NodeInfo: api.NodeSystemInfo{ MachineID: "123", @@ -2754,6 +2759,12 @@ func testDockerRuntimeVersion(t *testing.T) { updatedNode.Status.Conditions[i].LastHeartbeatTime = unversioned.Time{} updatedNode.Status.Conditions[i].LastTransitionTime = unversioned.Time{} } + + // Version skew workaround. See: https://github.com/kubernetes/kubernetes/issues/16961 + if updatedNode.Status.Conditions[len(updatedNode.Status.Conditions)-1].Type != api.NodeReady { + t.Errorf("unexpected node condition order. NodeReady should be last.") + } + if !reflect.DeepEqual(expectedNode, updatedNode) { t.Errorf("unexpected objects: %s", util.ObjectDiff(expectedNode, updatedNode)) } @@ -2792,14 +2803,6 @@ func TestUpdateExistingNodeStatus(t *testing.T) { Spec: api.NodeSpec{}, Status: api.NodeStatus{ Conditions: []api.NodeCondition{ - { - Type: api.NodeReady, - Status: api.ConditionTrue, - Reason: "KubeletReady", - Message: fmt.Sprintf("kubelet is posting ready status"), - LastHeartbeatTime: unversioned.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), - LastTransitionTime: unversioned.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), - }, { Type: api.NodeOutOfDisk, Status: api.ConditionTrue, @@ -2808,6 +2811,14 @@ func TestUpdateExistingNodeStatus(t *testing.T) { LastHeartbeatTime: unversioned.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), LastTransitionTime: unversioned.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), }, + { + Type: api.NodeReady, + Status: api.ConditionTrue, + Reason: "KubeletReady", + Message: fmt.Sprintf("kubelet is posting ready status"), + LastHeartbeatTime: unversioned.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + LastTransitionTime: unversioned.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, }, Capacity: api.ResourceList{ api.ResourceCPU: *resource.NewMilliQuantity(3000, resource.DecimalSI), @@ -2854,14 +2865,6 @@ func TestUpdateExistingNodeStatus(t *testing.T) { Spec: api.NodeSpec{}, Status: api.NodeStatus{ Conditions: []api.NodeCondition{ - { - Type: api.NodeReady, - Status: api.ConditionTrue, - Reason: "KubeletReady", - Message: fmt.Sprintf("kubelet is posting ready status"), - LastHeartbeatTime: unversioned.Time{}, // placeholder - LastTransitionTime: unversioned.Time{}, // placeholder - }, { Type: api.NodeOutOfDisk, Status: api.ConditionTrue, @@ -2870,6 +2873,14 @@ func TestUpdateExistingNodeStatus(t *testing.T) { LastHeartbeatTime: unversioned.Time{}, // placeholder LastTransitionTime: unversioned.Time{}, // placeholder }, + { + Type: api.NodeReady, + Status: api.ConditionTrue, + Reason: "KubeletReady", + Message: fmt.Sprintf("kubelet is posting ready status"), + LastHeartbeatTime: unversioned.Time{}, // placeholder + LastTransitionTime: unversioned.Time{}, // placeholder + }, }, NodeInfo: api.NodeSystemInfo{ MachineID: "123", @@ -2921,6 +2932,11 @@ func TestUpdateExistingNodeStatus(t *testing.T) { updatedNode.Status.Conditions[i].LastTransitionTime = unversioned.Time{} } + // Version skew workaround. See: https://github.com/kubernetes/kubernetes/issues/16961 + if updatedNode.Status.Conditions[len(updatedNode.Status.Conditions)-1].Type != api.NodeReady { + t.Errorf("unexpected node condition order. NodeReady should be last.") + } + if !reflect.DeepEqual(expectedNode, updatedNode) { t.Errorf("expected \n%v\n, got \n%v", expectedNode, updatedNode) } @@ -2976,14 +2992,6 @@ func TestUpdateNodeStatusWithoutContainerRuntime(t *testing.T) { Spec: api.NodeSpec{}, Status: api.NodeStatus{ Conditions: []api.NodeCondition{ - { - Type: api.NodeReady, - Status: api.ConditionFalse, - Reason: "KubeletNotReady", - Message: fmt.Sprintf("container runtime is down"), - LastHeartbeatTime: unversioned.Time{}, - LastTransitionTime: unversioned.Time{}, - }, { Type: api.NodeOutOfDisk, Status: api.ConditionFalse, @@ -2992,6 +3000,14 @@ func TestUpdateNodeStatusWithoutContainerRuntime(t *testing.T) { LastHeartbeatTime: unversioned.Time{}, LastTransitionTime: unversioned.Time{}, }, + { + Type: api.NodeReady, + Status: api.ConditionFalse, + Reason: "KubeletNotReady", + Message: fmt.Sprintf("container runtime is down"), + LastHeartbeatTime: unversioned.Time{}, + LastTransitionTime: unversioned.Time{}, + }, }, NodeInfo: api.NodeSystemInfo{ MachineID: "123", @@ -3042,6 +3058,11 @@ func TestUpdateNodeStatusWithoutContainerRuntime(t *testing.T) { updatedNode.Status.Conditions[i].LastTransitionTime = unversioned.Time{} } + // Version skew workaround. See: https://github.com/kubernetes/kubernetes/issues/16961 + if updatedNode.Status.Conditions[len(updatedNode.Status.Conditions)-1].Type != api.NodeReady { + t.Errorf("unexpected node condition order. NodeReady should be last.") + } + if !reflect.DeepEqual(expectedNode, updatedNode) { t.Errorf("unexpected objects: %s", util.ObjectDiff(expectedNode, updatedNode)) } From 5c72696aad662bfd19604879c542cb8b700e78df Mon Sep 17 00:00:00 2001 From: Aaron Levy Date: Thu, 19 Nov 2015 19:06:39 -0800 Subject: [PATCH 2/2] explicitly check "Ready" condition in validate-cluster --- cluster/validate-cluster.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster/validate-cluster.sh b/cluster/validate-cluster.sh index e3682b219d9..9b448c98479 100755 --- a/cluster/validate-cluster.sh +++ b/cluster/validate-cluster.sh @@ -40,7 +40,7 @@ while true; do # Suppress errors from kubectl output because during cluster bootstrapping # for clusters where the master node is registered, the apiserver will become # available and then get restarted as the kubelet configures the docker bridge. - nodes_status=$("${KUBE_ROOT}/cluster/kubectl.sh" get nodes -o template --template='{{range .items}}{{with index .status.conditions 0}}{{.type}}:{{.status}},{{end}}{{end}}' --api-version=v1) || true + nodes_status=$("${KUBE_ROOT}/cluster/kubectl.sh" get nodes -o template --template='{{range .items}}{{range .status.conditions}}{{if eq .type "Ready"}}{{.type}}:{{.status}},{{end}}{{end}}{{end}}' --api-version=v1) || true found=$(echo "${nodes_status}" | tr "," "\n" | grep -c 'Ready:') || true ready=$(echo "${nodes_status}" | tr "," "\n" | grep -c 'Ready:True') || true