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

[Helm] Add optional podLabels to operator pod and Expose Metrics ports #4424

Merged
merged 3 commits into from
Apr 30, 2021

Conversation

oliverbaehler
Copy link
Contributor

@oliverbaehler oliverbaehler commented Apr 16, 2021

Signed-off-by: Oliver Bähler oliver.baehler@bedag.ch

This Pull Request adds two new features:

  1. If the config.metricsPort value is not 0 a port on the container is exposed with that value. Currently this port is not exposed which makes it impossible for a prometheus monitor to scrape them.
  2. podLabels adds the option to label the operator pod with additional labels. We currently have the use case that we need to add some labels to the pod and that option currently does not exist.

I was also wondering if it would be interesting to have a prometheus serviceMonitor built-in to the chart? So that endusers could just flick some options and have their metrics scraped by prometheus. Let me know if that's something you see would add value to the chart and I will be willing to commit the code to this pull request :).

@elasticmachine
Copy link

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

1 similar comment
@elasticmachine
Copy link

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@cla-checker-service
Copy link

cla-checker-service bot commented Apr 16, 2021

💚 CLA has been signed

@botelastic botelastic bot added the triage label Apr 16, 2021
Signed-off-by: Oliver Bähler <oliver.baehler@bedag.ch>
@pebrc
Copy link
Collaborator

pebrc commented Apr 19, 2021

Jenkins test this please.

Signed-off-by: Oliver Bähler <oliverbaehler@hotmail.com>
@oliverbaehler
Copy link
Contributor Author

My latests commit adds the option to deploy a podMonitor with the eck-operator. I tested it locally and it seemed to work fine. Let me know what you think :)

Signed-off-by: Oliver Bähler <oliverbaehler@hotmail.com>
@david-kow david-kow added the >enhancement Enhancement of existing functionality label Apr 20, 2021
@botelastic botelastic bot removed the triage label Apr 20, 2021
@oliverbaehler
Copy link
Contributor Author

oliverbaehler commented Apr 28, 2021

Is there any timeframe on when this will be included? :) (if even)

@barkbay
Copy link
Contributor

barkbay commented Apr 30, 2021

Jenkins test this please

@barkbay barkbay added the v1.6.0 label Apr 30, 2021
Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

For the record I did a test using the following Prometheus resource:

apiVersion: monitoring.coreos.com/v1
kind: Prometheus
metadata:
  name: prometheus
  namespace: prom
spec:
  podMonitorNamespaceSelector:
    matchLabels:
      prometheus: monitoring
  podMonitorSelector:
    matchLabels:
      team: eck

Once created the operator namespace has been labeled with prometheus=monitoring

Then the Helm chart has been deployed with the following command:

% helm install --devel --debug my-elastic-operator my-eck/eck-operator --set=config.metricsPort=9090  --set=image.tag=1.5.0 --set=podMonitor.enabled=true --set=podMonitor.labels.team=eck

Which triggered the creation of the podMonitor resource:

% k get podmonitors.monitoring.coreos.com
NAME               AGE
elastic-operator   2m30s

I was also able to get the operator metrics in the Prometheus console.

Thanks !

@barkbay barkbay merged commit ea74a40 into elastic:master Apr 30, 2021
@idanmo idanmo changed the title [Helm]: Add podLabels and Expose Metrics ports [Helm] Add optional podLabels to operator pod and Expose Metrics ports May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality v1.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants