jfrog / charts

JFrog official Helm Charts
https://jfrog.com/integration/helm-repository/
Apache License 2.0
255 stars 444 forks source link

Request for Feature Enhancement to Allow Individual Changes in Artifactory Readiness Probes Configuration #1889

Open edlyn-liew opened 4 months ago

edlyn-liew commented 4 months ago

Is this a request for help?:


Is this a BUG REPORT or FEATURE REQUEST? (choose one): FEATURE REQUEST

Which chart: Artifactory - 7.84.12

What happened: I would like to request an enhancement to the configuration of individual parameters in the readiness probes in the chart. The current layout of the chart does not allow us to make individual changes to parameters such as periodSeconds value without modifying the entire block of configuration. This limitation poses a challenge, particularly when upstream values, such as the readiness endpoint, undergo changes.

Currently, the readiness probe configuration is defined as follows:

router:
    readinessProbe:
      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/readiness
        initialDelaySeconds: {{ if semverCompare "<v1.20.0-0" .Capabilities.KubeVersion.Version }}60{{ else }}0{{ end }}
        periodSeconds: 60
        timeoutSeconds: {{ .Values.probes.timeoutSeconds }}
        failureThreshold: 5
        successThreshold: 1

In the above configuration, altering the periodSeconds parameter requires modification of the entire config block. This approach is not ideal for a couple of reasons:

To address these issues, we propose the following enhancement: Refactor the chart to allow individual parameters within the readiness probe configuration to be overridden without requiring the entire block to be modified. For example, the configuration could be structured to allow direct modification of each standalone parameter:

router:
  readinessProbe:
    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/readiness
    initialDelaySeconds: {{ if semverCompare "<v1.20.0-0" .Capabilities.KubeVersion.Version }}60{{ else }}0{{ end }}
    periodSeconds: 5
    timeoutSeconds: {{ .Values.probes.timeoutSeconds }}
    failureThreshold: 5
    successThreshold: 1

As a result, this change would allow individual parameters to be easily overridden in values files without impacting other parts of the configuration. It would significantly enhance the flexibility and maintainability of the chart.

RobinDuhan commented 4 months ago

Hi @edlyn-liew-octo, Thank you for the feedback, we will evaluate internally and come back.