Merge pull request #4943 from damdo/should-bool-add-failure-message

🌱 test: add failure message to Eventually().Should(bool) everywhere

api/v1beta2/awscluster_webhook_test.go

@@ -18,6 +18,7 @@ package v1beta2
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"testing"
 	"time"
@@ -597,7 +598,7 @@ func TestAWSClusterValidateCreate(t *testing.T) {
 			g.Eventually(func() bool {
 				err := testEnv.Get(ctx, key, c)
 				return err == nil
-			}, 10*time.Second).Should(BeTrue())
+			}, 10*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed getting the newly created cluster %q", cluster.Name))
 
 			if tt.expect != nil {
 				tt.expect(g, c.Spec.ControlPlaneLoadBalancer)

controllers/awscluster_controller_test.go

@@ -215,7 +215,7 @@ func TestAWSClusterReconcilerIntegrationTests(t *testing.T) {
 			}
 			err := testEnv.Get(ctx, key, cluster)
 			return err == nil
-		}, 10*time.Second).Should(BeTrue())
+		}, 10*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed getting the newly created cluster %q", awsCluster.Name))
 
 		defer teardown()
 		defer t.Cleanup(func() {
@@ -316,7 +316,7 @@ func TestAWSClusterReconcilerIntegrationTests(t *testing.T) {
 			}
 			err := testEnv.Get(ctx, key, cluster)
 			return err == nil
-		}, 10*time.Second).Should(BeTrue())
+		}, 10*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed getting the newly created cluster %q", awsCluster.Name))
 
 		defer teardown()
 		defer t.Cleanup(func() {
@@ -424,7 +424,7 @@ func TestAWSClusterReconcilerIntegrationTests(t *testing.T) {
 			}
 			err := testEnv.Get(ctx, key, cluster)
 			return err == nil
-		}, 10*time.Second).Should(BeTrue())
+		}, 10*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed getting the newly created cluster %q", awsCluster.Name))
 
 		defer teardown()
 		defer t.Cleanup(func() {
@@ -532,7 +532,8 @@ func TestAWSClusterReconcilerIntegrationTests(t *testing.T) {
 			}
 			err := testEnv.Get(ctx, key, cluster)
 			return err == nil
-		}, 10*time.Second).Should(BeTrue())
+		}, 10*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed getting the newly created cluster %q", awsCluster.Name))
+
 		defer t.Cleanup(func() {
 			g.Expect(testEnv.Cleanup(ctx, &awsCluster, controllerIdentity, ns)).To(Succeed())
 		})
@@ -597,7 +598,7 @@ func TestAWSClusterReconcilerIntegrationTests(t *testing.T) {
 			}
 			err := testEnv.Get(ctx, key, cluster)
 			return err == nil
-		}, 10*time.Second).Should(BeTrue())
+		}, 10*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed getting the newly created cluster %q", awsCluster.Name))
 
 		defer t.Cleanup(func() {
 			g.Expect(testEnv.Cleanup(ctx, &awsCluster, controllerIdentity, ns)).To(Succeed())

controllers/awscluster_controller_unit_test.go

@@ -628,7 +628,7 @@ func createCluster(g *WithT, awsCluster *infrav1.AWSCluster, namespace string) {
 			}
 			err := testEnv.Get(ctx, key, cluster)
 			return err == nil
-		}, 10*time.Second).Should(BeTrue())
+		}, 10*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed getting the newly created cluster %q", awsCluster.Name))
 	}
 }
 

controllers/awsmachine_controller_test.go

@@ -450,7 +450,7 @@ func createAWSMachine(g *WithT, awsMachine *infrav1.AWSMachine) {
 			Namespace: awsMachine.Namespace,
 		}
 		return testEnv.Get(ctx, key, machine) == nil
-	}, 10*time.Second).Should(BeTrue())
+	}, 10*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed get the newly created machine %q", awsMachine.Name))
 }
 
 func getAWSMachine() *infrav1.AWSMachine {

controllers/awsmachine_controller_unit_test.go

@@ -2459,7 +2459,7 @@ func TestAWSMachineReconcilerReconcile(t *testing.T) {
 					}
 					err = testEnv.Get(ctx, key, machine)
 					return err == nil
-				}, 10*time.Second).Should(BeTrue())
+				}, 10*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed get the newly created machine %q", tc.awsMachine.Name))
 
 				result, err := reconciler.Reconcile(ctx, ctrl.Request{
 					NamespacedName: client.ObjectKey{

controlplane/eks/controllers/awsmanagedcontrolplane_controller_test.go

@@ -141,7 +141,7 @@ func TestAWSManagedControlPlaneReconcilerIntegrationTests(t *testing.T) {
 			}
 			err := testEnv.Get(ctx, key, controlPlane)
 			return err == nil
-		}, 10*time.Second).Should(BeTrue())
+		}, 10*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed getting the newly created AWSManagedControlPlane %q", awsManagedControlPlane.Name))
 
 		defer t.Cleanup(func() {
 			g.Expect(testEnv.Cleanup(ctx, &cluster, &awsManagedCluster, &awsManagedControlPlane, controllerIdentity, ns)).To(Succeed())

exp/controlleridentitycreator/awscontrolleridentity_controller_test.go

@@ -54,6 +54,6 @@ func TestAWSControllerIdentityController(t *testing.T) {
 				return true
 			}
 			return false
-		}, 10*time.Second).Should(BeTrue())
+		}, 10*time.Second).Should(BeTrue(), "Eventually failed ensuring AWSClusterControllerIdentity instance is created")
 	})
 }

exp/instancestate/awsinstancestate_controller_test.go

@@ -135,14 +135,14 @@ func TestAWSInstanceStateController(t *testing.T) {
 				exist = exist && ok
 			}
 			return exist
-		}, 10*time.Second).Should(BeTrue())
+		}, 10*time.Second).Should(BeTrue(), "Eventually failed ensuring queue URLs are up-to-date")
 
 		deleteAWSCluster(g, "aws-cluster-2")
 		t.Log("Ensuring we stop tracking deleted queue")
 		g.Eventually(func() bool {
 			_, ok := instanceStateReconciler.queueURLs.Load("aws-cluster-2")
 			return ok
-		}, 10*time.Second).Should(BeFalse())
+		}, 10*time.Second).Should(BeFalse(), "Eventually failed ensuring we stop tracking deleted queue")
 
 		persistObject(g, createAWSCluster("aws-cluster-3"))
 		t.Log("Ensuring newly created cluster is added to tracked clusters")
@@ -153,7 +153,7 @@ func TestAWSInstanceStateController(t *testing.T) {
 				exist = exist && ok
 			}
 			return exist
-		}, 10*time.Second).Should(BeTrue())
+		}, 10*time.Second).Should(BeTrue(), "Eventually failed ensuring newly created cluster is added to the tracked clusters")
 
 		t.Log("Ensuring machine is labelled with correct instance state")
 		g.Eventually(func() bool {
@@ -166,7 +166,7 @@ func TestAWSInstanceStateController(t *testing.T) {
 			labels := m.GetLabels()
 			val := labels[Ec2InstanceStateLabelKey]
 			return val == "shutting-down"
-		}, 10*time.Second).Should(BeTrue())
+		}, 10*time.Second).Should(BeTrue(), "Eventually failed ensuring machine is labelled with correct instance state")
 	})
 }
 

exp/instancestate/helpers_test.go

@@ -18,6 +18,7 @@ package instancestate
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -50,7 +51,7 @@ func persistObject(g *WithT, o client.Object) {
 	g.Eventually(func() bool {
 		err := k8sClient.Get(ctx, lookupKey, o)
 		return err == nil
-	}, time.Second*10).Should(BeTrue())
+	}, time.Second*10).Should(BeTrue(), fmt.Sprintf("Eventually failed getting the newly created object %v", lookupKey))
 }
 
 func deleteAWSCluster(g *WithT, name string) {

test/e2e/shared/aws.go

@@ -509,7 +509,7 @@ func deleteResourcesInCloudFormation(prov client.ConfigProvider, t *cfn_bootstra
 			}
 			code, ok := awserrors.Code(err)
 			return err == nil || (ok && code == iam.ErrCodeNoSuchEntityException)
-		}, 5*time.Minute, 5*time.Second).Should(BeTrue())
+		}, 5*time.Minute, 5*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed deleting the following role: %q", role.RoleName))
 	}
 	for _, profile := range instanceProfiles {
 		By(fmt.Sprintf("cleanup for profile with name '%s'", profile.InstanceProfileName))
@@ -522,7 +522,7 @@ func deleteResourcesInCloudFormation(prov client.ConfigProvider, t *cfn_bootstra
 			}
 			code, ok := awserrors.Code(err)
 			return err == nil || (ok && code == iam.ErrCodeNoSuchEntityException)
-		}, 5*time.Minute, 5*time.Second).Should(BeTrue())
+		}, 5*time.Minute, 5*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed cleaning up profile with name %q", profile.InstanceProfileName))
 	}
 	for _, group := range groups {
 		repeat := false
@@ -534,7 +534,7 @@ func deleteResourcesInCloudFormation(prov client.ConfigProvider, t *cfn_bootstra
 			}
 			code, ok := awserrors.Code(err)
 			return err == nil || (ok && code == iam.ErrCodeNoSuchEntityException)
-		}, 5*time.Minute, 5*time.Second).Should(BeTrue())
+		}, 5*time.Minute, 5*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed deleting group %q", group.GroupName))
 	}
 	for _, policy := range policies {
 		policies, err := iamSvc.ListPolicies(&iam.ListPoliciesInput{})
@@ -554,7 +554,7 @@ func deleteResourcesInCloudFormation(prov client.ConfigProvider, t *cfn_bootstra
 						}
 						code, ok := awserrors.Code(err)
 						return err == nil || (ok && code == iam.ErrCodeNoSuchEntityException)
-					}, 5*time.Minute, 5*time.Second).Should(BeTrue())
+					}, 5*time.Minute, 5*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed to delete policy %q", p.String()))
 					// TODO: why is there a break here? Don't we want to clean up everything?
 					break
 				}
@@ -1130,7 +1130,7 @@ func WaitForInstanceState(e2eCtx *E2EContext, clusterName string, state string)
 			return true
 		}
 		return false
-	}, 5*time.Minute, 5*time.Second).Should(BeTrue())
+	}, 5*time.Minute, 5*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed waiting for all cluster's EC2 instance to be in %q state", state))
 
 	return false
 }
@@ -1547,7 +1547,7 @@ func WaitForNatGatewayState(e2eCtx *E2EContext, gatewayID string, state string)
 		gw, _ := GetNatGateway(e2eCtx, gatewayID)
 		gwState := *gw.State
 		return gwState == state
-	}, 3*time.Minute, 5*time.Second).Should(BeTrue())
+	}, 3*time.Minute, 5*time.Second).Should(BeTrue(), fmt.Sprintf("Eventually failed waiting for NAT Gateway to be in %q state", state))
 	return false
 }
 

test/e2e/shared/suite.go

@@ -144,7 +144,7 @@ func Node1BeforeSuite(e2eCtx *E2EContext) []byte {
 				success = false
 			}
 			return success
-		}, 10*time.Minute, 5*time.Second).Should(BeTrue())
+		}, 10*time.Minute, 5*time.Second).Should(BeTrue(), "Should've eventually succeeded creating an AWS CloudFormation stack")
 	}
 
 	ensureStackTags(e2eCtx.AWSSession, bootstrapTemplate.Spec.StackName, bootstrapTags)

test/e2e/suites/managed/addon_helpers.go

@@ -65,7 +65,7 @@ func waitForEKSAddonToHaveStatus(input waitForEKSAddonToHaveStatusInput, interva
 		}
 
 		return false, nil
-	}, intervals...).Should(BeTrue())
+	}, intervals...).Should(BeTrue(), fmt.Sprintf("Eventually failed waiting for EKS addon %q to have status %q for EKS cluster %q", input.AddonName, input.AddonStatus, input.ControlPlane.Spec.EKSClusterName))
 }
 
 type checkEKSAddonConfigurationInput struct {
@@ -102,5 +102,5 @@ func checkEKSAddonConfiguration(input checkEKSAddonConfigurationInput, intervals
 		}
 
 		return false, nil
-	}, intervals...).Should(BeTrue())
+	}, intervals...).Should(BeTrue(), fmt.Sprintf("Eventually failed waiting for EKS addon %q to have config %q for EKS cluster %q", input.AddonName, input.AddonConfiguration, input.ControlPlane.Spec.EKSClusterName))
 }

test/e2e/suites/managed/control_plane_helpers.go

@@ -68,7 +68,7 @@ func waitForControlPlaneToBeUpgraded(input waitForControlPlaneToBeUpgradedInput,
 		default:
 			return false, nil
 		}
-	}, intervals...).Should(BeTrue())
+	}, intervals...).Should(BeTrue(), fmt.Sprintf("Eventually failed waiting for EKS control-plane to be upgraded to kubernetes version %q", input.UpgradeVersion))
 }
 
 type GetControlPlaneByNameInput struct {
@@ -89,7 +89,7 @@ func GetControlPlaneByName(ctx context.Context, input GetControlPlaneByNameInput
 			return err
 		}
 		return nil
-	}, 2*time.Minute, 5*time.Second).Should(Succeed())
+	}, 2*time.Minute, 5*time.Second).Should(Succeed(), fmt.Sprintf("Eventually failed to get AWSManagedControlPlane object '%s/%s'", input.Namespace, input.Name))
 	Expect(input.Getter.Get(ctx, key, cp)).To(Succeed(), "Failed to get AWSManagedControlPlane object %s/%s", input.Namespace, input.Name)
 	return cp
 }

test/e2e/suites/managed/machine_deployment_helpers.go

@@ -58,7 +58,7 @@ func waitForMachineDeploymentDeleted(ctx context.Context, input waitForMachineDe
 		err := input.Getter.Get(ctx, key, mp)
 		notFound := apierrors.IsNotFound(err)
 		return notFound
-	}, intervals...).Should(BeTrue())
+	}, intervals...).Should(BeTrue(), fmt.Sprintf("Eventually failed waiting for MachineDeployment %q to be deleted", input.MachineDeployment.GetName()))
 }
 
 type waitForMachineDeletedInput struct {
@@ -77,5 +77,5 @@ func waitForMachineDeleted(ctx context.Context, input waitForMachineDeletedInput
 		err := input.Getter.Get(ctx, key, mp)
 		notFound := apierrors.IsNotFound(err)
 		return notFound
-	}, intervals...).Should(BeTrue())
+	}, intervals...).Should(BeTrue(), fmt.Sprintf("Eventually failed waiting for Machine %q to be deleted", input.Machine.GetName()))
 }

test/e2e/suites/managed/machine_pool_helpers.go

@@ -58,5 +58,5 @@ func waitForMachinePoolDeleted(ctx context.Context, input waitForMachinePoolDele
 		err := input.Getter.Get(ctx, key, mp)
 		notFound := apierrors.IsNotFound(err)
 		return notFound
-	}, intervals...).Should(BeTrue())
+	}, intervals...).Should(BeTrue(), fmt.Sprintf("Eventually failed waiting for machine pool %q to be deleted", input.MachinePool.GetName()))
 }

test/e2e/suites/unmanaged/helpers_test.go

@@ -703,7 +703,7 @@ func waitForStatefulSetRunning(info statefulSetInfo, k8sclient crclient.Client)
 			}
 			return *statefulset.Spec.Replicas == statefulset.Status.ReadyReplicas, nil
 		}, 10*time.Minute, 30*time.Second,
-	).Should(BeTrue())
+	).Should(BeTrue(), fmt.Sprintf("Eventually failed waiting for StatefulSet %s to be running", info.name))
 }
 
 // LatestCIReleaseForVersion returns the latest ci release of a specific version.

test/e2e/suites/unmanaged/unmanaged_functional_test.go

@@ -566,7 +566,7 @@ var _ = ginkgo.Context("[unmanaged] [functional]", func() {
 				subnetError := "Failed to create instance: failed to run instance: InvalidSubnetID.NotFound: " +
 					"The subnet ID '%s' does not exist"
 				return isErrorEventExists(namespace.Name, md1Name, "FailedCreate", fmt.Sprintf(subnetError, "invalid-subnet"), eventList)
-			}, e2eCtx.E2EConfig.GetIntervals("", "wait-worker-nodes")...).Should(BeTrue())
+			}, e2eCtx.E2EConfig.GetIntervals("", "wait-worker-nodes")...).Should(BeTrue(), "Eventually failed waiting for 'invalid subnet ID' event to be reported")
 
 			ginkgo.By("Creating Machine Deployment in non-configured Availability Zone")
 			md2Name := clusterName + "-md-2"
@@ -584,7 +584,7 @@ var _ = ginkgo.Context("[unmanaged] [functional]", func() {
 				eventList := getEvents(namespace.Name)
 				azError := "Failed to create instance: no subnets available in availability zone \"%s\""
 				return isErrorEventExists(namespace.Name, md2Name, "FailedCreate", fmt.Sprintf(azError, *invalidAz), eventList)
-			}, e2eCtx.E2EConfig.GetIntervals("", "wait-worker-nodes")...).Should(BeTrue())
+			}, e2eCtx.E2EConfig.GetIntervals("", "wait-worker-nodes")...).Should(BeTrue(), "Eventually failed waiting for 'no subnet available in AZ' event to be reported")
 		})
 	})
 
@@ -705,7 +705,7 @@ var _ = ginkgo.Context("[unmanaged] [functional]", func() {
 					machineList := getAWSMachinesForDeployment(ns2.Name, *md2[0])
 					labels := machineList.Items[0].GetLabels()
 					return labels[instancestate.Ec2InstanceStateLabelKey] == string(infrav1.InstanceStateTerminated)
-				}, e2eCtx.E2EConfig.GetIntervals("", "wait-machine-status")...).Should(BeTrue())
+				}, e2eCtx.E2EConfig.GetIntervals("", "wait-machine-status")...).Should(BeTrue(), "Eventually failed waiting for AWSMachine to be labelled as terminated")
 
 				ginkgo.By("Waiting for machine to reach Failed state")
 				statusChecks := []framework.MachineStatusCheck{framework.MachinePhaseCheck(string(clusterv1.MachinePhaseFailed))}
@@ -878,7 +878,7 @@ var _ = ginkgo.Context("[unmanaged] [functional]", func() {
 				}
 				wlClusterInfra.Peering = aPeering
 				return aPeering != nil
-			}, 60*time.Second).Should(BeTrue())
+			}, 60*time.Second).Should(BeTrue(), "Eventually failed waiting for peering to be accepted")
 
 			ginkgo.By("Creating security groups")
 			mgmtSG, _ := shared.CreateSecurityGroup(e2eCtx, mgmtClusterName+"-all", mgmtClusterName+"-all", *mgmtClusterInfra.VPC.VpcId)
@@ -1135,7 +1135,8 @@ var _ = ginkgo.Context("[unmanaged] [functional]", func() {
 				Expect(err).To(BeNil())
 				return conditions.IsFalse(awsCluster, infrav1.VpcEndpointsReadyCondition) &&
 					conditions.GetReason(awsCluster, infrav1.VpcEndpointsReadyCondition) == clusterv1.DeletedReason
-			}, e2eCtx.E2EConfig.GetIntervals("", "wait-delete-cluster")...).Should(BeTrue())
+			}, e2eCtx.E2EConfig.GetIntervals("", "wait-delete-cluster")...).Should(BeTrue(),
+				"Eventually failed waiting for AWSCluster to show VPC endpoint as deleted in conditions")
 		})
 	})
 })