odcs

Clone

#490 Improve dependency resolution for modules
Merged 3 years ago by lsedlar. Opened 3 years ago by lsedlar.
lsedlar/odcs module-deps-base-stream  into  master

file modified
+3 -1 
@@ -432,7 +432,9 @@ 
 

              )
 

  

          expand = not compose.flags & COMPOSE_FLAGS["no_deps"]
 

-         new_mbs_modules = mbs.validate_module_list(specified_mbs_modules, expand=expand)
 

+         new_mbs_modules = mbs.validate_module_list(
 

+             specified_mbs_modules, expand=expand, base_modules=conf.base_module_names
 

+         )
 

  

          uids = sorted(
 

              "{name}:{stream}:{version}:{context}".format(**m)

file modified
+32 -4 
@@ -20,6 +20,7 @@ 
 

  # SOFTWARE.
 

  #
 

  

+ import re
 

  import requests
 

  from collections import defaultdict
 

  
@@ -153,13 +154,15 @@ 
 

              ret.append(module)
 

          return ret
 

  

-     def _add_new_dependencies(self, module_map, modules):
 

+     def _add_new_dependencies(self, module_map, modules, base_modules=None):
 

          """
 

          Helper for ``validate_module_list()`` - scans ``modules`` and adds any missing
 

          requirements to ``module_map``.
 

  

          :param module_map: dict mapping module name:stream to module.
 

          :param modules: the list of modules to scan for dependencies.
 

+         :param base_modules: a list of names for base modules; their
 

+                 dependencies should not be analyzed
 

          :return: a list of any modules that were added to ``module_map``.
 

          """
 

  
@@ -170,22 +173,45 @@ 
 

              )
 

              mmd = mmd.upgrade(2)
 

  

+             min_bound = 0
 

+             base_module_stream_version = None
 

+             for base_module in module["base_module_buildrequires"]:
 

+                 base_module_stream_version = base_module["stream_version"]
 

+                 # Let's extract the major version from the stream.
 

+                 match = re.search("^[^0-9]*([0-9]+)", base_module["stream"])
 

+                 if match:
 

+                     major_version = match.group(1)
 

+                     # The lower bound is based on major version extract from
 

+                     # the stream name, and padded with zeros to be the same
 

+                     # size as the integer part of the original stream version.
 

+                     min_bound = major_version + "0" * (
 

+                         len(str(int(base_module_stream_version))) - len(major_version)
 

+                     )
 

+ 

              # Check runtime dependency (name:stream) of a module and if this
 

              # dependency is already in module_map/new_modules, do nothing.
 

              # But otherwise get the latest module in this name:stream from MBS
 

              # and add it to new_modules/module_map.
 

              for deps in mmd.get_dependencies():
 

                  for name in deps.get_runtime_modules():
 

+                     if base_modules and name in base_modules:
 

+                         continue
 

                      for stream in deps.get_runtime_streams(name):
 

                          key = "%s:%s" % (name, stream)
 

                          if key not in module_map:
 

-                             new_module = self.get_latest_modules(key)
 

+                             new_module = self.get_latest_modules(
 

+                                 key,
 

+                                 base_module_br_stream_version_lte=str(
 

+                                     base_module_stream_version
 

+                                 ),
 

+                                 base_module_br_stream_version_gte=min_bound,
 

+                             )
 

                              new_modules += new_module
 

                              module_map[key] = [new_modules]
 

  

          return new_modules
 

  

-     def validate_module_list(self, modules, expand=True):
 

+     def validate_module_list(self, modules, expand=True, base_modules=None):
 

          """
 

          Given a list of modules as returned by `get_modules()`, checks that
 

          there are no conflicting duplicates, removes any exact duplicates,
 
@@ -196,6 +222,8 @@ 
 

                  ``get_latest_module()``
 

          :param expand: if required modules should be included in the returned
 

                  list.
 

+         :param base_modules: a list of base module names, their dependencies
 

+                 should not be resolved.
 

          :return: the list of modules with deduplication and expansion.
 

          :raises ModuleLookupError: if a required module couldn't be found, or a
 

                  conflict occurred when resolving dependencies.
 
@@ -248,7 +276,7 @@ 
 

              added_module_list = new_modules
 

              while True:
 

                  added_module_list = self._add_new_dependencies(
 

-                     module_map, added_module_list
 

+                     module_map, added_module_list, base_modules
 

                  )
 

                  if len(added_module_list) == 0:
 

                      break

file modified
+48 -1 
@@ -23,6 +23,7 @@ 
 

  

  from functools import wraps
 

  import json
 

+ import re
 

  import responses
 

  from six.moves.urllib.parse import urlparse, parse_qs
 

  
@@ -40,7 +41,16 @@ 
 

      return mod_index.dump_to_string()
 

  

  

- def make_module(name, stream, version, requires={}, mdversion=1, context=None, state=5):
 

+ def make_module(
 

+     name,
 

+     stream,
 

+     version,
 

+     requires={},
 

+     mdversion=1,
 

+     context=None,
 

+     state=5,
 

+     base_module_stream=None,
 

+ ):
 

      if mdversion == 1:
 

          mmd = Modulemd.ModuleStreamV1.new(name, stream)
 

      else:
 
@@ -60,6 +70,16 @@ 
 

              deps.add_runtime_stream(req_name, req_stream)
 

          mmd.add_dependencies(deps)
 

  

+     base_module_stream = base_module_stream or stream
 

+     try:
 

+         m = re.match("[^0-9]*([0-9]+)((?:\\.[0-9]+)*)", base_module_stream)
 

+         parts = (m.group(2) or "").split(".")
 

+         base_module_stream_version = int(
 

+             m.group(1) + "".join(p.zfill(2) for p in parts)
 

+         )
 

+     except Exception:
 

+         base_module_stream_version = 0
 

+ 

      return {
 

          "name": name,
 

          "stream": stream,
 
@@ -67,6 +87,13 @@ 
 

          "context": context or "00000000",
 

          "modulemd": dump_mmd(mmd),
 

          "state": state,
 

+         "base_module_buildrequires": [
 

+             {
 

+                 "name": "platform",
 

+                 "stream": base_module_stream,
 

+                 "stream_version": base_module_stream_version,
 

+             },
 

+         ],
 

      }
 

  

  
@@ -104,6 +131,16 @@ 
 

      make_module("parent", "master", 1, {}, 2, context="b"),
 

      make_module("testcontexts", "master", 1, {"parent": "master"}, 2, context="a"),
 

      make_module("testcontexts", "master", 1, {"parent": "master"}, 2, context="b"),
 

+     # test depend on older
 

+     make_module(
 

+         "leaf", "master", 1, {"lib": "master"}, 2, base_module_stream="el8.3.0.z"
 

+     ),
 

+     make_module(
 

+         "lib", "master", 1, {}, 2, context="abcdef", base_module_stream="el8.2.0.z"
 

+     ),
 

+     make_module(
 

+         "lib", "master", 1, {}, 2, context="fedcba", base_module_stream="el8.4.0"
 

+     ),
 

  ]
 

  

  
@@ -147,6 +184,16 @@ 
 

                              break
 

                      if module["state"] not in states:
 

                          skip = True
 

+                     if "base_module_br_stream_version_lte" in query:
 

+                         if module["base_module_buildrequires"][0][
 

+                             "stream_version"
 

+                         ] > float(query["base_module_br_stream_version_lte"][0]):
 

+                             skip = True
 

+                     if "base_module_br_stream_version_gte" in query:
 

+                         if module["base_module_buildrequires"][0][
 

+                             "stream_version"
 

+                         ] < float(query["base_module_br_stream_version_gte"][0]):
 

+                             skip = True
 

                      if skip:
 

                          continue
 

                      body["items"].append(module)

file modified
+18 
@@ -139,6 +139,24 @@ 
 

          self.assertEqual(c.source, "")
 

  

      @mock_mbs()
 

+     def test_resolve_compose_uses_older_module(self):
 

+         c = Compose.create(
 

+             db.session,
 

+             "me",
 

+             PungiSourceType.MODULE,
 

+             "leaf:master",
 

+             COMPOSE_RESULTS["repository"],
 

+             3600,
 

+         )
 

+         db.session.commit()
 

+ 

+         resolve_compose(c)
 

+         db.session.commit()
 

+ 

+         c = db.session.query(Compose).filter(Compose.id == 1).one()
 

+         self.assertEqual(c.source, "leaf:master:1:00000000 lib:master:1:abcdef")
 

+ 

+     @mock_mbs()
 

      def test_resolve_compose_module_include_done_modules(self):
 

          c = Compose.create(
 

              db.session,

When dependencies are resolved for modules, originally ODCS would just take whatever is the latest build of the module. This is not great if there are multiple release going on at the same time.

Example situation: user requests mod:master:1:00000000 built against release 1.2.3. It depends on module foo:bar. If there happens to be a build of foo:bar built against release 1.3.0 (or even 2.0.0), it will be included in the compose.

To fix that, this patch looks at stream version of platform module for the initial module (10203), and will require all dependencies to have platform stream version equal or less than that (<10203). It will also construct lower bound based on major release version (>10000).

rebased onto 92e9a21b3257a9370c5ff4c85a9e84f66e8c2c09
3 years ago

@jkaluza, can you check if this makes sense for Fedora as well?

rebased onto 6d05e04e07deeb5e8a9b561fefd0ec0d0446e54c
3 years ago

2 new commits added

  • mbs: Use base module stream when resolving dependencies
  • mbs: Ignore platform module in the dependency loop
3 years ago

+1 from Fedora point of view.
Edited 3 years ago by jkaluza

rebased onto 9846acc82d2b9eb4b96798a34a064f2b57c2b728
3 years ago

pretty please pagure-ci rebuild
3 years ago

rebased onto ce3ffa0
3 years ago

Pull-Request has been merged by lsedlar
3 years ago
Metadata
None
No Tags
Changes Summary 4
+3 -1
file changed
server/odcs/server/backend.py
+32 -4
file changed
server/odcs/server/mbs.py
+48 -1
file changed
server/tests/mbs.py
+18 -0
file changed
server/tests/test_backend.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.