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.

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

GCE: Don't update URL Map if unchanged #117

Merged
merged 1 commit into from
Jan 10, 2017
Merged

GCE: Don't update URL Map if unchanged #117

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
26 changes: 26 additions & 0 deletions controllers/gce/loadbalancers/fakes.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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