Make all changes reportable

We want to report on "changes" triggered by a timer.  Since those
aren't really changes, they are represented by a class called
NullChange which carries as much of the interface for a Change
object as possible (not much) so that they can be enqueued in
pipelines.

If a NullChange is reportable, then pretty much any kind of
change should be considered reportable.  So remove the flag that
indicates that NullChanges and Refs are not reportable.

Only attempt to format a change report if there is an action
defined for that pipeline (in case the default formatting process
attempts to access a change attribute that is inappropriate).
Remove checks that try to avoid formatting or sending a report
based on attributes of the change (which are no longer relevant).

Add a test for using a timer trigger with an smtp reporter.

Add validation of the attributes that an smtp reporter can use in
the layout file.

Allow the operator to configure a Subject for smtp reports.

Change-Id: Icd067a7600c2922a318b9ade11b3946df4e53065

doc/source/reporters.rst

@@ -42,8 +42,8 @@ SMTP Configuration
 zuul.conf contains the SMTP server and default to/from as describe
 in :ref:`zuulconf`.
 
-Each pipeline can overwrite the to or from address by providing
-alternatives as arguments to the reporter. For example, ::
+Each pipeline can overwrite the subject or the to or from address by
+providing alternatives as arguments to the reporter. For example, ::
 
   pipelines:
     - name: post-merge
@@ -57,3 +57,4 @@ alternatives as arguments to the reporter. For example, ::
         smtp:
           to: you@example.com
           from: alternative@example.com
+          subject: Change {change} failed

tests/fixtures/layout-timer-smtp.yaml

@@ -0,0 +1,23 @@
+pipelines:
+  - name: periodic
+    manager: IndependentPipelineManager
+    trigger:
+      timer:
+        - time: '* * * * * */10'
+    success:
+      smtp:
+        to: alternative_me@example.com
+        from: zuul_from@example.com
+        subject: 'Periodic check for {change.project} succeeded'
+
+jobs:
+  - name: project-bitrot-stable-old
+    success-pattern: http://logs.example.com/{job.name}/{build.number}
+  - name: project-bitrot-stable-older
+    success-pattern: http://logs.example.com/{job.name}/{build.number}
+
+projects:
+  - name: org/project
+    periodic:
+      - project-bitrot-stable-old
+      - project-bitrot-stable-older

tests/test_scheduler.py

@@ -2992,6 +2992,45 @@ class TestScheduler(testtools.TestCase):
         self.assertEqual(A.messages[0],
                          FakeSMTP.messages[1]['body'])
 
+    def test_timer_smtp(self):
+        "Test that a periodic job is triggered"
+        self.config.set('zuul', 'layout_config',
+                        'tests/fixtures/layout-timer-smtp.yaml')
+        self.sched.reconfigure(self.config)
+        self.registerJobs()
+
+        start = time.time()
+        failed = True
+        while ((time.time() - start) < 30):
+            if len(self.history) == 2:
+                failed = False
+                break
+            else:
+                time.sleep(1)
+
+        if failed:
+            raise Exception("Expected jobs never ran")
+
+        self.waitUntilSettled()
+
+        self.assertEqual(self.getJobFromHistory(
+            'project-bitrot-stable-old').result, 'SUCCESS')
+        self.assertEqual(self.getJobFromHistory(
+            'project-bitrot-stable-older').result, 'SUCCESS')
+
+        self.assertEqual(len(FakeSMTP.messages), 1)
+
+        # A.messages only holds what FakeGerrit places in it. Thus we
+        # work on the knowledge of what the first message should be as
+        # it is only configured to go to SMTP.
+
+        self.assertEqual('zuul_from@example.com',
+                         FakeSMTP.messages[0]['from_email'])
+        self.assertEqual(['alternative_me@example.com'],
+                         FakeSMTP.messages[0]['to_email'])
+        self.assertIn('Subject: Periodic check for org/project succeeded',
+                      FakeSMTP.messages[0]['headers'])
+
     def test_client_enqueue(self):
         "Test that the RPC client can enqueue a change"
         A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')

zuul/layoutvalidator.py

@@ -55,7 +55,11 @@ class LayoutSchema(object):
                                {'timer': toList(timer_trigger)}))
 
     report_actions = {'gerrit': variable_dict,
-                      'smtp': variable_dict}
+                      'smtp': {'to': str,
+                               'from': str,
+                               'subject': str,
+                               },
+                      }
 
     pipeline = {v.Required('name'): str,
                 v.Required('manager'): manager,

zuul/model.py

@@ -661,7 +661,6 @@ class QueueItem(object):
 
 class Changeish(object):
     """Something like a change; either a change or a ref"""
-    is_reportable = False
 
     def __init__(self, project):
         self.project = project
@@ -680,8 +679,6 @@ class Changeish(object):
 
 
 class Change(Changeish):
-    is_reportable = True
-
     def __init__(self, project):
         super(Change, self).__init__(project)
         self.branch = None
@@ -729,8 +726,6 @@ class Change(Changeish):
 
 
 class Ref(Changeish):
-    is_reportable = False
-
     def __init__(self, project):
         super(Ref, self).__init__(project)
         self.ref = None
@@ -767,7 +762,8 @@ class Ref(Changeish):
 
 
 class NullChange(Changeish):
-    is_reportable = False
+    def __repr__(self):
+        return '<NullChange for %s>' % (self.project)
 
     def _id(self):
         return None

zuul/reporter/smtp.py

@@ -46,7 +46,11 @@ class Reporter(object):
         to_email = params['to']\
             if 'to' in params else self.smtp_default_to
         msg = MIMEText(message)
-        msg['Subject'] = "Report change %s" % change
+        if 'subject' in params:
+            subject = params['subject'].format(change=change)
+        else:
+            subject = "Report for change %s" % change
+        msg['Subject'] = subject
         msg['From'] = from_email
         msg['To'] = to_email
 

zuul/scheduler.py

@@ -893,10 +893,6 @@ class BasePipelineManager(object):
         """
         report_errors = []
         if len(action_reporters) > 0:
-            if not change.number:
-                self.log.debug("Not reporting change %s: No number present."
-                               % change)
-                return
             for action_reporter in action_reporters:
                 ret = action_reporter.report(change, message)
                 if ret:
@@ -1220,7 +1216,7 @@ class BasePipelineManager(object):
         return True
 
     def reportItem(self, item):
-        if item.change.is_reportable and item.reported:
+        if item.reported:
             raise Exception("Already reported change %s" % item.change)
         ret = self._reportItem(item)
         if self.changes_merge:
@@ -1237,9 +1233,7 @@ class BasePipelineManager(object):
                 raise MergeFailure("Change %s failed to merge" % item.change)
 
     def _reportItem(self, item):
-        if not item.change.is_reportable:
-            return False
-        if item.change.is_reportable and item.reported:
+        if item.reported:
             return 0
         self.log.debug("Reporting change %s" % item.change)
         ret = True  # Means error as returned by trigger.report
@@ -1251,18 +1245,19 @@ class BasePipelineManager(object):
         else:
             actions = self.pipeline.failure_actions
             item.setReportedResult('FAILURE')
-        report = self.formatReport(item)
         item.reported = True
-        try:
-            self.log.info("Reporting change %s, actions: %s" %
-                          (item.change, actions))
-            ret = self.sendReport(actions, item.change, report)
-            if ret:
-                self.log.error("Reporting change %s received: %s" %
-                               (item.change, ret))
-        except:
-            self.log.exception("Exception while reporting:")
-            item.setReportedResult('ERROR')
+        if actions:
+            report = self.formatReport(item)
+            try:
+                self.log.info("Reporting change %s, actions: %s" %
+                              (item.change, actions))
+                ret = self.sendReport(actions, item.change, report)
+                if ret:
+                    self.log.error("Reporting change %s received: %s" %
+                                   (item.change, ret))
+            except:
+                self.log.exception("Exception while reporting:")
+                item.setReportedResult('ERROR')
         self.updateBuildDescriptions(item.current_build_set)
         return ret