Refactor functions for Status & Conditions handling

[zroubalik]

Proposal

As disscuseed on #2765 (comment), we should unify the functions for Status & Conditions handling. Referencing controller related stuff here is suboptimal.

There are two locations with similar functions:

keda/controllers/keda/util/status.go

Lines 30 to 63 in 9f0ee58

	func SetStatusConditions(ctx context.Context, client runtimeclient.StatusClient, logger logr.Logger, object interface{}, conditions *kedav1alpha1.Conditions) error {
	var patch runtimeclient.Patch
	runtimeObj := object.(runtimeclient.Object)
	switch obj := runtimeObj.(type) {
	case *kedav1alpha1.ScaledObject:
	patch = runtimeclient.MergeFrom(obj.DeepCopy())
	obj.Status.Conditions = *conditions
	case *kedav1alpha1.ScaledJob:
	patch = runtimeclient.MergeFrom(obj.DeepCopy())
	obj.Status.Conditions = *conditions
	default:
	err := fmt.Errorf("unknown scalable object type %v", obj)
	logger.Error(err, "Failed to patch Objects Status with Conditions")
	return err
	}
	err := client.Status().Patch(ctx, runtimeObj, patch)
	if err != nil {
	logger.Error(err, "Failed to patch Objects Status with Conditions")
	}
	return err
	}
	// UpdateScaledObjectStatus patches the given ScaledObject with the updated status passed to it or returns an error.
	func UpdateScaledObjectStatus(ctx context.Context, client runtimeclient.StatusClient, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject, status *kedav1alpha1.ScaledObjectStatus) error {
	patch := runtimeclient.MergeFrom(scaledObject.DeepCopy())
	scaledObject.Status = *status
	err := client.Status().Patch(ctx, scaledObject, patch)
	if err != nil {
	logger.Error(err, "Failed to patch ScaledObjects Status")
	}
	return err
	}

keda/pkg/scaling/executor/scale_executor.go

Lines 64 to 132 in 9f0ee58

	func (e *scaleExecutor) updateLastActiveTime(ctx context.Context, logger logr.Logger, object interface{}) error {
	var patch runtimeclient.Patch
	now := metav1.Now()
	runtimeObj := object.(runtimeclient.Object)
	switch obj := runtimeObj.(type) {
	case *kedav1alpha1.ScaledObject:
	patch = runtimeclient.MergeFrom(obj.DeepCopy())
	obj.Status.LastActiveTime = &now
	case *kedav1alpha1.ScaledJob:
	patch = runtimeclient.MergeFrom(obj.DeepCopy())
	obj.Status.LastActiveTime = &now
	default:
	err := fmt.Errorf("unknown scalable object type %v", obj)
	logger.Error(err, "Failed to patch Objects Status")
	return err
	}
	err := e.client.Status().Patch(ctx, runtimeObj, patch)
	if err != nil {
	logger.Error(err, "Failed to patch Objects Status")
	}
	return err
	}
	func (e *scaleExecutor) setCondition(ctx context.Context, logger logr.Logger, object interface{}, status metav1.ConditionStatus, reason string, message string, setCondition func(kedav1alpha1.Conditions, metav1.ConditionStatus, string, string)) error {
	var patch runtimeclient.Patch
	runtimeObj := object.(runtimeclient.Object)
	switch obj := runtimeObj.(type) {
	case *kedav1alpha1.ScaledObject:
	patch = runtimeclient.MergeFrom(obj.DeepCopy())
	setCondition(obj.Status.Conditions, status, reason, message)
	case *kedav1alpha1.ScaledJob:
	patch = runtimeclient.MergeFrom(obj.DeepCopy())
	setCondition(obj.Status.Conditions, status, reason, message)
	default:
	err := fmt.Errorf("unknown scalable object type %v", obj)
	logger.Error(err, "Failed to patch Objects Status")
	return err
	}
	err := e.client.Status().Patch(ctx, runtimeObj, patch)
	if err != nil {
	logger.Error(err, "Failed to patch Objects Status")
	}
	return err
	}
	func (e *scaleExecutor) setReadyCondition(ctx context.Context, logger logr.Logger, object interface{}, status metav1.ConditionStatus, reason string, message string) error {
	active := func(conditions kedav1alpha1.Conditions, status metav1.ConditionStatus, reason string, message string) {
	conditions.SetReadyCondition(status, reason, message)
	}
	return e.setCondition(ctx, logger, object, status, reason, message, active)
	}
	func (e *scaleExecutor) setActiveCondition(ctx context.Context, logger logr.Logger, object interface{}, status metav1.ConditionStatus, reason string, message string) error {
	active := func(conditions kedav1alpha1.Conditions, status metav1.ConditionStatus, reason string, message string) {
	conditions.SetActiveCondition(status, reason, message)
	}
	return e.setCondition(ctx, logger, object, status, reason, message, active)
	}
	func (e *scaleExecutor) setFallbackCondition(ctx context.Context, logger logr.Logger, object interface{}, status metav1.ConditionStatus, reason string, message string) error {
	fallback := func(conditions kedav1alpha1.Conditions, status metav1.ConditionStatus, reason string, message string) {
	conditions.SetFallbackCondition(status, reason, message)
	}
	return e.setCondition(ctx, logger, object, status, reason, message, fallback)
	}

We should refactor this to have it in one place.

Use-Case

No response

Anything else?

No response

[catouc]

@zroubalik I've tried to implement something that would pull out the function into the util package in #3020 but I'm a bit stuck on one test failing and I can't really figure out why the mock calls are not hit. Especially when other tests that should in my mind fail, then work.

But I thought it's probably worth a quick check in to see if this direction is even desired or useful before I sink more time into something that ultimately can't be merged :)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[tobotg]

@JorTurFer @zroubalik I'm giving this issue a try if it's ok.