search

aerogearcatalog / metrics-apb

1 stars 23 forks source link

Split grafana and prometheus into separate pods #7

Closed darahayes closed 6 years ago

darahayes commented 6 years ago

The result of this PR is that grafana and prometheus are split into their own pods.

In addition, a new service called prometheus-internal was added which directly exposes the prometheus container. (no proxy). This additional service is needed so grafana can access prometheus as a datasource. This approach was outlined in the metrics proposal. I'm looking for suggestions on the name though.

The datasource config file seems to be broken at the moment, so you will not see the prometheus data source show up automatically when the apb is run. You can manually add the prometheus service afterwards using the hostname http://prometheus-internal

I'm aware that there is still plenty of room for improvement in this APB which I think should be tackled in a separate PR.

darahayes commented 6 years ago

cc @wtrocki

wtrocki commented 6 years ago

@darahayes Verified locally. It works like dream for general dashboard.

darahayes commented 6 years ago

@paolobueno suggested splitting the grafana and prometheus provisioning into separate roles. While I initially agreed with this approach, I did some investigation and I've concluded there's no benefit in doing this for the time being.

The ansibleplaybookbundle org has a number of example APBs that provision multiple services e.g. (Application + Database) and they seem to keep them in the same roles.

@paolobueno @wtrocki what do you think?

wtrocki commented 6 years ago

@darahayes Do you need me to verify that again?

darahayes commented 6 years ago

Yes please @wtrocki . Now that the name has been changed I think it would be wise to reverify.

wtrocki commented 6 years ago

@paolobueno Can you verify that as well? I'm happy to merge (already approved this change)

paolobueno commented 6 years ago

@wtrocki tried verfiying but blocked on my local origin cluster: https://gist.github.com/paolobueno/c3430125b2ab43f22884597f6314b29d

Feel free to go ahead and merge!

wtrocki commented 6 years ago

@paolobueno Looks like missing selinux options for openshift containers.

Any non trivial PR's should have 2 approvals. Added more people to share ideas.

wtrocki commented 6 years ago

@pb82 thanks for help with verification. Let's merge this.