Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple rules from docker labels containers with traefik.<servicename>.* properties #1257

Merged
merged 1 commit into from
Mar 14, 2017

Conversation

benoitf
Copy link
Contributor

@benoitf benoitf commented Mar 8, 2017

It matches expectation of #444
If labels contain for example:

  • traefik.mycustomservice.port=443
  • traefik.mycustomservice.frontend.rule=Path:/mycustomservice
  • traefik.anothercustomservice.port=8080
  • traefik.anothercustomservice.frontend.rule=Path:/anotherservice

all traffic to frontend on path /mycustomservice is redirected to the port 443 of the container defining the label while using path /anotherservice will redirect to the port 8080 of the docker container defining the label

Available properties are
traefik.<service-name>.port
traefik.<service-name>.weight
traefik.<service-name>.protocol
traefik.<service-name>.frontend.backend
traefik.<service-name>.frontend.entryPoints
traefik.<service-name>.frontend.rule
traefik.<service-name>.frontend.passHostHeader
traefik.<service-name>.frontend.priority
traefik.<service-name>.frontend.rule

  • More documentation in the docs/toml.md file
  • new tests added to docker_tests.go

Change-Id: Ifaa3bb00ef0a0f38aa189e0ca1586fde8c5ed862
Signed-off-by: Florent BENOIT fbenoit@codenvy.com

Copy link
Contributor

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good, just one small comment 👼


// Extract entrypoints from labels for a given service and a given docker container
func (provider *Docker) getServiceEntryPoints(container dockerData, serviceName string) []string {
if entryPoints, ok := extractServicesLabels(container.Labels)[serviceName]["frontend.entryPoints"]; ok {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a huge fan of that notation ; would have prefered to pass serviceName and the key (here frontend.entryPoints) as arguments (and thus make it sure these function don't care how the label are extracted, using maps or something else).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok so you prefer something more generic ?
getServiceValue $container $serviceName $key ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I don't want to surface the fact that this is a map or something else 👼

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if entryPoints, ok := getContainerServiceLabel(container, serviceName, "frontend.entryPoints")

looks of for you ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks perfect to me 👼 👍

Copy link
Contributor Author

@benoitf benoitf Mar 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated to call getContainerServiceLabel (this new function is delegating to extractServicesLabels)
Let me know if it's ok or if you want to remove as well the intermediate extractServicesLabels

@@ -2271,3 +2271,613 @@ func TestSwarmTaskParsing(t *testing.T) {
}
}
}

func TestDockerGetServiceProtocol(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to work on these, because our unit tests are way too verbose (but it's orthogonal to this PR, just thinking outloud 🤔 ).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah for sure they are verbose ^^

@benoitf benoitf force-pushed the docker-services branch 2 times, most recently from cd36713 to ca74d6d Compare March 10, 2017 16:12
@benoitf
Copy link
Contributor Author

benoitf commented Mar 10, 2017

updated

@benoitf
Copy link
Contributor Author

benoitf commented Mar 11, 2017

@intelradoux you can use Host in the front-end rule. So AFAIK this is possible with this PR
You define two services with two different Host front-end rule

Copy link
Contributor

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🐯

Copy link
Member

@emilevauge emilevauge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benoitf Really good job 😍
LGTM
Could you squash your commits before we merge it ?

@benoitf benoitf force-pushed the docker-services branch 2 times, most recently from 600596e to d2df258 Compare March 13, 2017 18:51
@benoitf
Copy link
Contributor Author

benoitf commented Mar 13, 2017

@emilevauge ok done

   - traefik.mycustomservice.port=443
  -  traefik.mycustomservice.frontend.rule=Path:/mycustomservice
   - traefik.anothercustomservice.port=8080
  -  traefik.anothercustomservice.frontend.rule=Path:/anotherservice

all traffic to frontend /mycustomservice is redirected to the port 443 of the container while using /anotherservice will redirect to the port 8080 of the docker container

More documentation in the docs/toml.md file

Change-Id: Ifaa3bb00ef0a0f38aa189e0ca1586fde8c5ed862
Signed-off-by: Florent BENOIT <fbenoit@codenvy.com>
@timoreimann timoreimann merged commit 2a61c90 into traefik:master Mar 14, 2017
@timoreimann
Copy link
Contributor

timoreimann commented Mar 14, 2017

Merci, @benoitf! Merged. 😃

@benoitf benoitf deleted the docker-services branch March 14, 2017 10:28
@benoitf
Copy link
Contributor Author

benoitf commented Mar 14, 2017

thx. So it won't be in 1.2 ?

@vdemeester vdemeester added this to the 1.3 milestone Mar 14, 2017
@vdemeester
Copy link
Contributor

@benoitf yeah, putting this in 1.3 milestone. 1.2 is already freezed so… 👼 😅

@benoitf
Copy link
Contributor Author

benoitf commented Mar 14, 2017

ok :)

@dcrystalj
Copy link

dcrystalj commented Mar 21, 2017

I am testing your PR now.

I created simple service as

    docker service create \
    --name=visualizer \
    --network=proxy \
    --constraint=node.role==manager \
    --mount=type=bind,src=/var/run/docker.sock,dst=/var/run/docker.sock \
    --label traefik.visualizer.port=8080 \
    --label traefik.visualizer.network=proxy \
    --label traefik.visualizer.frontend.rule=Host:visualizer.traefik \
    --label traefik.visualizer.frontend.entryPoints=http,https \
    manomarks/visualizer:latest

While putting old school traefik.port is working this .visualizer. as <service-name> does not. In traefik gui on port 8080 this service is not shown.

I am running image containous/traefik:experimental

@timoreimann
Copy link
Contributor

@dcrystalj if you run into problems, please create a new issue and / or get in touch with the Traefik community on Slack.

Thanks!

@alwaysastudent
Copy link

@benoitf - Does this support multiple services if they reside on same port ? What if I want to hit two different paths based on the Host header ?

@reflog
Copy link

reflog commented Feb 26, 2018

Hi! Any chance to get this working on ECS backend?

@ldez
Copy link
Contributor

ldez commented Feb 26, 2018

@reflog yes it's in my todo list.

@reflog
Copy link

reflog commented Feb 26, 2018

@ldez - that's awesome! Is there any way to simulate this behavior right now? I am trying add these two rules (they worked in docker):

      - "traefik.test.frontend.rule=Host:whoami.docker.localhost;Path:/;ReplacePath:/api"
      - "traefik.test2.frontend.rule=Host:whoami.docker.localhost;PathPrefix:/api"

Any suggestion on how do this this in ECS? Basically, if user comes to / , redirect to /api , otherwise, all requests start with /api.

Thanks in advance!

@ldez
Copy link
Contributor

ldez commented Feb 26, 2018

@reflog It's not the good place for questions.

Come to the Traefik community Slack

@koliyo
Copy link

koliyo commented Feb 27, 2018

Ignore this, already resolved! Thanks for this feature!

@rjchicago
Copy link

rjchicago commented Aug 10, 2018

Tested and using this feature now - thanks!

for other finding this, here was my simple test...

Labels in docker compose:
- traefik.route1.frontend.rule=PathPrefixStrip:/traefik;PathPrefix:/traefik
- traefik.route2.frontend.rule=Host:traefik.docker-swarm.local
- traefik.route3.frontend.rule=Host:test-traefik.docker-swarm.local

Simple verification:
curl -s http://192.168.99.100/traefik/health | jq
curl -s http://traefik.docker-swarm.local/health | jq
curl -s http://test-traefik.docker-swarm.local/health | jq

In Traefik, this shows as this...
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants