#1075 FACTORY-3136: set module build state to 'failed' imediately
Merged 8 months ago by jkaluza. Opened 8 months ago by vmaljulin.
vmaljulin/fm-orchestrator FACTORY-3136  into  master

@@ -78,12 +78,14 @@ 

          c.state = koji.BUILD_STATES['FAILED']

          c.state_reason = "Failed to build artifact %s: %s" % (c.package, str(e))

          log.exception(e)

+         c.module_build.transition(conf, models.BUILD_STATES['failed'])

          return

  

      if not c.task_id and c.state == koji.BUILD_STATES['BUILDING']:

          c.state = koji.BUILD_STATES['FAILED']

          c.state_reason = ("Failed to build artifact %s: "

                            "Builder did not return task ID" % (c.package))

+         c.module_build.transition(conf, models.BUILD_STATES['failed'])

          return

  

  

@@ -861,6 +861,18 @@ 

          # Make sure that both components in the batch were submitted

          assert len(mock_sbc.mock_calls) == 2

  

+     def test_start_build_component_failed_state(self, default_buildroot_groups):

+         """

+         Tests whether exception occured while building sets the state to failed

+         """

+         builder = mock.MagicMock()

+         builder.build.side_effect = Exception('Something have gone terribly wrong')

+         component = mock.MagicMock()

+ 

+         module_build_service.utils.batches.start_build_component(builder, component)

+ 

+         assert component.state == koji.BUILD_STATES['FAILED']

+ 

      @patch('module_build_service.utils.batches.start_build_component')

      @patch('module_build_service.config.Config.rebuild_strategy',

             new_callable=mock.PropertyMock, return_value='only-changed')

Signed-off-by: Valerij Maljulin vmaljuli@redhat.com
I hope that I understood this issue correctly. Do we have/need a test for this?

@vmaljulin This seems to fix #1009.

Please update the commit message to reference this issue (by adding Fixes #1009) instead of the Red Hat internal Jira, so that everyone has access to the context of this change. Thanks!

Can you add test for this into ./tests/test_utils/test_utils.py in class TestBatches? It shouldn't be hard, something like this:

  • create builder = MagicMock() and set the build method to raise an exception like builder.build.side_effect = Exception("Expected build exception").
  • create fake Component the same way.
  • Call start_build_component in the with pytest.raises(...) block (this is already used elsewhere in test_utils.py).

Should builder.module.transition also be called in this if-block?

@vmaljulin, did you have a chance to address the comments here?

@lucarval Yes I'm working on this and will update this pull-request soon

rebased onto 8adce75

8 months ago

Pull-Request has been merged by jkaluza

8 months ago