recommendation post processor for integer CPU

[dbenque]

Which component this PR applies to?

vertical-pod-autoscaler/recommender

What type of PR is this?

/kind feature

What this PR does / why we need it:

It introduces a recommendation post processor that allows users to have a CPU integer as recommendation.
Thanks to that it is now possible to use VPA with applications using CPU management with static policy

Which issue(s) this PR fixes:

Fixes #5236

Special notes for your reviewer:

As discussed during the SIG meeting the user interface is based on an annotations. This could be structured in the spec of the VPA once the feature will have gained maturity.

Does this PR introduce a user-facing change?

The "cpu integer post-processor" can be enabled using flag --cpu-integer-post-processor-enabled=true when starting the VPA recommender.
The post-processor would round-up CPU recommendation for containers referenced by a VPA annotation with this format:
vpa-post-processor.kubernetes.io/{containerName}_integerCPU=true

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

This post processor is optional. To run it in the recommender the command-line flag should be set:
--cpu-integer-post-processor-enabled=true, then only the pods/containers that have the relevant annotation are affected.

[jbartosik]

I took a first pass. I couldn't look at tests yet.

[voelzmo]

What's the contract about UncappedTarget? Probably post-processors should leave it as-is, like limit capping does?

[dbenque]

it make sense that the UncappedTarget is not touched by the capping post-processor, but I would say that the same does not apply to other post-processors than capping.
I don't think we need to keep a UnXYZTarget for each XYZ post-processor

[voelzmo]

The annotation can also be turned off by setting it to false, so can you please also add some tests for the case where
vpaPostProcessorPrefix + "container1" + vpaPostProcessorIntegerCPUSuffix: "false"

While writing this, I'm not sure if we need this additional case? How is this different from the annotation not being present for a container? As an alternative, the annotation could list the containers for which this post-processor is turned on?

[dbenque]

"false" and annotation not being present are equivalent indeed.

We already have the support for multiple container with the current code (list of annotation, one per container), adding another way of expressing the same thing may introduce confusion and complexity.

[jbartosik]

I think we also need some documentation:

• Release note
• Some description of how to use this.

[dbenque]

I think we also need some documentation:

• Release note
• Some description of how to use this.

I have added documentation in the last commit 17ef628

Where should I put information for the release note? I have updated the PR section Does this PR introduce a user-facing change , is that the right place?

[jbartosik]

Mostly looks good, there are some things I want to make sure are consistent (mostly flags, labels, and docs). I'm not sure when I'll be able to finish the review (I might have to take some time off). I want to merge this before I cut release (#5355).

[jbartosik]

/lgtm

[jbartosik]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dbenque, jbartosik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• vertical-pod-autoscaler/OWNERS [jbartosik]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment