search

kubernetes-sigs / aws-load-balancer-controller

A Kubernetes controller for Elastic Load Balancers
https://kubernetes-sigs.github.io/aws-load-balancer-controller/
Apache License 2.0
3.8k stars 1.4k forks source link

Service Monitor #3694

Open rjop-hccgt opened 1 month ago

rjop-hccgt commented 1 month ago

Describe the bug I believe there is an issue with the Helm chart, particularly in this line (https://github.com/kubernetes-sigs/aws-load-balancer-controller/blob/main/helm/aws-load-balancer-controller/templates/deployment.yaml#L30). I think the logic should be inverted and the prometheus labels should be added only when the .Values.serviceMonitor.enabled flag is set to true

Steps to reproduce

  1. In the values.yaml set the serviceMonitor.enabled to true.
  2. Run helm template ./ --values values.yaml | kubectl apply -f -

Expected outcome The AWS Load Balancer controller deployment shouldn't have the prometheus annotations enabled.

Environment

Additional Context:

omerap12 commented 1 month ago

Yes, I'll take this

omerap12 commented 1 month ago

/assign

omerap12 commented 3 weeks ago

ping @rjop-hccgt

rjop-hccgt commented 3 weeks ago

thanks @omerap12

oliviassss commented 3 weeks ago

@rjop-hccgt, @omerap12, I think this is on purpose to avoid double scraping by prometheus, see: https://github.com/kubernetes-sigs/aws-load-balancer-controller/issues/2601

omerap12 commented 2 weeks ago

Hey @oliviassss , I checked, and you're right—it’s done on purpose. But, with our current approach, we're forcing the use of Prometheus, either by using ServiceMonitor or annotations on deployments. Should we really be enforcing this?