Add support fort kubelet stats summary #306

cpuguy83 · 2018-08-09T00:20:18Z

This allows hipster or metrics-server to scrape stats from VK for providers that support it.
Of course added an implementation for the Azure provider.

By default this enables a new HTTP listener on :10255 with a /stats/summary endpoint.
hitting this endpoint will give a result like this:

{
  "node": {
    "nodeName": "vk-metrics",
    "startTime": "2018-08-08T23:52:26Z"
  },
  "pods": [
    {
      "podRef": {
        "name": "nginx",
        "namespace": "default",
        "uid": "4305386a-9b66-11e8-b3ec-000d3a5dae4d"
      },
      "startTime": "2018-08-01T23:53:34Z",
      "containers": [
        {
          "name": "nginx",
          "startTime": "2018-08-01T23:53:34Z",
          "cpu": {
            "time": "2018-08-09T00:14:00Z",
            "usageNanoCores": 0
          },
          "memory": {
            "time": "2018-08-09T00:14:00Z",
            "usageBytes": 23470080
          },
          "userDefinedMetrics": null
        }
      ],
      "cpu": {
        "time": "2018-08-09T00:14:00Z",
        "usageNanoCores": 0
      },
      "memory": {
        "time": "2018-08-09T00:14:00Z",
        "usageBytes": 23470080
      },
      "network": {
        "time": "2018-08-09T00:14:00Z",
        "name": "",
        "rxBytes": 254,
        "txBytes": 172
      }
    }
  ]
}

Everything is great... except it's not actually working. metrics-server is scraping that stats but not storing the results.
This leads to, in the above case, a message in the metrics server logs when you try to fetch metrics from it like reststorage.go:93] No metrics for pod default/nginx, even though I know it (or something... something else for me to check) grabbing metrics from VK.

Posting this here for review/discussion while I try to figuring out why the stats are not actually being picked up. Also please let me know if you have some ideas as to what might be causing this.

cpuguy83 · 2018-08-09T00:24:27Z

providers/azure/metrics.go

+			stat.Network.TxBytes = &bytes
+		}
+		stat.Network.Time = metav1.NewTime(data.Timestamp)
+		stat.Network.InterfaceStats.Name = "eth0"


Cheating on the name here. Info is not available in the API AFAICT.
Also wanted to make sure that a missing name wasn't causing my issues with metrics-server (it's not...)

cpuguy83 · 2018-08-09T00:25:13Z

providers/azure/aci.go

@@ -216,6 +217,7 @@ func NewACIProvider(config string, rm *manager.ResourceManager, nodeName, operat
 	p.nodeName = nodeName
 	p.internalIP = internalIP
 	p.daemonEndpointPort = daemonEndpointPort
+	p.startTime = time.Now()


Cheating here a bit... really was just checking if this was an issue with metrics-server... doesn't appear to be.

cpuguy83 · 2018-08-10T00:44:39Z

Ok, so after adding in some extra logging, it seems metrics-server is not populating pod CPU stats (have to investigate why still as vk is providing it), when the metrics-server API is hit it skips the pod because the pod CPU stats are missing.

All in all, the CPU stats are a bit rough for me to wrap my head around how to report. Right now taking the provided average millicores, converting to nano, and multiplying by 60 (stat interval from ACI) to produce the number of nanoseconds of core time for the container... and also having to reset the core time for each stat request.

The k8s stats summary does support nanocore usage, but metrics-server (and hipster) don't use it from what I can tell.

Anyway, please let me know if you have some suggestions here.

robbiezhang · 2018-08-10T05:12:01Z

providers/azure/metrics.go

+	stats "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1"
+)
+
+const startTimeFormat = "2006-01-01 15:04:05 -0700 MST"


Ah nice catch, that would be why I'm getting negative uptime :)

robbiezhang · 2018-08-10T05:16:34Z

vkubelet/vkubelet.go

@@ -151,6 +151,11 @@ func New(nodeName, operatingSystem, namespace, kubeConfig, taint, provider, prov
 	}

 	go ApiserverStart(p)
+	if metricsAddr != "" {


since it has a default value :10255, we don't need expect it's empty

robbiezhang · 2018-08-10T05:18:45Z

vkubelet/apiserver.go

@@ -32,13 +33,50 @@ func ApiserverStart(provider Provider) {
 	r := mux.NewRouter()
 	r.HandleFunc("/containerLogs/{namespace}/{pod}/{container}", ApiServerHandler).Methods("GET")
 	r.HandleFunc("/exec/{namespace}/{pod}/{container}", ApiServerHandlerExec).Methods("POST")
+	r.HandleFunc("/stats/summary", MetricsSummaryHandler).Methods("GET")


remove as they are added in MetricsServerStart function

robbiezhang · 2018-08-10T05:20:23Z

vkubelet/apiserver.go

@@ -32,13 +33,50 @@ func ApiserverStart(provider Provider) {
 	r := mux.NewRouter()
 	r.HandleFunc("/containerLogs/{namespace}/{pod}/{container}", ApiServerHandler).Methods("GET")
 	r.HandleFunc("/exec/{namespace}/{pod}/{container}", ApiServerHandlerExec).Methods("POST")
+	r.HandleFunc("/stats/summary", MetricsSummaryHandler).Methods("GET")
+	r.HandleFunc("/stats/summary/", MetricsSummaryHandler).Methods("GET")
 	r.NotFoundHandler = http.HandlerFunc(NotFound)


set the NotFoundHandler in the MetricsServer too.

cpuguy83 · 2018-08-10T18:29:18Z

Ok, so the real main issue is that it wants node level stats...

https://github.com/kubernetes-incubator/metrics-server/blob/f90c6705d2381ea2db1a6343da6c400bd2ef4cb2/metrics/storage/util/util.go#L27-L34

I'm not sure if any value makes sense for these stats considering that metrics-server assumes all the pods are on the node.

cpuguy83 · 2018-08-10T18:30:49Z

I should note metrics-server and heapster were nearly identical codebases until a couple of days ago. So maybe metrics-server HEAD behaves differently, I will have to look into it.

cpuguy83 · 2018-08-13T22:38:17Z

Ok, this is working.

Please pay extra attention to stat calculations. @robbiezhang I'm sure you have more insight into what that cpu stat is actually saying about the container group.

cpuguy83 · 2018-08-14T23:04:16Z

Ok, I've removed WIP from this.

robbiezhang · 2018-08-16T23:06:14Z

providers/azure/metrics.go

+		p.metricsSyncTime = time.Now()
+	}()
+
+	cgs, err := p.aciClient.ListContainerGroups(p.resourceGroup)


I think we should get the pods from resource manager. Ideally, it should be in-sync with the container groups in the resource group. Since it's in-memory, it's more lightweight, and won't be throttled by ARM.

You need to filter out the terminated and pending ones.

robbiezhang · 2018-08-16T23:20:18Z

providers/azure/client/aci/types.go

+// MetricMetadataValue stores extra metadata about a metric
+// In particular it is used to provide details about the breakdown of a metric dimension.
+type MetricMetadataValue struct {
+	Name  ValueDescriptor


`json:"name"`

robbiezhang · 2018-08-16T23:21:46Z

providers/azure/metrics.go

+	p.metricsSync.Lock()
+	defer p.metricsSync.Unlock()
+
+	if time.Now().Sub(p.metricsSyncTime) < 30*time.Second {


1 minute? since ACI metrics is PT1M

robbiezhang · 2018-08-16T23:30:51Z

providers/azure/metrics.go

+	}
+
+	var errGroup errgroup.Group
+	chResult := make(chan stats.PodStats, len(cgs.Value))


by value? prefer pointer to avoid object copy.

robbiezhang · 2018-08-16T23:32:44Z

providers/azure/metrics.go

+
+				// average is the average number of millicores over a 1 minute interval (which is the interval we are pulling the stats for)
+				nanoCores := uint64(data.Average * 1000000)
+				usuageNanoSeconds := nanoCores * 60


typo 'usage'

robbiezhang · 2018-08-16T23:59:48Z

vkubelet/apiserver.go

+	if metricsAddr != "" {
+		go MetricsServerStart(metricsAddr)
+	} else {
+		log.Println("skipping metrics server startup sicne no address was provided")


typo: ... startup 'since' no address ...

robbiezhang · 2018-08-17T00:03:46Z

vkubelet/apiserver.go

+	r.HandleFunc("/stats/summary", MetricsSummaryHandler).Methods("GET")
+	r.HandleFunc("/stats/summary/", MetricsSummaryHandler).Methods("GET")
+	r.NotFoundHandler = http.HandlerFunc(NotFound)
+	http.ListenAndServe(addr, r)


log the error

robbiezhang · 2018-08-17T00:08:52Z

vkubelet/apiserver.go

+	}
+
+	if _, err := w.Write(b); err != nil {
+		io.WriteString(w, err.Error())


what if it still fail?

robbiezhang · 2018-08-17T00:11:27Z

vkubelet/apiserver.go

@@ -34,11 +37,58 @@ func ApiserverStart(provider Provider) {
 	r.HandleFunc("/exec/{namespace}/{pod}/{container}", ApiServerHandlerExec).Methods("POST")
 	r.NotFoundHandler = http.HandlerFunc(NotFound)

+	if metricsAddr != "" {
+		go MetricsServerStart(metricsAddr)


can we move the Metrics Server into a new file, and start it from the vkubelet.go?

The reason I didn't do this for now is right now ApiServerStart is setting a global (for provider) and these will race with each other.
I'd rather refactor this in a separate PR.

robbiezhang

some minor issues and typos

cpuguy83 · 2018-08-17T20:54:00Z

This is all fixed up.

robbiezhang

LGTM. do you want to merge #323 first, and integrate with the new logging in this PR?

cpuguy83 · 2018-08-17T21:53:10Z

Yeah, that might be easier 😃

This adds a new, optional, interface for providers that want to provide stats.

cpuguy83 · 2018-08-20T18:37:47Z

This is all integrated, merging.

mcraken · 2018-10-18T15:52:54Z

Is the stats endpoint running now for the virtual kubelet? I see from the metrics server that it is still not able to scrap metrics because it can not find the stats endpoint. The virtual kubelet version is 1.11.2.

Add support fort kubelet stats summary

cpuguy83 mentioned this pull request Aug 9, 2018

Pods (Container Instances) not able to publish metrics #279

Closed

cpuguy83 requested review from robbiezhang and jeremyrickard August 9, 2018 00:22

cpuguy83 commented Aug 9, 2018

View reviewed changes

cpuguy83 force-pushed the metrics branch 2 times, most recently from b4c0314 to b06f18b Compare August 9, 2018 19:38

cpuguy83 mentioned this pull request Aug 9, 2018

help for virtual-kubelet stats kubernetes-sigs/metrics-server#96

Closed

robbiezhang reviewed Aug 10, 2018

View reviewed changes

cpuguy83 force-pushed the metrics branch from b06f18b to cb17bb5 Compare August 13, 2018 22:32

cpuguy83 force-pushed the metrics branch 3 times, most recently from c24012b to b331fbf Compare August 14, 2018 22:58

cpuguy83 changed the title ~~[WIP] Add support fort kubelet stats summary~~ Add support fort kubelet stats summary Aug 14, 2018

cpuguy83 force-pushed the metrics branch from b331fbf to dcd9f55 Compare August 15, 2018 19:32

robbiezhang reviewed Aug 16, 2018

View reviewed changes

robbiezhang reviewed Aug 17, 2018

View reviewed changes

cpuguy83 force-pushed the metrics branch 2 times, most recently from 411c11f to 2960aa2 Compare August 17, 2018 20:51

cpuguy83 force-pushed the metrics branch from 2960aa2 to e267681 Compare August 17, 2018 21:23

robbiezhang approved these changes Aug 17, 2018

View reviewed changes

cpuguy83 added 3 commits August 17, 2018 16:55

Update kubelet vendor to pull in stats API

17cc303

Add errgroup dep which will be used for ACI stats

6284757

Add supports for stats in ACI provider

e8abca0

This adds a new, optional, interface for providers that want to provide stats.

cpuguy83 force-pushed the metrics branch from e267681 to e8abca0 Compare August 18, 2018 00:04

cpuguy83 merged commit 99e0cfa into virtual-kubelet:master Aug 20, 2018

cpuguy83 deleted the metrics branch August 20, 2018 18:37

This was referenced Sep 20, 2018

[Azure] Cannot get metrics from virtual kubelet (Unable to get metrics for resource cpu: no metrics returned from heapster) #228

Closed

Investigation: Pod Metrics #168

Closed

cpuguy83 added a commit to cpuguy83/virtual-kubelet that referenced this pull request Feb 27, 2019

Merge pull request virtual-kubelet#306 from cpuguy83/metrics

a102151

Add support fort kubelet stats summary

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support fort kubelet stats summary #306

Add support fort kubelet stats summary #306

cpuguy83 commented Aug 9, 2018

cpuguy83 Aug 9, 2018

cpuguy83 Aug 9, 2018

cpuguy83 commented Aug 10, 2018

robbiezhang Aug 10, 2018 •

edited

cpuguy83 Aug 10, 2018

robbiezhang Aug 10, 2018

robbiezhang Aug 10, 2018

robbiezhang Aug 10, 2018

cpuguy83 commented Aug 10, 2018

cpuguy83 commented Aug 10, 2018

cpuguy83 commented Aug 13, 2018

cpuguy83 commented Aug 14, 2018

robbiezhang Aug 16, 2018

robbiezhang Aug 16, 2018 •

edited

robbiezhang Aug 16, 2018

robbiezhang Aug 16, 2018

robbiezhang Aug 16, 2018 •

edited

robbiezhang Aug 16, 2018

robbiezhang Aug 16, 2018

robbiezhang Aug 17, 2018

robbiezhang Aug 17, 2018

robbiezhang Aug 17, 2018

cpuguy83 Aug 17, 2018

robbiezhang left a comment

cpuguy83 commented Aug 17, 2018

robbiezhang left a comment

cpuguy83 commented Aug 17, 2018

cpuguy83 commented Aug 20, 2018

mcraken commented Oct 18, 2018

Add support fort kubelet stats summary #306

Add support fort kubelet stats summary #306

Conversation

cpuguy83 commented Aug 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpuguy83 commented Aug 10, 2018

robbiezhang Aug 10, 2018 • edited

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

cpuguy83 commented Aug 10, 2018

cpuguy83 commented Aug 10, 2018

cpuguy83 commented Aug 13, 2018

cpuguy83 commented Aug 14, 2018

Choose a reason for hiding this comment

robbiezhang Aug 16, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robbiezhang Aug 16, 2018 • edited

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

Choose a reason for hiding this comment

robbiezhang left a comment

Choose a reason for hiding this comment

cpuguy83 commented Aug 17, 2018

robbiezhang left a comment

Choose a reason for hiding this comment

cpuguy83 commented Aug 17, 2018

cpuguy83 commented Aug 20, 2018

mcraken commented Oct 18, 2018

robbiezhang Aug 10, 2018 •

edited

robbiezhang Aug 16, 2018 •

edited

robbiezhang Aug 16, 2018 •

edited