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

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

Closed
fredsig opened this issue Jul 12, 2023 · 1 comment · Fixed by #274
Closed

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

fredsig opened this issue Jul 12, 2023 · 1 comment · Fixed by #274
Assignees
Labels
type: bug Something isn't working

Comments

@fredsig
Copy link

fredsig commented Jul 12, 2023

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
@fredsig fredsig added the type: bug Something isn't working label Jul 12, 2023
@fredsig fredsig changed the title Setting PrometheusMetrics.Enabled to true doesn't add targetPort Setting config.PrometheusMetrics.Enabled to true doesn't add targetPort Jul 12, 2023
@TylerHelmuth
Copy link
Contributor

@fredsig thanks for finding this.

@TylerHelmuth TylerHelmuth self-assigned this Jul 12, 2023
TylerHelmuth added a commit that referenced this issue Jul 12, 2023
… PrometheusMetrics is enabled (#274)

<!--
Thank you for contributing to the project! 💜
Please see our [OSS process
document](https://github.com/honeycombio/home/blob/main/honeycomb-oss-lifecycle-and-practices.md#)
to get an idea of how we operate.
-->

## Which problem is this PR solving?

<please describe the issue>

- Closes #273

## Short description of the changes

- Fixes typo where `enabled` was used instead of `Enabled`

## How to verify that this has the expected result

Tested with `helm template`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
2 participants