Division by zero in v0.6.0

[EraYaN]

Running:
kubectl vpa-recommendation --show-stats --output wide --sort-columns mem-diff,cpu-diff

I get the following error:

NAME   MODE   TARGET   CPU REQUEST   CPU TARGET   % CPU DIFF   MEMORY REQUEST   MEMORY TARGET   % MEMORY DIFF

panic: division by zero

goroutine 1 [running]:
math/big.nat.div({0x0?, 0x1?, 0x0?}, {0x0?, 0x0?, 0xc00012f0d8?}, {0x0?, 0xc000708450?, 0x1000000000001?}, {0x0, ...})
        /opt/hostedtoolcache/go/1.19.2/x64/src/math/big/natdiv.go:507 +0x34b
math/big.(*Int).Quo(0xc0005c7410, 0xc0005c73b0, 0xc0005c73e0)
        /opt/hostedtoolcache/go/1.19.2/x64/src/math/big/int.go:211 +0x78
gopkg.in/inf%2ev0.(*Dec).quoRem(0xc0005c7410, 0xc0005c73b0, 0xc0005c73e0, 0x0, 0x0, 0x0, 0x0)
        /home/runner/go/pkg/mod/gopkg.in/inf.v0@v0.9.1/dec.go:331 +0x654
gopkg.in/inf%2ev0.(*Dec).quo(0xc000422c80?, 0x0?, 0x28?, {0x1b350e0?, 0x269f140?}, {0x1b3bbf8, 0xc0000b3610})
        /home/runner/go/pkg/mod/gopkg.in/inf.v0@v0.9.1/dec.go:267 +0x14b
gopkg.in/inf%2ev0.(*Dec).QuoRound(...)
        /home/runner/go/pkg/mod/gopkg.in/inf.v0@v0.9.1/dec.go:257
github.com/wI2L/kubectl-vpa-recommendation/cli.table.meanQuantities({0x0?, 0x0, 0x176d820?}, 0xc0005c7201?)
        /home/runner/work/kubectl-vpa-recommendation/kubectl-vpa-recommendation/cli/table.go:299 +0xc5
github.com/wI2L/kubectl-vpa-recommendation/cli.table.printStats({0x0?, 0x0, 0x0}, {0x1b33fc0?, 0xc000012018?})
        /home/runner/work/kubectl-vpa-recommendation/kubectl-vpa-recommendation/cli/table.go:243 +0x604
github.com/wI2L/kubectl-vpa-recommendation/cli.table.Print({0x0, 0x0, 0x0}, {0x1b33fc0, 0xc000012018}, 0xc0001f0360)
        /home/runner/work/kubectl-vpa-recommendation/kubectl-vpa-recommendation/cli/table.go:200 +0x618
github.com/wI2L/kubectl-vpa-recommendation/cli.(*CommandOptions).Execute(0xc000424000)
        /home/runner/work/kubectl-vpa-recommendation/kubectl-vpa-recommendation/cli/command.go:204 +0x66b
github.com/wI2L/kubectl-vpa-recommendation/cli.(*CommandOptions).Run(0x128aee0?, 0xc0000f9c00?, {0xc0003fc5b0, 0x0, 0x7})
        /home/runner/work/kubectl-vpa-recommendation/kubectl-vpa-recommendation/cli/command.go:103 +0x7b
github.com/spf13/cobra.(*Command).execute(0xc000357400, {0xc00003e090, 0x7, 0x7})
        /home/runner/go/pkg/mod/github.com/spf13/cobra@v1.3.0/command.go:860 +0x663
github.com/spf13/cobra.(*Command).ExecuteC(0xc000357400)
        /home/runner/go/pkg/mod/github.com/spf13/cobra@v1.3.0/command.go:974 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
        /home/runner/go/pkg/mod/github.com/spf13/cobra@v1.3.0/command.go:902
k8s.io/component-base/cli.run(0xc000357400)
        /home/runner/go/pkg/mod/k8s.io/component-base@v0.23.4/cli/run.go:146 +0x317
k8s.io/component-base/cli.Run(0xc0000e7d00?)
        /home/runner/go/pkg/mod/k8s.io/component-base@v0.23.4/cli/run.go:46 +0x1d
main.main()
        /home/runner/work/kubectl-vpa-recommendation/kubectl-vpa-recommendation/cmd/kubectl-vpa-recommendation/kubectl-vpa-recommendation.go:44 +0x145

It seems like
meanQuantities chokes on some of our vpa's recommandations. I have dumped the vpas to yaml but there is a ton of internal info in those, so I'm not sure if I can share those.

Most of them have one container with something like this:

- containerName: webservices
  lowerBound:
    cpu: 15m
    memory: "248036781"
  target:
    cpu: 15m
    memory: "272061154"
  uncappedTarget:
    cpu: 15m
    memory: "272061154"
  upperBound:
    cpu: 15m
    memory: "336054630"

and a couple will have two containers.

EDIT: Alright kubectl vpa-recommendation doesn't actually return any VPAs. All our VPAs target a custom CRD, might that be the issue? Seems like it doesn't realize the number of matched VPAs is actually 0 because they are all targeting some unsupported kind.

[wI2L]

Hello,

meanQuantities is part of the --show-stats calculations of the "global stats" for all VPAs of a namespace. The division by zero and the fact that kubectl vpa-recommendation that returns no VPA clearly might indeed seems to be the source of the problem.

The doc states in the Limitations section:

Unlike the official VPA recommender, which is fully generic and handle any kind of "scalable" resources, the plugin recognize only some well-known controllers such as: CronJob, DaemonSet, Deployment, Job, ReplicaSet, ReplicationController, StatefulSet.

kubectl-vpa-recommendation/cli/command.go

Line 229 in 8f936b6

tc, err := vpa.NewTargetController(co.Client, v.Spec.TargetRef, v.Namespace)

Did you get logs about the VPA target kind being unsupported ?
When the plugin cannot bind a VPA with it's target, it doesn't append it to the "table". And the len of the yable is used for the states computations:

kubectl-vpa-recommendation/cli/table.go

Line 299 in 8f936b6

tmp.QuoRound(dec, inf.NewDec(int64(len(t)), 0), dec.Scale(), inf.RoundDown)

I'll fix the panic.

Regarding your use case, the plugin would need to be able to fetch any kind, tho this might cause other issues (a CRD with the /scale subresource will probably not have the same spec for resource requests/limits, which are used for the computation of the stats):

https://github.com/wI2L/kubectl-vpa-recommendation/blob/master/vpa/target.go#L73

Do you mind sharing some information about the spec of your custom resources?

[EraYaN]

kubectl vpa-recommendation -v5

gives a ton of I0823 13:18:59.714861 13838 command.go:231] couldn't get target for vpa default/<name> lines.

Our CRD does have a scale subresource, so the newer versions of the vpa recommender force us to update our targets from the Deployments to the CRD.

Our CRD has this in its Spec:

Resources *corev1.ResourceRequirements `json:"resources"`

So that is not the same like it is for deployments where resources are per container.

For us it would be acceptable to let us add a command line option like --kind="KindName" --resourcePath=".spec.resources". Or some way of helping it resolve to the pod selectors themselves like what the recommender does.

Maybe you can even depend on the recommender to steal this piece of code to get the selectors from the scale resource: https://github.com/kubernetes/autoscaler/blob/master/vertical-pod-autoscaler/pkg/target/fetcher.go#L119-L146 then you can map to the pods and grab the current resources from that.

We define those as follows:

+kubebuilder:subresource:scale:specpath=.spec.replicas,statuspath=.status.deployment.replicas,selectorpath=.status.scale.selector

[wI2L]

@EraYaN I'll fix the panic first, but to support custom target kinds, this will require a bit of refactor, and I won't have time to work on it immediately. If you are willing to work on it, let me know.

For us it would be acceptable to let us add a command line option like --kind="KindName" --resourcePath=".spec.resources". Or some way of helping it resolve to the pod selectors themselves like what the recommender does.

I don't think a --kind="KindName" flag is necessary, the target reference in the VPA should have enough information to fetch the custom resource, as long as it has a /scale subresource (same way the VPA recommender does it).

Regarding the second flag, --resourcePath=".spec.resources", I agree. By default the plugin should try to look into the common path .spec.template.spec which works for well-known controllers, otherwise this should be specified manually for custom paths.

[EraYaN]

Alright I gave it a go in #4