More changes to make buildconfigs/imagestreams work:

- ensure we have only one imagestream for given component/branch at a time
- make buildconfig name form 'component-branch'
- checkout the built repo in order to find out branch and base image (will be useful later)

osbs/api.py

@@ -12,12 +12,14 @@
 import sys
 from functools import wraps
 
-from .constants import SIMPLE_BUILD_TYPE, PROD_WITHOUT_KOJI_BUILD_TYPE, PROD_WITH_SECRET_BUILD_TYPE
+from .constants import (SIMPLE_BUILD_TYPE, PROD_WITHOUT_KOJI_BUILD_TYPE,
+                        PROD_WITH_SECRET_BUILD_TYPE, BUILD_PENDING_STATES, BUILD_RUNNING_STATES)
 from osbs.build.build_request import BuildManager
 from osbs.build.build_response import BuildResponse
 from osbs.constants import DEFAULT_NAMESPACE, PROD_BUILD_TYPE
 from osbs.core import Openshift
-from osbs.exceptions import OsbsResponseException, OsbsException
+from osbs.exceptions import OsbsException
+from osbs.utils import checkout_git_repo, get_git_branch, get_base_image
 
 
 # Decorator for API methods.
@@ -128,17 +130,50 @@ def create_build_from_buildrequest(self, build_request, namespace=DEFAULT_NAMESP
         return build_response
 
     def _create_build_config_and_build(self, build_json, namespace):
-        build_config = self.os.create_build_config(json.dumps(build_json), namespace=namespace)
-        build_config_name = build_config.json()['metadata']['name']
+        build_config_name = build_json['metadata']['name']
+
+        # check if a build already exists for this config; if so then raise
+        # TODO: could we do this more effectively than traversing all?
+        running_builds = self.os.list_builds(namespace=namespace).json()
+        for rb in running_builds['items']:
+            try:
+                phase = rb['status']['phase'].lower()
+                bc = rb['metadata']['labels']['buildconfig']
+                if bc == build_config_name and \
+                        (phase in BUILD_PENDING_STATES + BUILD_RUNNING_STATES):
+                    raise OsbsException('A build for %s in state %s, can\'t proceed.' %
+                                        (build_config_name, phase))
+            except KeyError:
+                # rb wasn't created from this buildconfig
+                pass
+
+        try:
+            # see if there's already a build config
+            self.os.get_build_config(build_config_name)
+            # if so, then just update it
+            logger.debug('build config for %s already exists, updating...', build_config_name)
+            self.os.update_build_config(build_config_name, build_json, namespace)
+        except OsbsException:
+            # if it doesn't exist, then create it
+            logger.debug('build config for %s doesn\'t exist, creating...', build_config_name)
+            self.os.create_build_config(json.dumps(build_json), namespace=namespace)
         return self.os.start_build(build_config_name, namespace=namespace)
 
+    def _get_git_branch_and_base_image(self, git_uri, git_ref):
+        code_dir = checkout_git_repo(git_uri, git_ref)
+        git_branch = get_git_branch(code_dir)
+        base_image = get_base_image(code_dir)
+        return git_branch, base_image
+
     @osbsapi
     def create_prod_build(self, git_uri, git_ref, user, component, target, architecture, yum_repourls=None,
                           namespace=DEFAULT_NAMESPACE, **kwargs):
+        git_branch, base_image = self._get_git_branch_and_base_image(git_uri, git_ref)
         build_request = self.get_build_request(PROD_BUILD_TYPE)
         build_request.set_params(
             git_uri=git_uri,
             git_ref=git_ref,
+            git_branch=git_branch,
             user=user,
             component=component,
             registry_uri=self.build_conf.get_registry_uri(),
@@ -163,10 +198,12 @@ def create_prod_build(self, git_uri, git_ref, user, component, target, architect
     @osbsapi
     def create_prod_with_secret_build(self, git_uri, git_ref, user, component, target, architecture,
                                       yum_repourls=None, namespace=DEFAULT_NAMESPACE, **kwargs):
+        git_branch, base_image = self._get_git_branch_and_base_image(git_uri, git_ref)
         build_request = self.get_build_request(PROD_WITH_SECRET_BUILD_TYPE)
         build_request.set_params(
             git_uri=git_uri,
             git_ref=git_ref,
+            git_branch=git_branch,
             user=user,
             component=component,
             registry_uri=self.build_conf.get_registry_uri(),
@@ -195,10 +232,12 @@ def create_prod_with_secret_build(self, git_uri, git_ref, user, component, targe
     @osbsapi
     def create_prod_without_koji_build(self, git_uri, git_ref, user, component, architecture, yum_repourls=None,
                                        namespace=DEFAULT_NAMESPACE, **kwargs):
+        git_branch, base_image = self._get_git_branch_and_base_image(git_uri, git_ref)
         build_request = self.get_build_request(PROD_BUILD_TYPE)
         build_request.set_params(
             git_uri=git_uri,
             git_ref=git_ref,
+            git_branch=git_branch,
             user=user,
             component=component,
             registry_uri=self.build_conf.get_registry_uri(),
@@ -219,10 +258,12 @@ def create_prod_without_koji_build(self, git_uri, git_ref, user, component, arch
     @osbsapi
     def create_simple_build(self, git_uri, git_ref, user, component, yum_repourls=None,
                             namespace=DEFAULT_NAMESPACE, **kwargs):
+        git_branch, base_image = self._get_git_branch_and_base_image(git_uri, git_ref)
         build_request = self.get_build_request(SIMPLE_BUILD_TYPE)
         build_request.set_params(
             git_uri=git_uri,
             git_ref=git_ref,
+            git_branch=git_branch,
             user=user,
             component=component,
             registry_uri=self.build_conf.get_registry_uri(),

osbs/build/spec.py

@@ -66,28 +66,23 @@ def __init__(self):
 
     @BuildParam.value.setter
     def value(self, val):  # pylint: disable=W0221
-        d = datetime.datetime.now().strftime("%Y%m%d-%H%M%S")
-
         # build ID has to conform to:
         #  * 63 chars at most
         #  * (([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?
-        build_id = "%s-%s" % (val, d)
 
-        if len(build_id) > 63:
+        if len(val) > 63:
             # component + timestamp > 63
-            d_len = len(d)
-            new_prefix = val[:63 - d_len - 1]
-            new_name = "%s-%s" % (new_prefix, d)
-            logger.warning("'%s' is too long, changing to '%s'", build_id, new_name)
-            build_id = new_name
+            new_name = val[:63]
+            logger.warning("'%s' is too long, changing to '%s'", val, new_name)
+            val = new_name
 
         build_id_re = re.compile(r"^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$")
-        match = build_id_re.match(build_id)
+        match = build_id_re.match(val)
         if not match:
-            logger.error("'%s' is not valid build ID", build_id)
+            logger.error("'%s' is not valid build ID", val)
             raise OsbsValidationException("Build ID '%s', doesn't match regex '%s'" %
-                                          (build_id, build_id_re))
-        BuildParam.value.fset(self, build_id)
+                                          (val, build_id_re))
+        BuildParam.value.fset(self, val)
 
 
 class BuildTypeSpec(object):
@@ -111,6 +106,7 @@ def __repr__(self):
 class CommonSpec(BuildTypeSpec):
     git_uri = BuildParam('git_uri')
     git_ref = BuildParam('git_ref', default=DEFAULT_GIT_REF)
+    git_branch = BuildParam('git_branch')
     user = UserParam()
     component = BuildParam('component')
     registry_uri = BuildParam('registry_uri')
@@ -129,11 +125,12 @@ def __init__(self):
             self.openshift_uri,
         ]
 
-    def set_params(self, git_uri=None, git_ref=None, registry_uri=None, user=None,
+    def set_params(self, git_uri=None, git_ref=None, git_branch=None, registry_uri=None, user=None,
                    component=None, openshift_uri=None, yum_repourls=None,
                    metadata_plugin_use_auth=None, **kwargs):
         self.git_uri.value = git_uri
         self.git_ref.value = git_ref
+        self.git_branch.value = git_branch
         self.user.value = user
         self.component.value = component
         # We only want the hostname[:port]
@@ -145,7 +142,7 @@ def set_params(self, git_uri=None, git_ref=None, registry_uri=None, user=None,
             raise OsbsValidationException("yum_repourls must be a list")
         self.yum_repourls.value = yum_repourls or []
         self.metadata_plugin_use_auth.value = metadata_plugin_use_auth
-        self.name.value = self.component.value
+        self.name.value = '%s-%s' % (self.component.value, self.git_branch.value)
 
 
 class ProdSpec(CommonSpec):

osbs/core.py

@@ -164,6 +164,11 @@ def create_build_config(self, build_config_json, namespace=DEFAULT_NAMESPACE):
         return self._post(url, data=build_config_json,
                           headers={"Content-Type": "application/json"})
 
+    def update_build_config(self, build_config_id, build_config_json, namespace=DEFAULT_NAMESPACE):
+        url = self._build_url("namespaces/%s/buildconfigs/%s" % (namespace, build_config_id))
+        return self._put(url, data=build_config_json,
+                         headers={"Content-Type": "application/json"})
+
     def instantiate_build_config(self, build_config_id, namespace=DEFAULT_NAMESPACE):
         url = self._build_url("namespaces/%s/buildconfigs/%s/instantiate" % (
             namespace, build_config_id))

osbs/utils.py

@@ -8,6 +8,11 @@
 from __future__ import print_function, absolute_import, unicode_literals
 
 import copy
+import os
+import subprocess
+import tempfile
+
+from dockerfile_parse import DockerfileParser
 
 
 def graceful_chain_get(d, *args):
@@ -20,3 +25,22 @@ def graceful_chain_get(d, *args):
         except (AttributeError, KeyError):
             return None
     return t
+
+
+def checkout_git_repo(uri, commit):
+    tmpdir = tempfile.mkdtemp()
+    subprocess.check_call(['git', 'clone', uri, '-b', commit, tmpdir], stdout=subprocess.PIPE,
+                          stderr=subprocess.PIPE)
+    return tmpdir
+
+
+def get_git_branch(repo_dir):
+    p = subprocess.Popen(['git', 'rev-parse', '--abbrev-ref', 'HEAD'], stdout=subprocess.PIPE,
+                         stderr=subprocess.PIPE, cwd=repo_dir)
+    return p.communicate()[0].strip()
+
+
+def get_base_image(repo_dir):
+    df_path = os.path.join(repo_dir, 'Dockerfile')
+    df = DockerfileParser(df_path)
+    return df.baseimage

requirements.txt

@@ -1 +1,2 @@
+dockerfile-parse
 pycurl

tests/fake_api.py

@@ -16,7 +16,7 @@
 from osbs.http import Response
 from osbs.conf import Configuration
 from osbs.api import OSBS
-from tests.constants import TEST_BUILD, TEST_BUILD_CONFIG
+from tests.constants import TEST_BUILD, TEST_COMPONENT, TEST_GIT_REF
 from tempfile import NamedTemporaryFile
 
 try:
@@ -38,6 +38,12 @@ def process_authorize(content):
     }
 
 
+def buildconfig_not_found(content):
+    return {
+        "status_code": 404,
+    }
+
+
 DEFINITION = {
     "/osapi/v1beta3/namespaces/default/builds/": {
         "get": {
@@ -92,11 +98,25 @@ def process_authorize(content):
             "file": "created_build_config_test-build-config-123.json",
         }
     },
-    "/osapi/v1beta3/namespaces/default/buildconfigs/%s/instantiate" % TEST_BUILD_CONFIG: {
+    "/osapi/v1beta3/namespaces/default/buildconfigs/%s-%s/instantiate" % \
+        (TEST_COMPONENT, TEST_GIT_REF): {
         "post": {
             "file": "instantiated_test-build-config-123.json",
         }
     },
+    # use both version with ending slash and without it
+    "/osapi/v1beta3/namespaces/default/buildconfigs/%s-%s" % (TEST_COMPONENT, TEST_GIT_REF): {
+        "get": {
+            "custom_callback": buildconfig_not_found,
+            "file": "not_found_build-config-component-master.json",
+        }
+    },
+    "/osapi/v1beta3/namespaces/default/buildconfigs/%s-%s/" % (TEST_COMPONENT, TEST_GIT_REF): {
+        "get": {
+            "custom_callback": buildconfig_not_found,
+            "file": "not_found_build-config-component-master.json",
+        }
+    },
 }
 
 

tests/mock_jsons/0.5.2/created_build_config_test-build-config-123.json

@@ -21,7 +21,7 @@
         "output": {
             "to": {
                 "kind": "DockerImage",
-                "name": "10.34.2.242:5000/bkabrda/foo:20150729150321"
+                "name": "10.34.2.242:5000/bkabrda/master/component:20150729150321"
             }
         },
         "source": {
@@ -33,6 +33,6 @@
         }
     },
     "metadata": {
-        "name": "test-build-config-123"
+        "name": "component-master"
     }
 }

tests/mock_jsons/0.5.2/instantiated_test-build-config-123.json

@@ -2,7 +2,7 @@
     "status": {
         "config": {
             "kind": "BuildConfig",
-            "name": "foo-20150729-152103",
+            "name": "component-master",
             "namespace": "default"
         },
         "phase": "New"
@@ -34,7 +34,7 @@
         "output": {
             "to": {
                 "kind": "DockerImage",
-                "name": "10.34.2.242:5000/bkabrda/foo:20150729152103"
+                "name": "10.34.2.242:5000/bkabrda/master/component:20150729152103"
             }
         },
         "source": {
@@ -50,11 +50,11 @@
         "uid": "a96af1c9-35f4-11e5-a19a-525400667fb8",
         "creationTimestamp": "2015-07-29T13:21:03Z",
         "resourceVersion": "3823",
-        "selfLink": "/osapi/v1beta3/namespaces/default/buildconfigs/foo-20150729-152103-1/instantiate",
+        "selfLink": "/osapi/v1beta3/namespaces/default/buildconfigs/component-master/instantiate",
         "labels": {
-            "buildconfig": "foo-20150729-152103"
+            "buildconfig": "component-master"
         },
-        "name": "foo-20150729-152103-1",
+        "name": "component-master",
         "namespace": "default"
     }
 }

tests/mock_jsons/0.5.2/not_found_build-config-component-master.json

@@ -0,0 +1 @@
+{}

tests/mock_jsons/0.5.4/created_build_config_test-build-config-123.json

@@ -21,7 +21,7 @@
         "output": {
             "to": {
                 "kind": "DockerImage",
-                "name": "10.34.2.242:5000/bkabrda/foo:20150729150321"
+                "name": "10.34.2.242:5000/bkabrda/master/component:20150729150321"
             }
         },
         "source": {
@@ -33,6 +33,6 @@
         }
     },
     "metadata": {
-        "name": "test-build-config-123"
+        "name": "component-master"
     }
 }

tests/mock_jsons/0.5.4/instantiated_test-build-config-123.json

@@ -2,7 +2,7 @@
     "status": {
         "config": {
             "kind": "BuildConfig",
-            "name": "foo-20150729-152103",
+            "name": "component-master",
             "namespace": "default"
         },
         "phase": "New"
@@ -34,7 +34,7 @@
         "output": {
             "to": {
                 "kind": "DockerImage",
-                "name": "10.34.2.242:5000/bkabrda/foo:20150729152103"
+                "name": "10.34.2.242:5000/bkabrda/master/component:20150729152103"
             }
         },
         "source": {
@@ -50,11 +50,11 @@
         "uid": "a96af1c9-35f4-11e5-a19a-525400667fb8",
         "creationTimestamp": "2015-07-29T13:21:03Z",
         "resourceVersion": "3823",
-        "selfLink": "/osapi/v1beta3/namespaces/default/buildconfigs/foo-20150729-152103-1/instantiate",
+        "selfLink": "/osapi/v1beta3/namespaces/default/buildconfigs/component-master/instantiate",
         "labels": {
-            "buildconfig": "foo-20150729-152103"
+            "buildconfig": "component-master"
         },
-        "name": "foo-20150729-152103-1",
+        "name": "component-master",
         "namespace": "default"
     }
 }

tests/mock_jsons/0.5.4/not_found_build-config-component-master.json

@@ -0,0 +1 @@
+{}

tests/requirements.txt

@@ -1,2 +1,3 @@
+flexmock
 pytest
 six