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

document ResourceUsage and Metrics CR #975

Merged
merged 8 commits into from
Mar 25, 2024

Conversation

caozhuozi
Copy link
Contributor

What type of PR is this?

/kind documentation

What this PR does / why we need it:

The PR tries to document ResourceUsage and Metrics CR.

Which issue(s) this PR fixes:

Fixes #942

Special notes for your reviewer:

Please feel free to leave comments.

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/documentation Categorizes issue or PR as related to documentation. labels Feb 14, 2024
Copy link

netlify bot commented Feb 14, 2024

Deploy Preview for k8s-kwok ready!

Name Link
🔨 Latest commit 8d97842
🔍 Latest deploy log https://app.netlify.com/sites/k8s-kwok/deploys/65fab2dc21b04100089f64cb
😎 Deploy Preview https://deploy-preview-975--k8s-kwok.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 14, 2024
@k8s-ci-robot 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 Feb 14, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @caozhuozi. 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.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 14, 2024
@caozhuozi caozhuozi force-pushed the docs/resource-usage branch 4 times, most recently from 2a2571c to 1a19492 Compare February 15, 2024 02:10
@wzshiming
Copy link
Member

/ok-to-test

@k8s-ci-robot 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 Feb 18, 2024
site/content/en/docs/user/resource-usage-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/resource-usage-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/metrics-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/metrics-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/resource-usage-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/metrics-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/resource-usage-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/metrics-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/resource-usage-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/metrics-configuration.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Feb 18, 2024
@caozhuozi caozhuozi force-pushed the docs/resource-usage branch 2 times, most recently from 8289da7 to 2d87b17 Compare February 19, 2024 01:42
Copy link
Contributor

@windsonsea windsonsea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pick some more nits

site/content/en/docs/user/metrics-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/metrics-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/metrics-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/resource-usage-configuration.md Outdated Show resolved Hide resolved
site/content/en/docs/user/resource-usage-configuration.md Outdated Show resolved Hide resolved
@windsonsea
Copy link
Contributor

lgtm for the text and you can mark those comments Resolved and try to pass the ci test.

@caozhuozi
Copy link
Contributor Author

@wzshiming Any new comments?

site/content/en/docs/user/kwok-in-cluster.md Show resolved Hide resolved
Comment on lines 65 to 82
The `metrics` field are used to customize the return body of the installed metrics endpoint.
`metrics` is a list of specific configuration items, with each corresponding to a Prometheus style metric:
* `name` defines the metric name.
* `labels` defines the metric labels, with each item corresponding to a specific metric label.
- `name` is a const string that provides the label name.
- `value` is represented as a CEL expression that dynamically determines the label value.
For example: you can use `node.metadata.name` to reference the node name as the label value.
* `help` defines the help string of a metric.
* `kind` defines the type of the metric: `counter`, `guage` or `histogram`.
* `dimension` defines where the data comes from. It could be `node`, `pod`, or `container`.
* `value` is a CEL expression that defines the metric value if `kind` is `counter` or `guage`.
Please refer to [built-in CEL extension functions] for an exhausted list that you can use to simulate the metric value.
* `buckets` is exclusively for customizing the data of the metric of kind `histogram`.
- `le`, which defines the histogram bucket’s upper threshold, has the same meaning as the one of Prometheus histogram bucket.
That is, each bucket contains values less than or equal to `le`.
- `value` is a CEL expression that provides the value of the bucket.
- `hidden` indicates whether to show the bucket in the metric.
But the value of the bucket will be calculated and cumulated into the next bucket.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section feels redundant, as there is already automatically generated documentation from API, we should update the API comments.

Comment on lines 87 to 97
## built-in CEL extension functions

As `kwok` doesn't actually run containers, to simulate the resource usage data, `kwok` provides the following CEL
extension functions you can use in a CEL expression to help calculate the simulation data flexibly and dynamically:
* `Now()`: returns the current timestamp.
* `Rand()`: returns a random `float64` value.
* `SinceSecond()` returns the seconds elapsed since a resource (pod or node) was created.
For example: `pod.SinceSecond()`, `node.SinceSecond()`.
* `UnixSecond()` returns the Unix time of a given time of type `time.Time`.
For example: , `UnixSecond(Now())`, `UnixSecond(node.metadata.creationTimestamp)`.
* `Quantity()` returns a float64 value of a given Quantity value. For example: `Quantity("100m")`, `Quantity("10Mi")`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should devote a new page to generic template functions, it also includes #836

Comment on lines 38 to 46
To associate a ResourceUsage with a certain pod to be simulated, users must ensure `metadata.name` and `metadata.namespace` are inconsistent with the name and namespace of the target pod.

The resource usages of a pod are specified via `usages` field.
The `usages` field are organized by groups, with each corresponding to a collection of containers that shares a same resource usage simulation setting.
Each group consists of a list of container names (`containers`) and the shared resource usage setting (`usage`).

{{< hint "info" >}}
If `containers` is not given in a group, the `usage` in that group will be applied to all containers of the target pod.
{{< /hint >}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this part of the description consistent with Exec, Logs, Attach, PortForward.

@caozhuozi
Copy link
Contributor Author

caozhuozi commented Mar 15, 2024

Hi, @wzshiming. I'm sorry for the late update.
In this new commit, I made the following change based on your comments:

  1. add 2 separate pages for the go template and CEL expressions respectively.
  2. replace resource usage demo and svg files.
  3. add more description in "Set up default CRs of resource usage (optional) based on your comment: document ResourceUsage and Metrics CR #975 (comment)

For the comment "Make the writing consistent across pages“, I'm going to file a separate PR to do this because I think it would be more clear.

You can give comments and leave this PR unmerged until the PR for writing consistency is ready and get reviewed.

@caozhuozi caozhuozi force-pushed the docs/resource-usage branch from 73424d7 to 2d469d9 Compare March 17, 2024 16:26
@caozhuozi
Copy link
Contributor Author

Thank you for your great patience in reviewing and helping with modifications. @wzshiming @windsonsea

@wzshiming
Copy link
Member

[*] Verifying ends newline...
Add a new line in ends for below files
./site/content/en/docs/user/cel-expressions.md
./site/content/en/docs/user/stages-configuration.md

@caozhuozi
Copy link
Contributor Author

[*] Verifying ends newline...
Add a new line in ends for below files
./site/content/en/docs/user/cel-expressions.md
./site/content/en/docs/user/stages-configuration.md

I ignored the CI result as it seems like it has been always failing before. sry about this...

@caozhuozi
Copy link
Contributor Author

caozhuozi commented Mar 21, 2024

I've checked the failed CI stage this time. The logs of the failed one: "test-kwokctl" seems not related to the documentation.

@wzshiming
Copy link
Member

/approve
/lgtm
/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Mar 25, 2024
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 25, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: caozhuozi, wzshiming

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 25, 2024
@k8s-ci-robot k8s-ci-robot merged commit 5c13cee into kubernetes-sigs:main Mar 25, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do we have document about how to perform Resource usage simulation?
4 participants