WIP

cmd/glbc/main.go

@@ -199,6 +199,7 @@ func main() {
 		ASMConfigMapName:      flags.F.ASMConfigMapBasedConfigCMName,
 		EndpointSlicesEnabled: flags.F.EnableEndpointSlices,
 		MaxIGSize:             flags.F.MaxIGSize,
+		EnableL4ILBDualStack:  flags.F.EnableL4ILBDualStack,
 	}
 	ctx := ingctx.NewControllerContext(kubeConfig, kubeClient, backendConfigClient, frontendConfigClient, svcNegClient, ingParamsClient, svcAttachmentClient, cloud, namer, kubeSystemUID, ctxConfig)
 	go app.RunHTTPServer(ctx.HealthCheck)

pkg/context/context.go

@@ -135,6 +135,7 @@ type ControllerContextConfig struct {
 	ASMConfigMapName      string
 	EndpointSlicesEnabled bool
 	MaxIGSize             int
+	EnableL4ILBDualStack  bool
 }
 
 // NewControllerContext returns a new shared set of informers.

pkg/healthchecksl4/healthchecksl4.go

@@ -44,6 +44,7 @@ const (
 	gceHcHealthyThreshold = int64(1)
 	// Defaults to 3 * 8 = 24 seconds before the LB will steer traffic away.
 	gceHcUnhealthyThreshold = int64(3)
+	l4ILBIPv6HCRangeString  = "2600:2d00:1:b029::/64"
 )
 
 var (
@@ -114,9 +115,8 @@ func (l4hc *l4HealthChecks) EnsureDualStackHealthCheck(svc *corev1.Service, name
 	namespacedName := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace}
 
 	hcName := namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHC)
-	hcFwName := namer.L4HealthCheckFirewall(svc.Namespace, svc.Name, sharedHC)
 	hcPath, hcPort := helpers.GetServiceHealthCheckPathPort(svc)
-	klog.V(3).Infof("Ensuring L4 healthcheck: %s and firewall rule %s from service %s, shared: %v.", hcName, hcFwName, namespacedName.String(), sharedHC)
+	klog.V(3).Infof("Ensuring L4 healthcheck: %s for service %s, shared: %v.", hcName, namespacedName.String(), sharedHC)
 
 	if sharedHC {
 		hcPath, hcPort = gce.GetNodesHealthCheckPath(), gce.GetNodesHealthCheckPort()
@@ -140,7 +140,8 @@ func (l4hc *l4HealthChecks) EnsureDualStackHealthCheck(svc *corev1.Service, name
 	}
 
 	if needsIPv4 {
-		klog.V(3).Infof("Healthcheck created, ensuring firewall rule %s", hcFwName)
+		hcFwName := namer.L4HealthCheckFirewall(svc.Namespace, svc.Name, sharedHC)
+		klog.V(3).Infof("Healthcheck created, ensuring IPv4 firewall rule %s for service %s", hcFwName, namespacedName.String())
 		err = l4hc.ensureFirewall(svc, hcFwName, hcPort, sharedHC, nodeNames)
 		if err != nil {
 			return &EnsureHealthCheckResult{
@@ -153,7 +154,7 @@ func (l4hc *l4HealthChecks) EnsureDualStackHealthCheck(svc *corev1.Service, name
 
 	if needsIPv6 {
 		ipv6HCFWName := namer.L4IPv6HealthCheckFirewall(svc.Namespace, svc.Name, sharedHC)
-		klog.V(3).Infof("Healthcheck created, ensuring ipv6 firewall rule %s", ipv6HCFWName)
+		klog.V(3).Infof("Healthcheck created, ensuring IPv6 firewall rule %s for service %s", ipv6HCFWName, namespacedName.String())
 		err = l4hc.ensureIPv6Firewall(svc, ipv6HCFWName, hcPort, sharedHC, nodeNames)
 		if err != nil {
 			return &EnsureHealthCheckResult{
@@ -170,7 +171,7 @@ func (l4hc *l4HealthChecks) EnsureDualStackHealthCheck(svc *corev1.Service, name
 func (l4hc *l4HealthChecks) ensureIPv6Firewall(svc *corev1.Service, ipv6HCFWName string, hcPort int32, isSharedHC bool, nodeNames []string) error {
 	hcFWRParams := firewalls.FirewallParams{
 		PortRanges:   []string{strconv.Itoa(int(hcPort))},
-		SourceRanges: L4ILBIPv6HCRange.StringSlice(),
+		SourceRanges: []string{l4ILBIPv6HCRangeString},
 		Protocol:     string(corev1.ProtocolTCP),
 		Name:         ipv6HCFWName,
 		NodeNames:    nodeNames,
@@ -180,12 +181,6 @@ func (l4hc *l4HealthChecks) ensureIPv6Firewall(svc *corev1.Service, ipv6HCFWName
 
 // DeleteHealthCheck deletes health check (and firewall rule) for l4 service. Checks if shared resources are safe to delete.
 func (l4hc *l4HealthChecks) DeleteHealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error) {
-	return l4hc.DeleteDualStackHealthCheck(svc, namer, sharedHC, scope, l4Type, false)
-}
-
-// DeleteDualStackHealthCheck deletes health check (and firewall rule) for l4 service.
-// Checks if shared resources are safe to delete. Deletes IPv6 firewall if asked
-func (l4hc *l4HealthChecks) DeleteDualStackHealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, handleIPv6 bool) (string, error) {
 	hcName := namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHC)
 	hcFwName := namer.L4HealthCheckFirewall(svc.Namespace, svc.Name, sharedHC)
 	namespacedName := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace}
@@ -206,16 +201,19 @@ func (l4hc *l4HealthChecks) DeleteDualStackHealthCheck(svc *corev1.Service, name
 		klog.V(2).Infof("Failed to delete healthcheck %s: shared health check in use.", hcName)
 		return "", nil
 	}
-	// Health check deleted, now delete the firewall rule
-	if handleIPv6 {
-		errResource, err := l4hc.deleteIPv6HealthCheckFirewall(svc, hcName, namer, sharedHC, l4Type)
-		if err != nil {
-			return errResource, err
-		}
-	}
 	return l4hc.deleteHealthCheckFirewall(svc, hcName, hcFwName, sharedHC, l4Type)
 }
 
+// DeleteDualStackHealthCheck deletes health check, ipv4 and ipv6 firewall rules for l4 service.
+// Checks if shared resources are safe to delete.
+func (l4hc *l4HealthChecks) DeleteDualStackHealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error) {
+	errResource, err := l4hc.DeleteHealthCheck(svc, namer, sharedHC, scope, l4Type)
+	if err != nil {
+		return errResource, err
+	}
+	return l4hc.deleteIPv6HealthCheckFirewall(svc, namer, sharedHC, l4Type)
+}
+
 func (l4hc *l4HealthChecks) ensureL4HealthCheckInternal(hcName string, svcName types.NamespacedName, shared bool, path string, port int32, scope meta.KeyType, l4Type utils.L4LBType) (*composite.HealthCheck, string, error) {
 	hc, err := l4hc.hcProvider.Get(hcName, scope)
 	if err != nil {
@@ -294,7 +292,8 @@ func (l4hc *l4HealthChecks) deleteHealthCheckFirewall(svc *corev1.Service, hcNam
 	return "", nil
 }
 
-func (l4hc *l4HealthChecks) deleteIPv6HealthCheckFirewall(svc *corev1.Service, hcName string, namer namer.L4ResourcesNamer, sharedHC bool, l4type utils.L4LBType) (string, error) {
+func (l4hc *l4HealthChecks) deleteIPv6HealthCheckFirewall(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, l4type utils.L4LBType) (string, error) {
+	hcName := namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHC)
 	ipv6hcFwName := namer.L4IPv6HealthCheckFirewall(svc.Namespace, svc.Name, sharedHC)
 	return l4hc.deleteHealthCheckFirewall(svc, hcName, ipv6hcFwName, sharedHC, l4type)
 }

pkg/healthchecksl4/healthchecksl4_test.go

@@ -128,5 +128,4 @@ func TestNewHealthCheck(t *testing.T) {
 			t.Errorf("HealthCheck Scope mismatch! %v != %v", hc.Scope, v.scope)
 		}
 	}
-
 }

pkg/healthchecksl4/interfaces.go

@@ -17,7 +17,7 @@ type L4HealthChecks interface {
 	// DeleteHealthCheck deletes health check (and firewall rule) for l4 service
 	DeleteHealthCheck(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error)
 	// DeleteDualStackHealthCheck deletes health check (and firewall rule) for l4 service, deletes IPv6 if asked
-	DeleteDualStackHealthCheck(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, handleIPv6 bool) (string, error)
+	DeleteDualStackHealthCheck(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error)
 }
 
 type EnsureHealthCheckResult struct {

pkg/l4lb/l4controller.go

@@ -19,6 +19,7 @@ package l4lb
 import (
 	"fmt"
 	"reflect"
+	"strings"
 	"sync"
 	"time"
 
@@ -33,7 +34,6 @@ import (
 	"k8s.io/ingress-gce/pkg/backends"
 	"k8s.io/ingress-gce/pkg/context"
 	"k8s.io/ingress-gce/pkg/controller/translator"
-	"k8s.io/ingress-gce/pkg/flags"
 	"k8s.io/ingress-gce/pkg/forwardingrules"
 	l4metrics "k8s.io/ingress-gce/pkg/l4lb/metrics"
 	"k8s.io/ingress-gce/pkg/loadbalancers"
@@ -72,6 +72,7 @@ type L4Controller struct {
 	syncTracker         utils.TimeTracker
 	forwardingRules     ForwardingRulesGetter
 	sharedResourcesLock sync.Mutex
+	enableDualStack     bool
 }
 
 // NewILBController creates a new instance of the L4 ILB controller.
@@ -90,6 +91,7 @@ func NewILBController(ctx *context.ControllerContext, stopCh chan struct{}) *L4C
 		namer:           ctx.L4Namer,
 		translator:      ctx.Translator,
 		forwardingRules: forwardingrules.New(ctx.Cloud, meta.VersionGA, meta.Regional),
+		enableDualStack: ctx.EnableL4ILBDualStack,
 	}
 	l4c.backendPool = backends.NewPool(ctx.Cloud, l4c.namer)
 	l4c.NegLinker = backends.NewNEGLinker(l4c.backendPool, negtypes.NewAdapter(ctx.Cloud), ctx.Cloud, ctx.SvcNegInformer.GetIndexer())
@@ -214,7 +216,7 @@ func (l4c *L4Controller) processServiceCreateOrUpdate(key string, service *v1.Se
 		Cloud:            l4c.ctx.Cloud,
 		Namer:            l4c.namer,
 		Recorder:         l4c.ctx.Recorder(service.Namespace),
-		DualStackEnabled: flags.F.EnableL4ILBDualStack,
+		DualStackEnabled: l4c.enableDualStack,
 	}
 	l4 := loadbalancers.NewL4Handler(l4ilbParams)
 	syncResult := l4.EnsureInternalLoadBalancer(nodeNames, service)
@@ -244,8 +246,12 @@ func (l4c *L4Controller) processServiceCreateOrUpdate(key string, service *v1.Se
 		syncResult.Error = err
 		return syncResult
 	}
-	l4c.ctx.Recorder(service.Namespace).Eventf(service, v1.EventTypeNormal, "SyncLoadBalancerSuccessful",
-		"Successfully ensured load balancer resources")
+	if l4c.enableDualStack {
+		l4c.emitEnsuredDualStackEvent(service)
+	} else {
+		l4c.ctx.Recorder(service.Namespace).Eventf(service, v1.EventTypeNormal, "SyncLoadBalancerSuccessful",
+			"Successfully ensured load balancer resources")
+	}
 	if err = updateL4ResourcesAnnotations(l4c.ctx, service, syncResult.Annotations); err != nil {
 		l4c.ctx.Recorder(service.Namespace).Eventf(service, v1.EventTypeWarning, "SyncLoadBalancerFailed",
 			"Failed to update annotations for load balancer, err: %v", err)
@@ -255,13 +261,22 @@ func (l4c *L4Controller) processServiceCreateOrUpdate(key string, service *v1.Se
 	return syncResult
 }
 
+func (l4c *L4Controller) emitEnsuredDualStackEvent(service *v1.Service) {
+	var ipFamilies []string
+	for _, ipFamily := range service.Spec.IPFamilies {
+		ipFamilies = append(ipFamilies, string(ipFamily))
+	}
+	l4c.ctx.Recorder(service.Namespace).Eventf(service, v1.EventTypeNormal, "SyncLoadBalancerSuccessful",
+		"Successfully ensured %v load balancer resources", strings.Join(ipFamilies, " "))
+}
+
 func (l4c *L4Controller) processServiceDeletion(key string, svc *v1.Service) *loadbalancers.L4ILBSyncResult {
 	l4ilbParams := &loadbalancers.L4ILBParams{
 		Service:          svc,
 		Cloud:            l4c.ctx.Cloud,
 		Namer:            l4c.namer,
 		Recorder:         l4c.ctx.Recorder(svc.Namespace),
-		DualStackEnabled: flags.F.EnableL4ILBDualStack,
+		DualStackEnabled: l4c.enableDualStack,
 	}
 	l4 := loadbalancers.NewL4Handler(l4ilbParams)
 	l4c.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeNormal, "DeletingLoadBalancer", "Deleting load balancer for %s", key)

pkg/loadbalancers/forwarding_rules_ipv6.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
-	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	corev1 "k8s.io/api/core/v1"
@@ -15,15 +14,15 @@ import (
 	"k8s.io/legacy-cloud-providers/gce"
 )
 
-func (l *L4) ensureIPv6ForwardingRule(bsLink string, options gce.ILBOptions) (*composite.ForwardingRule, error) {
-	expectedIPv6FwdRule, err := l.buildExpectedIPv6ForwardingRule(bsLink, options)
+func (l4 *L4) ensureIPv6ForwardingRule(bsLink string, options gce.ILBOptions) (*composite.ForwardingRule, error) {
+	expectedIPv6FwdRule, err := l4.buildExpectedIPv6ForwardingRule(bsLink, options)
 	if err != nil {
-		return nil, fmt.Errorf("l.buildExpectedIPv6ForwardingRule(%s, %v) returned error %w, want nil", bsLink, options, err)
+		return nil, fmt.Errorf("l4.buildExpectedIPv6ForwardingRule(%s, %v) returned error %w, want nil", bsLink, options, err)
 	}
 
-	existingIPv6FwdRule, err := l.forwardingRules.Get(expectedIPv6FwdRule.Name)
+	existingIPv6FwdRule, err := l4.forwardingRules.Get(expectedIPv6FwdRule.Name)
 	if err != nil {
-		return nil, fmt.Errorf("l.forwardingRules.GetForwardingRule(%s) returned error %w, want nil", expectedIPv6FwdRule.Name, err)
+		return nil, fmt.Errorf("l4.forwardingRules.GetForwardingRule(%s) returned error %w, want nil", expectedIPv6FwdRule.Name, err)
 	}
 
 	if existingIPv6FwdRule != nil {
@@ -35,47 +34,39 @@ func (l *L4) ensureIPv6ForwardingRule(bsLink string, options gce.ILBOptions) (*c
 			klog.V(2).Infof("ensureIPv6ForwardingRule: Skipping update of unchanged ipv6 forwarding rule - %s", expectedIPv6FwdRule.Name)
 			return existingIPv6FwdRule, nil
 		}
-
-		frDiff := cmp.Diff(existingIPv6FwdRule, expectedIPv6FwdRule, cmpopts.IgnoreFields(composite.ForwardingRule{}, "IPAddress"))
-		klog.V(2).Infof("ensureIPv6ForwardingRule: forwarding rule changed - Existing - %+v\n, New - %+v\n, Diff(-existing, +new) - %s\n. Deleting existing ipv6 forwarding rule.", existingIPv6FwdRule, expectedIPv6FwdRule, frDiff)
-
-		err = l.forwardingRules.Delete(existingIPv6FwdRule.Name)
+		err = l4.deleteChangedIPv6ForwardingRule(existingIPv6FwdRule, expectedIPv6FwdRule)
 		if err != nil {
 			return nil, err
 		}
-		l.recorder.Eventf(l.Service, corev1.EventTypeNormal, events.SyncIngress, "ForwardingRule %q deleted", existingIPv6FwdRule.Name)
 	}
 	klog.V(2).Infof("ensureIPv6ForwardingRule: Creating/Recreating forwarding rule - %s", expectedIPv6FwdRule.Name)
-	err = l.forwardingRules.Create(expectedIPv6FwdRule)
+	err = l4.forwardingRules.Create(expectedIPv6FwdRule)
 	if err != nil {
 		return nil, err
 	}
 
-	createdFr, err := l.forwardingRules.Get(expectedIPv6FwdRule.Name)
+	createdFr, err := l4.forwardingRules.Get(expectedIPv6FwdRule.Name)
 	return createdFr, err
 }
 
-func (l *L4) buildExpectedIPv6ForwardingRule(bsLink string, options gce.ILBOptions) (*composite.ForwardingRule, error) {
-	frName := l.getIPv6FRName()
+func (l4 *L4) buildExpectedIPv6ForwardingRule(bsLink string, options gce.ILBOptions) (*composite.ForwardingRule, error) {
+	frName := l4.getIPv6FRName()
 
-	frDesc, err := utils.MakeL4IPv6ForwardingRuleDescription(l.Service)
+	frDesc, err := utils.MakeL4IPv6ForwardingRuleDescription(l4.Service)
 	if err != nil {
 		return nil, fmt.Errorf("failed to compute description for forwarding rule %s, err: %w", frName, err)
 	}
 
-	subnetworkURL := l.cloud.SubnetworkURL()
+	subnetworkURL := l4.cloud.SubnetworkURL()
 
 	if options.SubnetName != "" {
-		key, err := l.CreateKey(frName)
+		subnetworkURL, err = l4.getSubnetworkURLByName(options.SubnetName)
 		if err != nil {
 			return nil, err
 		}
-		subnetKey := *key
-		subnetKey.Name = options.SubnetName
-		subnetworkURL = cloud.SelfLink(meta.VersionGA, l.cloud.NetworkProjectID(), "subnetworks", &subnetKey)
 	}
 
-	svcPorts := l.Service.Spec.Ports
+	svcPorts := l4.Service.Spec.Ports
 	ports := utils.GetPorts(svcPorts)
 	protocol := utils.GetProtocol(svcPorts)
 
@@ -87,7 +78,7 @@ func (l *L4) buildExpectedIPv6ForwardingRule(bsLink string, options gce.ILBOptio
 		LoadBalancingScheme: string(cloud.SchemeInternal),
 		BackendService:      bsLink,
 		IpVersion:           "IPV6",
-		Network:             l.cloud.NetworkURL(),
+		Network:             l4.cloud.NetworkURL(),
 		Subnetwork:          subnetworkURL,
 		AllowGlobalAccess:   options.AllowGlobalAccess,
 		NetworkTier:         cloud.NetworkTierPremium.ToGCEValue(),
@@ -100,6 +91,18 @@ func (l *L4) buildExpectedIPv6ForwardingRule(bsLink string, options gce.ILBOptio
 	return fr, nil
 }
 
+func (l4 *L4) deleteChangedIPv6ForwardingRule(existingFwdRule *composite.ForwardingRule, expectedFwdRule *composite.ForwardingRule) error {
+	frDiff := cmp.Diff(existingFwdRule, expectedFwdRule, cmpopts.IgnoreFields(composite.ForwardingRule{}, "IPAddress"))
+	klog.V(2).Infof("IPv6 forwarding rule changed - Existing - %+v\n, New - %+v\n, Diff(-existing, +new) - %s\n. Deleting existing ipv6 forwarding rule.", existingFwdRule, expectedFwdRule, frDiff)
+
+	err := l4.forwardingRules.Delete(existingFwdRule.Name)
+	if err != nil {
+		return err
+	}
+	l4.recorder.Eventf(l4.Service, corev1.EventTypeNormal, events.SyncIngress, "ForwardingRule %q deleted", existingFwdRule.Name)
+	return nil
+}
+
 func EqualIPv6ForwardingRules(fr1, fr2 *composite.ForwardingRule) (bool, error) {
 	id1, err := cloud.ParseResourceURL(fr1.BackendService)
 	if err != nil {