Remove using node ports for L4 NetLB RBS Services

NodePorts are not needed for NetLB RBS and were copied from legacy
implementation. They were also slowing down our controller, because we
were updating same instance group with node port for every service

pkg/instances/instances.go

@@ -122,7 +122,7 @@ func (i *Instances) ensureInstanceGroupAndPorts(name, zone string, ports []int64
 			return nil, err
 		}
 	} else {
-		klog.V(5).Infof("Instance group %v/%v already exists.", zone, name)
+		klog.V(2).Infof("Instance group %v/%v already exists.", zone, name)
 	}
 
 	// Build map of existing ports

pkg/l4lb/l4netlbcontroller.go

@@ -19,6 +19,7 @@ package l4lb
 import (
 	"fmt"
 	"reflect"
+	"time"
 
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
@@ -428,6 +429,12 @@ func (lc *L4NetLBController) syncInternal(service *v1.Service) *loadbalancers.L4
 		return nil
 	}
 
+	startTime := time.Now()
+	klog.Infof("Syncing L4 NetLB RBS service %s/%s", service.Namespace, service.Name)
+	defer func() {
+		klog.Infof("Finished syncing L4 NetLB RBS service %s/%s, time taken: %v", service.Namespace, service.Name, time.Since(startTime))
+	}()
+
 	if err := common.EnsureServiceFinalizer(service, common.NetLBFinalizerV2, lc.ctx.KubeClient); err != nil {
 		return &loadbalancers.L4NetLBSyncResult{Error: fmt.Errorf("Failed to attach L4 External LoadBalancer finalizer to service %s/%s, err %w", service.Namespace, service.Name, err)}
 	}
@@ -479,6 +486,11 @@ func (lc *L4NetLBController) syncInternal(service *v1.Service) *loadbalancers.L4
 }
 
 func (lc *L4NetLBController) ensureBackendLinking(service *v1.Service) error {
+	start := time.Now()
+	klog.V(2).Infof("Linking backend service with instance groups for service %s/%s", service.Namespace, service.Name)
+	defer func() {
+		klog.V(2).Infof("Finished linking backend service with instance groups for service %s/%s, time taken: %v", service.Namespace, service.Name, time.Since(start))
+	}()
 	zones, err := lc.translator.ListZones(utils.CandidateNodesPredicate)
 	if err != nil {
 		return err
@@ -489,7 +501,6 @@ func (lc *L4NetLBController) ensureBackendLinking(service *v1.Service) error {
 	servicePort := utils.ServicePort{
 		ID:           portId,
 		BackendNamer: lc.namer,
-		NodePort:     utils.GetServiceNodePort(service),
 		L4RBSEnabled: true,
 	}
 
@@ -499,16 +510,30 @@ func (lc *L4NetLBController) ensureBackendLinking(service *v1.Service) error {
 func (lc *L4NetLBController) ensureInstanceGroups(service *v1.Service, nodeNames []string) error {
 	// TODO(kl52752) Move instance creation and deletion logic to NodeController
 	// to avoid race condition between controllers
-	nodePorts := utils.GetNodePorts(service.Spec.Ports)
-	_, err := lc.instancePool.EnsureInstanceGroupsAndPorts(lc.ctx.ClusterNamer.InstanceGroup(), nodePorts)
+	start := time.Now()
+	klog.V(2).Infof("Ensuring instance groups for L4 NetLB Service %s/%s, len(nodeNames): %v", service.Namespace, service.Name, len(nodeNames))
+	defer func() {
+		klog.V(2).Infof("Finished ensuring instance groups for L4 NetLB Service %s/%s, time taken: %v", service.Namespace, service.Name, time.Since(start))
+	}()
+
+	// L4 NetLB does not use node ports, so we provide empty slice
+	var nodePorts []int64
+	igName := lc.ctx.ClusterNamer.InstanceGroup()
+	_, err := lc.instancePool.EnsureInstanceGroupsAndPorts(igName, nodePorts)
 	if err != nil {
-		return err
+		return fmt.Errorf("lc.instancePool.EnsureInstanceGroupsAndPorts(%s, %v) returned error %w", igName, nodePorts, err)
 	}
+
 	return lc.instancePool.Sync(nodeNames)
 }
 
 // garbageCollectRBSNetLB cleans-up all gce resources related to service and removes NetLB finalizer
 func (lc *L4NetLBController) garbageCollectRBSNetLB(key string, svc *v1.Service) *loadbalancers.L4NetLBSyncResult {
+	startTime := time.Now()
+	klog.Infof("Deleting L4 NetLB RBS service %s/%s", svc.Namespace, svc.Name)
+	defer func() {
+		klog.Infof("Finished deleting L4 NetLB service %s/%s, time taken: %v", svc.Namespace, svc.Name, time.Since(startTime))
+	}()
 	l4NetLBParams := &loadbalancers.L4NetLBParams{
 		Service:  svc,
 		Cloud:    lc.ctx.Cloud,

pkg/l4lb/l4netlbcontroller_test.go

@@ -650,18 +650,16 @@ func TestProcessServiceCreationFailed(t *testing.T) {
 		addMockFunc   func(*cloud.MockGCE)
 		expectedError string
 	}{{addMockFunc: func(c *cloud.MockGCE) { c.MockInstanceGroups.GetHook = test.GetErrorInstanceGroupHook },
-		expectedError: "GetErrorInstanceGroupHook"},
+		expectedError: "lc.instancePool.EnsureInstanceGroupsAndPorts(k8s-ig--aaaaa, []) returned error GetErrorInstanceGroupHook"},
 		{addMockFunc: func(c *cloud.MockGCE) { c.MockInstanceGroups.ListHook = test.ListErrorHook },
 			expectedError: "ListErrorHook"},
 		{addMockFunc: func(c *cloud.MockGCE) { c.MockInstanceGroups.InsertHook = test.InsertErrorHook },
-			expectedError: "InsertErrorHook"},
+			expectedError: "lc.instancePool.EnsureInstanceGroupsAndPorts(k8s-ig--aaaaa, []) returned error InsertErrorHook"},
 
 		{addMockFunc: func(c *cloud.MockGCE) { c.MockInstanceGroups.AddInstancesHook = test.AddInstancesErrorHook },
 			expectedError: "AddInstances: [AddInstancesErrorHook]"},
 		{addMockFunc: func(c *cloud.MockGCE) { c.MockInstanceGroups.ListInstancesHook = test.ListInstancesWithErrorHook },
 			expectedError: "ListInstancesWithErrorHook"},
-		{addMockFunc: func(c *cloud.MockGCE) { c.MockInstanceGroups.SetNamedPortsHook = test.SetNamedPortsErrorHook },
-			expectedError: "SetNamedPortsErrorHook"},
 	} {
 		lc := newL4NetLBServiceController()
 		param.addMockFunc((lc.ctx.Cloud.Compute().(*cloud.MockGCE)))