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.

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

Use type LoadBalancer instead of string #966

Merged
merged 1 commit into from
Dec 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 l.namer.LoadBalancer().String()
}

// 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(l7.namer.LoadBalancer().String()).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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: 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 {
skmatti marked this conversation as resolved.
Show resolved Hide resolved
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