From 2663eec7a2107bbdade28d76c4ecd4e6eebdfbfe Mon Sep 17 00:00:00 2001
From: Jakub Warczarek <jakub.warczarek@konghq.com>
Date: Tue, 9 May 2023 16:02:31 +0200
Subject: [PATCH] CR improvement

---
 pkg/utils/kubernetes/networking/services.go | 37 ++++++++++-----------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/pkg/utils/kubernetes/networking/services.go b/pkg/utils/kubernetes/networking/services.go
index 169234397..a6435b928 100644
--- a/pkg/utils/kubernetes/networking/services.go
+++ b/pkg/utils/kubernetes/networking/services.go
@@ -56,7 +56,8 @@ func WaitForServiceLoadBalancerAddress(ctx context.Context, c kubernetes.Interfa
 // type Service to allow connections to the Service and port from outside the cluster while
 // the connection attempts are made using the LoadBalancer public address.
 func WaitForConnectionOnServicePort(ctx context.Context, c kubernetes.Interface, namespace, name string, port int, dialTimeout time.Duration) error {
-	service, err := c.CoreV1().Services(namespace).Get(ctx, name, metav1.GetOptions{})
+	svcClient := c.CoreV1().Services(namespace)
+	service, err := svcClient.Get(ctx, name, metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
@@ -83,7 +84,8 @@ func WaitForConnectionOnServicePort(ctx context.Context, c kubernetes.Interface,
 	// the endpoints of the target server is manually created, but not chosen from pods by labels in selector.
 	// so we need to manually create the same endpoints as the target service has here.
 	if len(service.Spec.Selector) == 0 {
-		endpointSlices, err := c.DiscoveryV1().EndpointSlices(namespace).List(
+		epsClient := c.DiscoveryV1().EndpointSlices(namespace)
+		endpointSlices, err := epsClient.List(
 			ctx, metav1.ListOptions{LabelSelector: discoveryv1.LabelServiceName + "=" + name},
 		)
 		if err != nil {
@@ -93,39 +95,35 @@ func WaitForConnectionOnServicePort(ctx context.Context, c kubernetes.Interface,
 		// Recreate EndpointSlices for the lb service with proper metadata.
 		tempEndpointSlices := endpointSlices.DeepCopy().Items
 		for i := range tempEndpointSlices {
+			epsName := fmt.Sprintf("%s-%d", lbServiceName, i)
 			tempEndpointSlices[i].ObjectMeta = metav1.ObjectMeta{
 				Namespace: namespace,
-				Name:      fmt.Sprintf("%s-%d", lbServiceName, i),
+				Name:      epsName,
 				Labels: map[string]string{
 					discoveryv1.LabelServiceName: lbServiceName, // Maps EndpointSlice to Service.
 					correspondingSvcNameLabel:    name,
 				},
 			}
-			_, err = c.DiscoveryV1().EndpointSlices(namespace).Create(ctx, &tempEndpointSlices[i], metav1.CreateOptions{})
-			if err != nil {
+			if _, err = epsClient.Create(ctx, &tempEndpointSlices[i], metav1.CreateOptions{}); err != nil {
 				return err
 			}
-		}
-
-		defer func() {
-			for _, eps := range tempEndpointSlices {
-				err := c.DiscoveryV1().EndpointSlices(namespace).Delete(ctx, eps.Name, metav1.DeleteOptions{})
+			// For each successfully created temporary EndpointSlice ensure deletion on return from the function.
+			defer func(epsName string) {
+				err := epsClient.Delete(ctx, epsName, metav1.DeleteOptions{})
 				if err != nil && !errors.IsNotFound(err) {
-					fmt.Printf("failed to delete endpoints %s/%s after testing, error %v\n",
-						namespace, eps.Name, err,
+					fmt.Printf("failed to delete EndpointSlice %s/%s after testing, error %v\n",
+						namespace, epsName, err,
 					)
 				}
-			}
-		}()
+			}(epsName)
+		}
 	}
 
-	_, err = c.CoreV1().Services(namespace).Create(ctx, tempLoadBalancer, metav1.CreateOptions{})
-	if err != nil {
+	if _, err = svcClient.Create(ctx, tempLoadBalancer, metav1.CreateOptions{}); err != nil {
 		return err
 	}
-
 	defer func() {
-		err := c.CoreV1().Services(namespace).Delete(ctx, lbServiceName, metav1.DeleteOptions{})
+		err := svcClient.Delete(ctx, lbServiceName, metav1.DeleteOptions{})
 		if err != nil && !errors.IsNotFound(err) {
 			fmt.Printf("failed to delete service %s/%s after testing, error %v\n",
 				namespace, lbServiceName, err)
@@ -145,8 +143,7 @@ func WaitForConnectionOnServicePort(ctx context.Context, c kubernetes.Interface,
 			return fmt.Errorf("context completed while waiting for %s:%d to be connected", ip, port)
 		case <-ticker.C:
 			dialer := &net.Dialer{Timeout: dialTimeout}
-			_, err := dialer.Dial("tcp", address)
-			if err == nil {
+			if _, err := dialer.Dial("tcp", address); err == nil {
 				return nil
 			}
 		}