integr8ly / application-monitoring-operator

Operator for installing the Application Monitoring Stack on OpenShift (Prometheus, AlertManager, Grafana)
Apache License 2.0
30 stars 45 forks source link

Intly 3920 #93

Closed StevenTobin closed 4 years ago

StevenTobin commented 4 years ago

JIRA: https://issues.jboss.org/browse/INTLY-3920

Add the prometheus/alertmanager-isntance-namespaces flags to allow the prometheus operator to define which namespaces to watch for custom resources.

davidkirwan commented 4 years ago

Hmm few thoughts that came to mind, in 3.11 ansible installer, does the CRD get applied at update time? May only be once at RHMI install time, I'm not sure.

The fact we are changing the structure of the CRD, the openshift-monitoring cluster monitoring prometheus install also uses this same CRD. If we're planning to just overwrite it in the cluster we might run into issues in the future with the cluster monitoring stack..

I spoke to @pb82 about it but he said it should be ok.

if the fields are in the CR but not in the CRD -> they will be ignored (If you are only adding some new fields to the CR, and not the CRD, it will depend on the operator version if they will be handled or not, even with the older version installed (which does not have those fields in its spec))

if the fields are in the CRD but not in the CR -> possibly a validation error
StevenTobin commented 4 years ago

I'm adding to the ApplicationMonitoring CRD. The openshift-monitoring prometheus doesn't touch that CRD. And the upgrade logic for AMO in RHMI is scale down the pod, oc apply everything and then scale up, so adding to the CRD shouldn't be an issue.

davidkirwan commented 4 years ago

Verification Steps:

  1. create application-monitoring namespace
  2. apply crds/roles/operator_roles:
    • oc apply -f deploy/crds/
    • oc apply -f deploy/operator_roles/
    • oc apply -f oc apply -f deploy/roles/
    • oc apply -f https://raw.githubusercontent.com/integr8ly/grafana-operator/master/deploy/crds/Grafana.yaml
    • oc apply -f https://raw.githubusercontent.com/integr8ly/grafana-operator/master/deploy/crds/GrafanaDashboard.yaml
    • oc apply -f https://raw.githubusercontent.com/integr8ly/grafana-operator/master/deploy/crds/GrafanaDataSource.yaml
  3. run the application monitoring operator locally: operator-sdk up local --namespace "application-monitoring"
  4. create an instance of the CR: oc apply -f deploy/examples/ApplicationMonitoring.yaml --namespace "application-monitoring"
  5. I see the following added to the prometheus-operator deployment as expected:
            - '--prometheus-instance-namespaces=application-monitoring'
            - '--alertmanager-instance-namespaces=application-monitoring'

@StevenTobin how can I verify from this point on ?

davidkirwan commented 4 years ago

Spoke with @StevenTobin the verification steps are correct.

/lgtm