From 00daedccfd970b65fa8ca032b6fe551ed920018c Mon Sep 17 00:00:00 2001 From: mprahl Date: Mon, 22 Apr 2019 11:04:11 -0400 Subject: [PATCH] Allow the virtual streams of a base module to be queryable in the database and API --- .../6d503efcd2b8_virtual_streams_table.py | 111 ++++++++++++++++++ module_build_service/models.py | 62 +++++++++- module_build_service/resolver/DBResolver.py | 6 +- module_build_service/resolver/MBSResolver.py | 10 +- module_build_service/resolver/base.py | 2 +- module_build_service/utils/general.py | 30 ++++- module_build_service/utils/mse.py | 21 +--- module_build_service/utils/views.py | 7 +- tests/__init__.py | 16 ++- tests/test_models/test_models.py | 13 ++ tests/test_resolver/test_mbs.py | 7 +- tests/test_views/test_views.py | 31 ++++- 12 files changed, 281 insertions(+), 35 deletions(-) create mode 100644 module_build_service/migrations/versions/6d503efcd2b8_virtual_streams_table.py diff --git a/module_build_service/migrations/versions/6d503efcd2b8_virtual_streams_table.py b/module_build_service/migrations/versions/6d503efcd2b8_virtual_streams_table.py new file mode 100644 index 00000000..ec921369 --- /dev/null +++ b/module_build_service/migrations/versions/6d503efcd2b8_virtual_streams_table.py @@ -0,0 +1,111 @@ +"""Add a table for virtual streams + +Revision ID: 6d503efcd2b8 +Revises: 79babdadc942 +Create Date: 2019-04-23 19:28:52.206109 + +""" + +# revision identifiers, used by Alembic. +revision = '6d503efcd2b8' +down_revision = '79babdadc942' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.orm import relationship +from sqlalchemy.ext.declarative import declarative_base + +# Data migration imports +from module_build_service import Modulemd, conf + + +Base = declarative_base() + +# Define the tables for the data migration +module_builds_to_virtual_streams = sa.Table( + 'module_builds_to_virtual_streams', + Base.metadata, + sa.Column('module_build_id', sa.Integer, sa.ForeignKey('module_builds.id')), + sa.Column('virtual_stream_id', sa.Integer, sa.ForeignKey('virtual_streams.id')), +) + + +class ModuleBuild(Base): + __tablename__ = "module_builds" + id = sa.Column(sa.Integer, primary_key=True) + modulemd = sa.Column(sa.String, nullable=False) + name = sa.Column(sa.String, nullable=False) + virtual_streams = relationship( + 'VirtualStream', secondary=module_builds_to_virtual_streams, back_populates='module_builds' + ) + + +class VirtualStream(Base): + __tablename__ = 'virtual_streams' + id = sa.Column(sa.Integer, primary_key=True) + name = sa.Column(sa.String, nullable=False, unique=True) + module_builds = relationship( + 'ModuleBuild', secondary=module_builds_to_virtual_streams, back_populates='virtual_streams' + ) + + +def upgrade(): + op.create_table( + 'virtual_streams', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('name', sa.String(), nullable=False), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('name'), + ) + op.create_table( + 'module_builds_to_virtual_streams', + sa.Column('module_build_id', sa.Integer(), nullable=False), + sa.Column('virtual_stream_id', sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(['module_build_id'], ['module_builds.id']), + sa.ForeignKeyConstraint(['virtual_stream_id'], ['virtual_streams.id']), + sa.UniqueConstraint( + 'module_build_id', 'virtual_stream_id', name='unique_module_to_virtual_stream' + ), + ) + + bind = op.get_bind() + session = sa.orm.Session(bind=bind) + for build in session.query(ModuleBuild).all(): + # Only process base modules with modulemds set + if build.name not in conf.base_module_names or not build.modulemd: + continue + + try: + mmd = Modulemd.Module().new_from_string(build.modulemd) + mmd.upgrade() + except Exception: + # If the modulemd isn't parseable, then skip this build + continue + + try: + virtual_streams = mmd.get_xmd()['mbs']['virtual_streams'] + except KeyError: + # If there are no virtual_streams configured, then just skip this build + continue + + for virtual_stream in virtual_streams: + virtual_stream_obj = session.query(VirtualStream).filter_by(name=virtual_stream).first() + # Create the virtual stream entry if it doesn't exist + if not virtual_stream_obj: + virtual_stream_obj = VirtualStream(name=virtual_stream) + session.add(virtual_stream_obj) + session.commit() + + if virtual_stream_obj not in build.virtual_streams: + build.virtual_streams.append(virtual_stream_obj) + session.add(build) + + session.commit() + + # Always close the transaction + session.commit() + + +def downgrade(): + op.drop_table('module_builds_to_virtual_streams') + op.drop_table('virtual_streams') diff --git a/module_build_service/models.py b/module_build_service/models.py index 9cbc4480..5a27fdec 100644 --- a/module_build_service/models.py +++ b/module_build_service/models.py @@ -182,6 +182,15 @@ module_builds_to_module_buildrequires = db.Table( ) +module_builds_to_virtual_streams = db.Table( + 'module_builds_to_virtual_streams', + db.Column('module_build_id', db.Integer, db.ForeignKey('module_builds.id'), nullable=False), + db.Column('virtual_stream_id', db.Integer, db.ForeignKey('virtual_streams.id'), nullable=False), + db.UniqueConstraint( + 'module_build_id', 'virtual_stream_id', name='unique_module_to_virtual_stream') +) + + class ModuleBuild(MBSBase): __tablename__ = "module_builds" id = db.Column(db.Integer, primary_key=True) @@ -208,6 +217,11 @@ class ModuleBuild(MBSBase): time_completed = db.Column(db.DateTime) new_repo_task_id = db.Column(db.Integer) rebuild_strategy = db.Column(db.String, nullable=False) + virtual_streams = db.relationship( + 'VirtualStream', + secondary=module_builds_to_virtual_streams, + back_populates='module_builds', + ) # A monotonically increasing integer that represents which batch or # iteration this module is currently on for successive rebuilds of its @@ -318,20 +332,24 @@ class ModuleBuild(MBSBase): return query @staticmethod - def get_last_builds_in_stream(session, name, stream, **kwargs): + def get_last_builds_in_stream(session, name, stream, virtual_streams=None, **kwargs): """ Returns the latest builds in "ready" state for given name:stream. :param session: SQLAlchemy session. :param str name: Name of the module to search builds for. :param str stream: Stream of the module to search builds for. + :param list virtual_streams: a list of the virtual streams to filter on. The filtering uses + "or" logic. When falsy, no filtering occurs. :param dict kwargs: Key/value pairs passed to SQLAlchmey filter_by method allowing to set additional filter for results. """ # Prepare the subquery to find out all unique name:stream records. - return ModuleBuild._get_last_builds_in_stream_query(session, name, stream, **kwargs).all() + query = ModuleBuild._get_last_builds_in_stream_query(session, name, stream, **kwargs) + query = ModuleBuild._add_virtual_streams_filter(session, query, virtual_streams) + return query.all() @staticmethod def get_last_build_in_stream(session, name, stream, **kwargs): @@ -385,7 +403,27 @@ class ModuleBuild(MBSBase): .filter(ModuleBuild.stream_version >= min_stream_version) @staticmethod - def get_last_builds_in_stream_version_lte(session, name, stream_version): + def _add_virtual_streams_filter(session, query, virtual_streams): + """ + Adds a filter on ModuleBuild.virtual_streams to an existing query. + + :param session: a SQLAlchemy session + :param query: a SQLAlchemy query to add the filtering to + :param list virtual_streams: a list of the virtual streams to filter on. The filtering uses + "or" logic. When falsy, no filtering occurs. + :return: the query with the added virtual stream filters + """ + if not virtual_streams: + return query + + return query.join( + VirtualStream, ModuleBuild.virtual_streams + ).filter( + VirtualStream.name.in_(virtual_streams) + ).distinct(ModuleBuild.id) + + @staticmethod + def get_last_builds_in_stream_version_lte(session, name, stream_version, virtual_streams=None): """ Returns the latest builds in "ready" state for given name:stream limited by `stream_version`. The `stream_version` is int generated by `get_stream_version(...)` @@ -396,6 +434,8 @@ class ModuleBuild(MBSBase): :param session: SQLAlchemy session. :param str name: Name of the module to search builds for. :param int stream_version: Maximum stream_version to search builds for. + :param list virtual_streams: A list of the virtual streams to filter on. The filtering uses + "or" logic. When falsy, no filtering occurs. """ query = session.query(ModuleBuild)\ .filter(ModuleBuild.name == name)\ @@ -403,6 +443,7 @@ class ModuleBuild(MBSBase): .order_by(ModuleBuild.version.desc()) query = ModuleBuild._add_stream_version_lte_filter(session, query, stream_version) + query = ModuleBuild._add_virtual_streams_filter(session, query, virtual_streams) builds = query.all() @@ -762,6 +803,7 @@ class ModuleBuild(MBSBase): } for record in self.state_trace(self.id) ], 'state_url': state_url, + 'virtual_streams': [virtual_stream.name for virtual_stream in self.virtual_streams], }) return rv @@ -878,6 +920,20 @@ class ModuleBuild(MBSBase): INVERSE_BUILD_STATES[self.state], self.batch, self.state_reason)) +class VirtualStream(MBSBase): + __tablename__ = 'virtual_streams' + id = db.Column(db.Integer, primary_key=True) + name = db.Column(db.String, nullable=False, unique=True) + module_builds = db.relationship( + 'ModuleBuild', + secondary=module_builds_to_virtual_streams, + back_populates='virtual_streams', + ) + + def __repr__(self): + return ''.format(self.id, self.name) + + class ModuleBuildTrace(MBSBase): __tablename__ = "module_builds_trace" id = db.Column(db.Integer, primary_key=True) diff --git a/module_build_service/resolver/DBResolver.py b/module_build_service/resolver/DBResolver.py index 4221c0a5..074b94b9 100644 --- a/module_build_service/resolver/DBResolver.py +++ b/module_build_service/resolver/DBResolver.py @@ -54,7 +54,7 @@ class DBResolver(GenericResolver): 'Cannot find any module builds for %s:%s' % (name, stream)) def get_module_modulemds(self, name, stream, version=None, context=None, strict=False, - stream_version_lte=False): + stream_version_lte=False, virtual_streams=None): """ Gets the module modulemds from the resolver. :param name: a string of the module's name @@ -68,6 +68,8 @@ class DBResolver(GenericResolver): :kwarg stream_version_lte: If True and if the `stream` can be transformed to "stream version", the returned list will include all the modules with stream version less than or equal the stream version computed from `stream`. + :kwarg virtual_streams: a list of the virtual streams to filter on. The filtering uses "or" + logic. When falsy, no filtering occurs. :return: List of Modulemd metadata instances matching the query """ from module_build_service.utils import load_mmd @@ -83,7 +85,7 @@ class DBResolver(GenericResolver): stream, right_pad=False))) >= 5): stream_version = models.ModuleBuild.get_stream_version(stream) builds = models.ModuleBuild.get_last_builds_in_stream_version_lte( - session, name, stream_version) + session, name, stream_version, virtual_streams) else: builds = models.ModuleBuild.get_last_builds_in_stream( session, name, stream) diff --git a/module_build_service/resolver/MBSResolver.py b/module_build_service/resolver/MBSResolver.py index f61faeaf..e7aff688 100644 --- a/module_build_service/resolver/MBSResolver.py +++ b/module_build_service/resolver/MBSResolver.py @@ -128,7 +128,7 @@ class MBSResolver(GenericResolver): return rv[0] def get_module_modulemds(self, name, stream, version=None, context=None, strict=False, - stream_version_lte=False): + stream_version_lte=False, virtual_streams=None): """ Gets the module modulemds from the resolver. :param name: a string of the module's name @@ -139,6 +139,11 @@ class MBSResolver(GenericResolver): be returned. :kwarg strict: Normally this function returns [] if no module can be found. If strict=True, then a UnprocessableEntity is raised. + :kwarg stream_version_lte: If True and if the `stream` can be transformed to + "stream version", the returned list will include all the modules with stream version + less than or equal the stream version computed from `stream`. + :kwarg virtual_streams: a list of the virtual streams to filter on. The filtering uses "or" + logic. When falsy, no filtering occurs. :return: List of Modulemd metadata instances matching the query """ yaml = None @@ -153,6 +158,9 @@ class MBSResolver(GenericResolver): stream_version = models.ModuleBuild.get_stream_version(stream) extra_args["stream_version_lte"] = stream_version + if virtual_streams: + extra_args["virtual_stream"] = virtual_streams + modules = self._get_modules(name, stream, version, context, strict=strict, **extra_args) if not modules: return [] diff --git a/module_build_service/resolver/base.py b/module_build_service/resolver/base.py index c07c76ec..c9ca54c2 100644 --- a/module_build_service/resolver/base.py +++ b/module_build_service/resolver/base.py @@ -107,7 +107,7 @@ class GenericResolver(six.with_metaclass(ABCMeta)): @abstractmethod def get_module_modulemds(self, name, stream, version=None, context=None, strict=False, - stream_version_lte=None): + stream_version_lte=None, virtual_streams=None): raise NotImplementedError() @abstractmethod diff --git a/module_build_service/utils/general.py b/module_build_service/utils/general.py index 492569e0..9ab7d622 100644 --- a/module_build_service/utils/general.py +++ b/module_build_service/utils/general.py @@ -27,7 +27,7 @@ import inspect import hashlib import time from datetime import datetime -from six import text_type +from six import text_type, string_types from module_build_service import conf, log, models, Modulemd, glib from module_build_service.errors import ( @@ -347,6 +347,20 @@ def import_mmd(session, mmd, check_buildrequires=True): except (ValueError, KeyError): disttag_marking = None + try: + virtual_streams = mmd.get_xmd()["mbs"]["virtual_streams"] + except (ValueError, KeyError): + virtual_streams = [] + + # Verify that the virtual streams are the correct type + if virtual_streams and ( + not isinstance(virtual_streams, list) or + any(not isinstance(vs, string_types) for vs in virtual_streams) + ): + msg = "The virtual streams must be a list of strings" + log.error(msg) + raise UnprocessableEntity(msg) + # If it is a base module, then make sure the value that will be used in the RPM disttags # doesn't contain a dash since a dash isn't allowed in the release field of the NVR if name in conf.base_module_names: @@ -426,6 +440,20 @@ def import_mmd(session, mmd, check_buildrequires=True): session.add(build) session.commit() + + for virtual_stream in virtual_streams: + vs_obj = session.query(models.VirtualStream).filter_by(name=virtual_stream).first() + if not vs_obj: + vs_obj = models.VirtualStream(name=virtual_stream) + session.add(vs_obj) + session.commit() + + if vs_obj not in build.virtual_streams: + build.virtual_streams.append(vs_obj) + session.add(build) + + session.commit() + msg = "Module {} imported".format(nsvc) log.info(msg) msgs.append(msg) diff --git a/module_build_service/utils/mse.py b/module_build_service/utils/mse.py index a294f4b5..33453957 100644 --- a/module_build_service/utils/mse.py +++ b/module_build_service/utils/mse.py @@ -242,29 +242,12 @@ def _get_base_module_mmds(mmd): # Get the list of compatible virtual streams. virtual_streams = xmd["mbs"]["virtual_streams"] - mmds = resolver.get_module_modulemds(name, stream, stream_version_lte=True) + mmds = resolver.get_module_modulemds( + name, stream, stream_version_lte=True, virtual_streams=virtual_streams) ret_chunk = [] # Add the returned mmds to the `seen` set to avoid querying those individually if # they are part of the buildrequire streams for this base module for mmd_ in mmds: - mmd_ns = ":".join([mmd_.get_name(), mmd_.get_stream()]) - xmd = mmd_.get_xmd() - if "mbs" not in xmd.keys() or "virtual_streams" not in xmd["mbs"].keys(): - # This module does not contain any virtual stream, so it cannot - # be compatible with our buildrequired module. - continue - - # Check if some virtual stream from buildrequired module exists in - # this module. That would mean the modules are compatible. - mmd_virtual_streams = xmd["mbs"]["virtual_streams"] - for virtual_stream in virtual_streams: - if virtual_stream in mmd_virtual_streams: - break - else: - # No stream from `virtual_streams` exist in `mmd_virtual_streams`, so this - # module is not compatible with our buildrequired module. - continue - mmd_ns = ":".join([mmd_.get_name(), mmd_.get_stream()]) # An extra precaution to ensure there are no duplicates in the event the sorting # above wasn't flawless diff --git a/module_build_service/utils/views.py b/module_build_service/utils/views.py index 676dd54e..86f7452e 100644 --- a/module_build_service/utils/views.py +++ b/module_build_service/utils/views.py @@ -207,7 +207,8 @@ def filter_module_builds(flask_request): """ search_query = dict() special_columns = set(( - 'time_submitted', 'time_modified', 'time_completed', 'state', 'stream_version_lte',)) + 'time_submitted', 'time_modified', 'time_completed', 'state', 'stream_version_lte', + 'virtual_stream',)) columns = models.ModuleBuild.__table__.columns.keys() for key in set(request.args.keys()) - special_columns: # Only filter on valid database columns but skip columns that are treated specially or @@ -283,6 +284,10 @@ def filter_module_builds(flask_request): elif context == 'before': query = query.filter(column <= item_datetime) + # Multiple virtual_streams can be supplied for "or" logic filtering + virtual_streams = flask_request.args.getlist('virtual_stream') + query = models.ModuleBuild._add_virtual_streams_filter(db.session, query, virtual_streams) + stream_version_lte = flask_request.args.get('stream_version_lte') if stream_version_lte is not None: invalid_error = ('An invalid value of stream_version_lte was provided. It must be an ' diff --git a/tests/__init__.py b/tests/__init__.py index 7167252f..a6a21d83 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -33,7 +33,9 @@ import module_build_service from module_build_service import db from module_build_service.utils import get_rpm_release, import_mmd from module_build_service.config import init_config -from module_build_service.models import ModuleBuild, ComponentBuild, make_session, BUILD_STATES +from module_build_service.models import ( + ModuleBuild, ComponentBuild, VirtualStream, make_session, BUILD_STATES, +) from module_build_service import glib, Modulemd @@ -791,4 +793,16 @@ def make_module(nsvc, requires_list=None, build_requires_list=None, base_module= db.session.add(module_build) db.session.commit() + if virtual_streams: + for virtual_stream in virtual_streams: + vs_obj = db.session.query(VirtualStream).filter_by(name=virtual_stream).first() + if not vs_obj: + vs_obj = VirtualStream(name=virtual_stream) + db.session.add(vs_obj) + db.session.commit() + + if vs_obj not in module_build.virtual_streams: + module_build.virtual_streams.append(vs_obj) + db.session.commit() + return module_build diff --git a/tests/test_models/test_models.py b/tests/test_models/test_models.py index 1bec6cd9..ac1e3191 100644 --- a/tests/test_models/test_models.py +++ b/tests/test_models/test_models.py @@ -172,3 +172,16 @@ class TestModelsGetStreamsContexts: build.context) for build in builds]) assert builds == set(['platform:f29.1.0:15:c11', 'platform:f29.1.0:15:c11.another', 'platform:f29.2.0:1:c11']) + + def test_add_virtual_streams_filter(self): + clean_database(False) + make_module("platform:f29.1.0:10:c1", {}, {}, virtual_streams=["f29"]) + make_module("platform:f29.1.0:15:c1", {}, {}, virtual_streams=["f29"]) + make_module("platform:f29.3.0:15:old_version", {}, {}, virtual_streams=["f28", "f29"]) + make_module("platform:f29.3.0:20:c11", {}, {}, virtual_streams=["f30"]) + + with make_session(conf) as session: + query = session.query(ModuleBuild).filter_by(name="platform") + query = ModuleBuild._add_virtual_streams_filter(session, query, ["f28", "f29"]) + count = query.count() + assert count == 3 diff --git a/tests/test_resolver/test_mbs.py b/tests/test_resolver/test_mbs.py index d190e378..5d4a2acb 100644 --- a/tests/test_resolver/test_mbs.py +++ b/tests/test_resolver/test_mbs.py @@ -55,8 +55,8 @@ class TestMBSModule: mock_session().get.return_value = mock_res resolver = mbs_resolver.GenericResolver.create(tests.conf, backend='mbs') - module_mmds = resolver.get_module_modulemds('testmodule', 'master', '20180205135154', - '9c690d0e') + module_mmds = resolver.get_module_modulemds( + 'testmodule', 'master', '20180205135154', '9c690d0e', virtual_streams=["f28"]) nsvcs = set( '{}:{}:{}:{}'.format(m.peek_name(), m.peek_stream(), m.peek_version(), m.peek_context()) @@ -73,7 +73,8 @@ class TestMBSModule: "order_desc_by": "version", "page": 1, "per_page": 10, - "state": "ready" + "state": "ready", + "virtual_stream": ["f28"], } mock_session().get.assert_called_once_with(mbs_url, params=expected_query) assert nsvcs == expected diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 4ef66ed5..62bdd2cf 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -191,6 +191,7 @@ class TestViews: assert data['modulemd'] == to_text_type(mmd.read()) assert data['name'] == 'nginx' assert data['owner'] == 'Moe Szyslak' + assert data['rebuild_strategy'] == 'changed-and-after' assert data['scmurl'] == ('git://pkgs.domain.local/modules/nginx' '?#ba95886c7a443b36a9ce31abda1f9bef22f2f8c9') assert data['scratch'] is False @@ -224,8 +225,7 @@ class TestViews: assert data['time_modified'] == u'2016-09-03T11:25:32Z' assert data['time_submitted'] == u'2016-09-03T11:23:20Z' assert data['version'] == '2' - assert data['rebuild_strategy'] == 'changed-and-after' - assert data['siblings'] == [] + assert data['virtual_streams'] == [] def test_query_build_with_br_verbose_mode(self): reuse_component_init_data() @@ -674,6 +674,31 @@ class TestViews: elif stream_version_lte == '293000': assert total == 3 + @pytest.mark.parametrize('virtual_streams', ([], ('f28',), ('f29',), ('f28', 'f29'))) + def test_query_builds_filter_virtual_streams(self, virtual_streams): + # Populate some platform modules with virtual streams + init_data(data_size=1, multiple_stream_versions=True) + url = '/module-build-service/1/module-builds/?name=platform&verbose=true' + for virtual_stream in virtual_streams: + url += '&virtual_stream={}'.format(virtual_stream) + rv = self.client.get(url) + data = json.loads(rv.data) + total = data['meta']['total'] + if virtual_streams == ('f28',): + assert total == 1 + for module in data['items']: + assert module['virtual_streams'] == ['f28'] + elif virtual_streams == ('f29',): + assert total == 3 + for module in data['items']: + assert module['virtual_streams'] == ['f29'] + elif virtual_streams == ('f28', 'f29'): + assert total == 4 + for module in data['items']: + assert len(set(module['virtual_streams']) - set(['f28', 'f29'])) == 0 + elif len(virtual_streams) == 0: + assert total == 5 + def test_query_builds_order_by(self): build = db.session.query(module_build_service.models.ModuleBuild).filter_by(id=2).one() build.name = 'candy' @@ -1974,7 +1999,7 @@ class TestViews: platform_mmd.set_stream(platform_override) if platform_override == 'el8.0.0': xmd = from_variant_dict(platform_mmd.get_xmd()) - xmd['mbs']['virtual_streams'] = 'el8' + xmd['mbs']['virtual_streams'] = ['el8'] platform_mmd.set_xmd(dict_values(xmd)) import_mmd(db.session, platform_mmd)