#322 Request ODCS compose with all RPMs installed in the *unshipped* container image.
Merged 5 years ago by jkaluza. Opened 5 years ago by jkaluza.
jkaluza/freshmaker odcs-builds-source  into  master

@@ -39,6 +39,14 @@ 

  from freshmaker.odcsclient import COMPOSE_STATES

  

  

+ class ODCSComposeNotReady(Exception):

+     """

+     Raised when ODCS compose is still generating and therefore not ready

+     to be used to build an image.

+     """

+     pass

+ 

+ 

  def fail_event_on_handler_exception(func):

      """

      Decorator which marks the models.Event associated with handler by
@@ -196,7 +204,8 @@ 

          elif type(db_object) == ArtifactBuild:

              self._db_event_id = db_object.event.id

              self._db_artifact_build_id = db_object.id

-             self._log_prefix = "%s: " % str(db_object.event)

+             # Prefix logs with "<models.Event> (<models.ArtifactBuild>):".

+             self._log_prefix = "%s (%s): " % (str(db_object.event), str(db_object))

          else:

              raise ProgrammingError(

                  "Unsupported context type passed to BaseHandler.set_context()")
@@ -480,9 +489,28 @@ 

          # cases, we want to convert compose_ids to repository URLs. Otherwise,

          # just pass compose_ids to OSBS via Koji.

          if repo_urls:

-             repo_urls += [self.odcs_get_compose(

-                 compose_id)['result_repofile']

-                 for compose_id in compose_ids]

+             for compose_id in compose_ids:

+                 odcs_compose = self.odcs_get_compose(compose_id)

+                 if odcs_compose["state"] in [COMPOSE_STATES['wait'],

+                                              COMPOSE_STATES['generating']]:

+                     # In case the ODCS compose is still generating, raise an

+                     # exception.

+                     msg = ("Compose %s is not in done state. Waiting with "

+                            "rebuild." % (str(compose_id)))

+                     self.log_info(msg)

+                     raise ODCSComposeNotReady(msg)

+                 elif odcs_compose["state"] != COMPOSE_STATES["done"]:

+                     # In case the compose is not in 'done' state, mark the

+                     # build as failed. We should never get expired here,

+                     # because the compose has been submitted just a minutes

+                     # ago. If we get "expired" here, it is sign of an issue,

+                     # because that means we are trying to build quite old

+                     # image.

+                     build.transition(

+                         ArtifactBuildState.FAILED.value,

+                         "Compose %s is not in 'done' state." % str(compose_id))

+                     return

+                 repo_urls.append(odcs_compose["result_repofile"])

              compose_ids = []

  

          return self.build_container(
@@ -538,7 +566,12 @@ 

          def build_image(build):

              self.set_context(build)

              repo_urls = self.get_repo_urls(build)

-             build.build_id = self.build_image_artifact_build(build, repo_urls)

+             try:

+                 build.build_id = self.build_image_artifact_build(build, repo_urls)

+             except ODCSComposeNotReady:

+                 # We skip this image for now. It will be built once the ODCS

+                 # compose is finished.

+                 return

              if build.build_id:

                  build.transition(

                      ArtifactBuildState.BUILD.value,

@@ -25,6 +25,7 @@ 

  import json

  import koji

  import requests

+ import kobo.rpmlib

  

  from six.moves import cStringIO

  from six.moves import configparser
@@ -136,16 +137,11 @@ 

  

          # Log what we are going to rebuild

          self._check_images_to_rebuild(db_event, builds)

- 

-         if event.advisory.state == 'SHIPPED_LIVE':

-             # As mentioned above, no need to wait for the event of new compose

-             # is generated in ODCS, so we can start to rebuild the first batch

-             # from here immediately.

-             self.start_to_build_images(

-                 db_event.get_image_builds_in_first_batch(db.session))

+         self.start_to_build_images(

+             db_event.get_image_builds_in_first_batch(db.session))

  

          msg = ('Waiting for composes to finish in order to start to '

-                'schedule base images for rebuild.')

+                'schedule images for rebuild.')

          db_event.transition(EventState.BUILDING, msg)

  

          return []
@@ -314,6 +310,53 @@ 

  

          return new_compose

  

+     def _prepare_odcs_compose_with_image_rpms(self, image):

+         """

+         Request a compose from ODCS for builds included in Errata advisory

+ 

+         Run a compose in ODCS to contain required RPMs for rebuilding images

+         later.

+ 

+         :param dict image: Container image representation as returned by

+             LightBlue class.

+         :return: a mapping returned from ODCS that represents the request

+             compose.

+         :rtype: dict

+         """

+ 

+         if not image.get('rpm_manifest'):

+             self.log_warn('"rpm_manifest" not set in image.')

+             return

+ 

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

+         if not rpm_manifest.get('rpms'):

+             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"])

+ 

+         if not self.dry_run:

+             with krb_context():

+                 new_compose = create_odcs_client().new_compose(

+                     "", 'build', packages=packages, builds=builds,

+                     sigkeys=conf.odcs_sigkeys, flags=["no_deps"])

+         else:

+             new_compose = self._fake_odcs_new_compose(

+                 "", 'build', packages=packages,

+                 builds=builds)

+ 

+         self.log_info("Started generating ODCS 'build' type compose %d." % (

+             new_compose["id"]))

+ 

+         return new_compose

+ 

      def _get_base_image_build_target(self, image):

          dockerfile = image.dockerfile

          image_build_conf_url = dockerfile['content_url'].replace(
@@ -491,6 +534,9 @@ 

              stored into database.

          :rtype: dict

          """

+         db_event = Event.get_or_create(

+             db.session, event.msg_id, event.search_key, event.__class__)

+ 

          # Used as tmp dict with {brew_build_nvr: ArtifactBuild, ...} mapping.

          builds = builds or {}

  
@@ -500,6 +546,10 @@ 

  

          for batch in batches:

              for image in batch:

+                 # Reset context to db_event for each iteration before

+                 # the ArtifactBuild is created.

+                 self.set_context(db_event)

+ 

                  nvr = image["brew"]["build"]

                  if nvr in builds:

                      self.log_debug("Skipping recording build %s, "
@@ -539,6 +589,10 @@ 

                      original_nvr=nvr,

                      rebuilt_nvr=rebuilt_nvr)

  

+                 # Set context to particular build so logging shows this build

+                 # in case of error.

+                 self.set_context(build)

+ 

                  build.transition(state, state_reason)

  

                  build_args = {}
@@ -577,6 +631,18 @@ 

                              build.add_composes(db.session, [db_compose])

                              db.session.commit()

  

+                     # Unpublished images can contain unreleased RPMs, so generate

+                     # the ODCS compose with all the RPMs in the image to allow

+                     # installation of possibly unreleased RPMs.

+                     if not image["published"]:

+                         compose = self._prepare_odcs_compose_with_image_rpms(image)

+                         if compose:

+                             db_compose = Compose(odcs_compose_id=compose['id'])

+                             db.session.add(db_compose)

+                             db.session.commit()

+                             build.add_composes(db.session, [db_compose])

+                             db.session.commit()

+ 

                      # TODO: uncomment following code after boot.iso compose is

                      # deployed in ODCS server.

  #                    if image.is_base_image:
@@ -598,6 +664,9 @@ 

  

                  builds[nvr] = build

  

+         # Reset context to db_event.

+         self.set_context(db_event)

+ 

          return builds

  

      def _filter_out_not_allowed_builds(self, image):

file modified
+12
@@ -181,6 +181,7 @@ 

              "error": None,

              "arches": None,

              "odcs_compose_ids": None,

+             "published": None,

          }

  

      @region.cache_on_arguments()
@@ -492,6 +493,16 @@ 

                      "is suspicious.", self["brew"]["build"])

          self.update({"content_sets": []})

  

+     def resolve_published(self, lb_instance):

+         # Get the published version of this image to find out if the image

+         # was actually published.

+         image = lb_instance.get_images_by_nvrs(

+             self["brew"]["build"], published=True)

+         if image:

+             self["published"] = True

+         else:

+             self["published"] = False

+ 

      def resolve(self, lb_instance, children=None):

          """

          Resolves the Container image - populates additional metadata by
@@ -501,6 +512,7 @@ 

          """

          self.resolve_commit()

          self.resolve_content_sets(lb_instance, children)

+         self.resolve_published(lb_instance)

  

  

  class LightBlue(object):

@@ -95,6 +95,7 @@ 

              },

              "generate_pulp_repos": True,

              "odcs_compose_ids": None,

+             "published": False,

          })

          self.image_b = ContainerImage({

              'repository': 'repo_2',
@@ -115,6 +116,7 @@ 

              },

              "generate_pulp_repos": True,

              "odcs_compose_ids": None,

+             "published": False,

          })

          self.image_c = ContainerImage({

              'repository': 'repo_2',
@@ -136,6 +138,7 @@ 

              },

              "generate_pulp_repos": True,

              "odcs_compose_ids": None,

+             "published": False,

          })

          self.image_d = ContainerImage({

              'repository': 'repo_2',
@@ -157,6 +160,7 @@ 

              },

              "generate_pulp_repos": True,

              "odcs_compose_ids": None,

+             "published": False,

          })

          self.image_e = ContainerImage({

              'repository': 'repo_2',
@@ -178,6 +182,7 @@ 

              },

              "generate_pulp_repos": True,

              "odcs_compose_ids": None,

+             "published": False,

          })

          self.image_f = ContainerImage({

              'repository': 'repo_2',
@@ -199,6 +204,7 @@ 

              },

              "generate_pulp_repos": True,

              "odcs_compose_ids": None,

+             "published": False,

          })

          # For simplicify, mocking _find_images_to_rebuild to just return one

          # batch, which contains images found for rebuild from parent to
@@ -375,7 +381,7 @@ 

          handler.handle(self.rhsa_event)

  

          prepare_yum_repos_for_rebuilds.assert_called_once()

-         start_to_build_images.assert_not_called()

+         start_to_build_images.assert_called_once()

  

          db_event = Event.get(db.session, self.rhsa_event.msg_id)

          self.assertEqual(EventState.BUILDING.value, db_event.state)

@@ -323,6 +323,7 @@ 

              "generate_pulp_repos": True,

              "arches": "x86_64",

              "odcs_compose_ids": [10, 11],

+             "published": False,

          })

  

      @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.create_odcs_client')
@@ -648,6 +649,71 @@ 

              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'}]}]}

+ 

+     @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.'

+            'create_odcs_client')

+     @patch('time.sleep')

+     def test_prepare_odcs_compose_with_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",

+         }

+ 

+         image = self._get_fake_container_image()

+ 

+         handler = ErrataAdvisoryRPMsSignedHandler()

+         compose = handler._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 compose

+         odcs.new_compose.assert_called_once_with(

+             '', 'build', builds=set(['avalon-logkit-2.1-14.el7', 'apache-commons-lang-2.6-15.el7']),

+             flags=['no_deps'], packages=set([u'avalon-logkit', u'apache-commons-lang']), sigkeys=[])

+ 

+     def test_prepare_odcs_compose_with_image_rpms_no_rpm_manifest(self):

+         handler = ErrataAdvisoryRPMsSignedHandler()

+ 

+         compose = handler._prepare_odcs_compose_with_image_rpms({})

+         self.assertEqual(compose, None)

+ 

+         compose = handler._prepare_odcs_compose_with_image_rpms(

+             {"rpm_manifest": []})

+         self.assertEqual(compose, None)

+ 

+         compose = handler._prepare_odcs_compose_with_image_rpms(

+             {"rpm_manifest": [{"rpms": []}]})

+         self.assertEqual(compose, None)

+ 

  

  class TestErrataAdvisoryStateChangedHandler(helpers.ModelsTestCase):

  
@@ -894,6 +960,7 @@ 

                  "generate_pulp_repos": True,

                  "arches": "x86_64",

                  "odcs_compose_ids": None,

+                 "published": False,

              })],

              [ContainerImage({

                  "brew": {
@@ -937,6 +1004,7 @@ 

                  "generate_pulp_repos": True,

                  "arches": "x86_64",

                  "odcs_compose_ids": None,

+                 "published": False,

              })]

          ]

  
@@ -983,6 +1051,7 @@ 

                  "generate_pulp_repos": False,

                  "arches": "x86_64",

                  "odcs_compose_ids": None,

+                 "published": False,

              })]

          ]

  
@@ -1019,7 +1088,8 @@ 

                  "commit": "123456789",

                  "target": "target-candidate",

                  "git_branch": "rhel-7",

-                 "error": None

+                 "error": None,

+                 "published": False,

              })],

              [ContainerImage({

                  "brew": {
@@ -1059,7 +1129,8 @@ 

                  "commit": "987654321",

                  "target": "target-candidate",

                  "git_branch": "rhel-7",

-                 "error": None

+                 "error": None,

+                 "published": False,

              })]

          ]

  
@@ -1114,6 +1185,7 @@ 

                  "arches": "x86_64",

                  "generate_pulp_repos": True,

                  "odcs_compose_ids": None,

+                 "published": False,

              })],

              [ContainerImage({

                  "brew": {
@@ -1157,6 +1229,7 @@ 

                  "arches": "x86_64",

                  "generate_pulp_repos": True,

                  "odcs_compose_ids": None,

+                 "published": False,

              })]

          ]

  
@@ -1203,6 +1276,7 @@ 

                  "error": "Some error occurs while getting this image.",

                  "arches": "x86_64",

                  "odcs_compose_ids": None,

+                 "published": False,

              })]

          ]

  
@@ -1239,6 +1313,7 @@ 

                  "error": "Some error occurs while getting this image.",

                  "arches": "x86_64",

                  "odcs_compose_ids": None,

+                 "published": False,

              })]

          ]

  
@@ -1275,6 +1350,7 @@ 

                  "error": "Some error occured.",

                  "arches": "x86_64",

                  "odcs_compose_ids": None,

+                 "published": False,

              })],

              [ContainerImage({

                  "brew": {
@@ -1317,6 +1393,7 @@ 

                  "error": "Some error occured too.",

                  "arches": "x86_64",

                  "odcs_compose_ids": None,

+                 "published": False,

              })]

          ]

  
@@ -1357,6 +1434,7 @@ 

                  "error": "Some error occured.",

                  "arches": "x86_64",

                  "odcs_compose_ids": None,

+                 "published": False,

              })],

          ]

  

file modified
+19 -1
@@ -29,7 +29,7 @@ 

  

  from freshmaker import db

  from freshmaker.events import ErrataAdvisoryRPMsSignedEvent

- from freshmaker.handlers import ContainerBuildHandler

+ from freshmaker.handlers import ContainerBuildHandler, ODCSComposeNotReady

  from freshmaker.models import (

      ArtifactBuild, ArtifactBuildState, ArtifactBuildCompose,

      Compose, Event, EVENT_TYPES
@@ -37,6 +37,7 @@ 

  from freshmaker.errors import UnprocessableEntity, ProgrammingError

  from freshmaker.types import ArtifactType, EventState

  from freshmaker.config import any_, all_

+ from freshmaker.odcsclient import COMPOSE_STATES

  from tests import helpers

  

  
@@ -152,6 +153,7 @@ 

              return {

                  "id": compose_id,

                  "result_repofile": "http://localhost/%d.repo" % compose_id,

+                 "state": COMPOSE_STATES["done"],

              }

  

          self.patch_odcs_get_compose = patch(
@@ -225,6 +227,22 @@ 

              arch_override='x86_64', compose_ids=[], isolated=True,

              koji_parent_build=None, release='2', repo_urls=repo_urls)

  

+     @patch("freshmaker.handlers.ContainerBuildHandler.build_container")

+     def test_build_image_artifact_build_repo_urls_compose_not_ready(

+             self, build_container):

+ 

+         def mocked_odcs_get_compose(compose_id):

+             return {

+                 "id": compose_id,

+                 "result_repofile": "http://localhost/%d.repo" % compose_id,

+                 "state": COMPOSE_STATES["generating"],

+             }

+         self.odcs_get_compose.side_effect = mocked_odcs_get_compose

+ 

+         with self.assertRaises(ODCSComposeNotReady):

+             handler = MyHandler()

+             handler.build_image_artifact_build(self.build_1, ["http://localhost/x.repo"])

+ 

  

  class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase):

      """Test BaseHandler.allow_build"""

file modified
+8 -4
@@ -1015,6 +1015,7 @@ 

                                   "error": None,

                                   "arches": None,

                                   "odcs_compose_ids": None,

+                                  "published": True,

                                   "brew": {

                                       "completion_date": u"20170421T04:27:51.000-0400",

                                       "build": "package-name-2-4-12.10",
@@ -1055,12 +1056,14 @@ 

                               },

                           ])

  

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

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

      @patch('os.path.exists')

      @patch('freshmaker.kojiservice.KojiService.get_build')

      @patch('freshmaker.kojiservice.KojiService.get_task_request')

-     def test_parent_images_with_package(self, get_task_request, get_build,

-                                         exists, cont_images):

+     def test_parent_images_with_package(

+             self, get_task_request, get_build, exists, cont_images,

+             resolve_published):

  

          get_build.return_value = {"task_id": 123456}

          get_task_request.return_value = [
@@ -1134,13 +1137,14 @@ 

  

          self.assertEqual(0, len(ret))

  

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

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

      @patch('os.path.exists')

      @patch('freshmaker.kojiservice.KojiService.get_build')

      @patch('freshmaker.kojiservice.KojiService.get_task_request')

      def test_parent_images_with_package_last_parent_content_sets(

-             self, get_task_request, get_build, exists, cont_images):

- 

+             self, get_task_request, get_build, exists, cont_images,

+             resolve_published):

          get_build.return_value = {"task_id": 123456}

          get_task_request.return_value = [

              "git://example.com/rpms/repo-1#commit_hash1", "target1", {}]

So far, Freshmaker rebuilds just shipped container images, but for release-driver
use-case, it needs to be able to rebuild also unshipped container images.

Such images can contain unshipped RPMs, so our normal way of getting them from
Pulp repositories defined by content sets will not work - Pulp repos don't
contain unshipped RPMs.

In this PR, we therefore request ODCS compose with all the RPMs installed
in the unshipped container image and use it together with Pulp repos. Using
this approach, unshipped container image can get even the so-far unshipped RPMs.

Would it make sense to check for the state that we want? In this case it can only be done. If it's removed for example, OSBS won't know to regenerate it because a URL is given and not the ODCS compose ID.

This could be simplified:

if not image.get('rpm_manifest'):
  ...

This could be simplified:

if not rpm_manifest.get('rpms'):
  ...

rebased onto 508ced9

5 years ago

Thanks for the review.

I will merge it on Monday after ODCS upgrade, so we won't get freshmaker-dev with this feature enabled and old ODCS where this feature is not supported...

Commit ff5ce9f fixes this pull-request

Pull-Request has been merged by jkaluza

5 years ago

Pull-Request has been merged by jkaluza

5 years ago