grafana / loki

Like Prometheus, but for logs.
https://grafana.com/loki
GNU Affero General Public License v3.0
23.54k stars 3.4k forks source link

helm chart should not install Prometheus CRDs #7488

Open sathieu opened 1 year ago

sathieu commented 1 year ago

Describe the bug

Prometheus is installed using another chart (or any other method). The prometheus CRDs are provided by this installation method. Installing also the CRD from the loki chart may produce CRD upgrade and downgrade with possible data lost.

To Reproduce Steps to reproduce the behavior:

  1. Install prometheus-stack and loki charts with ArgoCD

-> There is a diff, between the CRD version in prometheus-stack and in loki.

Expected behavior

No conflict

Environment:

Screenshots, Promtail config, or terminal output If applicable, add any output to help explain your problem.

dotdc commented 1 year ago

If you use ArgoCD, you can simply skip CRD's installation using skipCrds: true:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: loki
  namespace: argocd
  labels:
    app.kubernetes.io/managed-by: argocd
  finalizers:
  - resources-finalizer.argocd.argoproj.io
spec:
  project: {{ .Values.project }}
  source:
    chart: loki
    repoURL: https://grafana.github.io/helm-charts
    targetRevision: {{ .Values.loki.version }}
    helm:
      skipCrds: true
sathieu commented 1 year ago

@dotdc Thanks for the info, but I still thinks this is a bad practice to include CRDs from other components.

I can move the CRDs to templates (with an if), if requested.

jeschkies commented 1 year ago

As I mentioned in the pull requests the CRDs are used by the Grafana Agent Operator that is installed with this Helm Chart. We could couple the. Also, why is the Prometheus chart installing a CRD that's conflicting. E.g. our service monitor soups only target the Loki installation.

sathieu commented 1 year ago

The agent-operator chart has ServiceMonitor CRD too and a few others. Maybe move the prometheusrules.yaml CRD there and remove ServiceMonitor from the loki chart? This way, all those CRDs won't be installed when the agent is disabled.

trallnag commented 1 year ago

Hasn't this been merged as part of https://github.com/grafana/loki/pull/7499

pblgomez commented 1 year ago

It would be good to have an option to disable prometheus CRD's but for now what I use is:

ignoreDifferences:
  - group: '*'
    jsonPointers:
    - /
    kind: CustomResourceDefinition
b-a-t commented 1 year ago

Also, why is the Prometheus chart installing a CRD that's conflicting. E.g. our service monitor soups only target the Loki installation.

Well, it's the way around. Why Loki(ok, grafana-agent-operator, which is brought as a dependency for self-monitoring) installs Prometheus CRDs, namely:

monitoring.coreos.com_podmonitors.yaml
monitoring.coreos.com_probes.yaml
monitoring.coreos.com_servicemonitors.yaml

Those clearly belong to Prometheus(-operator) and should be left to it for management and ownership.

I recall seeing somewhere in the CHANGELOG that G-A-O is shipped with the Prometheus library now and it actually can handle those resources by itself, but most of the people do use kube-prometheus-stack in their setups and introducing it's half-way replacement creates a lot of headache.

It would be great if grafana-agent-operator could have this part of functionality switchable and be enabled only if there is no local Prometheus installed.

batazor commented 1 year ago

Or at least update prometheus-crd to the latest versions to avoid conflicts

PR: https://github.com/grafana/loki/pull/9041

batazor commented 1 year ago

any updates?

azkore commented 1 year ago

Hasn't this been merged as part of #7499

That PR removed prometheus CRDs from the loki chart itself. However, when the monitoring feature is active, the Loki chart deploys the grafana-agent-operator as a dependency. This operator chart includes Prometheus CRDs. Probably this issue needs to be moved to https://github.com/grafana/helm-charts or to https://github.com/grafana/agent

kuzm1ch commented 1 year ago

+1 to this thread, another scenario can be to specify which group of crds we can exclude from deploying:

crds:
  grafana-agent:
     enabled: true
  prometheus:
     enabled: true

and skip the installation of Prometheus crds with disable option. inspired by kube-prometheus-stack but haven't tested yet.

For now, I will handle crds in a separate folder with "helm skip crd" option, but it will be good to have something similar to what was proposed.

Berman510 commented 11 months ago

+1 to this. The above would be amazing.

nicl-dev commented 7 months ago

+1 from me too, please take a look at the PR. It's a pain to skip the installation of CRDs in Argo and then manage installation and upgrades manually.

Nico-Hab commented 6 months ago

This just completely broke my dev cluster. +1 Loki should not install CRDs of other projects

visdmin commented 3 months ago

Plus 1️⃣ for this issue !