Implement merging strategy

nginx-controller/controller/controller.go

@@ -293,7 +293,7 @@ func (lbc *LoadBalancerController) syncEndp(key string) {
 				continue
 			}
 			glog.V(3).Infof("Updating Endpoints for %v/%v", ing.Name, ing.Namespace)
-			name := ing.Namespace + "-" + ing.Name
+			name := ing.Namespace + "/" + ing.Name
 			lbc.cnf.UpdateEndpoints(name, ingEx)
 		}
 	}
@@ -473,12 +473,9 @@ func (lbc *LoadBalancerController) syncIng(key string) {
 		return
 	}
 
-	// defaut/some-ingress -> default-some-ingress
-	name := strings.Replace(key, "/", "-", -1)
-
 	if !ingExists {
 		glog.V(2).Infof("Deleting Ingress: %v\n", key)
-		lbc.cnf.DeleteIngress(name)
+		lbc.cnf.DeleteIngress(key)
 	} else {
 		glog.V(2).Infof("Adding or Updating Ingress: %v\n", key)
 
@@ -488,7 +485,7 @@ func (lbc *LoadBalancerController) syncIng(key string) {
 			lbc.ingQueue.requeueAfter(key, err, 5*time.Second)
 			return
 		}
-		lbc.cnf.AddOrUpdateIngress(name, ingEx)
+		lbc.cnf.AddOrUpdateIngress(key, ingEx)
 	}
 }
 

nginx-controller/main.go

@@ -29,6 +29,15 @@ var (
 	nginxConfigMaps = flag.String("nginx-configmaps", "",
 		`Specifies a configmaps resource that can be used to customize NGINX
 		configuration. The value must follow the following format: <namespace>/<name>`)
+
+	enableMerging = flag.Bool("enable-merging", false,
+		`Enables merging of ingress rules in multiple ingress objects targeting the same host.
+		By default referencing the same host in multiple ingress objects will result in an error,
+		and only the first ingress object will be used by the ingress controller.
+		If this flag is enabled these rules will be merged using the server generated of
+		the oldest ingress object as a base and adding the locations and settings of
+		every ingress object in descending oder of their age.
+		This is similar to the behavoir of other nginx ingress controller.`)
 )
 
 func main() {
@@ -55,7 +64,13 @@ func main() {
 	ngxc, _ := nginx.NewNginxController("/etc/nginx/", local)
 	ngxc.Start()
 	config := nginx.NewDefaultConfig()
-	cnf := nginx.NewConfigurator(ngxc, config)
+	var collisionHandler nginx.CollisionHandler
+	if *enableMerging {
+		collisionHandler = nginx.NewMergingCollisionHandler()
+	} else {
+		collisionHandler = nginx.NewDenyCollisionHandler()
+	}
+	cnf := nginx.NewConfigurator(ngxc, collisionHandler, config)
 	lbc, _ := controller.NewLoadBalancerController(kubeClient, 30*time.Second, *watchNamespace, cnf, *nginxConfigMaps)
 	lbc.Run()
 }

nginx-controller/nginx/collision_handler.go

@@ -0,0 +1,29 @@
+package nginx
+
+import "k8s.io/kubernetes/pkg/apis/extensions"
+
+// CollisionHandler rosolves collisions in the generated configuration, so the nginx configuration stays valid.
+type CollisionHandler interface {
+	// AddConfigs is resolving definition collisions that would result in a invalid nginx configuration
+	AddConfigs(ing *extensions.Ingress, ingServers []Server) (changed []Server, err error)
+
+	// RemoveConfigs frees server definitions, so they can be used again
+	RemoveConfigs(key string) (changed []Server, deleted []Server, err error)
+}
+
+type cacheEntry struct {
+	Ingress extensions.Ingress
+	Servers []Server
+}
+
+type cacheEntryList []cacheEntry
+
+func (list cacheEntryList) Len() int {
+	return len(list)
+}
+func (list cacheEntryList) Less(i, j int) bool {
+	return list[i].Ingress.CreationTimestamp.Before(list[j].Ingress.CreationTimestamp)
+}
+func (list cacheEntryList) Swap(i, j int) {
+	list[i], list[j] = list[j], list[i]
+}

nginx-controller/nginx/collision_handler_test.go

@@ -0,0 +1,123 @@
+package nginx
+
+import (
+	"time"
+
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/unversioned"
+	"k8s.io/kubernetes/pkg/apis/extensions"
+)
+
+var (
+	//
+	// Ingress 1
+	//
+	ingress1Upstream1 = Upstream{
+		Name: "default-ing1-one.example.com-svc1",
+	}
+	ingress1Location1 = Location{
+		Path:     "/1",
+		Upstream: ingress1Upstream1,
+	}
+	ingress1Server1 = Server{
+		Name:      "one.example.com",
+		Locations: []Location{ingress1Location1},
+		Upstreams: []Upstream{ingress1Upstream1},
+	}
+	// ingress1Config1 = IngressNginxConfig{
+	// 	Upstreams: []Upstream{ingress1Upstream1},
+	// 	Server:    ingress1Server1,
+	// }
+	ingress1 = extensions.Ingress{
+		ObjectMeta: api.ObjectMeta{
+			Name:              "ing1",
+			Namespace:         "default",
+			CreationTimestamp: unversioned.NewTime(time.Now().Add(-2 * time.Hour)),
+		},
+	}
+
+	//
+	// Ingress 2
+	//
+	ingress2Upstream1 = Upstream{
+		Name: "default-ing2-one.example.com-svc1",
+	}
+	ingress2Upstream2 = Upstream{
+		Name: "default-ing2-two.example.com-svc1",
+	}
+	ingress2Location1 = Location{
+		Path:     "/1",
+		Upstream: ingress2Upstream1,
+	}
+	ingress2Location2 = Location{
+		Path:     "/2",
+		Upstream: ingress2Upstream1,
+	}
+	ingress2Location3 = Location{
+		Path:     "/3",
+		Upstream: ingress2Upstream2,
+	}
+	ingress2Server1 = Server{
+		Name:      "one.example.com",
+		Upstreams: []Upstream{ingress2Upstream1},
+		Locations: []Location{ingress2Location1, ingress2Location2},
+	}
+	ingress2Server2 = Server{
+		Name:      "two.example.com",
+		Upstreams: []Upstream{ingress2Upstream2},
+		Locations: []Location{ingress2Location3},
+	}
+	// ingress2Config1 = IngressNginxConfig{
+	// 	Upstreams: []Upstream{ingress2Upstream1},
+	// 	Server:    ingress2Server1,
+	// }
+	// ingress2Config2 = IngressNginxConfig{
+	// 	Upstreams: []Upstream{ingress2Upstream2},
+	// 	Server:    ingress2Server2,
+	// }
+	ingress2 = extensions.Ingress{
+		ObjectMeta: api.ObjectMeta{
+			Name:      "ing2",
+			Namespace: "default",
+			// this ingress is older than ing1, so it shoud be the base of the merge,
+			// which means that ing2 will override conflicting locations, but not server settings
+			CreationTimestamp: unversioned.NewTime(time.Now().Add(-4 * time.Hour)),
+		},
+	}
+
+	//
+	// Ingress 3
+	//
+	ingress3Upstream1 = Upstream{
+		Name: "default-ing3-one.example.com-svc1",
+	}
+	ingress3Location1 = Location{
+		Path:     "/1",
+		Upstream: ingress3Upstream1,
+	}
+	ingress3Server1 = Server{
+		Name:      "one.example.com",
+		Locations: []Location{ingress3Location1},
+		// this server introduces additional settings,
+		// because the merging process is additive they will be
+		// used regardless of age
+		SSL:                   true,
+		SSLCertificate:        "cert.pem",
+		SSLCertificateKey:     "cert.pem",
+		HTTP2:                 true,
+		HSTS:                  true,
+		HSTSMaxAge:            2000,
+		HSTSIncludeSubdomains: true,
+	}
+	// ingress3Config1 = IngressNginxConfig{
+	// 	Upstreams: []Upstream{ingress3Upstream1},
+	// 	Server:    ingress3Server1,
+	// }
+	ingress3 = extensions.Ingress{
+		ObjectMeta: api.ObjectMeta{
+			Name:              "ing3",
+			Namespace:         "default",
+			CreationTimestamp: unversioned.NewTime(time.Now().Add(-1 * time.Hour)),
+		},
+	}
+)

nginx-controller/nginx/configurator.go

@@ -14,16 +14,18 @@ const emptyHost = ""
 
 // Configurator transforms an Ingress resource into NGINX Configuration
 type Configurator struct {
-	nginx  *NginxController
-	config *Config
-	lock   sync.Mutex
+	nginx            *NginxController
+	config           *Config
+	collisionHandler CollisionHandler
+	lock             sync.Mutex
 }
 
 // NewConfigurator creates a new Configurator
-func NewConfigurator(nginx *NginxController, config *Config) *Configurator {
+func NewConfigurator(nginx *NginxController, collisionHandler CollisionHandler, config *Config) *Configurator {
 	cnf := Configurator{
-		nginx:  nginx,
-		config: config,
+		nginx:            nginx,
+		config:           config,
+		collisionHandler: collisionHandler,
 	}
 
 	return &cnf
@@ -39,8 +41,15 @@ func (cnf *Configurator) AddOrUpdateIngress(name string, ingEx *IngressEx) {
 	defer cnf.lock.Unlock()
 
 	pems := cnf.updateCertificates(ingEx)
-	nginxCfg := cnf.generateNginxCfg(ingEx, pems)
-	cnf.nginx.AddOrUpdateIngress(name, nginxCfg)
+	generatedServers := cnf.generateNginxCfg(ingEx, pems)
+	servers, err := cnf.collisionHandler.AddConfigs(ingEx.Ingress, generatedServers)
+	if err != nil {
+		glog.Errorf("Error when checking generated servers for collisions: %v", err)
+		return
+	}
+	for _, server := range servers {
+		cnf.nginx.AddOrUpdateConfig(server.Name, server)
+	}
 	if err := cnf.nginx.Reload(); err != nil {
 		glog.Errorf("Error when adding or updating ingress %q: %q", name, err)
 	}
@@ -79,21 +88,14 @@ func (cnf *Configurator) updateCertificates(ingEx *IngressEx) map[string]string
 
 	return pems
 }
-func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]string) IngressNginxConfig {
-	ingCfg := cnf.createConfig(ingEx)
 
-	upstreams := make(map[string]Upstream)
+func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]string) []Server {
+	ingCfg := cnf.createConfig(ingEx)
 
 	wsServices := getWebsocketServices(ingEx)
 	rewrites := getRewrites(ingEx)
 	sslServices := getSSLServices(ingEx)
 
-	if ingEx.Ingress.Spec.Backend != nil {
-		name := getNameForUpstream(ingEx.Ingress, emptyHost, ingEx.Ingress.Spec.Backend.ServiceName)
-		upstream := cnf.createUpstream(ingEx, name, ingEx.Ingress.Spec.Backend, ingEx.Ingress.Namespace)
-		upstreams[name] = upstream
-	}
-
 	var servers []Server
 
 	for _, rule := range ingEx.Ingress.Spec.Rules {
@@ -107,27 +109,8 @@ func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]stri
 			glog.Warningf("Host field of ingress rule in %v/%v is empty", ingEx.Ingress.Namespace, ingEx.Ingress.Name)
 		}
 
-		server := Server{
-			Name:                  serverName,
-			HTTP2:                 ingCfg.HTTP2,
-			ProxyProtocol:         ingCfg.ProxyProtocol,
-			HSTS:                  ingCfg.HSTS,
-			HSTSMaxAge:            ingCfg.HSTSMaxAge,
-			HSTSIncludeSubdomains: ingCfg.HSTSIncludeSubdomains,
-			RealIPHeader:          ingCfg.RealIPHeader,
-			SetRealIPFrom:         ingCfg.SetRealIPFrom,
-			RealIPRecursive:       ingCfg.RealIPRecursive,
-			ProxyHideHeaders:      ingCfg.ProxyHideHeaders,
-			ProxyPassHeaders:      ingCfg.ProxyPassHeaders,
-		}
-
-		if pemFile, ok := pems[serverName]; ok {
-			server.SSL = true
-			server.SSLCertificate = pemFile
-			server.SSLCertificateKey = pemFile
-		}
-
 		var locations []Location
+		upstreams := map[string]Upstream{}
 		rootLocation := false
 
 		for _, path := range rule.HTTP.Paths {
@@ -152,13 +135,41 @@ func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]stri
 			locations = append(locations, loc)
 		}
 
-		server.Locations = locations
+		server := Server{
+			Name:                  serverName,
+			Locations:             locations,
+			HTTP2:                 ingCfg.HTTP2,
+			ProxyProtocol:         ingCfg.ProxyProtocol,
+			HSTS:                  ingCfg.HSTS,
+			HSTSMaxAge:            ingCfg.HSTSMaxAge,
+			HSTSIncludeSubdomains: ingCfg.HSTSIncludeSubdomains,
+			RealIPHeader:          ingCfg.RealIPHeader,
+			SetRealIPFrom:         ingCfg.SetRealIPFrom,
+			RealIPRecursive:       ingCfg.RealIPRecursive,
+			ProxyHideHeaders:      ingCfg.ProxyHideHeaders,
+			ProxyPassHeaders:      ingCfg.ProxyPassHeaders,
+		}
+
+		if pemFile, ok := pems[serverName]; ok {
+			server.SSL = true
+			server.SSLCertificate = pemFile
+			server.SSLCertificateKey = pemFile
+		}
+
+		// server.Locations = locations
+		// server.Upstreams = upstreamMapToSlice(upstreams)
 		servers = append(servers, server)
 	}
 
 	if len(ingEx.Ingress.Spec.Rules) == 0 && ingEx.Ingress.Spec.Backend != nil {
+		upsName := getNameForUpstream(ingEx.Ingress, emptyHost, ingEx.Ingress.Spec.Backend.ServiceName)
+		upstream := cnf.createUpstream(ingEx, upsName, ingEx.Ingress.Spec.Backend, ingEx.Ingress.Namespace)
+		location := createLocation(pathOrDefault("/"), upstream, &ingCfg, wsServices[ingEx.Ingress.Spec.Backend.ServiceName], rewrites[ingEx.Ingress.Spec.Backend.ServiceName], sslServices[ingEx.Ingress.Spec.Backend.ServiceName])
+
 		server := Server{
 			Name:                  emptyHost,
+			Upstreams:             []Upstream{upstream},
+			Locations:             []Location{location},
 			HTTP2:                 ingCfg.HTTP2,
 			ProxyProtocol:         ingCfg.ProxyProtocol,
 			HSTS:                  ingCfg.HSTS,
@@ -177,18 +188,10 @@ func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]stri
 			server.SSLCertificateKey = pemFile
 		}
 
-		var locations []Location
-
-		upsName := getNameForUpstream(ingEx.Ingress, emptyHost, ingEx.Ingress.Spec.Backend.ServiceName)
-
-		loc := createLocation(pathOrDefault("/"), upstreams[upsName], &ingCfg, wsServices[ingEx.Ingress.Spec.Backend.ServiceName], rewrites[ingEx.Ingress.Spec.Backend.ServiceName], sslServices[ingEx.Ingress.Spec.Backend.ServiceName])
-		locations = append(locations, loc)
-
-		server.Locations = locations
 		servers = append(servers, server)
 	}
 
-	return IngressNginxConfig{Upstreams: upstreamMapToSlice(upstreams), Servers: servers}
+	return servers
 }
 
 func (cnf *Configurator) createConfig(ingEx *IngressEx) Config {
@@ -397,7 +400,16 @@ func (cnf *Configurator) DeleteIngress(name string) {
 	cnf.lock.Lock()
 	defer cnf.lock.Unlock()
 
-	cnf.nginx.DeleteIngress(name)
+	changedServers, deletedServers, err := cnf.collisionHandler.RemoveConfigs(name)
+	if err != nil {
+		glog.Errorf("Error when removing ingress %q from collision handler: %v", name, err)
+	}
+	for _, server := range changedServers {
+		cnf.nginx.AddOrUpdateConfig(server.Name, server)
+	}
+	for _, server := range deletedServers {
+		cnf.nginx.DeleteConfig(server.Name)
+	}
 	if err := cnf.nginx.Reload(); err != nil {
 		glog.Errorf("Error when removing ingress %q: %q", name, err)
 	}