Skip to content

Commit

Permalink
Make autoscaling configuration available to the HPA reconciler. (#4237)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
markusthoemmes authored and knative-prow-robot committed Jun 4, 2019
1 parent e94ad95 commit 8968e75
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 9 deletions.
27 changes: 25 additions & 2 deletions pkg/reconciler/autoscaling/hpa/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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
}
8 changes: 5 additions & 3 deletions pkg/reconciler/autoscaling/hpa/hpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
39 changes: 35 additions & 4 deletions pkg/reconciler/autoscaling/hpa/hpa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()},
}
}))
}
Expand Down Expand Up @@ -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)

0 comments on commit 8968e75

Please sign in to comment.