Skip to content

Commit

Permalink
Refactor LB unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@spencerhance
spencerhance committed Aug 12, 2019
1 parent dc3147f commit 0c126fb
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 40 deletions.
4 changes: 2 additions & 2 deletions pkg/loadbalancers/features/features_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ var (
}

fakeFeatureToVersions = map[string]*ResourceVersions{
fakeGaFeature: NewResourceVersions(),
fakeGaFeature: GAResourceVersions,
fakeAlphaFeature: &fakeAlphaFeatureVersions,
fakeBetaFeature: &fakeBetaFeatureVersions,
fakeAlphaFeatureUrlMapOnly: &fakeAlphaFeatureUrlMapOnlyVersions,
Expand Down Expand Up @@ -180,7 +180,7 @@ func TestVersionsFromFeatures(t *testing.T) {
}{
{
desc: "No Features",
expected: NewResourceVersions(),
expected: GAResourceVersions,
},
{
desc: "Alpha features",
Expand Down
88 changes: 60 additions & 28 deletions pkg/loadbalancers/l7s_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ package loadbalancers

import (
"fmt"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"k8s.io/ingress-gce/pkg/composite"
"k8s.io/ingress-gce/pkg/loadbalancers/features"
"net/http"
"strings"
"testing"
Expand Down Expand Up @@ -166,13 +169,15 @@ func TestGC(t *testing.T) {
generateKey(longName[:20], longName[:20]),
}

versions := features.GAResourceVersions

for _, key := range otherKeys {
createFakeLoadbalancer(cloud, otherNamer, key)
createFakeLoadbalancer(cloud, otherNamer, key, versions, defaultScope)
}

for _, tc := range testCases {
for _, key := range tc.gcpLBs {
createFakeLoadbalancer(l7sPool.cloud, l7sPool.namer, key)
createFakeLoadbalancer(l7sPool.cloud, l7sPool.namer, key, versions, defaultScope)
}

err := l7sPool.GC(tc.ingressLBs)
Expand All @@ -182,7 +187,7 @@ func TestGC(t *testing.T) {

// check if other LB are not deleted
for _, key := range otherKeys {
if err := checkFakeLoadBalancer(l7sPool.cloud, otherNamer, key, true); err != nil {
if err := checkFakeLoadBalancer(l7sPool.cloud, otherNamer, key, versions, defaultScope, true); err != nil {
t.Errorf("For case %q and key %q, do not expect err: %v", tc.desc, key, err)
}
}
Expand All @@ -196,17 +201,17 @@ 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() {
if err := checkFakeLoadBalancer(l7sPool.cloud, l7sPool.namer, key, false); err != nil {
if err := checkFakeLoadBalancer(l7sPool.cloud, l7sPool.namer, key, 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 {
if err := checkFakeLoadBalancer(l7sPool.cloud, l7sPool.namer, key, true); err != nil {
if err := checkFakeLoadBalancer(l7sPool.cloud, l7sPool.namer, key, versions, defaultScope, true); err != nil {
t.Errorf("For case %q and key %q, do not expect err: %v", tc.desc, key, err)
}
removeFakeLoadBalancer(l7sPool.cloud, l7sPool.namer, key)
removeFakeLoadBalancer(l7sPool.cloud, l7sPool.namer, key, versions, defaultScope)
}
}
}
Expand All @@ -230,21 +235,23 @@ func TestDoNotGCWantedLB(t *testing.T) {
l7sPool.cloud.CreateURLMap(&compute.UrlMap{Name: "random--name1"})
l7sPool.cloud.CreateURLMap(&compute.UrlMap{Name: "k8s-random-random--1111111111111111"})

versions := features.GAResourceVersions

for _, tc := range testCases {
createFakeLoadbalancer(l7sPool.cloud, namer, tc.key)
createFakeLoadbalancer(l7sPool.cloud, namer, tc.key, versions, defaultScope)
err := l7sPool.GC([]string{tc.key})
if err != nil {
t.Errorf("For case %q, do not expect err: %v", tc.desc, err)
}

if err := checkFakeLoadBalancer(l7sPool.cloud, namer, tc.key, true); err != nil {
if err := checkFakeLoadBalancer(l7sPool.cloud, namer, tc.key, versions, defaultScope, true); err != nil {
t.Errorf("For case %q, do not expect err: %v", tc.desc, err)
}
urlMaps, _ := l7sPool.cloud.ListURLMaps()
if len(urlMaps) != 1+numOfExtraUrlMap {
t.Errorf("For case %q, expect %d urlmaps, but got %d.", tc.desc, 1+numOfExtraUrlMap, len(urlMaps))
}
removeFakeLoadBalancer(l7sPool.cloud, namer, tc.key)
removeFakeLoadBalancer(l7sPool.cloud, namer, tc.key, versions, defaultScope)
}
}

Expand All @@ -264,20 +271,22 @@ func TestGCToLeakLB(t *testing.T) {
testCases = append(testCases, testCase{fmt.Sprintf("Ingress Key is %d characters long.", i), generateKeyWithLength(i)})
}

versions := features.GAResourceVersions

for _, tc := range testCases {
createFakeLoadbalancer(l7sPool.cloud, namer, tc.key)
createFakeLoadbalancer(l7sPool.cloud, namer, tc.key, versions, defaultScope)
err := l7sPool.GC([]string{})
if err != nil {
t.Errorf("For case %q, do not expect err: %v", tc.desc, err)
}

if len(tc.key) >= resourceLeakLimit {
if err := checkFakeLoadBalancer(l7sPool.cloud, namer, tc.key, true); err != nil {
if err := checkFakeLoadBalancer(l7sPool.cloud, namer, tc.key, versions, defaultScope, true); err != nil {
t.Errorf("For case %q, do not expect err: %v", tc.desc, err)
}
removeFakeLoadBalancer(l7sPool.cloud, namer, tc.key)
removeFakeLoadBalancer(l7sPool.cloud, namer, tc.key, versions, defaultScope)
} else {
if err := checkFakeLoadBalancer(l7sPool.cloud, namer, tc.key, false); err != nil {
if err := checkFakeLoadBalancer(l7sPool.cloud, namer, tc.key, versions, defaultScope, false); err != nil {
t.Errorf("For case %q, do not expect err: %v", tc.desc, err)
}
}
Expand All @@ -291,40 +300,63 @@ func newTestLoadBalancerPool() LoadBalancerPool {
return NewLoadBalancerPool(fakeGCECloud, namer, ctx)
}

func createFakeLoadbalancer(cloud *gce.Cloud, namer *utils.Namer, key string) {
lbName := namer.LoadBalancer(key)
cloud.CreateGlobalForwardingRule(&compute.ForwardingRule{Name: namer.ForwardingRule(lbName, utils.HTTPProtocol)})
cloud.CreateTargetHTTPProxy(&compute.TargetHttpProxy{Name: namer.TargetProxy(lbName, utils.HTTPProtocol)})
cloud.CreateURLMap(&compute.UrlMap{Name: namer.UrlMap(lbName)})
func createFakeLoadbalancer(cloud *gce.Cloud, namer *utils.Namer, lbKey string, versions *features.ResourceVersions, scope meta.KeyType) {
lbName := namer.LoadBalancer(lbKey)
key, _ := composite.CreateKey(cloud, "", scope)

key.Name = namer.ForwardingRule(lbName, utils.HTTPProtocol)
composite.CreateForwardingRule(cloud, key, &composite.ForwardingRule{Name: key.Name, Version: versions.ForwardingRule})

key.Name = namer.TargetProxy(lbName, utils.HTTPProtocol)
composite.CreateTargetHttpProxy(cloud, key, &composite.TargetHttpProxy{Name: key.Name, Version: versions.TargetHttpProxy})

key.Name = namer.UrlMap(lbName)
composite.CreateUrlMap(cloud, key, &composite.UrlMap{Name: key.Name, Version: versions.UrlMap})

cloud.ReserveGlobalAddress(&compute.Address{Name: namer.ForwardingRule(lbName, utils.HTTPProtocol)})

}

func removeFakeLoadBalancer(cloud *gce.Cloud, namer *utils.Namer, key string) {
lbName := namer.LoadBalancer(key)
cloud.DeleteGlobalForwardingRule(namer.ForwardingRule(lbName, utils.HTTPProtocol))
cloud.DeleteTargetHTTPProxy(namer.TargetProxy(lbName, utils.HTTPProtocol))
cloud.DeleteURLMap(namer.UrlMap(lbName))
func removeFakeLoadBalancer(cloud *gce.Cloud, namer *utils.Namer, lbKey string, versions *features.ResourceVersions, scope meta.KeyType) {
lbName := namer.LoadBalancer(lbKey)

key, _ := composite.CreateKey(cloud, "", scope)
key.Name = namer.ForwardingRule(lbName, utils.HTTPProtocol)
composite.DeleteForwardingRule(cloud, key, versions.ForwardingRule)

key.Name = namer.TargetProxy(lbName, utils.HTTPProtocol)
composite.DeleteTargetHttpProxy(cloud, key, versions.TargetHttpProxy)

key.Name = namer.UrlMap(lbName)
composite.DeleteUrlMap(cloud, key, versions.UrlMap)

cloud.DeleteGlobalAddress(namer.ForwardingRule(lbName, utils.HTTPProtocol))
}

func checkFakeLoadBalancer(cloud *gce.Cloud, namer *utils.Namer, key string, expectPresent bool) error {
func checkFakeLoadBalancer(cloud *gce.Cloud, namer *utils.Namer, lbKey string, versions *features.ResourceVersions, scope meta.KeyType, expectPresent bool) error {
var err error
lbName := namer.LoadBalancer(key)
_, err = cloud.GetGlobalForwardingRule(namer.ForwardingRule(lbName, utils.HTTPProtocol))
lbName := namer.LoadBalancer(lbKey)
key, _ := composite.CreateKey(cloud, namer.ForwardingRule(lbName, utils.HTTPProtocol), scope)

_, err = composite.GetForwardingRule(cloud, key, versions.ForwardingRule)
if expectPresent && err != nil {
return err
}
if !expectPresent && err.(*googleapi.Error).Code != http.StatusNotFound {
return fmt.Errorf("expect GlobalForwardingRule %q to not present: %v", key, err)
}
_, err = cloud.GetTargetHTTPProxy(namer.TargetProxy(lbName, utils.HTTPProtocol))

key.Name = namer.TargetProxy(lbName, utils.HTTPProtocol)
_, err = composite.GetTargetHttpProxy(cloud, key, versions.TargetHttpProxy)
if expectPresent && err != nil {
return err
}
if !expectPresent && err.(*googleapi.Error).Code != http.StatusNotFound {
return fmt.Errorf("expect TargetHTTPProxy %q to not present: %v", key, err)
}
_, err = cloud.GetURLMap(namer.UrlMap(lbName))

key.Name = namer.UrlMap(lbName)
_, err = composite.GetUrlMap(cloud, key, versions.UrlMap)
if expectPresent && err != nil {
return err
}
Expand Down
25 changes: 15 additions & 10 deletions pkg/loadbalancers/loadbalancers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package loadbalancers
import (
"context"
"fmt"
"k8s.io/ingress-gce/pkg/loadbalancers/features"
"net/http"
"strconv"
"strings"
Expand Down Expand Up @@ -646,7 +647,7 @@ func TestIdenticalHostnameCerts(t *testing.T) {
verifyCertAndProxyLink(expectCerts, expectCerts, j, t)
// Fetch the target proxy certs and go through in order
verifyProxyCertsInOrder(" foo.com", j, t)
j.pool.Delete(lbInfo.Name, defaultVersion, defaultScope)
j.pool.Delete(lbInfo.Name, features.GAResourceVersions, defaultScope)
}
}

Expand Down Expand Up @@ -702,7 +703,7 @@ func TestIdenticalHostnameCertsPreShared(t *testing.T) {
verifyCertAndProxyLink(expectCerts, expectCerts, j, t)
// Fetch the target proxy certs and go through in order
verifyProxyCertsInOrder(" foo.com", j, t)
j.pool.Delete(lbInfo.Name, defaultVersion, defaultScope)
j.pool.Delete(lbInfo.Name, features.GAResourceVersions, defaultScope)
}
}

Expand Down Expand Up @@ -1191,20 +1192,24 @@ func TestList(t *testing.T) {
}

if _, err := j.pool.Ensure(lbInfo); err != nil {
t.Fatalf("j.pool.Ensure() = err %v", err)
t.Fatalf("j.pool.Ensure() = %v; want nil", err)
}

lbNames, _, err := j.pool.List()
urlMaps, err := j.pool.List(key, defaultVersion)
if err != nil {
t.Fatalf("j.pool.List() = err %v", err)
t.Fatalf("j.pool.List(%q, %q) = %v, want nil", key, defaultVersion, err)
}
var umNames []string
for _, um := range urlMaps {
umNames = append(umNames, um.Name)
}

expected := []string{"old-l7--uid1", "test--uid1"}
expected := []string{"k8s-um-test--uid1", "k8s-um-old-l7--uid1"}

if len(lbNames) != 2 ||
!slice.ContainsString(lbNames, expected[0], nil) ||
!slice.ContainsString(lbNames, expected[1], nil) {
t.Fatalf("j.pool.List() returned %+v, want %+v", lbNames, expected)
for _, name := range expected {
if !slice.ContainsString(umNames, name, nil) {
t.Fatalf("j.pool.List(%q, %q) returned names %v, want %v", key, defaultVersion, umNames, expected)
}
}
}

Expand Down

0 comments on commit 0c126fb

Please sign in to comment.