From a67ea75decb3bccd09b61bbf7ab720f56ebd687a Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Tue, 9 Jul 2013 20:56:22 +0200 Subject: [PATCH 1/4] openbsd_pkg: handle pkg_add quirks better. This fixes a problem when trying to install a package with a specific version number from a local directory and the local directory is checked after a remote repository: Error from http://ftp.eu.openbsd.org/pub/OpenBSD/[...]/packagename-1.0.tgz ftp: Error retrieving file: 404 Not Found packagename-1.0: ok --- library/packaging/openbsd_pkg | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/library/packaging/openbsd_pkg b/library/packaging/openbsd_pkg index c4ac57d8a2..16892283a7 100644 --- a/library/packaging/openbsd_pkg +++ b/library/packaging/openbsd_pkg @@ -102,7 +102,7 @@ def get_package_state(name, specific_version): return False # Function used to make sure a package is present. -def package_present(name, installed_state, module): +def package_present(name, installed_state, specific_version, module): if module.check_mode: install_cmd = 'pkg_add -In' else: @@ -113,16 +113,28 @@ def package_present(name, installed_state, module): # Attempt to install the package (rc, stdout, stderr) = execute_command("%s %s" % (install_cmd, name), syslogging) - # pkg_add returns 0 even if the package does not exist - # so depend on stderr instead if something bad happened. - if stderr: - rc = 1 - changed=False + # The behaviour of pkg_add is a bit different depending on if a + # specific version is supplied or not. + # + # When a specific version is supplied the return code will be 0 when + # a package is found and 1 when it is not, if a version is not + # supplied the tool will exit 0 in both cases: + if specific_version: + # Depend on the return code. + if rc: + changed=False else: + # Depend on stderr instead and fake the return code. + if stderr: + rc = 1 + changed=False + + if rc == 0: if module.check_mode: module.exit_json(changed=True) changed=True + else: rc = 0 stdout = '' @@ -234,7 +246,7 @@ def main(): # Perform requested action if state in ['installed', 'present']: - (rc, stdout, stderr, changed) = package_present(name, installed_state, module) + (rc, stdout, stderr, changed) = package_present(name, installed_state, specific_version, module) elif state in ['absent', 'removed']: (rc, stdout, stderr, changed) = package_absent(name, installed_state, module) elif state == 'latest': From 0e4e6e8a84d344753a340220d087cd97888aaeec Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Wed, 10 Jul 2013 18:19:01 +0200 Subject: [PATCH 2/4] openbsd_pkg: Handle another pkg_add gotcha * Add '-m' to pkg_add incovation to get access to the "packagename-1.0: ok" message. * Watch for that message if we are about to fail because of stderr in package_present(). --- library/packaging/openbsd_pkg | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/library/packaging/openbsd_pkg b/library/packaging/openbsd_pkg index 16892283a7..b16a0c0648 100644 --- a/library/packaging/openbsd_pkg +++ b/library/packaging/openbsd_pkg @@ -104,9 +104,9 @@ def get_package_state(name, specific_version): # Function used to make sure a package is present. def package_present(name, installed_state, specific_version, module): if module.check_mode: - install_cmd = 'pkg_add -In' - else: - install_cmd = 'pkg_add -I' + install_cmd = 'pkg_add -Imn' + else: + install_cmd = 'pkg_add -Im' if installed_state is False: @@ -124,10 +124,21 @@ def package_present(name, installed_state, specific_version, module): if rc: changed=False else: - # Depend on stderr instead and fake the return code. + # Depend on stderr instead. if stderr: - rc = 1 - changed=False + # There is a corner case where having an empty directory in + # installpath prior to the right location will result in a + # "file:/local/package/directory/ is empty" message on stderr + # while still installing the package, so we need to look for + # for a message like "packagename-1.0: ok" just in case. + match = re.search("\W%s-[^:]+: ok\W" % name, stdout) + if match: + # It turns out we were able to install the package. + pass + else: + # We really did fail, fake the return code. + rc = 1 + changed=False if rc == 0: if module.check_mode: From af52cd6ec127e839ba15e69302d9a2be33f99eea Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Wed, 10 Jul 2013 21:06:35 +0200 Subject: [PATCH 3/4] openbsd_pkg: sync package_latest(). This diff syncs package_latest() with the changes to package_present(). I have not managed to figure out how to handle the cornercases where stderr is set but the command has not failed, so leave a FIXME blob for other adventurers. --- library/packaging/openbsd_pkg | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/library/packaging/openbsd_pkg b/library/packaging/openbsd_pkg index b16a0c0648..f02860a99e 100644 --- a/library/packaging/openbsd_pkg +++ b/library/packaging/openbsd_pkg @@ -183,16 +183,24 @@ def package_latest(name, installed_state, specific_version, module): else: changed = False - # 'pkg_add -u' returns 0 even when something strange happened, stderr - # should be empty if everything went fine. - if stderr: - rc=1 + # FIXME: This part is problematic. Based on the issues mentioned (and + # handled) in package_present() it is not safe to blindly trust stderr + # as an indicator that the command failed, and in the case with + # empty installpath directories this will break. + # + # For now keep this safeguard here, but ignore it if we managed to + # parse out a successful update above. This way we will report a + # successful run when we actually modify something but fail + # otherwise. + if changed != True: + if stderr: + rc=1 return (rc, stdout, stderr, changed) else: # If package was not installed at all just make it present. - return package_present(name, installed_state, module) + return package_present(name, installed_state, specific_version, module) # Function used to make sure a package is not installed. def package_absent(name, installed_state, module): From 1b08da26fda05341a03d0b5647bdd06d4b9b8c57 Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Wed, 10 Jul 2013 21:23:10 +0200 Subject: [PATCH 4/4] openbsd_pkg: some whitespace cleanup. --- library/packaging/openbsd_pkg | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/packaging/openbsd_pkg b/library/packaging/openbsd_pkg index f02860a99e..cfce758f99 100644 --- a/library/packaging/openbsd_pkg +++ b/library/packaging/openbsd_pkg @@ -56,7 +56,7 @@ EXAMPLES = ''' # select whether we dump additional debug info through syslog syslogging = False -# Function used for executing commands. +# Function used for executing commands. def execute_command(cmd, syslogging): if syslogging: syslog.openlog('ansible-%s' % os.path.basename(__file__)) @@ -115,13 +115,13 @@ def package_present(name, installed_state, specific_version, module): # The behaviour of pkg_add is a bit different depending on if a # specific version is supplied or not. - # + # # When a specific version is supplied the return code will be 0 when # a package is found and 1 when it is not, if a version is not # supplied the tool will exit 0 in both cases: if specific_version: # Depend on the return code. - if rc: + if rc: changed=False else: # Depend on stderr instead. @@ -158,7 +158,7 @@ def package_present(name, installed_state, specific_version, module): def package_latest(name, installed_state, specific_version, module): if module.check_mode: upgrade_cmd = 'pkg_add -umn' - else: + else: upgrade_cmd = 'pkg_add -um' pre_upgrade_name = ''