Fix error handling in package release cli #1553
Conversation
There was a problem hiding this comment.
Thank you so much for the fix @goshkis!
Do you think you would be able to add a test for this?
| err = o.releaseResources(appSpec, *pkgBuild, &pkgConfigs.Pkgs[0], &pkgConfigs.PkgMetadatas[0]) | ||
| if err != nil { | ||
| return nil | ||
| return err | ||
| } |
There was a problem hiding this comment.
nit: We can just do a return directly
return o.releaseResources(appSpec, *pkgBuild, &pkgConfigs.Pkgs[0], &pkgConfigs.PkgMetadatas[0])There was a problem hiding this comment.
It's not the last call in the Run(), so proposed change will skip o.printNextSteps() below.
Signed-off-by: Egor Pak <goshkis@gmail.com>
I looked at the tests in the repo, and think this will take some time for me to understand how to add the new test properly. |
We are ok with skipping the test for now |
What this PR does / why we need it:
kctrl package release ... --repo-output ...command is silently ignoring errors from releaseResources function.Say, if incompatible ytt version is installed, this command will not yield any errors but no content will be written to the destination.
The bug seems to be in a simple mistake unconditionally returning nil in case of any errors
Which issue(s) this PR fixes:
Fixes #1552
Does this PR introduce a user-facing change?
Additional Notes for your reviewer:
Review Checklist:
a link to that PR
change
Additional documentation e.g., Proposal, usage docs, etc.: