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

Allow fuse type mounts without :/ in device string #41502

Closed
wants to merge 1 commit into from

Conversation

DBezemer
Copy link

SUMMARY

Allow fuse type mounts in the gathered mount information when not matching traditional formatting with ":/"

Fixes #41494

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

setup

ANSIBLE VERSION
ansible 2.5.3
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /bin/ansible
  python version = 2.7.5 (default, Feb 20 2018, 09:19:12) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. 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 Jun 13, 2018
@jborean93 jborean93 removed the needs_triage Needs a first human triage before being processed. label Jun 14, 2018
@ansibot ansibot added small_patch 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 Jun 22, 2018
@ansibot ansibot added support:community This issue/PR relates to code supported by the Ansible community. and removed support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 18, 2018
@ansibot ansibot added the community_review In order to be merged, this PR must follow the community review workflow. label Oct 24, 2018
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed community_review In order to be merged, this PR must follow the community review workflow. support:community This issue/PR relates to code supported by the Ansible community. labels Nov 26, 2018
@gundalow gundalow added the candidate_to_close Think we can close this, though need to check with Core label Nov 29, 2018
@maxamillion maxamillion added performance and removed candidate_to_close Think we can close this, though need to check with Core labels Nov 30, 2018
@abadger
Copy link
Contributor

abadger commented Nov 30, 2018

We talked about this today and think that this PR will fix the bug but it has the potential to hang, timeout, and then lose information about any mounts that would have come afterwards. We think we have a strategy that you could implement to address that.

The ramifications of doing that will be:

  • One mount that timeouts will not break gathering facts for other mounts
  • The total potential timeout period will be longer. Instead of being one timeout that covers loading all of the mounts it will be the sum of timeouts for each mount point.

@bcoca
Copy link
Member

bcoca commented Nov 30, 2018

quick draft of proposed solution #49398

@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jan 24, 2019
@@ -454,7 +454,7 @@ def get_mount_facts(self):
for fields in mtab_entries:
device, mount, fstype, options = fields[0], fields[1], fields[2], fields[3]

if not device.startswith('/') and ':/' not in device:
if not device.startswith('/') and ':/' not in device and not fstype.startswith('fuse'):
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than continuing to make this conditional more complex, I suggest making a filter function to encapsulate this logic. Then the list of mtab_entries can be filtered before looping.

filtered_mtab_entries = (e for e in mtab_entries if _filter_mtab_entries(e))

Or the filter() built-in could be used rather than a generator:

filtered_mtab_entries = filter(_filter_mtab_entries, mtab_entries)

@ansibot ansibot added pre_azp This PR was last tested before migration to Azure Pipelines. and removed 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 3, 2020
@s-hertel
Copy link
Contributor

@DBezemer Are you able to update this PR?

needs_info

@ansibot ansibot added the needs_info This issue requires further information. Please answer any outstanding questions. label Nov 19, 2021
@DBezemer
Copy link
Author

@s-hertel I will not be able to work on this PR.

@ansibot ansibot removed the needs_info This issue requires further information. Please answer any outstanding questions. label Nov 22, 2021
@bcoca
Copy link
Member

bcoca commented Mar 16, 2022

related #24644

@jborean93 jborean93 added affects_2.14 and removed affects_2.7 This issue/PR affects Ansible v2.7 labels Aug 10, 2022
@jborean93
Copy link
Contributor

Closing due to extra requirements by #41502 (comment) to handle things like hangs and the original author is unable to continue with the PR.

@jborean93 jborean93 closed this Feb 22, 2023
@ansible ansible locked and limited conversation to collaborators Mar 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.14 bug This issue/PR relates to a bug. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_contributor This PR is the first contribution by a new community member. performance pre_azp This PR was last tested before migration to Azure Pipelines. small_patch 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.

Setup module missing fuse mounts
10 participants