Secret retry #84
Secret retry #84
Conversation
|
@thetechnick thx! I've added few comments. It doesn't fix #78 though, but it's an important step towards it.
I'll create a separate issue for the task of adding error events.
Not sure if it's necessary, since if there is a missing secret, its gets caught by this PR. |
|
@pleshakov |
| ingEx := lbc.createIngress(&ing) | ||
| ingEx, err := lbc.createIngress(&ing) | ||
| if err != nil { | ||
| lbc.ingQueue.requeueAfter(key, err, 5*time.Second) |
There was a problem hiding this comment.
I don't think it's necessary to requeue the Ingress when endpoints change. Better to ignore such an Ingress with an error.
| @@ -464,8 +472,7 @@ func (lbc *LoadBalancerController) createIngress(ing *extensions.Ingress) nginx. | |||
| secretName := tls.SecretName | |||
| secret, err := lbc.client.Secrets(ing.Namespace).Get(secretName) | |||
| if err != nil { | |||
| glog.Warningf("Error retrieving secret %v for Ingress %v: %v", secretName, ing.Name, err) | |||
| continue | |||
| return ingEx, fmt.Errorf("Error retrieving secret %v for Ingress %v: %v", secretName, ing.Name, err) | |||
There was a problem hiding this comment.
Better to return nil instead of ingEx
Agree, It covers #78. I misunderstood the issue a little bit. There is a separate problem of updating a certificate: if a secret changes, the controller won't update the certificate. |
| ingEx, err := lbc.createIngress(&ing) | ||
| if err != nil { | ||
| glog.Warningf("Error updating endpoints for %v/%v: %v, skipping", ing.Namespace, ing.Name, err) | ||
| return |
There was a problem hiding this comment.
a small bug. must be continue instead of return. This leads to Ignoring the rest of the Ingresses.
thetechnick
force-pushed
the
secret-retry
branch
from
5881ccb to
46eff6c
Compare
|
@thetechnick thx! Please make changes related to a review in a separate commit, instead of changing the last commit. This way it's easier to review it and also get notified about it. I will squash the commits before the merging (forgot to do it for this pull request) |
If a certificate secret referenced in an ingress object is not found,
the ingress controller will reject the ingress object. but retries every 5s.
This fixes #78.
I have tested this with success on my private cluster merged together with #77 and #81 and kube-lego.
This is a quite simple approach to the issue, next steps to improve this feature are:
I think we can cover point 1 when we implement the event logging system also improving #77,
point 2 is more difficult, but I think most users will use only one host per ingress object, so this is not an issue in most cases.