Add block io in the container stats of CRI #108575 #109660

yanghesong · 2022-04-26T03:25:27Z

What type of PR is this?

/kind feature
/kind api-change

What this PR does / why we need it:

Block I/O is the stats of tracking how many bytes are read/written from/to the block device on the node, for a particular container.
Refer to
#108575
kubernetes-sigs/cri-tools#898

Which issue(s) this PR fixes:

#108575

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Block io is supported in CRI. Block io state can be aquire by CRI.

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

k8s-ci-robot · 2022-04-26T03:25:29Z

Please note that we're already in Test Freeze for the release-1.24 branch. This means every merged PR has to be cherry-picked into the release branch to be part of the upcoming v1.24.0 release.

k8s-ci-robot · 2022-04-26T03:25:36Z

Hi @yanghesong. Thanks for your PR.

I'm waiting for a kubernetes 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-triage-robot · 2022-04-26T04:09:49Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

pacoxu · 2022-04-26T08:48:20Z

@yanghesong: Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

Please follow the guide above to rebase the branch.

yanghesong · 2022-04-26T14:15:33Z

@yanghesong: Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

Please follow the guide above to rebase the branch.

Done.

pacoxu · 2022-05-05T07:54:03Z

/ok-to-test

pacoxu · 2022-05-05T08:55:10Z

/triage accepted
/priority important-longterm

since containerd and cri-o block io support is almost ready. #92287 (comment)

Current status of blockio throttling in container runtimes:

CRI-O: support released in v1.22, see cri-o/cri-o#4873.
Containerd: a fully compatible containerd/containerd#5490 is open.

mrunalp · 2023-04-20T22:51:39Z

@haircommander @bobbypage ptal

haircommander · 2023-04-21T12:40:57Z

staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.proto

@@ -1627,6 +1627,8 @@ message ContainerStats {
    MemoryUsage memory = 3;
    // Usage of the writable layer.
    FilesystemUsage writable_layer = 4;
+    // Usage of the Block IO
+    BlockIOUsage block_io = 5;


Is this possibly relevant to the windows stats object as well?

cc @marosset

were we going to deprecate the duplicate object definitions?

no the path forward is to have them defined for each OS to allow customized stats for each (when eventually appropriate)

thx :-) let it be written!

haircommander · 2023-04-24T17:53:42Z

so this brings up an architectural question I don't have a great answer for yet: should the stats in this object only be for populating Kubelet's /stats/summary API or should they be extended for debugging with crictl? I am not sure, but I lean to the former (as that was the purpose of adding it in the first place). if that's the case--since kubelet doesn't use this in /stats/summary, I'm not sure we should add this metric

pacoxu · 2023-04-25T09:21:21Z

CRI will be changed in version of v1 and v1alpha

The release note is not clear. You should mention that block io is supported.

yanghesong · 2023-04-26T05:23:18Z

CRI will be changed in version of v1 and v1alpha

The release note is not clear. You should mention that block io is supported.

Done. Thanks!

mikebrow

LGTM

mikebrow · 2023-04-26T18:38:52Z

so this brings up an architectural question I don't have a great answer for yet: should the stats in this object only be for populating Kubelet's /stats/summary API or should they be extended for debugging with crictl? I am not sure, but I lean to the former (as that was the purpose of adding it in the first place). if that's the case--since kubelet doesn't use this in /stats/summary, I'm not sure we should add this metric

seems like a useful stat.. rancher/cloud client tools may also use it..

I'd expect /stats/summary to have a --raw option, at min, that would show the new field?

pacoxu · 2023-09-07T10:59:54Z

PR needs rebase.

Do you have time to rebase this? We may have this in v1.29 release.

yanghesong · 2023-09-09T00:21:18Z

PR needs rebase.

Do you have time to rebase this? We may have this in v1.29 release.

I will rebase it as soon as possible.

Add block io in the container stats of CRI. Signed-off-by: yanghesong <hesong.yang@foxmail.com>

k8s-ci-robot · 2023-09-11T04:48:01Z

New changes are detected. LGTM label has been removed.

k8s-ci-robot · 2023-09-11T04:48:31Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mikebrow, yanghesong
Once this PR has been reviewed and has the lgtm label, please ask for approval from mrunalp. 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:

staging/src/k8s.io/cri-api/pkg/OWNERS

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

yanghesong · 2023-09-11T05:22:16Z

/test pull-kubernetes-node-e2e-containerd

yanghesong · 2023-09-11T08:17:16Z

PR needs rebase.

Do you have time to rebase this? We may have this in v1.29 release.

Rebase has been done. /cc @pacoxu

haircommander · 2023-09-11T15:00:49Z

personally I am a NACK on this. I understand the motivation but I don't think this is in scope of /stats/summary API. I think this is better suited for a prometheus metric or something else. I am not staunchly opposed so I could be convinced but I think we should be wary of what we expose here

k8s-ci-robot · 2023-10-20T23:25:00Z

PR needs rebase.

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-triage-robot · 2024-02-04T07:17:26Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and 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:

Reopen this PR with /reopen
Mark this PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

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

/close

k8s-ci-robot · 2024-02-04T07:17:32Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and 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:

Reopen this PR with /reopen

Mark this PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

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

/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 requested review from pacoxu and yujuhong April 26, 2022 03:26

yanghesong force-pushed the Add_block_IO_in_the_ContainerStats branch from 6654baa to 818962a Compare April 26, 2022 14:13

k8s-ci-robot removed the do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. label Apr 26, 2022

SergeyKanzhelev added this to Triage in SIG Node PR Triage Apr 27, 2022

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 May 5, 2022

marquiz moved this from Needs Reviewer to Needs Approver in SIG Node PR Triage Apr 13, 2023

haircommander reviewed Apr 21, 2023

View reviewed changes

mikebrow approved these changes Apr 26, 2023

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 9, 2023

pacoxu moved this from Needs Approver to Waiting on Author in SIG Node PR Triage Sep 7, 2023

pacoxu mentioned this pull request Sep 7, 2023

Add block I/O in the ContainerStats of CRI #108575

Open

Add block I/O in the ContainerStats of CRI kubernetes#108575

6e670b0

Add block io in the container stats of CRI. Signed-off-by: yanghesong <hesong.yang@foxmail.com>

yanghesong force-pushed the Add_block_IO_in_the_ContainerStats branch from ebf3922 to 6e670b0 Compare September 11, 2023 04:47

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 11, 2023

k8s-ci-robot requested a review from marquiz September 11, 2023 04:48

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 11, 2023

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2023

dims added the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Oct 24, 2023

k8s-ci-robot closed this Feb 4, 2024

SIG Node PR Triage automation moved this from Waiting on Author to Done Feb 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add block io in the container stats of CRI #108575 #109660

Add block io in the container stats of CRI #108575 #109660

yanghesong commented Apr 26, 2022 •

edited

k8s-ci-robot commented Apr 26, 2022

k8s-ci-robot commented Apr 26, 2022

k8s-triage-robot commented Apr 26, 2022

pacoxu commented Apr 26, 2022

yanghesong commented Apr 26, 2022

pacoxu commented May 5, 2022

pacoxu commented May 5, 2022 •

edited

mrunalp commented Apr 20, 2023

haircommander Apr 21, 2023

mikebrow Apr 26, 2023

haircommander Apr 26, 2023

mikebrow Apr 26, 2023

haircommander commented Apr 24, 2023

pacoxu commented Apr 25, 2023

yanghesong commented Apr 26, 2023

mikebrow left a comment

mikebrow commented Apr 26, 2023

pacoxu commented Sep 7, 2023

yanghesong commented Sep 9, 2023

k8s-ci-robot commented Sep 11, 2023

k8s-ci-robot commented Sep 11, 2023

yanghesong commented Sep 11, 2023

yanghesong commented Sep 11, 2023

haircommander commented Sep 11, 2023

k8s-ci-robot commented Oct 20, 2023

k8s-triage-robot commented Feb 4, 2024

k8s-ci-robot commented Feb 4, 2024

Add block io in the container stats of CRI #108575 #109660

Add block io in the container stats of CRI #108575 #109660

Conversation

yanghesong commented Apr 26, 2022 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

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

k8s-ci-robot commented Apr 26, 2022

k8s-ci-robot commented Apr 26, 2022

k8s-triage-robot commented Apr 26, 2022

pacoxu commented Apr 26, 2022

yanghesong commented Apr 26, 2022

pacoxu commented May 5, 2022

pacoxu commented May 5, 2022 • edited

mrunalp commented Apr 20, 2023

haircommander Apr 21, 2023

Choose a reason for hiding this comment

mikebrow Apr 26, 2023

Choose a reason for hiding this comment

haircommander Apr 26, 2023

Choose a reason for hiding this comment

mikebrow Apr 26, 2023

Choose a reason for hiding this comment

haircommander commented Apr 24, 2023

pacoxu commented Apr 25, 2023

yanghesong commented Apr 26, 2023

mikebrow left a comment

Choose a reason for hiding this comment

mikebrow commented Apr 26, 2023

pacoxu commented Sep 7, 2023

yanghesong commented Sep 9, 2023

k8s-ci-robot commented Sep 11, 2023

k8s-ci-robot commented Sep 11, 2023

yanghesong commented Sep 11, 2023

yanghesong commented Sep 11, 2023

haircommander commented Sep 11, 2023

k8s-ci-robot commented Oct 20, 2023

k8s-triage-robot commented Feb 4, 2024

k8s-ci-robot commented Feb 4, 2024

yanghesong commented Apr 26, 2022 •

edited

pacoxu commented May 5, 2022 •

edited