Install the bpf program for all pods and collect/expose the data #13

robertgzr · 2017-07-26T16:28:45Z

This introduces a bpf controller that runs the lookup look for every cgroup we attach a program to and executes handlers that write the tracked data to Prometheus metrics that get exposed on port 9101. The top subcommand uses the same code but implements different handlers. It also improves overall logging and adds a flag to adjust the verbosity.

Closes #4, #5, #6, #9

alban · 2017-07-27T08:59:21Z

Gopkg.lock

@@ -104,6 +104,12 @@
  revision = "f3f9494671f93fcff853e3c6e9e948b3eb71e590"

 [[projects]]
+  name = "github.com/go-stack/stack"


why is this pkg mentioned here? it does not seem to be used anywhere (neither in cgnet or in the vendored pkgs), or vendored?

AFAIU this file is generated by the package manager.

So to answer your question, one would have to look into the dep's behaviour.

alban · 2017-07-27T08:59:47Z

Dockerfile

@@ -5,5 +5,5 @@ RUN apk update && apk add --no-cache libc6-compat
 COPY cgnet /cgnet

 EXPOSE 9101
-ENTRYPOINT ["/cgnet", "serve"]
+ENTRYPOINT ["/cgnet", "--verbose", "2", "export"


the parameters could be on the CMD line

alban · 2017-07-27T09:01:23Z

Gopkg.lock

-  revision = "b105bd37f74e5d9dc7b6ad7806715c7a2b83fd3f"
-  version = "v2.11"
+  packages = [".","term"]
+  revision = "74a0988b5f804e8ce9ff74fca4f16980776dff29"


any reason for using master instead of a released version?

alban · 2017-07-27T09:09:12Z

bpf/controller.go

+			return
+		case <-c.quit:
+			return
+		case <-time.After(1000 * time.Millisecond):


could use time.Second

alban · 2017-07-27T09:11:43Z

bpf/controller.go

+			return
+		case <-time.After(1000 * time.Millisecond):
+			lookup(c.module, packetsKey, c.packetsHandler)
+			lookup(c.module, bytesKey, c.bytesHandler)


Why using a callback in parameter 3 instead of just having the lookup function returning the value?

c.packetsHandle(lookup(c.module, packetsKey))

alban · 2017-07-27T09:13:02Z

bpf/src/cgnet.c

@@ -19,7 +19,7 @@
 #include <uapi/linux/ip.h>
 #include "bpf_helpers.h"

-struct bpf_map_def SEC("maps/count") count_map = {
+struct bpf_map_def SEC("maps/cgnet") count_map = {


Could you use the same name for the section and the variable name?
something like:

struct bpf_map_def SEC("maps/count_map") count_map = {

Please add a comment to document:

what is the key: define the keys nearby the map definition, so it's clearer.

what is the value: a counter

how it is used to communicate: values initialized to zero by userspace and incremented by the ebpf program

Just before the map definition:

#define PACKETS_KEY 0 #define BYTES_KEY 1

In the count_packets function:

int packets_key = PACKETS_KEY, bytes_key = BYTES_KEY;

alban · 2017-07-27T09:20:39Z

cmd/serve.go

-				metrics.GlobalPodMetrics.TotalNumberPods.Add(1)
+				podlog := log.New("pod", e.PodSelfLink)
+
+				cgPath := buildCgroupPath(cgroupRoot, e.PodUID, e.PodQOSClass)


is it possible to get this from the Kubernetes API rather than assuming the path is built in a specific way?

/cc @asymmetric @nhlfr

I think you could follow this and do something like client.Get().Namespace("foo").Resource("bar").URL().

@asymmetric But that only gives me the resource URL I need the path to the pods cgroup here

@alban I looked though http://godoc.org/k8s.io/api/core/v1#Pod to find a way to get the cgroup for pods but building it ourselfs seems to be the only way. Maybe @nhlfr knows...

If k8s does not provide it, you might have to ask Docker the leader pid of the container and then check in /proc/$pid/cgroup...

You could file an issue and add a // FIXME: ... if you want to do that later.

alban · 2017-07-27T09:22:21Z

cmd/serve.go

 			case kube.DeletePodEvent:
-				metrics.GlobalPodMetrics.TotalNumberPods.Sub(1)
+				metrics.TotalNum().Sub(1)
+				log.Debug("pod gone", "pod", e.PodSelfLink, "cgroup", buildCgroupPath(cgroupRoot, e.PodUID, e.PodQOSClass))


cleanup the bpf module? Or at least. add a // FIXME: ... to remember

That was actually why I filed #7 because I didn't know if you had to do any cleanup work.

Will fix this now...

This introduces a bpf controller that runs the lookup loop for every cgroup we attach a program to, reads the map and executes handlers that write the tracked data to Prometheus metrics. The top subcommand uses the same code but implements different handlers. Closes #4, closes #5, closes #6, closes #9

robertgzr · 2017-08-15T08:56:39Z

Any more feedback here?

dongsupark

Other than that, LGTM.
"top" command works fine.
I didn't try out the prometheus part though.

dongsupark · 2017-08-21T10:06:08Z

bpf/bpf.go

+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package bpf

 import (
 	"bytes"
 	"fmt"
 	"log"


"log" doesn't seem to be used anywhere, so we get build error.

dongsupark · 2017-08-21T10:08:08Z

bpf/controller.go

+func (c *Controller) Stop() {
+	c.quit <- struct{}{}
+	if err := c.module.Close(); err != nil {
+		Log.Error("error closing bpf.Module", "err", err)


Shouldn't it be log instead of Log?
And log needs to be imported, otherwise we get build errors.

Ah that's left over from the logging I introduced but split out of this PR. thx will use the log pkg here

robertgzr added this to the 0.1.0 milestone Jul 26, 2017

robertgzr requested review from alban, asymmetric and iaguis July 26, 2017 16:28

robertgzr force-pushed the robertgzr/bpf-per-pod branch 2 times, most recently from 98d10c0 to 3be4279 Compare July 26, 2017 16:50

alban suggested changes Jul 27, 2017

View reviewed changes

robertgzr mentioned this pull request Jul 31, 2017

Get pod cgroup path #16

Open

robertgzr force-pushed the robertgzr/bpf-per-pod branch from 3be4279 to e334fb8 Compare August 2, 2017 13:01

robertgzr force-pushed the robertgzr/bpf-per-pod branch from e334fb8 to e193f74 Compare August 2, 2017 13:04

robertgzr requested review from nhlfr and dongsupark August 2, 2017 13:07

dongsupark suggested changes Aug 21, 2017

View reviewed changes

asymmetric removed request for asymmetric and nhlfr February 12, 2018 14:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Install the bpf program for all pods and collect/expose the data #13

Install the bpf program for all pods and collect/expose the data #13

robertgzr commented Jul 26, 2017 •

edited

Loading

alban Jul 27, 2017

asymmetric Jul 27, 2017 •

edited

Loading

alban Jul 27, 2017

alban Jul 27, 2017

alban Jul 27, 2017

alban Jul 27, 2017

alban Jul 27, 2017

alban Jul 27, 2017

alban Jul 27, 2017

asymmetric Jul 27, 2017

robertgzr Jul 31, 2017

alban Jul 31, 2017

alban Jul 27, 2017

robertgzr Jul 31, 2017

robertgzr commented Aug 15, 2017

dongsupark left a comment

dongsupark Aug 21, 2017

dongsupark Aug 21, 2017

robertgzr Aug 21, 2017

Install the bpf program for all pods and collect/expose the data #13

Are you sure you want to change the base?

Install the bpf program for all pods and collect/expose the data #13

Conversation

robertgzr commented Jul 26, 2017 • edited Loading

Choose a reason for hiding this comment

asymmetric Jul 27, 2017 • edited Loading

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

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

robertgzr commented Aug 15, 2017

dongsupark left a 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

robertgzr commented Jul 26, 2017 •

edited

Loading

asymmetric Jul 27, 2017 •

edited

Loading