From 636f3a5c259ed8acf6bc7f6a3ef4361ebdbee624 Mon Sep 17 00:00:00 2001 From: Slavik Panasovets Date: Wed, 27 Jul 2022 12:16:58 +0000 Subject: [PATCH] Add Dual-Stack support to L4 ILB --- cmd/glbc/main.go | 1 + pkg/annotations/service.go | 30 +- pkg/context/context.go | 1 + pkg/firewalls/firewalls_l4.go | 1 - pkg/flags/flags.go | 2 + pkg/healthchecksl4/healthchecksl4.go | 114 +++- pkg/healthchecksl4/healthchecksl4_test.go | 1 - pkg/healthchecksl4/interfaces.go | 23 +- pkg/l4lb/l4controller.go | 73 ++- pkg/l4lb/l4controller_test.go | 115 ++++- pkg/l4lb/l4lbcommon.go | 10 + pkg/l4lb/l4netlbcontroller_test.go | 2 +- pkg/loadbalancers/forwarding_rules_ipv6.go | 123 +++++ pkg/loadbalancers/l4.go | 228 +++++--- pkg/loadbalancers/l4_test.go | 573 +++++++++++++++++++-- pkg/loadbalancers/l4ipv6.go | 111 ++++ pkg/loadbalancers/l4netlb.go | 3 +- pkg/loadbalancers/l4syncresult.go | 8 + pkg/test/utils.go | 21 + pkg/utils/ipfamily.go | 33 ++ pkg/utils/ipfamily_test.go | 104 ++++ pkg/utils/namer/interfaces.go | 6 + pkg/utils/namer/l4_namer.go | 53 +- pkg/utils/namer/l4_namer_test.go | 51 +- pkg/utils/utils.go | 17 + pkg/utils/utils_test.go | 44 ++ 26 files changed, 1553 insertions(+), 195 deletions(-) create mode 100644 pkg/loadbalancers/forwarding_rules_ipv6.go create mode 100644 pkg/loadbalancers/l4ipv6.go create mode 100644 pkg/utils/ipfamily.go create mode 100644 pkg/utils/ipfamily_test.go diff --git a/cmd/glbc/main.go b/cmd/glbc/main.go index 9186489714..14b62df6e3 100644 --- a/cmd/glbc/main.go +++ b/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) diff --git a/pkg/annotations/service.go b/pkg/annotations/service.go index 536ff5cc00..55b94bb213 100644 --- a/pkg/annotations/service.go +++ b/pkg/annotations/service.go @@ -73,6 +73,7 @@ const ( // ProtocolHTTP2 protocol for a service ProtocolHTTP2 AppProtocol = "HTTP2" + IPv6Suffix = "-ipv6" // ServiceStatusPrefix is the prefix used in annotations used to record // debug information in the Service annotations. This is applicable to L4 ILB services. ServiceStatusPrefix = "service.kubernetes.io" @@ -82,24 +83,39 @@ const ( // UDPForwardingRuleKey is the annotation key used by l4 controller to record // GCP UDP forwarding rule name. UDPForwardingRuleKey = ServiceStatusPrefix + "/udp-" + ForwardingRuleResource + // TCPForwardingRuleIPv6Key is the annotation key used by l4 controller to record + // GCP IPv6 TCP forwarding rule name. + TCPForwardingRuleIPv6Key = TCPForwardingRuleKey + IPv6Suffix + // UDPForwardingRuleIPv6Key is the annotation key used by l4 controller to record + // GCP IPv6 UDP forwarding rule name. + UDPForwardingRuleIPv6Key = UDPForwardingRuleKey + IPv6Suffix // BackendServiceKey is the annotation key used by l4 controller to record // GCP Backend service name. BackendServiceKey = ServiceStatusPrefix + "/" + BackendServiceResource // FirewallRuleKey is the annotation key used by l4 controller to record // GCP Firewall rule name. FirewallRuleKey = ServiceStatusPrefix + "/" + FirewallRuleResource + // FirewallRuleIPv6Key is the annotation key used by l4 controller to record + // GCP IPv6 Firewall rule name. + FirewallRuleIPv6Key = FirewallRuleKey + IPv6Suffix // HealthcheckKey is the annotation key used by l4 controller to record // GCP Healthcheck name. HealthcheckKey = ServiceStatusPrefix + "/" + HealthcheckResource // FirewallRuleForHealthcheckKey is the annotation key used by l4 controller to record // the firewall rule name that allows healthcheck traffic. - FirewallRuleForHealthcheckKey = ServiceStatusPrefix + "/" + FirewallForHealthcheckResource - ForwardingRuleResource = "forwarding-rule" - BackendServiceResource = "backend-service" - FirewallRuleResource = "firewall-rule" - HealthcheckResource = "healthcheck" - FirewallForHealthcheckResource = "firewall-rule-for-hc" - AddressResource = "address" + FirewallRuleForHealthcheckKey = ServiceStatusPrefix + "/" + FirewallForHealthcheckResource + // FirewallRuleForHealthcheckIPv6Key is the annotation key used by l4 controller to record + // the firewall rule name that allows IPv6 healthcheck traffic. + FirewallRuleForHealthcheckIPv6Key = FirewallRuleForHealthcheckKey + IPv6Suffix + ForwardingRuleResource = "forwarding-rule" + ForwardingRuleIPv6Resource = ForwardingRuleResource + IPv6Suffix + BackendServiceResource = "backend-service" + FirewallRuleResource = "firewall-rule" + FirewallRuleIPv6Resource = FirewallRuleResource + IPv6Suffix + HealthcheckResource = "healthcheck" + FirewallForHealthcheckResource = "firewall-rule-for-hc" + FirewallForHealthcheckIPv6Resource = FirewallRuleForHealthcheckKey + IPv6Suffix + AddressResource = "address" // TODO(slavik): import this from gce_annotations when it will be merged in k8s RBSAnnotationKey = "cloud.google.com/l4-rbs" RBSEnabled = "enabled" diff --git a/pkg/context/context.go b/pkg/context/context.go index 71c881608a..52a65e8944 100644 --- a/pkg/context/context.go +++ b/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. diff --git a/pkg/firewalls/firewalls_l4.go b/pkg/firewalls/firewalls_l4.go index 8cede6bf40..5325945cba 100644 --- a/pkg/firewalls/firewalls_l4.go +++ b/pkg/firewalls/firewalls_l4.go @@ -160,7 +160,6 @@ func ensureFirewall(svc *v1.Service, shared bool, params *FirewallParams, cloud // EnsureL4LBFirewallForHc creates or updates firewall rule for shared or non-shared health check to nodes func EnsureL4LBFirewallForHc(svc *v1.Service, shared bool, params *FirewallParams, cloud *gce.Cloud, recorder record.EventRecorder) error { - params.SourceRanges = gce.L4LoadBalancerSrcRanges() return ensureFirewall(svc, shared, params, cloud, recorder) } diff --git a/pkg/flags/flags.go b/pkg/flags/flags.go index 8edd9936dd..caf5e422bb 100644 --- a/pkg/flags/flags.go +++ b/pkg/flags/flags.go @@ -108,6 +108,7 @@ var ( EnableTrafficScaling bool EnableEndpointSlices bool EnablePinhole bool + EnableL4ILBDualStack bool EnableMultipleIGs bool MaxIGSize int }{ @@ -249,6 +250,7 @@ L7 load balancing. CSV values accepted. Example: -node-port-ranges=80,8080,400-5 flag.BoolVar(&F.EnableTrafficScaling, "enable-traffic-scaling", false, "Enable support for Service {max-rate-per-endpoint, capacity-scaler}") flag.BoolVar(&F.EnableEndpointSlices, "enable-endpoint-slices", false, "Enable using Endpoint Slices API instead of Endpoints API") flag.BoolVar(&F.EnablePinhole, "enable-pinhole", false, "Enable Pinhole firewall feature") + flag.BoolVar(&F.EnableL4ILBDualStack, "enable-l4ilb-dual-stack", false, "Enable Dual-Stack handling for L4 Internal Load Balancers") flag.BoolVar(&F.EnableMultipleIGs, "enable-multiple-igs", false, "Enable using multiple unmanaged instance groups") flag.IntVar(&F.MaxIGSize, "max-ig-size", 1000, "Max number of instances in Instance Group") flag.DurationVar(&F.MetricsExportInterval, "metrics-export-interval", 10*time.Minute, `Period for calculating and exporting metrics related to state of managed objects.`) diff --git a/pkg/healthchecksl4/healthchecksl4.go b/pkg/healthchecksl4/healthchecksl4.go index 1f0992c8d0..40ec494891 100644 --- a/pkg/healthchecksl4/healthchecksl4.go +++ b/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) + L4ILBIPv6HCRange = "2600:2d00:1:b029::/64" ) var ( @@ -105,13 +106,16 @@ func GetInstance() *l4HealthChecks { // Firewall rules are always created at in the Global scope (vs // Regional). This means that one Firewall rule is created for // Services of different scope (Global vs Regional). -func (l4hc *l4HealthChecks) EnsureHealthCheckWithFirewall(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string) *EnsureL4HealthCheckResult { +func (l4hc *l4HealthChecks) EnsureHealthCheckWithFirewall(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string) *EnsureHealthCheckResult { + return l4hc.EnsureHealthCheckWithDualStackFirewalls(svc, namer, sharedHC, scope, l4Type, nodeNames, true, false) +} + +func (l4hc *l4HealthChecks) EnsureHealthCheckWithDualStackFirewalls(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string, needsIPv4 bool, needsIPv6 bool) *EnsureHealthCheckResult { 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() @@ -123,25 +127,44 @@ func (l4hc *l4HealthChecks) EnsureHealthCheckWithFirewall(svc *corev1.Service, n hcLink, err := l4hc.ensureHealthCheck(hcName, namespacedName, sharedHC, hcPath, hcPort, scope, l4Type) if err != nil { - return &EnsureL4HealthCheckResult{ + return &EnsureHealthCheckResult{ GceResourceInError: annotations.HealthcheckResource, Err: err, } } - klog.V(3).Infof("Healthcheck created, ensuring firewall rule %s", hcFwName) - err = l4hc.ensureFirewall(svc, hcFwName, hcPort, sharedHC, nodeNames) - if err != nil { - return &EnsureL4HealthCheckResult{ - GceResourceInError: annotations.HealthcheckResource, - Err: err, + hcResult := &EnsureHealthCheckResult{ + HCName: hcName, + HCLink: hcLink, + } + + if needsIPv4 { + hcFwName := namer.L4HealthCheckFirewall(svc.Namespace, svc.Name, sharedHC) + klog.V(3).Infof("Ensuring IPv4 firewall rule %s for health check %s for service %s", hcFwName, hcName, namespacedName.String()) + err = l4hc.ensureIPv4Firewall(svc, hcFwName, hcPort, sharedHC, nodeNames) + if err != nil { + return &EnsureHealthCheckResult{ + GceResourceInError: annotations.FirewallForHealthcheckResource, + Err: err, + } } + hcResult.HCFirewallRuleName = hcFwName } - return &EnsureL4HealthCheckResult{ - HCName: hcName, - HCLink: hcLink, - HCFirewallRuleName: hcFwName, + + if needsIPv6 { + ipv6HCFWName := namer.L4IPv6HealthCheckFirewall(svc.Namespace, svc.Name, sharedHC) + klog.V(3).Infof("Ensuring IPv6 firewall rule %s for health check %s for service %s", ipv6HCFWName, hcName, namespacedName.String()) + err = l4hc.ensureIPv6Firewall(svc, ipv6HCFWName, hcPort, sharedHC, nodeNames) + if err != nil { + return &EnsureHealthCheckResult{ + GceResourceInError: annotations.FirewallForHealthcheckIPv6Resource, + Err: err, + } + } + hcResult.HCFirewallRuleIPv6Name = ipv6HCFWName } + + return hcResult } func (l4hc *l4HealthChecks) ensureHealthCheck(hcName string, svcName types.NamespacedName, shared bool, path string, port int32, scope meta.KeyType, l4Type utils.L4LBType) (string, error) { @@ -184,12 +207,11 @@ func (l4hc *l4HealthChecks) ensureHealthCheck(hcName string, svcName types.Names return selfLink, err } -// ensureFirewall rule for `svc`. +// ensureIPv4Firewall rule for `svc`. // // L4 ILB and L4 NetLB Services with ExternalTrafficPolicy=Cluster use the same firewall // rule at global scope. -func (l4hc *l4HealthChecks) ensureFirewall(svc *corev1.Service, hcFwName string, hcPort int32, sharedHC bool, nodeNames []string) error { - // Add firewall rule for healthchecks to nodes +func (l4hc *l4HealthChecks) ensureIPv4Firewall(svc *corev1.Service, hcFwName string, hcPort int32, sharedHC bool, nodeNames []string) error { hcFWRParams := firewalls.FirewallParams{ PortRanges: []string{strconv.Itoa(int(hcPort))}, SourceRanges: gce.L4LoadBalancerSrcRanges(), @@ -200,8 +222,31 @@ func (l4hc *l4HealthChecks) ensureFirewall(svc *corev1.Service, hcFwName string, return firewalls.EnsureL4LBFirewallForHc(svc, sharedHC, &hcFWRParams, l4hc.cloud, l4hc.recorderFactory.Recorder(svc.Namespace)) } -// DeleteHealthCheckWithFirewall deletes health check (and firewall rule) for l4 service. Checks if shared resources are safe to delete. +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: []string{L4ILBIPv6HCRange}, + Protocol: string(corev1.ProtocolTCP), + Name: ipv6HCFWName, + NodeNames: nodeNames, + } + return firewalls.EnsureL4LBFirewallForHc(svc, isSharedHC, &hcFWRParams, l4hc.cloud, l4hc.recorderFactory.Recorder(svc.Namespace)) +} + func (l4hc *l4HealthChecks) DeleteHealthCheckWithFirewall(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error) { + return l4hc.deleteHealthCheckWithDualStackFirewalls(svc, namer, sharedHC, scope, l4Type, false) +} + +// DeleteHealthCheckWithDualStackFirewalls deletes health check, ipv4 and ipv6 firewall rules for l4 service. +// Checks if shared resources are safe to delete. +func (l4hc *l4HealthChecks) DeleteHealthCheckWithDualStackFirewalls(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error) { + return l4hc.deleteHealthCheckWithDualStackFirewalls(svc, namer, sharedHC, scope, l4Type, true) +} + +// deleteHealthCheckWithDualStackFirewalls deletes health check, ipv4 firewall rule +// and ipv6 firewall if running in dual-stack mode for l4 service. +// Checks if shared resources are safe to delete. +func (l4hc *l4HealthChecks) deleteHealthCheckWithDualStackFirewalls(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, handleIPv6 bool) (string, error) { if sharedHC { // We need to acquire a controller-wide mutex to ensure that in the case of a healthcheck shared between loadbalancers that the sync of the GCE resources is not performed in parallel. l4hc.sharedResourcesLock.Lock() @@ -217,14 +262,16 @@ func (l4hc *l4HealthChecks) DeleteHealthCheckWithFirewall(svc *corev1.Service, n return "", nil } - // Health check deleted, now delete the firewall rule - return l4hc.deleteHealthCheckFirewall(svc, namer, sharedHC, l4Type) + resourceInError, err := l4hc.deleteIPv4HealthCheckFirewall(svc, namer, sharedHC, l4Type) + if handleIPv6 { + resourceInError, err = l4hc.deleteIPv6HealthCheckFirewall(svc, namer, sharedHC, l4Type) + } + return resourceInError, err } func (l4hc *l4HealthChecks) deleteHealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType) (bool, error) { hcName := namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHC) klog.V(3).Infof("Deleting L4 healthcheck %s for service %s/%s, shared: %v, scope: %v", hcName, svc.Namespace, svc.Name, sharedHC, scope) - err := l4hc.hcProvider.Delete(hcName, scope) if err != nil { // Ignore deletion error due to health check in use by another resource. @@ -238,24 +285,39 @@ func (l4hc *l4HealthChecks) deleteHealthCheck(svc *corev1.Service, namer namer.L return true, nil } -func (l4hc *l4HealthChecks) deleteHealthCheckFirewall(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, l4Type utils.L4LBType) (string, error) { +func (l4hc *l4HealthChecks) deleteIPv4HealthCheckFirewall(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, l4type utils.L4LBType) (string, error) { hcName := namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHC) hcFwName := namer.L4HealthCheckFirewall(svc.Namespace, svc.Name, sharedHC) + klog.V(3).Infof("Deleting IPv4 Firewall %s for health check %s", hcFwName, hcName) + return l4hc.deleteHealthCheckFirewall(svc, hcName, hcFwName, sharedHC, l4type) +} + +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) + + klog.V(3).Infof("Deleting IPv6 Firewall %s for health check %s", ipv6hcFwName, hcName) + return l4hc.deleteHealthCheckFirewall(svc, hcName, ipv6hcFwName, sharedHC, l4type) +} + +func (l4hc *l4HealthChecks) deleteHealthCheckFirewall(svc *corev1.Service, hcName, hcFwName string, sharedHC bool, l4Type utils.L4LBType) (string, error) { + namespacedName := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} + safeToDelete, err := l4hc.healthCheckFirewallSafeToDelete(hcName, sharedHC, l4Type) if err != nil { - klog.Errorf("Failed to delete health check firewall rule %s for service %s/%s - %v", hcFwName, svc.Namespace, svc.Name, err) + klog.Errorf("Failed to delete health check firewall rule %s for service %s - %v", hcFwName, namespacedName.String(), err) return annotations.HealthcheckResource, err } if !safeToDelete { - klog.V(3).Infof("Failed to delete health check firewall rule %s: health check is in use.", hcName) + klog.V(3).Infof("Failed to delete health check firewall rule %s: health check in use.", hcName) return "", nil } - klog.V(3).Infof("Deleting healthcheck firewall rule %s for health check %s", hcFwName, hcName) + klog.V(3).Infof("Deleting healthcheck firewall rule named: %s", hcFwName) // Delete healthcheck firewall rule if no healthcheck uses the firewall rule. err = l4hc.deleteFirewall(hcFwName, svc) if err != nil { - klog.Errorf("Failed to delete firewall rule %s for loadbalancer service %s/%s, err %v", hcFwName, svc.Namespace, svc.Name, err) + klog.Errorf("Failed to delete firewall rule %s for loadbalancer service %s, err %v", hcFwName, namespacedName.String(), err) return annotations.FirewallForHealthcheckResource, err } return "", nil diff --git a/pkg/healthchecksl4/healthchecksl4_test.go b/pkg/healthchecksl4/healthchecksl4_test.go index 448736c8cc..dcef54f8de 100644 --- a/pkg/healthchecksl4/healthchecksl4_test.go +++ b/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) } } - } diff --git a/pkg/healthchecksl4/interfaces.go b/pkg/healthchecksl4/interfaces.go index add46160c9..69ac835785 100644 --- a/pkg/healthchecksl4/interfaces.go +++ b/pkg/healthchecksl4/interfaces.go @@ -10,18 +10,23 @@ import ( // L4HealthChecks defines methods for creating and deleting health checks (and their firewall rules) for l4 services type L4HealthChecks interface { - // EnsureHealthCheckWithFirewall creates health check with firewall rule for l4 service. - EnsureHealthCheckWithFirewall(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string) *EnsureL4HealthCheckResult - // DeleteHealthCheckWithFirewall deletes health check with firewall rule for l4 service. + // EnsureHealthCheckWithFirewall creates health check (and firewall rule) for l4 service. + EnsureHealthCheckWithFirewall(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string) *EnsureHealthCheckResult + // EnsureHealthCheckWithDualStackFirewalls creates health check (and firewall rule) for l4 service. Handles both IPv4 and IPv6. + EnsureHealthCheckWithDualStackFirewalls(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string, needsIPv4 bool, needsIPv6 bool) *EnsureHealthCheckResult + // DeleteHealthCheckWithFirewall deletes health check (and firewall rule) for l4 service. DeleteHealthCheckWithFirewall(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error) + // DeleteHealthCheckWithDualStackFirewalls deletes health check (and firewall rule) for l4 service, deletes IPv6 firewalls if asked. + DeleteHealthCheckWithDualStackFirewalls(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error) } -type EnsureL4HealthCheckResult struct { - HCName string - HCLink string - HCFirewallRuleName string - GceResourceInError string - Err error +type EnsureHealthCheckResult struct { + HCName string + HCLink string + HCFirewallRuleName string + HCFirewallRuleIPv6Name string + GceResourceInError string + Err error } type healthChecksProvider interface { diff --git a/pkg/l4lb/l4controller.go b/pkg/l4lb/l4controller.go index 138dcdeef0..0d59ae3e47 100644 --- a/pkg/l4lb/l4controller.go +++ b/pkg/l4lb/l4controller.go @@ -19,6 +19,7 @@ package l4lb import ( "fmt" "reflect" + "strings" "sync" "time" @@ -71,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. @@ -89,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()) @@ -209,10 +212,11 @@ func (l4c *L4Controller) processServiceCreateOrUpdate(key string, service *v1.Se // Use the same function for both create and updates. If controller crashes and restarts, // all existing services will show up as Service Adds. l4ilbParams := &loadbalancers.L4ILBParams{ - Service: service, - Cloud: l4c.ctx.Cloud, - Namer: l4c.namer, - Recorder: l4c.ctx.Recorder(service.Namespace), + Service: service, + Cloud: l4c.ctx.Cloud, + Namer: l4c.namer, + Recorder: l4c.ctx.Recorder(service.Namespace), + DualStackEnabled: l4c.enableDualStack, } l4 := loadbalancers.NewL4Handler(l4ilbParams) syncResult := l4.EnsureInternalLoadBalancer(nodeNames, service) @@ -242,23 +246,43 @@ 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 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) - syncResult.Error = fmt.Errorf("failed to set resource annotations, err: %w", err) - return syncResult + if l4c.enableDualStack { + l4c.emitEnsuredDualStackEvent(service) + if err = updateL4DualStackResourcesAnnotations(l4c.ctx, service, syncResult.Annotations); err != nil { + l4c.ctx.Recorder(service.Namespace).Eventf(service, v1.EventTypeWarning, "SyncLoadBalancerFailed", + "Failed to update Dual Stack annotations for load balancer, err: %v", err) + syncResult.Error = fmt.Errorf("failed to set Dual Stack resource annotations, err: %w", err) + return syncResult + } + } 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) + syncResult.Error = fmt.Errorf("failed to set resource annotations, err: %w", err) + return syncResult + } } 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), + Service: svc, + Cloud: l4c.ctx.Cloud, + Namer: l4c.namer, + Recorder: l4c.ctx.Recorder(svc.Namespace), + DualStackEnabled: l4c.enableDualStack, } l4 := loadbalancers.NewL4Handler(l4ilbParams) l4c.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeNormal, "DeletingLoadBalancer", "Deleting load balancer for %s", key) @@ -277,11 +301,20 @@ func (l4c *L4Controller) processServiceDeletion(key string, svc *v1.Service) *lo return result } // Also remove any ILB annotations from the service metadata - if err := updateL4ResourcesAnnotations(l4c.ctx, svc, nil); err != nil { - l4c.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeWarning, "DeleteLoadBalancer", - "Error resetting resource annotations for load balancer: %v", err) - result.Error = fmt.Errorf("failed to reset resource annotations, err: %w", err) - return result + if l4c.enableDualStack { + if err := updateL4DualStackResourcesAnnotations(l4c.ctx, svc, nil); err != nil { + l4c.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeWarning, "DeleteLoadBalancer", + "Error resetting resource annotations for load balancer: %v", err) + result.Error = fmt.Errorf("failed to reset resource annotations, err: %w", err) + return result + } + } else { + if err := updateL4ResourcesAnnotations(l4c.ctx, svc, nil); err != nil { + l4c.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeWarning, "DeleteLoadBalancer", + "Error resetting resource annotations for load balancer: %v", err) + result.Error = fmt.Errorf("failed to reset resource annotations, err: %w", err) + return result + } } if err := common.EnsureDeleteServiceFinalizer(svc, common.ILBFinalizerV2, l4c.ctx.KubeClient); err != nil { l4c.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeWarning, "DeleteLoadBalancerFailed", diff --git a/pkg/l4lb/l4controller_test.go b/pkg/l4lb/l4controller_test.go index a440eea214..8ffff41879 100644 --- a/pkg/l4lb/l4controller_test.go +++ b/pkg/l4lb/l4controller_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/ingress-gce/pkg/healthchecksl4" "k8s.io/ingress-gce/pkg/loadbalancers" + "k8s.io/ingress-gce/pkg/utils" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" @@ -50,13 +51,20 @@ const ( ) var ( - ilbAnnotationKeys = []string{ - annotations.FirewallRuleKey, + ilbCommonAnnotationKeys = []string{ annotations.BackendServiceKey, annotations.HealthcheckKey, + } + ilbIPv4AnnotationKeys = []string{ + annotations.FirewallRuleKey, annotations.TCPForwardingRuleKey, annotations.FirewallRuleForHealthcheckKey, } + ilbIPv6AnnotationKeys = []string{ + annotations.FirewallRuleIPv6Key, + annotations.TCPForwardingRuleIPv6Key, + annotations.FirewallRuleForHealthcheckIPv6Key, + } ) func newServiceController(t *testing.T, fakeGCE *gce.Cloud) *L4Controller { @@ -129,6 +137,17 @@ func getKeyForSvc(svc *api_v1.Service, t *testing.T) string { return key } +func calculateExpectedAnnotationsKeys(svc *api_v1.Service) []string { + expectedAnnotations := ilbCommonAnnotationKeys + if utils.NeedsIPv4(svc) { + expectedAnnotations = append(expectedAnnotations, ilbIPv4AnnotationKeys...) + } + if utils.NeedsIPv6(svc) { + expectedAnnotations = append(expectedAnnotations, ilbIPv6AnnotationKeys...) + } + return expectedAnnotations +} + func verifyILBServiceProvisioned(t *testing.T, svc *api_v1.Service) { t.Helper() @@ -139,8 +158,9 @@ func verifyILBServiceProvisioned(t *testing.T, svc *api_v1.Service) { t.Errorf("Invalid LoadBalancer status field in service - %+v", svc.Status.LoadBalancer) } + expectedAnnotationsKeys := calculateExpectedAnnotationsKeys(svc) var missingKeys []string - for _, key := range ilbAnnotationKeys { + for _, key := range expectedAnnotationsKeys { if _, ok := svc.Annotations[key]; !ok { missingKeys = append(missingKeys, key) } @@ -161,13 +181,14 @@ func verifyILBServiceNotProvisioned(t *testing.T, svc *api_v1.Service) { t.Errorf("Expected LoadBalancer status to be empty, Got %v", svc.Status.LoadBalancer) } + expectedAnnotationsKeys := calculateExpectedAnnotationsKeys(svc) var missingKeys []string - for _, key := range ilbAnnotationKeys { + for _, key := range expectedAnnotationsKeys { if _, ok := svc.Annotations[key]; !ok { missingKeys = append(missingKeys, key) } } - if len(missingKeys) != len(ilbAnnotationKeys) { + if len(missingKeys) != len(expectedAnnotationsKeys) { t.Errorf("Unexpected ILB annotations present, Got %v", svc.Annotations) } } @@ -618,6 +639,7 @@ func TestProcessServiceWithDelayedNEGAdd(t *testing.T) { } func TestProcessServiceOnError(t *testing.T) { + t.Parallel() l4c := newServiceController(t, newFakeGCEWithInsertError()) prevMetrics, err := test.GetL4ILBErrorMetric() if err != nil { @@ -639,3 +661,86 @@ func TestProcessServiceOnError(t *testing.T) { } prevMetrics.ValidateDiff(currMetrics, expectMetrics, t) } + +func TestCreateDeleteDualStackService(t *testing.T) { + testCases := []struct { + ipFamilies []api_v1.IPFamily + desc string + }{ + { + ipFamilies: []api_v1.IPFamily{api_v1.IPv4Protocol}, + desc: "Create and delete IPv4 ILB", + }, + { + ipFamilies: []api_v1.IPFamily{api_v1.IPv4Protocol, api_v1.IPv6Protocol}, + desc: "Create and delete IPv4 IPv6 ILB", + }, + { + ipFamilies: []api_v1.IPFamily{api_v1.IPv6Protocol}, + desc: "Create and delete IPv6 ILB", + }, + { + ipFamilies: []api_v1.IPFamily{api_v1.IPv6Protocol, api_v1.IPv4Protocol}, + desc: "Create and delete IPv6 IPv4 ILB", + }, + { + ipFamilies: []api_v1.IPFamily{}, + desc: "Create and delete ILB with empty IP families", + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + l4c := newServiceController(t, newFakeGCE()) + l4c.enableDualStack = true + prevMetrics, err := test.GetL4ILBLatencyMetric() + if err != nil { + t.Errorf("Error getting L4 ILB latency metrics err: %v", err) + } + newSvc := test.NewL4ILBDualStackService(8080, api_v1.ProtocolTCP, tc.ipFamilies, api_v1.ServiceExternalTrafficPolicyTypeCluster) + addILBService(l4c, newSvc) + addNEG(l4c, newSvc) + err = l4c.sync(getKeyForSvc(newSvc, t)) + if err != nil { + t.Errorf("Failed to sync newly added service %s, err %v", newSvc.Name, err) + } + // List the service and ensure that it contains the finalizer as well as Status field. + newSvc, err = l4c.client.CoreV1().Services(newSvc.Namespace).Get(context2.TODO(), newSvc.Name, v1.GetOptions{}) + if err != nil { + t.Errorf("Failed to lookup service %s, err: %v", newSvc.Name, err) + } + verifyILBServiceProvisioned(t, newSvc) + currMetrics, metricErr := test.GetL4ILBLatencyMetric() + if metricErr != nil { + t.Errorf("Error getting L4 ILB latency metrics err: %v", metricErr) + } + prevMetrics.ValidateDiff(currMetrics, &test.L4LBLatencyMetricInfo{CreateCount: 1, UpperBoundSeconds: 1}, t) + + // Remove the Internal LoadBalancer annotation, this should trigger a cleanup. + delete(newSvc.Annotations, gce.ServiceAnnotationLoadBalancerType) + updateILBService(l4c, newSvc) + err = l4c.sync(getKeyForSvc(newSvc, t)) + if err != nil { + t.Errorf("Failed to sync updated service %s, err %v", newSvc.Name, err) + } + + // List the service and ensure that it doesn't contain the finalizer as well as Status field. + newSvc, err = l4c.client.CoreV1().Services(newSvc.Namespace).Get(context2.TODO(), newSvc.Name, v1.GetOptions{}) + if err != nil { + t.Errorf("Failed to lookup service %s, err: %v", newSvc.Name, err) + } + verifyILBServiceNotProvisioned(t, newSvc) + currMetrics, metricErr = test.GetL4ILBLatencyMetric() + if metricErr != nil { + t.Errorf("Error getting L4 ILB latency metrics err: %v", metricErr) + } + prevMetrics.ValidateDiff(currMetrics, &test.L4LBLatencyMetricInfo{CreateCount: 1, DeleteCount: 1, UpperBoundSeconds: 1}, t) + newSvc.DeletionTimestamp = &v1.Time{} + updateILBService(l4c, newSvc) + key, _ := common.KeyFunc(newSvc) + if err = l4c.sync(key); err != nil { + t.Errorf("Failed to sync deleted service %s, err %v", key, err) + } + }) + } +} diff --git a/pkg/l4lb/l4lbcommon.go b/pkg/l4lb/l4lbcommon.go index 43c635854c..79c0b4253a 100644 --- a/pkg/l4lb/l4lbcommon.go +++ b/pkg/l4lb/l4lbcommon.go @@ -73,6 +73,16 @@ func updateL4ResourcesAnnotations(ctx *context.ControllerContext, svc *v1.Servic return patch.PatchServiceObjectMetadata(ctx.KubeClient.CoreV1(), svc, *newObjectMeta) } +// updateL4DualStackResourcesAnnotations this function checks if new annotations should be added to dual-stack service and patch service metadata if needed. +func updateL4DualStackResourcesAnnotations(ctx *context.ControllerContext, svc *v1.Service, newL4LBAnnotations map[string]string) error { + newObjectMeta := computeNewAnnotationsIfNeeded(svc, newL4LBAnnotations, loadbalancers.L4DualStackResourceAnnotationKeys) + if newObjectMeta == nil { + return nil + } + klog.V(3).Infof("Patching annotations of service %v/%v", svc.Namespace, svc.Name) + return patch.PatchServiceObjectMetadata(ctx.KubeClient.CoreV1(), svc, *newObjectMeta) +} + // deleteL4RBSAnnotations deletes all annotations which could be added by L4 ELB RBS controller func deleteL4RBSAnnotations(ctx *context.ControllerContext, svc *v1.Service) error { newObjectMeta := computeNewAnnotationsIfNeeded(svc, nil, loadbalancers.L4RBSAnnotations) diff --git a/pkg/l4lb/l4netlbcontroller_test.go b/pkg/l4lb/l4netlbcontroller_test.go index 6ddd905649..063b50f5ec 100644 --- a/pkg/l4lb/l4netlbcontroller_test.go +++ b/pkg/l4lb/l4netlbcontroller_test.go @@ -877,7 +877,7 @@ func TestHealthCheckWhenExternalTrafficPolicyWasUpdated(t *testing.T) { // Update ExternalTrafficPolicy to Cluster check if shared HC was created err = updateAndAssertExternalTrafficPolicy(newSvc, lc, v1.ServiceExternalTrafficPolicyTypeCluster, hcNameShared) if err != nil { - t.Errorf("Error asserthing shared health check %v", err) + t.Errorf("Error asserting shared health check %v", err) } newSvc.DeletionTimestamp = &metav1.Time{} updateNetLBService(lc, newSvc) diff --git a/pkg/loadbalancers/forwarding_rules_ipv6.go b/pkg/loadbalancers/forwarding_rules_ipv6.go new file mode 100644 index 0000000000..99beae94bd --- /dev/null +++ b/pkg/loadbalancers/forwarding_rules_ipv6.go @@ -0,0 +1,123 @@ +package loadbalancers + +import ( + "fmt" + + "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + corev1 "k8s.io/api/core/v1" + "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/events" + "k8s.io/ingress-gce/pkg/utils" + "k8s.io/klog/v2" + "k8s.io/legacy-cloud-providers/gce" +) + +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("l4.buildExpectedIPv6ForwardingRule(%s, %v) returned error %w, want nil", bsLink, options, err) + } + + existingIPv6FwdRule, err := l4.forwardingRules.Get(expectedIPv6FwdRule.Name) + if err != nil { + return nil, fmt.Errorf("l4.forwardingRules.GetForwardingRule(%s) returned error %w, want nil", expectedIPv6FwdRule.Name, err) + } + + if existingIPv6FwdRule != nil { + equal, err := EqualIPv6ForwardingRules(existingIPv6FwdRule, expectedIPv6FwdRule) + if err != nil { + return existingIPv6FwdRule, err + } + if equal { + klog.V(2).Infof("ensureIPv6ForwardingRule: Skipping update of unchanged ipv6 forwarding rule - %s", expectedIPv6FwdRule.Name) + return existingIPv6FwdRule, nil + } + err = l4.deleteChangedIPv6ForwardingRule(existingIPv6FwdRule, expectedIPv6FwdRule) + if err != nil { + return nil, err + } + } + klog.V(2).Infof("ensureIPv6ForwardingRule: Creating/Recreating forwarding rule - %s", expectedIPv6FwdRule.Name) + err = l4.forwardingRules.Create(expectedIPv6FwdRule) + if err != nil { + return nil, err + } + + createdFr, err := l4.forwardingRules.Get(expectedIPv6FwdRule.Name) + return createdFr, err +} + +func (l4 *L4) buildExpectedIPv6ForwardingRule(bsLink string, options gce.ILBOptions) (*composite.ForwardingRule, error) { + frName := l4.getIPv6FRName() + + 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 := l4.cloud.SubnetworkURL() + + if options.SubnetName != "" { + subnetworkURL, err = l4.getSubnetworkURLByName(options.SubnetName) + if err != nil { + return nil, err + } + } + + svcPorts := l4.Service.Spec.Ports + ports := utils.GetPorts(svcPorts) + protocol := utils.GetProtocol(svcPorts) + + fr := &composite.ForwardingRule{ + Name: frName, + Description: frDesc, + IPProtocol: string(protocol), + Ports: ports, + LoadBalancingScheme: string(cloud.SchemeInternal), + BackendService: bsLink, + IpVersion: "IPV6", + Network: l4.cloud.NetworkURL(), + Subnetwork: subnetworkURL, + AllowGlobalAccess: options.AllowGlobalAccess, + NetworkTier: cloud.NetworkTierPremium.ToGCEValue(), + } + if len(ports) > maxL4ILBPorts { + fr.Ports = nil + fr.AllPorts = true + } + + 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 { + return false, fmt.Errorf("EqualIPv6ForwardingRules(): failed to parse backend resource URL from FR, err - %w", err) + } + id2, err := cloud.ParseResourceURL(fr2.BackendService) + if err != nil { + return false, fmt.Errorf("EqualIPv6ForwardingRules(): failed to parse resource URL from FR, err - %w", err) + } + return fr1.IPProtocol == fr2.IPProtocol && + fr1.LoadBalancingScheme == fr2.LoadBalancingScheme && + utils.EqualStringSets(fr1.Ports, fr2.Ports) && + id1.Equal(id2) && + fr1.AllowGlobalAccess == fr2.AllowGlobalAccess && + fr1.AllPorts == fr2.AllPorts && + fr1.Subnetwork == fr2.Subnetwork && + fr1.NetworkTier == fr2.NetworkTier, nil +} diff --git a/pkg/loadbalancers/l4.go b/pkg/loadbalancers/l4.go index d6a646af42..e2ab8441bd 100644 --- a/pkg/loadbalancers/l4.go +++ b/pkg/loadbalancers/l4.go @@ -53,6 +53,7 @@ type L4 struct { NamespacedName types.NamespacedName forwardingRules ForwardingRulesProvider healthChecks healthchecksl4.L4HealthChecks + enableDualStack bool } // L4ILBSyncResult contains information about the outcome of an L4 ILB sync. It stores the list of resource name annotations, @@ -68,10 +69,11 @@ type L4ILBSyncResult struct { } type L4ILBParams struct { - Service *corev1.Service - Cloud *gce.Cloud - Namer namer.L4ResourcesNamer - Recorder record.EventRecorder + Service *corev1.Service + Cloud *gce.Cloud + Namer namer.L4ResourcesNamer + Recorder record.EventRecorder + DualStackEnabled bool } // NewL4Handler creates a new L4Handler for the given L4 service. @@ -85,6 +87,7 @@ func NewL4Handler(params *L4ILBParams) *L4 { Service: params.Service, healthChecks: healthchecksl4.GetInstance(), forwardingRules: forwardingrules.New(params.Cloud, meta.VersionGA, scope), + enableDualStack: params.DualStackEnabled, } l4.NamespacedName = types.NamespacedName{Name: params.Service.Name, Namespace: params.Service.Namespace} l4.backendPool = backends.NewPool(l4.cloud, l4.namer) @@ -108,32 +111,15 @@ func getILBOptions(svc *corev1.Service) gce.ILBOptions { func (l4 *L4) EnsureInternalLoadBalancerDeleted(svc *corev1.Service) *L4ILBSyncResult { klog.V(2).Infof("EnsureInternalLoadBalancerDeleted(%s): attempting delete of load balancer resources", l4.NamespacedName.String()) result := &L4ILBSyncResult{SyncType: SyncTypeDelete, StartTime: time.Now()} - // All resources use the L4Backend Name, except forwarding rule. - name := l4.namer.L4Backend(svc.Namespace, svc.Name) - frName := l4.GetFRName() - // If any resource deletion fails, log the error and continue cleanup. - err := l4.forwardingRules.Delete(frName) - if err != nil { - klog.Errorf("Failed to delete forwarding rule for internal loadbalancer service %s, err %v", l4.NamespacedName.String(), err) - result.Error = err - result.GCEResourceInError = annotations.ForwardingRuleResource - } - if err = ensureAddressDeleted(l4.cloud, frName, l4.cloud.Region()); err != nil { - klog.Errorf("Failed to delete address for internal loadbalancer service %s, err %v", l4.NamespacedName.String(), err) - result.Error = err - result.GCEResourceInError = annotations.AddressResource - } - // delete firewall rule allowing load balancer source ranges - firewallName := l4.namer.L4Firewall(l4.Service.Namespace, l4.Service.Name) - err = l4.deleteFirewall(firewallName) - if err != nil { - klog.Errorf("Failed to delete firewall rule %s for internal loadbalancer service %s, err %v", firewallName, l4.NamespacedName.String(), err) - result.GCEResourceInError = annotations.FirewallRuleResource - result.Error = err + l4.deleteIPv4ResourcesOnDelete(result) + if l4.enableDualStack { + l4.deleteIPv6ResourcesOnDelete(result) } + // Delete backend service - err = utils.IgnoreHTTPNotFound(l4.backendPool.Delete(name, meta.VersionGA, meta.Regional)) + bsName := l4.namer.L4Backend(svc.Namespace, svc.Name) + err := utils.IgnoreHTTPNotFound(l4.backendPool.Delete(bsName, meta.VersionGA, meta.Regional)) if err != nil { klog.Errorf("Failed to delete backends for internal loadbalancer service %s, err %v", l4.NamespacedName.String(), err) result.GCEResourceInError = annotations.BackendServiceResource @@ -147,13 +133,61 @@ func (l4 *L4) EnsureInternalLoadBalancerDeleted(svc *corev1.Service) *L4ILBSyncR // When service is deleted we need to check both health checks shared and non-shared // and delete them if needed. for _, isShared := range []bool{true, false} { - resourceInError, err := l4.healthChecks.DeleteHealthCheckWithFirewall(svc, l4.namer, isShared, meta.Global, utils.ILB) + if l4.enableDualStack { + resourceInError, err := l4.healthChecks.DeleteHealthCheckWithDualStackFirewalls(svc, l4.namer, isShared, meta.Global, utils.ILB) + if err != nil { + result.GCEResourceInError = resourceInError + result.Error = err + } + } else { + resourceInError, err := l4.healthChecks.DeleteHealthCheckWithFirewall(svc, l4.namer, isShared, meta.Global, utils.ILB) + if err != nil { + result.GCEResourceInError = resourceInError + result.Error = err + } + } + } + return result +} + +func (l4 *L4) deleteIPv4ResourcesOnDelete(result *L4ILBSyncResult) { + l4.deleteIPv4ResourcesAnnotationBased(result, false) +} + +func (l4 *L4) deleteIPv4ResourcesOnSync(result *L4ILBSyncResult) { + l4.deleteIPv4ResourcesAnnotationBased(result, true) +} + +func (l4 *L4) deleteIPv4ResourcesAnnotationBased(result *L4ILBSyncResult, shouldCheckAnnotations bool) { + frName := l4.GetFRName() + if !shouldCheckAnnotations || l4.hasAnnotation(annotations.TCPForwardingRuleKey) || l4.hasAnnotation(annotations.UDPForwardingRuleKey) { + err := l4.forwardingRules.Delete(frName) if err != nil { - result.GCEResourceInError = resourceInError + klog.Errorf("Failed to delete forwarding rule for internal loadbalancer service %s, err %v", l4.NamespacedName.String(), err) + result.Error = err + result.GCEResourceInError = annotations.ForwardingRuleResource + } + } + + // Deleting non-existent address do not print error audit logs, and we don't store address in annotations + // that's why we can delete it without checking annotation + err := ensureAddressDeleted(l4.cloud, frName, l4.cloud.Region()) + if err != nil { + klog.Errorf("Failed to delete address for internal loadbalancer service %s, err %v", l4.NamespacedName.String(), err) + result.Error = err + result.GCEResourceInError = annotations.AddressResource + } + + // delete firewall rule allowing load balancer source ranges + if !shouldCheckAnnotations || l4.hasAnnotation(annotations.FirewallRuleKey) { + firewallName := l4.namer.L4Firewall(l4.Service.Namespace, l4.Service.Name) + err = l4.deleteFirewall(firewallName) + if err != nil { + klog.Errorf("Failed to delete firewall rule %s for internal loadbalancer service %s, err %v", firewallName, l4.NamespacedName.String(), err) + result.GCEResourceInError = annotations.FirewallRuleResource result.Error = err } } - return result } func (l4 *L4) deleteFirewall(name string) error { @@ -200,19 +234,12 @@ func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service name := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) options := getILBOptions(l4.Service) - // create healthcheck - sharedHC := !helpers.RequestsOnlyLocalTraffic(l4.Service) - hcResult := l4.healthChecks.EnsureHealthCheckWithFirewall(l4.Service, l4.namer, sharedHC, meta.Global, utils.ILB, nodeNames) - - if hcResult.Err != nil { - result.GCEResourceInError = hcResult.GceResourceInError - result.Error = hcResult.Err + hcLink := l4.provideHealthChecks(nodeNames, result) + if result.Error != nil { return result } - result.Annotations[annotations.HealthcheckKey] = hcResult.HCName servicePorts := l4.Service.Spec.Ports - portRanges := utils.GetServicePortRanges(servicePorts) protocol := utils.GetProtocol(servicePorts) // Check if protocol has changed for this service. In this case, forwarding rule should be deleted before @@ -222,6 +249,7 @@ func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service if err != nil { klog.Errorf("Failed to lookup existing backend service, ignoring err: %v", err) } + existingFR, err := l4.forwardingRules.Get(l4.GetFRName()) if existingBS != nil && existingBS.Protocol != string(protocol) { klog.Infof("Protocol changed from %q to %q for service %s", existingBS.Protocol, string(protocol), l4.NamespacedName) @@ -235,10 +263,19 @@ func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service if err != nil { klog.Errorf("Failed to delete forwarding rule %s, err %v", frName, err) } + + if l4.enableDualStack { + // Delete ipv6 forwarding rule if it exists + ipv6FrName := l4.getIPv6FRNameWithProtocol(existingBS.Protocol) + err = l4.forwardingRules.Delete(ipv6FrName) + if err != nil { + klog.Errorf("Failed to delete ipv6 forwarding rule %s, err %v", ipv6FrName, err) + } + } } // ensure backend service - bs, err := l4.backendPool.EnsureL4BackendService(name, hcResult.HCLink, string(protocol), string(l4.Service.Spec.SessionAffinity), + bs, err := l4.backendPool.EnsureL4BackendService(name, hcLink, string(protocol), string(l4.Service.Spec.SessionAffinity), string(cloud.SchemeInternal), l4.NamespacedName, meta.VersionGA) if err != nil { result.GCEResourceInError = annotations.BackendServiceResource @@ -246,13 +283,83 @@ func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service return result } result.Annotations[annotations.BackendServiceKey] = name - // create fr rule + + if l4.enableDualStack { + if utils.NeedsIPv4(l4.Service) { + l4.ensureIPv4Resources(result, nodeNames, options, bs, existingFR) + if result.Error != nil { + return result + } + } else { + l4.deleteIPv4ResourcesOnSync(result) + } + if utils.NeedsIPv6(l4.Service) { + l4.ensureIPv6Resources(result, nodeNames, options, bs.SelfLink) + if result.Error != nil { + return result + } + } else { + l4.deleteIPv6ResourcesOnSync(result) + } + } else { + l4.ensureIPv4Resources(result, nodeNames, options, bs, existingFR) + } + + result.MetricsState.InSuccess = true + if options.AllowGlobalAccess { + result.MetricsState.EnabledGlobalAccess = true + } + // SubnetName is overwritten to nil value if Alpha feature gate for custom subnet + // is not enabled. So, a non empty subnet name at this point implies that the + // feature is in use. + if options.SubnetName != "" { + result.MetricsState.EnabledCustomSubnet = true + } + return result +} + +func (l4 *L4) provideHealthChecks(nodeNames []string, result *L4ILBSyncResult) string { + sharedHC := !helpers.RequestsOnlyLocalTraffic(l4.Service) + var hcResult *healthchecksl4.EnsureHealthCheckResult + if l4.enableDualStack { + hcResult = l4.healthChecks.EnsureHealthCheckWithDualStackFirewalls(l4.Service, l4.namer, sharedHC, meta.Global, utils.ILB, nodeNames, utils.NeedsIPv4(l4.Service), utils.NeedsIPv6(l4.Service)) + if hcResult.HCFirewallRuleName != "" { + result.Annotations[annotations.FirewallRuleForHealthcheckKey] = hcResult.HCFirewallRuleName + } + if hcResult.HCFirewallRuleIPv6Name != "" { + result.Annotations[annotations.FirewallRuleForHealthcheckIPv6Key] = hcResult.HCFirewallRuleIPv6Name + } + } else { + hcResult = l4.healthChecks.EnsureHealthCheckWithFirewall(l4.Service, l4.namer, sharedHC, meta.Global, utils.ILB, nodeNames) + result.Annotations[annotations.FirewallRuleForHealthcheckKey] = hcResult.HCFirewallRuleName + } + if hcResult.Err != nil { + result.GCEResourceInError = hcResult.GceResourceInError + result.Error = hcResult.Err + return "" + } + result.Annotations[annotations.HealthcheckKey] = hcResult.HCName + return hcResult.HCLink +} + +func (l4 *L4) getSubnetworkURLByName(subnetName string) (string, error) { + subnetKey, err := l4.CreateKey(subnetName) + if err != nil { + return "", err + } + return cloud.SelfLink(meta.VersionGA, l4.cloud.NetworkProjectID(), "subnetworks", subnetKey), nil +} + +// ensureIPv4Resources creates resources specific to IPv4 L4 Load Balancers: +// - IPv4 Forwarding Rule +// - IPv4 Firewall +func (l4 *L4) ensureIPv4Resources(result *L4ILBSyncResult, nodeNames []string, options gce.ILBOptions, bs *composite.BackendService, existingFR *composite.ForwardingRule) { fr, err := l4.ensureForwardingRule(bs.SelfLink, options, existingFR) if err != nil { klog.Errorf("EnsureInternalLoadBalancer: Failed to create forwarding rule - %v", err) result.GCEResourceInError = annotations.ForwardingRuleResource result.Error = err - return result + return } if fr.IPProtocol == string(corev1.ProtocolTCP) { result.Annotations[annotations.TCPForwardingRuleKey] = fr.Name @@ -264,16 +371,20 @@ func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service sourceRanges, err := helpers.GetLoadBalancerSourceRanges(l4.Service) if err != nil { result.Error = err - return result + return } + + fwName := l4.namer.L4Firewall(l4.Service.Namespace, l4.Service.Name) + servicePorts := l4.Service.Spec.Ports + protocol := utils.GetProtocol(servicePorts) + portRanges := utils.GetServicePortRanges(servicePorts) // Add firewall rule for ILB traffic to nodes - firewallName := l4.namer.L4Firewall(l4.Service.Namespace, l4.Service.Name) nodesFWRParams := firewalls.FirewallParams{ PortRanges: portRanges, SourceRanges: sourceRanges.StringSlice(), DestinationRanges: []string{fr.IPAddress}, Protocol: string(protocol), - Name: firewallName, + Name: fwName, NodeNames: nodeNames, L4Type: utils.ILB, } @@ -281,29 +392,16 @@ func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service if err := firewalls.EnsureL4LBFirewallForNodes(l4.Service, &nodesFWRParams, l4.cloud, l4.recorder); err != nil { result.GCEResourceInError = annotations.FirewallRuleResource result.Error = err - return result + return } - result.Annotations[annotations.FirewallRuleKey] = firewallName - result.Annotations[annotations.FirewallRuleForHealthcheckKey] = hcResult.HCFirewallRuleName + result.Annotations[annotations.FirewallRuleKey] = fwName - result.MetricsState.InSuccess = true - if options.AllowGlobalAccess { - result.MetricsState.EnabledGlobalAccess = true - } - // SubnetName is overwritten to nil value if Alpha feature gate for custom subnet - // is not enabled. So, a non empty subnet name at this point implies that the - // feature is in use. - if options.SubnetName != "" { - result.MetricsState.EnabledCustomSubnet = true - } - result.Status = &corev1.LoadBalancerStatus{Ingress: []corev1.LoadBalancerIngress{{IP: fr.IPAddress}}} - return result + result.Status = utils.AddIPToLBStatus(result.Status, fr.IPAddress) } -func (l4 *L4) getSubnetworkURLByName(subnetName string) (string, error) { - subnetKey, err := l4.CreateKey(subnetName) - if err != nil { - return "", err +func (l4 *L4) hasAnnotation(annotationKey string) bool { + if _, ok := l4.Service.Annotations[annotationKey]; ok { + return true } - return cloud.SelfLink(meta.VersionGA, l4.cloud.NetworkProjectID(), "subnetworks", subnetKey), nil + return false } diff --git a/pkg/loadbalancers/l4_test.go b/pkg/loadbalancers/l4_test.go index d0d8636413..d29ea42b12 100644 --- a/pkg/loadbalancers/l4_test.go +++ b/pkg/loadbalancers/l4_test.go @@ -1389,17 +1389,304 @@ func TestEnsureInternalLoadBalancerAllPorts(t *testing.T) { assertILBResourcesDeleted(t, l4) } +func TestEnsureInternalDualStackLoadBalancer(t *testing.T) { + t.Parallel() + nodeNames := []string{"test-node-1"} + vals := gce.DefaultTestClusterValues() + + namer := namer_util.NewL4Namer(kubeSystemUID, nil) + + testCases := []struct { + ipFamilies []v1.IPFamily + trafficPolicy v1.ServiceExternalTrafficPolicyType + desc string + }{ + { + desc: "Test ipv4 ipv6 service", + ipFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + trafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, + }, + { + desc: "Test ipv4 ipv6 local service", + ipFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + trafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal, + }, + { + desc: "Test ipv6 ipv4 service", + ipFamilies: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}, + trafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, + }, + { + desc: "Test ipv4 service", + ipFamilies: []v1.IPFamily{v1.IPv4Protocol}, + trafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, + }, + { + desc: "Test ipv6 service", + ipFamilies: []v1.IPFamily{v1.IPv6Protocol}, + trafficPolicy: v1.ServiceExternalTrafficPolicyTypeCluster, + }, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.desc, func(t *testing.T) { + t.Parallel() + + fakeGCE := getFakeGCECloud(vals) + + svc := test.NewL4ILBDualStackService(8080, v1.ProtocolTCP, tc.ipFamilies, tc.trafficPolicy) + + l4ilbParams := &L4ILBParams{ + Service: svc, + Cloud: fakeGCE, + Namer: namer, + Recorder: record.NewFakeRecorder(100), + DualStackEnabled: true, + } + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) + + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { + t.Errorf("Unexpected error when adding nodes %v", err) + } + + result := l4.EnsureInternalLoadBalancer(nodeNames, svc) + if result.Error != nil { + t.Errorf("Failed to ensure loadBalancer, err %v", result.Error) + } + if len(result.Status.Ingress) == 0 { + t.Errorf("Got empty loadBalancer status using handler %v", l4) + } + l4.Service.Annotations = result.Annotations + assertDualStackILBResources(t, l4, nodeNames) + + l4.EnsureInternalLoadBalancerDeleted(l4.Service) + assertDualStackILBResourcesDeleted(t, l4) + }) + } +} + +// This is exhaustive test that checks for all possible transitions of +// - ServiceExternalTrafficPolicy +// - Protocol +// - IPFamilies +// for dual-stack service. In total 401 combinations +func TestDualStackLoadBalancerTransitions(t *testing.T) { + t.Parallel() + nodeNames := []string{"test-node-1"} + vals := gce.DefaultTestClusterValues() + + namer := namer_util.NewL4Namer(kubeSystemUID, nil) + + trafficPolicyStates := []v1.ServiceExternalTrafficPolicyType{v1.ServiceExternalTrafficPolicyTypeLocal, v1.ServiceExternalTrafficPolicyTypeCluster} + protocols := []v1.Protocol{v1.ProtocolTCP, v1.ProtocolUDP} + ipFamiliesStates := [][]v1.IPFamily{ + {v1.IPv4Protocol}, + {v1.IPv4Protocol, v1.IPv6Protocol}, + {v1.IPv6Protocol}, + {v1.IPv6Protocol, v1.IPv4Protocol}, + {}, + } + + for _, initialIPFamily := range ipFamiliesStates { + for _, finalIPFamily := range ipFamiliesStates { + for _, initialTrafficPolicy := range trafficPolicyStates { + for _, finalTrafficPolicy := range trafficPolicyStates { + for _, initialProtocol := range protocols { + for _, finalProtocol := range protocols { + initialIPFamily := initialIPFamily + finalIPFamily := finalIPFamily + initialTrafficPolicy := initialTrafficPolicy + finalTrafficPolicy := finalTrafficPolicy + initialProtocol := initialProtocol + finalProtocol := finalProtocol + + var stringInitialIPFamily []string + for _, f := range initialIPFamily { + stringInitialIPFamily = append(stringInitialIPFamily, string(f)) + } + + var stringFinalIPFamily []string + for _, f := range finalIPFamily { + stringFinalIPFamily = append(stringFinalIPFamily, string(f)) + } + desc := struct { + fromIPFamily string + toIPFamily string + fromTrafficPolicy string + toTrafficPolicy string + fromProtocol string + toProtocol string + }{ + strings.Join(stringInitialIPFamily, ","), + strings.Join(stringFinalIPFamily, ","), + string(initialTrafficPolicy), + string(finalTrafficPolicy), + string(initialProtocol), + string(finalProtocol), + } + + t.Run(fmt.Sprintf("+%v", desc), func(t *testing.T) { + t.Parallel() + + fakeGCE := getFakeGCECloud(vals) + + svc := test.NewL4ILBDualStackService(8080, initialProtocol, initialIPFamily, initialTrafficPolicy) + l4ilbParams := &L4ILBParams{ + Service: svc, + Cloud: fakeGCE, + Namer: namer, + Recorder: record.NewFakeRecorder(100), + DualStackEnabled: true, + } + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) + + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { + t.Errorf("Unexpected error when adding nodes %v", err) + } + + result := l4.EnsureInternalLoadBalancer(nodeNames, svc) + svc.Annotations = result.Annotations + assertDualStackILBResources(t, l4, nodeNames) + + finalSvc := test.NewL4ILBDualStackService(8080, finalProtocol, finalIPFamily, finalTrafficPolicy) + finalSvc.Annotations = svc.Annotations + l4.Service = finalSvc + + result = l4.EnsureInternalLoadBalancer(nodeNames, svc) + finalSvc.Annotations = result.Annotations + assertDualStackILBResources(t, l4, nodeNames) + + l4.EnsureInternalLoadBalancerDeleted(l4.Service) + assertDualStackILBResourcesDeleted(t, l4) + }) + } + } + } + } + } + } +} + +func TestDualStackLBCleansOnlyAnnotationResources(t *testing.T) { + t.Parallel() + nodeNames := []string{"test-node-1"} + vals := gce.DefaultTestClusterValues() + + testCases := []struct { + desc string + ipFamiliesStates [2][]v1.IPFamily + annotationsToDelete []string + verifyResourcesNotDeleted func(l4 *L4) error + }{ + { + desc: "Should not delete IPv6 resources if they not exist in annotation", + ipFamiliesStates: [2][]v1.IPFamily{{v1.IPv4Protocol, v1.IPv6Protocol}, {v1.IPv4Protocol}}, + annotationsToDelete: []string{annotations.TCPForwardingRuleIPv6Key, annotations.FirewallRuleIPv6Key, annotations.FirewallRuleForHealthcheckIPv6Key}, + verifyResourcesNotDeleted: func(l4 *L4) error { + // Verify IPv6 Firewall was not deleted + ipv6FWName := l4.namer.L4IPv6Firewall(l4.Service.Namespace, l4.Service.Name) + err := verifyFirewallNotExists(l4, ipv6FWName) + if err == nil { + return fmt.Errorf("firewall rule %s was deleted, expected not", ipv6FWName) + } + + // Verify IPv6 Forwarding Rule was not deleted + ipv6FRName := l4.getIPv6FRName() + err = verifyForwardingRuleNotExists(l4, ipv6FRName) + if err == nil { + return fmt.Errorf("forwarding rule %s was deleted, expected not", ipv6FRName) + } + return nil + }, + }, + { + desc: "Should not delete IPv4 resources if they not exist in annotation", + ipFamiliesStates: [2][]v1.IPFamily{{v1.IPv6Protocol, v1.IPv4Protocol}, {v1.IPv6Protocol}}, + annotationsToDelete: []string{annotations.TCPForwardingRuleKey, annotations.FirewallRuleKey, annotations.FirewallRuleForHealthcheckKey}, + verifyResourcesNotDeleted: func(l4 *L4) error { + // Verify IPv6 Firewall was not deleted + backendServiceName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) + err := verifyFirewallNotExists(l4, backendServiceName) + if err == nil { + return fmt.Errorf("firewall rule %s was deleted, expected not", backendServiceName) + } + + // Verify IPv6 Forwarding Rule was not deleted + ipv4FRName := l4.GetFRName() + err = verifyForwardingRuleNotExists(l4, ipv4FRName) + if err == nil { + return fmt.Errorf("forwarding rule %s was deleted, expected not", ipv4FRName) + } + return nil + }, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.desc, func(t *testing.T) { + t.Parallel() + + namer := namer_util.NewL4Namer(kubeSystemUID, nil) + fakeGCE := getFakeGCECloud(vals) + + svc := test.NewL4ILBService(false, 8080) + l4ilbParams := &L4ILBParams{ + Service: svc, + Cloud: fakeGCE, + Namer: namer, + Recorder: record.NewFakeRecorder(100), + DualStackEnabled: true, + } + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) + + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { + t.Errorf("Unexpected error when adding nodes %v", err) + } + + svc.Spec.IPFamilies = tc.ipFamiliesStates[0] + result := l4.EnsureInternalLoadBalancer(nodeNames, svc) + svc.Annotations = result.Annotations + assertDualStackILBResources(t, l4, nodeNames) + + // Delete resources annotation + for _, annotationToDelete := range tc.annotationsToDelete { + delete(svc.Annotations, annotationToDelete) + } + svc.Spec.IPFamilies = tc.ipFamiliesStates[1] + + // Run new sync. Controller should not delete resources, if they don't exist in annotation + result = l4.EnsureInternalLoadBalancer(nodeNames, svc) + svc.Annotations = result.Annotations + + err := tc.verifyResourcesNotDeleted(l4) + if err != nil { + t.Errorf("tc.verifyResourcesNotDeleted(_) returned error %v, want nil", err) + } + + l4.EnsureInternalLoadBalancerDeleted(l4.Service) + // After complete deletion, IPv6 and IPv4 resources should be cleaned up, even if the were leaked + assertDualStackILBResourcesDeleted(t, l4) + }) + } +} + func assertILBResources(t *testing.T, l4 *L4, nodeNames []string, resourceAnnotations map[string]string) { t.Helper() - err := verifyNodesFirewall(l4, nodeNames) + err := verifyIPv4NodesFirewall(l4, nodeNames) if err != nil { - t.Errorf("verifyNodesFirewall(_, %v) returned error %v, want nil", nodeNames, err) + t.Errorf("verifyIPv4NodesFirewall(_, %v) returned error %v, want nil", nodeNames, err) } - err = verifyHealthCheckFirewall(l4, nodeNames) + err = verifyIPv4HealthCheckFirewall(l4, nodeNames) if err != nil { - t.Errorf("verifyHealthCheckFirewall(_, %v) returned error %v, want nil", nodeNames, err) + t.Errorf("verifyIPv4HealthCheckFirewall(_, %v) returned error %v, want nil", nodeNames, err) } healthCheck, err := getAndVerifyHealthCheck(l4) @@ -1412,7 +1699,7 @@ func assertILBResources(t *testing.T, l4 *L4, nodeNames []string, resourceAnnota t.Errorf("getAndVerifyBackendService(_, %v) returned error %v, want nil", healthCheck, err) } - err = verifyForwardingRule(l4, backendService.SelfLink) + err = verifyIPv4ForwardingRule(l4, backendService.SelfLink) if err != nil { t.Errorf("verifyForwardingRule(_, %s) returned error %v, want nil", backendService.SelfLink, err) } @@ -1423,6 +1710,111 @@ func assertILBResources(t *testing.T, l4 *L4, nodeNames []string, resourceAnnota } } +func assertDualStackILBResources(t *testing.T, l4 *L4, nodeNames []string) { + t.Helper() + + healthCheck, err := getAndVerifyHealthCheck(l4) + if err != nil { + t.Errorf("getAndVerifyHealthCheck(_) returned error %v, want nil", err) + } + + backendService, err := getAndVerifyBackendService(l4, healthCheck) + if err != nil { + t.Errorf("getAndVerifyBackendService(_, %v) returned error %v, want nil", healthCheck, err) + } + + if utils.NeedsIPv4(l4.Service) { + err = verifyIPv4ForwardingRule(l4, backendService.SelfLink) + if err != nil { + t.Errorf("verifyIPv4ForwardingRule(_, %s) returned error %v, want nil", backendService.SelfLink, err) + } + + err = verifyIPv4NodesFirewall(l4, nodeNames) + if err != nil { + t.Errorf("verifyIPv4NodesFirewall(_, %s) returned error %v, want nil", nodeNames, err) + } + + err = verifyIPv4HealthCheckFirewall(l4, nodeNames) + if err != nil { + t.Errorf("verifyIPv4HealthCheckFirewall(_, %s) returned error %v, want nil", nodeNames, err) + } + } else { + err = verifyIPv4ResourcesDeletedOnSync(l4) + if err != nil { + t.Errorf("verifyIPv4ResourcesDeletedOnSync(_) returned error %v, want nil", err) + } + } + if utils.NeedsIPv6(l4.Service) { + err = verifyIPv6ForwardingRule(l4, backendService.SelfLink) + if err != nil { + t.Errorf("verifyIPv6ForwardingRule(_, %s) returned error %v, want nil", backendService.SelfLink, err) + } + + err = verifyIPv6NodesFirewall(l4, nodeNames) + if err != nil { + t.Errorf("verifyIPv6NodesFirewall(_, %v) returned error %v, want nil", nodeNames, err) + } + + err = verifyIPv6HealthCheckFirewall(l4, nodeNames) + if err != nil { + t.Errorf("verifyIPv6HealthCheckFirewall(_, %v) returned error %v, want nil", nodeNames, err) + } + } else { + err = verifyIPv6ResourcesDeletedOnSync(l4) + if err != nil { + t.Errorf("verifyIPv6ResourcesDeletedOnSync(_) returned error %v, want nil", err) + } + } + + expectedAnnotations := buildExpectedAnnotations(l4) + if !reflect.DeepEqual(expectedAnnotations, l4.Service.Annotations) { + diff := cmp.Diff(expectedAnnotations, l4.Service.Annotations) + t.Errorf("Expected annotations %v, got %v, diff %v", expectedAnnotations, l4.Service.Annotations, diff) + } +} + +func buildExpectedAnnotations(l4 *L4) map[string]string { + isSharedHC := !servicehelper.RequestsOnlyLocalTraffic(l4.Service) + proto := utils.GetProtocol(l4.Service.Spec.Ports) + + backendName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) + hcName := l4.namer.L4HealthCheck(l4.Service.Namespace, l4.Service.Name, isSharedHC) + + expectedAnnotations := map[string]string{ + annotations.BackendServiceKey: backendName, + annotations.HealthcheckKey: hcName, + } + + if utils.NeedsIPv4(l4.Service) { + hcFwName := l4.namer.L4HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, isSharedHC) + + expectedAnnotations[annotations.FirewallRuleForHealthcheckKey] = hcFwName + expectedAnnotations[annotations.FirewallRuleKey] = backendName + + ipv4FRName := l4.GetFRName() + if proto == v1.ProtocolTCP { + expectedAnnotations[annotations.TCPForwardingRuleKey] = ipv4FRName + } else { + expectedAnnotations[annotations.UDPForwardingRuleKey] = ipv4FRName + } + } + if utils.NeedsIPv6(l4.Service) { + ipv6hcFwName := l4.namer.L4IPv6HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, isSharedHC) + ipv6FirewallName := l4.namer.L4IPv6Firewall(l4.Service.Namespace, l4.Service.Name) + + expectedAnnotations[annotations.FirewallRuleForHealthcheckIPv6Key] = ipv6hcFwName + expectedAnnotations[annotations.FirewallRuleIPv6Key] = ipv6FirewallName + + ipv6FRName := l4.getIPv6FRName() + if proto == v1.ProtocolTCP { + expectedAnnotations[annotations.TCPForwardingRuleIPv6Key] = ipv6FRName + } else { + expectedAnnotations[annotations.UDPForwardingRuleIPv6Key] = ipv6FRName + } + } + return expectedAnnotations +} + func getAndVerifyHealthCheck(l4 *L4) (*composite.HealthCheck, error) { isSharedHC := !servicehelper.RequestsOnlyLocalTraffic(l4.Service) hcName := l4.namer.L4HealthCheck(l4.Service.Namespace, l4.Service.Name, isSharedHC) @@ -1476,8 +1868,17 @@ func getAndVerifyBackendService(l4 *L4, healthCheck *composite.HealthCheck) (*co return bs, nil } -func verifyForwardingRule(l4 *L4, backendServiceLink string) error { +func verifyIPv4ForwardingRule(l4 *L4, backendServiceLink string) error { frName := l4.GetFRName() + return verifyForwardingRule(l4, frName, backendServiceLink) +} + +func verifyIPv6ForwardingRule(l4 *L4, backendServiceLink string) error { + ipv6FrName := l4.getIPv6FRName() + return verifyForwardingRule(l4, ipv6FrName, backendServiceLink) +} + +func verifyForwardingRule(l4 *L4, frName string, backendServiceLink string) error { fwdRule, err := composite.GetForwardingRule(l4.cloud, meta.RegionalKey(frName, l4.cloud.Region()), meta.VersionGA) if err != nil { return fmt.Errorf("failed to fetch forwarding rule %s - err %w", frName, err) @@ -1514,7 +1915,7 @@ func verifyForwardingRule(l4 *L4, backendServiceLink string) error { return nil } -func verifyNodesFirewall(l4 *L4, nodeNames []string) error { +func verifyIPv4NodesFirewall(l4 *L4, nodeNames []string) error { fwName := l4.namer.L4Firewall(l4.Service.Namespace, l4.Service.Name) fwDesc, err := utils.MakeL4LBServiceDescription(utils.ServiceKeyFunc(l4.Service.Namespace, l4.Service.Name), "", meta.VersionGA, false, utils.ILB) if err != nil { @@ -1528,7 +1929,18 @@ func verifyNodesFirewall(l4 *L4, nodeNames []string) error { return verifyFirewall(l4, nodeNames, fwName, fwDesc, sourceRanges.StringSlice()) } -func verifyHealthCheckFirewall(l4 *L4, nodeNames []string) error { +func verifyIPv6NodesFirewall(l4 *L4, nodeNames []string) error { + ipv6FirewallName := l4.namer.L4IPv6Firewall(l4.Service.Namespace, l4.Service.Name) + + fwDesc, err := utils.MakeL4LBServiceDescription(utils.ServiceKeyFunc(l4.Service.Namespace, l4.Service.Name), "", meta.VersionGA, false, utils.ILB) + if err != nil { + return fmt.Errorf("failed to create description for resources, err %w", err) + } + + return verifyFirewall(l4, nodeNames, ipv6FirewallName, fwDesc, []string{"0::0/0"}) +} + +func verifyIPv4HealthCheckFirewall(l4 *L4, nodeNames []string) error { isSharedHC := !servicehelper.RequestsOnlyLocalTraffic(l4.Service) hcFwName := l4.namer.L4HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, isSharedHC) @@ -1540,6 +1952,18 @@ func verifyHealthCheckFirewall(l4 *L4, nodeNames []string) error { return verifyFirewall(l4, nodeNames, hcFwName, hcFwDesc, gce.L4LoadBalancerSrcRanges()) } +func verifyIPv6HealthCheckFirewall(l4 *L4, nodeNames []string) error { + isSharedHC := !servicehelper.RequestsOnlyLocalTraffic(l4.Service) + + ipv6hcFwName := l4.namer.L4IPv6HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, isSharedHC) + hcFwDesc, err := utils.MakeL4LBFirewallDescription(utils.ServiceKeyFunc(l4.Service.Namespace, l4.Service.Name), "", meta.VersionGA, isSharedHC) + if err != nil { + return fmt.Errorf("failed to calculate decsription for health check for service %v, error %v", l4.Service, err) + } + + return verifyFirewall(l4, nodeNames, ipv6hcFwName, hcFwDesc, []string{healthchecksl4.L4ILBIPv6HCRange}) +} + func verifyFirewall(l4 *L4, nodeNames []string, firewallName, expectedDescription string, expectedSourceRanges []string) error { firewall, err := l4.cloud.GetFirewall(firewallName) if err != nil { @@ -1559,33 +1983,6 @@ func verifyFirewall(l4 *L4, nodeNames []string, firewallName, expectedDescriptio return nil } -func buildExpectedAnnotations(l4 *L4) map[string]string { - isSharedHC := !servicehelper.RequestsOnlyLocalTraffic(l4.Service) - proto := utils.GetProtocol(l4.Service.Spec.Ports) - - backendName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) - hcName := l4.namer.L4HealthCheck(l4.Service.Namespace, l4.Service.Name, isSharedHC) - - expectedAnnotations := map[string]string{ - annotations.BackendServiceKey: backendName, - annotations.HealthcheckKey: hcName, - } - - hcFwName := l4.namer.L4HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, isSharedHC) - expectedAnnotations[annotations.FirewallRuleForHealthcheckKey] = hcFwName - - fwName := l4.namer.L4Firewall(l4.Service.Namespace, l4.Service.Name) - expectedAnnotations[annotations.FirewallRuleKey] = fwName - - frName := l4.GetFRName() - if proto == v1.ProtocolTCP { - expectedAnnotations[annotations.TCPForwardingRuleKey] = frName - } else { - expectedAnnotations[annotations.UDPForwardingRuleKey] = frName - } - return expectedAnnotations -} - func assertILBResourcesDeleted(t *testing.T, l4 *L4) { t.Helper() @@ -1674,3 +2071,109 @@ func verifyAddressNotExists(l4 *L4, addressName string) error { } return nil } + +func assertDualStackILBResourcesDeleted(t *testing.T, l4 *L4) { + t.Helper() + + err := verifyCommonDualStackResourcesDeleted(l4) + if err != nil { + t.Errorf("verifyCommonDualStackResourcesDeleted(_) returned erorr %v, want nil", err) + } + + err = verifyIPv4ResourcesDeletedOnSync(l4) + if err != nil { + t.Errorf("verifyIPv4ResourcesDeletedOnSync(_) returned erorr %v, want nil", err) + } + + err = verifyIPv6ResourcesDeletedOnSync(l4) + if err != nil { + t.Errorf("verifyIPv4ResourcesDeletedOnSync(_) returned erorr %v, want nil", err) + } + + // Check health check firewalls separately, because we don't clean them on sync, only on final deletion + ipv4HcFwNameShared := l4.namer.L4HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, true) + ipv6HcFwNameShared := l4.namer.L4IPv6HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, true) + ipv4HcFwNameNonShared := l4.namer.L4HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, false) + ipv6HcFwNameNonShared := l4.namer.L4IPv6HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, false) + + fwNames := []string{ + ipv4HcFwNameShared, + ipv4HcFwNameNonShared, + ipv6HcFwNameShared, + ipv6HcFwNameNonShared, + } + + for _, fwName := range fwNames { + err = verifyFirewallNotExists(l4, fwName) + if err != nil { + t.Errorf("verifyFirewallNotExists(_, %s) returned error %v, want nil", fwName, err) + } + } +} + +func verifyCommonDualStackResourcesDeleted(l4 *L4) error { + backendServiceName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) + + err := verifyBackendServiceNotExists(l4, backendServiceName) + if err != nil { + return fmt.Errorf("verifyBackendServiceNotExists(_, %s)", backendServiceName) + } + + hcNameShared := l4.namer.L4HealthCheck(l4.Service.Namespace, l4.Service.Name, true) + err = verifyHealthCheckNotExists(l4, hcNameShared) + if err != nil { + return fmt.Errorf("verifyHealthCheckNotExists(_, %s)", hcNameShared) + } + + hcNameNonShared := l4.namer.L4HealthCheck(l4.Service.Namespace, l4.Service.Name, false) + err = verifyHealthCheckNotExists(l4, hcNameNonShared) + if err != nil { + return fmt.Errorf("verifyHealthCheckNotExists(_, %s)", hcNameNonShared) + } + + err = verifyAddressNotExists(l4, backendServiceName) + if err != nil { + return fmt.Errorf("verifyAddressNotExists(_, %s)", backendServiceName) + } + return nil +} + +// we don't delete ipv4 health check firewall on sync +func verifyIPv4ResourcesDeletedOnSync(l4 *L4) error { + ipv4FwName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) + err := verifyFirewallNotExists(l4, ipv4FwName) + if err != nil { + return fmt.Errorf("verifyFirewallNotExists(_, %s) returned error %w, want nil", ipv4FwName, err) + } + + ipv4FrName := l4.GetFRName() + err = verifyForwardingRuleNotExists(l4, ipv4FrName) + if err != nil { + return fmt.Errorf("verifyForwardingRuleNotExists(_, %s) returned error %w, want nil", ipv4FrName, err) + } + + addressName := ipv4FwName + err = verifyAddressNotExists(l4, addressName) + if err != nil { + return fmt.Errorf("verifyAddressNotExists(_, %s)", addressName) + } + + return nil +} + +// we don't delete ipv6 health check firewall on sync +func verifyIPv6ResourcesDeletedOnSync(l4 *L4) error { + ipv6FwName := l4.namer.L4IPv6Firewall(l4.Service.Namespace, l4.Service.Name) + err := verifyFirewallNotExists(l4, ipv6FwName) + if err != nil { + return fmt.Errorf("verifyFirewallNotExists(_, %s) returned error %w, want nil", ipv6FwName, err) + } + + ipv6FrName := l4.getIPv6FRName() + err = verifyForwardingRuleNotExists(l4, ipv6FrName) + if err != nil { + return fmt.Errorf("verifyForwardingRuleNotExists(_, %s) returned error %w, want nil", ipv6FrName, err) + } + + return nil +} diff --git a/pkg/loadbalancers/l4ipv6.go b/pkg/loadbalancers/l4ipv6.go new file mode 100644 index 0000000000..3e5021c5dc --- /dev/null +++ b/pkg/loadbalancers/l4ipv6.go @@ -0,0 +1,111 @@ +package loadbalancers + +import ( + "strings" + + corev1 "k8s.io/api/core/v1" + "k8s.io/ingress-gce/pkg/annotations" + "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/firewalls" + "k8s.io/ingress-gce/pkg/utils" + "k8s.io/klog/v2" + "k8s.io/legacy-cloud-providers/gce" +) + +// ensureIPv6Resources creates resources specific to IPv6 L4 Load Balancers: +// - IPv6 Forwarding Rule +// - IPv6 Firewall +// it also adds IPv6 address to LB status +func (l4 *L4) ensureIPv6Resources(syncResult *L4ILBSyncResult, nodeNames []string, options gce.ILBOptions, bsLink string) { + ipv6fr, err := l4.ensureIPv6ForwardingRule(bsLink, options) + if err != nil { + klog.Errorf("ensureIPv6Resources: Failed to create ipv6 forwarding rule - %v", err) + syncResult.GCEResourceInError = annotations.ForwardingRuleIPv6Resource + syncResult.Error = err + return + } + + if ipv6fr.IPProtocol == string(corev1.ProtocolTCP) { + syncResult.Annotations[annotations.TCPForwardingRuleIPv6Key] = ipv6fr.Name + } else { + syncResult.Annotations[annotations.UDPForwardingRuleIPv6Key] = ipv6fr.Name + } + + firewallName := l4.namer.L4IPv6Firewall(l4.Service.Namespace, l4.Service.Name) + err = l4.ensureIPv6Firewall(ipv6fr, firewallName, nodeNames) + if err != nil { + syncResult.GCEResourceInError = annotations.FirewallRuleIPv6Resource + syncResult.Error = err + return + } + syncResult.Annotations[annotations.FirewallRuleIPv6Key] = firewallName + + trimmedIPv6Address := strings.Split(ipv6fr.IPAddress, "/")[0] + syncResult.Status = utils.AddIPToLBStatus(syncResult.Status, trimmedIPv6Address) +} + +func (l4 *L4) deleteIPv6ResourcesOnSync(syncResult *L4ILBSyncResult) { + l4.deleteIPv6ResourcesAnnotationBased(syncResult, true) +} + +func (l4 *L4) deleteIPv6ResourcesOnDelete(syncResult *L4ILBSyncResult) { + l4.deleteIPv6ResourcesAnnotationBased(syncResult, false) +} + +func (l4 *L4) deleteIPv6ResourcesAnnotationBased(syncResult *L4ILBSyncResult, shouldCheckAnnotations bool) { + if !shouldCheckAnnotations || l4.hasAnnotation(annotations.TCPForwardingRuleIPv6Key) || l4.hasAnnotation(annotations.UDPForwardingRuleIPv6Key) { + err := l4.deleteIPv6ForwardingRule() + if err != nil { + klog.Errorf("Failed to delete ipv6 forwarding rule for internal loadbalancer service %s, err %v", l4.NamespacedName.String(), err) + syncResult.Error = err + syncResult.GCEResourceInError = annotations.ForwardingRuleIPv6Resource + } + } + + if !shouldCheckAnnotations || l4.hasAnnotation(annotations.FirewallRuleIPv6Key) { + err := l4.deleteIPv6Firewall() + if err != nil { + klog.Errorf("Failed to delete ipv6 firewall rule for internal loadbalancer service %s, err %v", l4.NamespacedName.String(), err) + syncResult.GCEResourceInError = annotations.FirewallRuleIPv6Resource + syncResult.Error = err + } + } +} + +func (l4 *L4) getIPv6FRName() string { + protocol := utils.GetProtocol(l4.Service.Spec.Ports) + return l4.getIPv6FRNameWithProtocol(string(protocol)) +} + +func (l4 *L4) getIPv6FRNameWithProtocol(protocol string) string { + return l4.namer.L4IPv6ForwardingRule(l4.Service.Namespace, l4.Service.Name, strings.ToLower(protocol)) +} + +func (l4 *L4) ensureIPv6Firewall(forwardingRule *composite.ForwardingRule, firewallName string, nodeNames []string) error { + svcPorts := l4.Service.Spec.Ports + portRanges := utils.GetServicePortRanges(svcPorts) + protocol := utils.GetProtocol(svcPorts) + + ipv6nodesFWRParams := firewalls.FirewallParams{ + PortRanges: portRanges, + // TODO(panslava): support .spec.loadBalancerSourceRanges + SourceRanges: []string{"0::0/0"}, + DestinationRanges: []string{forwardingRule.IPAddress}, + Protocol: string(protocol), + Name: firewallName, + NodeNames: nodeNames, + L4Type: utils.ILB, + } + + return firewalls.EnsureL4LBFirewallForNodes(l4.Service, &ipv6nodesFWRParams, l4.cloud, l4.recorder) +} + +func (l4 *L4) deleteIPv6ForwardingRule() error { + ipv6FrName := l4.getIPv6FRName() + return l4.forwardingRules.Delete(ipv6FrName) +} + +func (l4 *L4) deleteIPv6Firewall() error { + ipv6FirewallName := l4.namer.L4IPv6Firewall(l4.Service.Namespace, l4.Service.Name) + return l4.deleteFirewall(ipv6FirewallName) +} diff --git a/pkg/loadbalancers/l4netlb.go b/pkg/loadbalancers/l4netlb.go index 2d3390b8aa..6311ee6eef 100644 --- a/pkg/loadbalancers/l4netlb.go +++ b/pkg/loadbalancers/l4netlb.go @@ -158,7 +158,8 @@ func (l4netlb *L4NetLB) EnsureFrontend(nodeNames []string, svc *corev1.Service) } else { result.Annotations[annotations.UDPForwardingRuleKey] = fr.Name } - result.Status = &corev1.LoadBalancerStatus{Ingress: []corev1.LoadBalancerIngress{{IP: fr.IPAddress}}} + + result.Status = utils.AddIPToLBStatus(result.Status, fr.IPAddress) result.MetricsState.IsPremiumTier = fr.NetworkTier == cloud.NetworkTierPremium.ToGCEValue() result.MetricsState.IsManagedIP = ipAddrType == IPAddrManaged diff --git a/pkg/loadbalancers/l4syncresult.go b/pkg/loadbalancers/l4syncresult.go index ca1b27c8dd..7c87aac28d 100644 --- a/pkg/loadbalancers/l4syncresult.go +++ b/pkg/loadbalancers/l4syncresult.go @@ -35,3 +35,11 @@ var L4LBResourceAnnotationKeys = []string{ } var L4RBSAnnotations = append(L4LBResourceAnnotationKeys, annotations.RBSAnnotationKey) + +var l4IPv6AnnotationKeys = []string{ + annotations.FirewallRuleIPv6Key, + annotations.FirewallRuleForHealthcheckIPv6Key, + annotations.TCPForwardingRuleIPv6Key, + annotations.UDPForwardingRuleIPv6Key, +} +var L4DualStackResourceAnnotationKeys = append(L4LBResourceAnnotationKeys, l4IPv6AnnotationKeys...) diff --git a/pkg/test/utils.go b/pkg/test/utils.go index af7294e3f5..ead95efd1d 100644 --- a/pkg/test/utils.go +++ b/pkg/test/utils.go @@ -97,6 +97,27 @@ func NewL4ILBService(onlyLocal bool, port int) *api_v1.Service { return svc } +// NewL4ILBDualStackService creates a Service of type LoadBalancer with the Internal annotation and provided ipFamilies and ipFamilyPolicy +func NewL4ILBDualStackService(port int, protocol api_v1.Protocol, ipFamilies []api_v1.IPFamily, externalTrafficPolicy api_v1.ServiceExternalTrafficPolicyType) *api_v1.Service { + svc := &api_v1.Service{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: testServiceName, + Namespace: testServiceNamespace, + Annotations: map[string]string{gce.ServiceAnnotationLoadBalancerType: string(gce.LBTypeInternal)}, + }, + Spec: api_v1.ServiceSpec{ + Type: api_v1.ServiceTypeLoadBalancer, + SessionAffinity: api_v1.ServiceAffinityClientIP, + Ports: []api_v1.ServicePort{ + {Name: "testport", Port: int32(port), Protocol: protocol}, + }, + ExternalTrafficPolicy: externalTrafficPolicy, + IPFamilies: ipFamilies, + }, + } + return svc +} + func NewL4LegacyNetLBServiceWithoutPorts() *api_v1.Service { svc := &api_v1.Service{ ObjectMeta: meta_v1.ObjectMeta{ diff --git a/pkg/utils/ipfamily.go b/pkg/utils/ipfamily.go new file mode 100644 index 0000000000..6cb119d8cb --- /dev/null +++ b/pkg/utils/ipfamily.go @@ -0,0 +1,33 @@ +package utils + +import "k8s.io/api/core/v1" + +func NeedsIPv6(service *v1.Service) bool { + return supportsIPFamily(service, v1.IPv6Protocol) +} + +func NeedsIPv4(service *v1.Service) bool { + if service == nil { + return false + } + // Should never happen, defensive coding if kube-api did not populate IPFamilies + if len(service.Spec.IPFamilies) == 0 { + return true + } + + return supportsIPFamily(service, v1.IPv4Protocol) +} + +func supportsIPFamily(service *v1.Service, ipFamily v1.IPFamily) bool { + if service == nil { + return false + } + + ipFamilies := service.Spec.IPFamilies + for _, ipf := range ipFamilies { + if ipf == ipFamily { + return true + } + } + return false +} diff --git a/pkg/utils/ipfamily_test.go b/pkg/utils/ipfamily_test.go new file mode 100644 index 0000000000..0f999c06b9 --- /dev/null +++ b/pkg/utils/ipfamily_test.go @@ -0,0 +1,104 @@ +package utils + +import ( + "testing" + + "k8s.io/api/core/v1" +) + +func TestNeedsIPv6(t *testing.T) { + testCases := []struct { + service *v1.Service + wantNeedsIPv6 bool + desc string + }{ + { + desc: "Should return false for nil pointer", + service: nil, + wantNeedsIPv6: false, + }, + { + desc: "Should detect ipv6 for dual-stack ip families", + service: &v1.Service{Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + }}, + wantNeedsIPv6: true, + }, + { + desc: "Should not detect ipv6 for only ipv4 families", + service: &v1.Service{Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, + }}, + wantNeedsIPv6: false, + }, + { + desc: "Should detect ipv6 for only ipv6 families", + service: &v1.Service{Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv6Protocol}, + }}, + wantNeedsIPv6: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + needsIPv6 := NeedsIPv6(tc.service) + + if needsIPv6 != tc.wantNeedsIPv6 { + t.Errorf("NeedsIPv6(%v) returned %t, not equal to expected wantNeedsIPv6 = %t", tc.service, needsIPv6, tc.wantNeedsIPv6) + } + }) + } +} + +func TestNeedsIPv4(t *testing.T) { + testCases := []struct { + service *v1.Service + wantNeedsIPv4 bool + desc string + }{ + { + desc: "Should return false for nil pointer", + service: nil, + wantNeedsIPv4: false, + }, + { + desc: "Should handle dual-stack ip families", + service: &v1.Service{Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + }}, + wantNeedsIPv4: true, + }, + { + desc: "Should handle only ipv4 family", + service: &v1.Service{Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, + }}, + wantNeedsIPv4: true, + }, + { + desc: "Should not handle only ipv6 family", + service: &v1.Service{Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv6Protocol}, + }}, + wantNeedsIPv4: false, + }, + { + desc: "Empty families should be recognized as IPv4. Should never happen in real life", + service: &v1.Service{Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{}, + }}, + wantNeedsIPv4: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + needsIPv4 := NeedsIPv4(tc.service) + + if needsIPv4 != tc.wantNeedsIPv4 { + t.Errorf("NeedsIPv4(%v) returned %t, not equal to expected wantNeedsIPv6 = %t", tc.service, needsIPv4, tc.wantNeedsIPv4) + } + }) + } +} diff --git a/pkg/utils/namer/interfaces.go b/pkg/utils/namer/interfaces.go index 51a294ed0d..ab73feddb6 100644 --- a/pkg/utils/namer/interfaces.go +++ b/pkg/utils/namer/interfaces.go @@ -89,10 +89,16 @@ type L4ResourcesNamer interface { L4ForwardingRule(namespace, name, protocol string) string // L4Firewall returns the name of the firewall rule for the given service L4Firewall(namespace, name string) string + // L4IPv6Firewall returns the name of the ipv6 firewall rule for the given service + L4IPv6Firewall(namespace, name string) string // L4HealthCheck returns the names of the Healthcheck L4HealthCheck(namespace, name string, shared bool) string // L4HealthCheckFirewall returns the names of the Healthcheck Firewall L4HealthCheckFirewall(namespace, name string, shared bool) string + // L4IPv6ForwardingRule returns the name of the IPv6 forwarding rule for the given service and protocol. + L4IPv6ForwardingRule(namespace, name, protocol string) string + // L4IPv6HealthCheckFirewall returns the name of the IPv6 L4 LB health check firewall rule. + L4IPv6HealthCheckFirewall(namespace, name string, shared bool) string // IsNEG returns if the given name is a VM_IP_NEG name. IsNEG(name string) bool } diff --git a/pkg/utils/namer/l4_namer.go b/pkg/utils/namer/l4_namer.go index 63d2a4af03..69ea24f7e2 100644 --- a/pkg/utils/namer/l4_namer.go +++ b/pkg/utils/namer/l4_namer.go @@ -17,6 +17,7 @@ const ( maximumL4CombinedLength = 39 sharedHcSuffix = "l4-shared-hc" firewallHcSuffix = "-fw" + ipv6Suffix = "ipv6" sharedFirewallHcSuffix = sharedHcSuffix + firewallHcSuffix maxResourceNameLength = 63 ) @@ -71,6 +72,16 @@ func (namer *L4Namer) L4Firewall(namespace, name string) string { return namer.L4Backend(namespace, name) } +// L4IPv6Firewall returns the gce IPv6 Firewall name based on the service namespace and name +// Naming convention: +// +// k8s2-{uid}-{ns}-{name}-{suffix}-ipv6 +// +// Output name is at most 63 characters. +func (namer *L4Namer) L4IPv6Firewall(namespace, name string) string { + return getSuffixedName(namer.L4Backend(namespace, name), "-"+ipv6Suffix) +} + // L4ForwardingRule returns the name of the L4 forwarding rule name based on the service namespace, name and protocol. // Naming convention: // @@ -100,11 +111,30 @@ func (namer *L4Namer) L4HealthCheck(namespace, name string, shared bool) string // L4HealthCheckFirewall returns the name of the L4 LB Healthcheck Firewall func (namer *L4Namer) L4HealthCheckFirewall(namespace, name string, shared bool) string { - if !shared { - l4Name := namer.L4Backend(namespace, name) - return namer.hcFirewallName(l4Name) + if shared { + return strings.Join([]string{namer.v2Prefix, namer.v2ClusterUID, sharedFirewallHcSuffix}, "-") + } + l4Name := namer.L4Backend(namespace, name) + return namer.hcFirewallName(l4Name, "") +} + +// L4IPv6ForwardingRule returns the name of the L4 forwarding rule name based on the service namespace, name and protocol. +// Naming convention: +// +// k8s2-{protocol}-{uid}-{ns}-{name}-{suffix} +// +// Output name is at most 63 characters. +func (namer *L4Namer) L4IPv6ForwardingRule(namespace, name, protocol string) string { + return getSuffixedName(namer.L4ForwardingRule(namespace, name, protocol), "-"+ipv6Suffix) +} + +// L4IPv6HealthCheckFirewall returns the name of the IPv6 L4 LB health check firewall rule. +func (namer *L4Namer) L4IPv6HealthCheckFirewall(namespace, name string, shared bool) string { + if shared { + return strings.Join([]string{namer.v2Prefix, namer.v2ClusterUID, sharedFirewallHcSuffix, ipv6Suffix}, "-") } - return strings.Join([]string{namer.v2Prefix, namer.v2ClusterUID, sharedFirewallHcSuffix}, "-") + l4Name := namer.L4Backend(namespace, name) + return namer.hcFirewallName(l4Name, "-"+ipv6Suffix) } // IsNEG indicates if the given name is a NEG following the L4 naming convention. @@ -121,13 +151,16 @@ func (n *L4Namer) getClusterSuffix(namespace, name string) string { // hcFirewallName generates the firewall name for the given healthcheck. // It ensures that the name is at most 63 chars long. -func (n *L4Namer) hcFirewallName(hcName string) string { - return getSuffixedName(hcName, firewallHcSuffix) +func (n *L4Namer) hcFirewallName(hcName, suffix string) string { + return getSuffixedName(hcName, firewallHcSuffix+suffix) +} + +func getTrimmedNamespacedName(namespace, name string, maxLength int) string { + return strings.Join(TrimFieldsEvenly(maxLength, namespace, name), "-") } func getSuffixedName(name string, suffix string) string { - trimmedName := ensureSpaceForSuffix(name, suffix) - return trimmedName + suffix + return ensureSpaceForSuffix(name, suffix) + suffix } func ensureSpaceForSuffix(name string, suffix string) string { @@ -137,7 +170,3 @@ func ensureSpaceForSuffix(name string, suffix string) string { } return name } - -func getTrimmedNamespacedName(namespace, name string, maxLength int) string { - return strings.Join(TrimFieldsEvenly(maxLength, namespace, name), "-") -} diff --git a/pkg/utils/namer/l4_namer_test.go b/pkg/utils/namer/l4_namer_test.go index d1bf0e3c95..d8de8d9c1a 100644 --- a/pkg/utils/namer/l4_namer_test.go +++ b/pkg/utils/namer/l4_namer_test.go @@ -10,16 +10,19 @@ func TestL4Namer(t *testing.T) { longstring1 := "012345678901234567890123456789012345678901234567890123456789abc" longstring2 := "012345678901234567890123456789012345678901234567890123456789pqr" testCases := []struct { - desc string - namespace string - name string - proto string - sharedHC bool - expectFRName string - expectNEGName string - expectFWName string - expectHcFwName string - expectHcName string + desc string + namespace string + name string + proto string + sharedHC bool + expectFRName string + expectIPv6FRName string + expectNEGName string + expectFWName string + expectIPv6FWName string + expectHcFwName string + expectIPv6HcFName string + expectHcName string }{ { "simple case", @@ -28,9 +31,12 @@ func TestL4Namer(t *testing.T) { "TCP", false, "k8s2-tcp-7kpbhpki-namespace-name-956p2p7x", + "k8s2-tcp-7kpbhpki-namespace-name-956p2p7x-ipv6", "k8s2-7kpbhpki-namespace-name-956p2p7x", "k8s2-7kpbhpki-namespace-name-956p2p7x", + "k8s2-7kpbhpki-namespace-name-956p2p7x-ipv6", "k8s2-7kpbhpki-namespace-name-956p2p7x-fw", + "k8s2-7kpbhpki-namespace-name-956p2p7x-fw-ipv6", "k8s2-7kpbhpki-namespace-name-956p2p7x", }, { @@ -40,9 +46,12 @@ func TestL4Namer(t *testing.T) { "TCP", true, "k8s2-tcp-7kpbhpki-namespace-name-956p2p7x", + "k8s2-tcp-7kpbhpki-namespace-name-956p2p7x-ipv6", "k8s2-7kpbhpki-namespace-name-956p2p7x", "k8s2-7kpbhpki-namespace-name-956p2p7x", + "k8s2-7kpbhpki-namespace-name-956p2p7x-ipv6", "k8s2-7kpbhpki-l4-shared-hc-fw", + "k8s2-7kpbhpki-l4-shared-hc-fw-ipv6", "k8s2-7kpbhpki-l4-shared-hc", }, { @@ -52,9 +61,12 @@ func TestL4Namer(t *testing.T) { "UDP", false, "k8s2-udp-7kpbhpki-012345678901234567-01234567890123456-hwm400mg", + "k8s2-udp-7kpbhpki-012345678901234567-01234567890123456-hwm-ipv6", "k8s2-7kpbhpki-01234567890123456789-0123456789012345678-hwm400mg", "k8s2-7kpbhpki-01234567890123456789-0123456789012345678-hwm400mg", + "k8s2-7kpbhpki-01234567890123456789-0123456789012345678-hwm-ipv6", "k8s2-7kpbhpki-01234567890123456789-0123456789012345678-hwm40-fw", + "k8s2-7kpbhpki-01234567890123456789-0123456789012345678--fw-ipv6", "k8s2-7kpbhpki-01234567890123456789-0123456789012345678-hwm400mg", }, { @@ -64,9 +76,12 @@ func TestL4Namer(t *testing.T) { "UDP", true, "k8s2-udp-7kpbhpki-012345678901234567-01234567890123456-hwm400mg", + "k8s2-udp-7kpbhpki-012345678901234567-01234567890123456-hwm-ipv6", "k8s2-7kpbhpki-01234567890123456789-0123456789012345678-hwm400mg", "k8s2-7kpbhpki-01234567890123456789-0123456789012345678-hwm400mg", + "k8s2-7kpbhpki-01234567890123456789-0123456789012345678-hwm-ipv6", "k8s2-7kpbhpki-l4-shared-hc-fw", + "k8s2-7kpbhpki-l4-shared-hc-fw-ipv6", "k8s2-7kpbhpki-l4-shared-hc", }, } @@ -74,25 +89,37 @@ func TestL4Namer(t *testing.T) { newNamer := NewL4Namer(kubeSystemUID, nil) for _, tc := range testCases { frName := newNamer.L4ForwardingRule(tc.namespace, tc.name, strings.ToLower(tc.proto)) + ipv6FrName := newNamer.L4IPv6ForwardingRule(tc.namespace, tc.name, strings.ToLower(tc.proto)) negName := newNamer.L4Backend(tc.namespace, tc.name) fwName := newNamer.L4Firewall(tc.namespace, tc.name) + ipv6FWName := newNamer.L4IPv6Firewall(tc.namespace, tc.name) hcName := newNamer.L4HealthCheck(tc.namespace, tc.name, tc.sharedHC) hcFwName := newNamer.L4HealthCheckFirewall(tc.namespace, tc.name, tc.sharedHC) - if len(frName) > maxResourceNameLength || len(negName) > maxResourceNameLength || len(fwName) > maxResourceNameLength || len(hcName) > maxResourceNameLength || len(hcFwName) > maxResourceNameLength { - t.Errorf("%s: got len(frName) == %v, len(negName) == %v, len(fwName) == %v, len(hcName) == %v, len(hcFwName) == %v want <= 63", tc.desc, len(frName), len(negName), len(fwName), len(hcName), len(hcFwName)) + ipv6hcFwName := newNamer.L4IPv6HealthCheckFirewall(tc.namespace, tc.name, tc.sharedHC) + if len(frName) > maxResourceNameLength || len(ipv6FrName) > maxResourceNameLength || len(negName) > maxResourceNameLength || len(fwName) > maxResourceNameLength || len(ipv6FWName) > maxResourceNameLength || len(hcName) > maxResourceNameLength || len(hcFwName) > maxResourceNameLength || len(ipv6hcFwName) > maxResourceNameLength { + t.Errorf("%s: got len(frName) == %v, got len(ipv6FrName) == %v, len(negName) == %v, len(fwName) == %v, len(ipv6FWName) == %v, len(hcName) == %v, len(hcFwName) == %v, len(ipv6hcFwName) == %v want <= 63", tc.desc, len(frName), len(ipv6FrName), len(negName), len(fwName), len(ipv6FWName), len(hcName), len(hcFwName), len(ipv6hcFwName)) } if frName != tc.expectFRName { t.Errorf("%s ForwardingRuleName: got %q, want %q", tc.desc, frName, tc.expectFRName) } + if ipv6FrName != tc.expectIPv6FRName { + t.Errorf("%s IPv6 ForwardingRuleName: got %q, want %q", tc.desc, ipv6FrName, tc.expectIPv6FRName) + } if negName != tc.expectNEGName { t.Errorf("%s VMIPNEGName: got %q, want %q", tc.desc, negName, tc.expectFRName) } if fwName != tc.expectFWName { t.Errorf("%s FirewallName: got %q, want %q", tc.desc, fwName, tc.expectFWName) } + if ipv6FWName != tc.expectIPv6FWName { + t.Errorf("%s IPv6 FirewallName: got %q, want %q", tc.desc, ipv6FWName, tc.expectIPv6FWName) + } if hcFwName != tc.expectHcFwName { t.Errorf("%s FirewallName For Healthcheck: got %q, want %q", tc.desc, hcFwName, tc.expectHcFwName) } + if ipv6hcFwName != tc.expectIPv6HcFName { + t.Errorf("%s IPv6 FirewallName For Healthcheck: got %q, want %q", tc.desc, ipv6hcFwName, tc.expectIPv6HcFName) + } if hcName != tc.expectHcName { t.Errorf("%s HealthCheckName: got %q, want %q", tc.desc, hcName, tc.expectHcName) } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 674882647e..4f780619fd 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -756,6 +756,10 @@ func MakeL4LBServiceDescription(svcName, ip string, version meta.Version, shared return (&L4LBResourceDescription{ServiceName: svcName, ServiceIP: ip, APIVersion: version}).Marshal() } +func MakeL4IPv6ForwardingRuleDescription(service *api_v1.Service) (string, error) { + return (&L4LBResourceDescription{ServiceName: ServiceKeyFunc(service.Namespace, service.Name)}).Marshal() +} + // NewStringPointer returns a pointer to the provided string literal func NewStringPointer(s string) *string { return &s @@ -819,3 +823,16 @@ func GetServiceNodePort(service *api_v1.Service) int64 { } return int64(service.Spec.Ports[0].NodePort) } + +func AddIPToLBStatus(status *api_v1.LoadBalancerStatus, ips ...string) *api_v1.LoadBalancerStatus { + if status == nil { + status = &api_v1.LoadBalancerStatus{ + Ingress: []api_v1.LoadBalancerIngress{}, + } + } + + for _, ip := range ips { + status.Ingress = append(status.Ingress, api_v1.LoadBalancerIngress{IP: ip}) + } + return status +} diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 939f4efb95..dfcdbc9f2e 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -1522,3 +1522,47 @@ func TestGetServicePortRanges(t *testing.T) { }) } } + +func TestAddIPToLBStatus(t *testing.T) { + testCases := []struct { + desc string + status *api_v1.LoadBalancerStatus + ipsToAdd []string + expectedStatus *api_v1.LoadBalancerStatus + }{ + { + desc: "Should create empty status ingress if no IPs provided", + status: nil, + ipsToAdd: []string{}, + expectedStatus: &api_v1.LoadBalancerStatus{Ingress: []api_v1.LoadBalancerIngress{}}, + }, + { + desc: "Should add IPs to the empty status", + status: nil, + ipsToAdd: []string{"1.1.1.1", "0::0"}, + expectedStatus: &api_v1.LoadBalancerStatus{Ingress: []api_v1.LoadBalancerIngress{ + {IP: "1.1.1.1"}, {IP: "0::0"}, + }}, + }, + { + desc: "Should add IP to the existing status", + status: &api_v1.LoadBalancerStatus{Ingress: []api_v1.LoadBalancerIngress{ + {IP: "0::0"}, + }}, + ipsToAdd: []string{"1.1.1.1"}, + expectedStatus: &api_v1.LoadBalancerStatus{Ingress: []api_v1.LoadBalancerIngress{ + {IP: "0::0"}, {IP: "1.1.1.1"}, + }}, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + newStatus := AddIPToLBStatus(tc.status, tc.ipsToAdd...) + + if !reflect.DeepEqual(tc.expectedStatus, newStatus) { + t.Errorf("newStatus = %v, not equal to expectedStatus = %v", newStatus, tc.expectedStatus) + } + }) + } +}