kpt icon indicating copy to clipboard operation
kpt copied to clipboard

render library: surface function results on validation failure

Open natasha41575 opened this issue 1 year ago • 2 comments

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

natasha41575 avatar Aug 09 '22 22:08 natasha41575

cc @droot @mortent

natasha41575 avatar Aug 09 '22 22:08 natasha41575

I'll see if I can add a test for this

natasha41575 avatar Aug 09 '22 22:08 natasha41575

@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 avatar Aug 10 '22 21:08 natasha41575

@natasha41575 These changes seem fine to me, but I think we want @droot or @mengqiy to verify this.

mortent avatar Aug 16 '22 04:08 mortent

@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

natasha41575 avatar Aug 18 '22 17:08 natasha41575

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.

natasha41575 avatar Aug 23 '22 20:08 natasha41575