-
Notifications
You must be signed in to change notification settings - Fork 38.6k
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
Add event generation when create/delete of balancers fails. #8887
Conversation
Partial fix for #8760 - we should open another bug to report it in stats.loadBalancer.condition |
@@ -206,6 +215,7 @@ func (s *ServiceController) processDelta(delta *cache.Delta) (error, bool) { | |||
case cache.Sync: | |||
err, retry := s.createLoadBalancerIfNeeded(namespacedName, service, cachedService.service) | |||
if err != nil { | |||
s.eventRecorder.Event(service, "creating balancer failed", err.Error()) |
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.
Nit (because I believe these become part of our contract?): balancer -> loadbalancer ?
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.
done.
Two nits if events are considered part of our contract (i.e. difficult to change). LGTM though. Thank you for fixing - I was looking at fixing as part of #8892, but then saw you had implemented this already :-) |
Once kubernetes#8887 merges this should show errors in load balancer creation. Even before then, it may show another error. Bug kubernetes#8892
@justinsb comments addressed, please re-check. Thanks! |
LGTM - thanks again! |
Add event generation when create/delete of balancers fails.
Once kubernetes#8887 merges this should show errors in load balancer creation. Even before then, it may show another error. Bug kubernetes#8892
Closes #8760