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

Defining the mountpoints per cluster for the filesystem metrics #146

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

dex4er
Copy link
Collaborator

@dex4er dex4er commented Feb 13, 2025

Fixes #145

Signed-off-by: Piotr Roszatycki <piotr.roszatycki@gmail.com>
Signed-off-by: Piotr Roszatycki <piotr.roszatycki@gmail.com>
@dex4er dex4er self-assigned this Feb 13, 2025
@dex4er dex4er added the bug Something isn't working label Feb 13, 2025
@dex4er
Copy link
Collaborator Author

dex4er commented Feb 13, 2025

I'm not sure if we should leave / as default or set it to .*

image

@robertobandini Do you have any AKS for verification maybe? I tested it on EKS with Ubuntu and Bottlerocket OS.

Before:

Ubuntu

image

Bottlerocket

image

After:

Ubuntu

image

Bottlerocket

image

@dex4er
Copy link
Collaborator Author

dex4er commented Feb 13, 2025

Ok, so the finding is that / is too less and .* is too much. node-exporter duplicates some filesystems if they are mounted many times then numbers are summed incorrectly. In my case it should be ~1TB but the same filesystem is bind-mounted 4 times.

The safe default would be /|/local|/mnt|/mnt/.*|/var which should cover the most important systems (Bottlerocket, COS, Talos. Ubuntu)

Edit:

Uh, /var and /mnt are doubled on Bottlerocket too then we won't have universal regexp for every OS

@robertobandini
Copy link
Member

Uh, /var and /mnt are doubled on Bottlerocket too then we won't have universal regexp for every OS

Ok, so If I understood, we need a different value depending on the OS, isn't it?
Maybe we can autodetect it?
At first glance in summary on AKS the old default value works on Linux nodes, but if needed I can do an in-depth test.

Note: to see the disk metrics I had to put Helm instead of Auto detect; it seems to me that something needs to be revised regarding auto detect, what do you think?

Anyway: excellent and fair proposal and excellent work!

@dex4er
Copy link
Collaborator Author

dex4er commented Feb 13, 2025

Maybe it should be heuristic for the default value per different OS like ie. for node shell. Something is better than nothing then I left just the single value per cluster to set.

@dex4er dex4er merged commit 3813624 into freelensapp:main Feb 13, 2025
9 checks passed
@dex4er dex4er deleted the bug/full-disk-usage branch February 13, 2025 22:22
@dex4er dex4er added this to the v1.0.0 milestone Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The graph about node disk usage shows only root disk
2 participants