Repository: kinvolk/lokomotive
Pull request #1168 (2020-11-06 14:08:03)

test/components/flatcar-linux-update-operator: fix flaky idle test
**Includes commits from #1167, as otherwise `Packet` pipeline would fail**

See commit messages for details.


Patch:
diff --git a/pkg/components/cluster-autoscaler/component.go b/pkg/components/cluster-autoscaler/component.go
index a425fd67..505bec7b 100644
--- a/pkg/components/cluster-autoscaler/component.go
+++ b/pkg/components/cluster-autoscaler/component.go
@@ -169,14 +169,15 @@ func getClusterWorkers(clusterName, facility string, devices []packngo.Device) [
 func findDuplicatedDevices(devices []packngo.Device) []packngo.Device {
 	duplicatedDevices := []packngo.Device{}
 
-	deviceMap := map[string]struct{}{}
+	deviceMap := map[string]string{}
 
 	for _, d := range devices {
-		if _, ok := deviceMap[d.Hostname]; ok {
+		id, ok := deviceMap[d.Hostname]
+		if ok && d.ID != id {
 			duplicatedDevices = append(duplicatedDevices, d)
 		}
 
-		deviceMap[d.Hostname] = struct{}{}
+		deviceMap[d.Hostname] = d.ID
 	}
 
 	return duplicatedDevices


Test patch:
diff --git a/pkg/components/cluster-autoscaler/component_internal_test.go b/pkg/components/cluster-autoscaler/component_internal_test.go
index 4c7f4734..fce51641 100644
--- a/pkg/components/cluster-autoscaler/component_internal_test.go
+++ b/pkg/components/cluster-autoscaler/component_internal_test.go
@@ -108,44 +108,49 @@ func TestGetClusterWorkersFilterNonWorkers(t *testing.T) {
 	}
 }
 
-func TestFindDuplicatedDevicesNonUniqueHostname(t *testing.T) {
-	device := packngo.Device{
-		DeviceRaw: packngo.DeviceRaw{
-			Hostname: "foo",
-		},
-	}
-
+// Packet API seems to have a bug, that when you use Project API key for querying devices,
+// it returns duplicated entries. However IDs of the devices should be the same, so it should
+// be safe to relax the check on those, as if there would actually be 2 different devices with
+// the same name, they should have different IDs.
+func TestFindDuplicatedDevicesDuplicateHostnamesAndIDs(t *testing.T) {
 	d := []packngo.Device{
-		device,
-		device,
 		{
 			DeviceRaw: packngo.DeviceRaw{
-				Hostname: "bar",
+				ID:       "bar",
+				Hostname: "foo",
+			},
+		},
+		{
+			DeviceRaw: packngo.DeviceRaw{
+				ID:       "bar",
+				Hostname: "foo",
 			},
 		},
 	}
 
-	if w := findDuplicatedDevices(d); len(w) != 1 {
-		t.Fatalf("two devices with the same hostname should be treated as duplicates")
+	if w := findDuplicatedDevices(d); len(w) != 0 {
+		t.Fatalf("two devices with the same hostname and same IDs should not be treated as duplicates")
 	}
 }
 
-func TestFindDuplicatedDevicesUniqueHostname(t *testing.T) {
+func TestFindDuplicatedDevicesDuplicateHostnamesUniqueIDs(t *testing.T) {
 	d := []packngo.Device{
 		{
 			DeviceRaw: packngo.DeviceRaw{
-				Hostname: "bar",
+				ID:       "baz",
+				Hostname: "foo",
 			},
 		},
 		{
 			DeviceRaw: packngo.DeviceRaw{
+				ID:       "bar",
 				Hostname: "foo",
 			},
 		},
 	}
 
-	if w := findDuplicatedDevices(d); len(w) != 0 {
-		t.Fatalf("two devices with the different hostname should not be treated as duplicates")
+	if w := findDuplicatedDevices(d); len(w) == 0 {
+		t.Fatalf("two devices with the same hostname but different IDs should be treated as duplicates")
 	}
 }
 
@@ -182,16 +187,21 @@ func TestGetWorkerUserdataNoUserdataOnError(t *testing.T) {
 }
 
 func TestGetWorkerUserdataDuplicatedWorkers(t *testing.T) {
-	device := packngo.Device{
-		DeviceRaw: packngo.DeviceRaw{
-			Hostname: "worker",
-			Facility: &packngo.Facility{},
-		},
-	}
-
 	d := []packngo.Device{
-		device,
-		device,
+		{
+			DeviceRaw: packngo.DeviceRaw{
+				ID:       "foo",
+				Hostname: "worker",
+				Facility: &packngo.Facility{},
+			},
+		},
+		{
+			DeviceRaw: packngo.DeviceRaw{
+				ID:       "bar",
+				Hostname: "worker",
+				Facility: &packngo.Facility{},
+			},
+		},
 	}
 
 	if _, err := getWorkerUserdata("", "", d); err == nil {
@@ -202,6 +212,7 @@ func TestGetWorkerUserdataDuplicatedWorkers(t *testing.T) {
 func TestGetWorkerUserdataEmptyUserdata(t *testing.T) {
 	device := packngo.Device{
 		DeviceRaw: packngo.DeviceRaw{
+			ID:       "foo",
 			Hostname: "worker",
 			Facility: &packngo.Facility{},
 		},
@@ -223,6 +234,7 @@ func TestGetWorkerUserdataFirstDevice(t *testing.T) {
 	d := []packngo.Device{
 		{
 			DeviceRaw: packngo.DeviceRaw{
+				ID:       "foo",
 				UserData: expected,
 				Hostname: "worker",
 				Facility: &packngo.Facility{},
@@ -230,6 +242,7 @@ func TestGetWorkerUserdataFirstDevice(t *testing.T) {
 		},
 		{
 			DeviceRaw: packngo.DeviceRaw{
+				ID:       "bar",
 				UserData: "bar",
 				Hostname: "worker2",
 				Facility: &packngo.Facility{},
@@ -248,6 +261,7 @@ func TestGetWorkerUserdataReturnBase64(t *testing.T) {
 	d := []packngo.Device{
 		{
 			DeviceRaw: packngo.DeviceRaw{
+				ID:       "foo",
 				UserData: expected,
 				Hostname: "worker",
 				Facility: &packngo.Facility{},
@@ -266,6 +280,7 @@ func TestGetWorkerUserdataDuplicatedWorkersDifferentClusters(t *testing.T) {
 
 	device := packngo.Device{
 		DeviceRaw: packngo.DeviceRaw{
+			ID:       "foo",
 			UserData: "foo",
 			Hostname: "worker",
 			Facility: &packngo.Facility{
@@ -279,6 +294,7 @@ func TestGetWorkerUserdataDuplicatedWorkersDifferentClusters(t *testing.T) {
 		device,
 		{
 			DeviceRaw: packngo.DeviceRaw{
+				ID:       "bar",
 				UserData: "foo",
 				Hostname: "worker",
 				Facility: &packngo.Facility{
@@ -299,33 +315,51 @@ func TestGetWorkerUserdataDuplicatedWorkersIncludeHostnames(t *testing.T) {
 	hostnameA := "worker1"
 	hostnameB := "worker2"
 
-	deviceA := packngo.Device{
-		DeviceRaw: packngo.DeviceRaw{
-			Hostname: hostnameA,
-			Facility: &packngo.Facility{
-				Code: facility,
+	d := []packngo.Device{
+		{
+			DeviceRaw: packngo.DeviceRaw{
+				ID:       "foo",
+				Hostname: hostnameA,
+				Facility: &packngo.Facility{
+					Code: facility,
+				},
 			},
 		},
-	}
-
-	deviceB := packngo.Device{
-		DeviceRaw: packngo.DeviceRaw{
-			Hostname: hostnameB,
-			Facility: &packngo.Facility{
-				Code: facility,
+		{
+			DeviceRaw: packngo.DeviceRaw{
+				ID:       "bar",
+				Hostname: hostnameA,
+				Facility: &packngo.Facility{
+					Code: facility,
+				},
+			},
+		},
+		{
+			DeviceRaw: packngo.DeviceRaw{
+				ID:       "baz",
+				Hostname: hostnameB,
+				Facility: &packngo.Facility{
+					Code: facility,
+				},
+			},
+		},
+		{
+			DeviceRaw: packngo.DeviceRaw{
+				ID:       "doh",
+				Hostname: hostnameA,
+				Facility: &packngo.Facility{
+					Code: facility,
+				},
 			},
 		},
 	}
 
-	d := []packngo.Device{
-		deviceA,
-		deviceA,
-		deviceB,
-		deviceB,
+	_, err := getWorkerUserdata("", facility, d)
+	if err == nil {
+		t.Fatalf("expected duplicate device error")
 	}
 
-	_, err := getWorkerUserdata("", facility, d)
 	if !strings.Contains(err.Error(), hostnameA) && !strings.Contains(err.Error(), hostnameB) {
-		t.Fatalf("error should include all duplicated hostnames, got: %v", err)
+		t.Fatalf("error should include all duplicated hostnames, got: %q", err)
 	}
 }
diff --git a/test/components/flatcar-linux-update-operator/fluo_reboot_test.go b/test/components/flatcar-linux-update-operator/fluo_reboot_test.go
index 438f40c2..862880f0 100644
--- a/test/components/flatcar-linux-update-operator/fluo_reboot_test.go
+++ b/test/components/flatcar-linux-update-operator/fluo_reboot_test.go
@@ -56,15 +56,15 @@ func TestNodeCanReachIdleState(t *testing.T) {
 
 	// This annotation should be set on the node during the cluster creation, if it is not set then
 	// something changed in the cluster setup process in the CI.
-	if _, ok := chosenNode.Annotations[annotation]; !ok {
-		t.Fatalf("Annotation %q not found on the node.", annotation)
-	}
+	if _, ok := chosenNode.Annotations[annotation]; ok {
+		t.Logf("Annotation %q found on the node, removing.", annotation)
 
-	// Remove the annotation that disables node reboot.
-	delete(chosenNode.Annotations, annotation)
+		// Remove the annotation that disables node reboot.
+		delete(chosenNode.Annotations, annotation)
 
-	if _, err := client.CoreV1().Nodes().Update(context.Background(), &chosenNode, metav1.UpdateOptions{}); err != nil {
-		t.Fatalf("Removing node annotation %q: %v", annotation, err)
+		if _, err := client.CoreV1().Nodes().Update(context.Background(), &chosenNode, metav1.UpdateOptions{}); err != nil {
+			t.Fatalf("Removing node annotation %q: %v", annotation, err)
+		}
 	}
 
 	// Wait for the FLUO to add following annotation key value pair to the chosen node. This may
@@ -73,7 +73,7 @@ func TestNodeCanReachIdleState(t *testing.T) {
 	statusAnnotationKey := "flatcar-linux-update.v1.flatcar-linux.net/status"
 	statusAnnotationVal := "UPDATE_STATUS_IDLE"
 
-	if err := wait.PollImmediate(testutil.RetryInterval, testutil.Timeout, func() (done bool, err error) {
+	if err := wait.PollImmediate(testutil.RetryInterval, testutil.TimeoutSlow, func() (done bool, err error) {
 		node, err := client.CoreV1().Nodes().Get(context.Background(), chosenNode.Name, metav1.GetOptions{})
 		if err != nil {
 			return false, fmt.Errorf("Getting node %q: %v", chosenNode.Name, err)
diff --git a/test/components/flatcar-linux-update-operator/fluo_test.go b/test/components/flatcar-linux-update-operator/fluo_test.go
index 0f86a357..5ef958ca 100644
--- a/test/components/flatcar-linux-update-operator/fluo_test.go
+++ b/test/components/flatcar-linux-update-operator/fluo_test.go
@@ -15,7 +15,7 @@
 // +build aws aws_edge packet
 // +build e2e
 
-package fluo
+package fluo_test
 
 import (
 	"testing"