Fix readme docs and typo code notes. #1237

yanggangtony · 2023-04-04T14:38:02Z

What this PR does / why we need it:
This pr is cleanup .
Update some readme docs , and other code typo notes.

k8s-ci-robot · 2023-04-04T14:38:12Z

Welcome @yanggangtony!

It looks like this is your first PR to kubernetes-sigs/metrics-server 🎉. 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-sigs/metrics-server 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. 😃

k8s-ci-robot · 2023-04-04T14:38:13Z

Hi @yanggangtony. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

dgrisonnet · 2023-04-04T15:56:42Z

FAQ.md

@@ -103,7 +103,7 @@ Metrics Server was tested to run within clusters up to 5000 nodes with an averag

 #### How often metrics are scraped?

-Default 60 seconds, can be changed using `metric-resolution` flag. We are not recommending setting values below 15s, as this is the resolution of metrics calculated by Kubelet.
+Default 10 seconds, can be changed using `metric-resolution` flag. We are not recommending setting values below 10s, as this is the resolution of metrics calculated by Kubelet.


Default is 60 seconds in code: https://github.com/kubernetes-sigs/metrics-server/blob/master/cmd/metrics-server/app/options/options.go#L105

Also, the fact that we are not recommending below 15s is pretty much set in stone since this is the value we are running scalability tests with.

dgrisonnet · 2023-04-04T15:57:10Z

manifests/base/deployment.yaml

@@ -25,7 +25,7 @@ spec:
          - --secure-port=10250
          - --kubelet-preferred-address-types=InternalIP,ExternalIP,Hostname
          - --kubelet-use-node-status-port
-          - --metric-resolution=15s
+          - --metric-resolution=10s


what is the reasoning behind that change?

yanggangtony · 2023-04-04T23:18:39Z

@dgrisonnet
Oh, sorry, total mistaken for the 60s default....
I will rebased that..

I see the code options.go , is judge the 10s..

Why we not suggest the user the minimum=10 , but now is 15?

Default 60 seconds, can be changed using `metric-resolution` flag. We are not recommending setting values below 15s, as this is the resolution of metrics calculated by Kubelet.

-->

Default 60 seconds, can be changed using `metric-resolution` flag. We are not recommending setting values below 10s, as this is the resolution of metrics calculated by Kubelet.

JoaoBraveCoding

lgtm

Disclaimer: I'm not a maintainer just helping with reviews

dgrisonnet · 2023-04-05T18:19:10Z

@yanggangtony I thought this was what we were running in scale tests but that is not the case. The reasoning behind 10s is explained in #773 (comment).

Maybe we could make that clearer in the doc, but generally we are testing with 15s so that's the value we have confidence with.

logicalhan · 2023-04-06T16:50:36Z

/triage accepted

1 Signed-off-by: yanggang <gang.yang@daocloud.io>

yanggangtony · 2023-04-16T22:41:37Z

Thanks for review.
Supplement a commit , for the typo code notes.
And changed the title and the pr description.

yanggangtony · 2023-04-17T02:26:47Z

/retest

yanggangtony · 2023-04-17T06:00:28Z

/retest

yanggangtony · 2023-04-17T13:08:57Z

/retest

yangjunmyfm192085

/ok-to-test

k8s-ci-robot · 2023-04-17T14:29:30Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JoaoBraveCoding, yanggangtony, yangjunmyfm192085
Once this PR has been reviewed and has the lgtm label, please assign serathius for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

k8s-triage-robot · 2023-07-16T15:39:32Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

yanggangtony · 2023-07-19T05:38:52Z

/close

k8s-ci-robot · 2023-07-19T05:38:56Z

@yanggangtony: Closed this PR.

In response to this:

/close

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/test-infra repository.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 4, 2023

k8s-ci-robot requested review from s-urbaniak and stevehipwell April 4, 2023 14:38

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 4, 2023

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Apr 4, 2023

dgrisonnet reviewed Apr 4, 2023

View reviewed changes

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 4, 2023

yanggangtony changed the title ~~Fix docs and default value for --metric-resolution~~ Fix faq.md docs Apr 4, 2023

JoaoBraveCoding approved these changes Apr 5, 2023

View reviewed changes

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 6, 2023

Fix readme docs and typo code notes.'

d65ffc5

1 Signed-off-by: yanggang <gang.yang@daocloud.io>

yanggangtony changed the title ~~Fix faq.md docs~~ Fix readme docs and typo code notes. Apr 16, 2023

yangjunmyfm192085 approved these changes Apr 17, 2023

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 17, 2023

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 16, 2023

k8s-ci-robot closed this Jul 19, 2023

yanggangtony deleted the fix-docs-and-default-value branch July 19, 2023 05:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix readme docs and typo code notes. #1237

Fix readme docs and typo code notes. #1237

yanggangtony commented Apr 4, 2023 •

edited

Loading

k8s-ci-robot commented Apr 4, 2023

k8s-ci-robot commented Apr 4, 2023

dgrisonnet Apr 4, 2023 •

edited

Loading

dgrisonnet Apr 4, 2023

yanggangtony commented Apr 4, 2023 •

edited

Loading

JoaoBraveCoding left a comment

dgrisonnet commented Apr 5, 2023

logicalhan commented Apr 6, 2023

yanggangtony commented Apr 16, 2023

yanggangtony commented Apr 17, 2023

yanggangtony commented Apr 17, 2023

yanggangtony commented Apr 17, 2023

yangjunmyfm192085 left a comment

k8s-ci-robot commented Apr 17, 2023

k8s-triage-robot commented Jul 16, 2023

yanggangtony commented Jul 19, 2023

k8s-ci-robot commented Jul 19, 2023

Fix readme docs and typo code notes. #1237

Fix readme docs and typo code notes. #1237

Conversation

yanggangtony commented Apr 4, 2023 • edited Loading

k8s-ci-robot commented Apr 4, 2023

k8s-ci-robot commented Apr 4, 2023

dgrisonnet Apr 4, 2023 • edited Loading

Choose a reason for hiding this comment

dgrisonnet Apr 4, 2023

Choose a reason for hiding this comment

yanggangtony commented Apr 4, 2023 • edited Loading

JoaoBraveCoding left a comment

Choose a reason for hiding this comment

dgrisonnet commented Apr 5, 2023

logicalhan commented Apr 6, 2023

yanggangtony commented Apr 16, 2023

yanggangtony commented Apr 17, 2023

yanggangtony commented Apr 17, 2023

yanggangtony commented Apr 17, 2023

yangjunmyfm192085 left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 17, 2023

k8s-triage-robot commented Jul 16, 2023

yanggangtony commented Jul 19, 2023

k8s-ci-robot commented Jul 19, 2023

yanggangtony commented Apr 4, 2023 •

edited

Loading

dgrisonnet Apr 4, 2023 •

edited

Loading

yanggangtony commented Apr 4, 2023 •

edited

Loading