Merge pull request #28751 from Random-Liu/use-patch-to-set-node-condition

Automatic merge from submit-queue

Change route controller to use patch to set node condition.

Change the route controller to use `PatchStatus` function in `NodeExpansion` to update node condition.
@caesarxuchao 

/cc @wojtek-t 


[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]()

pkg/controller/route/routecontroller.go

@@ -24,10 +24,12 @@ import (
 
 	"github.com/golang/glog"
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	"k8s.io/kubernetes/pkg/cloudprovider"
 	"k8s.io/kubernetes/pkg/util/metrics"
+	nodeutil "k8s.io/kubernetes/pkg/util/node"
 	"k8s.io/kubernetes/pkg/util/wait"
 )
 
@@ -162,32 +164,22 @@ func (rc *RouteController) reconcile(nodes []api.Node, routes []*cloudprovider.R
 	return nil
 }
 
-func updateNetworkingCondition(node *api.Node, routeCreated bool) {
+func (rc *RouteController) updateNetworkingCondition(nodeName string, routeCreated bool) error {
-	_, networkingCondition := api.GetNodeCondition(&node.Status, api.NodeNetworkUnavailable)
+	var err error
-	currentTime := unversioned.Now()
+	for i := 0; i < updateNodeStatusMaxRetries; i++ {
-	if routeCreated {
+		// Patch could also fail, even though the chance is very slim. So we still do
-		if networkingCondition != nil && networkingCondition.Status != api.ConditionFalse {
+		// patch in the retry loop.
-			networkingCondition.Status = api.ConditionFalse
+		currentTime := unversioned.Now()
-			networkingCondition.Reason = "RouteCreated"
+		if routeCreated {
-			networkingCondition.Message = "RouteController created a route"
+			err = nodeutil.SetNodeCondition(rc.kubeClient, nodeName, api.NodeCondition{
-			networkingCondition.LastTransitionTime = currentTime
-		} else if networkingCondition == nil {
-			node.Status.Conditions = append(node.Status.Conditions, api.NodeCondition{
 				Type:               api.NodeNetworkUnavailable,
 				Status:             api.ConditionFalse,
 				Reason:             "RouteCreated",
 				Message:            "RouteController created a route",
 				LastTransitionTime: currentTime,
 			})
-		}
+		} else {
-	} else {
+			err = nodeutil.SetNodeCondition(rc.kubeClient, nodeName, api.NodeCondition{
-		if networkingCondition != nil && networkingCondition.Status != api.ConditionTrue {
-			networkingCondition.Status = api.ConditionTrue
-			networkingCondition.Reason = "NoRouteCreated"
-			networkingCondition.Message = "RouteController failed to create a route"
-			networkingCondition.LastTransitionTime = currentTime
-		} else if networkingCondition == nil {
-			node.Status.Conditions = append(node.Status.Conditions, api.NodeCondition{
 				Type:               api.NodeNetworkUnavailable,
 				Status:             api.ConditionTrue,
 				Reason:             "NoRouteCreated",
@@ -195,28 +187,14 @@ func updateNetworkingCondition(node *api.Node, routeCreated bool) {
 				LastTransitionTime: currentTime,
 			})
 		}
-	}
+		if err == nil {
-}
-
-func (rc *RouteController) updateNetworkingCondition(nodeName string, routeCreated bool) error {
-	var err error
-	for i := 0; i < updateNodeStatusMaxRetries; i++ {
-		node, err := rc.kubeClient.Core().Nodes().Get(nodeName)
-		if err != nil {
-			glog.Errorf("Error geting node: %v", err)
-			continue
-		}
-		updateNetworkingCondition(node, routeCreated)
-		// TODO: Use Patch instead once #26381 is merged.
-		// See kubernetes/node-problem-detector#9 for details.
-		if _, err = rc.kubeClient.Core().Nodes().UpdateStatus(node); err == nil {
 			return nil
 		}
-		if i+1 < updateNodeStatusMaxRetries {
+		if i == updateNodeStatusMaxRetries || !errors.IsConflict(err) {
-			glog.Errorf("Error updating node %s, retrying: %v", node.Name, err)
+			glog.Errorf("Error updating node %s: %v", nodeName, err)
-		} else {
+			return err
-			glog.Errorf("Error updating node %s: %v", node.Name, err)
 		}
+		glog.Errorf("Error updating node %s, retrying: %v", nodeName, err)
 	}
 	return err
 }

pkg/util/node/node.go

@@ -17,14 +17,17 @@ limitations under the License.
 package node
 
 import (
+	"encoding/json"
 	"fmt"
 	"net"
 	"os/exec"
 	"strings"
+	"time"
 
 	"github.com/golang/glog"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/unversioned"
+	clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 )
 
 func GetHostname(hostnameOverride string) string {
@@ -81,3 +84,21 @@ func GetZoneKey(node *api.Node) string {
 	// As a nice side-benefit, the null character is not printed by fmt.Print or glog
 	return region + ":\x00:" + failureDomain
 }
+
+// SetNodeCondition updates specific node condition with patch operation.
+func SetNodeCondition(c clientset.Interface, node string, condition api.NodeCondition) error {
+	generatePatch := func(condition api.NodeCondition) ([]byte, error) {
+		raw, err := json.Marshal(&[]api.NodeCondition{condition})
+		if err != nil {
+			return nil, err
+		}
+		return []byte(fmt.Sprintf(`{"status":{"conditions":%s}}`, raw)), nil
+	}
+	condition.LastHeartbeatTime = unversioned.NewTime(time.Now())
+	patch, err := generatePatch(condition)
+	if err != nil {
+		return nil
+	}
+	_, err = c.Core().Nodes().PatchStatus(node, patch)
+	return err
+}