Refactor handling of default buildroot modules (Ursa Prime)

This removes support for default_modules_url in the Platform XMD and
gets the list of default name:stream combinations to include in the buildroot
from https://pagure.io/releng/fedora-module-defaults.

Addresses #1402

docker/Dockerfile-tests

@@ -4,6 +4,7 @@ WORKDIR /build
 RUN yum -y update
 RUN yum -y install epel-release yum-utils
 RUN yum-config-manager --add-repo https://kojipkgs.fedoraproject.org/repos-dist/epel7Server-infra/latest/x86_64/
+# Replace the pinned libmodulemd RPMs with python2-libmodulemd2 after https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-e093131fa7
 RUN yum -y install \
     --nogpgcheck \
     --setopt=deltarpm=0 \
@@ -29,7 +30,8 @@ RUN yum -y install \
     python-futures \
     python-koji \
     python-ldap3 \
-    python2-libmodulemd \
+    https://kojipkgs.fedoraproject.org//packages/libmodulemd2/2.8.0/2.el7/x86_64/libmodulemd2-2.8.0-2.el7.x86_64.rpm \
+    https://kojipkgs.fedoraproject.org//packages/libmodulemd2/2.8.0/2.el7/x86_64/python2-libmodulemd2-2.8.0-2.el7.x86_64.rpm \
     python-mock \
     python-munch \
     python-pip \

docker/Dockerfile-tests-py3

@@ -1,6 +1,7 @@
 FROM fedora:29
 
 WORKDIR /build
+# Replace the pinned libmodulemd RPMs with python3-libmodulemd after https://bodhi.fedoraproject.org/updates/FEDORA-2019-ccf39d5166
 RUN dnf -y install \
     --nogpgcheck \
     --setopt=deltarpm=0 \
@@ -20,7 +21,8 @@ RUN dnf -y install \
     python3-flask-sqlalchemy \
     python3-koji \
     python3-ldap3 \
-    python3-libmodulemd \
+    https://kojipkgs.fedoraproject.org//packages/libmodulemd/2.8.0/1.fc29/x86_64/libmodulemd-2.8.0-1.fc29.x86_64.rpm \
+    https://kojipkgs.fedoraproject.org//packages/libmodulemd/2.8.0/1.fc29/x86_64/python3-libmodulemd-2.8.0-1.fc29.x86_64.rpm \
     python3-munch \
     python3-pip \
     python3-prometheus_client \

docs/VIRTUAL_MODULES.rst

@@ -72,10 +72,14 @@ Custom fields in xmd:
   module in the disttag of the RPMS being built. If the stream is not the appropriate value, then
   this can be overridden with a custom value using this property. This value can't contain a dash,
   since that is an invalid character in the disttag.
-- ``default_modules_url`` -  the URL to the list of modules, in the format of ``name:stream``
-  separated by new lines, to include as default modules for any module that buildrequires this
-  module. Any default modules with conflicting streams will be ignored as well as any default module
-  not found in the MBS database. This field only applies to base modules.
+- ``use_default_modules`` - denotes if MBS should include default modules associated with it. The
+  default modules are taken from the SCM repo configured in the ``default_modules_scm_url`` xmd
+  field or in the MBS configuration ``default_modules_scm_url`` as a fallback. Any default modules
+  with conflicting streams will be ignored as well as any default module not found in the MBS
+  database. This field only applies to base modules.
+- ``default_modules_scm_url`` - the SCM repo to find the default modules associated with the base
+  module. If this is not specified, the MBS configuration ``default_modules_scm_url`` is used
+  instead. See the ``use_default_modules`` xmd field for more information.
 
 
 Virtual Streams

module_build_service/config.py

@@ -671,6 +671,24 @@ class Config(object):
             "desc": "The number of threads when submitting component builds to an external build "
                     "system.",
         },
+        "default_modules_scm_url": {
+            "type": str,
+            "default": "https://pagure.io/releng/fedora-module-defaults.git",
+            "desc": "The SCM URL to the default modules repo, which will be used to determine "
+                    "which buildrequires to automatically include. This can be overridden with "
+                    "the xmd.mbs.default_modules_scm_url key in the base module's modulemd.",
+        },
+        "uses_rawhide": {
+            "type": bool,
+            "default": True,
+            "desc": "Denotes if the concept of rawhide exists in the infrastructure of this "
+                    "MBS deployment.",
+        },
+        "rawhide_branch": {
+            "type": str,
+            "default": "master",
+            "desc": "Denotes the branch used for rawhide.",
+        },
     }
 
     def __init__(self, conf_section_obj):

module_build_service/scheduler/default_modules.py

@@ -20,18 +20,21 @@
 # SOFTWARE.
 import errno
 import os
+import tempfile
+import shutil
 
 import dnf
 import kobo.rpmlib
-import requests
+import koji
+import six.moves.xmlrpc_client as xmlrpclib
 
-from module_build_service import conf, log, models
+from module_build_service import conf, log, models, Modulemd, scm
 from module_build_service.builder.KojiModuleBuilder import (
     koji_retrying_multicall_map, KojiModuleBuilder,
 )
 from module_build_service.errors import UnprocessableEntity
 from module_build_service.resolver.base import GenericResolver
-from module_build_service.utils.request_utils import requests_session
+from module_build_service.utils import retry
 
 
 def add_default_modules(db_session, mmd, arches):
@@ -74,52 +77,28 @@ def add_default_modules(db_session, mmd, arches):
 
         bm_mmd = bm.mmd()
         bm_xmd = bm_mmd.get_xmd()
-        default_modules_url = bm_xmd.get("mbs", {}).get("default_modules_url")
-        if not default_modules_url:
-            log.debug("The base module %s does not have any default modules", bm_nsvc)
+        use_default_modules = bm_xmd.get("mbs", {}).get("use_default_modules", False)
+        default_modules_scm_url = bm_xmd.get("mbs", {}).get("default_modules_scm_url")
+        if not (use_default_modules or default_modules_scm_url):
+            log.info('The base module %s has no default modules', bm_mmd.get_nsvc())
             continue
 
-        try:
-            rv = requests_session.get(default_modules_url, timeout=10)
-        except requests.RequestException:
-            msg = (
-                "The connection failed when getting the default modules associated with {}"
-                .format(bm_nsvc)
-            )
-            log.exception(msg)
-            raise RuntimeError(msg)
-
-        if not rv.ok:
-            log.error(
-                "The request to get the default modules associated with %s failed with the status "
-                'code %d and error "%s"',
-                bm_nsvc, rv.status_code, rv.text,
-            )
-            raise RuntimeError(
-                "Failed to retrieve the default modules for {}".format(bm_mmd.get_nsvc())
-            )
-
-        default_modules = [m.strip() for m in rv.text.strip().split("\n")]
-        for default_module in default_modules:
-            try:
-                name, stream = default_module.split(":")
-            except ValueError:
-                log.error(
-                    'The default module "%s" from %s is in an invalid format',
-                    default_module, rv.url,
-                )
-                continue
-
+        # If the base module does not provide a default_modules_scm_url, use the default that is
+        # configured
+        default_modules_scm_url = default_modules_scm_url or conf.default_modules_scm_url
+        default_modules = _get_default_modules(bm.stream, default_modules_scm_url)
+        for name, stream in default_modules.items():
+            ns = "{}:{}".format(name, stream)
             if name in buildrequires:
                 conflicting_stream = buildrequires[name]["stream"]
                 if stream == conflicting_stream:
-                    log.info("The default module %s is already a buildrequire", default_module)
+                    log.info("The default module %s is already a buildrequire", ns)
                     continue
 
                 log.info(
                     "The default module %s will not be added as a buildrequire since %s:%s "
                     "is already a buildrequire",
-                    default_module, name, conflicting_stream,
+                    ns, name, conflicting_stream,
                 )
                 continue
 
@@ -131,12 +110,12 @@ def add_default_modules(db_session, mmd, arches):
                 # are aware of which modules are not in the database, and can add those that are as
                 # buildrequires.
                 resolver = GenericResolver.create(db_session, conf)
-                resolved = resolver.resolve_requires([default_module])
+                resolved = resolver.resolve_requires([ns])
             except UnprocessableEntity:
                 log.warning(
                     "The default module %s from %s is not in the database and couldn't be added as "
                     "a buildrequire",
-                    default_module, bm_nsvc
+                    ns, bm_nsvc,
                 )
                 continue
 
@@ -155,6 +134,80 @@ def add_default_modules(db_session, mmd, arches):
         _handle_collisions(mmd, arches)
 
 
+def _get_default_modules(stream, default_modules_scm_url):
+    """
+    Get the base module's default modules.
+
+    :param str stream: the stream of the base module
+    :param str default_modules_scm_url: the SCM URL to the default modules
+    :return: a dictionary where the keys are default module names and the values are default module
+        streams
+    :rtype: dict
+    :raise ValueError: if no default modules can be retrieved for that stream
+    """
+    scm_obj = scm.SCM(default_modules_scm_url)
+    temp_dir = tempfile.mkdtemp()
+    try:
+        log.debug("Cloning the default modules repo at %s", default_modules_scm_url)
+        scm_obj.clone(temp_dir)
+        log.debug("Checking out the branch %s", stream)
+        try:
+            scm_obj.checkout_ref(stream)
+        except UnprocessableEntity:
+            # If the checkout fails, try seeing if this is a rawhide build. In this case, the branch
+            # should actually be conf.rawhide_branch. The check to see if this is a rawhide build
+            # is done after the first checkout failure for performance reasons, since it avoids an
+            # unnecessary connection and query to Koji.
+            if conf.uses_rawhide:
+                log.debug(
+                    "Checking out the branch %s from the default modules repo failed. Trying to "
+                    "determine if this stream represents rawhide.",
+                    stream,
+                )
+                if _get_rawhide_version() == stream:
+                    log.debug(
+                        "The stream represents rawhide, will try checking out %s",
+                        conf.rawhide_branch,
+                    )
+                    # There's no try/except here because we want the outer except block to
+                    # catch this in the event the rawhide branch doesn't exist
+                    scm_obj.checkout_ref(conf.rawhide_branch)
+                else:
+                    # If it's not a rawhide build, then the branch should have existed
+                    raise
+            else:
+                # If it's not a rawhide build, then the branch should have existed
+                raise
+
+        idx = Modulemd.ModuleIndex.new()
+        idx.update_from_defaults_directory(
+            path=scm_obj.sourcedir,
+            overrides_path=os.path.join(scm_obj.sourcedir, "overrides"),
+            strict=True,
+        )
+        return idx.get_default_streams()
+    except:  # noqa: E722
+        msg = "Failed to retrieve the default modules"
+        log.exception(msg)
+        raise ValueError(msg)
+    finally:
+        shutil.rmtree(temp_dir)
+
+
+@retry(wait_on=(xmlrpclib.ProtocolError, koji.GenericError))
+def _get_rawhide_version():
+    """
+    Query Koji to find the rawhide version from the build target.
+
+    :return: the rawhide version (e.g. "f32")
+    :rtype: str
+    """
+    koji_session = KojiModuleBuilder.get_session(conf, login=False)
+    build_target = koji_session.getBuildTarget("rawhide")
+    if build_target:
+        return build_target["build_tag_name"].partition("-build")[0]
+
+
 def _handle_collisions(mmd, arches):
     """
     Find any RPMs in the buildrequired base modules that collide with the buildrequired modules.

module_build_service/scm.py

@@ -99,6 +99,7 @@ class SCM(object):
             self.commit = match.group("commit")
             self.branch = branch if branch else "master"
             self.version = None
+            self._cloned = False
         else:
             raise ValidationError("Unhandled SCM scheme: %s" % self.scheme)
 
@@ -158,6 +159,36 @@ class SCM(object):
     def _run(cmd, chdir=None, log_stdout=False):
         return SCM._run_without_retry(cmd, chdir, log_stdout)
 
+    def clone(self, scmdir):
+        """
+        Clone the repo from SCM.
+
+        :param str scmdir: the working directory
+        :raises UnprocessableEntity: if the clone fails
+        """
+        if self._cloned:
+            return
+
+        if not self.scheme == "git":
+            raise RuntimeError("clone: Unhandled SCM scheme.")
+
+        if not self.sourcedir:
+            self.sourcedir = os.path.join(scmdir, self.name)
+
+        module_clone_cmd = ["git", "clone", "-q", "--no-checkout", self.repository, self.sourcedir]
+        SCM._run(module_clone_cmd, chdir=scmdir)
+        self._cloned = True
+
+    def checkout_ref(self, ref):
+        """
+        Checkout the input reference.
+
+        :param str ref: the SCM reference (hash, branch, etc.) to check out
+        :raises UnprocessableEntity: if the checkout fails
+        """
+        module_checkout_cmd = ["git", "checkout", "-q", ref]
+        SCM._run(module_checkout_cmd, chdir=self.sourcedir)
+
     def checkout(self, scmdir):
         """Checkout the module from SCM.
 
@@ -167,17 +198,12 @@ class SCM(object):
         """
         # TODO: sanity check arguments
         if self.scheme == "git":
-            self.sourcedir = "%s/%s" % (scmdir, self.name)
+            if not self._cloned:
+                self.clone(scmdir)
 
-            module_clone_cmd = [
-                "git", "clone", "-q", "--no-checkout", self.repository, self.sourcedir
-            ]
-            module_checkout_cmd = ["git", "checkout", "-q", self.commit]
-            # perform checkouts
-            SCM._run(module_clone_cmd, chdir=scmdir)
             try:
-                SCM._run(module_checkout_cmd, chdir=self.sourcedir)
-            except RuntimeError as e:
+                self.checkout_ref(self.commit)
+            except UnprocessableEntity as e:
                 if (
                     e.message.endswith(' did not match any file(s) known to git.\\n"')
                     or "fatal: reference is not a tree: " in e.message

tests/test_scheduler/test_default_modules.py

@@ -19,22 +19,21 @@
 # SOFTWARE.
 from collections import namedtuple
 import errno
-import textwrap
 
 import dnf
-from mock import call, Mock, patch
+from mock import call, Mock, patch, PropertyMock
 import pytest
-import requests
 
+from module_build_service.errors import UnprocessableEntity
 from module_build_service.models import ModuleBuild
 from module_build_service.scheduler import default_modules
 from module_build_service.utils.general import load_mmd, mmd_to_str
-from tests import clean_database, make_module_in_db, read_staged_data
+from tests import clean_database, conf, make_module_in_db, read_staged_data
 
 
 @patch("module_build_service.scheduler.default_modules._handle_collisions")
-@patch("module_build_service.scheduler.default_modules.requests_session")
-def test_add_default_modules(mock_requests_session, mock_hc, db_session):
+@patch("module_build_service.scheduler.default_modules._get_default_modules")
+def test_add_default_modules(mock_get_dm, mock_hc, db_session):
     """
     Test that default modules present in the database are added, and the others are ignored.
     """
@@ -55,30 +54,29 @@ def test_add_default_modules(mock_requests_session, mock_hc, db_session):
     assert platform
     platform_mmd = platform.mmd()
     platform_xmd = mmd.get_xmd()
-    default_modules_url = "http://domain.local/default_modules.txt"
-    platform_xmd["mbs"]["default_modules_url"] = default_modules_url
+    platform_xmd["mbs"]["use_default_modules"] = True
     platform_mmd.set_xmd(platform_xmd)
     platform.modulemd = mmd_to_str(platform_mmd)
     db_session.commit()
 
-    mock_requests_session.get.return_value.ok = True
-    # Also ensure that if there's an invalid line, it's just ignored
-    mock_requests_session.get.return_value.text = textwrap.dedent("""\
-        nodejs:11
-        python:3
-        ruby:2.6
-        some invalid stuff
-    """)
+    mock_get_dm.return_value = {
+        "nodejs": "11",
+        "python": "3",
+        "ruby": "2.6",
+    }
     default_modules.add_default_modules(db_session, mmd, ["x86_64"])
     # Make sure that the default modules were added. ruby:2.6 will be ignored since it's not in
     # the database
     assert set(mmd.get_xmd()["mbs"]["buildrequires"].keys()) == {"nodejs", "platform", "python"}
-    mock_requests_session.get.assert_called_once_with(default_modules_url, timeout=10)
+    mock_get_dm.assert_called_once_with(
+        "f28",
+        "https://pagure.io/releng/fedora-module-defaults.git",
+    )
     mock_hc.assert_called_once()
 
 
-@patch("module_build_service.scheduler.default_modules.requests_session")
-def test_add_default_modules_not_linked(mock_requests_session, db_session):
+@patch("module_build_service.scheduler.default_modules._get_default_modules")
+def test_add_default_modules_not_linked(mock_get_dm, db_session):
     """
     Test that no default modules are added when they aren't linked from the base module.
     """
@@ -87,11 +85,10 @@ def test_add_default_modules_not_linked(mock_requests_session, db_session):
     assert set(mmd.get_xmd()["mbs"]["buildrequires"].keys()) == {"platform"}
     default_modules.add_default_modules(db_session, mmd, ["x86_64"])
     assert set(mmd.get_xmd()["mbs"]["buildrequires"].keys()) == {"platform"}
-    mock_requests_session.get.assert_not_called()
+    mock_get_dm.assert_not_called()
 
 
-@patch("module_build_service.scheduler.default_modules.requests_session")
-def test_add_default_modules_platform_not_available(mock_requests_session, db_session):
+def test_add_default_modules_platform_not_available(db_session):
     """
     Test that an exception is raised when the platform module that is buildrequired is missing.
 
@@ -105,11 +102,10 @@ def test_add_default_modules_platform_not_available(mock_requests_session, db_se
         default_modules.add_default_modules(db_session, mmd, ["x86_64"])
 
 
-@pytest.mark.parametrize("connection_error", (True, False))
-@patch("module_build_service.scheduler.default_modules.requests_session")
-def test_add_default_modules_request_failed(mock_requests_session, connection_error, db_session):
+@patch("module_build_service.scheduler.default_modules._get_default_modules")
+def test_add_default_modules_request_failed(mock_get_dm, db_session):
     """
-    Test that an exception is raised when the request to get the default modules failed.
+    Test that an exception is raised when the call to _get_default_modules failed.
     """
     clean_database()
     make_module_in_db("python:3:12345:1", db_session=db_session)
@@ -128,25 +124,101 @@ def test_add_default_modules_request_failed(mock_requests_session, connection_er
     assert platform
     platform_mmd = platform.mmd()
     platform_xmd = mmd.get_xmd()
-    default_modules_url = "http://domain.local/default_modules.txt"
-    platform_xmd["mbs"]["default_modules_url"] = default_modules_url
+    platform_xmd["mbs"]["use_default_modules"] = True
     platform_mmd.set_xmd(platform_xmd)
     platform.modulemd = mmd_to_str(platform_mmd)
     db_session.commit()
 
-    if connection_error:
-        mock_requests_session.get.side_effect = requests.ConnectionError("some error")
-        expected_error = (
-            "The connection failed when getting the default modules associated with "
-            "platform:f28:3:00000000"
+    expected_error = "some error"
+    mock_get_dm.side_effect = ValueError(expected_error)
+
+    with pytest.raises(ValueError, match=expected_error):
+        default_modules.add_default_modules(db_session, mmd, ["x86_64"])
+
+
+@pytest.mark.parametrize("is_rawhide", (True, False))
+@patch('shutil.rmtree')
+@patch('tempfile.mkdtemp')
+@patch("module_build_service.scheduler.default_modules.Modulemd.ModuleIndex.new")
+@patch("module_build_service.scheduler.default_modules.scm.SCM")
+@patch("module_build_service.scheduler.default_modules._get_rawhide_version")
+def test_get_default_modules(
+    mock_get_rawhide, mock_scm, mock_mmd_new, mock_mkdtemp, mock_rmtree, is_rawhide,
+):
+    """
+    Test that _get_default_modules returns the default modules.
+    """
+    mock_scm.return_value.sourcedir = "/some/path"
+    if is_rawhide:
+        mock_scm.return_value.checkout_ref.side_effect = [
+            UnprocessableEntity("invalid branch"),
+            None,
+        ]
+        mock_get_rawhide.return_value = "f32"
+
+    expected = {"nodejs": "11"}
+    mock_mmd_new.return_value.get_default_streams.return_value = expected
+
+    rv = default_modules._get_default_modules("f32", conf.default_modules_scm_url)
+
+    assert rv == expected
+    if is_rawhide:
+        mock_scm.return_value.checkout_ref.assert_has_calls(
+            [call("f32"), call(conf.rawhide_branch)]
         )
     else:
-        mock_requests_session.get.return_value.ok = False
-        mock_requests_session.get.return_value.text = "some error"
-        expected_error = "Failed to retrieve the default modules for platform:f28:3:00000000"
+        mock_scm.return_value.checkout_ref.assert_called_once_with("f32")
 
-    with pytest.raises(RuntimeError, match=expected_error):
-        default_modules.add_default_modules(db_session, mmd, ["x86_64"])
+
+@pytest.mark.parametrize("uses_rawhide", (True, False))
+@patch('shutil.rmtree')
+@patch('tempfile.mkdtemp')
+@patch(
+    "module_build_service.scheduler.default_modules.conf.uses_rawhide",
+    new_callable=PropertyMock,
+)
+@patch("module_build_service.scheduler.default_modules.Modulemd.ModuleIndex.new")
+@patch("module_build_service.scheduler.default_modules.scm.SCM")
+@patch("module_build_service.scheduler.default_modules._get_rawhide_version")
+def test_get_default_modules_invalid_branch(
+    mock_get_rawhide, mock_scm, mock_mmd_new, mock_uses_rawhide, mock_mkdtemp, mock_rmtree,
+    uses_rawhide,
+):
+    """
+    Test that _get_default_modules raises an exception with an invalid branch.
+    """
+    mock_uses_rawhide.return_value = uses_rawhide
+    mock_scm.return_value.sourcedir = "/some/path"
+    mock_scm.return_value.checkout_ref.side_effect = [
+        UnprocessableEntity("invalid branch"),
+        UnprocessableEntity("invalid branch"),
+    ]
+    if uses_rawhide:
+        mock_get_rawhide.return_value = "f32"
+    else:
+        mock_get_rawhide.return_value = "something_else"
+
+    with pytest.raises(ValueError, match="Failed to retrieve the default modules"):
+        default_modules._get_default_modules("f32", conf.default_modules_scm_url)
+
+    mock_mmd_new.assert_not_called()
+    if uses_rawhide:
+        mock_scm.return_value.checkout_ref.assert_has_calls(
+            [call("f32"), call(conf.rawhide_branch)],
+        )
+    else:
+        mock_scm.return_value.checkout_ref.assert_called_once_with("f32")
+
+
+@patch("module_build_service.scheduler.default_modules.KojiModuleBuilder")
+def test_get_rawhide_version(mock_koji_builder):
+    """
+    Test that _get_rawhide_version will return rawhide Fedora version.
+    """
+    mock_koji_builder.get_session.return_value.getBuildTarget.return_value = {
+        "build_tag_name": "f32-build",
+    }
+    assert default_modules._get_rawhide_version() == "f32"
 
 
 @patch("module_build_service.scheduler.default_modules.KojiModuleBuilder.get_session")