Skip to content

Commit

Permalink
When fetching the pod data for a known core Kind, use a typed client …
Browse files Browse the repository at this point in the history
…request instead of Unstructured (kedacore#1457)

* When fetching the pod data for a known core Kind, use a typed client request instead of Unstructured. This allows those requests to hit the informer cache while Unstructured gets cannot.

If controller-runtime adds support for reading Unstructured gets from the cache, this could be reverted in the future. This also adds the permissions required for setting up watches.

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

* Fix controller-gen syntax.

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

* Update changelog.

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

* Replace reflect magic with a direct implementation for Deployment and StatefulSet.

These are the only two core types which can be scaled anyway and this code is much less likely to break at runtime in unexpected ways.

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

* Only request cache permissions for Deployments and StatefulSets.

These should be the only two types that are needed with it, so minimal permissions are nice.

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>
  • Loading branch information
coderanger authored and ycabrer committed Mar 1, 2021
1 parent 067c78c commit 1965813
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
- Optimize Kafka scaler's `getLagForPartition` function. ([#1464](https://github.com/kedacore/keda/pull/1464))
- Reduce unnecessary /scale requests from ScaledObject controller ([#1453](https://github.com/kedacore/keda/pull/1453))
- Add support for the WATCH_NAMESPACE environment variable to the operator ([#1474](https://github.com/kedacore/keda/pull/1474))
- Improve performance when fetching pod information ([#1457](https://github.com/kedacore/keda/pull/1457))


### Breaking Changes
Expand Down
8 changes: 8 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ rules:
- '*/scale'
verbs:
- '*'
- apiGroups:
- apps
resources:
- deployments
- statefulsets
verbs:
- list
- watch
- apiGroups:
- autoscaling
resources:
Expand Down
1 change: 1 addition & 0 deletions controllers/scaledobject_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
// +kubebuilder:rbac:groups="",resources=pods;services;services;secrets;external,verbs=get;list;watch
// +kubebuilder:rbac:groups="*",resources="*/scale",verbs="*"
// +kubebuilder:rbac:groups="*",resources="*",verbs=get
// +kubebuilder:rbac:groups="apps",resources=deployments;statefulsets,verbs=list;watch

// ScaledObjectReconciler reconciles a ScaledObject object
type ScaledObjectReconciler struct {
Expand Down
58 changes: 41 additions & 17 deletions pkg/scaling/scale_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/go-logr/logr"
appsv1 "k8s.io/api/apps/v1"
"k8s.io/api/autoscaling/v2beta2"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -373,28 +374,51 @@ func (h *scaleHandler) buildScalers(withTriggers *kedav1alpha1.WithTriggers, pod
func (h *scaleHandler) getPods(scalableObject interface{}) (*corev1.PodTemplateSpec, string, error) {
switch obj := scalableObject.(type) {
case *kedav1alpha1.ScaledObject:
unstruct := &unstructured.Unstructured{}
unstruct.SetGroupVersionKind(obj.Status.ScaleTargetGVKR.GroupVersionKind())
if err := h.client.Get(context.TODO(), client.ObjectKey{Namespace: obj.Namespace, Name: obj.Spec.ScaleTargetRef.Name}, unstruct); err != nil {
// resource doesn't exist
h.logger.Error(err, "Target resource doesn't exist", "resource", obj.Status.ScaleTargetGVKR.GVKString(), "name", obj.Spec.ScaleTargetRef.Name)
return nil, "", err
}

withPods := &duckv1.WithPod{}
if err := duck.FromUnstructured(unstruct, withPods); err != nil {
h.logger.Error(err, "Cannot convert unstructured into PodSpecable Duck-type", "object", unstruct)
// Try to get a real object instance for better cache usage, but fall back to an Unstructured if needed.
podTemplateSpec := corev1.PodTemplateSpec{}
gvk := obj.Status.ScaleTargetGVKR.GroupVersionKind()
objKey := client.ObjectKey{Namespace: obj.Namespace, Name: obj.Status.ScaleTargetGVKR.Resource}
switch {
// For core types, use a typed client so we get an informer-cache-backed Get to reduce API load.
case gvk.Group == "apps" && gvk.Kind == "Deployment":
deployment := &appsv1.Deployment{}
if err := h.client.Get(context.TODO(), objKey, deployment); err != nil {
// resource doesn't exist
h.logger.Error(err, "Target deployment doesn't exist", "resource", gvk.String(), "name", objKey.Name)
return nil, "", err
}
podTemplateSpec.ObjectMeta = deployment.ObjectMeta
podTemplateSpec.Spec = deployment.Spec.Template.Spec
case gvk.Group == "apps" && gvk.Kind == "StatefulSet":
statefulSet := &appsv1.StatefulSet{}
if err := h.client.Get(context.TODO(), objKey, statefulSet); err != nil {
// resource doesn't exist
h.logger.Error(err, "Target deployment doesn't exist", "resource", gvk.String(), "name", objKey.Name)
return nil, "", err
}
podTemplateSpec.ObjectMeta = statefulSet.ObjectMeta
podTemplateSpec.Spec = statefulSet.Spec.Template.Spec
default:
unstruct := &unstructured.Unstructured{}
unstruct.SetGroupVersionKind(gvk)
if err := h.client.Get(context.TODO(), objKey, unstruct); err != nil {
// resource doesn't exist
h.logger.Error(err, "Target resource doesn't exist", "resource", gvk.String(), "name", objKey.Name)
return nil, "", err
}
withPods := &duckv1.WithPod{}
if err := duck.FromUnstructured(unstruct, withPods); err != nil {
h.logger.Error(err, "Cannot convert Unstructured into PodSpecable Duck-type", "object", unstruct)
}
podTemplateSpec.ObjectMeta = withPods.ObjectMeta
podTemplateSpec.Spec = withPods.Spec.Template.Spec
}

if withPods.Spec.Template.Spec.Containers == nil {
h.logger.V(1).Info("There aren't any containers found in the ScaleTarget, therefore it is no possible to inject environment properties", "resource", obj.Status.ScaleTargetGVKR.GVKString(), "name", obj.Spec.ScaleTargetRef.Name)
if podTemplateSpec.Spec.Containers == nil || len(podTemplateSpec.Spec.Containers) == 0 {
h.logger.V(1).Info("There aren't any containers found in the ScaleTarget, therefore it is no possible to inject environment properties", "resource", gvk.String(), "name", obj.Spec.ScaleTargetRef.Name)
return nil, "", nil
}

podTemplateSpec := corev1.PodTemplateSpec{
ObjectMeta: withPods.ObjectMeta,
Spec: withPods.Spec.Template.Spec,
}
return &podTemplateSpec, obj.Spec.ScaleTargetRef.EnvSourceContainerName, nil
case *kedav1alpha1.ScaledJob:
return &obj.Spec.JobTargetRef.Template, obj.Spec.EnvSourceContainerName, nil
Expand Down

0 comments on commit 1965813

Please sign in to comment.