Decouple Function Execution from Pipeline Step Definition
Problem
The pipeline step schema specifically states whether the user wants the function to be run from a local binary with exec or a container with image. This is particularly fitting for kpt fn eval where the user tends to know if they have the binary on their machine or not.
However, if we look at the example of Porch, a library user of kpt, we see a very striking issue: Porch has to reimplement the runtime/runner/evaluator pipeline from scratch to handle a cloud-native (kubernetes-oriented) usecase. In doing so, the whole image/exec separation is turned on its head, as only the image field can be used, but then that sometimes results in the usage of a binary based on how the different evaluators are set up.
Whilst this is not a major problem from the perspective of the kpt CLI, the meaning of the step definition changes based on whether the kpt CLI or Porch is used during evaluation, and in Porch's case, the meaning of image and exec are somewhat contradictory.
Proposal
function field
Deprecate both the image and exec fields in the step definition in favour of a field that solely identifies the function, not where it comes from. For now, let's call this field function.
The function field has to contain the unique identifier of the KRM function, no registries, no file paths. The suggested format would be restricted to alphanumeric ASCII with the addition of _ and - (naturally, all the current catalog functions fit this criteria).
Based on this ID, a multi-evaluator could route the request the an appropriate "concrete" evaluator based on which one can actually execute the function given the constraint of the tag field. This is basically what Porch does already, just using the image field.
We would have to agree on the precedency between "evaluator speed" and "latest version". If the tag is ~0.1, the ExecFn evaluator has version 0.1.1 and the ContainerFn evaluator has 0.1.2, which do we choose? If we let the user decide, at what level do we configure that?
Support --function in fn eval and the function field in fn render
Initially, it can be a simple multi-evaluator which uses ExecFn if the passed function can be found in the PATH, otherwise it uses ContainerFn. Rewiring what the --allow-exec flag does works flawlessly with this implementation as well.
Evaluator parity
kpt has 2 concrete evaluators: ExecFn and ContainerFn, whilst Porch has 3: builtinruntime, executableevalautor and podevaluator. Although they do the same thing, ExecFn and executableevaluator are not the same, meaning there is little overlap.
These should be unified into 1 repo, potentially even one separate from the main kpt repo.
Porch should then import these evaluators instead. The pod manager and pod cache code should stay Porch-only IMO. Maybe have a simple pod-evaluator with a rich interface for kpt and derive the complicated one for Porch?
Pre-planned evaluation path
Currently, multievaluator works by sending the request to each evaluator sequentially and proceeding to the next one if it gets a "NotFound" error or returning the results upon success.
Instead, each evaluator should have a "resolver" to determine whether it can actually execute that step. This resolution should be done before the actual pipeline execution, determining which evaluator each step will use and what exact versions of the functions. This way, a dry-run or "plan" can be done without calling into any evaluation code or sending package contents.
multiruntime is a technically what is supposed work like this, but the capability is only used by builtinRuntime.
Additionally, I believe the whole runtime vs runner vs evaluator should be restructured. Probably just have evaluators that kpt can pick from in a predictable manner (or let a replaceable multievaluator handle that work).
Auxiliary Proposals
These can probably be broken out into their own separate issues.
Split the kpt library and CLI into separate go modules
In the kpt repo, we currently have these modules:
. (the main kpt module)
api
documentation
mdtogo
We have been debating if fragmenting repos into multiple modules is bad for maintainability, as we have to update several go.mod files to fully update a whole repository.
The main downside of not splitting up the codebase is that if someone wants to use some very basic part of the code (like the API types) they have to pull in all the heavier dependencies of the CLI unnecessarily, which can result in them having to do a breaking update of a module the API types don't use.
It would be a good idea to find a fitting border along which the library part (not just the lib folder) and the CLI part can be split.
The idea is just to have one library module, but it may make sense to have multiple, if we consider minimizing dependencies.
As stated previously, the evaluators could be separated as well.
kpt fn install
Add a new fn install ${function_name} command which would basically just do go install github.com/kptdev/krm-functions-catalog/${function_name} or pull the binary from the GitHub release (currently we do not publish binaries in the releases in the catalog).
Since installing multiple versions of functions is commonplace, we need to copy the built/downloaded binary in a way that kpt can not only easily access it, but easily determine what functions and what versions are installed.
If we want to support both local building and downloading, the command could have a switch like --prebuilt, which builds the binary if false and downloads it if true (I would suggest having it false by default). Having both means we support more architectures.
Allow some way indicate resource usage of a step
YAML is notoriously resource intensive to parse, and for Go specifically, memory usage could peak at around 10x the file size.
Add on top of that the potentially large schema that the already large YAML is being validated against and the RAM usage skyrockets.
And then there is also multi-file mutation/validation...
Thus, it would be a good idea to allow the package creator to signal in advance that "hey, this step might require a gigabyte of memory" right in the step definition.
This way, whatever evaluator is going to try to run that step can take measures to avoid OOM errors, maybe even flat-out refuse to evaluate because the system cannot handle that demand.
No concrete implementation proposal for now.
Decouple Function Execution from Pipeline Step Definition
Problem
The pipeline step schema specifically states whether the user wants the function to be run from a local binary with
execor a container withimage. This is particularly fitting forkpt fn evalwhere the user tends to know if they have the binary on their machine or not.However, if we look at the example of Porch, a library user of kpt, we see a very striking issue: Porch has to reimplement the runtime/runner/evaluator pipeline from scratch to handle a cloud-native (kubernetes-oriented) usecase. In doing so, the whole image/exec separation is turned on its head, as only the
imagefield can be used, but then that sometimes results in the usage of a binary based on how the different evaluators are set up.Whilst this is not a major problem from the perspective of the kpt CLI, the meaning of the step definition changes based on whether the kpt CLI or Porch is used during evaluation, and in Porch's case, the meaning of
imageandexecare somewhat contradictory.Proposal
functionfieldDeprecate both the
imageandexecfields in the step definition in favour of a field that solely identifies the function, not where it comes from. For now, let's call this fieldfunction.The
functionfield has to contain the unique identifier of the KRM function, no registries, no file paths. The suggested format would be restricted to alphanumeric ASCII with the addition of_and-(naturally, all the current catalog functions fit this criteria).Based on this ID, a multi-evaluator could route the request the an appropriate "concrete" evaluator based on which one can actually execute the function given the constraint of the
tagfield. This is basically what Porch does already, just using theimagefield.We would have to agree on the precedency between "evaluator speed" and "latest version". If the
tagis~0.1, theExecFnevaluator has version0.1.1and theContainerFnevaluator has0.1.2, which do we choose? If we let the user decide, at what level do we configure that?Support
--functioninfn evaland thefunctionfield infn renderInitially, it can be a simple multi-evaluator which uses
ExecFnif the passedfunctioncan be found in the PATH, otherwise it usesContainerFn. Rewiring what the--allow-execflag does works flawlessly with this implementation as well.Evaluator parity
kpt has 2 concrete evaluators:
ExecFnandContainerFn, whilst Porch has 3:builtinruntime,executableevalautorandpodevaluator. Although they do the same thing,ExecFnandexecutableevaluatorare not the same, meaning there is little overlap.These should be unified into 1 repo, potentially even one separate from the main kpt repo.
Porch should then import these evaluators instead. The pod manager and pod cache code should stay Porch-only IMO. Maybe have a simple pod-evaluator with a rich interface for kpt and derive the complicated one for Porch?
Pre-planned evaluation path
Currently,
multievaluatorworks by sending the request to each evaluator sequentially and proceeding to the next one if it gets a "NotFound" error or returning the results upon success.Instead, each evaluator should have a "resolver" to determine whether it can actually execute that step. This resolution should be done before the actual pipeline execution, determining which evaluator each step will use and what exact versions of the functions. This way, a dry-run or "plan" can be done without calling into any evaluation code or sending package contents.
multiruntimeis a technically what is supposed work like this, but the capability is only used bybuiltinRuntime.Additionally, I believe the whole runtime vs runner vs evaluator should be restructured. Probably just have evaluators that kpt can pick from in a predictable manner (or let a replaceable multievaluator handle that work).
Auxiliary Proposals
These can probably be broken out into their own separate issues.
Split the kpt library and CLI into separate go modules
In the kpt repo, we currently have these modules:
.(the main kpt module)apidocumentationmdtogoWe have been debating if fragmenting repos into multiple modules is bad for maintainability, as we have to update several
go.modfiles to fully update a whole repository.The main downside of not splitting up the codebase is that if someone wants to use some very basic part of the code (like the API types) they have to pull in all the heavier dependencies of the CLI unnecessarily, which can result in them having to do a breaking update of a module the API types don't use.
It would be a good idea to find a fitting border along which the library part (not just the
libfolder) and the CLI part can be split.The idea is just to have one library module, but it may make sense to have multiple, if we consider minimizing dependencies.
As stated previously, the evaluators could be separated as well.
kpt fn installAdd a new
fn install ${function_name}command which would basically just dogo install github.com/kptdev/krm-functions-catalog/${function_name}or pull the binary from the GitHub release (currently we do not publish binaries in the releases in the catalog).Since installing multiple versions of functions is commonplace, we need to copy the built/downloaded binary in a way that kpt can not only easily access it, but easily determine what functions and what versions are installed.
If we want to support both local building and downloading, the command could have a switch like
--prebuilt, which builds the binary if false and downloads it if true (I would suggest having it false by default). Having both means we support more architectures.Allow some way indicate resource usage of a step
YAML is notoriously resource intensive to parse, and for Go specifically, memory usage could peak at around 10x the file size.
Add on top of that the potentially large schema that the already large YAML is being validated against and the RAM usage skyrockets.
And then there is also multi-file mutation/validation...
Thus, it would be a good idea to allow the package creator to signal in advance that "hey, this step might require a gigabyte of memory" right in the step definition.
This way, whatever evaluator is going to try to run that step can take measures to avoid OOM errors, maybe even flat-out refuse to evaluate because the system cannot handle that demand.
No concrete implementation proposal for now.