#686 Wait for components to be tagged also in final tag before marking module as done.
Merged 6 years ago by ralph. Opened 6 years ago by jkaluza.
jkaluza/fm-orchestrator wait-for-final-tag  into  master

@@ -0,0 +1,24 @@ 

+ """Add component_builds.build_time_only and component_builds.tagged_in_final.

+ 

+ Revision ID: c11a3cfec2a9

+ Revises: 3b17cd6dc583

+ Create Date: 2017-09-15 15:23:55.357689

+ 

+ """

+ 

+ # revision identifiers, used by Alembic.

+ revision = 'c11a3cfec2a9'

+ down_revision = '3b17cd6dc583'

+ 

+ from alembic import op

+ import sqlalchemy as sa

+ 

+ 

+ def upgrade():

+     op.add_column('component_builds', sa.Column('build_time_only', sa.Boolean(), nullable=True))

+     op.add_column('component_builds', sa.Column('tagged_in_final', sa.Boolean(), nullable=True))

+ 

+ 

+ def downgrade():

+     op.drop_column('component_builds', 'tagged_in_final')

+     op.drop_column('component_builds', 'build_time_only')

@@ -454,8 +454,13 @@ 

      state_reason = db.Column(db.String)

      # This stays as None until the build completes.

      nvr = db.Column(db.String)

-     # True when this component build is tagged into buildroot.

+     # True when this component build is tagged into buildroot (-build tag).

      tagged = db.Column(db.Boolean, default=False)

+     # True when this component build is tagged into final tag.

+     tagged_in_final = db.Column(db.Boolean, default=False)

+     # True when this component build is build-time only (should be tagged only

+     # to -build tag)

+     build_time_only = db.Column(db.Boolean, default=False)

  

      # A monotonically increasing integer that represents which batch or

      # iteration this *component* is currently in.  This relates to the owning

@@ -124,6 +124,11 @@ 

              # Do not tag packages which belong to -build tag to final tag.

              if not install:

                  builder.tag_artifacts(built_components_in_batch)

+             else:

+                 # For components which should not be tagged in final Koji tag,

+                 # set the build_time_only to True so we do not wait for the

+                 # non-existing tagging task to finish.

+                 component_build.build_time_only = True

  

          session.commit()

      elif (any([c.state != koji.BUILD_STATES['BUILDING']

@@ -40,9 +40,6 @@ 

  

      # Find our ModuleBuild associated with this tagged artifact.

      tag = msg.tag

-     if not tag.endswith('-build'):

-         log.debug("Tag %r does not end with '-build' suffix, ignoring" % tag)

-         return

      module_build = models.ModuleBuild.from_tag_change_event(session, msg)

      if not module_build:

          log.debug("No module build found associated with koji tag %r" % tag)
@@ -59,7 +56,10 @@ 

                                                              msg.msg_id))

  

      # Mark the component as tagged

-     component.tagged = True

+     if tag.endswith('-build'):

+         component.tagged = True

+     else:

+         component.tagged_in_final = True

      session.commit()

  

      unbuilt_components_in_batch = [
@@ -75,7 +75,8 @@ 

      # have been built successfully.

      untagged_components = [

          c for c in module_build.up_to_current_batch()

-         if not c.tagged and c.state == koji.BUILD_STATES['COMPLETE']

+         if (not c.tagged or (not c.tagged_in_final and not c.build_time_only)) and

+         c.state == koji.BUILD_STATES['COMPLETE']

      ]

  

      further_work = []
@@ -90,8 +91,9 @@ 

              if c.state == koji.BUILD_STATES['BUILDING'] or not c.state

          ]

          if unbuilt_components:

-             log.info("All components in batch tagged, regenerating repo for tag %s", tag)

-             task_id = builder.koji_session.newRepo(tag)

+             repo_tag = builder.module_build_tag['name']

+             log.info("All components in batch tagged, regenerating repo for tag %s", repo_tag)

+             task_id = builder.koji_session.newRepo(repo_tag)

              module_build.new_repo_task_id = task_id

          else:

              # In case this is the last batch, we do not need to regenerate the

@@ -79,6 +79,7 @@ 

          builder = mock.MagicMock()

          builder.koji_session = koji_session

          builder.buildroot_ready.return_value = False

+         builder.module_build_tag = {"name": "module-testmodule-build"}

          create_builder.return_value = builder

  

          module_build = module_build_service.models.ModuleBuild.query.filter_by(id=2).one()
@@ -88,6 +89,7 @@ 

          for c in module_build.up_to_current_batch():

              c.state = koji.BUILD_STATES["COMPLETE"]

              c.tagged = True

+             c.tagged_in_final = True

  

          module_build.batch = 2

          for c in module_build.current_batch():
@@ -99,6 +101,11 @@ 

              'id', 'module-testmodule-build', "perl-Tangerine")

          module_build_service.scheduler.handlers.tags.tagged(

              config=conf, session=db.session, msg=msg)

+         # Tag the first component to the final tag.

+         msg = module_build_service.messaging.KojiTagChange(

+             'id', 'module-testmodule', "perl-Tangerine")

+         module_build_service.scheduler.handlers.tags.tagged(

+             config=conf, session=db.session, msg=msg)

  

          # newRepo should not be called, because there are still components

          # to tag.
@@ -110,6 +117,16 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              config=conf, session=db.session, msg=msg)

  

+         # newRepo should not be called, because the component has not been

+         # tagged to final tag so far.

+         self.assertTrue(not koji_session.newRepo.called)

+ 

+         # Tag the first component to the final tag.

+         msg = module_build_service.messaging.KojiTagChange(

+             'id', 'module-testmodule', "perl-List-Compare")

+         module_build_service.scheduler.handlers.tags.tagged(

+             config=conf, session=db.session, msg=msg)

+ 

          # newRepo should be called now - all components have been tagged.

          koji_session.newRepo.assert_called_once_with("module-testmodule-build")

  
@@ -138,6 +155,7 @@ 

          builder = mock.MagicMock()

          builder.koji_session = koji_session

          builder.buildroot_ready.return_value = False

+         builder.module_build_tag = {"name": "module-testmodule-build"}

          create_builder.return_value = builder

  

          module_build = module_build_service.models.ModuleBuild.query.filter_by(id=2).one()
@@ -152,6 +170,11 @@ 

              'id', 'module-testmodule-build', "perl-Tangerine")

          module_build_service.scheduler.handlers.tags.tagged(

              config=conf, session=db.session, msg=msg)

+         # Tag the perl-List-Compare component to final tag.

+         msg = module_build_service.messaging.KojiTagChange(

+             'id', 'module-testmodule', "perl-Tangerine")

+         module_build_service.scheduler.handlers.tags.tagged(

+             config=conf, session=db.session, msg=msg)

  

          # newRepo should not be called, because perl-List-Compare has not been

          # built yet.
@@ -174,6 +197,7 @@ 

          builder = mock.MagicMock()

          builder.koji_session = koji_session

          builder.buildroot_ready.return_value = False

+         builder.module_build_tag = {"name": "module-testmodule-build"}

          create_builder.return_value = builder

  

          module_build = module_build_service.models.ModuleBuild.query.filter_by(id=2).one()
@@ -183,6 +207,7 @@ 

          for c in module_build.up_to_current_batch():

              c.state = koji.BUILD_STATES["COMPLETE"]

              c.tagged = True

+             c.tagged_in_final = True

  

          module_build.batch = 2

          component = module_build_service.models.ComponentBuild.query\
@@ -198,6 +223,11 @@ 

              'id', 'module-testmodule-build', "perl-List-Compare")

          module_build_service.scheduler.handlers.tags.tagged(

              config=conf, session=db.session, msg=msg)

+         # Tag the perl-List-Compare component to final tag.

+         msg = module_build_service.messaging.KojiTagChange(

+             'id', 'module-testmodule', "perl-List-Compare")

+         module_build_service.scheduler.handlers.tags.tagged(

+             config=conf, session=db.session, msg=msg)

  

          # newRepo should be called now - all successfully built

          # components have been tagged.
@@ -231,6 +261,7 @@ 

          builder = mock.MagicMock()

          builder.koji_session = koji_session

          builder.buildroot_ready.return_value = False

+         builder.module_build_tag = {"name": "module-testmodule-build"}

          create_builder.return_value = builder

  

          module_build = module_build_service.models.ModuleBuild.query.filter_by(id=2).one()
@@ -244,6 +275,11 @@ 

              'id', 'module-testmodule-build', "perl-Tangerine")

          module_build_service.scheduler.handlers.tags.tagged(

              config=conf, session=db.session, msg=msg)

+         # Tag the first component to the final tag.

+         msg = module_build_service.messaging.KojiTagChange(

+             'id', 'module-testmodule', "perl-Tangerine")

+         module_build_service.scheduler.handlers.tags.tagged(

+             config=conf, session=db.session, msg=msg)

  

          # newRepo should not be called, because there are still components

          # to tag.
@@ -254,6 +290,11 @@ 

              'id', 'module-testmodule-build', "perl-List-Compare")

          module_build_service.scheduler.handlers.tags.tagged(

              config=conf, session=db.session, msg=msg)

+         # Tag the second component to final tag.

+         msg = module_build_service.messaging.KojiTagChange(

+             'id', 'module-testmodule', "perl-List-Compare")

+         module_build_service.scheduler.handlers.tags.tagged(

+             config=conf, session=db.session, msg=msg)

  

          # newRepo should not be called, because there are still components

          # to tag.
@@ -264,6 +305,11 @@ 

              'id', 'module-testmodule-build', "module-build-macros")

          module_build_service.scheduler.handlers.tags.tagged(

              config=conf, session=db.session, msg=msg)

+         # Tag the component from first batch to final tag.

+         msg = module_build_service.messaging.KojiTagChange(

+             'id', 'module-testmodule', "module-build-macros")

+         module_build_service.scheduler.handlers.tags.tagged(

+             config=conf, session=db.session, msg=msg)

  

          # newRepo should be called now - all components have been tagged.

          koji_session.newRepo.assert_called_once_with("module-testmodule-build")
@@ -275,3 +321,75 @@ 

          # newRepo task_id should be stored in database, so we can check its

          # status later in poller.

          self.assertEqual(module_build.new_repo_task_id, 123456)

+ 

+ 

+     @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

+            return_value={'build': [], 'srpm-build': []})

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

+     @patch("module_build_service.builder.GenericBuilder.create_from_module")

+     def test_newrepo_build_time_only(

+             self, create_builder, koji_get_session, dbg):

+         """

+         Test the component.build_time_only is respected in tag handler.

+         """

+         koji_session = mock.MagicMock()

+         koji_session.getTag = lambda tag_name: {'name': tag_name}

+         koji_session.getTaskInfo.return_value = {'state': koji.TASK_STATES['CLOSED']}

+         koji_session.newRepo.return_value = 123456

+         koji_get_session.return_value = koji_session

+ 

+         builder = mock.MagicMock()

+         builder.koji_session = koji_session

+         builder.buildroot_ready.return_value = False

+         builder.module_build_tag = {"name": "module-testmodule-build"}

+         create_builder.return_value = builder

+ 

+         module_build = module_build_service.models.ModuleBuild.query.filter_by(id=2).one()

+ 

+         # Set previous components as COMPLETE and tagged.

+         module_build.batch = 1

+         for c in module_build.up_to_current_batch():

+             c.state = koji.BUILD_STATES["COMPLETE"]

+             c.tagged = True

+             c.tagged_in_final = True

+ 

+         module_build.batch = 2

+         component = module_build_service.models.ComponentBuild.query\

+             .filter_by(package='perl-Tangerine', module_id=module_build.id).one()

+         component.state = koji.BUILD_STATES["COMPLETE"]

+         component.build_time_only = True

+         component.tagged = False

+         component.tagged_in_final = False

+         component = module_build_service.models.ComponentBuild.query\

+             .filter_by(package='perl-List-Compare', module_id=module_build.id).one()

+         component.state = koji.BUILD_STATES["COMPLETE"]

+         db.session.commit()

+ 

+         # Tag the perl-Tangerine component to the buildroot.

+         msg = module_build_service.messaging.KojiTagChange(

+             'id', 'module-testmodule-build', "perl-Tangerine")

+         module_build_service.scheduler.handlers.tags.tagged(

+             config=conf, session=db.session, msg=msg)

+         self.assertTrue(not koji_session.newRepo.called)

+         # Tag the perl-List-Compare component to the buildroot.

+         msg = module_build_service.messaging.KojiTagChange(

+             'id', 'module-testmodule-build', "perl-List-Compare")

+         module_build_service.scheduler.handlers.tags.tagged(

+             config=conf, session=db.session, msg=msg)

+         # Tag the perl-List-Compare component to final tag.

+         msg = module_build_service.messaging.KojiTagChange(

+             'id', 'module-testmodule', "perl-List-Compare")

+         module_build_service.scheduler.handlers.tags.tagged(

+             config=conf, session=db.session, msg=msg)

+ 

+         # newRepo should be called now - all successfully built

+         # components have been tagged.

+         koji_session.newRepo.assert_called_once_with("module-testmodule-build")

+ 

+         # Refresh our module_build object.

+         db.session.expunge(module_build)

+         module_build = module_build_service.models.ModuleBuild.query.filter_by(id=2).one()

+ 

+         # newRepo task_id should be stored in database, so we can check its

+         # status later in poller.

+         self.assertEqual(module_build.new_repo_task_id, 123456)

We used to wait only for tagging to -build Koji tag. This is not right, because when module is built, we create the list of components in Content Generator Koji code based on the final Koji tag. This final Koji tag then might not contain all the components, because we are not waiting for Koji to actually tag them there.

This PR adds new ArtifactBuild.tagged_in_final variable which works the same way as ArtifactBuild.tagged variable, but it tracks whether the component is tagged in the final tag.

The batch/module is marked as done only when component is tagged to both -build and final tags.

Note that MBS is frozen in prod, so to deploy that, we need to request freeze break for mbs... :(

rebased onto eb335522b12fca11e2917c778b5b3d08ecc71058

6 years ago

rebased onto 9b450cf6a14ef2484e30dd9c10bf95094c26e499

6 years ago

...here is default=False. Is there a reason for it?

I've actually fixed it few seconds before you submitted this comment :)

Can you add a message here explaining the migration please?

I'd prefer a different way to this because looking at the database table, I'd think this is tagged in the final tag but it's not. Just an idea, could we add a new column that denotes if it's a build artifact only and set that here instead?

Why are you also not setting component.tagged here? If it's tagged_in_final, shouldn't it also have component.tagged = True as well?

But what if it doesn't? If the tag doesn't end with -build then this code can't locally know that it is really tagged into -build. I vote keep it how it is here since it accurately reflects the event being handled.

tagged means it is tagged in the -build tag. I know it's kind of wrong name, but we didn't know we will have to track also tagging in final tag when we added tagged column...

@jkaluza Thanks for the explanation. Can you add a comment on the column in models.py explaining that?

rebased onto 999baa2

6 years ago

Pull-Request has been merged by ralph

6 years ago