Revert "release_stages: Split Paygen -> Signing, Paygen stages."

This reverts commit ad7e9100db55397979f0fd18b842f3dc1c0fb2f1.

Change-Id: Ic2ba36265735b5289fe483956a8f501f763b4963
Reviewed-on: https://chromium-review.googlesource.com/337354
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Tested-by: Ketaki Deshpande <ketakid@chromium.org>
diff --git a/cbuildbot/builders/simple_builders.py b/cbuildbot/builders/simple_builders.py
index f1bb747..ecbfc35 100644
--- a/cbuildbot/builders/simple_builders.py
+++ b/cbuildbot/builders/simple_builders.py
@@ -192,7 +192,6 @@
 
     stage_list += [
         [release_stages.SignerTestStage, board, archive_stage],
-        [release_stages.SigningStage, board, archive_stage],
         [release_stages.PaygenStage, board, archive_stage],
         [test_stages.ImageTestStage, board],
         [test_stages.UnitTestStage, board],
diff --git a/cbuildbot/stages/release_stages.py b/cbuildbot/stages/release_stages.py
index f91cac1..c382724 100644
--- a/cbuildbot/stages/release_stages.py
+++ b/cbuildbot/stages/release_stages.py
@@ -70,8 +70,8 @@
   """Paygen can't run with a release.conf config for the board."""
 
 
-class SigningStage(artifact_stages.ArchivingStage):
-  """Stage that waits for image signing.
+class PaygenStage(artifact_stages.ArchivingStage):
+  """Stage that generates release payloads.
 
   If this stage is created with a 'channels' argument, it can run
   independently. Otherwise, it's dependent on values queued up by
@@ -86,32 +86,49 @@
   # Timeout for the signing process. 2 hours in seconds.
   SIGNING_TIMEOUT = 2 * 60 * 60
 
-  def __init__(self, builder_run, board, archive_stage, **kwargs):
+  FINISHED = 'finished'
+
+  def __init__(self, builder_run, board, archive_stage, channels=None,
+               **kwargs):
     """Init that accepts the channels argument, if present.
 
     Args:
       builder_run: See builder_run on ArchivingStage.
       board: See board on ArchivingStage.
       archive_stage: See archive_stage on ArchivingStage.
+      channels: Explicit list of channels to generate payloads for.
+                If empty, will instead wait on values from push_image.
+                Channels is normally None in release builds, and normally set
+                for trybot 'payloads' builds.
     """
-    super(SigningStage, self).__init__(builder_run, board, archive_stage,
-                                       **kwargs)
-
-    # Used to remember partial results between retries.
+    super(PaygenStage, self).__init__(builder_run, board, archive_stage,
+                                      **kwargs)
     self.signing_results = {}
+    self.channels = channels
 
   def _HandleStageException(self, exc_info):
     """Override and don't set status to FAIL but FORGIVEN instead."""
-    exc_type, _exc_value, _exc_tb = exc_info
+    exc_type, exc_value, _exc_tb = exc_info
 
-    # Notify stages blocked on us if we error out.
-    self.board_runattrs.SetParallel('signed_images_ready', None)
+    # If Paygen fails to find anything needed in release.conf, treat it
+    # as a warning, not a failure. This is common during new board bring up.
+    if issubclass(exc_type, PaygenNoPaygenConfigForBoard):
+      return self._HandleExceptionAsWarning(exc_info)
 
     # Warn so people look at ArchiveStage for the real error.
     if issubclass(exc_type, MissingInstructionException):
       return self._HandleExceptionAsWarning(exc_info)
 
-    return super(SigningStage, self)._HandleStageException(exc_info)
+    # If the exception is a TestLabFailure that means we couldn't schedule the
+    # test. We don't fail the build for that. We do the CompoundFailure dance,
+    # because that's how we'll get failures from background processes returned
+    # to us.
+    if (issubclass(exc_type, failures_lib.TestLabFailure) or
+        (issubclass(exc_type, failures_lib.CompoundFailure) and
+         exc_value.MatchesFailureType(failures_lib.TestLabFailure))):
+      return self._HandleExceptionAsWarning(exc_info)
+
+    return super(PaygenStage, self)._HandleStageException(exc_info)
 
   def _JsonFromUrl(self, gs_ctx, url):
     """Fetch a GS Url, and parse it as Json.
@@ -151,14 +168,15 @@
     return (signer_json or {}).get('status', {}).get('status', '')
 
   def _CheckForResults(self, gs_ctx, instruction_urls_per_channel,
-                       channel_notifier=None):
+                       channel_notifier):
     """timeout_util.WaitForSuccess func to check a list of signer results.
 
     Args:
       gs_ctx: Google Storage Context.
       instruction_urls_per_channel: Urls of the signer result files
                                     we're expecting.
-      channel_notifier: Method to call when a channel is ready or None.
+      channel_notifier: BackgroundTaskRunner into which we push channels for
+                        processing.
 
     Returns:
       Number of results not yet collected.
@@ -209,8 +227,8 @@
         if self._SigningStatusFromJson(signer_result) != 'passed':
           channel_success = False
 
-      # If we successfully completed the channel, inform someone.
-      if channel_success and channel_notifier:
+      # If we successfully completed the channel, inform paygen.
+      if channel_success:
         channel_notifier(channel)
 
     return results_completed
@@ -238,12 +256,13 @@
 
   def _WaitForSigningResults(self,
                              instruction_urls_per_channel,
-                             channel_notifier=None):
+                             channel_notifier):
     """Do the work of waiting for signer results and logging them.
 
     Args:
       instruction_urls_per_channel: push_image data (see _WaitForPushImage).
-      channel_notifier: Method to call with channel name when ready or None.
+      channel_notifier: BackgroundTaskRunner into which we push channels for
+                        processing.
 
     Raises:
       ValueError: If the signer result isn't valid json.
@@ -290,78 +309,6 @@
     version = self._run.attrs.release_tag
 
     assert version, "We can't generate payloads without a release_tag."
-    logging.info("Waiting for image uploads for: %s, %s", board, version)
-
-    instruction_urls_per_channel = self._WaitForPushImage()
-
-    logging.info("Waiting for image signing for: %s, %s", board, version)
-    logging.info("GS errors are a normal part of the polling for results.")
-    self._WaitForSigningResults(instruction_urls_per_channel)
-
-    # Notify stages blocked on us that images are for the given channel list.
-    channels = instruction_urls_per_channel.keys()
-    self.board_runattrs.SetParallel('signed_images_ready', channels)
-
-
-class PaygenStage(artifact_stages.ArchivingStage):
-  """Stage that generates release payloads.
-
-  If this stage is created with a 'channels' argument, it can run
-  independently. Otherwise, it's dependent on values queued up by
-  the ArchiveStage (push_image).
-  """
-  option_name = 'paygen'
-  config_name = 'paygen'
-
-  def __init__(self, builder_run, board, archive_stage, channels=None,
-               **kwargs):
-    """Init that accepts the channels argument, if present.
-
-    Args:
-      builder_run: See builder_run on ArchivingStage.
-      board: See board on ArchivingStage.
-      archive_stage: See archive_stage on ArchivingStage.
-      channels: Explicit list of channels to generate payloads for.
-                If empty, will instead wait on values from push_image.
-                Channels is normally None in release builds, and normally set
-                for trybot 'payloads' builds.
-    """
-    super(PaygenStage, self).__init__(builder_run, board, archive_stage,
-                                      **kwargs)
-    self.channels = channels
-
-  def _HandleStageException(self, exc_info):
-    """Override and don't set status to FAIL but FORGIVEN instead."""
-    exc_type, exc_value, _exc_tb = exc_info
-
-    # If Paygen fails to find anything needed in release.conf, treat it
-    # as a warning. This is common during new board bring up.
-    if issubclass(exc_type, PaygenNoPaygenConfigForBoard):
-      return self._HandleExceptionAsWarning(exc_info)
-
-    # If the SigningStage failed, we warn that we didn't run, but don't fail
-    # outright. Let SigningStage decide if this should kill the build.
-    if issubclass(exc_type, SignerFailure):
-      return self._HandleExceptionAsWarning(exc_info)
-
-    # If the exception is a TestLabFailure that means we couldn't schedule the
-    # test. We don't fail the build for that. We do the CompoundFailure dance,
-    # because that's how we'll get failures from background processes returned
-    # to us.
-    if (issubclass(exc_type, failures_lib.TestLabFailure) or
-        (issubclass(exc_type, failures_lib.CompoundFailure) and
-         exc_value.MatchesFailureType(failures_lib.TestLabFailure))):
-      return self._HandleExceptionAsWarning(exc_info)
-
-    return super(PaygenStage, self)._HandleStageException(exc_info)
-
-  def PerformStage(self):
-    """Do the work of generating our release payloads."""
-    # Convert to release tools naming for boards.
-    board = self._current_board.replace('_', '-')
-    version = self._run.attrs.release_tag
-
-    assert version, "We can't generate payloads without a release_tag."
     logging.info("Generating payloads for: %s, %s", board, version)
 
     # Test to see if the current board has a Paygen configuration. We do
@@ -374,24 +321,22 @@
           'No release.conf entry was found for board %s. Get a TPM to fix.' %
           board)
 
-
-    # If we did not get an explicit channel list, we wait on the signing
-    # stage to give us a list, and so signal that signing is done.
-    if self.channels is None:
-      # Wait for channels from signing stage.
-      self.channels = self.board_runattrs.GetParallel(
-          'signed_images_ready', timeout=None)
-      if self.channels is None:
-        raise SignerFailure('SigningStage failed, no payloads generated.')
-
     with parallel.BackgroundTaskRunner(self._RunPaygenInProcess) as per_channel:
-      logging.info("Using channels: %s", self.channels)
-      # If we have an explicit list of channels, use it.
-      for channel in self.channels:
+      def channel_notifier(channel):
         per_channel.put((channel, board, version, self._run.debug,
                          self._run.config.paygen_skip_testing,
                          self._run.config.paygen_skip_delta_payloads))
 
+      if self.channels:
+        logging.info("Using explicit channels: %s", self.channels)
+        # If we have an explicit list of channels, use it.
+        for channel in self.channels:
+          channel_notifier(channel)
+      else:
+        instruction_urls_per_channel = self._WaitForPushImage()
+        self._WaitForSigningResults(instruction_urls_per_channel,
+                                    channel_notifier)
+
   def _RunPaygenInProcess(self, channel, board, version, debug,
                           disable_tests, skip_delta_payloads):
     """Helper for PaygenStage that invokes payload generation.
diff --git a/cbuildbot/stages/release_stages_unittest.py b/cbuildbot/stages/release_stages_unittest.py
index 570b136..89dbe09 100644
--- a/cbuildbot/stages/release_stages_unittest.py
+++ b/cbuildbot/stages/release_stages_unittest.py
@@ -25,10 +25,11 @@
 # pylint: disable=protected-access
 
 
-class SigningStageTest(generic_stages_unittest.AbstractStageTestCase,
-                       cbuildbot_unittest.SimpleBuilderTestCase):
-  """Test the SigningStage."""
+class PaygenStageTest(generic_stages_unittest.AbstractStageTestCase,
+                      cbuildbot_unittest.SimpleBuilderTestCase):
+  """Test the PaygenStageStage."""
 
+  BOT_ID = 'x86-mario-release'
   RELEASE_TAG = '0.0.1'
 
   SIGNER_RESULT = """
@@ -46,8 +47,8 @@
 
   def ConstructStage(self):
     archive_stage = artifact_stages.ArchiveStage(self._run, self._current_board)
-    return release_stages.SigningStage(self._run, self._current_board,
-                                       archive_stage)
+    return release_stages.PaygenStage(self._run, self._current_board,
+                                      archive_stage)
 
   def testWaitForPushImageSuccess(self):
     """Test waiting for input from PushImage."""
@@ -85,20 +86,6 @@
       for result in results:
         mock_gs_ctx.Cat.assert_any_call(result)
 
-  def testWaitForSigningResultsSuccessNoNotifier(self):
-    """Test that _WaitForSigningResults works when signing works."""
-    results = ['chan1_uri1.json', 'chan1_uri2.json', 'chan2_uri1.json']
-
-    with patch(release_stages.gs, 'GSContext') as mock_gs_ctx_init:
-      mock_gs_ctx = mock_gs_ctx_init.return_value
-      mock_gs_ctx.Cat.return_value = self.SIGNER_RESULT
-
-      stage = self.ConstructStage()
-      stage._WaitForSigningResults(self.INSNS_URLS_PER_CHANNEL, None)
-
-      for result in results:
-        mock_gs_ctx.Cat.assert_any_call(result)
-
   def testWaitForSigningResultsSuccessNothingSigned(self):
     """Test _WaitForSigningResults when there are no signed images."""
     with patch(release_stages.gs, 'GSContext') as mock_gs_ctx_init:
@@ -278,133 +265,145 @@
       })
       self.assertEqual(notifier.mock_calls, [])
 
-  def testPerformStageSuccess(self):
-    """Test that SigningStage works when signing works."""
-    stage = self.ConstructStage()
-
-    self.PatchObject(stage, '_WaitForPushImage',
-                     return_value=self.INSNS_URLS_PER_CHANNEL)
-    self.PatchObject(stage, '_WaitForSigningResults')
-
-    stage.PerformStage()
-
-    # Verify that we send the right notifications.
-    result = stage.board_runattrs.GetParallel('signed_images_ready', timeout=0)
-    self.assertEqual(result, ['chan1', 'chan2'])
-
-
-class PaygenStageTest(generic_stages_unittest.AbstractStageTestCase,
-                      cbuildbot_unittest.SimpleBuilderTestCase):
-  """Test the PaygenStageStage."""
-
-  # We use a variant board to make sure the '_' is translated to '-'.
-  BOT_ID = 'x86-alex_he-release'
-  RELEASE_TAG = '0.0.1'
-
-  def setUp(self):
-    self._Prepare()
-
-    # This method fetches a file from GS, mock it out.
-    self.validateMock = self.PatchObject(
-        paygen_build_lib, 'ValidateBoardConfig')
-
-  def ConstructStage(self):
-    return release_stages.PaygenStage(self._run, self._current_board, None)
+  def generateNotifyCalls(self, channels):
+    def side_effect(_, notifier):
+      for channel in channels:
+        notifier(channel)
+    return side_effect
 
   def testPerformStageSuccess(self):
     """Test that PaygenStage works when signing works."""
+
     with patch(release_stages.parallel, 'BackgroundTaskRunner') as background:
       queue = background().__enter__()
 
-      stage = self.ConstructStage()
-      stage.board_runattrs.SetParallel('signed_images_ready',
-                                       ['stable', 'beta'])
+      # This patch is only required for external builds with no config data.
+      with patch(paygen_build_lib, 'ValidateBoardConfig'):
 
-      stage.PerformStage()
+        stage = self.ConstructStage()
 
-      # Verify that we queue up work
-      self.assertEqual(
-          queue.put.call_args_list,
-          [mock.call(('stable', 'x86-alex-he', '0.0.1', False, True, False)),
-           mock.call(('beta', 'x86-alex-he', '0.0.1', False, True, False))])
+        with patch(stage, '_WaitForPushImage') as wait_push:
+          with patch(stage, '_WaitForSigningResults') as wait_signing:
+            wait_push.return_value = self.INSNS_URLS_PER_CHANNEL
+            wait_signing.side_effect = self.generateNotifyCalls(('stable',
+                                                                 'beta'))
+            stage.PerformStage()
 
-  def testPerformStageNoChannels(self):
+        # Verify that we queue up work
+        self.assertEqual(
+            queue.put.call_args_list,
+            [mock.call(('stable', 'x86-mario', '0.0.1', False, False, False)),
+             mock.call(('beta', 'x86-mario', '0.0.1', False, False, False))])
+
+  def testPerformStageSuccessVarientBoard(self):
+    """Test that SignerResultsStage works with varient boards.
+
+    Varient boards need some name conversion. Make sure that's okay.
+    """
+    self._current_board = 'x86-alex_he'
+
+    with patch(release_stages.parallel, 'BackgroundTaskRunner') as background:
+      queue = background().__enter__()
+
+      # This patch is only required for external builds with no config data.
+      with patch(paygen_build_lib, 'ValidateBoardConfig'):
+        stage = self.ConstructStage()
+
+        with patch(stage, '_WaitForPushImage') as wait_push:
+          with patch(stage, '_WaitForSigningResults') as wait_signing:
+            wait_push.return_value = self.INSNS_URLS_PER_CHANNEL
+            wait_signing.side_effect = self.generateNotifyCalls(('stable',
+                                                                 'beta'))
+            stage.PerformStage()
+
+        # Verify that we queue up work
+        self.assertEqual(
+            queue.put.call_args_list,
+            [mock.call(('stable', 'x86-alex-he', '0.0.1', False, False, False)),
+             mock.call(('beta', 'x86-alex-he', '0.0.1', False, False, False))])
+
+  def testPerformStageSigningFailed(self):
     """Test that PaygenStage works when signing works."""
     with patch(release_stages.parallel, 'BackgroundTaskRunner') as background:
       queue = background().__enter__()
 
-      stage = self.ConstructStage()
-      stage.board_runattrs.SetParallel('signed_images_ready', [])
+      # This patch is only required for external builds with no config data.
+      with patch(paygen_build_lib, 'ValidateBoardConfig'):
+        stage = self.ConstructStage()
 
-      stage.PerformStage()
+        with patch(stage, '_WaitForPushImage') as wait_push:
+          with patch(stage, '_WaitForSigningResults') as wait_signing:
+            wait_push.return_value = self.INSNS_URLS_PER_CHANNEL
+            wait_signing.side_effect = release_stages.SignerFailure
 
-      # Verify that we queue up work
-      self.assertEqual(queue.put.call_args_list, [])
+            self.assertRaises(release_stages.SignerFailure,
+                              stage.PerformStage)
 
-  def testPerformStageSigningFailure(self):
-    """Test that PaygenStage works when signing works."""
-    with patch(release_stages.parallel, 'BackgroundTaskRunner') as background:
-      queue = background().__enter__()
-
-      stage = self.ConstructStage()
-      stage.board_runattrs.SetParallel('signed_images_ready', None)
-
-      with self.assertRaises(release_stages.SignerFailure):
-        stage.PerformStage()
-
-      # Verify that we queue up work
-      self.assertEqual(queue.put.call_args_list, [])
+        # Ensure no work was queued up.
+        self.assertFalse(queue.put.called)
 
   def testPerformStageBackgroundFail(self):
     """Test that exception from background processes are properly handled."""
     with patch(paygen_build_lib, 'CreatePayloads') as create_payloads:
       create_payloads.side_effect = failures_lib.TestLabFailure
 
-      stage = release_stages.PaygenStage(
-          self._run, self._current_board,
-          archive_stage=None, channels=['foo', 'bar'])
+      # This patch is only required for external builds with no config data.
+      with patch(paygen_build_lib, 'ValidateBoardConfig'):
+        stage = release_stages.PaygenStage(
+            self._run, self._current_board,
+            archive_stage=None, channels=['foo', 'bar'])
 
-      with patch(stage, '_HandleExceptionAsWarning') as warning_handler:
-        warning_handler.return_value = (results_lib.Results.FORGIVEN,
-                                        'description',
-                                        0)
+        with patch(stage, '_HandleExceptionAsWarning') as warning_handler:
+          warning_handler.return_value = (results_lib.Results.FORGIVEN,
+                                          'description',
+                                          0)
 
-        stage.Run()
+          stage.Run()
 
-        # This proves the exception was turned into a warning.
-        self.assertTrue(warning_handler.called)
+          # This proves the exception was turned into a warning.
+          self.assertTrue(warning_handler.called)
 
   def testPerformStageTrybot(self):
     """Test the PerformStage alternate behavior for trybot runs."""
     with patch(release_stages.parallel, 'BackgroundTaskRunner') as background:
       queue = background().__enter__()
 
-      # The stage is constructed differently for trybots, so don't use
-      # ConstructStage.
-      stage = release_stages.PaygenStage(
-          self._run, self._current_board, archive_stage=None,
-          channels=['foo', 'bar'])
-      stage.PerformStage()
+      # This patch is only required for external builds with no config data.
+      with patch(paygen_build_lib, 'ValidateBoardConfig'):
+        # The stage is constructed differently for trybots, so don't use
+        # ConstructStage.
+        stage = release_stages.PaygenStage(
+            self._run, self._current_board, archive_stage=None,
+            channels=['foo', 'bar'])
+        with patch(stage, '_WaitForPushImage') as wait_push:
+          with patch(stage, '_WaitForSigningResults') as wait_signing:
+            stage.PerformStage()
 
-      # Notice that we didn't put anything in _wait_for_channel_signing, but
-      # still got results right away.
-      self.assertEqual(
-          queue.put.call_args_list,
-          [mock.call(('foo', 'x86-alex-he', '0.0.1', False, True, False)),
-           mock.call(('bar', 'x86-alex-he', '0.0.1', False, True, False))])
+          # Make sure we don't wait on push_image or signing in this case.
+          self.assertEqual(wait_push.mock_calls, [])
+          self.assertEqual(wait_signing.mock_calls, [])
+
+        # Notice that we didn't put anything in _wait_for_channel_signing, but
+        # still got results right away.
+        self.assertEqual(
+            queue.put.call_args_list,
+            [mock.call(('foo', 'x86-mario', '0.0.1', False, False, False)),
+             mock.call(('bar', 'x86-mario', '0.0.1', False, False, False))])
 
   def testPerformStageUnknownBoard(self):
     """Test that PaygenStage exits when an unknown board is specified."""
     self._current_board = 'unknown-board-name'
 
-    # Setup a board validation failure.
     badBoardException = paygen_build_lib.BoardNotConfigured(self._current_board)
-    self.validateMock.side_effect = badBoardException
 
-    stage = self.ConstructStage()
+    # This patch is only required for external builds with no config data.
+    with patch(paygen_build_lib, 'ValidateBoardConfig') as validate_boards:
+      validate_boards.side_effect = badBoardException
 
-    with self.assertRaises(release_stages.PaygenNoPaygenConfigForBoard):
-      stage.PerformStage()
+      stage = self.ConstructStage()
+
+      self.assertRaises(release_stages.PaygenNoPaygenConfigForBoard,
+                        stage.PerformStage)
 
   def testRunPaygenInProcess(self):
     """Test that _RunPaygenInProcess works in the simple case."""