Have function-runner output include image version tags in output#4421
Have function-runner output include image version tags in output#4421JamesMcDermott wants to merge 12 commits intokptdev:mainfrom
Conversation
✅ Deploy Preview for kptdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Updates the function-runner’s CLI output so that function image names shown in [RUNNING], [PASS], and [FAIL] lines include version tag information (including semver constraints and latest defaults), improving observability when tag is used or when tags are implicit.
Changes:
- Add
Tagmetadata to function results and use it to construct[RUNNING]display names with the right tag/constraint (defaulting tolatestwhen needed). - Update container execution to overwrite the displayed image on completion with the resolved image actually executed (important for semver constraints).
- Add unit test coverage for display-name construction and new e2e golden tests validating printed output.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/api/fnresult/v1/types.go | Adds Tag field to structured function result metadata. |
| internal/fnruntime/runner.go | Builds [RUNNING] display names including tag/constraint and updates name post-run to use resolved image. |
| internal/fnruntime/container.go | Updates FnResult.Image to the resolved image (or :latest when implicit) to drive [PASS]/[FAIL] output. |
| internal/fnruntime/runner_test.go | Adds table-driven tests for function image display-name construction. |
| e2e/testdata/fn-render/fn-failure-print-fn-version/starlark-failure-fn.yaml | Adds failing starlark config to validate [FAIL] output includes resolved tag. |
| e2e/testdata/fn-render/fn-failure-print-fn-version/resources.yaml | Adds resources for the failure scenario test package. |
| e2e/testdata/fn-render/fn-failure-print-fn-version/deployment_httpbin.yaml | Adds additional resource input for the failure scenario. |
| e2e/testdata/fn-render/fn-failure-print-fn-version/Kptfile | Defines a pipeline using tag so output must show constraint and resolved version. |
| e2e/testdata/fn-render/fn-failure-print-fn-version/.krmignore | Excludes .expected from function input processing. |
| e2e/testdata/fn-render/fn-failure-print-fn-version/.expected/config.yaml | Golden output asserting [RUNNING] shows constraint and [FAIL] shows resolved tag. |
| e2e/testdata/fn-render/basicpipeline-print-fn-versions/resources.yaml | Adds resources for the “basicpipeline” tag-printing scenario. |
| e2e/testdata/fn-render/basicpipeline-print-fn-versions/Kptfile | Exercises multiple tag cases: implicit latest, explicit tag, and semver constraints. |
| e2e/testdata/fn-render/basicpipeline-print-fn-versions/.krmignore | Excludes .expected from function input processing. |
| e2e/testdata/fn-render/basicpipeline-print-fn-versions/.expected/diff.patch | Golden diff output for resource mutations in the basic pipeline. |
| e2e/testdata/fn-render/basicpipeline-print-fn-versions/.expected/config.yaml | Golden stderr output asserting tag/constraint printing behavior across cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fee7097 to
bfcda2c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3651d10 to
bffca20
Compare
internal/fnruntime/container.go
Outdated
| } | ||
| f.Image, err = tagResolver.ResolveFunctionImage(f.Ctx, f.Image, f.Tag) | ||
| if err != nil { | ||
| f.FnResult.Image = buildFunctionDisplayName(f.FnResult) |
There was a problem hiding this comment.
For me, I think it would be better if we could get the ACTUAL image being RUN.
We could prob get this from the StdErr in the runCli fn below, the the f.FnResult.Image would display the exact tag/digest.
There was a problem hiding this comment.
Not sure that's possible at this point - a few points:
- Do you mean in this particular block? If we're at this line, we've had an error trying to resolve the
ImageandTag(which we didn't know without trying if it had an exact version or SemVer constraint) to an actual image - there's no way to know what the actual image would have been that would have been run if we hadn't errored out. I think giving back the same tag as configured is the best we can do in this situation. - Do you mean in the
[PASS]/[FAIL]lines after the run? That's already done by line 169 as is - a successfulResolveFunctionImage()call will have updatedf.Imageto include the exact resolved tag - Do you mean in
[RUNNING]output lines in general? That's not currently possible because at the point of resolving the actual image, the[RUNNING]line has already been printed, back inFunctionRunner.Filter()ininternal/fnruntime/runner.go. The printing there serves double duty for both Image and Exec runs - it'd be a lot of extra work (and repetition in wasm.go and exec.go) to split it out to a point where we can wait to print until we've resolved the actual image. - Perhaps a redesign where we pull the use of
ResolveFunctionImage()back up to the creation of theContainerFnfat runner.go line 108? That would probably work, but it seems like it would break the separation of "part where we're setting up and configuring" from "part where we start going on the network and actually running things". Let me know anyway. :)
There was a problem hiding this comment.
No, doesn't matter where the functionality lives, but just looking at the sample [RUNNING] output in some of the tests, I don't think it's that useful. ie [RUNNING] "ghcr.io/kptdev/krm-functions-catalog/set-namespace:0.4.1 - 0.4.3"
If we can get the exact tag/digest, I think it would be more useful for the user.
There was a problem hiding this comment.
Ah - fair enough. Reworked to bring it up to runner.go and added a new line type, [RESOLUTION FAIL].
example:
Package: "nginx"
[RUNNING] "ghcr.io/kptdev/krm-functions-catalog/kubeconform:latest"
[PASS] "ghcr.io/kptdev/krm-functions-catalog/kubeconform:latest" in 2.4s
Successfully executed 1 function(s) in 1 package(s)
Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
- also linting changes - also minor AI-suggested refactors for better readbility Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
- when `tag` present and image resolution errors out, add `tag` to printed image name - also new e2e test for this specific failure - reuse pre-run display name building logic Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
- minor Copilot review comments Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
- also addresses Copilot comment Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
- allows us to have exact resolved image version tags in [RUNNING] output lines as well as [PASS]/[FAIL] - add new [RESOLUTION FAIL] line type to account for image resolution failure Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
bffca20 to
0c54533
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: JamesMcDermott <james.j.mcdermott@ericsson.com>
Changes:
updaterunner.goso the display name in the printed output appends the function image'stagattribute (replacing any tag in the existingimageattribute), OR leaves an existing tag in theimageattribute if notagattribute, OR defaults to "latest", in that order of preferencerunner.go). This allows the exact image version tag to be included in "[RUNNING]" lines as wellcontainer.goto replace the display name in the printed output with the resolved image that was run, whether successfully or nottagattributeMotivation:
When using the
tagattribute (with or without a semantic version constraint) or when no version tag is included in eithertagorimage(defaulting to "latest"), function pipeline output does not usually include the version tag of the executed image.Fixes: #4410