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

Author: 08volt

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 {
@@ -388,7 +390,13 @@ func (lc *L4NetLBController) isRBSBasedService(svc *v1.Service, svcLogger klog.L
 	if svc.Spec.LoadBalancerClass != nil {
 		return annotations.HasLoadBalancerClass(svc, annotations.RegionalExternalLoadBalancerClass)
 	}
-	return annotations.HasRBSAnnotation(svc) || utils.HasL4NetLBFinalizerV2(svc) || utils.HasL4NetLBFinalizerV3(svc) || lc.hasRBSForwardingRule(svc, svcLogger)
+	if utils.HasL4NetLBFinalizerV1(svc) {
+		return false
+	}
+	if lc.hasLegacyForwardingRule(svc, svcLogger) {
+		return false
+	}
+	return lc.enableRBSDefault || annotations.HasRBSAnnotation(svc) || utils.HasL4NetLBFinalizerV2(svc) || utils.HasL4NetLBFinalizerV3(svc) || lc.hasRBSForwardingRule(svc, svcLogger)
 }
 
 func (lc *L4NetLBController) preventLegacyServiceHandling(service *v1.Service, key string, svcLogger klog.Logger) (bool, error) {
@@ -472,6 +480,23 @@ func (lc *L4NetLBController) hasRBSForwardingRule(svc *v1.Service, svcLogger klo
 	return existingFR != nil && existingFR.LoadBalancingScheme == string(cloud.SchemeExternal) && existingFR.BackendService != ""
 }
 
+// hasLegacyForwardingRule checks if services loadbalancer has forwarding rule pointing to target pool
+func (lc *L4NetLBController) hasLegacyForwardingRule(svc *v1.Service, svcLogger klog.Logger) bool {
+	frName := utils.LegacyForwardingRuleName(svc)
+
+	// to optimize number of api calls, at first, check if forwarding rule does not exists in annotation
+	if lc.hasForwardingRuleAnnotation(svc, frName) {
+		return false
+	}
+
+	existingFR, err := lc.forwardingRules.Get(frName)
+	if err != nil {
+		svcLogger.Error(err, "Error getting forwarding rule", "forwardingRule", frName)
+		return false
+	}
+	return existingFR != nil && existingFR.LoadBalancingScheme == string(cloud.SchemeExternal) && existingFR.Target != ""
+}
+
 func (lc *L4NetLBController) SystemHealth() error {
 	lastEnqueueTime := lc.enqueueTracker.Get()
 	lastSyncTime := lc.syncTracker.Get()

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)