From 8cb3f0835a0579b3506840edb845a7fa32e29d22 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Thu, 12 Oct 2017 17:50:03 +0200 Subject: [PATCH] Stickiness cookie name. --- provider/consul/consul_catalog.go | 2 +- provider/consul/consul_catalog_test.go | 67 +++++++++ provider/docker/docker.go | 12 +- provider/docker/docker_test.go | 78 ++++++++++ provider/ecs/ecs.go | 2 +- provider/kubernetes/kubernetes.go | 9 +- provider/kubernetes/kubernetes_test.go | 2 +- provider/kv/kv.go | 16 ++- provider/kv/kv_mock_test.go | 110 ++++++++++++++ provider/kv/kv_test.go | 189 ++++++++++++------------- provider/marathon/marathon.go | 12 +- provider/marathon/marathon_test.go | 37 +++-- provider/rancher/rancher.go | 12 +- provider/rancher/rancher_test.go | 95 +++++++++++++ server/server.go | 18 ++- templates/consul_catalog.tmpl | 2 +- templates/docker.tmpl | 2 +- templates/ecs.tmpl | 2 +- templates/kubernetes.tmpl | 2 +- templates/marathon.tmpl | 2 +- templates/rancher.tmpl | 2 +- 21 files changed, 525 insertions(+), 148 deletions(-) create mode 100644 provider/kv/kv_mock_test.go diff --git a/provider/consul/consul_catalog.go b/provider/consul/consul_catalog.go index dd2c75156..02230a621 100644 --- a/provider/consul/consul_catalog.go +++ b/provider/consul/consul_catalog.go @@ -398,7 +398,7 @@ func (p *CatalogProvider) hasStickinessLabel(tags []string) bool { stickyTag := p.getTag(types.LabelBackendLoadbalancerSticky, tags, "") if len(stickyTag) > 0 { - log.Warn("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) + log.Warnf("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) } stickiness := len(stickinessTag) > 0 && strings.EqualFold(strings.TrimSpace(stickinessTag), "true") diff --git a/provider/consul/consul_catalog_test.go b/provider/consul/consul_catalog_test.go index 7cd3c25eb..7b8ce4a7b 100644 --- a/provider/consul/consul_catalog_test.go +++ b/provider/consul/consul_catalog_test.go @@ -9,6 +9,7 @@ import ( "github.com/BurntSushi/ty/fun" "github.com/containous/traefik/types" "github.com/hashicorp/consul/api" + "github.com/stretchr/testify/assert" ) func TestConsulCatalogGetFrontendRule(t *testing.T) { @@ -891,3 +892,69 @@ func TestConsulCatalogGetBasicAuth(t *testing.T) { }) } } + +func TestConsulCatalogHasStickinessLabel(t *testing.T) { + testCases := []struct { + desc string + tags []string + expected bool + }{ + { + desc: "label missing", + tags: []string{}, + expected: false, + }, + { + desc: "sticky=true", + tags: []string{ + "traefik.backend.loadbalancer.sticky=true", + }, + expected: true, + }, + { + desc: "stickiness=true", + tags: []string{ + "traefik.backend.loadbalancer.stickiness=true", + }, + expected: true, + }, + { + desc: "sticky=true and stickiness=true", + tags: []string{ + "traefik.backend.loadbalancer.sticky=true", + "traefik.backend.loadbalancer.stickiness=true", + }, + expected: true, + }, + { + desc: "sticky=false and stickiness=true", + tags: []string{ + "traefik.backend.loadbalancer.sticky=true", + "traefik.backend.loadbalancer.stickiness=false", + }, + expected: true, + }, + { + desc: "sticky=true and stickiness=false", + tags: []string{ + "traefik.backend.loadbalancer.sticky=true", + "traefik.backend.loadbalancer.stickiness=false", + }, + expected: true, + }, + } + + provider := &CatalogProvider{ + Prefix: "traefik", + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + actual := provider.hasStickinessLabel(test.tags) + assert.Equal(t, actual, test.expected) + }) + } +} diff --git a/provider/docker/docker.go b/provider/docker/docker.go index ec0b7f707..2b1b8d61a 100644 --- a/provider/docker/docker.go +++ b/provider/docker/docker.go @@ -645,14 +645,16 @@ func (p *Provider) getWeight(container dockerData) string { } func (p *Provider) hasStickinessLabel(container dockerData) bool { - _, errStickiness := getLabel(container, types.LabelBackendLoadbalancerStickiness) + labelStickiness, errStickiness := getLabel(container, types.LabelBackendLoadbalancerStickiness) - label, errSticky := getLabel(container, types.LabelBackendLoadbalancerSticky) - if len(label) > 0 { - log.Warn("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) + labelSticky, errSticky := getLabel(container, types.LabelBackendLoadbalancerSticky) + if len(labelSticky) > 0 { + log.Warnf("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) } - return errStickiness == nil || (errSticky == nil && strings.EqualFold(strings.TrimSpace(label), "true")) + stickiness := errStickiness == nil && len(labelStickiness) > 0 && strings.EqualFold(strings.TrimSpace(labelStickiness), "true") + sticky := errSticky == nil && len(labelSticky) > 0 && strings.EqualFold(strings.TrimSpace(labelSticky), "true") + return stickiness || sticky } func (p *Provider) getStickinessCookieName(container dockerData) string { diff --git a/provider/docker/docker_test.go b/provider/docker/docker_test.go index a8f3a0bf0..0701dc2e2 100644 --- a/provider/docker/docker_test.go +++ b/provider/docker/docker_test.go @@ -10,6 +10,7 @@ import ( docker "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/go-connections/nat" + "github.com/stretchr/testify/assert" ) func TestDockerGetFrontendName(t *testing.T) { @@ -1051,3 +1052,80 @@ func TestDockerLoadDockerConfig(t *testing.T) { }) } } + +func TestDockerHasStickinessLabel(t *testing.T) { + testCases := []struct { + desc string + container docker.ContainerJSON + expected bool + }{ + { + desc: "no sticky/stickiness-label", + container: containerJSON(), + expected: false, + }, + { + desc: "sticky true", + container: containerJSON(labels(map[string]string{ + types.LabelBackendLoadbalancerSticky: "true", + })), + expected: true, + }, + { + desc: "sticky false", + container: containerJSON(labels(map[string]string{ + types.LabelBackendLoadbalancerSticky: "false", + })), + expected: false, + }, + { + desc: "stickiness true", + container: containerJSON(labels(map[string]string{ + types.LabelBackendLoadbalancerStickiness: "true", + })), + expected: true, + }, + { + desc: "stickiness false", + container: containerJSON(labels(map[string]string{ + types.LabelBackendLoadbalancerStickiness: "false", + })), + expected: false, + }, + { + desc: "sticky true + stickiness false", + container: containerJSON(labels(map[string]string{ + types.LabelBackendLoadbalancerSticky: "true", + types.LabelBackendLoadbalancerStickiness: "false", + })), + expected: true, + }, + { + desc: "sticky false + stickiness true", + container: containerJSON(labels(map[string]string{ + types.LabelBackendLoadbalancerSticky: "false", + types.LabelBackendLoadbalancerStickiness: "true", + })), + expected: true, + }, + { + desc: "sticky false + stickiness false", + container: containerJSON(labels(map[string]string{ + types.LabelBackendLoadbalancerSticky: "false", + types.LabelBackendLoadbalancerStickiness: "false", + })), + expected: false, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + dockerData := parseContainer(test.container) + provider := &Provider{} + actual := provider.hasStickinessLabel(dockerData) + assert.Equal(t, actual, test.expected) + }) + } +} diff --git a/provider/ecs/ecs.go b/provider/ecs/ecs.go index a8bb03365..d76e3f939 100644 --- a/provider/ecs/ecs.go +++ b/provider/ecs/ecs.go @@ -490,7 +490,7 @@ func (p *Provider) hasStickinessLabel(instances []ecsInstance) bool { stickyLabel := getFirstInstanceLabel(instances, types.LabelBackendLoadbalancerSticky) if len(stickyLabel) > 0 { - log.Warn("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) + log.Warnf("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) } stickiness := len(stickinessLabel) > 0 && strings.EqualFold(strings.TrimSpace(stickinessLabel), "true") sticky := len(stickyLabel) > 0 && strings.EqualFold(strings.TrimSpace(stickyLabel), "true") diff --git a/provider/kubernetes/kubernetes.go b/provider/kubernetes/kubernetes.go index a62f0c107..5d38b5af7 100644 --- a/provider/kubernetes/kubernetes.go +++ b/provider/kubernetes/kubernetes.go @@ -18,7 +18,6 @@ import ( "github.com/containous/traefik/log" "github.com/containous/traefik/provider" "github.com/containous/traefik/safe" - "github.com/containous/traefik/server/cookie" "github.com/containous/traefik/types" "k8s.io/client-go/pkg/api/v1" "k8s.io/client-go/pkg/apis/extensions/v1beta1" @@ -185,8 +184,8 @@ func (p *Provider) loadIngresses(k8sClient Client) (*types.Configuration, error) continue } - witelistSourceRangeAnnotation := i.Annotations[annotationKubernetesWhitelistSourceRange] - whitelistSourceRange := provider.SplitAndTrimString(witelistSourceRangeAnnotation) + whitelistSourceRangeAnnotation := i.Annotations[annotationKubernetesWhitelistSourceRange] + whitelistSourceRange := provider.SplitAndTrimString(whitelistSourceRangeAnnotation) if _, exists := templateObjects.Frontends[r.Host+pa.Path]; !exists { basicAuthCreds, err := handleBasicAuthConfig(i, k8sClient) @@ -250,12 +249,12 @@ func (p *Provider) loadIngresses(k8sClient Client) (*types.Configuration, error) } if len(service.Annotations[types.LabelBackendLoadbalancerSticky]) > 0 { - log.Warn("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) + log.Warnf("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) } if service.Annotations[types.LabelBackendLoadbalancerSticky] == "true" || service.Annotations[types.LabelBackendLoadbalancerStickiness] == "true" { templateObjects.Backends[r.Host+pa.Path].LoadBalancer.Stickiness = &types.Stickiness{ - CookieName: cookie.GenerateName(r.Host + pa.Path), + CookieName: r.Host + pa.Path, } } diff --git a/provider/kubernetes/kubernetes_test.go b/provider/kubernetes/kubernetes_test.go index 3ac0905fe..5e0a66bf9 100644 --- a/provider/kubernetes/kubernetes_test.go +++ b/provider/kubernetes/kubernetes_test.go @@ -1326,7 +1326,7 @@ func TestServiceAnnotations(t *testing.T) { LoadBalancer: &types.LoadBalancer{ Method: "wrr", Stickiness: &types.Stickiness{ - CookieName: "_4155f", + CookieName: "bar", }, }, }, diff --git a/provider/kv/kv.go b/provider/kv/kv.go index 36beb8650..c75d2f231 100644 --- a/provider/kv/kv.go +++ b/provider/kv/kv.go @@ -243,13 +243,17 @@ func (p *Provider) checkConstraints(keys ...string) bool { } func (p *Provider) hasStickinessLabel(rootPath string) bool { - stickiness, err := p.kvclient.Exists(rootPath + "/loadbalancer/stickiness") - if err != nil { - log.Debugf("Error occurs when check stickiness: %v", err) - } - sticky := p.get("false", rootPath, "/loadbalancer", "/sticky") + stickyValue := p.get("false", rootPath, "/loadbalancer", "/sticky") - return stickiness || (len(sticky) != 0 && strings.EqualFold(strings.TrimSpace(sticky), "true")) + sticky := len(stickyValue) != 0 && strings.EqualFold(strings.TrimSpace(stickyValue), "true") + if sticky { + log.Warnf("Deprecated configuration found: %s. Please use %s.", "loadbalancer/sticky", "loadbalancer/stickiness") + } + + stickinessValue := p.get("false", rootPath, "/loadbalancer", "/stickiness") + stickiness := len(stickinessValue) > 0 && strings.EqualFold(strings.TrimSpace(stickinessValue), "true") + + return stickiness || sticky } func (p *Provider) getStickinessCookieName(rootPath string) string { diff --git a/provider/kv/kv_mock_test.go b/provider/kv/kv_mock_test.go new file mode 100644 index 000000000..fddb2192e --- /dev/null +++ b/provider/kv/kv_mock_test.go @@ -0,0 +1,110 @@ +package kv + +import ( + "errors" + "strings" + + "github.com/containous/traefik/types" + "github.com/docker/libkv/store" +) + +type KvMock struct { + Provider +} + +func (provider *KvMock) loadConfig() *types.Configuration { + return nil +} + +// Override Get/List to return a error +type KvError struct { + Get error + List error +} + +// Extremely limited mock store so we can test initialization +type Mock struct { + Error KvError + KVPairs []*store.KVPair + WatchTreeMethod func() <-chan []*store.KVPair +} + +func (s *Mock) Put(key string, value []byte, opts *store.WriteOptions) error { + return errors.New("Put not supported") +} + +func (s *Mock) Get(key string) (*store.KVPair, error) { + if err := s.Error.Get; err != nil { + return nil, err + } + for _, kvPair := range s.KVPairs { + if kvPair.Key == key { + return kvPair, nil + } + } + return nil, store.ErrKeyNotFound +} + +func (s *Mock) Delete(key string) error { + return errors.New("Delete not supported") +} + +// Exists mock +func (s *Mock) Exists(key string) (bool, error) { + if err := s.Error.Get; err != nil { + return false, err + } + for _, kvPair := range s.KVPairs { + if strings.HasPrefix(kvPair.Key, key) { + return true, nil + } + } + return false, store.ErrKeyNotFound +} + +// Watch mock +func (s *Mock) Watch(key string, stopCh <-chan struct{}) (<-chan *store.KVPair, error) { + return nil, errors.New("Watch not supported") +} + +// WatchTree mock +func (s *Mock) WatchTree(prefix string, stopCh <-chan struct{}) (<-chan []*store.KVPair, error) { + return s.WatchTreeMethod(), nil +} + +// NewLock mock +func (s *Mock) NewLock(key string, options *store.LockOptions) (store.Locker, error) { + return nil, errors.New("NewLock not supported") +} + +// List mock +func (s *Mock) List(prefix string) ([]*store.KVPair, error) { + if err := s.Error.List; err != nil { + return nil, err + } + kv := []*store.KVPair{} + for _, kvPair := range s.KVPairs { + if strings.HasPrefix(kvPair.Key, prefix) && !strings.ContainsAny(strings.TrimPrefix(kvPair.Key, prefix), "/") { + kv = append(kv, kvPair) + } + } + return kv, nil +} + +// DeleteTree mock +func (s *Mock) DeleteTree(prefix string) error { + return errors.New("DeleteTree not supported") +} + +// AtomicPut mock +func (s *Mock) AtomicPut(key string, value []byte, previous *store.KVPair, opts *store.WriteOptions) (bool, *store.KVPair, error) { + return false, nil, errors.New("AtomicPut not supported") +} + +// AtomicDelete mock +func (s *Mock) AtomicDelete(key string, previous *store.KVPair) (bool, error) { + return false, errors.New("AtomicDelete not supported") +} + +// Close mock +func (s *Mock) Close() {} diff --git a/provider/kv/kv_test.go b/provider/kv/kv_test.go index 3daf2d98d..91aa64de8 100644 --- a/provider/kv/kv_test.go +++ b/provider/kv/kv_test.go @@ -1,10 +1,8 @@ package kv import ( - "errors" "reflect" "sort" - "strings" "testing" "time" @@ -237,14 +235,6 @@ func TestKvLast(t *testing.T) { } } -type KvMock struct { - Provider -} - -func (provider *KvMock) loadConfig() *types.Configuration { - return nil -} - func TestKvWatchTree(t *testing.T) { returnedChans := make(chan chan []*store.KVPair) provider := &KvMock{ @@ -288,91 +278,6 @@ func TestKvWatchTree(t *testing.T) { } } -// Override Get/List to return a error -type KvError struct { - Get error - List error -} - -// Extremely limited mock store so we can test initialization -type Mock struct { - Error KvError - KVPairs []*store.KVPair - WatchTreeMethod func() <-chan []*store.KVPair -} - -func (s *Mock) Put(key string, value []byte, opts *store.WriteOptions) error { - return errors.New("Put not supported") -} - -func (s *Mock) Get(key string) (*store.KVPair, error) { - if err := s.Error.Get; err != nil { - return nil, err - } - for _, kvPair := range s.KVPairs { - if kvPair.Key == key { - return kvPair, nil - } - } - return nil, store.ErrKeyNotFound -} - -func (s *Mock) Delete(key string) error { - return errors.New("Delete not supported") -} - -// Exists mock -func (s *Mock) Exists(key string) (bool, error) { - return false, errors.New("Exists not supported") -} - -// Watch mock -func (s *Mock) Watch(key string, stopCh <-chan struct{}) (<-chan *store.KVPair, error) { - return nil, errors.New("Watch not supported") -} - -// WatchTree mock -func (s *Mock) WatchTree(prefix string, stopCh <-chan struct{}) (<-chan []*store.KVPair, error) { - return s.WatchTreeMethod(), nil -} - -// NewLock mock -func (s *Mock) NewLock(key string, options *store.LockOptions) (store.Locker, error) { - return nil, errors.New("NewLock not supported") -} - -// List mock -func (s *Mock) List(prefix string) ([]*store.KVPair, error) { - if err := s.Error.List; err != nil { - return nil, err - } - kv := []*store.KVPair{} - for _, kvPair := range s.KVPairs { - if strings.HasPrefix(kvPair.Key, prefix) && !strings.ContainsAny(strings.TrimPrefix(kvPair.Key, prefix), "/") { - kv = append(kv, kvPair) - } - } - return kv, nil -} - -// DeleteTree mock -func (s *Mock) DeleteTree(prefix string) error { - return errors.New("DeleteTree not supported") -} - -// AtomicPut mock -func (s *Mock) AtomicPut(key string, value []byte, previous *store.KVPair, opts *store.WriteOptions) (bool, *store.KVPair, error) { - return false, nil, errors.New("AtomicPut not supported") -} - -// AtomicDelete mock -func (s *Mock) AtomicDelete(key string, previous *store.KVPair) (bool, error) { - return false, errors.New("AtomicDelete not supported") -} - -// Close mock -func (s *Mock) Close() {} - func TestKVLoadConfig(t *testing.T) { provider := &Provider{ Prefix: "traefik", @@ -463,3 +368,97 @@ func TestKVLoadConfig(t *testing.T) { t.Fatalf("expected %+v, got %+v", expected.Frontends, actual.Frontends) } } + +func TestKVHasStickinessLabel(t *testing.T) { + testCases := []struct { + desc string + KVPairs []*store.KVPair + expected bool + }{ + { + desc: "without option", + expected: false, + }, + { + desc: "with cookie name without stickiness=true", + KVPairs: []*store.KVPair{ + { + Key: "loadbalancer/stickiness/cookiename", + Value: []byte("foo"), + }, + }, + expected: false, + }, + { + desc: "stickiness=true", + KVPairs: []*store.KVPair{ + { + Key: "loadbalancer/stickiness", + Value: []byte("true"), + }, + }, + expected: true, + }, + { + desc: "stickiness=true and sticky=true", + KVPairs: []*store.KVPair{ + { + Key: "loadbalancer/stickiness", + Value: []byte("true"), + }, + { + Key: "loadbalancer/sticky", + Value: []byte("true"), + }, + }, + expected: true, + }, + { + desc: "stickiness=false and sticky=true", + KVPairs: []*store.KVPair{ + { + Key: "loadbalancer/stickiness", + Value: []byte("false"), + }, + { + Key: "loadbalancer/sticky", + Value: []byte("true"), + }, + }, + expected: true, + }, + { + desc: "stickiness=true and sticky=false", + KVPairs: []*store.KVPair{ + { + Key: "loadbalancer/stickiness", + Value: []byte("true"), + }, + { + Key: "loadbalancer/sticky", + Value: []byte("false"), + }, + }, + expected: true, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + p := &Provider{ + kvclient: &Mock{ + KVPairs: test.KVPairs, + }, + } + + actual := p.hasStickinessLabel("") + + if actual != test.expected { + t.Fatalf("expected %v, got %v", test.expected, actual) + } + }) + } +} diff --git a/provider/marathon/marathon.go b/provider/marathon/marathon.go index d8639b13c..982306bf7 100644 --- a/provider/marathon/marathon.go +++ b/provider/marathon/marathon.go @@ -430,14 +430,16 @@ func (p *Provider) getProtocol(application marathon.Application, serviceName str } func (p *Provider) hasStickinessLabel(application marathon.Application) bool { - _, okStickiness := p.getAppLabel(application, types.LabelBackendLoadbalancerStickiness) + labelStickiness, okStickiness := p.getAppLabel(application, types.LabelBackendLoadbalancerStickiness) - label, okSticky := p.getAppLabel(application, types.LabelBackendLoadbalancerSticky) - if len(label) > 0 { - log.Warn("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) + labelSticky, okSticky := p.getAppLabel(application, types.LabelBackendLoadbalancerSticky) + if len(labelSticky) > 0 { + log.Warnf("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) } - return okStickiness || (okSticky && strings.EqualFold(strings.TrimSpace(label), "true")) + stickiness := okStickiness && len(labelStickiness) > 0 && strings.EqualFold(strings.TrimSpace(labelStickiness), "true") + sticky := okSticky && len(labelSticky) > 0 && strings.EqualFold(strings.TrimSpace(labelSticky), "true") + return stickiness || sticky } func (p *Provider) getStickinessCookieName(application marathon.Application) string { diff --git a/provider/marathon/marathon_test.go b/provider/marathon/marathon_test.go index e78fd3f0b..145cefe5e 100644 --- a/provider/marathon/marathon_test.go +++ b/provider/marathon/marathon_test.go @@ -856,7 +856,7 @@ func TestMarathonGetProtocol(t *testing.T) { } func TestMarathonHasStickinessLabel(t *testing.T) { - cases := []struct { + testCases := []struct { desc string application marathon.Application expected bool @@ -867,35 +867,50 @@ func TestMarathonHasStickinessLabel(t *testing.T) { expected: false, }, { - desc: "label existing and value equals true (deprecated)", + desc: "sticky=true (deprecated)", application: application(label(types.LabelBackendLoadbalancerSticky, "true")), expected: true, }, { - desc: "label existing and value equals false (deprecated)", + desc: "sticky=false (deprecated)", application: application(label(types.LabelBackendLoadbalancerSticky, "false")), expected: false, }, { - desc: "label existing and value equals true", + desc: "stickiness=true", application: application(label(types.LabelBackendLoadbalancerStickiness, "true")), expected: true, }, { - desc: "label existing and value equals false ", + desc: "stickiness=false ", application: application(label(types.LabelBackendLoadbalancerStickiness, "true")), expected: true, }, + { + desc: "sticky=false stickiness=true ", + application: application( + label(types.LabelBackendLoadbalancerStickiness, "true"), + label(types.LabelBackendLoadbalancerSticky, "false")), + expected: true, + }, + { + desc: "sticky=true stickiness=false ", + application: application( + label(types.LabelBackendLoadbalancerStickiness, "false"), + label(types.LabelBackendLoadbalancerSticky, "true")), + expected: true, + }, } - for _, c := range cases { - c := c - t.Run(c.desc, func(t *testing.T) { + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { t.Parallel() + provider := &Provider{} - actual := provider.hasStickinessLabel(c.application) - if actual != c.expected { - t.Errorf("actual %q, expected %q", actual, c.expected) + actual := provider.hasStickinessLabel(test.application) + if actual != test.expected { + t.Errorf("actual %q, expected %q", actual, test.expected) } }) } diff --git a/provider/rancher/rancher.go b/provider/rancher/rancher.go index 6327f460b..948c3c30b 100644 --- a/provider/rancher/rancher.go +++ b/provider/rancher/rancher.go @@ -113,14 +113,16 @@ func (p *Provider) getCircuitBreakerExpression(service rancherData) string { } func (p *Provider) hasStickinessLabel(service rancherData) bool { - _, errStickiness := getServiceLabel(service, types.LabelBackendLoadbalancerStickiness) + labelStickiness, errStickiness := getServiceLabel(service, types.LabelBackendLoadbalancerStickiness) - label, errSticky := getServiceLabel(service, types.LabelBackendLoadbalancerSticky) - if len(label) > 0 { - log.Warn("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) + labelSticky, errSticky := getServiceLabel(service, types.LabelBackendLoadbalancerSticky) + if len(labelSticky) > 0 { + log.Warnf("Deprecated configuration found: %s. Please use %s.", types.LabelBackendLoadbalancerSticky, types.LabelBackendLoadbalancerStickiness) } - return errStickiness == nil || (errSticky == nil && strings.EqualFold(strings.TrimSpace(label), "true")) + stickiness := errStickiness == nil && len(labelStickiness) > 0 && strings.EqualFold(strings.TrimSpace(labelStickiness), "true") + sticky := errSticky == nil && len(labelSticky) > 0 && strings.EqualFold(strings.TrimSpace(labelSticky), "true") + return stickiness || sticky } func (p *Provider) getStickinessCookieName(service rancherData, backendName string) string { diff --git a/provider/rancher/rancher_test.go b/provider/rancher/rancher_test.go index 7fcf03568..d56b552e9 100644 --- a/provider/rancher/rancher_test.go +++ b/provider/rancher/rancher_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/containous/traefik/types" + "github.com/stretchr/testify/assert" ) func TestRancherServiceFilter(t *testing.T) { @@ -597,3 +598,97 @@ func TestRancherLoadRancherConfig(t *testing.T) { } } } + +func TestRancherHasStickinessLabel(t *testing.T) { + provider := &Provider{ + Domain: "rancher.localhost", + } + + testCases := []struct { + desc string + service rancherData + expected bool + }{ + { + desc: "no labels", + service: rancherData{ + Name: "test-service", + }, + expected: false, + }, + { + desc: "sticky=true", + service: rancherData{ + Name: "test-service", + Labels: map[string]string{ + types.LabelBackendLoadbalancerSticky: "true", + }, + }, + expected: true, + }, + { + desc: "stickiness=true", + service: rancherData{ + Name: "test-service", + Labels: map[string]string{ + types.LabelBackendLoadbalancerStickiness: "true", + }, + }, + expected: true, + }, + { + desc: "sticky=true and stickiness=true", + service: rancherData{ + Name: "test-service", + Labels: map[string]string{ + types.LabelBackendLoadbalancerSticky: "true", + types.LabelBackendLoadbalancerStickiness: "true", + }, + }, + expected: true, + }, + { + desc: "sticky=false and stickiness=false", + service: rancherData{ + Name: "test-service", + Labels: map[string]string{ + types.LabelBackendLoadbalancerSticky: "false", + types.LabelBackendLoadbalancerStickiness: "false", + }, + }, + expected: false, + }, + { + desc: "sticky=true and stickiness=false", + service: rancherData{ + Name: "test-service", + Labels: map[string]string{ + types.LabelBackendLoadbalancerSticky: "true", + types.LabelBackendLoadbalancerStickiness: "false", + }, + }, + expected: true, + }, + { + desc: "sticky=false and stickiness=true", + service: rancherData{ + Name: "test-service", + Labels: map[string]string{ + types.LabelBackendLoadbalancerSticky: "false", + types.LabelBackendLoadbalancerStickiness: "true", + }, + }, + expected: true, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + actual := provider.hasStickinessLabel(test.service) + assert.Equal(t, actual, test.expected) + }) + } +} diff --git a/server/server.go b/server/server.go index 24360d967..3a5a76cb9 100644 --- a/server/server.go +++ b/server/server.go @@ -1166,25 +1166,29 @@ func (server *Server) configureFrontends(frontends map[string]*types.Frontend) { func (*Server) configureBackends(backends map[string]*types.Backend) { for backendName, backend := range backends { if backend.LoadBalancer != nil && backend.LoadBalancer.Sticky { - log.Warn("Deprecated configuration found: %s. Please use %s.", "backend.LoadBalancer.Sticky", "backend.LoadBalancer.Stickiness") + log.Warnf("Deprecated configuration found: %s. Please use %s.", "backend.LoadBalancer.Sticky", "backend.LoadBalancer.Stickiness") } _, err := types.NewLoadBalancerMethod(backend.LoadBalancer) if err == nil { if backend.LoadBalancer != nil && backend.LoadBalancer.Stickiness == nil && backend.LoadBalancer.Sticky { - backend.LoadBalancer.Stickiness = &types.Stickiness{} + backend.LoadBalancer.Stickiness = &types.Stickiness{ + CookieName: "_TRAEFIK_BACKEND", + } } } else { log.Debugf("Validation of load balancer method for backend %s failed: %s. Using default method wrr.", backendName, err) var stickiness *types.Stickiness if backend.LoadBalancer != nil { - if backend.LoadBalancer.Stickiness != nil { - stickiness = backend.LoadBalancer.Stickiness - } else if backend.LoadBalancer.Sticky { - if backend.LoadBalancer.Stickiness == nil { - stickiness = &types.Stickiness{} + if backend.LoadBalancer.Stickiness == nil { + if backend.LoadBalancer.Sticky { + stickiness = &types.Stickiness{ + CookieName: "_TRAEFIK_BACKEND", + } } + } else { + stickiness = backend.LoadBalancer.Stickiness } } backend.LoadBalancer = &types.LoadBalancer{ diff --git a/templates/consul_catalog.tmpl b/templates/consul_catalog.tmpl index b055a4aa4..93777c55c 100644 --- a/templates/consul_catalog.tmpl +++ b/templates/consul_catalog.tmpl @@ -21,7 +21,7 @@ sticky = {{getAttribute "backend.loadbalancer.sticky" .Attributes "false"}} {{if hasStickinessLabel .Attributes}} [Backends."backend-{{$service}}".LoadBalancer.Stickiness] - cookieName = {{getStickinessCookieName .Attributes}} + cookieName = "{{getStickinessCookieName .Attributes}}" {{end}} {{if hasMaxconnAttributes .Attributes}} diff --git a/templates/docker.tmpl b/templates/docker.tmpl index 6e18a9904..ef6f69526 100644 --- a/templates/docker.tmpl +++ b/templates/docker.tmpl @@ -10,7 +10,7 @@ method = "{{getLoadBalancerMethod $backend}}" {{if hasStickinessLabel $backend}} [Backends."{{$backendName}}".LoadBalancer.Stickiness] - cookieName = {{getStickinessCookieName $backend}} + cookieName = "{{getStickinessCookieName $backend}}" {{end}} {{end}} diff --git a/templates/ecs.tmpl b/templates/ecs.tmpl index a325f7539..f42642ad4 100644 --- a/templates/ecs.tmpl +++ b/templates/ecs.tmpl @@ -3,7 +3,7 @@ method = "{{ getLoadBalancerMethod $instances}}" {{if hasStickinessLabel $instances}} [Backends.backend-{{ $serviceName }}.LoadBalancer.Stickiness] - cookieName = {{getStickinessCookieName $instances}} + cookieName = "{{getStickinessCookieName $instances}}" {{end}} {{range $index, $i := $instances}} diff --git a/templates/kubernetes.tmpl b/templates/kubernetes.tmpl index 47fec081c..bab94ae19 100644 --- a/templates/kubernetes.tmpl +++ b/templates/kubernetes.tmpl @@ -8,7 +8,7 @@ method = "{{$backend.LoadBalancer.Method}}" {{if $backend.LoadBalancer.Stickiness}} [Backends."{{$backendName}}".LoadBalancer.Stickiness] - cookieName = {{$backend.LoadBalancer.Stickiness.CookieName}} + cookieName = "{{$backend.LoadBalancer.Stickiness.CookieName}}" {{end}} {{range $serverName, $server := $backend.Servers}} [backends."{{$backendName}}".servers."{{$serverName}}"] diff --git a/templates/marathon.tmpl b/templates/marathon.tmpl index e5c802a63..aaa7f2e6a 100644 --- a/templates/marathon.tmpl +++ b/templates/marathon.tmpl @@ -22,7 +22,7 @@ method = "{{getLoadBalancerMethod $app }}" {{if hasStickinessLabel $app}} [Backends."backend{{getBackend $app $serviceName }}".LoadBalancer.Stickiness] - cookieName = {{getStickinessCookieName $app}} + cookieName = "{{getStickinessCookieName $app}}" {{end}} {{end}} {{ if hasCircuitBreakerLabels $app }} diff --git a/templates/rancher.tmpl b/templates/rancher.tmpl index 90a10ac56..1f911e5b2 100644 --- a/templates/rancher.tmpl +++ b/templates/rancher.tmpl @@ -10,7 +10,7 @@ method = "{{getLoadBalancerMethod $backend}}" {{if hasStickinessLabel $backend}} [Backends."{{$backendName}}".LoadBalancer.Stickiness] - cookieName = {{getStickinessCookieName $backend}} + cookieName = "{{getStickinessCookieName $backend}}" {{end}} {{end}}