Merge pull request #11 from souleb/fix-issue-4390974

fix: dpu host node should not stop when network is down

Author: aserdean

go-controller/pkg/node/default_node_network_controller_test.go

@@ -11,7 +11,6 @@ import (
 
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
 	adminpolicybasedrouteclient "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/adminpolicybasedroute/v1/apis/clientset/versioned/fake"
-	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kube"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kube/mocks"
 
 	ovntest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing"
@@ -776,20 +775,29 @@ func heartbeatDPUHostTest(app *cli.App, uplinkName, hostIP string) {
 		_, err = kubeFakeClient.CoordinationV1().Leases(defaultLeaseNS).Update(context.Background(), lease, metav1.UpdateOptions{})
 		Expect(err).NotTo(HaveOccurred())
 
-		//verify that error was reported
-		err = <-errChan
-		Expect(err).To(HaveOccurred())
+		Eventually(func(g Gomega) {
+			// check that the node is tainted
+			node, err = kubeFakeClient.CoreV1().Nodes().Get(context.Background(), nodeName, metav1.GetOptions{})
+			g.Expect(err).NotTo(HaveOccurred())
+			g.Expect(node.Spec.Taints).To(HaveLen(1))
+			g.Expect(node.Spec.Taints[0].Key).To(Equal(kapi.TaintNodeNetworkUnavailable))
+		}).ShouldNot(HaveOccurred())
 
-		// check that the node is tainted
-		node, err = kubeFakeClient.CoreV1().Nodes().Get(context.Background(), nodeName, metav1.GetOptions{})
+		// update the lease to make it valid again
+		lease, err = kubeFakeClient.CoordinationV1().Leases(defaultLeaseNS).Get(context.Background(), nodeName, metav1.GetOptions{})
 		Expect(err).NotTo(HaveOccurred())
-		Expect(node.Spec.Taints).To(HaveLen(1))
-		Expect(node.Spec.Taints[0].Key).To(Equal(networkUnavailableTaintKey))
-
-		// remove the taint
-		err = removeNodeNetworkUnavailableTaint(context.Background(), &kube.Kube{KClient: kubeFakeClient}, nodeName)
+		lease.Spec.RenewTime = &metav1.MicroTime{Time: time.Now()}
+		lease.Spec.LeaseDurationSeconds = ptr.To(int32(40))
+		_, err = kubeFakeClient.CoordinationV1().Leases(defaultLeaseNS).Update(context.Background(), lease, metav1.UpdateOptions{})
 		Expect(err).NotTo(HaveOccurred())
 
+		// check that the node is not tainted
+		Eventually(func(g Gomega) {
+			node, err = kubeFakeClient.CoreV1().Nodes().Get(context.Background(), nodeName, metav1.GetOptions{})
+			g.Expect(err).NotTo(HaveOccurred())
+			g.Expect(node.Spec.Taints).To(HaveLen(0))
+		}).ShouldNot(HaveOccurred())
+
 		return nil
 	}
 

go-controller/pkg/node/heartbeat_dpu.go

@@ -79,12 +79,13 @@ func (o IntervalOption) Apply(options *heartbeatOptions) {
 }
 
 type heartbeat struct {
-	nodeName  string
-	zone      string
-	client    kube.Interface
-	clientSet kubernetes.Interface
-	lease     *coordinationv1.Lease
-	errChan   chan error
+	nodeName    string
+	zone        string
+	client      kube.Interface
+	clientSet   kubernetes.Interface
+	lease       *coordinationv1.Lease
+	errChan     chan error
+	taintMarker bool
 	heartbeatOptions
 }
 
@@ -209,7 +210,6 @@ func (h *heartbeat) runDPUHost(ctx context.Context) error {
 				h.errChan <- nil
 				return
 			case <-ticker.C:
-				var errs []error
 				if err := wait.ExponentialBackoffWithContext(ctx,
 					wait.Backoff{
 						Duration: retryInterval,
@@ -228,13 +228,33 @@ func (h *heartbeat) runDPUHost(ctx context.Context) error {
 						h.errChan <- nil
 						return
 					}
-					errs = append(errs, fmt.Errorf("failed to check heartbeat lease: %w", err))
-					if err := setNodeNetworkUnavailableTaint(ctx, h.client, h.nodeName); err != nil {
-						klog.Errorf("Failed to set NetworkUnavailable taint: %v", err)
-						errs = append(errs, err)
+					if !h.taintMarker {
+						var errs []error
+						errs = append(errs, fmt.Errorf("failed to check heartbeat lease: %w", err))
+						if err := setNodeNetworkUnavailableTaint(ctx, h.client, h.nodeName); err != nil {
+							klog.Errorf("Failed to set NetworkUnavailable taint: %v", err)
+							errs = append(errs, err)
+							h.errChan <- kerrors.NewAggregate(errs)
+							// if we cannot set the taint, we need to return
+							// and exit the process, because pod can be scheduled
+							// on this node and we have an unhealthy node
+							return
+						}
+						// if we set the taint, we need to set the marker
+						// and move on until next heartbeat
+						h.taintMarker = true
+					}
+					continue
+				}
+				// if we are here, it means the heartbeat lease is valid
+				// and we can remove the taint
+				if h.taintMarker {
+					if err := removeNodeNetworkUnavailableTaint(ctx, h.client, h.nodeName); err != nil {
+						klog.Errorf("Failed to remove NetworkUnavailable taint: %v", err)
+						h.errChan <- err
+						return
 					}
-					h.errChan <- kerrors.NewAggregate(errs)
-					return
+					h.taintMarker = false
 				}
 			}
 		}