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

[bug] Hard-coded health path for readiness / liveness probes, yet ability to configure where it is exposed by Centrifugo #63

Open
vmcqgo opened this issue Aug 10, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@vmcqgo
Copy link

vmcqgo commented Aug 10, 2023

Describe the bug
While you can configure the health handler path for Centrifugo through the config key health_handler_prefix, the liveness and readiness probes' paths are hard-coded as /health in the deployment template:
https://github.com/centrifugal/helm-charts/blob/master/charts/centrifugo/templates/deployment.yaml#L154-L165

Version of Helm and Kubernetes:

Helm Version:

$ helm version
version.BuildInfo{Version:"v3.10.1", GitCommit:"9f88ccb6aee40b9a0535fcc7efea6055e1ef72c9", GitTreeState:"clean", GoVersion:"go1.18.7"}

Kubernetes Version:

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"23", GitVersion:"v1.23.17", GitCommit:"953be8927218ec8067e1af2641e540238ffd7576", GitTreeState:"clean", BuildDate:"2023-03-01T02:23:41Z", GoVersion:"go1.19.6", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"24+", GitVersion:"v1.24.15-eks-a5565ad", GitCommit:"af0b16a331376384fe84445fe2f8d4cb468b4148", GitTreeState:"clean", BuildDate:"2023-06-16T17:35:37Z", GoVersion:"go1.19.10", Compiler:"gc", Platform:"linux/amd64"}

Which version of the chart:
10.0.3

What happened:
Liveness / readiness probes are failing when a different health path is configured.

What you expected to happen:
Be able to configure the liveness and readiness probe paths or if the value for health_handler_prefix is not empty, use that value for probes path, too.

How to reproduce it (as minimally and precisely as possible):

values.yaml

config: 
    health_handler_prefix: /some-prefix/health
helm install my-release centrifugal/centrifugo --version version --values values.yaml
@vmcqgo vmcqgo added the bug Something isn't working label Aug 10, 2023
@vmcqgo vmcqgo changed the title [bug] issue title [bug] Hard-coded health path for readiness / liveness probes, yet ability to configure where it is exposed by Centrifugo Aug 10, 2023
@vmcqgo
Copy link
Author

vmcqgo commented Aug 10, 2023

A potential fix:

          livenessProbe:
            httpGet:
              path: {{ .Values.config.health_handler_prefix | default "/health" }}
              port: {{ .Values.internalService.port }}
            initialDelaySeconds: 30
            periodSeconds: 10
          readinessProbe:
            httpGet:
              path: {{ .Values.config.health_handler_prefix | default "/health" }}
              port: {{ .Values.internalService.port }}
            initialDelaySeconds: 3
            periodSeconds: 10

@FZambia
Copy link
Member

FZambia commented Aug 12, 2023

Hello @vmcqgo - thanks for pointing.

I think it's better to not use .Values.config.health_handler_prefix, but have a separate explicit option inside .Values.internalService

Sth like .Values.internalService.healthHttpEndpoint and use {{ .Values.internalService.healthHttpEndpoint | default "/health" }} for setting paths.

It's more configuration - two places instead of one, but feels more correct to me. Because Centrifugo's health_handler_prefix may be actually set over environment and using .Values.config.health_handler_prefix does not seem right from that perspective.

Surprisingly not-obvious thing :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants