Add a diagram shows how metrics-server handles kubectl top pod request #1654
Conversation
…is/metrics.k8s.io/v1beta1/:
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: CatherineF-dev The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
needs-triage
|
This issue is currently awaiting triage. If metrics-server contributors determine this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
the
size/S
CatherineF-dev
changed the title
|
This is very nice :) |
|
https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_metrics-server/1654/pull-metrics-server-verify/1920253007463714816 is been stuck something to do after golang ci update probably I will take a look but for now closing and reopening the PR will help |
|
/close |
|
@slashpai: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/reopen |
|
@slashpai: Reopened this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
|
/retest |
| participant PL as cache.GenericLister (Pods) | ||
|
|
||
| Client->>APIServer: GET /apis/metrics.k8s.io/v1beta1/namespaces/{ns}/pods[/{podName}] | ||
| APIServer->>MS: List(ctx, opts) or Get(ctx, name, opts) |
There was a problem hiding this comment.
Could we simplify this diagram to just show List and skip mention of Get. At the top you mention kubectl top pods which will do List as pod name is not specified.
| sequenceDiagram | ||
| participant Client | ||
| participant APIServer | ||
| participant MS as api.podMetrics |
There was a problem hiding this comment.
Would be good to be able to recognize participants that are part of Metrics Server and which are not. I also don't know if mentioning internal implementation details like structures storage.Storage brings any value or would just confuse people that don't know them.
Could maybe just simplify the diagram to have metrics server as one participant and maybe create a separate diagram for internal component architecture?
There was a problem hiding this comment.
Maybe we could squeeze in how metrics are collected from Kubelets
|
@CatherineF-dev: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/assign @serathius |
See diagram at https://github.com/kubernetes-sigs/metrics-server/blob/d8993efbc4c82ba49335c4a513575cdc0f8a8fea/README.md
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #