INTLY-2597/INTLY-2351 - retention & storage for metrics data

integr8ly / application-monitoring-operator

Operator for installing the Application Monitoring Stack on OpenShift (Prometheus, AlertManager, Grafana)

Apache License 2.0

30 stars 44 forks source link

INTLY-2597/INTLY-2351 - retention & storage for metrics data #56

Closed david-martin closed 5 years ago

david-martin commented 5 years ago

https://issues.jboss.org/browse/INTLY-2597

2 new fields are added to the ApplicationMonitoring CRD

prometheusRetention
prometheusStorageRequest

These allow the retention time & storage claim size to be (optionally) set.

The use case for integreatly is to set a retention of 45d, and a storage claim size depending on the expected size of data in OSD clusters (TBD). However, the integreatly values shouldn't influence the defaults. By default, there should be no storage claim. The retention default can be set to the prometheus default of 15d.

To verify:

follow the instructions to run the operator locally against a remote cluster using make code/run
verify the prometheus entrypoint inclucdes an arg --storage.tsdb.retention=15d
verify the prometheus pod is mounting a PVC of size 10Gi
verify metrics are retained after a restart of promtheus (can delete the prometheus pod and wait for a new one to start)

david-martin commented 5 years ago

@pb82 Could I get your thoughts on this, in particular how the upgrade in an existing cluster would work where the CRD changes?

See PR description for a summary of changes

pb82 commented 5 years ago

@david-martin Regarding the upgrade: we already install a newer version of the AOM CRD in the monitoring upgrade, so i don't see any problem with adding those additional fields and re-applying the CRD. We also let the AOM re-create Prometheus and Alertmanager, so once the upgrade installs the new AOM version, this changes should be available after the upgrade.

Is this a candidate for the 1.4.1->1.5.0 upgrade? If so i can add this to the existing upgrade tasks.

david-martin commented 5 years ago

Is this a candidate for the 1.4.1->1.5.0 upgrade? If so i can add this to the existing upgrade tasks.

Yes. I'll need to update the ApplicationMonitoring CR in the integr8ly installation repo too for this, yes?

pb82 commented 5 years ago

@david-martin No, we currently pull that from the release tag: https://github.com/integr8ly/installation/blob/master/roles/middleware_monitoring/defaults/main.yml#L12

So after this gets merged we need to create a new release and update the installer manifest.

pb82 commented 5 years ago

@david-martin A sorry, i meant the CRD. For the CR, yes, it has to be updated in the installer repo: https://github.com/integr8ly/installation/blob/master/roles/middleware_monitoring/defaults/main.yml#L39

david-martin commented 5 years ago

https://github.com/integr8ly/application-monitoring-operator/pull/57