appsody / appsody-operator

An Operator for deploying Appsody based applications to Kubernetes. This repo will be archived soon.
Apache License 2.0
18 stars 15 forks source link

Support ServiceMonitor for deployed applications #46

Closed arthurdm closed 5 years ago

arthurdm commented 5 years ago

We need a key under service (perhaps service.enableMonitoring) where under the covers our operator creates the ServiceMonitor that the Prometheus Operator is watching.

navidsh commented 5 years ago

With the following resource definitions, Prometheus operator was able to scrape metrics from Liberty through HTTPS with basic user registry enabled. Liberty used self-signed certificates.

ServiceMonitor:

apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
  labels:
    k8s-app: myapp-monitor
  name: myapp-monitor
  namespace: appsody-app
spec:
  endpoints:
    - basicAuth:
        password:
          key: password
          name: appsody-app
        username:
          key: username
          name: appsody-app
      interval: 5s
      path: /metrics
      scheme: https
      targetPort: 9443
      tlsConfig:
        insecureSkipVerify: true
  namespaceSelector:
    matchNames:
      - appsody-app
  selector:
    matchLabels:
      app.kubernetes.io/name: metrics

AppsodyApplication:

apiVersion: appsody.dev/v1beta1
kind: AppsodyApplication
metadata:
  name: metrics
  namespace: appsody-app
spec:
  applicationImage: 'navidsh/metrics:latest'
  expose: true
  livenessProbe:
    failureThreshold: 30
    httpGet:
      path: /health/live
      port: 9443
      scheme: HTTPS
    initialDelaySeconds: 30
    periodSeconds: 5
  pullPolicy: Always
  readinessProbe:
    failureThreshold: 30
    httpGet:
      path: /health/ready
      port: 9443
      scheme: HTTPS
    initialDelaySeconds: 30
    periodSeconds: 5
  resourceConstraints:
    requests:
      memory: 512Mi
  service:
    annotations:
      prometheus.io/scrape: 'true'
    port: 9443
    type: NodePort
  stack: java-microprofile

Prometheus:

apiVersion: monitoring.coreos.com/v1
kind: Prometheus
metadata:
  name: prometheus
  namespace: appsody-monitoring
  labels:
    prometheus: prometheus
spec:
  enableAdminAPI: false
  resources:
    requests:
      memory: 400Mi
  serviceAccountName: prometheus
  serviceMonitorNamespaceSelector:
    matchLabels:
      prometheus: monitoring
  serviceMonitorSelector:
    matchExpressions:
      - key: k8s-app
        operator: Exists
navidsh commented 5 years ago

I think we should consider the following when designing this:

arthurdm commented 5 years ago

Great work @navidsh - thanks. So we still need the prometheus.io/scrape annotation, even with ServiceMonitor?

arturdzm commented 5 years ago

Nope no need for annotation, we will need to figure out the route, when 9443 is used the expose won't work correctly, as Edge termination policy assumes HTTP connection to pod

navidsh commented 5 years ago

prometheus.io/scrape is added by the Appsody stack. Once we implement this issue, we can open an issue against stacks to remove this.

arturdzm commented 5 years ago

We will need a design for how to provide all this info, eg basic auth, prometheus labels, etc to SM.

arthurdm commented 5 years ago

Need to confirm the following:

  1. for MicroProfile / Java, SpringBoot / Java, and Node.js (express & loopback), what is the default metrics endpoint and does it require auth and/or https.

  2. Based on that, specify min config for operator CR.

  3. Finally, agree on how to configure this (top annotations, vs new fields, vs new field + annotations)

arturdzm commented 5 years ago
seabaylea commented 5 years ago

prometheus.io/scrape is added by the Appsody stack. Once we implement this issue, we can open an issue against stacks to remove this.

We should keep the existing annotations as well unless there's a good reason to remove them - anyone that's deploying Prometheus using Helm (which is most users) will still require the annotation.