flyteorg/flyte

[Housekeeping] Use passthrough workflowstore to perform CRD terminations / manipulations

Closed this issue · 2 comments

Describe the issue

FlytePropeller uses the workflowstore to abstract FltyeWorkflow CRD updates and retrievals from the k8s API. To provide additional functionality, this API is designed to wrap other implementations with additional layers. In the default-case this manifests as:
(1) the passthrough workflowstore which is a basic overlay of the k8s API.
(2) the resource version caching workflowstore which uses the k8s resource version to mitigate processing or updating stale workflows.

PRs for clearing managed fields from the CRD and tracking terminated workflows were implemented in the resource version caching workflowstore. This functionality should really be performed at the passthrough workflowstore level, or in another wrapped workflowstore.

What if we do not do this?

If users change configuration to remove the resource version caching workflowstore and instead use only the passthrough workflowstore then they will not benefit from these improvements.

Related component(s)

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes

I think the logic to clear CRD managed fields should be moved to the passthrough workflowstore, while the logic to track terminated workflows seems like it should be its own new wrapped workflowstore.

Question is, should the track terminated workflowstore wrap the passthrough workflowstore or the resource version caching workflowstore (or should the resource version caching workflowstore wrap the track terminated workflowstore)? The logic to track terminated workflows is unrelated to resource version caching, but if it wraps the passthrough workflowstore, then there would be no way to have both resource version caching and tracking terminated workflows together.

Another approach is to rework the workflow config to take in a list of policies, then chain them together. For example, if a new PolicyTrackTerminated policy is added, specifying both PolicyResourceVersionCaching and PolicyTrackTerminated will initialize a track terminated workflowstore that wraps a resource version caching workflowstore, which in turn wraps the passthrough workflowstore.

This however is a breaking change (or actually it may not be since a YAML string will be parsed as a single-element list). What do you think?