Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup code #1791

Merged
merged 2 commits into from
Aug 26, 2022
Merged

Cleanup code #1791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28095d8
Cleanup code:
cezarygerard Aug 25, 2022
0d860ec
Cleanup code:
cezarygerard Aug 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pkg/backends/ig_linker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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())
Expand Down
12 changes: 6 additions & 6 deletions pkg/backends/neg_linker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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 {
Expand Down
32 changes: 16 additions & 16 deletions pkg/loadbalancers/addresses.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,59 +30,59 @@ 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
}
if !manageStaticIP {
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"
Expand Down
74 changes: 37 additions & 37 deletions pkg/loadbalancers/certificates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,38 +28,38 @@ 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)
}
return nil
}

// 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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -140,33 +140,33 @@ 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
// to make sure we garbage collect any old certs that this instance might have lost track of due to crashes.
// 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)
Expand All @@ -179,30 +179,30 @@ 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)
}
}
}
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
}
Expand All @@ -211,17 +211,17 @@ 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)
}
}
}

// 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
Expand Down
Loading