Merge "Change mutex to counting semaphore" into feature/zuulv3
diff --git a/.zuul.yaml b/.zuul.yaml
index 98b880d..50223fa 100644
--- a/.zuul.yaml
+++ b/.zuul.yaml
@@ -31,7 +31,7 @@
 - job:
     name: tox-linters
     parent: tox
-    run: tox/docs
+    run: tox/linters
 
 - job:
     name: tox-py27
diff --git a/doc/source/datamodel.rst b/doc/source/developer/datamodel.rst
similarity index 100%
rename from doc/source/datamodel.rst
rename to doc/source/developer/datamodel.rst
diff --git a/doc/source/drivers.rst b/doc/source/developer/drivers.rst
similarity index 100%
rename from doc/source/drivers.rst
rename to doc/source/developer/drivers.rst
diff --git a/doc/source/developer.rst b/doc/source/developer/index.rst
similarity index 95%
rename from doc/source/developer.rst
rename to doc/source/developer/index.rst
index 527ea6e..986bbe4 100644
--- a/doc/source/developer.rst
+++ b/doc/source/developer/index.rst
@@ -12,4 +12,5 @@
 
    datamodel
    drivers
+   triggers
    testing
diff --git a/doc/source/testing.rst b/doc/source/developer/testing.rst
similarity index 100%
rename from doc/source/testing.rst
rename to doc/source/developer/testing.rst
diff --git a/doc/source/developer/triggers.rst b/doc/source/developer/triggers.rst
new file mode 100644
index 0000000..56f4a03
--- /dev/null
+++ b/doc/source/developer/triggers.rst
@@ -0,0 +1,19 @@
+Triggers
+========
+
+Triggers must inherit from :py:class:`~zuul.trigger.BaseTrigger` and, at a minimum,
+implement the :py:meth:`~zuul.trigger.BaseTrigger.getEventFilters` method.
+
+.. autoclass:: zuul.trigger.BaseTrigger
+   :members:
+
+Current list of triggers are:
+
+.. autoclass:: zuul.driver.gerrit.gerrittrigger.GerritTrigger
+   :members:
+
+.. autoclass:: zuul.driver.timer.timertrigger.TimerTrigger
+   :members:
+
+.. autoclass:: zuul.driver.zuul.zuultrigger.ZuulTrigger
+   :members:
diff --git a/doc/source/index.rst b/doc/source/index.rst
index 3f903db..fb30b92 100644
--- a/doc/source/index.rst
+++ b/doc/source/index.rst
@@ -24,7 +24,7 @@
    executors
    statsd
    client
-   developer
+   developer/index
 
 Indices and tables
 ==================
diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst
index d41ed89..56cc6a8 100644
--- a/doc/source/zuul.rst
+++ b/doc/source/zuul.rst
@@ -124,13 +124,6 @@
   optional value and ``1`` is used by default.
   ``status_expiry=1``
 
-**url_pattern**
-  If you are storing build logs external to the system that originally
-  ran jobs and wish to link to those logs when Zuul makes comments on
-  Gerrit changes for completed jobs this setting configures what the
-  URLs for those links should be.  Used by zuul-server only.
-  ``http://logs.example.com/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.number}``
-
 **job_name_in_report**
   Boolean value (``true`` or ``false``) that indicates whether the
   job name should be included in the report (normally only the URL
diff --git a/tests/fixtures/config/success-url/git/common-config/zuul.yaml b/tests/fixtures/config/success-url/git/common-config/zuul.yaml
index f2d5251..b3ecf6d 100644
--- a/tests/fixtures/config/success-url/git/common-config/zuul.yaml
+++ b/tests/fixtures/config/success-url/git/common-config/zuul.yaml
@@ -19,7 +19,7 @@
 
 - job:
     name: docs-draft-test
-    success-url: http://docs-draft.example.org/{build.parameters[LOG_PATH]}/publish-docs/
+    success-url: http://docs-draft.example.org/{change.number:.2}/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.uuid:.7}/publish-docs/
 
 - job:
     name: docs-draft-test2
diff --git a/tests/fixtures/zuul-connections-multiple-gerrits.conf b/tests/fixtures/zuul-connections-multiple-gerrits.conf
index b3182d7..d1522ec 100644
--- a/tests/fixtures/zuul-connections-multiple-gerrits.conf
+++ b/tests/fixtures/zuul-connections-multiple-gerrits.conf
@@ -3,7 +3,6 @@
 
 [zuul]
 tenant_config=main.yaml
-url_pattern=http://logs.example.com/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.number}
 job_name_in_report=true
 
 [merger]
diff --git a/tests/fixtures/zuul-connections-same-gerrit.conf b/tests/fixtures/zuul-connections-same-gerrit.conf
index 6156df4..8ddd0f1 100644
--- a/tests/fixtures/zuul-connections-same-gerrit.conf
+++ b/tests/fixtures/zuul-connections-same-gerrit.conf
@@ -3,7 +3,6 @@
 
 [zuul]
 tenant_config=config/zuul-connections-same-gerrit/main.yaml
-url_pattern=http://logs.example.com/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.number}
 job_name_in_report=true
 
 [merger]
diff --git a/tests/fixtures/zuul-git-driver.conf b/tests/fixtures/zuul-git-driver.conf
index 0a4e230..499b564 100644
--- a/tests/fixtures/zuul-git-driver.conf
+++ b/tests/fixtures/zuul-git-driver.conf
@@ -3,7 +3,6 @@
 
 [zuul]
 tenant_config=config/zuul-connections-same-gerrit/main.yaml
-url_pattern=http://logs.example.com/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.number}
 job_name_in_report=true
 
 [merger]
diff --git a/tests/fixtures/zuul-sql-driver-bad.conf b/tests/fixtures/zuul-sql-driver-bad.conf
index d91e2f6..a4df735 100644
--- a/tests/fixtures/zuul-sql-driver-bad.conf
+++ b/tests/fixtures/zuul-sql-driver-bad.conf
@@ -2,8 +2,7 @@
 server=127.0.0.1
 
 [zuul]
-tenant_config=main.yaml
-url_pattern=http://logs.example.com/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.number}
+layout_config=layout-connections-multiple-voters.yaml
 job_name_in_report=true
 
 [merger]
diff --git a/tests/fixtures/zuul.conf b/tests/fixtures/zuul.conf
index ce29310..cd80a45 100644
--- a/tests/fixtures/zuul.conf
+++ b/tests/fixtures/zuul.conf
@@ -3,7 +3,6 @@
 
 [zuul]
 tenant_config=main.yaml
-url_pattern=http://logs.example.com/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.number}
 job_name_in_report=true
 
 [merger]
diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py
index 514aa1f..e3c726f 100644
--- a/zuul/driver/gerrit/gerritconnection.py
+++ b/zuul/driver/gerrit/gerritconnection.py
@@ -93,6 +93,9 @@
             event.ref = refupdate.get('refName')
             event.oldrev = refupdate.get('oldRev')
             event.newrev = refupdate.get('newRev')
+        if event.project_name is None:
+            # ref-replica* events
+            event.project_name = data.get('project')
         # Map the event types to a field name holding a Gerrit
         # account attribute. See Gerrit stream-event documentation
         # in cmd-stream-events.html
diff --git a/zuul/driver/sql/alembic_reporter.ini b/zuul/driver/sql/alembic.ini
similarity index 100%
rename from zuul/driver/sql/alembic_reporter.ini
rename to zuul/driver/sql/alembic.ini
diff --git a/zuul/driver/sql/alembic_reporter/versions/1dd914d4a482_allow_score_to_be_null.py b/zuul/driver/sql/alembic_reporter/versions/1dd914d4a482_allow_score_to_be_null.py
new file mode 100644
index 0000000..b153cab
--- /dev/null
+++ b/zuul/driver/sql/alembic_reporter/versions/1dd914d4a482_allow_score_to_be_null.py
@@ -0,0 +1,25 @@
+"""Allow score to be null
+
+Revision ID: 1dd914d4a482
+Revises: 4d3ebd7f06b9
+Create Date: 2017-03-28 08:09:32.908643
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '1dd914d4a482'
+down_revision = '4d3ebd7f06b9'
+branch_labels = None
+depends_on = None
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    op.alter_column('zuul_buildset', 'score', nullable=True,
+                    existing_type=sa.Integer)
+
+
+def downgrade():
+    raise Exception("Downgrades not supported")
diff --git a/zuul/driver/sql/sqlconnection.py b/zuul/driver/sql/sqlconnection.py
index 69e53df..31bc13a 100644
--- a/zuul/driver/sql/sqlconnection.py
+++ b/zuul/driver/sql/sqlconnection.py
@@ -80,7 +80,7 @@
             sa.Column('change', sa.Integer, nullable=True),
             sa.Column('patchset', sa.Integer, nullable=True),
             sa.Column('ref', sa.String(255)),
-            sa.Column('score', sa.Integer),
+            sa.Column('score', sa.Integer, nullable=True),
             sa.Column('message', sa.TEXT()),
         )
 
diff --git a/zuul/driver/sql/sqlreporter.py b/zuul/driver/sql/sqlreporter.py
index 2129f53..d6e547d 100644
--- a/zuul/driver/sql/sqlreporter.py
+++ b/zuul/driver/sql/sqlreporter.py
@@ -28,6 +28,7 @@
     def __init__(self, driver, connection, config={}):
         super(SQLReporter, self).__init__(
             driver, connection, config)
+        # TODO(jeblair): document this is stored as NULL if unspecified
         self.result_score = config.get('score', None)
 
     def report(self, source, pipeline, item):
@@ -37,13 +38,6 @@
             self.log.warn("SQL reporter (%s) is disabled " % self)
             return
 
-        if self.driver.sched.config.has_option('zuul', 'url_pattern'):
-            url_pattern = self.driver.sched.config.get('zuul', 'url_pattern')
-        else:
-            url_pattern = None
-
-        score = self.config.get('score', 0)
-
         with self.connection.engine.begin() as conn:
             buildset_ins = self.connection.zuul_buildset_table.insert().values(
                 zuul_ref=item.current_build_set.ref,
@@ -52,7 +46,7 @@
                 change=item.change.number,
                 patchset=item.change.patchset,
                 ref=item.change.refspec,
-                score=score,
+                score=self.result_score,
                 message=self._formatItemReport(
                     pipeline, item, with_jobs=False),
             )
@@ -67,7 +61,7 @@
                     # information about the change.
                     continue
 
-                (result, url) = item.formatJobResult(job, url_pattern)
+                (result, url) = item.formatJobResult(job)
 
                 build_inserts.append({
                     'buildset_id': buildset_ins_result.inserted_primary_key,
diff --git a/zuul/executor/client.py b/zuul/executor/client.py
index 20bc3ef..90cfa9b 100644
--- a/zuul/executor/client.py
+++ b/zuul/executor/client.py
@@ -16,7 +16,6 @@
 import gear
 import json
 import logging
-import os
 import time
 import threading
 from uuid import uuid4
@@ -266,13 +265,6 @@
             params['ZUUL_REF'] = item.change.ref
             params['ZUUL_COMMIT'] = item.change.newrev
 
-        # The destination_path is a unique path for this build request
-        # and generally where the logs are expected to be placed
-        destination_path = os.path.join(item.change.getBasePath(),
-                                        pipeline.name, job.name, uuid[:7])
-        params['BASE_LOG_PATH'] = item.change.getBasePath()
-        params['LOG_PATH'] = destination_path
-
         # This is what we should be heading toward for parameters:
 
         # required:
diff --git a/zuul/executor/server.py b/zuul/executor/server.py
index 60b30c7..67fc5e6 100644
--- a/zuul/executor/server.py
+++ b/zuul/executor/server.py
@@ -646,10 +646,15 @@
                 nodepool_az=node.get('az'),
                 nodepool_provider=node.get('provider'),
                 nodepool_region=node.get('region'))
+
+            host_keys = []
+            for key in node.get('host_keys'):
+                host_keys.append("%s %s" % (ip, key))
+
             hosts.append(dict(
                 name=node['name'],
                 host_vars=host_vars,
-                host_keys=node.get('host_keys')))
+                host_keys=host_keys))
         return hosts
 
     def _blockPluginDirs(self, path):
diff --git a/zuul/model.py b/zuul/model.py
index 0ce332f..744c0f3 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -98,6 +98,13 @@
     return re.sub(' ', '-', name)
 
 
+class Attributes(object):
+    """A class to hold attributes for string formatting."""
+
+    def __init__(self, **kw):
+        setattr(self, '__dict__', kw)
+
+
 class Pipeline(object):
     """A configuration that ties triggers, reporters, managers and sources.
 
@@ -159,6 +166,9 @@
     def __repr__(self):
         return '<Pipeline %s>' % self.name
 
+    def getSafeAttributes(self):
+        return Attributes(name=self.name)
+
     def setManager(self, manager):
         self.manager = manager
 
@@ -186,7 +196,7 @@
             items.extend(shared_queue.queue)
         return items
 
-    def formatStatusJSON(self, url_pattern=None):
+    def formatStatusJSON(self):
         j_pipeline = dict(name=self.name,
                           description=self.description)
         j_queues = []
@@ -203,7 +213,7 @@
                     if j_changes:
                         j_queue['heads'].append(j_changes)
                     j_changes = []
-                j_changes.append(e.formatJSON(url_pattern))
+                j_changes.append(e.formatJSON())
                 if (len(j_changes) > 1 and
                         (j_changes[-2]['remaining_time'] is not None) and
                         (j_changes[-1]['remaining_time'] is not None)):
@@ -820,6 +830,9 @@
     def _get(self, name):
         return self.__dict__.get(name)
 
+    def getSafeAttributes(self):
+        return Attributes(name=self.name)
+
     def setRun(self):
         if not self.run:
             self.run = self.implied_run
@@ -1049,6 +1062,9 @@
         return ('<Build %s of %s on %s>' %
                 (self.uuid, self.job.name, self.worker))
 
+    def getSafeAttributes(self):
+        return Attributes(uuid=self.uuid)
+
 
 class Worker(object):
     """Information about the specific worker executing a Build."""
@@ -1489,10 +1505,10 @@
             fakebuild.result = 'SKIPPED'
             self.addBuild(fakebuild)
 
-    def formatJobResult(self, job, url_pattern=None):
+    def formatJobResult(self, job):
         build = self.current_build_set.getBuild(job.name)
         result = build.result
-        pattern = url_pattern
+        pattern = None
         if result == 'SUCCESS':
             if job.success_message:
                 result = job.success_message
@@ -1504,19 +1520,27 @@
             if job.failure_url:
                 pattern = job.failure_url
         url = None
+        # Produce safe versions of objects which may be useful in
+        # result formatting, but don't allow users to crawl through
+        # the entire data structure where they might be able to access
+        # secrets, etc.
+        safe_change = self.change.getSafeAttributes()
+        safe_pipeline = self.pipeline.getSafeAttributes()
+        safe_job = job.getSafeAttributes()
+        safe_build = build.getSafeAttributes()
         if pattern:
             try:
-                url = pattern.format(change=self.change,
-                                     pipeline=self.pipeline,
-                                     job=job,
-                                     build=build)
+                url = pattern.format(change=safe_change,
+                                     pipeline=safe_pipeline,
+                                     job=safe_job,
+                                     build=safe_build)
             except Exception:
                 pass  # FIXME: log this or something?
         if not url:
             url = build.url or job.name
         return (result, url)
 
-    def formatJSON(self, url_pattern=None):
+    def formatJSON(self):
         changeish = self.change
         ret = {}
         ret['active'] = self.active
@@ -1559,7 +1583,7 @@
             if build:
                 result = build.result
                 build_url = build.url
-                (unused, report_url) = self.formatJobResult(job, url_pattern)
+                (unused, report_url) = self.formatJobResult(job)
                 if build.start_time:
                     if build.end_time:
                         elapsed = int((build.end_time -
@@ -1702,6 +1726,12 @@
     def updatesConfig(self):
         return False
 
+    def getSafeAttributes(self):
+        return Attributes(project=self.project,
+                          ref=self.ref,
+                          oldrev=self.oldrev,
+                          newrev=self.newrev)
+
 
 class Change(Ref):
     """A proposed new state for a Project."""
@@ -1765,6 +1795,11 @@
             return True
         return False
 
+    def getSafeAttributes(self):
+        return Attributes(project=self.project,
+                          number=self.number,
+                          patchset=self.patchset)
+
 
 class TriggerEvent(object):
     """Incoming event from an external system."""
diff --git a/zuul/reporter/__init__.py b/zuul/reporter/__init__.py
index 6df3f1b..5e25e7c 100644
--- a/zuul/reporter/__init__.py
+++ b/zuul/reporter/__init__.py
@@ -111,14 +111,10 @@
         ret = ''
 
         config = self.connection.sched.config
-        if config.has_option('zuul', 'url_pattern'):
-            url_pattern = config.get('zuul', 'url_pattern')
-        else:
-            url_pattern = None
 
         for job in item.getJobs():
             build = item.current_build_set.getBuild(job.name)
-            (result, url) = item.formatJobResult(job, url_pattern)
+            (result, url) = item.formatJobResult(job)
             if not job.voting:
                 voting = ' (non-voting)'
             else:
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index 882133c..0fa1763 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -856,11 +856,6 @@
 
     def formatStatusJSON(self, tenant_name):
         # TODOv3(jeblair): use tenants
-        if self.config.has_option('zuul', 'url_pattern'):
-            url_pattern = self.config.get('zuul', 'url_pattern')
-        else:
-            url_pattern = None
-
         data = {}
 
         data['zuul_version'] = self.zuul_version
@@ -887,5 +882,5 @@
         data['pipelines'] = pipelines
         tenant = self.abide.tenants.get(tenant_name)
         for pipeline in tenant.layout.pipelines.values():
-            pipelines.append(pipeline.formatStatusJSON(url_pattern))
+            pipelines.append(pipeline.formatStatusJSON())
         return json.dumps(data)