[WIP] Deprecate max-scale/min-scale annotations for consistent naming

[rtnpro]

Signed-off-by: Ratnadeep Debnath [email protected]

Fixes #12932

Proposed Changes

• Replace autoscaling.knative.dev/max-scale annotation with autoscaling.knative.dev/scale-max
• Replace autoscaling.knative.dev/min-scale annotation with autoscaling.knative.dev/scale-min

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[knative-prow[bot]]

Hi @rtnpro. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions 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/test-infra repository.

[psschwei]

/ok-to-test

[codecov[bot]]

Codecov Report

Base: 86.69% // Head: 86.45% // Decreases project coverage by -0.23% :warning:

Coverage data is based on head (3da90a3) compared to base (8fc6d87). Patch coverage: 94.44% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12972      +/-   ##
==========================================
- Coverage   86.69%   86.45%   -0.24%     
==========================================
  Files         196      196              
  Lines       14450    14565     +115     
==========================================
+ Hits        12527    12592      +65     
- Misses       1626     1674      +48     
- Partials      297      299       +2     

Impacted Files	Coverage Δ
pkg/autoscaler/config/config.go	97.81% <91.30%> (+0.08%)	:arrow_up:
pkg/apis/autoscaling/annotation_validation.go	95.23% <100.00%> (ø)
pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go	100.00% <100.00%> (ø)
pkg/reconciler/revision/resolve.go	85.00% <0.00%> (-7.50%)	:arrow_down:
pkg/http/handler/timeout.go	84.76% <0.00%> (-5.03%)	:arrow_down:
pkg/autoscaler/statserver/server.go	75.91% <0.00%> (-1.87%)	:arrow_down:
pkg/apis/config/defaults.go	85.24% <0.00%> (-0.47%)	:arrow_down:
pkg/queue/sharedmain/main.go	0.61% <0.00%> (-0.03%)	:arrow_down:
cmd/activator/main.go	0.00% <0.00%> (ø)
cmd/controller/main.go	0.00% <0.00%> (ø)
... and 17 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[rtnpro]

@psschwei As part of this work, we should also update the cluster wide max-scale & min-scale parameters and the related tests, shouldn't we?

https://github.com/knative/serving/blob/b6e6baa6dc6697d0e7ddb3a12925f329a1f5064c/config/core/configmaps/autoscaler.yaml

[nader-ziada]

@psschwei As part of this work, we should also update the cluster wide max-scale & min-scale parameters and the related tests, shouldn't we?

https://github.com/knative/serving/blob/b6e6baa6dc6697d0e7ddb3a12925f329a1f5064c/config/core/configmaps/autoscaler.yaml

hello @rtnpro, thanks for your PR

yes, you are correct, the max-scale & min-scale should be updated cluster wide and in tests, but also please note the old annotations should not be removed right away, they should be deprecated first. So we will need to support both, make the new one the default and put a deprecation notice on the old one

[rtnpro]

Hey @psschwei I need an approval to run the CI tests

[rtnpro]

@psschwei The unit tests when run using go test ./... run successfully in my local development environment. 🥳

[knative-prow[bot]]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rtnpro Once this PR has been reviewed and has the lgtm label, please assign psschwei for approval by writing /assign @psschwei in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• pkg/OWNERS
• pkg/apis/OWNERS
• test/OWNERS

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[rtnpro]

@dprotaso @psschwei @xtaje I have updated the tests here. I need a review of this pull request. Please let me know if I need to make some more changes to make this pull request merge ready.

[dprotaso]

/hold @xtaje's comments made me rethink this change

See: https://github.com/knative/serving/issues/12932#issuecomment-1308889358

[dprotaso]

Going to close this out as there hasn't been any feedback on the issue