Skip to content

Commit

Permalink
Make KEP and implementation proposal consistent
Browse files Browse the repository at this point in the history
  • Loading branch information
voelzmo committed Oct 31, 2022
1 parent f74e054 commit 7f3ef8c
Showing 1 changed file with 6 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,18 @@ For some workloads, each eviction introduces disruptions for users. Examples inc
* Allow for resource-specific decisions: The desired policy may be different for CPU and Memory

## Proposal
Add a new field `EvictionRequirements` to [`PodUpdatePolicy`](https://github.com/kubernetes/autoscaler/blob/2f4385b72e304216cf745893747da45ef314898f/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go#L109) of type `[]EvictionRequirement`. A single `EvictionRequirement` defines a condition which must be `true` to allow eviction for the corresponding `Pod`. When multiple `EvictionRequirements` are specified for a `Pod`, all of them must evaluate to `true` to allow eviction.
Add a new field `EvictionRequirements` to [`PodUpdatePolicy`](https://github.com/kubernetes/autoscaler/blob/2f4385b72e304216cf745893747da45ef314898f/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go#L109) of type `[]*EvictionRequirement`. A single `EvictionRequirement` defines a condition which must be `true` to allow eviction for the corresponding `Pod`. When multiple `EvictionRequirements` are specified for a `Pod`, all of them must evaluate to `true` to allow eviction.

A single `EvictionRequirement` refers to a `Resource` and defines a `ChangeRequirement` comparing the new recommendation (`Target`) with the existing requests on a Pod (`Requests`). Possible values for `Resource` are `CPU`, `Memory` or `Any`. Possible values for `ChangeRequirement` are `TargetHigherThanRequests`, `TargetHigherThanOrEqualToRequests`, `TargetLowerThanRequests` and `TargetLowerThanOrEqualToRequests`.
A single `EvictionRequirement` specifices `Resources` and a `ChangeRequirement` comparing the new recommendation (`Target`) with the existing requests on a Pod (`Requests`). Possible values for `Resources` are `[CPU]` and `[Memory]` or both `[CPU,Memory]`. If `Resources: [CPU, Memory]`, the condition must be true for either of the two resources to allow for eviction. Possible values for `ChangeRequirement` are `TargetHigherThanRequests`, `TargetHigherThanOrEqualToRequests`, `TargetLowerThanRequests` and `TargetLowerThanOrEqualToRequests`.

Add validation to prevent users from adding `EvictionRequirements` which can never evaluate to `true`:
* Reject if more than one `EvictionRequirement` for `Resource: CPU` or `Resource: Memory` is found
* Reject if `Resource: Any` is specified together with `Resource: CPU` or `Resource: Memory`
* Reject if more than one `EvictionRequirement` for a single resource is found
* Reject if `Resource: [CPU, Memory]` is specified on one `EvictionRequirement` together with `Resource: [CPU]` or `Resource: [Memory]` on another `EvictionRequirement`

## Design Details
### Test Plan
* Add automated E2E tests verifying that VPA only evicts when `ChangingRequirements` evaluate to `true`
* Add automated E2E tests verifying that VPA only evicts when all `EvictionRequirements` evaluate to `true`
* Keep existing tests which verify that nothing changes when users don't specify any `EvictionRequirements`
* Add tests verifying the validation

## Alternatives
Expand Down

0 comments on commit 7f3ef8c

Please sign in to comment.