-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Bug] Inconsistency between customresource
interface and customresource
causes panic
#2202
Comments
/assign @rexagod |
Thanks for reporting this issue, found you have already used v2.8.2 as a mitigation. |
QQ: how did you notice this issue, is it possible to move some tests into this repo? If so, it won't break your components in the future. |
Unfortunately we did not implement a test able to catch this issue in particular. Perhaps we could have a test checking that a simple extended store can be generated. Here is an example of one: https://github.com/DataDog/datadog-agent/blob/0ee3251831d2a9da15f7c47e843ebaacc8b85aab/pkg/collector/corechecks/cluster/ksm/customresources/crd.go#L50 |
Any update on this issue ? @rexagod @CatherineF-dev |
Could you try fixing this issue? cc @AliDatadog I haven't looked into details, but I can help review. @rexagod might know details. |
@AliDatadog Are you using KSM as a library? We do not provide any guarantees on the exported APIs as of now. cc @dgrisonnet |
Issue Description:
After the alteration of the custom resources handling behavior in #1851, there has been a breaking change.
GVRFromType
is invariably invoked:kube-state-metrics/internal/store/builder.go
Line 200 in 76f42c1
ExpectedType()
persistently returnsunstructured.Unstructured{}
. This triggers panic if a different argument type is provided.The change leads to a breaking compatibility with the previous interface that we use internally to enhance KSM with other metrics.
Moreover, the
availableStore
index now usesgvr.String()
overResourceName
as perkube-state-metrics/internal/store/builder.go
Line 203 in 76f42c1
gvrString
is structured asResource=verticalpodautoscalers,apiregistration.k8s.io/v1
Expected Outcome:
I anticipated
ExpectedType()
to have the capacity to return any resource type andf.Name()
to serve as the resource index inavailableStore
.Another solution would be to introduce a new function
StoreIndex()
orStoreName()
in this interface.Reproducibility Steps:
To reproduce this issue, simply add VPA as a custom resource and observe that the code triggers panic.
Additional Information:
None applicable.
Environment Details:
I'm using the kind cluster 1.27.1.
kubectl version
): 1.27.1The text was updated successfully, but these errors were encountered: