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

Rancher host IP address #2101

Merged
merged 4 commits into from
Sep 15, 2017
Merged

Rancher host IP address #2101

merged 4 commits into from
Sep 15, 2017

Conversation

matq007
Copy link
Contributor

@matq007 matq007 commented Sep 11, 2017

Description

Until now traefik was setting up PrimaryIpAddress which is only set for managed network. When host network is chosen, traefik has to setup the IP address of the host machine where the container is running. As shown on the picture below ghot-1 is on host network and ghot is on managed network.

github

Fixes #1498

@ldez
Copy link
Contributor

ldez commented Sep 11, 2017

Thanks for your contribution 👍

Could you change the base branch of this PR from master to v1.4 ?

Is there an already open issue about this problem?

@matq007
Copy link
Contributor Author

matq007 commented Sep 11, 2017

Sure thing @ldez. No. Should I make an issue for it?

@matq007 matq007 changed the base branch from master to v1.4 September 11, 2017 16:29
@ldez
Copy link
Contributor

ldez commented Sep 11, 2017

No need to open a new issue.

It's not related to #1498 ?

@matq007
Copy link
Contributor Author

matq007 commented Sep 11, 2017

Oh right. I forgot about that. That's the one! :)

@ldez
Copy link
Contributor

ldez commented Sep 11, 2017

Could you rebase your PR on the v1.4 branch?

git rebase upstream/v1.4

with:

git remote -v

origin	git@github.com:matq007/traefik.git (fetch)
origin	git@github.com:matq007/traefik.git (push)
upstream	git@github.com:containous/traefik.git (fetch)
upstream	git@github.com:containous/traefik.git (push)

@ldez ldez modified the milestone: 1.4 Sep 11, 2017
@ldez ldez requested a review from SantoDE September 11, 2017 23:59
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.

@matq007 Many thanks for your PR.
I just have one suggestion.

rancher "github.com/rancher/go-rancher/client"
)

const labelRancheStackServiceName = "io.rancher.stack_service.name"
const hostNetwork = "host"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use a const block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing, on it right now :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please rebase it again to 1.4 branch?

@ldez
Copy link
Contributor

ldez commented Sep 12, 2017

@matq007 please do:

git checkout master
git branch -D fix-rancher-host
git checkout origin fix-rancher-host

Copy link
Collaborator

@SantoDE SantoDE left a comment

Choose a reason for hiding this comment

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

Hey @matq007 ,

thanks for your contribution. Tested it on my rancher dev cluster and it's working good, so LGTM 👼

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

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 👏 🐮

@traefiker traefiker merged commit 0a0cf87 into traefik:v1.4 Sep 15, 2017
@matq007 matq007 deleted the fix-rancher-host branch September 15, 2017 11:02
@juliens juliens changed the title Fix rancher host IP address Rancher host IP address Sep 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants