etcd healthz, hotloop fixes #19992
etcd healthz, hotloop fixes #19992
Conversation
liggitt
commented
Jun 13, 2018
•
liggitt
commented
- fixes CPU hotloop in etcd client (pending upstream review, needs picking to our branch and update-deps)
- fixes etcd healthz check to use an actual client (makes it less vulnerable to spurious dns lookup timeouts)
- fixes typo in healthz filter exemption
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
openshift-ci-robot
added
the
size/L
|
/retest |
| @@ -43,7 +43,7 @@ func withAssetServerRedirect(handler http.Handler, accessor *webConsolePublicURL | |||
| func (c *MasterConfig) withConsoleRedirection(handler, assetServerHandler http.Handler, accessor *webConsolePublicURLAccessor) http.Handler { | |||
| return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { | |||
| // blacklist well known paths so we do not risk recursion deadlocks | |||
| for _, prefix := range []string{"/apis", "/api", "/oapi", "/healtz", "/version"} { | |||
There was a problem hiding this comment.
Uups, I guess healthz was not the one that caused the loop :-)
| c.RESTOptionsGetter = &storageFactoryRestOptionsFactory{Options: *s, StorageFactory: factory} | ||
| return nil | ||
| } | ||
|
|
||
| func (s *EtcdOptions) addEtcdHealthEndpoint(c *server.Config) { | ||
| func (s *EtcdOptions) addEtcdHealthEndpoint(c *server.Config) error { | ||
| healthCheck, err := storagefactory.CreateHealthCheck(s.StorageConfig) |
There was a problem hiding this comment.
pre-existing, but if I called ApplyWithStorageFactoryTo wouldn't I expect to use the StorageConfig that's available from the StorageFactory?
There was a problem hiding this comment.
yes, though we lack the information required to get the specific config to set up the health check here. I don't see that getting addressed as part of this
|
lgtm |
liggitt
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
fix for cadvisor went in (sorry for that.) |
|
tagging per #19992 (comment) |
|
/retest |
