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

Support Host NetworkMode for ECS provider #2320

Merged
merged 4 commits into from
Oct 31, 2017

Conversation

FriggaHel
Copy link
Contributor

@FriggaHel FriggaHel commented Oct 25, 2017

This PR Allows to override exposed port by Label (traefik.port) when using ECS provider.
This add ability to use traefik with containers running with Host NetworkMode (#2169)

Fixes #2169

@FriggaHel FriggaHel force-pushed the support-ecs-host-network-mode branch from ff6c84a to 7326e02 Compare October 25, 2017 09:49
@ldez ldez changed the title Add traefik.backend.port label for ECS provider (Support Host NetworkMode) Support Host NetworkMode for ECS provider Oct 25, 2017
@FriggaHel FriggaHel force-pushed the support-ecs-host-network-mode branch from 7326e02 to 79fd9e7 Compare October 25, 2017 15:21
Copy link
Member

@mmatur mmatur left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @FriggaHel 👏

@@ -129,6 +129,7 @@ Labels can be used on task containers to override default behaviour:
| `traefik.protocol=https` | override the default `http` protocol |
| `traefik.weight=10` | assign this weight to the container |
| `traefik.enable=false` | disable this container in Træfik |
| `traefik.backend.port=80` | override the default `port` value. Overrides `NetworkBindings` from Docker Container |
Copy link
Member

Choose a reason for hiding this comment

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

Could you please use LabelPort instead of creating a new label.

@@ -554,6 +554,9 @@ func (p *Provider) getHost(i ecsInstance) string {
}

func (p *Provider) getPort(i ecsInstance) string {
if port := p.label(i, types.LabelBackendPort); port != "" {
Copy link
Member

Choose a reason for hiding this comment

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

Could you please use types.LabelPort instead of creating a new label.

@@ -24,6 +24,7 @@ const (
LabelTraefikFrontendWhitelistSourceRange = LabelPrefix + "frontend.whitelistSourceRange"
LabelBackend = LabelPrefix + "backend"
LabelBackendID = LabelPrefix + "backend.id"
LabelBackendPort = LabelPrefix + "backend.port"
Copy link
Member

Choose a reason for hiding this comment

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

Could you please use types.LabelPort instead of creating a new label.

@FriggaHel FriggaHel force-pushed the support-ecs-host-network-mode branch from 79fd9e7 to f6ef44b Compare October 26, 2017 16:31
@ldez ldez added the kind/enhancement a new or improved feature. label Oct 26, 2017
Copy link
Member

@mmatur mmatur 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
Contributor

@nmengin nmengin left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez added this to the 1.5 milestone Oct 31, 2017
Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ghost
Copy link

ghost commented Nov 10, 2017

Can you release that sooner than 1.5? That's not a big change, right?

@ldez
Copy link
Contributor

ldez commented Nov 10, 2017

@zyzop the releases on 1.4 branch are only for bug fixes and documentation.

@FriggaHel FriggaHel deleted the support-ecs-host-network-mode branch November 13, 2017 09:48
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.

6 participants