VPA: Fix invalid VPA startupBoost example and unclear error message

[omerap12]

When checking out the following PR: #8458
and applying this VPA and Deployment manifest:

apiVersion: "autoscaling.k8s.io/v1"
kind: VerticalPodAutoscaler
metadata:
  name: example-vpa
spec:
  targetRef:
    apiVersion: "apps/v1"
    kind: Deployment
    name: nginx-deployment
  updatePolicy:
    updateMode: "Auto"
  startupBoost:
    cpu:
      factor: 3
      duration: 10s
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx-deployment
  labels:
    app: nginx
spec:
  replicas: 3
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - name: nginx
        image: nginx:1.14.2
        ports:
        - containerPort: 80
        resources:
          requests:
            memory: "100Mi"
            cpu: "250m" # 250 millicores, or 0.25 CPU cores
          limits:
            memory: "200Mi"
            cpu: "500m"

The following error occurs:

ubuntu:~/autoscaler(pr-8458)$ k apply -f vpa-startup.yaml 
deployment.apps/nginx-deployment unchanged
Error from server: error when creating "vpa-startup.yaml": admission webhook "vpa.k8s.io" denied the request: invalid startupBoost: unexpected StartupBoost.CPU.Type value 

This example was taken from the official docs:
https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler/enhancements/7862-cpu-startup-boost#startup-cpu-boost-enabled--vpa-enabled

The error originates from this section of code:
https://github.com/kubernetes/autoscaler/blob/experimental-cpu-boost/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go#L218

The type field under startupBoost.cpu is required but missing in the example.

The correct manifest should be:

apiVersion: "autoscaling.k8s.io/v1"
kind: VerticalPodAutoscaler
metadata:
  name: example-vpa
spec:
  targetRef:
    apiVersion: "apps/v1"
    kind: Deployment
    name: nginx-deployment
  updatePolicy:
    updateMode: "Auto"
  startupBoost:
    cpu:
      type: "Factor"    # This line was missing
      factor: 3
      duration: 10s

We need:
1/ Documentation fix - Update the example in the docs to include the type field (PR should be tomaster branch).
2/ Error message improvement - Make the webhook error message more descriptive (PR should be toexperimental-cpu-boost branch).

/good-first-issue
/triage accepted
/cc @kamarabbas99 @adrianmoisey

[k8s-ci-robot]

@omerap12:
This request has been marked as suitable for new contributors.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

When checking out the following PR: #8458
and applying this VPA and Deployment manifest:

apiVersion: "autoscaling.k8s.io/v1"
kind: VerticalPodAutoscaler
metadata:
 name: example-vpa
spec:
 targetRef:
   apiVersion: "apps/v1"
   kind: Deployment
   name: nginx-deployment
 updatePolicy:
   updateMode: "Auto"
 startupBoost:
   cpu:
     factor: 3
     duration: 10s
---
apiVersion: apps/v1
kind: Deployment
metadata:
 name: nginx-deployment
 labels:
   app: nginx
spec:
 replicas: 3
 selector:
   matchLabels:
     app: nginx
 template:
   metadata:
     labels:
       app: nginx
   spec:
     containers:
     - name: nginx
       image: nginx:1.14.2
       ports:
       - containerPort: 80
       resources:
         requests:
           memory: "100Mi"
           cpu: "250m" # 250 millicores, or 0.25 CPU cores
         limits:
           memory: "200Mi"
           cpu: "500m"

The following error occurs:

ubuntu:~/autoscaler(pr-8458)$ k apply -f vpa-startup.yaml 
deployment.apps/nginx-deployment unchanged
Error from server: error when creating "vpa-startup.yaml": admission webhook "vpa.k8s.io" denied the request: invalid startupBoost: unexpected StartupBoost.CPU.Type value 

This example was taken from the official docs:
https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler/enhancements/7862-cpu-startup-boost#startup-cpu-boost-enabled--vpa-enabled

The error originates from this section of code:
https://github.com/kubernetes/autoscaler/blob/experimental-cpu-boost/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go#L218

The type field under startupBoost.cpu is required but missing in the example.

The correct manifest should be:

apiVersion: "autoscaling.k8s.io/v1"
kind: VerticalPodAutoscaler
metadata:
 name: example-vpa
spec:
 targetRef:
   apiVersion: "apps/v1"
   kind: Deployment
   name: nginx-deployment
 updatePolicy:
   updateMode: "Auto"
 startupBoost:
   cpu:
     type: "Factor"    # This line was missing
     factor: 3
     duration: 10s

We need:
1/ Documentation fix - Update the example in the docs to include the type field (PR should be tomaster branch).
2/ Error message improvement - Make the webhook error message more descriptive (PR should be toexperimental-cpu-boost branch).

/good-first-issue
/triage accepted
/cc @kamarabbas99 @adrianmoisey

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[omerap12]

/area vertical-pod-autoscaler

[adrianmoisey]

🤔
Is the fix here to default Type to Factor, or to make it required?
See this section: https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler/enhancements/7862-cpu-startup-boost#api-changes

The API example says required
But the text below says:

If not specified, StartupBoost.CPU.Type defaults to Factor.

[bpalko]

Hey all! @omerap12 @adrianmoisey

I've created a PR related to addressing the docs and examples. That PR can be found here.

As @adrianmoisey pointed out, there's a discrepancy between the defaulting and that fact the API spec notes it as a +required value. I opted to stick with the required perspective, and removed the defaulting content.

[kamarabbas99]

🤔 Is the fix here to default Type to Factor, or to make it required? See this section: https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler/enhancements/7862-cpu-startup-boost#api-changes

we decided to change it to Required during API review. #8417 (comment)

[omerap12]

🤔 Is the fix here to default Type to Factor, or to make it required? See this section: https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler/enhancements/7862-cpu-startup-boost#api-changes

The API example says required But the text below says:

If not specified, StartupBoost.CPU.Type defaults to Factor.

Yeah as @kamarabbas99 said we decided to go with required here

[omerap12]

Number 2 is still open.
/open

[adrianmoisey]

/reopen