From 397a6de033d42ffc9b0abe4da47c5af3447c4baf Mon Sep 17 00:00:00 2001
From: Luiz Carvalho <lucarval@redhat.com>
Date: Mar 10 2020 17:07:25 +0000
Subject: Aggregate multi-arch info in ContainerImage


Signed-off-by: Luiz Carvalho <lucarval@redhat.com>

---

diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py
index 8de7f43..893e328 100644
--- a/freshmaker/lightblue.py
+++ b/freshmaker/lightblue.py
@@ -137,6 +137,13 @@ class ContainerImage(dict):
     def create(cls, data):
         image = cls()
         image.update(data)
+
+        arch = data.get('architecture')
+        image['multi_arch_rpm_manifest'] = {}
+        rpm_manifest = data.get('rpm_manifest')
+        if arch and rpm_manifest:
+            image['multi_arch_rpm_manifest'][arch] = rpm_manifest
+
         return image
 
     def __hash__(self):
@@ -157,6 +164,23 @@ class ContainerImage(dict):
         else:
             self['error'] += "; " + str(err)
 
+    def update_multi_arch(self, image):
+        """
+        Update multi-arch attributes for this image from another image.
+
+        :param ContainerImage image: the container image object to copy multi
+            arch attributes from
+        :rtype: None
+        """
+        image_arch = image.get('architecture')
+        if not image_arch:
+            return
+
+        image_rpm_manifest = image.get('rpm_manifest')
+        if not image_rpm_manifest:
+            return
+        self['multi_arch_rpm_manifest'][image_arch] = image_rpm_manifest
+
     @property
     def is_base_image(self):
         return (self['parent'] is None and
@@ -631,10 +655,21 @@ class LightBlue(object):
         response = self._make_request(url, request)
 
         images = []
+        nvr_to_arches = {}
         for image_data in response['processed']:
-            image = ContainerImage()
-            image.update(image_data)
+            image = ContainerImage.create(image_data)
             images.append(image)
+
+            # TODO: In the future, we may want to combine different ContainerImage
+            # objects into a single object. For now, ensure that whichever object
+            # is used by caller contains multi-arch information.
+            nvr = image['brew']['build']
+            nvr_to_arches.setdefault(nvr, [])
+            nvr_to_arches[nvr].append(image)
+            for arch_image in nvr_to_arches[nvr][:-1]:
+                arch_image.update_multi_arch(image)
+                image.update_multi_arch(arch_image)
+
         return images
 
     def _set_container_repository_filters(
@@ -718,6 +753,7 @@ class LightBlue(object):
             {"field": "repositories.*.tags.*.name", "include": True, "recursive": True},
             {"field": "content_sets", "include": True, "recursive": True},
             {"field": "parent_brew_build", "include": True, "recursive": False},
+            {"field": "architecture", "include": True, "recursive": False},
         ]
         if include_rpm_manifest:
             if srpm_names:
diff --git a/freshmaker/odcsclient.py b/freshmaker/odcsclient.py
index 5de3133..0aebd96 100644
--- a/freshmaker/odcsclient.py
+++ b/freshmaker/odcsclient.py
@@ -100,7 +100,7 @@ class FreshmakerODCSClient(object):
 
     def _fake_odcs_new_compose(
             self, compose_source, tag, packages=None, results=None,
-            builds=None):
+            builds=None, arches=None):
         """
         Fake odcs.new_compose(...) method used in the dry run mode.
 
@@ -111,7 +111,7 @@ class FreshmakerODCSClient(object):
         """
         self.handler.log_info(
             "DRY RUN: Calling fake odcs.new_compose with args: %r",
-            (compose_source, tag, packages, results))
+            (compose_source, tag, packages, results, arches))
 
         # In case we run in DRY_RUN mode, we need to initialize
         # FAKE_COMPOSE_ID to the id of last ODCS compose to give the IDs
@@ -128,6 +128,8 @@ class FreshmakerODCSClient(object):
         }
         if builds:
             new_compose['builds'] = builds
+        if arches:
+            new_compose['arches'] = arches
 
         # Generate and inject the ODCSComposeStateChangeEvent event.
         event = ODCSComposeStateChangeEvent(
@@ -317,39 +319,46 @@ class FreshmakerODCSClient(object):
         :rtype: dict
         """
 
-        if not image.get('rpm_manifest'):
-            self.handler.log_warn('"rpm_manifest" not set in image.')
-            return
-
-        rpm_manifest = image["rpm_manifest"][0]
-        if not rpm_manifest.get('rpms'):
+        if not image.get('multi_arch_rpm_manifest'):
+            self.handler.log_warn('"multi_arch_rpm_manifest" not set in image.')
             return
 
         builds = set()
         packages = set()
-        for rpm in rpm_manifest["rpms"]:
-            parsed_nvr = kobo.rpmlib.parse_nvra(rpm["srpm_nevra"])
-            srpm_nvr = "%s-%s-%s" % (parsed_nvr["name"], parsed_nvr["version"],
-                                     parsed_nvr["release"])
-            builds.add(srpm_nvr)
-            parsed_nvr = kobo.rpmlib.parse_nvra(rpm["nvra"])
-            packages.add(parsed_nvr["name"])
-
-        # ODCS client expects list and not set for packages/builds, so convert
+
+        for rpm_manifest in image['multi_arch_rpm_manifest'].values():
+            # For some reason, the rpm manifest itself is always a wrapped in a list.
+            if not rpm_manifest:
+                continue
+            for rpm in rpm_manifest[0].get("rpms", []):
+                parsed_nvr = kobo.rpmlib.parse_nvra(rpm["srpm_nevra"])
+                srpm_nvr = "%s-%s-%s" % (parsed_nvr["name"], parsed_nvr["version"],
+                                         parsed_nvr["release"])
+                builds.add(srpm_nvr)
+                parsed_nvr = kobo.rpmlib.parse_nvra(rpm["nvra"])
+                packages.add(parsed_nvr["name"])
+
+        if not builds or not packages:
+            self.handler.log_warn('No builds or packages identified in image')
+            return
+
+        # ODCS client expects list and not set for packages/builds/arches, so convert
         # them to lists. Sorting the lists to make them easy to look up, e.g.
         # in logs, and easy to test.
         builds = sorted(builds)
         packages = sorted(packages)
+        arches = sorted(image['arches'].split())
 
         if not self.handler.dry_run:
             with krb_context():
                 new_compose = create_odcs_client().new_compose(
                     "", 'build', packages=packages, builds=builds,
-                    sigkeys=conf.odcs_sigkeys, flags=["no_deps"])
+                    arches=arches, sigkeys=conf.odcs_sigkeys,
+                    flags=["no_deps"])
         else:
             new_compose = self._fake_odcs_new_compose(
                 "", 'build', packages=packages,
-                builds=builds)
+                builds=builds, arches=arches)
 
         self.handler.log_info(
             "Started generating ODCS 'build' type compose %d." % (
diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py
index a95b74b..f071a55 100644
--- a/tests/test_lightblue.py
+++ b/tests/test_lightblue.py
@@ -332,6 +332,57 @@ class TestContainerImageObject(helpers.FreshmakerTestCase):
         self.assertEqual('1233829', image['_id'])
         self.assertEqual('20151210T10:09:35.000-0500', image['brew']['completion_date'])
 
+    def test_update_multi_arch(self):
+        rpm_manifest_x86_64 = [{'rpms': [{'name': 'spam'}]}]
+        image_x86_64 = ContainerImage.create({
+            '_id': '1233829',
+            'architecture': 'amd64',
+            'brew': {
+                'completion_date': '20151210T10:09:35.000-0500',
+                'build': 'jboss-webserver-3-webserver30-tomcat7-openshift-docker-1.1-6',
+                'package': 'jboss-webserver-3-webserver30-tomcat7-openshift-docker'
+            },
+            'rpm_manifest': rpm_manifest_x86_64,
+        })
+
+        rpm_manifest_s390x = [{'rpms': [{'name': 'maps'}]}]
+        image_s390x = ContainerImage.create({
+            '_id': '1233829',
+            'architecture': 's390x',
+            'brew': {
+                'completion_date': '20151210T10:09:35.000-0500',
+                'build': 'jboss-webserver-3-webserver30-tomcat7-openshift-docker-1.1-6',
+                'package': 'jboss-webserver-3-webserver30-tomcat7-openshift-docker'
+            },
+            'rpm_manifest': rpm_manifest_s390x,
+        })
+
+        self.assertEqual(image_x86_64['rpm_manifest'], rpm_manifest_x86_64)
+        self.assertEqual(image_x86_64['multi_arch_rpm_manifest'], {'amd64': rpm_manifest_x86_64})
+        self.assertEqual(image_s390x['rpm_manifest'], rpm_manifest_s390x)
+        self.assertEqual(image_s390x['multi_arch_rpm_manifest'], {'s390x': rpm_manifest_s390x})
+
+        image_x86_64.update_multi_arch(image_s390x)
+        self.assertEqual(image_x86_64['rpm_manifest'], rpm_manifest_x86_64)
+        self.assertEqual(image_x86_64['multi_arch_rpm_manifest'], {
+            'amd64': rpm_manifest_x86_64,
+            's390x': rpm_manifest_s390x
+        })
+        self.assertEqual(image_s390x['rpm_manifest'], rpm_manifest_s390x)
+        self.assertEqual(image_s390x['multi_arch_rpm_manifest'], {'s390x': rpm_manifest_s390x})
+
+        image_s390x.update_multi_arch(image_x86_64)
+        self.assertEqual(image_x86_64['rpm_manifest'], rpm_manifest_x86_64)
+        self.assertEqual(image_x86_64['multi_arch_rpm_manifest'], {
+            'amd64': rpm_manifest_x86_64,
+            's390x': rpm_manifest_s390x
+        })
+        self.assertEqual(image_s390x['rpm_manifest'], rpm_manifest_s390x)
+        self.assertEqual(image_s390x['multi_arch_rpm_manifest'], {
+            'amd64': rpm_manifest_x86_64,
+            's390x': rpm_manifest_s390x
+        })
+
     def test_log_error(self):
         image = ContainerImage.create({
             'brew': {
@@ -894,6 +945,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase):
                 {"field": "_id", "include": True},
                 {"field": "image_id", "include": True},
                 {"field": "brew", "include": True, "recursive": True},
+                {"field": "architecture", "include": True, "recursive": False},
             ],
         }
 
@@ -917,6 +969,74 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase):
         self.assertEqual('jboss-webserver-3-webserver30-tomcat7-openshift-docker',
                          image['brew']['package'])
 
+    @patch('freshmaker.lightblue.ContainerImage.update_multi_arch')
+    @patch('freshmaker.lightblue.requests.post')
+    def test_find_container_images_with_multi_arch(self, post, update_multi_arch):
+        post.return_value.status_code = http.client.OK
+        post.return_value.json.return_value = {
+            'modifiedCount': 0,
+            'resultMetadata': [],
+            'entityVersion': '0.0.12',
+            'hostname': self.fake_server_url,
+            'matchCount': 2,
+            'processed': [
+                {
+                    '_id': '57ea8d1f9c624c035f96f4b0',
+                    'image_id': 'e0f97342ddf6a09972434f98837b5fd8b5bed9390f32f1d63e8a7e4893208af7',
+                    'architecture': 'amd64',
+                    'brew': {
+                        'completion_date': '20151210T10:09:35.000-0500',
+                        'build': 'sadc-container-1.1-6',
+                        'package': 'sadc-container',
+                    },
+                },
+                {
+                    '_id': '57ea8d289c624c035f96f4db',
+                    'image_id': 'c1ef3345f36b901b0bddc7ab01ea3f3c83c886faa243e02553f475124eb4b46c',
+                    'architecture': 's390x',
+                    'brew': {
+                        'completion_date': '20151203T00:35:30.000-0500',
+                        'build': 'sadc-container-1.1-6',
+                        'package': 'sadc-container',
+                    },
+                }
+            ],
+            'status': 'COMPLETE',
+            'entity': 'containerImage'
+        }
+
+        fake_request = {
+            "objectType": "containerImage",
+            "projection": [
+                {"field": "_id", "include": True},
+                {"field": "image_id", "include": True},
+                {"field": "brew", "include": True, "recursive": True},
+                {"field": "architecture", "include": True, "recursive": False},
+            ],
+        }
+
+        with patch('os.path.exists'):
+            lb = LightBlue(server_url=self.fake_server_url,
+                           cert=self.fake_cert_file,
+                           private_key=self.fake_private_key)
+            images = lb.find_container_images(request=fake_request)
+
+        post.assert_called_once_with(
+            '{}/{}/'.format(lb.api_root, 'find/containerImage'),
+            data=json.dumps(fake_request),
+            verify=lb.verify_ssl,
+            cert=(self.fake_cert_file, self.fake_private_key),
+            headers={'Content-Type': 'application/json'}
+        )
+        self.assertEqual(2, len(images))
+        # Verify update_multi_arch is first called with the second image,
+        # then with the first image. This is to ensure all ContainerImage
+        # objects for the same Brew build have the same multi arch data.
+        self.assertEqual(
+            ['c1ef3345f36b901b0bddc7ab01ea3f3c83c886faa243e02553f475124eb4b46c',
+             'e0f97342ddf6a09972434f98837b5fd8b5bed9390f32f1d63e8a7e4893208af7'],
+            [call_args[0][0]['image_id'] for call_args in update_multi_arch.call_args_list])
+
     @patch('freshmaker.lightblue.requests.post')
     def test_find_container_repositories(self, post):
         post.return_value.status_code = http.client.OK
@@ -1274,6 +1394,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase):
                                  "git_branch": "mybranch",
                                  "error": None,
                                  "arches": None,
+                                 "multi_arch_rpm_manifest": {},
                                  "odcs_compose_ids": None,
                                  "parent_build_id": None,
                                  "parent_image_builds": None,
@@ -1816,6 +1937,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase):
                             {'field': 'repositories.*.tags.*.name', 'include': True, 'recursive': True},
                             {'field': 'content_sets', 'include': True, 'recursive': True},
                             {'field': 'parent_brew_build', 'include': True, 'recursive': False},
+                            {'field': 'architecture', 'include': True, 'recursive': False},
                             {'field': 'rpm_manifest.*.rpms', 'include': True, 'recursive': True},
                             {'field': 'rpm_manifest.*.rpms.*.srpm_name', 'include': True, 'recursive': True}],
              'objectType': 'containerImage'})
diff --git a/tests/test_odcsclient.py b/tests/test_odcsclient.py
index ec7d61c..951f1a7 100644
--- a/tests/test_odcsclient.py
+++ b/tests/test_odcsclient.py
@@ -26,6 +26,7 @@ from unittest.mock import patch, Mock
 from odcs.client.odcs import AuthMech
 
 from freshmaker import conf, db
+from freshmaker.lightblue import ContainerImage
 from freshmaker.models import Event, ArtifactBuild, Compose
 from freshmaker.odcsclient import create_odcs_client
 from freshmaker.types import ArtifactBuildState, EventState, ArtifactType
@@ -247,27 +248,33 @@ class TestPrepareYumRepo(helpers.ModelsTestCase):
             self.assertTrue(build.state_reason.endswith(
                 "of advisory 123 is the latest build in its candidate tag."))
 
-    def _get_fake_container_image(self):
-        return {
-            u'rpm_manifest': [
-                {u'rpms': [{u'architecture': u'noarch',
-                            u'gpg': u'199e2f91fd431d51',
-                            u'name': u'apache-commons-lang',
-                            u'nvra': u'apache-commons-lang-2.6-15.el7.noarch',
-                            u'release': u'15.el7',
-                            u'srpm_name': u'apache-commons-lang',
-                            u'srpm_nevra': u'apache-commons-lang-0:2.6-15.el7.src',
-                            u'summary': u'Provides a host of helper utilities for the java.lang API',
-                            u'version': u'2.6'},
-                           {u'architecture': u'noarch',
-                            u'gpg': u'199e2f91fd431d51',
-                            u'name': u'avalon-logkit',
-                            u'nvra': u'avalon-logkit-2.1-14.el7.noarch',
-                            u'release': u'14.el7',
-                            u'srpm_name': u'avalon-logkit',
-                            u'srpm_nevra': u'avalon-logkit-0:2.1-14.el7.src',
-                            u'summary': u'Java logging toolkit',
-                            u'version': u'2.1'}]}]}
+    def _get_fake_container_image(self, architecture='amd64', arches='x86_64'):
+        rpm_manifest = [{u'rpms': [{
+            u'architecture': architecture,
+            u'gpg': u'199e2f91fd431d51',
+            u'name': u'apache-commons-lang',
+            u'nvra': u'apache-commons-lang-2.6-15.el7.noarch',
+            u'release': u'15.el7',
+            u'srpm_name': u'apache-commons-lang',
+            u'srpm_nevra': u'apache-commons-lang-0:2.6-15.el7.src',
+            u'summary': u'Provides a host of helper utilities for the java.lang API',
+            u'version': u'2.6'
+        }, {
+            u'architecture': architecture,
+            u'gpg': u'199e2f91fd431d51',
+            u'name': u'avalon-logkit',
+            u'nvra': u'avalon-logkit-2.1-14.el7.noarch',
+            u'release': u'14.el7',
+            u'srpm_name': u'avalon-logkit',
+            u'srpm_nevra': u'avalon-logkit-0:2.1-14.el7.src',
+            u'summary': u'Java logging toolkit',
+            u'version': u'2.1'
+        }]}]
+        return ContainerImage.create({
+            u'arches': arches,  # Populated based on Brew build
+            u'architecture': architecture,  # Populated from Lightblue data
+            u'rpm_manifest': rpm_manifest,
+        })
 
     @patch('freshmaker.odcsclient.create_odcs_client')
     @patch('time.sleep')
@@ -295,7 +302,43 @@ class TestPrepareYumRepo(helpers.ModelsTestCase):
         # Ensure new_compose is called to request a new compose
         odcs.new_compose.assert_called_once_with(
             '', 'build', builds=['apache-commons-lang-2.6-15.el7', 'avalon-logkit-2.1-14.el7'],
-            flags=['no_deps'], packages=[u'apache-commons-lang', u'avalon-logkit'], sigkeys=[])
+            flags=['no_deps'], packages=[u'apache-commons-lang', u'avalon-logkit'], sigkeys=[],
+            arches=['x86_64'])
+
+    @patch('freshmaker.odcsclient.create_odcs_client')
+    @patch('time.sleep')
+    def test_prepare_odcs_compose_with_multi_arch_image_rpms(
+            self, sleep, create_odcs_client):
+        odcs = create_odcs_client.return_value
+        odcs.new_compose.return_value = {
+            "id": 3,
+            "result_repo": "http://localhost/composes/latest-odcs-3-1/compose/Temporary",
+            "result_repofile": "http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo",
+            "source": "f26",
+            "source_type": 1,
+            "state": 0,
+            "state_name": "wait",
+        }
+
+        arches = 's390x x86_64'
+        image_x86_64 = self._get_fake_container_image(architecture='amd64', arches=arches)
+        image_s390x = self._get_fake_container_image(architecture='s390x', arches=arches)
+
+        for image in (image_x86_64, image_s390x):
+            handler = MyHandler()
+            compose = handler.odcs.prepare_odcs_compose_with_image_rpms(image)
+
+            db.session.refresh(self.ev)
+            self.assertEqual(3, compose['id'])
+
+            # Ensure new_compose is called to request a new multi-arch
+            # compose regardless of which image is used.
+            odcs.new_compose.assert_called_once_with(
+                '', 'build', builds=['apache-commons-lang-2.6-15.el7', 'avalon-logkit-2.1-14.el7'],
+                flags=['no_deps'], packages=[u'apache-commons-lang', u'avalon-logkit'], sigkeys=[],
+                arches=['s390x', 'x86_64'])
+
+            odcs.reset_mock()
 
     @patch("freshmaker.consumer.get_global_consumer")
     def test_prepare_odcs_compose_with_image_rpms_dry_run(self, global_consumer):
@@ -324,11 +367,19 @@ class TestPrepareYumRepo(helpers.ModelsTestCase):
         self.assertEqual(compose, None)
 
         compose = handler.odcs.prepare_odcs_compose_with_image_rpms(
-            {"rpm_manifest": []})
+            {"multi_arch_rpm_manifest": {}})
+        self.assertEqual(compose, None)
+
+        compose = handler.odcs.prepare_odcs_compose_with_image_rpms(
+            {"multi_arch_rpm_manifest": {
+                "amd64": [],
+            }})
         self.assertEqual(compose, None)
 
         compose = handler.odcs.prepare_odcs_compose_with_image_rpms(
-            {"rpm_manifest": [{"rpms": []}]})
+            {"multi_arch_rpm_manifest": {
+                "amd64": [{"rpms": []}],
+            }})
         self.assertEqual(compose, None)
 
 

