Should we rethink the response of listing requests? #150
Comments
k8s-ci-robot
added
the
needs-triage
JorTurFer
changed the title
|
/assign |
k8s-ci-robot
added
triage/accepted
|
Hey @dgrisonnet |
|
Hey @JorTurFer, sorry for the late reply. I need to spend some time researching this particular topic as I am not familiar with all the details. I'll will try to look into it in the upcoming weeks. |
|
No worries, I just asked to keep alive the topic 😄 |
|
@JorTurFer did you conclude what is the best workaround or fix for this problem? With kubectl 1.26, I started getting these warnings for every kubectl command: Downgrading to kubectl 1.25.2 eliminated these warnings for me. I am running |
|
Metrics-server doesn't expose external metrics so that is a pretty odd error to see. What request did you make with kubectl? |
|
@dgrisonnet I did not do anything fancy, I just did: Pretty much any k8s command gives the same warning to stdout: I'm encountering the same issue reported here: kubernetes-sigs/metrics-server#157 I'm not sure why that issue was closed, or what the resolution to that issue should be. Downgrading kubectl to 1.25.2 eliminated the warnings for me. |
|
It's not metrics server that's complaining Keda does when there's no scaled object or job defined. |
|
This comment mentioned a band-aid solution of enabling I didn't try that because I didn't know if that would break anything else. It would be nice if I could better understand the root of the problem to know in which direction to go to fix this. |
|
What I did was make a dummy deployment with a |
|
Also probably check your |
|
@sansmoraxz Wow that's a weird workaround. Do you have a pointer in GitHub to an example of what you did? |
This doesn't report an issue with the same API. You might still have an APIService resource around trying to serve the Either way since this seems to be an error coming from KEDA I'd recommend asking for help in one of their channel because this is unrelated to the improvement mentioned in this issue. |
|
@sansmoraxz as you suggested: So as @dgrisonnet mentioned, this looks like a problem with keda. I will need to investigate further, |
|
@dgrisonnet , this is related with not having any ScaledObject (so, no metric is exposed) and the problem we have faced with the changes in kube-client related with the discovery client. |
|
Btw, did you have time to think about this? |
Yeah that was also my guess, but since this issue in custom-metrics-apiserver didn't seem to be the right place to handle this support case, I was trying to re route them to the correct channels.
Not yet sadly :/ I don't know what's the potential impact of that change could have on the apiserver/autoscaler. But, my main concern right now is that I was thinking about leveraging this exposition behavior to use discovery in order to implement #70 without having to introduce a complex configuration.
I am, let's try to schedule something offline, I would be happy to talk about that in-person :) |
|
We have been talking about this offline, and we agree with next steps: |
|
It seems to have been fixed in client-go |
Yeah, the problem was solved in Kubernetes repo, but we have discussed not treating metrics as resources. The PoC on going isn't only for fixing that problem, it's also to fix other problems related with huge metrics in the list |
Hi to all,
As you know, we have recently faced with the issue produced by the changes in kube-client. Basically, the issue was related with returning an empty list because empty was considered as error.
This has pushed me to think about the current situation and how we expose the custom-metrics information. I'd like to reopen the discussion about what we are returning during listing operations and if we really need to return the current metrics.
Currently, we are exposing custom/external metrics listing all of them or by name. I know that this is the global approach in Kubernetes for aggregated APIs, but I'm not sure if it makes sense (most probably due to my knowledge gap here).
Other aggregated APIs like
metrics.k8s.iosupports the same actions as us, but in that case, it makes sense because all the metrics available on every the namespace/node. For example, if I queryI will get a list with all the available values in the namespace and it makes sense because there eventually are pod metrics on every namespace,
podas metric is generic and it's an abstraction that we can query on every namespace.My doubts appear with custom/external metrics, for example in KEDA, we list all the metrics and expose them during listing requests. The problem is that each query is totally attached to the specific metric/namespace, I can't query
It's because this why I'm not sure about current approach. We are returning a list of metrics that can't be used for discovering anything, as we return only the metric name generated using the specific info but not the namespace. Basically, we are returning a useless list, because nothing can be done with it apart from knowing the whole list of metrics. This listing action can produce a timeout too, if the metric list is too long, printing an error without any useful information for end users. We have an endpoint which doesn't offer too much information, which is queried by the cluster/users really often, and which can produce timeouts easily in some situations. Another important point is that HPA controller doesn't use this listing endpoint at all, it uses directly the get endpoint providing namespace and metric name from the HPA.
Based on the reasons above, I think that following the same approach as
metrics.k8s.iocould be worth. My proposal is:kubectl get --raw "/apis/custom-external.metrics.k8s.io/v1beta1") return a single metric likecustom-metricandexternal-metric(depending on the API)Following this approach, we could be safe from changes in discovery processes, and also avoid timeouts listing all the available metrics. I'm not totally sure about the changes required for doing this and neither if they have to be done in this repo or directly in the implementations but I wanted to share my thoughts about this.
BTW, I'm willing to contribute with the changes if we agree with change something
The text was updated successfully, but these errors were encountered: