Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MissingValueAtPathCondition message is conditioned on object health #1356

Merged
merged 1 commit into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions pkg/apis/v1alpha1/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
2 changes: 1 addition & 1 deletion pkg/conditions/deliverable_conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
35 changes: 20 additions & 15 deletions pkg/conditions/owner_conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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),
Expand Down
42 changes: 37 additions & 5 deletions pkg/conditions/owner_conditions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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]"))
})
})
})
Expand Down
4 changes: 1 addition & 3 deletions pkg/conditions/workload_conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
36 changes: 33 additions & 3 deletions pkg/controllers/deliverable_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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 {
Expand Down Expand Up @@ -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() {
Expand Down
5 changes: 3 additions & 2 deletions pkg/controllers/workload_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
11 changes: 2 additions & 9 deletions pkg/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -131,6 +132,7 @@ type RetrieveOutputError struct {
BlueprintType string
QualifiedResource string
PassThroughInput string
Healthy metav1.ConditionStatus
}

func (e RetrieveOutputError) Error() string {
Expand Down Expand Up @@ -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)
Expand Down
28 changes: 13 additions & 15 deletions pkg/realizer/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
}

Expand Down
Loading
Loading