-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Differentiate liveness and readiness probes for router pods #19009
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ import ( | |
|
||
"github.com/golang/glog" | ||
|
||
templateplugin "github.com/openshift/origin/pkg/router/template" | ||
"k8s.io/apiserver/pkg/server/healthz" | ||
"k8s.io/kubernetes/pkg/probe" | ||
probehttp "k8s.io/kubernetes/pkg/probe/http" | ||
|
@@ -35,6 +36,28 @@ func HTTPBackendAvailable(u *url.URL) healthz.HealthzChecker { | |
}) | ||
} | ||
|
||
// HasSynced returns a healthz check that verifies the router has been synced at least | ||
// once. | ||
func HasSynced(router **templateplugin.TemplatePlugin) healthz.HealthzChecker { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sort of construct is a violation of our style guides. You should be passing an interface here that exposes the correct check method. Double pointers should never be used. Nothing in metrics should be aware of template plugin at all. |
||
return healthz.NamedCheck("has-synced", func(r *http.Request) error { | ||
if router != nil { | ||
if (*router).Router.SyncedAtLeastOnce() == true { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This construct is not appropriate. It should always be |
||
return nil | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you return early, elide the else, as per our style guide. |
||
return fmt.Errorf("Router not synced") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Errors should always be lower case, as per the style guide. |
||
} | ||
} | ||
return nil | ||
}) | ||
} | ||
|
||
func ControllerLive() healthz.HealthzChecker { | ||
return healthz.NamedCheck("controller", func(r *http.Request) error { | ||
return nil | ||
}) | ||
|
||
} | ||
|
||
// ProxyProtocolHTTPBackendAvailable returns a healthz check that verifies a backend supporting | ||
// the HAProxy PROXY protocol responds to a GET to the provided URL with 2xx or 3xx response. | ||
func ProxyProtocolHTTPBackendAvailable(u *url.URL) healthz.HealthzChecker { | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
@rajatchopra @pravisankar does this make it clearer? removing the reference to the healthzPort in favour of a more generic term
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.
+1