(FACT-2928) Better error for mountpoints stats #2371
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Dorin-Pleava
force-pushed
the
FACT-2928_better_error_reading_mountpoints_stats
branch
4 times, most recently
from
86ac611
to
ffa514f
Compare
When Sys::Filesystem.stat(mountopint_path) fails, it will throw `statvfs() function failed: No such file or directory`, but will not show what file is missing. Steps I used to reproduce the FACT-2928 error: `$ yum install bind-chroot -y` ... `$ systemctl start named-chroot` Place a binding to stop execution above https://github.com/puppetlabs/facter/blob/main/lib/facter/util/resolvers/filesystem_helper.rb#L20 `$ puppet facts show` `require 'sys/filesystem' => binding.pry Sys::Filesystem.stat(path)` When binding.pry is hit, run `$ systemctl stop named-chroot`, then continue execution.
Dorin-Pleava
force-pushed
the
FACT-2928_better_error_reading_mountpoints_stats
branch
from
ffa514f
to
29b668b
Compare
gimmyxd
reviewed
Apr 28, 2021
| get_bytes_data(mount, stats) | ||
| rescue Sys::Filesystem::Error => e | ||
| @log.debug("Could not get stats for mountpoint #{mount[:path]}, got #{e}") | ||
| mount[:size_bytes] = mount[:available_bytes] = mount[:used_bytes] = 0 |
There was a problem hiding this comment.
Does it help if we add this facts with value 0? since this is not the real information.
What happens if we do not set a value for this fields?
There was a problem hiding this comment.
I think it is for consistency, we have similar behavior in osx resolver:
There was a problem hiding this comment.
It will fail when trying to calculate total_bytes, https://github.com/puppetlabs/facter/blob/main/lib/facter/resolvers/mountpoints.rb#L74 and the fact will return nil.
I added the 0 values as the mountpoint fact already had some real data in it, like mount[:device], mount[:filesystem], mount[:path] and mount[:options]
gimmyxd
approved these changes
Apr 29, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When Sys::Filesystem.stat(mountopint_path) fails, it will throw
statvfs() function failed: No such file or directory, but will notshow what file is missing.
Steps I used to reproduce a similar error as in FACT-2928:
$ yum install bind-chroot -y...
$ systemctl start named-chrootPlace a binding to stop execution above https://github.com/puppetlabs/facter/blob/main/lib/facter/util/resolvers/filesystem_helper.rb#L20
$ puppet facts showrequire 'sys/filesystem' => binding.pry Sys::Filesystem.stat(path)When binding.pry is hit, run
$ systemctl stop named-chroot, thencontinue execution.