search

Unleash / helm-charts

Contains helm-charts for Unleash
Apache License 2.0
41 stars 56 forks source link

Add possibility to configure liveness and readiness probes for Unleash Proxy #76

Closed iteratec-hscaffino closed 1 year ago

iteratec-hscaffino commented 1 year ago

About the changes

I added the possibility to configure the liveness and the readiness probes for the Unleash Proxy.

This is important because if one hosts the proxy in a subpath, e.g. https://mycompany.com/app/unleash-proxy, then the variable PROXY_BASE_PATH needs to be configured in the env: block of the values.yaml AND the path for the liveness and readiness probes needs to be updated too.

Before this pull request the liveness and readiness path is hardcoded to /proxy/health. Now it is possible to configure it. I simply copied the corresponding block from the deployment.yaml file for the Helm chart for the Unleash backend. I also copied the default values to the values.yaml.

This PR does not close any issues.

Important files

Both files deployment.yaml and values.yaml are equally important. This is a small improvement.

Discussion points

I do not know if the default values for the liveness and readiness probes for the Unleash backend also make sense for the Unleash proxy.

kflynn66 commented 1 year ago

This would be helpful. We are using proxy with GitLab and custom kubernetes environment, and liveness and readiness probes are failing. This seems like it could be the likely cause.

stale[bot] commented 1 year ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

iteratec-hscaffino commented 1 year ago

@chriswk @thomasheartman sorry to bother you guys with a direct mention. Could you please have a look at this PR when you have time? It has been marked as stale and it would be nice to add these changes to the helm chart.

thomasheartman commented 1 year ago

No, not at all! Sorry, this must have slipped through the cracks on our end. This seems sensible to me, but @chriswk is more familiar with our helm charts, so I'll let him have the final say on this.

chriswk commented 1 year ago

Hi. I believe #79 should fix this as well, making it possible to configure the PROXY_BASE_PATH environment variable through a dedicated value setting, and then deriving health/readiness paths from the basePath

proxy:
  basePath: