[#1315] Fix for workload prematurely being declared unhealthy

pkg/apis/v1alpha1/conditions.go

@@ -159,15 +159,14 @@ const (
 // -- RUNNABLE ConditionType - RunTemplateReady ConditionReasons
 
 const (
-	ReadyRunTemplateReason                                    = "Ready"
-	NotFoundRunTemplateReason                                 = "RunTemplateNotFound"
-	StampedObjectRejectedByAPIServerRunTemplateReason         = "StampedObjectRejectedByAPIServer"
-	OutputPathNotSatisfiedRunTemplateReason                   = "OutputPathNotSatisfied"
-	TemplateStampFailureRunTemplateReason                     = "TemplateStampFailure"
-	FailedToListCreatedObjectsReason                          = "FailedToListCreatedObjects"
-	SetOfImmutableStampedObjectsIncludesNoHealthyObjectReason = "SetOfImmutableStampedObjectsIncludesNoHealthyObject"
-	UnknownErrorReason                                        = "UnknownError"
-	ClientBuilderErrorResourcesSubmittedReason                = "ClientBuilderError"
-	SucceededStampedObjectConditionReason                     = "SucceededCondition"
-	UnknownStampedObjectConditionReason                       = "Unknown"
+	ReadyRunTemplateReason                            = "Ready"
+	NotFoundRunTemplateReason                         = "RunTemplateNotFound"
+	StampedObjectRejectedByAPIServerRunTemplateReason = "StampedObjectRejectedByAPIServer"
+	OutputPathNotSatisfiedRunTemplateReason           = "OutputPathNotSatisfied"
+	TemplateStampFailureRunTemplateReason             = "TemplateStampFailure"
+	FailedToListCreatedObjectsReason                  = "FailedToListCreatedObjects"
+	UnknownErrorReason                                = "UnknownError"
+	ClientBuilderErrorResourcesSubmittedReason        = "ClientBuilderError"
+	SucceededStampedObjectConditionReason             = "SucceededCondition"
+	UnknownStampedObjectConditionReason               = "Unknown"
 )

pkg/conditions/deliverable_conditions.go

@@ -120,7 +120,7 @@ func AddConditionForResourceSubmittedDeliverable(conditionManager *ConditionMana
 			if typedErr.StampedObject == nil {
 				(*conditionManager).AddPositive(MissingPassThroughInputCondition(typedErr.PassThroughInput, typedErr.GetQualifiedResource()))
 			} else {
-				(*conditionManager).AddPositive(MissingValueAtPathCondition(isOwner, typedErr.StampedObject, typedErr.JsonPathExpression(), typedErr.GetQualifiedResource()))
+				(*conditionManager).AddPositive(MissingValueAtPathCondition(isOwner, typedErr.StampedObject, typedErr.JsonPathExpression(), typedErr.GetQualifiedResource(), typedErr.Healthy))
 			}
 		default:
 			(*conditionManager).AddPositive(UnknownResourceErrorCondition(isOwner, typedErr))

pkg/conditions/owner_conditions.go

@@ -61,17 +61,31 @@ func TemplateObjectRetrievalFailureCondition(isOwner bool, err error) metav1.Con
 	}
 }
 
-func MissingValueAtPathCondition(isOwner bool, obj *unstructured.Unstructured, expression string, qualifiedResource string) metav1.Condition {
+func MissingValueAtPathCondition(isOwner bool, obj *unstructured.Unstructured, expression string, qualifiedResource string, health metav1.ConditionStatus) metav1.Condition {
 	var namespaceMsg string
 	if obj.GetNamespace() != "" {
 		namespaceMsg = fmt.Sprintf(" in namespace [%s]", obj.GetNamespace())
 	}
+
+	var message string
+
+	switch health {
+	case metav1.ConditionTrue:
+		message = fmt.Sprintf("cannot read value [%s] from healthy object [%s/%s]%s, contact Platform Eng",
+			expression, qualifiedResource, obj.GetName(), namespaceMsg)
+	case metav1.ConditionFalse:
+		message = fmt.Sprintf("cannot read value [%s] from unhealthy object [%s/%s]%s, examine object, particularly whether it is receiving proper inputs",
+			expression, qualifiedResource, obj.GetName(), namespaceMsg)
+	default:
+		message = fmt.Sprintf("waiting to read value [%s] from object [%s/%s]%s",
+			expression, qualifiedResource, obj.GetName(), namespaceMsg)
+	}
+
 	return metav1.Condition{
-		Type:   getConditionType(isOwner),
-		Status: metav1.ConditionUnknown,
-		Reason: v1alpha1.MissingValueAtPathResourcesSubmittedReason,
-		Message: fmt.Sprintf("waiting to read value [%s] from resource [%s/%s]%s",
-			expression, qualifiedResource, obj.GetName(), namespaceMsg),
+		Type:    getConditionType(isOwner),
+		Status:  metav1.ConditionUnknown,
+		Reason:  v1alpha1.MissingValueAtPathResourcesSubmittedReason,
+		Message: message,
 	}
 }
 
@@ -102,15 +116,6 @@ func BlueprintsFailedToListCreatedObjectsCondition(isOwner bool, err error) meta
 	}
 }
 
-func NoHealthyImmutableObjectsCondition(isOwner bool, err error) metav1.Condition {
-	return metav1.Condition{
-		Type:    getConditionType(isOwner),
-		Status:  metav1.ConditionFalse,
-		Reason:  v1alpha1.SetOfImmutableStampedObjectsIncludesNoHealthyObjectReason,
-		Message: err.Error(),
-	}
-}
-
 func UnknownResourceErrorCondition(isOwner bool, err error) metav1.Condition {
 	return metav1.Condition{
 		Type:    getConditionType(isOwner),

pkg/conditions/owner_conditions_test.go

@@ -17,6 +17,7 @@ package conditions_test
 import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
@@ -38,18 +39,49 @@ var _ = Describe("Conditions", func() {
 		})
 
 		Context("stamped object has a namespace", func() {
-			It("has the correct message", func() {
+			var healthy metav1.ConditionStatus
+			BeforeEach(func() {
 				obj.SetNamespace("my-ns")
+			})
+
+			Context("healthy is true", func() {
+				BeforeEach(func() {
+					healthy = metav1.ConditionTrue
+				})
+
+				It("has the correct message", func() {
+					condition := conditions.MissingValueAtPathCondition(true, obj, "spec.foo", "widget.thing.io", healthy)
+					Expect(condition.Message).To(Equal("cannot read value [spec.foo] from healthy object [widget.thing.io/my-widget] in namespace [my-ns], contact Platform Eng"))
+				})
+			})
+
+			Context("healthy is false", func() {
+				BeforeEach(func() {
+					healthy = metav1.ConditionFalse
+				})
+
+				It("has the correct message", func() {
+					condition := conditions.MissingValueAtPathCondition(true, obj, "spec.foo", "widget.thing.io", healthy)
+					Expect(condition.Message).To(Equal("cannot read value [spec.foo] from unhealthy object [widget.thing.io/my-widget] in namespace [my-ns], examine object, particularly whether it is receiving proper inputs"))
+				})
+			})
+
+			Context("healthy is unknown", func() {
+				BeforeEach(func() {
+					healthy = metav1.ConditionUnknown
+				})
 
-				condition := conditions.MissingValueAtPathCondition(true, obj, "spec.foo", "widget.thing.io")
-				Expect(condition.Message).To(Equal("waiting to read value [spec.foo] from resource [widget.thing.io/my-widget] in namespace [my-ns]"))
+				It("has the correct message", func() {
+					condition := conditions.MissingValueAtPathCondition(true, obj, "spec.foo", "widget.thing.io", healthy)
+					Expect(condition.Message).To(Equal("waiting to read value [spec.foo] from object [widget.thing.io/my-widget] in namespace [my-ns]"))
+				})
 			})
 		})
 
 		Context("stamped object does not have a namespace", func() {
 			It("has the correct message", func() {
-				condition := conditions.MissingValueAtPathCondition(true, obj, "spec.foo", "widget.thing.io")
-				Expect(condition.Message).To(Equal("waiting to read value [spec.foo] from resource [widget.thing.io/my-widget]"))
+				condition := conditions.MissingValueAtPathCondition(true, obj, "spec.foo", "widget.thing.io", metav1.ConditionUnknown)
+				Expect(condition.Message).To(Equal("waiting to read value [spec.foo] from object [widget.thing.io/my-widget]"))
 			})
 		})
 	})

pkg/conditions/workload_conditions.go

@@ -88,13 +88,11 @@ func AddConditionForResourceSubmittedWorkload(conditionManager *ConditionManager
 		(*conditionManager).AddPositive(TemplateRejectedByAPIServerCondition(isOwner, typedErr))
 	case cerrors.ListCreatedObjectsError:
 		(*conditionManager).AddPositive(BlueprintsFailedToListCreatedObjectsCondition(isOwner, typedErr))
-	case cerrors.NoHealthyImmutableObjectsError:
-		(*conditionManager).AddPositive(NoHealthyImmutableObjectsCondition(isOwner, typedErr))
 	case cerrors.RetrieveOutputError:
 		if typedErr.StampedObject == nil {
 			(*conditionManager).AddPositive(MissingPassThroughInputCondition(typedErr.PassThroughInput, typedErr.GetQualifiedResource()))
 		} else {
-			(*conditionManager).AddPositive(MissingValueAtPathCondition(isOwner, typedErr.StampedObject, typedErr.JsonPathExpression(), typedErr.GetQualifiedResource()))
+			(*conditionManager).AddPositive(MissingValueAtPathCondition(isOwner, typedErr.StampedObject, typedErr.JsonPathExpression(), typedErr.GetQualifiedResource(), typedErr.Healthy))
 		}
 	case cerrors.ResolveTemplateOptionError:
 		(*conditionManager).AddPositive(ResolveTemplateOptionsErrorCondition(isOwner, typedErr))

pkg/controllers/deliverable_reconciler_test.go

@@ -796,6 +796,7 @@ var _ = Describe("DeliverableReconciler", func() {
 				var retrieveError cerrors.RetrieveOutputError
 				var wrappedError error
 				var stampedObject *unstructured.Unstructured
+				var healthy metav1.ConditionStatus
 
 				JustBeforeEach(func() {
 					stampedObject = &unstructured.Unstructured{}
@@ -814,6 +815,7 @@ var _ = Describe("DeliverableReconciler", func() {
 						BlueprintType:     cerrors.Delivery,
 						StampedObject:     stampedObject,
 						QualifiedResource: "mything.thing.io",
+						Healthy:           healthy,
 					}
 
 					rlzr.RealizeStub = func(ctx context.Context, resourceRealizer realizer.ResourceRealizer, deliveryName string, resources []realizer.OwnerResource, statuses statuses.ResourceStatuses) error {
@@ -950,9 +952,37 @@ var _ = Describe("DeliverableReconciler", func() {
 						wrappedError = stamp.NewJsonPathError("this.wont.find.anything", errors.New("some error"))
 					})
 
-					It("calls the condition manager to report", func() {
-						_, _ = reconciler.Reconcile(ctx, req)
-						Expect(conditionManager.AddPositiveArgsForCall(1)).To(Equal(conditions.MissingValueAtPathCondition(true, stampedObject, "this.wont.find.anything", "mything.thing.io")))
+					Context("and the RetrieveOutputError reports object as healthy", func() {
+						BeforeEach(func() {
+							healthy = metav1.ConditionTrue
+						})
+
+						It("calls the condition manager to report", func() {
+							_, _ = reconciler.Reconcile(ctx, req)
+							Expect(conditionManager.AddPositiveArgsForCall(1)).To(Equal(conditions.MissingValueAtPathCondition(true, stampedObject, "this.wont.find.anything", "mything.thing.io", metav1.ConditionTrue)))
+						})
+					})
+
+					Context("and the RetrieveOutputError reports object as unhealthy", func() {
+						BeforeEach(func() {
+							healthy = metav1.ConditionFalse
+						})
+
+						It("calls the condition manager to report", func() {
+							_, _ = reconciler.Reconcile(ctx, req)
+							Expect(conditionManager.AddPositiveArgsForCall(1)).To(Equal(conditions.MissingValueAtPathCondition(true, stampedObject, "this.wont.find.anything", "mything.thing.io", metav1.ConditionFalse)))
+						})
+					})
+
+					Context("and the RetrieveOutputError reports object health as unknown", func() {
+						BeforeEach(func() {
+							healthy = metav1.ConditionUnknown
+						})
+
+						It("calls the condition manager to report", func() {
+							_, _ = reconciler.Reconcile(ctx, req)
+							Expect(conditionManager.AddPositiveArgsForCall(1)).To(Equal(conditions.MissingValueAtPathCondition(true, stampedObject, "this.wont.find.anything", "mything.thing.io", metav1.ConditionUnknown)))
+						})
 					})
 
 					It("does not return an error", func() {

pkg/controllers/workload_reconciler_test.go

@@ -850,8 +850,9 @@ var _ = Describe("WorkloadReconciler", func() {
 
 				It("calls the condition manager to report", func() {
 					_, _ = reconciler.Reconcile(ctx, req)
-					Expect(conditionManager.AddPositiveArgsForCall(1)).To(
-						Equal(conditions.MissingValueAtPathCondition(true, stampedObject, "this.wont.find.anything", "mything.thing.io")))
+					var emptyConditionStatus metav1.ConditionStatus
+					Expect(conditionManager.AddPositiveArgsForCall(1)).
+						To(Equal(conditions.MissingValueAtPathCondition(true, stampedObject, "this.wont.find.anything", "mything.thing.io", emptyConditionStatus)))
 				})
 
 				It("does not return an error", func() {

pkg/errors/errors.go

@@ -19,6 +19,7 @@ import (
 	"strings"
 
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
@@ -131,6 +132,7 @@ type RetrieveOutputError struct {
 	BlueprintType     string
 	QualifiedResource string
 	PassThroughInput  string
+	Healthy           metav1.ConditionStatus
 }
 
 func (e RetrieveOutputError) Error() string {
@@ -193,15 +195,6 @@ type NoHealthyImmutableObjectsError struct {
 	BlueprintType string
 }
 
-func (e NoHealthyImmutableObjectsError) Error() string {
-	return fmt.Errorf("unable to retrieve outputs for resource [%s] in %s [%s]: %w",
-		e.ResourceName,
-		e.BlueprintType,
-		e.BlueprintName,
-		e.Err,
-	).Error()
-}
-
 func WrapUnhandledError(err error) error {
 	if IsUnhandledErrorType(err) {
 		return NewUnhandledError(err)

pkg/realizer/component.go

@@ -206,25 +206,23 @@ func (r *resourceRealizer) doImmutable(ctx context.Context, resource OwnerResour
 
 	var output *templates.Output
 
-	if latestSuccessfulObject == nil {
-		for _, obj := range allRunnableStampedObjects {
-			log.V(logger.DEBUG).Info("failed to retrieve output from any object", "considered", obj)
-		}
-
-		return template, stampedObject, nil, passThrough, templateName, errors.NoHealthyImmutableObjectsError{
-			Err:           fmt.Errorf("failed to find any healthy object in the set of immutable stamped objects"),
-			ResourceName:  resource.Name,
-			BlueprintName: blueprintName,
-			BlueprintType: errors.SupplyChain,
-		}
-	}
-
 	output, err = stampReader.Output(latestSuccessfulObject)
 
 	if err != nil {
-		qualifiedResource, rErr := utils.GetQualifiedResource(mapper, latestSuccessfulObject)
+		var (
+			qualifiedResource string
+			rErr              error
+			objectToReport    *unstructured.Unstructured
+		)
+		if latestSuccessfulObject == nil {
+			objectToReport = stampedObject
+		} else {
+			objectToReport = latestSuccessfulObject
+		}
+
+		qualifiedResource, rErr = utils.GetQualifiedResource(mapper, objectToReport)
 		if rErr != nil {
-			log.Error(err, "failed to retrieve qualified resource name", "object", latestSuccessfulObject)
+			log.Error(err, "failed to retrieve qualified resource name", "object", objectToReport)
 			qualifiedResource = "could not fetch - see the log line for 'failed to retrieve qualified resource name'"
 		}