Merge pull request #1651 from coreos-inc/fix-branches

Fix handling of multi-part branches in the build triggers

data/database.py

@@ -665,6 +665,12 @@ class BUILD_PHASE(object):
   WAITING = 'waiting'
   COMPLETE = 'complete'
 
+  @classmethod
+  def is_terminal_phase(cls, phase):
+    return (phase == cls.COMPLETE or
+            phase == cls.ERROR or
+            phase == cls.INTERNAL_ERROR)
+
 
 class QueueItem(BaseModel):
   queue_name = CharField(index=True, max_length=1024)

endpoints/api/build.py

@@ -87,36 +87,44 @@ def trigger_view(trigger, can_read=False, can_admin=False, for_build=False):
   return None
 
 
-def build_status_view(build_obj):
+def _get_build_status(build_obj):
+  """ Returns the updated build phase, status and (if any) error for the build object. """
   phase = build_obj.phase
   status = {}
   error = None
 
-  try:
+  # If the build is currently running, then load its "real-time" status from Redis.
-    status = build_logs.get_status(build_obj.uuid)
+  if not database.BUILD_PHASE.is_terminal_phase(phase):
-  except BuildStatusRetrievalError as bsre:
+    try:
-    phase = 'cannot_load'
+      status = build_logs.get_status(build_obj.uuid)
-    if SuperUserPermission().can():
+    except BuildStatusRetrievalError as bsre:
-      error = str(bsre)
+      phase = 'cannot_load'
-    else:
+      if SuperUserPermission().can():
-      error = 'Redis may be down. Please contact support.'
+        error = str(bsre)
+      else:
+        error = 'Redis may be down. Please contact support.'
 
-  if phase != 'cannot_load':
+    if phase != 'cannot_load':
-    # If the status contains a heartbeat, then check to see if has been written in the last few
+      # If the status contains a heartbeat, then check to see if has been written in the last few
-    # minutes. If not, then the build timed out.
+      # minutes. If not, then the build timed out.
-    if phase != database.BUILD_PHASE.COMPLETE and phase != database.BUILD_PHASE.ERROR:
       if status is not None and 'heartbeat' in status and status['heartbeat']:
         heartbeat = datetime.datetime.utcfromtimestamp(status['heartbeat'])
         if datetime.datetime.utcnow() - heartbeat > datetime.timedelta(minutes=1):
           phase = database.BUILD_PHASE.INTERNAL_ERROR
 
-    # If the phase is internal error, return 'error' instead if the number of retries
+      # If the phase is internal error, return 'error' instead if the number of retries
-    # on the queue item is 0.
+      # on the queue item is 0.
-    if phase == database.BUILD_PHASE.INTERNAL_ERROR:
+      if phase == database.BUILD_PHASE.INTERNAL_ERROR:
-      retry = build_obj.queue_id and dockerfile_build_queue.has_retries_remaining(build_obj.queue_id)
+        retry = (build_obj.queue_id and
-      if not retry:
+                 dockerfile_build_queue.has_retries_remaining(build_obj.queue_id))
-        phase = database.BUILD_PHASE.ERROR
+        if not retry:
+          phase = database.BUILD_PHASE.ERROR
 
+  return (phase, status, error)
+
+
+def build_status_view(build_obj):
+  phase, status, error = _get_build_status(build_obj)
   repo_namespace = build_obj.repository.namespace_user.username
   repo_name = build_obj.repository.name
 

endpoints/building.py

@@ -98,7 +98,7 @@ class PreparedBuild(object):
     return sha[0:7]
 
   def tags_from_ref(self, ref, default_branch=None):
-    branch = ref.split('/')[-1]
+    branch = ref.split('/', 2)[-1]
     tags = {branch}
 
     if branch == default_branch:

static/js/directives/ui/source-ref-link.js

@@ -29,7 +29,7 @@ angular.module('quay').directive('sourceRefLink', function () {
           return '';
         }
 
-        return parts[2];
+        return parts.slice(2).join('/');
       };
 
       $scope.getUrl = function(ref, template, kind) {

test/test_prepare_trigger.py

@@ -207,6 +207,27 @@ class TestPrepareTrigger(unittest.TestCase):
     self.assertSchema('bitbucket_webhook', expected, bb_webhook)
 
 
+  def test_github_webhook_payload_slash_branch(self):
+    expected = {
+      'commit': u'410f4cdf8ff09b87f245b13845e8497f90b90a4c',
+      'ref': u'refs/heads/slash/branch',
+      'git_url': u'git@github.com:josephschorr/anothertest.git',
+      'commit_info': {
+        'url': u'https://github.com/josephschorr/anothertest/commit/410f4cdf8ff09b87f245b13845e8497f90b90a4c',
+        'date': u'2015-09-11T14:26:16-04:00',
+        'message': u'Update Dockerfile',
+        'committer': {
+          'username': u'josephschorr',
+        },
+        'author': {
+          'username': u'josephschorr',
+        },
+      },
+    }
+
+    self.assertSchema('github_webhook_slash_branch', expected, gh_webhook)
+
+
   def test_github_webhook_payload(self):
     expected = {
       'commit': u'410f4cdf8ff09b87f245b13845e8497f90b90a4c',

test/test_trigger.py

@@ -16,6 +16,8 @@ class TestRegex(unittest.TestCase):
     self.assertMatches('ref/heads/master', '.+')
     self.assertMatches('ref/heads/master', 'heads/.+')
     self.assertMatches('ref/heads/master', 'heads/master')
+    self.assertMatches('ref/heads/slash/branch', 'heads/slash/branch')
+    self.assertMatches('ref/heads/slash/branch', 'heads/.+')
 
     self.assertDoesNotMatch('ref/heads/foobar', 'heads/master')
     self.assertDoesNotMatch('ref/heads/master', 'tags/master')
@@ -28,6 +30,27 @@ class TestRegex(unittest.TestCase):
     self.assertDoesNotMatch('ref/heads/delta', '(((heads/alpha)|(heads/beta))|(heads/gamma))|(heads/master)')
 
 
+class TestTags(unittest.TestCase):
+  def assertTagsForRef(self, ref, tags):
+    prepared = PreparedBuild()
+    prepared.tags_from_ref(ref, default_branch='master')
+    self.assertEquals(tags, prepared._tags)
+
+  def test_normal(self):
+    self.assertTagsForRef('ref/heads/somebranch', ['somebranch'])
+    self.assertTagsForRef('ref/heads/master', ['master', 'latest'])
+
+    self.assertTagsForRef('ref/tags/somebranch', ['somebranch'])
+    self.assertTagsForRef('ref/tags/master', ['master', 'latest'])
+
+  def test_slash(self):
+    self.assertTagsForRef('ref/heads/slash/branch', ['slash_branch'])
+    self.assertTagsForRef('ref/tags/slash/tag', ['slash_tag'])
+
+  def test_unusual(self):
+    self.assertTagsForRef('ref/heads/foobar#2', ['foobar_2'])
+
+
 class TestSkipBuild(unittest.TestCase):
   def testSkipNoMetadata(self):
     prepared = PreparedBuild()

test/triggerjson/github_webhook_slash_branch.json

@@ -0,0 +1,153 @@
+{
+  "ref": "refs/heads/slash/branch",
+  "before": "9ea43cab474709d4a61afb7e3340de1ffc405b41",
+  "after": "410f4cdf8ff09b87f245b13845e8497f90b90a4c",
+  "created": false,
+  "deleted": false,
+  "forced": false,
+  "base_ref": null,
+  "compare": "https://github.com/josephschorr/anothertest/compare/9ea43cab4747...410f4cdf8ff0",
+  "commits": [
+    {
+      "id": "410f4cdf8ff09b87f245b13845e8497f90b90a4c",
+      "distinct": true,
+      "message": "Update Dockerfile",
+      "timestamp": "2015-09-11T14:26:16-04:00",
+      "url": "https://github.com/josephschorr/anothertest/commit/410f4cdf8ff09b87f245b13845e8497f90b90a4c",
+      "author": {
+        "name": "josephschorr",
+        "email": "josephschorr@users.noreply.github.com",
+        "username": "josephschorr"
+      },
+      "committer": {
+        "name": "josephschorr",
+        "email": "josephschorr@users.noreply.github.com",
+        "username": "josephschorr"
+      },
+      "added": [],
+      "removed": [],
+      "modified": [
+        "Dockerfile"
+      ]
+    }
+  ],
+  "head_commit": {
+    "id": "410f4cdf8ff09b87f245b13845e8497f90b90a4c",
+    "distinct": true,
+    "message": "Update Dockerfile",
+    "timestamp": "2015-09-11T14:26:16-04:00",
+    "url": "https://github.com/josephschorr/anothertest/commit/410f4cdf8ff09b87f245b13845e8497f90b90a4c",
+    "author": {
+      "name": "josephschorr",
+      "email": "josephschorr@users.noreply.github.com",
+      "username": "josephschorr"
+    },
+    "committer": {
+      "name": "josephschorr",
+      "email": "josephschorr@users.noreply.github.com",
+      "username": "josephschorr"
+    },
+    "added": [],
+    "removed": [],
+    "modified": [
+      "Dockerfile"
+    ]
+  },
+  "repository": {
+    "id": 34876107,
+    "name": "anothertest",
+    "full_name": "josephschorr/anothertest",
+    "owner": {
+      "name": "josephschorr",
+      "email": "josephschorr@users.noreply.github.com"
+    },
+    "private": false,
+    "html_url": "https://github.com/josephschorr/anothertest",
+    "description": "",
+    "fork": false,
+    "url": "https://github.com/josephschorr/anothertest",
+    "forks_url": "https://api.github.com/repos/josephschorr/anothertest/forks",
+    "keys_url": "https://api.github.com/repos/josephschorr/anothertest/keys{/key_id}",
+    "collaborators_url": "https://api.github.com/repos/josephschorr/anothertest/collaborators{/collaborator}",
+    "teams_url": "https://api.github.com/repos/josephschorr/anothertest/teams",
+    "hooks_url": "https://api.github.com/repos/josephschorr/anothertest/hooks",
+    "issue_events_url": "https://api.github.com/repos/josephschorr/anothertest/issues/events{/number}",
+    "events_url": "https://api.github.com/repos/josephschorr/anothertest/events",
+    "assignees_url": "https://api.github.com/repos/josephschorr/anothertest/assignees{/user}",
+    "branches_url": "https://api.github.com/repos/josephschorr/anothertest/branches{/branch}",
+    "tags_url": "https://api.github.com/repos/josephschorr/anothertest/tags",
+    "blobs_url": "https://api.github.com/repos/josephschorr/anothertest/git/blobs{/sha}",
+    "git_tags_url": "https://api.github.com/repos/josephschorr/anothertest/git/tags{/sha}",
+    "git_refs_url": "https://api.github.com/repos/josephschorr/anothertest/git/refs{/sha}",
+    "trees_url": "https://api.github.com/repos/josephschorr/anothertest/git/trees{/sha}",
+    "statuses_url": "https://api.github.com/repos/josephschorr/anothertest/statuses/{sha}",
+    "languages_url": "https://api.github.com/repos/josephschorr/anothertest/languages",
+    "stargazers_url": "https://api.github.com/repos/josephschorr/anothertest/stargazers",
+    "contributors_url": "https://api.github.com/repos/josephschorr/anothertest/contributors",
+    "subscribers_url": "https://api.github.com/repos/josephschorr/anothertest/subscribers",
+    "subscription_url": "https://api.github.com/repos/josephschorr/anothertest/subscription",
+    "commits_url": "https://api.github.com/repos/josephschorr/anothertest/commits{/sha}",
+    "git_commits_url": "https://api.github.com/repos/josephschorr/anothertest/git/commits{/sha}",
+    "comments_url": "https://api.github.com/repos/josephschorr/anothertest/comments{/number}",
+    "issue_comment_url": "https://api.github.com/repos/josephschorr/anothertest/issues/comments{/number}",
+    "contents_url": "https://api.github.com/repos/josephschorr/anothertest/contents/{+path}",
+    "compare_url": "https://api.github.com/repos/josephschorr/anothertest/compare/{base}...{head}",
+    "merges_url": "https://api.github.com/repos/josephschorr/anothertest/merges",
+    "archive_url": "https://api.github.com/repos/josephschorr/anothertest/{archive_format}{/ref}",
+    "downloads_url": "https://api.github.com/repos/josephschorr/anothertest/downloads",
+    "issues_url": "https://api.github.com/repos/josephschorr/anothertest/issues{/number}",
+    "pulls_url": "https://api.github.com/repos/josephschorr/anothertest/pulls{/number}",
+    "milestones_url": "https://api.github.com/repos/josephschorr/anothertest/milestones{/number}",
+    "notifications_url": "https://api.github.com/repos/josephschorr/anothertest/notifications{?since,all,participating}",
+    "labels_url": "https://api.github.com/repos/josephschorr/anothertest/labels{/name}",
+    "releases_url": "https://api.github.com/repos/josephschorr/anothertest/releases{/id}",
+    "created_at": 1430426945,
+    "updated_at": "2015-04-30T20:49:05Z",
+    "pushed_at": 1441995976,
+    "git_url": "git://github.com/josephschorr/anothertest.git",
+    "ssh_url": "git@github.com:josephschorr/anothertest.git",
+    "clone_url": "https://github.com/josephschorr/anothertest.git",
+    "svn_url": "https://github.com/josephschorr/anothertest",
+    "homepage": null,
+    "size": 144,
+    "stargazers_count": 0,
+    "watchers_count": 0,
+    "language": null,
+    "has_issues": true,
+    "has_downloads": true,
+    "has_wiki": true,
+    "has_pages": false,
+    "forks_count": 0,
+    "mirror_url": null,
+    "open_issues_count": 0,
+    "forks": 0,
+    "open_issues": 0,
+    "watchers": 0,
+    "default_branch": "master",
+    "stargazers": 0,
+    "master_branch": "master"
+  },
+  "pusher": {
+    "name": "josephschorr",
+    "email": "josephschorr@users.noreply.github.com"
+  },
+  "sender": {
+    "login": "josephschorr",
+    "id": 4073002,
+    "avatar_url": "https://avatars.githubusercontent.com/u/4073002?v=3",
+    "gravatar_id": "",
+    "url": "https://api.github.com/users/josephschorr",
+    "html_url": "https://github.com/josephschorr",
+    "followers_url": "https://api.github.com/users/josephschorr/followers",
+    "following_url": "https://api.github.com/users/josephschorr/following{/other_user}",
+    "gists_url": "https://api.github.com/users/josephschorr/gists{/gist_id}",
+    "starred_url": "https://api.github.com/users/josephschorr/starred{/owner}{/repo}",
+    "subscriptions_url": "https://api.github.com/users/josephschorr/subscriptions",
+    "organizations_url": "https://api.github.com/users/josephschorr/orgs",
+    "repos_url": "https://api.github.com/users/josephschorr/repos",
+    "events_url": "https://api.github.com/users/josephschorr/events{/privacy}",
+    "received_events_url": "https://api.github.com/users/josephschorr/received_events",
+    "type": "User",
+    "site_admin": false
+  }
+}