#1201 Allow importing modules without a Koji tag
Merged 5 years ago by mprahl. Opened 5 years ago by mprahl.

file modified
+2 -1
@@ -67,7 +67,8 @@ 

  - ``mse`` - this is an internal identifier used by MBS to know if this is a legacy module build

    prior to module stream expansion. This should always be ``TRUE``.

  - ``koji_tag`` - this defines the Koji tag with the RPMs that are part of this module. For base

-   modules this will likely be a tag representing a buildroot.

+   modules this will likely be a tag representing a buildroot. If this is a metadata-only module,

+   then this can be left unset.

  - ``virtual_streams`` - the list of streams which groups multiple modules together. For more

    information on this field, see the ``Virtual Streams`` section below.

  - ``disttag_marking`` - if this module is a base module, then MBS will use the stream of the base

@@ -245,6 +245,11 @@ 

                  if not build:

                      raise RuntimeError(

                          'Buildrequired module %s %r does not exist in MBS db' % (br_name, details))

+ 

+                 # If the buildrequire is a meta-data only module with no Koji tag set, then just

+                 # skip it

+                 if build.koji_tag is None:

+                     continue

                  module_tags.setdefault(build.koji_tag, [])

                  module_tags[build.koji_tag].append(build.mmd())

  

@@ -294,6 +294,10 @@ 

                  db.session, name, details['stream'])

              if local_modules:

                  for m in local_modules:

+                     # If the buildrequire is a meta-data only module with no Koji tag set, then just

+                     # skip it

+                     if m.koji_tag is None:

+                         continue

                      module_tags[m.koji_tag] = m.mmd()

                  continue

  
@@ -305,6 +309,10 @@ 

              for m in modules:

                  if m["koji_tag"] in module_tags:

                      continue

+                 # If the buildrequire is a meta-data only module with no Koji tag set, then just

+                 # skip it

+                 if m["koji_tag"] is None:

+                     continue

                  module_tags.setdefault(m["koji_tag"], [])

                  module_tags[m["koji_tag"]].append(self.extract_modulemd(m["modulemd"]))

  

@@ -364,14 +364,29 @@ 

          log.error(msg)

          raise UnprocessableEntity(msg)

  

-     # Get the koji_tag.

-     try:

-         xmd = mmd.get_xmd()

-         koji_tag = xmd["mbs"]["koji_tag"]

-     except KeyError:

-         msg = "'koji_tag' is not set in xmd['mbs'] for module {}".format(nsvc)

-         log.error(msg)

-         raise UnprocessableEntity(msg)

+     if len(mmd.get_dependencies()) > 1:

+         raise UnprocessableEntity(

+             "The imported module's dependencies list should contain just one element")

+ 

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

+     # Set some defaults in xmd["mbs"] if they're not provided by the user

+     if "mbs" not in xmd:

+         xmd["mbs"] = {"mse": True}

+ 

+     if mmd.get_dependencies():

+         brs = set(mmd.get_dependencies()[0].get_buildrequires().keys())

+         xmd_brs = set(xmd["mbs"].get("buildrequires", {}).keys())

+         if brs - xmd_brs:

+             raise UnprocessableEntity(

+                 'The imported module buildrequires other modules, but the metadata in the '

+                 'xmd["mbs"]["buildrequires"] dictionary is missing entries')

+     elif "buildrequires" not in xmd["mbs"]:

+         xmd["mbs"]["buildrequires"] = {}

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

+ 

+     koji_tag = xmd['mbs'].get('koji_tag')

+     if koji_tag is None:

+         log.warning("'koji_tag' is not set in xmd['mbs'] for module {}".format(nsvc))

  

      # Get the ModuleBuild from DB.

      build = models.ModuleBuild.get_build_from_nsvc(
@@ -397,6 +412,11 @@ 

      build.time_completed = datetime.utcnow()

      if build.name in conf.base_module_names:

          build.stream_version = models.ModuleBuild.get_stream_version(stream)

+ 

+     # Record the base modules this module buildrequires

+     for base_module in build.get_buildrequired_base_modules():

+         build.buildrequires.append(base_module)

+ 

      session.add(build)

      session.commit()

      msg = "Module {} imported".format(nsvc)

@@ -0,0 +1,2 @@ 

+ x]��j1�S�)������*���j��N�C���E�y���`x�u�`m��M�'?������&y�d�$�#�Xp��&�

+ r`�f�H&bƒ�"ɋ��H���!+��eo�n�7ڲ4���_�6��3��Lp�3���j�F;.���G�e�˿�Q�;�����������D������3G� 

\ No newline at end of file

@@ -0,0 +1,1 @@ 

+ x�R���0L���\��,��:�*\��*i� Ŋ\Z��P��!�2$ׄ�F;�����5�;nY�dB��H��79/������ ��zx�y��>8B���qw��w\��<�F����¾�fM@i��@	�#���*$N�99�b�c���%�'�:�C*cJ22�fә3�]�3v痯�j�R���|"��0I~�
Q*��W�ng�IaH>R�[d�w֑ ��mU��:Rlc���Z�'g;�.������_
]]��Jn���/���鷲���^]RZe,��:�$0���/�&�I(#�ܗ�,vp�+���f2�x�&=�F�-!�d��%���*�%|�0�<!�ҋҮ�Ѫ���$6�ڽ��yx�C��5�����푺�[d���=k�O�����s��jJ�J��T�x�����Z�x� 

\ No newline at end of file

@@ -0,0 +1,2 @@ 

+ x]��J1D]�+z/H'���� �	�{��30/b���pݹ��8�۶p�=��

+ ��Gʜ9E�s���E�Չ0�FECQNٜ�u�&O�R��je&劄I9$�aj�a�,��Ϙ�˺��)k��v���u��R��l�Hֹ����\�uq\�vv�W���?+xb�/˼ϲ)ܵ/�n~�KF4 

\ No newline at end of file

@@ -0,0 +1,2 @@ 

+ x]��j!Ds�+�X��V��S>a��0cfؿ_��R�*�zP�hm�.���

+ �#Wf�����m�%��R���jN��=@�ѣ�0�	�ta����5$����#?c=:|m�v�]��n��+|v���K9�8�@�{x�h����8澝]�n��bO1&�IM��^�v��Dk 

\ No newline at end of file

@@ -0,0 +1,1 @@ 

+ e3ca243c664440cdb8b704b0d5c32ed433bee4da

@@ -0,0 +1,28 @@ 

+ document: modulemd

+ version: 1

+ data:

+   description: A metadata-only module build for product X

+   name: build

+   license:

+     module: [MIT]

+   stream: product1.2

+   summary: A metadata-only module build for a product X

+   version: 1

+   context: 00000000

+   dependencies:

+     buildrequires:

+       platform: f28

+     requires:

+       platform: f28

+   xmd:

+     mbs:

+       buildrequires:

+         platform:

+           filtered_rpms: []

+           ref: virtual

+           stream: f28

+           version: '3'

+           context: '00000000'

+       commit: virtual

+       requires: {}

+       mse: true

@@ -0,0 +1,38 @@ 

+ document: modulemd

+ version: 1

+ data:

+     summary: A test module in all its beautiful beauty

+     description: >-

+         This module demonstrates how to write simple modulemd files And

+         can be used for testing the build and release pipeline. ’

+     license:

+         module: [ MIT ]

+     dependencies:

+         buildrequires:

+             platform: f28

+             build: product1.2

+         requires:

+             platform: f28

+     references:

+         community: https://docs.pagure.org/modularity/

+         documentation: https://fedoraproject.org/wiki/Fedora_Packaging_Guidelines_for_Modules

+     profiles:

+         default:

+             rpms:

+             - tangerine

+     api:

+         rpms:

+         - perl-Tangerine

+         - tangerine

+     components:

+         rpms:

+             perl-List-Compare:

+                 rationale: A dependency of tangerine.

+                 ref: master

+             perl-Tangerine:

+                 rationale: Provides API for this module and is a dependency of tangerine.

+                 ref: master

+             tangerine:

+                 rationale: Provides API for this module.

+                 buildorder: 10

+                 ref: master

@@ -106,6 +106,7 @@ 

      on_finalize_cb = None

      on_buildroot_add_artifacts_cb = None

      on_tag_artifacts_cb = None

+     on_buildroot_add_repos_cb = None

  

      @module_build_service.utils.validate_koji_tag('tag_name')

      def __init__(self, owner, module, config, tag_name, components):
@@ -122,6 +123,7 @@ 

          FakeModuleBuilder.on_finalize_cb = None

          FakeModuleBuilder.on_buildroot_add_artifacts_cb = None

          FakeModuleBuilder.on_tag_artifacts_cb = None

+         FakeModuleBuilder.on_buildroot_add_repos_cb = None

          FakeModuleBuilder.DEFAULT_GROUPS = None

          FakeModuleBuilder.backend = 'test'

  
@@ -172,7 +174,8 @@ 

              self._send_repo_done()

  

      def buildroot_add_repos(self, dependencies):

-         pass

+         if FakeModuleBuilder.on_buildroot_add_repos_cb:

+             FakeModuleBuilder.on_buildroot_add_repos_cb(self, dependencies)

  

      def tag_artifacts(self, artifacts, dest_tag=True):

          if FakeModuleBuilder.on_tag_artifacts_cb:
@@ -1372,6 +1375,43 @@ 

          module = db.session.query(models.ModuleBuild).get(module_build_id)

          assert module.state == models.BUILD_STATES['build']

  

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

+     @patch('module_build_service.scm.SCM')

+     def test_submit_br_metadata_only_module(

+             self, mocked_scm, mocked_get_user, conf_system, dbg, hmsc):

+         """

+         Test that when a build is submitted with a buildrequire without a Koji tag,

+         MBS doesn't supply it as a dependency to the builder.

+         """

+         metadata_mmd = module_build_service.utils.load_mmd(

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

+         module_build_service.utils.import_mmd(db.session, metadata_mmd)

+ 

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

+                 '620ec77321b2ea7b0d67d82992dda3e1d67055b4')

+         post_url = '/module-build-service/1/module-builds/'

+         post_data = {

+             'branch': 'master',

+             'scmurl': 'https://src.stg.fedoraproject.org/modules/'

+                       'testmodule.git?#620ec77321b2ea7b0d67d82992dda3e1d67055b4',

+         }

+         rv = self.client.post(post_url, data=json.dumps(post_data))

+         assert rv.status_code == 201

+ 

+         data = json.loads(rv.data)

+         module_build_id = data['id']

+ 

+         def on_buildroot_add_repos_cb(cls, dependencies):

+             # Make sure that the metadata module is not present since it doesn't have a Koji tag

+             assert dependencies.keys() == ['module-f28-build']

+ 

+         FakeModuleBuilder.on_buildroot_add_repos_cb = on_buildroot_add_repos_cb

+         stop = module_build_service.scheduler.make_simple_stop_condition(db.session)

+         module_build_service.scheduler.main([], stop)

+ 

+         module = db.session.query(models.ModuleBuild).get(module_build_id)

+         assert module.state == models.BUILD_STATES['ready']

+ 

  

  @patch("module_build_service.config.Config.system",

         new_callable=PropertyMock, return_value="testlocal")

@@ -321,6 +321,32 @@ 

              assert mmd_context == models.DEFAULT_MODULE_CONTEXT

              assert build.context == models.DEFAULT_MODULE_CONTEXT

  

+     def test_import_mmd_multiple_dependencies(self):

+         mmd = Modulemd.Module().new_from_file(

+             path.join(BASE_DIR, '..', 'staged_data', 'formatted_testmodule.yaml'))

+         mmd.upgrade()

+         mmd.add_dependencies(mmd.get_dependencies()[0])

+ 

+         expected_error = 'The imported module\'s dependencies list should contain just one element'

+         with pytest.raises(UnprocessableEntity) as e:

+             module_build_service.utils.import_mmd(db.session, mmd)

+             assert str(e.value) == expected_error

+ 

+     def test_import_mmd_no_xmd_buildrequires(self):

+         mmd = Modulemd.Module().new_from_file(

+             path.join(BASE_DIR, '..', 'staged_data', 'formatted_testmodule.yaml'))

+         mmd.upgrade()

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

+         del xmd['mbs']['buildrequires']

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

+ 

+         expected_error = (

+             'The imported module buildrequires other modules, but the metadata in the '

+             'xmd["mbs"]["buildrequires"] dictionary is missing entries')

+         with pytest.raises(UnprocessableEntity) as e:

+             module_build_service.utils.import_mmd(db.session, mmd)

+             assert str(e.value) == expected_error

+ 

      @pytest.mark.parametrize('stream, disttag_marking, error_msg', (

          ('f28', None, None),

          ('f28', 'fedora28', None),

@@ -1575,7 +1575,7 @@ 

          rv = self.client.post(

              post_url,

              data=json.dumps({'scmurl': 'file://' + scm_base_dir + (

-                 '/mariadb?#7cf8fb26db8dbfea075eb5f898cc053139960250')}))

+                 '/mariadb?#e9742ed681f82e3ef5281fc652b4e68a3826cea6')}))

          data = json.loads(rv.data)

  

          assert 'Module mariadb:10.2:20180724000000:00000000 imported' in data['messages']
@@ -1605,7 +1605,7 @@ 

          rv = self.client.post(

              post_url,

              data=json.dumps({'scmurl': 'file://' + scm_base_dir + (

-                 '/mariadb?#1a43ea22cd32f235c2f119de1727a37902a49f20')}))

+                 '/mariadb?#8b43f38cdafdd773e7d0308e758105bf9f0f67a8')}))

          data = json.loads(rv.data)

  

          assert 'Module mariadb:10.2:20180724065109:00000000 imported' in data['messages']
@@ -1651,29 +1651,13 @@ 

          rv = self.client.post(

              post_url,

              data=json.dumps({'scmurl': 'file://' + scm_base_dir + (

-                 '/mariadb?#cb7cf7069059141e0797ad2cf5a559fb673ef43d')}))

+                 '/mariadb?#f7c5c7218c9a197d7fd245eeb4eee3d7abffd75d')}))

          data = json.loads(rv.data)

  

          assert data['error'] == 'Unprocessable Entity'

          assert re.match(r'The modulemd .* is invalid\. Please verify the syntax is correct',

                          data['message'])

  

-     @pytest.mark.parametrize('api_version', [1, 2])

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

-     @patch.object(module_build_service.config.Config, 'scmurls',

-                   new_callable=PropertyMock, return_value=['file://'])

-     def test_import_build_scm_missing_koji_tag(self, mocked_scmurls, mocked_get_user,

-                                                api_version):

-         post_url = '/module-build-service/{0}/import-module/'.format(api_version)

-         rv = self.client.post(

-             post_url,

-             data=json.dumps({'scmurl': 'file://' + scm_base_dir + (

-                 '/mariadb?#9ab5fdeba83eb3382413ee8bc06299344ef4477d')}))

-         data = json.loads(rv.data)

- 

-         assert data['error'] == 'Unprocessable Entity'

-         assert data['message'].startswith('\'koji_tag\' is not set in xmd[\'mbs\'] for module')

- 

      def test_buildrequires_is_included_in_json_output(self):

          # Inject xmd/mbs/buildrequires into an existing module build for

          # assertion later.

These module builds will basically act as metadata-only module builds. This will be more useful as additional features stem from these types of builds.

The tests will fail until it is rebased on #1200.

pretty please pagure-ci rebuild

5 years ago

I'm not sure if it is OK to keep this set to empty dict without any checks. It seems you presume that the imported modulemd file will have this set in case it has non-empty buildrequires. Can we have a check for this? Something like (not tested):

if mmd.get_dependencies() and "buildrequires" not in xmd["mbs"]:
    raise UnprocessableEntity('Imported module buildrequires another module, but specific name:stream:version:context is not defined in xmd["mbs"]["buildrequires"] list')

Maybe even do it smarter and check that all buildrequires have matching record in xmd buildrequires list.

I think it is not a blocker, but it might be easy for poor RCM guy to not set this and spend time finding out why the dependency is not picked up.

rebased onto f1480fe

5 years ago

@jkaluza let me know if this is what you had in mind.

Pull-Request has been merged by mprahl

5 years ago
Changes Summary 23
+2 -1
file changed
docs/VIRTUAL_MODULES.rst
+5 -0
file changed
module_build_service/resolver/DBResolver.py
+8 -0
file changed
module_build_service/resolver/MBSResolver.py
+28 -8
file changed
module_build_service/utils/general.py
+0
file added
tests/scm_data/mariadb/objects/34/a463979786199c1b41ca21eb309cf3ce5ce789
+0
file added
tests/scm_data/mariadb/objects/48/83b004d723a15cdc266c0280fe26c424ae10db
+2
file added
tests/scm_data/mariadb/objects/60/5345608fcc41d4e297cf062c653ebfdfe52476
+0
file added
tests/scm_data/mariadb/objects/6c/d6a18e15a4f23a1eaa7040e7f4da9be1aed36d
+0
file added
tests/scm_data/mariadb/objects/6e/a27c8576449ad5a12c1b6d27b50ff32c854ee5
+1
file added
tests/scm_data/mariadb/objects/7f/96fc61223e4bc4c93c7c286cca98f6efaa3716
+0
file added
tests/scm_data/mariadb/objects/8b/43f38cdafdd773e7d0308e758105bf9f0f67a8
+0
file added
tests/scm_data/mariadb/objects/e3/ca243c664440cdb8b704b0d5c32ed433bee4da
+0
file added
tests/scm_data/mariadb/objects/e4/75d55e2b78ad81f1b08bcb33a0a15ad24ef8a9
+2
file added
tests/scm_data/mariadb/objects/e9/742ed681f82e3ef5281fc652b4e68a3826cea6
+2
file added
tests/scm_data/mariadb/objects/f7/c5c7218c9a197d7fd245eeb4eee3d7abffd75d
+0
file added
tests/scm_data/mariadb/objects/fb/9351bd6f2439a73d78de0822fefe64410f905c
+0
file added
tests/scm_data/mariadb/objects/fc/399d16e46feb3af72a692b316a241c21bae1d0
+1
file added
tests/scm_data/mariadb/refs/heads/master
+28
file added
tests/staged_data/build_metadata_module.yaml
+38
file added
tests/staged_data/testmodule_br_metadata_module.yaml
+41 -1
file changed
tests/test_build/test_build.py
+26 -0
file changed
tests/test_utils/test_utils.py
+3 -19
file changed
tests/test_views/test_views.py