Merge pull request buildpacks-community#1226 from evankanderson/unconditional-logic

Followup on 1221 - further condition improvements

Author: tomkennedy513

pkg/reconciler/image/image_test.go

@@ -2387,7 +2387,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
 												Type:    corev1alpha1.ConditionReady,
 												Status:  corev1.ConditionFalse,
 												Reason:  image.BuildFailedReason,
-												Message: failureMessage,
+												Message: fmt.Sprintf("Build %s failed: %s", failedBuild.Name, failureMessage),
 											},
 											{
 												Type:   buildapi.ConditionBuilderReady,
@@ -2409,7 +2409,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
 				it("deletes a failed build if more than the limit", func() {
 					imageWithBuilder.Spec.FailedBuildHistoryLimit = limit(4)
 					imageWithBuilder.Status.LatestBuildRef = "image-name-build-5"
-					imageWithBuilder.Status.Conditions = conditionNotReady()
+					imageWithBuilder.Status.Conditions = conditionNotReady(imageWithBuilder.Status.LatestBuildRef)
 					imageWithBuilder.Status.BuildCounter = 5
 					sourceResolver := resolvedSourceResolver(imageWithBuilder)
 
@@ -2647,12 +2647,13 @@ func conditionReady() corev1alpha1.Conditions {
 	}
 }
 
-func conditionNotReady() corev1alpha1.Conditions {
+func conditionNotReady(failedBuild string) corev1alpha1.Conditions {
 	return corev1alpha1.Conditions{
 		{
-			Type:   corev1alpha1.ConditionReady,
-			Status: corev1.ConditionFalse,
-			Reason: image.BuildFailedReason,
+			Type:    corev1alpha1.ConditionReady,
+			Status:  corev1.ConditionFalse,
+			Reason:  image.BuildFailedReason,
+			Message: fmt.Sprintf("Build %s failed: ", failedBuild),
 		},
 		{
 			Type:   buildapi.ConditionBuilderReady,

pkg/reconciler/image/reconcile_build.go

@@ -16,6 +16,7 @@ import (
 const (
 	BuildRunningReason     = "BuildRunning"
 	ResolverNotReadyReason = "ResolverNotReady"
+	UnknownStateReason     = "UnknownState"
 	BuildFailedReason      = "BuildFailed"
 	UpToDateReason         = "UpToDate"
 )
@@ -76,53 +77,36 @@ func (c *Reconciler) reconcileBuild(ctx context.Context, image *buildapi.Image,
 }
 
 func noScheduledBuild(buildNeeded corev1.ConditionStatus, builder buildapi.BuilderResource, build *buildapi.Build, sourceResolver *buildapi.SourceResolver) corev1alpha1.Conditions {
-	if !builder.Ready() {
-		return corev1alpha1.Conditions{
-			{
-				Type:               corev1alpha1.ConditionReady,
-				Status:             corev1.ConditionFalse,
-				Reason:             buildapi.BuilderNotReady,
-				Message:            builderError(builder),
-				LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
-			},
-			builderCondition(builder),
-		}
-	}
-	if buildNeeded == corev1.ConditionUnknown {
-		message := "Build status unknown"
-		if !sourceResolver.Ready() {
-			message = fmt.Sprintf("SourceResolver %s is not ready", sourceResolver.GetName())
-		}
-		return corev1alpha1.Conditions{
-			{
-				Type:               corev1alpha1.ConditionReady,
-				Status:             corev1.ConditionUnknown,
-				Reason:             ResolverNotReadyReason,
-				Message:            message,
-				LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
-			},
-			builderCondition(builder),
-		}
+	ready := corev1alpha1.Condition{
+		Type:               corev1alpha1.ConditionReady,
+		Status:             corev1.ConditionUnknown,
+		LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
 	}
 
-	buildReason := UpToDateReason
-	buildMessage := "Last build succeeded"
-
-	if !build.Status.GetCondition(corev1alpha1.ConditionSucceeded).IsTrue() {
-		buildReason = BuildFailedReason
-		buildMessage = "Last build did not succeed"
+	switch {
+	case !builder.Ready():
+		ready.Status = corev1.ConditionFalse
+		ready.Reason = buildapi.BuilderNotReady
+		ready.Message = builderError(builder)
+	case buildNeeded == corev1.ConditionUnknown && !sourceResolver.Ready():
+		ready.Status = corev1.ConditionUnknown
+		ready.Reason = ResolverNotReadyReason
+		ready.Message = fmt.Sprintf("SourceResolver %s is not ready", sourceResolver.GetName())
+	case buildNeeded == corev1.ConditionUnknown && sourceResolver.Ready():
+		ready.Status = corev1.ConditionUnknown
+		ready.Reason = UnknownStateReason
+		ready.Message = "Build status unknown"
+	case build.Status.GetCondition(corev1alpha1.ConditionSucceeded).IsTrue():
+		ready.Status = corev1.ConditionTrue
+		ready.Reason = UpToDateReason
+		ready.Message = defaultMessageIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded), "Last build succeeded")
+	default:
+		ready.Status = unknownStatusIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded))
+		ready.Reason = BuildFailedReason
+		ready.Message = fmt.Sprintf("Build %s failed: %s", build.Name, defaultMessageIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded), "unknown error"))
 	}
 
-	return corev1alpha1.Conditions{
-		{
-			Type:               corev1alpha1.ConditionReady,
-			Status:             unknownStatusIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded)),
-			Reason:             buildReason,
-			Message:            defaultMessageIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded), buildMessage),
-			LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
-		},
-		builderCondition(builder),
-	}
+	return corev1alpha1.Conditions{ready, builderCondition(builder)}
 
 }