@@ -49,10 +49,6 @@ func (bc *BuildController) HandleBuild(build *buildapi.Build) error {

 	}

 	if err := bc.nextBuildStatus(build); err != nil {

-		// TODO: all build errors should be retried, and build error should not be a permanent status change.

-		// Instead, we should requeue this build request using the same backoff logic as the scheduler.

-		//build.Status = buildapi.BuildStatusError

-		//build.Message = err.Error()

 		return fmt.Errorf("Build failed with error %s/%s: %v", build.Namespace, build.Name, err)

 	}

@@ -187,12 +183,12 @@ func (bc *BuildPodController) HandlePod(pod *kapi.Pod) error {

 		glog.V(4).Infof("Updating build %s status %s -> %s", build.Name, build.Status, nextStatus)

 		build.Status = nextStatus

 		if build.Status == buildapi.BuildStatusComplete || build.Status == buildapi.BuildStatusFailed || build.Status == buildapi.BuildStatusCancelled {

-			dummy := util.Now()

-			build.CompletionTimestamp = &dummy

+			now := util.Now()

+			build.CompletionTimestamp = &now

 		}

 		if build.Status == buildapi.BuildStatusRunning {

-			dummy := util.Now()

-			build.StartTimestamp = &dummy

+			now := util.Now()

+			build.StartTimestamp = &now

 		}

 		if err := bc.BuildUpdater.Update(build.Namespace, build); err != nil {

 			return fmt.Errorf("Failed to update build %s: %v", build.Name, err)

@@ -214,8 +210,8 @@ func (bc *BuildPodController) CancelBuild(build *buildapi.Build, pod *kapi.Pod)

 	}

 	build.Status = buildapi.BuildStatusCancelled

-	dummy := util.Now()

-	build.CompletionTimestamp = &dummy

+	now := util.Now()

+	build.CompletionTimestamp = &now

 	if err := bc.BuildUpdater.Update(build.Namespace, build); err != nil {

 		return err

 	}

@@ -7,6 +7,7 @@ import (

 	"github.com/golang/glog"

 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"

+	kerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"

 	kclient "github.com/GoogleCloudPlatform/kubernetes/pkg/client"

 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache"

 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/record"

@@ -26,11 +27,27 @@ import (

 	imageapi "github.com/openshift/origin/pkg/image/api"

 )

-// limitedLogAndRetry stops retrying after 60 attempts.  Given the throttler configuration,

-// this means this event has been retried over a period of at least 50 seconds.

-func limitedLogAndRetry(obj interface{}, err error, count int) bool {

-	kutil.HandleError(err)

-	return count < 60

+const maxRetries = 60

+

+// limitedLogAndRetry stops retrying after maxTimeout, failing the build.

+func limitedLogAndRetry(buildupdater buildclient.BuildUpdater, maxTimeout time.Duration) controller.RetryFunc {

+	return func(obj interface{}, err error, retries controller.Retry) bool {

+		kutil.HandleError(err)

+		if time.Since(retries.StartTimestamp.Time) < maxTimeout {

+			return true

+		}

+		build := obj.(*buildapi.Build)

+		build.Status = buildapi.BuildStatusFailed

+		build.Message = err.Error()

+		now := kutil.Now()

+		build.CompletionTimestamp = &now

+		glog.V(3).Infof("Giving up retrying build %s/%s: %v", build.Namespace, build.Name, err)

+		if err := buildupdater.Update(build.Namespace, build); err != nil {

+			// retry update, but only on error other than NotFound

+			return !kerrors.IsNotFound(err)

+		}

+		return false

+	}

 }

 // BuildControllerFactory constructs BuildController objects

@@ -67,8 +84,12 @@ func (factory *BuildControllerFactory) Create() controller.RunnableController {

 	}

 	return &controller.RetryController{

-		Queue:        queue,

-		RetryManager: controller.NewQueueRetryManager(queue, cache.MetaNamespaceKeyFunc, limitedLogAndRetry, kutil.NewTokenBucketRateLimiter(1, 10)),

+		Queue: queue,

+		RetryManager: controller.NewQueueRetryManager(

+			queue,

+			cache.MetaNamespaceKeyFunc,

+			limitedLogAndRetry(factory.BuildUpdater, 30*time.Minute),

+			kutil.NewTokenBucketRateLimiter(1, 10)),

 		Handle: func(obj interface{}) error {

 			build := obj.(*buildapi.Build)

 			return buildController.HandleBuild(build)

@@ -111,8 +132,15 @@ func (factory *BuildPodControllerFactory) Create() controller.RunnableController

 	}

 	return &controller.RetryController{

-		Queue:        queue,

-		RetryManager: controller.NewQueueRetryManager(queue, cache.MetaNamespaceKeyFunc, limitedLogAndRetry, kutil.NewTokenBucketRateLimiter(1, 10)),

+		Queue: queue,

+		RetryManager: controller.NewQueueRetryManager(

+			queue,

+			cache.MetaNamespaceKeyFunc,

+			func(obj interface{}, err error, retries controller.Retry) bool {

+				kutil.HandleError(err)

+				return retries.Count < maxRetries

+			},

+			kutil.NewTokenBucketRateLimiter(1, 10)),

 		Handle: func(obj interface{}) error {

 			pod := obj.(*kapi.Pod)

 			return buildPodController.HandlePod(pod)

@@ -151,15 +179,12 @@ func (factory *ImageChangeControllerFactory) Create() controller.RunnableControl

 		RetryManager: controller.NewQueueRetryManager(

 			queue,

 			cache.MetaNamespaceKeyFunc,

-			func(obj interface{}, err error, count int) bool {

+			func(obj interface{}, err error, retries controller.Retry) bool {

 				kutil.HandleError(err)

 				if _, isFatal := err.(buildcontroller.ImageChangeControllerFatalError); isFatal {

 					return false

 				}

-				if count > 60 {

-					return false

-				}

-				return true

+				return retries.Count < maxRetries

 			},

 			kutil.NewTokenBucketRateLimiter(1, 10),

 		),

new file mode 100644

@@ -0,0 +1,58 @@

+package factory

+

+import (

+	"errors"

+	"strings"

+	"testing"

+	"time"

+

+	kutil "github.com/GoogleCloudPlatform/kubernetes/pkg/util"

+	buildapi "github.com/openshift/origin/pkg/build/api"

+	controller "github.com/openshift/origin/pkg/controller"

+)

+

+type buildUpdater struct {

+	Build *buildapi.Build

+}

+

+func (b *buildUpdater) Update(namespace string, build *buildapi.Build) error {

+	b.Build = build

+	return nil

+}

+

+func TestLimitedLogAndRetryFinish(t *testing.T) {

+	updater := &buildUpdater{}

+	err := errors.New("funky error")

+

+	now := kutil.Now()

+	retry := controller.Retry{0, kutil.Date(now.Year(), now.Month(), now.Day(), now.Hour(), now.Minute()-31, now.Second(), now.Nanosecond(), now.Location())}

+	if limitedLogAndRetry(updater, 30*time.Minute)(&buildapi.Build{Status: buildapi.BuildStatusNew}, err, retry) {

+		t.Error("Expected no more retries after reaching timeout!")

+	}

+	if updater.Build == nil {

+		t.Fatal("BuildUpdater wasn't called!")

+	}

+	if updater.Build.Status != buildapi.BuildStatusFailed {

+		t.Errorf("Expected status %s, got %s!", buildapi.BuildStatusFailed, updater.Build.Status)

+	}

+	if !strings.Contains(updater.Build.Message, err.Error()) {

+		t.Errorf("Expected message to contain %v, got %s!", err, updater.Build.Status)

+	}

+	if updater.Build.CompletionTimestamp == nil {

+		t.Error("Expected CompletionTimestamp to be set!")

+	}

+}

+

+func TestLimitedLogAndRetryProcessing(t *testing.T) {

+	updater := &buildUpdater{}

+	err := errors.New("funky error")

+

+	now := kutil.Now()

+	retry := controller.Retry{0, kutil.Date(now.Year(), now.Month(), now.Day(), now.Hour(), now.Minute()-10, now.Second(), now.Nanosecond(), now.Location())}

+	if !limitedLogAndRetry(updater, 30*time.Minute)(&buildapi.Build{Status: buildapi.BuildStatusNew}, err, retry) {

+		t.Error("Expected more retries!")

+	}

+	if updater.Build != nil {

+		t.Fatal("BuildUpdater shouldn't be called!")

+	}

+}

@@ -113,7 +113,11 @@ func (d *BuildDescriber) Describe(namespace, name string) (string, error) {

 		if build.Config != nil {

 			formatString(out, "Build Config", build.Config.Name)

 		}

-		formatString(out, "Status", bold(build.Status))

+		status := bold(build.Status)

+		if build.Status == buildapi.BuildStatusFailed {

+			status += " (" + build.Message + ")"

+		}

+		formatString(out, "Status", status)

 		if build.StartTimestamp != nil {

 			formatString(out, "Started", build.StartTimestamp.Time)

 		}

@@ -135,7 +139,10 @@ func (d *BuildDescriber) Describe(namespace, name string) (string, error) {

 func describeBuildDuration(build *buildapi.Build) string {

 	t := util.Now().Rfc3339Copy()

-	if build.StartTimestamp == nil && build.Status == buildapi.BuildStatusCancelled {

+	if build.StartTimestamp == nil &&

+		(build.Status == buildapi.BuildStatusCancelled ||

+			build.Status == buildapi.BuildStatusFailed ||

+			build.Status == buildapi.BuildStatusError) {

 		// time a build waited for its pod before ultimately being canceled before that pod was created

 		return fmt.Sprintf("waited for %s", build.CompletionTimestamp.Rfc3339Copy().Time.Sub(build.CreationTimestamp.Rfc3339Copy().Time))

 	} else if build.StartTimestamp == nil && build.Status != buildapi.BuildStatusCancelled {

@@ -222,7 +222,7 @@ func TestDescribeBuildDuration(t *testing.T) {

 			},

 			"1m0s",

 		},

-		{ // 5 - build cancelled before running, start time wasn't set yet

+		{ // 6 - build cancelled before running, start time wasn't set yet

 			&buildapi.Build{

 				ObjectMeta:          kapi.ObjectMeta{CreationTimestamp: creation},

 				CompletionTimestamp: &completion,

@@ -231,7 +231,7 @@ func TestDescribeBuildDuration(t *testing.T) {

 			},

 			"waited for 2m0s",

 		},

-		{ // 5 - build cancelled while running, start time is set already

+		{ // 7 - build cancelled while running, start time is set already

 			&buildapi.Build{

 				ObjectMeta:          kapi.ObjectMeta{CreationTimestamp: creation},

 				StartTimestamp:      &start,

@@ -241,6 +241,24 @@ func TestDescribeBuildDuration(t *testing.T) {

 			},

 			"1m0s",

 		},

+		{ // 8 - build failed before running, start time wasn't set yet

+			&buildapi.Build{

+				ObjectMeta:          kapi.ObjectMeta{CreationTimestamp: creation},

+				CompletionTimestamp: &completion,

+				Status:              buildapi.BuildStatusFailed,

+				Duration:            duration,

+			},

+			"waited for 2m0s",

+		},

+		{ // 9 - build error before running, start time wasn't set yet

+			&buildapi.Build{

+				ObjectMeta:          kapi.ObjectMeta{CreationTimestamp: creation},

+				CompletionTimestamp: &completion,

+				Status:              buildapi.BuildStatusError,

+				Duration:            duration,

+			},

+			"waited for 2m0s",

+		},

 	}

 	for i, tc := range tests {

@@ -69,7 +69,7 @@ type RetryManager interface {

 // RetryFunc should return true if the given object and error should be retried after

 // the provided number of times.

-type RetryFunc func(obj interface{}, err error, retries int) bool

+type RetryFunc func(obj interface{}, err error, retries Retry) bool

 // QueueRetryManager retries a resource by re-queueing it into a ReQueue as long as

 // retryFunc returns true.

@@ -83,14 +83,23 @@ type QueueRetryManager struct {

 	// retryFunc returns true if the resource and error returned should be retried.

 	retryFunc RetryFunc

-	// retries maps resources to their current retry count.

-	retries map[string]int

+	// retries maps resources to their current retry

+	retries map[string]Retry

 	// limits how fast retries can be enqueued to ensure you can't tight

 	// loop on retries.

 	limiter kutil.RateLimiter

 }

+// Retry describes provides additional information regarding retries.

+type Retry struct {

+	// Count is the number of retries

+	Count int

+

+	// StartTimestamp is retry start timestamp

+	StartTimestamp kutil.Time

+}

+

 // ReQueue is a queue that allows an object to be requeued

 type ReQueue interface {

 	Queue

@@ -103,7 +112,7 @@ func NewQueueRetryManager(queue ReQueue, keyFn kcache.KeyFunc, retryFn RetryFunc

 		queue:     queue,

 		keyFunc:   keyFn,

 		retryFunc: retryFn,

-		retries:   make(map[string]int),

+		retries:   make(map[string]Retry),

 		limiter:   limiter,

 	}

 }

@@ -115,7 +124,7 @@ func (r *QueueRetryManager) Retry(resource interface{}, err error) {

 	id, _ := r.keyFunc(resource)

 	if _, exists := r.retries[id]; !exists {

-		r.retries[id] = 0

+		r.retries[id] = Retry{0, kutil.Now()}

 	}

 	tries := r.retries[id]

@@ -124,7 +133,8 @@ func (r *QueueRetryManager) Retry(resource interface{}, err error) {

 		// It's important to use AddIfNotPresent to prevent overwriting newer

 		// state in the queue which may have arrived asynchronously.

 		r.queue.AddIfNotPresent(resource)

-		r.retries[id] = tries + 1

+		tries.Count = tries.Count + 1

+		r.retries[id] = tries

 	} else {

 		r.Forget(resource)

 	}

@@ -80,13 +80,10 @@ func TestQueueRetryManager_retries(t *testing.T) {

 		keyFunc: func(obj interface{}) (string, error) {

 			return obj.(testObj).id, nil

 		},

-		retryFunc: func(obj interface{}, err error, count int) bool {

-			if count > 4 {

-				return false

-			}

-			return true

+		retryFunc: func(obj interface{}, err error, r Retry) bool {

+			return r.Count < 5 && !r.StartTimestamp.IsZero()

 		},

-		retries: make(map[string]int),

+		retries: make(map[string]Retry),

 		limiter: kutil.NewTokenBucketRateLimiter(1000, 1000),

 	}

@@ -131,7 +128,7 @@ func TestRetryController_realFifoEventOrdering(t *testing.T) {

 	controller := &RetryController{

 		Queue:        fifo,

-		RetryManager: NewQueueRetryManager(fifo, keyFunc, func(_ interface{}, _ error, _ int) bool { return true }, kutil.NewTokenBucketRateLimiter(1000, 10)),

+		RetryManager: NewQueueRetryManager(fifo, keyFunc, func(_ interface{}, _ error, _ Retry) bool { return true }, kutil.NewTokenBucketRateLimiter(1000, 10)),

 		Handle: func(obj interface{}) error {

 			if e, a := 1, obj.(testObj).value; e != a {

 				t.Fatalf("expected to handle test value %d, got %d", e, a)

@@ -170,11 +167,8 @@ func TestRetryController_ratelimit(t *testing.T) {

 	limiter := &mockLimiter{}

 	retryManager := NewQueueRetryManager(fifo,

 		keyFunc,

-		func(_ interface{}, _ error, c int) bool {

-			if c < 15 {

-				return true

-			}

-			return false

+		func(_ interface{}, _ error, r Retry) bool {

+			return r.Count < 15

 		},

 		limiter,

 	)

@@ -64,12 +64,12 @@ func (factory *DeploymentConfigChangeControllerFactory) Create() controller.Runn

 		RetryManager: controller.NewQueueRetryManager(

 			queue,

 			cache.MetaNamespaceKeyFunc,

-			func(obj interface{}, err error, count int) bool {

+			func(obj interface{}, err error, retries controller.Retry) bool {

 				kutil.HandleError(err)

 				if _, isFatal := err.(fatalError); isFatal {

 					return false

 				}

-				if count > 0 {

+				if retries.Count > 0 {

 					return false

 				}

 				return true

@@ -72,7 +72,7 @@ func (factory *DeployerPodControllerFactory) Create() controller.RunnableControl

 		RetryManager: controller.NewQueueRetryManager(

 			podQueue,

 			cache.MetaNamespaceKeyFunc,

-			func(obj interface{}, err error, count int) bool { return count < 1 },

+			func(obj interface{}, err error, retries controller.Retry) bool { return retries.Count < 1 },

 			kutil.NewTokenBucketRateLimiter(1, 10),

 		),

 		Handle: func(obj interface{}) error {

@@ -84,12 +84,12 @@ func (factory *DeploymentControllerFactory) Create() controller.RunnableControll

 		RetryManager: controller.NewQueueRetryManager(

 			deploymentQueue,

 			cache.MetaNamespaceKeyFunc,

-			func(obj interface{}, err error, count int) bool {

+			func(obj interface{}, err error, retries controller.Retry) bool {

 				if _, isFatal := err.(fatalError); isFatal {

 					kutil.HandleError(err)

 					return false

 				}

-				if count > 1 {

+				if retries.Count > 1 {

 					return false

 				}

 				return true

@@ -61,7 +61,7 @@ func (factory *DeploymentCancellationControllerFactory) Create() controller.Runn

 		RetryManager: controller.NewQueueRetryManager(

 			deploymentQueue,

 			cache.MetaNamespaceKeyFunc,

-			func(obj interface{}, err error, count int) bool { return count < 1 },

+			func(obj interface{}, err error, retries controller.Retry) bool { return retries.Count < 1 },

 			kutil.NewTokenBucketRateLimiter(1, 10),

 		),

 		Handle: func(obj interface{}) error {

@@ -74,7 +74,7 @@ func (factory *DeploymentConfigControllerFactory) Create() controller.RunnableCo

 		RetryManager: controller.NewQueueRetryManager(

 			queue,

 			cache.MetaNamespaceKeyFunc,

-			func(obj interface{}, err error, count int) bool {

+			func(obj interface{}, err error, retries controller.Retry) bool {

 				kutil.HandleError(err)

 				// no retries for a fatal error

 				if _, isFatal := err.(fatalError); isFatal {

@@ -85,7 +85,7 @@ func (factory *DeploymentConfigControllerFactory) Create() controller.RunnableCo

 					return true

 				}

 				// no retries for anything else

-				if count > 0 {

+				if retries.Count > 0 {

 					return false

 				}

 				return true

@@ -73,12 +73,12 @@ func (factory *ImageChangeControllerFactory) Create() controller.RunnableControl

 		RetryManager: controller.NewQueueRetryManager(

 			queue,

 			cache.MetaNamespaceKeyFunc,

-			func(obj interface{}, err error, count int) bool {

+			func(obj interface{}, err error, retries controller.Retry) bool {

 				kutil.HandleError(err)

 				if _, isFatal := err.(fatalError); isFatal {

 					return false

 				}

-				if count > 0 {

+				if retries.Count > 0 {

 					return false

 				}

 				return true

@@ -47,9 +47,9 @@ func (f *ImportControllerFactory) Create() controller.RunnableController {

 		RetryManager: controller.NewQueueRetryManager(

 			q,

 			cache.MetaNamespaceKeyFunc,

-			func(obj interface{}, err error, count int) bool {

+			func(obj interface{}, err error, retries controller.Retry) bool {

 				util.HandleError(err)

-				return count < 5

+				return retries.Count < 5

 			},

 			kutil.NewTokenBucketRateLimiter(1, 10),

 		),

@@ -46,12 +46,12 @@ func (factory *NamespaceControllerFactory) Create() controller.RunnableControlle

 		RetryManager: controller.NewQueueRetryManager(

 			queue,

 			cache.MetaNamespaceKeyFunc,

-			func(obj interface{}, err error, count int) bool {

+			func(obj interface{}, err error, retries controller.Retry) bool {

 				kutil.HandleError(err)

 				if _, isFatal := err.(fatalError); isFatal {

 					return false

 				}

-				if count > 0 {

+				if retries.Count > 0 {

 					return false

 				}

 				return true