In-place update falled back to eviction when it actually succeeded

[helenhelen237]

Which component are you using?:

/area vertical-pod-autoscaler

What version of the component are you using?:

master

Component version:

What k8s version are you using (kubectl version)?: N/A

What environment is this in?:

N/A

What did you expect to happen?:

In the case where in-place resource patch succeeded and in-place annotation patch failed, the pod should still be updated in-place.

What happened instead?:

The pod was evicted.

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

Force in-place annotation patch to fail.

Anything else we need to know?:

When I did the experiment, there were 2 pods in my deployment. One pod was evicted, the other skipped the eviction due to pod disruption budget, and it never retried again to evict (note that in Recreate mode if a pod skipped eviction, it will try again later). When I describe that pod, it has an in-place update success event.

[iamzili]

could you please elaborate on what "in-place annotation patch failed" means?

Also, the reproduction steps are not clear. Please add more details there as well, such as step-by-step instructions to help the maintainers recreate the issue.

[helenhelen237]

I think this block of code should not return error when it fails to patch the annotation:

autoscaler/vertical-pod-autoscaler/pkg/updater/restriction/pods_inplace_restriction.go

Lines 151 to 161 in 31a62d2

	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))
	}

In-place update has 2 patch requests: resource resize patch request, annotation patch request.

In the case where the resource resize patch succeeded and annotation patch failed, InPlaceUpdate will return an error. In updater.go, if InPlaceUpdate returns an error, it will assume that the in-place update failed and will fall back to eviction:

autoscaler/vertical-pod-autoscaler/pkg/updater/logic/updater.go

Lines 300 to 305 in 31a62d2

	if err != nil {
	klog.V(0).InfoS("In-place resize failed, falling back to eviction", "error", err, "pod", klog.KObj(pod))
	metrics_updater.RecordFailedInPlaceUpdate(vpaSize, vpa.Name, vpa.Namespace, "InPlaceUpdateError")
	podsForEviction = append(podsForEviction, pod)
	continue
	}

But in fact, the in-place update succeeded in this case.

Here's how I validated:

To simulate this case, I modified the annotation patch code block to always return error, without sending the actual patch request. Updated the vpa-recommender image with the change.

I deployed a deployment with 2 pods, a VPA targeting the deployment with InPlaceOrRecreate mode. Modified VPA minAllowed and maxAllowed to force VPA resize the containers.

Then observed 1 pod was evicted, newly created pod was updated with expected resources. The other pod skipped the eviction due to pod disruption budget, and it never retried again to evict (note that in Recreate mode if a pod skipped eviction, it will try again later until the resource matches expectation). When I describe that pod, it has an in-place update success event and the resources is updated correctly.

[omerap12]

Thanks for opening this, @helenhelen237!
In general, errors during patch operations should not occur. With #9115, we’re adding a retry mechanism for patch operations, so if a transient error does occur, a retry should resolve it.

[omerap12]

/triage accepted

[omerap12]

/assign @adrianmoisey