From 28095d889ee3bd926789bf490cf639e78ebe761d Mon Sep 17 00:00:00 2001 From: Cezary Zawadka Date: Thu, 25 Aug 2022 17:19:05 +0200 Subject: [PATCH 1/2] Cleanup code: - get rid of one letter variables named 'L/l/I/|/!/1' - removed unused parameters from assert func in l4 tests no business logic or test logic has been changed in this commit --- pkg/backends/ig_linker.go | 8 +- pkg/backends/neg_linker.go | 12 +- pkg/loadbalancers/addresses.go | 32 +- pkg/loadbalancers/certificates.go | 74 ++-- pkg/loadbalancers/forwarding_rules.go | 82 ++--- pkg/loadbalancers/l4.go | 18 +- pkg/loadbalancers/l4_test.go | 493 +++++++++++++------------- pkg/loadbalancers/l4netlb_test.go | 2 +- pkg/loadbalancers/l7.go | 200 +++++------ pkg/loadbalancers/l7s.go | 64 ++-- pkg/loadbalancers/target_proxies.go | 118 +++--- pkg/loadbalancers/url_maps.go | 60 ++-- pkg/neg/syncers/transaction.go | 4 +- pkg/ratelimit/ratelimit.go | 10 +- 14 files changed, 587 insertions(+), 590 deletions(-) diff --git a/pkg/backends/ig_linker.go b/pkg/backends/ig_linker.go index 325561cd61..f8b91c06d6 100644 --- a/pkg/backends/ig_linker.go +++ b/pkg/backends/ig_linker.go @@ -78,10 +78,10 @@ func NewInstanceGroupLinker(instancePool instances.NodePool, backendPool Pool) L } // Link implements Link. -func (l *instanceGroupLinker) Link(sp utils.ServicePort, groups []GroupKey) error { +func (igl *instanceGroupLinker) Link(sp utils.ServicePort, groups []GroupKey) error { var igLinks []string for _, group := range groups { - ig, err := l.instancePool.Get(sp.IGName(), group.Zone) + ig, err := igl.instancePool.Get(sp.IGName(), group.Zone) if err != nil { return fmt.Errorf("error retrieving IG for linking with backend %+v: %w", sp, err) } @@ -91,7 +91,7 @@ func (l *instanceGroupLinker) Link(sp utils.ServicePort, groups []GroupKey) erro // ig_linker only supports L7 HTTP(s) External Load Balancer // Hardcoded here since IGs are not supported for non GA-Global right now // TODO(shance): find a way to remove hardcoded values - be, err := l.backendPool.Get(sp.BackendName(), meta.VersionGA, meta.Global) + be, err := igl.backendPool.Get(sp.BackendName(), meta.VersionGA, meta.Global) if err != nil { return err } @@ -126,7 +126,7 @@ func (l *instanceGroupLinker) Link(sp utils.ServicePort, groups []GroupKey) erro newBackends := backendsForIGs(addIGs, bm, &sp) be.Backends = append(originalIGBackends, newBackends...) - if err := l.backendPool.Update(be); err != nil { + if err := igl.backendPool.Update(be); err != nil { if utils.IsHTTPErrorCode(err, http.StatusBadRequest) { klog.V(2).Infof("Updating backend service backends with balancing mode %v failed, will try another mode. err:%v", bm, err) errs = append(errs, err.Error()) diff --git a/pkg/backends/neg_linker.go b/pkg/backends/neg_linker.go index 1229b28446..cf35fa5b9c 100644 --- a/pkg/backends/neg_linker.go +++ b/pkg/backends/neg_linker.go @@ -56,7 +56,7 @@ func NewNEGLinker( } // Link implements Link. -func (l *negLinker) Link(sp utils.ServicePort, groups []GroupKey) error { +func (nl *negLinker) Link(sp utils.ServicePort, groups []GroupKey) error { version := befeatures.VersionFromServicePort(&sp) var negSelfLinks []string var err error @@ -70,10 +70,10 @@ func (l *negLinker) Link(sp utils.ServicePort, groups []GroupKey) error { negUrl := "" svcNegKey := fmt.Sprintf("%s/%s", sp.ID.Service.Namespace, negName) - negUrl, ok := getNegUrlFromSvcneg(svcNegKey, group.Zone, l.svcNegLister) + negUrl, ok := getNegUrlFromSvcneg(svcNegKey, group.Zone, nl.svcNegLister) if !ok { klog.V(4).Infof("Falling back to use NEG API to retreive NEG url for NEG %q", negName) - neg, err := l.negGetter.GetNetworkEndpointGroup(negName, group.Zone, version) + neg, err := nl.negGetter.GetNetworkEndpointGroup(negName, group.Zone, version) if err != nil { return err } @@ -85,11 +85,11 @@ func (l *negLinker) Link(sp utils.ServicePort, groups []GroupKey) error { beName := sp.BackendName() scope := befeatures.ScopeFromServicePort(&sp) - key, err := composite.CreateKey(l.cloud, beName, scope) + key, err := composite.CreateKey(nl.cloud, beName, scope) if err != nil { return err } - backendService, err := composite.GetBackendService(l.cloud, key, version) + backendService, err := composite.GetBackendService(nl.cloud, key, version) if err != nil { return err } @@ -111,7 +111,7 @@ func (l *negLinker) Link(sp utils.ServicePort, groups []GroupKey) error { klog.V(2).Infof("Backends changed for service port %s, removing: %s, adding: %s, changed: %s", sp.ID, diff.toRemove(), diff.toAdd(), diff.changed) backendService.Backends = mergedBackend - return composite.UpdateBackendService(l.cloud, key, backendService) + return composite.UpdateBackendService(nl.cloud, key, backendService) } type backendDiff struct { diff --git a/pkg/loadbalancers/addresses.go b/pkg/loadbalancers/addresses.go index b38a3dd914..958410fc8d 100644 --- a/pkg/loadbalancers/addresses.go +++ b/pkg/loadbalancers/addresses.go @@ -30,13 +30,13 @@ import ( ) // checkStaticIP reserves a regional or global static IP allocated to the Forwarding Rule. -func (l *L7) checkStaticIP() (err error) { - if l.fw == nil || l.fw.IPAddress == "" { +func (l7 *L7) checkStaticIP() (err error) { + if l7.fw == nil || l7.fw.IPAddress == "" { return fmt.Errorf("will not create static IP without a forwarding rule") } - managedStaticIPName := l.namer.ForwardingRule(namer.HTTPProtocol) + managedStaticIPName := l7.namer.ForwardingRule(namer.HTTPProtocol) // Don't manage staticIPs if the user has specified an IP. - address, manageStaticIP, err := l.getEffectiveIP() + address, manageStaticIP, err := l7.getEffectiveIP() if err != nil { return err } @@ -44,45 +44,45 @@ func (l *L7) checkStaticIP() (err error) { klog.V(3).Infof("Not managing user specified static IP %v", address) if flags.F.EnableDeleteUnusedFrontends { // Delete ingress controller managed static ip if exists. - if ip, ok := l.ingress.Annotations[annotations.StaticIPKey]; ok && ip == managedStaticIPName { - return l.deleteStaticIP() + if ip, ok := l7.ingress.Annotations[annotations.StaticIPKey]; ok && ip == managedStaticIPName { + return l7.deleteStaticIP() } } return nil } - key, err := l.CreateKey(managedStaticIPName) + key, err := l7.CreateKey(managedStaticIPName) if err != nil { return err } - ip, _ := composite.GetAddress(l.cloud, key, meta.VersionGA) + ip, _ := composite.GetAddress(l7.cloud, key, meta.VersionGA) if ip == nil { klog.V(3).Infof("Creating static ip %v", managedStaticIPName) - address := l.newStaticAddress(managedStaticIPName) + address := l7.newStaticAddress(managedStaticIPName) - err = composite.CreateAddress(l.cloud, key, address) + err = composite.CreateAddress(l7.cloud, key, address) if err != nil { if utils.IsHTTPErrorCode(err, http.StatusConflict) || utils.IsHTTPErrorCode(err, http.StatusBadRequest) { klog.V(3).Infof("IP %v(%v) is already reserved, assuming it is OK to use.", - l.fw.IPAddress, managedStaticIPName) + l7.fw.IPAddress, managedStaticIPName) return nil } return err } - ip, err = composite.GetAddress(l.cloud, key, meta.VersionGA) + ip, err = composite.GetAddress(l7.cloud, key, meta.VersionGA) if err != nil { return err } } - l.ip = ip + l7.ip = ip return nil } -func (l *L7) newStaticAddress(name string) *composite.Address { - isInternal := utils.IsGCEL7ILBIngress(&l.ingress) - address := &composite.Address{Name: name, Address: l.fw.IPAddress, Version: meta.VersionGA} +func (l7 *L7) newStaticAddress(name string) *composite.Address { + isInternal := utils.IsGCEL7ILBIngress(&l7.ingress) + address := &composite.Address{Name: name, Address: l7.fw.IPAddress, Version: meta.VersionGA} if isInternal { // Used for L7 ILB address.AddressType = "INTERNAL" diff --git a/pkg/loadbalancers/certificates.go b/pkg/loadbalancers/certificates.go index 44203d393e..f5fa7087ea 100644 --- a/pkg/loadbalancers/certificates.go +++ b/pkg/loadbalancers/certificates.go @@ -28,30 +28,30 @@ import ( const SslCertificateMissing = "SslCertificateMissing" -func (l *L7) checkSSLCert() error { - isL7ILB := utils.IsGCEL7ILBIngress(l.runtimeInfo.Ingress) - tr := translator.NewTranslator(isL7ILB, l.namer) - env := &translator.Env{Region: l.cloud.Region(), Project: l.cloud.ProjectID()} - translatorCerts := tr.ToCompositeSSLCertificates(env, l.runtimeInfo.TLSName, l.runtimeInfo.TLS, l.Versions().SslCertificate) +func (l7 *L7) checkSSLCert() error { + isL7ILB := utils.IsGCEL7ILBIngress(l7.runtimeInfo.Ingress) + tr := translator.NewTranslator(isL7ILB, l7.namer) + env := &translator.Env{Region: l7.cloud.Region(), Project: l7.cloud.ProjectID()} + translatorCerts := tr.ToCompositeSSLCertificates(env, l7.runtimeInfo.TLSName, l7.runtimeInfo.TLS, l7.Versions().SslCertificate) // Use both pre-shared and secret-based certs if available, // combining encountered errors. errs := []error{} // Get updated value of certificate for comparison - existingSecretsSslCerts, err := l.getIngressManagedSslCerts() + existingSecretsSslCerts, err := l7.getIngressManagedSslCerts() if err != nil { errs = append(errs, err) // Do not continue if getIngressManagedSslCerts() failed. return utils.JoinErrs(errs) } - l.oldSSLCerts = existingSecretsSslCerts - sslCerts, err := l.createSslCertificates(existingSecretsSslCerts, translatorCerts) + l7.oldSSLCerts = existingSecretsSslCerts + sslCerts, err := l7.createSslCertificates(existingSecretsSslCerts, translatorCerts) if err != nil { errs = append(errs, err) } - l.sslCerts = sslCerts + l7.sslCerts = sslCerts if len(errs) > 0 { return utils.JoinErrs(errs) } @@ -59,7 +59,7 @@ func (l *L7) checkSSLCert() error { } // createSslCertificates creates SslCertificates based on kubernetes secrets in Ingress configuration. -func (l *L7) createSslCertificates(existingCerts, translatorCerts []*composite.SslCertificate) ([]*composite.SslCertificate, error) { +func (l7 *L7) createSslCertificates(existingCerts, translatorCerts []*composite.SslCertificate) ([]*composite.SslCertificate, error) { var result []*composite.SslCertificate existingCertsMap := getMapfromCertList(existingCerts) @@ -95,23 +95,23 @@ func (l *L7) createSslCertificates(existingCerts, translatorCerts []*composite.S } // Controller needs to create the certificate, no need to check if it exists and delete. If it did exist, it // would have been listed in the populateSSLCert function and matched in the check above. - klog.V(2).Infof("Creating new sslCertificate %q for LB %q", translatorCert.Name, l) - translatorCert.Version = l.Versions().SslCertificate - key, err := l.CreateKey(translatorCert.Name) + klog.V(2).Infof("Creating new sslCertificate %q for LB %q", translatorCert.Name, l7) + translatorCert.Version = l7.Versions().SslCertificate + key, err := l7.CreateKey(translatorCert.Name) if err != nil { - klog.Errorf("l.CreateKey(%s) = %v", translatorCert.Name, err) + klog.Errorf("l7.CreateKey(%s) = %v", translatorCert.Name, err) return nil, err } - err = composite.CreateSslCertificate(l.cloud, key, translatorCert) + err = composite.CreateSslCertificate(l7.cloud, key, translatorCert) if err != nil { - klog.Errorf("Failed to create new sslCertificate %q for %q - %v", translatorCert.Name, l, err) + klog.Errorf("Failed to create new sslCertificate %q for %q - %v", translatorCert.Name, l7, err) failedCerts = append(failedCerts, translatorCert.Name+" Error:"+err.Error()) continue } visitedCertMap[translatorCert.Name] = fmt.Sprintf("secret cert:%q", translatorCert.Certificate) // Get SSLCert - cert, err := composite.GetSslCertificate(l.cloud, key, translatorCert.Version) + cert, err := composite.GetSslCertificate(l7.cloud, key, translatorCert.Version) if err != nil { klog.Errorf("GetSslCertificate(_, %v, %v) = %v", key, translatorCert.Version, err) return nil, err @@ -140,7 +140,7 @@ func getMapfromCertList(certs []*composite.SslCertificate) map[string]*composite // getIngressManagedSslCerts fetches SslCertificate resources created and managed by this load balancer // instance. These SslCertificate resources were created based on kubernetes secrets in Ingress // configuration. -func (l *L7) getIngressManagedSslCerts() ([]*composite.SslCertificate, error) { +func (l7 *L7) getIngressManagedSslCerts() ([]*composite.SslCertificate, error) { var result []*composite.SslCertificate // Currently we list all certs available in gcloud and filter the ones managed by this loadbalancer instance. This is @@ -148,25 +148,25 @@ func (l *L7) getIngressManagedSslCerts() ([]*composite.SslCertificate, error) { // Can be a performance issue if there are too many global certs, default quota is only 10. // Use an empty name parameter since we only care about the scope // TODO: (shance) refactor this so we don't need an empty arg - key, err := l.CreateKey("") + key, err := l7.CreateKey("") if err != nil { return nil, err } - version := l.Versions().SslCertificate - certs, err := composite.ListSslCertificates(l.cloud, key, version) + version := l7.Versions().SslCertificate + certs, err := composite.ListSslCertificates(l7.cloud, key, version) if err != nil { return nil, err } for _, c := range certs { - if l.namer.IsCertNameForLB(c.Name) { - klog.V(4).Infof("Populating ssl cert %s for l7 %s", c.Name, l) + if l7.namer.IsCertNameForLB(c.Name) { + klog.V(4).Infof("Populating ssl cert %s for l7 %s", c.Name, l7) result = append(result, c) } } if len(result) == 0 { // Check for legacy cert since that follows a different naming convention klog.V(4).Infof("Looking for legacy ssl certs") - expectedCertLinks, err := l.getSslCertLinkInUse() + expectedCertLinks, err := l7.getSslCertLinkInUse() if err != nil { // Return nil if target proxy doesn't exist. return nil, utils.IgnoreHTTPNotFound(err) @@ -179,16 +179,16 @@ func (l *L7) getIngressManagedSslCerts() ([]*composite.SslCertificate, error) { continue } - if !l.namer.IsLegacySSLCert(name) { + if !l7.namer.IsLegacySSLCert(name) { continue } - key, err := l.CreateKey(name) + key, err := l7.CreateKey(name) if err != nil { return nil, err } - cert, _ := composite.GetSslCertificate(l.cloud, key, version) + cert, _ := composite.GetSslCertificate(l7.cloud, key, version) if cert != nil { - klog.V(4).Infof("Populating legacy ssl cert %s for l7 %s", cert.Name, l) + klog.V(4).Infof("Populating legacy ssl cert %s for l7 %s", cert.Name, l7) result = append(result, cert) } } @@ -196,13 +196,13 @@ func (l *L7) getIngressManagedSslCerts() ([]*composite.SslCertificate, error) { return result, nil } -func (l *L7) deleteOldSSLCerts() { - if len(l.oldSSLCerts) == 0 { +func (l7 *L7) deleteOldSSLCerts() { + if len(l7.oldSSLCerts) == 0 { return } - certsMap := getMapfromCertList(l.sslCerts) - for _, cert := range l.oldSSLCerts { - if !l.namer.IsCertNameForLB(cert.Name) && !l.namer.IsLegacySSLCert(cert.Name) { + certsMap := getMapfromCertList(l7.sslCerts) + for _, cert := range l7.oldSSLCerts { + if !l7.namer.IsCertNameForLB(cert.Name) && !l7.namer.IsLegacySSLCert(cert.Name) { // retain cert if it is managed by GCE(non-ingress) continue } @@ -211,8 +211,8 @@ func (l *L7) deleteOldSSLCerts() { continue } klog.V(3).Infof("Cleaning up old SSL Certificate %s", cert.Name) - key, _ := l.CreateKey(cert.Name) - if certErr := utils.IgnoreHTTPNotFound(composite.DeleteSslCertificate(l.cloud, key, l.Versions().SslCertificate)); certErr != nil { + key, _ := l7.CreateKey(cert.Name) + if certErr := utils.IgnoreHTTPNotFound(composite.DeleteSslCertificate(l7.cloud, key, l7.Versions().SslCertificate)); certErr != nil { klog.Errorf("Old cert %s delete failed - %v", cert.Name, certErr) } } @@ -220,8 +220,8 @@ func (l *L7) deleteOldSSLCerts() { // Returns true if the input array of certs is identical to the certs in the L7 config. // Returns false if there is any mismatch -func (l *L7) compareCerts(certLinks []string) bool { - certsMap := getMapfromCertList(l.sslCerts) +func (l7 *L7) compareCerts(certLinks []string) bool { + certsMap := getMapfromCertList(l7.sslCerts) if len(certLinks) != len(certsMap) { klog.V(4).Infof("Loadbalancer has %d certs, target proxy has %d certs", len(certsMap), len(certLinks)) return false diff --git a/pkg/loadbalancers/forwarding_rules.go b/pkg/loadbalancers/forwarding_rules.go index af72812e5f..e4f61d80f4 100644 --- a/pkg/loadbalancers/forwarding_rules.go +++ b/pkg/loadbalancers/forwarding_rules.go @@ -38,66 +38,66 @@ import ( // maxL4ILBPorts is the maximum number of ports that can be specified in an L4 ILB Forwarding Rule const maxL4ILBPorts = 5 -func (l *L7) checkHttpForwardingRule() (err error) { - if l.tp == nil { +func (l7 *L7) checkHttpForwardingRule() (err error) { + if l7.tp == nil { return fmt.Errorf("cannot create forwarding rule without proxy") } - name := l.namer.ForwardingRule(namer.HTTPProtocol) - address, _, err := l.getEffectiveIP() + name := l7.namer.ForwardingRule(namer.HTTPProtocol) + address, _, err := l7.getEffectiveIP() if err != nil { return err } - fw, err := l.checkForwardingRule(namer.HTTPProtocol, name, l.tp.SelfLink, address) + fw, err := l7.checkForwardingRule(namer.HTTPProtocol, name, l7.tp.SelfLink, address) if err != nil { return err } - l.fw = fw + l7.fw = fw return nil } -func (l *L7) checkHttpsForwardingRule() (err error) { - if l.tps == nil { - klog.V(3).Infof("No https target proxy for %v, not created https forwarding rule", l) +func (l7 *L7) checkHttpsForwardingRule() (err error) { + if l7.tps == nil { + klog.V(3).Infof("No https target proxy for %v, not created https forwarding rule", l7) return nil } - name := l.namer.ForwardingRule(namer.HTTPSProtocol) - address, _, err := l.getEffectiveIP() + name := l7.namer.ForwardingRule(namer.HTTPSProtocol) + address, _, err := l7.getEffectiveIP() if err != nil { return err } - fws, err := l.checkForwardingRule(namer.HTTPSProtocol, name, l.tps.SelfLink, address) + fws, err := l7.checkForwardingRule(namer.HTTPSProtocol, name, l7.tps.SelfLink, address) if err != nil { return err } - l.fws = fws + l7.fws = fws return nil } -func (l *L7) checkForwardingRule(protocol namer.NamerProtocol, name, proxyLink, ip string) (existing *composite.ForwardingRule, err error) { - key, err := l.CreateKey(name) +func (l7 *L7) checkForwardingRule(protocol namer.NamerProtocol, name, proxyLink, ip string) (existing *composite.ForwardingRule, err error) { + key, err := l7.CreateKey(name) if err != nil { return nil, err } - version := l.Versions().ForwardingRule - description, err := l.description() + version := l7.Versions().ForwardingRule + description, err := l7.description() if err != nil { return nil, err } - isL7ILB := utils.IsGCEL7ILBIngress(l.runtimeInfo.Ingress) - tr := translator.NewTranslator(isL7ILB, l.namer) - env := &translator.Env{VIP: ip, Network: l.cloud.NetworkURL(), Subnetwork: l.cloud.SubnetworkURL()} - fr := tr.ToCompositeForwardingRule(env, protocol, version, proxyLink, description, l.runtimeInfo.StaticIPSubnet) + isL7ILB := utils.IsGCEL7ILBIngress(l7.runtimeInfo.Ingress) + tr := translator.NewTranslator(isL7ILB, l7.namer) + env := &translator.Env{VIP: ip, Network: l7.cloud.NetworkURL(), Subnetwork: l7.cloud.SubnetworkURL()} + fr := tr.ToCompositeForwardingRule(env, protocol, version, proxyLink, description, l7.runtimeInfo.StaticIPSubnet) - existing, _ = composite.GetForwardingRule(l.cloud, key, version) + existing, _ = composite.GetForwardingRule(l7.cloud, key, version) if existing != nil && (fr.IPAddress != "" && existing.IPAddress != fr.IPAddress || existing.PortRange != fr.PortRange) { klog.Warningf("Recreating forwarding rule %v(%v), so it has %v(%v)", existing.IPAddress, existing.PortRange, fr.IPAddress, fr.PortRange) - if err = utils.IgnoreHTTPNotFound(composite.DeleteForwardingRule(l.cloud, key, version)); err != nil { + if err = utils.IgnoreHTTPNotFound(composite.DeleteForwardingRule(l7.cloud, key, version)); err != nil { return nil, err } existing = nil - l.recorder.Eventf(l.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "ForwardingRule %q deleted", key.Name) + l7.recorder.Eventf(l7.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "ForwardingRule %q deleted", key.Name) } if existing == nil { // This is a special case where exactly one of http or https forwarding rule @@ -105,11 +105,11 @@ func (l *L7) checkForwardingRule(protocol namer.NamerProtocol, name, proxyLink, // In this case, the forwarding rule needs to be created with the same static ip. // Note that this is not needed when user specifies a static IP. if ip == "" { - managedStaticIPName := l.namer.ForwardingRule(namer.HTTPProtocol) + managedStaticIPName := l7.namer.ForwardingRule(namer.HTTPProtocol) // Get static IP address if ingress has static IP annotation. // Note that this Static IP annotation is applied by ingress controller. - if currentIPName, exists := l.ingress.Annotations[annotations.StaticIPKey]; exists && currentIPName == managedStaticIPName { - currentIP, _ := l.cloud.GetGlobalAddress(managedStaticIPName) + if currentIPName, exists := l7.ingress.Annotations[annotations.StaticIPKey]; exists && currentIPName == managedStaticIPName { + currentIP, _ := l7.cloud.GetGlobalAddress(managedStaticIPName) if currentIP != nil { klog.V(3).Infof("Ingress managed static IP %s(%s) exists, using it to create forwarding rule %s", currentIPName, currentIP.Address, name) fr.IPAddress = currentIP.Address @@ -118,16 +118,16 @@ func (l *L7) checkForwardingRule(protocol namer.NamerProtocol, name, proxyLink, } klog.V(3).Infof("Creating forwarding rule for proxy %q and ip %v:%v", proxyLink, ip, protocol) - if err = composite.CreateForwardingRule(l.cloud, key, fr); err != nil { + if err = composite.CreateForwardingRule(l7.cloud, key, fr); err != nil { return nil, err } - l.recorder.Eventf(l.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "ForwardingRule %q created", key.Name) + l7.recorder.Eventf(l7.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "ForwardingRule %q created", key.Name) - key, err = l.CreateKey(name) + key, err = l7.CreateKey(name) if err != nil { return nil, err } - existing, err = composite.GetForwardingRule(l.cloud, key, version) + existing, err = composite.GetForwardingRule(l7.cloud, key, version) if err != nil { return nil, err } @@ -138,11 +138,11 @@ func (l *L7) checkForwardingRule(protocol namer.NamerProtocol, name, proxyLink, } else { klog.V(3).Infof("Forwarding rule %v has the wrong proxy, setting %v overwriting %v", existing.Name, existing.Target, proxyLink) - key, err := l.CreateKey(existing.Name) + key, err := l7.CreateKey(existing.Name) if err != nil { return nil, err } - if err := composite.SetProxyForForwardingRule(l.cloud, key, existing, proxyLink); err != nil { + if err := composite.SetProxyForForwardingRule(l7.cloud, key, existing, proxyLink); err != nil { return nil, err } } @@ -152,7 +152,7 @@ func (l *L7) checkForwardingRule(protocol namer.NamerProtocol, name, proxyLink, // getEffectiveIP returns a string with the IP to use in the HTTP and HTTPS // forwarding rules, a boolean indicating if this is an IP the controller // should manage or not and an error if the specified IP was not found. -func (l *L7) getEffectiveIP() (string, bool, error) { +func (l7 *L7) getEffectiveIP() (string, bool, error) { // A note on IP management: // User specifies a different IP on startup: @@ -170,8 +170,8 @@ func (l *L7) getEffectiveIP() (string, bool, error) { // or deletes/modifies the Ingress. // TODO: Handle the last case better. - if l.runtimeInfo.StaticIPName != "" { - key, err := l.CreateKey(l.runtimeInfo.StaticIPName) + if l7.runtimeInfo.StaticIPName != "" { + key, err := l7.CreateKey(l7.runtimeInfo.StaticIPName) if err != nil { return "", false, err } @@ -179,16 +179,16 @@ func (l *L7) getEffectiveIP() (string, bool, error) { // Existing static IPs allocated to forwarding rules will get orphaned // till the Ingress is torn down. // TODO(shance): Replace version - if ip, err := composite.GetAddress(l.cloud, key, meta.VersionGA); err != nil || ip == nil { + if ip, err := composite.GetAddress(l7.cloud, key, meta.VersionGA); err != nil || ip == nil { return "", false, fmt.Errorf("the given static IP name %v doesn't translate to an existing static IP.", - l.runtimeInfo.StaticIPName) + l7.runtimeInfo.StaticIPName) } else { - l.runtimeInfo.StaticIPSubnet = ip.Subnetwork + l7.runtimeInfo.StaticIPSubnet = ip.Subnetwork return ip.Address, false, nil } } - if l.ip != nil { - return l.ip.Address, true, nil + if l7.ip != nil { + return l7.ip.Address, true, nil } return "", true, nil } diff --git a/pkg/loadbalancers/l4.go b/pkg/loadbalancers/l4.go index fb07b1269a..00d1299104 100644 --- a/pkg/loadbalancers/l4.go +++ b/pkg/loadbalancers/l4.go @@ -51,7 +51,7 @@ type L4 struct { Service *corev1.Service ServicePort utils.ServicePort NamespacedName types.NamespacedName - l4HealthChecks healthchecks.L4HealthChecks + healthChecks healthchecks.L4HealthChecks forwardingRules ForwardingRulesProvider } @@ -77,20 +77,20 @@ type L4ILBParams struct { // NewL4Handler creates a new L4Handler for the given L4 service. func NewL4Handler(params *L4ILBParams) *L4 { var scope meta.KeyType = meta.Regional - l := &L4{ + l4 := &L4{ cloud: params.Cloud, scope: scope, namer: params.Namer, recorder: params.Recorder, Service: params.Service, - l4HealthChecks: healthchecks.L4(), + healthChecks: healthchecks.L4(), forwardingRules: forwardingrules.New(params.Cloud, meta.VersionGA, scope), } - l.NamespacedName = types.NamespacedName{Name: params.Service.Name, Namespace: params.Service.Namespace} - l.backendPool = backends.NewPool(l.cloud, l.namer) - l.ServicePort = utils.ServicePort{ID: utils.ServicePortID{Service: l.NamespacedName}, BackendNamer: l.namer, + l4.NamespacedName = types.NamespacedName{Name: params.Service.Name, Namespace: params.Service.Namespace} + l4.backendPool = backends.NewPool(l4.cloud, l4.namer) + l4.ServicePort = utils.ServicePort{ID: utils.ServicePortID{Service: l4.NamespacedName}, BackendNamer: l4.namer, VMIPNEGEnabled: true} - return l + return l4 } // CreateKey generates a meta.Key for a given GCE resource name. @@ -151,7 +151,7 @@ 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.l4HealthChecks.DeleteHealthCheck(svc, l4.namer, isShared, meta.Global, utils.ILB) + resourceInError, err := l4.healthChecks.DeleteHealthCheck(svc, l4.namer, isShared, meta.Global, utils.ILB) if err != nil { result.GCEResourceInError = resourceInError result.Error = err @@ -206,7 +206,7 @@ func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service // create healthcheck sharedHC := !helpers.RequestsOnlyLocalTraffic(l4.Service) - hcResult := l4.l4HealthChecks.EnsureL4HealthCheck(l4.Service, l4.namer, sharedHC, meta.Global, utils.ILB, nodeNames) + hcResult := l4.healthChecks.EnsureL4HealthCheck(l4.Service, l4.namer, sharedHC, meta.Global, utils.ILB, nodeNames) if hcResult.Err != nil { result.GCEResourceInError = hcResult.GceResourceInError diff --git a/pkg/loadbalancers/l4_test.go b/pkg/loadbalancers/l4_test.go index 3d087ee591..2526ce9f7e 100644 --- a/pkg/loadbalancers/l4_test.go +++ b/pkg/loadbalancers/l4_test.go @@ -74,22 +74,22 @@ func TestEnsureInternalBackendServiceUpdates(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - bsName := l.namer.L4Backend(l.Service.Namespace, l.Service.Name) - _, err := l.backendPool.EnsureL4BackendService(bsName, "", "TCP", string(svc.Spec.SessionAffinity), string(cloud.SchemeInternal), l.NamespacedName, meta.VersionGA) + bsName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) + _, err := l4.backendPool.EnsureL4BackendService(bsName, "", "TCP", string(svc.Spec.SessionAffinity), string(cloud.SchemeInternal), l4.NamespacedName, meta.VersionGA) if err != nil { t.Errorf("Failed to ensure backend service %s - err %v", bsName, err) } // Update the Internal Backend Service with a new ServiceAffinity - _, err = l.backendPool.EnsureL4BackendService(bsName, "", "TCP", string(v1.ServiceAffinityNone), string(cloud.SchemeInternal), l.NamespacedName, meta.VersionGA) + _, err = l4.backendPool.EnsureL4BackendService(bsName, "", "TCP", string(v1.ServiceAffinityNone), string(cloud.SchemeInternal), l4.NamespacedName, meta.VersionGA) if err != nil { t.Errorf("Failed to ensure backend service %s - err %v", bsName, err) } - key := meta.RegionalKey(bsName, l.cloud.Region()) - bs, err := composite.GetBackendService(l.cloud, key, meta.VersionGA) + key := meta.RegionalKey(bsName, l4.cloud.Region()) + bs, err := composite.GetBackendService(l4.cloud, key, meta.VersionGA) if err != nil { t.Errorf("Failed to get backend service %s - err %v", bsName, err) } @@ -101,11 +101,11 @@ func TestEnsureInternalBackendServiceUpdates(t *testing.T) { newTimeout := int64(backends.DefaultConnectionDrainingTimeoutSeconds * 2) bs.ConnectionDraining.DrainingTimeoutSec = newTimeout bs.SessionAffinity = strings.ToUpper(string(v1.ServiceAffinityClientIP)) - err = composite.UpdateBackendService(l.cloud, key, bs) + err = composite.UpdateBackendService(l4.cloud, key, bs) if err != nil { t.Errorf("Failed to update backend service with new connection draining timeout - err %v", err) } - bs, err = l.backendPool.EnsureL4BackendService(bsName, "", "TCP", string(v1.ServiceAffinityNone), string(cloud.SchemeInternal), l.NamespacedName, meta.VersionGA) + bs, err = l4.backendPool.EnsureL4BackendService(bsName, "", "TCP", string(v1.ServiceAffinityNone), string(cloud.SchemeInternal), l4.NamespacedName, meta.VersionGA) if err != nil { t.Errorf("Failed to ensure backend service %s - err %v", bsName, err) } @@ -131,25 +131,25 @@ func TestEnsureInternalLoadBalancer(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil { + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) - backendServiceName := l.namer.L4Backend(l.Service.Namespace, l.Service.Name) - key := meta.RegionalKey(backendServiceName, l.cloud.Region()) - bs, err := composite.GetBackendService(l.cloud, key, meta.VersionGA) + backendServiceName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) + key := meta.RegionalKey(backendServiceName, l4.cloud.Region()) + bs, err := composite.GetBackendService(l4.cloud, key, meta.VersionGA) if err != nil { t.Errorf("Failed to lookup backend service, err %v", err) } @@ -159,19 +159,19 @@ func TestEnsureInternalLoadBalancer(t *testing.T) { } // Add a backend list to simulate NEG linker populating the backends. bs.Backends = []*composite.Backend{{Group: "test"}} - if err := composite.UpdateBackendService(l.cloud, key, bs); err != nil { + if err := composite.UpdateBackendService(l4.cloud, key, bs); err != nil { t.Errorf("Failed updating backend service, err %v", err) } // Simulate a periodic sync. The backends list should not be reconciled. - result = l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) - bs, err = composite.GetBackendService(l.cloud, meta.RegionalKey(backendServiceName, l.cloud.Region()), meta.VersionGA) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) + bs, err = composite.GetBackendService(l4.cloud, meta.RegionalKey(backendServiceName, l4.cloud.Region()), meta.VersionGA) if err != nil { t.Errorf("Failed to lookup backend service, err %v", err) } @@ -194,28 +194,28 @@ func TestEnsureInternalLoadBalancerTypeChange(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil { + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + result := l4.EnsureInternalLoadBalancer(nodeNames, svc) if result.Error != nil { t.Errorf("Unexpected error %v", result.Error) } if len(result.Status.Ingress) == 0 { - t.Errorf("Got empty loadBalancer status using handler %v", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) // Now add the latest annotation and change scheme to external svc.Annotations[gce.ServiceAnnotationLoadBalancerType] = "" // This will be invoked by service_controller - if result = l.EnsureInternalLoadBalancerDeleted(svc); result.Error != nil { + if result = l4.EnsureInternalLoadBalancerDeleted(svc); result.Error != nil { t.Errorf("Failed to ensure loadBalancer, err %v", result.Error) } - assertInternalLbResourcesDeleted(t, svc, true, l) + assertInternalLbResourcesDeleted(t, l4) } func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { @@ -234,35 +234,35 @@ func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil { + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - lbName := l.namer.L4Backend(svc.Namespace, svc.Name) + lbName := l4.namer.L4Backend(svc.Namespace, svc.Name) // Create the expected resources necessary for an Internal Load Balancer sharedHC := !servicehelper.RequestsOnlyLocalTraffic(svc) - hcResult := l.l4HealthChecks.EnsureL4HealthCheck(l.Service, l.namer, sharedHC, meta.Global, utils.ILB, []string{}) + hcResult := l4.healthChecks.EnsureL4HealthCheck(l4.Service, l4.namer, sharedHC, meta.Global, utils.ILB, []string{}) if hcResult.Err != nil { t.Errorf("Failed to create healthcheck, err %v", hcResult.Err) } - _, err := l.backendPool.EnsureL4BackendService(lbName, hcResult.HCLink, "TCP", string(l.Service.Spec.SessionAffinity), - string(cloud.SchemeInternal), l.NamespacedName, meta.VersionGA) + _, err := l4.backendPool.EnsureL4BackendService(lbName, hcResult.HCLink, "TCP", string(l4.Service.Spec.SessionAffinity), + string(cloud.SchemeInternal), l4.NamespacedName, meta.VersionGA) if err != nil { t.Errorf("Failed to create backendservice, err %v", err) } - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) } // TestEnsureInternalLoadBalancerClearPreviousResources creates ILB resources with incomplete configuration and verifies @@ -283,17 +283,17 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName) + _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName) if err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - lbName := l.namer.L4Backend(svc.Namespace, svc.Name) - frName := l.GetFRName() - key, err := composite.CreateKey(l.cloud, frName, meta.Regional) + lbName := l4.namer.L4Backend(svc.Namespace, svc.Name) + frName := l4.GetFRName() + key, err := composite.CreateKey(l4.cloud, frName, meta.Regional) if err != nil { t.Errorf("Unexpected error when creating key - %v", err) } @@ -306,7 +306,7 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { IPProtocol: "TCP", LoadBalancingScheme: string(cloud.SchemeInternal), } - if err = composite.CreateForwardingRule(l.cloud, key, existingFwdRule); err != nil { + if err = composite.CreateForwardingRule(l4.cloud, key, existingFwdRule); err != nil { t.Errorf("Failed to create fake forwarding rule %s, err %v", lbName, err) } key.Name = lbName @@ -324,7 +324,7 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { fakeGCE.CreateFirewall(existingFirewall) sharedHealthCheck := !servicehelper.RequestsOnlyLocalTraffic(svc) - hcName := l.namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHealthCheck) + hcName := l4.namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHealthCheck) // Create a healthcheck with an incomplete fields existingHC := &composite.HealthCheck{Name: hcName} @@ -352,7 +352,7 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { if err = composite.CreateForwardingRule(fakeGCE, key, existingFwdRule); err != nil { t.Errorf("Failed to update forwarding rule with new BS link, err %v", err) } - if result := l.EnsureInternalLoadBalancer(nodeNames, svc); result.Error != nil { + if result := l4.EnsureInternalLoadBalancer(nodeNames, svc); result.Error != nil { t.Errorf("Failed to ensure loadBalancer %s, err %v", lbName, result.Error) } key.Name = frName @@ -409,16 +409,16 @@ func TestUpdateResourceLinks(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName) + _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName) if err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - lbName := l.namer.L4Backend(svc.Namespace, svc.Name) - key, err := composite.CreateKey(l.cloud, lbName, meta.Regional) + lbName := l4.namer.L4Backend(svc.Namespace, svc.Name) + key, err := composite.CreateKey(l4.cloud, lbName, meta.Regional) if err != nil { t.Errorf("Unexpected error when creating key - %v", err) } @@ -453,12 +453,12 @@ func TestUpdateResourceLinks(t *testing.T) { if !reflect.DeepEqual(bs.HealthChecks, []string{"hc1", "hc2"}) { t.Errorf("Unexpected healthchecks in backend service - %v", bs.HealthChecks) } - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + result := l4.EnsureInternalLoadBalancer(nodeNames, svc) if result.Error != nil { t.Errorf("Failed to ensure loadBalancer %s, err %v", lbName, result.Error) } // verifies that the right healthcheck is present - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) // ensure that the other healthchecks still exist. key.Name = "hc1" @@ -493,20 +493,20 @@ func TestEnsureInternalLoadBalancerHealthCheckConfigurable(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName) + _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName) if err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - lbName := l.namer.L4Backend(svc.Namespace, svc.Name) - key, err := composite.CreateKey(l.cloud, lbName, meta.Regional) + lbName := l4.namer.L4Backend(svc.Namespace, svc.Name) + key, err := composite.CreateKey(l4.cloud, lbName, meta.Regional) if err != nil { t.Errorf("Unexpected error when creating key - %v", err) } sharedHealthCheck := !servicehelper.RequestsOnlyLocalTraffic(svc) - hcName := l.namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHealthCheck) + hcName := l4.namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHealthCheck) // Create a healthcheck with an incorrect threshold, default value is 8s. existingHC := &composite.HealthCheck{Name: hcName, CheckIntervalSec: 6000} @@ -514,7 +514,7 @@ func TestEnsureInternalLoadBalancerHealthCheckConfigurable(t *testing.T) { t.Errorf("Failed to create fake healthcheck %s, err %v", hcName, err) } - if result := l.EnsureInternalLoadBalancer(nodeNames, svc); result.Error != nil { + if result := l4.EnsureInternalLoadBalancer(nodeNames, svc); result.Error != nil { t.Errorf("Failed to ensure loadBalancer %s, err %v", lbName, result.Error) } @@ -542,27 +542,27 @@ func TestEnsureInternalLoadBalancerDeleted(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil { + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) // Delete the loadbalancer. - result = l.EnsureInternalLoadBalancerDeleted(svc) + result = l4.EnsureInternalLoadBalancerDeleted(svc) if result.Error != nil { t.Errorf("Unexpected error %v", result.Error) } - assertInternalLbResourcesDeleted(t, svc, true, l) + assertInternalLbResourcesDeleted(t, l4) } func TestEnsureInternalLoadBalancerDeletedTwiceDoesNotError(t *testing.T) { @@ -580,34 +580,34 @@ func TestEnsureInternalLoadBalancerDeletedTwiceDoesNotError(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil { + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) // Delete the loadbalancer - result = l.EnsureInternalLoadBalancerDeleted(svc) + result = l4.EnsureInternalLoadBalancerDeleted(svc) if result.Error != nil { t.Errorf("Unexpected error %v", result.Error) } - assertInternalLbResourcesDeleted(t, svc, true, l) + assertInternalLbResourcesDeleted(t, l4) // Deleting the loadbalancer and resources again should not cause an error. - result = l.EnsureInternalLoadBalancerDeleted(svc) + result = l4.EnsureInternalLoadBalancerDeleted(svc) if result.Error != nil { t.Errorf("Unexpected error %v", result.Error) } - assertInternalLbResourcesDeleted(t, svc, true, l) + assertInternalLbResourcesDeleted(t, l4) } func TestEnsureInternalLoadBalancerDeletedWithSharedHC(t *testing.T) { @@ -624,19 +624,19 @@ func TestEnsureInternalLoadBalancerDeletedWithSharedHC(t *testing.T) { if result != nil && result.Error != nil { t.Fatalf("Error ensuring service err: %v", result.Error) } - svc2, l, result := ensureService(fakeGCE, namer, nodeNames, vals.ZoneName, 8081, t) + svc2, l4, result := ensureService(fakeGCE, namer, nodeNames, vals.ZoneName, 8081, t) if result != nil && result.Error != nil { t.Fatalf("Error ensuring service err: %v", result.Error) } // Delete the loadbalancer. - result = l.EnsureInternalLoadBalancerDeleted(svc2) + result = l4.EnsureInternalLoadBalancerDeleted(svc2) if result.Error != nil { t.Errorf("Unexpected error %v", result.Error) } // When health check is shared we expect that hc firewall rule will not be deleted. - hcFwName := l.namer.L4HealthCheckFirewall(l.Service.Namespace, l.Service.Name, true) - firewall, err := l.cloud.GetFirewall(hcFwName) + hcFwName := l4.namer.L4HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, true) + firewall, err := l4.cloud.GetFirewall(hcFwName) if err != nil || firewall == nil { t.Errorf("Expected firewall exists err: %v, fwR: %v", err, firewall) } @@ -651,7 +651,7 @@ func TestHealthCheckFirewallDeletionWithNetLB(t *testing.T) { namer := namer_util.NewL4Namer(kubeSystemUID, nil) // Create ILB Service - ilbSvc, l, result := ensureService(fakeGCE, namer, nodeNames, vals.ZoneName, 8081, t) + ilbSvc, l4, result := ensureService(fakeGCE, namer, nodeNames, vals.ZoneName, 8081, t) if result != nil && result.Error != nil { t.Fatalf("Error ensuring service err: %v", result.Error) } @@ -660,7 +660,7 @@ func TestHealthCheckFirewallDeletionWithNetLB(t *testing.T) { netlbSvc := test.NewL4NetLBRBSService(8080) l4NetLB := NewL4NetLB(netlbSvc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) // make sure both ilb and netlb use the same l4 healtcheck instance - l4NetLB.l4HealthChecks = l.l4HealthChecks + l4NetLB.l4HealthChecks = l4.healthChecks // create netlb resources xlbResult := l4NetLB.EnsureFrontend(nodeNames, netlbSvc) @@ -673,15 +673,15 @@ func TestHealthCheckFirewallDeletionWithNetLB(t *testing.T) { assertNetLbResources(t, netlbSvc, l4NetLB, nodeNames) // Delete the ILB loadbalancer - result = l.EnsureInternalLoadBalancerDeleted(ilbSvc) + result = l4.EnsureInternalLoadBalancerDeleted(ilbSvc) if result.Error != nil { t.Errorf("Unexpected error %v", result.Error) } // When NetLB health check uses the same firewall rules we expect that hc firewall rule will not be deleted. - hcName := l.namer.L4HealthCheck(l.Service.Namespace, l.Service.Name, true) - hcFwName := l.namer.L4HealthCheckFirewall(l.Service.Namespace, l.Service.Name, true) - firewall, err := l.cloud.GetFirewall(hcFwName) + hcName := l4.namer.L4HealthCheck(l4.Service.Namespace, l4.Service.Name, true) + hcFwName := l4.namer.L4HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, true) + firewall, err := l4.cloud.GetFirewall(hcFwName) if err != nil { t.Errorf("Expected error: firewall exists, got %v", err) } @@ -690,7 +690,7 @@ func TestHealthCheckFirewallDeletionWithNetLB(t *testing.T) { } // The healthcheck itself should be deleted. - healthcheck, err := l.cloud.GetHealthCheck(hcName) + healthcheck, err := l4.cloud.GetHealthCheck(hcName) if err == nil || healthcheck != nil { t.Errorf("Expected error when looking up shared healthcheck after deletion") } @@ -704,22 +704,22 @@ func ensureService(fakeGCE *gce.Cloud, namer *namer_util.L4Namer, nodeNames []st Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, zoneName); err != nil { + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, zoneName); err != nil { return nil, nil, &L4ILBSyncResult{Error: fmt.Errorf("Unexpected error when adding nodes %v", err)} } - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + result := l4.EnsureInternalLoadBalancer(nodeNames, svc) if result.Error != nil { return nil, nil, result } if len(result.Status.Ingress) == 0 { - result.Error = fmt.Errorf("Got empty loadBalancer status using handler %v", l) + result.Error = fmt.Errorf("Got empty loadBalancer status using handler %v", l4) return nil, nil, result } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) - return svc, l, nil + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) + return svc, l4, nil } func TestEnsureInternalLoadBalancerWithSpecialHealthCheck(t *testing.T) { @@ -735,10 +735,10 @@ func TestEnsureInternalLoadBalancerWithSpecialHealthCheck(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil { + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } @@ -747,21 +747,21 @@ func TestEnsureInternalLoadBalancerWithSpecialHealthCheck(t *testing.T) { svc.Spec.Type = v1.ServiceTypeLoadBalancer svc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeLocal - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) - lbName := l.namer.L4Backend(svc.Namespace, svc.Name) - key, err := composite.CreateKey(l.cloud, lbName, meta.Global) + lbName := l4.namer.L4Backend(svc.Namespace, svc.Name) + key, err := composite.CreateKey(l4.cloud, lbName, meta.Global) if err != nil { t.Errorf("Unexpected error when creating key - %v", err) } - hc, err := composite.GetHealthCheck(l.cloud, key, meta.VersionGA) + hc, err := composite.GetHealthCheck(l4.cloud, key, meta.VersionGA) if err != nil || hc == nil { t.Errorf("Failed to get healthcheck, err %v", err) } @@ -847,28 +847,28 @@ func TestEnsureInternalLoadBalancerErrors(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - //lbName := l.namer.L4Backend(params.service.Namespace, params.service.Name) - frName := l.GetFRName() - key, err := composite.CreateKey(l.cloud, frName, meta.Regional) + //lbName :=l4.namer.L4Backend(params.service.Namespace, params.service.Name) + frName := l4.GetFRName() + key, err := composite.CreateKey(l4.cloud, frName, meta.Regional) if err != nil { t.Errorf("Unexpected error when creating key - %v", err) } - _, err = test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName) + _, err = test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName) if err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } // Create a dummy forwarding rule in order to trigger a delete in the EnsureInternalLoadBalancer function. - if err = composite.CreateForwardingRule(l.cloud, key, &composite.ForwardingRule{Name: frName}); err != nil { + if err = composite.CreateForwardingRule(l4.cloud, key, &composite.ForwardingRule{Name: frName}); err != nil { t.Errorf("Failed to create fake forwarding rule %s, err %v", frName, err) } // Inject error hooks after creating the forwarding rule. if tc.injectMock != nil { tc.injectMock(fakeGCE.Compute().(*cloud.MockGCE)) } - result := l.EnsureInternalLoadBalancer(nodeNames, params.service) + result := l4.EnsureInternalLoadBalancer(nodeNames, params.service) if result.Error == nil { t.Errorf("Expected error when %s", desc) } @@ -891,24 +891,24 @@ func TestEnsureLoadBalancerDeletedSucceedsOnXPN(t *testing.T) { nodeNames := []string{"test-node-1"} namer := namer_util.NewL4Namer(kubeSystemUID, nil) recorder := record.NewFakeRecorder(100) - l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, recorder, &sync.Mutex{})) - _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName) + l4 := NewL4Handler(svc, fakeGCE, meta.Regional, namer, recorder, &sync.Mutex{})) + _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName) if err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - fwName := l.namer.L4Backend(svc.Namespace, svc.Name) - status, err := l.EnsureInternalLoadBalancer(nodeNames, svc, &metrics.L4ILBServiceState{}) + fwName :=l4.namer.L4Backend(svc.Namespace, svc.Name) + status, err :=l4.EnsureInternalLoadBalancer(nodeNames, svc, &metrics.L4ILBServiceState{}) if err != nil { t.Errorf("Failed to ensure loadBalancer, err %v", err) } if len(status.Ingress) == 0 { - t.Errorf("Got empty loadBalancer status using handler %v", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames) + assertInternalLbResources(t, svc, l4, nodeNames) c.MockFirewalls.DeleteHook = mock.DeleteFirewallsUnauthorizedErrHook - err = l.EnsureInternalLoadBalancerDeleted(svc) + err =l4.EnsureInternalLoadBalancerDeleted(svc) if err != nil { t.Errorf("Failed to delete loadBalancer, err %v", err) } @@ -936,41 +936,41 @@ func TestEnsureInternalLoadBalancerEnableGlobalAccess(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil { + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - frName := l.GetFRName() - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + frName := l4.GetFRName() + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) // Change service to include the global access annotation svc.Annotations[gce.ServiceAnnotationILBAllowGlobalAccess] = "true" - result = l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) descString, err := utils.MakeL4LBServiceDescription(utils.ServiceKeyFunc(svc.Namespace, svc.Name), "1.2.3.0", meta.VersionGA, false, utils.ILB) if err != nil { t.Errorf("Unexpected error when creating description - %v", err) } - key, err := composite.CreateKey(l.cloud, frName, meta.Regional) + key, err := composite.CreateKey(l4.cloud, frName, meta.Regional) if err != nil { t.Errorf("Unexpected error when creating key - %v", err) } - fwdRule, err := composite.GetForwardingRule(l.cloud, key, meta.VersionGA) + fwdRule, err := composite.GetForwardingRule(l4.cloud, key, meta.VersionGA) if err != nil { t.Errorf("Unexpected error when looking up forwarding rule - %v", err) } @@ -982,15 +982,15 @@ func TestEnsureInternalLoadBalancerEnableGlobalAccess(t *testing.T) { } // remove the annotation and disable global access. delete(svc.Annotations, gce.ServiceAnnotationILBAllowGlobalAccess) - result = l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } // make sure GlobalAccess field is off. - fwdRule, err = composite.GetForwardingRule(l.cloud, key, meta.VersionGA) + fwdRule, err = composite.GetForwardingRule(l4.cloud, key, meta.VersionGA) if err != nil { t.Errorf("Unexpected error when looking up forwarding rule - %v", err) } @@ -1001,13 +1001,13 @@ func TestEnsureInternalLoadBalancerEnableGlobalAccess(t *testing.T) { if fwdRule.Description != descString { t.Errorf("Expected description %s, Got %s", descString, fwdRule.Description) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) // Delete the service - result = l.EnsureInternalLoadBalancerDeleted(svc) + result = l4.EnsureInternalLoadBalancerDeleted(svc) if result.Error != nil { t.Errorf("Unexpected error %v", err) } - assertInternalLbResourcesDeleted(t, svc, true, l) + assertInternalLbResourcesDeleted(t, l4) } func TestEnsureInternalLoadBalancerCustomSubnet(t *testing.T) { @@ -1024,23 +1024,23 @@ func TestEnsureInternalLoadBalancerCustomSubnet(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil { + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) - frName := l.GetFRName() - fwdRule, err := composite.GetForwardingRule(l.cloud, meta.RegionalKey(frName, l.cloud.Region()), meta.VersionGA) + frName := l4.GetFRName() + fwdRule, err := composite.GetForwardingRule(l4.cloud, meta.RegionalKey(frName, l4.cloud.Region()), meta.VersionGA) if err != nil || fwdRule == nil { t.Errorf("Unexpected error looking up forwarding rule - err %v", err) } @@ -1052,18 +1052,18 @@ func TestEnsureInternalLoadBalancerCustomSubnet(t *testing.T) { requestedIP := "4.5.6.7" svc.Annotations[gce.ServiceAnnotationILBSubnet] = "test-subnet" svc.Spec.LoadBalancerIP = requestedIP - result = l.EnsureInternalLoadBalancer(nodeNames, svc) + result = l4.EnsureInternalLoadBalancer(nodeNames, svc) if err != nil { t.Errorf("Failed to ensure loadBalancer, err %v", err) } if len(result.Status.Ingress) == 0 { - t.Errorf("Got empty loadBalancer status using handler %v", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) if result.Status.Ingress[0].IP != requestedIP { t.Fatalf("Reserved IP %s not propagated, Got '%s'", requestedIP, result.Status.Ingress[0].IP) } - fwdRule, err = composite.GetForwardingRule(l.cloud, meta.RegionalKey(frName, l.cloud.Region()), meta.VersionGA) + fwdRule, err = composite.GetForwardingRule(l4.cloud, meta.RegionalKey(frName, l4.cloud.Region()), meta.VersionGA) if err != nil || fwdRule == nil { t.Errorf("Unexpected error looking up forwarding rule - err %v", err) } @@ -1073,18 +1073,18 @@ func TestEnsureInternalLoadBalancerCustomSubnet(t *testing.T) { // Change to a different subnet svc.Annotations[gce.ServiceAnnotationILBSubnet] = "another-subnet" - result = l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) if result.Status.Ingress[0].IP != requestedIP { t.Errorf("Reserved IP %s not propagated, Got %s", requestedIP, result.Status.Ingress[0].IP) } - fwdRule, err = composite.GetForwardingRule(l.cloud, meta.RegionalKey(frName, l.cloud.Region()), meta.VersionGA) + fwdRule, err = composite.GetForwardingRule(l4.cloud, meta.RegionalKey(frName, l4.cloud.Region()), meta.VersionGA) if err != nil || fwdRule == nil { t.Errorf("Unexpected error looking up forwarding rule - err %v", err) } @@ -1093,15 +1093,15 @@ func TestEnsureInternalLoadBalancerCustomSubnet(t *testing.T) { } // remove the annotation - ILB should revert to default subnet. delete(svc.Annotations, gce.ServiceAnnotationILBSubnet) - result = l.EnsureInternalLoadBalancer(nodeNames, svc) + result = l4.EnsureInternalLoadBalancer(nodeNames, svc) if result.Error != nil { t.Errorf("Failed to ensure loadBalancer, err %v", result.Error) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) if len(result.Status.Ingress) == 0 { - t.Errorf("Got empty loadBalancer status using handler %v", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - fwdRule, err = composite.GetForwardingRule(l.cloud, meta.RegionalKey(frName, l.cloud.Region()), meta.VersionGA) + fwdRule, err = composite.GetForwardingRule(l4.cloud, meta.RegionalKey(frName, l4.cloud.Region()), meta.VersionGA) if err != nil || fwdRule == nil { t.Errorf("Unexpected error %v", err) } @@ -1109,11 +1109,11 @@ func TestEnsureInternalLoadBalancerCustomSubnet(t *testing.T) { t.Errorf("Unexpected subnet value '%s' in ILB ForwardingRule.", fwdRule.Subnetwork) } // Delete the loadbalancer - result = l.EnsureInternalLoadBalancerDeleted(svc) + result = l4.EnsureInternalLoadBalancerDeleted(svc) if result.Error != nil { t.Errorf("Unexpected error deleting loadbalancer - err %v", result.Error) } - assertInternalLbResourcesDeleted(t, svc, true, l) + assertInternalLbResourcesDeleted(t, l4) } func TestEnsureInternalFirewallPortRanges(t *testing.T) { @@ -1128,10 +1128,10 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - fwName := l.namer.L4Backend(l.Service.Namespace, l.Service.Name) + fwName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) tc := struct { Input []int Result []string @@ -1143,7 +1143,7 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) { c.MockFirewalls.PatchHook = nil nodeNames := []string{"test-node-1"} - _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName) + _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName) if err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } @@ -1159,11 +1159,11 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) { Protocol: string(v1.ProtocolTCP), IP: "1.2.3.4", } - err = firewalls.EnsureL4FirewallRule(l.cloud, utils.ServiceKeyFunc(svc.Namespace, svc.Name), &fwrParams /*sharedRule = */, false) + err = firewalls.EnsureL4FirewallRule(l4.cloud, utils.ServiceKeyFunc(svc.Namespace, svc.Name), &fwrParams /*sharedRule = */, false) if err != nil { t.Errorf("Unexpected error %v when ensuring firewall rule %s for svc %+v", err, fwName, svc) } - existingFirewall, err := l.cloud.GetFirewall(fwName) + existingFirewall, err := l4.cloud.GetFirewall(fwName) if err != nil || existingFirewall == nil || len(existingFirewall.Allowed) == 0 { t.Errorf("Unexpected error %v when looking up firewall %s, Got firewall %+v", err, fwName, existingFirewall) } @@ -1189,10 +1189,10 @@ func TestEnsureInternalLoadBalancerModifyProtocol(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName) + _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName) if err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } @@ -1200,9 +1200,9 @@ func TestEnsureInternalLoadBalancerModifyProtocol(t *testing.T) { // before deleting the forwarding rule. c.MockRegionBackendServices.UpdateHook = func(ctx context.Context, key *meta.Key, be *compute.BackendService, m *cloud.MockRegionBackendServices) error { // Check FRnames with both protocols to make sure there is no leak or incorrect update. - frNames := []string{l.getFRNameWithProtocol("TCP"), l.getFRNameWithProtocol("UDP")} + frNames := []string{l4.getFRNameWithProtocol("TCP"), l4.getFRNameWithProtocol("UDP")} for _, name := range frNames { - key, err := composite.CreateKey(l.cloud, name, meta.Regional) + key, err := composite.CreateKey(l4.cloud, name, meta.Regional) if err != nil { return fmt.Errorf("unexpected error when creating key - %v", err) } @@ -1218,20 +1218,20 @@ func TestEnsureInternalLoadBalancerModifyProtocol(t *testing.T) { return mock.UpdateRegionBackendServiceHook(ctx, key, be, m) } - frName := l.getFRNameWithProtocol("TCP") - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + frName := l4.getFRNameWithProtocol("TCP") + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) - key, err := composite.CreateKey(l.cloud, frName, meta.Regional) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) + key, err := composite.CreateKey(l4.cloud, frName, meta.Regional) if err != nil { t.Errorf("Unexpected error when creating key - %v", err) } - fwdRule, err := composite.GetForwardingRule(l.cloud, key, meta.VersionGA) + fwdRule, err := composite.GetForwardingRule(l4.cloud, key, meta.VersionGA) if err != nil { t.Errorf("Unexpected error when looking up forwarding rule - %v", err) } @@ -1240,24 +1240,24 @@ func TestEnsureInternalLoadBalancerModifyProtocol(t *testing.T) { } // change the protocol to UDP svc.Spec.Ports[0].Protocol = v1.ProtocolUDP - result = l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) // Make sure the old forwarding rule is deleted - fwdRule, err = composite.GetForwardingRule(l.cloud, key, meta.VersionGA) + fwdRule, err = composite.GetForwardingRule(l4.cloud, key, meta.VersionGA) if !utils.IsNotFoundError(err) { t.Errorf("Failed to delete ForwardingRule %s", frName) } - frName = l.getFRNameWithProtocol("UDP") - if key, err = composite.CreateKey(l.cloud, frName, meta.Regional); err != nil { + frName = l4.getFRNameWithProtocol("UDP") + if key, err = composite.CreateKey(l4.cloud, frName, meta.Regional); err != nil { t.Errorf("Unexpected error when creating key - %v", err) } - if fwdRule, err = composite.GetForwardingRule(l.cloud, key, meta.VersionGA); err != nil { + if fwdRule, err = composite.GetForwardingRule(l4.cloud, key, meta.VersionGA); err != nil { t.Errorf("Unexpected error when looking up forwarding rule - %v", err) } if fwdRule.IPProtocol != "UDP" { @@ -1265,11 +1265,11 @@ func TestEnsureInternalLoadBalancerModifyProtocol(t *testing.T) { } // Delete the service - result = l.EnsureInternalLoadBalancerDeleted(svc) + result = l4.EnsureInternalLoadBalancerDeleted(svc) if err != nil { t.Errorf("Unexpected error %v", err) } - assertInternalLbResourcesDeleted(t, svc, true, l) + assertInternalLbResourcesDeleted(t, l4) } func TestEnsureInternalLoadBalancerAllPorts(t *testing.T) { @@ -1287,26 +1287,26 @@ func TestEnsureInternalLoadBalancerAllPorts(t *testing.T) { Namer: namer, Recorder: record.NewFakeRecorder(100), } - l := NewL4Handler(l4ilbParams) - l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) - if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil { + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) } - result := l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) - frName := l.getFRNameWithProtocol("TCP") - key, err := composite.CreateKey(l.cloud, frName, meta.Regional) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) + frName := l4.getFRNameWithProtocol("TCP") + key, err := composite.CreateKey(l4.cloud, frName, meta.Regional) if err != nil { t.Errorf("Unexpected error when creating key - %v", err) } - fwdRule, err := composite.GetForwardingRule(l.cloud, key, meta.VersionGA) + fwdRule, err := composite.GetForwardingRule(l4.cloud, key, meta.VersionGA) if err != nil { t.Errorf("Unexpected error when looking up forwarding rule - %v", err) } @@ -1322,15 +1322,15 @@ func TestEnsureInternalLoadBalancerAllPorts(t *testing.T) { {Name: "testport", Port: int32(8300), Protocol: "TCP"}, {Name: "testport", Port: int32(8400), Protocol: "TCP"}, } - result = l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) - fwdRule, err = composite.GetForwardingRule(l.cloud, key, meta.VersionGA) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) + fwdRule, err = composite.GetForwardingRule(l4.cloud, key, meta.VersionGA) if err != nil { t.Errorf("Unexpected error when looking up forwarding rule - %v", err) } @@ -1348,15 +1348,15 @@ func TestEnsureInternalLoadBalancerAllPorts(t *testing.T) { {Name: "testport", Port: int32(8400), Protocol: "TCP"}, } expectPorts := []string{"8090", "8100", "8300", "8400"} - result = l.EnsureInternalLoadBalancer(nodeNames, svc) + 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", l) + t.Errorf("Got empty loadBalancer status using handler %v", l4) } - assertInternalLbResources(t, svc, l, nodeNames, result.Annotations) - fwdRule, err = composite.GetForwardingRule(l.cloud, key, meta.VersionGA) + assertInternalLbResources(t, svc, l4, nodeNames, result.Annotations) + fwdRule, err = composite.GetForwardingRule(l4.cloud, key, meta.VersionGA) if err != nil { t.Errorf("Unexpected error when looking up forwarding rule - %v", err) } @@ -1367,17 +1367,17 @@ func TestEnsureInternalLoadBalancerAllPorts(t *testing.T) { t.Errorf("Expected AllPorts field to be unset in forwarding rule - %+v", fwdRule) } // Delete the service - result = l.EnsureInternalLoadBalancerDeleted(svc) + result = l4.EnsureInternalLoadBalancerDeleted(svc) if result.Error != nil { t.Errorf("Unexpected error %v", result.Error) } - assertInternalLbResourcesDeleted(t, svc, true, l) + assertInternalLbResourcesDeleted(t, l4) } -func assertInternalLbResources(t *testing.T, apiService *v1.Service, l *L4, nodeNames []string, resourceAnnotations map[string]string) { +func assertInternalLbResources(t *testing.T, apiService *v1.Service, l4 *L4, nodeNames []string, resourceAnnotations map[string]string) { // Check that Firewalls are created for the LoadBalancer and the HealthCheck sharedHC := !servicehelper.RequestsOnlyLocalTraffic(apiService) - resourceName := l.namer.L4Backend(l.Service.Namespace, l.Service.Name) + resourceName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) resourceDesc, err := utils.MakeL4LBServiceDescription(utils.ServiceKeyFunc(apiService.Namespace, apiService.Name), "", meta.VersionGA, false, utils.ILB) if err != nil { @@ -1389,8 +1389,8 @@ func assertInternalLbResources(t *testing.T, apiService *v1.Service, l *L4, node } proto := utils.GetProtocol(apiService.Spec.Ports) expectedAnnotations := make(map[string]string) - hcName := l.namer.L4HealthCheck(apiService.Namespace, apiService.Name, sharedHC) - hcFwName := l.namer.L4HealthCheckFirewall(apiService.Namespace, apiService.Name, sharedHC) + hcName := l4.namer.L4HealthCheck(apiService.Namespace, apiService.Name, sharedHC) + hcFwName := l4.namer.L4HealthCheckFirewall(apiService.Namespace, apiService.Name, sharedHC) // hcDesc is the resource description for healthcheck and firewall rule allowing healthcheck. hcDesc := resourceDesc if sharedHC { @@ -1411,7 +1411,7 @@ func assertInternalLbResources(t *testing.T, apiService *v1.Service, l *L4, node t.Errorf("Got the same name %q for LB firewall rule and Healthcheck firewall rule", hcFwName) } for _, info := range fwNamesAndDesc { - firewall, err := l.cloud.GetFirewall(info.fwName) + firewall, err := l4.cloud.GetFirewall(info.fwName) if err != nil { t.Fatalf("Failed to fetch firewall rule %q - err %v", info.fwName, err) } @@ -1427,7 +1427,7 @@ func assertInternalLbResources(t *testing.T, apiService *v1.Service, l *L4, node } // Check that HealthCheck is created - healthcheck, err := composite.GetHealthCheck(l.cloud, meta.GlobalKey(hcName), meta.VersionGA) + healthcheck, err := composite.GetHealthCheck(l4.cloud, meta.GlobalKey(hcName), meta.VersionGA) if err != nil { t.Errorf("Failed to fetch healthcheck %s - err %v", hcName, err) } @@ -1442,9 +1442,9 @@ func assertInternalLbResources(t *testing.T, apiService *v1.Service, l *L4, node // Check that BackendService exists backendServiceName := resourceName - key := meta.RegionalKey(backendServiceName, l.cloud.Region()) - backendServiceLink := cloud.SelfLink(meta.VersionGA, l.cloud.ProjectID(), "backendServices", key) - bs, err := composite.GetBackendService(l.cloud, key, meta.VersionGA) + key := meta.RegionalKey(backendServiceName, l4.cloud.Region()) + backendServiceLink := cloud.SelfLink(meta.VersionGA, l4.cloud.ProjectID(), "backendServices", key) + bs, err := composite.GetBackendService(l4.cloud, key, meta.VersionGA) if err != nil { t.Errorf("Failed to fetch backend service %s - err %v", backendServiceName, err) } @@ -1463,8 +1463,8 @@ func assertInternalLbResources(t *testing.T, apiService *v1.Service, l *L4, node } expectedAnnotations[annotations.BackendServiceKey] = backendServiceName // Check that ForwardingRule is created - frName := l.GetFRName() - fwdRule, err := composite.GetForwardingRule(l.cloud, meta.RegionalKey(frName, l.cloud.Region()), meta.VersionGA) + frName := l4.GetFRName() + fwdRule, err := composite.GetForwardingRule(l4.cloud, meta.RegionalKey(frName, l4.cloud.Region()), meta.VersionGA) if err != nil { t.Errorf("Failed to fetch forwarding rule %s - err %v", frName, err) @@ -1480,10 +1480,10 @@ func assertInternalLbResources(t *testing.T, apiService *v1.Service, l *L4, node } subnet := apiService.Annotations[gce.ServiceAnnotationILBSubnet] if subnet == "" { - subnet = l.cloud.SubnetworkURL() + subnet = l4.cloud.SubnetworkURL() } else { key.Name = subnet - subnet = cloud.SelfLink(meta.VersionGA, l.cloud.ProjectID(), "subnetworks", key) + subnet = cloud.SelfLink(meta.VersionGA, l4.cloud.ProjectID(), "subnetworks", key) } if fwdRule.Subnetwork != subnet { t.Errorf("Unexpected subnetwork %q in forwarding rule, expected %q", @@ -1494,7 +1494,7 @@ func assertInternalLbResources(t *testing.T, apiService *v1.Service, l *L4, node } else { expectedAnnotations[annotations.UDPForwardingRuleKey] = frName } - addr, err := l.cloud.GetRegionAddress(frName, l.cloud.Region()) + addr, err := l4.cloud.GetRegionAddress(frName, l4.cloud.Region()) if err == nil || addr != nil { t.Errorf("Expected error when looking up ephemeral address, got %v", addr) } @@ -1503,50 +1503,47 @@ func assertInternalLbResources(t *testing.T, apiService *v1.Service, l *L4, node } } -func assertInternalLbResourcesDeleted(t *testing.T, apiService *v1.Service, firewallsDeleted bool, l *L4) { - frName := l.GetFRName() - resourceName := l.namer.L4Backend(l.Service.Namespace, l.Service.Name) - hcNameShared := l.namer.L4HealthCheck(l.Service.Namespace, l.Service.Name, true) - hcFwNameShared := l.namer.L4HealthCheckFirewall(l.Service.Namespace, l.Service.Name, true) - hcNameNonShared := l.namer.L4HealthCheck(l.Service.Namespace, l.Service.Name, false) - hcFwNameNonShared := l.namer.L4HealthCheckFirewall(l.Service.Namespace, l.Service.Name, false) - - if firewallsDeleted { - // Check that Firewalls are deleted for the LoadBalancer and the HealthCheck - fwNames := []string{ - resourceName, - hcFwNameShared, - hcFwNameNonShared, - } +func assertInternalLbResourcesDeleted(t *testing.T, l4 *L4) { + frName := l4.GetFRName() + resourceName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) + hcNameShared := l4.namer.L4HealthCheck(l4.Service.Namespace, l4.Service.Name, true) + hcFwNameShared := l4.namer.L4HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, true) + hcNameNonShared := l4.namer.L4HealthCheck(l4.Service.Namespace, l4.Service.Name, false) + hcFwNameNonShared := l4.namer.L4HealthCheckFirewall(l4.Service.Namespace, l4.Service.Name, false) - for _, fwName := range fwNames { - firewall, err := l.cloud.GetFirewall(fwName) - if err == nil || firewall != nil { - t.Errorf("Expected error when looking up firewall rule after deletion") - } + fwNames := []string{ + resourceName, + hcFwNameShared, + hcFwNameNonShared, + } + + for _, fwName := range fwNames { + firewall, err := l4.cloud.GetFirewall(fwName) + if err == nil || firewall != nil { + t.Errorf("Expected error when looking up firewall rule after deletion") } } // Check forwarding rule is deleted - fwdRule, err := l.cloud.GetRegionForwardingRule(frName, l.cloud.Region()) + fwdRule, err := l4.cloud.GetRegionForwardingRule(frName, l4.cloud.Region()) if err == nil || fwdRule != nil { t.Errorf("Expected error when looking up forwarding rule after deletion") } // Check that HealthChecks are deleted - healthcheck, err := l.cloud.GetHealthCheck(hcNameShared) + healthcheck, err := l4.cloud.GetHealthCheck(hcNameShared) if err == nil || healthcheck != nil { t.Errorf("Expected error when looking up shared healthcheck after deletion") } - healthcheck, err = l.cloud.GetHealthCheck(hcNameNonShared) + healthcheck, err = l4.cloud.GetHealthCheck(hcNameNonShared) if err == nil || healthcheck != nil { t.Errorf("Expected error when looking up non-shared healthcheck after deletion") } - bs, err := l.cloud.GetRegionBackendService(resourceName, l.cloud.Region()) + bs, err := l4.cloud.GetRegionBackendService(resourceName, l4.cloud.Region()) if err == nil || bs != nil { t.Errorf("Expected error when looking up backend service after deletion") } - addr, err := l.cloud.GetRegionAddress(resourceName, l.cloud.Region()) + addr, err := l4.cloud.GetRegionAddress(resourceName, l4.cloud.Region()) if err == nil || addr != nil { t.Errorf("Expected error when looking up IP address after deletion") } diff --git a/pkg/loadbalancers/l4netlb_test.go b/pkg/loadbalancers/l4netlb_test.go index 180adf620c..772325bd46 100644 --- a/pkg/loadbalancers/l4netlb_test.go +++ b/pkg/loadbalancers/l4netlb_test.go @@ -169,7 +169,7 @@ func TestHealthCheckFirewallDeletionWithILB(t *testing.T) { l4NetLB := NewL4NetLB(netlbSvc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) // make sure both ilb and netlb use the same l4 healtcheck instance - l4NetLB.l4HealthChecks = l4ilb.l4HealthChecks + l4NetLB.l4HealthChecks = l4ilb.healthChecks // create netlb resources result := l4NetLB.EnsureFrontend(nodeNames, netlbSvc) diff --git a/pkg/loadbalancers/l7.go b/pkg/loadbalancers/l7.go index 17e2b139ca..797d484930 100644 --- a/pkg/loadbalancers/l7.go +++ b/pkg/loadbalancers/l7.go @@ -110,109 +110,109 @@ type L7 struct { // String returns the name of the loadbalancer. // Warning: This should be used only for logging and should not be used to // retrieve/ delete gce resource names. -func (l *L7) String() string { - return l.namer.LoadBalancer().String() +func (l7 *L7) String() string { + return l7.namer.LoadBalancer().String() } // Versions returns the struct listing the versions for every resource -func (l *L7) Versions() *features.ResourceVersions { - return features.VersionsFromIngress(&l.ingress) +func (l7 *L7) Versions() *features.ResourceVersions { + return features.VersionsFromIngress(&l7.ingress) } // CreateKey creates a meta.Key for use with composite types -func (l *L7) CreateKey(name string) (*meta.Key, error) { - return composite.CreateKey(l.cloud, name, l.scope) +func (l7 *L7) CreateKey(name string) (*meta.Key, error) { + return composite.CreateKey(l7.cloud, name, l7.scope) } // Regional returns true if the l7 scope is regional -func (l *L7) Regional() bool { - return l.scope == meta.Regional +func (l7 *L7) Regional() bool { + return l7.scope == meta.Regional } // RuntimeInfo returns the L7RuntimeInfo associated with the L7 load balancer. -func (l *L7) RuntimeInfo() *L7RuntimeInfo { - return l.runtimeInfo +func (l7 *L7) RuntimeInfo() *L7RuntimeInfo { + return l7.runtimeInfo } // UrlMap returns the UrlMap associated with the L7 load balancer. -func (l *L7) UrlMap() *composite.UrlMap { - return l.um +func (l7 *L7) UrlMap() *composite.UrlMap { + return l7.um } -func (l *L7) edgeHop() error { - sslConfigured := l.runtimeInfo.TLS != nil || l.runtimeInfo.TLSName != "" +func (l7 *L7) edgeHop() error { + sslConfigured := l7.runtimeInfo.TLS != nil || l7.runtimeInfo.TLSName != "" // Return an error if user configuration species that both HTTP & HTTPS are not to be configured. - if !l.runtimeInfo.AllowHTTP && !sslConfigured { + if !l7.runtimeInfo.AllowHTTP && !sslConfigured { return errAllProtocolsDisabled } // Check for invalid L7-ILB HTTPS config before attempting sync - if utils.IsGCEL7ILBIngress(l.runtimeInfo.Ingress) && sslConfigured && l.runtimeInfo.AllowHTTP { - l.recorder.Eventf(l.runtimeInfo.Ingress, corev1.EventTypeWarning, "WillNotConfigureFrontend", "gce-internal Ingress class does not currently support both HTTP and HTTPS served on the same IP (kubernetes.io/ingress.allow-http must be false when using HTTPS).") + if utils.IsGCEL7ILBIngress(l7.runtimeInfo.Ingress) && sslConfigured && l7.runtimeInfo.AllowHTTP { + l7.recorder.Eventf(l7.runtimeInfo.Ingress, corev1.EventTypeWarning, "WillNotConfigureFrontend", "gce-internal Ingress class does not currently support both HTTP and HTTPS served on the same IP (kubernetes.io/ingress.allow-http must be false when using HTTPS).") return fmt.Errorf("error invalid internal ingress https config") } - if err := l.ensureComputeURLMap(); err != nil { + if err := l7.ensureComputeURLMap(); err != nil { return err } if flags.F.EnableFrontendConfig { - if err := l.ensureRedirectURLMap(); err != nil { + if err := l7.ensureRedirectURLMap(); err != nil { return fmt.Errorf("ensureRedirectUrlMap() = %v", err) } } - if l.runtimeInfo.AllowHTTP { - if err := l.edgeHopHttp(); err != nil { + if l7.runtimeInfo.AllowHTTP { + if err := l7.edgeHopHttp(); err != nil { return err } - } else if flags.F.EnableDeleteUnusedFrontends && requireDeleteFrontend(l.ingress, namer.HTTPProtocol) { - if err := l.deleteHttp(features.VersionsFromIngress(&l.ingress)); err != nil { + } else if flags.F.EnableDeleteUnusedFrontends && requireDeleteFrontend(l7.ingress, namer.HTTPProtocol) { + if err := l7.deleteHttp(features.VersionsFromIngress(&l7.ingress)); err != nil { return err } - klog.V(2).Infof("Successfully deleted unused HTTP frontend resources for load-balancer %s", l) + klog.V(2).Infof("Successfully deleted unused HTTP frontend resources for load-balancer %s", l7) } // Defer promoting an ephemeral to a static IP until it's really needed. - if l.runtimeInfo.AllowHTTP && sslConfigured { - klog.V(3).Infof("checking static ip for %v", l) - if err := l.checkStaticIP(); err != nil { + if l7.runtimeInfo.AllowHTTP && sslConfigured { + klog.V(3).Infof("checking static ip for %v", l7) + if err := l7.checkStaticIP(); err != nil { return err } } if sslConfigured { - klog.V(3).Infof("validating https for %v", l) - if err := l.edgeHopHttps(); err != nil { + klog.V(3).Infof("validating https for %v", l7) + if err := l7.edgeHopHttps(); err != nil { return err } - } else if flags.F.EnableDeleteUnusedFrontends && requireDeleteFrontend(l.ingress, namer.HTTPSProtocol) { - if err := l.deleteHttps(features.VersionsFromIngress(&l.ingress)); err != nil { + } else if flags.F.EnableDeleteUnusedFrontends && requireDeleteFrontend(l7.ingress, namer.HTTPSProtocol) { + if err := l7.deleteHttps(features.VersionsFromIngress(&l7.ingress)); err != nil { return err } - klog.V(2).Infof("Successfully deleted unused HTTPS frontend resources for load-balancer %s", l) + klog.V(2).Infof("Successfully deleted unused HTTPS frontend resources for load-balancer %s", l7) } return nil } -func (l *L7) edgeHopHttp() error { - if err := l.checkProxy(); err != nil { +func (l7 *L7) edgeHopHttp() error { + if err := l7.checkProxy(); err != nil { return err } - if err := l.checkHttpForwardingRule(); err != nil { + if err := l7.checkHttpForwardingRule(); err != nil { return err } return nil } -func (l *L7) edgeHopHttps() error { - defer l.deleteOldSSLCerts() - if err := l.checkSSLCert(); err != nil { +func (l7 *L7) edgeHopHttps() error { + defer l7.deleteOldSSLCerts() + if err := l7.checkSSLCert(); err != nil { return err } - if err := l.checkHttpsProxy(); err != nil { + if err := l7.checkHttpsProxy(); err != nil { return err } - return l.checkHttpsForwardingRule() + return l7.checkHttpsForwardingRule() } // requireDeleteFrontend returns true if gce loadbalancer resources needs to deleted for given protocol. @@ -242,45 +242,45 @@ func requireDeleteFrontend(ing v1.Ingress, protocol namer.NamerProtocol) bool { } // GetIP returns the ip associated with the forwarding rule for this l7. -func (l *L7) GetIP() string { - if l.fw != nil { - return l.fw.IPAddress +func (l7 *L7) GetIP() string { + if l7.fw != nil { + return l7.fw.IPAddress } - if l.fws != nil { - return l.fws.IPAddress + if l7.fws != nil { + return l7.fws.IPAddress } return "" } // deleteForwardingRule deletes forwarding rule for given protocol. -func (l *L7) deleteForwardingRule(versions *features.ResourceVersions, protocol namer.NamerProtocol) error { - frName := l.namer.ForwardingRule(protocol) +func (l7 *L7) deleteForwardingRule(versions *features.ResourceVersions, protocol namer.NamerProtocol) error { + frName := l7.namer.ForwardingRule(protocol) klog.V(2).Infof("Deleting forwarding rule %v", frName) - key, err := l.CreateKey(frName) + key, err := l7.CreateKey(frName) if err != nil { return err } - if err := utils.IgnoreHTTPNotFound(composite.DeleteForwardingRule(l.cloud, key, versions.ForwardingRule)); err != nil { + if err := utils.IgnoreHTTPNotFound(composite.DeleteForwardingRule(l7.cloud, key, versions.ForwardingRule)); err != nil { return err } return nil } // deleteTargetProxy deletes target proxy for given protocol. -func (l *L7) deleteTargetProxy(versions *features.ResourceVersions, protocol namer.NamerProtocol) error { - tpName := l.namer.TargetProxy(protocol) +func (l7 *L7) deleteTargetProxy(versions *features.ResourceVersions, protocol namer.NamerProtocol) error { + tpName := l7.namer.TargetProxy(protocol) klog.V(2).Infof("Deleting target %v proxy %v", protocol, tpName) - key, err := l.CreateKey(tpName) + key, err := l7.CreateKey(tpName) if err != nil { return err } switch protocol { case namer.HTTPProtocol: - if err := utils.IgnoreHTTPNotFound(composite.DeleteTargetHttpProxy(l.cloud, key, versions.TargetHttpProxy)); err != nil { + if err := utils.IgnoreHTTPNotFound(composite.DeleteTargetHttpProxy(l7.cloud, key, versions.TargetHttpProxy)); err != nil { return err } case namer.HTTPSProtocol: - if err := utils.IgnoreHTTPNotFound(composite.DeleteTargetHttpsProxy(l.cloud, key, versions.TargetHttpsProxy)); err != nil { + if err := utils.IgnoreHTTPNotFound(composite.DeleteTargetHttpsProxy(l7.cloud, key, versions.TargetHttpsProxy)); err != nil { return err } default: @@ -290,66 +290,66 @@ func (l *L7) deleteTargetProxy(versions *features.ResourceVersions, protocol nam } // deleteHttp deletes http forwarding rule and target http proxy. -func (l *L7) deleteHttp(versions *features.ResourceVersions) error { +func (l7 *L7) deleteHttp(versions *features.ResourceVersions) error { // Delete http forwarding rule. - if err := l.deleteForwardingRule(versions, namer.HTTPProtocol); err != nil { + if err := l7.deleteForwardingRule(versions, namer.HTTPProtocol); err != nil { return err } // Delete target http proxy. - return l.deleteTargetProxy(versions, namer.HTTPProtocol) + return l7.deleteTargetProxy(versions, namer.HTTPProtocol) } // deleteHttps deletes https forwarding rule, target https proxy and ingress controller // managed ssl certificates. -func (l *L7) deleteHttps(versions *features.ResourceVersions) error { +func (l7 *L7) deleteHttps(versions *features.ResourceVersions) error { // Delete https forwarding rule. - if err := l.deleteForwardingRule(versions, namer.HTTPSProtocol); err != nil { + if err := l7.deleteForwardingRule(versions, namer.HTTPSProtocol); err != nil { return err } // Get list of ssl certificates owned by this load-balancer that needs to be deleted. // We are using https target proxy to list legacy certs, so this list needs to be // populated before deleting https target proxy. - secretsSslCerts, err := l.getIngressManagedSslCerts() + secretsSslCerts, err := l7.getIngressManagedSslCerts() if err != nil { return err } // Delete target https proxy. - if err := l.deleteTargetProxy(versions, namer.HTTPSProtocol); err != nil { + if err := l7.deleteTargetProxy(versions, namer.HTTPSProtocol); err != nil { return err } // Delete ingress managed ssl certificates those created from a secret, // not referencing a pre-created GCE cert or managed certificates. - return l.deleteSSLCertificates(secretsSslCerts, versions) + return l7.deleteSSLCertificates(secretsSslCerts, versions) } // deleteSSLCertificates deletes given ssl certificates. -func (l *L7) deleteSSLCertificates(sslCertificates []*composite.SslCertificate, versions *features.ResourceVersions) error { +func (l7 *L7) deleteSSLCertificates(sslCertificates []*composite.SslCertificate, versions *features.ResourceVersions) error { if len(sslCertificates) == 0 { return nil } var certErr error for _, cert := range sslCertificates { klog.V(2).Infof("Deleting sslcert %s", cert.Name) - key, err := l.CreateKey(cert.Name) + key, err := l7.CreateKey(cert.Name) if err != nil { return err } - if err := utils.IgnoreHTTPNotFound(composite.DeleteSslCertificate(l.cloud, key, versions.SslCertificate)); err != nil { + if err := utils.IgnoreHTTPNotFound(composite.DeleteSslCertificate(l7.cloud, key, versions.SslCertificate)); err != nil { klog.Errorf("Old cert delete failed - %v", err) certErr = err } } - l.sslCerts = nil + l7.sslCerts = nil return certErr } // deleteStaticIP deletes ingress managed static ip. -func (l *L7) deleteStaticIP() error { - frName := l.namer.ForwardingRule(namer.HTTPProtocol) - ip, err := l.cloud.GetGlobalAddress(frName) +func (l7 *L7) deleteStaticIP() error { + frName := l7.namer.ForwardingRule(namer.HTTPProtocol) + ip, err := l7.cloud.GetGlobalAddress(frName) if ip != nil && utils.IgnoreHTTPNotFound(err) == nil { klog.V(2).Infof("Deleting static IP %v(%v)", ip.Name, ip.Address) - if err := utils.IgnoreHTTPNotFound(l.cloud.DeleteGlobalAddress(ip.Name)); err != nil { + if err := utils.IgnoreHTTPNotFound(l7.cloud.DeleteGlobalAddress(ip.Name)); err != nil { return err } } @@ -359,88 +359,88 @@ func (l *L7) deleteStaticIP() error { // Cleanup deletes resources specific to this l7 in the right order. // forwarding rule -> target proxy -> url map // This leaves backends and health checks, which are shared across loadbalancers. -func (l *L7) Cleanup(versions *features.ResourceVersions) error { +func (l7 *L7) Cleanup(versions *features.ResourceVersions) error { var err error // Delete http frontend resources. - if err := l.deleteHttp(versions); err != nil { + if err := l7.deleteHttp(versions); err != nil { return err } // Delete static ip. - if err := l.deleteStaticIP(); err != nil { + if err := l7.deleteStaticIP(); err != nil { return err } // Delete https frontend resources. - if err := l.deleteHttps(versions); err != nil { + if err := l7.deleteHttps(versions); err != nil { return err } // Delete URL map. - umName := l.namer.UrlMap() + umName := l7.namer.UrlMap() klog.V(2).Infof("Deleting URL Map %v", umName) - key, err := l.CreateKey(umName) + key, err := l7.CreateKey(umName) if err != nil { return err } - if err := utils.IgnoreHTTPNotFound(composite.DeleteUrlMap(l.cloud, key, versions.UrlMap)); err != nil { + if err := utils.IgnoreHTTPNotFound(composite.DeleteUrlMap(l7.cloud, key, versions.UrlMap)); err != nil { return err } // Delete RedirectUrlMap if exists if flags.F.EnableFrontendConfig { - umName, supported := l.namer.RedirectUrlMap() + umName, supported := l7.namer.RedirectUrlMap() if !supported { // Skip deletion return nil } klog.V(2).Infof("Deleting Redirect URL Map %v", umName) - key, err := l.CreateKey(umName) + key, err := l7.CreateKey(umName) if err != nil { return err } - if err := utils.IgnoreHTTPNotFound(composite.DeleteUrlMap(l.cloud, key, versions.UrlMap)); err != nil { + if err := utils.IgnoreHTTPNotFound(composite.DeleteUrlMap(l7.cloud, key, versions.UrlMap)); err != nil { return err } } return nil } -func (l *L7) getFrontendAnnotations(existing map[string]string) map[string]string { +func (l7 *L7) getFrontendAnnotations(existing map[string]string) map[string]string { if existing == nil { existing = map[string]string{} } var certs []string - for _, cert := range l.sslCerts { + for _, cert := range l7.sslCerts { certs = append(certs, cert.Name) } - existing[annotations.UrlMapKey] = l.um.Name + existing[annotations.UrlMapKey] = l7.um.Name // Forwarding rule and target proxy might not exist if allowHTTP == false - if l.fw != nil { - existing[annotations.HttpForwardingRuleKey] = l.fw.Name + if l7.fw != nil { + existing[annotations.HttpForwardingRuleKey] = l7.fw.Name } else { delete(existing, annotations.HttpForwardingRuleKey) } - if l.tp != nil { - existing[annotations.TargetHttpProxyKey] = l.tp.Name + if l7.tp != nil { + existing[annotations.TargetHttpProxyKey] = l7.tp.Name } else { delete(existing, annotations.TargetHttpProxyKey) } // HTTPs resources might not exist if TLS == nil - if l.fws != nil { - existing[annotations.HttpsForwardingRuleKey] = l.fws.Name + if l7.fws != nil { + existing[annotations.HttpsForwardingRuleKey] = l7.fws.Name } else { delete(existing, annotations.HttpsForwardingRuleKey) } - if l.tps != nil { - existing[annotations.TargetHttpsProxyKey] = l.tps.Name + if l7.tps != nil { + existing[annotations.TargetHttpsProxyKey] = l7.tps.Name } else { delete(existing, annotations.TargetHttpsProxyKey) } // Handle Https Redirect Map if flags.F.EnableFrontendConfig { - if l.redirectUm != nil { - existing[annotations.RedirectUrlMapKey] = l.redirectUm.Name + if l7.redirectUm != nil { + existing[annotations.RedirectUrlMapKey] = l7.redirectUm.Name } else { delete(existing, annotations.RedirectUrlMapKey) } @@ -449,8 +449,8 @@ func (l *L7) getFrontendAnnotations(existing map[string]string) map[string]strin // Note that ingress IP annotation is not deleted when user disables one of http/https. // This is because the promoted static IP is retained for use and will be deleted only // when load-balancer is deleted or user specifies a different IP. - if l.ip != nil { - existing[annotations.StaticIPKey] = l.ip.Name + if l7.ip != nil { + existing[annotations.StaticIPKey] = l7.ip.Name } if len(certs) > 0 { existing[annotations.SSLCertKey] = strings.Join(certs, ",") @@ -499,13 +499,13 @@ func GCEResourceName(ingAnnotations map[string]string, resourceName string) stri } // description gets a description for the ingress GCP resources. -func (l *L7) description() (string, error) { - if l.runtimeInfo.Ingress == nil { - return "", fmt.Errorf("missing Ingress object to construct description for %s", l) +func (l7 *L7) description() (string, error) { + if l7.runtimeInfo.Ingress == nil { + return "", fmt.Errorf("missing Ingress object to construct description for %s", l7) } - namespace := l.runtimeInfo.Ingress.ObjectMeta.Namespace - ingressName := l.runtimeInfo.Ingress.ObjectMeta.Name + namespace := l7.runtimeInfo.Ingress.ObjectMeta.Namespace + ingressName := l7.runtimeInfo.Ingress.ObjectMeta.Name namespacedName := types.NamespacedName{Name: ingressName, Namespace: namespace} return fmt.Sprintf(`{"kubernetes.io/ingress-name": %q}`, namespacedName.String()), nil diff --git a/pkg/loadbalancers/l7s.go b/pkg/loadbalancers/l7s.go index ac318348b3..ffdb14bc3e 100644 --- a/pkg/loadbalancers/l7s.go +++ b/pkg/loadbalancers/l7s.go @@ -57,12 +57,12 @@ func NewLoadBalancerPool(cloud *gce.Cloud, v1NamerHelper namer_util.V1FrontendNa } // Ensure implements LoadBalancerPool. -func (l *L7s) Ensure(ri *L7RuntimeInfo) (*L7, error) { +func (l7s *L7s) Ensure(ri *L7RuntimeInfo) (*L7, error) { lb := &L7{ runtimeInfo: ri, - cloud: l.cloud, - namer: l.namerFactory.Namer(ri.Ingress), - recorder: l.recorderProducer.Recorder(ri.Ingress.Namespace), + cloud: l7s.cloud, + namer: l7s.namerFactory.Namer(ri.Ingress), + recorder: l7s.recorderProducer.Recorder(ri.Ingress.Namespace), scope: features.ScopeFromIngress(ri.Ingress), ingress: *ri.Ingress, } @@ -80,14 +80,14 @@ func (l *L7s) Ensure(ri *L7RuntimeInfo) (*L7, error) { } // delete deletes a loadbalancer by frontend namer. -func (l *L7s) delete(namer namer_util.IngressFrontendNamer, versions *features.ResourceVersions, scope meta.KeyType) error { +func (l7s *L7s) delete(namer namer_util.IngressFrontendNamer, versions *features.ResourceVersions, scope meta.KeyType) error { if !namer.IsValidLoadBalancer() { klog.V(2).Infof("Loadbalancer name %s invalid, skipping GC", namer.LoadBalancer()) return nil } lb := &L7{ runtimeInfo: &L7RuntimeInfo{}, - cloud: l.cloud, + cloud: l7s.cloud, namer: namer, scope: scope, } @@ -101,15 +101,15 @@ func (l *L7s) delete(namer namer_util.IngressFrontendNamer, versions *features.R } // list returns a list of urlMaps (the top level LB resource) that belong to the cluster. -func (l *L7s) list(key *meta.Key, version meta.Version) ([]*composite.UrlMap, error) { +func (l7s *L7s) list(key *meta.Key, version meta.Version) ([]*composite.UrlMap, error) { var result []*composite.UrlMap - urlMaps, err := composite.ListUrlMaps(l.cloud, key, version) + urlMaps, err := composite.ListUrlMaps(l7s.cloud, key, version) if err != nil { return nil, err } for _, um := range urlMaps { - if l.v1NamerHelper.NameBelongsToCluster(um.Name) { + if l7s.v1NamerHelper.NameBelongsToCluster(um.Name) { result = append(result, um) } } @@ -118,10 +118,10 @@ func (l *L7s) list(key *meta.Key, version meta.Version) ([]*composite.UrlMap, er } // GCv2 implements LoadBalancerPool. -func (l *L7s) GCv2(ing *v1.Ingress, scope meta.KeyType) error { +func (l7s *L7s) GCv2(ing *v1.Ingress, scope meta.KeyType) error { ingKey := common.NamespacedName(ing) klog.V(2).Infof("GCv2(%v)", ingKey) - if err := l.delete(l.namerFactory.Namer(ing), features.VersionsFromIngress(ing), scope); err != nil { + if err := l7s.delete(l7s.namerFactory.Namer(ing), features.VersionsFromIngress(ing), scope); err != nil { return err } klog.V(2).Infof("GCv2(%v) ok", ingKey) @@ -132,24 +132,24 @@ func (l *L7s) GCv2(ing *v1.Ingress, scope meta.KeyType) error { // (e.g. when a user migrates from ILB to ELB on the same ingress or vice versa.) // This only applies to the V2 Naming Scheme // TODO(shance): Refactor to avoid calling GCE every sync loop -func (l *L7s) FrontendScopeChangeGC(ing *v1.Ingress) (*meta.KeyType, error) { +func (l7s *L7s) FrontendScopeChangeGC(ing *v1.Ingress) (*meta.KeyType, error) { if ing == nil { return nil, nil } - namer := l.namerFactory.Namer(ing) + namer := l7s.namerFactory.Namer(ing) urlMapName := namer.UrlMap() currentScope := features.ScopeFromIngress(ing) for _, scope := range []meta.KeyType{meta.Global, meta.Regional} { if scope != currentScope { - key, err := composite.CreateKey(l.cloud, urlMapName, scope) + key, err := composite.CreateKey(l7s.cloud, urlMapName, scope) if err != nil { return nil, err } // Look for existing LBs with the same name but of a different scope - _, err = composite.GetUrlMap(l.cloud, key, features.VersionsFromIngress(ing).UrlMap) + _, err = composite.GetUrlMap(l7s.cloud, key, features.VersionsFromIngress(ing).UrlMap) if err == nil { klog.V(2).Infof("GC'ing ing %v for scope %q", ing, scope) return &scope, nil @@ -164,35 +164,35 @@ func (l *L7s) FrontendScopeChangeGC(ing *v1.Ingress) (*meta.KeyType, error) { // GCv1 implements LoadBalancerPool. // TODO(shance): Update to handle regional and global LB with same name -func (l *L7s) GCv1(names []string) error { +func (l7s *L7s) GCv1(names []string) error { klog.V(2).Infof("GCv1(%v)", names) knownLoadBalancers := make(map[namer_util.LoadBalancerName]bool) for _, n := range names { - knownLoadBalancers[l.v1NamerHelper.LoadBalancer(n)] = true + knownLoadBalancers[l7s.v1NamerHelper.LoadBalancer(n)] = true } // GC L7-ILB LBs if enabled - key, err := composite.CreateKey(l.cloud, "", meta.Regional) + key, err := composite.CreateKey(l7s.cloud, "", meta.Regional) if err != nil { return fmt.Errorf("error getting regional key: %v", err) } - urlMaps, err := l.list(key, features.L7ILBVersions().UrlMap) + urlMaps, err := l7s.list(key, features.L7ILBVersions().UrlMap) if err != nil { return fmt.Errorf("error listing regional LBs: %v", err) } - if err := l.gc(urlMaps, knownLoadBalancers, features.L7ILBVersions()); err != nil { + if err := l7s.gc(urlMaps, knownLoadBalancers, features.L7ILBVersions()); err != nil { return fmt.Errorf("error gc-ing regional LBs: %v", err) } // TODO(shance): fix list taking a key - urlMaps, err = l.list(meta.GlobalKey(""), meta.VersionGA) + urlMaps, err = l7s.list(meta.GlobalKey(""), meta.VersionGA) if err != nil { return fmt.Errorf("error listing global LBs: %v", err) } - if errors := l.gc(urlMaps, knownLoadBalancers, features.GAResourceVersions); errors != nil { + if errors := l7s.gc(urlMaps, knownLoadBalancers, features.GAResourceVersions); errors != nil { return fmt.Errorf("error gcing global LBs: %v", errors) } @@ -201,12 +201,12 @@ func (l *L7s) GCv1(names []string) error { // gc is a helper for GCv1. // TODO(shance): get versions from description -func (l *L7s) gc(urlMaps []*composite.UrlMap, knownLoadBalancers map[namer_util.LoadBalancerName]bool, versions *features.ResourceVersions) []error { +func (l7s *L7s) gc(urlMaps []*composite.UrlMap, knownLoadBalancers map[namer_util.LoadBalancerName]bool, versions *features.ResourceVersions) []error { var errors []error // Delete unknown loadbalancers for _, um := range urlMaps { - l7Name := l.v1NamerHelper.LoadBalancerForURLMap(um.Name) + l7Name := l7s.v1NamerHelper.LoadBalancerForURLMap(um.Name) if knownLoadBalancers[l7Name] { klog.V(3).Infof("Load balancer %v is still valid, not GC'ing", l7Name) @@ -219,7 +219,7 @@ func (l *L7s) gc(urlMaps []*composite.UrlMap, knownLoadBalancers map[namer_util. continue } - if err := l.delete(l.namerFactory.NamerForLoadBalancer(l7Name), versions, scope); err != nil { + if err := l7s.delete(l7s.namerFactory.NamerForLoadBalancer(l7Name), versions, scope); err != nil { errors = append(errors, fmt.Errorf("error deleting loadbalancer %q: %v", l7Name, err)) } } @@ -227,9 +227,9 @@ func (l *L7s) gc(urlMaps []*composite.UrlMap, knownLoadBalancers map[namer_util. } // Shutdown implements LoadBalancerPool. -func (l *L7s) Shutdown(ings []*v1.Ingress) error { +func (l7s *L7s) Shutdown(ings []*v1.Ingress) error { // Delete ingresses that use v1 naming scheme. - if err := l.GCv1([]string{}); err != nil { + if err := l7s.GCv1([]string{}); err != nil { return fmt.Errorf("error deleting load-balancers for v1 naming policy: %v", err) } // Delete ingresses that use v2 naming policy. @@ -238,7 +238,7 @@ func (l *L7s) Shutdown(ings []*v1.Ingress) error { return namer_util.FrontendNamingScheme(ing) == namer_util.V2NamingScheme }).AsList() for _, ing := range v2Ings { - if err := l.GCv2(ing, features.ScopeFromIngress(ing)); err != nil { + if err := l7s.GCv2(ing, features.ScopeFromIngress(ing)); err != nil { errs = append(errs, err) } } @@ -250,13 +250,13 @@ func (l *L7s) Shutdown(ings []*v1.Ingress) error { } // HasUrlMap implements LoadBalancerPool. -func (l *L7s) HasUrlMap(ing *v1.Ingress) (bool, error) { - namer := l.namerFactory.Namer(ing) - key, err := composite.CreateKey(l.cloud, namer.UrlMap(), features.ScopeFromIngress(ing)) +func (l7s *L7s) HasUrlMap(ing *v1.Ingress) (bool, error) { + namer := l7s.namerFactory.Namer(ing) + key, err := composite.CreateKey(l7s.cloud, namer.UrlMap(), features.ScopeFromIngress(ing)) if err != nil { return false, err } - if _, err := composite.GetUrlMap(l.cloud, key, features.VersionsFromIngress(ing).UrlMap); err != nil { + if _, err := composite.GetUrlMap(l7s.cloud, key, features.VersionsFromIngress(ing).UrlMap); err != nil { if utils.IsHTTPErrorCode(err, http.StatusNotFound) { return false, nil } diff --git a/pkg/loadbalancers/target_proxies.go b/pkg/loadbalancers/target_proxies.go index d4f657cabb..f478df67d9 100644 --- a/pkg/loadbalancers/target_proxies.go +++ b/pkg/loadbalancers/target_proxies.go @@ -33,173 +33,173 @@ const ( ) // checkProxy ensures the correct TargetHttpProxy for a loadbalancer -func (l *L7) checkProxy() (err error) { +func (l7 *L7) checkProxy() (err error) { // Get UrlMap Name, could be the url map or the redirect url map // TODO(shance): move to translator var umName string if flags.F.EnableFrontendConfig { - if l.redirectUm != nil && l.runtimeInfo.FrontendConfig.Spec.RedirectToHttps != nil && l.runtimeInfo.FrontendConfig.Spec.RedirectToHttps.Enabled { - umName = l.redirectUm.Name + if l7.redirectUm != nil && l7.runtimeInfo.FrontendConfig.Spec.RedirectToHttps != nil && l7.runtimeInfo.FrontendConfig.Spec.RedirectToHttps.Enabled { + umName = l7.redirectUm.Name } else { - umName = l.um.Name + umName = l7.um.Name } } else { - umName = l.um.Name + umName = l7.um.Name } - urlMapKey, err := l.CreateKey(umName) + urlMapKey, err := l7.CreateKey(umName) if err != nil { return err } - isL7ILB := utils.IsGCEL7ILBIngress(l.runtimeInfo.Ingress) - tr := translator.NewTranslator(isL7ILB, l.namer) + isL7ILB := utils.IsGCEL7ILBIngress(l7.runtimeInfo.Ingress) + tr := translator.NewTranslator(isL7ILB, l7.namer) - description, err := l.description() + description, err := l7.description() if err != nil { return err } - version := l.Versions().TargetHttpProxy + version := l7.Versions().TargetHttpProxy proxy := tr.ToCompositeTargetHttpProxy(description, version, urlMapKey) - key, err := l.CreateKey(proxy.Name) + key, err := l7.CreateKey(proxy.Name) if err != nil { return err } - currentProxy, _ := composite.GetTargetHttpProxy(l.cloud, key, version) + currentProxy, _ := composite.GetTargetHttpProxy(l7.cloud, key, version) if currentProxy == nil { - klog.V(3).Infof("Creating new http proxy for urlmap %v", l.um.Name) - key, err := l.CreateKey(proxy.Name) + klog.V(3).Infof("Creating new http proxy for urlmap %v", l7.um.Name) + key, err := l7.CreateKey(proxy.Name) if err != nil { return err } - if err = composite.CreateTargetHttpProxy(l.cloud, key, proxy); err != nil { + if err = composite.CreateTargetHttpProxy(l7.cloud, key, proxy); err != nil { return err } - currentProxy, err = composite.GetTargetHttpProxy(l.cloud, key, version) - l.recorder.Eventf(l.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q created", key.Name) + currentProxy, err = composite.GetTargetHttpProxy(l7.cloud, key, version) + l7.recorder.Eventf(l7.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q created", key.Name) if err != nil { return err } - l.tp = currentProxy + l7.tp = currentProxy return nil } if !utils.EqualResourcePaths(currentProxy.UrlMap, proxy.UrlMap) { klog.V(3).Infof("Proxy %v has the wrong url map, setting %v overwriting %v", currentProxy.Name, proxy.UrlMap, currentProxy.UrlMap) - key, err := l.CreateKey(currentProxy.Name) + key, err := l7.CreateKey(currentProxy.Name) if err != nil { return err } - if err := composite.SetUrlMapForTargetHttpProxy(l.cloud, key, currentProxy, proxy.UrlMap); err != nil { + if err := composite.SetUrlMapForTargetHttpProxy(l7.cloud, key, currentProxy, proxy.UrlMap); err != nil { return err } - l.recorder.Eventf(l.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q updated", key.Name) + l7.recorder.Eventf(l7.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q updated", key.Name) } - l.tp = currentProxy + l7.tp = currentProxy return nil } -func (l *L7) checkHttpsProxy() (err error) { - isL7ILB := utils.IsGCEL7ILBIngress(l.runtimeInfo.Ingress) - tr := translator.NewTranslator(isL7ILB, l.namer) - env := &translator.Env{FrontendConfig: l.runtimeInfo.FrontendConfig} +func (l7 *L7) checkHttpsProxy() (err error) { + isL7ILB := utils.IsGCEL7ILBIngress(l7.runtimeInfo.Ingress) + tr := translator.NewTranslator(isL7ILB, l7.namer) + env := &translator.Env{FrontendConfig: l7.runtimeInfo.FrontendConfig} - if len(l.sslCerts) == 0 { - klog.V(2).Infof("No SSL certificates for %q, will not create HTTPS Proxy.", l) + if len(l7.sslCerts) == 0 { + klog.V(2).Infof("No SSL certificates for %q, will not create HTTPS Proxy.", l7) return nil } - urlMapKey, err := l.CreateKey(l.um.Name) + urlMapKey, err := l7.CreateKey(l7.um.Name) if err != nil { return err } - description, err := l.description() - version := l.Versions().TargetHttpProxy - proxy, sslPolicySet, err := tr.ToCompositeTargetHttpsProxy(env, description, version, urlMapKey, l.sslCerts) + description, err := l7.description() + version := l7.Versions().TargetHttpProxy + proxy, sslPolicySet, err := tr.ToCompositeTargetHttpsProxy(env, description, version, urlMapKey, l7.sslCerts) if err != nil { return err } - key, err := l.CreateKey(proxy.Name) + key, err := l7.CreateKey(proxy.Name) if err != nil { return err } - currentProxy, _ := composite.GetTargetHttpsProxy(l.cloud, key, version) + currentProxy, _ := composite.GetTargetHttpsProxy(l7.cloud, key, version) if err != nil { return err } if currentProxy == nil { - klog.V(3).Infof("Creating new https Proxy for urlmap %q", l.um.Name) + klog.V(3).Infof("Creating new https Proxy for urlmap %q", l7.um.Name) - if err = composite.CreateTargetHttpsProxy(l.cloud, key, proxy); err != nil { + if err = composite.CreateTargetHttpsProxy(l7.cloud, key, proxy); err != nil { return err } - l.recorder.Eventf(l.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q created", key.Name) + l7.recorder.Eventf(l7.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q created", key.Name) - key, err = l.CreateKey(proxy.Name) + key, err = l7.CreateKey(proxy.Name) if err != nil { return err } - currentProxy, err = composite.GetTargetHttpsProxy(l.cloud, key, version) + currentProxy, err = composite.GetTargetHttpsProxy(l7.cloud, key, version) if err != nil { return err } - l.tps = currentProxy + l7.tps = currentProxy return nil } if !utils.EqualResourcePaths(currentProxy.UrlMap, proxy.UrlMap) { klog.V(2).Infof("Https Proxy %v has the wrong url map, setting %v overwriting %v", currentProxy.Name, proxy.UrlMap, currentProxy.UrlMap) - key, err := l.CreateKey(currentProxy.Name) + key, err := l7.CreateKey(currentProxy.Name) if err != nil { return err } - if err := composite.SetUrlMapForTargetHttpsProxy(l.cloud, key, currentProxy, proxy.UrlMap); err != nil { + if err := composite.SetUrlMapForTargetHttpsProxy(l7.cloud, key, currentProxy, proxy.UrlMap); err != nil { return err } - l.recorder.Eventf(l.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q updated", key.Name) + l7.recorder.Eventf(l7.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q updated", key.Name) } - if !l.compareCerts(currentProxy.SslCertificates) { + if !l7.compareCerts(currentProxy.SslCertificates) { klog.V(2).Infof("Https Proxy %q has the wrong ssl certs, setting %v overwriting %v", - currentProxy.Name, toCertNames(l.sslCerts), currentProxy.SslCertificates) + currentProxy.Name, toCertNames(l7.sslCerts), currentProxy.SslCertificates) var sslCertURLs []string - for _, cert := range l.sslCerts { + for _, cert := range l7.sslCerts { sslCertURLs = append(sslCertURLs, cert.SelfLink) } - key, err := l.CreateKey(currentProxy.Name) + key, err := l7.CreateKey(currentProxy.Name) if err != nil { return err } - if err := composite.SetSslCertificateForTargetHttpsProxy(l.cloud, key, currentProxy, sslCertURLs); err != nil { + if err := composite.SetSslCertificateForTargetHttpsProxy(l7.cloud, key, currentProxy, sslCertURLs); err != nil { return err } - l.recorder.Eventf(l.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q certs updated", key.Name) + l7.recorder.Eventf(l7.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q certs updated", key.Name) } if flags.F.EnableFrontendConfig && sslPolicySet { - if err := l.ensureSslPolicy(env, currentProxy, proxy.SslPolicy); err != nil { + if err := l7.ensureSslPolicy(env, currentProxy, proxy.SslPolicy); err != nil { return err } } - l.tps = currentProxy + l7.tps = currentProxy return nil } -func (l *L7) getSslCertLinkInUse() ([]string, error) { - proxyName := l.namer.TargetProxy(namer.HTTPSProtocol) - key, err := l.CreateKey(proxyName) +func (l7 *L7) getSslCertLinkInUse() ([]string, error) { + proxyName := l7.namer.TargetProxy(namer.HTTPSProtocol) + key, err := l7.CreateKey(proxyName) if err != nil { return nil, err } - proxy, err := composite.GetTargetHttpsProxy(l.cloud, key, l.Versions().TargetHttpsProxy) + proxy, err := composite.GetTargetHttpsProxy(l7.cloud, key, l7.Versions().TargetHttpsProxy) if err != nil { return nil, err } @@ -209,14 +209,14 @@ func (l *L7) getSslCertLinkInUse() ([]string, error) { // ensureSslPolicy ensures that the SslPolicy described in the frontendconfig is // properly applied to the proxy. -func (l *L7) ensureSslPolicy(env *translator.Env, currentProxy *composite.TargetHttpsProxy, policyLink string) error { +func (l7 *L7) ensureSslPolicy(env *translator.Env, currentProxy *composite.TargetHttpsProxy, policyLink string) error { if !utils.EqualResourceIDs(policyLink, currentProxy.SslPolicy) { - key, err := l.CreateKey(currentProxy.Name) + key, err := l7.CreateKey(currentProxy.Name) if err != nil { return err } - if err := composite.SetSslPolicyForTargetHttpsProxy(l.cloud, key, currentProxy, policyLink); err != nil { - l.recorder.Eventf(l.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q SSLPolicy updated", key.Name) + if err := composite.SetSslPolicyForTargetHttpsProxy(l7.cloud, key, currentProxy, policyLink); err != nil { + l7.recorder.Eventf(l7.runtimeInfo.Ingress, corev1.EventTypeNormal, events.SyncIngress, "TargetProxy %q SSLPolicy updated", key.Name) return err } } diff --git a/pkg/loadbalancers/url_maps.go b/pkg/loadbalancers/url_maps.go index f7cf809884..5d4f32ca97 100644 --- a/pkg/loadbalancers/url_maps.go +++ b/pkg/loadbalancers/url_maps.go @@ -31,23 +31,23 @@ import ( // ensureComputeURLMap retrieves the current URLMap and overwrites it if incorrect. If the resource // does not exist, the map is created. -func (l *L7) ensureComputeURLMap() error { - if l.runtimeInfo.UrlMap == nil { +func (l7 *L7) ensureComputeURLMap() error { + if l7.runtimeInfo.UrlMap == nil { return fmt.Errorf("cannot create urlmap without internal representation") } // Every update replaces the entire urlmap. // Use an empty name parameter since we only care about the scope // TODO: (shance) refactor this so we don't need an empty arg - key, err := l.CreateKey("") + key, err := l7.CreateKey("") if err != nil { return err } - expectedMap := translator.ToCompositeURLMap(l.runtimeInfo.UrlMap, l.namer, key) + expectedMap := translator.ToCompositeURLMap(l7.runtimeInfo.UrlMap, l7.namer, key) key.Name = expectedMap.Name - expectedMap.Version = l.Versions().UrlMap - currentMap, err := composite.GetUrlMap(l.cloud, key, expectedMap.Version) + expectedMap.Version = l7.Versions().UrlMap + currentMap, err := composite.GetUrlMap(l7.cloud, key, expectedMap.Version) if utils.IgnoreHTTPNotFound(err) != nil { return err } @@ -56,42 +56,42 @@ func (l *L7) ensureComputeURLMap() error { // Check for transitions between elb and ilb klog.V(2).Infof("Creating URLMap %q", expectedMap.Name) - if err := composite.CreateUrlMap(l.cloud, key, expectedMap); err != nil { + if err := composite.CreateUrlMap(l7.cloud, key, expectedMap); err != nil { return fmt.Errorf("CreateUrlMap: %v", err) } - l.recorder.Eventf(&l.ingress, apiv1.EventTypeNormal, events.SyncIngress, "UrlMap %q created", key.Name) - l.um = expectedMap + l7.recorder.Eventf(&l7.ingress, apiv1.EventTypeNormal, events.SyncIngress, "UrlMap %q created", key.Name) + l7.um = expectedMap return nil } if mapsEqual(currentMap, expectedMap) { - klog.V(4).Infof("URLMap for %q is unchanged", l) - l.um = currentMap + klog.V(4).Infof("URLMap for %q is unchanged", l7) + l7.um = currentMap return nil } - klog.V(2).Infof("Updating URLMap for %q", l) + klog.V(2).Infof("Updating URLMap for %q", l7) expectedMap.Fingerprint = currentMap.Fingerprint - if err := composite.UpdateUrlMap(l.cloud, key, expectedMap); err != nil { + if err := composite.UpdateUrlMap(l7.cloud, key, expectedMap); err != nil { return fmt.Errorf("UpdateURLMap: %v", err) } - l.recorder.Eventf(&l.ingress, apiv1.EventTypeNormal, events.SyncIngress, "UrlMap %q updated", key.Name) - l.um = expectedMap + l7.recorder.Eventf(&l7.ingress, apiv1.EventTypeNormal, events.SyncIngress, "UrlMap %q updated", key.Name) + l7.um = expectedMap return nil } -func (l *L7) ensureRedirectURLMap() error { - feConfig := l.runtimeInfo.FrontendConfig - isL7ILB := utils.IsGCEL7ILBIngress(&l.ingress) +func (l7 *L7) ensureRedirectURLMap() error { + feConfig := l7.runtimeInfo.FrontendConfig + isL7ILB := utils.IsGCEL7ILBIngress(&l7.ingress) - t := translator.NewTranslator(isL7ILB, l.namer) - env := &translator.Env{FrontendConfig: feConfig, Ing: &l.ingress} + t := translator.NewTranslator(isL7ILB, l7.namer) + env := &translator.Env{FrontendConfig: feConfig, Ing: &l7.ingress} - name, namerSupported := l.namer.RedirectUrlMap() - expectedMap := t.ToRedirectUrlMap(env, l.Versions().UrlMap) + name, namerSupported := l7.namer.RedirectUrlMap() + expectedMap := t.ToRedirectUrlMap(env, l7.Versions().UrlMap) // Cannot enable for internal ingress if expectedMap != nil && isL7ILB { @@ -106,7 +106,7 @@ func (l *L7) ensureRedirectURLMap() error { return nil } - key, err := l.CreateKey(name) + key, err := l7.CreateKey(name) if err != nil { return err } @@ -114,37 +114,37 @@ func (l *L7) ensureRedirectURLMap() error { // Do not expect to have a RedirectUrlMap if expectedMap == nil { // Check if we need to GC - status, ok := l.ingress.Annotations[annotations.RedirectUrlMapKey] + status, ok := l7.ingress.Annotations[annotations.RedirectUrlMapKey] if !ok || status == "" { return nil } else { - if err := composite.DeleteUrlMap(l.cloud, key, l.Versions().UrlMap); err != nil { + if err := composite.DeleteUrlMap(l7.cloud, key, l7.Versions().UrlMap); err != nil { // Do not block LB sync if this fails klog.Errorf("DeleteUrlMap(%s) = %v", key, err) // Signal to the rest of the controller that the UrlMap still exists - l.redirectUm = &composite.UrlMap{Name: key.Name} + l7.redirectUm = &composite.UrlMap{Name: key.Name} } } return nil } - currentMap, err := composite.GetUrlMap(l.cloud, key, l.Versions().UrlMap) + currentMap, err := composite.GetUrlMap(l7.cloud, key, l7.Versions().UrlMap) if utils.IgnoreHTTPNotFound(err) != nil { return err } if currentMap == nil { - if err := composite.CreateUrlMap(l.cloud, key, expectedMap); err != nil { + if err := composite.CreateUrlMap(l7.cloud, key, expectedMap); err != nil { return err } } else if compareRedirectUrlMaps(expectedMap, currentMap) { expectedMap.Fingerprint = currentMap.Fingerprint - if err := composite.UpdateUrlMap(l.cloud, key, expectedMap); err != nil { + if err := composite.UpdateUrlMap(l7.cloud, key, expectedMap); err != nil { return err } } - l.redirectUm = expectedMap + l7.redirectUm = expectedMap return nil } diff --git a/pkg/neg/syncers/transaction.go b/pkg/neg/syncers/transaction.go index 6267b22a4e..9fb05fd554 100644 --- a/pkg/neg/syncers/transaction.go +++ b/pkg/neg/syncers/transaction.go @@ -112,9 +112,9 @@ func NewTransactionSyncer( svcNegClient svcnegclient.Interface, customName bool, enableEndpointSlices bool, - l klog.Logger) negtypes.NegSyncer { + log klog.Logger) negtypes.NegSyncer { - logger := l.WithName("Syncer").WithValues("service", klog.KRef(negSyncerKey.Namespace, negSyncerKey.Name), "negName", negSyncerKey.NegName) + logger := log.WithName("Syncer").WithValues("service", klog.KRef(negSyncerKey.Namespace, negSyncerKey.Name), "negName", negSyncerKey.NegName) // TransactionSyncer implements the syncer core ts := &transactionSyncer{ diff --git a/pkg/ratelimit/ratelimit.go b/pkg/ratelimit/ratelimit.go index 5c3fc87158..ec37d81d21 100644 --- a/pkg/ratelimit/ratelimit.go +++ b/pkg/ratelimit/ratelimit.go @@ -74,10 +74,10 @@ func NewGCERateLimiter(specs []string, operationPollInterval time.Duration) (*GC } // Accept looks up the associated flowcontrol.RateLimiter (if exists) and waits on it. -func (l *GCERateLimiter) Accept(ctx context.Context, key *cloud.RateLimitKey) error { +func (grl *GCERateLimiter) Accept(ctx context.Context, key *cloud.RateLimitKey) error { var rl cloud.RateLimiter - impl := l.rateLimitImpl(key) + impl := grl.rateLimitImpl(key) if impl != nil { // Wrap the flowcontrol.RateLimiter with a AcceptRateLimiter and handle context. rl = &cloud.AcceptRateLimiter{Acceptor: impl} @@ -95,7 +95,7 @@ func (l *GCERateLimiter) Accept(ctx context.Context, key *cloud.RateLimitKey) er // Wait a minimum amount of time regardless of rate limiter. rl = &cloud.MinimumRateLimiter{ RateLimiter: rl, - Minimum: l.operationPollInterval, + Minimum: grl.operationPollInterval, } } @@ -104,7 +104,7 @@ func (l *GCERateLimiter) Accept(ctx context.Context, key *cloud.RateLimitKey) er // rateLimitImpl returns the flowcontrol.RateLimiter implementation // associated with the passed in key. -func (l *GCERateLimiter) rateLimitImpl(key *cloud.RateLimitKey) flowcontrol.RateLimiter { +func (grl *GCERateLimiter) rateLimitImpl(key *cloud.RateLimitKey) flowcontrol.RateLimiter { // Since the passed in key will have the ProjectID field filled in, we need to // create a copy which does not, so that retreiving the rate limiter implementation // through the map works as expected. @@ -114,7 +114,7 @@ func (l *GCERateLimiter) rateLimitImpl(key *cloud.RateLimitKey) flowcontrol.Rate Version: key.Version, Service: key.Service, } - return l.rateLimitImpls[keyCopy] + return grl.rateLimitImpls[keyCopy] } // Expected format of param is [version].[service].[operation] From 0d860ec2cb94646e33b0123f1b8495ad707addd1 Mon Sep 17 00:00:00 2001 From: Cezary Zawadka Date: Fri, 26 Aug 2022 11:09:42 +0200 Subject: [PATCH 2/2] Cleanup code: - get rid of one letter variables named 'L/l/I/|/!/1' - removed unused parameters from assert func in l4 tests - use better field names no business logic or test logic has been changed in this commit --- pkg/loadbalancers/l4_test.go | 2 +- pkg/loadbalancers/l4netlb.go | 8 ++++---- pkg/loadbalancers/l4netlb_test.go | 12 ++++++------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/loadbalancers/l4_test.go b/pkg/loadbalancers/l4_test.go index 2526ce9f7e..159f0e47d4 100644 --- a/pkg/loadbalancers/l4_test.go +++ b/pkg/loadbalancers/l4_test.go @@ -660,7 +660,7 @@ func TestHealthCheckFirewallDeletionWithNetLB(t *testing.T) { netlbSvc := test.NewL4NetLBRBSService(8080) l4NetLB := NewL4NetLB(netlbSvc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) // make sure both ilb and netlb use the same l4 healtcheck instance - l4NetLB.l4HealthChecks = l4.healthChecks + l4NetLB.healthChecks = l4.healthChecks // create netlb resources xlbResult := l4NetLB.EnsureFrontend(nodeNames, netlbSvc) diff --git a/pkg/loadbalancers/l4netlb.go b/pkg/loadbalancers/l4netlb.go index 8e108b7184..cf5aa88a9d 100644 --- a/pkg/loadbalancers/l4netlb.go +++ b/pkg/loadbalancers/l4netlb.go @@ -50,7 +50,7 @@ type L4NetLB struct { Service *corev1.Service ServicePort utils.ServicePort NamespacedName types.NamespacedName - l4HealthChecks healthchecks.L4HealthChecks + healthChecks healthchecks.L4HealthChecks forwardingRules ForwardingRulesProvider } @@ -91,7 +91,7 @@ func NewL4NetLB(service *corev1.Service, cloud *gce.Cloud, scope meta.KeyType, n Service: service, NamespacedName: types.NamespacedName{Name: service.Name, Namespace: service.Namespace}, backendPool: backends.NewPool(cloud, namer), - l4HealthChecks: healthchecks.L4(), + healthChecks: healthchecks.L4(), forwardingRules: forwardingrules.New(cloud, meta.VersionGA, scope), } portId := utils.ServicePortID{Service: l4netlb.NamespacedName} @@ -124,7 +124,7 @@ func (l4netlb *L4NetLB) EnsureFrontend(nodeNames []string, svc *corev1.Service) l4netlb.Service = svc sharedHC := !helpers.RequestsOnlyLocalTraffic(svc) - hcResult := l4netlb.l4HealthChecks.EnsureL4HealthCheck(l4netlb.Service, l4netlb.namer, sharedHC, l4netlb.scope, utils.XLB, nodeNames) + hcResult := l4netlb.healthChecks.EnsureL4HealthCheck(l4netlb.Service, l4netlb.namer, sharedHC, l4netlb.scope, utils.XLB, nodeNames) if hcResult.Err != nil { result.GCEResourceInError = hcResult.GceResourceInError @@ -224,7 +224,7 @@ func (l4netlb *L4NetLB) EnsureLoadBalancerDeleted(svc *corev1.Service) *L4NetLBS // 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 := l4netlb.l4HealthChecks.DeleteHealthCheck(svc, l4netlb.namer, isShared, meta.Regional, utils.XLB) + resourceInError, err := l4netlb.healthChecks.DeleteHealthCheck(svc, l4netlb.namer, isShared, meta.Regional, utils.XLB) if err != nil { result.GCEResourceInError = resourceInError result.Error = err diff --git a/pkg/loadbalancers/l4netlb_test.go b/pkg/loadbalancers/l4netlb_test.go index 772325bd46..29c048cff7 100644 --- a/pkg/loadbalancers/l4netlb_test.go +++ b/pkg/loadbalancers/l4netlb_test.go @@ -57,7 +57,7 @@ func TestEnsureL4NetLoadBalancer(t *testing.T) { namer := namer_util.NewL4Namer(kubeSystemUID, namer_util.NewNamer(vals.ClusterName, "cluster-fw")) l4netlb := NewL4NetLB(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) - l4netlb.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4netlb.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4netlb.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -108,7 +108,7 @@ func TestDeleteL4NetLoadBalancer(t *testing.T) { namer := namer_util.NewL4Namer(kubeSystemUID, namer_util.NewNamer(vals.ClusterName, "cluster-fw")) l4NetLB := NewL4NetLB(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) - l4NetLB.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4NetLB.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4NetLB.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -169,7 +169,7 @@ func TestHealthCheckFirewallDeletionWithILB(t *testing.T) { l4NetLB := NewL4NetLB(netlbSvc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) // make sure both ilb and netlb use the same l4 healtcheck instance - l4NetLB.l4HealthChecks = l4ilb.healthChecks + l4NetLB.healthChecks = l4ilb.healthChecks // create netlb resources result := l4NetLB.EnsureFrontend(nodeNames, netlbSvc) @@ -211,7 +211,7 @@ func ensureLoadBalancer(port int, vals gce.TestClusterValues, fakeGCE *gce.Cloud emptyNodes := []string{} l4NetLB := NewL4NetLB(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) - l4NetLB.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4NetLB.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) result := l4NetLB.EnsureFrontend(emptyNodes, svc) if result.Error != nil { @@ -354,7 +354,7 @@ func TestMetricsForStandardNetworkTier(t *testing.T) { namer := namer_util.NewL4Namer(kubeSystemUID, namer_util.NewNamer(vals.ClusterName, "cluster-fw")) l4netlb := NewL4NetLB(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) - l4netlb.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4netlb.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4netlb.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -401,7 +401,7 @@ func TestEnsureNetLBFirewallDestinations(t *testing.T) { svc := test.NewL4NetLBRBSService(8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) l4netlb := NewL4NetLB(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) - l4netlb.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4netlb.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4netlb.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err)