Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

example of post-processor to have integer CPU #63

Closed
wants to merge 3 commits into from

Conversation

dbenque
Copy link

@dbenque dbenque commented Oct 7, 2022

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 VPA for application using static CPU allocation

Which issue(s) this PR fixes:

Fixes kubernetes#5236

Special notes for your reviewer:

This PR is an example about how to use kubernetes#5239

Does this PR introduce a user-facing change?

It does not break any existing API.
With this implementation users activate the feature by adding an annotation their VPA objects:
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:
--recommendation-post-processors="integerCPU,capping"

@dbenque dbenque force-pushed the david.benque/poc-cpu-as-integer branch from e3d2622 to 12ee968 Compare October 17, 2022 12:25
@dbenque dbenque marked this pull request as ready for review October 17, 2022 13:11
Base automatically changed from david.benque/reco-post-processing to datadog-master-vpa November 28, 2022 14:23
@@ -57,6 +57,9 @@ var (
ctrPodNameLabel = flag.String("container-pod-name-label", "pod_name", `Label name to look for container pod names`)
ctrNameLabel = flag.String("container-name-label", "name", `Label name to look for container names`)
vpaObjectNamespace = flag.String("vpa-object-namespace", apiv1.NamespaceAll, "Namespace to search for VPA objects and pod stats. Empty means all namespaces will be used.")

// recommendation post processor flags
postProcessorCPUasInteger = flag.Bool("recommendation-post-processor-cpu-as-integer-enabled", false, `Enable recommendation post process to have CPU as integer if requested by user in VPA object`)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be a list of pre processors instead

// vpa-post-processor.kubernetes.io/{containerName}_integerCPU=true

vpaPostProcessorPrefix = "vpa-post-processor.kubernetes.io/"
vpaPostProcessorIntegerCPUSuffix = "_integerCPU"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be -integer-cpu ?

apiv1 "k8s.io/api/core/v1"
vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/model"
"math"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import ordering seems wrong

if resourceName != apiv1.ResourceCPU {
continue
}
v := float64(recommended.MilliValue()) / 1000
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1000. ?


// extractContainerName return the container name for the feature based on annotation key
// if the return value is empty that means that the key does not match
func extractContainerName(key, prefix, suffix string) string {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely to be re-used and should be in a shared module no ?

@dbenque
Copy link
Author

dbenque commented Apr 13, 2023

Merged upstream

@dbenque dbenque closed this Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants