Fix build controller performance issues

Adds an openshift.io/build.completed annotation to mark a completed
build that has been processed by the build controller and does not
need to be processed again. Also adds an openshift.io/build.accepted
annotation to force a build to be added to the controller queue.
Previously this was an update to the startTime of the build, which
would result in an incorrect value.

pkg/build/api/types.go

@@ -54,6 +54,14 @@ const (
 	// BuildConfigPausedAnnotation is an annotation that marks a BuildConfig as paused.
 	// New Builds cannot be instantiated from a paused BuildConfig.
 	BuildConfigPausedAnnotation = "openshift.io/build-config.paused"
+	// BuildCompletedAnnotation is an annotation that marks a Build as completed and
+	// will prevent the build controller from processing it further.
+	BuildCompletedAnnotation = "build.openshift.io/completed"
+	// BuildAcceptedAnnotation is an annotation used to update a build that can now be
+        // run based on the RunPolicy (e.g. Serial). Updating the build with this annotation
+        // forces the build to be processed by the build controller queue without waiting
+        // for a resync.
+	BuildAcceptedAnnotation = "build.openshift.io/accepted"
 )
 
 // +genclient=true

pkg/build/controller/controller.go

@@ -102,9 +102,13 @@ func (bc *BuildController) HandleBuild(build *buildapi.Build) error {
 	}
 
 	if buildutil.IsBuildComplete(build) {
+		if _, ok := build.Annotations[buildapi.BuildCompletedAnnotation]; ok {
+			return nil
+		}
 		if err := runPolicy.OnComplete(build); err != nil {
 			return err
 		}
+		build.Annotations[buildapi.BuildCompletedAnnotation] = "true"
 		return nil
 	}
 
@@ -493,6 +497,7 @@ func setBuildPodNameAnnotation(build *buildapi.Build, podName string) {
 
 func setBuildCompletionTimeAndDuration(build *buildapi.Build) {
 	now := unversioned.Now()
+
 	build.Status.CompletionTimestamp = &now
 	if build.Status.StartTimestamp != nil {
 		build.Status.Duration = build.Status.CompletionTimestamp.Rfc3339Copy().Time.Sub(build.Status.StartTimestamp.Rfc3339Copy().Time)

pkg/build/controller/policy/policy.go

@@ -9,7 +9,6 @@ import (
 	buildclient "github.com/openshift/origin/pkg/build/client"
 	buildutil "github.com/openshift/origin/pkg/build/util"
 	"k8s.io/kubernetes/pkg/api/errors"
-	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/util/wait"
 )
 
@@ -124,7 +123,7 @@ func GetNextConfigBuild(lister buildclient.BuildLister, namespace, buildConfigNa
 }
 
 // handleComplete represents the default OnComplete handler. This Handler will
-// check which build should be run next and update the StartTimestamp field for
+// check which build should be run next and set the accepted annotation for
 // that build. That will trigger HandleBuild() to process that build immediately
 // and as a result the build is immediately executed.
 func handleComplete(lister buildclient.BuildLister, updater buildclient.BuildUpdater, build *buildapi.Build) error {
@@ -139,9 +138,11 @@ func handleComplete(lister buildclient.BuildLister, updater buildclient.BuildUpd
 	if hasRunningBuilds || len(nextBuilds) == 0 {
 		return nil
 	}
-	now := unversioned.Now()
 	for _, build := range nextBuilds {
-		build.Status.StartTimestamp = &now
+		if _, ok := build.Annotations[buildapi.BuildAcceptedAnnotation]; ok {
+			continue
+		}
+		build.Annotations[buildapi.BuildAcceptedAnnotation] = "true"
 		err := wait.Poll(500*time.Millisecond, 5*time.Second, func() (bool, error) {
 			err := updater.Update(build.Namespace, build)
 			if err != nil && errors.IsConflict(err) {

pkg/build/controller/policy/policy_test.go

@@ -112,12 +112,12 @@ func TestHandleCompleteSerial(t *testing.T) {
 		t.Errorf("unexpected error: %v", err)
 	}
 
-	if resultBuilds.Items[1].Status.StartTimestamp == nil {
-		t.Errorf("build-2 should have Status.StartTimestamp set to trigger it")
+	if _, ok := resultBuilds.Items[1].Annotations[buildapi.BuildAcceptedAnnotation]; !ok {
+		t.Errorf("build-2 should have Annotation %s set to trigger it", buildapi.BuildAcceptedAnnotation)
 	}
 
-	if resultBuilds.Items[2].Status.StartTimestamp != nil {
-		t.Errorf("build-3 should not have Status.StartTimestamp set")
+	if _, ok := resultBuilds.Items[2].Annotations[buildapi.BuildAcceptedAnnotation]; ok {
+		t.Errorf("build-3 should not have Annotation %s set", buildapi.BuildAcceptedAnnotation)
 	}
 }
 
@@ -139,11 +139,11 @@ func TestHandleCompleteParallel(t *testing.T) {
 		t.Errorf("unexpected error: %v", err)
 	}
 
-	if resultBuilds.Items[1].Status.StartTimestamp == nil {
-		t.Errorf("build-2 should have Status.StartTimestamp set to trigger it")
+	if _, ok := resultBuilds.Items[1].Annotations[buildapi.BuildAcceptedAnnotation]; !ok {
+		t.Errorf("build-2 should have Annotation %s set to trigger it", buildapi.BuildAcceptedAnnotation)
 	}
 
-	if resultBuilds.Items[2].Status.StartTimestamp == nil {
-		t.Errorf("build-3 should have Status.StartTimestamp set to trigger it")
+	if _, ok := resultBuilds.Items[2].Annotations[buildapi.BuildAcceptedAnnotation]; !ok {
+		t.Errorf("build-3 should have Annotation %s set to trigger it", buildapi.BuildAcceptedAnnotation)
 	}
 }