Merge pull request #4799 from matthyx/flags

Move targetCPUPercentile into a flag

vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go

@@ -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.
@@ -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.
@@ -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))
@@ -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()
 }
 
@@ -224,6 +229,7 @@ type clusterStateFeeder struct {
  selectorFetcher target.VpaTargetSelectorFetcher
  memorySaveMode bool
  controllerFetcher controllerfetcher.ControllerFetcher
+ recommenderName string
 }
 
 func (feeder *clusterStateFeeder) InitFromHistoryProvider(historyProvider history.HistoryProvider) {
@@ -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) {
+ 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)
  }

vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go

@@ -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
 }
 
@@ -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"
@@ -80,6 +82,9 @@ func TestLoadPods(t *testing.T) {
  expectedSelector labels.Selector
  expectedConfigUnsupported *string
  expectedConfigDeprecated *string
+ expectedVpaFetch bool
+ recommenderName *string
+ recommender string
  }
 
  testCases := []testCase{
@@ -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",
@@ -98,6 +104,7 @@ func TestLoadPods(t *testing.T) {
  expectedSelector: labels.Nothing(),
  expectedConfigUnsupported: &unsupportedConditionNoExtraText,
  expectedConfigDeprecated: nil,
+ expectedVpaFetch: true,
  },
  {
  name: "targetRef selector",
@@ -119,6 +126,7 @@ func TestLoadPods(t *testing.T) {
  expectedSelector: parseLabelSelector("app = test"),
  expectedConfigUnsupported: nil,
  expectedConfigDeprecated: nil,
+ expectedVpaFetch: true,
  },
  {
  name: "no targetRef",
@@ -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",
@@ -139,6 +148,7 @@ func TestLoadPods(t *testing.T) {
  APIVersion: apiVersion,
  },
  expectedConfigUnsupported: &unsupportedConditionNoTargetRef,
+ expectedVpaFetch: true,
  },
  {
  name: "non-top-level targetRef",
@@ -159,6 +169,7 @@ func TestLoadPods(t *testing.T) {
  ApiVersion: apiVersion,
  },
  expectedConfigUnsupported: &unsupportedTargetRefHasParent,
+ expectedVpaFetch: true,
  },
  {
  name: "error checking if top-level-ref",
@@ -171,6 +182,7 @@ func TestLoadPods(t *testing.T) {
  APIVersion: "taxonomy",
  },
  expectedConfigUnsupported: &unsupportedConditionMudaMudaMuda,
+ expectedVpaFetch: true,
  findTopMostWellKnownOrScalableError: fmt.Errorf("muda muda muda"),
  },
  {
@@ -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,
  },
  }
 
@@ -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)
 
@@ -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
+ }
  assert.Contains(t, clusterState.Vpas, vpaID)
  storedVpa := clusterState.Vpas[vpaID]
  if tc.expectedSelector != nil {

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

@@ -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.
@@ -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)
 

vertical-pod-autoscaler/pkg/recommender/main.go

@@ -18,6 +18,7 @@ package main
 
 import (
  "flag"
+ "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/input"
  "time"
 
  apiv1 "k8s.io/api/core/v1"
@@ -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`)
@@ -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))
 
@@ -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 {

vertical-pod-autoscaler/pkg/recommender/routines/recommender.go

@@ -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(),