add support for cgroup CPUQuota and CPUPeriod prometheus limit #969

f0 · 2015-11-22T19:58:41Z

This adds support for the CPUQuota cgroup limit.

This actually does not work , i miss something, any hints?

cadvisorJenkinsBot · 2015-11-22T19:58:42Z

Can one of the admins verify this patch?

jimmidyson · 2015-11-22T20:18:05Z

Thanks for this. What doesn't work? No metric or no values reported in the metric?

f0 · 2015-11-22T20:20:53Z

cadvisor builds sucessfully, but in the prometheus output the quota is not exported , but is present in the specific cgroup

jimmidyson · 2015-11-22T20:23:54Z

Can you share the whole /metrics endpoint output please? Perhaps in a gist.

f0 · 2015-11-22T20:26:18Z

sure https://gist.github.com/f0/1938007f1d9054f57c56

jimmidyson · 2015-11-22T20:39:50Z

And you're sure you built & replaced cadvisor binary with the newly built binary? Everything looks fine to me but the output.

f0 · 2015-11-22T20:43:29Z

hm, yes i am sure but i double check ...

f0 · 2015-11-22T20:49:10Z

hm yes its the correct binary, but its not shown

jimmidyson · 2015-11-22T20:51:18Z

OK I'll build with your PR locally tomorrow & see if I can see what's going on.

f0 · 2015-11-22T20:54:01Z

@jimmidyson thx

jimmidyson · 2015-11-23T09:15:55Z

This works perfectly fine for me locally... How are you building? Just run make & you should get a binary in the root of the project dir to use.

jimmidyson · 2015-11-23T09:17:17Z

Well when I say it works perfectly I mean I get output in the /metrics endpoint.

But in logs (running with --logtostderr):

E1123 09:15:09.954624 11428 handler.go:149] raw driver: Failed to parse int "-1" from file "/sys/fs/cgroup/cpu,cpuacct/cpu.cfs_quota_us": strconv.ParseUint: parsing "-1": invalid syntax

Need a test for this as well if possible.

f0 · 2015-11-23T09:44:34Z

@jimmidyson yep this happens when no Quota is set, so it need to be coverd.... i run build/build.sh

f0 · 2015-11-23T10:31:45Z

@jimmidyson hm is there a specific go version needed to build ? atm i have 1.4.3

jimmidyson · 2015-11-23T10:36:57Z

1.4 should be fine. Do you get errors when building with 1.4?

f0 · 2015-11-23T11:24:14Z

@jimmidyson yes

fk@linux-3zvu:~/work/src/f0/cadvisor> make

formatting code
building binaries
cadvisor

github.com/google/cadvisor

usage: 6l [options] main.6
-1 use alternate profiling code
-8 assume 64-bit addresses
-B info
define ELF NT_GNU_BUILD_ID note
-C check Go calls to C code
-D addr
data address
-E sym
entry symbol
-I interp
set ELF interp
-L dir
add dir to library path
-H head
header type
-K add stack underflow checks
-O print pc-line tables
-Q debug byte-register code gen
-R rnd
address rounding
-S check type signatures
-T addr
text address
-V print version and exit
-W disassemble input
-X name value
define string data
-Z clear stack frame on entry
-a disassemble output
-c dump call graph
-d disable dynamic executable
-extld ld
linker to run in external mode
-extldflags ldflags
flags for external linker
-f ignore version mismatch
-g disable go package data checks
-installsuffix suffix
pkg directory suffix
-k sym
set field tracking symbol
-linkmode mode
set link mode (internal, external, auto)
-n dump symbol table
-o outfile
set output file
-r dir1:dir2:...
set ELF dynamic linker search path
-race
enable race detector
-s disable symbol table
-shared
generate shared object (implies -linkmode external)
-tmpdir dir
leave temporary files in this directory
-u reject unsafe packages
-v print link trace
-w disable DWARF generation
godep: go exit status 2
Makefile:32: recipe for target 'build' failed
make: *** [build] Error 1

jimmidyson · 2015-11-23T11:37:27Z

Just pushed a fix so you can build on 1.4 as well (ldflags was 1.5 stylee). Rebase against master & you should be able to build on 1.4 with make.

f0 · 2015-11-23T13:35:26Z

ok now i can build, and i got these errors, but why , the type has a field Quota

fk@linux-3zvu:~/work/src/f0/cadvisor> make
 formatting code
 building binaries
   cadvisor
 running tests
f0/cadvisor/container/raw
container/raw/handler.go:203: spec.Cpu.Quota undefined (type v1.CpuSpec has no field or method Quota)
f0/cadvisor/metrics
metrics/prometheus.go:523: container.Spec.Cpu.Quota undefined (type v1.CpuSpec has no field or method Quota)

f0 · 2015-11-23T19:32:39Z

@jimmidyson hm any Idea why spec.Cpu.Quota is not present, its in the files...

cadvisorJenkinsBot · 2015-11-23T20:11:24Z

Can one of the admins verify this patch?

f0 · 2015-11-23T20:12:02Z

updated with check for -1 (quota not set), but still not work

f0 · 2015-11-24T16:05:13Z

@jimmidyson ok have fixed some things, now i get these errors while building, but why, the type has this field? And it does not work in my environment....

 running tests
 github.com/f0/cadvisor/container/raw
container/raw/handler.go:206: spec.Cpu.Quota undefined (type v1.CpuSpec has no field or method Quota)
 github.com/f0/cadvisor/metrics
metrics/prometheus.go:523: container.Spec.Cpu.Quota undefined (type v1.CpuSpec has no field or method Quota)

jimmidyson · 2015-11-24T16:54:01Z

Discussed on IRC: cadvisor needs to be cloned to $GOPATH/src/github.com/google/cadvisor & a remote set to your fork, rather than cloning to $GOPATH/src/github.com/f0/cadvisor.

f0 · 2015-11-24T17:11:09Z

@jimmidyson after fixing gopath and some other orrors it finaly works THX

f0 · 2015-11-24T17:42:21Z

also adding support for the cpu period setting (with this we finally can calculate the percentual cpu usage )

f0 · 2015-11-25T16:09:21Z

@jimmidyson when will this be merged?

jimmidyson · 2015-12-08T20:35:02Z

metrics/testdata/prometheus_metrics

@@ -13,6 +13,9 @@ container_cpu_usage_seconds_total{cpu="cpu03",id="testcontainer",image="test",na
 # HELP container_cpu_user_seconds_total Cumulative user cpu time consumed in seconds.
 # TYPE container_cpu_user_seconds_total counter
 container_cpu_user_seconds_total{id="testcontainer",image="test",name="testcontaineralias"} 6e-09
+# container_spec_cpu_quota HELP CPU quota of the container


Check the format of the lines - HELP should be at the start of the comment. You can run tests locally btw with make test.

f0 · 2015-12-09T18:41:27Z

@jimmidyson

hm i dont get it now i have this error....
=== RUN TestPrometheusCollector
--- FAIL: TestPrometheusCollector (0.02s)
prometheus_test.go:189: want # HELP container_spec_cpu_quota CPU quota of the container, got # HELP container_fs_io_current Number of I/Os currently in progress

jimmidyson · 2015-12-09T18:57:28Z

The order of the output is important. Think metrics are ordered alphabetically so you'll need to put the new metric in the right place in the test file.

f0 · 2015-12-09T19:13:36Z

@jimmidyson ah ok so then the test should be after
container_start_time_seconds and before container_tasks_state, but this also gives me the above error(with another error text)

jimmidyson · 2015-12-09T19:21:39Z

Should be before container_start_time_seconds.

f0 · 2015-12-10T06:10:30Z

@jimmidyson also does not work

k8s-bot · 2015-12-11T21:46:14Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

googlebot · 2015-12-12T19:28:15Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

f0 · 2015-12-12T19:28:42Z

hm sorry false positive..... whyever the tests does not work
@jimmidyson i am out of ideas

googlebot · 2015-12-12T19:28:43Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

k8s-bot · 2015-12-14T20:53:45Z

EXPERIMENTAL - CAN IGNORE

Build/test failed for commit e4f9207.

jimmidyson · 2016-01-22T10:08:27Z

metrics/prometheus_test.go

@@ -55,7 +55,11 @@ func (p testSubcontainersInfoProvider) SubcontainersInfo(string, *info.Container
 				Aliases: []string{"testcontaineralias"},
 			},
 			Spec: info.ContainerSpec{
-				Image:        "test",
+				Image: "test",


You need to add in HasCpu: true, in here so that the CPU specs are output as metrics. You'll probably find then that there are a couple of other metrics which haven't been tested for that you'll need to insert into metrics/testdata/prometheus_metrics to get this to pass.

k8s-bot · 2016-01-28T18:43:47Z

Jenkins GCE e2e

Build/test failed for commit e4f9207.

Build Log

vishh · 2016-01-28T22:48:35Z

Ping @f0!

f0 · 2016-01-29T06:56:25Z

@vishh @jimmidyson sorry have not so much spare time atm, try to get this in the next days

jimmidyson · 2016-01-29T09:18:04Z

If you don't manage to get to it by Monday I can pull it into a separate PR & fix up the tests.

f0 · 2016-01-29T16:04:51Z

@jimmidyson feel free, i have no chance to get this until monday, i am 90% offline at the weekend , sorry.

k8s-bot · 2016-02-01T23:17:29Z

Jenkins GCE e2e

Build/test failed for commit e4f9207.

Build Log

googlebot · 2016-02-02T19:26:06Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

f0 · 2016-02-02T19:28:53Z

@jimmidyson @vishh finally done (and now i understand the test thing ....)

k8s-bot · 2016-02-02T19:30:02Z

Jenkins GCE e2e

Build/test passed for commit 3675a96.

Build Log

k8s-bot · 2016-02-02T19:32:14Z

Jenkins GCE e2e

Build/test passed for commit 78592db.

Build Log

k8s-bot · 2016-02-02T19:35:55Z

Jenkins GCE e2e

Build/test passed for commit 669bc4a.

Build Log

f0 · 2016-02-04T16:46:03Z

@jimmidyson @vishh any outstanding issues with this PR?

jimmidyson · 2016-02-04T16:51:32Z

no LGTM

add support for cgroup CPUQuota and CPUPeriod prometheus limit

f0 mentioned this pull request Nov 22, 2015

export CPUQuota Cgroup limit as prometheus metric #968

Closed

f0 closed this Nov 23, 2015

f0 reopened this Nov 23, 2015

f0 changed the title ~~add support for cgroup CPUQuota prometheus limit~~ add support for cgroup CPUQuota and CPUPeriod prometheus limit Nov 24, 2015

jimmidyson reviewed Dec 8, 2015
View reviewed changes

vishh assigned jimmidyson Jan 21, 2016

jimmidyson reviewed Jan 22, 2016
View reviewed changes

vishh mentioned this pull request Feb 1, 2016

Support for "--cpu-period" and "--cpu-quota" #1085

Open

add cgroup quota and period support / add missing tests

669bc4a

jimmidyson added a commit that referenced this pull request Feb 4, 2016

Merge pull request #969 from f0/master

a894672

add support for cgroup CPUQuota and CPUPeriod prometheus limit

jimmidyson merged commit a894672 into google:master Feb 4, 2016

add support for cgroup CPUQuota and CPUPeriod prometheus limit #969

add support for cgroup CPUQuota and CPUPeriod prometheus limit #969

Conversation

f0 commented Nov 22, 2015

cadvisorJenkinsBot commented Nov 22, 2015

jimmidyson commented Nov 22, 2015

f0 commented Nov 22, 2015

jimmidyson commented Nov 22, 2015

f0 commented Nov 22, 2015

jimmidyson commented Nov 22, 2015

f0 commented Nov 22, 2015

f0 commented Nov 22, 2015

jimmidyson commented Nov 22, 2015

f0 commented Nov 22, 2015

jimmidyson commented Nov 23, 2015

jimmidyson commented Nov 23, 2015

f0 commented Nov 23, 2015

f0 commented Nov 23, 2015

jimmidyson commented Nov 23, 2015

f0 commented Nov 23, 2015

github.com/google/cadvisor

jimmidyson commented Nov 23, 2015

f0 commented Nov 23, 2015

f0 commented Nov 23, 2015

cadvisorJenkinsBot commented Nov 23, 2015

f0 commented Nov 23, 2015

f0 commented Nov 24, 2015

jimmidyson commented Nov 24, 2015

f0 commented Nov 24, 2015

f0 commented Nov 24, 2015

f0 commented Nov 25, 2015

jimmidyson Dec 8, 2015

Choose a reason for hiding this comment

f0 commented Dec 9, 2015

jimmidyson commented Dec 9, 2015

f0 commented Dec 9, 2015

jimmidyson commented Dec 9, 2015

f0 commented Dec 10, 2015

k8s-bot commented Dec 11, 2015

googlebot commented Dec 12, 2015

f0 commented Dec 12, 2015

googlebot commented Dec 12, 2015

k8s-bot commented Dec 14, 2015

jimmidyson Jan 22, 2016

Choose a reason for hiding this comment

k8s-bot commented Jan 28, 2016

vishh commented Jan 28, 2016

f0 commented Jan 29, 2016

jimmidyson commented Jan 29, 2016

f0 commented Jan 29, 2016

k8s-bot commented Feb 1, 2016

googlebot commented Feb 2, 2016

f0 commented Feb 2, 2016

k8s-bot commented Feb 2, 2016

k8s-bot commented Feb 2, 2016

k8s-bot commented Feb 2, 2016

f0 commented Feb 4, 2016

jimmidyson commented Feb 4, 2016