fix: avoid panic because of VPA objects without target ref

[nabokihms]

Signed-off-by: m.nabokikh maksim.nabokikh@flant.com

What this PR does / why we need it:

VPA is a custom resource. CRDs for this resource can be found there.

The problem is that the targetRef field was not required by the spec, so there is a possibility to deploy VPA to the cluster without it, which makes kube-state-metrics panic.

I0921 13:39:41.487672       1 main.go:191] Communication with server successful
I0921 13:39:41.487934       1 main.go:225] Starting metrics server: 127.0.0.1:8081
I0921 13:39:41.488119       1 main.go:200] Starting kube-state-metrics self metrics server: 127.0.0.1:8082
I0921 13:39:41.488125       1 metrics_handler.go:96] Autosharding disabled
I0921 13:39:41.489736       1 builder.go:144] Active collectors: certificatesigningrequests,configmaps,cronjobs,daemonsets,deployments,endpoints,horizontalpodautoscalers,ingresses,jobs,limitranges,namespaces,nodes,persistentvolumeclaims,persistentvolumes,poddisruptionbudgets,pods,replicasets,replicationcontrollers,resourcequotas,secrets,services,statefulsets,storageclasses,verticalpodautoscalers
E0921 13:39:41.547405       1 runtime.go:73] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference)
goroutine 91 [running]:
k8s.io/apimachinery/pkg/util/runtime.logPanic(0x12c5a60, 0x1d35e80)
	/go/pkg/mod/k8s.io/apimachinery@v0.0.0-20190612205821-1799e75a0719/pkg/util/runtime/runtime.go:69 +0x7b
k8s.io/apimachinery/pkg/util/runtime.HandleCrash(0x0, 0x0, 0x0)
	/go/pkg/mod/k8s.io/apimachinery@v0.0.0-20190612205821-1799e75a0719/pkg/util/runtime/runtime.go:51 +0x89
panic(0x12c5a60, 0x1d35e80)
	/usr/local/go/src/runtime/panic.go:969 +0x1b9
k8s.io/kube-state-metrics/internal/store.wrapVPAFunc.func1(0x1403040, 0xc000164738, 0x8)
	/src/kube-state-metrics/internal/store/verticalpodautoscaler.go:250 +0x1f0
k8s.io/kube-state-metrics/pkg/metric.(*FamilyGenerator).Generate(...)
	/src/kube-state-metrics/pkg/metric/generator.go:39

Without target ref, VPA objects receive no recommendation update. It is pointless trying to expose metrics for them.

How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality)

It does not change cardinality.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

Did not manage to find one.

[k8s-ci-robot]

Welcome @nabokihms!

It looks like this is your first PR to kubernetes/kube-state-metrics 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kube-state-metrics has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[fpetkovski]

Awesome, thanks for catching this!

I just wonder if we want to still expose a metric with an empty targetRef apiversion, kind and name in these cases. The rule of thumb is to always expose raw data and use PromQL to filter it later on. A metric with empty values could be beneficial for alerting on VPAs without a target ref. Also, not emitting a metric could break clients that have queries which count the number of VPAs per cluster, namespace, team etc..

[nabokihms]

Make sense. My first thoughts were about exporting resources for targets because it is our main use-case. I will do as you've said.

[fpetkovski]

Great stuff.

/lgtm

[mrueg]

Suggested change

	k8sautoscaling "k8s.io/api/autoscaling/v1"
	autoscalingv1 "k8s.io/api/autoscaling/v1"

Can we refactor this to use a similar approach like metav1 below?

[nabokihms]

I copied it from the ksm code somewhere, but yes, no problem. I will make the change.

[mrueg]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fpetkovski, mrueg, nabokihms

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mrueg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ahmed-mez]

@mrueg @fpetkovski is this fix going to be backported to 2.2 and 2.1? Thanks!