Avoid log flooding with messages about missing missing registry/services #732
Conversation
| @@ -115,7 +120,7 @@ func (s ConfigSourceEtcd) Run() { | |||
| func (s ConfigSourceEtcd) GetServices() ([]api.Service, []api.Endpoints, error) { | |||
| response, err := s.client.Get(registryRoot+"/specs", true, false) | |||
| if err != nil { | |||
There was a problem hiding this comment.
Check for tools.IsEtcdNotFound(err) and return empty arrays (which should be []api.Service{}, not make(...)) without an error.
|
@brendandburns thanks for suggestion. I think decreasing verbosity of the log is the right thing to do here. I updated the PR. |
| @@ -134,6 +137,9 @@ func (s ConfigSourceEtcd) GetServices() ([]api.Service, []api.Endpoints, error) | |||
| retServices[i] = svc | |||
| endpoints, err := s.GetEndpoints(svc.ID) | |||
| if err != nil { | |||
| if tools.IsEtcdNotFound(err) { | |||
| return []api.Service{}, []api.Endpoints{}, err | |||
| } | |||
There was a problem hiding this comment.
You need the log message to be in the if block, and the return to be outside the if block.
There was a problem hiding this comment.
Are you sure? I thought we want to return when the err == IsEtcdNotFound, but in other cases just log the error and continue.
|
Friendly ping on this PR. Address comments and rebase, and we'll get it merged. Thanks! |
| @@ -137,6 +140,9 @@ func (s ConfigSourceEtcd) GetServices() ([]api.Service, []api.Endpoints, error) | |||
| endpoints, err := s.GetEndpoints(svc.ID) | |||
| if err != nil { | |||
| glog.Errorf("Couldn't get endpoints for %s : %v skipping", svc.ID, err) | |||
| if tools.IsEtcdNotFound(err) { | |||
| return []api.Service{}, []api.Endpoints{}, err | |||
| } | |||
There was a problem hiding this comment.
So if the endpoints were deleted from etcd, then there are truly no endpoints. You don't need to log in that case. If any other error occurs, we need to go to the next service (continue). So seems like it should be:
if err != nil {
if !isEtcdNotFound(err) {
log
continue
}
endpoints = []api.Endpoints{}
}
|
@smarterclayton @brendanburns fixed all comments and rebased, should be OK to merge now. |
| @@ -136,7 +139,11 @@ func (s ConfigSourceEtcd) GetServices() ([]api.Service, []api.Endpoints, error) | |||
| retServices[i] = svc | |||
| endpoints, err := s.GetEndpoints(svc.ID) | |||
| if err != nil { | |||
| glog.Errorf("Couldn't get endpoints for %s : %v skipping", svc.ID, err) | |||
| if tools.IsEtcdNotFound(err) { | |||
| glog.Errorf("Couldn't get endpoints for %s : %v skipping", svc.ID, err) | |||
There was a problem hiding this comment.
You need to be logging if there's an error that isn't IsEtcdNotFound
|
small nits, basically LGTM. |
|
@mfojtik can you correct this last condition? |
|
@smarterclayton @brendandburns done, sorry for this to take so long. |
|
LGTM, @smarterclayton are you good? If so, I'll merge. |
|
Yeah LGTM |
Avoid log flooding with messages about missing missing registry/services
Add backup text to process table if no processes detected.
Adding API section to Windows KEP
backend/extension: Allow the backend data to be empty
Bug 1956895: UPSTREAM: 101593: kubelet: change cgroup move message to log level 3
Enforce that `github.com/onsi/ginkgo/ginkgo` is considered dependency
Prevents this to appear in log every 2s: