-
Notifications
You must be signed in to change notification settings - Fork 270
fix(errorhandling): Filesystem check and error handling improvements #1964
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
base: main
Are you sure you want to change the base?
Conversation
d2ef0dc to
5985077
Compare
…ks in DNS and TCP retrans plugins Signed-off-by: Iti Agrawal <agrawaliti0107@gmail.com>
5985077 to
00ef41b
Compare
| // CheckAndMountFilesystems checks if required filesystems are mounted. | ||
| // Returns an error if any required filesystem is not available. | ||
| // This helps prevent os.Exit() calls from dependencies that expect these filesystems. | ||
| func CheckAndMountFilesystems(l *log.ZapLogger) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this testable? Does it make sense to pass a slice of the types into this function? That way you would be able to write tests for each one of the scenarios below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed them, Kept the list of fs constant in func, as its covering all the fspath for retina.
| for _, fs := range filesystems { | ||
| var statfs unix.Statfs_t | ||
| for _, path := range fs.paths { | ||
| if err := unix.Statfs(path, &statfs); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems odd, if unix.Statfs returns error we continue which could be missing files, permissions correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its checking all the path and if any required fs is not mounted it returns error, made the func more verbosed
f3a9a61 to
be4acce
Compare
Signed-off-by: Iti Agrawal <agrawaliti0107@gmail.com>
be4acce to
9fbd158
Compare
This pull request introduces a new mechanism to check and ensure required Linux filesystems are mounted before initializing plugins, improving error handling and preventing unexpected agent exits. The changes add a reusable function for filesystem checks and integrate it into the initialization logic of both the DNS and TCP retransmission plugins.
Filesystem check and error handling improvements:
CheckAndMountFilesystemsinpkg/plugin/common/filesystem_linux.goto verify and log the status of required filesystems (bpf,debugfs,tracefs) before plugin initialization, returning errors if critical filesystems are missing.pkg/plugin/dns/dns_linux.goandpkg/plugin/tcpretrans/tcpretrans_linux.goto callCheckAndMountFilesystemsin their respectiveInitmethods, logging errors and returning them to allow the agent to decide whether to continue or fail initialization. [1] [2]Code organization and dependencies:
commonpackage inpkg/plugin/tcpretrans/tcpretrans_linux.goto access the filesystem check functionality.fmtpackage import inpkg/plugin/dns/dns_linux.gofor error wrapping and formatting.# DescriptionPlease provide a brief description of the changes made in this pull request.
Related Issue
If this pull request is related to any issue, please mention it here. Additionally, make sure that the issue is assigned to you before submitting this pull request.
Checklist
git commit -S -s ...). See this documentation on signing commits.Screenshots (if applicable) or Testing Completed
Please add any relevant screenshots or GIFs to showcase the changes made.
Additional Notes
Add any additional notes or context about the pull request here.
Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.~
new error logs:

ticket: #1711