diff --git a/CHANGELOG.md b/CHANGELOG.md index 4299ef7b012..82f9abd4b62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ - Apache Kafka Scaler: Add `allowIdleConsumers` to the list of trigger parameters ([#1684](https://github.com/kedacore/keda/pull/1684)) - Fixed goroutine leaks in usage of timers ([#1704](https://github.com/kedacore/keda/pull/1704) | [#1739](https://github.com/kedacore/keda/pull/1739)) - Setting timeouts in the HTTP client used by the IBM MQ scaler ([#1758](https://github.com/kedacore/keda/pull/1758)) +- Fix cleanup of removed triggers ([#1768](https://github.com/kedacore/keda/pull/1768)) ### Breaking Changes diff --git a/controllers/hpa.go b/controllers/hpa.go index 40c9a50741a..13b1d545e6f 100644 --- a/controllers/hpa.go +++ b/controllers/hpa.go @@ -111,7 +111,8 @@ func (r *ScaledObjectReconciler) updateHPAIfNeeded(logger logr.Logger, scaledObj return err } - if !equality.Semantic.DeepDerivative(hpa.Spec, foundHpa.Spec) { + // DeepDerivative ignores extra entries in arrays which makes removing the last trigger not update things, so trigger and update any time the metrics count is different. + if len(hpa.Spec.Metrics) != len(foundHpa.Spec.Metrics) || !equality.Semantic.DeepDerivative(hpa.Spec, foundHpa.Spec) { logger.V(1).Info("Found difference in the HPA spec accordint to ScaledObject", "currentHPA", foundHpa.Spec, "newHPA", hpa.Spec) if r.Client.Update(context.TODO(), hpa) != nil { foundHpa.Spec = hpa.Spec diff --git a/controllers/scaledobject_controller_test.go b/controllers/scaledobject_controller_test.go index e16ef8da493..5a8802cd0d4 100644 --- a/controllers/scaledobject_controller_test.go +++ b/controllers/scaledobject_controller_test.go @@ -1,16 +1,23 @@ package controllers import ( + "context" "fmt" "github.com/golang/mock/gomock" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + kedav1alpha1 "github.com/kedacore/keda/v2/api/v1alpha1" "github.com/kedacore/keda/v2/pkg/mock/mock_client" "github.com/kedacore/keda/v2/pkg/mock/mock_scaling" "github.com/kedacore/keda/v2/pkg/scalers" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/log/zap" ) type GinkgoTestReporter struct{} @@ -176,4 +183,100 @@ var _ = Describe("ScaledObjectController", func() { }) }) }) + + Describe("functional tests", func() { + var deployment *appsv1.Deployment + + BeforeEach(func() { + deployment = &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "myapp", Namespace: "default"}, + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "myapp", + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "myapp", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + Image: "app", + }, + }, + }, + }, + }, + } + }) + + It("cleans up a deleted trigger from the HPA", func() { + // Create the scaling target. + err := k8sClient.Create(context.Background(), deployment) + Expect(err).ToNot(HaveOccurred()) + + // Create the ScaledObject with two triggers. + so := &kedav1alpha1.ScaledObject{ + ObjectMeta: metav1.ObjectMeta{Name: "clean-up-test", Namespace: "default"}, + Spec: kedav1alpha1.ScaledObjectSpec{ + ScaleTargetRef: &kedav1alpha1.ScaleTarget{ + Name: "myapp", + }, + Triggers: []kedav1alpha1.ScaleTriggers{ + { + Type: "cron", + Metadata: map[string]string{ + "timezone": "UTC", + "start": "0 * * * *", + "end": "1 * * * *", + "desiredReplicas": "1", + }, + }, + { + Type: "cron", + Metadata: map[string]string{ + "timezone": "UTC", + "start": "2 * * * *", + "end": "3 * * * *", + "desiredReplicas": "2", + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), so) + Expect(err).ToNot(HaveOccurred()) + + // Get and confirm the HPA. + hpa := &autoscalingv2beta2.HorizontalPodAutoscaler{} + Eventually(func() error { + return k8sClient.Get(context.Background(), types.NamespacedName{Name: "keda-hpa-clean-up-test", Namespace: "default"}, hpa) + }).ShouldNot(HaveOccurred()) + Expect(hpa.Spec.Metrics).To(HaveLen(2)) + Expect(hpa.Spec.Metrics[0].External.Metric.Name).To(Equal("cron-UTC-0xxxx-1xxxx")) + Expect(hpa.Spec.Metrics[1].External.Metric.Name).To(Equal("cron-UTC-2xxxx-3xxxx")) + + // Remove the second trigger. + Eventually(func() error { + err = k8sClient.Get(context.Background(), types.NamespacedName{Name: "clean-up-test", Namespace: "default"}, so) + Expect(err).ToNot(HaveOccurred()) + so.Spec.Triggers = so.Spec.Triggers[:1] + return k8sClient.Update(context.Background(), so) + }).ShouldNot(HaveOccurred()) + + // Wait until the HPA is updated. + Eventually(func() int { + err = k8sClient.Get(context.Background(), types.NamespacedName{Name: "keda-hpa-clean-up-test", Namespace: "default"}, hpa) + Expect(err).ToNot(HaveOccurred()) + return len(hpa.Spec.Metrics) + }).Should(Equal(1)) + // And it should only be the first one left. + Expect(hpa.Spec.Metrics[0].External.Metric.Name).To(Equal("cron-UTC-0xxxx-1xxxx")) + }) + }) }) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 612af92cb04..19e3a1fc305 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -23,13 +23,16 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - // "k8s.io/client-go/kubernetes/scheme" - + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/envtest/printer" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" - // kedav1alpha1 "github.com/kedacore/keda/v2/api/v1alpha1" + + kedav1alpha1 "github.com/kedacore/keda/v2/api/v1alpha1" // +kubebuilder:scaffold:imports ) @@ -38,7 +41,9 @@ import ( // These tests use Ginkgo (BDD-style Go testing framework). Refer to // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. +var cfg *rest.Config var testEnv *envtest.Environment +var k8sClient client.Client func TestAPIs(t *testing.T) { RegisterFailHandler(Fail) @@ -56,31 +61,43 @@ var _ = BeforeSuite(func(done Done) { CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")}, } - // var err error - // cfg, err = testEnv.Start() - // Expect(err).ToNot(HaveOccurred()) - // Expect(cfg).ToNot(BeNil()) + var err error + cfg, err = testEnv.Start() + Expect(err).ToNot(HaveOccurred()) + Expect(cfg).ToNot(BeNil()) - // err = kedav1alpha1.AddToScheme(scheme.Scheme) - // Expect(err).NotTo(HaveOccurred()) + err = kedav1alpha1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) - // err = kedav1alpha1.AddToScheme(scheme.Scheme) - // Expect(err).NotTo(HaveOccurred()) + // +kubebuilder:scaffold:scheme - // err = kedav1alpha1.AddToScheme(scheme.Scheme) - // Expect(err).NotTo(HaveOccurred()) + k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme.Scheme, + }) + Expect(err).ToNot(HaveOccurred()) - // +kubebuilder:scaffold:scheme + err = (&ScaledObjectReconciler{ + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + Recorder: k8sManager.GetEventRecorderFor("keda-operator"), + Log: ctrl.Log.WithName("controllers").WithName("ScaledObject"), + }).SetupWithManager(k8sManager) + Expect(err).ToNot(HaveOccurred()) + + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(err).ToNot(HaveOccurred()) + Expect(k8sClient).ToNot(BeNil()) - // k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) - // Expect(err).ToNot(HaveOccurred()) - // Expect(k8sClient).ToNot(BeNil()) + go func() { + err = k8sManager.Start(ctrl.SetupSignalHandler()) + Expect(err).ToNot(HaveOccurred()) + }() close(done) }, 60) var _ = AfterSuite(func() { - // By("tearing down the test environment") - // err := testEnv.Stop() - // Expect(err).ToNot(HaveOccurred()) + By("tearing down the test environment") + err := testEnv.Stop() + Expect(err).ToNot(HaveOccurred()) })