Skip to content

Migrating existing front-end namer logic to Legacy #860

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

Merged
k8s-ci-robot merged 1 commit into from
Oct 21, 2019
Merged

Migrating existing front-end namer logic to Legacy #860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 12 additions & 19 deletions pkg/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
"k8s.io/ingress-gce/pkg/tls"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/ingress-gce/pkg/utils/common"
"k8s.io/ingress-gce/pkg/utils/namer"
"k8s.io/klog"
)

Expand Down Expand Up @@ -114,7 +115,7 @@ func NewLoadBalancerController(
hasSynced: ctx.HasSynced,
nodes: NewNodeController(ctx, instancePool),
instancePool: instancePool,
l7Pool: loadbalancers.NewLoadBalancerPool(ctx.Cloud, ctx.ClusterNamer, ctx),
l7Pool: loadbalancers.NewLoadBalancerPool(ctx.Cloud, ctx.ClusterNamer, ctx, namer.NewFrontendNamerFactory(ctx.ClusterNamer)),
backendSyncer: backends.NewBackendSyncer(backendPool, healthChecker, ctx.Cloud),
negLinker: backends.NewNEGLinker(backendPool, negtypes.NewAdapter(ctx.Cloud), ctx.Cloud),
igLinker: backends.NewInstanceGroupLinker(instancePool, backendPool),
Expand All @@ -128,12 +129,12 @@ func NewLoadBalancerController(
AddFunc: func(obj interface{}) {
addIng := obj.(*v1beta1.Ingress)
if !utils.IsGLBCIngress(addIng) {
klog.V(4).Infof("Ignoring add for ingress %v based on annotation %v", common.ToString(addIng), annotations.IngressClassKey)
klog.V(4).Infof("Ignoring add for ingress %v based on annotation %v", common.NamespacedName(addIng), annotations.IngressClassKey)
return
}

klog.V(3).Infof("Ingress %v added, enqueuing", common.ToString(addIng))
lbc.ctx.Recorder(addIng.Namespace).Eventf(addIng, apiv1.EventTypeNormal, "ADD", common.ToString(addIng))
klog.V(3).Infof("Ingress %v added, enqueuing", common.NamespacedName(addIng))
lbc.ctx.Recorder(addIng.Namespace).Eventf(addIng, apiv1.EventTypeNormal, "ADD", common.NamespacedName(addIng))
lbc.ingQueue.Enqueue(obj)
},
DeleteFunc: func(obj interface{}) {
Expand All @@ -143,16 +144,16 @@ func NewLoadBalancerController(
return
}
if delIng.ObjectMeta.DeletionTimestamp != nil {
klog.V(2).Infof("Ignoring delete event for Ingress %v, deletion will be handled via the finalizer", common.ToString(delIng))
klog.V(2).Infof("Ignoring delete event for Ingress %v, deletion will be handled via the finalizer", common.NamespacedName(delIng))
return
}

if !utils.IsGLBCIngress(delIng) {
klog.V(4).Infof("Ignoring delete for ingress %v based on annotation %v", common.ToString(delIng), annotations.IngressClassKey)
klog.V(4).Infof("Ignoring delete for ingress %v based on annotation %v", common.NamespacedName(delIng), annotations.IngressClassKey)
return
}

klog.V(3).Infof("Ingress %v deleted, enqueueing", common.ToString(delIng))
klog.V(3).Infof("Ingress %v deleted, enqueueing", common.NamespacedName(delIng))
lbc.ingQueue.Enqueue(obj)
},
UpdateFunc: func(old, cur interface{}) {
Expand All @@ -162,16 +163,16 @@ func NewLoadBalancerController(
// If ingress was GLBC Ingress, we need to track ingress class change
// and run GC to delete LB resources.
if utils.IsGLBCIngress(oldIng) {
klog.V(4).Infof("Ingress %v class was changed, enqueuing", common.ToString(curIng))
klog.V(4).Infof("Ingress %v class was changed, enqueuing", common.NamespacedName(curIng))
lbc.ingQueue.Enqueue(cur)
return
}
return
}
if reflect.DeepEqual(old, cur) {
klog.V(3).Infof("Periodic enqueueing of %v", common.ToString(curIng))
klog.V(3).Infof("Periodic enqueueing of %v", common.NamespacedName(curIng))
} else {
klog.V(3).Infof("Ingress %v changed, enqueuing", common.ToString(curIng))
klog.V(3).Infof("Ingress %v changed, enqueuing", common.NamespacedName(curIng))
}

lbc.ingQueue.Enqueue(cur)
Expand Down Expand Up @@ -553,15 +554,8 @@ func (lbc *LoadBalancerController) updateIngressStatus(l7 *loadbalancers.L7, ing

// toRuntimeInfo returns L7RuntimeInfo for the given ingress.
func (lbc *LoadBalancerController) toRuntimeInfo(ing *v1beta1.Ingress, urlMap *utils.GCEURLMap) (*loadbalancers.L7RuntimeInfo, error) {
k, err := common.KeyFunc(ing)
if err != nil {
return nil, fmt.Errorf("cannot get key for Ingress %v/%v: %v", ing.Namespace, ing.Name, err)
}

var tls []*loadbalancers.TLSCerts

annotations := annotations.FromIngress(ing)
tls, err = lbc.tlsLoader.Load(ing)
tls, err := lbc.tlsLoader.Load(ing)
if err != nil {
if apierrors.IsNotFound(err) {
// TODO: this path should be removed when external certificate managers migrate to a better solution.
Expand All @@ -585,7 +579,6 @@ func (lbc *LoadBalancerController) toRuntimeInfo(ing *v1beta1.Ingress, urlMap *u
}

return &loadbalancers.L7RuntimeInfo{
Name: k,
TLS: tls,
TLSName: annotations.UseNamedTLS(),
Ingress: ing,
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func newLoadBalancerController() *LoadBalancerController {
lbc := NewLoadBalancerController(ctx, stopCh)
// TODO(rramkumar): Fix this so we don't have to override with our fake
lbc.instancePool = instances.NewNodePool(instances.NewFakeInstanceGroups(sets.NewString(), namer), namer)
lbc.l7Pool = loadbalancers.NewLoadBalancerPool(fakeGCE, namer, events.RecorderProducerMock{})
lbc.l7Pool = loadbalancers.NewLoadBalancerPool(fakeGCE, namer, events.RecorderProducerMock{}, namer_util.NewFrontendNamerFactory(namer))
lbc.instancePool.Init(&instances.FakeZoneLister{Zones: []string{"zone-a"}})

lbc.hasSynced = func() bool { return true }
Expand Down
2 changes: 1 addition & 1 deletion pkg/loadbalancers/addresses.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (l *L7) checkStaticIP() (err error) {
klog.V(3).Infof("Not managing user specified static IP %v", address)
return nil
}
staticIPName := l.namer.ForwardingRule(l.Name, namer.HTTPProtocol)
staticIPName := l.namer.ForwardingRule(namer.HTTPProtocol)
ip, _ := l.cloud.GetGlobalAddress(staticIPName)
if ip == nil {
klog.V(3).Infof("Creating static ip %v", staticIPName)
Expand Down
18 changes: 9 additions & 9 deletions pkg/loadbalancers/certificates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (l *L7) createSslCertificates(existingCerts []*composite.SslCertificate) ([
for _, tlsCert := range l.runtimeInfo.TLS {
ingCert := tlsCert.Cert
ingKey := tlsCert.Key
gcpCertName := l.namer.SSLCertName(l.Name, tlsCert.CertHash)
gcpCertName := l.namer.SSLCertName(tlsCert.CertHash)

if addedBy, exists := visitedCertMap[gcpCertName]; exists {
klog.V(3).Infof("Secret %q has a certificate already used by %v", tlsCert.Name, addedBy)
Expand All @@ -98,7 +98,7 @@ func (l *L7) createSslCertificates(existingCerts []*composite.SslCertificate) ([
}
// 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", gcpCertName, l.Name)
klog.V(2).Infof("Creating new sslCertificate %q for LB %q", gcpCertName, l)
cert := &composite.SslCertificate{
Name: gcpCertName,
Certificate: ingCert,
Expand All @@ -112,7 +112,7 @@ func (l *L7) createSslCertificates(existingCerts []*composite.SslCertificate) ([
}
err = composite.CreateSslCertificate(l.cloud, key, cert)
if err != nil {
klog.Errorf("Failed to create new sslCertificate %q for %q - %v", gcpCertName, l.Name, err)
klog.Errorf("Failed to create new sslCertificate %q for %q - %v", gcpCertName, l, err)
failedCerts = append(failedCerts, gcpCertName+" Error:"+err.Error())
continue
}
Expand Down Expand Up @@ -157,7 +157,7 @@ func (l *L7) getSslCertificates(names []string) ([]*composite.SslCertificate, er
continue
}

klog.V(2).Infof("Using existing SslCertificate %v for %v", name, l.Name)
klog.V(2).Infof("Using existing SslCertificate %v for %v", name, l)
result = append(result, cert)
}
if len(failedCerts) != 0 {
Expand Down Expand Up @@ -213,8 +213,8 @@ func (l *L7) getIngressManagedSslCerts() ([]*composite.SslCertificate, error) {
return nil, err
}
for _, c := range certs {
if l.namer.IsCertUsedForLB(l.Name, c.Name) {
klog.V(4).Infof("Populating ssl cert %s for l7 %s", c.Name, l.Name)
if l.namer.IsCertNameForLB(c.Name) {
klog.V(4).Infof("Populating ssl cert %s for l7 %s", c.Name, l)
result = append(result, c)
}
}
Expand All @@ -234,7 +234,7 @@ func (l *L7) getIngressManagedSslCerts() ([]*composite.SslCertificate, error) {
continue
}

if !l.namer.IsLegacySSLCert(l.Name, name) {
if !l.namer.IsLegacySSLCert(name) {
continue
}
key, err := l.CreateKey(name)
Expand All @@ -243,7 +243,7 @@ func (l *L7) getIngressManagedSslCerts() ([]*composite.SslCertificate, error) {
}
cert, _ := composite.GetSslCertificate(l.cloud, key, version)
if cert != nil {
klog.V(4).Infof("Populating legacy ssl cert %s for l7 %s", cert.Name, l.Name)
klog.V(4).Infof("Populating legacy ssl cert %s for l7 %s", cert.Name, l)
result = append(result, cert)
}
}
Expand All @@ -257,7 +257,7 @@ func (l *L7) deleteOldSSLCerts() {
}
certsMap := getMapfromCertList(l.sslCerts)
for _, cert := range l.oldSSLCerts {
if !l.namer.IsCertUsedForLB(l.Name, cert.Name) && !l.namer.IsLegacySSLCert(l.Name, cert.Name) {
if !l.namer.IsCertNameForLB(cert.Name) && !l.namer.IsLegacySSLCert(cert.Name) {
// retain cert if it is managed by GCE(non-ingress)
continue
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/loadbalancers/forwarding_rules.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (l *L7) checkHttpForwardingRule() (err error) {
if l.tp == nil {
return fmt.Errorf("cannot create forwarding rule without proxy")
}
name := l.namer.ForwardingRule(l.Name, namer.HTTPProtocol)
name := l.namer.ForwardingRule(namer.HTTPProtocol)
address, _ := l.getEffectiveIP()
fw, err := l.checkForwardingRule(name, l.tp.SelfLink, address, httpDefaultPortRange)
if err != nil {
Expand All @@ -46,10 +46,10 @@ func (l *L7) checkHttpForwardingRule() (err error) {

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.Name)
klog.V(3).Infof("No https target proxy for %v, not created https forwarding rule", l)
return nil
}
name := l.namer.ForwardingRule(l.Name, namer.HTTPSProtocol)
name := l.namer.ForwardingRule(namer.HTTPSProtocol)
address, _ := l.getEffectiveIP()
fws, err := l.checkForwardingRule(name, l.tps.SelfLink, address, httpsDefaultPortRange)
if err != nil {
Expand Down
33 changes: 16 additions & 17 deletions pkg/loadbalancers/l7.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ const (

// L7RuntimeInfo is info passed to this module from the controller runtime.
type L7RuntimeInfo struct {
// Name is the name of a loadbalancer.
Name string
// IP is the desired ip of the loadbalancer, eg from a staticIP.
IP string
// TLS are the tls certs to use in termination.
Expand Down Expand Up @@ -85,14 +83,8 @@ type TLSCerts struct {
CertHash string
}

// String returns the load balancer name
func (l *L7RuntimeInfo) String() string {
return l.Name
}

// L7 represents a single L7 loadbalancer.
type L7 struct {
Name string
// runtimeInfo is non-cloudprovider information passed from the controller.
runtimeInfo *L7RuntimeInfo
// ingress stores the ingress
Expand All @@ -119,13 +111,20 @@ type L7 struct {
// prevents leakage if there's a failure along the way.
oldSSLCerts []*composite.SslCertificate
// namer is used to compute names of the various sub-components of an L7.
namer *namer.Namer
namer namer.IngressFrontendNamer
// recorder is used to generate k8s Events.
recorder record.EventRecorder
// resource type stores the KeyType of the resources in the loadbalancer (e.g. Regional)
scope meta.KeyType
}

// 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.LbName()
}

// Version() returns the struct listing the versions for every resource
func (l *L7) Versions() *features.ResourceVersions {
return features.VersionsFromIngress(&l.ingress)
Expand Down Expand Up @@ -168,14 +167,14 @@ func (l *L7) edgeHop() error {
// Defer promoting an ephemeral to a static IP until it's really needed.
sslConfigured := l.runtimeInfo.TLS != nil || l.runtimeInfo.TLSName != ""
if l.runtimeInfo.AllowHTTP && sslConfigured {
klog.V(3).Infof("checking static ip for %v", l.Name)
klog.V(3).Infof("checking static ip for %v", l)
if err := l.checkStaticIP(); err != nil {
return err
}
}
if sslConfigured {
willConfigureFrontend = true
klog.V(3).Infof("validating https for %v", l.Name)
klog.V(3).Infof("validating https for %v", l)
if err := l.edgeHopHttps(); err != nil {
return err
}
Expand Down Expand Up @@ -228,7 +227,7 @@ func (l *L7) Cleanup(versions *features.ResourceVersions) error {
var key *meta.Key
var err error

fwName := l.namer.ForwardingRule(l.Name, namer.HTTPProtocol)
fwName := l.namer.ForwardingRule(namer.HTTPProtocol)
klog.V(2).Infof("Deleting global forwarding rule %v", fwName)
if key, err = l.CreateKey(fwName); err != nil {
return err
Expand All @@ -237,7 +236,7 @@ func (l *L7) Cleanup(versions *features.ResourceVersions) error {
return err
}

fwsName := l.namer.ForwardingRule(l.Name, namer.HTTPSProtocol)
fwsName := l.namer.ForwardingRule(namer.HTTPSProtocol)
klog.V(2).Infof("Deleting global forwarding rule %v", fwsName)
if key, err = l.CreateKey(fwsName); err != nil {
return err
Expand All @@ -254,7 +253,7 @@ func (l *L7) Cleanup(versions *features.ResourceVersions) error {
}
}

tpName := l.namer.TargetProxy(l.Name, namer.HTTPProtocol)
tpName := l.namer.TargetProxy(namer.HTTPProtocol)
klog.V(2).Infof("Deleting target http proxy %v", tpName)
if key, err = l.CreateKey(tpName); err != nil {
return err
Expand All @@ -263,7 +262,7 @@ func (l *L7) Cleanup(versions *features.ResourceVersions) error {
return err
}

tpsName := l.namer.TargetProxy(l.Name, namer.HTTPSProtocol)
tpsName := l.namer.TargetProxy(namer.HTTPSProtocol)
klog.V(2).Infof("Deleting target https proxy %v", tpsName)
if key, err = l.CreateKey(tpsName); err != nil {
return err
Expand Down Expand Up @@ -296,7 +295,7 @@ func (l *L7) Cleanup(versions *features.ResourceVersions) error {
}
}

umName := l.namer.UrlMap(l.Name)
umName := l.namer.UrlMap()
klog.V(2).Infof("Deleting URL Map %v", umName)
if key, err = l.CreateKey(umName); err != nil {
return err
Expand Down Expand Up @@ -376,7 +375,7 @@ 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.Name)
return "", fmt.Errorf("missing Ingress object to construct description for %s", l)
}

namespace := l.runtimeInfo.Ingress.ObjectMeta.Namespace
Expand Down
Loading