Merge pull request #75374 from liggitt/node-lease-flaky

test/e2e/common/BUILD

@@ -54,6 +54,7 @@ go_library(
         "//staging/src/k8s.io/api/autoscaling/v1:go_default_library",
         "//staging/src/k8s.io/api/coordination/v1beta1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
@@ -62,6 +63,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library",

test/e2e/common/node_lease.go

@@ -22,7 +22,10 @@ import (
 
 	coordv1beta1 "k8s.io/api/coordination/v1beta1"
 	corev1 "k8s.io/api/core/v1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/diff"
+	"k8s.io/apimachinery/pkg/util/wait"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/test/e2e/framework"
 	testutils "k8s.io/kubernetes/test/utils"
@@ -103,35 +106,45 @@ var _ = framework.KubeDescribe("NodeLease", func() {
 			// enough time has passed. So for here, keep checking the time diff
 			// between 2 NodeStatus report, until it is longer than lease duration
 			// (the same as nodeMonitorGracePeriod), or it doesn't change for at least leaseDuration
-			lastHeartbeatTime := getReadyConditionHeartbeatTime(f.ClientSet, nodeName)
+			lastHeartbeatTime, lastStatus := getHeartbeatTimeAndStatus(f.ClientSet, nodeName)
 			lastObserved := time.Now()
-			Eventually(func() error {
-				currentHeartbeatTime := getReadyConditionHeartbeatTime(f.ClientSet, nodeName)
+			err = wait.Poll(time.Second, 5*time.Minute, func() (bool, error) {
+				currentHeartbeatTime, currentStatus := getHeartbeatTimeAndStatus(f.ClientSet, nodeName)
 				currentObserved := time.Now()
 
-				switch {
-				case currentHeartbeatTime == lastHeartbeatTime:
+				if currentHeartbeatTime == lastHeartbeatTime {
 					if currentObserved.Sub(lastObserved) > 2*leaseDuration {
 						// heartbeat hasn't changed while watching for at least 2*leaseDuration, success!
 						framework.Logf("node status heartbeat is unchanged for %s, was waiting for at least %s, success!", currentObserved.Sub(lastObserved), 2*leaseDuration)
-						return nil
+						return true, nil
 					}
 					framework.Logf("node status heartbeat is unchanged for %s, waiting for %s", currentObserved.Sub(lastObserved), 2*leaseDuration)
-					return fmt.Errorf("node status heartbeat is unchanged for %s, waiting for %s", currentObserved.Sub(lastObserved), 2*leaseDuration)
+					return false, nil
+				}
 
-				case currentHeartbeatTime != lastHeartbeatTime:
-					if currentHeartbeatTime.Sub(lastHeartbeatTime) > leaseDuration {
-						// heartbeat time changed, but the diff was greater than leaseDuration, success!
-						framework.Logf("node status heartbeat changed in %s, was waiting for at least %s, success!", currentHeartbeatTime.Sub(lastHeartbeatTime), leaseDuration)
-						return nil
-					}
+				if currentHeartbeatTime.Sub(lastHeartbeatTime) >= leaseDuration {
+					// heartbeat time changed, but the diff was greater than leaseDuration, success!
+					framework.Logf("node status heartbeat changed in %s, was waiting for at least %s, success!", currentHeartbeatTime.Sub(lastHeartbeatTime), leaseDuration)
+					return true, nil
+				}
+
+				if !apiequality.Semantic.DeepEqual(lastStatus, currentStatus) {
+					// heartbeat time changed, but there were relevant changes in the status, keep waiting
+					framework.Logf("node status heartbeat changed in %s (with other status changes), waiting for %s", currentHeartbeatTime.Sub(lastHeartbeatTime), leaseDuration)
+					framework.Logf("%s", diff.ObjectReflectDiff(lastStatus, currentStatus))
 					lastHeartbeatTime = currentHeartbeatTime
 					lastObserved = currentObserved
-					framework.Logf("node status heartbeat changed in %s, waiting for %s", currentHeartbeatTime.Sub(lastHeartbeatTime), leaseDuration)
-					return fmt.Errorf("node status heartbeat changed in %s, waiting for %s", currentHeartbeatTime.Sub(lastHeartbeatTime), leaseDuration)
+					lastStatus = currentStatus
+					return false, nil
 				}
-				return nil
-			}, 5*time.Minute, time.Second).Should(BeNil())
+
+				// heartbeat time changed, with no other status changes, in less time than we expected, so fail.
+				return false, fmt.Errorf("node status heartbeat changed in %s (with no other status changes), was waiting for %s", currentHeartbeatTime.Sub(lastHeartbeatTime), leaseDuration)
+			})
+			// a timeout is acceptable, since it means we waited 5 minutes and didn't see any unwarranted node status updates
+			if err != nil && err != wait.ErrWaitTimeout {
+				Expect(err).NotTo(HaveOccurred(), "error waiting for infrequent nodestatus update")
+			}
 
 			By("verify node is still in ready status even though node status report is infrequent")
 			// This check on node status is only meaningful when this e2e test is
@@ -145,12 +158,14 @@ var _ = framework.KubeDescribe("NodeLease", func() {
 	})
 })
 
-func getReadyConditionHeartbeatTime(clientSet clientset.Interface, nodeName string) time.Time {
+func getHeartbeatTimeAndStatus(clientSet clientset.Interface, nodeName string) (time.Time, corev1.NodeStatus) {
 	node, err := clientSet.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{})
 	Expect(err).To(BeNil())
 	_, readyCondition := testutils.GetNodeCondition(&node.Status, corev1.NodeReady)
 	Expect(readyCondition.Status).To(Equal(corev1.ConditionTrue))
-	return readyCondition.LastHeartbeatTime.Time
+	heartbeatTime := readyCondition.LastHeartbeatTime.Time
+	readyCondition.LastHeartbeatTime = metav1.Time{}
+	return heartbeatTime, node.Status
 }
 
 func expectLease(lease *coordv1beta1.Lease, nodeName string) error {