Skip to content

Conversation

@clwluvw
Copy link
Contributor

@clwluvw clwluvw commented Apr 24, 2024

The hidden file might have an endline, so the check for equals to 1 would not be true. Trimming the spaces would help to cover this case.

@google-cla
Copy link

google-cla bot commented Apr 24, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@k8s-ci-robot
Copy link
Collaborator

Hi @clwluvw. Thanks for your PR.

I'm waiting for a google 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.

The hidden file might have an endline, so the check for equals to 1 would not be true.
Trimming the spaces would help to cover this case.

Signed-off-by: Seena Fallah <[email protected]>
@iwankgb
Copy link
Collaborator

iwankgb commented Apr 27, 2024

/ok-to-test

@clwluvw
Copy link
Contributor Author

clwluvw commented May 24, 2024

/retest

@clwluvw
Copy link
Contributor Author

clwluvw commented Oct 3, 2024

Hi @iwankgb - Can you please help here with why the CI is not happy?

@iwankgb iwankgb closed this Oct 4, 2024
@iwankgb iwankgb reopened this Oct 4, 2024
@iwankgb iwankgb merged commit 96c346e into google:master Oct 4, 2024
12 checks passed
@ringerc
Copy link

ringerc commented Jun 3, 2025

If you see errors like Failed to get disk map: open /sys/block/nvme0c0n1/dev: no such file or directory but

# cat  /sys/block/nvme0c0n1/hidden    
1

then this might well be the problem. Check:

# od -x /sys/block/nvme0c0n1/hidden
0000000 0a31
0000002

Here, my hidden files have a leading linefeed, which means they won't match == "1".

@clwluvw
Copy link
Contributor Author

clwluvw commented Jun 3, 2025

Here, my hidden files have a leading linefeed, which means they won't match == "1".

This PR must actually have addressed such. Are you running a version >= v0.51.0?

ringerc pushed a commit to ringerc/google-cadvisor-patches that referenced this pull request Jun 3, 2025
sysfs: trim spaces in device hidden check

(cherry picked from commit 96c346e)

Backports google#3260 from
cadvisor v0.51.0.

This is needed to fix container_fs_ metrics on systems with multipath
devices. Without it, errors like:

    Failed to get disk map: open /sys/block/nvme0c0n1/dev

will be logged, and cadvisor will emit nonsensical metrics with
the label device="" for filesystem I/O.

See google#3693 for full explanation.

Signed-off-by: Craig Ringer <[email protected]>
@ringerc
Copy link

ringerc commented Jun 3, 2025

@clwluvw Yes - I've been able to verify that Red Hat OpenShift (v4.18) is currently shipping a kubelet openshift-kubelet-4.18.0-202505060638.p0.gaf98ede.assembly.stream.el9 that embeds an older fork of cadvisor based on v0.49.0 (https://github.com/openshift/google-cadvisor/tree/v0.49.0-openshift-4.17-3) that omits this fix.

I raised openshift#30 to ask them to backport the fix. Thanks very much for writing the fix.

I don't think this fix is entirely comprehensive - see #3693 for details. But it will prevent problems with hidden devices as are used in multi-path I/O systems, so it'll solve the immediate problem.

ringerc pushed a commit to ringerc/google-cadvisor-patches that referenced this pull request Jun 3, 2025
sysfs: trim spaces in device hidden check

(cherry picked from commit 96c346e)

Backports google#3522 from
cadvisor v0.51.0.

This is needed to fix container_fs_ metrics on systems with multipath
devices. Without it, errors like:

    Failed to get disk map: open /sys/block/nvme0c0n1/dev

will be logged, and cadvisor will emit nonsensical metrics with
the label device="" for filesystem I/O.

See google#3693 for full explanation.

Signed-off-by: Craig Ringer <[email protected]>
harche added a commit to openshift/google-cadvisor that referenced this pull request Jun 13, 2025
Backport pull request google#3522 from clwluvw/hidden-device in cadvisor v0.51.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants