Enable GCM and GCL instead of InfluxDB on GCE #7751

saad-ali · 2015-05-05T00:00:28Z

Enabled Google Cloud Monitoring (GCM) and Google Cloud Logging (GCL) instead of InfluxDB for Google Compute Engine (GCE) deployments.
Modified cluster addon setup process to take JSON files in addition to YAML
Converted all Heapster Kubernetes config files to JSON
Get rid of all Heapster v1beta1 configs

vishh · 2015-05-05T04:00:07Z

cc @zmerlynn for salt config changes.

saad-ali · 2015-05-05T06:13:30Z

E2E is green:

Ran 38 of 43 Specs in 1820.331 seconds
SUCCESS! -- 38 Passed | 0 Failed | 1 Pending | 4 Skipped I0504 23:12:37.766365   23115 driver.go:96] All tests pass

a-robinson · 2015-05-05T06:59:15Z

cluster/addons/cluster-monitoring/google/heapster-service.json

+    "ports": [
+      {
+        "port": 80,
+        "targetPort": 8082


Shouldn't the heapster rc pod template expose this port?

Good point. The heapster service is not needed, I will remove it from both influxdb and google.

a-robinson · 2015-05-05T07:08:17Z

What's the motivation for switching to JSON? Just a few days ago @bgrant0607 wrote that "We should just use YAML for examples. It's generally more user-friendly than JSON, and supports comments."

vishh · 2015-05-05T16:42:40Z

cluster/addons/cluster-monitoring/influxdb/heapster-controller.json

+            "env": [
+              {
+                "name": "FLAGS",
+                "value": "--poll_duration=2m --stats_resolution=1m --sink influxdb:http://$MONITORING_INFLUXDB_SERVICE_HOST:$MONITORING_INFLUXDB_PORT_8086_TCP_PORT"


Why use env variables? Why not use DNS? This could be http://monitoring-influxdb:80.
The advantage with dns is that all these services can be started in parallel.

I don't like the idea of setting the port to a constant, I prefer to have it dynamically picked up--so if the service is ever modified and the ports are changed, this will continue to work.

I don't see why this changes whether the sinks can be started in parallel.

The env variables won't be set for heapster pod unless the influxdb service has been created. Since we start all the monitoring services together, there is no guarantee that that influxdb service will be created before heapster.
Support for multi-port DNS is on the way. until then we can hardcode port 80. WDYT?

If that is the case, there would be no guarantee that Kube-DNS would resolve monitoring-influxdb correctly--i.e. if heapster pod gets created before the heapster service, Kube-DNS would not have a entry for monitoring-influxdb--we would end up with the same problem.

With the env variables approach, if the pod comes up before the service, and the variables are not set, heapster will terminate with error--and Kubernetes will create a new container which should have the variables set.

The heapster pod is supposed to handle backend failures. So if the dns name
does not resolve, it will retry internally before deciding to crash/fail.
Crashing without retires might confuse users.

On Tue, May 5, 2015 at 2:30 PM, Saad Ali notifications@github.com wrote:

In cluster/addons/cluster-monitoring/influxdb/heapster-controller.json
#7751 (comment)
:

"template": {

"metadata": {

"labels": {

"name": "heapster",

"kubernetes.io/cluster-service": "true"

}

},

"spec": {

"containers": [

{

"image": "gcr.io/google_containers/heapster:v0.11.0",

"name": "heapster",

"env": [

{

"name": "FLAGS",

"value": "--poll_duration=2m --stats_resolution=1m --sink influxdb:http://$MONITORING_INFLUXDB_SERVICE_HOST:$MONITORING_INFLUXDB_PORT_8086_TCP_PORT"

If that is the case, there would be no guarantee that Kube-DNS would
resolve monitoring-influxdb correctly--i.e. if heapster pod gets created
before the heapster service, Kube-DNS would not have a entry for
monitoring-influxdb--we would end up with the same problem.

With the env variables approach, if the pod comes up before the service,
and the variables are not set, heapster will terminate with error--and
Kubernetes will create a new container which should have the variables set.

—
Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/7751/files#r29715109
.

That's reasonable. I'll go ahead and make the change.

saad-ali · 2015-05-05T22:56:22Z

PTAL.

Regarding JSON: There should be no reason that the cluster addons mechanism doesn't support both JSON and YAML. And it is good to have examples of both in our code. The change was suggested by @vishh -- his argument, as I understood it, was that YAML would internally be translated in to JSON anyway. That said, I don't feel too strongly about it and would be willing to change it back to YAML.

Rerunning E2E for verification.

bgrant0607 · 2015-05-05T22:58:54Z

See #7257

a-robinson · 2015-05-05T23:12:38Z

I agree that the mechanism should support both, just wondering whether we should be writing any in JSON anymore. I'll defer to @bgrant0607 - do you care whether these cluster addon configs are in YAML vs JSON?

saad-ali · 2015-05-05T23:14:50Z

FYI, E2E is green:

Ran 38 of 43 Specs in 1687.865 seconds
SUCCESS! -- 38 Passed | 0 Failed | 1 Pending | 4 Skipped I0505 16:13:05.378063   24215 driver.go:96] All tests pass

bgrant0607 · 2015-05-06T04:59:16Z

Let's please leave the configs in YAML.

saad-ali · 2015-05-06T06:30:00Z

PTAL Switched to YAML

E2E is clean:

Ran 38 of 43 Specs in 1653.461 seconds
SUCCESS! -- 38 Passed | 0 Failed | 1 Pending | 4 Skipped I0505 22:08:03.650021   24184 driver.go:96] All tests pass

a-robinson · 2015-05-06T16:07:53Z

cluster/addons/cluster-monitoring/google/heapster-controller.yaml

@@ -0,0 +1,32 @@
+apiVersion: "v1beta3"


You don't need to quote any of the strings in the yaml files (except perhaps if the string is "true", judging by the deleted files).

a-robinson · 2015-05-06T16:08:48Z

I'm sorry for asking you to change this again, but can you un-quote all the strings in the yaml files?

saad-ali · 2015-05-06T22:05:35Z

No worries. Quotes removed. PTAL

Re-confirmed E2E:

Ran 38 of 43 Specs in 1800.775 seconds
SUCCESS! -- 38 Passed | 0 Failed | 1 Pending | 4 Skipped I0506 14:54:35.184922   29922 driver.go:96] All tests pass

vishh · 2015-05-06T22:10:09Z

cluster/gce/config-test.sh

+#   none     - No cluster monitoring setup 
+#   influxdb - Heapster, InfluxDB, and Grafana 
+#   google   - Heapster, Google Cloud Metrics, and Google Cloud Logging 
+ENABLE_CLUSTER_MONITORING="${KUBE_ENABLE_CLUSTER_MONITORING:-influxdb}"


Ideally we want to use google backend here as well. We need to update the monitoring e2e tests to not to depend on the default cluster addons before switching to google backend.

Sure, we can do that separately.

vishh · 2015-05-06T22:12:29Z

LGTM

a-robinson · 2015-05-06T22:13:11Z

cluster/gce/config-default.sh

+# Optional: Cluster monitoring to setup as part of the cluster bring up:
+#   none     - No cluster monitoring setup 
+#   influxdb - Heapster, InfluxDB, and Grafana 
+#   google   - Heapster, Google Cloud Metrics, and Google Cloud Logging 


Google Cloud Monitoring, not Google Cloud Metrics

a-robinson · 2015-05-06T22:13:24Z

cluster/gce/config-test.sh

+# Optional: Cluster monitoring to setup as part of the cluster bring up:
+#   none     - No cluster monitoring setup 
+#   influxdb - Heapster, InfluxDB, and Grafana 
+#   google   - Heapster, Google Cloud Metrics, and Google Cloud Logging 


Influxdb for Google Compute Engine deployments.

a-robinson · 2015-05-06T22:34:02Z

Thanks! LGTM.

Enable GCM and GCL instead of InfluxDB on GCE

piosz · 2015-05-13T07:29:44Z

cc @jszczepkowski

googlebot added the cla: yes label May 5, 2015

roberthbailey assigned a-robinson May 5, 2015

saad-ali force-pushed the updateHeapsterConfigs branch from ebba6ff to 92609d1 Compare May 5, 2015 05:40

a-robinson reviewed May 5, 2015
View reviewed changes

vishh reviewed May 5, 2015
View reviewed changes

saad-ali force-pushed the updateHeapsterConfigs branch 2 times, most recently from 2cbdfc3 to 6811387 Compare May 5, 2015 22:45

saad-ali force-pushed the updateHeapsterConfigs branch 2 times, most recently from 4cf8ccd to 6588486 Compare May 6, 2015 05:35

a-robinson reviewed May 6, 2015
View reviewed changes

saad-ali force-pushed the updateHeapsterConfigs branch from 6588486 to 970e206 Compare May 6, 2015 21:11

vishh reviewed May 6, 2015
View reviewed changes

a-robinson reviewed May 6, 2015
View reviewed changes

saad-ali force-pushed the updateHeapsterConfigs branch from 970e206 to fac850d Compare May 6, 2015 22:20

Enable Google Cloud Monitoring and Google Cloud Logging instead of

4569de7

Influxdb for Google Compute Engine deployments.

saad-ali force-pushed the updateHeapsterConfigs branch from fac850d to 4569de7 Compare May 6, 2015 22:23

a-robinson added a commit that referenced this pull request May 6, 2015

Merge pull request #7751 from saad-ali/updateHeapsterConfigs

0909cce

Enable GCM and GCL instead of InfluxDB on GCE

a-robinson merged commit 0909cce into kubernetes:master May 6, 2015

saad-ali deleted the updateHeapsterConfigs branch May 6, 2015 23:58

saad-ali mentioned this pull request May 11, 2015

Heapster collects node and pod events #5638

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable GCM and GCL instead of InfluxDB on GCE #7751

Enable GCM and GCL instead of InfluxDB on GCE #7751

saad-ali commented May 5, 2015

vishh commented May 5, 2015

saad-ali commented May 5, 2015

a-robinson May 5, 2015

saad-ali May 5, 2015

a-robinson commented May 5, 2015

vishh May 5, 2015

saad-ali May 5, 2015

vishh May 5, 2015

saad-ali May 5, 2015

vishh May 5, 2015

saad-ali May 5, 2015

saad-ali commented May 5, 2015

bgrant0607 commented May 5, 2015

a-robinson commented May 5, 2015

saad-ali commented May 5, 2015

bgrant0607 commented May 6, 2015

saad-ali commented May 6, 2015

a-robinson May 6, 2015

a-robinson commented May 6, 2015

saad-ali commented May 6, 2015

vishh May 6, 2015

saad-ali May 6, 2015

vishh commented May 6, 2015

a-robinson May 6, 2015

saad-ali May 6, 2015

a-robinson May 6, 2015

saad-ali May 6, 2015

a-robinson commented May 6, 2015

piosz commented May 13, 2015

Enable GCM and GCL instead of InfluxDB on GCE #7751

Enable GCM and GCL instead of InfluxDB on GCE #7751

Conversation

saad-ali commented May 5, 2015

vishh commented May 5, 2015

saad-ali commented May 5, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

a-robinson commented May 5, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saad-ali commented May 5, 2015

bgrant0607 commented May 5, 2015

a-robinson commented May 5, 2015

saad-ali commented May 5, 2015

bgrant0607 commented May 6, 2015

saad-ali commented May 6, 2015

Choose a reason for hiding this comment

a-robinson commented May 6, 2015

saad-ali commented May 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishh commented May 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

a-robinson commented May 6, 2015

piosz commented May 13, 2015