Support sticky sessions under SWARM Mode. #1024 #1033

foleymic · 2017-01-11T15:39:13Z

SWARM Mode has it's own built in Load balancer, so if we want to leverage sticky sessions, or if we would just prefer to bypass it and go directly to the containers (aka tasks), via
--label traefik.backend.bypass.swarm.loadbalancer=true
then we need to let Traefik know about the underlying tasks and register them as services within it's backend.

See #1024 for more details

emilevauge

Hey @foleymic! Thanks a lot for your contribution!
I have few comments:

I don't really like the option name traefik.backend.bypass.swarm.loadbalancer ;) It's too long.
I would get this option as default, and rename the option to traefik.backend.loadbalancer.swarm instead.
Could you complete documentation in doc/?

@vdemeester WDYT?

emilevauge · 2017-01-11T18:13:34Z

provider/docker.go


 	for _, service := range serviceList {
 		dockerData := parseService(service, networkMap)

-		dockerDataList = append(dockerDataList, dockerData)
+		sticky, _ := strconv.ParseBool(provider.getSticky(dockerData))


I think we should remove this line. We don't need to know the stickiness to bypass or not the swarm LB.

I did this because you need to automatically bypass the swarm LB if you want to use sticky sessions. Basically if you explicitly set the flag to bypass or you are using sticky sessions then you should bypass the swarm LB.

I made this change. Thanks.

joke · 2017-01-11T19:32:35Z

Hi,

out of curiosity how does this relate to the swarm dnsrr endpoint mode. From a docker swarm point of view that mode is supposed to be used for loadbalancing. There are is an traefik issues regarding this topic: #833 as well.

foleymic · 2017-01-11T20:50:49Z

@emilevauge - thanks for the option name suggestion, I agree that it is way to verbose. Just so I'm clear, with the new name, are you suggesting that when traefik.backend.loadbalancer.swarm=true we use Swarm's internal load balancer (and sticky won't work) and that we make the default to bypass? I think that makes sense and would fit better in with what Traefik is doing, but wanted to make sure this was your intent.

foleymic · 2017-01-11T21:09:50Z

@joke - The intention of this PR is to bypass the Docker's swarm enpoint dssrr. Without this change, Traefik will only register the swarm service as a single server backend, even if that service has multiple containers behind it. In that case, there is no way for Traefik to honor sticky sessions because he is always directing traffic to a single VIP. From there, Swarm would do the round robin load balancing between the individual containers/tasks.
Regarding #833, with this option set, it will essentially avoid the issue seen in that ticket, but this is only because you no longer hit the swarm VIP, and let Traefik round robin the requests to the containers.

emilevauge · 2017-01-12T12:21:14Z

@foleymic

are you suggesting that when traefik.backend.loadbalancer.swarm=true we use Swarm's internal load balancer (and sticky won't work) and that we make the default to bypass? I think that makes sense and would fit better in with what Traefik is doing, but wanted to make sure this was your intent.

Exactly :)

foleymic · 2017-01-12T13:29:11Z

@emilevauge got it. Thanks. I will make the change today.

emilevauge · 2017-01-19T17:15:53Z

docs/toml.md

@@ -795,6 +795,7 @@ Labels can be used on containers to override default behaviour:
 - `traefik.backend.maxconn.extractorfunc=client.ip`: set the function to be used against the request to determine what to limit maximum connections to the backend by. Must be used in conjunction with the above label to take effect.
 - `traefik.backend.loadbalancer.method=drr`: override the default `wrr` load balancer algorithm
 - `traefik.backend.loadbalancer.sticky=true`: enable backend sticky sessions
+- `traefik.backend.loadbalancer.swarm=true `: use Swarm's inbuilt load balncer (only relevant under Swarm Mode).


*balancer ;)

@emilevauge - thanks for catching that. Fixing it now.

emilevauge

One small comment ;)
Other than that LGTM!
Could you also rebase your PR?
/cc @containous/traefik

foleymic · 2017-01-19T17:46:27Z

Thanks @emilevauge. I rebased and corrected toml.md.

vdemeester

Few comments. I need to try this out still 👼

vdemeester · 2017-01-19T18:05:25Z

docs/user-guide/swarm-mode.md

 docker-machine ssh manager "docker service create \
 	--name whoami1 \
 	--label traefik.port=80 \
 	--network traefik-net \
+	--label traefik.frontend.rule=Host:whoam1.traefik \
+	--label traefik.backend=whoami1 \
+	--label traefik.backend.loadbalancer.sticky=true \


Is this up-to-date ? (isn't it supposed to be traefik.backend.loadbalance.swarm ?)

This is correct. Default behavior is traefik.backend.loadbalancer.swarm=true, so I left it out here but used the existing sticky flag to demonstrate that we can maintain sticky sessions under swarm mode. Let me know if you think I should include traefik.backend.loadbalancer.swarm=true in the guide as well.

Ohh I see 😅

@vdemeester - I pushed your other recommended changes.

Thanks,
Mike

vdemeester · 2017-01-19T18:09:54Z

provider/docker.go

+
+func listTasks(ctx context.Context, dockerClient client.APIClient, serviceID string,
+	serviceDockerData dockerData, networkMap map[string]*dockertypes.NetworkResource) ([]dockerData, error) {
+	taskList, err := dockerClient.TaskList(ctx, dockertypes.TaskListOptions{})


You should be able to use filters here (and thus not get all tasks and having to iterate over it).

filters := filters.NewArgs() filters.Add("service", serviceID) taskList, err := dockerClient.TaskList(ctx, dockertypes.TaskListOptions{Filters: filters}) // […]

Good idea - I'll make that change.

vdemeester · 2017-01-19T18:11:16Z

provider/docker.go

+	if task.NetworksAttachments != nil {
+		dockerData.NetworkSettings.Networks = make(map[string]*networkData)
+		for _, virtualIP := range task.NetworksAttachments {
+			networkService := networkMap[virtualIP.Network.ID]


Could be the following

if networkService, present := networkMap[virtualIP.Network.ID]; present { // […] }

Thanks @vdemeester. I'm trying this out now.

foleymic · 2017-01-26T20:07:04Z

@vdemeester - I just wanted to reach out to see if you have any other concerns or changes you would like on this PR.

vdemeester

This is currently not working as expected, any value for the traefik.backend.loadbalancer.swarm label means true which is wrong. getIsBackendLBSwarm should return the value of the label if present.
This changes the default behavior : the swarm load-balancer is disabled by default with this PR. ping @emilevauge @containous/traefik to discuss if that's what we want or not.

When the swarm load balancer is disabled, I'm getting 404 … not sure why…

vdemeester · 2017-02-01T14:28:32Z

provider/docker.go

@@ -463,6 +463,13 @@ func (provider *Docker) getSticky(container dockerData) string {
 	return "false"
 }

+func (provider *Docker) getIsBackendLBSwarm(container dockerData) string {
+	if _, err := getLabel(container, "traefik.backend.loadbalancer.swarm"); err == nil {
+		return "true"


I'm not sure that's right here. It will always return true, whatever the value of the label is — and false otherwise 😅

Yeah, I meant to question that when I noticed that was how the sticky label was handled (getSticky) - I ended up doing the same thing here to be consistent. Shall I correct them both?

I'll look into the 404 error as well.

ohhh 😓 yeah, I would think it needs to be fixed (both)

emilevauge · 2017-02-01T18:11:52Z

@vdemeester

This changes the default behavior : the swarm load-balancer is disabled by default with this PR. ping @emilevauge @containous/traefik to discuss if that's what we want or not.

#1033 (comment)

I think this is a good idea to NOT use Swarm LB by default.

vdemeester · 2017-02-01T21:33:00Z

I think this is a good idea to NOT use Swarm LB by default.

SGTM, was asking just to make sure 👼

emilevauge · 2017-02-02T17:02:41Z

@foleymic we are going to freeze code for v1.2 tomorrow. If you want your PR to be in this release, please address #1033 (review) :)

foleymic · 2017-02-02T22:58:49Z

@emilevauge & @vdemeester - I just pushed the requested changes. I was not able to reproduce the 404 errors that Vincent mentioned. Let me know if you're still seeing it.

foleymic · 2017-02-03T00:38:33Z

@vdemeester - I see the 404 now. If I don't include the front-end rule I get it. I'm looking at it now. Because in non-swarm mode I'm setting the dockerData.name to be the service name + the task slot number (e.g. whoami0.1) so if the front-end rule is not provided traefik will generate one for you off of the this dockerData.name (as container.name). I guess that means it's not matching with the backend somewhere because of this.

I fixed this by adding a new string to the dockerData struct called ServiceName. In swarm mode when using traefik to load balance between containers (traefik.backend.loadbalancer.swarm=false) ServiceName will be the name of the Service, while Name will be the Name of the task (e.g. whoami0.1). If traefik.backend.loadbalancer.swarm=true then ServiceName and Name will be the same.

emilevauge · 2017-02-03T14:20:29Z

@foleymic Docker integration tests are failing

foleymic · 2017-02-03T16:04:26Z

@emilevauge Sorry for not catching the test errors before I pushed last night. The issue is that when testing the front end and backend rules, dockerDate struct is manually created and is still relying on the container name versus the service name. Any concerns with me updating the tests to use ServiceName instead?

emilevauge · 2017-02-03T16:05:49Z

@foleymic Not sure, seems like a breaking change no ?

foleymic · 2017-02-03T16:24:05Z

@emilevauge Yeah, I was hesitant to suggest it at first. The whole issue I had was due to the way the rules were generated based off the container name versus the service name. Prior to this PR, they were the same in swarm mode because it never went down to the task/container level. Now with this PR, I added ServiceName and left Name to mean be the container name (which is what I thought the original intent was by looking at the code). My first pass at fixing this was to add taskname to the struct and leave name as is, but I was worried about not knowing when we needed the actual container instance (task name) versus the service name. Let me take a look down that path again - it may be a safer approach.

Update - I found a much simpler solution. Just set dockerData.ServiceName to dockerData.Name in parseContainer. Now that I understand the tests a bit better, I will also try to add some swarm mode tests as a seperate PR.

emilevauge · 2017-02-05T11:53:45Z

ping @vdemeester :)

SWARM Mode has it's own built in Load balancer, so if we want to leverage sticky sessions, or if we would just prefer to bypass it and go directly to the containers (aka tasks), via --label traefik.backend.disable.swarm.loadbalancer=true then we need to let Traefik know about the underlying tasks and register them as services within it's backend.

In Swarm mode with with Docker Swarm’s Load Balancer disabled (traefik.backend.loadbalancer.swarm=false) service name will be the name of the docker service and name will be the container task name (e.g. whoami0.1). When generating backend and fronted rules, we will use service name instead of name if a rule is not provided. Initialize dockerData.ServiceName to dockerData.Name to support non-swarm mode.

vdemeester

LGTM 🐸 🐳 🐝

kevtainer · 2017-11-02T20:46:26Z

I'm fairly confused by this ticket. Are we able to use dns-rr mode with the traefik network with swarm lb disabled? I see a reference to #833 but no clear indication whether or not it is resolved by this change.

vdemeester added the status/1-needs-design-review label Jan 11, 2017

vdemeester requested review from vdemeester and emilevauge January 11, 2017 15:49

vdemeester added the area/provider/docker label Jan 11, 2017

vdemeester self-assigned this Jan 11, 2017

foleymic force-pushed the feature-1024 branch 3 times, most recently from ed1eabf to 1788aef Compare January 11, 2017 16:10

emilevauge requested changes Jan 11, 2017

View reviewed changes

foleymic force-pushed the feature-1024 branch 2 times, most recently from 3e5827c to 59cec1c Compare January 13, 2017 02:07

emilevauge reviewed Jan 19, 2017

View reviewed changes

emilevauge approved these changes Jan 19, 2017

View reviewed changes

foleymic force-pushed the feature-1024 branch from 59cec1c to 8a911e5 Compare January 19, 2017 17:45

vdemeester added status/2-needs-review and removed status/1-needs-design-review labels Jan 19, 2017

vdemeester suggested changes Jan 19, 2017

View reviewed changes

foleymic force-pushed the feature-1024 branch 3 times, most recently from e5aa77c to b2f6657 Compare January 19, 2017 21:54

rebaseTraefik force-pushed the feature-1024 branch from b2f6657 to f966b41 Compare January 20, 2017 13:43

vdemeester force-pushed the feature-1024 branch from f966b41 to 5c2d498 Compare January 20, 2017 14:17

emilevauge force-pushed the feature-1024 branch 2 times, most recently from 9f4a9dd to 630012c Compare January 24, 2017 15:45

emilevauge force-pushed the feature-1024 branch 2 times, most recently from 8ba0b66 to 6acd687 Compare January 24, 2017 20:32

vdemeester mentioned this pull request Jan 27, 2017

Traefik on Swarm mode 1.13.rc3 seems broken #945

Closed

vdemeester suggested changes Feb 1, 2017

View reviewed changes

foleymic force-pushed the feature-1024 branch from 36fe851 to 779a944 Compare February 2, 2017 22:54

foleymic force-pushed the feature-1024 branch from 57beba0 to d1deacd Compare February 3, 2017 18:03

emilevauge modified the milestone: 1.2 Feb 6, 2017

Mike Foley added 2 commits February 6, 2017 14:44

emilevauge force-pushed the feature-1024 branch from d1deacd to e0a4c58 Compare February 6, 2017 13:44

vdemeester approved these changes Feb 6, 2017

View reviewed changes

emilevauge merged commit db63e84 into traefik:master Feb 6, 2017

ldez added kind/enhancement a new or improved feature. and removed status/2-needs-review labels Apr 29, 2017

ldez mentioned this pull request Jun 8, 2017

Swarm mode - support sticky sessions #1024

Closed

Support sticky sessions under SWARM Mode. #1024 #1033

Support sticky sessions under SWARM Mode. #1024 #1033

Conversation

foleymic commented Jan 11, 2017 • edited

emilevauge left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joke commented Jan 11, 2017 • edited

foleymic commented Jan 11, 2017

foleymic commented Jan 11, 2017

emilevauge commented Jan 12, 2017

foleymic commented Jan 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilevauge left a comment

Choose a reason for hiding this comment

foleymic commented Jan 19, 2017

vdemeester left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

foleymic Jan 19, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

foleymic commented Jan 26, 2017

vdemeester left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

foleymic Feb 1, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilevauge commented Feb 1, 2017

vdemeester commented Feb 1, 2017

emilevauge commented Feb 2, 2017

foleymic commented Feb 2, 2017

foleymic commented Feb 3, 2017 • edited

emilevauge commented Feb 3, 2017

foleymic commented Feb 3, 2017

emilevauge commented Feb 3, 2017

foleymic commented Feb 3, 2017 • edited

emilevauge commented Feb 5, 2017

vdemeester left a comment

Choose a reason for hiding this comment

kevtainer commented Nov 2, 2017

foleymic commented Jan 11, 2017 •

edited

emilevauge left a comment •

edited

joke commented Jan 11, 2017 •

edited

foleymic Jan 19, 2017 •

edited

vdemeester left a comment •

edited

foleymic Feb 1, 2017 •

edited

foleymic commented Feb 3, 2017 •

edited

foleymic commented Feb 3, 2017 •

edited