[VPA] controlledResources field dropped from containerPolicy

[samgiles]

Which component are you using?:

vertical-pod-autoscaler

What version of the component are you using?:

Component version: 0.9.0

What k8s version are you using (kubectl version)?:

kubectl version Output

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.3", GitCommit:"1e11e4a2108024935ecfcb2912226cedeafd99df", GitTreeState:"clean", BuildDate:"2020-10-14T18:49:28Z", GoVersion:"go1.15.2", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.2", GitCommit:"f5743093fd1c663cb0cbc89748f730662345d44d", GitTreeState:"clean", BuildDate:"2020-09-16T13:32:58Z", GoVersion:"go1.15", Compiler:"gc", Platform:"linux/amd64"
# And reproducible on...
$ kubectl version
Client Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.3", GitCommit:"1e11e4a2108024935ecfcb2912226cedeafd99df", GitTreeState:"clean", BuildDate:"2020-10-14T18:49:28Z", GoVersion:"go1.15.2", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"17+", GitVersion:"v1.17.12-eks-7684af", GitCommit:"7684af4ac41370dd109ac13817023cb8063e3d45", GitTreeState:"clean", BuildDate:"2020-10-20T22:57:40Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"linux/amd64"

What environment is this in?:

EKS and Minikube.

What did you expect to happen?:

When using controlledResources as part of a containerPolicy, we expect to see the VPA only consider the resources referred to in the controlledResources property.

What happened instead?:

What we see instead is the controlledResources property disappears from the VPA resource at some point, and so the VPA falls back to considering CPU and memory.

How to reproduce it (as minimally and precisely as possible):

We've managed to reproduce this locally in fresh minikube installation and on one of our EKS clusters running the following:

#!/usr/bin/env bash

./hack/vpa-up.sh

kubectl rollout status deployment/vpa-updater -n kube-system
kubectl rollout status deployment/vpa-admission-controller -n kube-system
kubectl rollout status deployment/vpa-recommender -n kube-system


cat <<EOF | kubectl apply -f -
---
apiVersion: "autoscaling.k8s.io/v1"
kind: VerticalPodAutoscaler
metadata:
  name: hamster-vpa
spec:
  targetRef:
    apiVersion: "apps/v1"
    kind: Deployment
    name: hamster
  resourcePolicy:
    containerPolicies:
      - containerName: 'hamster'
        minAllowed:
          memory: 50Mi
        maxAllowed:
          memory: 500Mi
        controlledResources: ["memory"]
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: hamster
spec:
  selector:
    matchLabels:
      app: hamster
  replicas: 2
  template:
    metadata:
      labels:
        app: hamster
    spec:
      securityContext:
        runAsNonRoot: true
        runAsUser: 65534 # nobody
      containers:
        - name: hamster
          image: k8s.gcr.io/ubuntu-slim:0.1
          resources:
            requests:
              cpu: 50m
              memory: 50Mi
          command: ["/bin/sh"]
          args:
            - "-c"
            - "while true; do timeout 0.5s yes >/dev/null; sleep 0.5s; done"
EOF

Resulting VPA Resource:

kind: VerticalPodAutoscaler
apiVersion: autoscaling.k8s.io/v1
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: >
      {"apiVersion":"autoscaling.k8s.io/v1","kind":"VerticalPodAutoscaler","metadata":{"annotations":{},"name":"hamster-vpa","namespace":"default"},"spec":{"resourcePolicy":{"containerPolicies":[{"containerName":"hamster","controlledResources":["memory"],"maxAllowed":{"memory":"500Mi"},"minAllowed":{"memory":"50Mi"}}]},"targetRef":{"apiVersion":"apps/v1","kind":"Deployment","name":"hamster"}}}
  creationTimestamp: '2021-01-08T14:32:48Z'
  generation: 1
  managedFields:
    - apiVersion: autoscaling.k8s.io/v1
      fieldsType: FieldsV1
      fieldsV1:
        'f:metadata':
          'f:annotations':
            .: {}
            'f:kubectl.kubernetes.io/last-applied-configuration': {}
        'f:spec':
          .: {}
          'f:resourcePolicy':
            .: {}
            'f:containerPolicies': {}
          'f:targetRef':
            .: {}
            'f:apiVersion': {}
            'f:kind': {}
            'f:name': {}
      manager: kubectl-client-side-apply
      operation: Update
      time: '2021-01-08T14:32:48Z'
  name: hamster-vpa
  namespace: default
  resourceVersion: '9208'
  selfLink: >-
    /apis/autoscaling.k8s.io/v1/namespaces/default/verticalpodautoscalers/hamster-vpa
  uid: bdc0cca8-b034-45d7-9f3b-7a84940486c9
spec:
  resourcePolicy:
    containerPolicies:
      - containerName: hamster
        maxAllowed:
          memory: 500Mi
        minAllowed:
          memory: 50Mi
  targetRef:
    apiVersion: apps/v1
    kind: Deployment
    name: hamster

Note the last-applied-update does include the controlledResources, however the containerPolicy no longer lists the controlledResources property.

Anything else we need to know?:

This happens on a completely clean minikube installation:

$ minikube version
minikube version: v1.14.2
commit: 2c82918e2347188e21c4e44c8056fc80408bce1
$ minikube start
$ minikube addons enable metrics-server
$ ./test-case.sh

And on a fairly standard EKS cluster. (versions noted above).

[bskiba]

I'm working on reproducing this on 1.19 with VPA 0.9.0.

Couple additional questions:

1. Do you get recommendation for this VPA object and if yes, can you share it?
2. The minikube version is v1.14.2, does it translate to Kubernetes 1.14?
3. When you say that the controlledResources property disappears from the VPA resource at some point, do you know when that happens? Is it right after creation?

[bskiba]

Actually I reproduced this, and it seems the controlledResources disappear right at the beginning.

However, if you checkout tag vertical-pod-autoscaler-0.9.0 in this repo and run TAG=0.9.0 ./hack/vpa-up.sh, the controlled resources field works correctly. I suspect recent changes to the CRD definition changed something. Investiagting.

[bskiba]

I think the problem is that the current storage version is v1beta2, which did not have the controlledResources field, and we don't preserve unknown fields. So when we save a v1 version object, all fields that were not present in v1beta2 are dropped (unfortunately silently).

[samgiles]

Fantastic! Thanks for looking into that! 🙌🏼