#309 Limit the LB query using list of container images in case ManualRebuildWithAdvisoryEvent is handled.
Merged 10 months ago by jkaluza. Opened 10 months ago by jkaluza.
jkaluza/freshmaker rebuild-with-advisory  into  master

@@ -32,6 +32,7 @@ 

  from freshmaker import conf, db, log

  from freshmaker.events import ErrataAdvisoryRPMsSignedEvent

  from freshmaker.events import ODCSComposeStateChangeEvent

+ from freshmaker.events import ManualRebuildWithAdvisoryEvent

  from freshmaker.handlers import ContainerBuildHandler, fail_event_on_handler_exception

  from freshmaker.kojiservice import koji_service

  from freshmaker.lightblue import LightBlue

@@ -668,6 +669,11 @@ 

              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.

          self.log_info(

@@ -676,7 +682,8 @@ 

          batches = lb.find_images_to_rebuild(

              srpm_names, content_sets,

              filter_fnc=self._filter_out_not_allowed_builds,

-             published=published, release_category=release_category)

+             published=published, release_category=release_category,

+             leaf_container_images=leaf_container_images)

          return batches

  

      def _find_build_srpm_name(self, build_nvr):

file modified
+84 -5

@@ -819,6 +819,73 @@ 

          images = new_images

          return images

  

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

+                            srpm_names=None):

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

+         `nvrs`.

+ 

+         :param list nvrs: List of NVRs defining the containerImages to return.

+         :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

+         :return: List of containerImages.

+         :rtype: list of ContainerImages.

+         """

+         image_request = {

+             "objectType": "containerImage",

+             "query": {

+                 "$and": [

+                     {

+                         "$or": [{

+                             "field": "brew.build",

+                             "op": "=",

+                             "rvalue": nvr

+                         } for nvr in nvrs]

+                     },

+                     {

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

+                         "op": "=",

+                         "rvalue": "buildfile"

+                     },

+                 ]

+             },

+             "projection": self._get_default_projection()

+         }

+ 

+         if content_sets is not None:

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

+                 {

+                     "$or": [{

+                         "field": "content_sets.*",

+                         "op": "=",

+                         "rvalue": r

+                     } for r in content_sets]

+                 }

+             )

+ 

+         if srpm_names is not None:

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

+                 {

+                     "$or": [{

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

+                         "op": "=",

+                         "rvalue": srpm_name

+                     } for srpm_name in srpm_names]

+                 }

+             )

+ 

+         if published is not None:

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

+                 {

+                     "field": "repositories.*.published",

+                     "op": "=",

+                     "rvalue": published

+                 })

+ 

+         images = self.find_container_images(image_request)

+         return images

+ 

      def find_unpublished_image_for_build(self, build):

          """

          Returns the unpublished variant of Docker image specified by `build`

@@ -1014,7 +1081,8 @@ 

      def find_images_with_packages_from_content_set(

              self, srpm_names, content_sets, filter_fnc=None,

              published=True, deprecated=False,

-             release_category="Generally Available"):

+             release_category="Generally Available",

+             leaf_container_images=None):

          """Query lightblue and find containers which contain given

          package from one of content sets

  

@@ -1033,6 +1101,9 @@ 

              repositories

          :param str release_category: filter only repositories with specific

              release category (options: Deprecated, Generally Available, Beta, Tech Preview)

+         :param list leaf_container_images: List of NVRs of leaf images to

+             consider for the rebuild. If not set, all images found in

+             Lightblue will be considered for rebuild.

  

          :return: a list of dictionaries with three keys - repository, commit and

              srpm_nevra. Repository is a name git repository including the

@@ -1045,8 +1116,12 @@ 

              published, deprecated, release_category)

          if not repos:

              return []

-         images = self.find_images_with_included_srpms(

-             content_sets, srpm_names, repos, published)

+         if not leaf_container_images:

+             images = self.find_images_with_included_srpms(

+                 content_sets, srpm_names, repos, published)

+         else:

+             images = self.get_images_by_nvrs(

+                 leaf_container_images, published, content_sets, srpm_names)

  

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

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

@@ -1284,7 +1359,8 @@ 

  

      def find_images_to_rebuild(

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

-             release_category="Generally Available", filter_fnc=None):

+             release_category="Generally Available", filter_fnc=None,

+             leaf_container_images=None):

          """

          Find images to rebuild through image build layers

  

@@ -1309,10 +1385,13 @@ 

              will not be considered for a rebuild as well as its parent images.

              This function is used to filter out images not allowed by

              Freshmaker configuration.

+         :param list leaf_container_images: List of NVRs of leaf images to

+             consider for the rebuild. If not set, all images found in

+             Lightblue will be considered for rebuild.

          """

          images = self.find_images_with_packages_from_content_set(

              srpm_names, content_sets, filter_fnc, published, deprecated,

-             release_category)

+             release_category, leaf_container_images=leaf_container_images)

  

          def _get_images_to_rebuild(image):

              """

@@ -226,10 +226,11 @@ 

  

      def test_can_handle_manual_rebuild_with_advisory_event(self):

          event = ManualRebuildWithAdvisoryEvent(

-             "123", ["foo-container", "bar-container"],

+             "123",

              ErrataAdvisory(123, "RHBA-2017", "REL_PREP", [],

                             security_impact="",

-                            product_short_name="product"))

+                            product_short_name="product"),

+             ["foo-container", "bar-container"])

          handler = ErrataAdvisoryRPMsSignedHandler()

          ret = handler.can_handle(event)

          self.assertTrue(ret)

@@ -632,6 +633,12 @@ 

              ErrataAdvisory(123, "RHBA-2017", "REL_PREP", [],

                             security_impact="",

                             product_short_name="product"))

+         self.manual_event = ManualRebuildWithAdvisoryEvent(

+             "123",

+             ErrataAdvisory(123, "RHBA-2017", "REL_PREP", [],

+                            security_impact="",

+                            product_short_name="product"),

+             ["foo", "bar"])

          self.handler = ErrataAdvisoryRPMsSignedHandler()

          self.handler.event = self.event

  

@@ -652,7 +659,8 @@ 

          self.find_images_to_rebuild.assert_called_once_with(

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

              filter_fnc=self.handler._filter_out_not_allowed_builds,

-             published=True, release_category='Generally Available')

+             published=True, release_category='Generally Available',

+             leaf_container_images=None)

  

      @patch.object(freshmaker.conf, 'handler_build_whitelist', new={

          'ErrataAdvisoryRPMsSignedHandler': {

@@ -668,7 +676,8 @@ 

          self.find_images_to_rebuild.assert_called_once_with(

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

              filter_fnc=self.handler._filter_out_not_allowed_builds,

-             published=True, release_category='Generally Available')

+             published=True, release_category='Generally Available',

+             leaf_container_images=None)

  

      @patch.object(freshmaker.conf, 'handler_build_whitelist', new={

          'ErrataAdvisoryRPMsSignedHandler': {

@@ -686,7 +695,8 @@ 

          self.find_images_to_rebuild.assert_called_once_with(

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

              filter_fnc=self.handler._filter_out_not_allowed_builds,

-             published=None, release_category=None)

+             published=None, release_category=None,

+             leaf_container_images=None)

  

      @patch.object(freshmaker.conf, 'handler_build_whitelist', new={

          'ErrataAdvisoryRPMsSignedHandler': {

@@ -702,4 +712,23 @@ 

          self.find_images_to_rebuild.assert_called_once_with(

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

              filter_fnc=self.handler._filter_out_not_allowed_builds,

-             published=True, release_category='Generally Available')

+             published=True, release_category='Generally Available',

+             leaf_container_images=None)

+ 

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

+         'ErrataAdvisoryRPMsSignedHandler': {

+             'image': {'advisory_name': 'RHBA-*',

+                       'published': True}

+         }

+     })

+     @patch('os.path.exists', return_value=True)

+     def test_manual_event_leaf_container_images(self, exists):

+         self.handler.event = self.manual_event

+         for x in self.handler._find_images_to_rebuild(123456):

+             pass

+ 

+         self.find_images_to_rebuild.assert_called_once_with(

+             set(['httpd']), ['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
+38

@@ -1360,6 +1360,44 @@ 

                  "openssl",

                  ["dummy-content-set-1"])

  

+     @patch('freshmaker.lightblue.ContainerImage.resolve')

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

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

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

+     def test_images_with_content_set_packages_leaf_container_images(

+             self, exists, cont_images, cont_repos, resolve):

+ 

+         exists.return_value = True

+         cont_images.return_value = self.fake_container_images

+         cont_repos.return_value = self.fake_repositories_with_content_sets

+ 

+         lb = LightBlue(server_url=self.fake_server_url,

+                        cert=self.fake_cert_file,

+                        private_key=self.fake_private_key)

+         lb.find_images_with_packages_from_content_set(

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

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

+         cont_images.assert_called_once_with(

+             {'query': {

+                 '$and': [

+                     {'$or': [

+                         {'field': 'brew.build', 'rvalue': 'foo', 'op': '='},

+                         {'field': 'brew.build', 'rvalue': 'bar', 'op': '='}]},

+                     {'field': 'parsed_data.files.*.key', 'rvalue': 'buildfile', 'op': '='},

+                     {'$or': [{'field': 'content_sets.*', 'rvalue': 'dummy-content-set', 'op': '='}]},

+                     {'$or': [{'field': 'rpm_manifest.*.rpms.*.srpm_name', 'rvalue': 'openssl', 'op': '='}]},

+                     {'field': 'repositories.*.published', 'rvalue': True, 'op': '='}]},

+              'projection': [{'field': 'brew', 'include': True, 'recursive': True},

+                             {'field': 'parsed_data.files', 'include': True, 'recursive': True},

+                             {'field': 'parsed_data.layers.*', 'include': True, 'recursive': True},

+                             {'field': 'repositories.*.published', 'include': True, 'recursive': True},

+                             {'field': 'repositories.*.repository', 'include': True, 'recursive': True},

+                             {'field': 'repositories.*.tags.*.name', 'include': True, 'recursive': True},

+                             {'field': 'content_sets', 'include': True, 'recursive': True},

+                             {'field': 'rpm_manifest.*.rpms', 'include': True, 'recursive': True},

+                             {'field': 'rpm_manifest.*.rpms.*.srpm_name', 'include': True, 'recursive': True}],

+              'objectType': 'containerImage'})

+ 

  

  class TestEntityVersion(helpers.FreshmakerTestCase):

      """Test case for ensuring correct entity version in request"""

In previous PR, I made it possible to generate and handle ManualRebuildWithAdvisoryEvent in the ErrataAdvisoryRPMSSignedHandler which is responsible for building updated container images as a result of advisory update.

The ManualRebuildWithAdvisoryEvent contains container_images list with NVRs of container images to rebuild. This list is so far not respected by the handler - it would rebuild all the images no matter which ones have been marked for rebuild when triggering manual rebuild.

This PR changes the ErrataAdvisoryRPMsSignedHandler to respect container_images list - it passes this list down to Lightblue class which uses it to get just those container images from Lightblue instead of querying all images in lightblue.

Pull-Request has been merged by jkaluza

10 months ago