From 4c5239e82fd1f285376cc5a918700278d90bd7e7 Mon Sep 17 00:00:00 2001 From: Ross Williams Date: Tue, 12 Oct 2021 18:37:39 +0000 Subject: [PATCH] pkgng: fix check_mode for annotate Actions specified in the `annotate` parameter would always be performed, even if `check_mode=yes`. This commit fixes `check_mode` for the annotation functions and adds integration tests to ensure that check mode is honored in the future. --- .../3526-pkgng-add-integration-tests.yml | 1 + plugins/modules/packaging/os/pkgng.py | 43 ++++++++++--------- .../targets/pkgng/tasks/freebsd.yml | 21 +++++++++ 3 files changed, 45 insertions(+), 20 deletions(-) diff --git a/changelogs/fragments/3526-pkgng-add-integration-tests.yml b/changelogs/fragments/3526-pkgng-add-integration-tests.yml index faa38de6cf..9e0a0030ef 100644 --- a/changelogs/fragments/3526-pkgng-add-integration-tests.yml +++ b/changelogs/fragments/3526-pkgng-add-integration-tests.yml @@ -3,3 +3,4 @@ bugfixes: - 'pkgng - PR #3393 (https://github.com/ansible-collections/community.general/pull/3393) broke `check_mode` so that the module always reports `not changed`; fix regression so module reports number of upgrade or install actions that would be performed' - 'pkgng - The module will now convert a single space- or comma-separated name parameter to a list. The documentation had given wrong examples in which multiple space- or comma-separated packages were specified in the `name` parameter, rather than using correct YAML list syntax. `pkgng` module documentation has also been updated' - 'pkgng - `annotation` functionality was broken; uncovered by new integration tests. Small/typo fixes have restored the functionality.' + - 'pkgng - annotation actions performed even if `check_mode=yes`' diff --git a/plugins/modules/packaging/os/pkgng.py b/plugins/modules/packaging/os/pkgng.py index acc9996020..e5dea0c75d 100644 --- a/plugins/modules/packaging/os/pkgng.py +++ b/plugins/modules/packaging/os/pkgng.py @@ -333,11 +333,12 @@ def annotation_add(module, pkgng_path, package, tag, value, dir_arg): _value = annotation_query(module, pkgng_path, package, tag, dir_arg) if not _value: # Annotation does not exist, add it. - rc, out, err = module.run_command('%s %s annotate -y -A %s %s "%s"' - % (pkgng_path, dir_arg, package, tag, value)) - if rc != 0: - module.fail_json(msg="could not annotate %s: %s" - % (package, out), stderr=err) + if not module.check_mode: + rc, out, err = module.run_command('%s %s annotate -y -A %s %s "%s"' + % (pkgng_path, dir_arg, package, tag, value)) + if rc != 0: + module.fail_json(msg="could not annotate %s: %s" + % (package, out), stderr=err) return True elif _value != value: # Annotation exists, but value differs @@ -353,11 +354,12 @@ def annotation_add(module, pkgng_path, package, tag, value, dir_arg): def annotation_delete(module, pkgng_path, package, tag, value, dir_arg): _value = annotation_query(module, pkgng_path, package, tag, dir_arg) if _value: - rc, out, err = module.run_command('%s %s annotate -y -D %s %s' - % (pkgng_path, dir_arg, package, tag)) - if rc != 0: - module.fail_json(msg="could not delete annotation to %s: %s" - % (package, out), stderr=err) + if not module.check_mode: + rc, out, err = module.run_command('%s %s annotate -y -D %s %s' + % (pkgng_path, dir_arg, package, tag)) + if rc != 0: + module.fail_json(msg="could not delete annotation to %s: %s" + % (package, out), stderr=err) return True return False @@ -372,17 +374,18 @@ def annotation_modify(module, pkgng_path, package, tag, value, dir_arg): # No change in value return False else: - rc, out, err = module.run_command('%s %s annotate -y -M %s %s "%s"' - % (pkgng_path, dir_arg, package, tag, value)) + if not module.check_mode: + rc, out, err = module.run_command('%s %s annotate -y -M %s %s "%s"' + % (pkgng_path, dir_arg, package, tag, value)) - # pkg sometimes exits with rc == 1, even though the modification succeeded - # Check the output for a success message - if ( - rc != 0 - and re.search(r'^%s-[^:]+: Modified annotation tagged: %s' % (package, tag), out, flags=re.MULTILINE) is None - ): - module.fail_json(msg="failed to annotate %s, could not change annotation %s to %s: %s" - % (package, tag, value, out), stderr=err) + # pkg sometimes exits with rc == 1, even though the modification succeeded + # Check the output for a success message + if ( + rc != 0 + and re.search(r'^%s-[^:]+: Modified annotation tagged: %s' % (package, tag), out, flags=re.MULTILINE) is None + ): + module.fail_json(msg="failed to annotate %s, could not change annotation %s to %s: %s" + % (package, tag, value, out), stderr=err) return True diff --git a/tests/integration/targets/pkgng/tasks/freebsd.yml b/tests/integration/targets/pkgng/tasks/freebsd.yml index 2b5706c1a5..9efe2b50f0 100644 --- a/tests/integration/targets/pkgng/tasks/freebsd.yml +++ b/tests/integration/targets/pkgng/tasks/freebsd.yml @@ -314,6 +314,24 @@ command: 'pkg annotate -q -S {{ pkgng_test_pkg_name }} ansibletest_example8' register: pkgng_example8_add_annotation_verify +- name: Install and annotate single package (checkmode, not changed) + pkgng: + name: '{{ pkgng_test_pkg_name }}' + annotation: '+ansibletest_example8=added' + check_mode: yes + register: pkgng_example8_add_annotation_checkmode_nochange + +- name: Install and annotate single package (checkmode, changed) + pkgng: + name: '{{ pkgng_test_pkg_name }}' + annotation: '+ansibletest_example8_checkmode=added' + check_mode: yes + register: pkgng_example8_add_annotation_checkmode_change + +- name: Verify check_mode did not add an annotation + command: 'pkg annotate -q -S {{ pkgng_test_pkg_name }} ansibletest_example8_checkmode' + register: pkgng_example8_add_annotation_checkmode_change_verify + - name: Modify annotation on single package pkgng: name: '{{ pkgng_test_pkg_name }}' @@ -346,6 +364,9 @@ that: - pkgng_example8_add_annotation.changed - pkgng_example8_add_annotation_failure.failed + - pkgng_example8_add_annotation_checkmode_nochange is not changed + - pkgng_example8_add_annotation_checkmode_change is changed + - 'pkgng_example8_add_annotation_checkmode_change_verify.stdout_lines | count == 0' - 'pkgng_example8_add_annotation_verify.stdout_lines | first == "added"' - pkgng_example8_modify_annotation.changed - pkgng_example8_modify_annotation_failure.failed