Skip to content
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

Empty external metrics list will fail the client-go CachedDiscoveryClient caching and make helm upgrade resending api call #729

Closed
jacobycwang opened this issue May 26, 2024 · 5 comments

Comments

@jacobycwang
Copy link

The custom-metrics-stackdriver-adapter returns an empty list in ListAllExternalMetrics (e.g., kubectl get --raw '/apis/external.metrics.k8s.io/v1beta1'). Although this allows the HPA to function, it causes side effects in other Kubernetes ecosystem tools, such as Helm.

// ListAllExternalMetrics returns a list of available external metrics.
// Not implemented (currently returns empty list).
func (p *StackdriverProvider) ListAllExternalMetrics() []provider.ExternalMetricInfo {
return []provider.ExternalMetricInfo{}
}

We encountered an issue where helm upgrade sends over 400 API requests. This is because Helm relies on the client-go CachedDiscoveryClient for validation. CachedDiscoveryClient caches responses and saves them to a file if the response is not empty. When the custom-metrics-stackdriver-adapter returns an empty list for external.metrics.k8s.io/v1beta1, it is never cached, causing the CachedDiscoveryClient to continuously refresh all API resources (not just external.metrics.k8s.io/v1beta1). This results in a high number of API requests being sent.
https://github.com/kubernetes/client-go/blob/c7396197f39ed43fb0369c054360fcb989d65c88/discovery/cached/disk/cached_discovery.go#L82C35-L82C65
https://github.com/kubernetes/client-go/blob/c7396197f39ed43fb0369c054360fcb989d65c88/discovery/cached/memory/memcache.go#L104

@CatherineF-dev
Copy link
Contributor

CatherineF-dev commented Jun 27, 2024

From grafana/tanka#1076, it might be related to #690 which bumped client-go version to fix vulnerabilities.

@CatherineF-dev
Copy link
Contributor

CatherineF-dev commented Jun 27, 2024

Could you try this image: gcr.io/gke-release/custom-metrics-stackdriver-adapter:v0.14.2-gke.0 ?

@raywainman
Copy link

Helm recently upgraded from client-go v0.29 to v0.30 in helm/helm#13021.

@CatherineF-dev
Copy link
Contributor

Adding a fake metric here #743

@CatherineF-dev
Copy link
Contributor

CatherineF-dev commented Jul 19, 2024

Hi, we will release a new image version soon.
Main branch is still using the old image version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants