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

[chart] Don't set prometheus.io pod annotations when ServiceMonitor is enabled #2601

Closed
z0rc opened this issue Apr 6, 2022 · 7 comments · Fixed by #2606
Closed

[chart] Don't set prometheus.io pod annotations when ServiceMonitor is enabled #2601

z0rc opened this issue Apr 6, 2022 · 7 comments · Fixed by #2606
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.

Comments

@z0rc
Copy link

z0rc commented Apr 6, 2022

Describe the bug
https://github.com/kubernetes-sigs/aws-load-balancer-controller/blob/v2.4.1/helm/aws-load-balancer-controller/templates/deployment.yaml#L25-L26 are set always. But with ServiceMonitor enabled, these labels still present. This results in double scraping, as prometheus discovers both pod annotations and ServiceMonitor as independent scraping targets.

Steps to reproduce
Enable ServiceMonitor via helm values, observe, that prometheus.io annotations still set on pods.

Expected outcome
When ServiceMonitor is enabled, prometheus.io pod annotations should be omitted.

Environment

  • AWS Load Balancer controller version: 2.4.1
  • Kubernetes version: 1.22
  • Using EKS (yes/no), if so version? Yes

Additional Context: None

@kishorj kishorj added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Apr 6, 2022
@tamboliasir1
Copy link
Contributor

Hi @z0rc.. Hi I would like to work on this. Just wanted to confirm so I wanted to check serviceMonitor is enabled or not from values if it is enabled then omit Prometheus annotations right?

@z0rc
Copy link
Author

z0rc commented Apr 7, 2022

@tamboliasir1 you are correct, wrap prometheus.io annotations in if block and enable annotations when .Values.serviceMonitor.enabled is false.

@tamboliasir1
Copy link
Contributor

Got it.. Thank you @z0rc

@tamboliasir1
Copy link
Contributor

/assign

@tamboliasir1
Copy link
Contributor

tamboliasir1 commented Apr 10, 2022

Hi @z0rc .. Just wanted to confirm condition will look something like this right?
{{- if not .Values.serviceMonitor.enabled }} prometheus.io/scrape: "true" prometheus.io/port: "{{ (split ":" .Values.metricsBindAddr)._1 | default 8080 }}" {{- end}}

@z0rc
Copy link
Author

z0rc commented Apr 10, 2022

@tamboliasir1 aside from github formatting, this looks okay. But it's better to create a PR and do a code review there.

@tamboliasir1
Copy link
Contributor

Hi @z0rc I have created a PR please review. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.
Projects
None yet
3 participants