From 67ac2092f5861a951a43daf30eb64bbcdc41e072 Mon Sep 17 00:00:00 2001 From: Muaaz Saleem Date: Tue, 11 Dec 2018 14:39:34 +0100 Subject: [PATCH] Look at total replicas instead of readyReplicas (#21) * Look at total replicas instead of readyReplicas In cases where replicas take a bit of time to get ready, the Stackset Controller just keeps on scaling. To avoid this issue, using the total replicas instead of replicas that are ready. Signed-off-by: Muhammad Muaaz Saleem * Fix bitbucket clones Signed-off-by: Alexey Ermakov * Updating .gitignore Signed-off-by: Muhammad Muaaz Saleem * Fixing Typo Signed-off-by: Muhammad Muaaz Saleem * Adding tests to avoid regression Testing that collectors depend on Resource.Status.Replicas and not Resource.Status.ReadyReplicas Signed-off-by: Muhammad Muaaz Saleem --- .gitignore | 2 + .travis.yml | 1 + pkg/collector/skipper_collector.go | 4 +- pkg/collector/skipper_collector_test.go | 87 +++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 pkg/collector/skipper_collector_test.go diff --git a/.gitignore b/.gitignore index f4f7988..4f93e5e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ build/ apiserver.local.config/ +.idea/ +vendor/ diff --git a/.travis.yml b/.travis.yml index e2acdf5..f51eb7f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,5 @@ language: go +dist: xenial go: - "1.11.2" diff --git a/pkg/collector/skipper_collector.go b/pkg/collector/skipper_collector.go index ba4863d..f274809 100644 --- a/pkg/collector/skipper_collector.go +++ b/pkg/collector/skipper_collector.go @@ -152,13 +152,13 @@ func targetRefReplicas(client kubernetes.Interface, hpa *autoscalingv2beta1.Hori if err != nil { return 0, err } - replicas = deployment.Status.ReadyReplicas + replicas = deployment.Status.Replicas case "StatefulSet": sts, err := client.AppsV1().StatefulSets(hpa.Namespace).Get(hpa.Spec.ScaleTargetRef.Name, metav1.GetOptions{}) if err != nil { return 0, err } - replicas = sts.Status.ReadyReplicas + replicas = sts.Status.Replicas } return replicas, nil diff --git a/pkg/collector/skipper_collector_test.go b/pkg/collector/skipper_collector_test.go new file mode 100644 index 0000000..8b01aa8 --- /dev/null +++ b/pkg/collector/skipper_collector_test.go @@ -0,0 +1,87 @@ +package collector + +import ( + "testing" + + "github.com/stretchr/testify/require" + "k8s.io/api/apps/v1" + autoscalingv2beta1 "k8s.io/api/autoscaling/v2beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func TestTargetRefReplicasDeployments(t *testing.T) { + client := fake.NewSimpleClientset() + name := "some-app" + defaultNamespace := "default" + deployment, err := newDeployment(client, defaultNamespace, name) + require.NoError(t, err) + + // Create an HPA with the deployment as ref + hpa, err := client.AutoscalingV2beta1().HorizontalPodAutoscalers(deployment.Namespace). + Create(newHPA(defaultNamespace, name, "Deployment")) + require.NoError(t, err) + + replicas, err := targetRefReplicas(client, hpa) + require.NoError(t, err) + require.Equal(t, deployment.Status.Replicas, replicas) +} + +func TestTargetRefReplicasStatefulSets(t *testing.T) { + client := fake.NewSimpleClientset() + name := "some-app" + defaultNamespace := "default" + statefulSet, err := newStatefulSet(client, defaultNamespace, name) + require.NoError(t, err) + + // Create an HPA with the statefulSet as ref + hpa, err := client.AutoscalingV2beta1().HorizontalPodAutoscalers(statefulSet.Namespace). + Create(newHPA(defaultNamespace, name, "StatefulSet")) + require.NoError(t, err) + + replicas, err := targetRefReplicas(client, hpa) + require.NoError(t, err) + require.Equal(t, statefulSet.Status.Replicas, replicas) +} + +func newHPA(namesapce string, refName string, refKind string) *autoscalingv2beta1.HorizontalPodAutoscaler { + return &autoscalingv2beta1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: namesapce, + }, + Spec: autoscalingv2beta1.HorizontalPodAutoscalerSpec{ + ScaleTargetRef: autoscalingv2beta1.CrossVersionObjectReference{ + Name: refName, + Kind: refKind, + }, + }, + Status: autoscalingv2beta1.HorizontalPodAutoscalerStatus{}, + } +} + +func newDeployment(client *fake.Clientset, namespace string, name string) (*v1.Deployment, error) { + return client.AppsV1().Deployments(namespace).Create(&v1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1.DeploymentSpec{}, + Status: v1.DeploymentStatus{ + ReadyReplicas: 1, + Replicas: 2, + }, + }) +} + +func newStatefulSet(client *fake.Clientset, namespace string, name string) (*v1.StatefulSet, error) { + return client.AppsV1().StatefulSets(namespace).Create(&v1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Status: v1.StatefulSetStatus{ + ReadyReplicas: 1, + Replicas: 2, + }, + }) +}