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

Move targetCPUPercentile into a flag #4799

Merged
merged 1 commit into from
Sep 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 29 additions & 13 deletions vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,16 @@ import (
)

const (
evictionWatchRetryWait = 10 * time.Second
evictionWatchJitterFactor = 0.5
scaleCacheLoopPeriod time.Duration = 7 * time.Second
scaleCacheEntryLifetime time.Duration = time.Hour
scaleCacheEntryFreshnessTime time.Duration = 10 * time.Minute
scaleCacheEntryJitterFactor float64 = 1.
defaultResyncPeriod time.Duration = 10 * time.Minute
defaultRecommenderName = "default"
evictionWatchRetryWait = 10 * time.Second
evictionWatchJitterFactor = 0.5
scaleCacheLoopPeriod = 7 * time.Second
scaleCacheEntryLifetime = time.Hour
scaleCacheEntryFreshnessTime = 10 * time.Minute
scaleCacheEntryJitterFactor float64 = 1.
defaultResyncPeriod = 10 * time.Minute
// DefaultRecommenderName designates the recommender that will handle VPA objects which don't specify
// recommender name explicitly (and so implicitly specify that the default recommender should handle them)
DefaultRecommenderName = "default"
)

// ClusterStateFeeder can update state of ClusterState object.
Expand Down Expand Up @@ -94,6 +96,7 @@ type ClusterStateFeederFactory struct {
SelectorFetcher target.VpaTargetSelectorFetcher
MemorySaveMode bool
ControllerFetcher controllerfetcher.ControllerFetcher
RecommenderName string
}

// Make creates new ClusterStateFeeder with internal data providers, based on kube client.
Expand All @@ -109,12 +112,13 @@ func (m ClusterStateFeederFactory) Make() *clusterStateFeeder {
selectorFetcher: m.SelectorFetcher,
memorySaveMode: m.MemorySaveMode,
controllerFetcher: m.ControllerFetcher,
recommenderName: m.RecommenderName,
}
}

// NewClusterStateFeeder creates new ClusterStateFeeder with internal data providers, based on kube client config.
// Deprecated; Use ClusterStateFeederFactory instead.
func NewClusterStateFeeder(config *rest.Config, clusterState *model.ClusterState, memorySave bool, namespace, metricsClientName string) ClusterStateFeeder {
func NewClusterStateFeeder(config *rest.Config, clusterState *model.ClusterState, memorySave bool, namespace, metricsClientName string, recommenderName string) ClusterStateFeeder {
kubeClient := kube_client.NewForConfigOrDie(config)
podLister, oomObserver := NewPodListerAndOOMObserver(kubeClient, namespace)
factory := informers.NewSharedInformerFactoryWithOptions(kubeClient, defaultResyncPeriod, informers.WithNamespace(namespace))
Expand All @@ -131,6 +135,7 @@ func NewClusterStateFeeder(config *rest.Config, clusterState *model.ClusterState
SelectorFetcher: target.NewVpaTargetSelectorFetcher(config, kubeClient, factory),
MemorySaveMode: memorySave,
ControllerFetcher: controllerFetcher,
RecommenderName: recommenderName,
}.Make()
}

Expand Down Expand Up @@ -224,6 +229,7 @@ type clusterStateFeeder struct {
selectorFetcher target.VpaTargetSelectorFetcher
memorySaveMode bool
controllerFetcher controllerfetcher.ControllerFetcher
recommenderName string
}

func (feeder *clusterStateFeeder) InitFromHistoryProvider(historyProvider history.HistoryProvider) {
Expand Down Expand Up @@ -349,10 +355,20 @@ func filterVPAs(feeder *clusterStateFeeder, allVpaCRDs []*vpa_types.VerticalPodA
klog.V(3).Infof("Start selecting the vpaCRDs.")
var vpaCRDs []*vpa_types.VerticalPodAutoscaler
for _, vpaCRD := range allVpaCRDs {
currentRecommenderName := defaultRecommenderName
if !implicitDefaultRecommender(vpaCRD.Spec.Recommenders) && !selectsRecommender(vpaCRD.Spec.Recommenders, &currentRecommenderName) {
klog.V(6).Infof("Ignoring vpaCRD %s in namespace %s as current recommender's name %v doesn't appear among its recommenders", vpaCRD.Name, vpaCRD.Namespace, currentRecommenderName)
continue
if feeder.recommenderName == DefaultRecommenderName {
if !implicitDefaultRecommender(vpaCRD.Spec.Recommenders) && !selectsRecommender(vpaCRD.Spec.Recommenders, &feeder.recommenderName) {
klog.V(6).Infof("Ignoring vpaCRD %s in namespace %s as current recommender's name %v doesn't appear among its recommenders", vpaCRD.Name, vpaCRD.Namespace, feeder.recommenderName)
continue
}
} else {
if implicitDefaultRecommender(vpaCRD.Spec.Recommenders) {
jbartosik marked this conversation as resolved.
Show resolved Hide resolved
klog.V(6).Infof("Ignoring vpaCRD %s in namespace %s as %v recommender doesn't process CRDs implicitly destined to %v recommender", vpaCRD.Name, vpaCRD.Namespace, feeder.recommenderName, DefaultRecommenderName)
continue
}
if !selectsRecommender(vpaCRD.Spec.Recommenders, &feeder.recommenderName) {
klog.V(6).Infof("Ignoring vpaCRD %s in namespace %s as current recommender's name %v doesn't appear among its recommenders", vpaCRD.Name, vpaCRD.Namespace, feeder.recommenderName)
continue
}
}
vpaCRDs = append(vpaCRDs, vpaCRD)
}
Expand Down
105 changes: 102 additions & 3 deletions vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type fakeControllerFetcher struct {
err error
}

func (f *fakeControllerFetcher) FindTopMostWellKnownOrScalable(controller *controllerfetcher.ControllerKeyWithAPIVersion) (*controllerfetcher.ControllerKeyWithAPIVersion, error) {
func (f *fakeControllerFetcher) FindTopMostWellKnownOrScalable(_ *controllerfetcher.ControllerKeyWithAPIVersion) (*controllerfetcher.ControllerKeyWithAPIVersion, error) {
return f.key, f.err
}

Expand All @@ -52,6 +52,8 @@ func parseLabelSelector(selector string) labels.Selector {
}

var (
recommenderName = "name"
empty = ""
unsupportedConditionTextFromFetcher = "Cannot read targetRef. Reason: targetRef not defined"
unsupportedConditionNoExtraText = "Cannot read targetRef"
unsupportedConditionNoTargetRef = "Cannot read targetRef"
Expand Down Expand Up @@ -80,6 +82,9 @@ func TestLoadPods(t *testing.T) {
expectedSelector labels.Selector
expectedConfigUnsupported *string
expectedConfigDeprecated *string
expectedVpaFetch bool
recommenderName *string
recommender string
}

testCases := []testCase{
Expand All @@ -90,6 +95,7 @@ func TestLoadPods(t *testing.T) {
expectedSelector: labels.Nothing(),
expectedConfigUnsupported: &unsupportedConditionTextFromFetcher,
expectedConfigDeprecated: nil,
expectedVpaFetch: true,
},
{
name: "also no selector but no error",
Expand All @@ -98,6 +104,7 @@ func TestLoadPods(t *testing.T) {
expectedSelector: labels.Nothing(),
expectedConfigUnsupported: &unsupportedConditionNoExtraText,
expectedConfigDeprecated: nil,
expectedVpaFetch: true,
},
{
name: "targetRef selector",
Expand All @@ -119,6 +126,7 @@ func TestLoadPods(t *testing.T) {
expectedSelector: parseLabelSelector("app = test"),
expectedConfigUnsupported: nil,
expectedConfigDeprecated: nil,
expectedVpaFetch: true,
},
{
name: "no targetRef",
Expand All @@ -127,6 +135,7 @@ func TestLoadPods(t *testing.T) {
expectedSelector: labels.Nothing(),
expectedConfigUnsupported: nil,
expectedConfigDeprecated: nil,
expectedVpaFetch: true,
},
{
name: "can't decide if top-level-ref",
Expand All @@ -139,6 +148,7 @@ func TestLoadPods(t *testing.T) {
APIVersion: apiVersion,
},
expectedConfigUnsupported: &unsupportedConditionNoTargetRef,
expectedVpaFetch: true,
},
{
name: "non-top-level targetRef",
Expand All @@ -159,6 +169,7 @@ func TestLoadPods(t *testing.T) {
ApiVersion: apiVersion,
},
expectedConfigUnsupported: &unsupportedTargetRefHasParent,
expectedVpaFetch: true,
},
{
name: "error checking if top-level-ref",
Expand All @@ -171,6 +182,7 @@ func TestLoadPods(t *testing.T) {
APIVersion: "taxonomy",
},
expectedConfigUnsupported: &unsupportedConditionMudaMudaMuda,
expectedVpaFetch: true,
findTopMostWellKnownOrScalableError: fmt.Errorf("muda muda muda"),
},
{
Expand All @@ -192,6 +204,78 @@ func TestLoadPods(t *testing.T) {
ApiVersion: apiVersion,
},
expectedConfigUnsupported: nil,
expectedVpaFetch: true,
},
{
name: "no recommenderName",
selector: parseLabelSelector("app = test"),
fetchSelectorError: nil,
targetRef: &autoscalingv1.CrossVersionObjectReference{
Kind: kind,
Name: name1,
APIVersion: apiVersion,
},
topMostWellKnownOrScalableKey: &controllerfetcher.ControllerKeyWithAPIVersion{
ControllerKey: controllerfetcher.ControllerKey{
Kind: kind,
Name: name1,
Namespace: namespace,
},
ApiVersion: apiVersion,
},
expectedSelector: parseLabelSelector("app = test"),
expectedConfigUnsupported: nil,
expectedConfigDeprecated: nil,
expectedVpaFetch: false,
recommenderName: &empty,
},
{
name: "recommenderName doesn't match recommender",
selector: parseLabelSelector("app = test"),
fetchSelectorError: nil,
targetRef: &autoscalingv1.CrossVersionObjectReference{
Kind: kind,
Name: name1,
APIVersion: apiVersion,
},
topMostWellKnownOrScalableKey: &controllerfetcher.ControllerKeyWithAPIVersion{
ControllerKey: controllerfetcher.ControllerKey{
Kind: kind,
Name: name1,
Namespace: namespace,
},
ApiVersion: apiVersion,
},
expectedSelector: parseLabelSelector("app = test"),
expectedConfigUnsupported: nil,
expectedConfigDeprecated: nil,
expectedVpaFetch: false,
recommenderName: &recommenderName,
recommender: "other",
},
{
name: "recommenderName matches recommender",
selector: parseLabelSelector("app = test"),
fetchSelectorError: nil,
targetRef: &autoscalingv1.CrossVersionObjectReference{
Kind: kind,
Name: name1,
APIVersion: apiVersion,
},
topMostWellKnownOrScalableKey: &controllerfetcher.ControllerKeyWithAPIVersion{
ControllerKey: controllerfetcher.ControllerKey{
Kind: kind,
Name: name1,
Namespace: namespace,
},
ApiVersion: apiVersion,
},
expectedSelector: parseLabelSelector("app = test"),
expectedConfigUnsupported: nil,
expectedConfigDeprecated: nil,
expectedVpaFetch: true,
recommenderName: &recommenderName,
recommender: recommenderName,
},
}

Expand All @@ -201,7 +285,11 @@ func TestLoadPods(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

vpa := test.VerticalPodAutoscaler().WithName("testVpa").WithContainer("container").WithNamespace("testNamespace").WithTargetRef(tc.targetRef).Get()
vpaBuilder := test.VerticalPodAutoscaler().WithName("testVpa").WithContainer("container").WithNamespace("testNamespace").WithTargetRef(tc.targetRef)
if tc.recommender != "" {
vpaBuilder = vpaBuilder.WithRecommender(tc.recommender)
}
vpa := vpaBuilder.Get()
vpaLister := &test.VerticalPodAutoscalerListerMock{}
vpaLister.On("List").Return([]*vpa_types.VerticalPodAutoscaler{vpa}, nil)

Expand All @@ -217,15 +305,26 @@ func TestLoadPods(t *testing.T) {
err: tc.findTopMostWellKnownOrScalableError,
},
}
if tc.recommenderName == nil {
clusterStateFeeder.recommenderName = DefaultRecommenderName
} else {
clusterStateFeeder.recommenderName = *tc.recommenderName
}

targetSelectorFetcher.EXPECT().Fetch(vpa).Return(tc.selector, tc.fetchSelectorError)
if tc.expectedVpaFetch {
targetSelectorFetcher.EXPECT().Fetch(vpa).Return(tc.selector, tc.fetchSelectorError)
}
clusterStateFeeder.LoadVPAs()

vpaID := model.VpaID{
Namespace: vpa.Namespace,
VpaName: vpa.Name,
}

if !tc.expectedVpaFetch {
assert.NotContains(t, clusterState.Vpas, vpaID)
return
jbartosik marked this conversation as resolved.
Show resolved Hide resolved
}
assert.Contains(t, clusterState.Vpas, vpaID)
storedVpa := clusterState.Vpas[vpaID]
if tc.expectedSelector != nil {
Expand Down
4 changes: 2 additions & 2 deletions vertical-pod-autoscaler/pkg/recommender/logic/recommender.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ var (
safetyMarginFraction = flag.Float64("recommendation-margin-fraction", 0.15, `Fraction of usage added as the safety margin to the recommended request`)
podMinCPUMillicores = flag.Float64("pod-recommendation-min-cpu-millicores", 25, `Minimum CPU recommendation for a pod`)
podMinMemoryMb = flag.Float64("pod-recommendation-min-memory-mb", 250, `Minimum memory recommendation for a pod`)
targetCPUPercentile = flag.Float64("target-cpu-percentile", 0.9, "CPU usage percentile that will be used as a base for CPU target recommendation. Doesn't affect CPU lower bound, CPU upper bound nor memory recommendations.")
)

// PodResourceRecommender computes resource recommendation for a Vpa object.
Expand Down Expand Up @@ -99,15 +100,14 @@ func FilterControlledResources(estimation model.Resources, controlledResources [

// CreatePodResourceRecommender returns the primary recommender.
func CreatePodResourceRecommender() PodResourceRecommender {
targetCPUPercentile := 0.9
lowerBoundCPUPercentile := 0.5
upperBoundCPUPercentile := 0.95

targetMemoryPeaksPercentile := 0.9
lowerBoundMemoryPeaksPercentile := 0.5
upperBoundMemoryPeaksPercentile := 0.95

targetEstimator := NewPercentileEstimator(targetCPUPercentile, targetMemoryPeaksPercentile)
targetEstimator := NewPercentileEstimator(*targetCPUPercentile, targetMemoryPeaksPercentile)
lowerBoundEstimator := NewPercentileEstimator(lowerBoundCPUPercentile, lowerBoundMemoryPeaksPercentile)
upperBoundEstimator := NewPercentileEstimator(upperBoundCPUPercentile, upperBoundMemoryPeaksPercentile)

Expand Down
9 changes: 4 additions & 5 deletions vertical-pod-autoscaler/pkg/recommender/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package main

import (
"flag"
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/input"
"time"

apiv1 "k8s.io/api/core/v1"
Expand All @@ -32,10 +33,8 @@ import (
klog "k8s.io/klog/v2"
)

// DefaultRecommenderName denotes the current recommender name as the "default" one.
const DefaultRecommenderName = "default"

var (
recommenderName = flag.String("recommender-name", input.DefaultRecommenderName, "Set the recommender name. Recommender will generate recommendations for VPAs that configure the same recommender name. If the recommender name is left as default it will also generate recommendations that don't explicitly specify recommender. You shouldn't run two recommenders with the same name in a cluster.")
metricsFetcherInterval = flag.Duration("recommender-interval", 1*time.Minute, `How often metrics should be fetched`)
checkpointsGCInterval = flag.Duration("checkpoints-gc-interval", 10*time.Minute, `How often orphaned checkpoints should be garbage collected`)
prometheusAddress = flag.String("prometheus-address", "", `Where to reach for Prometheus metrics`)
Expand Down Expand Up @@ -71,7 +70,7 @@ var (
func main() {
klog.InitFlags(nil)
kube_flag.InitFlags()
klog.V(1).Infof("Vertical Pod Autoscaler %s Recommender: %v", common.VerticalPodAutoscalerVersion, DefaultRecommenderName)
klog.V(1).Infof("Vertical Pod Autoscaler %s Recommender: %v", common.VerticalPodAutoscalerVersion, recommenderName)

config := common.CreateKubeConfigOrDie(*kubeconfig, float32(*kubeApiQps), int(*kubeApiBurst))

Expand All @@ -83,7 +82,7 @@ func main() {
metrics_quality.Register()

useCheckpoints := *storage != "prometheus"
recommender := routines.NewRecommender(config, *checkpointsGCInterval, useCheckpoints, *vpaObjectNamespace)
recommender := routines.NewRecommender(config, *checkpointsGCInterval, useCheckpoints, *vpaObjectNamespace, *recommenderName)

promQueryTimeout, err := time.ParseDuration(*queryTimeout)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,14 +254,14 @@ func (c RecommenderFactory) Make() Recommender {
// NewRecommender creates a new recommender instance.
// Dependencies are created automatically.
// Deprecated; use RecommenderFactory instead.
func NewRecommender(config *rest.Config, checkpointsGCInterval time.Duration, useCheckpoints bool, namespace string) Recommender {
func NewRecommender(config *rest.Config, checkpointsGCInterval time.Duration, useCheckpoints bool, namespace string, recommenderName string) Recommender {
clusterState := model.NewClusterState(AggregateContainerStateGCInterval)
kubeClient := kube_client.NewForConfigOrDie(config)
factory := informers.NewSharedInformerFactoryWithOptions(kubeClient, defaultResyncPeriod, informers.WithNamespace(namespace))
controllerFetcher := controllerfetcher.NewControllerFetcher(config, kubeClient, factory, scaleCacheEntryFreshnessTime, scaleCacheEntryLifetime, scaleCacheEntryJitterFactor)
return RecommenderFactory{
ClusterState: clusterState,
ClusterStateFeeder: input.NewClusterStateFeeder(config, clusterState, *memorySaver, namespace, "default-metrics-client"),
ClusterStateFeeder: input.NewClusterStateFeeder(config, clusterState, *memorySaver, namespace, "default-metrics-client", recommenderName),
ControllerFetcher: controllerFetcher,
CheckpointWriter: checkpoint.NewCheckpointWriter(clusterState, vpa_clientset.NewForConfigOrDie(config).AutoscalingV1()),
VpaClient: vpa_clientset.NewForConfigOrDie(config).AutoscalingV1(),
Expand Down
Loading