#982 Fix getting the name-version of filtered RPMs.
Closed 8 months ago by mprahl. Opened 8 months ago by jkaluza.
jkaluza/fm-orchestrator filters  into  master

@@ -1070,6 +1070,17 @@ 

  

          return weights

  

+     @classmethod

+     def get_built_rpms_in_module_build(cls, build):

+         """

+         :param ModuleBuild build: Module build to get the built RPMs from.

+         :return: list of NVRs

+         """

+         koji_session = KojiModuleBuilder.get_session(conf, None)

+         rpms = koji_session.listTaggedRPMS(build.koji_tag, latest=True)[0]

+         nvrs = set(kobo.rpmlib.make_nvr(rpm, force_epoch=True) for rpm in rpms)

+         return list(nvrs)

+ 

      def finalize(self):

          # Only import to koji CG if the module is "done".

          if self.config.koji_enable_content_generator and self.module.state == 3:

@@ -337,6 +337,14 @@ 

          raise NotImplementedError()

  

      @classmethod

+     def get_built_rpms_in_module_build(cls, build):

+         """

+         :param ModuleBuild build: Module build to get the built RPMs from.

+         :return: list of NVRs

+         """

+         raise NotImplementedError()

+ 

+     @classmethod

      def recover_orphaned_artifact(cls, component_build):

          """

          Searches for a complete build of an artifact belonging to the module and sets the

@@ -20,6 +20,9 @@ 

  # SOFTWARE.

  #

  # Written by Matt Prahl <mprahl@redhat.com>

+ #            Jan Kaluza <jkaluza@redhat.com>

+ 

+ import kobo.rpmlib

  

  from module_build_service import log

  from module_build_service.resolver.base import GenericResolver

@@ -177,6 +180,7 @@ 

          :param requires: a dictionary with the module name as the key and the stream as the value

          :return: a dictionary

          """

+         from module_build_service.builder import GenericBuilder

          new_requires = {}

          with models.make_session(self.config) as session:

              for nsvc in requires:

@@ -236,9 +240,12 @@ 

                  # Find out the particular NVR of filtered packages

                  rpm_filter = mmd.get_rpm_filter()

                  if rpm_filter and rpm_filter.get():

-                     for rpm in build.component_builds:

-                         if rpm.package in rpm_filter.get():

-                             filtered_rpms.append(rpm.nvr)

+                     rpm_filter = rpm_filter.get()

+                     built_nvrs = GenericBuilder.get_built_rpms_in_module_build(build)

+                     for nvr in built_nvrs:

+                         parsed_nvr = kobo.rpmlib.parse_nvr(nvr)

+                         if parsed_nvr["name"] in rpm_filter:

+                             filtered_rpms.append(nvr)

  

                  new_requires[module_name] = {

                      'ref': commit_hash,

@@ -510,3 +510,23 @@ 

          else:

              expected_calls = []

          assert session.packageListBlock.mock_calls == expected_calls

+ 

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session')

+     def test_get_built_rpms_in_module_build(self, get_session):

+         session = MagicMock()

+         session.listTaggedRPMS.return_value = ([

+             {'build_id': 735939, 'name': 'tar', 'extra': None, 'arch': 'ppc64le',

+              'buildtime': 1533299221, 'id': 6021394, 'epoch': 2, 'version': '1.30',

+              'metadata_only': False, 'release': '4.el8+1308+551bfa71',

+              'buildroot_id': 4321122, 'payloadhash': '0621ab2091256d21c47dcac868e7fc2a',

+              'size': 878684},

+             {'build_id': 735939, 'name': 'bar', 'extra': None, 'arch': 'ppc64le',

+              'buildtime': 1533299221, 'id': 6021394, 'epoch': 2, 'version': '1.30',

+              'metadata_only': False, 'release': '4.el8+1308+551bfa71',

+              'buildroot_id': 4321122, 'payloadhash': '0621ab2091256d21c47dcac868e7fc2a',

+              'size': 878684}], [])

+         get_session.return_value = session

+ 

+         ret = KojiModuleBuilder.get_built_rpms_in_module_build(self.module)

+         assert set(ret) == set(

+             ['bar-2:1.30-4.el8+1308+551bfa71', 'tar-2:1.30-4.el8+1308+551bfa71'])

@@ -120,6 +120,37 @@ 

              ]

              assert set(result) == set(expected)

  

+     @patch("module_build_service.builder.base.GenericBuilder.get_built_rpms_in_module_build")

+     def test_resolve_requires(self, built_rpms):

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

+         mmd = build.mmd()

+         filter_list = Modulemd.SimpleSet()

+         filter_list.add("foo")

+         filter_list.add("bar")

+         mmd.set_rpm_filter(filter_list)

+         build.modulemd = mmd.dumps()

+         db.session.commit()

+ 

+         built_rpms.return_value = [

+             "foo-0:2.4.48-3.el8+1308+551bfa71",

+             "foo-debuginfo-0:2.4.48-3.el8+1308+551bfa71",

+             "bar-0:2.5.48-3.el8+1308+551bfa71",

+             "bar-debuginfo-0:2.5.48-3.el8+1308+551bfa71",

+             "x-0:2.5.48-3.el8+1308+551bfa71",

+             "x-debuginfo-0:2.5.48-3.el8+1308+551bfa71"]

+ 

+         resolver = mbs_resolver.GenericResolver.create(tests.conf, backend='db')

+         result = resolver.resolve_requires([":".join([

+             build.name, build.stream, build.version, build.context])])

+ 

+         assert result == {

+             'testmodule': {

+                 'stream': 'master', 'version': '20170109091357', 'context': u'78e4a6fd',

+                 'ref': 'ff1ea79fc952143efeed1851aa0aa006559239ba',

+                 'filtered_rpms': [

+                     'foo-0:2.4.48-3.el8+1308+551bfa71',

+                     'bar-0:2.5.48-3.el8+1308+551bfa71']}}

+ 

      def test_resolve_profiles(self):

          """

          Tests that the profiles get resolved recursively

Previous code expected the MMD RPM filters to actually be component filters - that means the previous code expected component names like "httpd" there, but in fact, the MMD filter is RPM based, so there are RPM names of particular RPMs built from the component - like for example "httpd-debuginfo", "mod_ssl", "httpd-filesystem", ...

Because of that, the "Conflicts" section of module-build-macros did not contain all RPMs - it only contained those with same name as SRPM name from which they were built.

This PR fixes this by listing all the RPMs tagged in Koji tag of each (build-)required module and checking if the package is filtered by MMD filter section. If so, it stores it into the filtered_rpms as we did previously and it later gets added to Conflicts section of module-build-service.

Optional: You could assign rpm_filter.get() to a variable to avoid calling it on every iteration of the loop.

listTaggedRPMs should be listTaggedRPMS (capital S at the end)

Optional: Since builds isn't used, you could avoid storing it in memory with something like:

rpms = koji_session.listTaggedRPMs(build.koji_tag, latest=True)[0]

Optional: Since you parse the NVR in code that uses this method, perhaps you could store the parsed NVR as a list of dictionaries where each dict would be:

{
    'name': name,
    'version': version,
    'release': release,
    'epoch': epoch,
    'nvr': nvr
}

Then you wouldn't have to call kobo it code that uses method.

As mentioned previously, listTaggedRPMs should be listTaggedRPMS.

@jkaluza looks good. I left one mandatory comment to address and the rest are optional. Jenkins tests are broken and are fixed as part of PR#981 so as long as the tests pass locally, the tests should be fine.

@jkaluza could you please rebase this so that the Jenkins tests run?

rebased onto d4b8cd5bf51c4cd215a866a8a69f85f4bc99af08

8 months ago

@jkaluza could you please rebase this again?

rebased onto 7b8e833c036971d05aef0e2dc3c791fc5b32ee68

8 months ago

rebased onto 4aa7c96ecc49332876ebd378ac63fed9ae18d68f

8 months ago

rebased onto 1e778e523f5fe19fe97de95c723e0acc8d52031b

8 months ago

rebased onto 62a793a

8 months ago

This switched over to #987

Pull-Request has been closed by mprahl

8 months ago