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

BSD: use /proc/mounts instead of /etc/fstab while mount fact gathering #71113

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

joernc-gmail
Copy link

@joernc-gmail joernc-gmail commented Aug 5, 2020

SUMMARY

Use /proc/mounts instead of /etc/fstab to find current mounts. Helps to find ZFS mounts, which are usually not documented in fstab any more.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

NetBSD hardware facts module

ADDITIONAL INFORMATION

I am not a Python programmer, I merely copied and pasted the preceding code that already utilizes /proc.

@ansibot ansibot added affects_2.11 bsd BSD community core_review In order to be merged, this PR must follow the core review workflow. feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Aug 5, 2020
@krytarowski
Copy link

krytarowski commented Aug 5, 2020

How about parsing the output from mount(8) command?

@Akasurde Akasurde changed the title use /proc/mounts instead of /etc/fstab BSD: use /proc/mounts instead of /etc/fstab while mount fact gathering Aug 6, 2020
fstab = get_file_content('/etc/fstab')

if not fstab:
if not os.access("/proc/mounts", os.R_OK):
Copy link
Member

@Akasurde Akasurde Aug 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a changelog entry for this change and test case?

Copy link
Author

@joernc-gmail joernc-gmail Aug 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I did provide a changelog entry. Where should one go, i.e. how do I provide one?

And can you point me to the current tests for the NetBSD facts? As I said: I am no Python programmer. I could try to tweak an already present test, but I cannot create one from scratch.

Copy link
Member

@bcoca bcoca Oct 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

proc/mounts is not guaranteed, so its should fallback to fstab and/or make the source configurable since fstab can be in different locations ... though really a parsable mount output would be the perfect solution

@Akasurde Akasurde requested review from relrod and samdoran Aug 6, 2020
@joernc-gmail
Copy link
Author

joernc-gmail commented Aug 6, 2020

@Shrews Shrews removed the needs_triage Needs a first human triage before being processed. label Aug 6, 2020
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Aug 14, 2020
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. pre_azp This PR was last tested before migration to Azure Pipelines. and removed core_review In order to be merged, this PR must follow the core review workflow. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 4, 2020
@bcoca bcoca self-assigned this Jan 12, 2022
@mattclay
Copy link
Member

mattclay commented Jun 22, 2022

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Jun 22, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. pre_azp This PR was last tested before migration to Azure Pipelines. labels Jun 22, 2022
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.11 affects_2.15 bsd BSD community core_review In order to be merged, this PR must follow the core review workflow. feature This issue/PR relates to a feature request. new_contributor This PR is the first contribution by a new community member. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants