From 5064e67d376f2838eb64e27698f621f90a88568e Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Thu, 27 Jun 2019 21:24:15 +0200 Subject: [PATCH] yum: take care of stale/invalid yum.pid (#58457) * yum: take care of stale/invalid yum.pid * Add changelog --- .../fragments/yum-handle-stale-lock-file.yaml | 2 + lib/ansible/module_utils/yumdnf.py | 24 ++++++++---- lib/ansible/modules/packaging/os/dnf.py | 5 +++ lib/ansible/modules/packaging/os/yum.py | 39 +++++++++++++++++++ test/integration/targets/yum/tasks/lock.yml | 28 +++++++++++++ test/integration/targets/yum/tasks/main.yml | 5 +++ 6 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/yum-handle-stale-lock-file.yaml create mode 100644 test/integration/targets/yum/tasks/lock.yml diff --git a/changelogs/fragments/yum-handle-stale-lock-file.yaml b/changelogs/fragments/yum-handle-stale-lock-file.yaml new file mode 100644 index 00000000000..464134aa977 --- /dev/null +++ b/changelogs/fragments/yum-handle-stale-lock-file.yaml @@ -0,0 +1,2 @@ +bugfixes: + - yum - handle stale/invalid yum.pid lock file (https://github.com/ansible/ansible/issues/57189) diff --git a/lib/ansible/module_utils/yumdnf.py b/lib/ansible/module_utils/yumdnf.py index b8ed14844aa..0b3de6519ae 100644 --- a/lib/ansible/module_utils/yumdnf.py +++ b/lib/ansible/module_utils/yumdnf.py @@ -126,15 +126,25 @@ class YumDnf(with_metaclass(ABCMeta, object)): # default isn't a bad idea self.lockfile = '/var/run/yum.pid' + def is_lockfile_pid_valid(self): + raise NotImplementedError + + def _is_lockfile_present(self): + return (os.path.isfile(self.lockfile) or glob.glob(self.lockfile)) and self.is_lockfile_pid_valid() + def wait_for_lock(self): '''Poll until the lock is removed if timeout is a positive number''' - if (os.path.isfile(self.lockfile) or glob.glob(self.lockfile)): - if self.lock_timeout > 0: - for iteration in range(0, self.lock_timeout): - time.sleep(1) - if not os.path.isfile(self.lockfile) and not glob.glob(self.lockfile): - return - self.module.fail_json(msg='{0} lockfile is held by another process'.format(self.pkg_mgr_name)) + + if not self._is_lockfile_present(): + return + + if self.lock_timeout > 0: + for iteration in range(0, self.lock_timeout): + time.sleep(1) + if not self._is_lockfile_present(): + return + + self.module.fail_json(msg='{0} lockfile is held by another process'.format(self.pkg_mgr_name)) def listify_comma_sep_strings_in_list(self, some_list): """ diff --git a/lib/ansible/modules/packaging/os/dnf.py b/lib/ansible/modules/packaging/os/dnf.py index 1010cd9cc11..995180667e8 100644 --- a/lib/ansible/modules/packaging/os/dnf.py +++ b/lib/ansible/modules/packaging/os/dnf.py @@ -329,6 +329,11 @@ class DnfModule(YumDnf): except AttributeError: self.with_modules = False + def is_lockfile_pid_valid(self): + # FIXME? it looks like DNF takes care of invalid lock files itself? + # https://github.com/ansible/ansible/issues/57189 + return True + def _sanitize_dnf_error_msg(self, spec, error): """ For unhandled dnf.exceptions.Error scenarios, there are certain error diff --git a/lib/ansible/modules/packaging/os/yum.py b/lib/ansible/modules/packaging/os/yum.py index 18174a0ab1b..eb42eb7a565 100644 --- a/lib/ansible/modules/packaging/os/yum.py +++ b/lib/ansible/modules/packaging/os/yum.py @@ -336,6 +336,7 @@ from ansible.module_utils._text import to_native, to_text from ansible.module_utils.urls import fetch_url from ansible.module_utils.yumdnf import YumDnf, yumdnf_argument_spec +import errno import os import re import tempfile @@ -410,6 +411,44 @@ class YumModule(YumDnf): else: raise e + def is_lockfile_pid_valid(self): + try: + with open(self.lockfile, 'r') as f: + oldpid = int(f.readline()) + except ValueError: + # invalid data + os.unlink(self.lockfile) + return False + except (IOError, OSError) as e: + self.module.fail_json(msg="Failure opening %s: %s" % (self.lockfile, to_native(e))) + + if oldpid == os.getpid(): + # that's us? + os.unlink(self.lockfile) + return False + + try: + with open("/proc/%d/stat" % oldpid, 'r') as f: + stat = f.readline() + + if stat.split()[2] == 'Z': + # Zombie + os.unlink(self.lockfile) + return False + except IOError: + try: + os.kill(oldpid, 0) + except OSError as e: + if e.errno == errno.ESRCH: + # No such process + os.unlink(self.lockfile) + return False + + self.module.fail_json(msg="Unable to check PID %s in %s: %s" % (oldpid, self.lockfile, to_native(e))) + + # another copy seems to be running + return True + def yum_base(self): my = yum.YumBase() my.preconf.debuglevel = 0 diff --git a/test/integration/targets/yum/tasks/lock.yml b/test/integration/targets/yum/tasks/lock.yml new file mode 100644 index 00000000000..3f585c1daae --- /dev/null +++ b/test/integration/targets/yum/tasks/lock.yml @@ -0,0 +1,28 @@ +- block: + - name: Make sure testing package is not installed + yum: + name: sos + state: absent + + - name: Create bogus lock file + copy: + content: bogus content for this lock file + dest: /var/run/yum.pid + + - name: Install a package, lock file should be deleted by the module + yum: + name: sos + state: present + register: yum_result + + - assert: + that: + - yum_result is success + + always: + - name: Clean up + yum: + name: sos + state: absent + + when: ansible_pkg_mgr == 'yum' diff --git a/test/integration/targets/yum/tasks/main.yml b/test/integration/targets/yum/tasks/main.yml index fc27cd57469..8f904eb2e96 100644 --- a/test/integration/targets/yum/tasks/main.yml +++ b/test/integration/targets/yum/tasks/main.yml @@ -60,3 +60,8 @@ - import_tasks: check_mode_consistency.yml when: - (ansible_distribution in ['RedHat', 'CentOS', 'ScientificLinux'] and ansible_distribution_major_version|int == 7) + + +- import_tasks: lock.yml + when: + - ansible_distribution in ['RedHat', 'CentOS', 'ScientificLinux']