From 592a12dca23e2292f93a15980a4f1f51b37aafcf Mon Sep 17 00:00:00 2001 From: Diego de Oliveira Date: Sun, 26 Mar 2017 16:59:08 -0300 Subject: [PATCH] Fix unsound behavior The IP-Per-Task feature changed the behavior for clients without this configuration (using the task IP instead of task hostname). This patch make the new behavior available just for Mesos installation with IP-Per-Task enabled. It also make it possible to force the use of task's hostname. --- docs/toml.md | 10 +++ provider/marathon/marathon.go | 11 ++- provider/marathon/marathon_test.go | 132 ++++++++++++++++++++++++----- traefik.sample.toml | 10 +++ 4 files changed, 137 insertions(+), 26 deletions(-) diff --git a/docs/toml.md b/docs/toml.md index d34eea065..e3d4ae678 100644 --- a/docs/toml.md +++ b/docs/toml.md @@ -969,6 +969,16 @@ domain = "marathon.localhost" # Default: "10s" # # keepAlive = "10s" + +# By default, a task's IP address (as returned by the Marathon API) is used as +# backend server if an IP-per-task configuration can be found; otherwise, the +# name of the host running the task is used. +# The latter behavior can be enforced by enabling this switch. +# +# Optional +# Default: false +# +# forceTaskHostname: false ``` Labels can be used on containers to override default behaviour: diff --git a/provider/marathon/marathon.go b/provider/marathon/marathon.go index 0cd3c6177..e92e9d577 100644 --- a/provider/marathon/marathon.go +++ b/provider/marathon/marathon.go @@ -42,6 +42,7 @@ type Provider struct { TLS *provider.ClientTLS `description:"Enable Docker TLS support"` DialerTimeout flaeg.Duration `description:"Set a non-default connection timeout for Marathon"` KeepAlive flaeg.Duration `description:"Set a non-default TCP Keep Alive time in seconds"` + ForceTaskHostname bool `description:"Force to use the task IP hostname."` Basic *Basic marathonClient marathon.Marathon } @@ -526,11 +527,15 @@ func (p *Provider) getBackendServer(task marathon.Task, applications []marathon. log.Errorf("Unable to get marathon application from task %s", task.AppID) return "" } - if len(task.IPAddresses) == 0 { + switch { + case application.IPAddressPerTask == nil || p.ForceTaskHostname: + return task.Host + case len(task.IPAddresses) == 0: + log.Errorf("Missing marathon IPAddress from task %s", task.AppID) return "" - } else if len(task.IPAddresses) == 1 { + case len(task.IPAddresses) == 1: return task.IPAddresses[0].IPAddress - } else { + default: ipAddressIdxStr, ok := p.getLabel(application, "traefik.ipAddressIdx") if !ok { log.Errorf("Unable to get marathon IPAddress from task %s", task.AppID) diff --git a/provider/marathon/marathon_test.go b/provider/marathon/marathon_test.go index 7fc6bbfa9..0a67f7223 100644 --- a/provider/marathon/marathon_test.go +++ b/provider/marathon/marathon_test.go @@ -1,10 +1,13 @@ package marathon import ( + "encoding/json" "errors" "reflect" "testing" + "fmt" + "github.com/containous/traefik/mocks" "github.com/containous/traefik/testhelpers" "github.com/containous/traefik/types" @@ -108,7 +111,7 @@ func TestMarathonLoadConfig(t *testing.T) { "backend-test": { Servers: map[string]types.Server{ "server-test": { - URL: "http://127.0.0.1:80", + URL: "http://localhost:80", Weight: 0, }, }, @@ -161,7 +164,7 @@ func TestMarathonLoadConfig(t *testing.T) { "backend-testLoadBalancerAndCircuitBreaker.dot": { Servers: map[string]types.Server{ "server-testLoadBalancerAndCircuitBreaker-dot": { - URL: "http://127.0.0.1:80", + URL: "http://localhost:80", Weight: 0, }, }, @@ -219,7 +222,7 @@ func TestMarathonLoadConfig(t *testing.T) { "backend-testMaxConn": { Servers: map[string]types.Server{ "server-testMaxConn": { - URL: "http://127.0.0.1:80", + URL: "http://localhost:80", Weight: 0, }, }, @@ -274,7 +277,7 @@ func TestMarathonLoadConfig(t *testing.T) { "backend-testMaxConnOnlySpecifyAmount": { Servers: map[string]types.Server{ "server-testMaxConnOnlySpecifyAmount": { - URL: "http://127.0.0.1:80", + URL: "http://localhost:80", Weight: 0, }, }, @@ -326,7 +329,7 @@ func TestMarathonLoadConfig(t *testing.T) { "backend-testMaxConnOnlyExtractorFunc": { Servers: map[string]types.Server{ "server-testMaxConnOnlyExtractorFunc": { - URL: "http://127.0.0.1:80", + URL: "http://localhost:80", Weight: 0, }, }, @@ -337,27 +340,37 @@ func TestMarathonLoadConfig(t *testing.T) { } for _, c := range cases { - fakeClient := newFakeClient(c.applicationsError, c.applications, c.tasksError, c.tasks) - provider := &Provider{ - Domain: "docker.localhost", - ExposedByDefault: true, - marathonClient: fakeClient, + appID := "" + if len(c.applications.Apps) > 0 { + appID = c.applications.Apps[0].ID } - actualConfig := provider.loadMarathonConfig() - fakeClient.AssertExpectations(t) - if c.expectedNil { - if actualConfig != nil { - t.Fatalf("Should have been nil, got %v", actualConfig) + t.Run(fmt.Sprintf("Running case: %s", appID), func(t *testing.T) { + fakeClient := newFakeClient(c.applicationsError, c.applications, c.tasksError, c.tasks) + provider := &Provider{ + Domain: "docker.localhost", + ExposedByDefault: true, + marathonClient: fakeClient, } - } else { - // Compare backends - if !reflect.DeepEqual(actualConfig.Backends, c.expectedBackends) { - t.Fatalf("expected %#v, got %#v", c.expectedBackends, actualConfig.Backends) + actualConfig := provider.loadMarathonConfig() + fakeClient.AssertExpectations(t) + if c.expectedNil { + if actualConfig != nil { + t.Fatalf("Should have been nil, got %v", actualConfig) + } + } else { + // Compare backends + if !reflect.DeepEqual(actualConfig.Backends, c.expectedBackends) { + expected, _ := json.Marshal(c.expectedBackends) + actual, _ := json.Marshal(actualConfig.Backends) + t.Fatalf("expected\t %s\n, \tgot %s\n", expected, actual) + } + if !reflect.DeepEqual(actualConfig.Frontends, c.expectedFrontends) { + expected, _ := json.Marshal(c.expectedFrontends) + actual, _ := json.Marshal(actualConfig.Frontends) + t.Fatalf("expected\t %s\n, got\t %s\n", expected, actual) + } } - if !reflect.DeepEqual(actualConfig.Frontends, c.expectedFrontends) { - t.Fatalf("expected %#v, got %#v", c.expectedFrontends, actualConfig.Frontends) - } - } + }) } } @@ -1451,3 +1464,76 @@ func TestMarathonGetSubDomain(t *testing.T) { } } } + +func TestGetBackendServer(t *testing.T) { + + applications := []struct { + forceTaskHostname bool + application marathon.Application + expected string + }{ + { + application: marathon.Application{ + ID: "app-without-IP-per-task", + }, + expected: "sample.com", + }, + { + application: marathon.Application{ + + ID: "app-with-IP-per-task", + IPAddressPerTask: &marathon.IPAddressPerTask{ + Discovery: &marathon.Discovery{ + Ports: &[]marathon.Port{ + { + Number: 8880, + Name: "p1", + }, + }, + }, + }, + }, + expected: "192.168.0.1", + }, { + forceTaskHostname: true, + application: marathon.Application{ + ID: "app-with-hostname-enforced", + IPAddressPerTask: &marathon.IPAddressPerTask{ + Discovery: &marathon.Discovery{ + Ports: &[]marathon.Port{ + { + Number: 8880, + Name: "p1", + }, + }, + }, + }, + }, + expected: "sample.com", + }, + } + + for _, app := range applications { + t.Run(fmt.Sprintf("running %s", app.application.ID), func(t *testing.T) { + provider := &Provider{} + provider.ForceTaskHostname = app.forceTaskHostname + + applications := []marathon.Application{app.application} + + task := marathon.Task{ + AppID: app.application.ID, + Host: "sample.com", + IPAddresses: []*marathon.IPAddress{ + { + IPAddress: "192.168.0.1", + }, + }, + } + + actual := provider.getBackendServer(task, applications) + if actual != app.expected { + t.Errorf("App %s, expected %q, got %q", task.AppID, app.expected, actual) + } + }) + } +} diff --git a/traefik.sample.toml b/traefik.sample.toml index a2a6a8f5d..dc466dd49 100644 --- a/traefik.sample.toml +++ b/traefik.sample.toml @@ -599,6 +599,16 @@ # # keepAlive = "10s" +# By default, a task's IP address (as returned by the Marathon API) is used as +# backend server if an IP-per-task configuration can be found; otherwise, the +# name of the host running the task is used. +# The latter behavior can be enforced by enabling this switch. +# +# Optional +# Default: false +# +# forceTaskHostname: false + ################################################################ # Mesos configuration backend ################################################################