Enable InfluxDB/Grafana for GCE in addition to GCL. Disable GCM #9309
Conversation
|
EXPERIMENTAL JENKINS PR BUILDER: e2e build succeeded. |
|
EXPERIMENTAL JENKINS PR BUILDER: e2e build succeeded. |
| {% if pillar.get('enable_cluster_monitoring', '').lower() == 'googleinfluxdb' %} | ||
| /etc/kubernetes/addons/cluster-monitoring/googleinfluxdb: | ||
| file.recurse: | ||
| - source: salt://kube-addons/cluster-monitoring |
There was a problem hiding this comment.
I am little bit confused here: you introduced a new mode called googleinfluxdb which enable both GCM and influxdb, right? If I understood it correctly, shouldn't the source here points to cluster/addons/cluster-monitoring/googleinfluxdb which contains yaml files defining this mode?
There was a problem hiding this comment.
In order to enable both GCM/GCL and InfluxDB/Grafana we need to enable all the services/controllers for both (meaning we need everything in cluster/addons/cluster-monitoring/influxdb and cluster/addons/cluster-monitoring/google) except for the heapster-controller files which need to be modified to be able to pass to both backends instead of just one or the other (hence the new heapster-controller-combined file).
So when googleinfluxdb is specified, we pick up all the files under cluster/addons/cluster-monitoring/* including the google, influxdb, and the new googleinfluxdb folders, but we exclude the heapster-controller files (googleinfluxdb contains the new combined heapster-controller-combined file).
An alternative would be to copy all the non-heapster-controller files from influxdb to the new googleinfluxdb folder. That way we could just include googleinfluxdb folder and be done. But I'd prefer not to do that (people will have to remember to modify both instances of each file). Let me know if you feel strongly otherwise.
|
AFAIK #9028 is a blanket bug for improving support for Influxdb. This PR by itself will not resolve that. FYI! |
|
@vishh Let's sync up offline with @dchen1107. |
|
@vissh what kind of improving support for influxdb you expected being included here? Could you please list here? Currently if the user runs a kubernetes cluster on GCE, instead of GKE, by default GCM is enabled for them, but not much metrics at all. And Influxdb and Grafana is disable for them completely. This PR is enabling those open source solution by default, it will be better than today. Also we can iterate them later. |
|
PTAL. New revision disables GCM, enables InfluxDB/Grafana, and leaves GCL enabled. |
|
EXPERIMENTAL JENKINS PR BUILDER: e2e build succeeded. |
saad-ali
changed the title
| command: | ||
| - /heapster | ||
| - --source=kubernetes:https://kubernetes | ||
| - --sink=gcl |
There was a problem hiding this comment.
This will result in events being pushed to both influxdb and GCL. Shall we update heapster to make events optional in influxdb?
There was a problem hiding this comment.
Yep, that's a good idea. Opened up kubernetes-retired/heapster/issues/321
|
LGTM |
|
Can you add LGTM label so that on-call can merge. |
vishh
added
the
lgtm
|
@saad-ali: I assume you have either manually tested this PR or ran e2e. |
|
Yep, manually tested to verify that InfluxDB and Grafana come up without issue. |
|
GCE e2e build/test failed for commit a839f47. |
|
Jenkins |
Enable InfluxDB/Grafana for GCE in addition to GCL. Disable GCM
#9028