Merge pull request #31 from RedHatSatellite/rel212

release 2.1.2
RedHatSatellite · Jul 21, 2021 · f2a0ce0 · f2a0ce0
2 parents c02ca44 + 869a3cd
commit f2a0ce0
Show file tree

Hide file tree

Showing 73 changed files with 4,633 additions and 4,502 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -6,6 +6,18 @@ redhat.satellite Release Notes
 
 This changelog describes changes after version 0.8.1.
 
+v2.1.2
+======
+
+Bugfixes
+--------
+
+- activation_key - submit organization_id when querying subs, required for Katello 4.1
+- content_view_version_cleanup - sort content view versions before deleting (https://github.com/RedHatSatellite/satellite-ansible-collection/issues/30, https://bugzilla.redhat.com/show_bug.cgi?id=1980274)
+- content_view_version_cleanup role - properly clean up when users set keep=0 (https://bugzilla.redhat.com/show_bug.cgi?id=1974314)
+- host, compute_profile - when resolving cluster and other values in vm_attrs, compare them as strings (https://github.com/theforeman/foreman-ansible-modules/issues/1245)
+- subscription_info - mark ``organization`` parameter as required, to match Katello
+
 v2.1.1
 ======
 

diff --git a/Makefile b/Makefile
@@ -14,7 +14,7 @@ COLLECTION_COMMAND ?= ansible-galaxy
 SANITY_OPTS = --venv
 TEST =
 FLAGS =
-PYTEST = pytest -n 4 --boxed -v
+PYTEST = pytest -n 4 --boxed -vv
 
 APIPIE_VERSION ?= v0.3.2
 
@@ -63,17 +63,17 @@ test-other:
 	$(PYTEST) -k 'not test_crud.py'
 
 livetest: $(MANIFEST) | tests/test_playbooks/vars/server.yml
-	pytest -v 'tests/test_crud.py::test_crud' --vcrmode live
+	pytest -vv 'tests/test_crud.py::test_crud' --vcrmode live
 
 test_%: FORCE $(MANIFEST) | tests/test_playbooks/vars/server.yml
-	pytest -v 'tests/test_crud.py::test_crud[$*]' 'tests/test_crud.py::test_check_mode[$*]' $(FLAGS)
+	pytest -vv 'tests/test_crud.py::test_crud[$*]' 'tests/test_crud.py::test_check_mode[$*]' $(FLAGS)
 
 livetest_%: FORCE $(MANIFEST) | tests/test_playbooks/vars/server.yml
-	pytest -v 'tests/test_crud.py::test_crud[$*]' --vcrmode live $(FLAGS)
+	pytest -vv 'tests/test_crud.py::test_crud[$*]' --vcrmode live $(FLAGS)
 
 record_%: FORCE $(MANIFEST)
 	$(RM) tests/test_playbooks/fixtures/$*-*.yml
-	pytest -v 'tests/test_crud.py::test_crud[$*]' --vcrmode record $(FLAGS)
+	pytest -vv 'tests/test_crud.py::test_crud[$*]' --vcrmode record $(FLAGS)
 
 clean_%: FORCE $(MANIFEST)
 	ansible-playbook --tags teardown,cleanup -i tests/inventory/hosts 'tests/test_playbooks/$*.yml'

diff --git a/changelogs/changelog.yaml b/changelogs/changelog.yaml
@@ -470,3 +470,23 @@ releases:
     - bz1967649-usergroup_lookup.yml
     - bz1967904-dont_override_params.yml
     release_date: '2021-06-22'
+  2.1.2:
+    changes:
+      bugfixes:
+      - activation_key - submit organization_id when querying subs, required for Katello
+        4.1
+      - content_view_version_cleanup - sort content view versions before deleting
+        (https://github.com/RedHatSatellite/satellite-ansible-collection/issues/30,
+        https://bugzilla.redhat.com/show_bug.cgi?id=1980274)
+      - content_view_version_cleanup role - properly clean up when users set keep=0
+        (https://bugzilla.redhat.com/show_bug.cgi?id=1974314)
+      - host, compute_profile - when resolving cluster and other values in vm_attrs,
+        compare them as strings (https://github.com/theforeman/foreman-ansible-modules/issues/1245)
+      - subscription_info - mark ``organization`` parameter as required, to match
+        Katello
+    fragments:
+    - 1245-search_vm_attr_as_string.yml
+    - BZ1974314-cv_cleanup_keep_0.yml
+    - bz1980274.yml
+    - katello41-subscription-org-required.yml
+    release_date: '2021-07-13'
diff --git a/docs/testing.md b/docs/testing.md
@@ -65,6 +65,18 @@ To re-record, execute:
 $ make record_global_parameter
 ```
 
+Callback tests are part of integration tests, to run just callback tests, execute:
+
+```console
+$ make test TEST="tests/test_callback.py"
+```
+
+To re-record just callback JSON fixtures, execute:
+
+```console
+$ make test TEST="tests/test_callback.py --vcrmode record"
+```
+
 ### Ansible Sanity
 
 To run the Ansible Sanity tests, execute:

diff --git a/galaxy.yml b/galaxy.yml
@@ -32,6 +32,7 @@ authors:
   - "Josh Swanson <[email protected]>"
   - "Kirill Shirinkin <[email protected]>"
   - "Lester Claudio <[email protected]>"
+  - "Lukáš Zapletal <[email protected]>"
   - "Manisha Singhal <[email protected]>"
   - "Manuel Bonk <[email protected]>"
   - "Marcelo Moreira de Mello <[email protected]>"
@@ -66,7 +67,7 @@ authors:
   - "metalcated <[email protected]>"
   - "russianguppie <[email protected]>"
   - "willtome <[email protected]>"
-version: "2.1.1"
+version: "2.1.2"
 license:
   - "GPL-3.0-or-later"
 tags:

diff --git a/plugins/callback/foreman.py b/plugins/callback/foreman.py
@@ -66,6 +66,20 @@
         ini:
           - section: callback_foreman
             key: verify_certs
+      dir_store:
+        description:
+          - When set, callback does not perform HTTP calls but stores results in a given directory.
+          - For each report, new file in the form of SEQ_NO-hostname.json is created.
+          - For each facts, new file in the form of SEQ_NO-hostname.json is created.
+          - The value must be a valid directory.
+          - This is meant for debugging and testing purposes.
+          - When set to blank (default) this functionality is turned off.
+        env:
+          - name: FOREMAN_DIR_STORE
+        default: ''
+        ini:
+          - section: callback_foreman
+            key: dir_store
       disable_callback:
         description:
           - Toggle to make the callback plugin disable itself even if it is loaded.
@@ -111,7 +125,7 @@ def build_log(data):
                     'source': source,
                 },
                 'messages': {
-                    'message': json.dumps(msg),
+                    'message': json.dumps(msg, sort_keys=True),
                 },
                 'level': level,
             }
@@ -158,6 +172,7 @@ def set_options(self, task_keys=None, var_options=None, direct=None):
         self.foreman_url = self.get_option('url')
         ssl_cert = self.get_option('client_cert')
         ssl_key = self.get_option('client_key')
+        self.dir_store = self.get_option('dir_store')
 
         if not HAS_REQUESTS:
             self._disable_plugin(u'The `requests` python module is not installed')
@@ -195,13 +210,33 @@ def _ssl_verify(self, option):
 
         return verify
 
+    def _send_data(self, endpoint, host, data):
+        if endpoint == 'facts':
+            url = self.foreman_url + '/api/v2/hosts/facts'
+        elif endpoint == 'report':
+            url = self.foreman_url + '/api/v2/config_reports'
+        else:
+            self._display.warning(u'Unknown endpoint type: {type}'.format(type=endpoint))
+
+        if len(self.dir_store) > 0:
+            filename = u'{host}.json'.format(host=to_text(host))
+            filename = os.path.join(self.dir_store, filename)
+            with open(filename, 'w') as f:
+                json.dump(data, f, indent=2, sort_keys=True)
+        else:
+            try:
+                response = self.session.post(url=url, json=data)
+                response.raise_for_status()
+            except requests.exceptions.RequestException as err:
+                self._display.warning(u'Sending data to Foreman at {url} failed for {host}: {err}'.format(
+                    host=to_text(host), err=to_text(err), url=to_text(self.foreman_url)))
+
     def send_facts(self):
         """
         Sends facts to Foreman, to be parsed by foreman_ansible fact
         parser.  The default fact importer should import these facts
         properly.
         """
-        url = self.foreman_url + '/api/v2/hosts/facts'
 
         for host, facts in self.facts.items():
             facts = {
@@ -213,21 +248,14 @@ def send_facts(self):
                 },
             }
 
-            try:
-                response = self.session.post(url=url, json=facts)
-                response.raise_for_status()
-            except requests.exceptions.RequestException as err:
-                self._display.warning(u'Sending facts to Foreman at {url} failed for {host}: {err}'.format(
-                    host=to_text(host), err=to_text(err), url=to_text(self.foreman_url)))
+            self._send_data('facts', host, facts)
 
     def send_reports(self, stats):
         """
         Send reports to Foreman to be parsed by its config report
         importer. THe data is in a format that Foreman can handle
         without writing another report importer.
         """
-        url = self.foreman_url + '/api/v2/config_reports'
-
         for host in stats.processed.keys():
             total = stats.summarize(host)
             report = {
@@ -252,12 +280,8 @@ def send_reports(self, stats):
             if self.check_mode:
                 report['config_report']['status']['pending'] = total['changed']
                 report['config_report']['status']['applied'] = 0
-            try:
-                response = self.session.post(url=url, json=report)
-                response.raise_for_status()
-            except requests.exceptions.RequestException as err:
-                self._display.warning(u'Sending report to Foreman at {url} failed for {host}: {err}'.format(
-                    host=to_text(host), err=to_text(err), url=to_text(self.foreman_url)))
+
+            self._send_data('report', host, report)
 
             self.items[host] = []
 

diff --git a/plugins/module_utils/foreman_helper.py b/plugins/module_utils/foreman_helper.py
@@ -835,7 +835,7 @@ def find_compute_resource_parts(self, part_name, name, compute_resource, cluster
         api_name = 'available_{0}'.format(part_name)
         available_parts = self.resource_action('compute_resources', api_name, params=additional_params,
                                                ignore_check_mode=True, record_change=False)['results']
-        part = next((part for part in available_parts if part['name'] == name or part['id'] == name), None)
+        part = next((part for part in available_parts if str(part['name']) == str(name) or str(part['id']) == str(name)), None)
         if part is None:
             err_msg = "Could not find {0} '{1}' on compute resource '{2}'.".format(part_name, name, compute_resource.get('name'))
             self.fail_json(msg=err_msg)

diff --git a/plugins/modules/activation_key.py b/plugins/modules/activation_key.py
@@ -282,6 +282,7 @@ def main():
                 activation_key = module.ensure_entity('activation_keys', module.foreman_params, activation_key, params=scope)
 
             ak_scope = {'activation_key_id': activation_key['id']}
+            ak_scope.update(scope)
             if subscriptions is not None:
                 desired_subscriptions = []
                 for subscription in subscriptions:

diff --git a/plugins/modules/subscription_info.py b/plugins/modules/subscription_info.py
@@ -30,6 +30,7 @@
   - "Evgeni Golov (@evgeni)"
 extends_documentation_fragment:
   - redhat.satellite.foreman
+  - redhat.satellite.foreman.katelloinfomodule
   - redhat.satellite.foreman.infomodule
 '''
 
@@ -62,16 +63,16 @@
 '''
 
 from ansible_collections.redhat.satellite.plugins.module_utils.foreman_helper import (
-    ForemanInfoAnsibleModule
+    KatelloInfoAnsibleModule
 )
 
 
-class ForemanSubscriptionInfo(ForemanInfoAnsibleModule):
+class KatelloSubscriptionInfo(KatelloInfoAnsibleModule):
     pass
 
 
 def main():
-    module = ForemanSubscriptionInfo()
+    module = KatelloSubscriptionInfo()
 
     with module.api_connection():
         module.run()

diff --git a/requirements-dev.txt b/requirements-dev.txt
@@ -1,11 +1,15 @@
 pytest
 pytest-xdist
+pytest-clarity; python_version >= '3.6'
 vcrpy
-ansible_runner
-python-debian
+ansible_runner<2.0; python_version < '3.6'
+ansible_runner; python_version >= '3.6'
+python-debian<0.1.40; python_version < '3.6'
+python-debian; python_version >= '3.6'
 rpm-py-installer
 rstcheck
 docker
+ruamel.yaml.clib<0.2.3; python_version < '3.6'
 cryptography<3.1; python_version < '3.6'
 -r requirements-lint.txt
 -r requirements.txt

diff --git a/roles/content_view_version_cleanup/tasks/find_and_delete_unused_cv_versions.yml b/roles/content_view_version_cleanup/tasks/find_and_delete_unused_cv_versions.yml
@@ -17,4 +17,4 @@
     cv_name: "{{ cv.name }}"
     cv_versions: "{{ (versions.resources | rejectattr('environments') | rejectattr('composite_content_view_ids') |
       rejectattr('published_in_composite_content_view_ids') | map(attribute='version') | map('float') | sort |
-      map('string') | list )[:-satellite_content_view_version_cleanup_keep] }}"
+      map('string') | reverse | list )[satellite_content_view_version_cleanup_keep:] }}"
diff --git a/roles/content_view_version_cleanup/tasks/main.yml b/roles/content_view_version_cleanup/tasks/main.yml
@@ -1,4 +1,10 @@
 ---
+- name: "verify satellite_content_view_version_cleanup_keep is set"
+  assert:
+    that:
+      - satellite_content_view_version_cleanup_keep|int >= 0
+    fail_msg: "satellite_content_view_version_cleanup_keep needs to be set to >= 0"
+
 - name: "find all content views"
   redhat.satellite.resource_info:
     server_url: "{{ satellite_server_url | default(omit) }}"
@@ -14,17 +20,17 @@
   include_tasks: delete_cv_versions.yml
   vars:
     cv_name: "{{ ccv.name }}"
-    cv_versions: "{{ (ccv.versions | rejectattr('environment_ids') | map(attribute='version') | list)[:-satellite_content_view_version_cleanup_keep] }}"
+    cv_versions: "{{ (ccv.versions | rejectattr('environment_ids') | map(attribute='version') | map('float') | sort | map('string') | reverse | list)[satellite_content_view_version_cleanup_keep:] }}"
   loop: "{{ all_cvs.resources | selectattr('composite') | list }}"
   loop_control:
     label: "{{ ccv.label }}"
     loop_var: "ccv"
-  when: (ccv.versions | rejectattr('environment_ids') | map(attribute='version') | list)[:-satellite_content_view_version_cleanup_keep]
+  when: (ccv.versions | rejectattr('environment_ids') | map(attribute='version') | reverse | list)[satellite_content_view_version_cleanup_keep:]
 
 - name: "find and delete unused content view versions"
   include_tasks: find_and_delete_unused_cv_versions.yml
   loop: "{{ all_cvs.resources | rejectattr('composite') | list }}"
   loop_control:
     label: "{{ cv.label }}"
     loop_var: "cv"
-  when: (cv.versions | rejectattr('environment_ids') | map(attribute='version') | list)[:-satellite_content_view_version_cleanup_keep]
+  when: (cv.versions | rejectattr('environment_ids') | map(attribute='version') | reverse | list)[satellite_content_view_version_cleanup_keep:]
diff --git a/tests/callback/three_hosts b/tests/callback/three_hosts
@@ -0,0 +1,4 @@
+[local]
+testhost ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}"
+testhostA ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}"
+testhostB ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}"
diff --git a/tests/callback/three_hosts.yml b/tests/callback/three_hosts.yml
@@ -0,0 +1,71 @@
+---
+- hosts: testhost
+  gather_facts: no
+  vars:
+    foo: foo bar
+  tasks:
+    - name: Changed task
+      command: echo foo
+      changed_when: true
+      notify: test handlers
+
+    - name: Ok task
+      command: echo foo
+      changed_when: false
+
+    - name: Failed task
+      fail:
+        msg: no reason
+      ignore_errors: yes
+
+    - name: Skipped task
+      command: echo foo
+      when: false
+
+    - name: Task with var in name ({{ foo }})
+      command: echo foo
+
+    - name: Loop task
+      command: echo foo
+      loop:
+        - 1
+        - 2
+        - 3
+      loop_control:
+        label: foo-{{ item }}
+
+  handlers:
+    - name: Test handler 1
+      command: echo foo
+      listen: test handlers
+
+    - name: Test handler 2
+      command: echo foo
+      changed_when: false
+      listen: test handlers
+
+    - name: Test handler 3
+      command: echo foo
+      listen: test handlers
+
+- hosts:
+  - testhostA
+  - testhostB
+  gather_facts: no
+  tasks:
+    - name: Changed task
+      command: echo foo
+      changed_when: true
+
+    - name: Ok task
+      command: echo foo
+      changed_when: false
+
+    - name: Failed task
+      fail:
+        msg: no reason
+      ignore_errors: yes
+
+    - name: Skipped task
+      command: echo foo
+      when: false