filesystem usage stats are wrong in v2 api

[derekwaynecarr]

Go reuses memory, so the result was that things that took the pointer value of stat would get a random result from a different filesystem for available, capacity, etc. It caused the output of /v2.1/machinestats to just be really wrong and confusing ;-)

/cc @ronnielai @vishh @timstclair @ncdc @pmorie

[vishh]

Good catch. LGTM

[derekwaynecarr]

Thanks... it took most of my afternoon to track down ;-) I need 🍻

[k8s-bot]

Jenkins GCE e2e

Build/test passed for commit 8551b0e.

• Build Log

[vishh]

We need to improve testing around the critical APIs..

[vishh]

Lol. Yeah. Time for 🍻 🍻

[jimmidyson]

@derekwaynecarr Sorry for being dense but could you explain the problem & how this fixes it? Got so much to learn!

[timstclair]

@jimmidyson The problem is that Go reuses the memory for the object in a range clause, and the for loop is taking the address of some fields on that struct:

...
            Usage:      &stat.Usage,
            Available:  &stat.Available,
            InodesFree: &stat.InodesFree,
...

This means that every machine stats's Usage, Available, InodesFree will all point to the same memory address, which will contain the last item that was iterated over. The solution forces a new struct to be allocated for each iteration.

[jimmidyson]

Aha thanks @timstclair!

[ichekrygin]

@derekwaynecarr: do you know by any chance if this fix made its way into v1.3.5?

[timstclair]

Nope. Kubernetes v1.3.5 shipped with cAdvisor v0.23.7. This change was recently cherrypicked into the v0.23 branch, but hasn't been cherrypicked into Kubernetes yet.