honeycombio / helm-charts

Helm repository and charts for Honeycomb
Apache License 2.0
30 stars 39 forks source link

Setting config.PrometheusMetrics.Enabled to true doesn't add targetPort #273

Closed fredsig closed 1 year ago

fredsig commented 1 year ago

Versions

Chart: 2.1.0 Helm: 3.11.2 Kubernetes cluster: 1.23

Steps to reproduce

The latest version of the chart doesn't add a targetPort for both the service and deployment resources when PrometheusMetrics is enabled because the template is looking for enabled but not Enabled (which is case sensitive). The following will trigger the port to be added but of course, will fail the config parsing:

  PrometheusMetrics:
    Enabled: true
    enabled: true

Snippet of the helm diff output for the Service resource after setting enabled: true:

      - port: 4317
        targetPort: grpc
        protocol: TCP
        name: grpc
+     - port: 9090
+       targetPort: metrics
+       protocol: TCP
+       name: metrics

    selector:
      app.kubernetes.io/name: refinery
      app.kubernetes.io/instance: honeycomb-refinery

Additional context Only workaround is to fork the Chart and fix it with on both service and deployment templates:

    {{- if .Values.config.PrometheusMetrics.Enabled }}
    - port: 9090
      targetPort: metrics
      protocol: TCP
      name: metrics
TylerHelmuth commented 1 year ago

@fredsig thanks for finding this.