fm-orchestrator

Clone
Source Code
GIT

#1032 Prefix the module version and disttag based on the platform it buildrequires
Merged 5 years ago by mprahl. Opened 5 years ago by mprahl.
platform-stream-prefixes  into  master

file modified
+3 -3 
@@ -411,9 +411,9 @@ 
 

              'desc': ('The distinguished name of the container or organizational unit containing '
 

                       'the groups in LDAP')},
 

          'base_module_names': {
 

-             'type': set,
 

-             'default': set(['platform', 'bootstrap']),
 

-             'desc': ("Set of module names which defines the product version "
 

+             'type': list,
 

+             'default': ['platform'],
 

+             'desc': ("List of base module names which define the product version "
 

                       "(by their stream) of modules depending on them.")},
 

          'base_module_koji_arches': {
 

              'type': dict,

file modified
+3 -3 
@@ -156,7 +156,7 @@ 
 

  

      def resolve_profiles(self, mmd, keys):
 

          """
 

-         :param mmd: ModuleMetadata instance of module
 

+         :param mmd: Modulemd.Module instance of module
 

          :param keys: list of modulemd installation profiles to include in
 

                       the result.
 

          :return: Dictionary with keys set according to `keys` param and values
 
@@ -213,8 +213,8 @@ 
 

          :param strict: Normally this function returns None if no module can be
 

              found.  If strict=True, then an UnprocessableEntity is raised.
 

          :return: a mapping containing buildrequire modules info in key/value pairs,
 

-             where key is koji_tag and value is the ModuleMetadata object.
 

-         :rtype: dict(str, :class:`ModuleMetadata`)
 

+             where key is koji_tag and value is the Modulemd.Module object.
 

+         :rtype: dict(str, :class:`Modulemd.Module`)
 

          """
 

  

          if mmd:

file modified
+1 -1 
@@ -223,7 +223,7 @@ 
 

          cg_build_koji_tag = conf.koji_cg_default_build_tag
 

          if conf.system not in ['koji', 'test']:
 

              # In case of non-koji backend, we want to get the dependencies
 

-             # of the local module build based on ModuleMetadata, because the
 

+             # of the local module build based on Modulemd.Module, because the
 

              # local build is not stored in the external MBS and therefore we
 

              # cannot query it using the `query` as for Koji below.
 

              dependencies = resolver.get_module_build_dependencies(

file modified
+19 -1 
@@ -212,8 +212,26 @@ 
 

      mse_build_ids = module_build.siblings + [module_build.id or 0]
 

      mse_build_ids.sort()
 

      index = mse_build_ids[0]
 

-     return "{prefix}{index}+{dist_hash}".format(
 

+     try:
 

+         buildrequires = module_build.mmd().get_xmd()['mbs']['buildrequires']
 

+     except (ValueError, KeyError):
 

+         log.warn('Module build {0} does not have buildrequires in its xmd'.format(module_build.id))
 

+         buildrequires = None
 

+ 

+     base_module_stream = ''
 

+     if buildrequires:
 

+         for base_module in conf.base_module_names:
 

+             base_module_stream = buildrequires.get(base_module, {}).get('stream', '')
 

+             if base_module_stream:
 

+                 base_module_stream += '+'
 

+                 break
 

+         else:

The mighty for/else block! :)

 
+             log.warn('Module build {0} does not buildrequire a base module ({1})'
 

+                      .format(module_build.id, ' or '.join(conf.base_module_names)))
 

+ 

+     return '{prefix}{base_module_stream}{index}+{dist_hash}'.format(
 

          prefix=conf.default_dist_tag_prefix,
 

+         base_module_stream=base_module_stream,
 

          index=index,
 

          dist_hash=dist_hash,
 

      )

file modified
+1 -1 
@@ -121,7 +121,7 @@ 
 

      the list of module metadata objects defined by `requires` dict.
 

  

      :param session: SQLAlchemy DB session.
 

-     :param requires: Modulemetadata requires or buildrequires.
 

+     :param requires: Modulemd.Module requires or buildrequires.
 

      :param mmds: Dictionary with already handled name:streams as a keys and lists
 

          of resulting mmds as values.
 

      :param recursive: If True, the requires are checked recursively.

file modified
+75 -7 
@@ -32,6 +32,7 @@ 
 

  

  import kobo.rpmlib
 

  import requests
 

+ from gi.repository import GLib
 

  

  import module_build_service.scm
 

  import module_build_service.resolver
 
@@ -116,7 +117,7 @@ 
 

      Prepares the modulemd for the MBS. This does things such as replacing the
 

      branches of components with commit hashes and adding metadata in the xmd
 

      dictionary.
 

-     :param mmd: the ModuleMetadata object to format
 

+     :param mmd: the Modulemd.Module object to format
 

      :param scmurl: the url to the modulemd
 

      """
 

      # Import it here, because SCM uses utils methods and fails to import
 
@@ -207,6 +208,70 @@ 
 

      mmd.set_xmd(glib.dict_values(xmd))
 

  

  

+ def get_prefixed_version(mmd):
 

+         """
 

+         Return the prefixed version of the module based on the buildrequired base module stream.
 

+ 

+         :param mmd: the Modulemd.Module object to format
 

+         :return: the prefixed version
 

+         :rtype: int
 

+         """
 

+         xmd = mmd.get_xmd()
 

+         version = mmd.get_version()
 

+         base_module_stream = None
 

+         for base_module in conf.base_module_names:
 

+             # xmd is a GLib Variant and doesn't support .get() syntax
 

+             try:
 

+                 base_module_stream = xmd['mbs']['buildrequires'].get(base_module, {}).get('stream')
 

+                 if base_module_stream:
 

+                     # Break after finding the first base module that is buildrequired
 

+                     break
 

+             except KeyError:
 

+                 log.warning('The module\'s mmd is missing information in the xmd section')
 

+                 return version
 

+         else:
 

+             log.warning('This module does not buildrequire a base module ({0})'
 

+                         .format(' or '.join(conf.base_module_names)))
 

+             return version
 

+ 

+         # The platform version (e.g. prefix1.2.0 => 010200)
 

+         version_prefix = ''
 

+         for char in base_module_stream:
 

+             try:
 

+                 # See if the current character is an integer, signifying the version
 

+                 # has started
 

+                 int(char)
 

+                 version_prefix += char
 

+             except ValueError:
 

+                 # If version_prefix isn't set, then a digit hasn't been encountered
 

+                 if version_prefix:
 

+                     # If the character is a period and the version_prefix is set, then
 

+                     # the loop is still processing the version part of the stream
 

+                     if char == '.':
 

+                         version_prefix += '.'
 

+                     # If the version_prefix is set and the character is not a period or
 

+                     # digit, then the remainder of the stream is a suffix like "-beta"
 

+                     else:
 

+                         break
 

+ 

+         # Remove the periods and pad the numbers if necessary
 

+         version_prefix = ''.join([section.zfill(2) for section in version_prefix.split('.')])
 

+ 

+         if not version_prefix:
 

+             log.warning('The "{0}" stream "{1}" couldn\'t be used to prefix the module\'s '
 

+                         'version'.format(base_module, base_module_stream))
 

+             return version
 

+ 

+         # Since the version must be stored as a number, we convert the string back to
 

+         # an integer which consequently drops the leading zero if there is one
 

+         new_version = int(version_prefix + str(version))
 

+         if new_version > GLib.MAXUINT64:
 

+             log.warning('The "{0}" stream "{1}" caused the module\'s version prefix to be '
 

+                         'too long'.format(base_module, base_module_stream))
 

+             return version
 

+         return new_version
 

+ 

+ 

  def validate_mmd(mmd):
 

      """Validate module metadata
 

  
@@ -397,12 +462,15 @@ 
 

      modules = []
 

  

      for mmd in mmds:
 

+         # Prefix the version of the modulemd based on the base module it buildrequires
 

+         version = get_prefixed_version(mmd)
 

+         mmd.set_version(version)
 

+         version_str = str(version)
 

+ 

          log.debug('Checking whether module build already exists: %s.',
 

-                   ":".join([mmd.get_name(), mmd.get_stream(),
 

-                            str(mmd.get_version()), mmd.get_context()]))
 

+                   ":".join([mmd.get_name(), mmd.get_stream(), version_str, mmd.get_context()]))
 

          module = models.ModuleBuild.get_build_from_nsvc(
 

-             db.session, mmd.get_name(), mmd.get_stream(), str(mmd.get_version()),
 

-             mmd.get_context())
 

+             db.session, mmd.get_name(), mmd.get_stream(), version_str, mmd.get_context())
 

          if module:
 

              if module.state != models.BUILD_STATES['failed']:
 

                  err_msg = ('Module (state=%s) already exists. Only a new build or resubmission of '
 
@@ -438,7 +506,7 @@ 
 

                  conf,
 

                  name=mmd.get_name(),
 

                  stream=mmd.get_stream(),
 

-                 version=str(mmd.get_version()),
 

+                 version=version_str,
 

                  modulemd=mmd.dumps(),
 

                  scmurl=url,
 

                  username=username,
 
@@ -451,7 +519,7 @@ 
 

          db.session.commit()
 

          modules.append(module)
 

          log.info("%s submitted build of %s, stream=%s, version=%s, context=%s", username,
 

-                  mmd.get_name(), mmd.get_stream(), mmd.get_version(), mmd.get_context())
 

+                  mmd.get_name(), mmd.get_stream(), version_str, mmd.get_context())
 

      return modules

file modified
+2 -2 
@@ -869,7 +869,7 @@ 
 

          build_one = models.ModuleBuild()
 

          build_one.name = 'testmodule'
 

          build_one.stream = 'master'
 

-         build_one.version = 20180205135154
 

+         build_one.version = '2820180205135154'
 

          build_one.build_context = 'return_runtime_context'
 

          build_one.ref_build_context = 'return_runtime_context'
 

          build_one.runtime_context = '9c690d0e'
 
@@ -994,7 +994,7 @@ 
 

          build_one = models.ModuleBuild()
 

          build_one.name = 'testmodule'
 

          build_one.stream = 'master'
 

-         build_one.version = 20180205135154
 

+         build_one.version = '2820180205135154'
 

          build_one.build_context = 'return_runtime_context'
 

          build_one.ref_build_context = 'return_runtime_context'
 

          build_one.runtime_context = '9c690d0e'

file modified
+24 -1 
@@ -29,7 +29,8 @@ 
 

  from module_build_service import models, conf
 

  from module_build_service.errors import ProgrammingError, ValidationError, UnprocessableEntity
 

  from tests import (
 

-     reuse_component_init_data, db, reuse_shared_userspace_init_data, clean_database, init_data)
 

+     reuse_component_init_data, db, reuse_shared_userspace_init_data, clean_database, init_data,
 

+     scheduler_init_data)
 

  import mock
 

  import koji
 

  import pytest
 
@@ -281,6 +282,12 @@ 
 

          assert release_one == "module+2+b8645bbb"
 

          assert release_two == "module+2+17e35784"
 

  

+     def test_get_rpm_release_platform_stream(self):
 

+         scheduler_init_data(1)
 

+         build_one = models.ModuleBuild.query.get(2)
 

+         release = module_build_service.utils.get_rpm_release(build_one)
 

+         assert release == 'module+f28+2+814cfa39'
 

+ 

      @pytest.mark.parametrize('scmurl', [
 

          ('git://pkgs.stg.fedoraproject.org/modules/testmodule.git'
 

           '?#620ec77321b2ea7b0d67d82992dda3e1d67055b4'),
 
@@ -579,6 +586,22 @@ 
 

          is_eol = module_build_service.utils.submit._is_eol_in_pdc('mariadb', '10.1')
 

          assert is_eol
 

  

+     def test_get_prefixed_version_f28(self):
 

+         scheduler_init_data(1)
 

+         build_one = models.ModuleBuild.query.get(2)
 

+         v = module_build_service.utils.submit.get_prefixed_version(build_one.mmd())
 

+         assert v == 2820180205135154
 

+ 

+     def test_get_prefixed_version_fl701(self):
 

+         scheduler_init_data(1)
 

+         build_one = models.ModuleBuild.query.get(2)
 

+         mmd = build_one.mmd()
 

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

+         xmd['mbs']['buildrequires']['platform']['stream'] = 'fl7.0.1-beta'
 

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

+         v = module_build_service.utils.submit.get_prefixed_version(mmd)
 

+         assert v == 7000120180205135154
 

+ 

  

  class DummyModuleBuilder(GenericBuilder):
 

      """

file modified
+1 -1 
@@ -685,7 +685,7 @@ 
 

          assert data['name'] == 'testmodule'
 

          assert data['scmurl'] == ('git://pkgs.stg.fedoraproject.org/modules/testmodule.git'
 

                                    '?#68931c90de214d9d13feefbd35246a81b6cb8d49')
 

-         assert data['version'] == '1'
 

+         assert data['version'] == '281'
 

          assert data['time_submitted'] is not None
 

          assert data['time_modified'] is not None
 

          assert data['time_completed'] is None

This PR makes it so that if a module buildrequires platform:f7.0.1, then the module version is prefixed with 70001 and the dist tag of the module components will be something like module+f7.0.1+2+814cfa39.

This addresses FACTORY-2924.

@ralph could you please review this when you can?

rebased onto 314688f
5 years ago

3 new commits added

  • Prefix the component disttag with the platform stream
  • Prefix the module version based on the platform it buildrequires
  • Correct the docstrings referring to the old modulemd type
5 years ago

Not a big issue, but wouldn't it make sense to use entries from conf.base_module_names instead of hardcoded "platform"? This applies also to other places where "platform" is.
Edited 5 years ago by jkaluza

I'm not sure if that's an issue or not, but I think this is not going to work for versions of base module greater than 18 if you use three x.y.z versioning, because MAXUINT64 is 18446744073709551615 and f19.0.0 would be transformed to 19000020180205135154 if I imagine that right based on the tests. This is probably not an issue for RHEL or Fedora (unless fedora starts doing 28.0.0 base modules for some reason), but just something I wanted to point out.
Edited 5 years ago by jkaluza

Sure, I can do that.

I'm not sure if that's an issue or not, but I think this is not going to work for versions of base module greater than 18 if you use three x.y.z versioning, because MAXUINT64 is 18446744073709551615 and f19.0.0 would be transformed to 19000020180205135154 if I imagine that right based on the tests. This is probably not an issue for RHEL or Fedora (unless fedora starts doing 28.0.0 base modules for some reason), but just something I wanted to point out.

Yes, it's one of the shortcomings of the design and in the future, we'll likely have to make the module version a string if we encounter these issues.

4 new commits added

  • Prefix the component disttag with the platform stream
  • Prefix the module version based on the platform it buildrequires
  • Set the default base_module_names config option to be platform
  • Correct the docstrings referring to the old modulemd type
5 years ago

4 new commits added

  • Prefix the component disttag with the platform stream
  • Prefix the module version based on the platform it buildrequires
  • Set the default base_module_names config option to be platform
  • Correct the docstrings referring to the old modulemd type
5 years ago

4 new commits added

  • Prefix the component disttag with the platform stream
  • Prefix the module version based on the platform it buildrequires
  • Set the default base_module_names config option to be platform
  • Correct the docstrings referring to the old modulemd type
5 years ago

@jkaluza could you please review again?

The mighty for/else block! :)

+1.

5 new commits added

  • Prefix the component disttag with the platform stream
  • Prefix the module version based on the platform it buildrequires
  • Change the config base_module_names to be a list instead of a set to allow ordering by the admin
  • Set the default base_module_names config option to be platform
  • Correct the docstrings referring to the old modulemd type
5 years ago

+1

Pull-Request has been merged by mprahl
5 years ago
Changes Summary 9
+3 -3
file changed
module_build_service/config.py
+3 -3
file changed
module_build_service/resolver/MBSResolver.py
+1 -1
file changed
module_build_service/scheduler/handlers/modules.py
+19 -1
file changed
module_build_service/utils/general.py
+1 -1
file changed
module_build_service/utils/mse.py
+75 -7
file changed
module_build_service/utils/submit.py
+2 -2
file changed
tests/test_build/test_build.py
+24 -1
file changed
tests/test_utils/test_utils.py
+1 -1
file changed
tests/test_views/test_views.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.