Merge pull request #12842 from bparees/fatal_retries

Merged by openshift-bot

Author: OpenShift Bot

pkg/build/controller/config_controller.go

@@ -22,13 +22,13 @@ type ConfigControllerFatalError struct {
 }
 
 // Error returns the error string for this fatal error
-func (e ConfigControllerFatalError) Error() string {
+func (e *ConfigControllerFatalError) Error() string {
 	return fmt.Sprintf("fatal error processing BuildConfig: %s", e.Reason)
 }
 
 // IsFatal returns true if err is a fatal error
 func IsFatal(err error) bool {
-	_, isFatal := err.(ConfigControllerFatalError)
+	_, isFatal := err.(*ConfigControllerFatalError)
 	return isFatal
 }
 
@@ -73,7 +73,10 @@ func (c *BuildConfigController) HandleBuildConfig(bc *buildapi.BuildConfig) erro
 		if kerrors.IsConflict(err) {
 			instantiateErr = fmt.Errorf("unable to instantiate Build for BuildConfig %s/%s due to a conflicting update: %v", bc.Namespace, bc.Name, err)
 			utilruntime.HandleError(instantiateErr)
-		} else if buildgenerator.IsFatal(err) || kerrors.IsNotFound(err) || kerrors.IsBadRequest(err) {
+		} else if buildgenerator.IsFatal(err) || kerrors.IsNotFound(err) || kerrors.IsBadRequest(err) || kerrors.IsForbidden(err) {
+			instantiateErr = fmt.Errorf("gave up on Build for BuildConfig %s/%s due to fatal error: %v", bc.Namespace, bc.Name, err)
+			utilruntime.HandleError(instantiateErr)
+			c.Recorder.Event(bc, kapi.EventTypeWarning, "BuildConfigInstantiateFailed", instantiateErr.Error())
 			return &ConfigControllerFatalError{err.Error()}
 		} else {
 			instantiateErr = fmt.Errorf("error instantiating Build from BuildConfig %s/%s: %v", bc.Namespace, bc.Name, err)

pkg/build/controller/controller.go

@@ -188,7 +188,7 @@ func (bc *BuildController) nextBuildPhase(build *buildapi.Build) error {
 		build.Status.Reason = buildapi.StatusReasonCannotCreateBuildPodSpec
 		build.Status.Message = buildapi.StatusMessageCannotCreateBuildPodSpec
 		if strategy.IsFatal(err) {
-			return strategy.FatalError(fmt.Sprintf("failed to create a build pod spec for build %s/%s: %v", build.Namespace, build.Name, err))
+			return &strategy.FatalError{fmt.Sprintf("failed to create a build pod spec for build %s/%s: %v", build.Namespace, build.Name, err)}
 		}
 		return fmt.Errorf("failed to create a build pod spec for build %s/%s: %v", build.Namespace, build.Name, err)
 	}

pkg/build/controller/strategy/custom.go

@@ -31,7 +31,7 @@ func (bs *CustomBuildStrategy) CreateBuildPod(build *buildapi.Build) (*kapi.Pod,
 	if len(strategy.BuildAPIVersion) != 0 {
 		gv, err := unversioned.ParseGroupVersion(strategy.BuildAPIVersion)
 		if err != nil {
-			return nil, FatalError(fmt.Sprintf("failed to parse buildAPIVersion specified in custom build strategy (%q): %v", strategy.BuildAPIVersion, err))
+			return nil, &FatalError{fmt.Sprintf("failed to parse buildAPIVersion specified in custom build strategy (%q): %v", strategy.BuildAPIVersion, err)}
 		}
 		codec = kapi.Codecs.LegacyCodec(gv)
 	}

pkg/build/controller/strategy/util.go

@@ -28,16 +28,19 @@ const (
 var whitelistEnvVarNames = []string{"BUILD_LOGLEVEL", "GIT_SSL_NO_VERIFY"}
 
 // FatalError is an error which can't be retried.
-type FatalError string
+type FatalError struct {
+	// Reason the fatal error occurred
+	Reason string
+}
 
 // Error implements the error interface.
-func (e FatalError) Error() string {
-	return string(e)
+func (e *FatalError) Error() string {
+	return fmt.Sprintf("fatal error: %s", e.Reason)
 }
 
 // IsFatal returns true if the error is fatal
 func IsFatal(err error) bool {
-	_, isFatal := err.(FatalError)
+	_, isFatal := err.(*FatalError)
 	return isFatal
 }
 

pkg/build/generator/generator.go

@@ -32,13 +32,13 @@ type GeneratorFatalError struct {
 }
 
 // Error returns the error string for this fatal error
-func (e GeneratorFatalError) Error() string {
+func (e *GeneratorFatalError) Error() string {
 	return fmt.Sprintf("fatal error generating Build from BuildConfig: %s", e.Reason)
 }
 
 // IsFatal returns true if err is a fatal error
 func IsFatal(err error) bool {
-	_, isFatal := err.(GeneratorFatalError)
+	_, isFatal := err.(*GeneratorFatalError)
 	return isFatal
 }