Repository: ansible/ansible
Pull request: handle yum and dnf lockfiles - fixes #44120
Signed-off-by: Adam Miller <admiller@redhat.com>

##### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
handle yum and dnf lockfiles

Fixes #44120

<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Feature Pull Request


##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
yum
dnf
yumdnf

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes -->
```paste below
ansible 2.8.0.dev0 (modules/yumdnf 8cf47310a6) last updated 2018/09/07 12:15:42 (GMT -500)
  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 = /home/admiller/src/dev/ansible/lib/ansible
  executable location = /home/admiller/src/dev/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)]
```


Problem statement:
Make Yum module handle yum locks without hanging.
<!---
Verify first that your issue/request is not already reported on GitHub.
THIS FORM WILL BE READ BY A MACHINE, COMPLETE ALL SECTIONS AS DESCRIBED.
Also test if the latest release, and devel branch are affected too.
ALWAYS add information AFTER (OUTSIDE) these html comments.
Otherwise it may end up being automatically closed by our bot. -->

##### SUMMARY
<!--- Explain the problem briefly -->
If there is a lock on yum with a similar error:

```
Another app is currently holding the yum lock; waiting for it to exit...
  The other application is: yum
```

##### ISSUE TYPE
 - Feature Idea

##### COMPONENT NAME
<!--- Insert, BELOW THIS COMMENT, the name of the module, plugin, task or feature.
Do not include extra details here, e.g. "vyos_command" not "the network module vyos_command" or the full path-->

##### ANSIBLE VERSION
<!--- Paste, BELOW THIS COMMENT, verbatim output from "ansible --version" between quotes below -->
```
ansible --version
ansible 2.6.2
  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, Nov 20 2015, 02:00:19) [GCC 4.8.5 20150623 (Red Hat 4.8.5-4)]
```

##### CONFIGURATION
<!--- If using Ansible 2.4 or above, paste, BELOW THIS COMMENT, the results of "ansible-config dump --only-changed"
Otherwise, mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).-->

##### OS / ENVIRONMENT
<!--- Mention, BELOW THIS COMMENT, the OS you are running Ansible from, and the OS you are
managing, or say "N/A" for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.-->

N/A

Hints from issue discussion:
Situation can be reproduced by running yum in another terminal or by creating a lock file.

```
echo 1 > /var/run/yum.pid
```
Lock is there for a reason, I'm pretty sure that it's unsafe to automagically remove it.
We're not going to remove it, we're going to detect the lock and fail the module accordingly.
prototype patch:

```diff
diff --git a/lib/ansible/module_utils/yumdnf.py b/lib/ansible/module_utils/yumdnf.py
index 0d53914462..219e82fc4b 100644
--- a/lib/ansible/module_utils/yumdnf.py
+++ b/lib/ansible/module_utils/yumdnf.py
@@ -11,6 +11,7 @@
 
 import os
 import tempfile
+import time
 from abc import ABCMeta, abstractmethod
 
 from ansible.module_utils._text import to_native
@@ -42,6 +43,8 @@ yumdnf_argument_spec = dict(
         update_cache=dict(type='bool', default=False, aliases=['expire-cache']),
         update_only=dict(required=False, default="no", type='bool'),
         validate_certs=dict(type='bool', default=True),
+        yumlock_poll=dict(type='int', default=-1),
+        yumlock_sleep=dict(type='int', default=10)
         # this should not be needed, but exists as a failsafe
     ),
     required_one_of=[['name', 'list']],
@@ -97,3 +100,14 @@ class YumDnf(with_metaclass(ABCMeta, object)):
     @abstractmethod
     def run(self):
         raise NotImplementedError
+
+
+def wait_for_yum_lock(lockfile='/var/run/yum.pid', seconds=10, interval=-1):
+    '''Poll until the lock is removed if interval is a positive number'''
+    if os.path.isfile(lockfile) and interval > 0:
+        for iteration in range(0, interval):
+            time.sleep(seconds)
+            if not os.path.isfile(lockfile):
+                break
+        if os.path.isfile(lockfile):
+            raise Exception('YUM lockfile was not released')
diff --git a/lib/ansible/modules/packaging/os/yum.py b/lib/ansible/modules/packaging/os/yum.py
index 2fe93fb7a0..50b9684192 100644
--- a/lib/ansible/modules/packaging/os/yum.py
+++ b/lib/ansible/modules/packaging/os/yum.py
@@ -309,7 +309,7 @@ EXAMPLES = '''
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils._text import to_native
 from ansible.module_utils.urls import fetch_url
-from ansible.module_utils.yumdnf import YumDnf, yumdnf_argument_spec
+from ansible.module_utils.yumdnf import YumDnf, yumdnf_argument_spec, wait_for_yum_lock
 
 import os
 import re
@@ -1429,6 +1429,11 @@ class YumModule(YumDnf):
         if not HAS_YUM_PYTHON:
             error_msgs.append('The Python 2 yum module is needed for this module. If you require Python 3 support use the `dnf` Ansible module instead.')
 
+        wait_for_yum_lock(
+            seconds=self.module.params['yumlock_sleep'],
+            interval=self.module.params['yumlock_poll']
+        )
+
         if self.disable_excludes and yum.__version_info__ < (3, 4):
             self.module.fail_json(msg="'disable_includes' is available in yum version 3.4 and onwards.")
 
@@ -1506,6 +1511,8 @@ def main():
     )
 
     module_implementation = YumModule(module)
+    for x in dir(module_implementation.module):
+        import q; q(x)
     module_implementation.run()
```

Commit message: handle yum and dnf lockfiles - fixes #44120

Signed-off-by: Adam Miller <admiller@redhat.com>

Patch to review:
diff --git a/lib/ansible/module_utils/yumdnf.py b/lib/ansible/module_utils/yumdnf.py
index aa3e0138a67781..732ed64b65eebf 100644
--- a/lib/ansible/module_utils/yumdnf.py
+++ b/lib/ansible/module_utils/yumdnf.py
@@ -10,6 +10,8 @@
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 import os
+import time
+import glob
 import tempfile
 from abc import ABCMeta, abstractmethod
 
@@ -43,6 +45,8 @@
         update_only=dict(required=False, default="no", type='bool'),
         validate_certs=dict(type='bool', default=True),
         # this should not be needed, but exists as a failsafe
+        lock_poll=dict(type='int', default=-1),
+        lock_timeout=dict(type='int', default=10),
     ),
     required_one_of=[['name', 'list']],
     mutually_exclusive=[['name', 'list']],
@@ -84,6 +88,8 @@ def __init__(self, module):
         self.update_only = self.module.params['update_only']
         self.update_cache = self.module.params['update_cache']
         self.validate_certs = self.module.params['validate_certs']
+        self.lock_poll = self.module.params['lock_poll']
+        self.lock_timeout = self.module.params['lock_timeout']
 
         # It's possible someone passed a comma separated string since it used
         # to be a string type, so we should handle that
@@ -92,6 +98,20 @@ def __init__(self, module):
         self.enablerepo = self.listify_comma_sep_strings_in_list(self.enablerepo)
         self.exclude = self.listify_comma_sep_strings_in_list(self.exclude)
 
+        # This should really be redefined by both the yum and dnf module but a
+        # default isn't a bad idea
+        self.lockfile = '/var/run/yum.pid'
+
+    def wait_for_lock(self):
+        '''Poll until the lock is removed if interval is a positive number'''
+        if os.path.isfile(self.lockfile) and self.lock_timeout > 0:
+            for iteration in range(0, self.lock_timeout):
+                time.sleep(self.lock_poll)
+                if not os.path.isfile(self.lockfile) or not glob.glob(self.lockfile):
+                    break
+            if os.path.isfile(self.lockfile) or glob.glob(self.lockfile):
+                self.module.fail_json(msg='{0} lockfile was not released'.format(self.pkg_mgr_name))
+
     def listify_comma_sep_strings_in_list(self, some_list):
         """
         method to accept a list of strings as the parameter, find any strings
diff --git a/lib/ansible/modules/packaging/os/dnf.py b/lib/ansible/modules/packaging/os/dnf.py
index 1a50c2eccc3580..9f2657711d473a 100644
--- a/lib/ansible/modules/packaging/os/dnf.py
+++ b/lib/ansible/modules/packaging/os/dnf.py
@@ -184,6 +184,22 @@
     default: "no"
     type: bool
     version_added: "2.7"
+  lock_poll:
+    description:
+      - Poll interval to wait for the dnf lockfile to be freed.
+      - "By default this is set to -1, if you set it to a positive integer it will enable to polling"
+    required: false
+    default: -1
+    type: int
+    version_added: "2.8"
+  lock_timeout:
+    description:
+      - Amount of time to wait for the dnf lockfile to be freed
+      - This should be set along with C(lock_poll) to enable the lockfile polling.
+    required: false
+    default: 10
+    type: int
+    version_added: "2.8"
 notes:
   - When used with a `loop:` each package will be processed individually, it is much more efficient to pass the list directly to the `name` option.
   - Group removal doesn't work if the group was installed with Ansible because
@@ -286,6 +302,8 @@ def __init__(self, module):
         super(DnfModule, self).__init__(module)
 
         self._ensure_dnf()
+        self.lockfile = "/var/cache/dnf/*_lock.pid"
+        self.pkg_mgr_name = "dnf"
 
     def _sanitize_dnf_error_msg(self, spec, error):
         """
diff --git a/lib/ansible/modules/packaging/os/yum.py b/lib/ansible/modules/packaging/os/yum.py
index dad69ef87223f4..db801c0a2d4c3d 100644
--- a/lib/ansible/modules/packaging/os/yum.py
+++ b/lib/ansible/modules/packaging/os/yum.py
@@ -192,6 +192,22 @@
     default: "no"
     type: bool
     version_added: "2.7"
+  lock_poll:
+    description:
+      - Poll interval to wait for the yum lockfile to be freed.
+      - "By default this is set to -1, if you set it to a positive integer it will enable to polling"
+    required: false
+    default: -1
+    type: int
+    version_added: "2.8"
+  lock_timeout:
+    description:
+      - Amount of time to wait for the yum lockfile to be freed
+      - This should be set along with C(lock_poll) to enable the lockfile polling.
+    required: false
+    default: 10
+    type: int
+    version_added: "2.8"
 notes:
   - When used with a `loop:` each package will be processed individually,
     it is much more efficient to pass the list directly to the `name` option.
@@ -374,6 +390,9 @@ def __init__(self, module):
         # This populates instance vars for all argument spec params
         super(YumModule, self).__init__(module)
 
+        self.pkg_mgr_name = "yum"
+        self.lockfile = '/var/run/yum.pid'
+
     def fetch_rpm_from_url(self, spec):
         # FIXME: Remove this once this PR is merged:
         #   https://github.com/ansible/ansible/pull/19172
@@ -1440,6 +1459,8 @@ def run(self):
         if not HAS_YUM_PYTHON:
             error_msgs.append('The Python 2 yum module is needed for this module. If you require Python 3 support use the `dnf` Ansible module instead.')
 
+        self.wait_for_lock()
+
         if self.disable_excludes and yum.__version_info__ < (3, 4):
             self.module.fail_json(msg="'disable_includes' is available in yum version 3.4 and onwards.")
 


Code review comments:
diff_hunk: @@ -92,6 +98,20 @@ def __init__(self, module):
         self.enablerepo = self.listify_comma_sep_strings_in_list(self.enablerepo)
         self.exclude = self.listify_comma_sep_strings_in_list(self.exclude)
 
+        # This should really be redefined by both the yum and dnf module but a
+        # default isn't a bad idea
+        self.lockfile = '/var/run/yum.pid'
+
+    def wait_for_lock(self):
+        '''Poll until the lock is removed if interval is a positive number'''
+        if os.path.isfile(self.lockfile) and self.lock_timeout > 0:
path: lib/ansible/module_utils/yumdnf.py
text: @user1:
Won't this check fail for `dnf` since it is using a glob?

@author:
@user1 it sure will, I'll fix.