MissingValueAtPathCondition message is conditioned on object health (#…

…1356)

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

@@ -117,7 +117,7 @@ func AddConditionForResourceSubmittedDeliverable(conditionManager *ConditionMana
  case stamp.DeploymentConditionError:
  (*conditionManager).AddPositive(DeploymentConditionNotMetCondition(typedErr))
  case stamp.JsonPathError:
- (*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

@@ -79,10 +79,8 @@ 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:
- (*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))
  case cerrors.TemplateOptionsMatchError:

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'"
  }