From 3cc061641c07aa26dba068dcb4bebe4dbf912bb4 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Fri, 26 May 2023 11:49:46 -0700 Subject: [PATCH] Use case per suggestions in 1221. Also improve failed build error message Signed-off-by: Evan Anderson --- pkg/reconciler/image/image_test.go | 13 ++--- pkg/reconciler/image/reconcile_build.go | 70 ++++++++++--------------- 2 files changed, 34 insertions(+), 49 deletions(-) diff --git a/pkg/reconciler/image/image_test.go b/pkg/reconciler/image/image_test.go index 1e514a438..cb4f78111 100644 --- a/pkg/reconciler/image/image_test.go +++ b/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, diff --git a/pkg/reconciler/image/reconcile_build.go b/pkg/reconciler/image/reconcile_build.go index a41e51e4e..8c72756f6 100644 --- a/pkg/reconciler/image/reconcile_build.go +++ b/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)} }