Release 3.3.2.

* dicts and lists change-tracking is fixed

* added changelog fragment

* fixed sanity check

(cherry picked from commit 56acd4356f)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

CHANGELOG.rst

@@ -6,6 +6,24 @@ Community General Release Notes
 
 This changelog describes changes after version 2.0.0.
 
+v3.3.2
+======
+
+Release Summary
+---------------
+
+Extraordinary bugfix release to fix some annoying bugs.
+
+Bugfixes
+--------
+
+- archive - fixed task failure when using the ``remove`` option with a ``path`` containing nested files for ``format``s other than ``zip`` (https://github.com/ansible-collections/community.general/issues/2919).
+- lvol - honor ``check_mode`` on thinpool (https://github.com/ansible-collections/community.general/issues/2934).
+- module_helper module utils - fixed change-tracking for dictionaries and lists (https://github.com/ansible-collections/community.general/pull/2951).
+- npm - correctly handle cases where a dependency does not have a ``version`` property because it is either missing or invalid (https://github.com/ansible-collections/community.general/issues/2917).
+- pacman - fix changed status when ignorepkg has been defined (https://github.com/ansible-collections/community.general/issues/1758).
+- snap - fixed the order of the ``--classic`` parameter in the command line invocation (https://github.com/ansible-collections/community.general/issues/2916).
+
 v3.3.1
 ======
 

changelogs/changelog.yaml

@@ -1408,3 +1408,26 @@ releases:
     - 2912-snap-module-helper.yml
     - 3.3.1.yml
     release_date: '2021-07-01'
+  3.3.2:
+    changes:
+      bugfixes:
+      - archive - fixed task failure when using the ``remove`` option with a ``path``
+        containing nested files for ``format``s other than ``zip`` (https://github.com/ansible-collections/community.general/issues/2919).
+      - lvol - honor ``check_mode`` on thinpool (https://github.com/ansible-collections/community.general/issues/2934).
+      - module_helper module utils - fixed change-tracking for dictionaries and lists
+        (https://github.com/ansible-collections/community.general/pull/2951).
+      - npm - correctly handle cases where a dependency does not have a ``version``
+        property because it is either missing or invalid (https://github.com/ansible-collections/community.general/issues/2917).
+      - pacman - fix changed status when ignorepkg has been defined (https://github.com/ansible-collections/community.general/issues/1758).
+      - snap - fixed the order of the ``--classic`` parameter in the command line
+        invocation (https://github.com/ansible-collections/community.general/issues/2916).
+      release_summary: Extraordinary bugfix release to fix some annoying bugs.
+    fragments:
+    - 2918-snap-param-order.yml
+    - 2923-archive-remove-bugfix.yml
+    - 2924-npm-fix-package-json.yml
+    - 2935-lvol-support_check_mode_thinpool.yml
+    - 2936-pacman-fix_changed_status_when_ignorepkg_has_been_defined.yml
+    - 2951-mh-vars-deepcopy.yml
+    - 3.3.2.yml
+    release_date: '2021-07-08'

galaxy.yml

@@ -1,6 +1,6 @@
 namespace: community
 name: general
-version: 3.3.1
+version: 3.3.2
 readme: README.md
 authors:
   - Ansible (https://github.com/ansible)

plugins/module_utils/mh/mixins/vars.py

@@ -6,6 +6,8 @@
 from __future__ import absolute_import, division, print_function
 __metaclass__ = type
 
+import copy
+
 
 class VarMeta(object):
     NOTHING = object()
@@ -30,11 +32,11 @@ class VarMeta(object):
         if fact is not None:
             self.fact = fact
         if initial_value is not self.NOTHING:
-            self.initial_value = initial_value
+            self.initial_value = copy.deepcopy(initial_value)
 
     def set_value(self, value):
         if not self.init:
-            self.initial_value = value
+            self.initial_value = copy.deepcopy(value)
             self.init = True
         self.value = value
         return self

plugins/modules/files/archive.py

@@ -399,13 +399,14 @@ class Archive(object):
 
     def remove_targets(self):
         for path in self.successes:
-            try:
-                if os.path.isdir(path):
-                    shutil.rmtree(path)
-                else:
-                    os.remove(path)
-            except OSError:
-                self.errors.append(_to_native(path))
+            if os.path.exists(path):
+                try:
+                    if os.path.isdir(path):
+                        shutil.rmtree(path)
+                    else:
+                        os.remove(path)
+                except OSError:
+                    self.errors.append(_to_native(path))
         for path in self.paths:
             try:
                 if os.path.isdir(path):

plugins/modules/packaging/language/npm.py

@@ -216,7 +216,6 @@ class Npm(object):
             self.module.fail_json(msg="Failed to parse NPM output with error %s" % to_native(e))
         if 'dependencies' in data:
             for dep, props in data['dependencies'].items():
-                dep_version = dep + '@' + str(props['version'])
 
                 if 'missing' in props and props['missing']:
                     missing.append(dep)
@@ -224,7 +223,9 @@ class Npm(object):
                     missing.append(dep)
                 else:
                     installed.append(dep)
-                    installed.append(dep_version)
+                    if 'version' in props and props['version']:
+                        dep_version = dep + '@' + str(props['version'])
+                        installed.append(dep_version)
             if self.name_version and self.name_version not in installed:
                 missing.append(self.name)
         # Named dependency not installed

plugins/modules/packaging/os/pacman.py

@@ -254,16 +254,23 @@ def upgrade(module, pacman_path):
         # e.g., "ansible 2.7.1-1 -> 2.7.2-1"
         regex = re.compile(r'([\w+\-.@]+) (\S+-\S+) -> (\S+-\S+)')
         for p in data:
-            m = regex.search(p)
-            packages.append(m.group(1))
-            if module._diff:
-                diff['before'] += "%s-%s\n" % (m.group(1), m.group(2))
-                diff['after'] += "%s-%s\n" % (m.group(1), m.group(3))
+            if '[ignored]' not in p:
+                m = regex.search(p)
+                packages.append(m.group(1))
+                if module._diff:
+                    diff['before'] += "%s-%s\n" % (m.group(1), m.group(2))
+                    diff['after'] += "%s-%s\n" % (m.group(1), m.group(3))
         if module.check_mode:
-            module.exit_json(changed=True, msg="%s package(s) would be upgraded" % (len(data)), packages=packages, diff=diff)
+            if packages:
+                module.exit_json(changed=True, msg="%s package(s) would be upgraded" % (len(data)), packages=packages, diff=diff)
+            else:
+                module.exit_json(changed=False, msg='Nothing to upgrade', packages=packages)
         rc, stdout, stderr = module.run_command(cmdupgrade, check_rc=False)
         if rc == 0:
-            module.exit_json(changed=True, msg='System upgraded', packages=packages, diff=diff)
+            if packages:
+                module.exit_json(changed=True, msg='System upgraded', packages=packages, diff=diff)
+            else:
+                module.exit_json(changed=False, msg='Nothing to upgrade', packages=packages)
         else:
             module.fail_json(msg="Could not upgrade")
     else:

plugins/modules/packaging/os/snap.py

@@ -133,10 +133,10 @@ class Snap(CmdStateModuleHelper):
     module = dict(
         argument_spec={
             'name': dict(type='list', elements='str', required=True),
-            'state': dict(type='str', required=False, default='present',
+            'state': dict(type='str', default='present',
                           choices=['absent', 'present', 'enabled', 'disabled']),
-            'classic': dict(type='bool', required=False, default=False),
-            'channel': dict(type='str', required=False, default='stable'),
+            'classic': dict(type='bool', default=False),
+            'channel': dict(type='str', default='stable'),
         },
         supports_check_mode=True,
     )
@@ -205,7 +205,7 @@ class Snap(CmdStateModuleHelper):
         self.vars.snaps_installed = actionable_snaps
         if self.module.check_mode:
             return
-        params = ['classic', 'channel', 'state']  # get base cmd parts
+        params = ['state', 'classic', 'channel']  # get base cmd parts
         has_one_pkg_params = bool(self.vars.classic) or self.vars.channel != 'stable'
         has_multiple_snaps = len(actionable_snaps) > 1
         if has_one_pkg_params and has_multiple_snaps:

plugins/modules/system/lvol.py

@@ -471,9 +471,9 @@ def main():
                 if size_opt == 'l':
                     module.fail_json(changed=False, msg="Thin volume sizing with percentage not supported.")
                 size_opt = 'V'
-                cmd = "%s %s -n %s -%s %s%s %s -T %s/%s" % (lvcreate_cmd, yesopt, lv, size_opt, size, size_unit, opts, vg, thinpool)
+                cmd = "%s %s %s -n %s -%s %s%s %s -T %s/%s" % (lvcreate_cmd, test_opt, yesopt, lv, size_opt, size, size_unit, opts, vg, thinpool)
             elif thinpool and not lv:
-                cmd = "%s %s -%s %s%s %s -T %s/%s" % (lvcreate_cmd, yesopt, size_opt, size, size_unit, opts, vg, thinpool)
+                cmd = "%s %s %s -%s %s%s %s -T %s/%s" % (lvcreate_cmd, test_opt, yesopt, size_opt, size, size_unit, opts, vg, thinpool)
             else:
                 cmd = "%s %s %s -n %s -%s %s%s %s %s %s" % (lvcreate_cmd, test_opt, yesopt, lv, size_opt, size, size_unit, opts, vg, pvs)
             rc, dummy, err = module.run_command(cmd)

tests/integration/targets/archive/tasks/remove.yml

@@ -148,7 +148,39 @@
 - name: verify that excluded sub file is still present
   file: path={{ output_dir }}/tmpdir/sub/subfile.txt state=file
 
-- name: remove temporary directory
+- name: prep our files in tmpdir again
+  copy: src={{ item }} dest={{ output_dir }}/tmpdir/{{ item }}
+  with_items:
+    - foo.txt
+    - bar.txt
+    - empty.txt
+    - sub
+    - sub/subfile.txt
+
+- name: archive using gz and remove src directory
+  archive:
+    path:
+      - "{{ output_dir }}/tmpdir/"
+    dest: "{{ output_dir }}/archive_remove_05.gz"
+    format: gz
+    remove: yes
+    exclude_path: "{{ output_dir }}/tmpdir/sub/subfile.txt"
+  register: archive_remove_result_05
+
+- name: verify that the files archived
+  file: path={{ output_dir }}/archive_remove_05.gz state=file
+
+- name: Verify source files were removed
   file:
     path: "{{ output_dir }}/tmpdir"
     state: absent
+  register: archive_source_file_removal_05
+
+- name: Verify that task status is success
+  assert:
+    that:
+      - archive_remove_result_05 is success
+      - archive_source_file_removal_05 is not changed
+
+- name: remove our gz
+  file: path="{{ output_dir }}/archive_remove_05.gz" state=absent

tests/integration/targets/snap/tasks/default.yml

@@ -10,6 +10,12 @@
     name: snapd
     state: started
 
+- name: Create link /snap
+  file:
+    src: /var/lib/snapd/snap
+    dest: /snap
+    state: link
+
 - name: Inform that snap is installed
   set_fact:
     has_snap: true

tests/integration/targets/snap/tasks/main.yml

@@ -95,4 +95,51 @@
           - remove_check is changed
           - remove_again is not changed
           - remove_again_check is not changed
+
+    - name: Make sure package from classic snap is not installed
+      community.general.snap:
+        name: nvim
+        state: absent
+
+    - name: Install package from classic snap
+      community.general.snap:
+        name: nvim
+        state: present
+        classic: true
+      register: classic_install
+
+    # testing classic idempotency
+    - name: Install package from classic snap again
+      community.general.snap:
+        name: nvim
+        state: present
+        classic: true
+      register: classic_install_again
+
+    - name: Assert package has been installed just once
+      assert:
+        that:
+          - classic_install is changed
+          - classic_install_again is not changed
+
+    # this is just testing if a package which has been installed
+    # with true classic can be removed without setting classic to true
+    - name: Remove package from classic snap without setting classic to true
+      community.general.snap:
+        name: nvim
+        state: absent
+      register: classic_remove_without_true_classic
+
+    - name: Remove package from classic snap with setting classic to true
+      community.general.snap:
+        name: nvim
+        state: absent
+        classic: true
+      register: classic_remove_with_true_classic
+
+    - name: Assert package has been removed without setting classic to true
+      assert:
+        that:
+          - classic_remove_without_true_classic is changed
+          - classic_remove_with_true_classic is not changed
   when: has_snap

tests/unit/plugins/module_utils/test_module_helper.py

@@ -151,17 +151,45 @@ def test_vardict():
     assert vd.meta('a').diff is False
     assert vd.meta('a').change is False
     vd['b'] = 456
+    assert vd.meta('b').output is True
+    assert vd.meta('b').diff is False
+    assert vd.meta('b').change is False
     vd.set_meta('a', diff=True, change=True)
     vd.set_meta('b', diff=True, output=False)
     vd['c'] = 789
+    assert vd.has_changed('c') is False
     vd['a'] = 'new_a'
+    assert vd.has_changed('a') is True
     vd['c'] = 'new_c'
+    assert vd.has_changed('c') is False
+    vd['b'] = 'new_b'
+    assert vd.has_changed('b') is False
     assert vd.a == 'new_a'
     assert vd.c == 'new_c'
     assert vd.output() == {'a': 'new_a', 'c': 'new_c'}
     assert vd.diff() == {'before': {'a': 123}, 'after': {'a': 'new_a'}}, "diff={0}".format(vd.diff())
 
 
+def test_variable_meta_change():
+    vd = VarDict()
+    vd.set('a', 123, change=True)
+    vd.set('b', [4, 5, 6], change=True)
+    vd.set('c', {'m': 7, 'n': 8, 'o': 9}, change=True)
+    vd.set('d', {'a1': {'a11': 33, 'a12': 34}}, change=True)
+
+    vd.a = 1234
+    assert vd.has_changed('a') is True
+    vd.b.append(7)
+    assert vd.b == [4, 5, 6, 7]
+    assert vd.has_changed('b')
+    vd.c.update({'p': 10})
+    assert vd.c == {'m': 7, 'n': 8, 'o': 9, 'p': 10}
+    assert vd.has_changed('c')
+    vd.d['a1'].update({'a13': 35})
+    assert vd.d == {'a1': {'a11': 33, 'a12': 34, 'a13': 35}}
+    assert vd.has_changed('d')
+
+
 class MockMH(object):
     changed = None
 

tests/unit/plugins/modules/packaging/language/test_npm.py

@@ -52,6 +52,25 @@ class NPMModuleTestCase(ModuleTestCase):
             call(['/testbin/npm', 'install', '--global', 'coffee-script'], check_rc=True, cwd=None),
         ])
 
+    def test_present_missing(self):
+        set_module_args({
+            'name': 'coffee-script',
+            'global': 'true',
+            'state': 'present',
+        })
+        self.module_main_command.side_effect = [
+            (0, '{"dependencies": {"coffee-script": {"missing" : true}}}', ''),
+            (0, '{}', ''),
+        ]
+
+        result = self.module_main(AnsibleExitJson)
+
+        self.assertTrue(result['changed'])
+        self.module_main_command.assert_has_calls([
+            call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None),
+            call(['/testbin/npm', 'install', '--global', 'coffee-script'], check_rc=True, cwd=None),
+        ])
+
     def test_present_version(self):
         set_module_args({
             'name': 'coffee-script',