Skip to content
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

liveness: add health status liveness probe sidecar #1665

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/cephcsi.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ func init() {
flag.BoolVar(&conf.ForceKernelCephFS, "forcecephkernelclient", false, "enable Ceph Kernel clients on kernel < 4.17 which support quotas")

// liveness/grpc metrics related flags
flag.IntVar(&conf.MetricsPort, "metricsport", 8080, "TCP port for liveness/grpc metrics requests")
flag.StringVar(&conf.MetricsPort, "metricsport", "8080", "TCP port for liveness/grpc metrics requests")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is wrong, it will likely miss any integer-formatting checks. Better keep it an int.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically yes, I'm trying to save the conversion from int to string here, as the JoinHostPort() from net package expects port in string form.

Also see:
https:/kubernetes-csi/livenessprobe/blob/bba64df584a52d98ddf1904b7f8ec20a2828257c/cmd/livenessprobe/main.go#L39

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't it cause an upgrade issue? if someone just updated the image (which is the case for minor releases)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it will cause any upgrade issues.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we please check once?

flag.StringVar(&conf.MetricsPath, "metricspath", "/metrics", "path of prometheus endpoint where metrics will be available")
flag.DurationVar(&conf.PollTime, "polltime", time.Second*pollTime, "time interval in seconds between each poll")
flag.DurationVar(&conf.PoolTimeout, "timeout", time.Second*probeTimeout, "probe timeout in seconds")
flag.DurationVar(&conf.ProbeTimeout, "timeout", time.Second*probeTimeout, "probe timeout in seconds")

flag.BoolVar(&conf.EnableGRPCMetrics, "enablegrpcmetrics", false, "[DEPRECATED] enable grpc metrics")
flag.StringVar(&conf.HistogramOption, "histogramoption", "0.5,2,6",
Expand Down
1 change: 1 addition & 0 deletions deploy/cephfs/kubernetes/csi-cephfsplugin-provisioner.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ spec:
image: quay.io/cephcsi/cephcsi:canary
args:
- "--type=liveness"
- "--v=5"
- "--endpoint=$(CSI_ENDPOINT)"
- "--metricsport=8681"
- "--metricspath=/metrics"
Expand Down
1 change: 1 addition & 0 deletions deploy/cephfs/kubernetes/csi-cephfsplugin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ spec:
image: quay.io/cephcsi/cephcsi:canary
args:
- "--type=liveness"
- "--v=5"
- "--endpoint=$(CSI_ENDPOINT)"
- "--metricsport=8681"
- "--metricspath=/metrics"
Expand Down
1 change: 1 addition & 0 deletions deploy/rbd/kubernetes/csi-rbdplugin-provisioner.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ spec:
image: quay.io/cephcsi/cephcsi:canary
args:
- "--type=liveness"
- "--v=5"
- "--endpoint=$(CSI_ENDPOINT)"
- "--metricsport=8680"
- "--metricspath=/metrics"
Expand Down
1 change: 1 addition & 0 deletions deploy/rbd/kubernetes/csi-rbdplugin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ spec:
image: quay.io/cephcsi/cephcsi:canary
args:
- "--type=liveness"
- "--v=5"
- "--endpoint=$(CSI_ENDPOINT)"
- "--metricsport=8680"
- "--metricspath=/metrics"
Expand Down
2 changes: 1 addition & 1 deletion internal/liveness/liveness.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func Run(conf *util.Config) {
util.ExtendedLogMsg("Liveness Running")

// start liveness collection
go recordLiveness(conf.Endpoint, conf.DriverName, conf.PollTime, conf.PoolTimeout)
go recordLiveness(conf.Endpoint, conf.DriverName, conf.PollTime, conf.ProbeTimeout)

// start up prometheus endpoint
util.StartMetricsServer(conf)
Expand Down
3 changes: 1 addition & 2 deletions internal/util/httpserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"net"
"net/http"
"net/url"
"strconv"

"github.com/prometheus/client_golang/prometheus/promhttp"
)
Expand All @@ -17,7 +16,7 @@ func ValidateURL(c *Config) error {

// StartMetricsServer starts http server.
func StartMetricsServer(c *Config) {
addr := net.JoinHostPort(c.MetricsIP, strconv.Itoa(c.MetricsPort))
addr := net.JoinHostPort(c.MetricsIP, c.MetricsPort)
http.Handle(c.MetricsPath, promhttp.Handler())
err := http.ListenAndServe(addr, nil)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions internal/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ type Config struct {
HistogramOption string // Histogram option for grpc metrics, should be comma separated value, ex:= "0.5,2,6" where start=0.5 factor=2, count=6
MetricsIP string // TCP port for liveness/ metrics requests
PidLimit int // PID limit to configure through cgroups")
MetricsPort int // TCP port for liveness/grpc metrics requests
MetricsPort string // TCP port for liveness/grpc metrics requests
PollTime time.Duration // time interval in seconds between each poll
PoolTimeout time.Duration // probe timeout in seconds
ProbeTimeout time.Duration // probe timeout in seconds
EnableGRPCMetrics bool // option to enable grpc metrics

IsControllerServer bool // if set to true start provisoner server
Expand Down