-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Don't failed traefik start if register and subscribe failed on acme #2977
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
acme/acme.go
Outdated
@@ -344,7 +344,9 @@ Let's Encrypt functionality will be limited until traefik is restarted.`, err) | |||
log.Info("Register...") | |||
reg, err := a.client.Register() | |||
if err != nil { | |||
return err | |||
log.Errorf(`Failed to register user: %s | |||
Let's Encrypt functionality will be limited until traefik is restarted.`, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't Traefik be capitalized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe: "You will need to restart Traefik to resume normal operation with Let's Encrypt" more accurate?
acme/acme.go
Outdated
@@ -357,7 +359,9 @@ Let's Encrypt functionality will be limited until traefik is restarted.`, err) | |||
// Let's Encrypt Subscriber Agreement renew ? | |||
reg, err := a.client.QueryRegistration() | |||
if err != nil { | |||
return err | |||
log.Errorf(`Failed to renew subscriber agreement: %s | |||
Let's Encrypt functionality will be limited until traefik is restarted.`, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What does this PR do?
Log error when registering or AgreeToOS failed instead of fatal !
Motivation
Traefik still started when LE doesn't work
Fix #2976