feat(l4): Add support for enabling L4 NetLB Regional Backend Services by default

cmd/glbc/main.go

@@ -365,6 +365,7 @@ func main() {
 		EnableL4ILBZonalAffinity:                  flags.F.EnableL4ILBZonalAffinity,
 		EnableL4NetLBForwardingRulesOptimizations: flags.F.EnableL4NetLBForwardingRulesOptimizations,
 		ReadOnlyMode:                              flags.F.ReadOnlyMode,
+		EnableL4NetLBRBSByDefault:                 flags.F.EnableL4NetLBRBSByDefault,
 	}
 	ctx, err := ingctx.NewControllerContext(kubeClient, backendConfigClient, frontendConfigClient, firewallCRClient, svcNegClient, svcAttachmentClient, networkClient, nodeTopologyClient, eventRecorderKubeClient, cloud, namer, kubeSystemUID, ctxConfig, rootLogger)
 	if err != nil {

pkg/context/context.go

@@ -138,6 +138,7 @@ type ControllerContextConfig struct {
 	EnableL4ILBMixedProtocol                  bool
 	EnableL4NetLBMixedProtocol                bool
 	EnableL4NetLBForwardingRulesOptimizations bool
+	EnableL4NetLBRBSByDefault                 bool
 }
 
 // NewControllerContext returns a new shared set of informers.

pkg/flags/flags.go

@@ -149,6 +149,7 @@ var F = struct {
 	EnableNEGsForIngress                      bool
 	L4ILBLegacyHeadStartTime                  time.Duration
 	EnableIPv6NodeNEGEndpoints                bool
+	EnableL4NetLBRBSByDefault                 bool
 
 	// ===============================
 	// DEPRECATED FLAGS
@@ -362,6 +363,7 @@ L7 load balancing. CSV values accepted. Example: -node-port-ranges=80,8080,400-5
 	flag.BoolVar(&F.EnableNEGsForIngress, "enable-negs-for-ingress", true, "Allow the NEG controller to create NEGs for Ingress services.")
 	flag.DurationVar(&F.L4ILBLegacyHeadStartTime, "prevent-legacy-race-l4-ilb", 0*time.Second, "Delay before processing new L4 ILB services without existing finalizers. This gives the legacy controller a head start to claim the service, preventing a race condition upon service creation.")
 	flag.BoolVar(&F.EnableIPv6NodeNEGEndpoints, "enable-ipv6-node-neg-endpoints", false, "Enable populating IPv6 addresses for Node IPs in GCE_VM_IP NEGs.")
+	flag.BoolVar(&F.EnableL4NetLBRBSByDefault, "enable-l4-netlb-rbs-by-default", false, "Enable L4 NetLB Regional Backend Services by default for new L4 NetLB services.")
 }
 
 func Validate() {

pkg/l4lb/l4netlbcontroller.go

@@ -85,6 +85,7 @@ type L4NetLBController struct {
 	serviceVersions                    *serviceVersionsTracker
 	enableNEGSupport                   bool
 	enableNEGAsDefault                 bool
+	enableRBSDefault                   bool
 
 	hasSynced func() bool
 
@@ -121,6 +122,7 @@ func NewL4NetLBController(
 		serviceVersions:                    NewServiceVersionsTracker(),
 		logger:                             logger,
 		hasSynced:                          ctx.HasSynced,
+		enableRBSDefault:                   ctx.EnableL4NetLBRBSByDefault,
 	}
 	var networkLister cache.Indexer
 	if ctx.NetworkInformer != nil {
@@ -358,6 +360,7 @@ func (lc *L4NetLBController) shouldProcessService(newSvc, oldSvc *v1.Service, sv
 	warnL4FinalizerRemoved(lc.ctx, oldSvc, newSvc)
 
 	if !lc.isRBSBasedService(newSvc, svcLogger) && !lc.isRBSBasedService(oldSvc, svcLogger) {
+		svcLogger.V(4).Info("Ignoring non RBS based NetLB service")
 		return false, false
 	}
 	if lc.needsAddition(newSvc, oldSvc) || lc.needsUpdate(newSvc, oldSvc) || lc.needsDeletion(newSvc, svcLogger) {
@@ -383,25 +386,46 @@ func (lc *L4NetLBController) isRBSBasedService(svc *v1.Service, svcLogger klog.L
 	// Check if the type=LoadBalancer, so we don't execute API calls o non-LB services
 	// this call is nil-safe
 	if !utils.IsLoadBalancerServiceType(svc) {
+		svcLogger.V(4).Info("Service is not of type LoadBalancer")
 		return false
 	}
 	if svc.Spec.LoadBalancerClass != nil {
+		svcLogger.V(4).Info("Service has LoadBalancerClass annotation", "loadBalancerClass", *svc.Spec.LoadBalancerClass)
 		return annotations.HasLoadBalancerClass(svc, annotations.RegionalExternalLoadBalancerClass)
 	}
-	return annotations.HasRBSAnnotation(svc) || utils.HasL4NetLBFinalizerV2(svc) || utils.HasL4NetLBFinalizerV3(svc) || lc.hasRBSForwardingRule(svc, svcLogger)
+	if lc.enableRBSDefault {
+		svcLogger.V(4).Info("RBS is enabled by default, treating service as RBS based")
+		return true
+	}
+	if utils.HasL4NetLBFinalizerV2(svc) || utils.HasL4NetLBFinalizerV3(svc) {
+		svcLogger.V(4).Info("Service has L4 NetLB RBS finalizer")
+		return true
+	}
+	if annotations.HasRBSAnnotation(svc) {
+		svcLogger.V(4).Info("Service has RBS annotation")
+		return true
+	}
+	if lc.hasRBSForwardingRule(svc, svcLogger) {
+		svcLogger.V(4).Info("Service has RBS forwarding rule")
+		return true
+	}
+	return false
 }
 
 func (lc *L4NetLBController) preventLegacyServiceHandling(service *v1.Service, key string, svcLogger klog.Logger) (bool, error) {
-	if (annotations.HasRBSAnnotation(service) || annotations.HasLoadBalancerClass(service, annotations.RegionalExternalLoadBalancerClass)) && lc.hasTargetPoolForwardingRule(service, svcLogger) {
+	svcLogger.Info("Checking for legacy target pool service with RBS annotation or finalizers", "finalizers", service.ObjectMeta.Finalizers)
+	if lc.isRBSBasedService(service, svcLogger) && (lc.hasTargetPoolForwardingRule(service, svcLogger) || utils.HasL4NetLBFinalizerV1(service)) {
 		if utils.HasL4NetLBFinalizerV2(service) || utils.HasL4NetLBFinalizerV3(service) {
 			// If we found that RBS finalizer was attached to service, it means that RBS controller
 			// had a race condition on Service creation with Legacy Controller.
 			// It should only happen during service creation, and we should clean up RBS resources
+			svcLogger.Info("Detected Target Pool on RBS service with RBS finalizer. Cleaning up RBS resources to prevent race condition.", "finalizers", service.ObjectMeta.Finalizers)
 			return true, lc.preventTargetPoolRaceWithRBSOnCreation(service, key, svcLogger)
 		} else {
 			// Target Pool to RBS migration is NOT yet supported and causes service to break (for now).
 			// If we detect RBS annotation on legacy service, we remove RBS annotation,
 			// so service stays with Legacy Target Pool implementation
+			svcLogger.Info("Detected Target Pool on service with RBS annotation. Removing RBS annotation to prevent unsupported migration.", "finalizers", service.ObjectMeta.Finalizers)
 			return true, lc.preventExistingTargetPoolToRBSMigration(service, svcLogger)
 		}
 	}
@@ -411,6 +435,7 @@ func (lc *L4NetLBController) preventLegacyServiceHandling(service *v1.Service, k
 func (lc *L4NetLBController) hasTargetPoolForwardingRule(service *v1.Service, svcLogger klog.Logger) bool {
 	frName := utils.LegacyForwardingRuleName(service)
 	if lc.hasForwardingRuleAnnotation(service, frName) {
+		svcLogger.V(4).Info("Service does not have Target Pool forwarding rule annotation", "forwardingRule", frName)
 		return false
 	}
 
@@ -420,8 +445,10 @@ func (lc *L4NetLBController) hasTargetPoolForwardingRule(service *v1.Service, sv
 		return false
 	}
 	if existingFR != nil && existingFR.Target != "" {
+		svcLogger.V(4).Info("Service has Target Pool forwarding rule", "forwardingRule", frName, "targetPool", strings.Split(existingFR.Target, "/")[len(strings.Split(existingFR.Target, "/"))-1])
 		return true
 	}
+	svcLogger.V(4).Info("Service does not have Target Pool forwarding rule", "forwardingRule", frName)
 	return false
 }
 

pkg/l4lb/l4netlbcontroller_test.go

@@ -309,7 +309,7 @@ func getFakeGCECloud(vals gce.TestClusterValues) *gce.Cloud {
 	return fakeGCE
 }
 
-func buildContext(vals gce.TestClusterValues, readOnlyMode bool) (*ingctx.ControllerContext, error) {
+func buildContext(vals gce.TestClusterValues, readOnlyMode bool, isRBSdefault bool) (*ingctx.ControllerContext, error) {
 	fakeGCE := getFakeGCECloud(vals)
 	kubeClient := fake.NewSimpleClientset()
 	networkClient := netfake.NewSimpleClientset()
@@ -318,22 +318,31 @@ func buildContext(vals gce.TestClusterValues, readOnlyMode bool) (*ingctx.Contro
 	namer := namer.NewNamer(clusterUID, "", klog.TODO())
 
 	ctxConfig := ingctx.ControllerContextConfig{
-		Namespace:         v1.NamespaceAll,
-		ResyncPeriod:      1 * time.Minute,
-		NumL4NetLBWorkers: 5,
-		MaxIGSize:         1000,
-		ReadOnlyMode:      readOnlyMode,
+		Namespace:                 v1.NamespaceAll,
+		ResyncPeriod:              1 * time.Minute,
+		NumL4NetLBWorkers:         5,
+		MaxIGSize:                 1000,
+		ReadOnlyMode:              readOnlyMode,
+		EnableL4NetLBRBSByDefault: isRBSdefault,
 	}
 	return ingctx.NewControllerContext(kubeClient, nil, nil, nil, svcNegClient, nil, networkClient, nil, kubeClient /*kube client to be used for events*/, fakeGCE, namer, "" /*kubeSystemUID*/, ctxConfig, klog.TODO())
 }
 
 func newL4NetLBServiceController() *L4NetLBController {
-	return createL4NetLBServiceController(test.DefaultTestClusterValues(), false)
+	return createL4NetLBServiceController(test.DefaultTestClusterValues(), false, false)
 }
 
-func createL4NetLBServiceController(vals gce.TestClusterValues, readOnlyMode bool) *L4NetLBController {
+func newL4NetLBServiceControllerReadOnlyMode(readOnlyMode bool) *L4NetLBController {
+	return createL4NetLBServiceController(test.DefaultTestClusterValues(), readOnlyMode, false)
+}
+
+func newL4NetLBServiceControllerRBSDefault(isRBSdefault bool) *L4NetLBController {
+	return createL4NetLBServiceController(test.DefaultTestClusterValues(), false, isRBSdefault)
+}
+
+func createL4NetLBServiceController(vals gce.TestClusterValues, readOnlyMode bool, isRBSdefault bool) *L4NetLBController {
 	stopCh := make(chan struct{})
-	ctx, err := buildContext(vals, readOnlyMode)
+	ctx, err := buildContext(vals, readOnlyMode, isRBSdefault)
 	if err != nil {
 		klog.Fatalf("Failed to build context: %v", err)
 	}
@@ -1663,6 +1672,7 @@ func TestIsRBSBasedService(t *testing.T) {
 		finalizers       []string
 		annotations      map[string]string
 		frHook           getForwardingRuleHook
+		isRBSDefault     bool
 		expectRBSService bool
 	}{
 		{
@@ -1699,13 +1709,30 @@ func TestIsRBSBasedService(t *testing.T) {
 			frHook:           test.GetLegacyForwardingRule,
 			expectRBSService: false,
 		},
+		{
+			desc:             "Should detect RBS by default",
+			isRBSDefault:     true,
+			expectRBSService: true,
+		},
+		{
+			desc:             "Should not detect RBS by forwarding rule pointed to target pool, even if RBS is default",
+			frHook:           test.GetLegacyForwardingRule,
+			isRBSDefault:     true,
+			expectRBSService: false,
+		},
+		{
+			desc:             "Legacy service should not be marked as RBS, even if RBS is default",
+			finalizers:       []string{common.NetLBFinalizerV1},
+			isRBSDefault:     true,
+			expectRBSService: false,
+		},
 	}
 
 	for _, testCase := range testCases {
 		t.Run(testCase.desc, func(t *testing.T) {
 			// Setup
 			svc := test.NewL4LegacyNetLBService(8080, 30234)
-			controller := newL4NetLBServiceController()
+			controller := newL4NetLBServiceControllerRBSDefault(testCase.isRBSDefault)
 			svc.Annotations = testCase.annotations
 			svc.ObjectMeta.Finalizers = testCase.finalizers
 			controller.ctx.Cloud.Compute().(*cloud.MockGCE).MockForwardingRules.GetHook = testCase.frHook
@@ -2317,7 +2344,7 @@ func TestEnsureExternalLoadBalancerClass(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			lc := createL4NetLBServiceController(test.DefaultTestClusterValues(), false)
+			lc := newL4NetLBServiceController()
 
 			svc := test.NewL4LBServiceWithLoadBalancerClass(tc.loadBalancerClass)
 			if tc.loadBalancerClass == "" {
@@ -2420,7 +2447,7 @@ func TestEnsureReadOnlyModeDoesNotProvision(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			lc := createL4NetLBServiceController(test.DefaultTestClusterValues(), tc.readOnlyModeEnabled)
+			lc := newL4NetLBServiceControllerReadOnlyMode(tc.readOnlyModeEnabled)
 
 			svc := test.NewL4LBServiceWithLoadBalancerClass(tc.loadBalancerClass)
 			if tc.loadBalancerClass == "" {

pkg/utils/common/finalizer.go

@@ -40,6 +40,8 @@ const (
 	ILBFinalizerV2 = "gke.networking.io/l4-ilb-v2"
 	// NegFinalizerKey is the finalizer used by neg controller to ensure NEG CRs are deleted after corresponding negs are deleted
 	NegFinalizerKey = "networking.gke.io/neg-finalizer"
+	// NetLBFinalizerV1 is the finalizer used by legacy cloud controller manager that manage L4 External LoadBalancer services.
+	NetLBFinalizerV1 = "gke.networking.io/l4-netlb-v1"
 	// NetLBFinalizerV2 is the finalizer used by newer controllers that manage L4 External LoadBalancer services.
 	NetLBFinalizerV2 = "gke.networking.io/l4-netlb-v2"
 	// NetLBFinalizerV3 is the finalizer used by the NEG backed variant of the L4 External LoadBalancer services.

pkg/utils/utils.go

@@ -761,6 +761,11 @@ func HasL4ILBFinalizerV2(svc *api_v1.Service) bool {
 	return slice.ContainsString(svc.ObjectMeta.Finalizers, common.ILBFinalizerV2, nil)
 }
 
+// HasL4NetLBFinalizerV1 returns true if the given Service has NetLBFinalizerV1
+func HasL4NetLBFinalizerV1(svc *api_v1.Service) bool {
+	return slice.ContainsString(svc.ObjectMeta.Finalizers, common.NetLBFinalizerV1, nil)
+}
+
 // HasL4NetLBFinalizerV2 returns true if the given Service has NetLBFinalizerV2
 func HasL4NetLBFinalizerV2(svc *api_v1.Service) bool {
 	return slice.ContainsString(svc.ObjectMeta.Finalizers, common.NetLBFinalizerV2, nil)