diff --git a/vertical-pod-autoscaler/RELEASE.md b/vertical-pod-autoscaler/RELEASE.md index 9b7fc80b981..19d5b1c0ae0 100644 --- a/vertical-pod-autoscaler/RELEASE.md +++ b/vertical-pod-autoscaler/RELEASE.md @@ -9,7 +9,7 @@ Before doing the release for the first time check if you have all the necessary There are the following steps of the release process: 1. [ ] Open issue to track the release. -2. [ ] Update VPA version const. +2. [ ] Rollup all changes. 3. [ ] Build and stage images. 4. [ ] Test the release. 5. [ ] Promote image. @@ -20,7 +20,7 @@ There are the following steps of the release process: Open a new issue to track the release, use the [vpa_release](https://github.com/kubernetes/autoscaler/issues/new?&template=vpa_release.md) template. We use the issue to communicate what is state of the release. -## Update VPA version const +## Rollup all changes 1. [ ] Wait for all VPA changes that will be in the release to merge. 2. [ ] Wait for [the end to end tests](https://testgrid.k8s.io/sig-autoscaling-vpa) to run with all VPA changes @@ -31,13 +31,12 @@ We use the issue to communicate what is state of the release. ### New minor release -1. [ ] Change the version in - [common/version-go](https://github.com/kubernetes/autoscaler/blob/master/vertical-pod-autoscaler/common/version.go) - to `1.${next-minor}.0`, -2. [ ] Commit and merge the change, -3. [ ] Go to the merged change, -4. [ ] [Create a new branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-and-deleting-branches-within-your-repository) named `vpa-release-1.${next-minor}` from the +1. [ ] [Create a new branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-and-deleting-branches-within-your-repository) named `vpa-release-1.${next-minor}` from the merged change. +2. [ ] In the **main branch**, change the version in + [common/version-go](https://github.com/kubernetes/autoscaler/blob/master/vertical-pod-autoscaler/common/version.go) + to `1.${next-minor}.0`. +3. [ ] Commit and merge the change. ### New patch release diff --git a/vertical-pod-autoscaler/common/version.go b/vertical-pod-autoscaler/common/version.go index cfd4803cd8e..06a88725a66 100644 --- a/vertical-pod-autoscaler/common/version.go +++ b/vertical-pod-autoscaler/common/version.go @@ -21,7 +21,7 @@ package common var gitCommit = "" // versionCore is the version of VPA. -const versionCore = "1.3.0" +const versionCore = "1.4.0" // VerticalPodAutoscalerVersion returns the version of the VPA. func VerticalPodAutoscalerVersion() string { diff --git a/vertical-pod-autoscaler/deploy/admission-controller-deployment.yaml b/vertical-pod-autoscaler/deploy/admission-controller-deployment.yaml index 462a7df16d4..c324b65a070 100644 --- a/vertical-pod-autoscaler/deploy/admission-controller-deployment.yaml +++ b/vertical-pod-autoscaler/deploy/admission-controller-deployment.yaml @@ -47,15 +47,3 @@ spec: - name: tls-certs secret: secretName: vpa-tls-certs ---- -apiVersion: v1 -kind: Service -metadata: - name: vpa-webhook - namespace: kube-system -spec: - ports: - - port: 443 - targetPort: 8000 - selector: - app: vpa-admission-controller diff --git a/vertical-pod-autoscaler/deploy/admission-controller-service.yaml b/vertical-pod-autoscaler/deploy/admission-controller-service.yaml new file mode 100644 index 00000000000..8239dca15b6 --- /dev/null +++ b/vertical-pod-autoscaler/deploy/admission-controller-service.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: Service +metadata: + name: vpa-webhook + namespace: kube-system +spec: + ports: + - port: 443 + targetPort: 8000 + selector: + app: vpa-admission-controller diff --git a/vertical-pod-autoscaler/deploy/vpa-rbac.yaml b/vertical-pod-autoscaler/deploy/vpa-rbac.yaml index 0ff58467b8b..fda8cec53b5 100644 --- a/vertical-pod-autoscaler/deploy/vpa-rbac.yaml +++ b/vertical-pod-autoscaler/deploy/vpa-rbac.yaml @@ -121,6 +121,32 @@ rules: - create --- apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: system:vpa-updater-in-place +rules: + - apiGroups: + - "" + resources: + - pods/resize + - pods # required for patching vpaInPlaceUpdated annotations onto the pod + verbs: + - patch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: system:vpa-updater-in-place-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: system:vpa-updater-in-place +subjects: + - kind: ServiceAccount + name: vpa-updater + namespace: kube-system +--- +apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: name: system:metrics-reader diff --git a/vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml b/vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml index 70345adcccd..70adb552bb3 100644 --- a/vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml +++ b/vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml @@ -458,6 +458,7 @@ spec: - "Off" - Initial - Recreate + - InPlaceOrRecreate - Auto type: string type: object diff --git a/vertical-pod-autoscaler/docs/api.md b/vertical-pod-autoscaler/docs/api.md index 53863e326d5..f7e03b0611c 100644 --- a/vertical-pod-autoscaler/docs/api.md +++ b/vertical-pod-autoscaler/docs/api.md @@ -155,7 +155,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `updateMode` _[UpdateMode](#updatemode)_ | Controls when autoscaler applies changes to the pod resources.
The default is 'Auto'. | | Enum: [Off Initial Recreate Auto]
| +| `updateMode` _[UpdateMode](#updatemode)_ | Controls when autoscaler applies changes to the pod resources.
The default is 'Auto'. | | Enum: [Off Initial Recreate InPlaceOrRecreate Auto]
| | `minReplicas` _integer_ | Minimal number of replicas which need to be alive for Updater to attempt
pod eviction (pending other checks like PDB). Only positive values are
allowed. Overrides global '--min-replicas' flag. | | | | `evictionRequirements` _[EvictionRequirement](#evictionrequirement) array_ | EvictionRequirements is a list of EvictionRequirements that need to
evaluate to true in order for a Pod to be evicted. If more than one
EvictionRequirement is specified, all of them need to be fulfilled to allow eviction. | | | @@ -208,7 +208,7 @@ _Underlying type:_ _string_ UpdateMode controls when autoscaler applies changes to the pod resources. _Validation:_ -- Enum: [Off Initial Recreate Auto] +- Enum: [Off Initial Recreate InPlaceOrRecreate Auto] _Appears in:_ - [PodUpdatePolicy](#podupdatepolicy) @@ -218,7 +218,8 @@ _Appears in:_ | `Off` | UpdateModeOff means that autoscaler never changes Pod resources.
The recommender still sets the recommended resources in the
VerticalPodAutoscaler object. This can be used for a "dry run".
| | `Initial` | UpdateModeInitial means that autoscaler only assigns resources on pod
creation and does not change them during the lifetime of the pod.
| | `Recreate` | UpdateModeRecreate means that autoscaler assigns resources on pod
creation and additionally can update them during the lifetime of the
pod by deleting and recreating the pod.
| -| `Auto` | UpdateModeAuto means that autoscaler assigns resources on pod creation
and additionally can update them during the lifetime of the pod,
using any available update method. Currently this is equivalent to
Recreate, which is the only available update method.
| +| `Auto` | UpdateModeAuto means that autoscaler assigns resources on pod creation
and additionally can update them during the lifetime of the pod,
using any available update method. Currently this is equivalent to
Recreate.
| +| `InPlaceOrRecreate` | UpdateModeInPlaceOrRecreate means that autoscaler tries to assign resources in-place.
If this is not possible (e.g., resizing takes too long or is infeasible), it falls back to the
"Recreate" update mode.
Requires VPA level feature gate "InPlaceOrRecreate" to be enabled
on the admission and updater pods.
Requires cluster feature gate "InPlacePodVerticalScaling" to be enabled.
| #### VerticalPodAutoscaler diff --git a/vertical-pod-autoscaler/docs/features.md b/vertical-pod-autoscaler/docs/features.md index bc1f8d90f48..b97ddc7cee1 100644 --- a/vertical-pod-autoscaler/docs/features.md +++ b/vertical-pod-autoscaler/docs/features.md @@ -4,6 +4,8 @@ - [Limits control](#limits-control) - [Memory Value Humanization](#memory-value-humanization) +- [CPU Recommendation Rounding](#cpu-recommendation-rounding) +- [In-Place Updates](#in-place-updates-inplaceorrecreate) ## Limits control @@ -50,4 +52,78 @@ To enable this feature, set the --round-cpu-millicores flag when running the VPA ```bash --round-cpu-millicores=50 -``` \ No newline at end of file +``` + +## In-Place Updates (`InPlaceOrRecreate`) + +> [!WARNING] +> FEATURE STATE: VPA v1.4.0 [alpha] + +VPA supports in-place updates to reduce disruption when applying resource recommendations. This feature leverages Kubernetes' in-place update capabilities (which is in beta as of Kubernetes 1.33) to modify container resources without requiring pod recreation. +For more information, see [AEP-4016: Support for in place updates in VPA](https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler/enhancements/4016-in-place-updates-support) + +### Usage + +To use in-place updates, set the VPA's `updateMode` to `InPlaceOrRecreate`: +```yaml +apiVersion: autoscaling.k8s.io/v1 +kind: VerticalPodAutoscaler +metadata: + name: my-vpa +spec: + updatePolicy: + updateMode: "InPlaceOrRecreate" +``` + +### Behavior + +When using `InPlaceOrRecreate` mode, VPA will first attempt to apply updates in-place, if in-place update fails, VPA will fall back to pod recreation. +Updates are attempted when: +* Container requests are outside the recommended bounds +* Quick OOM occurs +* For long-running pods (>12h), when recommendations differ significantly (>10%) + +Important Notes + +* Disruption Possibility: While in-place updates aim to minimize disruption, they cannot guarantee zero disruption as the container runtime is responsible for the actual resize operation. + +* Memory Limit Downscaling: In the beta version, memory limit downscaling is not supported for pods with resizePolicy: PreferNoRestart. In such cases, VPA will fall back to pod recreation. + +### Requirements: + +* Kubernetes 1.33+ with `InPlacePodVerticalScaling` feature gate enabled +* VPA version 1.4.0+ with `InPlaceOrRecreate` feature gate enabled + +### Configuration + +Enable the feature by setting the following flags in VPA components ( for both updater and admission-controller ): + +```bash +--feature-gates=InPlaceOrRecreate=true +``` + +### Limitations + +* All containers in a pod are updated together (partial updates not supported) +* Memory downscaling requires careful consideration to prevent OOMs +* Updates still respect VPA's standard update conditions and timing restrictions +* In-place updates will fail if they would result in a change to the pod's QoS class + +### Fallback Behavior + +VPA will fall back to pod recreation in the following scenarios: + +* In-place update is [infeasible](https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/1287-in-place-update-pod-resources/README.md#resize-status) (node resources, etc.) +* Update is [deferred](https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/1287-in-place-update-pod-resources/README.md#resize-status) for more than 5 minutes +* Update is in progress for more than 1 hour +* [Pod QoS](https://kubernetes.io/docs/concepts/workloads/pods/pod-qos/) class would change due to the update +* Memory limit downscaling is required with [PreferNoRestart policy](https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/1287-in-place-update-pod-resources/README.md#container-resize-policy) + +### Monitoring + +VPA provides metrics to track in-place update operations: + +* `vpa_in_place_updatable_pods_total`: Number of pods matching in-place update criteria +* `vpa_in_place_updated_pods_total`: Number of pods successfully updated in-place +* `vpa_vpas_with_in_place_updatable_pods_total`: Number of VPAs with pods eligible for in-place updates +* `vpa_vpas_with_in_place_updated_pods_total`: Number of VPAs with successfully in-place updated pods \ No newline at end of file diff --git a/vertical-pod-autoscaler/docs/flags.md b/vertical-pod-autoscaler/docs/flags.md index cd222eed5e5..b66dfc4ee1c 100644 --- a/vertical-pod-autoscaler/docs/flags.md +++ b/vertical-pod-autoscaler/docs/flags.md @@ -12,6 +12,7 @@ This document is auto-generated from the flag definitions in the VPA admission-c | `--address` | ":8944" | The address to expose Prometheus metrics. | | `--alsologtostderr` | | log to standard error as well as files (no effect when -logtostderr=true) | | `--client-ca-file` | "/etc/tls-certs/caCert.pem" | Path to CA PEM file. | +| `--feature-gates` | | A set of key=value pairs that describe feature gates for alpha/experimental features. Options are: | | `--ignored-vpa-object-namespaces` | | A comma-separated list of namespaces to ignore when searching for VPA objects. Leave empty to avoid ignoring any namespaces. These namespaces will not be cleaned by the garbage collector. | | `--kube-api-burst` | 10 | QPS burst limit when making requests to Kubernetes apiserver | | `--kube-api-qps` | 5 | QPS limit when making requests to Kubernetes apiserver | @@ -65,6 +66,7 @@ This document is auto-generated from the flag definitions in the VPA recommender | `--cpu-integer-post-processor-enabled` | | Enable the cpu-integer recommendation post processor. The post processor will round up CPU recommendations to a whole CPU for pods which were opted in by setting an appropriate label on VPA object (experimental) | | `--external-metrics-cpu-metric` | | ALPHA. Metric to use with external metrics provider for CPU usage. | | `--external-metrics-memory-metric` | | ALPHA. Metric to use with external metrics provider for memory usage. | +| `--feature-gates` | | A set of key=value pairs that describe feature gates for alpha/experimental features. Options are: | | `--history-length` | "8d" | How much time back prometheus have to be queried to get historical metrics | | `--history-resolution` | "1h" | Resolution at which Prometheus is queried for historical metrics | | `--humanize-memory` | | Convert memory values in recommendations to the highest appropriate SI unit with up to 2 decimal places for better readability. | @@ -135,6 +137,7 @@ This document is auto-generated from the flag definitions in the VPA updater cod | `--eviction-rate-burst` | 1 | Burst of pods that can be evicted. | | `--eviction-rate-limit` | | Number of pods that can be evicted per seconds. A rate limit set to 0 or -1 will disable | | `--eviction-tolerance` | 0.5 | Fraction of replica count that can be evicted for update, if more than one pod can be evicted. | +| `--feature-gates` | | A set of key=value pairs that describe feature gates for alpha/experimental features. Options are: | | `--ignored-vpa-object-namespaces` | | A comma-separated list of namespaces to ignore when searching for VPA objects. Leave empty to avoid ignoring any namespaces. These namespaces will not be cleaned by the garbage collector. | | `--in-recommendation-bounds-eviction-lifetime-threshold` | 12h0m0s | Pods that live for at least that long can be evicted even if their request is within the [MinRecommended...MaxRecommended] range | | `--kube-api-burst` | 10 | QPS burst limit when making requests to Kubernetes apiserver | diff --git a/vertical-pod-autoscaler/docs/installation.md b/vertical-pod-autoscaler/docs/installation.md index 516a5866d7e..70268019eeb 100644 --- a/vertical-pod-autoscaler/docs/installation.md +++ b/vertical-pod-autoscaler/docs/installation.md @@ -138,6 +138,16 @@ To print YAML contents with all resources that would be understood by The output of that command won't include secret information generated by [pkg/admission-controller/gencerts.sh](https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler/pkg/admission-controller/gencerts.sh) script. +### Feature gates + +To install VPA with feature gates, you can specify the environment variable `$FEATURE_GATES`. + +For example, to enable the `InPlaceOrRecreate` feature gate: + +```console +FEATURE_GATES="InPlaceOrRecreate=true" ./hack/vpa-up.sh +``` + ## Tear down Note that if you stop running VPA in your cluster, the resource requests diff --git a/vertical-pod-autoscaler/e2e/go.mod b/vertical-pod-autoscaler/e2e/go.mod index 97b899c4597..c65221b4625 100644 --- a/vertical-pod-autoscaler/e2e/go.mod +++ b/vertical-pod-autoscaler/e2e/go.mod @@ -14,7 +14,7 @@ require ( k8s.io/apimachinery v0.32.0 k8s.io/autoscaler/vertical-pod-autoscaler v1.2.1 k8s.io/client-go v0.32.0 - k8s.io/component-base v0.32.0 + k8s.io/component-base v0.32.2 k8s.io/klog/v2 v2.130.1 k8s.io/kubernetes v1.32.0 k8s.io/pod-security-admission v0.32.0 diff --git a/vertical-pod-autoscaler/e2e/go.sum b/vertical-pod-autoscaler/e2e/go.sum index 61add5f6b00..0bd3a239118 100644 --- a/vertical-pod-autoscaler/e2e/go.sum +++ b/vertical-pod-autoscaler/e2e/go.sum @@ -92,6 +92,8 @@ github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= +github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= +github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/btree v1.0.1 h1:gK4Kx5IaGY9CD5sPJ36FHiBJ6ZXl0kilRiiCj+jdYp4= diff --git a/vertical-pod-autoscaler/e2e/v1/actuation.go b/vertical-pod-autoscaler/e2e/v1/actuation.go index 124d2c40822..be4be3502c9 100644 --- a/vertical-pod-autoscaler/e2e/v1/actuation.go +++ b/vertical-pod-autoscaler/e2e/v1/actuation.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/autoscaler/vertical-pod-autoscaler/e2e/utils" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + restriction "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/restriction" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/annotations" clientset "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/test/e2e/framework" @@ -50,6 +51,236 @@ import ( "github.com/onsi/gomega" ) +var _ = ActuationSuiteE2eDescribe("Actuation", ginkgo.Label("FG:InPlaceOrRecreate"), func() { + f := framework.NewDefaultFramework("vertical-pod-autoscaling") + f.NamespacePodSecurityEnforceLevel = podsecurity.LevelBaseline + + ginkgo.BeforeEach(func() { + checkInPlaceOrRecreateTestsEnabled(f, true, true) + }) + + ginkgo.It("still applies recommendations on restart when update mode is InPlaceOrRecreate", func() { + ginkgo.By("Setting up a hamster deployment") + SetupHamsterDeployment(f, "100m", "100Mi", defaultHamsterReplicas) + podList, err := GetHamsterPods(f) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + podSet := MakePodSet(podList) + + ginkgo.By("Setting up a VPA CRD in mode InPlaceOrRecreate") + containerName := GetHamsterContainerNameByIndex(0) + vpaCRD := test.VerticalPodAutoscaler(). + WithName("hamster-vpa"). + WithNamespace(f.Namespace.Name). + WithTargetRef(hamsterTargetRef). + WithUpdateMode(vpa_types.UpdateModeInPlaceOrRecreate). + WithContainer(containerName). + AppendRecommendation( + test.Recommendation(). + WithContainer(containerName). + WithTarget("200m", ""). + WithLowerBound("200m", ""). + WithUpperBound("200m", ""). + GetContainerResources()). + Get() + + InstallVPA(f, vpaCRD) + updatedCPURequest := ParseQuantityOrDie("200m") + + ginkgo.By(fmt.Sprintf("Waiting for pods to be evicted, hoping it won't happen, sleep for %s", VpaEvictionTimeout.String())) + CheckNoPodsEvicted(f, podSet) + ginkgo.By("Forcefully killing one pod") + killPod(f, podList) + + ginkgo.By("Checking that request was modified after forceful restart") + updatedPodList, _ := GetHamsterPods(f) + var foundUpdated int32 + for _, pod := range updatedPodList.Items { + podRequest := getCPURequest(pod.Spec) + framework.Logf("podReq: %v", podRequest) + if podRequest.Cmp(updatedCPURequest) == 0 { + foundUpdated += 1 + } + } + gomega.Expect(foundUpdated).To(gomega.Equal(defaultHamsterReplicas)) + }) + + // TODO: add e2e test to verify metrics are getting updated + ginkgo.It("applies in-place updates to all containers when update mode is InPlaceOrRecreate", func() { + ginkgo.By("Setting up a hamster deployment") + d := NewNHamstersDeployment(f, 2 /*number of containers*/) + d.Spec.Template.Spec.Containers[0].Resources.Requests = apiv1.ResourceList{ + apiv1.ResourceCPU: ParseQuantityOrDie("100m"), + apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"), + } + d.Spec.Template.Spec.Containers[1].Resources.Requests = apiv1.ResourceList{ + apiv1.ResourceCPU: ParseQuantityOrDie("100m"), + apiv1.ResourceMemory: ParseQuantityOrDie("100Mi"), + } + targetCPU := "200m" + targetMemory := "200Mi" + _ = startDeploymentPods(f, d) // 3 replicas + container1Name := GetHamsterContainerNameByIndex(0) + container2Name := GetHamsterContainerNameByIndex(1) + podList, err := GetHamsterPods(f) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + ginkgo.By("Setting up a VPA CRD") + vpaCRD := test.VerticalPodAutoscaler(). + WithName("hamster-vpa"). + WithNamespace(f.Namespace.Name). + WithTargetRef(hamsterTargetRef). + WithContainer(container1Name). + WithContainer(container2Name). + WithUpdateMode(vpa_types.UpdateModeInPlaceOrRecreate). + AppendRecommendation( + test.Recommendation(). + WithContainer(container1Name). + WithTarget(targetCPU, targetMemory). + WithLowerBound(targetCPU, targetMemory). + WithUpperBound(targetCPU, targetMemory). + GetContainerResources()). + AppendRecommendation( + test.Recommendation(). + WithContainer(container2Name). + WithTarget(targetCPU, targetMemory). + WithLowerBound(targetCPU, targetMemory). + WithUpperBound(targetCPU, targetMemory). + GetContainerResources()). + Get() + + InstallVPA(f, vpaCRD) + + ginkgo.By("Checking that resources were modified due to in-place update, not due to evictions") + err = WaitForPodsUpdatedWithoutEviction(f, podList) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + ginkgo.By("Checking that container resources were actually updated") + gomega.Eventually(func() error { + updatedPodList, err := GetHamsterPods(f) + if err != nil { + return err + } + for _, pod := range updatedPodList.Items { + for _, container := range pod.Status.ContainerStatuses { + cpuRequest := container.Resources.Requests[apiv1.ResourceCPU] + memoryRequest := container.Resources.Requests[apiv1.ResourceMemory] + if cpuRequest.Cmp(ParseQuantityOrDie(targetCPU)) != 0 { + framework.Logf("%v/%v has not been updated to %v yet: currently=%v", pod.Name, container.Name, targetCPU, cpuRequest.String()) + return fmt.Errorf("%s CPU request not updated", container.Name) + } + if memoryRequest.Cmp(ParseQuantityOrDie(targetMemory)) != 0 { + framework.Logf("%v/%v has not been updated to %v yet: currently=%v", pod.Name, container.Name, targetMemory, memoryRequest.String()) + return fmt.Errorf("%s Memory request not updated", container.Name) + } + } + } + return nil + }, VpaInPlaceTimeout*3, 15*time.Second).Should(gomega.Succeed()) + }) + + ginkgo.It("falls back to evicting pods when in-place update is Infeasible when update mode is InPlaceOrRecreate", func() { + ginkgo.By("Setting up a hamster deployment") + replicas := int32(2) + SetupHamsterDeployment(f, "100m", "100Mi", replicas) + podList, err := GetHamsterPods(f) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + ginkgo.By("Setting up a VPA CRD") + containerName := GetHamsterContainerNameByIndex(0) + updatedCPU := "999" // infeasible target + vpaCRD := test.VerticalPodAutoscaler(). + WithName("hamster-vpa"). + WithNamespace(f.Namespace.Name). + WithTargetRef(hamsterTargetRef). + WithContainer(containerName). + WithUpdateMode(vpa_types.UpdateModeInPlaceOrRecreate). + AppendRecommendation( + test.Recommendation(). + WithContainer(containerName). + WithTarget(updatedCPU, ""). + WithLowerBound("200m", ""). + WithUpperBound("200m", ""). + GetContainerResources()). + Get() + + InstallVPA(f, vpaCRD) + + ginkgo.By("Waiting for pods to be evicted") + err = WaitForPodsEvicted(f, podList) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }) + + ginkgo.It("falls back to evicting pods when resize is Deferred and more than 5 minute has elapsed since last in-place update when update mode is InPlaceOrRecreate", func() { + ginkgo.By("Setting up a hamster deployment") + replicas := int32(2) + SetupHamsterDeployment(f, "100m", "100Mi", replicas) + podList, err := GetHamsterPods(f) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + ginkgo.By("Setting up a VPA CRD") + containerName := GetHamsterContainerNameByIndex(0) + + // we can force deferred resize by setting the target CPU to the allocatable CPU of the node + // it will be close enough to the node capacity, such that the kubelet defers instead of marking it infeasible + nodeName := podList.Items[0].Spec.NodeName + node, err := f.ClientSet.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + allocatableCPU := node.Status.Allocatable[apiv1.ResourceCPU] + updatedCPU := allocatableCPU.String() + + vpaCRD := test.VerticalPodAutoscaler(). + WithName("hamster-vpa"). + WithNamespace(f.Namespace.Name). + WithTargetRef(hamsterTargetRef). + WithContainer(containerName). + WithUpdateMode(vpa_types.UpdateModeInPlaceOrRecreate). + AppendRecommendation( + test.Recommendation(). + WithContainer(containerName). + WithTarget(updatedCPU, ""). + WithLowerBound("200m", ""). + WithUpperBound("200m", ""). + GetContainerResources()). + Get() + + InstallVPA(f, vpaCRD) + + ginkgo.By("Waiting for status to be Deferred") + gomega.Eventually(func() error { + updatedPodList, err := GetHamsterPods(f) + if err != nil { + return err + } + for _, pod := range updatedPodList.Items { + if pod.Status.Resize == apiv1.PodResizeStatusDeferred { + return nil + } + } + return fmt.Errorf("status not deferred") + }, VpaInPlaceTimeout, 5*time.Second).Should(gomega.Succeed()) + + ginkgo.By("Making sure pods are not evicted yet") + gomega.Consistently(func() error { + updatedPodList, err := GetHamsterPods(f) + if err != nil { + return fmt.Errorf("failed to get pods: %v", err) + } + for _, pod := range updatedPodList.Items { + request := getCPURequestFromStatus(pod.Status) + if request.Cmp(ParseQuantityOrDie(updatedCPU)) == 0 { + framework.Logf("%v/%v updated to %v, that wasn't supposed to happen this early", pod.Name, containerName, updatedCPU) + return fmt.Errorf("%s CPU request should not have been updated", containerName) + } + } + return nil + }, restriction.DeferredResizeUpdateTimeout, 10*time.Second).Should(gomega.Succeed()) + + ginkgo.By("Waiting for pods to be evicted") + err = WaitForPodsEvicted(f, podList) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }) +}) + var _ = ActuationSuiteE2eDescribe("Actuation", func() { f := framework.NewDefaultFramework("vertical-pod-autoscaling") f.NamespacePodSecurityEnforceLevel = podsecurity.LevelBaseline @@ -519,6 +750,10 @@ func getCPURequest(podSpec apiv1.PodSpec) resource.Quantity { return podSpec.Containers[0].Resources.Requests[apiv1.ResourceCPU] } +func getCPURequestFromStatus(podStatus apiv1.PodStatus) resource.Quantity { + return podStatus.ContainerStatuses[0].Resources.Requests[apiv1.ResourceCPU] +} + func killPod(f *framework.Framework, podList *apiv1.PodList) { f.ClientSet.CoreV1().Pods(f.Namespace.Name).Delete(context.TODO(), podList.Items[0].Name, metav1.DeleteOptions{}) err := WaitForPodsRestarted(f, podList) diff --git a/vertical-pod-autoscaler/e2e/v1/admission_controller.go b/vertical-pod-autoscaler/e2e/v1/admission_controller.go index e3d526b3f06..502a07b8186 100644 --- a/vertical-pod-autoscaler/e2e/v1/admission_controller.go +++ b/vertical-pod-autoscaler/e2e/v1/admission_controller.go @@ -37,10 +37,62 @@ import ( "github.com/onsi/gomega" ) +const ( + webhookConfigName = "vpa-webhook-config" + webhookName = "vpa.k8s.io" +) + +var _ = AdmissionControllerE2eDescribe("Admission-controller", ginkgo.Label("FG:InPlaceOrRecreate"), func() { + f := framework.NewDefaultFramework("vertical-pod-autoscaling") + f.NamespacePodSecurityEnforceLevel = podsecurity.LevelBaseline + + ginkgo.BeforeEach(func() { + checkInPlaceOrRecreateTestsEnabled(f, true, false) + waitForVpaWebhookRegistration(f) + }) + + ginkgo.It("starts pods with new recommended request with InPlaceOrRecreate mode", func() { + d := NewHamsterDeploymentWithResources(f, ParseQuantityOrDie("100m") /*cpu*/, ParseQuantityOrDie("100Mi") /*memory*/) + + ginkgo.By("Setting up a VPA CRD") + containerName := GetHamsterContainerNameByIndex(0) + vpaCRD := test.VerticalPodAutoscaler(). + WithName("hamster-vpa"). + WithNamespace(f.Namespace.Name). + WithTargetRef(hamsterTargetRef). + WithContainer(containerName). + WithUpdateMode(vpa_types.UpdateModeInPlaceOrRecreate). + AppendRecommendation( + test.Recommendation(). + WithContainer(containerName). + WithTarget("250m", "200Mi"). + WithLowerBound("250m", "200Mi"). + WithUpperBound("250m", "200Mi"). + GetContainerResources()). + Get() + + InstallVPA(f, vpaCRD) + + ginkgo.By("Setting up a hamster deployment") + podList := startDeploymentPods(f, d) + + // Originally Pods had 100m CPU, 100Mi of memory, but admission controller + // should change it to recommended 250m CPU and 200Mi of memory. + for _, pod := range podList.Items { + gomega.Expect(pod.Spec.Containers[0].Resources.Requests[apiv1.ResourceCPU]).To(gomega.Equal(ParseQuantityOrDie("250m"))) + gomega.Expect(pod.Spec.Containers[0].Resources.Requests[apiv1.ResourceMemory]).To(gomega.Equal(ParseQuantityOrDie("200Mi"))) + } + }) +}) + var _ = AdmissionControllerE2eDescribe("Admission-controller", func() { f := framework.NewDefaultFramework("vertical-pod-autoscaling") f.NamespacePodSecurityEnforceLevel = podsecurity.LevelBaseline + ginkgo.BeforeEach(func() { + waitForVpaWebhookRegistration(f) + }) + ginkgo.It("starts pods with new recommended request", func() { d := NewHamsterDeploymentWithResources(f, ParseQuantityOrDie("100m") /*cpu*/, ParseQuantityOrDie("100Mi") /*memory*/) @@ -907,7 +959,6 @@ var _ = AdmissionControllerE2eDescribe("Admission-controller", func() { gomega.Expect(err).To(gomega.HaveOccurred(), "Invalid VPA object accepted") gomega.Expect(err.Error()).To(gomega.MatchRegexp(`.*admission webhook .*vpa.* denied the request: .*`), "Admission controller did not inspect the object") }) - }) func startDeploymentPods(f *framework.Framework, deployment *appsv1.Deployment) *apiv1.PodList { @@ -962,3 +1013,17 @@ func startDeploymentPods(f *framework.Framework, deployment *appsv1.Deployment) gomega.Expect(err).NotTo(gomega.HaveOccurred(), "when listing pods after deployment resize") return podList } + +func waitForVpaWebhookRegistration(f *framework.Framework) { + ginkgo.By("Waiting for VPA webhook registration") + gomega.Eventually(func() bool { + webhook, err := f.ClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations().Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) + if err != nil { + return false + } + if webhook != nil && len(webhook.Webhooks) > 0 && webhook.Webhooks[0].Name == webhookName { + return true + } + return false + }, 3*time.Minute, 5*time.Second).Should(gomega.BeTrue(), "Webhook was not registered in the cluster") +} diff --git a/vertical-pod-autoscaler/e2e/v1/common.go b/vertical-pod-autoscaler/e2e/v1/common.go index d1c479df900..80fb4db2046 100644 --- a/vertical-pod-autoscaler/e2e/v1/common.go +++ b/vertical-pod-autoscaler/e2e/v1/common.go @@ -20,6 +20,7 @@ import ( "context" "encoding/json" "fmt" + "strings" "time" ginkgo "github.com/onsi/ginkgo/v2" @@ -36,6 +37,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" vpa_clientset "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" clientset "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/test/e2e/framework" framework_deployment "k8s.io/kubernetes/test/e2e/framework/deployment" @@ -53,9 +55,15 @@ const ( // VpaEvictionTimeout is a timeout for VPA to restart a pod if there are no // mechanisms blocking it (for example PDB). VpaEvictionTimeout = 3 * time.Minute + // VpaInPlaceTimeout is a timeout for the VPA to finish in-place resizing a + // pod, if there are no mechanisms blocking it. + VpaInPlaceTimeout = 2 * time.Minute defaultHamsterReplicas = int32(3) defaultHamsterBackoffLimit = int32(10) + + // VpaNamespace is the default namespace that holds the all the VPA components. + VpaNamespace = "kube-system" ) var hamsterTargetRef = &autoscaling.CrossVersionObjectReference{ @@ -67,38 +75,35 @@ var hamsterTargetRef = &autoscaling.CrossVersionObjectReference{ var hamsterLabels = map[string]string{"app": "hamster"} // SIGDescribe adds sig-autoscaling tag to test description. -func SIGDescribe(text string, body func()) bool { - return ginkgo.Describe(fmt.Sprintf("[sig-autoscaling] %v", text), body) -} - -// E2eDescribe describes a VPA e2e test. -func E2eDescribe(scenario, name string, body func()) bool { - return SIGDescribe(fmt.Sprintf("[VPA] [%s] [v1] %s", scenario, name), body) +// Takes args that are passed to ginkgo.Describe. +func SIGDescribe(scenario, name string, args ...interface{}) bool { + full := fmt.Sprintf("[sig-autoscaling] [VPA] [%s] [v1] %s", scenario, name) + return ginkgo.Describe(full, args...) } // RecommenderE2eDescribe describes a VPA recommender e2e test. -func RecommenderE2eDescribe(name string, body func()) bool { - return E2eDescribe(recommenderComponent, name, body) +func RecommenderE2eDescribe(name string, args ...interface{}) bool { + return SIGDescribe(recommenderComponent, name, args...) } // UpdaterE2eDescribe describes a VPA updater e2e test. -func UpdaterE2eDescribe(name string, body func()) bool { - return E2eDescribe(updateComponent, name, body) +func UpdaterE2eDescribe(name string, args ...interface{}) bool { + return SIGDescribe(updateComponent, name, args...) } // AdmissionControllerE2eDescribe describes a VPA admission controller e2e test. -func AdmissionControllerE2eDescribe(name string, body func()) bool { - return E2eDescribe(admissionControllerComponent, name, body) +func AdmissionControllerE2eDescribe(name string, args ...interface{}) bool { + return SIGDescribe(admissionControllerComponent, name, args...) } // FullVpaE2eDescribe describes a VPA full stack e2e test. -func FullVpaE2eDescribe(name string, body func()) bool { - return E2eDescribe(fullVpaSuite, name, body) +func FullVpaE2eDescribe(name string, args ...interface{}) bool { + return SIGDescribe(fullVpaSuite, name, args...) } // ActuationSuiteE2eDescribe describes a VPA actuation e2e test. -func ActuationSuiteE2eDescribe(name string, body func()) bool { - return E2eDescribe(actuationSuite, name, body) +func ActuationSuiteE2eDescribe(name string, args ...interface{}) bool { + return SIGDescribe(actuationSuite, name, args...) } // GetHamsterContainerNameByIndex returns name of i-th hamster container. @@ -555,3 +560,108 @@ func InstallLimitRangeWithMin(f *framework.Framework, minCpuLimit, minMemoryLimi minMemoryLimitQuantity := ParseQuantityOrDie(minMemoryLimit) installLimitRange(f, &minCpuLimitQuantity, &minMemoryLimitQuantity, nil, nil, lrType) } + +// WaitForPodsUpdatedWithoutEviction waits for pods to be updated without any evictions taking place over the polling +// interval. +func WaitForPodsUpdatedWithoutEviction(f *framework.Framework, initialPods *apiv1.PodList) error { + framework.Logf("waiting for at least one pod to be updated without eviction") + err := wait.PollUntilContextTimeout(context.TODO(), pollInterval, VpaInPlaceTimeout, false, func(context.Context) (bool, error) { + podList, err := GetHamsterPods(f) + if err != nil { + return false, err + } + resourcesHaveDiffered := false + podMissing := false + for _, initialPod := range initialPods.Items { + found := false + for _, pod := range podList.Items { + if initialPod.Name == pod.Name { + found = true + for num, container := range pod.Status.ContainerStatuses { + for resourceName, resourceLimit := range container.Resources.Limits { + initialResourceLimit := initialPod.Status.ContainerStatuses[num].Resources.Limits[resourceName] + if !resourceLimit.Equal(initialResourceLimit) { + framework.Logf("%s/%s: %s limit status(%v) differs from initial limit spec(%v)", pod.Name, container.Name, resourceName, resourceLimit.String(), initialResourceLimit.String()) + resourcesHaveDiffered = true + } + } + for resourceName, resourceRequest := range container.Resources.Requests { + initialResourceRequest := initialPod.Status.ContainerStatuses[num].Resources.Requests[resourceName] + if !resourceRequest.Equal(initialResourceRequest) { + framework.Logf("%s/%s: %s request status(%v) differs from initial request spec(%v)", pod.Name, container.Name, resourceName, resourceRequest.String(), initialResourceRequest.String()) + resourcesHaveDiffered = true + } + } + } + } + } + if !found { + podMissing = true + } + } + if podMissing { + return true, fmt.Errorf("a pod was erroneously evicted") + } + if resourcesHaveDiffered { + framework.Logf("after checking %d pods, resources have started to differ for at least one of them", len(podList.Items)) + return true, nil + } + return false, nil + }) + framework.Logf("finished waiting for at least one pod to be updated without eviction") + return err +} + +// checkInPlaceOrRecreateTestsEnabled check for enabled feature gates in the cluster used for the +// InPlaceOrRecreate VPA feature. +// Use this in a "beforeEach" call before any suites that use InPlaceOrRecreate featuregate. +func checkInPlaceOrRecreateTestsEnabled(f *framework.Framework, checkAdmission, checkUpdater bool) { + ginkgo.By("Checking InPlacePodVerticalScaling cluster feature gate is on") + + podList, err := f.ClientSet.CoreV1().Pods("kube-system").List(context.TODO(), metav1.ListOptions{ + LabelSelector: "component=kube-apiserver", + }) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + apiServerPod := podList.Items[0] + gomega.Expect(apiServerPod.Spec.Containers).To(gomega.HaveLen(1)) + apiServerContainer := apiServerPod.Spec.Containers[0] + gomega.Expect(apiServerContainer.Name).To(gomega.Equal("kube-apiserver")) + if !anyContainsSubstring(apiServerContainer.Command, "InPlacePodVerticalScaling=true") { + ginkgo.Skip("Skipping suite: InPlacePodVerticalScaling feature gate is not enabled on the cluster level") + } + + if checkUpdater { + ginkgo.By("Checking InPlaceOrRecreate VPA feature gate is enabled for updater") + + deploy, err := f.ClientSet.AppsV1().Deployments(VpaNamespace).Get(context.TODO(), "vpa-updater", metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(deploy.Spec.Template.Spec.Containers).To(gomega.HaveLen(1)) + vpaUpdaterPod := deploy.Spec.Template.Spec.Containers[0] + gomega.Expect(vpaUpdaterPod.Name).To(gomega.Equal("updater")) + if !anyContainsSubstring(vpaUpdaterPod.Args, fmt.Sprintf("%s=true", string(features.InPlaceOrRecreate))) { + ginkgo.Skip("Skipping suite: InPlaceOrRecreate feature gate is not enabled for the VPA updater") + } + } + + if checkAdmission { + ginkgo.By("Checking InPlaceOrRecreate VPA feature gate is enabled for admission controller") + + deploy, err := f.ClientSet.AppsV1().Deployments(VpaNamespace).Get(context.TODO(), "vpa-admission-controller", metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(deploy.Spec.Template.Spec.Containers).To(gomega.HaveLen(1)) + vpaAdmissionPod := deploy.Spec.Template.Spec.Containers[0] + gomega.Expect(vpaAdmissionPod.Name).To(gomega.Equal("admission-controller")) + if !anyContainsSubstring(vpaAdmissionPod.Args, fmt.Sprintf("%s=true", string(features.InPlaceOrRecreate))) { + ginkgo.Skip("Skipping suite: InPlaceOrRecreate feature gate is not enabled for VPA admission controller") + } + } +} + +func anyContainsSubstring(arr []string, substr string) bool { + for _, s := range arr { + if strings.Contains(s, substr) { + return true + } + } + return false +} diff --git a/vertical-pod-autoscaler/e2e/v1/full_vpa.go b/vertical-pod-autoscaler/e2e/v1/full_vpa.go index f390ec5a7be..ec1467f58a5 100644 --- a/vertical-pod-autoscaler/e2e/v1/full_vpa.go +++ b/vertical-pod-autoscaler/e2e/v1/full_vpa.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" + vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/test" "k8s.io/kubernetes/test/e2e/framework" podsecurity "k8s.io/pod-security-admission/api" @@ -60,73 +61,149 @@ var _ = FullVpaE2eDescribe("Pods under VPA", func() { f := framework.NewDefaultFramework("vertical-pod-autoscaling") f.NamespacePodSecurityEnforceLevel = podsecurity.LevelBaseline - ginkgo.BeforeEach(func() { - ns := f.Namespace.Name - ginkgo.By("Setting up a hamster deployment") - rc = NewDynamicResourceConsumer("hamster", ns, KindDeployment, - replicas, - 1, /*initCPUTotal*/ - 10, /*initMemoryTotal*/ - 1, /*initCustomMetric*/ - initialCPU, /*cpuRequest*/ - initialMemory, /*memRequest*/ - f.ClientSet, - f.ScalesGetter) + ginkgo.Describe("with InPlaceOrRecreate update mode", ginkgo.Label("FG:InPlaceOrRecreate"), func() { + ginkgo.BeforeEach(func() { + checkInPlaceOrRecreateTestsEnabled(f, true, false) + + ns := f.Namespace.Name + ginkgo.By("Setting up a hamster deployment") + rc = NewDynamicResourceConsumer("hamster", ns, KindDeployment, + replicas, + 1, /*initCPUTotal*/ + 10, /*initMemoryTotal*/ + 1, /*initCustomMetric*/ + initialCPU, /*cpuRequest*/ + initialMemory, /*memRequest*/ + f.ClientSet, + f.ScalesGetter) + + ginkgo.By("Setting up a VPA CRD") + targetRef := &autoscaling.CrossVersionObjectReference{ + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "hamster", + } - ginkgo.By("Setting up a VPA CRD") - targetRef := &autoscaling.CrossVersionObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", - Name: "hamster", - } + containerName := GetHamsterContainerNameByIndex(0) + vpaCRD := test.VerticalPodAutoscaler(). + WithName("hamster-vpa"). + WithNamespace(f.Namespace.Name). + WithTargetRef(targetRef). + WithContainer(containerName). + WithUpdateMode(vpa_types.UpdateModeInPlaceOrRecreate). + AppendRecommendation( + test.Recommendation(). + WithContainer(containerName). + WithTarget("250m", "200Mi"). + WithLowerBound("250m", "200Mi"). + WithUpperBound("250m", "200Mi"). + GetContainerResources()). + Get() + + InstallVPA(f, vpaCRD) + }) - containerName := GetHamsterContainerNameByIndex(0) - vpaCRD := test.VerticalPodAutoscaler(). - WithName("hamster-vpa"). - WithNamespace(f.Namespace.Name). - WithTargetRef(targetRef). - WithContainer(containerName). - AppendRecommendation( - test.Recommendation(). - WithContainer(containerName). - WithTarget("250m", "200Mi"). - WithLowerBound("250m", "200Mi"). - WithUpperBound("250m", "200Mi"). - GetContainerResources()). - Get() + ginkgo.It("have cpu requests growing with usage", func() { + // initial CPU usage is low so a minimal recommendation is expected + err := waitForResourceRequestInRangeInPods( + f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceCPU, + ParseQuantityOrDie(minimalCPULowerBound), ParseQuantityOrDie(minimalCPUUpperBound)) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // consume more CPU to get a higher recommendation + rc.ConsumeCPU(600 * replicas) + err = waitForResourceRequestInRangeInPods( + f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceCPU, + ParseQuantityOrDie("500m"), ParseQuantityOrDie("1300m")) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }) - InstallVPA(f, vpaCRD) + ginkgo.It("have memory requests growing with usage", func() { + // initial memory usage is low so a minimal recommendation is expected + err := waitForResourceRequestInRangeInPods( + f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceMemory, + ParseQuantityOrDie(minimalMemoryLowerBound), ParseQuantityOrDie(minimalMemoryUpperBound)) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // consume more memory to get a higher recommendation + // NOTE: large range given due to unpredictability of actual memory usage + rc.ConsumeMem(1024 * replicas) + err = waitForResourceRequestInRangeInPods( + f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceMemory, + ParseQuantityOrDie("900Mi"), ParseQuantityOrDie("4000Mi")) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }) }) - ginkgo.It("have cpu requests growing with usage", func() { - // initial CPU usage is low so a minimal recommendation is expected - err := waitForResourceRequestInRangeInPods( - f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceCPU, - ParseQuantityOrDie(minimalCPULowerBound), ParseQuantityOrDie(minimalCPUUpperBound)) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) + ginkgo.Describe("with Recreate updateMode", func() { + ginkgo.BeforeEach(func() { + ns := f.Namespace.Name + ginkgo.By("Setting up a hamster deployment") + rc = NewDynamicResourceConsumer("hamster", ns, KindDeployment, + replicas, + 1, /*initCPUTotal*/ + 10, /*initMemoryTotal*/ + 1, /*initCustomMetric*/ + initialCPU, /*cpuRequest*/ + initialMemory, /*memRequest*/ + f.ClientSet, + f.ScalesGetter) + + ginkgo.By("Setting up a VPA CRD") + targetRef := &autoscaling.CrossVersionObjectReference{ + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "hamster", + } - // consume more CPU to get a higher recommendation - rc.ConsumeCPU(600 * replicas) - err = waitForResourceRequestInRangeInPods( - f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceCPU, - ParseQuantityOrDie("500m"), ParseQuantityOrDie("1300m")) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) - }) + containerName := GetHamsterContainerNameByIndex(0) + vpaCRD := test.VerticalPodAutoscaler(). + WithName("hamster-vpa"). + WithNamespace(f.Namespace.Name). + WithTargetRef(targetRef). + WithContainer(containerName). + AppendRecommendation( + test.Recommendation(). + WithContainer(containerName). + WithTarget("250m", "200Mi"). + WithLowerBound("250m", "200Mi"). + WithUpperBound("250m", "200Mi"). + GetContainerResources()). + Get() + + InstallVPA(f, vpaCRD) + }) - ginkgo.It("have memory requests growing with usage", func() { - // initial memory usage is low so a minimal recommendation is expected - err := waitForResourceRequestInRangeInPods( - f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceMemory, - ParseQuantityOrDie(minimalMemoryLowerBound), ParseQuantityOrDie(minimalMemoryUpperBound)) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) + ginkgo.It("have cpu requests growing with usage", func() { + // initial CPU usage is low so a minimal recommendation is expected + err := waitForResourceRequestInRangeInPods( + f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceCPU, + ParseQuantityOrDie(minimalCPULowerBound), ParseQuantityOrDie(minimalCPUUpperBound)) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // consume more CPU to get a higher recommendation + rc.ConsumeCPU(600 * replicas) + err = waitForResourceRequestInRangeInPods( + f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceCPU, + ParseQuantityOrDie("500m"), ParseQuantityOrDie("1300m")) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }) - // consume more memory to get a higher recommendation - // NOTE: large range given due to unpredictability of actual memory usage - rc.ConsumeMem(1024 * replicas) - err = waitForResourceRequestInRangeInPods( - f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceMemory, - ParseQuantityOrDie("900Mi"), ParseQuantityOrDie("4000Mi")) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) + ginkgo.It("have memory requests growing with usage", func() { + // initial memory usage is low so a minimal recommendation is expected + err := waitForResourceRequestInRangeInPods( + f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceMemory, + ParseQuantityOrDie(minimalMemoryLowerBound), ParseQuantityOrDie(minimalMemoryUpperBound)) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // consume more memory to get a higher recommendation + // NOTE: large range given due to unpredictability of actual memory usage + rc.ConsumeMem(1024 * replicas) + err = waitForResourceRequestInRangeInPods( + f, pollTimeout, metav1.ListOptions{LabelSelector: "name=hamster"}, apiv1.ResourceMemory, + ParseQuantityOrDie("900Mi"), ParseQuantityOrDie("4000Mi")) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }) }) }) diff --git a/vertical-pod-autoscaler/e2e/v1/updater.go b/vertical-pod-autoscaler/e2e/v1/updater.go index ccf0f07ded1..a72cdf6b1eb 100644 --- a/vertical-pod-autoscaler/e2e/v1/updater.go +++ b/vertical-pod-autoscaler/e2e/v1/updater.go @@ -140,6 +140,77 @@ var _ = UpdaterE2eDescribe("Updater", func() { }) }) +var _ = UpdaterE2eDescribe("Updater", ginkgo.Label("FG:InPlaceOrRecreate"), func() { + f := framework.NewDefaultFramework("vertical-pod-autoscaling") + f.NamespacePodSecurityEnforceLevel = podsecurity.LevelBaseline + + ginkgo.BeforeEach(func() { + checkInPlaceOrRecreateTestsEnabled(f, false, true) + }) + + ginkgo.It("In-place update pods when Admission Controller status available", func() { + const statusUpdateInterval = 10 * time.Second + + ginkgo.By("Setting up the Admission Controller status") + stopCh := make(chan struct{}) + statusUpdater := status.NewUpdater( + f.ClientSet, + status.AdmissionControllerStatusName, + status.AdmissionControllerStatusNamespace, + statusUpdateInterval, + "e2e test", + ) + defer func() { + // Schedule a cleanup of the Admission Controller status. + // Status is created outside the test namespace. + ginkgo.By("Deleting the Admission Controller status") + close(stopCh) + err := f.ClientSet.CoordinationV1().Leases(status.AdmissionControllerStatusNamespace). + Delete(context.TODO(), status.AdmissionControllerStatusName, metav1.DeleteOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }() + statusUpdater.Run(stopCh) + + podList := setupPodsForUpscalingInPlace(f) + initialPods := podList.DeepCopy() + + ginkgo.By("Waiting for pods to be in-place updated") + err := WaitForPodsUpdatedWithoutEviction(f, initialPods) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }) + + ginkgo.It("Does not evict pods for downscaling in-place", func() { + const statusUpdateInterval = 10 * time.Second + + ginkgo.By("Setting up the Admission Controller status") + stopCh := make(chan struct{}) + statusUpdater := status.NewUpdater( + f.ClientSet, + status.AdmissionControllerStatusName, + status.AdmissionControllerStatusNamespace, + statusUpdateInterval, + "e2e test", + ) + defer func() { + // Schedule a cleanup of the Admission Controller status. + // Status is created outside the test namespace. + ginkgo.By("Deleting the Admission Controller status") + close(stopCh) + err := f.ClientSet.CoordinationV1().Leases(status.AdmissionControllerStatusNamespace). + Delete(context.TODO(), status.AdmissionControllerStatusName, metav1.DeleteOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }() + statusUpdater.Run(stopCh) + + podList := setupPodsForDownscalingInPlace(f, nil) + initialPods := podList.DeepCopy() + + ginkgo.By("Waiting for pods to be in-place downscaled") + err := WaitForPodsUpdatedWithoutEviction(f, initialPods) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }) +}) + func setupPodsForUpscalingEviction(f *framework.Framework) *apiv1.PodList { return setupPodsForEviction(f, "100m", "100Mi", nil) } @@ -165,6 +236,7 @@ func setupPodsForEviction(f *framework.Framework, hamsterCPU, hamsterMemory stri WithName("hamster-vpa"). WithNamespace(f.Namespace.Name). WithTargetRef(controller). + WithUpdateMode(vpa_types.UpdateModeRecreate). WithEvictionRequirements(er). WithContainer(containerName). AppendRecommendation( @@ -180,3 +252,48 @@ func setupPodsForEviction(f *framework.Framework, hamsterCPU, hamsterMemory stri return podList } + +func setupPodsForUpscalingInPlace(f *framework.Framework) *apiv1.PodList { + return setupPodsForInPlace(f, "100m", "100Mi", nil, true) +} + +func setupPodsForDownscalingInPlace(f *framework.Framework, er []*vpa_types.EvictionRequirement) *apiv1.PodList { + return setupPodsForInPlace(f, "500m", "500Mi", er, true) +} + +func setupPodsForInPlace(f *framework.Framework, hamsterCPU, hamsterMemory string, er []*vpa_types.EvictionRequirement, withRecommendation bool) *apiv1.PodList { + controller := &autoscaling.CrossVersionObjectReference{ + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "hamster-deployment", + } + ginkgo.By(fmt.Sprintf("Setting up a hamster %v", controller.Kind)) + setupHamsterController(f, controller.Kind, hamsterCPU, hamsterMemory, defaultHamsterReplicas) + podList, err := GetHamsterPods(f) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + ginkgo.By("Setting up a VPA CRD") + containerName := GetHamsterContainerNameByIndex(0) + vpaBuilder := test.VerticalPodAutoscaler(). + WithName("hamster-vpa"). + WithNamespace(f.Namespace.Name). + WithTargetRef(controller). + WithUpdateMode(vpa_types.UpdateModeInPlaceOrRecreate). + WithEvictionRequirements(er). + WithContainer(containerName) + + if withRecommendation { + vpaBuilder = vpaBuilder.AppendRecommendation( + test.Recommendation(). + WithContainer(containerName). + WithTarget(containerName, "200m"). + WithLowerBound(containerName, "200m"). + WithUpperBound(containerName, "200m"). + GetContainerResources()) + } + + vpaCRD := vpaBuilder.Get() + InstallVPA(f, vpaCRD) + + return podList +} diff --git a/vertical-pod-autoscaler/hack/deploy-for-e2e-locally.sh b/vertical-pod-autoscaler/hack/deploy-for-e2e-locally.sh index ce2a8529ef7..42057b451aa 100755 --- a/vertical-pod-autoscaler/hack/deploy-for-e2e-locally.sh +++ b/vertical-pod-autoscaler/hack/deploy-for-e2e-locally.sh @@ -80,13 +80,13 @@ for i in ${COMPONENTS}; do fi if [ $i == admission-controller ] ; then (cd ${SCRIPT_ROOT}/pkg/${i} && bash ./gencerts.sh e2e || true) + kubectl apply -f ${SCRIPT_ROOT}/deploy/admission-controller-service.yaml fi ALL_ARCHITECTURES=${ARCH} make --directory ${SCRIPT_ROOT}/pkg/${i} docker-build REGISTRY=${REGISTRY} TAG=${TAG} docker tag ${REGISTRY}/vpa-${i}-${ARCH}:${TAG} ${REGISTRY}/vpa-${i}:${TAG} kind load docker-image ${REGISTRY}/vpa-${i}:${TAG} done - for i in ${COMPONENTS}; do if [ $i == recommender-externalmetrics ] ; then kubectl delete namespace monitoring --ignore-not-found=true @@ -96,6 +96,6 @@ for i in ${COMPONENTS}; do kubectl apply -f ${SCRIPT_ROOT}/hack/e2e/metrics-pump.yaml kubectl apply -f ${SCRIPT_ROOT}/hack/e2e/${i}-deployment.yaml else - REGISTRY=${REGISTRY} TAG=${TAG} ${SCRIPT_ROOT}/hack/vpa-process-yaml.sh ${SCRIPT_ROOT}/deploy/${i}-deployment.yaml | kubectl apply -f - + REGISTRY=${REGISTRY} TAG=${TAG} ${SCRIPT_ROOT}/hack/vpa-process-yaml.sh ${SCRIPT_ROOT}/deploy/${i}-deployment.yaml | kubectl apply -f - fi done diff --git a/vertical-pod-autoscaler/hack/deploy-for-e2e.sh b/vertical-pod-autoscaler/hack/deploy-for-e2e.sh index 96364671c3e..346f2ac0c26 100755 --- a/vertical-pod-autoscaler/hack/deploy-for-e2e.sh +++ b/vertical-pod-autoscaler/hack/deploy-for-e2e.sh @@ -69,6 +69,7 @@ gcloud auth configure-docker -q for i in ${COMPONENTS}; do if [ $i == admission-controller ] ; then (cd ${SCRIPT_ROOT}/pkg/${i} && bash ./gencerts.sh e2e || true) + kubectl apply -f ${SCRIPT_ROOT}/deploy/admission-controller-service.yaml fi ALL_ARCHITECTURES=amd64 make --directory ${SCRIPT_ROOT}/pkg/${i} release done diff --git a/vertical-pod-autoscaler/hack/kind-config.yaml b/vertical-pod-autoscaler/hack/kind-config.yaml new file mode 100644 index 00000000000..9d20acec394 --- /dev/null +++ b/vertical-pod-autoscaler/hack/kind-config.yaml @@ -0,0 +1,4 @@ +kind: Cluster +apiVersion: kind.x-k8s.io/v1alpha4 +featureGates: + InPlacePodVerticalScaling: true diff --git a/vertical-pod-autoscaler/hack/run-e2e-locally.sh b/vertical-pod-autoscaler/hack/run-e2e-locally.sh index 3fe23b6b634..642391c4623 100755 --- a/vertical-pod-autoscaler/hack/run-e2e-locally.sh +++ b/vertical-pod-autoscaler/hack/run-e2e-locally.sh @@ -75,7 +75,7 @@ kind delete cluster -n kind -q echo "Creating KIND cluster 'kind'" KIND_VERSION="kindest/node:v1.32.0" -if ! kind create cluster --image=${KIND_VERSION}; then +if ! kind create cluster --image=${KIND_VERSION} --config ${SCRIPT_ROOT}/hack/kind-config.yaml; then echo "Failed to create KIND cluster. Exiting. Make sure kind version is updated." echo "Available versions: https://github.com/kubernetes-sigs/kind/releases" exit 1 diff --git a/vertical-pod-autoscaler/hack/run-e2e-tests.sh b/vertical-pod-autoscaler/hack/run-e2e-tests.sh index e0970907a6d..f7f724925cb 100755 --- a/vertical-pod-autoscaler/hack/run-e2e-tests.sh +++ b/vertical-pod-autoscaler/hack/run-e2e-tests.sh @@ -50,7 +50,7 @@ case ${SUITE} in recommender|updater|admission-controller|actuation|full-vpa) export KUBECONFIG=$HOME/.kube/config pushd ${SCRIPT_ROOT}/e2e - go test ./v1/*go -v --test.timeout=90m --args --ginkgo.v=true --ginkgo.focus="\[VPA\] \[${SUITE}\]" --report-dir=${WORKSPACE} --disable-log-dump --ginkgo.timeout=90m + go test ./v1/*go -v --test.timeout=150m --args --ginkgo.v=true --ginkgo.focus="\[VPA\] \[${SUITE}\]" --report-dir=${WORKSPACE} --disable-log-dump --ginkgo.timeout=150m V1_RESULT=$? popd echo v1 test result: ${V1_RESULT} diff --git a/vertical-pod-autoscaler/hack/vpa-process-yaml.sh b/vertical-pod-autoscaler/hack/vpa-process-yaml.sh index fe166b83b21..8e70bce1800 100755 --- a/vertical-pod-autoscaler/hack/vpa-process-yaml.sh +++ b/vertical-pod-autoscaler/hack/vpa-process-yaml.sh @@ -18,14 +18,36 @@ set -o errexit set -o nounset set -o pipefail -SCRIPT_ROOT=$(dirname ${BASH_SOURCE})/.. - function print_help { echo "ERROR! Usage: vpa-process-yaml.sh +" echo "Script will output content of YAML files separated with YAML document" echo "separator and substituting REGISTRY and TAG for pod images" } +# Requires input from stdin, otherwise hangs. If the input is a Deployment manifest, +# apply kubectl patch to add feature gates specified in the FEATURE_GATES environment variable. +# e.g. cat file.yaml | apply_feature_gate +function apply_feature_gate() { + local input="" + while IFS= read -r line; do + input+="$line"$'\n' + done + + # matching precisely "kind: Deployment" to avoid matching "kind: DeploymentConfig" or a line with extra whitespace + if echo "$input" | grep -qE '^kind: Deployment$'; then + if [ -n "${FEATURE_GATES}" ]; then + if ! echo "$input" | kubectl patch --type=json --local -p='[{"op": "add", "path": "/spec/template/spec/containers/0/args/-", "value": "--feature-gates='"${FEATURE_GATES}"'"}]' -o yaml -f - 2>/dev/null; then + # If it fails, there was no args field, so we need to add it + echo "$input" | kubectl patch --type=json --local -p='[{"op": "add", "path": "/spec/template/spec/containers/0/args", "value": ["--feature-gates='"${FEATURE_GATES}"'"]}]' -o yaml -f - + fi + else + echo "$input" + fi + else + echo "$input" + fi +} + if [ $# -eq 0 ]; then print_help exit 1 @@ -36,6 +58,7 @@ DEFAULT_TAG="1.3.0" REGISTRY_TO_APPLY=${REGISTRY-$DEFAULT_REGISTRY} TAG_TO_APPLY=${TAG-$DEFAULT_TAG} +FEATURE_GATES=${FEATURE_GATES:-""} if [ "${REGISTRY_TO_APPLY}" != "${DEFAULT_REGISTRY}" ]; then (>&2 echo "WARNING! Using image repository from REGISTRY env variable (${REGISTRY_TO_APPLY}) instead of ${DEFAULT_REGISTRY}.") @@ -46,7 +69,6 @@ if [ "${TAG_TO_APPLY}" != "${DEFAULT_TAG}" ]; then fi for i in $*; do - sed -e "s,${DEFAULT_REGISTRY}/\([a-z-]*\):.*,${REGISTRY_TO_APPLY}/\1:${TAG_TO_APPLY}," $i - echo "" + sed -e "s,${DEFAULT_REGISTRY}/\([a-z-]*\):.*,${REGISTRY_TO_APPLY}/\1:${TAG_TO_APPLY}," $i | apply_feature_gate echo "---" done diff --git a/vertical-pod-autoscaler/hack/vpa-process-yamls.sh b/vertical-pod-autoscaler/hack/vpa-process-yamls.sh index 41e19ba192b..ddd87ed2668 100755 --- a/vertical-pod-autoscaler/hack/vpa-process-yamls.sh +++ b/vertical-pod-autoscaler/hack/vpa-process-yamls.sh @@ -66,9 +66,11 @@ for i in $COMPONENTS; do if [[ ${ACTION} == create || ${ACTION} == apply ]] ; then # Allow gencerts to fail silently if certs already exist (bash ${SCRIPT_ROOT}/pkg/admission-controller/gencerts.sh || true) + kubectl apply -f ${SCRIPT_ROOT}/deploy/admission-controller-service.yaml elif [ ${ACTION} == delete ] ; then (bash ${SCRIPT_ROOT}/pkg/admission-controller/rmcerts.sh || true) (bash ${SCRIPT_ROOT}/pkg/admission-controller/delete-webhook.sh || true) + kubectl delete -f ${SCRIPT_ROOT}/deploy/admission-controller-service.yaml --ignore-not-found fi fi if [[ ${ACTION} == print ]]; then diff --git a/vertical-pod-autoscaler/pkg/admission-controller/main.go b/vertical-pod-autoscaler/pkg/admission-controller/main.go index 5e93466f6ee..66ce8b9f70a 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/main.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/main.go @@ -24,6 +24,7 @@ import ( "strings" "time" + "github.com/spf13/pflag" "k8s.io/client-go/informers" kube_client "k8s.io/client-go/kubernetes" kube_flag "k8s.io/component-base/cli/flag" @@ -36,6 +37,7 @@ import ( "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa" vpa_clientset "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target" controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange" @@ -81,6 +83,7 @@ func main() { commonFlags := common.InitCommonFlags() klog.InitFlags(nil) common.InitLoggingFlags() + features.MutableFeatureGate.AddFlag(pflag.CommandLine) kube_flag.InitFlags() klog.V(1).InfoS("Starting Vertical Pod Autoscaler Admission Controller", "version", common.VerticalPodAutoscalerVersion()) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/handler_test.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/handler_test.go index aa8c5aa7aba..6477eeb33d9 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/handler_test.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/handler_test.go @@ -54,6 +54,10 @@ type fakePatchCalculator struct { err error } +func (*fakePatchCalculator) PatchResourceTarget() patch.PatchResourceTarget { + return patch.Pod +} + func (c *fakePatchCalculator) CalculatePatches(_ *apiv1.Pod, _ *vpa_types.VerticalPodAutoscaler) ( []resource_admission.PatchRecord, error) { return c.patches, c.err diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/calculator.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/calculator.go index 8b92d2204bb..0cd12571390 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/calculator.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/calculator.go @@ -23,7 +23,22 @@ import ( vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" ) +// PatchResourceTarget is the type of resource that can be patched. +type PatchResourceTarget string + +const ( + // Pod refers to the pod resource itself. + Pod PatchResourceTarget = "Pod" + // Resize refers to the resize subresource of the pod. + Resize PatchResourceTarget = "Resize" + + // Future subresources can be added here. + // e.g. Status PatchResourceTarget = "Status" +) + // Calculator is capable of calculating required patches for pod. type Calculator interface { CalculatePatches(pod *core.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]resource.PatchRecord, error) + // PatchResourceTarget returns the resource this calculator should calculate patches for. + PatchResourceTarget() PatchResourceTarget } diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/observed_containers.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/observed_containers.go index 98e7262e5ad..f80b79bf954 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/observed_containers.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/observed_containers.go @@ -31,6 +31,10 @@ func (*observedContainers) CalculatePatches(pod *core.Pod, _ *vpa_types.Vertical return []resource_admission.PatchRecord{GetAddAnnotationPatch(annotations.VpaObservedContainersLabel, vpaObservedContainersValue)}, nil } +func (*observedContainers) PatchResourceTarget() PatchResourceTarget { + return Pod +} + // NewObservedContainersCalculator returns calculator for // observed containers patches. func NewObservedContainersCalculator() Calculator { diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/resource_updates.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/resource_updates.go index 9be03f1c00d..963dfa29c07 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/resource_updates.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/resource_updates.go @@ -21,7 +21,6 @@ import ( "strings" core "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" resource_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation" @@ -47,6 +46,10 @@ func NewResourceUpdatesCalculator(recommendationProvider recommendation.Provider } } +func (*resourcesUpdatesPatchCalculator) PatchResourceTarget() PatchResourceTarget { + return Pod +} + func (c *resourcesUpdatesPatchCalculator) CalculatePatches(pod *core.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]resource_admission.PatchRecord, error) { result := []resource_admission.PatchRecord{} @@ -78,7 +81,7 @@ func getContainerPatch(pod *core.Pod, i int, annotationsPerContainer vpa_api_uti // Add empty resources object if missing. if pod.Spec.Containers[i].Resources.Limits == nil && pod.Spec.Containers[i].Resources.Requests == nil { - patches = append(patches, getPatchInitializingEmptyResources(i)) + patches = append(patches, GetPatchInitializingEmptyResources(i)) } annotations, found := annotationsPerContainer[pod.Spec.Containers[i].Name] @@ -96,34 +99,11 @@ func getContainerPatch(pod *core.Pod, i int, annotationsPerContainer vpa_api_uti func appendPatchesAndAnnotations(patches []resource_admission.PatchRecord, annotations []string, current core.ResourceList, containerIndex int, resources core.ResourceList, fieldName, resourceName string) ([]resource_admission.PatchRecord, []string) { // Add empty object if it's missing and we're about to fill it. if current == nil && len(resources) > 0 { - patches = append(patches, getPatchInitializingEmptyResourcesSubfield(containerIndex, fieldName)) + patches = append(patches, GetPatchInitializingEmptyResourcesSubfield(containerIndex, fieldName)) } for resource, request := range resources { - patches = append(patches, getAddResourceRequirementValuePatch(containerIndex, fieldName, resource, request)) + patches = append(patches, GetAddResourceRequirementValuePatch(containerIndex, fieldName, resource, request)) annotations = append(annotations, fmt.Sprintf("%s %s", resource, resourceName)) } return patches, annotations } - -func getAddResourceRequirementValuePatch(i int, kind string, resource core.ResourceName, quantity resource.Quantity) resource_admission.PatchRecord { - return resource_admission.PatchRecord{ - Op: "add", - Path: fmt.Sprintf("/spec/containers/%d/resources/%s/%s", i, kind, resource), - Value: quantity.String()} -} - -func getPatchInitializingEmptyResources(i int) resource_admission.PatchRecord { - return resource_admission.PatchRecord{ - Op: "add", - Path: fmt.Sprintf("/spec/containers/%d/resources", i), - Value: core.ResourceRequirements{}, - } -} - -func getPatchInitializingEmptyResourcesSubfield(i int, kind string) resource_admission.PatchRecord { - return resource_admission.PatchRecord{ - Op: "add", - Path: fmt.Sprintf("/spec/containers/%d/resources/%s", i, kind), - Value: core.ResourceList{}, - } -} diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/util.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/util.go index 7bdea7988ab..0c68ab6cd55 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/util.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch/util.go @@ -19,6 +19,9 @@ package patch import ( "fmt" + core "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + resource_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource" ) @@ -39,3 +42,30 @@ func GetAddAnnotationPatch(annotationName, annotationValue string) resource_admi Value: annotationValue, } } + +// GetAddResourceRequirementValuePatch returns a patch record to add resource requirements to a container. +func GetAddResourceRequirementValuePatch(i int, kind string, resource core.ResourceName, quantity resource.Quantity) resource_admission.PatchRecord { + return resource_admission.PatchRecord{ + Op: "add", + Path: fmt.Sprintf("/spec/containers/%d/resources/%s/%s", i, kind, resource), + Value: quantity.String()} +} + +// GetPatchInitializingEmptyResources returns a patch record to initialize an empty resources object for a container. +func GetPatchInitializingEmptyResources(i int) resource_admission.PatchRecord { + return resource_admission.PatchRecord{ + Op: "add", + Path: fmt.Sprintf("/spec/containers/%d/resources", i), + Value: core.ResourceRequirements{}, + } +} + +// GetPatchInitializingEmptyResourcesSubfield returns a patch record to initialize an empty subfield +// (e.g., "requests" or "limits") within a container's resources object. +func GetPatchInitializingEmptyResourcesSubfield(i int, kind string) resource_admission.PatchRecord { + return resource_admission.PatchRecord{ + Op: "add", + Path: fmt.Sprintf("/spec/containers/%d/resources/%s", i, kind), + Value: core.ResourceList{}, + } +} diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go index 6888efac1aa..b553a236f9d 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go @@ -30,15 +30,17 @@ import ( "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/metrics/admission" ) var ( possibleUpdateModes = map[vpa_types.UpdateMode]interface{}{ - vpa_types.UpdateModeOff: struct{}{}, - vpa_types.UpdateModeInitial: struct{}{}, - vpa_types.UpdateModeRecreate: struct{}{}, - vpa_types.UpdateModeAuto: struct{}{}, + vpa_types.UpdateModeOff: struct{}{}, + vpa_types.UpdateModeInitial: struct{}{}, + vpa_types.UpdateModeRecreate: struct{}{}, + vpa_types.UpdateModeAuto: struct{}{}, + vpa_types.UpdateModeInPlaceOrRecreate: struct{}{}, } possibleScalingModes = map[vpa_types.ContainerScalingMode]interface{}{ @@ -121,6 +123,9 @@ func ValidateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error { if _, found := possibleUpdateModes[*mode]; !found { return fmt.Errorf("unexpected UpdateMode value %s", *mode) } + if (*mode == vpa_types.UpdateModeInPlaceOrRecreate) && !features.Enabled(features.InPlaceOrRecreate) && isCreate { + return fmt.Errorf("in order to use UpdateMode %s, you must enable feature gate %s in the admission-controller args", vpa_types.UpdateModeInPlaceOrRecreate, features.InPlaceOrRecreate) + } if minReplicas := vpa.Spec.UpdatePolicy.MinReplicas; minReplicas != nil && *minReplicas <= 0 { return fmt.Errorf("MinReplicas has to be positive, got %v", *minReplicas) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go index 06efced1e0c..3f11995fccc 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go @@ -24,7 +24,10 @@ import ( apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + featuregatetesting "k8s.io/component-base/featuregate/testing" + vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" ) const ( @@ -42,11 +45,13 @@ func TestValidateVPA(t *testing.T) { validScalingMode := vpa_types.ContainerScalingModeAuto scalingModeOff := vpa_types.ContainerScalingModeOff controlledValuesRequestsAndLimits := vpa_types.ContainerControlledValuesRequestsAndLimits + inPlaceOrRecreateUpdateMode := vpa_types.UpdateModeInPlaceOrRecreate tests := []struct { - name string - vpa vpa_types.VerticalPodAutoscaler - isCreate bool - expectError error + name string + vpa vpa_types.VerticalPodAutoscaler + isCreate bool + expectError error + inPlaceOrRecreateFeatureGateDisabled bool }{ { name: "empty update", @@ -78,6 +83,42 @@ func TestValidateVPA(t *testing.T) { }, expectError: fmt.Errorf("unexpected UpdateMode value bad"), }, + { + name: "creating VPA with InPlaceOrRecreate update mode not allowed by disabled feature gate", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + UpdatePolicy: &vpa_types.PodUpdatePolicy{ + UpdateMode: &inPlaceOrRecreateUpdateMode, + }, + }, + }, + isCreate: true, + inPlaceOrRecreateFeatureGateDisabled: true, + expectError: fmt.Errorf("in order to use UpdateMode %s, you must enable feature gate %s in the admission-controller args", vpa_types.UpdateModeInPlaceOrRecreate, features.InPlaceOrRecreate), + }, + { + name: "updating VPA with InPlaceOrRecreate update mode allowed by disabled feature gate", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + UpdatePolicy: &vpa_types.PodUpdatePolicy{ + UpdateMode: &inPlaceOrRecreateUpdateMode, + }, + }, + }, + isCreate: false, + inPlaceOrRecreateFeatureGateDisabled: true, + expectError: nil, + }, + { + name: "InPlaceOrRecreate update mode enabled by feature gate", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + UpdatePolicy: &vpa_types.PodUpdatePolicy{ + UpdateMode: &inPlaceOrRecreateUpdateMode, + }, + }, + }, + }, { name: "zero minReplicas", vpa: vpa_types.VerticalPodAutoscaler{ @@ -282,6 +323,9 @@ func TestValidateVPA(t *testing.T) { } for _, tc := range tests { t.Run(fmt.Sprintf("test case: %s", tc.name), func(t *testing.T) { + if !tc.inPlaceOrRecreateFeatureGateDisabled { + featuregatetesting.SetFeatureGateDuringTest(t, features.MutableFeatureGate, features.InPlaceOrRecreate, true) + } err := ValidateVPA(&tc.vpa, tc.isCreate) if tc.expectError == nil { assert.NoError(t, err) diff --git a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go index db9d43a686a..6ae164ce4ca 100644 --- a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go +++ b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go @@ -151,7 +151,7 @@ type PodUpdatePolicy struct { } // UpdateMode controls when autoscaler applies changes to the pod resources. -// +kubebuilder:validation:Enum=Off;Initial;Recreate;Auto +// +kubebuilder:validation:Enum=Off;Initial;Recreate;InPlaceOrRecreate;Auto type UpdateMode string const ( @@ -169,8 +169,15 @@ const ( // UpdateModeAuto means that autoscaler assigns resources on pod creation // and additionally can update them during the lifetime of the pod, // using any available update method. Currently this is equivalent to - // Recreate, which is the only available update method. + // Recreate. UpdateModeAuto UpdateMode = "Auto" + // UpdateModeInPlaceOrRecreate means that autoscaler tries to assign resources in-place. + // If this is not possible (e.g., resizing takes too long or is infeasible), it falls back to the + // "Recreate" update mode. + // Requires VPA level feature gate "InPlaceOrRecreate" to be enabled + // on the admission and updater pods. + // Requires cluster feature gate "InPlacePodVerticalScaling" to be enabled. + UpdateModeInPlaceOrRecreate UpdateMode = "InPlaceOrRecreate" ) // PodResourcePolicy controls how autoscaler computes the recommended resources diff --git a/vertical-pod-autoscaler/pkg/features/features.go b/vertical-pod-autoscaler/pkg/features/features.go new file mode 100644 index 00000000000..513973f6395 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/features/features.go @@ -0,0 +1,63 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package features + +import ( + "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/version" + "k8s.io/component-base/featuregate" + + "k8s.io/autoscaler/vertical-pod-autoscaler/common" +) + +const ( + // Every feature gate should add method here following this template: + // + // // alpha: v1.X + // // components: admission-controller, recommender, updater + // + // MyFeature featuregate.Feature = "MyFeature". + // + // Feature gates should be listed in alphabetical, case-sensitive + // (upper before any lower case character) order. This reduces the risk + // of code conflicts because changes are more likely to be scattered + // across the file. + // + // In each feature gate description, you must specify "components". + // The feature must be enabled by the --feature-gates argument on each listed component. + + // alpha: v1.4.0 + // components: admission-controller, updater + + // InPlaceOrRecreate enables the InPlaceOrRecreate update mode to be used. + // Requires KEP-1287 InPlacePodVerticalScaling feature-gate to be enabled on the cluster. + InPlaceOrRecreate featuregate.Feature = "InPlaceOrRecreate" +) + +// MutableFeatureGate is a mutable, versioned, global FeatureGate. +var MutableFeatureGate featuregate.MutableVersionedFeatureGate = featuregate.NewFeatureGate() + +// Enabled is a helper function for MutableFeatureGate.Enabled(f) +func Enabled(f featuregate.Feature) bool { + return MutableFeatureGate.Enabled(f) +} + +func init() { + // set the emulation version to align with VPA versioning system + runtime.Must(MutableFeatureGate.SetEmulationVersion(version.MustParse(common.VerticalPodAutoscalerVersion()))) + runtime.Must(MutableFeatureGate.AddVersioned(defaultVersionedFeatureGates)) +} diff --git a/vertical-pod-autoscaler/pkg/features/versioned_features.go b/vertical-pod-autoscaler/pkg/features/versioned_features.go new file mode 100644 index 00000000000..6c93265adf8 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/features/versioned_features.go @@ -0,0 +1,33 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package features + +import ( + "k8s.io/apimachinery/pkg/util/version" + "k8s.io/component-base/featuregate" +) + +// defaultVersionedFeatureGates consists of all known VPA-specific feature keys with VersionedSpecs. +// To add a new feature, define a key for it in pkg/features/features.go and add it here. The features will be +// available throughout Kubernetes binaries. + +// Entries are alphabetized. +var defaultVersionedFeatureGates = map[featuregate.Feature]featuregate.VersionedSpecs{ + InPlaceOrRecreate: { + {Version: version.MustParse("1.4.0"), Default: false, PreRelease: featuregate.Alpha}, + }, +} diff --git a/vertical-pod-autoscaler/pkg/recommender/main.go b/vertical-pod-autoscaler/pkg/recommender/main.go index 5a801754d52..d58e2903ae1 100644 --- a/vertical-pod-autoscaler/pkg/recommender/main.go +++ b/vertical-pod-autoscaler/pkg/recommender/main.go @@ -40,6 +40,7 @@ import ( "k8s.io/autoscaler/vertical-pod-autoscaler/common" vpa_clientset "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/checkpoint" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/input" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/input/history" @@ -131,6 +132,8 @@ func main() { leaderElection := defaultLeaderElectionConfiguration() componentbaseoptions.BindLeaderElectionFlags(&leaderElection, pflag.CommandLine) + features.MutableFeatureGate.AddFlag(pflag.CommandLine) + kube_flag.InitFlags() klog.V(1).InfoS("Vertical Pod Autoscaler Recommender", "version", common.VerticalPodAutoscalerVersion(), "recommenderName", *recommenderName) diff --git a/vertical-pod-autoscaler/pkg/updater/inplace/inplace_updated.go b/vertical-pod-autoscaler/pkg/updater/inplace/inplace_updated.go new file mode 100644 index 00000000000..31ea45a0990 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/updater/inplace/inplace_updated.go @@ -0,0 +1,46 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package inplace + +import ( + core "k8s.io/api/core/v1" + + resource_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch" + + vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/annotations" +) + +type inPlaceUpdate struct{} + +// CalculatePatches returns a patch that adds a "vpaInPlaceUpdated" annotation +// to the pod, marking it as having been requested to be updated in-place by VPA. +func (*inPlaceUpdate) CalculatePatches(pod *core.Pod, _ *vpa_types.VerticalPodAutoscaler) ([]resource_admission.PatchRecord, error) { + vpaInPlaceUpdatedValue := annotations.GetVpaInPlaceUpdatedValue() + return []resource_admission.PatchRecord{patch.GetAddAnnotationPatch(annotations.VpaInPlaceUpdatedLabel, vpaInPlaceUpdatedValue)}, nil +} + +func (*inPlaceUpdate) PatchResourceTarget() patch.PatchResourceTarget { + return patch.Pod +} + +// NewInPlaceUpdatedCalculator returns calculator for +// observed containers patches. +func NewInPlaceUpdatedCalculator() patch.Calculator { + return &inPlaceUpdate{} +} diff --git a/vertical-pod-autoscaler/pkg/updater/inplace/resource_updates.go b/vertical-pod-autoscaler/pkg/updater/inplace/resource_updates.go new file mode 100644 index 00000000000..44d9ea1ace1 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/updater/inplace/resource_updates.go @@ -0,0 +1,88 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package inplace + +import ( + "fmt" + + core "k8s.io/api/core/v1" + + resource_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation" + vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + vpa_api_util "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa" +) + +type resourcesInplaceUpdatesPatchCalculator struct { + recommendationProvider recommendation.Provider +} + +// NewResourceInPlaceUpdatesCalculator returns a calculator for +// in-place resource update patches. +func NewResourceInPlaceUpdatesCalculator(recommendationProvider recommendation.Provider) patch.Calculator { + return &resourcesInplaceUpdatesPatchCalculator{ + recommendationProvider: recommendationProvider, + } +} + +// PatchResourceTarget returns the resize subresource to apply calculator patches. +func (*resourcesInplaceUpdatesPatchCalculator) PatchResourceTarget() patch.PatchResourceTarget { + return patch.Resize +} + +// CalculatePatches calculates a JSON patch from a VPA's recommendation to send to the pod "resize" subresource as an in-place resize. +func (c *resourcesInplaceUpdatesPatchCalculator) CalculatePatches(pod *core.Pod, vpa *vpa_types.VerticalPodAutoscaler) ([]resource_admission.PatchRecord, error) { + result := []resource_admission.PatchRecord{} + + containersResources, _, err := c.recommendationProvider.GetContainersResourcesForPod(pod, vpa) + if err != nil { + return []resource_admission.PatchRecord{}, fmt.Errorf("Failed to calculate resource patch for pod %s/%s: %v", pod.Namespace, pod.Name, err) + } + + for i, containerResources := range containersResources { + newPatches := getContainerPatch(pod, i, containerResources) + result = append(result, newPatches...) + } + + return result, nil +} + +func getContainerPatch(pod *core.Pod, i int, containerResources vpa_api_util.ContainerResources) []resource_admission.PatchRecord { + var patches []resource_admission.PatchRecord + // Add empty resources object if missing. + if pod.Spec.Containers[i].Resources.Limits == nil && + pod.Spec.Containers[i].Resources.Requests == nil { + patches = append(patches, patch.GetPatchInitializingEmptyResources(i)) + } + + patches = appendPatches(patches, pod.Spec.Containers[i].Resources.Requests, i, containerResources.Requests, "requests") + patches = appendPatches(patches, pod.Spec.Containers[i].Resources.Limits, i, containerResources.Limits, "limits") + + return patches +} + +func appendPatches(patches []resource_admission.PatchRecord, current core.ResourceList, containerIndex int, resources core.ResourceList, fieldName string) []resource_admission.PatchRecord { + // Add empty object if it's missing and we're about to fill it. + if current == nil && len(resources) > 0 { + patches = append(patches, patch.GetPatchInitializingEmptyResourcesSubfield(containerIndex, fieldName)) + } + for resource, request := range resources { + patches = append(patches, patch.GetAddResourceRequirementValuePatch(containerIndex, fieldName, resource, request)) + } + return patches +} diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater.go b/vertical-pod-autoscaler/pkg/updater/logic/updater.go index b0029315372..7e7d0f1b21b 100644 --- a/vertical-pod-autoscaler/pkg/updater/logic/updater.go +++ b/vertical-pod-autoscaler/pkg/updater/logic/updater.go @@ -31,6 +31,8 @@ import ( kube_client "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" + utils "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/utils" + corescheme "k8s.io/client-go/kubernetes/scheme" clientv1 "k8s.io/client-go/kubernetes/typed/core/v1" v1lister "k8s.io/client-go/listers/core/v1" @@ -38,14 +40,16 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/klog/v2" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" vpa_clientset "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned/scheme" vpa_lister "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/listers/autoscaling.k8s.io/v1" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target" controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" - "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/eviction" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/priority" + restriction "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/restriction" metrics_updater "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/metrics/updater" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/status" vpa_api_util "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa" @@ -61,11 +65,12 @@ type updater struct { vpaLister vpa_lister.VerticalPodAutoscalerLister podLister v1lister.PodLister eventRecorder record.EventRecorder - evictionFactory eviction.PodsEvictionRestrictionFactory + restrictionFactory restriction.PodsRestrictionFactory recommendationProcessor vpa_api_util.RecommendationProcessor evictionAdmission priority.PodEvictionAdmission priorityProcessor priority.PriorityProcessor evictionRateLimiter *rate.Limiter + inPlaceRateLimiter *rate.Limiter selectorFetcher target.VpaTargetSelectorFetcher useAdmissionControllerStatus bool statusValidator status.Validator @@ -77,7 +82,7 @@ type updater struct { func NewUpdater( kubeClient kube_client.Interface, vpaClient *vpa_clientset.Clientset, - minReplicasForEvicition int, + minReplicasForEviction int, evictionRateLimit float64, evictionRateBurst int, evictionToleranceFraction float64, @@ -90,19 +95,29 @@ func NewUpdater( priorityProcessor priority.PriorityProcessor, namespace string, ignoredNamespaces []string, + patchCalculators []patch.Calculator, ) (Updater, error) { evictionRateLimiter := getRateLimiter(evictionRateLimit, evictionRateBurst) - factory, err := eviction.NewPodsEvictionRestrictionFactory(kubeClient, minReplicasForEvicition, evictionToleranceFraction) + // TODO: Create in-place rate limits for the in-place rate limiter + inPlaceRateLimiter := getRateLimiter(evictionRateLimit, evictionRateBurst) + factory, err := restriction.NewPodsRestrictionFactory( + kubeClient, + minReplicasForEviction, + evictionToleranceFraction, + patchCalculators, + ) if err != nil { - return nil, fmt.Errorf("Failed to create eviction restriction factory: %v", err) + return nil, fmt.Errorf("Failed to create restriction factory: %v", err) } + return &updater{ vpaLister: vpa_api_util.NewVpasLister(vpaClient, make(chan struct{}), namespace), podLister: newPodLister(kubeClient, namespace), eventRecorder: newEventRecorder(kubeClient), - evictionFactory: factory, + restrictionFactory: factory, recommendationProcessor: recommendationProcessor, evictionRateLimiter: evictionRateLimiter, + inPlaceRateLimiter: inPlaceRateLimiter, evictionAdmission: evictionAdmission, priorityProcessor: priorityProcessor, selectorFetcher: selectorFetcher, @@ -149,8 +164,8 @@ func (u *updater) RunOnce(ctx context.Context) { continue } if vpa_api_util.GetUpdateMode(vpa) != vpa_types.UpdateModeRecreate && - vpa_api_util.GetUpdateMode(vpa) != vpa_types.UpdateModeAuto { - klog.V(3).InfoS("Skipping VPA object because its mode is not \"Recreate\" or \"Auto\"", "vpa", klog.KObj(vpa)) + vpa_api_util.GetUpdateMode(vpa) != vpa_types.UpdateModeAuto && vpa_api_util.GetUpdateMode(vpa) != vpa_types.UpdateModeInPlaceOrRecreate { + klog.V(3).InfoS("Skipping VPA object because its mode is not \"InPlaceOrRecreate\", \"Recreate\" or \"Auto\"", "vpa", klog.KObj(vpa)) continue } selector, err := u.selectorFetcher.Fetch(ctx, vpa) @@ -198,32 +213,89 @@ func (u *updater) RunOnce(ctx context.Context) { // wrappers for metrics which are computed every loop run controlledPodsCounter := metrics_updater.NewControlledPodsCounter() evictablePodsCounter := metrics_updater.NewEvictablePodsCounter() + inPlaceUpdatablePodsCounter := metrics_updater.NewInPlaceUpdtateablePodsCounter() vpasWithEvictablePodsCounter := metrics_updater.NewVpasWithEvictablePodsCounter() vpasWithEvictedPodsCounter := metrics_updater.NewVpasWithEvictedPodsCounter() + vpasWithInPlaceUpdatablePodsCounter := metrics_updater.NewVpasWithInPlaceUpdtateablePodsCounter() + vpasWithInPlaceUpdatedPodsCounter := metrics_updater.NewVpasWithInPlaceUpdtatedPodsCounter() + // using defer to protect against 'return' after evictionRateLimiter.Wait defer controlledPodsCounter.Observe() defer evictablePodsCounter.Observe() defer vpasWithEvictablePodsCounter.Observe() defer vpasWithEvictedPodsCounter.Observe() + // separate counters for in-place + defer inPlaceUpdatablePodsCounter.Observe() + defer vpasWithInPlaceUpdatablePodsCounter.Observe() + defer vpasWithInPlaceUpdatedPodsCounter.Observe() // NOTE: this loop assumes that controlledPods are filtered - // to contain only Pods controlled by a VPA in auto or recreate mode + // to contain only Pods controlled by a VPA in auto, recreate, or inPlaceOrRecreate mode for vpa, livePods := range controlledPods { vpaSize := len(livePods) controlledPodsCounter.Add(vpaSize, vpaSize) - evictionLimiter := u.evictionFactory.NewPodsEvictionRestriction(livePods, vpa) - podsForUpdate := u.getPodsUpdateOrder(filterNonEvictablePods(livePods, evictionLimiter), vpa) - evictablePodsCounter.Add(vpaSize, len(podsForUpdate)) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := u.restrictionFactory.GetCreatorMaps(livePods, vpa) + if err != nil { + klog.ErrorS(err, "Failed to get creator maps") + continue + } + + evictionLimiter := u.restrictionFactory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + inPlaceLimiter := u.restrictionFactory.NewPodsInPlaceRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + + podsForInPlace := make([]*apiv1.Pod, 0) + podsForEviction := make([]*apiv1.Pod, 0) + updateMode := vpa_api_util.GetUpdateMode(vpa) + + if updateMode == vpa_types.UpdateModeInPlaceOrRecreate && features.Enabled(features.InPlaceOrRecreate) { + podsForInPlace = u.getPodsUpdateOrder(filterNonInPlaceUpdatablePods(livePods, inPlaceLimiter), vpa) + inPlaceUpdatablePodsCounter.Add(vpaSize, len(podsForInPlace)) + } else { + // If the feature gate is not enabled but update mode is InPlaceOrRecreate, updater will always fallback to eviction. + if updateMode == vpa_types.UpdateModeInPlaceOrRecreate { + klog.InfoS("Warning: feature gate is not enabled for this updateMode", "featuregate", features.InPlaceOrRecreate, "updateMode", vpa_types.UpdateModeInPlaceOrRecreate) + } + podsForEviction = u.getPodsUpdateOrder(filterNonEvictablePods(livePods, evictionLimiter), vpa) + evictablePodsCounter.Add(vpaSize, len(podsForEviction)) + } + withInPlaceUpdatable := false + withInPlaceUpdated := false withEvictable := false withEvicted := false - for _, pod := range podsForUpdate { + + for _, pod := range podsForInPlace { + withInPlaceUpdatable = true + decision := inPlaceLimiter.CanInPlaceUpdate(pod) + + if decision == utils.InPlaceDeferred { + klog.V(0).InfoS("In-place update deferred", "pod", klog.KObj(pod)) + continue + } else if decision == utils.InPlaceEvict { + podsForEviction = append(podsForEviction, pod) + continue + } + err = u.inPlaceRateLimiter.Wait(ctx) + if err != nil { + klog.V(0).InfoS("In-place rate limiter wait failed for in-place resize", "error", err) + return + } + err := inPlaceLimiter.InPlaceUpdate(pod, vpa, u.eventRecorder) + if err != nil { + klog.V(0).InfoS("In-place update failed", "error", err, "pod", klog.KObj(pod)) + continue + } + withInPlaceUpdated = true + metrics_updater.AddInPlaceUpdatedPod(vpaSize) + } + + for _, pod := range podsForEviction { withEvictable = true if !evictionLimiter.CanEvict(pod) { continue } - err := u.evictionRateLimiter.Wait(ctx) + err = u.evictionRateLimiter.Wait(ctx) if err != nil { klog.V(0).InfoS("Eviction rate limiter wait failed", "error", err) return @@ -238,6 +310,12 @@ func (u *updater) RunOnce(ctx context.Context) { } } + if withInPlaceUpdatable { + vpasWithInPlaceUpdatablePodsCounter.Add(vpaSize, 1) + } + if withInPlaceUpdated { + vpasWithInPlaceUpdatedPodsCounter.Add(vpaSize, 1) + } if withEvictable { vpasWithEvictablePodsCounter.Add(vpaSize, 1) } @@ -248,17 +326,17 @@ func (u *updater) RunOnce(ctx context.Context) { timer.ObserveStep("EvictPods") } -func getRateLimiter(evictionRateLimit float64, evictionRateLimitBurst int) *rate.Limiter { - var evictionRateLimiter *rate.Limiter - if evictionRateLimit <= 0 { +func getRateLimiter(rateLimit float64, rateLimitBurst int) *rate.Limiter { + var rateLimiter *rate.Limiter + if rateLimit <= 0 { // As a special case if the rate is set to rate.Inf, the burst rate is ignored // see https://github.com/golang/time/blob/master/rate/rate.go#L37 - evictionRateLimiter = rate.NewLimiter(rate.Inf, 0) + rateLimiter = rate.NewLimiter(rate.Inf, 0) klog.V(1).InfoS("Rate limit disabled") } else { - evictionRateLimiter = rate.NewLimiter(rate.Limit(evictionRateLimit), evictionRateLimitBurst) + rateLimiter = rate.NewLimiter(rate.Limit(rateLimit), rateLimitBurst) } - return evictionRateLimiter + return rateLimiter } // getPodsUpdateOrder returns list of pods that should be updated ordered by update priority @@ -276,24 +354,30 @@ func (u *updater) getPodsUpdateOrder(pods []*apiv1.Pod, vpa *vpa_types.VerticalP return priorityCalculator.GetSortedPods(u.evictionAdmission) } -func filterNonEvictablePods(pods []*apiv1.Pod, evictionRestriction eviction.PodsEvictionRestriction) []*apiv1.Pod { +func filterPods(pods []*apiv1.Pod, predicate func(*apiv1.Pod) bool) []*apiv1.Pod { result := make([]*apiv1.Pod, 0) for _, pod := range pods { - if evictionRestriction.CanEvict(pod) { + if predicate(pod) { result = append(result, pod) } } return result } +func filterNonInPlaceUpdatablePods(pods []*apiv1.Pod, inplaceRestriction restriction.PodsInPlaceRestriction) []*apiv1.Pod { + return filterPods(pods, func(pod *apiv1.Pod) bool { + return inplaceRestriction.CanInPlaceUpdate(pod) != utils.InPlaceDeferred + }) +} + +func filterNonEvictablePods(pods []*apiv1.Pod, evictionRestriction restriction.PodsEvictionRestriction) []*apiv1.Pod { + return filterPods(pods, evictionRestriction.CanEvict) +} + func filterDeletedPods(pods []*apiv1.Pod) []*apiv1.Pod { - result := make([]*apiv1.Pod, 0) - for _, pod := range pods { - if pod.DeletionTimestamp == nil { - result = append(result, pod) - } - } - return result + return filterPods(pods, func(pod *apiv1.Pod) bool { + return pod.DeletionTimestamp == nil + }) } func newPodLister(kubeClient kube_client.Interface, namespace string) v1lister.PodLister { diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go b/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go index 64357d4924e..8586ea77cbd 100644 --- a/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go +++ b/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go @@ -22,6 +22,9 @@ import ( "testing" "time" + restriction "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/restriction" + utils "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/utils" + "golang.org/x/time/rate" v1 "k8s.io/api/autoscaling/v1" @@ -33,11 +36,12 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" + featuregatetesting "k8s.io/component-base/featuregate/testing" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" target_mock "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/mock" - "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/eviction" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/priority" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/status" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/test" @@ -55,24 +59,63 @@ func TestRunOnce_Mode(t *testing.T) { updateMode vpa_types.UpdateMode expectFetchCalls bool expectedEvictionCount int + expectedInPlacedCount int + canEvict bool + canInPlaceUpdate utils.InPlaceDecision }{ { name: "with Auto mode", updateMode: vpa_types.UpdateModeAuto, expectFetchCalls: true, expectedEvictionCount: 5, + expectedInPlacedCount: 0, + canEvict: true, + canInPlaceUpdate: utils.InPlaceApproved, }, { name: "with Initial mode", updateMode: vpa_types.UpdateModeInitial, expectFetchCalls: false, expectedEvictionCount: 0, + expectedInPlacedCount: 0, + canEvict: true, + canInPlaceUpdate: utils.InPlaceApproved, }, { name: "with Off mode", updateMode: vpa_types.UpdateModeOff, expectFetchCalls: false, expectedEvictionCount: 0, + expectedInPlacedCount: 0, + canEvict: true, + canInPlaceUpdate: utils.InPlaceApproved, + }, + { + name: "with InPlaceOrRecreate mode expecting in-place updates", + updateMode: vpa_types.UpdateModeInPlaceOrRecreate, + expectFetchCalls: true, + expectedEvictionCount: 0, + expectedInPlacedCount: 5, + canEvict: true, + canInPlaceUpdate: utils.InPlaceApproved, + }, + { + name: "with InPlaceOrRecreate mode expecting fallback to evictions", + updateMode: vpa_types.UpdateModeInPlaceOrRecreate, + expectFetchCalls: true, + expectedEvictionCount: 5, + expectedInPlacedCount: 0, + canEvict: true, + canInPlaceUpdate: utils.InPlaceEvict, + }, + { + name: "with InPlaceOrRecreate mode expecting no evictions or in-place", + updateMode: vpa_types.UpdateModeInPlaceOrRecreate, + expectFetchCalls: true, + expectedEvictionCount: 0, + expectedInPlacedCount: 0, + canEvict: false, + canInPlaceUpdate: utils.InPlaceDeferred, }, } for _, tc := range tests { @@ -83,6 +126,8 @@ func TestRunOnce_Mode(t *testing.T) { newFakeValidator(true), tc.expectFetchCalls, tc.expectedEvictionCount, + tc.expectedInPlacedCount, + tc.canInPlaceUpdate, ) }) } @@ -94,18 +139,21 @@ func TestRunOnce_Status(t *testing.T) { statusValidator status.Validator expectFetchCalls bool expectedEvictionCount int + expectedInPlacedCount int }{ { name: "with valid status", statusValidator: newFakeValidator(true), expectFetchCalls: true, expectedEvictionCount: 5, + expectedInPlacedCount: 0, }, { name: "with invalid status", statusValidator: newFakeValidator(false), expectFetchCalls: false, expectedEvictionCount: 0, + expectedInPlacedCount: 0, }, } for _, tc := range tests { @@ -116,6 +164,8 @@ func TestRunOnce_Status(t *testing.T) { tc.statusValidator, tc.expectFetchCalls, tc.expectedEvictionCount, + tc.expectedInPlacedCount, + utils.InPlaceApproved, ) }) } @@ -127,7 +177,10 @@ func testRunOnceBase( statusValidator status.Validator, expectFetchCalls bool, expectedEvictionCount int, + expectedInPlacedCount int, + canInPlaceUpdate utils.InPlaceDecision, ) { + featuregatetesting.SetFeatureGateDuringTest(t, features.MutableFeatureGate, features.InPlaceOrRecreate, true) ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -151,6 +204,7 @@ func testRunOnceBase( } pods := make([]*apiv1.Pod, livePods) eviction := &test.PodsEvictionRestrictionMock{} + inplace := &test.PodsInPlaceRestrictionMock{} for i := range pods { pods[i] = test.Pod().WithName("test_"+strconv.Itoa(i)). @@ -159,11 +213,18 @@ func testRunOnceBase( Get() pods[i].Labels = labels + + inplace.On("CanInPlaceUpdate", pods[i]).Return(canInPlaceUpdate) + inplace.On("InPlaceUpdate", pods[i], nil).Return(nil) + eviction.On("CanEvict", pods[i]).Return(true) eviction.On("Evict", pods[i], nil).Return(nil) } - factory := &fakeEvictFactory{eviction} + factory := &restriction.FakePodsRestrictionFactory{ + Eviction: eviction, + InPlace: inplace, + } vpaLister := &test.VerticalPodAutoscalerListerMock{} podLister := &test.PodListerMock{} @@ -173,12 +234,14 @@ func testRunOnceBase( Name: rc.Name, APIVersion: rc.APIVersion, } + vpaObj := test.VerticalPodAutoscaler(). WithContainer(containerName). WithTarget("2", "200M"). WithMinAllowed(containerName, "1", "100M"). WithMaxAllowed(containerName, "3", "1G"). - WithTargetRef(targetRef).Get() + WithTargetRef(targetRef). + Get() vpaObj.Spec.UpdatePolicy = &vpa_types.PodUpdatePolicy{UpdateMode: &updateMode} vpaLister.On("List").Return([]*vpa_types.VerticalPodAutoscaler{vpaObj}, nil).Once() @@ -188,8 +251,9 @@ func testRunOnceBase( updater := &updater{ vpaLister: vpaLister, podLister: podLister, - evictionFactory: factory, + restrictionFactory: factory, evictionRateLimiter: rate.NewLimiter(rate.Inf, 0), + inPlaceRateLimiter: rate.NewLimiter(rate.Inf, 0), evictionAdmission: priority.NewDefaultPodEvictionAdmission(), recommendationProcessor: &test.FakeRecommendationProcessor{}, selectorFetcher: mockSelectorFetcher, @@ -204,11 +268,16 @@ func testRunOnceBase( } updater.RunOnce(context.Background()) eviction.AssertNumberOfCalls(t, "Evict", expectedEvictionCount) + inplace.AssertNumberOfCalls(t, "InPlaceUpdate", expectedInPlacedCount) } func TestRunOnceNotingToProcess(t *testing.T) { eviction := &test.PodsEvictionRestrictionMock{} - factory := &fakeEvictFactory{eviction} + inplace := &test.PodsInPlaceRestrictionMock{} + factory := &restriction.FakePodsRestrictionFactory{ + Eviction: eviction, + InPlace: inplace, + } vpaLister := &test.VerticalPodAutoscalerListerMock{} podLister := &test.PodListerMock{} vpaLister.On("List").Return(nil, nil).Once() @@ -216,8 +285,9 @@ func TestRunOnceNotingToProcess(t *testing.T) { updater := &updater{ vpaLister: vpaLister, podLister: podLister, - evictionFactory: factory, + restrictionFactory: factory, evictionRateLimiter: rate.NewLimiter(rate.Inf, 0), + inPlaceRateLimiter: rate.NewLimiter(rate.Inf, 0), evictionAdmission: priority.NewDefaultPodEvictionAdmission(), recommendationProcessor: &test.FakeRecommendationProcessor{}, useAdmissionControllerStatus: true, @@ -243,14 +313,6 @@ func TestGetRateLimiter(t *testing.T) { } } -type fakeEvictFactory struct { - evict eviction.PodsEvictionRestriction -} - -func (f fakeEvictFactory) NewPodsEvictionRestriction(pods []*apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler) eviction.PodsEvictionRestriction { - return f.evict -} - type fakeValidator struct { isValid bool } @@ -288,7 +350,7 @@ func TestRunOnceIgnoreNamespaceMatchingPods(t *testing.T) { } pods := make([]*apiv1.Pod, livePods) eviction := &test.PodsEvictionRestrictionMock{} - + inplace := &test.PodsInPlaceRestrictionMock{} for i := range pods { pods[i] = test.Pod().WithName("test_"+strconv.Itoa(i)). AddContainer(test.Container().WithName(containerName).WithCPURequest(resource.MustParse("1")).WithMemRequest(resource.MustParse("100M")).Get()). @@ -300,7 +362,10 @@ func TestRunOnceIgnoreNamespaceMatchingPods(t *testing.T) { eviction.On("Evict", pods[i], nil).Return(nil) } - factory := &fakeEvictFactory{eviction} + factory := &restriction.FakePodsRestrictionFactory{ + Eviction: eviction, + InPlace: inplace, + } vpaLister := &test.VerticalPodAutoscalerListerMock{} podLister := &test.PodListerMock{} @@ -310,13 +375,15 @@ func TestRunOnceIgnoreNamespaceMatchingPods(t *testing.T) { Name: rc.Name, APIVersion: rc.APIVersion, } + vpaObj := test.VerticalPodAutoscaler(). WithNamespace("default"). WithContainer(containerName). WithTarget("2", "200M"). WithMinAllowed(containerName, "1", "100M"). WithMaxAllowed(containerName, "3", "1G"). - WithTargetRef(targetRef).Get() + WithTargetRef(targetRef). + Get() vpaLister.On("List").Return([]*vpa_types.VerticalPodAutoscaler{vpaObj}, nil).Once() @@ -326,8 +393,9 @@ func TestRunOnceIgnoreNamespaceMatchingPods(t *testing.T) { updater := &updater{ vpaLister: vpaLister, podLister: podLister, - evictionFactory: factory, + restrictionFactory: factory, evictionRateLimiter: rate.NewLimiter(rate.Inf, 0), + inPlaceRateLimiter: rate.NewLimiter(rate.Inf, 0), evictionAdmission: priority.NewDefaultPodEvictionAdmission(), recommendationProcessor: &test.FakeRecommendationProcessor{}, selectorFetcher: mockSelectorFetcher, @@ -358,6 +426,7 @@ func TestRunOnceIgnoreNamespaceMatching(t *testing.T) { updater.RunOnce(context.Background()) eviction.AssertNumberOfCalls(t, "Evict", 0) + eviction.AssertNumberOfCalls(t, "InPlaceUpdate", 0) } func TestNewEventRecorder(t *testing.T) { diff --git a/vertical-pod-autoscaler/pkg/updater/main.go b/vertical-pod-autoscaler/pkg/updater/main.go index 4a183b95cd5..a384aff9dca 100644 --- a/vertical-pod-autoscaler/pkg/updater/main.go +++ b/vertical-pod-autoscaler/pkg/updater/main.go @@ -36,9 +36,13 @@ import ( "k8s.io/klog/v2" "k8s.io/autoscaler/vertical-pod-autoscaler/common" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation" vpa_clientset "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target" controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/inplace" updater "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/logic" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/priority" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange" @@ -88,6 +92,8 @@ func main() { leaderElection := defaultLeaderElectionConfiguration() componentbaseoptions.BindLeaderElectionFlags(&leaderElection, pflag.CommandLine) + features.MutableFeatureGate.AddFlag(pflag.CommandLine) + kube_flag.InitFlags() klog.V(1).InfoS("Vertical Pod Autoscaler Updater", "version", common.VerticalPodAutoscalerVersion()) @@ -185,6 +191,10 @@ func run(healthCheck *metrics.HealthCheck, commonFlag *common.CommonFlags) { ignoredNamespaces := strings.Split(commonFlag.IgnoredVpaObjectNamespaces, ",") + recommendationProvider := recommendation.NewProvider(limitRangeCalculator, vpa_api_util.NewCappingRecommendationProcessor(limitRangeCalculator)) + + calculators := []patch.Calculator{inplace.NewResourceInPlaceUpdatesCalculator(recommendationProvider), inplace.NewInPlaceUpdatedCalculator()} + // TODO: use SharedInformerFactory in updater updater, err := updater.NewUpdater( kubeClient, @@ -202,6 +212,7 @@ func run(healthCheck *metrics.HealthCheck, commonFlag *common.CommonFlags) { priority.NewProcessor(), commonFlag.VpaObjectNamespace, ignoredNamespaces, + calculators, ) if err != nil { klog.ErrorS(err, "Failed to create updater") diff --git a/vertical-pod-autoscaler/pkg/updater/restriction/fake_pods_restriction.go b/vertical-pod-autoscaler/pkg/updater/restriction/fake_pods_restriction.go new file mode 100644 index 00000000000..4637f0b2cc7 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/updater/restriction/fake_pods_restriction.go @@ -0,0 +1,46 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package restriction + +import ( + apiv1 "k8s.io/api/core/v1" + + vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" +) + +// FakePodsRestrictionFactory is a fake implementation of the PodsRestrictionFactory interface. +type FakePodsRestrictionFactory struct { + // Eviction is the fake eviction restriction. + Eviction PodsEvictionRestriction + // InPlace is the fake in-place restriction. + InPlace PodsInPlaceRestriction +} + +// NewPodsEvictionRestriction returns the fake eviction restriction. +func (f *FakePodsRestrictionFactory) NewPodsEvictionRestriction(creatorToSingleGroupStatsMap map[podReplicaCreator]singleGroupStats, podToReplicaCreatorMap map[string]podReplicaCreator) PodsEvictionRestriction { + return f.Eviction +} + +// NewPodsInPlaceRestriction returns the fake in-place restriction. +func (f *FakePodsRestrictionFactory) NewPodsInPlaceRestriction(creatorToSingleGroupStatsMap map[podReplicaCreator]singleGroupStats, podToReplicaCreatorMap map[string]podReplicaCreator) PodsInPlaceRestriction { + return f.InPlace +} + +// GetCreatorMaps returns nil maps. +func (f *FakePodsRestrictionFactory) GetCreatorMaps(pods []*apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler) (map[podReplicaCreator]singleGroupStats, map[string]podReplicaCreator, error) { + return nil, nil, nil +} diff --git a/vertical-pod-autoscaler/pkg/updater/restriction/pods_eviction_restriction.go b/vertical-pod-autoscaler/pkg/updater/restriction/pods_eviction_restriction.go new file mode 100644 index 00000000000..4a423781e33 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/updater/restriction/pods_eviction_restriction.go @@ -0,0 +1,112 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package restriction + +import ( + "context" + "fmt" + "time" + + apiv1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kube_client "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/record" + "k8s.io/klog/v2" + "k8s.io/utils/clock" + + vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" +) + +// PodsEvictionRestriction controls pods evictions. It ensures that we will not evict too +// many pods from one replica set. For replica set will allow to evict one pod or more if +// evictionToleranceFraction is configured. +type PodsEvictionRestriction interface { + // Evict sends eviction instruction to the api client. + // Returns error if pod cannot be evicted or if client returned error. + Evict(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error + // CanEvict checks if pod can be safely evicted + CanEvict(pod *apiv1.Pod) bool +} + +// PodsEvictionRestrictionImpl is the implementation of the PodsEvictionRestriction interface. +type PodsEvictionRestrictionImpl struct { + client kube_client.Interface + podToReplicaCreatorMap map[string]podReplicaCreator + creatorToSingleGroupStatsMap map[podReplicaCreator]singleGroupStats + clock clock.Clock + lastInPlaceAttemptTimeMap map[string]time.Time +} + +// CanEvict checks if pod can be safely evicted +func (e *PodsEvictionRestrictionImpl) CanEvict(pod *apiv1.Pod) bool { + cr, present := e.podToReplicaCreatorMap[getPodID(pod)] + if present { + singleGroupStats, present := e.creatorToSingleGroupStatsMap[cr] + if pod.Status.Phase == apiv1.PodPending { + return true + } + if present { + if isInPlaceUpdating(pod) { + return CanEvictInPlacingPod(pod, singleGroupStats, e.lastInPlaceAttemptTimeMap, e.clock) + } + return singleGroupStats.isPodDisruptable() + } + } + return false +} + +// Evict sends eviction instruction to api client. Returns error if pod cannot be evicted or if client returned error +// Does not check if pod was actually evicted after eviction grace period. +func (e *PodsEvictionRestrictionImpl) Evict(podToEvict *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error { + cr, present := e.podToReplicaCreatorMap[getPodID(podToEvict)] + if !present { + return fmt.Errorf("pod not suitable for eviction %s/%s: not in replicated pods map", podToEvict.Namespace, podToEvict.Name) + } + + if !e.CanEvict(podToEvict) { + return fmt.Errorf("cannot evict pod %s/%s: eviction budget exceeded", podToEvict.Namespace, podToEvict.Name) + } + + eviction := &policyv1.Eviction{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: podToEvict.Namespace, + Name: podToEvict.Name, + }, + } + err := e.client.CoreV1().Pods(podToEvict.Namespace).EvictV1(context.TODO(), eviction) + if err != nil { + klog.ErrorS(err, "Failed to evict pod", "pod", klog.KObj(podToEvict)) + return err + } + eventRecorder.Event(podToEvict, apiv1.EventTypeNormal, "EvictedByVPA", + "Pod was evicted by VPA Updater to apply resource recommendation.") + + eventRecorder.Event(vpa, apiv1.EventTypeNormal, "EvictedPod", + "VPA Updater evicted Pod "+podToEvict.Name+" to apply resource recommendation.") + + if podToEvict.Status.Phase != apiv1.PodPending { + singleGroupStats, present := e.creatorToSingleGroupStatsMap[cr] + if !present { + return fmt.Errorf("Internal error - cannot find stats for replication group %v", cr) + } + singleGroupStats.evicted = singleGroupStats.evicted + 1 + e.creatorToSingleGroupStatsMap[cr] = singleGroupStats + } + + return nil +} diff --git a/vertical-pod-autoscaler/pkg/updater/restriction/pods_eviction_restriction_test.go b/vertical-pod-autoscaler/pkg/updater/restriction/pods_eviction_restriction_test.go new file mode 100644 index 00000000000..4bb4b032762 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/updater/restriction/pods_eviction_restriction_test.go @@ -0,0 +1,259 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package restriction + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + baseclocktest "k8s.io/utils/clock/testing" + + vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/test" +) + +func TestEvictTooFewReplicas(t *testing.T) { + replicas := int32(5) + livePods := 5 + + rc := apiv1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rc", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicationController", + }, + Spec: apiv1.ReplicationControllerSpec{ + Replicas: &replicas, + }, + } + + pods := make([]*apiv1.Pod, livePods) + for i := range pods { + pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() + } + + basicVpa := getBasicVpa() + factory, err := getRestrictionFactory(&rc, nil, nil, nil, 10, 0.5, nil, nil, nil) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa) + assert.NoError(t, err) + eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + + for _, pod := range pods { + assert.False(t, eviction.CanEvict(pod)) + } + + for _, pod := range pods { + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) + assert.Error(t, err, "Error expected") + } +} + +func TestEvictionTolerance(t *testing.T) { + replicas := int32(5) + livePods := 5 + tolerance := 0.8 + + rc := apiv1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rc", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicationController", + }, + Spec: apiv1.ReplicationControllerSpec{ + Replicas: &replicas, + }, + } + + pods := make([]*apiv1.Pod, livePods) + for i := range pods { + pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() + } + + basicVpa := getBasicVpa() + factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2 /*minReplicas*/, tolerance, nil, nil, nil) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa) + assert.NoError(t, err) + eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + + for _, pod := range pods { + assert.True(t, eviction.CanEvict(pod)) + } + + for _, pod := range pods[:4] { + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) + assert.Nil(t, err, "Should evict with no error") + } + for _, pod := range pods[4:] { + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) + assert.Error(t, err, "Error expected") + } +} + +func TestEvictAtLeastOne(t *testing.T) { + replicas := int32(5) + livePods := 5 + tolerance := 0.1 + + rc := apiv1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rc", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicationController", + }, + Spec: apiv1.ReplicationControllerSpec{ + Replicas: &replicas, + }, + } + + pods := make([]*apiv1.Pod, livePods) + for i := range pods { + pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() + } + + basicVpa := getBasicVpa() + factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2, tolerance, nil, nil, nil) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa) + assert.NoError(t, err) + eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + + for _, pod := range pods { + assert.True(t, eviction.CanEvict(pod)) + } + + for _, pod := range pods[:1] { + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) + assert.Nil(t, err, "Should evict with no error") + } + for _, pod := range pods[1:] { + err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) + assert.Error(t, err, "Error expected") + } +} + +func TestEvictEmitEvent(t *testing.T) { + rc := apiv1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rc", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicationController", + }, + } + + index := 0 + generatePod := func() test.PodBuilder { + index++ + return test.Pod().WithName(fmt.Sprintf("test-%v", index)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta) + } + + basicVpa := getBasicVpa() + + testCases := []struct { + name string + replicas int32 + evictionTolerance float64 + vpa *vpa_types.VerticalPodAutoscaler + pods []podWithExpectations + errorExpected bool + }{ + { + name: "Pods that can be evicted", + replicas: 4, + evictionTolerance: 0.5, + vpa: basicVpa, + pods: []podWithExpectations{ + { + pod: generatePod().WithPhase(apiv1.PodPending).Get(), + canEvict: true, + evictionSuccess: true, + }, + { + pod: generatePod().WithPhase(apiv1.PodPending).Get(), + canEvict: true, + evictionSuccess: true, + }, + }, + errorExpected: false, + }, + { + name: "Pod that can not be evicted", + replicas: 4, + evictionTolerance: 0.5, + vpa: basicVpa, + pods: []podWithExpectations{ + + { + pod: generatePod().Get(), + canEvict: false, + evictionSuccess: false, + }, + }, + errorExpected: true, + }, + } + + for _, testCase := range testCases { + rc.Spec = apiv1.ReplicationControllerSpec{ + Replicas: &testCase.replicas, + } + pods := make([]*apiv1.Pod, 0, len(testCase.pods)) + for _, p := range testCase.pods { + pods = append(pods, p.pod) + } + clock := baseclocktest.NewFakeClock(time.Time{}) + factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance, clock, map[string]time.Time{}, nil) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, testCase.vpa) + assert.NoError(t, err) + eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + + for _, p := range testCase.pods { + mockRecorder := test.MockEventRecorder() + mockRecorder.On("Event", mock.Anything, apiv1.EventTypeNormal, "EvictedByVPA", mock.Anything).Return() + mockRecorder.On("Event", mock.Anything, apiv1.EventTypeNormal, "EvictedPod", mock.Anything).Return() + + errGot := eviction.Evict(p.pod, testCase.vpa, mockRecorder) + if testCase.errorExpected { + assert.Error(t, errGot) + } else { + assert.NoError(t, errGot) + } + + if p.canEvict { + mockRecorder.AssertNumberOfCalls(t, "Event", 2) + + } else { + mockRecorder.AssertNumberOfCalls(t, "Event", 0) + } + } + } +} diff --git a/vertical-pod-autoscaler/pkg/updater/restriction/pods_inplace_restriction.go b/vertical-pod-autoscaler/pkg/updater/restriction/pods_inplace_restriction.go new file mode 100644 index 00000000000..8a86cef1b57 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/updater/restriction/pods_inplace_restriction.go @@ -0,0 +1,218 @@ +/* +Copyright 2015 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package restriction + +import ( + "context" + "fmt" + "time" + + apiv1 "k8s.io/api/core/v1" + kube_client "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/record" + "k8s.io/klog/v2" + "k8s.io/utils/clock" + + vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" + + "encoding/json" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8stypes "k8s.io/apimachinery/pkg/types" + + utils "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/utils" + + resource_updates "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch" +) + +// TODO: Make these configurable by flags +const ( + // DeferredResizeUpdateTimeout defines the duration during which an in-place resize request + // is considered deferred. If the resize is not completed within this time, it falls back to eviction. + DeferredResizeUpdateTimeout = 5 * time.Minute + + // InProgressResizeUpdateTimeout defines the duration during which an in-place resize request + // is considered in progress. If the resize is not completed within this time, it falls back to eviction. + InProgressResizeUpdateTimeout = 1 * time.Hour +) + +// PodsInPlaceRestriction controls pods in-place updates. It ensures that we will not update too +// many pods from one replica set. For replica set will allow to update one pod or more if +// inPlaceToleranceFraction is configured. +type PodsInPlaceRestriction interface { + // InPlaceUpdate attempts to actuate the in-place resize. + // Returns error if client returned error. + InPlaceUpdate(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error + // CanInPlaceUpdate checks if pod can be safely updated in-place. If not, it will return a decision to potentially evict the pod. + CanInPlaceUpdate(pod *apiv1.Pod) utils.InPlaceDecision +} + +// PodsInPlaceRestrictionImpl is the implementation of the PodsInPlaceRestriction interface. +type PodsInPlaceRestrictionImpl struct { + client kube_client.Interface + podToReplicaCreatorMap map[string]podReplicaCreator + creatorToSingleGroupStatsMap map[podReplicaCreator]singleGroupStats + patchCalculators []patch.Calculator + clock clock.Clock + lastInPlaceAttemptTimeMap map[string]time.Time +} + +// CanInPlaceUpdate checks if pod can be safely updated +func (ip *PodsInPlaceRestrictionImpl) CanInPlaceUpdate(pod *apiv1.Pod) utils.InPlaceDecision { + if !features.Enabled(features.InPlaceOrRecreate) { + return utils.InPlaceEvict + } + + cr, present := ip.podToReplicaCreatorMap[getPodID(pod)] + if present { + singleGroupStats, present := ip.creatorToSingleGroupStatsMap[cr] + if pod.Status.Phase == apiv1.PodPending { + return utils.InPlaceDeferred + } + if present { + if isInPlaceUpdating(pod) { + canEvict := CanEvictInPlacingPod(pod, singleGroupStats, ip.lastInPlaceAttemptTimeMap, ip.clock) + if canEvict { + return utils.InPlaceEvict + } + return utils.InPlaceDeferred + } + if singleGroupStats.isPodDisruptable() { + return utils.InPlaceApproved + } + } + } + klog.V(4).InfoS("Can't in-place update pod, but not falling back to eviction. Waiting for next loop", "pod", klog.KObj(pod)) + return utils.InPlaceDeferred +} + +// InPlaceUpdate sends calculates patches and sends resize request to api client. Returns error if pod cannot be in-place updated or if client returned error. +// Does not check if pod was actually in-place updated after grace period. +func (ip *PodsInPlaceRestrictionImpl) InPlaceUpdate(podToUpdate *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error { + cr, present := ip.podToReplicaCreatorMap[getPodID(podToUpdate)] + if !present { + return fmt.Errorf("pod not suitable for in-place update %v: not in replicated pods map", podToUpdate.Name) + } + + if ip.CanInPlaceUpdate(podToUpdate) != utils.InPlaceApproved { + return fmt.Errorf("cannot in-place update pod %s", klog.KObj(podToUpdate)) + } + + // separate patches since we have to patch resize and spec separately + resizePatches := []resource_updates.PatchRecord{} + annotationPatches := []resource_updates.PatchRecord{} + if podToUpdate.Annotations == nil { + annotationPatches = append(annotationPatches, patch.GetAddEmptyAnnotationsPatch()) + } + for _, calculator := range ip.patchCalculators { + p, err := calculator.CalculatePatches(podToUpdate, vpa) + if err != nil { + return err + } + klog.V(4).InfoS("Calculated patches for pod", "pod", klog.KObj(podToUpdate), "patches", p) + if calculator.PatchResourceTarget() == patch.Resize { + resizePatches = append(resizePatches, p...) + } else { + annotationPatches = append(annotationPatches, p...) + } + } + if len(resizePatches) > 0 { + patch, err := json.Marshal(resizePatches) + if err != nil { + return err + } + + res, err := ip.client.CoreV1().Pods(podToUpdate.Namespace).Patch(context.TODO(), podToUpdate.Name, k8stypes.JSONPatchType, patch, metav1.PatchOptions{}, "resize") + if err != nil { + return err + } + klog.V(4).InfoS("In-place patched pod /resize subresource using patches", "pod", klog.KObj(res), "patches", string(patch)) + + if len(annotationPatches) > 0 { + patch, err := json.Marshal(annotationPatches) + if err != nil { + return err + } + res, err = ip.client.CoreV1().Pods(podToUpdate.Namespace).Patch(context.TODO(), podToUpdate.Name, k8stypes.JSONPatchType, patch, metav1.PatchOptions{}) + if err != nil { + return err + } + klog.V(4).InfoS("Patched pod annotations", "pod", klog.KObj(res), "patches", string(patch)) + } + } else { + return fmt.Errorf("no resource patches were calculated to apply") + } + + // TODO(maxcao13): If this keeps getting called on the same object with the same reason, it is considered a patch request. + // And we fail to have the corresponding rbac for it. So figure out if we need this later. + // Do we even need to emit an event? The node might reject the resize request. If so, should we rename this to InPlaceResizeAttempted? + // eventRecorder.Event(podToUpdate, apiv1.EventTypeNormal, "InPlaceResizedByVPA", "Pod was resized in place by VPA Updater.") + + singleGroupStats, present := ip.creatorToSingleGroupStatsMap[cr] + if !present { + klog.InfoS("Internal error - cannot find stats for replication group", "pod", klog.KObj(podToUpdate), "podReplicaCreator", cr) + } else { + singleGroupStats.inPlaceUpdateInitiated = singleGroupStats.inPlaceUpdateInitiated + 1 + ip.creatorToSingleGroupStatsMap[cr] = singleGroupStats + } + + return nil +} + +// CanEvictInPlacingPod checks if the pod can be evicted while it is currently in the middle of an in-place update. +func CanEvictInPlacingPod(pod *apiv1.Pod, singleGroupStats singleGroupStats, lastInPlaceAttemptTimeMap map[string]time.Time, clock clock.Clock) bool { + if !isInPlaceUpdating(pod) { + return false + } + lastUpdate, exists := lastInPlaceAttemptTimeMap[getPodID(pod)] + if !exists { + klog.V(4).InfoS("In-place update in progress for pod but no lastUpdateTime found, setting it to now", "pod", klog.KObj(pod)) + lastUpdate = clock.Now() + lastInPlaceAttemptTimeMap[getPodID(pod)] = lastUpdate + } + + if singleGroupStats.isPodDisruptable() { + // TODO(maxcao13): fix this after 1.33 KEP changes + // if currently inPlaceUpdating, we should only fallback to eviction if the update has failed. i.e: one of the following conditions: + // 1. .status.resize: Infeasible + // 2. .status.resize: Deferred + more than 5 minutes has elapsed since the lastInPlaceUpdateTime + // 3. .status.resize: InProgress + more than 1 hour has elapsed since the lastInPlaceUpdateTime + switch pod.Status.Resize { + case apiv1.PodResizeStatusDeferred: + if clock.Since(lastUpdate) > DeferredResizeUpdateTimeout { + klog.V(4).InfoS(fmt.Sprintf("In-place update deferred for more than %v, falling back to eviction", DeferredResizeUpdateTimeout), "pod", klog.KObj(pod)) + return true + } + case apiv1.PodResizeStatusInProgress: + if clock.Since(lastUpdate) > InProgressResizeUpdateTimeout { + klog.V(4).InfoS(fmt.Sprintf("In-place update in progress for more than %v, falling back to eviction", InProgressResizeUpdateTimeout), "pod", klog.KObj(pod)) + return true + } + case apiv1.PodResizeStatusInfeasible: + klog.V(4).InfoS("In-place update infeasible, falling back to eviction", "pod", klog.KObj(pod)) + return true + default: + klog.V(4).InfoS("In-place update status unknown, falling back to eviction", "pod", klog.KObj(pod)) + return true + } + return false + } + klog.V(4).InfoS("Would be able to evict, but already resizing", "pod", klog.KObj(pod)) + return false +} diff --git a/vertical-pod-autoscaler/pkg/updater/restriction/pods_inplace_restriction_test.go b/vertical-pod-autoscaler/pkg/updater/restriction/pods_inplace_restriction_test.go new file mode 100644 index 00000000000..1c08f427fb1 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/updater/restriction/pods_inplace_restriction_test.go @@ -0,0 +1,360 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package restriction + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + featuregatetesting "k8s.io/component-base/featuregate/testing" + baseclocktest "k8s.io/utils/clock/testing" + + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" + utils "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/utils" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/test" +) + +type CanInPlaceUpdateTestParams struct { + name string + pods []*apiv1.Pod + replicas int32 + evictionTolerance float64 + lastInPlaceAttempt time.Time + expectedInPlaceDecision utils.InPlaceDecision +} + +func TestCanInPlaceUpdate(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, features.MutableFeatureGate, features.InPlaceOrRecreate, true) + + rc := apiv1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rc", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicationController", + }, + } + + index := 0 + generatePod := func() test.PodBuilder { + index++ + return test.Pod().WithName(fmt.Sprintf("test-%v", index)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta) + } + // NOTE: the pod we are checking for CanInPlaceUpdate will always be the first one for these tests + whichPodIdxForCanInPlaceUpdate := 0 + + testCases := []CanInPlaceUpdateTestParams{ + { + name: "CanInPlaceUpdate=InPlaceApproved - (half of 3)", + pods: []*apiv1.Pod{ + generatePod().Get(), + generatePod().Get(), + generatePod().Get(), + }, + replicas: 3, + evictionTolerance: 0.5, + lastInPlaceAttempt: time.Time{}, + expectedInPlaceDecision: utils.InPlaceApproved, + }, + { + name: "CanInPlaceUpdate=InPlaceDeferred - no pods can be in-placed, one missing", + pods: []*apiv1.Pod{ + generatePod().Get(), + generatePod().Get(), + }, + replicas: 3, + evictionTolerance: 0.5, + lastInPlaceAttempt: time.Time{}, + expectedInPlaceDecision: utils.InPlaceDeferred, + }, + { + name: "CanInPlaceUpdate=InPlaceApproved - small tolerance, all running", + pods: []*apiv1.Pod{ + generatePod().Get(), + generatePod().Get(), + generatePod().Get(), + }, + replicas: 3, + evictionTolerance: 0.1, + lastInPlaceAttempt: time.Time{}, + expectedInPlaceDecision: utils.InPlaceApproved, + }, + { + name: "CanInPlaceUpdate=InPlaceApproved - small tolerance, one missing", + pods: []*apiv1.Pod{ + generatePod().Get(), + generatePod().Get(), + }, + replicas: 3, + evictionTolerance: 0.5, + lastInPlaceAttempt: time.Time{}, + expectedInPlaceDecision: utils.InPlaceDeferred, + }, + { + name: "CanInPlaceUpdate=InPlaceDeferred - resize Deferred, conditions not met to fallback", + pods: []*apiv1.Pod{ + generatePod().WithResizeStatus(apiv1.PodResizeStatusDeferred).Get(), + generatePod().Get(), + generatePod().Get(), + }, + replicas: 3, + evictionTolerance: 0.5, + lastInPlaceAttempt: time.UnixMilli(3600000), // 1 hour from epoch + expectedInPlaceDecision: utils.InPlaceDeferred, + }, + { + name: ("CanInPlaceUpdate=InPlaceEvict - resize inProgress for more too long"), + pods: []*apiv1.Pod{ + generatePod().WithResizeStatus(apiv1.PodResizeStatusInProgress).Get(), + generatePod().Get(), + generatePod().Get(), + }, + replicas: 3, + evictionTolerance: 0.5, + lastInPlaceAttempt: time.UnixMilli(0), // epoch (too long ago...) + expectedInPlaceDecision: utils.InPlaceEvict, + }, + { + name: "CanInPlaceUpdate=InPlaceDeferred - resize InProgress, conditions not met to fallback", + pods: []*apiv1.Pod{ + generatePod().WithResizeStatus(apiv1.PodResizeStatusInProgress).Get(), + generatePod().Get(), + generatePod().Get(), + }, + replicas: 3, + evictionTolerance: 0.5, + lastInPlaceAttempt: time.UnixMilli(3600000), // 1 hour from epoch + expectedInPlaceDecision: utils.InPlaceDeferred, + }, + { + name: "CanInPlaceUpdate=InPlaceEvict - infeasible", + pods: []*apiv1.Pod{ + generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(), + generatePod().Get(), + generatePod().Get(), + }, + replicas: 3, + evictionTolerance: 0.5, + lastInPlaceAttempt: time.Time{}, + expectedInPlaceDecision: utils.InPlaceEvict, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + rc.Spec = apiv1.ReplicationControllerSpec{ + Replicas: &tc.replicas, + } + + selectedPod := tc.pods[whichPodIdxForCanInPlaceUpdate] + + clock := baseclocktest.NewFakeClock(time.UnixMilli(3600001)) // 1 hour from epoch + 1 millis + lipatm := map[string]time.Time{getPodID(selectedPod): tc.lastInPlaceAttempt} + + factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2, tc.evictionTolerance, clock, lipatm, GetFakeCalculatorsWithFakeResourceCalc()) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(tc.pods, getIPORVpa()) + assert.NoError(t, err) + inPlace := factory.NewPodsInPlaceRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + + result := inPlace.CanInPlaceUpdate(selectedPod) + assert.Equal(t, tc.expectedInPlaceDecision, result) + }) + } +} + +func TestInPlaceDisabledFeatureGate(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, features.MutableFeatureGate, features.InPlaceOrRecreate, false) + + replicas := int32(5) + livePods := 5 + tolerance := 1.0 + + rc := apiv1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rc", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicationController", + }, + Spec: apiv1.ReplicationControllerSpec{ + Replicas: &replicas, + }, + } + + pods := make([]*apiv1.Pod, livePods) + for i := range pods { + pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() + } + + basicVpa := getBasicVpa() + factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2, tolerance, nil, nil, GetFakeCalculatorsWithFakeResourceCalc()) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa) + assert.NoError(t, err) + inplace := factory.NewPodsInPlaceRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + + for _, pod := range pods { + assert.Equal(t, utils.InPlaceEvict, inplace.CanInPlaceUpdate(pod)) + } +} + +func TestInPlaceTooFewReplicas(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, features.MutableFeatureGate, features.InPlaceOrRecreate, true) + + replicas := int32(5) + livePods := 5 + tolerance := 0.5 + + rc := apiv1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rc", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicationController", + }, + Spec: apiv1.ReplicationControllerSpec{ + Replicas: &replicas, + }, + } + + pods := make([]*apiv1.Pod, livePods) + for i := range pods { + pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() + } + + clock := baseclocktest.NewFakeClock(time.Time{}) + lipatm := map[string]time.Time{} + + basicVpa := getIPORVpa() + factory, err := getRestrictionFactory(&rc, nil, nil, nil, 10 /*minReplicas*/, tolerance, clock, lipatm, GetFakeCalculatorsWithFakeResourceCalc()) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa) + assert.NoError(t, err) + inplace := factory.NewPodsInPlaceRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + + for _, pod := range pods { + assert.Equal(t, utils.InPlaceDeferred, inplace.CanInPlaceUpdate(pod)) + } + + for _, pod := range pods { + err := inplace.InPlaceUpdate(pod, basicVpa, test.FakeEventRecorder()) + assert.Error(t, err, "Error expected") + } +} + +func TestEvictionToleranceForInPlace(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, features.MutableFeatureGate, features.InPlaceOrRecreate, true) + + replicas := int32(5) + livePods := 5 + tolerance := 0.8 + + rc := apiv1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rc", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicationController", + }, + Spec: apiv1.ReplicationControllerSpec{ + Replicas: &replicas, + }, + } + + pods := make([]*apiv1.Pod, livePods) + for i := range pods { + pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() + } + + clock := baseclocktest.NewFakeClock(time.Time{}) + lipatm := map[string]time.Time{} + + basicVpa := getIPORVpa() + factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2 /*minReplicas*/, tolerance, clock, lipatm, GetFakeCalculatorsWithFakeResourceCalc()) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa) + assert.NoError(t, err) + inplace := factory.NewPodsInPlaceRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + + for _, pod := range pods { + assert.Equal(t, utils.InPlaceApproved, inplace.CanInPlaceUpdate(pod)) + } + + for _, pod := range pods[:4] { + err := inplace.InPlaceUpdate(pod, basicVpa, test.FakeEventRecorder()) + assert.Nil(t, err, "Should evict with no error") + } + for _, pod := range pods[4:] { + err := inplace.InPlaceUpdate(pod, basicVpa, test.FakeEventRecorder()) + assert.Error(t, err, "Error expected") + } +} + +func TestInPlaceAtLeastOne(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, features.MutableFeatureGate, features.InPlaceOrRecreate, true) + + replicas := int32(5) + livePods := 5 + tolerance := 0.1 + + rc := apiv1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rc", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicationController", + }, + Spec: apiv1.ReplicationControllerSpec{ + Replicas: &replicas, + }, + } + + pods := make([]*apiv1.Pod, livePods) + for i := range pods { + pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() + } + + basicVpa := getBasicVpa() + factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2, tolerance, nil, nil, GetFakeCalculatorsWithFakeResourceCalc()) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa) + assert.NoError(t, err) + inplace := factory.NewPodsInPlaceRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + + for _, pod := range pods { + assert.Equal(t, utils.InPlaceApproved, inplace.CanInPlaceUpdate(pod)) + } + + for _, pod := range pods[:1] { + err := inplace.InPlaceUpdate(pod, basicVpa, test.FakeEventRecorder()) + assert.Nil(t, err, "Should in-place update with no error") + } + for _, pod := range pods[1:] { + err := inplace.InPlaceUpdate(pod, basicVpa, test.FakeEventRecorder()) + assert.Error(t, err, "Error expected") + } +} diff --git a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go b/vertical-pod-autoscaler/pkg/updater/restriction/pods_restriction_factory.go similarity index 63% rename from vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go rename to vertical-pod-autoscaler/pkg/updater/restriction/pods_restriction_factory.go index ca6452d8e16..9dc99fa5b6e 100644 --- a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go +++ b/vertical-pod-autoscaler/pkg/updater/restriction/pods_restriction_factory.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +Copyright 2025 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -14,24 +14,23 @@ See the License for the specific language governing permissions and limitations under the License. */ -package eviction +package restriction import ( - "context" "fmt" "time" appsv1 "k8s.io/api/apps/v1" apiv1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" appsinformer "k8s.io/client-go/informers/apps/v1" coreinformer "k8s.io/client-go/informers/core/v1" kube_client "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/tools/record" "k8s.io/klog/v2" + "k8s.io/utils/clock" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" ) @@ -39,48 +38,7 @@ const ( resyncPeriod time.Duration = 1 * time.Minute ) -// PodsEvictionRestriction controls pods evictions. It ensures that we will not evict too -// many pods from one replica set. For replica set will allow to evict one pod or more if -// evictionToleranceFraction is configured. -type PodsEvictionRestriction interface { - // Evict sends eviction instruction to the api client. - // Returns error if pod cannot be evicted or if client returned error. - Evict(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error - // CanEvict checks if pod can be safely evicted - CanEvict(pod *apiv1.Pod) bool -} - -type podsEvictionRestrictionImpl struct { - client kube_client.Interface - podToReplicaCreatorMap map[string]podReplicaCreator - creatorToSingleGroupStatsMap map[podReplicaCreator]singleGroupStats -} - -type singleGroupStats struct { - configured int - pending int - running int - evictionTolerance int - evicted int -} - -// PodsEvictionRestrictionFactory creates PodsEvictionRestriction -type PodsEvictionRestrictionFactory interface { - // NewPodsEvictionRestriction creates PodsEvictionRestriction for given set of pods, - // controlled by a single VPA object. - NewPodsEvictionRestriction(pods []*apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler) PodsEvictionRestriction -} - -type podsEvictionRestrictionFactoryImpl struct { - client kube_client.Interface - rcInformer cache.SharedIndexInformer // informer for Replication Controllers - ssInformer cache.SharedIndexInformer // informer for Stateful Sets - rsInformer cache.SharedIndexInformer // informer for Replica Sets - dsInformer cache.SharedIndexInformer // informer for Daemon Sets - minReplicas int - evictionToleranceFraction float64 -} - +// ControllerKind is the type of controller that can manage a pod. type controllerKind string const ( @@ -97,107 +55,132 @@ type podReplicaCreator struct { Kind controllerKind } -// CanEvict checks if pod can be safely evicted -func (e *podsEvictionRestrictionImpl) CanEvict(pod *apiv1.Pod) bool { - cr, present := e.podToReplicaCreatorMap[getPodID(pod)] - if present { - singleGroupStats, present := e.creatorToSingleGroupStatsMap[cr] - if pod.Status.Phase == apiv1.PodPending { - return true - } - if present { - shouldBeAlive := singleGroupStats.configured - singleGroupStats.evictionTolerance - if singleGroupStats.running-singleGroupStats.evicted > shouldBeAlive { - return true - } - // If all pods are running and eviction tolerance is small evict 1 pod. - if singleGroupStats.running == singleGroupStats.configured && - singleGroupStats.evictionTolerance == 0 && - singleGroupStats.evicted == 0 { - return true - } - } - } - return false +// PodsRestrictionFactory is a factory for creating PodsEvictionRestriction and PodsInPlaceRestriction. +type PodsRestrictionFactory interface { + GetCreatorMaps(pods []*apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler) (map[podReplicaCreator]singleGroupStats, map[string]podReplicaCreator, error) + NewPodsEvictionRestriction(creatorToSingleGroupStatsMap map[podReplicaCreator]singleGroupStats, podToReplicaCreatorMap map[string]podReplicaCreator) PodsEvictionRestriction + NewPodsInPlaceRestriction(creatorToSingleGroupStatsMap map[podReplicaCreator]singleGroupStats, podToReplicaCreatorMap map[string]podReplicaCreator) PodsInPlaceRestriction } -// Evict sends eviction instruction to api client. Returns error if pod cannot be evicted or if client returned error -// Does not check if pod was actually evicted after eviction grace period. -func (e *podsEvictionRestrictionImpl) Evict(podToEvict *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error { - cr, present := e.podToReplicaCreatorMap[getPodID(podToEvict)] - if !present { - return fmt.Errorf("pod not suitable for eviction %s/%s: not in replicated pods map", podToEvict.Namespace, podToEvict.Name) - } - - if !e.CanEvict(podToEvict) { - return fmt.Errorf("cannot evict pod %s/%s: eviction budget exceeded", podToEvict.Namespace, podToEvict.Name) - } - - eviction := &policyv1.Eviction{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: podToEvict.Namespace, - Name: podToEvict.Name, - }, - } - err := e.client.CoreV1().Pods(podToEvict.Namespace).EvictV1(context.TODO(), eviction) - if err != nil { - klog.ErrorS(err, "Failed to evict pod", "pod", klog.KObj(podToEvict)) - return err - } - eventRecorder.Event(podToEvict, apiv1.EventTypeNormal, "EvictedByVPA", - "Pod was evicted by VPA Updater to apply resource recommendation.") - - eventRecorder.Event(vpa, apiv1.EventTypeNormal, "EvictedPod", - "VPA Updater evicted Pod "+podToEvict.Name+" to apply resource recommendation.") - - if podToEvict.Status.Phase != apiv1.PodPending { - singleGroupStats, present := e.creatorToSingleGroupStatsMap[cr] - if !present { - return fmt.Errorf("Internal error - cannot find stats for replication group %v", cr) - } - singleGroupStats.evicted = singleGroupStats.evicted + 1 - e.creatorToSingleGroupStatsMap[cr] = singleGroupStats - } - - return nil +// PodsRestrictionFactoryImpl is the implementation of the PodsRestrictionFactory interface. +type PodsRestrictionFactoryImpl struct { + client kube_client.Interface + rcInformer cache.SharedIndexInformer // informer for Replication Controllers + ssInformer cache.SharedIndexInformer // informer for Stateful Sets + rsInformer cache.SharedIndexInformer // informer for Replica Sets + dsInformer cache.SharedIndexInformer // informer for Daemon Sets + minReplicas int + evictionToleranceFraction float64 + clock clock.Clock + lastInPlaceAttemptTimeMap map[string]time.Time + patchCalculators []patch.Calculator } -// NewPodsEvictionRestrictionFactory creates PodsEvictionRestrictionFactory -func NewPodsEvictionRestrictionFactory(client kube_client.Interface, minReplicas int, - evictionToleranceFraction float64) (PodsEvictionRestrictionFactory, error) { - rcInformer, err := setUpInformer(client, replicationController) +// NewPodsRestrictionFactory creates a new PodsRestrictionFactory. +func NewPodsRestrictionFactory(client kube_client.Interface, minReplicas int, evictionToleranceFraction float64, patchCalculators []patch.Calculator) (PodsRestrictionFactory, error) { + rcInformer, err := setupInformer(client, replicationController) if err != nil { return nil, fmt.Errorf("Failed to create rcInformer: %v", err) } - ssInformer, err := setUpInformer(client, statefulSet) + ssInformer, err := setupInformer(client, statefulSet) if err != nil { return nil, fmt.Errorf("Failed to create ssInformer: %v", err) } - rsInformer, err := setUpInformer(client, replicaSet) + rsInformer, err := setupInformer(client, replicaSet) if err != nil { return nil, fmt.Errorf("Failed to create rsInformer: %v", err) } - dsInformer, err := setUpInformer(client, daemonSet) + dsInformer, err := setupInformer(client, daemonSet) if err != nil { return nil, fmt.Errorf("Failed to create dsInformer: %v", err) } - return &podsEvictionRestrictionFactoryImpl{ + return &PodsRestrictionFactoryImpl{ client: client, rcInformer: rcInformer, // informer for Replication Controllers - ssInformer: ssInformer, // informer for Replica Sets - rsInformer: rsInformer, // informer for Stateful Sets + ssInformer: ssInformer, // informer for Stateful Sets + rsInformer: rsInformer, // informer for Replica Sets dsInformer: dsInformer, // informer for Daemon Sets minReplicas: minReplicas, - evictionToleranceFraction: evictionToleranceFraction}, nil + evictionToleranceFraction: evictionToleranceFraction, + clock: &clock.RealClock{}, + lastInPlaceAttemptTimeMap: make(map[string]time.Time), + patchCalculators: patchCalculators, + }, nil } -// NewPodsEvictionRestriction creates PodsEvictionRestriction for a given set of pods, -// controlled by a single VPA object. -func (f *podsEvictionRestrictionFactoryImpl) NewPodsEvictionRestriction(pods []*apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler) PodsEvictionRestriction { - // We can evict pod only if it is a part of replica set - // For each replica set we can evict only a fraction of pods. - // Evictions may be later limited by pod disruption budget if configured. +func (f *PodsRestrictionFactoryImpl) getReplicaCount(creator podReplicaCreator) (int, error) { + switch creator.Kind { + case replicationController: + rcObj, exists, err := f.rcInformer.GetStore().GetByKey(creator.Namespace + "/" + creator.Name) + if err != nil { + return 0, fmt.Errorf("replication controller %s/%s is not available, err: %v", creator.Namespace, creator.Name, err) + } + if !exists { + return 0, fmt.Errorf("replication controller %s/%s does not exist", creator.Namespace, creator.Name) + } + rc, ok := rcObj.(*apiv1.ReplicationController) + if !ok { + return 0, fmt.Errorf("Failed to parse Replication Controller") + } + if rc.Spec.Replicas == nil || *rc.Spec.Replicas == 0 { + return 0, fmt.Errorf("replication controller %s/%s has no replicas config", creator.Namespace, creator.Name) + } + return int(*rc.Spec.Replicas), nil + case replicaSet: + rsObj, exists, err := f.rsInformer.GetStore().GetByKey(creator.Namespace + "/" + creator.Name) + if err != nil { + return 0, fmt.Errorf("replica set %s/%s is not available, err: %v", creator.Namespace, creator.Name, err) + } + if !exists { + return 0, fmt.Errorf("replica set %s/%s does not exist", creator.Namespace, creator.Name) + } + rs, ok := rsObj.(*appsv1.ReplicaSet) + if !ok { + return 0, fmt.Errorf("Failed to parse Replicaset") + } + if rs.Spec.Replicas == nil || *rs.Spec.Replicas == 0 { + return 0, fmt.Errorf("replica set %s/%s has no replicas config", creator.Namespace, creator.Name) + } + return int(*rs.Spec.Replicas), nil + case statefulSet: + ssObj, exists, err := f.ssInformer.GetStore().GetByKey(creator.Namespace + "/" + creator.Name) + if err != nil { + return 0, fmt.Errorf("stateful set %s/%s is not available, err: %v", creator.Namespace, creator.Name, err) + } + if !exists { + return 0, fmt.Errorf("stateful set %s/%s does not exist", creator.Namespace, creator.Name) + } + ss, ok := ssObj.(*appsv1.StatefulSet) + if !ok { + return 0, fmt.Errorf("Failed to parse StatefulSet") + } + if ss.Spec.Replicas == nil || *ss.Spec.Replicas == 0 { + return 0, fmt.Errorf("stateful set %s/%s has no replicas config", creator.Namespace, creator.Name) + } + return int(*ss.Spec.Replicas), nil + case daemonSet: + dsObj, exists, err := f.dsInformer.GetStore().GetByKey(creator.Namespace + "/" + creator.Name) + if err != nil { + return 0, fmt.Errorf("daemon set %s/%s is not available, err: %v", creator.Namespace, creator.Name, err) + } + if !exists { + return 0, fmt.Errorf("daemon set %s/%s does not exist", creator.Namespace, creator.Name) + } + ds, ok := dsObj.(*appsv1.DaemonSet) + if !ok { + return 0, fmt.Errorf("Failed to parse DaemonSet") + } + if ds.Status.NumberReady == 0 { + return 0, fmt.Errorf("daemon set %s/%s has no number ready pods", creator.Namespace, creator.Name) + } + return int(ds.Status.NumberReady), nil + } + return 0, nil +} +// GetCreatorMaps is a helper function that returns a map of pod replica creators to their single group stats +// and a map of pod ids to pod replica creator from a list of pods and it's corresponding VPA. +func (f *PodsRestrictionFactoryImpl) GetCreatorMaps(pods []*apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler) (map[podReplicaCreator]singleGroupStats, map[string]podReplicaCreator, error) { livePods := make(map[podReplicaCreator][]*apiv1.Pod) for _, pod := range pods { @@ -245,33 +228,44 @@ func (f *podsEvictionRestrictionFactoryImpl) NewPodsEvictionRestriction(pods []* singleGroup := singleGroupStats{} singleGroup.configured = configured - singleGroup.evictionTolerance = int(float64(configured) * f.evictionToleranceFraction) + singleGroup.evictionTolerance = int(float64(configured) * f.evictionToleranceFraction) // truncated for _, pod := range replicas { podToReplicaCreatorMap[getPodID(pod)] = creator if pod.Status.Phase == apiv1.PodPending { singleGroup.pending = singleGroup.pending + 1 } + if isInPlaceUpdating(pod) { + singleGroup.inPlaceUpdateOngoing = singleGroup.inPlaceUpdateOngoing + 1 + } } singleGroup.running = len(replicas) - singleGroup.pending creatorToSingleGroupStatsMap[creator] = singleGroup + } - return &podsEvictionRestrictionImpl{ + return creatorToSingleGroupStatsMap, podToReplicaCreatorMap, nil +} + +// NewPodsEvictionRestriction creates a new PodsEvictionRestriction. +func (f *PodsRestrictionFactoryImpl) NewPodsEvictionRestriction(creatorToSingleGroupStatsMap map[podReplicaCreator]singleGroupStats, podToReplicaCreatorMap map[string]podReplicaCreator) PodsEvictionRestriction { + return &PodsEvictionRestrictionImpl{ client: f.client, podToReplicaCreatorMap: podToReplicaCreatorMap, - creatorToSingleGroupStatsMap: creatorToSingleGroupStatsMap} + creatorToSingleGroupStatsMap: creatorToSingleGroupStatsMap, + clock: f.clock, + lastInPlaceAttemptTimeMap: f.lastInPlaceAttemptTimeMap, + } } -func getPodReplicaCreator(pod *apiv1.Pod) (*podReplicaCreator, error) { - creator := managingControllerRef(pod) - if creator == nil { - return nil, nil - } - podReplicaCreator := &podReplicaCreator{ - Namespace: pod.Namespace, - Name: creator.Name, - Kind: controllerKind(creator.Kind), +// NewPodsInPlaceRestriction creates a new PodsInPlaceRestriction. +func (f *PodsRestrictionFactoryImpl) NewPodsInPlaceRestriction(creatorToSingleGroupStatsMap map[podReplicaCreator]singleGroupStats, podToReplicaCreatorMap map[string]podReplicaCreator) PodsInPlaceRestriction { + return &PodsInPlaceRestrictionImpl{ + client: f.client, + podToReplicaCreatorMap: podToReplicaCreatorMap, + creatorToSingleGroupStatsMap: creatorToSingleGroupStatsMap, + clock: f.clock, + lastInPlaceAttemptTimeMap: f.lastInPlaceAttemptTimeMap, + patchCalculators: f.patchCalculators, } - return podReplicaCreator, nil } func getPodID(pod *apiv1.Pod) string { @@ -281,78 +275,17 @@ func getPodID(pod *apiv1.Pod) string { return pod.Namespace + "/" + pod.Name } -func (f *podsEvictionRestrictionFactoryImpl) getReplicaCount(creator podReplicaCreator) (int, error) { - switch creator.Kind { - case replicationController: - rcObj, exists, err := f.rcInformer.GetStore().GetByKey(creator.Namespace + "/" + creator.Name) - if err != nil { - return 0, fmt.Errorf("replication controller %s/%s is not available, err: %v", creator.Namespace, creator.Name, err) - } - if !exists { - return 0, fmt.Errorf("replication controller %s/%s does not exist", creator.Namespace, creator.Name) - } - rc, ok := rcObj.(*apiv1.ReplicationController) - if !ok { - return 0, fmt.Errorf("Failed to parse Replication Controller") - } - if rc.Spec.Replicas == nil || *rc.Spec.Replicas == 0 { - return 0, fmt.Errorf("replication controller %s/%s has no replicas config", creator.Namespace, creator.Name) - } - return int(*rc.Spec.Replicas), nil - - case replicaSet: - rsObj, exists, err := f.rsInformer.GetStore().GetByKey(creator.Namespace + "/" + creator.Name) - if err != nil { - return 0, fmt.Errorf("replica set %s/%s is not available, err: %v", creator.Namespace, creator.Name, err) - } - if !exists { - return 0, fmt.Errorf("replica set %s/%s does not exist", creator.Namespace, creator.Name) - } - rs, ok := rsObj.(*appsv1.ReplicaSet) - if !ok { - return 0, fmt.Errorf("Failed to parse Replicaset") - } - if rs.Spec.Replicas == nil || *rs.Spec.Replicas == 0 { - return 0, fmt.Errorf("replica set %s/%s has no replicas config", creator.Namespace, creator.Name) - } - return int(*rs.Spec.Replicas), nil - - case statefulSet: - ssObj, exists, err := f.ssInformer.GetStore().GetByKey(creator.Namespace + "/" + creator.Name) - if err != nil { - return 0, fmt.Errorf("stateful set %s/%s is not available, err: %v", creator.Namespace, creator.Name, err) - } - if !exists { - return 0, fmt.Errorf("stateful set %s/%s does not exist", creator.Namespace, creator.Name) - } - ss, ok := ssObj.(*appsv1.StatefulSet) - if !ok { - return 0, fmt.Errorf("Failed to parse StatefulSet") - } - if ss.Spec.Replicas == nil || *ss.Spec.Replicas == 0 { - return 0, fmt.Errorf("stateful set %s/%s has no replicas config", creator.Namespace, creator.Name) - } - return int(*ss.Spec.Replicas), nil - - case daemonSet: - dsObj, exists, err := f.dsInformer.GetStore().GetByKey(creator.Namespace + "/" + creator.Name) - if err != nil { - return 0, fmt.Errorf("daemon set %s/%s is not available, err: %v", creator.Namespace, creator.Name, err) - } - if !exists { - return 0, fmt.Errorf("daemon set %s/%s does not exist", creator.Namespace, creator.Name) - } - ds, ok := dsObj.(*appsv1.DaemonSet) - if !ok { - return 0, fmt.Errorf("Failed to parse DaemonSet") - } - if ds.Status.NumberReady == 0 { - return 0, fmt.Errorf("daemon set %s/%s has no number ready pods", creator.Namespace, creator.Name) - } - return int(ds.Status.NumberReady), nil +func getPodReplicaCreator(pod *apiv1.Pod) (*podReplicaCreator, error) { + creator := managingControllerRef(pod) + if creator == nil { + return nil, nil } - - return 0, nil + podReplicaCreator := &podReplicaCreator{ + Namespace: pod.Namespace, + Name: creator.Name, + Kind: controllerKind(creator.Kind), + } + return podReplicaCreator, nil } func managingControllerRef(pod *apiv1.Pod) *metav1.OwnerReference { @@ -366,7 +299,7 @@ func managingControllerRef(pod *apiv1.Pod) *metav1.OwnerReference { return &managingController } -func setUpInformer(kubeClient kube_client.Interface, kind controllerKind) (cache.SharedIndexInformer, error) { +func setupInformer(kubeClient kube_client.Interface, kind controllerKind) (cache.SharedIndexInformer, error) { var informer cache.SharedIndexInformer switch kind { case replicationController: @@ -392,3 +325,28 @@ func setUpInformer(kubeClient kube_client.Interface, kind controllerKind) (cache } return informer, nil } + +type singleGroupStats struct { + configured int + pending int + running int + evictionTolerance int + evicted int + inPlaceUpdateOngoing int // number of pods from last loop that are still in-place updating + inPlaceUpdateInitiated int // number of pods from the current loop that have newly requested in-place resize +} + +// isPodDisruptable checks if all pods are running and eviction tolerance is small, we can +// disrupt the current pod. +func (s *singleGroupStats) isPodDisruptable() bool { + shouldBeAlive := s.configured - s.evictionTolerance + actuallyAlive := s.running - (s.evicted + s.inPlaceUpdateInitiated) + return actuallyAlive > shouldBeAlive || + (s.configured == s.running && s.evictionTolerance == 0 && s.evicted == 0 && s.inPlaceUpdateInitiated == 0) + // we don't want to block pods from being considered for eviction if tolerance is small and some pods are potentially stuck resizing +} + +// isInPlaceUpdating checks whether or not the given pod is currently in the middle of an in-place update +func isInPlaceUpdating(podToCheck *apiv1.Pod) bool { + return podToCheck.Status.Resize != "" +} diff --git a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go b/vertical-pod-autoscaler/pkg/updater/restriction/pods_restriction_factory_test.go similarity index 55% rename from vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go rename to vertical-pod-autoscaler/pkg/updater/restriction/pods_restriction_factory_test.go index 855ced5f2ca..a2e557e2099 100644 --- a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go +++ b/vertical-pod-autoscaler/pkg/updater/restriction/pods_restriction_factory_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package eviction +package restriction import ( "fmt" @@ -22,7 +22,8 @@ import ( "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" + + resource_admission "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" @@ -32,22 +33,42 @@ import ( coreinformer "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/cache" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/utils/clock" + baseclocktest "k8s.io/utils/clock/testing" + "k8s.io/utils/ptr" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/utils" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/test" + vpa_api_util "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa" ) type podWithExpectations struct { - pod *apiv1.Pod - canEvict bool - evictionSuccess bool + pod *apiv1.Pod + canEvict bool + evictionSuccess bool + canInPlaceUpdate utils.InPlaceDecision + inPlaceUpdateSuccess bool } func getBasicVpa() *vpa_types.VerticalPodAutoscaler { return test.VerticalPodAutoscaler().WithContainer("any").Get() } -func TestEvictReplicatedByController(t *testing.T) { +func getIPORVpa() *vpa_types.VerticalPodAutoscaler { + vpa := getBasicVpa() + vpa.Spec.UpdatePolicy = &vpa_types.PodUpdatePolicy{ + UpdateMode: ptr.To(vpa_types.UpdateModeInPlaceOrRecreate), + } + return vpa +} + +func TestDisruptReplicatedByController(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, features.MutableFeatureGate, features.InPlaceOrRecreate, true) + rc := apiv1.ReplicationController{ ObjectMeta: metav1.ObjectMeta{ Name: "rc", @@ -268,29 +289,217 @@ func TestEvictReplicatedByController(t *testing.T) { }, }, }, + { + name: "In-place update only first pod (half of 3).", + replicas: 3, + evictionTolerance: 0.5, + vpa: getIPORVpa(), + pods: []podWithExpectations{ + { + pod: generatePod().Get(), + canInPlaceUpdate: utils.InPlaceApproved, + inPlaceUpdateSuccess: true, + }, + { + pod: generatePod().Get(), + canInPlaceUpdate: utils.InPlaceApproved, + inPlaceUpdateSuccess: false, + }, + { + pod: generatePod().Get(), + canInPlaceUpdate: utils.InPlaceApproved, + inPlaceUpdateSuccess: false, + }, + }, + }, + { + name: "For small eviction tolerance at least one pod is in-place resized.", + replicas: 3, + evictionTolerance: 0.1, + vpa: getIPORVpa(), + pods: []podWithExpectations{ + { + pod: generatePod().Get(), + canInPlaceUpdate: utils.InPlaceApproved, + inPlaceUpdateSuccess: true, + }, + { + pod: generatePod().Get(), + canInPlaceUpdate: utils.InPlaceApproved, + inPlaceUpdateSuccess: false, + }, + { + pod: generatePod().Get(), + canInPlaceUpdate: utils.InPlaceApproved, + inPlaceUpdateSuccess: false, + }, + }, + }, + { + name: "Ongoing in-placing pods will not get resized again, but may be considered for eviction or deferred.", + replicas: 3, + evictionTolerance: 0.1, + vpa: getIPORVpa(), + pods: []podWithExpectations{ + { + pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(), + canInPlaceUpdate: utils.InPlaceEvict, + inPlaceUpdateSuccess: false, + }, + { + pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInProgress).Get(), + canInPlaceUpdate: utils.InPlaceDeferred, + inPlaceUpdateSuccess: false, + }, + { + pod: generatePod().Get(), + canInPlaceUpdate: utils.InPlaceApproved, + inPlaceUpdateSuccess: true, + }, + }, + }, + { + name: "Cannot in-place a single Pod under default settings.", + replicas: 1, + evictionTolerance: 0.5, + vpa: getIPORVpa(), + pods: []podWithExpectations{ + { + pod: generatePod().Get(), + canInPlaceUpdate: utils.InPlaceDeferred, + inPlaceUpdateSuccess: false, + }, + }, + }, + { + name: "Can in-place even a single Pod using PodUpdatePolicy.MinReplicas.", + replicas: 1, + evictionTolerance: 0.5, + vpa: func() *vpa_types.VerticalPodAutoscaler { + vpa := getIPORVpa() + vpa.Spec.UpdatePolicy.MinReplicas = ptr.To(int32(1)) + return vpa + }(), + pods: []podWithExpectations{ + { + pod: generatePod().Get(), + canInPlaceUpdate: utils.InPlaceApproved, + inPlaceUpdateSuccess: true, + }, + }, + }, + { + name: "First pod can be evicted without violation of tolerance, even if other evictable pods have ongoing resizes.", + replicas: 3, + evictionTolerance: 0.5, + vpa: getBasicVpa(), + pods: []podWithExpectations{ + { + pod: generatePod().Get(), + canEvict: true, + evictionSuccess: true, + }, + { + pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(), + canEvict: true, + evictionSuccess: false, + }, + { + pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(), + canEvict: true, + evictionSuccess: false, + }, + }, + }, + { + name: "No pods are evictable even if some pods are stuck resizing, but some are missing and eviction tolerance is small.", + replicas: 4, + evictionTolerance: 0.1, + vpa: getBasicVpa(), + pods: []podWithExpectations{ + { + pod: generatePod().Get(), + canEvict: false, + evictionSuccess: false, + }, + { + pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(), + canEvict: false, + evictionSuccess: false, + }, + { + pod: generatePod().Get(), + canEvict: false, + evictionSuccess: false, + }, + }, + }, + { + name: "All pods, including resizing pods, are evictable due to large tolerance.", + replicas: 3, + evictionTolerance: 1, + vpa: getBasicVpa(), + pods: []podWithExpectations{ + { + pod: generatePod().Get(), + canEvict: true, + evictionSuccess: true, + }, + { + pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(), + canEvict: true, + evictionSuccess: true, + }, + { + pod: generatePod().Get(), + canEvict: true, + evictionSuccess: true, + }, + }, + }, } for _, testCase := range testCases { - rc.Spec = apiv1.ReplicationControllerSpec{ - Replicas: &testCase.replicas, - } - pods := make([]*apiv1.Pod, 0, len(testCase.pods)) - for _, p := range testCase.pods { - pods = append(pods, p.pod) - } - factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance) - eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa) - for i, p := range testCase.pods { - assert.Equalf(t, p.canEvict, eviction.CanEvict(p.pod), "TC %v - unexpected CanEvict result for pod-%v %#v", testCase.name, i, p.pod) - } - for i, p := range testCase.pods { - err := eviction.Evict(p.pod, testCase.vpa, test.FakeEventRecorder()) - if p.evictionSuccess { - assert.NoErrorf(t, err, "TC %v - unexpected Evict result for pod-%v %#v", testCase.name, i, p.pod) - } else { - assert.Errorf(t, err, "TC %v - unexpected Evict result for pod-%v %#v", testCase.name, i, p.pod) + t.Run(testCase.name, func(t *testing.T) { + rc.Spec = apiv1.ReplicationControllerSpec{ + Replicas: &testCase.replicas, } - } + pods := make([]*apiv1.Pod, 0, len(testCase.pods)) + for _, p := range testCase.pods { + pods = append(pods, p.pod) + } + factory, err := getRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance, baseclocktest.NewFakeClock(time.Time{}), make(map[string]time.Time), GetFakeCalculatorsWithFakeResourceCalc()) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, testCase.vpa) + assert.NoError(t, err) + eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + inplace := factory.NewPodsInPlaceRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) + updateMode := vpa_api_util.GetUpdateMode(testCase.vpa) + for i, p := range testCase.pods { + if updateMode == vpa_types.UpdateModeInPlaceOrRecreate { + assert.Equalf(t, p.canInPlaceUpdate, inplace.CanInPlaceUpdate(p.pod), "TC %v - unexpected CanInPlaceUpdate result for pod-%v %#v", testCase.name, i, p.pod) + } else { + assert.Equalf(t, p.canEvict, eviction.CanEvict(p.pod), "TC %v - unexpected CanEvict result for pod-%v %#v", testCase.name, i, p.pod) + } + } + for i, p := range testCase.pods { + if updateMode == vpa_types.UpdateModeInPlaceOrRecreate { + err := inplace.InPlaceUpdate(p.pod, testCase.vpa, test.FakeEventRecorder()) + if p.inPlaceUpdateSuccess { + assert.NoErrorf(t, err, "TC %v - unexpected InPlaceUpdate result for pod-%v %#v", testCase.name, i, p.pod) + } else { + assert.Errorf(t, err, "TC %v - unexpected InPlaceUpdate result for pod-%v %#v", testCase.name, i, p.pod) + } + } else { + err := eviction.Evict(p.pod, testCase.vpa, test.FakeEventRecorder()) + if p.evictionSuccess { + assert.NoErrorf(t, err, "TC %v - unexpected Evict result for pod-%v %#v", testCase.name, i, p.pod) + } else { + assert.Errorf(t, err, "TC %v - unexpected Evict result for pod-%v %#v", testCase.name, i, p.pod) + } + } + } + }) } } @@ -317,8 +526,11 @@ func TestEvictReplicatedByReplicaSet(t *testing.T) { } basicVpa := getBasicVpa() - factory, _ := getEvictionRestrictionFactory(nil, &rs, nil, nil, 2, 0.5) - eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) + factory, err := getRestrictionFactory(nil, &rs, nil, nil, 2, 0.5, nil, nil, nil) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa) + assert.NoError(t, err) + eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) for _, pod := range pods { assert.True(t, eviction.CanEvict(pod)) @@ -357,8 +569,11 @@ func TestEvictReplicatedByStatefulSet(t *testing.T) { } basicVpa := getBasicVpa() - factory, _ := getEvictionRestrictionFactory(nil, nil, &ss, nil, 2, 0.5) - eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) + factory, err := getRestrictionFactory(nil, nil, &ss, nil, 2, 0.5, nil, nil, nil) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa) + assert.NoError(t, err) + eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) for _, pod := range pods { assert.True(t, eviction.CanEvict(pod)) @@ -396,8 +611,11 @@ func TestEvictReplicatedByDaemonSet(t *testing.T) { } basicVpa := getBasicVpa() - factory, _ := getEvictionRestrictionFactory(nil, nil, nil, &ds, 2, 0.5) - eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) + factory, err := getRestrictionFactory(nil, nil, nil, &ds, 2, 0.5, nil, nil, nil) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa) + assert.NoError(t, err) + eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) for _, pod := range pods { assert.True(t, eviction.CanEvict(pod)) @@ -432,8 +650,11 @@ func TestEvictReplicatedByJob(t *testing.T) { } basicVpa := getBasicVpa() - factory, _ := getEvictionRestrictionFactory(nil, nil, nil, nil, 2, 0.5) - eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) + factory, err := getRestrictionFactory(nil, nil, nil, nil, 2, 0.5, nil, nil, nil) + assert.NoError(t, err) + creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa) + assert.NoError(t, err) + eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap) for _, pod := range pods { assert.True(t, eviction.CanEvict(pod)) @@ -449,223 +670,9 @@ func TestEvictReplicatedByJob(t *testing.T) { } } -func TestEvictTooFewReplicas(t *testing.T) { - replicas := int32(5) - livePods := 5 - - rc := apiv1.ReplicationController{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rc", - Namespace: "default", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "ReplicationController", - }, - Spec: apiv1.ReplicationControllerSpec{ - Replicas: &replicas, - }, - } - - pods := make([]*apiv1.Pod, livePods) - for i := range pods { - pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() - } - - basicVpa := getBasicVpa() - factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 10, 0.5) - eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) - - for _, pod := range pods { - assert.False(t, eviction.CanEvict(pod)) - } - - for _, pod := range pods { - err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) - assert.Error(t, err, "Error expected") - } -} - -func TestEvictionTolerance(t *testing.T) { - replicas := int32(5) - livePods := 5 - tolerance := 0.8 - - rc := apiv1.ReplicationController{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rc", - Namespace: "default", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "ReplicationController", - }, - Spec: apiv1.ReplicationControllerSpec{ - Replicas: &replicas, - }, - } - - pods := make([]*apiv1.Pod, livePods) - for i := range pods { - pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() - } - - basicVpa := getBasicVpa() - factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2 /*minReplicas*/, tolerance) - eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) - - for _, pod := range pods { - assert.True(t, eviction.CanEvict(pod)) - } - - for _, pod := range pods[:4] { - err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) - assert.Nil(t, err, "Should evict with no error") - } - for _, pod := range pods[4:] { - err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) - assert.Error(t, err, "Error expected") - } -} - -func TestEvictAtLeastOne(t *testing.T) { - replicas := int32(5) - livePods := 5 - tolerance := 0.1 - - rc := apiv1.ReplicationController{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rc", - Namespace: "default", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "ReplicationController", - }, - Spec: apiv1.ReplicationControllerSpec{ - Replicas: &replicas, - }, - } - - pods := make([]*apiv1.Pod, livePods) - for i := range pods { - pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get() - } - - basicVpa := getBasicVpa() - factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, tolerance) - eviction := factory.NewPodsEvictionRestriction(pods, basicVpa) - - for _, pod := range pods { - assert.True(t, eviction.CanEvict(pod)) - } - - for _, pod := range pods[:1] { - err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) - assert.Nil(t, err, "Should evict with no error") - } - for _, pod := range pods[1:] { - err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder()) - assert.Error(t, err, "Error expected") - } -} - -func TestEvictEmitEvent(t *testing.T) { - rc := apiv1.ReplicationController{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rc", - Namespace: "default", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "ReplicationController", - }, - } - - index := 0 - generatePod := func() test.PodBuilder { - index++ - return test.Pod().WithName(fmt.Sprintf("test-%v", index)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta) - } - - basicVpa := getBasicVpa() - - testCases := []struct { - name string - replicas int32 - evictionTolerance float64 - vpa *vpa_types.VerticalPodAutoscaler - pods []podWithExpectations - errorExpected bool - }{ - { - name: "Pods that can be evicted", - replicas: 4, - evictionTolerance: 0.5, - vpa: basicVpa, - pods: []podWithExpectations{ - { - pod: generatePod().WithPhase(apiv1.PodPending).Get(), - canEvict: true, - evictionSuccess: true, - }, - { - pod: generatePod().WithPhase(apiv1.PodPending).Get(), - canEvict: true, - evictionSuccess: true, - }, - }, - errorExpected: false, - }, - { - name: "Pod that can not be evicted", - replicas: 4, - evictionTolerance: 0.5, - vpa: basicVpa, - pods: []podWithExpectations{ - - { - pod: generatePod().Get(), - canEvict: false, - evictionSuccess: false, - }, - }, - errorExpected: true, - }, - } - - for _, testCase := range testCases { - rc.Spec = apiv1.ReplicationControllerSpec{ - Replicas: &testCase.replicas, - } - pods := make([]*apiv1.Pod, 0, len(testCase.pods)) - for _, p := range testCase.pods { - pods = append(pods, p.pod) - } - factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance) - eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa) - - for _, p := range testCase.pods { - mockRecorder := test.MockEventRecorder() - mockRecorder.On("Event", mock.Anything, apiv1.EventTypeNormal, "EvictedByVPA", mock.Anything).Return() - mockRecorder.On("Event", mock.Anything, apiv1.EventTypeNormal, "EvictedPod", mock.Anything).Return() - - errGot := eviction.Evict(p.pod, testCase.vpa, mockRecorder) - if testCase.errorExpected { - assert.Error(t, errGot) - } else { - assert.NoError(t, errGot) - } - - if p.canEvict { - mockRecorder.AssertNumberOfCalls(t, "Event", 2) - - } else { - mockRecorder.AssertNumberOfCalls(t, "Event", 0) - } - } - } -} - -func getEvictionRestrictionFactory(rc *apiv1.ReplicationController, rs *appsv1.ReplicaSet, +func getRestrictionFactory(rc *apiv1.ReplicationController, rs *appsv1.ReplicaSet, ss *appsv1.StatefulSet, ds *appsv1.DaemonSet, minReplicas int, - evictionToleranceFraction float64) (PodsEvictionRestrictionFactory, error) { + evictionToleranceFraction float64, clock clock.Clock, lipuatm map[string]time.Time, patchCalculators []patch.Calculator) (PodsRestrictionFactory, error) { kubeClient := &fake.Clientset{} rcInformer := coreinformer.NewReplicationControllerInformer(kubeClient, apiv1.NamespaceAll, 0*time.Second, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}) @@ -700,17 +707,52 @@ func getEvictionRestrictionFactory(rc *apiv1.ReplicationController, rs *appsv1.R } } - return &podsEvictionRestrictionFactoryImpl{ + return &PodsRestrictionFactoryImpl{ client: kubeClient, - rsInformer: rsInformer, rcInformer: rcInformer, ssInformer: ssInformer, + rsInformer: rsInformer, dsInformer: dsInformer, minReplicas: minReplicas, evictionToleranceFraction: evictionToleranceFraction, + clock: clock, + lastInPlaceAttemptTimeMap: lipuatm, + patchCalculators: patchCalculators, }, nil } func getTestPodName(index int) string { return fmt.Sprintf("test-%v", index) } + +type fakeResizePatchCalculator struct { + patches []resource_admission.PatchRecord + err error +} + +func (c *fakeResizePatchCalculator) CalculatePatches(_ *apiv1.Pod, _ *vpa_types.VerticalPodAutoscaler) ( + []resource_admission.PatchRecord, error) { + return c.patches, c.err +} + +func (c *fakeResizePatchCalculator) PatchResourceTarget() patch.PatchResourceTarget { + return patch.Resize +} + +func NewFakeCalculatorWithInPlacePatches() patch.Calculator { + return &fakeResizePatchCalculator{ + patches: []resource_admission.PatchRecord{ + { + Op: "fakeop", + Path: "fakepath", + Value: apiv1.ResourceList{}, + }, + }, + } +} + +func GetFakeCalculatorsWithFakeResourceCalc() []patch.Calculator { + return []patch.Calculator{ + NewFakeCalculatorWithInPlacePatches(), + } +} diff --git a/vertical-pod-autoscaler/pkg/updater/utils/types.go b/vertical-pod-autoscaler/pkg/updater/utils/types.go new file mode 100644 index 00000000000..8ddb85a0eb8 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/updater/utils/types.go @@ -0,0 +1,29 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils + +// InPlaceDecision is the type of decision that can be made for a pod. +type InPlaceDecision string + +const ( + // InPlaceApproved means we can in-place update the pod. + InPlaceApproved InPlaceDecision = "InPlaceApproved" + // InPlaceDeferred means we can't in-place update the pod right now, but we will wait for the next loop to check for in-placeability again + InPlaceDeferred InPlaceDecision = "InPlaceDeferred" + // InPlaceEvict means we will attempt to evict the pod. + InPlaceEvict InPlaceDecision = "InPlaceEvict" +) diff --git a/vertical-pod-autoscaler/pkg/utils/annotations/vpa_inplace_update.go b/vertical-pod-autoscaler/pkg/utils/annotations/vpa_inplace_update.go new file mode 100644 index 00000000000..1083b657455 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/utils/annotations/vpa_inplace_update.go @@ -0,0 +1,27 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package annotations + +const ( + // VpaInPlaceUpdatedLabel is a label used by the vpa inplace updated annotation. + VpaInPlaceUpdatedLabel = "vpaInPlaceUpdated" +) + +// GetVpaInPlaceUpdatedValue creates an annotation value for a given pod. +func GetVpaInPlaceUpdatedValue() string { + return "true" +} diff --git a/vertical-pod-autoscaler/pkg/utils/metrics/updater/updater.go b/vertical-pod-autoscaler/pkg/utils/metrics/updater/updater.go index ae3d6f89dde..a0ec4a9ca45 100644 --- a/vertical-pod-autoscaler/pkg/utils/metrics/updater/updater.go +++ b/vertical-pod-autoscaler/pkg/utils/metrics/updater/updater.go @@ -76,13 +76,47 @@ var ( }, []string{"vpa_size_log2"}, ) + inPlaceUpdatableCount = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Namespace: metricsNamespace, + Name: "in_place_Updatable_pods_total", + Help: "Number of Pods matching in place update criteria.", + }, []string{"vpa_size_log2"}, + ) + + inPlaceUpdatedCount = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricsNamespace, + Name: "in_place_updated_pods_total", + Help: "Number of Pods updated in-place by Updater to apply a new recommendation.", + }, []string{"vpa_size_log2"}, + ) + + vpasWithInPlaceUpdatablePodsCount = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Namespace: metricsNamespace, + Name: "vpas_with_in_place_Updatable_pods_total", + Help: "Number of VPA objects with at least one Pod matching in place update criteria.", + }, []string{"vpa_size_log2"}, + ) + + vpasWithInPlaceUpdatedPodsCount = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Namespace: metricsNamespace, + Name: "vpas_with_in_place_updated_pods_total", + Help: "Number of VPA objects with at least one in-place updated Pod.", + }, []string{"vpa_size_log2"}, + ) + + // TODO: Add metrics for failed in-place update attempts + functionLatency = metrics.CreateExecutionTimeMetric(metricsNamespace, "Time spent in various parts of VPA Updater main loop.") ) // Register initializes all metrics for VPA Updater func Register() { - prometheus.MustRegister(controlledCount, evictableCount, evictedCount, vpasWithEvictablePodsCount, vpasWithEvictedPodsCount, functionLatency) + prometheus.MustRegister(controlledCount, evictableCount, evictedCount, vpasWithEvictablePodsCount, vpasWithEvictedPodsCount, inPlaceUpdatableCount, inPlaceUpdatedCount, vpasWithInPlaceUpdatablePodsCount, vpasWithInPlaceUpdatedPodsCount, functionLatency) } // NewExecutionTimer provides a timer for Updater's RunOnce execution @@ -124,6 +158,27 @@ func AddEvictedPod(vpaSize int) { evictedCount.WithLabelValues(strconv.Itoa(log2)).Inc() } +// NewInPlaceUpdtateablePodsCounter returns a wrapper for counting Pods which are matching in-place update criteria +func NewInPlaceUpdtateablePodsCounter() *SizeBasedGauge { + return newSizeBasedGauge(evictableCount) +} + +// NewVpasWithInPlaceUpdtateablePodsCounter returns a wrapper for counting VPA objects with Pods matching in-place update criteria +func NewVpasWithInPlaceUpdtateablePodsCounter() *SizeBasedGauge { + return newSizeBasedGauge(vpasWithEvictablePodsCount) +} + +// NewVpasWithInPlaceUpdtatedPodsCounter returns a wrapper for counting VPA objects with evicted Pods +func NewVpasWithInPlaceUpdtatedPodsCounter() *SizeBasedGauge { + return newSizeBasedGauge(vpasWithEvictedPodsCount) +} + +// AddInPlaceUpdatedPod increases the counter of pods updated in place by Updater, by given VPA size +func AddInPlaceUpdatedPod(vpaSize int) { + log2 := metrics.GetVpaSizeLog2(vpaSize) + inPlaceUpdatedCount.WithLabelValues(strconv.Itoa(log2)).Inc() +} + // Add increases the counter for the given VPA size func (g *SizeBasedGauge) Add(vpaSize int, value int) { log2 := metrics.GetVpaSizeLog2(vpaSize) diff --git a/vertical-pod-autoscaler/pkg/utils/test/test_pod.go b/vertical-pod-autoscaler/pkg/utils/test/test_pod.go index e5f66e81a56..047c9673e34 100644 --- a/vertical-pod-autoscaler/pkg/utils/test/test_pod.go +++ b/vertical-pod-autoscaler/pkg/utils/test/test_pod.go @@ -29,6 +29,8 @@ type PodBuilder interface { WithLabels(labels map[string]string) PodBuilder WithAnnotations(annotations map[string]string) PodBuilder WithPhase(phase apiv1.PodPhase) PodBuilder + WithQOSClass(class apiv1.PodQOSClass) PodBuilder + WithResizeStatus(resizeStatus apiv1.PodResizeStatus) PodBuilder Get() *apiv1.Pod } @@ -47,6 +49,8 @@ type podBuilderImpl struct { labels map[string]string annotations map[string]string phase apiv1.PodPhase + qosClass apiv1.PodQOSClass + resizeStatus apiv1.PodResizeStatus } func (pb *podBuilderImpl) WithLabels(labels map[string]string) PodBuilder { @@ -86,6 +90,18 @@ func (pb *podBuilderImpl) WithPhase(phase apiv1.PodPhase) PodBuilder { return &r } +func (pb *podBuilderImpl) WithQOSClass(class apiv1.PodQOSClass) PodBuilder { + r := *pb + r.qosClass = class + return &r +} + +func (pb *podBuilderImpl) WithResizeStatus(resizeStatus apiv1.PodResizeStatus) PodBuilder { + r := *pb + r.resizeStatus = resizeStatus + return &r +} + func (pb *podBuilderImpl) Get() *apiv1.Pod { startTime := metav1.Time{ Time: testTimestamp, @@ -126,6 +142,12 @@ func (pb *podBuilderImpl) Get() *apiv1.Pod { if pb.phase != "" { pod.Status.Phase = pb.phase } + if pb.qosClass != "" { + pod.Status.QOSClass = pb.qosClass + } + if pb.resizeStatus != "" { + pod.Status.Resize = pb.resizeStatus + } return pod } diff --git a/vertical-pod-autoscaler/pkg/utils/test/test_utils.go b/vertical-pod-autoscaler/pkg/utils/test/test_utils.go index 257f613504f..33beb7dadd4 100644 --- a/vertical-pod-autoscaler/pkg/utils/test/test_utils.go +++ b/vertical-pod-autoscaler/pkg/utils/test/test_utils.go @@ -33,6 +33,7 @@ import ( vpa_types_v1beta1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta1" vpa_lister "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/listers/autoscaling.k8s.io/v1" vpa_lister_v1beta1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/listers/autoscaling.k8s.io/v1beta1" + utils "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/utils" ) var ( @@ -121,6 +122,23 @@ func (m *PodsEvictionRestrictionMock) CanEvict(pod *apiv1.Pod) bool { return args.Bool(0) } +// PodsInPlaceRestrictionMock is a mock of PodsInPlaceRestriction +type PodsInPlaceRestrictionMock struct { + mock.Mock +} + +// InPlaceUpdate is a mock implementation of PodsInPlaceRestriction.InPlaceUpdate +func (m *PodsInPlaceRestrictionMock) InPlaceUpdate(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error { + args := m.Called(pod, eventRecorder) + return args.Error(0) +} + +// CanInPlaceUpdate is a mock implementation of PodsInPlaceRestriction.CanInPlaceUpdate +func (m *PodsInPlaceRestrictionMock) CanInPlaceUpdate(pod *apiv1.Pod) utils.InPlaceDecision { + args := m.Called(pod) + return args.Get(0).(utils.InPlaceDecision) +} + // PodListerMock is a mock of PodLister type PodListerMock struct { mock.Mock