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.

Sign up for GitHub

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

Jump to bottom

Don't reload configuration when rancher server is down #2706

Merged
merged 4 commits into from
Jan 22, 2018
Merged

Don't reload configuration when rancher server is down #2706

merged 4 commits into from
Jan 22, 2018

Conversation

wacken89
Copy link
Contributor

@wacken89 wacken89 commented Jan 15, 2018 •
edited by ldez
Loading

What does this PR do?

Doesn't reload Traefik configuration when Rancher sever is down

Motivation

We using Traefik in production, it critical for us if Rancher server is down and traefik does not proxying requests

Fixes #2284

@wacken89 wacken89 requested a review from a team as a code owner January 15, 2018 10:55
@ldez
Copy link
Contributor

ldez commented Jan 15, 2018 •
edited
Loading

@wacken89 Happy to see your first Go PR 🎉

Could you fix those issues:

---> Making bundle: validate-golint (in .)
Errors from golint:
provider/rancher/api.go:95:8: var checkRancherApi should be checkRancherAPI
provider/rancher/api.go:95:25: var rancherErrApi should be rancherErrAPI

ldez
ldez suggested changes Jan 15, 2018
View reviewed changes
provider/rancher/api.go Outdated
checkRancherApi, rancherErrApi := rancherClient.ApiKey.List(withoutPagination)

if rancherErrApi != nil {
log.Errorf("Cannot establish connection: %+v, rancher API return: %+v;\nSkipping refresh Data from Provider API", rancherErrApi, checkRancherApi)
Copy link
Contributor

Choose a reason for hiding this comment

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

could you remove the \n and add a . at the end of the sentence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ldez ldez changed the title Reload configuration when rancher server is down Don't reload configuration when rancher server is down Jan 15, 2018
@ldez ldez added the kind/bug/fix a bug fix label Jan 15, 2018
wacken89
wacken89 commented Jan 15, 2018
View reviewed changes
Copy link
Contributor Author

@wacken89 wacken89 left a comment

Choose a reason for hiding this comment

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

Done

provider/rancher/api.go Outdated
checkRancherApi, rancherErrApi := rancherClient.ApiKey.List(withoutPagination)

if rancherErrApi != nil {
log.Errorf("Cannot establish connection: %+v, rancher API return: %+v;\nSkipping refresh Data from Provider API", rancherErrApi, checkRancherApi)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ldez
Copy link
Contributor

ldez commented Jan 15, 2018

@wacken89 Could you rebase ? (The CI failed due to a fixed pb with one test.)

@ldez ldez force-pushed the reload-configuration-when-rancher-server-is-down branch from 26b63f1 to bad9724 Compare January 15, 2018 15:26
@ldez ldez added this to the 1.5 milestone Jan 15, 2018
SantoDE
SantoDE approved these changes Jan 22, 2018
View reviewed changes
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 @wacken89 ,

thanks for your PR :-)

Thanks that you found a way to reuse the old client. Looks way better ;)

LGTM 👼

@ldez ldez force-pushed the reload-configuration-when-rancher-server-is-down branch from 54560f4 to 58b5a1c Compare January 22, 2018 08:56
@ldez ldez changed the base branch from master to v1.5 January 22, 2018 08:56
@ldez ldez removed the bot/no-merge label Jan 22, 2018
ldez
ldez approved these changes Jan 22, 2018
View reviewed changes
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

nmengin
nmengin approved these changes Jan 22, 2018
View reviewed changes
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 force-pushed the reload-configuration-when-rancher-server-is-down branch from 58b5a1c to aa733d7 Compare January 22, 2018 09:28
@traefiker traefiker merged commit fc00e1c into traefik:v1.5 Jan 22, 2018
@ldez ldez mentioned this pull request Feb 20, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nmengin nmengin nmengin approved these changes

@ldez ldez ldez approved these changes

@SantoDE SantoDE SantoDE approved these changes

Assignees
No one assigned
Labels
area/provider/rancher kind/bug/fix a bug fix size/S
Projects
None yet
Milestone
1.5
Development

Successfully merging this pull request may close these issues.
5 participants
@wacken89 @ldez @SantoDE @nmengin @traefiker