fm-orchestrator

Clone
Source Code
GIT

#1219 Handle streams in base module stream
Merged 5 years ago by mprahl. Opened 5 years ago by cqi.
cqi/fm-orchestrator handle-z-stream  into  master

file modified
+2 
@@ -126,6 +126,8 @@ 
 

      ALLOWED_GROUPS_TO_IMPORT_MODULE = set(['mbs-import-module'])
 

      GREENWAVE_DECISION_CONTEXT = 'osci_compose_gate_modules'
 

  

+     STREAM_SUFFIXES = {r'^el\d+\.\d+\.\d+\.z$': 0.1}
 

+ 

  

  class ProdConfiguration(BaseConfiguration):
 

      pass

file modified
+7 
@@ -547,6 +547,13 @@ 
 

                       'buildrequired. These modules can set xmd.mbs.disttag_marking to do so. MBS '
 

                       'will use this list order to determine which modules take precedence.')
 

          },
 

+         'stream_suffixes': {
 

+             'type': dict,
 

+             'default': {},
 

+             'desc': 'A mapping of platform stream regular expressions and the '
 

+                     'corresponding suffix added to formatted stream version. '
 

+                     'For example, {r"regexp": 0.1, ...}'
 

+         }
 

      }
 

  

      def __init__(self, conf_section_obj):

file added
+42 
@@ -0,0 +1,42 @@ 
 

+ """Convert type of ModuleBuild.stream_version to float
 

+ 

+ Revision ID: 79babdadc942
 

+ Revises: 60c6093dde9e
 

+ Create Date: 2019-04-19 09:18:40.771633
 

+ 

+ """
 

+ 

+ import math
 

+ 

+ # revision identifiers, used by Alembic.
 

+ revision = '79babdadc942'
 

+ down_revision = '60c6093dde9e'
 

+ 

+ from alembic import op
 

+ import sqlalchemy as sa
 

+ 

+ 

+ def upgrade():
 

+     with op.batch_alter_table('module_builds') as b:
 

+         b.alter_column('stream_version', existing_type=sa.Integer, type_=sa.Float)
 

+ 

+ 

+ modulebuild = sa.Table(
 

+     'module_builds',
 

+     sa.MetaData(),
 

+     sa.Column('id', sa.Integer, primary_key=True),
 

+     sa.Column('stream_version', sa.Float()),
 

+ )
 

+ 

+ 

+ def downgrade():
 

+     # For PostgreSQL, changing column type from float to int causes the result
 

+     # to not be the floor value. So, do it manually in advance.
 

+     connection = op.get_bind()
 

+     for build in connection.execute(modulebuild.select()):
 

+         connection.execute(
 

+             modulebuild.update().where(modulebuild.c.id == build.id).values(
 

+                 stream_version=math.floor(build.stream_version)))
 

+ 

+     with op.batch_alter_table('module_builds') as b:
 

+         b.alter_column('stream_version', existing_type=sa.Float, type_=sa.Integer)

file modified
+23 -14 
@@ -27,19 +27,21 @@ 
 

  """
 

  

  import contextlib
 

+ import hashlib
 

  import json
 

+ import re
 

  from collections import OrderedDict
 

  from datetime import datetime
 

- import hashlib
 

+ 

  import sqlalchemy
 

- from sqlalchemy.orm import validates, scoped_session, sessionmaker, load_only
 

  from flask import has_app_context
 

- from module_build_service import db, log, get_url_for, app, conf, Modulemd
 

- from module_build_service.glib import from_variant_dict
 

- import module_build_service.messaging
 

- 

- from sqlalchemy.orm import lazyload
 

  from sqlalchemy import func, and_
 

+ from sqlalchemy.orm import lazyload
 

+ from sqlalchemy.orm import validates, scoped_session, sessionmaker, load_only
 

+ 

+ import module_build_service.messaging
 

+ from module_build_service.glib import from_variant_dict
 

+ from module_build_service import db, log, get_url_for, app, conf, Modulemd
 

  

  DEFAULT_MODULE_CONTEXT = '00000000'
 

  
@@ -213,7 +215,7 @@ 
 

      batch = db.Column(db.Integer, default=0)
 

  

      # This is only used for base modules for ordering purposes (f27.0.1 => 270001)
 

-     stream_version = db.Column(db.Integer)
 

+     stream_version = db.Column(db.Float)
 

      buildrequires = db.relationship(
 

          'ModuleBuild',
 

          secondary=module_builds_to_module_buildrequires,
 
@@ -785,8 +787,9 @@ 
 

          :param str stream: the module stream
 

          :kwarg bool right_pad: determines if the right side of the stream version should be padded
 

              with zeroes (e.g. `f27` => `27` vs `270000`)
 

-         :return: a stream version represented as an integer
 

-         :rtype: int or None if the stream doesn't have a valid version
 

+         :return: a stream version represented as a float. Stream suffix could
 

+             be added according to config ``stream_suffixes``.
 

+         :rtype: float or None if the stream doesn't have a valid version
 

          """
 

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

          version = ''
 
@@ -806,14 +809,20 @@ 
 

                      break
 

  

          # Remove the periods and pad the numbers if necessary
 

-         version = ''.join([section.zfill(2) for section in version.split('.')])
 

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

  

          if version:
 

              if right_pad:
 

                  version += (6 - len(version)) * '0'
 

-             # 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
 

-             return int(version)
 

+ 

+             result = float(version)
 

+ 

+             for regexp, suffix in conf.stream_suffixes.items():
 

+                 if re.match(regexp, stream):
 

+                     result += suffix
 

+                     break
 

+ 

+             return result
 

  

      def get_buildrequired_base_modules(self):
 

          """

file modified
+4 -2 
@@ -23,6 +23,7 @@ 
 

  #            Matt Prahl <mprahl@redhat.com>
 

  #            Jan Kaluza <jkaluza@redhat.com>
 

  import json
 

+ import math
 

  import re
 

  import time
 

  import shutil
 
@@ -268,12 +269,13 @@ 
 

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

      version_prefix = models.ModuleBuild.get_stream_version(base_module_stream, right_pad=False)
 

  

-     if not version_prefix:
 

+     if version_prefix is None:
 

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

                      'version'.format(base_module, base_module_stream))
 

          return version
 

  

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

+     # Strip the stream suffix because Modulemd requires version to be an integer
 

+     new_version = int(str(int(math.floor(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))

file modified
+8 -8 
@@ -179,14 +179,14 @@ 
 

          "buildrequires, xmd_buildrequires, expected", (
 

              # BR all platform streams -> build for all platform streams.
 

              ({"platform": []}, {}, [
 

-                 [["platform:el8.2.0:0:c0:x86_64"],
 

+                 [["platform:el8.2.0.z:0:c0:x86_64"],
 

                   ["platform:el8.1.0:0:c0:x86_64"],
 

                   ["platform:el8.0.0:0:c0:x86_64"],
 

                   ["platform:el7.6.0:0:c0:x86_64"]],
 

              ]),
 

              # BR "el8" platform stream -> build for all el8 platform streams.
 

              ({"platform": ["el8"]}, {}, [
 

-                 [["platform:el8.2.0:0:c0:x86_64"],
 

+                 [["platform:el8.2.0.z:0:c0:x86_64"],
 

                   ["platform:el8.1.0:0:c0:x86_64"],
 

                   ["platform:el8.0.0:0:c0:x86_64"]],
 

              ]),
 
@@ -202,16 +202,16 @@ 
 

              ]),
 

              # BR platform:el8.2.0 and gtk:3, this time gtk:3 build against el8.2.0 exists
 

              # -> use both platform and gtk from el8.2.0 and build once.
 

-             ({"platform": ["el8"], "gtk": ["3"]}, ["platform:el8.2.0"], [
 

-                 [["platform:el8.2.0:0:c0:x86_64", "gtk:3:1:c8:x86_64", ]],
 

+             ({"platform": ["el8"], "gtk": ["3"]}, ["platform:el8.2.0.z"], [
 

+                 [["platform:el8.2.0.z:0:c0:x86_64", "gtk:3:1:c8:x86_64", ]],
 

              ]),
 

              # BR platform:el8.2.0 and mess:1 which is built against platform:el8.1.0 and
 

              # requires gtk:3 which is built against platform:el8.2.0 and platform:el8.0.0
 

              # -> Use platform:el8.2.0 and
 

              # -> cherry-pick mess:1 from el8.1.0 and
 

              # -> use gtk:3:1 from el8.2.0.
 

-             ({"platform": ["el8"], "mess": ["1"]}, ["platform:el8.2.0"], [
 

-                 [["platform:el8.2.0:0:c0:x86_64", "mess:1:0:c0:x86_64", "gtk:3:1:c8:x86_64", ]],
 

+             ({"platform": ["el8"], "mess": ["1"]}, ["platform:el8.2.0.z"], [
 

+                 [["platform:el8.2.0.z:0:c0:x86_64", "mess:1:0:c0:x86_64", "gtk:3:1:c8:x86_64", ]],
 

              ]),
 

              # BR platform:el8.1.0 and mess:1 which is built against platform:el8.1.0 and
 

              # requires gtk:3 which is built against platform:el8.2.0 and platform:el8.0.0
 
@@ -237,10 +237,10 @@ 
 

              # (nsvc, buildrequires, expanded_buildrequires, virtual_streams)
 

              ("platform:el8.0.0:0:c0", {}, {}, ["el8"]),
 

              ("platform:el8.1.0:0:c0", {}, {}, ["el8"]),
 

-             ("platform:el8.2.0:0:c0", {}, {}, ["el8"]),
 

+             ("platform:el8.2.0.z:0:c0", {}, {}, ["el8"]),
 

              ("platform:el7.6.0:0:c0", {}, {}, ["el7"]),
 

              ("gtk:3:0:c8", {"platform": ["el8"]}, {"platform:el8.0.0"}, None),
 

-             ("gtk:3:1:c8", {"platform": ["el8"]}, {"platform:el8.2.0"}, None),
 

+             ("gtk:3:1:c8", {"platform": ["el8"]}, {"platform:el8.2.0.z"}, None),
 

              ("mess:1:0:c0", [{"gtk": ["3"], "platform": ["el8"]}], {"platform:el8.1.0"}, None),
 

          )
 

          for n, req, xmd_br, virtual_streams in modules:

file modified
+20 -12 
@@ -21,12 +21,14 @@ 
 

  # Written by Ralph Bean <rbean@redhat.com>
 

  

  import os
 

- from module_build_service.utils import to_text_type
 

+ import pytest
 

  

- from tests.test_models import init_data, module_build_from_modulemd
 

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

+ from mock import patch
 

  from module_build_service import conf, Modulemd
 

  from module_build_service.models import ComponentBuild, ModuleBuild, make_session
 

+ from module_build_service.utils import to_text_type
 

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

+ from tests.test_models import init_data, module_build_from_modulemd
 

  

  

  class TestModels:
 
@@ -94,15 +96,21 @@ 
 

          build_one = ModuleBuild.query.get(2)
 

          assert build_one.siblings == [3, 4]
 

  

-     def test_get_stream_version(self):
 

-         """Test the ModuleBuild.get_stream_version method when right_pad is True."""
 

-         assert ModuleBuild.get_stream_version('f27') == 270000
 

-         assert ModuleBuild.get_stream_version('f27.02.30') == 270230
 

- 

-     def test_get_stream_version_no_right_pad(self):
 

-         """Test the ModuleBuild.get_stream_version method when right_pad is False."""
 

-         assert ModuleBuild.get_stream_version('f27', False) == 27
 

-         assert ModuleBuild.get_stream_version('f27.02.30', False) == 270230
 

+     @pytest.mark.parametrize('stream,right_pad,expected', [
 

+         ['f27', True, 270000.0],
 

+         ['f27.02.30', True, 270230.0],
 

+         ['f27', False, 27.0],
 

+         ['f27.02.30', False, 270230.0],
 

+         ['el8', True, 080000.0],
 

+         ['el8.1.0', True, 080100.0],
 

+         ['el8.z', True, 080000.2],
 

+         ['el8.1.0.z', True, 080100.3],
 

+     ])
 

+     @patch.object(conf, 'stream_suffixes', new={
 

+         r'el\d+\.z': 0.2, r'el\d+\.\d+\.\d+\.z': 0.3
 

+     })
 

+     def test_get_stream_version(self, stream, right_pad, expected):
 

+         assert expected == ModuleBuild.get_stream_version(stream, right_pad)
 

  

  

  class TestModelsGetStreamsContexts:

A base module's stream (the platform for RHEL) could have Z-stream suffix, e.g.
el8.0.0.z, this patch handles this Z-stream suffix and other potential streams
by returning the stream version as a float with configured suffix value. For
example, el8.1.0.z would be parsed as 080100.1. Note that, the 0.1 is totally
configured in config and it actually could be any value according to concrete
cases in practice.

Config STREAM_SUFFIXES is enabled in TestConfiguration so that tests depending
on the return value from ModuleBuild.get_stream_version are covered.

Part fixture of test TestMMDResolver.test_solve_virtual_streams is updated by
adding Z-stream suffix to platform:el8.2.0 in order to ensure this patch does
not break the MMD resolver.

Addresses FACTORY-4307

Signed-off-by: Chenxiong Qi cqi@redhat.com

Since this is a Red Hat specific implementation detail, I think it should be removed.

is not => to not be

Nice catch!

Could you explain why these imports were rearranged?

Usually, the structure is:

# standard library imports
import hashlib
import json

# third-party imports
import sqlalchemy
import flask

# local imports
import module_build_service.messaging

If you want to clean up the imports, then we should likely stick to this structure. This isn't a blocker for merging the PR though.

This will need to change if #1218 is merged before this one.

This comment needs to be updated.

@cqi I left a few minor comments that need addressing but this looks great.

rebased onto 7c993f9
5 years ago

This will need to change if #1218 is merged before this one.

I would prefer to merge this pr before #1218. Then, I will update #1218.

Could you explain why these imports were rearranged?

I just had a quick thought to sort these lines by import and from..import. These lines are updated according to this structure.

This comment needs to be updated.

I just simply remove the comment since return value is described in docstring.

Other comments are addressed. PTAL.

:thumbsup:

Commit e72538b fixes this pull-request

Pull-Request has been merged by mprahl
5 years ago

Pull-Request has been merged by mprahl
5 years ago
Changes Summary 7
+2 -0
file changed
conf/config.py
+7 -0
file changed
module_build_service/config.py
+42
file added
module_build_service/migrations/versions/79babdadc942_convert_type_of_modulebuild_stream_.py
+23 -14
file changed
module_build_service/models.py
+4 -2
file changed
module_build_service/utils/submit.py
+8 -8
file changed
tests/test_mmd_resolver.py
+20 -12
file changed
tests/test_models/test_models.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.