kpt
kpt copied to clipboard
render library: surface function results on validation failure
Fixes https://github.com/GoogleContainerTools/kpt/issues/3423
cc @ChristopherFry
This PR surfaces the function Results
from the function runner library when a validation function encounters a failure (rather than returning a generic error message). As a side effect, the format of the logs on the server side change slightly.
Before:
Output on CLI side:
$ kpt alpha rpkg push test-deployments-9ed61849dae3bbc9345afc7c61e8b5d10a81e176 ../foo -ndefault > /dev/null
Error: Internal error occurred: fn.render: pkg /:
pkg.render:
pipeline.run: error: function failure
Output on server side:
I0809 20:41:49.839213 3642946 render.go:59] Package "/":
I0809 20:41:49.839425 3642946 render.go:59] [RUNNING] "gcr.io/kpt-fn/starlark:v0.4.3"
I0809 20:41:49.839443 3642946 render.go:59]
I0809 20:41:49.842271 3642946 render.go:59] [FAIL] "gcr.io/kpt-fn/starlark:v0.4.3" in 0s
I0809 20:41:49.842298 3642946 render.go:59] Results:
[error]: fail: validation always fails
After:
Output on CLI side:
$ kpt alpha rpkg push test-deployments-9ed61849dae3bbc9345afc7c61e8b5d10a81e176 ../foo -ndefault
Error: Internal error occurred: fn.render: pkg /:
pkg.render:
pipeline.run:
[RUNNING] "gcr.io/kpt-fn/starlark:v0.4.3"
[FAIL] "gcr.io/kpt-fn/starlark:v0.4.3" in 0s
Results:
[error]: fail: validation always fails
Output on server side:
I0809 22:15:45.327832 3668588 render.go:59] Package "/":
I0809 22:15:45.331214 3668588 render.go:59] [RUNNING] "gcr.io/kpt-fn/starlark:v0.4.3"
[FAIL] "gcr.io/kpt-fn/starlark:v0.4.3" in 0s
Results:
[error]: fail: validation always fails
cc @droot @mortent
I'll see if I can add a test for this
@mortent In addition to changing the output for porch, this PR changes the output slightly for kpt fn render
and kpt fn eval
. More details in https://github.com/GoogleContainerTools/kpt/pull/3451#discussion_r942756230 and https://github.com/GoogleContainerTools/kpt/pull/3451#discussion_r942924045. I wanted to double check with you to see if you have any concerns about that. If the changes to the output there are not acceptable, I might need to take a look at a bigger restructure of some of this library code.
@natasha41575 These changes seem fine to me, but I think we want @droot or @mengqiy to verify this.
@natasha41575 These changes seem fine to me, but I think we want @droot or @mengqiy to verify this.
👍 Will ask @droot to take a look when he comes back
Chatted with @droot offline about this.
This PR tries to expose the fnResults as a string in the error message, but that doesn't make sense as a porch API. We think it makes more sense to expose the fnResults
in the status field of the returned resources when render fails. I plan to update this PR accordingly.