pulp_distribution: fix distribution promotion

Promotion of one distribution to the publication of another distribution
currently fails. We see:

    'str object' has no attribute 'pulp_href'

This change fixes the issue by refactoring the complex jinja logic into
ansible filters.

Also fixes an issue when a distribution has a state of absent - we
should not need to specify a repository or base_path.

Fixes: #34
Fixes: #35

Author: markgoddard

plugins/filter/filters.py

@@ -12,6 +12,9 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from ansible import errors
+
+
 def sort_publications(pubs):
     """Sort a list of publications by repository version.
 
@@ -25,9 +28,71 @@ def key(pub):
     return sorted(pubs, key=key, reverse=True)
 
 
+def find_publication_for_distribution(dist, repos, pubs, dists):
+    """Find a publication to distribute.
+
+    The distribution object, `dist`, may include different fields to control
+    the search:
+
+    * repository: use latest version
+    * repository & version: use a specific version
+    * distribution: use the same publication as another distribution
+
+    :param dist: Distribution to find a publication for.
+    :param repos: Existing repos in Pulp.
+    :param pubs: Existing publications in Pulp.
+    :param dists: Existing distributions in Pulp.
+    """
+    if dist.get("state") == "absent":
+        return
+
+    if "repository" in dist:
+        if "distribution" in dist:
+            raise errors.AnsibleFilterError("Cannot specify 'distribution' with 'repository'")
+        repository = dist["repository"]
+        repository_href = [repo for repo in repos if repo["name"] == repository][0]["pulp_href"]
+        if "version" in dist:
+            version = dist["version"]
+            version_href = "%sversions/%d/" % (repository_href, version)
+        else:
+            version_href = None
+
+        for pub in pubs:
+            if pub["repository"] == repository_href:
+                if not version_href or version_href == pub["repository_version"]:
+                    return pub
+    elif "distribution" in dist:
+        if "version" in dist:
+            raise errors.AnsibleFilterError("Cannot specify 'version' with 'distribution'")
+        other_dist = dist["distribution"]
+        pub_href = [d for d in dists if d["name"] == other_dist][0]["publication"]
+        for pub in pubs:
+            if pub["pulp_href"] == pub_href:
+                return pub
+    raise errors.AnsibleFilterError("Could not find a matching publication")
+
+
+def publication_has_distributions(pub, pubs, dists):
+    """Return whether a publication has any distributions.
+
+    :param pub: Publication to find a distribution for.
+    :param pubs: Existing publications in Pulp.
+    :param dists: Existing distributions in Pulp.
+    """
+    if not pub:
+        return False
+    pub_href = pub["pulp_href"]
+    for dist in dists:
+        if dist["publication"] == pub_href:
+            return True
+    return False
+
+
 class FilterModule(object):
 
     def filters(self):
         return {
-            "sort_publications": sort_publications
+            "sort_publications": sort_publications,
+            "find_publication_for_distribution": find_publication_for_distribution,
+            "publication_has_distributions": publication_has_distributions,
         }

roles/pulp_distribution/tasks/deb.yml

@@ -36,28 +36,22 @@
 
 - name: Ensure Deb distributions are defined
   vars:
-    repo: "{{ pulp_repos_list.repositories | selectattr('name', 'equalto', item.repository) | first }}"
     all_pubs_list: "{{ pulp_pubs_list.publications + pulp_verbatim_pubs_list.publications }}"
-    # If the distribution references a specific version:
-    specific_pub: "{{ all_pubs_list | selectattr('repository_version', 'equalto', repo.pulp_href ~ 'versions/' ~ item.version | default ~ '/') }}"
-    # If the distribution uses the latest version:
-    latest_pub: "{{ all_pubs_list | selectattr('repository', 'equalto', repo.pulp_href) | stackhpc.pulp.sort_publications }}"
-    # If another distribution is being promoted to this one:
-    promoted_dist: "{{ pulp_dists_list.distributions | selectattr('name', 'equalto', item.distribution | default) }}"
-    promoted_pub: "{{ all_pubs_list | selectattr('pulp_href', 'equalto', (promoted_dist | first).publication | default) }}"
-    # Pick the right publication based on the type of distribution.
-    pubs: "{{ specific_pub if item.version is defined else promoted_pub if item.distribution is defined else latest_pub }}"
-    # Whether any distributions exist for this publication.
+    publication: >-
+      {{ item | stackhpc.pulp.find_publication_for_distribution(pulp_repos_list.repositories,
+                                                                all_pubs_list,
+                                                                pulp_dists_list.distributions) }}
     pub_has_dists: >-
-      {{ pulp_dists_list.distributions | selectattr('publication', 'equalto', pubs[0].pulp_href if item.state == 'present' else None) | list | length > 0 }}
+      {{ publication | stackhpc.pulp.publication_has_distributions(all_pubs_list,
+                                                                   pulp_dists_list.distributions) }}
   pulp.squeezer.deb_distribution:
     pulp_url: "{{ pulp_url }}"
     username: "{{ pulp_username }}"
     password: "{{ pulp_password }}"
     validate_certs: "{{ pulp_validate_certs | bool }}"
     name: "{{ item.name }}"
-    base_path: "{{ item.base_path }}"
-    publication: "{{ pubs[0].pulp_href if item.state == 'present' else omit }}"
+    base_path: "{{ item.base_path | default(omit) }}"
+    publication: "{{ publication.pulp_href if item.state == 'present' else omit }}"
     content_guard: "{{ item.content_guard | default(omit) }}"
     state: "{{ item.state }}"
   with_items: "{{ pulp_distribution_deb }}"

roles/pulp_distribution/tasks/rpm.yml

@@ -25,27 +25,21 @@
 
 - name: Ensure RPM distributions are defined
   vars:
-    repo: "{{ pulp_repos_list.repositories | selectattr('name', 'equalto', item.repository) | first }}"
-    # If the distribution references a specific version:
-    specific_pub: "{{ pulp_pubs_list.publications | selectattr('repository_version', 'equalto', repo.pulp_href ~ 'versions/' ~ item.version | default ~ '/') }}"
-    # If the distribution uses the latest version:
-    latest_pub: "{{ pulp_pubs_list.publications | selectattr('repository', 'equalto', repo.pulp_href) | stackhpc.pulp.sort_publications }}"
-    # If another distribution is being promoted to this one:
-    promoted_dist: "{{ pulp_dists_list.distributions | selectattr('name', 'equalto', item.distribution | default) }}"
-    promoted_pub: "{{ pulp_pubs_list.publications | selectattr('pulp_href', 'equalto', (promoted_dist | first).publication | default) }}"
-    # Pick the right publication based on the type of distribution.
-    pubs: "{{ specific_pub if item.version is defined else promoted_pub if item.distribution is defined else latest_pub }}"
-    # Whether any distributions exist for this publication.
+    publication: >-
+      {{ item | stackhpc.pulp.find_publication_for_distribution(pulp_repos_list.repositories,
+                                                                pulp_pubs_list.publications,
+                                                                pulp_dists_list.distributions) }}
     pub_has_dists: >-
-      {{ pulp_dists_list.distributions | selectattr('publication', 'equalto', pubs[0].pulp_href if item.state == 'present' else None) | list | length > 0 }}
+      {{ publication | stackhpc.pulp.publication_has_distributions(pulp_pubs_list.publications,
+                                                                   pulp_dists_list.distributions) }}
   pulp.squeezer.rpm_distribution:
     pulp_url: "{{ pulp_url }}"
     username: "{{ pulp_username }}"
     password: "{{ pulp_password }}"
     validate_certs: "{{ pulp_validate_certs | bool }}"
     name: "{{ item.name }}"
-    base_path: "{{ item.base_path }}"
-    publication: "{{ pubs[0].pulp_href if item.state == 'present' else omit }}"
+    base_path: "{{ item.base_path | default(omit) }}"
+    publication: "{{ publication.pulp_href if item.state == 'present' else omit }}"
     content_guard: "{{ item.content_guard | default(omit) }}"
     state: "{{ item.state }}"
   with_items: "{{ pulp_distribution_rpm }}"

tests/test_deb_distribution.yml

@@ -39,16 +39,14 @@
             version: 1
             state: present
 
-    # FIXME: This currently fails due to
-    # https://github.com/stackhpc/ansible-collection-pulp/issues/34.
-    # - include_role:
-    #     name: pulp_distribution
-    #   vars:
-    #     pulp_distribution_deb:
-    #       - name: test_deb_distribution_distribution
-    #         base_path: test_deb_distribution_distribution
-    #         distribution: test_deb_distribution
-    #         state: present
+    - include_role:
+        name: pulp_distribution
+      vars:
+        pulp_distribution_deb:
+          - name: test_deb_distribution_distribution
+            base_path: test_deb_distribution_distribution
+            distribution: test_deb_distribution
+            state: present
 
     - name: Query repository
       pulp.squeezer.deb_repository:
@@ -87,16 +85,14 @@
         name: test_deb_distribution_version_1
       register: dist_version_1_result
 
-    # FIXME: This currently fails due to
-    # https://github.com/stackhpc/ansible-collection-pulp/issues/34.
-    # - name: Query distribution distribution
-    #   pulp.squeezer.deb_distribution:
-    #     pulp_url: "{{ pulp_url }}"
-    #     username: "{{ pulp_username }}"
-    #     password: "{{ pulp_password }}"
-    #     validate_certs: "{{ pulp_validate_certs }}"
-    #     name: test_deb_distribution_distribution
-    #   register: dist_distribution_result
+    - name: Query distribution distribution
+      pulp.squeezer.deb_distribution:
+        pulp_url: "{{ pulp_url }}"
+        username: "{{ pulp_username }}"
+        password: "{{ pulp_password }}"
+        validate_certs: "{{ pulp_validate_certs }}"
+        name: test_deb_distribution_distribution
+      register: dist_distribution_result
 
     - name: Verify publication creation
       assert:
@@ -117,33 +113,22 @@
           - dist_version_1_result.distribution.base_path == "test_deb_distribution_version_1"
           - dist_version_1_result.distribution.publication == pub_result.publication.pulp_href
 
-    # FIXME: This currently fails due to
-    # https://github.com/stackhpc/ansible-collection-pulp/issues/34.
-    # - name: Verify distribution creation
-    #   assert:
-    #     that:
-    #       - dist_distribution_result.distribution.name == "test_deb_distribution_distribution"
-    #       - dist_distribution_result.distribution.base_path == "test_deb_distribution_distribution"
-    #       - dist_distribution_result.distribution.publication == pub_result.publication.pulp_href
+    - name: Verify distribution creation
+      assert:
+        that:
+          - dist_distribution_result.distribution.name == "test_deb_distribution_distribution"
+          - dist_distribution_result.distribution.base_path == "test_deb_distribution_distribution"
+          - dist_distribution_result.distribution.publication == pub_result.publication.pulp_href
 
     - include_role:
         name: pulp_distribution
       vars:
         pulp_distribution_deb:
           - name: test_deb_distribution
-            # FIXME: These should not be required.
-            repository: test_deb_repo
-            base_path: foo
             state: absent
           - name: test_deb_distribution_version_1
-            # FIXME: These should not be required.
-            repository: test_deb_repo
-            base_path: foo
             state: absent
           - name: test_deb_distribution_distribution
-            # FIXME: These should not be required.
-            repository: test_deb_repo
-            base_path: foo
             state: absent
 
     - include_role:

tests/test_rpm_distribution.yml

@@ -38,16 +38,14 @@
             version: 1
             state: present
 
-    # FIXME: This currently fails due to
-    # https://github.com/stackhpc/ansible-collection-pulp/issues/34.
-    # - include_role:
-    #     name: pulp_distribution
-    #   vars:
-    #     pulp_distribution_rpm:
-    #       - name: test_rpm_distribution_distribution
-    #         base_path: test_rpm_distribution_distribution
-    #         distribution: test_rpm_distribution
-    #         state: present
+    - include_role:
+        name: pulp_distribution
+      vars:
+        pulp_distribution_rpm:
+          - name: test_rpm_distribution_distribution
+            base_path: test_rpm_distribution_distribution
+            distribution: test_rpm_distribution
+            state: present
 
     - name: Query repository
       pulp.squeezer.rpm_repository:
@@ -86,16 +84,14 @@
         name: test_rpm_distribution_version_1
       register: dist_version_1_result
 
-    # FIXME: This currently fails due to
-    # https://github.com/stackhpc/ansible-collection-pulp/issues/34.
-    # - name: Query distribution distribution
-    #   pulp.squeezer.rpm_distribution:
-    #     pulp_url: "{{ pulp_url }}"
-    #     username: "{{ pulp_username }}"
-    #     password: "{{ pulp_password }}"
-    #     validate_certs: "{{ pulp_validate_certs }}"
-    #     name: test_rpm_distribution_distribution
-    #   register: dist_distribution_result
+    - name: Query distribution distribution
+      pulp.squeezer.rpm_distribution:
+        pulp_url: "{{ pulp_url }}"
+        username: "{{ pulp_username }}"
+        password: "{{ pulp_password }}"
+        validate_certs: "{{ pulp_validate_certs }}"
+        name: test_rpm_distribution_distribution
+      register: dist_distribution_result
 
     - name: Verify publication creation
       assert:
@@ -116,33 +112,22 @@
           - dist_version_1_result.distribution.base_path == "test_rpm_distribution_version_1"
           - dist_version_1_result.distribution.publication == pub_result.publication.pulp_href
 
-    # FIXME: This currently fails due to
-    # https://github.com/stackhpc/ansible-collection-pulp/issues/34.
-    # - name: Verify distribution creation
-    #   assert:
-    #     that:
-    #       - dist_distribution_result.distribution.name == "test_rpm_distribution_distribution"
-    #       - dist_distribution_result.distribution.base_path == "test_rpm_distribution_distribution"
-    #       - dist_distribution_result.distribution.publication == pub_result.publication.pulp_href
+    - name: Verify distribution creation
+      assert:
+        that:
+          - dist_distribution_result.distribution.name == "test_rpm_distribution_distribution"
+          - dist_distribution_result.distribution.base_path == "test_rpm_distribution_distribution"
+          - dist_distribution_result.distribution.publication == pub_result.publication.pulp_href
 
     - include_role:
         name: pulp_distribution
       vars:
         pulp_distribution_rpm:
           - name: test_rpm_distribution
-            # FIXME: These should not be required.
-            repository: test_rpm_repo
-            base_path: foo
             state: absent
           - name: test_rpm_distribution_version_1
-            # FIXME: These should not be required.
-            repository: test_rpm_repo
-            base_path: foo
             state: absent
           - name: test_rpm_distribution_distribution
-            # FIXME: These should not be required.
-            repository: test_rpm_repo
-            base_path: foo
             state: absent
 
     - include_role: