Merge pull request #2340 from Shopify/sticky-upstream-name

Fix buildupstream name to work with dynamic session affinity
kubernetes · Apr 12, 2018 · cd4ddb7 · cd4ddb7
2 parents 353c631 + 4b11fe4
commit cd4ddb7
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 12 deletions.
diff --git a/internal/file/bindata.go b/internal/file/bindata.go
diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go
@@ -629,8 +629,7 @@ func buildDenyVariable(a interface{}) string {
  return fmt.Sprintf("$deny_%v", denyPathSlugMap[l])
 }
 
-// TODO: Needs Unit Tests
-func buildUpstreamName(host string, b interface{}, loc interface{}) string {
+func buildUpstreamName(host string, b interface{}, loc interface{}, dynamicConfigurationEnabled bool) string {
 
  backends, ok := b.([]*ingress.Backend)
  if !ok {
@@ -646,14 +645,16 @@ func buildUpstreamName(host string, b interface{}, loc interface{}) string {
 
  upstreamName := location.Backend
 
- for _, backend := range backends {
- if backend.Name == location.Backend {
- if backend.SessionAffinity.AffinityType == "cookie" &&
- isSticky(host, location, backend.SessionAffinity.CookieSessionAffinity.Locations) {
- upstreamName = fmt.Sprintf("sticky-%v", upstreamName)
- }
+ if !dynamicConfigurationEnabled {
+ for _, backend := range backends {
+ if backend.Name == location.Backend {
+ if backend.SessionAffinity.AffinityType == "cookie" &&
+ isSticky(host, location, backend.SessionAffinity.CookieSessionAffinity.Locations) {
+ upstreamName = fmt.Sprintf("sticky-%v", upstreamName)
+ }
 
- break
+ break
+ }
  }
  }
 

diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go
@@ -709,3 +709,46 @@ func TestIsLocationInLocationList(t *testing.T) {
  }
  }
 }
+
+func TestBuildUpstreamName(t *testing.T) {
+ defaultBackend := "upstream-name"
+ defaultHost := "example.com"
+
+ for k, tc := range tmplFuncTestcases {
+ loc := &ingress.Location{
+ Path: tc.Path,
+ Rewrite: rewrite.Config{Target: tc.Target, AddBaseURL: tc.AddBaseURL, BaseURLScheme: tc.BaseURLScheme},
+ Backend: defaultBackend,
+ XForwardedPrefix: tc.XForwardedPrefix,
+ }
+
+ backend := &ingress.Backend{
+ Name: defaultBackend,
+ Secure: tc.SecureBackend,
+ }
+
+ expected := defaultBackend
+
+ if tc.Sticky {
+ if !tc.DynamicConfigurationEnabled {
+ expected = fmt.Sprintf("sticky-" + expected)
+ }
+
+ backend.SessionAffinity = ingress.SessionAffinityConfig{
+ AffinityType: "cookie",
+ CookieSessionAffinity: ingress.CookieSessionAffinity{
+ Locations: map[string][]string{
+ defaultHost: {tc.Path},
+ },
+ },
+ }
+ }
+
+ backends := []*ingress.Backend{backend}
+
+ pp := buildUpstreamName(defaultHost, backends, loc, tc.DynamicConfigurationEnabled)
+ if !strings.EqualFold(expected, pp) {
+ t.Errorf("%s: expected \n'%v'\nbut returned \n'%v'", k, expected, pp)
+ }
+ }
+}
diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl
@@ -772,7 +772,7 @@ stream {
  # ngx_auth_request module overrides variables in the parent request,
  # therefore we have to explicitly set this variable again so that when the parent request
  # resumes it has the correct value set for this variable so that Lua can pick backend correctly
- set $proxy_upstream_name "{{ buildUpstreamName $server.Hostname $all.Backends $location }}";
+ set $proxy_upstream_name "{{ buildUpstreamName $server.Hostname $all.Backends $location $all.DynamicConfigurationEnabled }}";
 
  proxy_pass_request_body off;
  proxy_set_header Content-Length "";
@@ -886,7 +886,7 @@ stream {
 
  {{ if $all.Cfg.EnableVtsStatus }}{{ if $location.VtsFilterKey }} vhost_traffic_status_filter_by_set_key {{ $location.VtsFilterKey }};{{ end }}{{ end }}
 
- set $proxy_upstream_name "{{ buildUpstreamName $server.Hostname $all.Backends $location }}";
+ set $proxy_upstream_name "{{ buildUpstreamName $server.Hostname $all.Backends $location $all.DynamicConfigurationEnabled }}";
 
  {{ $ing := (getIngressInformation $location.Ingress $location.Path) }}
  {{/* $ing.Metadata contains the Ingress metadata */}}