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

service_facts: Use LANG=C to run commands #44474

Merged
merged 1 commit into from
Aug 22, 2018

Conversation

tyll
Copy link
Contributor

@tyll tyll commented Aug 21, 2018

SUMMARY

This allows to parse the output when the user's locale changes the
commands' output. For example chkconfig uses 'Ein' and 'Aus' instead of
'on' and 'off' when using LANG=de_DE.UTF-8 breaking the service
detection on RHEL 6.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

service_facts

ANSIBLE VERSION
ansible 2.7.0.dev0 (service_facts_LANG_C be6a1236ad) last updated 2018/08/21 18:47:36 (GMT +200)
  config file = /home/till/.ansible.cfg
  configured module search path = [u'/home/till/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/till/scm/gh-ansible-ansible/lib/ansible
  executable location = /home/till/scm/gh-ansible-ansible/bin/ansible
  python version = 2.7.15 (default, May 16 2018, 17:50:09) [GCC 8.1.1 20180502 (Red Hat 8.1.1-1)]

ansible 2.6.1
  config file = /home/till/.ansible.cfg
  configured module search path = [u'/home/till/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.15 (default, May 16 2018, 17:50:09) [GCC 8.1.1 20180502 (Red Hat 8.1.1-1)]

ansible 2.6.2
  config file = /home/till/.ansible.cfg
  configured module search path = [u'/home/till/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.15 (default, May 16 2018, 17:50:09) [GCC 8.1.1 20180502 (Red Hat 8.1.1-1)]

ADDITIONAL INFORMATION
# before
ansible -m service_facts -i rhel6-cloud, all
rhel6-cloud | SKIPPED

# after
ansible -i rhel6-cloud,  -m service_facts all
rhel6-cloud | SUCCESS => {
    "ansible_facts": {
        "services": {
            "acpid": {
                "name": "acpid", 
                "source": "sysv", 
                "state": "running"
            }, 
[...]
            "udev-post": {
                "name": "udev-post", 
                "source": "sysv", 
                "state": "running"
            }
        }
    }, 
    "changed": false
}

@ansibot
Copy link
Contributor

ansibot commented Aug 21, 2018

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. labels Aug 21, 2018
@ansibot
Copy link
Contributor

ansibot commented Aug 21, 2018

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/system/service_facts.py:94:161: E501 line too long (173 > 160 characters)
lib/ansible/modules/system/service_facts.py:155:161: E501 line too long (161 > 160 characters)
lib/ansible/modules/system/service_facts.py:192:161: E501 line too long (164 > 160 characters)

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Aug 21, 2018
@tyll
Copy link
Contributor Author

tyll commented Aug 21, 2018

ready_for_review

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 21, 2018
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Aug 21, 2018
@@ -89,7 +91,8 @@ def gather_services(self):

# sysvinit
if service_path is not None and chkconfig_path is None:
rc, stdout, stderr = self.module.run_command("%s --status-all 2>&1 | grep -E \"\\[ (\\+|\\-) \\]\"" % service_path, use_unsafe_shell=True)
rc, stdout, stderr = self.module.run_command(
"%s --status-all 2>&1 | grep -E \"\\[ (\\+|\\-) \\]\"" % service_path, use_unsafe_shell=True, environ_update=LANG_C)
Copy link
Member

Choose a reason for hiding this comment

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

instead of updating each command just set run_command_environ_update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx, I changed this and also added LC_ALL to the override. When I understand man 7 locale correctly, only setting LC_ALL should suffice since it has the highest priority.

Copy link
Member

Choose a reason for hiding this comment

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

man page says that .. but not all programs obey that, though i think all the ones related to services do.

This allows to parse the output when the user's locale changes the
commands' output. For example chkconfig uses 'Ein' and 'Aus' instead of
'on' and 'off' when using LANG=de_DE.UTF-8 breaking the service
detection on RHEL 6.
@tyll
Copy link
Contributor Author

tyll commented Aug 22, 2018

ready_for_review

@bcoca bcoca merged commit 0fabf21 into ansible:devel Aug 22, 2018
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. small_patch support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants