PR feedback

stackrox · Dec 19, 2024 · 86a3959 · 86a3959
1 parent 22c1b7b
commit 86a3959
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 12 deletions.
diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go
@@ -158,7 +158,7 @@ var _ = Describe("Central", Ordered, func() {
 		It("should not expose URLs until the routes are created", func() {
 			testutil.SkipIf(!routesEnabled, skipRouteMsg)
 			var centralRequest public.CentralRequest
-			Expect(testutil.ObtainCentralRequest(ctx, client, centralRequestID, &centralRequest)).
+			Expect(testutil.GetCentralRequest(ctx, client, centralRequestID, &centralRequest)).
 				To(Succeed())
 			Expect(centralRequest.CentralUIURL).To(BeEmpty())
 			Expect(centralRequest.CentralDataURL).To(BeEmpty())
@@ -175,7 +175,7 @@ var _ = Describe("Central", Ordered, func() {
 			testutil.SkipIf(!routesEnabled, skipRouteMsg)
 
 			var centralRequest public.CentralRequest
-			Expect(testutil.ObtainCentralRequest(ctx, client, centralRequestID, &centralRequest)).
+			Expect(testutil.GetCentralRequest(ctx, client, centralRequestID, &centralRequest)).
 				To(Succeed())
 
 			var reencryptRoute openshiftRouteV1.Route
@@ -207,7 +207,7 @@ var _ = Describe("Central", Ordered, func() {
 			testutil.SkipIf(!dnsEnabled, testutil.SkipDNSMsg)
 
 			var centralRequest public.CentralRequest
-			Expect(testutil.ObtainCentralRequest(ctx, client, centralRequestID, &centralRequest)).
+			Expect(testutil.GetCentralRequest(ctx, client, centralRequestID, &centralRequest)).
 				To(Succeed())
 
 			var reencryptIngress openshiftRouteV1.RouteIngress
@@ -355,7 +355,7 @@ var _ = Describe("Central", Ordered, func() {
 		It("should delete external DNS entries", func() {
 			testutil.SkipIf(!dnsEnabled, testutil.SkipDNSMsg)
 			var centralRequest public.CentralRequest
-			Expect(testutil.ObtainCentralRequest(ctx, client, centralRequestID, &centralRequest)).
+			Expect(testutil.GetCentralRequest(ctx, client, centralRequestID, &centralRequest)).
 				To(Succeed())
 			dnsRecordsLoader := dns.NewRecordsLoader(route53Client, centralRequest)
 			Eventually(dnsRecordsLoader.LoadDNSRecords).
@@ -435,7 +435,7 @@ var _ = Describe("Central", Ordered, func() {
 		It("should delete external DNS entries", func() {
 			testutil.SkipIf(!dnsEnabled, testutil.SkipDNSMsg)
 			var centralRequest public.CentralRequest
-			Expect(testutil.ObtainCentralRequest(ctx, client, centralRequestID, &centralRequest)).
+			Expect(testutil.GetCentralRequest(ctx, client, centralRequestID, &centralRequest)).
 				To(Succeed())
 			dnsRecordsLoader := dns.NewRecordsLoader(route53Client, centralRequest)
 			Eventually(dnsRecordsLoader.LoadDNSRecords).
@@ -479,7 +479,7 @@ var _ = Describe("Central", Ordered, func() {
 				WithTimeout(extendedWaitTimeout).
 				WithPolling(defaultPolling).
 				Should(Succeed())
-			Expect(testutil.ObtainCentralRequest(ctx, client, centralRequestID, &readyCentralRequest)).
+			Expect(testutil.GetCentralRequest(ctx, client, centralRequestID, &readyCentralRequest)).
 				To(Succeed())
 		})
 

diff --git a/e2e/multicluster/multicluster_migration_test.go b/e2e/multicluster/multicluster_migration_test.go
@@ -106,7 +106,7 @@ var _ = Describe("Central Migration Test", Ordered, func() {
 
 		It("should have DNS CNAME records for cluster1 routes", func() {
 			testutil.SkipIf(!dnsEnabled, testutil.SkipDNSMsg)
-			testutil.ObtainCentralRequest(context.Background(), fleetmanagerClient, centralRequest.Id, &centralRequest)
+			testutil.GetCentralRequest(context.Background(), fleetmanagerClient, centralRequest.Id, &centralRequest)
 
 			dnsRecordsLoader := dns.NewRecordsLoader(route53Client, centralRequest)
 			routeService := k8s.NewRouteService(cluster1KubeClient, routeConfig)
@@ -157,7 +157,7 @@ var _ = Describe("Central Migration Test", Ordered, func() {
 		})
 		It("should have DNS CNAME records for cluster2 routes", func() {
 			testutil.SkipIf(!dnsEnabled, testutil.SkipDNSMsg)
-			testutil.ObtainCentralRequest(context.Background(), fleetmanagerClient, centralRequest.Id, &centralRequest)
+			testutil.GetCentralRequest(context.Background(), fleetmanagerClient, centralRequest.Id, &centralRequest)
 
 			dnsRecordsLoader := dns.NewRecordsLoader(route53Client, centralRequest)
 			routeService := k8s.NewRouteService(cluster2KubeClient, routeConfig)
@@ -229,8 +229,9 @@ var _ = Describe("Central Migration Test", Ordered, func() {
 func assertClusterAssignment(expectedClusterID string, centralID string, adminAPI fleetmanager.AdminAPI) {
 	var clusterAssignment string
 	Eventually(func() (err error) {
-		// assert the cluster ID outside the Eventually, since once we have a non-empty
-		// clusterAssignment it will not change so there is no need to keep polling
+		// Assert the cluster ID outside the Eventually. Once we have a non-empty
+		// return value for clusterAssignment, the value will never change. Because of that
+		// there is no need to keep polling for a new clusterAssignment if it does not match.
 		clusterAssignment, err = getClusterAssignment(centralID, adminAPI)
 		return err
 	}).
@@ -257,6 +258,7 @@ func getClusterAssignment(centralID string, adminAPI fleetmanager.AdminAPI) (str
 		if central.Id == centralID {
 			tenantExists = true
 			clusterID = central.ClusterId
+			break
 		}
 	}
 

diff --git a/e2e/testutil/testutil.go b/e2e/testutil/testutil.go
@@ -52,8 +52,8 @@ func SkipIf(condition bool, message string) {
 	}
 }
 
-// ObtainCentralRequest queries fleet-manager public API for the CentralRequest with id and stores in in the given pointer
-func ObtainCentralRequest(ctx context.Context, client *fleetmanager.Client, id string, request *public.CentralRequest) error {
+// GetCentralRequest queries fleet-manager public API for the CentralRequest with id and stores in in the given pointer
+func GetCentralRequest(ctx context.Context, client *fleetmanager.Client, id string, request *public.CentralRequest) error {
 	centralRequest, _, err := client.PublicAPI().GetCentralById(ctx, id)
 	if err != nil {
 		return fmt.Errorf("failed to obtain CentralRequest: %w", err)