Conversation
I know this issue was closed, but it's literally a 2 second fix to bring it in line with proper documented parsing of the mountinfo format.
Certain virtual filesystems (e.g. nsfs, SOME tmpfs, overlayfs, etc.)
should NOT duplicate the fstype as the source, as they have a *real*
source.
This PR applies logic to determine whether to use mountinfo field
10 as the device ('source') or to use the mouninfo field 3 ('root')
as the device.
|
merging #1933 into this PR as it's the same function and it's easier to manage the test cases, etc. with the changes in both. |
This commit properly handles logic to determine the real device, it properly filters out non-"real"/non-physical mounts, and it adds more test cases for mountfile parsing.
shirou
left a comment
shirou
left a comment
There was a problem hiding this comment.
Could you fix my comments and lint issue? Thank you!
|
@shirou should have those addressed within 6 hours or so once i'm back at a keyboard! |
|
I completely dropped this; my apologies! Things got busy at $dayjob. I'll try to have this addressed shortly. |
- Removed `fs` param from parseFieldsOnMountinfo sig entirely as it's unused with new logic-driven classification, and callers updated. - Added length check for both `fields` splits.
|
@shirou Any further changes needed? I requested a new review after implementing the change you requested, wasn't sure if you saw yet. |
shirou
left a comment
shirou
left a comment
There was a problem hiding this comment.
Thank you for the contribution! This PR is larger than expected, so I need some time to review it thoroughly. In general, if you want to contribute later, splitting changes into smaller PRs makes it easier to review.
That said, the changes look correct to me. However, this may also introduces breaking changes in the bind mount detection and filtering logic. I will add a "Important Changes" note in the next release.
I really appreciate your effort and contribution to this project!
|
Thank you! I think the majority of the changes were my comments 😆. I'll keep them smaller next time. Cheers! |
### What does this PR do? #incident-49623 Gopsutil 4.25.12 includes a bug which has a big impact on tags emitted by the disk check. Pinning 4.25.11 to fix it in the release. Root cause PR shirou/gopsutil#1931 ### Motivation Avoid the bug mentioned above. ### Describe how you validated your changes Will deploy a custom build internally to validate. ### Additional Notes A fix was already merged in gopsutil but it's safer for us to revert and investigate separately. Co-authored-by: axel.vonengel <axel.vonengel@datadoghq.com>
…efore regression (#46594) ### What does this PR do? #45287 introduced a version upgrade for gopsutil which had a regression (shirou/gopsutil#1931) with device naming / tagging in the disk check. Still investigating. ### Motivation ### Describe how you validated your changes ### Additional Notes Co-authored-by: jose-manuel-almaza <josemanuel.almaza@datadoghq.com> Co-authored-by: jeremy.hanna <jeremy.hanna@datadoghq.com>
3 participants
I know this issue was closed, but it's literally a 2 second fix to bring it in line with proper documented parsing of the mountinfo format.