openedx / openedx-k8s-harmony

A Prototype Helm Chart for deploying multiple Open edX instances (via Tutor) onto a cluster.
GNU Affero General Public License v3.0
10 stars 14 forks source link

Add monitoring using prometheus-stack #51

Closed gabor-boros closed 9 months ago

gabor-boros commented 1 year ago

Description

This PR adds the prometheus-stack Helm chart as a dependency to monitor the cluster, closing #37 and #38. The stack is crafted by Prometheus.

The default values are disabling Prometheus and Grafana, so if one only needs Prometheus but not Grafana, no extra steps are necessary.

Screenshot 2023-11-27 at 09 41 57

An example configuration for the chart (by the end user) could be

# values.yaml

prometheusstack:
  enabled: true

  grafana:
    enabled: true

    ingress:
      enabled: true
      ingressClassName: nginx
      annotations:
        cert-manager.io/cluster-issuer: "harmony-letsencrypt-global"
        nginx.ingress.kubernetes.io/ssl-redirect: "true"
      hosts:
        - grafana.example.com
      tls:
        - secretName: promstack-ingress-tls
          hosts:
            - grafana.example.com

Testing

  1. Setup a K8s cluster on the provider of your choice
  2. Apply the harmony chart with prometheusstack.enabled = false (not that it is false)
  3. Assign the domain name to the IP address you got
  4. Now, enable Prometheus and Grafana as shown above in the snippet
  5. Apply the changes and wait for propagation
  6. Navigate to grafana.<YOUR DOMAIN> and login with admin/prom-operator
  7. Check that a default chart is loaded, showing metrics of the cluster

OR: navigate to https://grafana.se6017.opencraft.hosting and continue from step 6.

Default Grafana password

Admin password is not pre-generated, because it is not picked up by the grafana pod yet -- this is a bug on their end. For more information, visit: https://github.com/prometheus-community/helm-charts/issues/3679

Author notes

This PR introduces clusterDomain which is a new Harmony chart-specific setting. At the moment, we use it for the NOTES.txt and echo's ingress. However, this could be used later with other templates in the chart.

Why not using the {{ .Values.clusterDomain }} or {{ tpl (.Values.clusterDomain | toYaml) . }} in the values.yaml? Simply put, we can't. It won't be parsed.

openedx-webhooks commented 1 year ago

Thanks for the pull request, @gabor-boros! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

itsjeyd commented 11 months ago

@gabor-boros I'm assuming you'll coordinate with other CCs/maintainers about getting a review, but let me know if I can help with that in any way.

jfavellar90 commented 11 months ago

@gabor-boros could you please rebase your changes? Please let me know to jump right into the review :)

gabor-boros commented 11 months ago

@jfavellar90 The PR is rebased now. You can progress with the review 😊

openedx-webhooks commented 9 months ago

@gabor-boros 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.