manticoresearch-helm icon indicating copy to clipboard operation
manticoresearch-helm copied to clipboard

Compatibity with new Prometheus Operators

Open barryhunter opened this issue 2 years ago • 3 comments

Our sysadmin is working on a new deployment of Prometheus, which requires additional configuration to monitor pods. Kinda fuzzy on the terminology, but needs CRDs defined. Either ServiceMonitor or PodMonitor
https://alexandrev.medium.com/prometheus-concepts-servicemonitor-and-podmonitor-8110ce904908

I think in terms of helm chart, needs PodMonitor at least for Worker set, as each pod should be monitored individually, rather than just the service.

For now, defining the PodMonitor outside the Helm chart, something like this

---    
apiVersion: monitoring.coreos.com/v1    
kind: PodMonitor    
metadata:    
  name: manticorert    
  namespace: staging    
  labels:    
    app.kubernetes.io/name: manticoresearch    
    app.kubernetes.io/instance: manticorert    
spec:    
  jobLabel: manticorert    
  namespaceSelector:    
    matchNames:    
    - staging    
  podMetricsEndpoints:    
  - interval: 30s    
    port: "8081"    
  selector:    
    matchLabels:    
      app.kubernetes.io/name: manticoresearch    
      app.kubernetes.io/instance: manticorert    

It seems to be selecting the right pods.

which perhaps ultimately should be generated by the helm chart. Not sure if other people would need this.

But also it seems that the individual container port, should be 'exposing' the 8081 port directly.
https://kubernetes.io/docs/tutorials/services/connect-applications-service/#exposing-pods-to-the-cluster
so the new operator can read the metrics.

We have a seperate (pre helm chart) manticore setup, and was able to easily get a manticore-exporter conainer added, but includs a defined port

      - name: manticore-exporter    
        image: manticoresearch/prometheus-exporter:3.6.0.0    
        imagePullPolicy: IfNotPresent    
        ports:   ## this seems to be required.     
        - containerPort: 8081    
          name: prometheus     
        env:    
        - name: MANTICORE_HOST    
          value: "127.0.0.1"    
        - name: MANTICORE_PORT    
          value: "9306"    
        livenessProbe:    
          httpGet:    
            path: /health    
            port: 8081    
          initialDelaySeconds: 3    
          periodSeconds: 3    

So could add it to the helm chart
https://github.com/manticoresoftware/manticoresearch-helm/blob/master/charts/manticoresearch/templates/manticore-balancer.yaml#L93

So the question is, would others be ok with this? Could I submit a PR to add the port to the monitor containers, and define a podMonitor?

barryhunter avatar Feb 09 '23 18:02 barryhunter

Hi Barry. Let me talk it over with the team.

sanikolaev avatar Feb 13 '23 06:02 sanikolaev

I'm the admin working with @barryhunter on this. Most charts implement this as an opt-in feature, for example:

  • https://github.com/kubernetes-sigs/external-dns/blob/master/charts/external-dns/values.yaml#L62-L106
  • https://github.com/kubernetes-sigs/external-dns/blob/master/charts/external-dns/templates/servicemonitor.yaml

In the case of ExternalDNS the metrics are exposed on the only port exposed by the Service so it needed nothing special; because you're using a separate exporter sidecar you'll need to make sure the port is exposed by the pods (and included in the Service if using a ServiceMonitor).

bootc avatar Feb 15 '23 11:02 bootc

➤ Klim Todrik commented:

Yes it's ok. As I right understand we using deprecated definition via annotations. So it will be great if you will add PodMonitor.

make sure the port is exposed by the pods yes it's exposed in dockerfile

githubmanticore avatar Feb 16 '23 17:02 githubmanticore