All Prometheus' targets are omitted if any single ServiceMonitor is configured incorrectly

[jollinshead]

We had:

• prometheus-operator running in its own namespace
• multiple ServiceMonitors in different namespaces which allowed Prometheus to scrape metrics from various sources

...however, when a new ServiceMonitor was created with a port number used (rather than the port name) all targets are removed from Prometheus.

- apiVersion: monitoring.coreos.com/v1alpha1
  kind: ServiceMonitor
  metadata:
    labels:
      app: seahorse-generic-app
    name: seahorse-generic-app
    namespace: seahorse
  spec:
    endpoints:
    - port: 80
    selector:
      matchLabels:
        release: seahorse

We understand port numbers are invalid (only names are valid), but the invalid configuration should not have cleared all targets in Prometheus.

FYI the error we saw in the prometheus-operator pod's logs was:

github.com/coreos/prometheus-operator/vendor/k8s.io/client-go/tools/cache/reflector.go:94: Failed to list *v1alpha1.ServiceMonitor: json: cannot unmarshal number into Go struct field Endpoint.port of type string

[brancz]

Yes this is something that shoulnd be part of validating manifests at creation time. Sadly this is currently not possible with third party resources, and I'm hesitant to making gracefully failing on errors in the workqueue, because that will likely cause actual errors to be swallowed and never seen.

Validation at time of creation will be possible one day though using user aggredated apiservers, which is one of the next things on our list to tackle.

[stale]

This issue has been automatically marked as stale because it has not had any activity in last 60d. Thank you for your contributions.

[alice-sawatzky]

still an issue with AlertRules. We had an AlertRule with an invalid for: value ("<nil>", from a buggy helm template), and it caused the prometheus pods to CrashLoop.

[paulfantom]

@alice-sawatzky your case looks like something that should be addressed by team responsible for helm charts as we are not maintaining them.

[alice-sawatzky]

this was an internal helm chart that i maintain. I fixed the bug just fine, the issue is that an invalid PrometheusRule object shouldn't take down Prometheus.

[brancz]

I agree that this is something that needs to be validated much more strictly. No single PrometheusRule should be able to take out alerting, and probably shouldn't be accepted in the first place. As in it shouldn't be possible to create in the first place.

[stale]

This issue has been automatically marked as stale because it has not had any activity in last 60d. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had any activity in last 60d. Thank you for your contributions.

[brancz]

This should now be fixed with the validating webhooks. Should you use the validating webhooks and this still occurs, please open a new issue.