From 7be12a142ca415af062a4c3791ec8f9a8ede1e09 Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Wed, 27 Apr 2022 17:26:28 +0200 Subject: [PATCH 1/8] Add KEP to introduce UpdateMode: UpscaleOnly --- .../enhancements/4831-upscale-only/README.md | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md diff --git a/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md b/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md new file mode 100644 index 000000000000..ba5f05a6c51d --- /dev/null +++ b/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md @@ -0,0 +1,39 @@ +# KEP-4831: VPA 'upscaling only' UpdateMode + + +- [Summary](#summary) +- [Motivation](#motivation) + - [Goals](#goals) + - [Non-Goals](#non-goals) +- [Proposal](#proposal) +- [Design Details](#design-details) + - [Test Plan](#test-plan) +- [Alternatives](#alternatives) + + +## Summary +VPA provides different `UpdateMode`s, describing if/how the VPA updater applies new recommendations to the Pods. They range from "recommendation only, don't make any changes" (`Off`), over "apply new recommendations when pods are re-scheduled, but don't evict" (`Initial`) to "evict a Pod to apply a new recommendation" (`Auto`). These existing UpdateModes work in the same way for scaling up and scaling down. We propose adding a new `UpdateMode` that disables active downscaling while still allowing for scaling up. + +## Motivation +For some workloads, each scaling operation introduces disruptions for users. Examples include anything running as a singleton (e.g. etcd) or workload that needs to persist data on each eviction (e.g. prometheus). Therefore, operators may want to limit the amount of scaling events without causing disruptions due to insufficient resources. This introduces a new tradeoff: Overspend on resources to increase uptime. It is possible to turn off active downscaling entirely or have it managed by an external component to allow downscaling only during certain time windows, days, or based on other criteria. + + +### Goals +* For a specific VPA object, allow to turn off active downscaling entirely while still allow for upscaling +* Allow for changes of `UpdateMode` during the lifetime of a VPA object +### Non-goals +* Have VPA take care of dynamically changing the `UpdateMode`: Reasons and already existing mechanism achieving a similar functionality will vary widely, this is not what VPA should be concerned with + +## Proposal +Add a new `UpdateMode` called `UpscaleOnly`, which works similar to `Auto` when scaling up, allowing the VPA to evict Pods and increase their resource requests, but prevents the VPA from evicting Pods when the new recommendation is smaller than the current one. Similarly to how the `Initial` mode works, this means that a smaller recommendation can still be applied to a Pod if it is recreated due to other reasons. + +Since the change is backward-compatible the suggestion is to extend `v1` version of VPA API, avoiding the hassle of introducing a new API version. + +## Design Details +### Test Plan +Add automated E2E tests with `spec.updatePolicy.updateMode: "UpscaleOnly"` verify that the VPA Updater does evict for higher recommendations, but not for lower ones. + +## Alternatives +### Run VPA in recommendation only mode +We currently achieve a similar functionality by running VPA in `spec.updatePolicy.updateMode: "Off"` and having a different controller inspect the recommendations and apply them based on certain criteria like scaling direction and time. +With this approach you either end up re-building half of the VPA (updater/webhook), or use a different mechanism to apply the recommendations, such as modifying the requests in the Pod owning Object – which has its own drawbacks. \ No newline at end of file From 93f5a8e30f1dc8bf87fcf3516de0af36c818b72a Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Mon, 2 May 2022 10:41:08 +0200 Subject: [PATCH 2/8] Clarify prometheus use-case --- .../enhancements/4831-upscale-only/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md b/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md index ba5f05a6c51d..726174df778c 100644 --- a/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md +++ b/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md @@ -15,7 +15,7 @@ VPA provides different `UpdateMode`s, describing if/how the VPA updater applies new recommendations to the Pods. They range from "recommendation only, don't make any changes" (`Off`), over "apply new recommendations when pods are re-scheduled, but don't evict" (`Initial`) to "evict a Pod to apply a new recommendation" (`Auto`). These existing UpdateModes work in the same way for scaling up and scaling down. We propose adding a new `UpdateMode` that disables active downscaling while still allowing for scaling up. ## Motivation -For some workloads, each scaling operation introduces disruptions for users. Examples include anything running as a singleton (e.g. etcd) or workload that needs to persist data on each eviction (e.g. prometheus). Therefore, operators may want to limit the amount of scaling events without causing disruptions due to insufficient resources. This introduces a new tradeoff: Overspend on resources to increase uptime. It is possible to turn off active downscaling entirely or have it managed by an external component to allow downscaling only during certain time windows, days, or based on other criteria. +For some workloads, each scaling operation introduces disruptions for users. Examples include anything running as a singleton (e.g. etcd) or workload that needs a lot of time to process data on startup after each eviction (e.g. prometheus). Therefore, operators may want to limit the amount of scaling events without causing disruptions due to insufficient resources. This introduces a new tradeoff: Overspend on resources to increase uptime. It is possible to turn off active downscaling entirely or have it managed by an external component to allow downscaling only during certain time windows, days, or based on other criteria. ### Goals @@ -36,4 +36,4 @@ Add automated E2E tests with `spec.updatePolicy.updateMode: "UpscaleOnly"` verif ## Alternatives ### Run VPA in recommendation only mode We currently achieve a similar functionality by running VPA in `spec.updatePolicy.updateMode: "Off"` and having a different controller inspect the recommendations and apply them based on certain criteria like scaling direction and time. -With this approach you either end up re-building half of the VPA (updater/webhook), or use a different mechanism to apply the recommendations, such as modifying the requests in the Pod owning Object – which has its own drawbacks. \ No newline at end of file +With this approach you either end up re-building half of the VPA (updater/webhook), or use a different mechanism to apply the recommendations, such as modifying the requests in the Pod owning Object – which has its own drawbacks. From aabb09a9f229f06f7434585e3c420058c95064fa Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Mon, 30 May 2022 15:37:23 +0200 Subject: [PATCH 3/8] Adapt to review comments --- .../enhancements/4831-upscale-only/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md b/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md index 726174df778c..13220ccd9614 100644 --- a/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md +++ b/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md @@ -25,7 +25,7 @@ For some workloads, each scaling operation introduces disruptions for users. Exa * Have VPA take care of dynamically changing the `UpdateMode`: Reasons and already existing mechanism achieving a similar functionality will vary widely, this is not what VPA should be concerned with ## Proposal -Add a new `UpdateMode` called `UpscaleOnly`, which works similar to `Auto` when scaling up, allowing the VPA to evict Pods and increase their resource requests, but prevents the VPA from evicting Pods when the new recommendation is smaller than the current one. Similarly to how the `Initial` mode works, this means that a smaller recommendation can still be applied to a Pod if it is recreated due to other reasons. +Add a new `UpdateMode` called `UpscaleOnly`, which works similar to `Auto` when scaling up, allowing the VPA to evict Pods when `target > current requests` for at least one of the `controlledResources`, but prevents the VPA from evicting Pods when `target <= current requests` for all `controlledResources`. Similarly to how the `Initial` mode works, this means that a smaller recommendation can still be applied to a Pod if it is recreated due to other reasons. Since the change is backward-compatible the suggestion is to extend `v1` version of VPA API, avoiding the hassle of introducing a new API version. From 16865417b33bc60e38d0d9fb89cc5db2376a5671 Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Thu, 28 Jul 2022 14:20:54 +0200 Subject: [PATCH 4/8] Adapt KEP according to review --- .../enhancements/4831-upscale-only/README.md | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md b/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md index 13220ccd9614..012effd96aa2 100644 --- a/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md +++ b/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md @@ -1,4 +1,4 @@ -# KEP-4831: VPA 'upscaling only' UpdateMode +# KEP-4831: Control VPA eviction behavior based on scaling direction and resource - [Summary](#summary) @@ -12,27 +12,28 @@ ## Summary -VPA provides different `UpdateMode`s, describing if/how the VPA updater applies new recommendations to the Pods. They range from "recommendation only, don't make any changes" (`Off`), over "apply new recommendations when pods are re-scheduled, but don't evict" (`Initial`) to "evict a Pod to apply a new recommendation" (`Auto`). These existing UpdateModes work in the same way for scaling up and scaling down. We propose adding a new `UpdateMode` that disables active downscaling while still allowing for scaling up. +VPA provides different `UpdateMode`s, describing if/how the VPA applies new recommendations to the Pods. They range from "recommendation only, don't make any changes" (`Off`), over "apply new recommendations when pods are re-scheduled, but don't evict" (`Initial`) to "evict a Pod to apply a new recommendation" (`Auto`). These existing `UpdateModes` work in the same way for scaling up and scaling down and are the same for all resources controlled by the VPA. We propose adding a new functionality which allows to control the conditions in which Pods can be evicted to apply a new recommendation based on the changes (are we scaling up or scaling down?) for each resource individually. ## Motivation -For some workloads, each scaling operation introduces disruptions for users. Examples include anything running as a singleton (e.g. etcd) or workload that needs a lot of time to process data on startup after each eviction (e.g. prometheus). Therefore, operators may want to limit the amount of scaling events without causing disruptions due to insufficient resources. This introduces a new tradeoff: Overspend on resources to increase uptime. It is possible to turn off active downscaling entirely or have it managed by an external component to allow downscaling only during certain time windows, days, or based on other criteria. - +For some workloads, each eviction introduces disruptions for users. Examples include anything running as a singleton (e.g. etcd) or workload that needs a lot of time to process data on startup (e.g. prometheus). Therefore, operators may want to limit the amount of evictions. We can do this by introducing a new tradeoff: Overspend on resources to increase uptime. It is possible to turn off eviction for downscaling entirely or have it managed by an external component to allow downscaling only during certain time windows, days, or based on other criteria. ### Goals -* For a specific VPA object, allow to turn off active downscaling entirely while still allow for upscaling -* Allow for changes of `UpdateMode` during the lifetime of a VPA object -### Non-goals -* Have VPA take care of dynamically changing the `UpdateMode`: Reasons and already existing mechanism achieving a similar functionality will vary widely, this is not what VPA should be concerned with +* For a specific VPA object, allow to turn off eviction for downscaling entirely while still allow for upscaling +* Allow for resource-specific decisions: The desired policy may be different for CPU and Memory ## Proposal -Add a new `UpdateMode` called `UpscaleOnly`, which works similar to `Auto` when scaling up, allowing the VPA to evict Pods when `target > current requests` for at least one of the `controlledResources`, but prevents the VPA from evicting Pods when `target <= current requests` for all `controlledResources`. Similarly to how the `Initial` mode works, this means that a smaller recommendation can still be applied to a Pod if it is recreated due to other reasons. +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`. -Since the change is backward-compatible the suggestion is to extend `v1` version of VPA API, avoiding the hassle of introducing a new API version. +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` ## Design Details ### Test Plan -Add automated E2E tests with `spec.updatePolicy.updateMode: "UpscaleOnly"` verify that the VPA Updater does evict for higher recommendations, but not for lower ones. - +* Add automated E2E tests verifying that VPA only evicts when `ChangingRequirements` evaluate to `true` +* Add tests verifying the validation ## Alternatives ### Run VPA in recommendation only mode We currently achieve a similar functionality by running VPA in `spec.updatePolicy.updateMode: "Off"` and having a different controller inspect the recommendations and apply them based on certain criteria like scaling direction and time. From 9bac7d86a7581f5921ffcf9c6ea5bdbaa9928b20 Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Thu, 8 Sep 2022 14:16:38 +0200 Subject: [PATCH 5/8] Add newline after header --- .../4831-control-eviction-behavior/README.md | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md diff --git a/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md b/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md new file mode 100644 index 000000000000..345164e84b2f --- /dev/null +++ b/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md @@ -0,0 +1,41 @@ +# KEP-4831: Control VPA eviction behavior based on scaling direction and resource + + +- [Summary](#summary) +- [Motivation](#motivation) + - [Goals](#goals) + - [Non-Goals](#non-goals) +- [Proposal](#proposal) +- [Design Details](#design-details) + - [Test Plan](#test-plan) +- [Alternatives](#alternatives) + + +## Summary +VPA provides different `UpdateMode`s, describing if/how the VPA applies new recommendations to the Pods. They range from "recommendation only, don't make any changes" (`Off`), over "apply new recommendations when pods are re-scheduled, but don't evict" (`Initial`) to "evict a Pod to apply a new recommendation" (`Auto`). These existing `UpdateModes` work in the same way for scaling up and scaling down and are the same for all resources controlled by the VPA. We propose adding a new functionality which allows to control the conditions in which Pods can be evicted to apply a new recommendation based on the changes (are we scaling up or scaling down?) for each resource individually. + +## Motivation +For some workloads, each eviction introduces disruptions for users. Examples include anything running as a singleton (e.g. etcd) or workload that needs a lot of time to process data on startup (e.g. prometheus). Therefore, operators may want to limit the amount of evictions. We can do this by introducing a new tradeoff: Overspend on resources to increase uptime. It is possible to turn off eviction for downscaling entirely or have it managed by an external component to allow downscaling only during certain time windows, days, or based on other criteria. + +### Goals +* For a specific VPA object, allow to turn off eviction for downscaling entirely while still allow for upscaling +* 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. + +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`. + +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` + +## Design Details +### Test Plan +* Add automated E2E tests verifying that VPA only evicts when `ChangingRequirements` evaluate to `true` +* Add tests verifying the validation + +## Alternatives +### Run VPA in recommendation only mode +We currently achieve a similar functionality by running VPA in `spec.updatePolicy.updateMode: "Off"` and having a different controller inspect the recommendations and apply them based on certain criteria like scaling direction and time. +With this approach you either end up re-building half of the VPA (updater/webhook), or use a different mechanism to apply the recommendations, such as modifying the requests in the Pod owning Object – which has its own drawbacks. From f74e054e162f4309eab7532f12d4e672642db639 Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Thu, 8 Sep 2022 14:19:26 +0200 Subject: [PATCH 6/8] Rename proposal directory to fit KEP title --- .../enhancements/4831-upscale-only/README.md | 40 ------------------- 1 file changed, 40 deletions(-) delete mode 100644 vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md diff --git a/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md b/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md deleted file mode 100644 index 012effd96aa2..000000000000 --- a/vertical-pod-autoscaler/enhancements/4831-upscale-only/README.md +++ /dev/null @@ -1,40 +0,0 @@ -# KEP-4831: Control VPA eviction behavior based on scaling direction and resource - - -- [Summary](#summary) -- [Motivation](#motivation) - - [Goals](#goals) - - [Non-Goals](#non-goals) -- [Proposal](#proposal) -- [Design Details](#design-details) - - [Test Plan](#test-plan) -- [Alternatives](#alternatives) - - -## Summary -VPA provides different `UpdateMode`s, describing if/how the VPA applies new recommendations to the Pods. They range from "recommendation only, don't make any changes" (`Off`), over "apply new recommendations when pods are re-scheduled, but don't evict" (`Initial`) to "evict a Pod to apply a new recommendation" (`Auto`). These existing `UpdateModes` work in the same way for scaling up and scaling down and are the same for all resources controlled by the VPA. We propose adding a new functionality which allows to control the conditions in which Pods can be evicted to apply a new recommendation based on the changes (are we scaling up or scaling down?) for each resource individually. - -## Motivation -For some workloads, each eviction introduces disruptions for users. Examples include anything running as a singleton (e.g. etcd) or workload that needs a lot of time to process data on startup (e.g. prometheus). Therefore, operators may want to limit the amount of evictions. We can do this by introducing a new tradeoff: Overspend on resources to increase uptime. It is possible to turn off eviction for downscaling entirely or have it managed by an external component to allow downscaling only during certain time windows, days, or based on other criteria. - -### Goals -* For a specific VPA object, allow to turn off eviction for downscaling entirely while still allow for upscaling -* 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. - -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`. - -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` - -## Design Details -### Test Plan -* Add automated E2E tests verifying that VPA only evicts when `ChangingRequirements` evaluate to `true` -* Add tests verifying the validation -## Alternatives -### Run VPA in recommendation only mode -We currently achieve a similar functionality by running VPA in `spec.updatePolicy.updateMode: "Off"` and having a different controller inspect the recommendations and apply them based on certain criteria like scaling direction and time. -With this approach you either end up re-building half of the VPA (updater/webhook), or use a different mechanism to apply the recommendations, such as modifying the requests in the Pod owning Object – which has its own drawbacks. From 7f3ef8cd5f5e8bb4b916c24bcffedce5b28615cd Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Mon, 31 Oct 2022 10:54:14 +0100 Subject: [PATCH 7/8] Make KEP and implementation proposal consistent --- .../4831-control-eviction-behavior/README.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md b/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md index 345164e84b2f..8217359d23ac 100644 --- a/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md +++ b/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md @@ -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 From 6dae4ae664f5aa36d26b88fbd5e2a249b229f7c2 Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Thu, 3 Nov 2022 13:56:04 +0100 Subject: [PATCH 8/8] make spellchecker happy --- .../enhancements/4831-control-eviction-behavior/README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md b/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md index 8217359d23ac..29c0c4f9501b 100644 --- a/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md +++ b/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior/README.md @@ -4,7 +4,6 @@ - [Summary](#summary) - [Motivation](#motivation) - [Goals](#goals) - - [Non-Goals](#non-goals) - [Proposal](#proposal) - [Design Details](#design-details) - [Test Plan](#test-plan) @@ -24,7 +23,7 @@ For some workloads, each eviction introduces disruptions for users. Examples inc ## 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. -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`. +A single `EvictionRequirement` specifies `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 a single resource is found