Merge pull request #6 from nginxinc/fix-same-service-in-one-ingress

Fixed the problem when a service is referenced from one ingress resou…
nginx · Mar 16, 2016 · 66cc6d5 · 66cc6d5
2 parents 48c9f33 + 67898d7
commit 66cc6d5
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 20 deletions.
diff --git a/nginx-controller/controller/controller.go b/nginx-controller/controller/controller.go
@@ -262,15 +262,18 @@ func (lbc *LoadBalancerController) updateNGINX(name string, ing *extensions.Ingr
 		}
 	}
 
-	var upstreams []nginx.Upstream
+	upstreams := make(map[string]nginx.Upstream)
 
 	for _, rule := range ing.Spec.Rules {
 		if rule.IngressRuleValue.HTTP == nil {
 			continue
 		}
 
 		for _, path := range rule.HTTP.Paths {
-			name := ing.Name + "-" + path.Backend.ServiceName
+			name := getNameForUpstream(ing, rule.Host, path.Backend.ServiceName)
+			if _, exists := upstreams[name]; exists {
+				continue
+			}
 			ups := nginx.NewUpstreamWithDefaultServer(name)
 
 			svcKey := ing.Namespace + "/" + path.Backend.ServiceName
@@ -296,8 +299,8 @@ func (lbc *LoadBalancerController) updateNGINX(name string, ing *extensions.Ingr
 					}
 				}
 			}
+			upstreams[name] = ups
 
-			upstreams = append(upstreams, ups)
 		}
 	}
 
@@ -315,21 +318,19 @@ func (lbc *LoadBalancerController) updateNGINX(name string, ing *extensions.Ingr
 
 		for _, path := range rule.HTTP.Paths {
 			loc := nginx.Location{Path: path.Path}
-			upsName := ing.GetName() + "-" + path.Backend.ServiceName
+			upsName := getNameForUpstream(ing, rule.Host, path.Backend.ServiceName)
 
-			for _, ups := range upstreams {
-				if upsName == ups.Name {
-					loc.Upstream = ups
-				}
+			if ups, ok := upstreams[upsName]; ok {
+				loc.Upstream = ups
+				locations = append(locations, loc)
 			}
-			locations = append(locations, loc)
 		}
 
 		server.Locations = locations
 		servers = append(servers, server)
 	}
 
-	lbc.nginx.AddOrUpdateIngress(name, nginx.IngressNGINXConfig{Upstreams: upstreams, Servers: servers})
+	lbc.nginx.AddOrUpdateIngress(name, nginx.IngressNGINXConfig{Upstreams: upstreamMapToSlice(upstreams), Servers: servers})
 }
 
 func endpointsToUpstreamServers(endps api.Endpoints, servicePort int) []nginx.UpstreamServer {
@@ -348,3 +349,18 @@ func endpointsToUpstreamServers(endps api.Endpoints, servicePort int) []nginx.Up
 
 	return upsServers
 }
+
+func getNameForUpstream(ing *extensions.Ingress, host string, service string) string {
+	return fmt.Sprintf("%v-%v-%v-%v", ing.Namespace, ing.Name, host, service)
+}
+
+func upstreamMapToSlice(upstreams map[string]nginx.Upstream) []nginx.Upstream {
+	result := make([]nginx.Upstream, 0, len(upstreams))
+
+	for _, ups := range upstreams {
+		glog.Info(ups)
+		result = append(result, ups)
+	}
+
+	return result
+}
diff --git a/nginx-plus-controller/controller/controller.go b/nginx-plus-controller/controller/controller.go
@@ -155,22 +155,26 @@ func (lbc *LoadBalancerController) updateNGINX(name string, ing *extensions.Ingr
 		}
 	}
 
-	var upstreams []nginx.Upstream
+	upstreams := make(map[string]nginx.Upstream)
 
 	for _, rule := range ing.Spec.Rules {
 		if rule.IngressRuleValue.HTTP == nil {
 			continue
 		}
 
 		for _, path := range rule.HTTP.Paths {
-			name := ing.Name + "-" + path.Backend.ServiceName
+			name := getNameForUpstream(ing, rule.Host, path.Backend.ServiceName)
+			if _, exists := upstreams[name]; exists {
+				continue
+			}
 			upstream := nginx.Upstream{Name: name}
 			var upsServers []nginx.UpstreamServer
 			address := fmt.Sprintf("%v.%v.svc.cluster.local", path.Backend.ServiceName, ing.Namespace)
 			server := nginx.UpstreamServer{Address: address, Port: path.Backend.ServicePort.String()}
 			upsServers = append(upsServers, server)
 			upstream.UpstreamServers = upsServers
-			upstreams = append(upstreams, upstream)
+
+			upstreams[name] = upstream
 		}
 	}
 
@@ -188,19 +192,32 @@ func (lbc *LoadBalancerController) updateNGINX(name string, ing *extensions.Ingr
 
 		for _, path := range rule.HTTP.Paths {
 			loc := nginx.Location{Path: path.Path}
-			upsName := ing.GetName() + "-" + path.Backend.ServiceName
+			upsName := getNameForUpstream(ing, rule.Host, path.Backend.ServiceName)
 
-			for _, ups := range upstreams {
-				if upsName == ups.Name {
-					loc.Upstream = ups
-				}
+			if ups, ok := upstreams[upsName]; ok {
+				loc.Upstream = ups
+				locations = append(locations, loc)
 			}
-			locations = append(locations, loc)
 		}
 
 		server.Locations = locations
 		servers = append(servers, server)
 	}
 
-	lbc.nginx.AddOrUpdateIngress(name, nginx.IngressNGINXConfig{Upstreams: upstreams, Servers: servers})
+	lbc.nginx.AddOrUpdateIngress(name, nginx.IngressNGINXConfig{Upstreams: upstreamMapToSlice(upstreams), Servers: servers})
+}
+
+func getNameForUpstream(ing *extensions.Ingress, host string, service string) string {
+	return fmt.Sprintf("%v-%v-%v-%v", ing.Namespace, ing.Name, host, service)
+}
+
+func upstreamMapToSlice(upstreams map[string]nginx.Upstream) []nginx.Upstream {
+	result := make([]nginx.Upstream, 0, len(upstreams))
+
+	for _, ups := range upstreams {
+		glog.Info(ups)
+		result = append(result, ups)
+	}
+
+	return result
 }