#1180 Find compatible base modules based on the virtual streams.
Merged 5 years ago by jkaluza. Opened 5 years ago by jkaluza.
jkaluza/fm-orchestrator compat-platforms  into  master

file modified
+27 -15
@@ -358,21 +358,33 @@ 

          # the minimal `stream_version` is 280000.

          min_stream_version = (stream_version // 10000) * 10000

  

-         # Prepare the subquery to find out all unique name:stream records.

-         subq = session.query(

-             func.max(sqlalchemy.cast(ModuleBuild.version, db.BigInteger)).label("maxversion")

-         ).filter_by(name=name, state=BUILD_STATES["ready"]).filter(

-             stream_version <= stream_version).filter(

-                 stream_version >= min_stream_version).subquery('t2')

- 

-         # Use the subquery to actually return all the columns for its results.

-         query = session.query(ModuleBuild).join(

-             subq, and_(

-                 ModuleBuild.name == name,

-                 ModuleBuild.stream_version <= stream_version,

-                 ModuleBuild.stream_version >= min_stream_version,

-                 sqlalchemy.cast(ModuleBuild.version, db.BigInteger) == subq.c.maxversion))

-         return query.all()

+         query = session.query(ModuleBuild)\

+             .filter(ModuleBuild.name == name)\

+             .filter(ModuleBuild.state == BUILD_STATES["ready"])\

+             .filter(ModuleBuild.stream_version <= stream_version)\

+             .filter(ModuleBuild.stream_version >= min_stream_version)\

+             .order_by(ModuleBuild.version.desc())

+         builds = query.all()

+ 

+         # In case there are multiple versions of single name:stream build, we want to return

+         # the latest version only. The `builds` are ordered by "version" desc, so we

+         # can just get the first (greatest) version of name:stream.

+         # TODO: Is there a way how to do that nicely in the SQL query itself?

+         seen = {}  # {"n:s": v, ...}

+         ret = []

+         for build in builds:

+             ns = "%s:%s" % (build.name, build.stream)

+             if ns in seen and seen[ns] != build.version:

+                 # Skip the builds if we already handled this nsv before.

+                 continue

+             elif ns in seen and seen[ns] == build.version:

+                 # Different context of the NSV

+                 ret.append(build)

+                 continue

+ 

+             seen[ns] = build.version

+             ret.append(build)

+         return ret

  

      @staticmethod

      def get_build_by_koji_tag(session, tag):

@@ -218,12 +218,53 @@ 

                  if ns in seen:

                      continue

  

+                 # Get the MMD for particular buildrequired name:stream to find out the virtual

+                 # streams according to which we can find the compatible streams later.

+                 # The returned MMDs are all the module builds for name:stream in the highest

+                 # version. Given the base module does not depend on other modules, it can appear

+                 # only in single context and therefore the `mmds` should always contain just

+                 # zero or one module build.

+                 mmds = resolver.get_module_modulemds(name, stream)

+                 if not mmds:

+                     continue

+                 stream_mmd = mmds[0]

+ 

+                 # In case there are no virtual_streams in the buildrequired name:stream,

+                 # it is clear that there are no compatible streams, so return just this

+                 # `stream_mmd`.

+                 xmd = stream_mmd.get_xmd()

+                 if "mbs" not in xmd.keys() or "virtual_streams" not in xmd["mbs"].keys():

+                     seen.add(ns)

+                     ret.append(stream_mmd)

+                     continue

+ 

+                 # Get the list of compatible virtual streams.

+                 virtual_streams = xmd["mbs"]["virtual_streams"]

+ 

                  mmds = resolver.get_module_modulemds(name, stream, stream_version_lte=True)

                  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

                      if mmd_ns not in seen:

file modified
+12 -1
@@ -126,7 +126,14 @@ 

          for stream in ["f28.0.0", "f29.0.0", "f29.1.0", "f29.2.0"]:

              mmd.set_name("platform")

              mmd.set_stream(stream)

+ 

+             # Set the virtual_streams based on "fXY" to mark the platform streams

+             # with the same major stream_version compatible.

+             xmd = glib.from_variant_dict(mmd.get_xmd())

+             xmd['mbs']['virtual_streams'] = [stream[:3]]

+             mmd.set_xmd(glib.dict_values(xmd))

              import_mmd(db.session, mmd)

+ 

              # Just to possibly confuse tests by adding another base module.

              mmd.set_name("bootstrap")

              mmd.set_stream(stream)
@@ -724,7 +731,7 @@ 

  

  

  def make_module(nsvc, requires_list=None, build_requires_list=None, base_module=None,

-                 filtered_rpms=None, xmd=None, store_to_db=True):

+                 filtered_rpms=None, xmd=None, store_to_db=True, virtual_streams=None):

      """

      Creates new models.ModuleBuild defined by `nsvc` string with requires

      and buildrequires set according to ``requires_list`` and ``build_requires_list``.
@@ -742,6 +749,7 @@ 

          default key/value pairs are added if not present.

      :param bool store_to_db: whether to store created module metadata to the

          database.

+     :param list virtual_streams: List of virtual streams provided by this module.

      :return: New Module Build if set to store module metadata to database,

          otherwise the module metadata is returned.

      :rtype: ModuleBuild or Modulemd.Module
@@ -795,6 +803,9 @@ 

      if 'mse' not in xmd_mbs:

          xmd_mbs['mse'] = 'true'

  

+     if virtual_streams:

+         xmd_mbs['virtual_streams'] = virtual_streams

+ 

      mmd.set_xmd(glib.dict_values(xmd))

  

      if not store_to_db:

@@ -24,7 +24,7 @@ 

  from module_build_service.utils import to_text_type

  

  from tests.test_models import init_data, module_build_from_modulemd

- from tests import (init_data as init_data_contexts, clean_database)

+ from tests import (init_data as init_data_contexts, clean_database, make_module)

  from module_build_service import conf, Modulemd

  from module_build_service.models import ComponentBuild, ModuleBuild, make_session

  
@@ -142,3 +142,25 @@ 

              builds = set(["%s:%s:%s:%s" % (build.name, build.stream, str(build.version),

                                             build.context) for build in builds])

              assert builds == set(['platform:f29.0.0:3:00000000', 'platform:f29.1.0:3:00000000'])

+ 

+     def test_get_last_builds_in_stream_version_lte_different_versions(self):

+         """

+         Tests that get_last_builds_in_stream_version_lte works in case the

+         name:stream_ver modules have different versions.

+         """

+         clean_database(False)

+         make_module("platform:f29.1.0:10:old_version", {}, {}, virtual_streams=["f29"])

+         make_module("platform:f29.1.0:15:c11.another", {}, {}, virtual_streams=["f29"])

+         make_module("platform:f29.1.0:15:c11", {}, {}, virtual_streams=["f29"])

+         make_module("platform:f29.2.0:0:old_version", {}, {}, virtual_streams=["f29"])

+         make_module("platform:f29.2.0:1:c11", {}, {}, virtual_streams=["f29"])

+         make_module("platform:f29.3.0:15:old_version", {}, {}, virtual_streams=["f29"])

+         make_module("platform:f29.3.0:20:c11", {}, {}, virtual_streams=["f29"])

+ 

+         with make_session(conf) as session:

+             builds = ModuleBuild.get_last_builds_in_stream_version_lte(

+                 session, "platform", 290200)

+             builds = set(["%s:%s:%s:%s" % (build.name, build.stream, str(build.version),

+                                            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'])

@@ -337,9 +337,9 @@ 

          and lorem:1 modules which require base:f29 module requiring

          platform:f29 module :).

          """

-         f290000 = make_module("platform:f29.0.0:0:c11", {}, {})

-         f290100 = make_module("platform:f29.1.0:0:c11", {}, {})

-         f290200 = make_module("platform:f29.2.0:0:c11", {}, {})

+         f290000 = make_module("platform:f29.0.0:0:c11", {}, {}, virtual_streams=["f29"])

+         f290100 = make_module("platform:f29.1.0:0:c11", {}, {}, virtual_streams=["f29"])

+         f290200 = make_module("platform:f29.2.0:0:c11", {}, {}, virtual_streams=["f29"])

          make_module("gtk:1:0:c2", {"platform": ["f29"]}, {}, f290000)

          make_module("gtk:1:1:c2", {"platform": ["f29"]}, {}, f290100)

          make_module("gtk:1:2:c2", {"platform": ["f29"]}, {}, f290100)
@@ -362,7 +362,7 @@ 

              os.path.join(base_dir, 'staged_data', 'testmodule_v2.yaml'), True)

          deps = mmd.get_dependencies()

          brs = deps[0].get_buildrequires()

-         brs['platform'].set(['platform:f29.1.0', 'platform:f29.2.0'])

+         brs['platform'].set(['f29.1.0', 'f29.2.0'])

          deps[0].set_buildrequires(brs)

          mmd.set_dependencies(deps)

  
@@ -375,3 +375,31 @@ 

          for mmd_ in mmds:

              actual.add('{}:{}'.format(mmd_.get_name(), mmd_.get_stream()))

          assert actual == expected

+ 

+     @pytest.mark.parametrize('virtual_streams', (None, ["f29"], ["lp29"]))

+     def test__get_base_module_mmds_virtual_streams(self, virtual_streams):

+         """Ensure the correct results are returned without duplicates."""

+         init_data(data_size=1, multiple_stream_versions=True)

+         mmd = module_build_service.utils.load_mmd(

+             os.path.join(base_dir, 'staged_data', 'testmodule_v2.yaml'), True)

+         deps = mmd.get_dependencies()

+         brs = deps[0].get_buildrequires()

+         brs['platform'].set(['f29.2.0'])

+         deps[0].set_buildrequires(brs)

+         mmd.set_dependencies(deps)

+ 

+         make_module("platform:lp29.1.1:12:c11", {}, {}, virtual_streams=virtual_streams)

+ 

+         mmds = module_build_service.utils.mse._get_base_module_mmds(mmd)

+         if virtual_streams == ["f29"]:

+             expected = set(['platform:f29.0.0', 'platform:f29.1.0', 'platform:f29.2.0',

+                             'platform:lp29.1.1'])

+         else:

+             expected = set(['platform:f29.0.0', 'platform:f29.1.0', 'platform:f29.2.0'])

+         # Verify no duplicates were returned before doing set operations

+         assert len(mmds) == len(expected)

+         # Verify the expected ones were returned

+         actual = set()

+         for mmd_ in mmds:

+             actual.add('{}:{}'.format(mmd_.get_name(), mmd_.get_stream()))

+         assert actual == expected

@@ -1289,6 +1289,9 @@ 

      @patch('module_build_service.auth.get_user', return_value=user)

      @patch('module_build_service.scm.SCM')

      def test_submit_build_invalid_basemodule_stream(self, mocked_scm, mocked_get_user):

+         # By default tests do not provide platform:f28.0.0, but just platform:f28.

+         # Therefore we want to enable multiple_stream_versions.

+         init_data(2, multiple_stream_versions=True)

          FakeSCM(mocked_scm, 'testmodule', 'testmodule.yaml',

                  '620ec77321b2ea7b0d67d82992dda3e1d67055b4')

  
@@ -1302,12 +1305,12 @@ 

          rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps(data))

          result = json.loads(rv.data)

          assert result == {

-             'error': 'Bad Request',

-             'status': 400,

-             'message': ('No dependency combination was satisfied. Please verify the '

-                         'buildrequires in your modulemd have previously been built.')

+             'error': 'Unprocessable Entity',

+             'status': 422,

+             'message': ('None of the base module (platform) streams in the buildrequires '

+                         'section could be found')

          }

-         assert rv.status_code == 400

+         assert rv.status_code == 422

  

      @patch('module_build_service.auth.get_user', return_value=user)

      @patch('module_build_service.scm.SCM')

Before this commit, the compatible base modules for Module Stream Expansion
have been found without any limitation, just based on the stream version.
It was therefore possible that platform:lp29.0.0 was found as compatible
module for platform:f29.1.0 although those platform streams are not
compatible at all.

In this commit, the module can be treated as compatible only if it has
the same virtual stream as the input module. The idea behind this
is that both platform:f29.0.0 and platform:f29.1.0 should include
the virtual_streams: [f29] in their XMD section which tells MBS
that they are actually compatible. The lp29 stream will not
have the same virtual stream (most likely it won't have any virtual
stream at all).

The virtual_streams is already used for this use-case in MMDResolver,
but it was not used to limit the inputs to MMDResolver which is what
this commit is doing.

This commit also fixes the issue in get_last_builds_in_stream_version_lte
which was simply broken if multiple stream_versions of single base module
existed and their builds had different version. In this case, only
builds with single (randomly chosen) version were returned.

Could you add a comment stating this is the latest module with this name and stream? Otherwise it's not clear why this arbitrarily gets used to append to ret on line 18 of the diff.

:thumbsup: after fixing the comments

rebased onto 780ed11

5 years ago

Pull-Request has been merged by jkaluza

5 years ago