From 8968e75e394464d00982d1b0a4efb21644c006e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Th=C3=B6mmes?= Date: Tue, 4 Jun 2019 21:17:38 +0200 Subject: [PATCH] Make autoscaling configuration available to the HPA reconciler. (#4237) The HPA reconciler used to not need this configuration but future changes will rely on the autoscaler config. In particular, the default target knobs will need to be able to the HPA reconciler to be able to correctly determine the target concurrency for the HPA spec once /metric=concurrency gets plumbed into the HPA as well. --- pkg/reconciler/autoscaling/hpa/controller.go | 27 +++++++++++++- pkg/reconciler/autoscaling/hpa/hpa.go | 8 ++-- pkg/reconciler/autoscaling/hpa/hpa_test.go | 39 ++++++++++++++++++-- 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/pkg/reconciler/autoscaling/hpa/controller.go b/pkg/reconciler/autoscaling/hpa/controller.go index 7d2b6afded1e..3a8ef2b9f19b 100644 --- a/pkg/reconciler/autoscaling/hpa/controller.go +++ b/pkg/reconciler/autoscaling/hpa/controller.go @@ -17,11 +17,16 @@ limitations under the License. package hpa import ( + "context" + + "github.com/knative/pkg/configmap" "github.com/knative/pkg/controller" "github.com/knative/serving/pkg/apis/autoscaling" + "github.com/knative/serving/pkg/autoscaler" informers "github.com/knative/serving/pkg/client/informers/externalversions/autoscaling/v1alpha1" ninformers "github.com/knative/serving/pkg/client/informers/externalversions/networking/v1alpha1" "github.com/knative/serving/pkg/reconciler" + "github.com/knative/serving/pkg/reconciler/autoscaling/config" autoscalingv2beta1informers "k8s.io/client-go/informers/autoscaling/v2beta1" "k8s.io/client-go/tools/cache" ) @@ -30,6 +35,12 @@ const ( controllerAgentName = "hpa-class-podautoscaler-controller" ) +// configStore is a minimized interface of the actual configStore. +type configStore interface { + ToContext(ctx context.Context) context.Context + WatchConfigs(w configmap.Watcher) +} + // NewController returns a new HPA reconcile controller. func NewController( opts *reconciler.Options, @@ -47,10 +58,11 @@ func NewController( c.Logger.Info("Setting up hpa-class event handlers") onlyHpaClass := reconciler.AnnotationFilterFunc(autoscaling.ClassAnnotationKey, autoscaling.HPA, false) - paInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + paHandler := cache.FilteringResourceEventHandler{ FilterFunc: onlyHpaClass, Handler: controller.HandleAll(impl.Enqueue), - }) + } + paInformer.Informer().AddEventHandler(paHandler) hpaInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ FilterFunc: onlyHpaClass, @@ -61,5 +73,16 @@ func NewController( FilterFunc: onlyHpaClass, Handler: controller.HandleAll(impl.EnqueueControllerOf), }) + + c.Logger.Info("Setting up ConfigMap receivers") + configsToResync := []interface{}{ + &autoscaler.Config{}, + } + resync := configmap.TypeFilter(configsToResync...)(func(string, interface{}) { + controller.SendGlobalUpdates(paInformer.Informer(), paHandler) + }) + c.configStore = config.NewStore(c.Logger.Named("config-store"), resync) + c.configStore.WatchConfigs(opts.ConfigMapWatcher) + return impl } diff --git a/pkg/reconciler/autoscaling/hpa/hpa.go b/pkg/reconciler/autoscaling/hpa/hpa.go index 51b80200de64..152b066a5d03 100644 --- a/pkg/reconciler/autoscaling/hpa/hpa.go +++ b/pkg/reconciler/autoscaling/hpa/hpa.go @@ -48,9 +48,10 @@ import ( type Reconciler struct { *reconciler.Base - paLister listers.PodAutoscalerLister - sksLister nlisters.ServerlessServiceLister - hpaLister autoscalingv2beta1listers.HorizontalPodAutoscalerLister + paLister listers.PodAutoscalerLister + sksLister nlisters.ServerlessServiceLister + hpaLister autoscalingv2beta1listers.HorizontalPodAutoscalerLister + configStore configStore } var _ controller.Reconciler = (*Reconciler)(nil) @@ -63,6 +64,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { return nil } logger := logging.FromContext(ctx) + ctx = c.configStore.ToContext(ctx) logger.Debug("Reconcile hpa-class PodAutoscaler") original, err := c.paLister.PodAutoscalers(namespace).Get(name) diff --git a/pkg/reconciler/autoscaling/hpa/hpa_test.go b/pkg/reconciler/autoscaling/hpa/hpa_test.go index 542bc762a2c2..0af6c5246b0a 100644 --- a/pkg/reconciler/autoscaling/hpa/hpa_test.go +++ b/pkg/reconciler/autoscaling/hpa/hpa_test.go @@ -20,14 +20,18 @@ import ( "context" "testing" + "github.com/knative/pkg/configmap" "github.com/knative/pkg/controller" logtesting "github.com/knative/pkg/logging/testing" + "github.com/knative/pkg/system" autoscalingv1alpha1 "github.com/knative/serving/pkg/apis/autoscaling/v1alpha1" "github.com/knative/serving/pkg/apis/networking" nv1a1 "github.com/knative/serving/pkg/apis/networking/v1alpha1" + "github.com/knative/serving/pkg/autoscaler" fakeKna "github.com/knative/serving/pkg/client/clientset/versioned/fake" informers "github.com/knative/serving/pkg/client/informers/externalversions" "github.com/knative/serving/pkg/reconciler" + "github.com/knative/serving/pkg/reconciler/autoscaling/config" "github.com/knative/serving/pkg/reconciler/autoscaling/hpa/resources" aresources "github.com/knative/serving/pkg/reconciler/autoscaling/resources" @@ -58,6 +62,13 @@ func TestControllerCanReconcile(t *testing.T) { KubeClientSet: kubeClient, ServingClientSet: servingClient, Logger: logtesting.TestLogger(t), + ConfigMapWatcher: configmap.NewStaticWatcher(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: autoscaler.ConfigName, + }, + Data: map[string]string{}, + }), } servingInformer := informers.NewSharedInformerFactory(servingClient, 0) @@ -406,10 +417,11 @@ func TestReconcile(t *testing.T) { defer logtesting.ClearAll() table.Test(t, MakeFactory(func(listers *Listers, opt reconciler.Options) controller.Reconciler { return &Reconciler{ - Base: reconciler.NewBase(opt, controllerAgentName), - paLister: listers.GetPodAutoscalerLister(), - sksLister: listers.GetServerlessServiceLister(), - hpaLister: listers.GetHorizontalPodAutoscalerLister(), + Base: reconciler.NewBase(opt, controllerAgentName), + paLister: listers.GetPodAutoscalerLister(), + sksLister: listers.GetServerlessServiceLister(), + hpaLister: listers.GetHorizontalPodAutoscalerLister(), + configStore: &testConfigStore{config: defaultConfig()}, } })) } @@ -486,3 +498,22 @@ func deploy(namespace, name string, opts ...deploymentOption) *appsv1.Deployment } return s } + +func defaultConfig() *config.Config { + autoscalerConfig, _ := autoscaler.NewConfigFromMap(nil) + return &config.Config{ + Autoscaler: autoscalerConfig, + } +} + +type testConfigStore struct { + config *config.Config +} + +func (t *testConfigStore) ToContext(ctx context.Context) context.Context { + return config.ToContext(ctx, t.config) +} + +func (t *testConfigStore) WatchConfigs(w configmap.Watcher) {} + +var _ configStore = (*testConfigStore)(nil)