Blob Blame History Raw
From c6ab794eeffd728b6f20ac0aaeecbe393c9b100d Mon Sep 17 00:00:00 2001
From: "Owen W. Taylor" <otaylor@fishsoup.net>
Date: Wed, 29 Aug 2018 10:43:26 -0400
Subject: [PATCH] Support module specifications contain the context field

Add support for the context field of a module specification
NAME:STREAM:VERSION:CONTEXT - this allows for handling the case where a
module NAME:STREAM:VERSION is built multiple times for different sets
of dependencies.

Remove support for the old format 'NAME-STREAM-VERSION' - all tooling has
since been adapted.
---
 atomic_reactor/util.py | 81 ++++++++++++++++++++++--------------------
 tests/test_util.py     | 15 ++++----
 2 files changed, 51 insertions(+), 45 deletions(-)

diff --git a/atomic_reactor/util.py b/atomic_reactor/util.py
index 9287428..491fbe4 100644
--- a/atomic_reactor/util.py
+++ b/atomic_reactor/util.py
@@ -1351,16 +1351,19 @@ def get_parent_image_koji_data(workflow):
 
 
 class ModuleSpec(object):
-    def __init__(self, name, stream, version=None, profile=None):
+    def __init__(self, name, stream, version=None, context=None, profile=None):
         self.name = name
         self.stream = stream
         self.version = version
+        self.context = context
         self.profile = profile
 
     def to_str(self, include_profile=True):
         result = self.name + ':' + self.stream
         if self.version:
             result += ':' + self.version
+        if self.context:
+            result += ':' + self.context
         if include_profile and self.profile:
             result += '/' + self.profile
 
@@ -1374,45 +1377,47 @@ class ModuleSpec(object):
 
 
 def split_module_spec(module):
-    # Current module naming guidelines are at:
-    # https://docs.pagure.org/modularity/development/building-modules/naming-policy.html
-    # We simplify the possible NAME:STREAM:CONTEXT:ARCH/PROFILE and only care about
-    # NAME:STREAM or NAME:STREAM:VERSION with optional PROFILE. ARCH is determined by
-    # the architecture. CONTEXT may become important in the future, but we ignore it
-    # for now.
-    #
-    # Previously the separator was '-' instead of ':', which required hardcoding the
-    # format of VERSION to distinguish between HYPHENATED-NAME-STREAM and NAME-STREAM-VERSION.
-    # We support the old format for compatibility.
-    #
-    PATTERNS = [
-        (r'^([^:/]+):([^:/]+):([^:/]+)(?:/([^:/]+))?$', 3, 4),
-        (r'^([^:/]+):([^:/]+)(?:/([^:/]+))?$', None, 3),
-        (r'^(.+)-([^-]+)-(\d{14})$', 3, None),
-        (r'^(.+)-([^-]+)$', None, None)
-    ]
-
-    for pat, version_index, profile_index in PATTERNS:
-        m = re.match(pat, module)
-        if m:
-            name = m.group(1)
-            stream = m.group(2)
-            version = None
-            if version_index is not None:
-                version = m.group(version_index)
-            else:
-                version = None
-            if profile_index is not None:
-                profile = m.group(profile_index)
-            else:
-                profile = None
+    # We simplify the possible NAME:STREAM:VERSION:CONTEXT:ARCH/PROFILE by:
+    #  * Not supporting ARCH - this is determined by the architecture of the build
+    #  * Not supporting partial specifications like NAME:::CONTEXT
 
-            return ModuleSpec(name, stream, version, profile)
+    name = None
+    stream = None
+    version = None
+    context = None
+    profile = None
 
-    raise RuntimeError(
-        'Module specification should be NAME:STREAM[/PROFILE] or NAME:STREAM:VERSION[/PROFILE]. ' +
-        '(NAME-STREAM and NAME-STREAM-VERSION supported for compatibility.)'
-    )
+    if '/' in module:
+        module, profile = module.rsplit('/', 1)
+
+    parts = module.split(":")
+
+    if len(parts) < 2:
+        raise RuntimeError(
+            "Module specification {} must include at least NAME:STREAM"
+            .format(module))
+
+    if len(parts) > 4:
+        raise RuntimeError(
+            "Module specifiction {} should be NAME:STREAM[:VERSION[:CONTEXT]][/PROFILE]"
+            .format(module))
+
+    name = parts[0]
+    stream = parts[1]
+
+    if len(parts) >= 3:
+        version = parts[2]
+
+    if len(parts) >= 4:
+        context = parts[3]
+
+    def empty(x):
+        return x is not None and len(x) == 0
+
+    if empty(name) or empty(stream) or empty(version) or empty(context) or empty(profile):
+        raise RuntimeError("Module specification {} contains empty components".format(module))
+
+    return ModuleSpec(name, stream, version, context, profile)
 
 
 class OSBSLogs(object):
diff --git a/tests/test_util.py b/tests/test_util.py
index e5bf562..a4e4ff8 100644
--- a/tests/test_util.py
+++ b/tests/test_util.py
@@ -1019,22 +1019,23 @@ def test_clone_git_repo_retry(tmpdir, retry_times, raise_exc):
 
 
 @pytest.mark.parametrize(('module', 'should_raise', 'expected'), [
-    ('eog', True, None),
+    ('eog', "must include at least NAME:STREAM", None),
     ('eog:f26', False, ModuleSpec('eog', 'f26')),
     ('eog:f26/default', False, ModuleSpec('eog', 'f26', profile='default')),
-    ('eog-f26', False, ModuleSpec('eog', 'f26')),
     ('eog:f26:20170629213428', False, ModuleSpec('eog', 'f26', '20170629213428')),
+    ('eog:f26:20170629213428:12345678', False,
+     ModuleSpec('eog', 'f26', '20170629213428', '12345678')),
     ('eog:f26:20170629213428/default', False,
      ModuleSpec('eog', 'f26', '20170629213428', profile='default')),
-    ('eog-f26-20170629213428', False, ModuleSpec('eog', 'f26', '20170629213428')),
-    ('a-b-c-20176291342855', False, ModuleSpec('a-b', 'c', '20176291342855')),
-    ('a-b-c-d', False, ModuleSpec('a-b-c', 'd')),
-    ('a:b:c:d', True, None),
+    ('a:b:c:d:e', "should be NAME:STREAM[:VERSION[:CONTEXT]][/PROFILE]", None),
+    ('a:b::d', "contains empty components", None),
+    ('a:b:c:d/', "contains empty components", None),
 ])
 def test_split_module_spec(module, should_raise, expected):
     if should_raise:
-        with pytest.raises(RuntimeError):
+        with pytest.raises(RuntimeError) as e:
             split_module_spec(module)
+        assert should_raise in str(e)
     else:
         assert split_module_spec(module) == expected
 
-- 
2.17.1

From 2dccc51d96801ad9d2b6d0d05388e765bd5243ef Mon Sep 17 00:00:00 2001
From: "Owen W. Taylor" <otaylor@fishsoup.net>
Date: Wed, 29 Aug 2018 13:09:18 -0400
Subject: [PATCH] Get module information from Koji instead of the PDC

The information (modulemd file, list of built RPMs) that we were previously
looking up in the PDC is also available directly from Koji. Since there is
a plan to retire the PDC, switch over to reading the information from Koji.

There is some complexity because older versions of ODCS (like what is deployed
currently in Fedora) return the modules in the compose as NAME:STREAM:VERSION
instead of NAME:STREAM:VERSION:CONTEXT, and in that case, we have to hunt
for the right build in Koji.

With this change, Flatpak building will only work with Koji configuration
in the reactor config map, but that's the recommended set up in any case.
---
 atomic_reactor/koji_util.py                   |  52 +++++++
 .../plugins/build_orchestrate_build.py        |  10 +-
 atomic_reactor/plugins/exit_koji_import.py    |   2 +-
 atomic_reactor/plugins/exit_koji_promote.py   |   2 +-
 atomic_reactor/plugins/pre_reactor_config.py  |  12 --
 .../plugins/pre_resolve_module_compose.py     |  70 ++++------
 atomic_reactor/schemas/config.json            |  16 ---
 docs/flatpak.md                               |   1 -
 requirements-flatpak.txt                      |   1 -
 tests/constants.py                            |   4 -
 tests/koji/__init__.py                        |   8 ++
 tests/plugins/test_orchestrate_build.py       |   5 -
 tests/plugins/test_reactor_config.py          |  57 +-------
 tests/plugins/test_resolve_module_compose.py  | 131 ++++++++++++++----
 tests/test_koji_util.py                       | 122 +++++++++++++++-
 15 files changed, 312 insertions(+), 181 deletions(-)

diff --git a/atomic_reactor/koji_util.py b/atomic_reactor/koji_util.py
index b365607..3156fb7 100644
--- a/atomic_reactor/koji_util.py
+++ b/atomic_reactor/koji_util.py
@@ -210,3 +210,55 @@ def get_koji_task_owner(session, task_id, default=None):
     else:
         koji_task_owner = default
     return koji_task_owner
+
+
+def get_koji_module_build(session, module_spec):
+    """
+    Get build information from Koji for a module. The module specification must
+    include at least name, stream and version. For legacy support, you can omit
+    context if there is only one build of the specified NAME:STREAM:VERSION.
+
+    :param session: KojiSessionWrapper, Session for talking to Koji
+    :param module_spec: ModuleSpec, specification of the module version
+    :return: tuple, a dictionary of information about the build, and
+        a list of RPMs in the module build
+    """
+
+    if module_spec.context is not None:
+        # The easy case - we can build the koji "name-version-release" out of the
+        # module spec.
+        koji_nvr = "{}-{}-{}.{}".format(module_spec.name,
+                                        module_spec.stream,
+                                        module_spec.version,
+                                        module_spec.context)
+        logger.info("Looking up module build %s in Koji", koji_nvr)
+        build = session.getBuild(koji_nvr)
+    else:
+        # Without the context, we have to retrieve all builds for the module, and
+        # find the one we want. This is pretty inefficient, but won't be needed
+        # long-term.
+        logger.info("Listing all builds for %s in Koji", module_spec.name)
+        package_id = session.getPackageID(module_spec.name)
+        builds = session.listBuilds(packageID=package_id, type='module',
+                                    state=koji.BUILD_STATES['COMPLETE'])
+        build = None
+
+        for b in builds:
+            if '.' in b['release']:
+                version, context = b['release'].split('.', 1)
+                if version == module_spec.version:
+                    if build is not None:
+                        raise RuntimeError("Multiple builds found for {}"
+                                           .format(module_spec.to_str()))
+                    else:
+                        build = b
+
+        if build is None:
+            raise RuntimeError("No build found for {}".format(module_spec.to_str()))
+
+    archives = session.listArchives(buildID=build['build_id'])
+    assert len(archives) == 1
+
+    rpm_list = session.listRPMs(imageID=archives[0]['id'])
+
+    return build, rpm_list
diff --git a/atomic_reactor/plugins/build_orchestrate_build.py b/atomic_reactor/plugins/build_orchestrate_build.py
index 8aef641..02ba0e1 100644
--- a/atomic_reactor/plugins/build_orchestrate_build.py
+++ b/atomic_reactor/plugins/build_orchestrate_build.py
@@ -27,7 +27,7 @@ from atomic_reactor.build import BuildResult
 from atomic_reactor.plugin import BuildStepPlugin
 from atomic_reactor.plugins.pre_reactor_config import (get_config,
                                                        get_arrangement_version, get_koji,
-                                                       get_odcs, get_pdc, get_pulp,
+                                                       get_odcs, get_pulp,
                                                        get_prefer_schema1_digest, get_smtp,
                                                        get_source_registry, get_sources_command,
                                                        get_artifacts_allowed_domains,
@@ -338,14 +338,6 @@ class OrchestrateBuildPlugin(BuildStepPlugin):
         self.config_kwargs['odcs_url'] = odcs_map['api_url']
         self.config_kwargs['odcs_insecure'] = odcs_map.get('insecure', False)
 
-        pdc_fallback = {
-            'api_url': self.config_kwargs.get('pdc_url'),
-            'insecure': self.config_kwargs.get('pdc_insecure')
-        }
-        pdc_map = get_pdc(self.workflow, pdc_fallback)
-        self.config_kwargs['pdc_url'] = pdc_map['api_url']
-        self.config_kwargs['pdc_insecure'] = pdc_map.get('insecure', False)
-
         pulp_fallback = {'name': self.config_kwargs.get('pulp_registry_name')}
         pulp_map = get_pulp(self.workflow, pulp_fallback)
         self.config_kwargs['pulp_registry_name'] = pulp_map['name']
diff --git a/atomic_reactor/plugins/exit_koji_import.py b/atomic_reactor/plugins/exit_koji_import.py
index e59b557..2440932 100644
--- a/atomic_reactor/plugins/exit_koji_import.py
+++ b/atomic_reactor/plugins/exit_koji_import.py
@@ -26,7 +26,7 @@ try:
     from atomic_reactor.plugins.pre_flatpak_create_dockerfile import get_flatpak_source_info
     from atomic_reactor.plugins.pre_resolve_module_compose import get_compose_info
 except ImportError:
-    # modulemd and/or pdc_client not available
+    # modulemd not available
     def get_flatpak_source_info(_):
         return None
 
diff --git a/atomic_reactor/plugins/exit_koji_promote.py b/atomic_reactor/plugins/exit_koji_promote.py
index 6e3ab2d..5eaf259 100644
--- a/atomic_reactor/plugins/exit_koji_promote.py
+++ b/atomic_reactor/plugins/exit_koji_promote.py
@@ -32,7 +32,7 @@ try:
     from atomic_reactor.plugins.pre_flatpak_create_dockerfile import get_flatpak_source_info
     from atomic_reactor.plugins.pre_resolve_module_compose import get_compose_info
 except ImportError:
-    # modulemd and/or pdc_client not available
+    # modulemd not available
     def get_flatpak_source_info(_):
         return None
 
diff --git a/atomic_reactor/plugins/pre_reactor_config.py b/atomic_reactor/plugins/pre_reactor_config.py
index 6374ec0..ead2747 100644
--- a/atomic_reactor/plugins/pre_reactor_config.py
+++ b/atomic_reactor/plugins/pre_reactor_config.py
@@ -141,18 +141,6 @@ def get_smtp_session(workflow, fallback):
     return smtplib.SMTP(config['host'])
 
 
-def get_pdc(workflow, fallback=NO_FALLBACK):
-    return get_value(workflow, 'pdc', fallback)
-
-
-def get_pdc_session(workflow, fallback):
-    config = get_pdc(workflow, fallback)
-
-    from pdc_client import PDCClient
-    return PDCClient(server=config['api_url'], ssl_verify=not config.get('insecure', False),
-                     develop=True)
-
-
 def get_arrangement_version(workflow, fallback=NO_FALLBACK):
     return get_value(workflow, 'arrangement_version', fallback)
 
diff --git a/atomic_reactor/plugins/pre_resolve_module_compose.py b/atomic_reactor/plugins/pre_resolve_module_compose.py
index df5f040..b179320 100644
--- a/atomic_reactor/plugins/pre_resolve_module_compose.py
+++ b/atomic_reactor/plugins/pre_resolve_module_compose.py
@@ -17,9 +17,7 @@ Example configuration:
     'name': 'resolve_module_compose',
     'args': {'module_name': 'myapp',
              'module_stream': 'f26',
-             'module_version': '20170629185228',
-             'odcs_url': 'https://odcs.fedoraproject.org/odcs/1'},
-             'pdc_url': 'https://pdc.fedoraproject.org/rest_api/v1',}
+             'module_version': '20170629185228'}
 }
 """
 
@@ -34,10 +32,11 @@ except ValueError as e:
     raise ImportError(str(e))
 from gi.repository import Modulemd
 
+from atomic_reactor.koji_util import get_koji_module_build
 from atomic_reactor.plugin import PreBuildPlugin
 from atomic_reactor.util import split_module_spec
-from atomic_reactor.plugins.pre_reactor_config import (get_pdc_session, get_odcs_session,
-                                                       get_pdc, get_odcs)
+from atomic_reactor.plugins.pre_reactor_config import (get_koji_session, get_odcs_session,
+                                                       get_odcs, NO_FALLBACK)
 
 
 class ComposeInfo(object):
@@ -96,8 +95,8 @@ class ResolveModuleComposePlugin(PreBuildPlugin):
         :param odcs_url: URL of ODCS (On Demand Compose Service)
         :param odcs_insecure: If True, don't check SSL certificates for `odcs_url`
         :param odcs_openidc_secret_path: directory to look in for a `token` file (optional)
-        :param pdc_url: URL of PDC (Product Definition center))
-        :param pdc_insecure: If True, don't check SSL certificates for `pdc_url`
+        :param pdc_url: unused
+        :param pdc_insecure: unused
         :
         """
         # call parent constructor
@@ -116,12 +115,6 @@ class ResolveModuleComposePlugin(PreBuildPlugin):
         if not get_odcs(self.workflow, self.odcs_fallback)['api_url']:
             raise RuntimeError("odcs_url is required")
 
-        self.pdc_fallback = {
-            'api_url': pdc_url,
-            'insecure': pdc_insecure
-        }
-        if not get_pdc(self.workflow, self.pdc_fallback)['api_url']:
-            raise RuntimeError("pdc_url is required")
         self.data = None
 
     def read_configs_general(self):
@@ -131,46 +124,31 @@ class ResolveModuleComposePlugin(PreBuildPlugin):
             raise RuntimeError('"compose" config in container.yaml is required for Flatpaks')
 
     def _resolve_modules(self, compose_source):
-        resolved_modules = {}
-        # The effect of develop=True is that requests to the PDC are made without authentication;
-        # since we our interaction with the PDC is read-only, this is fine for our needs and
-        # makes things simpler.
-        pdc_client = get_pdc_session(self.workflow, self.pdc_fallback)
-
-        for module_spec in compose_source.strip().split():
-            try:
-                module = split_module_spec(module_spec)
-                if not module.version:
-                    raise RuntimeError
-            except RuntimeError:
-                raise RuntimeError("Cannot parse resolved module in compose: %s" % module_spec)
-
-            query = {
-                'variant_id': module.name,
-                'variant_version': module.stream,
-                'variant_release': module.version,
-                'active': True,
-            }
+        koji_session = get_koji_session(self.workflow, fallback=NO_FALLBACK)
 
-            self.log.info("Looking up module metadata for '%s' in the PDC", module_spec)
-            retval = pdc_client['unreleasedvariants/'](page_size=-1,
-                                                       fields=['modulemd', 'rpms'], **query)
-            # Error handling
-            if not retval:
-                raise RuntimeError("Failed to find module in PDC %r" % query)
-            if len(retval) != 1:
-                raise RuntimeError("Multiple modules in the PDC matched %r" % query)
-
-            objects = Modulemd.objects_from_string(retval[0]['modulemd'])
+        resolved_modules = {}
+        for module in compose_source.strip().split():
+            module_spec = split_module_spec(module)
+            build, rpm_list = get_koji_module_build(koji_session, module_spec)
+
+            # The returned RPM list contains source RPMs and RPMs for all
+            # architectures.
+            rpms = ['{name}-{epochnum}:{version}-{release}.{arch}.rpm'
+                    .format(epochnum=rpm['epoch'] or 0, **rpm)
+                    for rpm in rpm_list]
+
+            objects = Modulemd.objects_from_string(
+                build['extra']['typeinfo']['module']['modulemd_str'])
             assert len(objects) == 1
             mmd = objects[0]
             assert isinstance(mmd, Modulemd.Module)
             # Make sure we have a version 2 modulemd file
             mmd.upgrade()
-            rpms = set(retval[0]['rpms'])
 
-            resolved_modules[module.name] = ModuleInfo(module.name, module.stream, module.version,
-                                                       mmd, rpms)
+            resolved_modules[module_spec.name] = ModuleInfo(module_spec.name,
+                                                            module_spec.stream,
+                                                            module_spec.version,
+                                                            mmd, rpms)
         return resolved_modules
 
     def _resolve_compose(self):
diff --git a/atomic_reactor/schemas/config.json b/atomic_reactor/schemas/config.json
index 5270fa2..df0a230 100644
--- a/atomic_reactor/schemas/config.json
+++ b/atomic_reactor/schemas/config.json
@@ -260,22 +260,6 @@
         "additionalProperties": false,
         "required": ["host", "from_address"]
     },
-    "pdc": {
-        "description": "Product Definition Center (PDC) instance",
-        "type": "object",
-        "properties": {
-            "api_url": {
-                "description": "PDC api url, including api version",
-                "type": "string"
-            },
-            "insecure": {
-                "description": "Don't check SSL certificate for api_url",
-                "type": "boolean"
-            }
-        },
-        "additionalProperties": false,
-        "required": ["api_url"]
-    },
     "arrangement_version": {
         "description": "Arrangement version",
         "type": "integer"
diff --git a/docs/flatpak.md b/docs/flatpak.md
index 5dac33d..58a7168 100644
--- a/docs/flatpak.md
+++ b/docs/flatpak.md
@@ -5,7 +5,6 @@ In addition to building docker images from Dockerfiles, atomic-reactor can also
 Building Flatpaks requires, in addition to a Koji installation:
 
  * A [MBS (module-build-service)](https://pagure.io/fm-orchestrator/) instance set up to build modules in Koji
- * A [PDC (product definition center)](https://github.com/product-definition-center/product-definition-center) instance to store information about the built modules
  * An [ODCS (on demand compose service)](https://pagure.io/odcs/) instance to create repositories for the build modules
 
 A modified version of osbs-box for testing Flatpak building can be found at:
diff --git a/requirements-flatpak.txt b/requirements-flatpak.txt
index acb27ca..d3ade5d 100644
--- a/requirements-flatpak.txt
+++ b/requirements-flatpak.txt
@@ -1,2 +1 @@
 flatpak-module-tools
-pdc-client
diff --git a/tests/constants.py b/tests/constants.py
index c1b4642..ce53c71 100644
--- a/tests/constants.py
+++ b/tests/constants.py
@@ -93,10 +93,6 @@ smtp:
     send_to_submitter: True
     send_to_pkg_owner: True
 
-pdc:
-    api_url: https://pdc.example.com/rest_api/v1
-    insecure: True
-
 arrangement_version: 6
 
 artifacts_allowed_domains:
diff --git a/tests/koji/__init__.py b/tests/koji/__init__.py
index 643599f..3089abe 100644
--- a/tests/koji/__init__.py
+++ b/tests/koji/__init__.py
@@ -15,6 +15,14 @@ TASK_STATES = {
     'FAILED': 5,
 }
 
+BUILD_STATES = {
+    'BUILDING': 0,
+    'COMPLETE': 1,
+    'DELETED': 2,
+    'FAILED': 3,
+    'CANCELED': 4,
+}
+
 CHECKSUM_TYPES = {
     0: 'md5',
     1: 'sha1',
diff --git a/tests/plugins/test_orchestrate_build.py b/tests/plugins/test_orchestrate_build.py
index 7f3f558..4876041 100644
--- a/tests/plugins/test_orchestrate_build.py
+++ b/tests/plugins/test_orchestrate_build.py
@@ -314,9 +314,6 @@ def test_orchestrate_build(tmpdir, caplog, config_kwargs,
         reactor_dict['odcs'] = {'api_url': 'odcs_url'}
         expected_kwargs['odcs_insecure'] = False
         expected_kwargs['odcs_url'] = reactor_dict['odcs']['api_url']
-        reactor_dict['pdc'] = {'api_url': 'pdc_url'}
-        expected_kwargs['pdc_url'] = reactor_dict['pdc']['api_url']
-        expected_kwargs['pdc_insecure'] = False
         reactor_dict['pulp'] = {'name': 'pulp_name'}
         expected_kwargs['pulp_registry_name'] = reactor_dict['pulp']['name']
         reactor_dict['prefer_schema1_digest'] = False
@@ -374,7 +371,6 @@ def test_orchestrate_build(tmpdir, caplog, config_kwargs,
         expected_kwargs['registry_api_versions'] = ''
         expected_kwargs['source_registry_uri'] = None
         expected_kwargs['yum_proxy'] = None
-        expected_kwargs['pdc_url'] = None
         expected_kwargs['odcs_url'] = None
         expected_kwargs['smtp_additional_addresses'] = ''
         expected_kwargs['koji_root'] = None
@@ -382,7 +378,6 @@ def test_orchestrate_build(tmpdir, caplog, config_kwargs,
         expected_kwargs['smtp_to_submitter'] = None
         expected_kwargs['artifacts_allowed_domains'] = ''
         expected_kwargs['smtp_to_pkgowner'] = None
-        expected_kwargs['pdc_insecure'] = None
         expected_kwargs['prefer_schema1_digest'] = None
         expected_kwargs['pulp_registry_name'] = None
 
diff --git a/tests/plugins/test_reactor_config.py b/tests/plugins/test_reactor_config.py
index 2707277..3677e6f 100644
--- a/tests/plugins/test_reactor_config.py
+++ b/tests/plugins/test_reactor_config.py
@@ -19,12 +19,6 @@ import yaml
 import smtplib
 from copy import deepcopy
 
-try:
-    import pdc_client
-    PDC_AVAILABLE = True
-except ImportError:
-    PDC_AVAILABLE = False
-
 import atomic_reactor
 import koji
 from atomic_reactor.core import DockerTasker
@@ -44,7 +38,6 @@ from atomic_reactor.plugins.pre_reactor_config import (ReactorConfig,
                                                        get_pulp_session,
                                                        get_odcs_session,
                                                        get_smtp_session,
-                                                       get_pdc_session,
                                                        get_openshift_session,
                                                        get_clusters_client_config_path,
                                                        get_docker_registry,
@@ -515,7 +508,7 @@ class TestReactorConfigPlugin(object):
 
     @pytest.mark.parametrize('fallback', (True, False, None))
     @pytest.mark.parametrize('method', [
-        'koji', 'pulp', 'odcs', 'smtp', 'pdc', 'arrangement_version',
+        'koji', 'pulp', 'odcs', 'smtp', 'arrangement_version',
         'artifacts_allowed_domains', 'image_labels',
         'image_label_info_url_format', 'image_equal_labels',
         'openshift', 'group_manifests', 'platform_descriptors', 'prefer_schema1_digest',
@@ -1155,54 +1148,6 @@ class TestReactorConfigPlugin(object):
 
         get_smtp_session(workflow, fallback_map)
 
-    @pytest.mark.parametrize('fallback', (True, False))
-    @pytest.mark.parametrize(('config', 'raise_error'), [
-        ("""\
-          version: 1
-          pdc:
-             api_url: https://pdc.example.com/rest_api/v1
-        """, False),
-
-        ("""\
-          version: 1
-          pdc:
-        """, True),
-    ])
-    def test_get_pdc_session(self, fallback, config, raise_error):
-        tasker, workflow = self.prepare()
-        workflow.plugin_workspace[ReactorConfigPlugin.key] = {}
-
-        if raise_error:
-            with pytest.raises(Exception):
-                read_yaml(config, 'schemas/config.json')
-            return
-        config_json = read_yaml(config, 'schemas/config.json')
-
-        if not PDC_AVAILABLE:
-            return
-
-        auth_info = {
-            "server": config_json['pdc']['api_url'],
-            "ssl_verify": not config_json['pdc'].get('insecure', False),
-            "develop": True,
-        }
-
-        fallback_map = {}
-        if fallback:
-            fallback_map['api_url'] = config_json['pdc']['api_url']
-            fallback_map['insecure'] = config_json['pdc'].get('insecure', False)
-        else:
-            workflow.plugin_workspace[ReactorConfigPlugin.key][WORKSPACE_CONF_KEY] =\
-                ReactorConfig(config_json)
-
-        (flexmock(pdc_client.PDCClient)
-            .should_receive('__init__')
-            .with_args(**auth_info)
-            .once()
-            .and_return(None))
-
-        get_pdc_session(workflow, fallback_map)
-
     @pytest.mark.parametrize('fallback', (True, False))
     @pytest.mark.parametrize('build_json_dir', [
         None, "/tmp/build_json_dir",
diff --git a/tests/plugins/test_resolve_module_compose.py b/tests/plugins/test_resolve_module_compose.py
index 8fddcbf..9bd34d5 100644
--- a/tests/plugins/test_resolve_module_compose.py
+++ b/tests/plugins/test_resolve_module_compose.py
@@ -13,7 +13,6 @@ import responses
 import os
 import pytest
 import six
-from six.moves.urllib.parse import urlparse, parse_qs
 
 from atomic_reactor.inner import DockerBuildWorkflow
 try:
@@ -31,8 +30,24 @@ from atomic_reactor.source import VcsInfo, SourceConfig
 from atomic_reactor.util import ImageName
 from atomic_reactor.constants import REPO_CONTAINER_CONFIG
 
+try:
+    import koji
+except ImportError:
+    import inspect
+    import sys
+
+    # Find our mocked koji module
+    import tests.koji as koji
+    mock_koji_path = os.path.dirname(inspect.getfile(koji.ClientSession))
+    if mock_koji_path not in sys.path:
+        sys.path.append(os.path.dirname(mock_koji_path))
+
+    # Now load it properly, the same way the plugin will
+    del koji
+    import koji
+
 from tests.constants import (MOCK_SOURCE, FLATPAK_GIT, FLATPAK_SHA1)
-from tests.fixtures import docker_tasker, reactor_config_map  # noqa
+from tests.fixtures import docker_tasker  # noqa
 from tests.flatpak import FLATPAK_APP_MODULEMD, FLATPAK_APP_RPMS
 from tests.retry_mock import mock_get_retry_session
 
@@ -85,13 +100,19 @@ def mock_workflow(tmpdir):
 MODULE_NAME = 'eog'
 MODULE_STREAM = 'f26'
 MODULE_VERSION = "20170629213428"
+MODULE_CONTEXT = "01234567"
 MODULE_NS = MODULE_NAME + ':' + MODULE_STREAM
 MODULE_NSV = MODULE_NS + ':' + MODULE_VERSION
+MODULE_NSVC = MODULE_NSV + ':' + MODULE_CONTEXT
+MODULE_NVR = MODULE_NAME + "-" + MODULE_STREAM + "-" + MODULE_VERSION + "." + MODULE_CONTEXT
+
 
 ODCS_URL = 'https://odcs.fedoraproject.org/odcs/1'
 
-PDC_URL = 'https://pdc.fedoraproject.org/rest_api/v1'
-LATEST_VERSION_JSON = [{"modulemd": FLATPAK_APP_MODULEMD,
+LATEST_VERSION_JSON = [{"name": MODULE_NAME,
+                        "stream": MODULE_STREAM,
+                        "version": MODULE_VERSION,
+                        "modulemd": FLATPAK_APP_MODULEMD,
                         "rpms": FLATPAK_APP_RPMS}]
 
 
@@ -101,13 +122,76 @@ def compose_json(state, state_name):
         "id": 84,
         "owner": "Unknown",
         "result_repo": "http://odcs.fedoraproject.org/composes/latest-odcs-84-1/compose/Temporary",
-        "source": MODULE_NSV,
+        "source": MODULE_NSVC,
         "source_type": 2,
         "state": state,
         "state_name": state_name
     })
 
 
+def mock_koji_session():
+    session = flexmock()
+
+    (session
+     .should_receive('krb_login')
+     .and_return(True))
+
+    (session
+     .should_receive('getBuild')
+     .with_args(MODULE_NVR)
+     .and_return({
+         'build_id': 1138198,
+         'name': MODULE_NAME,
+         'version': MODULE_STREAM,
+         'release': MODULE_VERSION + "." + MODULE_CONTEXT,
+         'extra': {
+             'typeinfo': {
+                 'module': {
+                     'modulemd_str': FLATPAK_APP_MODULEMD
+                 }
+             }
+         }
+     }))
+
+    (session
+     .should_receive('listArchives')
+     .with_args(buildID=1138198)
+     .and_return(
+        [{'btype': 'module',
+          'build_id': 1138198,
+          'id': 147879}]))
+
+    (session
+     .should_receive('listRPMs')
+     .with_args(imageID=147879)
+     .and_return([
+         {'arch': 'src',
+          'epoch': None,
+          'id': 15197182,
+          'name': 'eog',
+          'release': '1.module_2123+73a9ef6f',
+          'version': '3.28.3'},
+         {'arch': 'x86_64',
+          'epoch': None,
+          'id': 15197187,
+          'metadata_only': False,
+          'name': 'eog',
+          'release': '1.module_2123+73a9ef6f',
+          'version': '3.28.3'},
+         {'arch': 'ppc64le',
+          'epoch': None,
+          'id': 15197188,
+          'metadata_only': False,
+          'name': 'eog',
+          'release': '1.module_2123+73a9ef6f',
+          'version': '3.28.3'},
+     ]))
+
+    (flexmock(koji)
+        .should_receive('ClientSession')
+        .and_return(session))
+
+
 @responses.activate  # noqa - docker_tasker fixture
 @pytest.mark.skipif(not MODULEMD_AVAILABLE,
                     reason="libmodulemd not available")
@@ -119,8 +203,7 @@ def compose_json(state, state_name):
     [MODULE_NSV],
     [MODULE_NSV, 'mod_name2-mod_stream2-mod_version2'],
 ))
-def test_resolve_module_compose(tmpdir, docker_tasker, compose_ids, modules,  # noqa
-                                reactor_config_map):
+def test_resolve_module_compose(tmpdir, docker_tasker, compose_ids, modules):
     secrets_path = os.path.join(str(tmpdir), "secret")
     os.mkdir(secrets_path)
     with open(os.path.join(secrets_path, "token"), "w") as f:
@@ -139,6 +222,7 @@ def test_resolve_module_compose(tmpdir, docker_tasker, compose_ids, modules,  #
 
     workflow = mock_workflow(tmpdir)
     mock_get_retry_session()
+    mock_koji_session()
 
     def handle_composes_post(request):
         assert request.headers['Authorization'] == 'Bearer green_eggs_and_ham'
@@ -173,33 +257,19 @@ def test_resolve_module_compose(tmpdir, docker_tasker, compose_ids, modules,  #
                            content_type='application/json',
                            callback=handle_composes_get)
 
-    def handle_unreleasedvariants(request):
-        query = parse_qs(urlparse(request.url).query)
-
-        assert query['variant_id'] == [MODULE_NAME]
-        assert query['variant_version'] == [MODULE_STREAM]
-        assert query['variant_release'] == [MODULE_VERSION]
-
-        return (200, {}, json.dumps(LATEST_VERSION_JSON))
-
-    responses.add_callback(responses.GET, PDC_URL + '/unreleasedvariants/',
-                           content_type='application/json',
-                           callback=handle_unreleasedvariants)
-
     args = {
         'odcs_url': ODCS_URL,
         'odcs_openidc_secret_path': secrets_path,
-        'pdc_url': PDC_URL,
         'compose_ids': compose_ids
     }
 
-    if reactor_config_map:
-        workflow.plugin_workspace[ReactorConfigPlugin.key] = {}
-        workflow.plugin_workspace[ReactorConfigPlugin.key][WORKSPACE_CONF_KEY] =\
-            ReactorConfig({'version': 1,
-                           'odcs': {'api_url': ODCS_URL,
-                                    'auth': {'openidc_dir': secrets_path}},
-                           'pdc': {'api_url': PDC_URL}})
+    workflow.plugin_workspace[ReactorConfigPlugin.key] = {}
+    workflow.plugin_workspace[ReactorConfigPlugin.key][WORKSPACE_CONF_KEY] =\
+        ReactorConfig({'version': 1,
+                       'odcs': {'api_url': ODCS_URL,
+                                'auth': {'openidc_dir': secrets_path}},
+                       'koji':  {'auth': {},
+                                 'hub_url': 'https://koji.example.com/hub'}})
 
     runner = PreBuildPluginsRunner(
         docker_tasker,
@@ -228,3 +298,8 @@ def test_resolve_module_compose(tmpdir, docker_tasker, compose_ids, modules,  #
         assert compose_info.base_module.stream == MODULE_STREAM
         assert compose_info.base_module.version == MODULE_VERSION
         assert compose_info.base_module.mmd.props.summary == 'Eye of GNOME Application Module'
+        assert compose_info.base_module.rpms == [
+            'eog-0:3.28.3-1.module_2123+73a9ef6f.src.rpm',
+            'eog-0:3.28.3-1.module_2123+73a9ef6f.x86_64.rpm',
+            'eog-0:3.28.3-1.module_2123+73a9ef6f.ppc64le.rpm',
+        ]
diff --git a/tests/test_koji_util.py b/tests/test_koji_util.py
index 558b1b6..b828ddb 100644
--- a/tests/test_koji_util.py
+++ b/tests/test_koji_util.py
@@ -29,10 +29,12 @@ except ImportError:
     import koji
 
 from atomic_reactor.koji_util import (koji_login, create_koji_session,
-                                      TaskWatcher, tag_koji_build)
+                                      TaskWatcher, tag_koji_build,
+                                      get_koji_module_build)
 from atomic_reactor import koji_util
 from atomic_reactor.plugin import BuildCanceledException
 from atomic_reactor.constants import HTTP_MAX_RETRIES
+from atomic_reactor.util import split_module_spec
 import flexmock
 import pytest
 
@@ -279,3 +281,121 @@ class TestTagKojiBuild(object):
         else:
             build_tag = tag_koji_build(session, build_id, target_name)
             assert build_tag == tag_name
+
+
+class TestGetKojiModuleBuild(object):
+    def mock_get_rpms(self, session):
+        (session
+            .should_receive('listArchives')
+            .with_args(buildID=1138198)
+            .once()
+            .and_return(
+                [{'btype': 'module',
+                  'build_id': 1138198,
+                  'id': 147879}]))
+        (session
+            .should_receive('listRPMs')
+            .with_args(imageID=147879)
+            .once()
+            .and_return([
+                {'arch': 'src',
+                 'epoch': None,
+                 'id': 15197182,
+                 'name': 'eog',
+                 'release': '1.module_2123+73a9ef6f',
+                 'version': '3.28.3'},
+                {'arch': 'x86_64',
+                 'epoch': None,
+                 'id': 15197187,
+                 'metadata_only': False,
+                 'name': 'eog',
+                 'release': '1.module_2123+73a9ef6f',
+                 'version': '3.28.3'},
+                {'arch': 'ppc64le',
+                 'epoch': None,
+                 'id': 15197188,
+                 'metadata_only': False,
+                 'name': 'eog',
+                 'release': '1.module_2123+73a9ef6f',
+                 'version': '3.28.3'},
+             ]))
+
+    def test_with_context(self):
+        module = 'eog:master:20180821163756:775baa8e'
+        module_koji_nvr = 'eog-master-20180821163756.775baa8e'
+        koji_return = {
+            'build_id': 1138198,
+            'name': 'eog',
+            'version': 'master',
+            'release': '20180821163756.775baa8e',
+            'extra': {
+                'typeinfo': {
+                    'module': {
+                        'modulemd_str': 'document: modulemd\nversion: 2'
+                    }
+                }
+            }
+        }
+
+        spec = split_module_spec(module)
+        session = flexmock()
+        (session
+            .should_receive('getBuild')
+            .with_args(module_koji_nvr)
+            .and_return(koji_return))
+        self.mock_get_rpms(session)
+
+        get_koji_module_build(session, spec)
+
+    @pytest.mark.parametrize(('koji_return', 'should_raise'), [
+        ([{
+            'build_id': 1138198,
+            'name': 'eog',
+            'version': 'master',
+            'release': '20180821163756.775baa8e',
+            'extra': {
+                'typeinfo': {
+                    'module': {
+                        'modulemd_str': 'document: modulemd\nversion: 2'
+                    }
+                }
+            }
+        }], None),
+        ([], "No build found for"),
+        ([{
+            'build_id': 1138198,
+            'name': 'eog',
+            'version': 'master',
+            'release': '20180821163756.775baa8e',
+          },
+          {
+            'build_id': 1138199,
+            'name': 'eog',
+            'version': 'master',
+            'release': '20180821163756.88888888',
+          }],
+         "Multiple builds found for"),
+    ])
+    def test_without_context(self, koji_return, should_raise):
+        module = 'eog:master:20180821163756'
+        spec = split_module_spec(module)
+
+        session = flexmock()
+        (session
+            .should_receive('getPackageID')
+            .with_args('eog')
+            .and_return(303))
+        (session
+            .should_receive('listBuilds')
+            .with_args(packageID=303,
+                       type='module',
+                       state=koji.BUILD_STATES['COMPLETE'])
+            .and_return(koji_return))
+
+        if should_raise:
+            with pytest.raises(Exception) as e:
+                get_koji_module_build(session, spec)
+            assert should_raise in str(e)
+        else:
+            self.mock_get_rpms(session)
+            get_koji_module_build(session, spec)
-- 
2.17.1