quarkiverse / quarkus-helm

Quarkus Extension to generate the Helm artifacts
Apache License 2.0
10 stars 8 forks source link

Probes are using the first port which is not consistent with the generated manifest or settings #269

Closed kjq closed 1 year ago

kjq commented 1 year ago

It looks like the Helm generated "probes" are using the wrong (first) port listed in the manifest. This varies across multiple projects during the same build - some will work (because it is http first) and others will not (because it is https first).

In the Kubernetes configuration (same across all services):

kubernetes:
      deployment-target: kubernetes
      liveness-probe:
        http-action-port-name: http
      ports:
        http:
          path: /blah
      readiness-probe:
        http-action-port-name: http
      startup-probe:
        http-action-port-name: http

Generated using the Helm extension (wrong) it takes the first defined ports. Running the same build across multiple projects, if that first port is http then it is right...

livenessProbe:
  failureThreshold: {{ .Values.helm.livenessProbe.failureThreshold }}
  httpGet:
    path: {{ .Values.helm.livenessProbe.httpGet.path }}
    port: {{ .Values.helm.ports.https }}
    scheme: {{ .Values.helm.livenessProbe.httpGet.scheme }}
ports:
  - containerPort: {{ .Values.helm.ports.https }}
    name: https
    protocol: TCP
  - containerPort: {{ .Values.helm.ports.http }}
    name: http
    protocol: TCP
readinessProbe:
  failureThreshold: {{ .Values.helm.readinessProbe.failureThreshold }}
  httpGet:
    path: {{ .Values.helm.readinessProbe.httpGet.path }}
    port: {{ .Values.helm.ports.https }}
    scheme: {{ .Values.helm.readinessProbe.httpGet.scheme }}
startupProbe:
  failureThreshold: {{ .Values.helm.startupProbe.failureThreshold }}
  httpGet:
    path: {{ .Values.helm.startupProbe.httpGet.path }}
    port: {{ .Values.helm.ports.https }}
    scheme: {{ .Values.helm.startupProbe.httpGet.scheme }}

Here is an example from the same build where the first port is http and it is correct:

ports:
  - containerPort: {{ .Values.helm.ports.http }}
    name: http
    protocol: TCP
  - containerPort: {{ .Values.helm.ports.https }}
    name: https
    protocol: TCP
readinessProbe:
  failureThreshold: {{ .Values.helm.readinessProbe.failureThreshold }}
  httpGet:
    path: {{ .Values.helm.readinessProbe.httpGet.path }}
    port: {{ .Values.helm.ports.http }}
    scheme: {{ .Values.helm.readinessProbe.httpGet.scheme }}

The regular Kubernetes generated manifest (Quarkus) is always correct:

livenessProbe:
            failureThreshold: 3
            httpGet:
              path: /q/health/live
              port: 8080
              scheme: HTTP
            initialDelaySeconds: 5
            periodSeconds: 10
            successThreshold: 1
            timeoutSeconds: 10

Running multiple times in a row demonstrates the issue.

As a workaround this seems to work for now:

        "port":
          path: "(kind == Deployment).spec.template.spec.containers.*.httpGet.port"
          expression: "{{ .Values.helm.ports.http }}"
Sgitario commented 1 year ago

Thanks for reporting @kjq I was aware of this issue and it was partially fixed in 1.0.8 (see the known issues section here: https://github.com/quarkiverse/quarkus-helm/releases/tag/1.0.8).

Unfortunately, the issue was caused by Dekorate (which is used by Quarkus) and fixed by this pull request, so we can't do much in Quarkus Helm.

The final fix is part of Quarkus Helm 1.0.9 and Quarkus 3.2.0.Final. If you can't bump to the latest Quarkus version, then the only workaround is to disable the https port generation in Quarkus by adding quarkus.http.insecure-requests=disabled.

I hope it helps and sorry for the inconvenience. I'm closing this issue, but feel free to reopen it if you think there is something else we could do on the Quarkus Helm side.