Merge pull request #20557 from ramr/fix-router-dyn-cookie-bug

Fix router bug where cookie is not set correctly when dynamic changes are enabled
openshift · Aug 13, 2018 · 0ac9443 · 0ac9443
2 parents edad963 + b1045db
commit 0ac9443
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 4 deletions.
diff --git a/images/router/haproxy/conf/haproxy-config.template b/images/router/haproxy/conf/haproxy-config.template
@@ -495,6 +495,7 @@ backend {{genBackendNamePrefix $cfg.TLSTermination}}:{{$cfgIdx}}
     {{- else }}
       {{- with $name := $dynamicConfigManager.ServerTemplateName $cfgIdx }}
         {{- with $size := $dynamicConfigManager.ServerTemplateSize $cfgIdx }}
+  dynamic-cookie-key {{$cfg.RoutingKeyName}}
   server-template {{$name}}- 1-{{$size}} 172.4.0.4:8765 check disabled
         {{- end }}
       {{- end }}
@@ -566,6 +567,7 @@ backend {{genBackendNamePrefix $cfg.TLSTermination}}:{{$cfgIdx}}
     {{- with $dynamicConfigManager }}
       {{- with $name := $dynamicConfigManager.ServerTemplateName $cfgIdx }}
         {{- with $size := $dynamicConfigManager.ServerTemplateSize $cfgIdx }}
+  dynamic-cookie-key {{$cfg.RoutingKeyName}}
   server-template {{$name}}- 1-{{$size}} 172.4.0.4:8765 check disabled
         {{- end }}
       {{- end }}

diff --git a/pkg/router/template/configmanager/haproxy/backend.go b/pkg/router/template/configmanager/haproxy/backend.go
@@ -2,6 +2,7 @@ package haproxy
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"strconv"
 	"strings"
@@ -167,6 +168,38 @@ func (b *Backend) Refresh() error {
 	return nil
 }
 
+// SetRoutingKey sets the cookie routing key for the haproxy backend.
+func (b *Backend) SetRoutingKey(k string) error {
+	glog.V(4).Infof("Setting routing key for %s", b.name)
+
+	cmd := fmt.Sprintf("set dynamic-cookie-key backend %s %s", b.name, k)
+	if err := b.executeCommand(cmd); err != nil {
+		return fmt.Errorf("setting routing key for backend %s: %v", b.name, err)
+	}
+
+	cmd = fmt.Sprintf("enable dynamic-cookie backend %s", b.name)
+	if err := b.executeCommand(cmd); err != nil {
+		return fmt.Errorf("enabling routing key for backend %s: %v", b.name, err)
+	}
+
+	return nil
+}
+
+// executeCommand runs a command using the haproxy dynamic config api client.
+func (b *Backend) executeCommand(cmd string) error {
+	responseBytes, err := b.client.Execute(cmd)
+	if err != nil {
+		return err
+	}
+
+	response := strings.TrimSpace(string(responseBytes))
+	if len(response) > 0 {
+		return errors.New(response)
+	}
+
+	return nil
+}
+
 // Disable stops serving traffic for all servers for a haproxy backend.
 func (b *Backend) Disable() error {
 	if _, err := b.Servers(); err != nil {

diff --git a/pkg/router/template/configmanager/haproxy/blueprint_plugin_test.go b/pkg/router/template/configmanager/haproxy/blueprint_plugin_test.go
@@ -43,7 +43,7 @@ func (cm *fakeConfigManager) FindBlueprint(id string) (*routeapi.Route, bool) {
 func (cm *fakeConfigManager) Register(id string, route *routeapi.Route) {
 }
 
-func (cm *fakeConfigManager) AddRoute(id string, route *routeapi.Route) error {
+func (cm *fakeConfigManager) AddRoute(id, routingKey string, route *routeapi.Route) error {
 	return nil
 }
 

diff --git a/pkg/router/template/configmanager/haproxy/manager.go b/pkg/router/template/configmanager/haproxy/manager.go
@@ -286,7 +286,7 @@ func (cm *haproxyConfigManager) Register(id string, route *routeapi.Route) {
 }
 
 // AddRoute adds a new route or updates an existing route.
-func (cm *haproxyConfigManager) AddRoute(id string, route *routeapi.Route) error {
+func (cm *haproxyConfigManager) AddRoute(id, routingKey string, route *routeapi.Route) error {
 	if cm.isReloading() {
 		return fmt.Errorf("Router reload in progress, cannot dynamically add route %s", id)
 	}
@@ -334,6 +334,18 @@ func (cm *haproxyConfigManager) AddRoute(id string, route *routeapi.Route) error
 		return fmt.Errorf("adding map associations for id %s: %v", id, err)
 	}
 
+	backendName := entry.BackendName()
+	glog.V(4).Infof("Finding backend %s ...", backendName)
+	backend, err := cm.client.FindBackend(backendName)
+	if err != nil {
+		return err
+	}
+
+	glog.V(4).Infof("Setting routing key for backend %s ...", backendName)
+	if err := backend.SetRoutingKey(routingKey); err != nil {
+		return err
+	}
+
 	glog.V(4).Infof("Route %s added using blueprint pool slot %s", id, slotName)
 	return nil
 }

diff --git a/pkg/router/template/router.go b/pkg/router/template/router.go
@@ -598,7 +598,7 @@ func (r *templateRouter) dynamicallyAddRoute(backendKey string, route *routeapi.
 		return false
 	}
 
-	err := r.dynamicConfigManager.AddRoute(backendKey, route)
+	err := r.dynamicConfigManager.AddRoute(backendKey, backend.RoutingKeyName, route)
 	if err != nil {
 		glog.V(4).Infof("Router will reload as the ConfigManager could not dynamically add route for backend %s: %v", backendKey, err)
 		return false

diff --git a/pkg/router/template/types.go b/pkg/router/template/types.go
@@ -190,7 +190,7 @@ type ConfigManager interface {
 	Register(id string, route *routeapi.Route)
 
 	// AddRoute adds a new route or updates an existing route.
-	AddRoute(id string, route *routeapi.Route) error
+	AddRoute(id, routingKey string, route *routeapi.Route) error
 
 	// RemoveRoute removes a route.
 	RemoveRoute(id string, route *routeapi.Route) error