#334 Do not rebuild container images with RPM which is already at least at the same version as the one in advisory.
Merged 7 months ago by jkaluza. Opened 7 months ago by jkaluza.
jkaluza/freshmaker compare-nvr  into  master

@@ -400,25 +400,19 @@ 

              published = None

              release_category = None

  

-         # For each RPM package in Errata advisory, find the SRPM package name.

-         srpm_names = set()

-         nvrs = errata.get_builds(errata_id)

-         for nvr in nvrs:

-             srpm_name = koji.parse_NVR(nvr)['name']

-             srpm_names.add(srpm_name)

- 

          # Limit the Lightblue query to particular leaf images if set in Event.

          leaf_container_images = None

          if isinstance(self.event, ManualRebuildWithAdvisoryEvent):

              leaf_container_images = self.event.container_images

  

-         # For each SRPM name, find out all the containers which include

-         # this SRPM name.

+         # For each SRPM NVR, find out all the containers which include

+         # this SRPM NVR.

+         srpm_nvrs = set(errata.get_builds(errata_id))

          self.log_info(

              "Going to find all the container images to rebuild as "

-             "result of %r update.", srpm_names)

+             "result of %r update.", srpm_nvrs)

          batches = lb.find_images_to_rebuild(

-             srpm_names, content_sets,

+             srpm_nvrs, content_sets,

              filter_fnc=self._filter_out_not_allowed_builds,

              published=published, release_category=release_category,

              leaf_container_images=leaf_container_images)

file modified
+98 -15

@@ -30,6 +30,7 @@ 

  import requests

  import six

  import dogpile.cache

+ import kobo.rpmlib

  from itertools import groupby

  

  from six.moves import http_client

@@ -750,15 +751,80 @@ 

                  ]

          return projection

  

+     def filter_out_images_with_lower_srpm_nvr(self, images, srpm_name_to_nvr):

+         """

+         Checks whether the input NVRs defined in `srpm_name_to_nvr` dict are

+         newer than the matching SRPM NVRs in the container image.

+ 

+         If all the SRPM NVRs in the container image are newer than matching

+         input NVRs, the container image is filtered out from the `images`

+         list.

+ 

+         For example: The httpd-2.4-1 RPM is released together with

+         httpd-container. In this case, Freshmaker would try to rebuild

+         httpd-container, because it contains httpd package. But this is not

+         needed, because latest httpd-container already contains that updated

+         package. Therefore we filter it out in this method.

+ 

+         :param list images: List of ContainerImage instances.

+         :param dict srpm_name_to_nvr: Dict with SRPM name as a key and NVR

+             as a value.

+         :rtype: list

+         :return: List of ContainerImage instances without the filtered images.

+         """

+         ret = []

+         for image in images:

+             if "rpm_manifest" not in image or not image["rpm_manifest"]:

+                 # Do not filter if we are not sure what RPMs are in the image.

+                 ret.append(image)

+                 continue

+             # There is always just single "rpm_manifest". Lightblue returns

+             # this as a list, because it is reference to

+             # containerImageRPMManifest.

+             rpm_manifest = image["rpm_manifest"][0]

+             if "rpms" not in rpm_manifest:

+                 # Do not filter if we are not sure what RPMs are in the image.

+                 ret.append(image)

+                 continue

+             # Check whether all the input SRPMs in the container image are

+             # older or newer and filter the container images in case they are

+             # not older.

+             rpms = rpm_manifest["rpms"]

+             for rpm in rpms:

+                 if "srpm_name" in rpm and rpm["srpm_name"] in srpm_name_to_nvr:

+                     image_srpm_nvr = kobo.rpmlib.parse_nvr(rpm["srpm_nevra"])

+                     input_srpm_nvr = kobo.rpmlib.parse_nvr(

+                         srpm_name_to_nvr[rpm["srpm_name"]])

+                     # compare_nvr return values:

+                     #   - nvr1 newer than nvr2: 1

+                     #   - same nvrs: 0

+                     #   - nvr1 older: -1

+                     # We want to rebuild only images with SRPM NVR lower than

+                     # input SRPM NVR, therefore we check for -1.

+                     if kobo.rpmlib.compare_nvr(image_srpm_nvr, input_srpm_nvr,

+                                                ignore_epoch=True) == -1:

+                         ret.append(image)

+                         break

+             else:

+                 # Oh-no, the mighty for/else block!

+                 # The else clause executes after the loop completes normally.

+                 # This means that the loop did not encounter a break statement.

+                 # In our case, this means that we filtered out the image.

+                 image.log_error(

+                     "Will not rebuild %s, because it does not contain "

+                     "older version of any input package: %r" % (

+                         image["brew"]["build"], srpm_name_to_nvr.values()))

+         return ret

+ 

      def find_images_with_included_srpms(

-             self, content_sets, srpm_names, repositories, published=True):

+             self, content_sets, srpm_nvrs, repositories, published=True):

          """Query lightblue and find containerImages in given

          containerRepositories. By default limit only to images which have been

          published to at least one repository and images which have latest tag.

  

          :param list content_sets: List of content_sets the image includes RPMs

              from.

-         :param list srpm_names: list of srpm_name (source rpm name) to look for

+         :param list srpm_nvrs: list of SRPM NVRs to look for

          :param list repositories: List of repository names to look for.

          :param bool published: whether to limit queries to published

              repositories

@@ -767,6 +833,12 @@ 

          for repo in repositories.values():

              auto_rebuild_tags |= set(repo["auto_rebuild_tags"])

  

+         # Lightblue cannot compare NVRs, so just ask for all the container

+         # images with any version/release of SRPM we are interested in and

+         # compare it on client side.

+         srpm_name_to_nvr = {koji.parse_NVR(srpm_nvr)["name"]: srpm_nvr

+                             for srpm_nvr in srpm_nvrs}

+ 

          image_request = {

              "objectType": "containerImage",

              "query": {

@@ -790,7 +862,7 @@ 

                              "field": "rpm_manifest.*.rpms.*.srpm_name",

                              "op": "=",

                              "rvalue": srpm_name

-                         } for srpm_name in srpm_names]

+                         } for srpm_name in srpm_name_to_nvr.keys()]

                      },

                      {

                          "field": "parsed_data.files.*.key",

@@ -799,7 +871,8 @@ 

                      },

                  ]

              },

-             "projection": self._get_default_projection(srpm_names=srpm_names)

+             "projection": self._get_default_projection(

+                 srpm_names=srpm_name_to_nvr.keys())

          }

  

          if published is not None:

@@ -831,10 +904,11 @@ 

                          new_images.append(image)

                          break

          images = new_images

+         images = self.filter_out_images_with_lower_srpm_nvr(images, srpm_name_to_nvr)

          return images

  

      def get_images_by_nvrs(self, nvrs, published=True, content_sets=None,

-                            srpm_names=None):

+                            srpm_nvrs=None):

          """Query lightblue and returns containerImages defined by list of

          `nvrs`.

  

@@ -842,7 +916,7 @@ 

          :param bool published: whether to limit queries to published images

          :param list content_sets: List of content_sets the image includes RPMs

              from.

-         :param list srpm_names: list of srpm_name (source rpm name) to look for

+         :param list srpm_nvrs: list of SRPM NVRs to look for

          :return: List of containerImages.

          :rtype: list of ContainerImages.

          """

@@ -878,14 +952,19 @@ 

                  }

              )

  

-         if srpm_names is not None:

+         if srpm_nvrs is not None:

+             # Lightblue cannot compare NVRs, so just ask for all the container

+             # images with any version/release of SRPM we are interested in and

+             # compare it on client side.

+             srpm_name_to_nvr = {koji.parse_NVR(srpm_nvr)["name"]: srpm_nvr

+                                 for srpm_nvr in srpm_nvrs}

              image_request["query"]["$and"].append(

                  {

                      "$or": [{

                          "field": "rpm_manifest.*.rpms.*.srpm_name",

                          "op": "=",

                          "rvalue": srpm_name

-                     } for srpm_name in srpm_names]

+                     } for srpm_name in srpm_name_to_nvr.keys()]

                  }

              )

  

@@ -898,6 +977,8 @@ 

                  })

  

          images = self.find_container_images(image_request)

+         if srpm_nvrs is not None:

+             images = self.filter_out_images_with_lower_srpm_nvr(images, srpm_name_to_nvr)

          return images

  

      def find_unpublished_image_for_build(self, build):

@@ -1093,14 +1174,14 @@ 

              images.append(image)

  

      def find_images_with_packages_from_content_set(

-             self, srpm_names, content_sets, filter_fnc=None,

+             self, srpm_nvrs, content_sets, filter_fnc=None,

              published=True, deprecated=False,

              release_category="Generally Available",

              leaf_container_images=None):

          """Query lightblue and find containers which contain given

          package from one of content sets

  

-         :param list srpm_names: list of srpm_name (source rpm name) to look for

+         :param list srpm_nvrs: list of SRPM NVRs to look for

          :param list content_sets: list of strings (content sets) to consider

              when looking for the packages

          :param function filter_fnc: Function called as

@@ -1132,12 +1213,12 @@ 

              return []

          if not leaf_container_images:

              images = self.find_images_with_included_srpms(

-                 content_sets, srpm_names, repos, published)

+                 content_sets, srpm_nvrs, repos, published)

          else:

              # The `leaf_container_images` can contain unpublished container image,

              # therefore set `published` to None.

              images = self.get_images_by_nvrs(

-                 leaf_container_images, None, content_sets, srpm_names)

+                 leaf_container_images, None, content_sets, srpm_nvrs)

  

          # There can be multi-arch images which share the same

          # image['brew']['build']. Freshmaker is not interested in the image

@@ -1374,7 +1455,7 @@ 

          return batches

  

      def find_images_to_rebuild(

-             self, srpm_names, content_sets, published=True, deprecated=False,

+             self, srpm_nvrs, content_sets, published=True, deprecated=False,

              release_category="Generally Available", filter_fnc=None,

              leaf_container_images=None):

          """

@@ -1386,7 +1467,7 @@ 

          image from N+1 must happen *after* all of the images from sub-list N

          have been rebuilt.

  

-         :param list srpm_names: List of srpm_name (source rpm name) to look for

+         :param list srpm_nvrs: List of SRPM NVRs to look for

          :param list content_sets: list of strings (content sets) to consider

              when looking for the packages

          :param bool published: whether to limit queries to published

@@ -1407,9 +1488,11 @@ 

              is not respected when `leaf_container_images` are used.

          """

          images = self.find_images_with_packages_from_content_set(

-             srpm_names, content_sets, filter_fnc, published, deprecated,

+             srpm_nvrs, content_sets, filter_fnc, published, deprecated,

              release_category, leaf_container_images=leaf_container_images)

  

+         srpm_names = [koji.parse_NVR(srpm_nvr)["name"] for srpm_nvr in srpm_nvrs]

+ 

          def _get_images_to_rebuild(image):

              """

              Find out parent images to rebuild, helper called from threadpool.

@@ -440,7 +440,7 @@ 

              pass

  

          self.find_images_to_rebuild.assert_called_once_with(

-             set(['httpd']), ['content-set-1'],

+             set(['httpd-2.4-11.el7']), ['content-set-1'],

              filter_fnc=self.handler._filter_out_not_allowed_builds,

              published=True, release_category='Generally Available',

              leaf_container_images=None)

@@ -457,7 +457,7 @@ 

              pass

  

          self.find_images_to_rebuild.assert_called_once_with(

-             set(['httpd']), ['content-set-1'],

+             set(['httpd-2.4-11.el7', 'httpd-2.2-11.el6']), ['content-set-1'],

              filter_fnc=self.handler._filter_out_not_allowed_builds,

              published=True, release_category='Generally Available',

              leaf_container_images=None)

@@ -476,7 +476,7 @@ 

              pass

  

          self.find_images_to_rebuild.assert_called_once_with(

-             set(['httpd']), ['content-set-1'],

+             set(['httpd-2.4-11.el7']), ['content-set-1'],

              filter_fnc=self.handler._filter_out_not_allowed_builds,

              published=None, release_category=None,

              leaf_container_images=None)

@@ -493,7 +493,7 @@ 

              pass

  

          self.find_images_to_rebuild.assert_called_once_with(

-             set(['httpd']), ['content-set-1'],

+             set(['httpd-2.4-11.el7']), ['content-set-1'],

              filter_fnc=self.handler._filter_out_not_allowed_builds,

              published=True, release_category='Generally Available',

              leaf_container_images=None)

@@ -511,7 +511,7 @@ 

              pass

  

          self.find_images_to_rebuild.assert_called_once_with(

-             set(['httpd']), ['content-set-1'],

+             set(['httpd-2.4-11.el7']), ['content-set-1'],

              filter_fnc=self.handler._filter_out_not_allowed_builds,

              published=True, release_category='Generally Available',

              leaf_container_images=["foo", "bar"])

file modified
+31 -8

@@ -657,6 +657,10 @@ 

              ContainerImage.create(data)

              for data in self.fake_images_with_parsed_data]

  

+         self.fake_container_images_floating_tag = [

+             ContainerImage.create(data)

+             for data in self.fake_images_with_parsed_data_floating_tag]

+ 

          self.fake_koji_builds = [{"task_id": 654321}, {"task_id": 123456}]

          self.fake_koji_task_requests = [

              ["git://pkgs.devel.redhat.com/rpms/repo-2#commit_hash2",

@@ -880,7 +884,7 @@ 

              self.fake_repositories_with_content_sets}

          cont_images.return_value = self.fake_images_with_parsed_data

          ret = lb.find_images_with_included_srpms(

-             ["content-set-1", "content-set-2"], ["openssl"], repositories)

+             ["content-set-1", "content-set-2"], ["openssl-1.2.3-2"], repositories)

  

          expected_image_request = {

              "objectType": "containerImage",

@@ -972,15 +976,34 @@ 

              repo["repository"]: repo for repo in

              self.fake_repositories_with_content_sets}

          cont_images.return_value = (

-             self.fake_images_with_parsed_data +

-             self.fake_images_with_parsed_data_floating_tag)

+             self.fake_container_images +

+             self.fake_container_images_floating_tag)

          ret = lb.find_images_with_included_srpms(

-             ["content-set-1", "content-set-2"], ["openssl"], repositories)

+             ["content-set-1", "content-set-2"], ["openssl-1.2.3-2"], repositories)

  

          self.assertEqual(

              [image["brew"]["build"] for image in ret],

              ['package-name-2-4-12.10', 'package-name-3-4-12.10'])

  

+     @patch('freshmaker.lightblue.LightBlue.find_container_images')

+     @patch('os.path.exists')

+     def test_images_with_included_newer_srpm(

+             self, exists, cont_images):

+ 

+         exists.return_value = True

+         lb = LightBlue(server_url=self.fake_server_url,

+                        cert=self.fake_cert_file,

+                        private_key=self.fake_private_key)

+         repositories = {

+             repo["repository"]: repo for repo in

+             self.fake_repositories_with_content_sets}

+         cont_images.return_value = (

+             self.fake_container_images +

+             self.fake_container_images_floating_tag)

+         ret = lb.find_images_with_included_srpms(

+             ["content-set-1", "content-set-2"], ["openssl-1.2.3-1"], repositories)

+         self.assertEqual(ret, [])

+ 

      def _filter_fnc(self, image):

          return image["brew"]["build"].startswith("filtered_")

  

@@ -1012,7 +1035,7 @@ 

                         cert=self.fake_cert_file,

                         private_key=self.fake_private_key)

          ret = lb.find_images_with_packages_from_content_set(

-             "openssl", ["dummy-content-set-1"], filter_fnc=self._filter_fnc)

+             set(["openssl-1.2.3-3"]), ["dummy-content-set-1"], filter_fnc=self._filter_fnc)

  

          # Only the first image should be returned, because the first one

          # is in repository "product1/repo1", but we have asked for images

@@ -1321,7 +1344,7 @@ 

          lb = LightBlue(server_url=self.fake_server_url,

                         cert=self.fake_cert_file,

                         private_key=self.fake_private_key)

-         batches = lb.find_images_to_rebuild(["dummy"], ["dummy"])

+         batches = lb.find_images_to_rebuild(["dummy-1-1"], ["dummy"])

  

          # Each of batch is sorted for assertion easily

          expected_batches = [

@@ -1361,7 +1384,7 @@ 

          lb = LightBlue(server_url=self.fake_server_url,

                         cert=self.fake_cert_file,

                         private_key=self.fake_private_key)

-         batches = lb.find_images_to_rebuild(["dummy"], ["dummy"])

+         batches = lb.find_images_to_rebuild(["dummy-1-1"], ["dummy"])

  

          self.assertEqual(len(batches), 1)

          self.assertEqual(len(batches[0]), 1)

@@ -1414,7 +1437,7 @@ 

                         cert=self.fake_cert_file,

                         private_key=self.fake_private_key)

          lb.find_images_with_packages_from_content_set(

-             ["openssl"], ["dummy-content-set"],

+             ["openssl-1.2.3-2"], ["dummy-content-set"],

              leaf_container_images=["foo", "bar"])

          cont_images.assert_called_once_with(

              {'query': {

If all the SRPM NVRs in the container image are newer than matching
input NVRs, the container image is filtered out from the images
list.

For example: The httpd-2.4-1 RPM is released together with
httpd-container. In this case, Freshmaker would try to rebuild
httpd-container, because it contains httpd package. But this is not
needed, because latest httpd-container already contains that updated
package. Therefore we filter it out in this method.

Pull-Request has been merged by jkaluza

7 months ago