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

consider http probes instead of exec probes? #1657

Open
billimek opened this issue Jul 20, 2022 · 3 comments
Open

consider http probes instead of exec probes? #1657

billimek opened this issue Jul 20, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@billimek
Copy link

Is this a request for help?: No


FEATURE REQUEST:

The artifactory helm chart by default leverages exec-style probes to invoke curl commands, e.g.

  livenessProbe:
    enabled: true
    config: |
      exec:
        command:
          - sh
          - -c
          - curl -s -k --fail --max-time {{ .Values.probes.timeoutSeconds }} {{ include "artifactory.scheme" . }}://localhost:{{ .Values.router.internalPort }}/router/api/v1/system/liveness
      initialDelaySeconds: {{ if semverCompare "<v1.20.0-0" .Capabilities.KubeVersion.Version }}90{{ else }}0{{ end }}
      periodSeconds: 10
      timeoutSeconds: {{ .Values.probes.timeoutSeconds }}
      failureThreshold: 5
      successThreshold: 1

Is the curl command necessary to properly probe the health of the instances, or is it possible to leverage https style probes to achieve the same end-result?

The reason for this ask is that there is currently an open kubernetes issue related to using exec-style probes and possibly resulting in high system CPU/load.

Version of Helm and Kubernetes: helm3, 1.22.8-gke.202

Which chart: artifactory

@eldada
Copy link
Contributor

eldada commented Jul 21, 2022

Hi @billimek.
We actually moved to using exec probes with curl because of kubernetes/kubernetes#89898.
We saw this happening in our production clusters (many false positives), and the move to the exec probes fixed it.

You can replace them with http probes without worry. It should work the same. This is why it's fully configurable.
We are aware of kubernetes/kubernetes#82440, and are also impacted by it, but when choosing between the two options, we prefer higher CPU but better availability.

@billimek
Copy link
Author

Thanks for the quick reply and context @eldada. I'll definitely watch kubernetes/kubernetes#89898 now too. It's too bad there are challenges with both exec and http probes!

@eldada
Copy link
Contributor

eldada commented Jul 21, 2022

Spoke too soon... we tested this internally and found some gaps in making it work with a simple switch to http.
I'm reopening this for now. Will come back to it once we have a working solution.

@eldada eldada reopened this Jul 21, 2022
@chukka chukka added the enhancement New feature or request label Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants