Skip to content

Commit

Permalink
Merge pull request #117 from thockin/dont-sync-unchanged-urlmaps
Browse files Browse the repository at this point in the history 
GCE: Don't update URL Map if unchanged
  • Loading branch information
@bprashanth
bprashanth committed Jan 10, 2017
2 parents 6f5a8ce + 0950910 commit f90e9ee
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 1 deletion.
26 changes: 26 additions & 0 deletions controllers/gce/loadbalancers/fakes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type FakeLoadBalancers struct {
IP []*compute.Address
Certs []*compute.SslCertificate
name string
calls []string // list of calls that were made
}

// TODO: There is some duplication between these functions and the name mungers in
Expand Down Expand Up @@ -102,6 +103,7 @@ func (f *FakeLoadBalancers) String() string {

// GetGlobalForwardingRule returns a fake forwarding rule.
func (f *FakeLoadBalancers) GetGlobalForwardingRule(name string) (*compute.ForwardingRule, error) {
f.calls = append(f.calls, "GetGlobalForwardingRule")
for i := range f.Fw {
if f.Fw[i].Name == name {
return f.Fw[i], nil
Expand All @@ -112,6 +114,7 @@ func (f *FakeLoadBalancers) GetGlobalForwardingRule(name string) (*compute.Forwa

// CreateGlobalForwardingRule fakes forwarding rule creation.
func (f *FakeLoadBalancers) CreateGlobalForwardingRule(proxyLink, ip, name, portRange string) (*compute.ForwardingRule, error) {
f.calls = append(f.calls, "CreateGlobalForwardingRule")
if ip == "" {
ip = fmt.Sprintf(testIPManager.ip())
}
Expand All @@ -129,6 +132,7 @@ func (f *FakeLoadBalancers) CreateGlobalForwardingRule(proxyLink, ip, name, port

// SetProxyForGlobalForwardingRule fakes setting a global forwarding rule.
func (f *FakeLoadBalancers) SetProxyForGlobalForwardingRule(fw *compute.ForwardingRule, proxyLink string) error {
f.calls = append(f.calls, "SetProxyForGlobalForwardingRule")
for i := range f.Fw {
if f.Fw[i].Name == fw.Name {
f.Fw[i].Target = proxyLink
Expand All @@ -139,6 +143,7 @@ func (f *FakeLoadBalancers) SetProxyForGlobalForwardingRule(fw *compute.Forwardi

// DeleteGlobalForwardingRule fakes deleting a global forwarding rule.
func (f *FakeLoadBalancers) DeleteGlobalForwardingRule(name string) error {
f.calls = append(f.calls, "DeleteGlobalForwardingRule")
fw := []*compute.ForwardingRule{}
for i := range f.Fw {
if f.Fw[i].Name != name {
Expand All @@ -151,6 +156,7 @@ func (f *FakeLoadBalancers) DeleteGlobalForwardingRule(name string) error {

// GetForwardingRulesWithIPs returns all forwarding rules that match the given ips.
func (f *FakeLoadBalancers) GetForwardingRulesWithIPs(ip []string) (fwRules []*compute.ForwardingRule) {
f.calls = append(f.calls, "GetForwardingRulesWithIPs")
ipSet := sets.NewString(ip...)
for i := range f.Fw {
if ipSet.Has(f.Fw[i].IPAddress) {
Expand All @@ -164,6 +170,7 @@ func (f *FakeLoadBalancers) GetForwardingRulesWithIPs(ip []string) (fwRules []*c

// GetUrlMap fakes getting url maps from the cloud.
func (f *FakeLoadBalancers) GetUrlMap(name string) (*compute.UrlMap, error) {
f.calls = append(f.calls, "GetUrlMap")
for i := range f.Um {
if f.Um[i].Name == name {
return f.Um[i], nil
Expand All @@ -174,6 +181,7 @@ func (f *FakeLoadBalancers) GetUrlMap(name string) (*compute.UrlMap, error) {

// CreateUrlMap fakes url-map creation.
func (f *FakeLoadBalancers) CreateUrlMap(backend *compute.BackendService, name string) (*compute.UrlMap, error) {
f.calls = append(f.calls, "CreateUrlMap")
urlMap := &compute.UrlMap{
Name: name,
DefaultService: backend.SelfLink,
Expand All @@ -185,6 +193,7 @@ func (f *FakeLoadBalancers) CreateUrlMap(backend *compute.BackendService, name s

// UpdateUrlMap fakes updating url-maps.
func (f *FakeLoadBalancers) UpdateUrlMap(urlMap *compute.UrlMap) (*compute.UrlMap, error) {
f.calls = append(f.calls, "UpdateUrlMap")
for i := range f.Um {
if f.Um[i].Name == urlMap.Name {
f.Um[i] = urlMap
Expand All @@ -196,6 +205,7 @@ func (f *FakeLoadBalancers) UpdateUrlMap(urlMap *compute.UrlMap) (*compute.UrlMa

// DeleteUrlMap fakes url-map deletion.
func (f *FakeLoadBalancers) DeleteUrlMap(name string) error {
f.calls = append(f.calls, "DeleteUrlMap")
um := []*compute.UrlMap{}
for i := range f.Um {
if f.Um[i].Name != name {
Expand All @@ -210,6 +220,7 @@ func (f *FakeLoadBalancers) DeleteUrlMap(name string) error {

// GetTargetHttpProxy fakes getting target http proxies from the cloud.
func (f *FakeLoadBalancers) GetTargetHttpProxy(name string) (*compute.TargetHttpProxy, error) {
f.calls = append(f.calls, "GetTargetHttpProxy")
for i := range f.Tp {
if f.Tp[i].Name == name {
return f.Tp[i], nil
Expand All @@ -220,6 +231,7 @@ func (f *FakeLoadBalancers) GetTargetHttpProxy(name string) (*compute.TargetHttp

// CreateTargetHttpProxy fakes creating a target http proxy.
func (f *FakeLoadBalancers) CreateTargetHttpProxy(urlMap *compute.UrlMap, name string) (*compute.TargetHttpProxy, error) {
f.calls = append(f.calls, "CreateTargetHttpProxy")
proxy := &compute.TargetHttpProxy{
Name: name,
UrlMap: urlMap.SelfLink,
Expand All @@ -231,6 +243,7 @@ func (f *FakeLoadBalancers) CreateTargetHttpProxy(urlMap *compute.UrlMap, name s

// DeleteTargetHttpProxy fakes deleting a target http proxy.
func (f *FakeLoadBalancers) DeleteTargetHttpProxy(name string) error {
f.calls = append(f.calls, "DeleteTargetHttpProxy")
tp := []*compute.TargetHttpProxy{}
for i := range f.Tp {
if f.Tp[i].Name != name {
Expand All @@ -243,6 +256,7 @@ func (f *FakeLoadBalancers) DeleteTargetHttpProxy(name string) error {

// SetUrlMapForTargetHttpProxy fakes setting an url-map for a target http proxy.
func (f *FakeLoadBalancers) SetUrlMapForTargetHttpProxy(proxy *compute.TargetHttpProxy, urlMap *compute.UrlMap) error {
f.calls = append(f.calls, "SetUrlMapForTargetHttpProxy")
for i := range f.Tp {
if f.Tp[i].Name == proxy.Name {
f.Tp[i].UrlMap = urlMap.SelfLink
Expand All @@ -255,6 +269,7 @@ func (f *FakeLoadBalancers) SetUrlMapForTargetHttpProxy(proxy *compute.TargetHtt

// GetTargetHttpsProxy fakes getting target http proxies from the cloud.
func (f *FakeLoadBalancers) GetTargetHttpsProxy(name string) (*compute.TargetHttpsProxy, error) {
f.calls = append(f.calls, "GetTargetHttpsProxy")
for i := range f.Tps {
if f.Tps[i].Name == name {
return f.Tps[i], nil
Expand All @@ -265,6 +280,7 @@ func (f *FakeLoadBalancers) GetTargetHttpsProxy(name string) (*compute.TargetHtt

// CreateTargetHttpsProxy fakes creating a target http proxy.
func (f *FakeLoadBalancers) CreateTargetHttpsProxy(urlMap *compute.UrlMap, cert *compute.SslCertificate, name string) (*compute.TargetHttpsProxy, error) {
f.calls = append(f.calls, "CreateTargetHttpsProxy")
proxy := &compute.TargetHttpsProxy{
Name: name,
UrlMap: urlMap.SelfLink,
Expand All @@ -277,6 +293,7 @@ func (f *FakeLoadBalancers) CreateTargetHttpsProxy(urlMap *compute.UrlMap, cert

// DeleteTargetHttpsProxy fakes deleting a target http proxy.
func (f *FakeLoadBalancers) DeleteTargetHttpsProxy(name string) error {
f.calls = append(f.calls, "DeleteTargetHttpsProxy")
tp := []*compute.TargetHttpsProxy{}
for i := range f.Tps {
if f.Tps[i].Name != name {
Expand All @@ -289,6 +306,7 @@ func (f *FakeLoadBalancers) DeleteTargetHttpsProxy(name string) error {

// SetUrlMapForTargetHttpsProxy fakes setting an url-map for a target http proxy.
func (f *FakeLoadBalancers) SetUrlMapForTargetHttpsProxy(proxy *compute.TargetHttpsProxy, urlMap *compute.UrlMap) error {
f.calls = append(f.calls, "SetUrlMapForTargetHttpsProxy")
for i := range f.Tps {
if f.Tps[i].Name == proxy.Name {
f.Tps[i].UrlMap = urlMap.SelfLink
Expand All @@ -299,6 +317,7 @@ func (f *FakeLoadBalancers) SetUrlMapForTargetHttpsProxy(proxy *compute.TargetHt

// SetSslCertificateForTargetHttpsProxy fakes out setting certificates.
func (f *FakeLoadBalancers) SetSslCertificateForTargetHttpsProxy(proxy *compute.TargetHttpsProxy, SSLCert *compute.SslCertificate) error {
f.calls = append(f.calls, "SetSslCertificateForTargetHttpsProxy")
found := false
for i := range f.Tps {
if f.Tps[i].Name == proxy.Name {
Expand All @@ -316,6 +335,7 @@ func (f *FakeLoadBalancers) SetSslCertificateForTargetHttpsProxy(proxy *compute.

// CheckURLMap checks the URL map.
func (f *FakeLoadBalancers) CheckURLMap(t *testing.T, l7 *L7, expectedMap map[string]utils.FakeIngressRuleValueMap) {
f.calls = append(f.calls, "CheckURLMap")
um, err := f.GetUrlMap(l7.um.Name)
if err != nil || um == nil {
t.Fatalf("%v", err)
Expand Down Expand Up @@ -378,6 +398,7 @@ func (f *FakeLoadBalancers) CheckURLMap(t *testing.T, l7 *L7, expectedMap map[st

// ReserveGlobalStaticIP fakes out static IP reservation.
func (f *FakeLoadBalancers) ReserveGlobalStaticIP(name, IPAddress string) (*compute.Address, error) {
f.calls = append(f.calls, "ReserveGlobalStaticIP")
ip := &compute.Address{
Name: name,
Address: IPAddress,
Expand All @@ -388,6 +409,7 @@ func (f *FakeLoadBalancers) ReserveGlobalStaticIP(name, IPAddress string) (*comp

// GetGlobalStaticIP fakes out static IP retrieval.
func (f *FakeLoadBalancers) GetGlobalStaticIP(name string) (*compute.Address, error) {
f.calls = append(f.calls, "GetGlobalStaticIP")
for i := range f.IP {
if f.IP[i].Name == name {
return f.IP[i], nil
Expand All @@ -398,6 +420,7 @@ func (f *FakeLoadBalancers) GetGlobalStaticIP(name string) (*compute.Address, er

// DeleteGlobalStaticIP fakes out static IP deletion.
func (f *FakeLoadBalancers) DeleteGlobalStaticIP(name string) error {
f.calls = append(f.calls, "DeleteGlobalStaticIP")
ip := []*compute.Address{}
for i := range f.IP {
if f.IP[i].Name != name {
Expand All @@ -412,6 +435,7 @@ func (f *FakeLoadBalancers) DeleteGlobalStaticIP(name string) error {

// GetSslCertificate fakes out getting ssl certs.
func (f *FakeLoadBalancers) GetSslCertificate(name string) (*compute.SslCertificate, error) {
f.calls = append(f.calls, "GetSslCertificate")
for i := range f.Certs {
if f.Certs[i].Name == name {
return f.Certs[i], nil
Expand All @@ -422,13 +446,15 @@ func (f *FakeLoadBalancers) GetSslCertificate(name string) (*compute.SslCertific

// CreateSslCertificate fakes out certificate creation.
func (f *FakeLoadBalancers) CreateSslCertificate(cert *compute.SslCertificate) (*compute.SslCertificate, error) {
f.calls = append(f.calls, "CreateSslCertificate")
cert.SelfLink = cert.Name
f.Certs = append(f.Certs, cert)
return cert, nil
}

// DeleteSslCertificate fakes out certificate deletion.
func (f *FakeLoadBalancers) DeleteSslCertificate(name string) error {
f.calls = append(f.calls, "DeleteSslCertificate")
certs := []*compute.SslCertificate{}
for i := range f.Certs {
if f.Certs[i].Name != name {
Expand Down
69 changes: 68 additions & 1 deletion controllers/gce/loadbalancers/loadbalancers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,6 @@ func (l *L7) UpdateUrlMap(ingressRules utils.GCEURLMap) error {
} else {
l.um.DefaultService = l.glbcDefaultBackend.SelfLink
}
glog.Infof("Updating url map:\n%+v", ingressRules)

// Every update replaces the entire urlmap.
// TODO: when we have multiple loadbalancers point to a single gce url map
Expand Down Expand Up @@ -728,6 +727,12 @@ func (l *L7) UpdateUrlMap(ingressRules utils.GCEURLMap) error {
}
l.um.PathMatchers = append(l.um.PathMatchers, pathMatcher)
}
oldMap, _ := l.cloud.GetUrlMap(l.um.Name)
if oldMap != nil && mapsEqual(oldMap, l.um) {
glog.Infof("UrlMap for l7 %v is unchanged", l.Name)
return nil
}
glog.Infof("Updating url map: %+v", ingressRules)
um, err := l.cloud.UpdateUrlMap(l.um)
if err != nil {
return err
Expand All @@ -736,6 +741,68 @@ func (l *L7) UpdateUrlMap(ingressRules utils.GCEURLMap) error {
return nil
}

func mapsEqual(a, b *compute.UrlMap) bool {
if a.DefaultService != b.DefaultService {
return false
}
if len(a.HostRules) != len(b.HostRules) {
return false
}
for i := range a.HostRules {
a := a.HostRules[i]
b := b.HostRules[i]
if a.Description != b.Description {
return false
}
if len(a.Hosts) != len(a.Hosts) {
return false
}
for i := range a.Hosts {
if a.Hosts[i] != b.Hosts[i] {
return false
}
}
if a.PathMatcher != b.PathMatcher {
return false
}
}
if len(a.PathMatchers) != len(b.PathMatchers) {
return false
}
for i := range a.PathMatchers {
a := a.PathMatchers[i]
b := b.PathMatchers[i]
if a.DefaultService != b.DefaultService {
return false
}
if a.Description != b.Description {
return false
}
if a.Name != b.Name {
return false
}
if len(a.PathRules) != len(a.PathRules) {
return false
}
for i := range a.PathRules {
a := a.PathRules[i]
b := b.PathRules[i]
if len(a.Paths) != len(a.Paths) {
return false
}
for i := range a.Paths {
if a.Paths[i] != b.Paths[i] {
return false
}
}
if a.Service != b.Service {
return false
}
}
}
return true
}

// Cleanup deletes resources specific to this l7 in the right order.
// forwarding rule -> target proxy -> url map
// This leaves backends and health checks, which are shared across loadbalancers.
Expand Down
42 changes: 42 additions & 0 deletions controllers/gce/loadbalancers/loadbalancers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,48 @@ func TestUpdateUrlMap(t *testing.T) {
f.CheckURLMap(t, l7, expectedMap)
}

func TestUpdateUrlMapNoChanges(t *testing.T) {
um1 := utils.GCEURLMap{
"foo.example.com": {
"/foo1": &compute.BackendService{SelfLink: "foo1svc"},
"/foo2": &compute.BackendService{SelfLink: "foo2svc"},
},
"bar.example.com": {
"/bar1": &compute.BackendService{SelfLink: "bar1svc"},
},
}
um1.PutDefaultBackend(&compute.BackendService{SelfLink: "default"})
um2 := utils.GCEURLMap{
"foo.example.com": {
"/foo1": &compute.BackendService{SelfLink: "foo1svc"},
"/foo2": &compute.BackendService{SelfLink: "foo2svc"},
},
"bar.example.com": {
"/bar1": &compute.BackendService{SelfLink: "bar1svc"},
},
}
um2.PutDefaultBackend(&compute.BackendService{SelfLink: "default"})

lbInfo := &L7RuntimeInfo{Name: "test", AllowHTTP: true}
f := NewFakeLoadBalancers(lbInfo.Name)
pool := newFakeLoadBalancerPool(f, t)
pool.Sync([]*L7RuntimeInfo{lbInfo})
l7, err := pool.Get(lbInfo.Name)
if err != nil {
t.Fatalf("%v", err)
}
for _, ir := range []utils.GCEURLMap{um1, um2} {
if err := l7.UpdateUrlMap(ir); err != nil {
t.Fatalf("%v", err)
}
}
for _, call := range f.calls {
if call == "UpdateUrlMap" {
t.Errorf("UpdateUrlMap() should not have been called")
}
}
}

func TestNameParsing(t *testing.T) {
clusterName := "123"
namer := utils.NewNamer(clusterName)
Expand Down

0 comments on commit f90e9ee

Please sign in to comment.