fix: autoscaling annotation format and empty image guard#3903
fix: autoscaling annotation format and empty image guard#3903Elvand-Lie wants to merge 1 commit into
Conversation
Two bug fixes: 1. pkg/knative/deployer.go: setServiceOptions used fmt.Sprintf(%f) to format autoscaling Target and Utilization annotations, producing values like 100.000000 instead of 100. Changed to %g which strips trailing zeros (100.0 -> 100, 70.5 -> 70.5). 2. pkg/functions/function.go: ImageNameWithDigest did not guard against an empty Build.Image. When called with a non-empty digest but empty image name, it produced the invalid OCI reference @sha256:... (no image name prefix). Added early return for empty image. Also adds a regression test TestSetServiceOptions_ScaleAnnotationFormat that verifies the annotation values are clean integers.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Elvand-Lie The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
knative-prow
Bot
added
size/M
|
Hi @Elvand-Lie. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Closing in favor of separate PRs per bug fix. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3903 +/- ##
==========================================
- Coverage 53.93% 51.72% -2.21%
==========================================
Files 200 200
Lines 23681 23682 +1
==========================================
- Hits 12772 12250 -522
- Misses 9674 10284 +610
+ Partials 1235 1148 -87
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Description
Two bug fixes:
1. Autoscaling annotations formatted with
%fproduce invalid valuessetServiceOptionsinpkg/knative/deployer.gousesfmt.Sprintf("%f", ...)to format scaleTargetandUtilizationannotations, producing strings like"100.000000"instead of"100". Changed%fto%gwhich strips trailing zeros.Files changed:
pkg/knative/deployer.go(lines 592, 598)2.
ImageNameWithDigestproduces invalid OCI reference whenBuild.Imageis emptyNo guard against empty
Build.ImageinImageNameWithDigest. When called with non-empty digest but empty image, produces invalid@sha256:...reference. Added early return for empty image.Files changed:
pkg/functions/function.go(line 864)Regression test
Added
TestSetServiceOptions_ScaleAnnotationFormatthat verifies scale annotations contain clean values ("100"not"100.000000").Files changed:
pkg/knative/deployer_test.goVerification
go vet ./pkg/knative ./pkg/functions- cleango test ./pkg/knative/ -count=1- all 20 tests passgo test ./pkg/functions/ -run TestFunction_ImageWithDigest -count=1- all 5 tests pass