Merge pull request #1839 from JacobTanenbaum/OCPBUGS-17731

OCPBUGS-17731: move clearInitialNodeNetworkUnavailableCondition to clustermanager

go-controller/pkg/clustermanager/network_cluster_controller.go

@@ -6,8 +6,11 @@ import (
 	"reflect"
 	"sync"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	corev1 "k8s.io/api/core/v1"
 	cache "k8s.io/client-go/tools/cache"
+	"k8s.io/client-go/util/retry"
 	"k8s.io/klog/v2"
 
 	idallocator "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/id"
@@ -253,6 +256,7 @@ func (h *networkClusterControllerEventHandler) AddResource(obj interface{}, from
 				node.Name, err)
 			return err
 		}
+		h.clearInitialNodeNetworkUnavailableCondition(node)
 	default:
 		return fmt.Errorf("no add function for object type %s", h.objType)
 	}
@@ -386,3 +390,51 @@ func (h *networkClusterControllerEventHandler) GetResourceFromInformerCache(key
 	}
 	return obj, err
 }
+
+// OVN uses an overlay and doesn't need GCE Routes, we need to
+// clear the NetworkUnavailable condition that kubelet adds to initial node
+// status when using GCE (done here: https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/cloud/node_controller.go#L237).
+// See discussion surrounding this here: https://github.com/kubernetes/kubernetes/pull/34398.
+// TODO: make upstream kubelet more flexible with overlays and GCE so this
+// condition doesn't get added for network plugins that don't want it, and then
+// we can remove this function.
+func (h *networkClusterControllerEventHandler) clearInitialNodeNetworkUnavailableCondition(origNode *corev1.Node) {
+	// If it is not a Cloud Provider node, then nothing to do.
+	if origNode.Spec.ProviderID == "" {
+		return
+	}
+
+	cleared := false
+	resultErr := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+		var err error
+
+		oldNode, err := h.ncc.watchFactory.GetNode(origNode.Name)
+		if err != nil {
+			return err
+		}
+		// Informer cache should not be mutated, so get a copy of the object
+		node := oldNode.DeepCopy()
+
+		for i := range node.Status.Conditions {
+			if node.Status.Conditions[i].Type == corev1.NodeNetworkUnavailable {
+				condition := &node.Status.Conditions[i]
+				if condition.Status != corev1.ConditionFalse && condition.Reason == "NoRouteCreated" {
+					condition.Status = corev1.ConditionFalse
+					condition.Reason = "RouteCreated"
+					condition.Message = "ovn-kube cleared kubelet-set NoRouteCreated"
+					condition.LastTransitionTime = metav1.Now()
+					if err = h.ncc.kube.UpdateNodeStatus(node); err == nil {
+						cleared = true
+					}
+				}
+				break
+			}
+		}
+		return err
+	})
+	if resultErr != nil {
+		klog.Errorf("Status update failed for local node %s: %v", origNode.Name, resultErr)
+	} else if cleared {
+		klog.Infof("Cleared node NetworkUnavailable/NoRouteCreated condition for %s", origNode.Name)
+	}
+}

go-controller/pkg/ovn/master.go

@@ -7,10 +7,8 @@ import (
 	"time"
 
 	kapi "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
-	"k8s.io/client-go/util/retry"
 	"k8s.io/klog/v2"
 	utilnet "k8s.io/utils/net"
 
@@ -497,54 +495,6 @@ func (oc *DefaultNetworkController) cleanupNodeResources(nodeName string) error
 	return nil
 }
 
-// OVN uses an overlay and doesn't need GCE Routes, we need to
-// clear the NetworkUnavailable condition that kubelet adds to initial node
-// status when using GCE (done here: https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/cloud/node_controller.go#L237).
-// See discussion surrounding this here: https://github.com/kubernetes/kubernetes/pull/34398.
-// TODO: make upstream kubelet more flexible with overlays and GCE so this
-// condition doesn't get added for network plugins that don't want it, and then
-// we can remove this function.
-func (oc *DefaultNetworkController) clearInitialNodeNetworkUnavailableCondition(origNode *kapi.Node) {
-	// If it is not a Cloud Provider node, then nothing to do.
-	if origNode.Spec.ProviderID == "" {
-		return
-	}
-
-	cleared := false
-	resultErr := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
-		var err error
-
-		oldNode, err := oc.watchFactory.GetNode(origNode.Name)
-		if err != nil {
-			return err
-		}
-		// Informer cache should not be mutated, so get a copy of the object
-		node := oldNode.DeepCopy()
-
-		for i := range node.Status.Conditions {
-			if node.Status.Conditions[i].Type == kapi.NodeNetworkUnavailable {
-				condition := &node.Status.Conditions[i]
-				if condition.Status != kapi.ConditionFalse && condition.Reason == "NoRouteCreated" {
-					condition.Status = kapi.ConditionFalse
-					condition.Reason = "RouteCreated"
-					condition.Message = "ovn-kube cleared kubelet-set NoRouteCreated"
-					condition.LastTransitionTime = metav1.Now()
-					if err = oc.kube.UpdateNodeStatus(node); err == nil {
-						cleared = true
-					}
-				}
-				break
-			}
-		}
-		return err
-	})
-	if resultErr != nil {
-		klog.Errorf("Status update failed for local node %s: %v", origNode.Name, resultErr)
-	} else if cleared {
-		klog.Infof("Cleared node NetworkUnavailable/NoRouteCreated condition for %s", origNode.Name)
-	}
-}
-
 // this is the worker function that does the periodic sync of nodes from kube API
 // and sbdb and deletes chassis that are stale
 func (oc *DefaultNetworkController) syncNodesPeriodic() {
@@ -783,7 +733,6 @@ func (oc *DefaultNetworkController) addUpdateLocalNodeEvent(node *kapi.Node, nSy
 				return err
 			}
 		}
-		oc.clearInitialNodeNetworkUnavailableCondition(node)
 		return nil
 	}
 
@@ -855,8 +804,6 @@ func (oc *DefaultNetworkController) addUpdateLocalNodeEvent(node *kapi.Node, nSy
 		errs = append(errs, fmt.Errorf("failed to set hybrid overlay annotations for node %s: %v", node.Name, err))
 	}
 
-	oc.clearInitialNodeNetworkUnavailableCondition(node)
-
 	if nSyncs.syncGw {
 		err := oc.syncNodeGateway(node, nil)
 		if err != nil {