Skip to content

Commit

Permalink
Use type LoadBalancer instead of string
Browse files Browse the repository at this point in the history
  • Loading branch information
skmatti committed Dec 9, 2019
1 parent e3fe2c5 commit 9456e92
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 100 deletions.
4 changes: 2 additions & 2 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -726,8 +726,8 @@ func TestIngressTagging(t *testing.T) {
addIngress(lbc, ing)
// Create URL map if enabled.
if tc.urlMapExists {
lbName := lbc.ctx.ClusterNamer.LoadBalancer(common.IngressKeyFunc(ing))
lbc.ctx.Cloud.CreateURLMap(&compute.UrlMap{Name: lbc.ctx.ClusterNamer.UrlMap(lbName)})
loadBalancer := lbc.ctx.ClusterNamer.LoadBalancer(common.IngressKeyFunc(ing))
lbc.ctx.Cloud.CreateURLMap(&compute.UrlMap{Name: lbc.ctx.ClusterNamer.UrlMap(loadBalancer)})
}

ingStoreKey := getKey(ing, t)
Expand Down
2 changes: 1 addition & 1 deletion pkg/loadbalancers/l7.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ type L7 struct {
// 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()
return string(l.namer.LoadBalancer())
}

// Versions returns the struct listing the versions for every resource
Expand Down
14 changes: 6 additions & 8 deletions pkg/loadbalancers/l7s.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"k8s.io/api/networking/v1beta1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/ingress-gce/pkg/common/operator"
"k8s.io/ingress-gce/pkg/composite"
"k8s.io/ingress-gce/pkg/events"
Expand Down Expand Up @@ -124,9 +123,9 @@ func (l *L7s) GCv2(ing *v1beta1.Ingress) error {
func (l *L7s) GCv1(names []string) error {
klog.V(2).Infof("GCv1(%v)", names)

knownLoadBalancers := sets.NewString()
knownLoadBalancers := make(map[namer_util.LoadBalancerName]bool)
for _, n := range names {
knownLoadBalancers.Insert(l.v1NamerHelper.LoadBalancer(n))
knownLoadBalancers[l.v1NamerHelper.LoadBalancer(n)] = true
}

// GC L7-ILB LBs if enabled
Expand Down Expand Up @@ -160,15 +159,14 @@ 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 sets.String, versions *features.ResourceVersions) []error {
func (l *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 {
nameParts := l.v1NamerHelper.ParseName(um.Name)
l7Name := l.v1NamerHelper.LoadBalancerFromLbName(nameParts.LbName)
l7Name := l.v1NamerHelper.LoadBalancerForURLMap(um.Name)

if knownLoadBalancers.Has(l7Name) {
if knownLoadBalancers[l7Name] {
klog.V(3).Infof("Load balancer %v is still valid, not GC'ing", l7Name)
continue
}
Expand All @@ -179,7 +177,7 @@ func (l *L7s) gc(urlMaps []*composite.UrlMap, knownLoadBalancers sets.String, ve
continue
}

if err := l.delete(l.namerFactory.NamerForLbName(l7Name), versions, scope); err != nil {
if err := l.delete(l.namerFactory.NamerForLoadBalancer(l7Name), versions, scope); err != nil {
errors = append(errors, fmt.Errorf("error deleting loadbalancer %q", l7Name))
}
}
Expand Down
14 changes: 7 additions & 7 deletions pkg/loadbalancers/l7s_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,13 @@ func TestGC(t *testing.T) {
versions := features.GAResourceVersions

for _, key := range otherKeys {
namer := otherFeNamerFactory.NamerForLbName(otherNamer.LoadBalancer(key))
namer := otherFeNamerFactory.NamerForLoadBalancer(otherNamer.LoadBalancer(key))
createFakeLoadbalancer(cloud, namer, versions, defaultScope)
}

for _, tc := range testCases {
for _, key := range tc.gcpLBs {
namer := namerFactory.NamerForLbName(v1NamerHelper.LoadBalancer(key))
namer := namerFactory.NamerForLoadBalancer(v1NamerHelper.LoadBalancer(key))
createFakeLoadbalancer(cloud, namer, versions, defaultScope)
}

Expand All @@ -196,7 +196,7 @@ func TestGC(t *testing.T) {

// check if other LB are not deleted
for _, key := range otherKeys {
namer := otherFeNamerFactory.NamerForLbName(otherNamer.LoadBalancer(key))
namer := otherFeNamerFactory.NamerForLoadBalancer(otherNamer.LoadBalancer(key))
if err := checkFakeLoadBalancer(cloud, namer, versions, defaultScope, true); err != nil {
t.Errorf("For case %q and key %q, do not expect err: %v", tc.desc, key, err)
}
Expand All @@ -211,15 +211,15 @@ func TestGC(t *testing.T) {
// check if the ones that are expected to be GC is actually GCed.
expectRemovedLBs := sets.NewString(tc.gcpLBs...).Difference(sets.NewString(tc.expectLBs...)).Difference(sets.NewString(tc.ingressLBs...))
for _, key := range expectRemovedLBs.List() {
namer := namerFactory.NamerForLbName(v1NamerHelper.LoadBalancer(key))
namer := namerFactory.NamerForLoadBalancer(v1NamerHelper.LoadBalancer(key))
if err := checkFakeLoadBalancer(cloud, namer, versions, defaultScope, false); err != nil {
t.Errorf("For case %q and key %q, do not expect err: %v", tc.desc, key, err)
}
}

// check if all expected LBs exists
for _, key := range tc.expectLBs {
namer := namerFactory.NamerForLbName(v1NamerHelper.LoadBalancer(key))
namer := namerFactory.NamerForLoadBalancer(v1NamerHelper.LoadBalancer(key))
if err := checkFakeLoadBalancer(cloud, namer, versions, defaultScope, true); err != nil {
t.Errorf("For case %q and key %q, do not expect err: %v", tc.desc, key, err)
}
Expand Down Expand Up @@ -249,7 +249,7 @@ func TestDoNotGCWantedLB(t *testing.T) {
versions := features.GAResourceVersions

for _, tc := range testCases {
namer := l7sPool.namerFactory.NamerForLbName(l7sPool.v1NamerHelper.LoadBalancer(tc.key))
namer := l7sPool.namerFactory.NamerForLoadBalancer(l7sPool.v1NamerHelper.LoadBalancer(tc.key))
createFakeLoadbalancer(l7sPool.cloud, namer, versions, defaultScope)
err := l7sPool.GCv1([]string{tc.key})
if err != nil {
Expand Down Expand Up @@ -285,7 +285,7 @@ func TestGCToLeakLB(t *testing.T) {
versions := features.GAResourceVersions

for _, tc := range testCases {
namer := l7sPool.namerFactory.NamerForLbName(l7sPool.v1NamerHelper.LoadBalancer(tc.key))
namer := l7sPool.namerFactory.NamerForLoadBalancer(l7sPool.v1NamerHelper.LoadBalancer(tc.key))
createFakeLoadbalancer(l7sPool.cloud, namer, versions, defaultScope)
err := l7sPool.GCv1([]string{})
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/loadbalancers/loadbalancers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func TestCertUpdate(t *testing.T) {
}

// Verify certs
t.Logf("lbName=%q, name=%q", feNamer.LbName(), certName1)
t.Logf("lbName=%q, name=%q", feNamer.LoadBalancer(), certName1)
expectCerts := map[string]string{certName1: lbInfo.TLS[0].Cert}
verifyCertAndProxyLink(expectCerts, expectCerts, j, t)

Expand Down Expand Up @@ -508,7 +508,7 @@ func TestUpgradeToNewCertNames(t *testing.T) {
UrlMap: gceUrlMap,
Ingress: ing,
}
oldCertName := "k8s-ssl-" + feNamer.LbName()
oldCertName := fmt.Sprintf("k8s-ssl-%s", feNamer.LoadBalancer())
tlsCert := createCert("key", "cert", "name")
lbInfo.TLS = []*TLSCerts{tlsCert}
newCertName := feNamer.SSLCertName(tlsCert.CertHash)
Expand Down Expand Up @@ -1123,7 +1123,7 @@ func TestClusterNameChange(t *testing.T) {

// Now the components should get renamed with the next suffix.
l7, err = j.pool.Ensure(lbInfo)
if err != nil || j.namer.ParseName(l7.namer.LbName()).ClusterName != newName {
if err != nil || j.namer.ParseName(string(l7.namer.LoadBalancer())).ClusterName != newName {
t.Fatalf("Expected L7 name to change.")
}
verifyHTTPSForwardingRuleAndProxyLinks(t, j, l7)
Expand Down
2 changes: 1 addition & 1 deletion pkg/loadbalancers/url_maps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestToComputeURLMap(t *testing.T) {
}

namerFactory := namer_util.NewFrontendNamerFactory(namer, "")
feNamer := namerFactory.NamerForLbName("ns/lb-name")
feNamer := namerFactory.NamerForLoadBalancer("ns/lb-name")
gotComputeURLMap := toCompositeURLMap(gceURLMap, feNamer, meta.GlobalKey("ns-lb-name"))
if !mapsEqual(gotComputeURLMap, wantComputeMap) {
t.Errorf("toComputeURLMap() = \n%+v\n want\n%+v", gotComputeURLMap, wantComputeMap)
Expand Down
28 changes: 14 additions & 14 deletions pkg/utils/namer/frontendnamer.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type Scheme string
type V1IngressFrontendNamer struct {
ing *v1beta1.Ingress
namer *Namer
lbName string
lbName LoadBalancerName
}

// newV1IngressFrontendNamer returns v1 frontend namer for given ingress.
Expand All @@ -68,8 +68,8 @@ func newV1IngressFrontendNamer(ing *v1beta1.Ingress, namer *Namer) IngressFronte
return &V1IngressFrontendNamer{ing: ing, namer: namer, lbName: lbName}
}

// newV1IngressFrontendNamerFromLBName returns v1 frontend namer for load balancer.
func newV1IngressFrontendNamerFromLBName(lbName string, namer *Namer) IngressFrontendNamer {
// newV1IngressFrontendNamerForLoadBalancer returns v1 frontend namer for load balancer.
func newV1IngressFrontendNamerForLoadBalancer(lbName LoadBalancerName, namer *Namer) IngressFrontendNamer {
return &V1IngressFrontendNamer{namer: namer, lbName: lbName}
}

Expand Down Expand Up @@ -103,8 +103,8 @@ func (ln *V1IngressFrontendNamer) IsLegacySSLCert(certName string) bool {
return ln.namer.IsLegacySSLCert(ln.lbName, certName)
}

// LbName implements IngressFrontendNamer.
func (ln *V1IngressFrontendNamer) LbName() string {
// LoadBalancer implements IngressFrontendNamer.
func (ln *V1IngressFrontendNamer) LoadBalancer() LoadBalancerName {
return ln.lbName
}

Expand All @@ -114,7 +114,7 @@ type V2IngressFrontendNamer struct {
// prefix for all resource names (ex.: "k8s").
prefix string
// Load balancer name to be included in resource name.
lbName string
lbName LoadBalancerName
// clusterUID is an 8 character hash to be included in resource names.
// This is immutable after the cluster is created. Kube-system uid which is
// immutable is used as cluster UID for v2 naming scheme.
Expand All @@ -135,12 +135,12 @@ type V2IngressFrontendNamer struct {
func newV2IngressFrontendNamer(ing *v1beta1.Ingress, kubeSystemUID string, prefix string) IngressFrontendNamer {
clusterUID := common.ContentHash(kubeSystemUID, clusterUIDLength)
namer := &V2IngressFrontendNamer{ing: ing, prefix: prefix, clusterUID: clusterUID}
// Initialize LbName.
// Initialize lbName.
truncFields := TrimFieldsEvenly(maximumAllowedCombinedLength, ing.Namespace, ing.Name)
truncNamespace := truncFields[0]
truncName := truncFields[1]
suffix := namer.suffix(kubeSystemUID, ing.Namespace, ing.Name)
namer.lbName = fmt.Sprintf("%s-%s-%s-%s", clusterUID, truncNamespace, truncName, suffix)
namer.lbName = LoadBalancerName(fmt.Sprintf("%s-%s-%s-%s", clusterUID, truncNamespace, truncName, suffix))
return namer
}

Expand Down Expand Up @@ -192,9 +192,9 @@ func (vn *V2IngressFrontendNamer) IsLegacySSLCert(certName string) bool {
return false
}

// LbName returns loadbalancer name.
// LoadBalancer returns loadbalancer name.
// Note that this is used for generating GCE resource names.
func (vn *V2IngressFrontendNamer) LbName() string {
func (vn *V2IngressFrontendNamer) LoadBalancer() LoadBalancerName {
return vn.lbName
}

Expand All @@ -207,7 +207,7 @@ func (vn *V2IngressFrontendNamer) suffix(uid, namespace, name string) string {

// lbNameToHash returns hash string of length 16 of lbName.
func (vn *V2IngressFrontendNamer) lbNameToHash() string {
return common.ContentHash(vn.lbName, 16)
return common.ContentHash(vn.lbName.String(), 16)
}

// FrontendNamerFactory implements IngressFrontendNamerFactory.
Expand Down Expand Up @@ -237,7 +237,7 @@ func (rn *FrontendNamerFactory) Namer(ing *v1beta1.Ingress) IngressFrontendNamer
}
}

// NamerForLbName implements IngressFrontendNamerFactory.
func (rn *FrontendNamerFactory) NamerForLbName(lbName string) IngressFrontendNamer {
return newV1IngressFrontendNamerFromLBName(lbName, rn.namer)
// NamerForLoadBalancer implements IngressFrontendNamerFactory.
func (rn *FrontendNamerFactory) NamerForLoadBalancer(lbName LoadBalancerName) IngressFrontendNamer {
return newV1IngressFrontendNamerForLoadBalancer(lbName, rn.namer)
}
Loading

0 comments on commit 9456e92

Please sign in to comment.