Changes error messages to follow Human Readable Structure convention

improves "lifecycle image has not been loaded" error message to include configmap name and location

Clarify error if clusterstack is not ready

Fix failing unit test

pkg/apis/build/v1alpha2/builder_resource.go

@@ -4,6 +4,7 @@ import corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1"
 
 type BuilderResource interface {
 	GetName() string
+	GetNamespace() string
 	BuildBuilderSpec() corev1alpha1.BuildBuilderSpec
 	Ready() bool
 	BuildpackMetadata() corev1alpha1.BuildpackMetadataList

pkg/apis/build/v1alpha2/image_builds_test.go

@@ -378,6 +378,7 @@ type TestBuilderResource struct {
 	LatestImage      string
 	LatestRunImage   string
 	Name             string
+	Namespace        string
 }
 
 func (t TestBuilderResource) ConditionReadyMessage() string {
@@ -410,3 +411,7 @@ func (t TestBuilderResource) GetName() string {
 func (t TestBuilderResource) GetKind() string {
 	return t.Kind
 }
+
+func (t TestBuilderResource) GetNamespace() string {
+	return t.Namespace
+}

pkg/apis/build/v1alpha2/image_lifecycle.go

@@ -21,7 +21,7 @@ func (im *Image) BuilderNotFound() corev1alpha1.Conditions {
 			Type:               corev1alpha1.ConditionReady,
 			Status:             corev1.ConditionFalse,
 			Reason:             BuilderNotFound,
-			Message:            fmt.Sprintf("Unable to find builder %s.", im.Spec.Builder.Name),
+			Message:            fmt.Sprintf("Error: Unable to find builder '%s' in namespace '%s'.", im.Spec.Builder.Name, im.Spec.Builder.Namespace),
 			LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
 		},
 	}

pkg/apis/build/v1alpha2/image_validation.go

@@ -72,7 +72,7 @@ func (i *Image) validateName(imageName string) *apis.FieldError {
 	msgs := validation.IsValidLabelValue(imageName)
 	if len(msgs) > 0 {
 		return &apis.FieldError{
-			Message: fmt.Sprintf("invalid image name: %s, name must be a a valid label", imageName),
+			Message: fmt.Sprintf("invalid image name: %s, name must be a valid label", imageName),
 			Paths:   []string{""},
 			Details: strings.Join(msgs, ","),
 		}

pkg/apis/build/v1alpha2/image_validation_test.go

@@ -282,12 +282,12 @@ func testImageValidation(t *testing.T, when spec.G, it spec.S) {
 
 		it("image name is too long", func() {
 			image.ObjectMeta.Name = "this-image-name-that-is-too-long-some-sha-that-is-long-82cb521d636b282340378d80a6307a08e3d4a4c4"
-			assertValidationError(image, ctx, errors.New("invalid image name: this-image-name-that-is-too-long-some-sha-that-is-long-82cb521d636b282340378d80a6307a08e3d4a4c4, name must be a a valid label: metadata.name\nmust be no more than 63 characters"))
+			assertValidationError(image, ctx, errors.New("invalid image name: this-image-name-that-is-too-long-some-sha-that-is-long-82cb521d636b282340378d80a6307a08e3d4a4c4, name must be a valid label: metadata.name\nmust be no more than 63 characters"))
 		})
 
 		it("invalid image name format", func() {
 			image.ObjectMeta.Name = "@NOT!!!VALID!!!"
-			errMsg := "invalid image name: @NOT!!!VALID!!!, name must be a a valid label: metadata.name\na valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')"
+			errMsg := "invalid image name: @NOT!!!VALID!!!, name must be a valid label: metadata.name\na valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')"
 
 			assertValidationError(image, ctx, errors.New(errMsg))
 		})

pkg/config/lifecycle_provider.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"context"
+	"knative.dev/pkg/system"
 	"sync/atomic"
 
 	"github.com/google/go-containerregistry/pkg/authn"
@@ -167,7 +168,7 @@ func (l *LifecycleProvider) isLifecycleLoaded() bool {
 func (l *LifecycleProvider) lifecycle() (*lifecycle, error) {
 	d, ok := l.lifecycleData.Load().(configmapRead)
 	if !ok {
-		return nil, errors.New("lifecycle image has not been loaded")
+		return nil, errors.Errorf("Error: lifecycle image has not been loaded from ConfigMap '%s' in namespace '%s'", LifecycleConfigName, system.Namespace())
 	}
 
 	return d.lifecycle, d.err

pkg/duckbuilder/duck_builder.go

@@ -20,6 +20,10 @@ func (b *DuckBuilder) GetName() string {
 	return b.Name
 }
 
+func (b *DuckBuilder) GetNamespace() string {
+	return b.Namespace
+}
+
 func (b *DuckBuilder) GetKind() string {
 	return b.Kind
 }

pkg/reconciler/builder/builder.go

@@ -200,7 +200,7 @@ func (c *Reconciler) reconcileBuilder(ctx context.Context, builder *buildapi.Bui
 	}
 
 	if !clusterStack.Status.GetCondition(corev1alpha1.ConditionReady).IsTrue() {
-		return buildapi.BuilderRecord{}, errors.Errorf("stack %s is not ready", clusterStack.Name)
+		return buildapi.BuilderRecord{}, errors.Errorf("Error: clusterstack '%s' is not ready", clusterStack.Name)
 	}
 
 	builderKeychain, err := c.KeychainFactory.KeychainForSecretRef(ctx, registry.SecretRef{

pkg/reconciler/builder/builder_test.go

@@ -446,7 +446,7 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) {
 										{
 											Type:    corev1alpha1.ConditionReady,
 											Status:  corev1.ConditionFalse,
-											Message: "stack some-stack is not ready",
+											Message: "Error: clusterstack 'some-stack' is not ready",
 										},
 									},
 								},

pkg/reconciler/image/build_required_test.go

@@ -57,6 +57,7 @@ func testImageBuilds(t *testing.T, when spec.G, it spec.S) {
 
 	builder := &TestBuilderResource{
 		Name:         "builder-Name",
+		Namespace:    "test-ns",
 		LatestImage:  "some/builder@sha256:builder-digest",
 		BuilderReady: true,
 		BuilderMetadata: []corev1alpha1.BuildpackMetadata{
@@ -804,6 +805,7 @@ type TestBuilderResource struct {
 	LatestImage      string
 	LatestRunImage   string
 	Name             string
+	Namespace        string
 	Kind             string
 }
 
@@ -837,3 +839,7 @@ func (t TestBuilderResource) GetKind() string {
 func (t TestBuilderResource) ConditionReadyMessage() string {
 	return ""
 }
+
+func (t TestBuilderResource) GetNamespace() string {
+	return t.Namespace
+}

pkg/reconciler/image/image_test.go

@@ -311,7 +311,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
 											Type:    corev1alpha1.ConditionReady,
 											Status:  corev1.ConditionFalse,
 											Reason:  "BuilderNotFound",
-											Message: "Unable to find builder builder-name.",
+											Message: "Error: Unable to find builder 'builder-name' in namespace ''.",
 										},
 									},
 								},
@@ -769,7 +769,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
 					WantCreates: nil,
 				})
 
-				assert.Equal(t, "SourceResolver image-name-source is not ready", imageWithBuilder.Status.GetCondition(corev1alpha1.ConditionReady).Message)
+				assert.Equal(t, "Error: SourceResolver 'image-name-source' is not ready", imageWithBuilder.Status.GetCondition(corev1alpha1.ConditionReady).Message)
 			})
 
 			it("does not schedule a build if the builder is not ready", func() {
@@ -799,13 +799,13 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
 												Type:    corev1alpha1.ConditionReady,
 												Status:  corev1.ConditionFalse,
 												Reason:  buildapi.BuilderNotReady,
-												Message: "Builder builder-name is not ready: something went wrong",
+												Message: "Error: Builder 'builder-name' is not ready in namespace 'some-namespace'; Message: something went wrong",
 											},
 											{
 												Type:    buildapi.ConditionBuilderReady,
 												Status:  corev1.ConditionFalse,
 												Reason:  buildapi.BuilderNotReady,
-												Message: "Builder builder-name is not ready: something went wrong",
+												Message: "Error: Builder 'builder-name' is not ready in namespace 'some-namespace'; Message: something went wrong",
 											},
 										},
 									},
@@ -2301,13 +2301,13 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) {
 												Type:    corev1alpha1.ConditionReady,
 												Status:  corev1.ConditionFalse,
 												Reason:  buildapi.BuilderNotReady,
-												Message: "Builder builder-name is not ready",
+												Message: "Error: Builder 'builder-name' is not ready in namespace 'some-namespace'",
 											},
 											{
 												Type:    buildapi.ConditionBuilderReady,
 												Status:  corev1.ConditionFalse,
 												Reason:  buildapi.BuilderNotReady,
-												Message: "Builder builder-name is not ready",
+												Message: "Error: Builder 'builder-name' is not ready in namespace 'some-namespace'",
 											},
 										},
 									},
@@ -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: fmt.Sprintf("Build %s failed: %s", failedBuild.Name, failureMessage),
+												Message: fmt.Sprintf("Error: Build '%s' in namespace '%s' failed: %s", failedBuild.Name, failedBuild.Namespace, 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.LatestBuildRef)
+					imageWithBuilder.Status.Conditions = conditionNotReady(imageWithBuilder)
 					imageWithBuilder.Status.BuildCounter = 5
 					sourceResolver := resolvedSourceResolver(imageWithBuilder)
 
@@ -2606,7 +2606,7 @@ func conditionReadyUnknown() corev1alpha1.Conditions {
 			Type:    corev1alpha1.ConditionReady,
 			Status:  corev1.ConditionUnknown,
 			Reason:  image.ResolverNotReadyReason,
-			Message: "SourceResolver image-name-source is not ready",
+			Message: "Error: SourceResolver 'image-name-source' is not ready",
 		},
 		{
 			Type:   buildapi.ConditionBuilderReady,
@@ -2622,7 +2622,7 @@ func conditionBuildExecuting(buildName string) corev1alpha1.Conditions {
 			Type:    corev1alpha1.ConditionReady,
 			Status:  corev1.ConditionUnknown,
 			Reason:  image.BuildRunningReason,
-			Message: fmt.Sprintf("%s is executing", buildName),
+			Message: fmt.Sprintf("Build '%s' is executing", buildName),
 		},
 		{
 			Type:   buildapi.ConditionBuilderReady,
@@ -2647,13 +2647,13 @@ func conditionReady() corev1alpha1.Conditions {
 	}
 }
 
-func conditionNotReady(failedBuild string) corev1alpha1.Conditions {
+func conditionNotReady(failedBuild *buildapi.Image) corev1alpha1.Conditions {
 	return corev1alpha1.Conditions{
 		{
 			Type:    corev1alpha1.ConditionReady,
 			Status:  corev1.ConditionFalse,
 			Reason:  image.BuildFailedReason,
-			Message: fmt.Sprintf("Build %s failed: ", failedBuild),
+			Message: fmt.Sprintf("Error: Build '%s' in namespace '%s' failed: ", failedBuild.Status.LatestBuildRef, failedBuild.Namespace),
 		},
 		{
 			Type:   buildapi.ConditionBuilderReady,

pkg/reconciler/image/reconcile_build.go

@@ -24,7 +24,7 @@ const (
 func (c *Reconciler) reconcileBuild(ctx context.Context, image *buildapi.Image, latestBuild *buildapi.Build, sourceResolver *buildapi.SourceResolver, builder buildapi.BuilderResource, buildCacheName string) (buildapi.ImageStatus, error) {
 	currentBuildNumber, err := buildCounter(latestBuild)
 	if err != nil {
-		return buildapi.ImageStatus{}, err
+		return buildapi.ImageStatus{}, errors.Wrap(err, "error parsing the image build number")
 	}
 
 	result, err := isBuildRequired(image, latestBuild, sourceResolver, builder)
@@ -41,7 +41,7 @@ func (c *Reconciler) reconcileBuild(ctx context.Context, image *buildapi.Image,
 		build := image.Build(sourceResolver, builder, latestBuild, result.ReasonsStr, result.ChangesStr, nextBuildNumber, priorityClass)
 		build, err = c.Client.KpackV1alpha2().Builds(build.Namespace).Create(ctx, build, metav1.CreateOptions{})
 		if err != nil {
-			return buildapi.ImageStatus{}, err
+			return buildapi.ImageStatus{}, errors.WithMessage(err, fmt.Sprintf("error creating build '%s' in namespace '%s'", build.Name, build.Namespace))
 		}
 
 		return buildapi.ImageStatus{
@@ -72,7 +72,7 @@ func (c *Reconciler) reconcileBuild(ctx context.Context, image *buildapi.Image,
 			BuildCacheName:             buildCacheName,
 		}, nil
 	default:
-		return buildapi.ImageStatus{}, errors.Errorf("unexpected build needed condition %s", result.ConditionStatus)
+		return buildapi.ImageStatus{}, errors.Errorf("Error: unexpected build needed condition %s", result.ConditionStatus)
 	}
 }
 
@@ -91,19 +91,19 @@ func noScheduledBuild(buildNeeded corev1.ConditionStatus, builder buildapi.Build
 	case buildNeeded == corev1.ConditionUnknown && !sourceResolver.Ready():
 		ready.Status = corev1.ConditionUnknown
 		ready.Reason = ResolverNotReadyReason
-		ready.Message = fmt.Sprintf("SourceResolver %s is not ready", sourceResolver.GetName())
+		ready.Message = fmt.Sprintf("Error: 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"
+		ready.Message = "Error: 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"))
+		ready.Message = fmt.Sprintf("Error: Build '%s' in namespace '%s' failed: %s", build.Name, build.Namespace, defaultMessageIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded), "unknown error"))
 	}
 
 	return corev1alpha1.Conditions{ready, builderCondition(builder)}
@@ -146,10 +146,10 @@ func builderCondition(builder buildapi.BuilderResource) corev1alpha1.Condition {
 }
 
 func builderError(builder buildapi.BuilderResource) string {
-	errorMessage := fmt.Sprintf("Builder %s is not ready", builder.GetName())
+	errorMessage := fmt.Sprintf("Error: Builder '%s' is not ready in namespace '%s'", builder.GetName(), builder.GetNamespace())
 
 	if message := builder.ConditionReadyMessage(); message != "" {
-		errorMessage = fmt.Sprintf("%s: %s", errorMessage, message)
+		errorMessage = fmt.Sprintf("%s; Message: %s", errorMessage, message)
 	}
 
 	return errorMessage
@@ -161,7 +161,7 @@ func scheduledBuildCondition(build *buildapi.Build) corev1alpha1.Conditions {
 			Type:               corev1alpha1.ConditionReady,
 			Status:             corev1.ConditionUnknown,
 			Reason:             BuildRunningReason,
-			Message:            fmt.Sprintf("%s is executing", build.Name),
+			Message:            fmt.Sprintf("Build '%s' is executing", build.Name),
 			LastTransitionTime: corev1alpha1.VolatileTime{Inner: metav1.Now()},
 		},
 		{
@@ -184,7 +184,7 @@ func buildCounter(build *buildapi.Build) (int64, error) {
 
 func buildRunningCondition(build *buildapi.Build, builder buildapi.BuilderResource) corev1alpha1.Conditions {
 	message := defaultMessageIfNil(build.Status.GetCondition(corev1alpha1.ConditionSucceeded),
-		fmt.Sprintf("%s is executing", build.Name))
+		fmt.Sprintf("Build '%s' is executing", build.Name))
 	return corev1alpha1.Conditions{
 		{
 			Type:               corev1alpha1.ConditionReady,