Skip to content

Commit

Permalink
Apply suggestions from code review.
Browse files Browse the repository at this point in the history
Co-authored-by: Patryk Prus <[email protected]>
  • Loading branch information
jm-franc and pr00se committed Nov 13, 2024
1 parent 23c4ff1 commit df5b1b9
Showing 1 changed file with 26 additions and 10 deletions.
36 changes: 26 additions & 10 deletions keps/sig-autoscaling/4951-configurable-hpa-tolerance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ type HPAScalingRules struct {
```

This new tolerance will be taken into account in the autoscaling controller
[replica_calculator.go][]. The update to the logic is:
[replica_calculator.go][]. The current logic is:

```golang
if math.Abs(1.0-usageRatio) <= c.tolerance { /* ... */ }
Expand All @@ -257,11 +257,11 @@ It will be replaced by:
+ if (1.0-downTolerance) <= usageRatio && usageRatio <= (1.0+upTolerance) { /* ... */ }
```

Since the added field is optional and has a default value compatible with the
current autoscaling behavior, this feature can be added to the current API
Since the added field is optional and it's omission results in no change to the existing
autoscaling behavior, this feature can be added to the current API
version `pkg/apis/autoscaling/v2`.

The feature presented in this KEP only allows to tune an existing parameter, and
The feature presented in this KEP only allows users to tune an existing parameter, and
as such doesn't require any new HPA Events or modify any Status. A new error is
emitted if a `tolerance` field is set to a negative value.

Expand Down Expand Up @@ -426,13 +426,15 @@ in back-to-back releases.

### Upgrade / Downgrade Strategy

Upgrades present no particular issue: the new field won't be set and the HPA
will behave like it does today. Users can use the new feature by setting the
new `tolerance` field (provided the Feature Gate is enabled).
#### Upgrade
Existing HPAs will continue to work as they do today, using the global `horizontal-pod-autoscaler-tolerance`
value from the `kube-controller-manager`. Users can use the new feature by enabling the Feature
Gate (alpha only) and setting the new `tolerance` field on an HPA.

On downgrades to a version that does not support this functionality, an HPA will
ignore any configured `tolerance` field, and use the default (as specified by
`--horizontal-pod-autoscaler-tolerance`).
#### Downgrade
On downgrade, all HPAs will revert to using the global `horizontal-pod-autoscaler-tolerance`
value from the `kube-controller-manager`, regardless of any configured `tolerance` value on the HPA
itself.

### Version Skew Strategy

Expand Down Expand Up @@ -495,6 +497,8 @@ Any change of default behavior may be surprising to users or break existing
automations, so be extremely careful here.
-->

No.

###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)?

<!--
Expand All @@ -508,8 +512,14 @@ feature.
NOTE: Also set `disable-supported` to `true` or `false` in `kep.yaml`.
-->

The feature can be disabled by restarting the `kube-controller-manager` with the feature gate set to `false`.

Any `tolerance` values set on existing HPAs will be ignored by the `kube-controller-manager` when the feature gate is off.

###### What happens if we reenable the feature if it was previously rolled back?

When the feature is re-enabled, any HPAs with configured `tolerance` values will use those when calculating replica counts, rather than the global tolerance from the `kube-controller-manager`.

###### Are there any tests for feature enablement/disablement?

<!--
Expand Down Expand Up @@ -687,6 +697,8 @@ Focusing mostly on:
heartbeats, leader election, etc.)
-->

No.

###### Will enabling / using this feature result in introducing new API types?

<!--
Expand All @@ -696,6 +708,8 @@ Describe them, providing:
- Supported number of objects per namespace (for namespace-scoped objects)
-->

No.

###### Will enabling / using this feature result in any new calls to the cloud provider?

<!--
Expand All @@ -704,6 +718,8 @@ Describe them, providing:
- Estimated increase:
-->

No.

###### Will enabling / using this feature result in increasing size or count of the existing API objects?

<!--
Expand Down

0 comments on commit df5b1b9

Please sign in to comment.