From 21c57a56335dbc9925e18549581068003229975e Mon Sep 17 00:00:00 2001 From: Federico Simoncelli Date: Wed, 22 Apr 2015 10:46:32 -0400 Subject: [PATCH 1/3] nodecontroller: remove unused ready event recording It's not nodecontroller setting the node to Ready so it's impossible to reach this condition. Signed-off-by: Federico Simoncelli --- pkg/cloudprovider/nodecontroller/nodecontroller.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/cloudprovider/nodecontroller/nodecontroller.go b/pkg/cloudprovider/nodecontroller/nodecontroller.go index 55510ebfb26..1b53bd5006c 100644 --- a/pkg/cloudprovider/nodecontroller/nodecontroller.go +++ b/pkg/cloudprovider/nodecontroller/nodecontroller.go @@ -612,9 +612,6 @@ func (nc *NodeController) monitorNodeStatus() error { } // Report node events. - if readyCondition.Status == api.ConditionTrue && lastReadyCondition.Status != api.ConditionTrue { - nc.recordNodeEvent(node, "ready") - } if readyCondition.Status == api.ConditionFalse && lastReadyCondition.Status != api.ConditionFalse { nc.recordNodeEvent(node, "not_ready") } From 2f503c57a559fdea8b93b660bb800cb59a001f66 Mon Sep 17 00:00:00 2001 From: Federico Simoncelli Date: Wed, 22 Apr 2015 10:48:38 -0400 Subject: [PATCH 2/3] nodecontroller: improve node status event recording This patch substitutes the misleading reason "unknown" for the event recording. For symmetry with kubelet's message "online" the conditions Unknown and False are reported as "offline". Signed-off-by: Federico Simoncelli --- .../nodecontroller/nodecontroller.go | 11 +++----- pkg/kubelet/kubelet.go | 25 ++++++------------- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/pkg/cloudprovider/nodecontroller/nodecontroller.go b/pkg/cloudprovider/nodecontroller/nodecontroller.go index 1b53bd5006c..98c7e07d710 100644 --- a/pkg/cloudprovider/nodecontroller/nodecontroller.go +++ b/pkg/cloudprovider/nodecontroller/nodecontroller.go @@ -424,7 +424,7 @@ func (nc *NodeController) recordNodeEvent(node *api.Node, event string) { glog.V(2).Infof("Recording %s event message for node %s", event, node.Name) // TODO: This requires a transaction, either both node status is updated // and event is recorded or neither should happen, see issue #6055. - nc.recorder.Eventf(ref, event, "Node %s is now %s", node.Name, event) + nc.recorder.Eventf(ref, event, "Node %s status is now: %s", node.Name, event) } // For a given node checks its conditions and tries to update it. Returns grace period to which given node @@ -611,12 +611,9 @@ func (nc *NodeController) monitorNodeStatus() error { } } - // Report node events. - if readyCondition.Status == api.ConditionFalse && lastReadyCondition.Status != api.ConditionFalse { - nc.recordNodeEvent(node, "not_ready") - } - if readyCondition.Status == api.ConditionUnknown && lastReadyCondition.Status != api.ConditionUnknown { - nc.recordNodeEvent(node, "unknown") + // Report node event. + if readyCondition.Status != api.ConditionTrue && lastReadyCondition.Status == api.ConditionTrue { + nc.recordNodeEvent(node, "NodeNotReady") } } } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 6cb89206e5c..796c789619e 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1717,22 +1717,11 @@ func (kl *Kubelet) updateNodeStatus() error { return fmt.Errorf("Update node status exceeds retry count") } -func (kl *Kubelet) recordNodeOnlineEvent() { +func (kl *Kubelet) recordNodeStatusEvent(event string) { + glog.V(2).Infof("Recording %s event message for node %s", event, kl.hostname) // TODO: This requires a transaction, either both node status is updated // and event is recorded or neither should happen, see issue #6055. - kl.recorder.Eventf(kl.nodeRef, "online", "Node %s is now online", kl.hostname) -} - -func (kl *Kubelet) recordNodeSchedulableEvent() { - // TODO: This requires a transaction, either both node status is updated - // and event is recorded or neither should happen, see issue #6055. - kl.recorder.Eventf(kl.nodeRef, "schedulable", "Node %s is now schedulable", kl.hostname) -} - -func (kl *Kubelet) recordNodeUnschedulableEvent() { - // TODO: This requires a transaction, either both node status is updated - // and event is recorded or neither should happen, see issue #6055. - kl.recorder.Eventf(kl.nodeRef, "unschedulable", "Node %s is now unschedulable", kl.hostname) + kl.recorder.Eventf(kl.nodeRef, event, "Node %s status is now: %s", kl.hostname, event) } // Maintains Node.Spec.Unschedulable value from previous run of tryUpdateNodeStatus() @@ -1828,7 +1817,7 @@ func (kl *Kubelet) tryUpdateNodeStatus() error { if node.Status.Conditions[i].Type == api.NodeReady { newCondition.LastTransitionTime = node.Status.Conditions[i].LastTransitionTime if node.Status.Conditions[i].Status != api.ConditionTrue { - kl.recordNodeOnlineEvent() + kl.recordNodeStatusEvent("NodeReady") } node.Status.Conditions[i] = newCondition updated = true @@ -1837,14 +1826,14 @@ func (kl *Kubelet) tryUpdateNodeStatus() error { if !updated { newCondition.LastTransitionTime = currentTime node.Status.Conditions = append(node.Status.Conditions, newCondition) - kl.recordNodeOnlineEvent() + kl.recordNodeStatusEvent("NodeReady") } if oldNodeUnschedulable != node.Spec.Unschedulable { if node.Spec.Unschedulable { - kl.recordNodeUnschedulableEvent() + kl.recordNodeStatusEvent("NodeNotSchedulable") } else { - kl.recordNodeSchedulableEvent() + kl.recordNodeStatusEvent("NodeSchedulable") } oldNodeUnschedulable = node.Spec.Unschedulable } From fefc65164a3e1ac6ce589a62fbe5376a000c46e2 Mon Sep 17 00:00:00 2001 From: Federico Simoncelli Date: Wed, 20 May 2015 07:09:18 -0400 Subject: [PATCH 3/3] kubelet: fix node ready condition logic Fixes #8585 Signed-off-by: Federico Simoncelli --- pkg/kubelet/kubelet.go | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 796c789619e..4cc3bcaed57 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1788,9 +1788,10 @@ func (kl *Kubelet) tryUpdateNodeStatus() error { }() currentTime := util.Now() - var newCondition api.NodeCondition + var newNodeReadyCondition api.NodeCondition + var oldNodeReadyConditionStatus api.ConditionStatus if containerRuntimeUp && networkConfigured { - newCondition = api.NodeCondition{ + newNodeReadyCondition = api.NodeCondition{ Type: api.NodeReady, Status: api.ConditionTrue, Reason: "kubelet is posting ready status", @@ -1804,7 +1805,7 @@ func (kl *Kubelet) tryUpdateNodeStatus() error { if !networkConfigured { reasons = append(reasons, "network not configured correctly") } - newCondition = api.NodeCondition{ + newNodeReadyCondition = api.NodeCondition{ Type: api.NodeReady, Status: api.ConditionFalse, Reason: strings.Join(reasons, ","), @@ -1815,20 +1816,27 @@ func (kl *Kubelet) tryUpdateNodeStatus() error { updated := false for i := range node.Status.Conditions { if node.Status.Conditions[i].Type == api.NodeReady { - newCondition.LastTransitionTime = node.Status.Conditions[i].LastTransitionTime - if node.Status.Conditions[i].Status != api.ConditionTrue { - kl.recordNodeStatusEvent("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] = newCondition + node.Status.Conditions[i] = newNodeReadyCondition updated = true } } if !updated { - newCondition.LastTransitionTime = currentTime - node.Status.Conditions = append(node.Status.Conditions, newCondition) - kl.recordNodeStatusEvent("NodeReady") + newNodeReadyCondition.LastTransitionTime = currentTime + node.Status.Conditions = append(node.Status.Conditions, newNodeReadyCondition) + } + if !updated || oldNodeReadyConditionStatus != newNodeReadyCondition.Status { + if newNodeReadyCondition.Status == api.ConditionTrue { + kl.recordNodeStatusEvent("NodeReady") + } else { + kl.recordNodeStatusEvent("NodeNotReady") + } } - if oldNodeUnschedulable != node.Spec.Unschedulable { if node.Spec.Unschedulable { kl.recordNodeStatusEvent("NodeNotSchedulable")