diff --git a/controllers/keda/kedacontroller_controller_test.go b/controllers/keda/kedacontroller_controller_test.go index 6f1b7a5f5..715f30be4 100644 --- a/controllers/keda/kedacontroller_controller_test.go +++ b/controllers/keda/kedacontroller_controller_test.go @@ -69,7 +69,7 @@ var _ = Describe("Deploying KedaController manifest", func() { }) AfterEach(func() { - manifest, err = changeAttribute(manifest, "namespace", namespace, scheme) + manifest, err = changeAttribute(manifest, "namespace", namespace, scheme, "") Expect(err).To(BeNil()) Expect(manifest.Delete()).Should(Succeed()) @@ -97,7 +97,7 @@ var _ = Describe("Deploying KedaController manifest", func() { It("Should not deploy KedaController", func() { - manifest, err = changeAttribute(manifest, "namespace", changedNamespace, scheme) + manifest, err = changeAttribute(manifest, "namespace", changedNamespace, scheme, "") Expect(err).To(BeNil()) Expect(manifest.Apply()).Should(Succeed()) @@ -137,10 +137,28 @@ var _ = Describe("Testing functionality", func() { ) BeforeEach(func() { + By("Applying a manifest that defaults operator logLevel back to info") scheme = k8sManager.GetScheme() manifest, err = createManifest(kedaManifestFilepath, k8sClient) Expect(err).To(BeNil()) + manifest, err = changeAttribute(manifest, "logLevel", "info", scheme, "default") + Expect(err).To(BeNil()) Expect(manifest.Apply()).Should(Succeed()) + + By("Waiting for the operator deployment to reflect the changes") + Eventually(func() error { + return deploymentHasRolledOut(deploymentName, namespace, "default") + }, timeout, interval).Should(Succeed()) + + By("Checking to make sure loglevel is set back to info") + u, err := getObject(ctx, "Deployment", deploymentName, namespace, k8sClient) + Expect(err).To(BeNil()) + err = scheme.Convert(u, dep, nil) + Expect(err).To(BeNil()) + + arg, err = getDepArg(dep, logLevelPrefix, containerName) + Expect(err).To(BeNil()) + Expect(arg).To(Equal("info")) }) Context("When changing \"--zap-log-level\"", func() { @@ -160,6 +178,8 @@ var _ = Describe("Testing functionality", func() { initialLogLevel: "error", actualLogLevel: "error", }, + // the default in the sample kedacontroller manifest is "info", so "info" in this list can also mean "make sure it doesn't change", + // it is supposed to ignore these two cases because the values are invalid. { initialLogLevel: "", actualLogLevel: "info", @@ -171,27 +191,33 @@ var _ = Describe("Testing functionality", func() { } for _, variant := range variants { - It(fmt.Sprintf("Should change it, initialLoglevel='%s', actualLoglevel='%s'", - variant.initialLogLevel, variant.actualLogLevel), func() { - - manifest, err = changeAttribute(manifest, "logLevel", variant.initialLogLevel, scheme) - _ = manifest.Apply() - - Eventually(func() error { - _, err = getObject(ctx, "Deployment", deploymentName, namespace, k8sClient) - return err - }, timeout, interval).Should(Succeed()) - - u, err := getObject(ctx, "Deployment", deploymentName, namespace, k8sClient) - Expect(err).To(BeNil()) - err = scheme.Convert(u, dep, nil) - Expect(err).To(BeNil()) - - arg, err = getDepArg(dep, logLevelPrefix, containerName) - Expect(err).To(BeNil()) - Expect(arg).To(Equal(variant.actualLogLevel)) - }) + caseName := fmt.Sprintf("Should change it, initialLoglevel='%s', actualLoglevel='%s'", variant.initialLogLevel, variant.actualLogLevel) + It(caseName, + func() { + By(fmt.Sprintf("Setting operator loglevel to %s in kedaController manifest", variant.initialLogLevel)) + manifest, err = changeAttribute(manifest, "logLevel", variant.initialLogLevel, scheme, caseName) + Expect(err).To(BeNil()) + err = manifest.Apply() + Expect(err).To(BeNil()) + + By("Waiting for the operator deployment to reflect the changes") + Eventually(func() error { + return deploymentHasRolledOut(deploymentName, namespace, caseName) + }, timeout, interval).Should(Succeed()) + + By("Checking to make sure the log level is " + variant.actualLogLevel) + u, err := getObject(ctx, "Deployment", deploymentName, namespace, k8sClient) + Expect(err).To(BeNil()) + err = scheme.Convert(u, dep, nil) + Expect(err).To(BeNil()) + + arg, err = getDepArg(dep, logLevelPrefix, containerName) + Expect(err).To(BeNil()) + Expect(arg).To(Equal(variant.actualLogLevel)) + + }) } + }) }) @@ -219,10 +245,28 @@ var _ = Describe("Testing functionality", func() { ) BeforeEach(func() { + By("Applying a manifest that defaults operator logLevel back to info") scheme = k8sManager.GetScheme() manifest, err = createManifest(kedaManifestFilepath, k8sClient) Expect(err).To(BeNil()) + manifest, err = changeAttribute(manifest, "logLevel", "info", scheme, "default") + Expect(err).To(BeNil()) Expect(manifest.Apply()).Should(Succeed()) + + By("Waiting for the operator deployment to reflect the changes") + Eventually(func() error { + return deploymentHasRolledOut(deploymentName, namespace, "default") + }, timeout, interval).Should(Succeed()) + + By("Checking to make sure loglevel is set back to info") + u, err := getObject(ctx, "Deployment", deploymentName, namespace, k8sClient) + Expect(err).To(BeNil()) + err = scheme.Convert(u, dep, nil) + Expect(err).To(BeNil()) + + arg, err = getDepArg(dep, logLevelPrefix, containerName) + Expect(err).To(BeNil()) + Expect(arg).To(Equal("info")) }) Context("When changing \"--zap-log-level\"", func() { @@ -242,6 +286,8 @@ var _ = Describe("Testing functionality", func() { initialLogLevel: "error", actualLogLevel: "error", }, + // the default in the sample kedacontroller manifest is "info", so "info" in this list can also mean "make sure it doesn't change", + // it is supposed to ignore these two cases because the values are invalid. { initialLogLevel: "", actualLogLevel: "info", @@ -253,17 +299,21 @@ var _ = Describe("Testing functionality", func() { } for _, variant := range variants { - It(fmt.Sprintf("Should change it, initialLoglevel='%s', actualLoglevel='%s'", - variant.initialLogLevel, variant.actualLogLevel), func() { + caseName := fmt.Sprintf("Should change it, initialLoglevel='%s', actualLoglevel='%s'", variant.initialLogLevel, variant.actualLogLevel) - manifest, err = changeAttribute(manifest, "logLevel", variant.initialLogLevel, scheme) - _ = manifest.Apply() + It(caseName, func() { + By(fmt.Sprintf("Setting admission loglevel to %s in kedaController manifest", variant.initialLogLevel)) + manifest, err = changeAttribute(manifest, "logLevel-admission", variant.initialLogLevel, scheme, caseName) + Expect(err).To(BeNil()) + err = manifest.Apply() + Expect(err).To(BeNil()) + By("Waiting for the admission deployment to reflect the changes") Eventually(func() error { - _, err = getObject(ctx, "Deployment", deploymentName, namespace, k8sClient) - return err + return deploymentHasRolledOut(deploymentName, namespace, caseName) }, timeout, interval).Should(Succeed()) + By("Checking to make sure the log level is " + variant.actualLogLevel) u, err := getObject(ctx, "Deployment", deploymentName, namespace, k8sClient) Expect(err).To(BeNil()) err = scheme.Convert(u, dep, nil) @@ -298,6 +348,10 @@ var _ = Describe("Testing audit flags", func() { ) When("Manipulating parameters", func() { + + // TODO(jkyros): come back to refactor this test, it doesn't proeprly reset between test cases, and + // if it did, it would be failing, because it's currently impossible to deconfigure audit because of + // how our manifestival code only operates if the settings are different from default/empty. BeforeEach(func() { scheme = k8sManager.GetScheme() dep = &appsv1.Deployment{} @@ -317,7 +371,7 @@ var _ = Describe("Testing audit flags", func() { value: "json", }, { - argument: "auditMaxAge=", + argument: "auditMaxAge", prefix: "--audit-log-maxage=", value: "1", }, @@ -333,13 +387,15 @@ var _ = Describe("Testing audit flags", func() { }, } for _, variant := range vars { + caseName := fmt.Sprintf("adds '%s' with value '%s'", variant.argument, variant.value) + It(caseName, func() { + manifest, err := changeAttribute(manifest, variant.argument, variant.value, scheme, caseName) + Expect(err).To(BeNil()) - It(fmt.Sprintf("adds '%s' with value '%s'", variant.argument, variant.value), func() { - manifest, err := changeAttribute(manifest, variant.argument, variant.value, scheme) Expect(manifest.Apply()).To(Succeed()) Eventually(func() error { - _, err = getObject(ctx, "Deployment", metricsServerName, namespace, k8sClient) - return err + return deploymentHasRolledOut(metricsServerName, namespace, caseName) + }, timeout, interval).Should(Succeed()) u, err := getObject(ctx, "Deployment", metricsServerName, namespace, k8sClient) Expect(err).To(BeNil()) @@ -369,18 +425,41 @@ func getDepArg(dep *appsv1.Deployment, prefix string, containerName string) (str return "", errors.New("Could not find a container: " + containerName) } -func changeAttribute(manifest mf.Manifest, attr string, value string, scheme *runtime.Scheme) (mf.Manifest, error) { +func changeAttribute(manifest mf.Manifest, attr string, value string, scheme *runtime.Scheme, annotation string) (mf.Manifest, error) { transformer := func(u *unstructured.Unstructured) error { kedaControllerInstance := &kedav1alpha1.KedaController{} if err := scheme.Convert(u, kedaControllerInstance, nil); err != nil { return err } + // Annotations might be nil, so we need to make sure we account for that + if kedaControllerInstance.Spec.Operator.DeploymentAnnotations == nil { + kedaControllerInstance.Spec.Operator.DeploymentAnnotations = make(map[string]string) + } + + if kedaControllerInstance.Spec.AdmissionWebhooks.DeploymentAnnotations == nil { + kedaControllerInstance.Spec.AdmissionWebhooks.DeploymentAnnotations = make(map[string]string) + } + if kedaControllerInstance.Spec.MetricsServer.DeploymentAnnotations == nil { + kedaControllerInstance.Spec.MetricsServer.DeploymentAnnotations = make(map[string]string) + } + // When we push through an attribute change, we also set an annotation matching the test case + // so we can be sure the controller reacted to our kedaControllerInstance updates and we have "our" + // changes, not just some changes that might be from a previous test case + kedaControllerInstance.Spec.Operator.DeploymentAnnotations["testCase"] = annotation + kedaControllerInstance.Spec.AdmissionWebhooks.DeploymentAnnotations["testCase"] = annotation + kedaControllerInstance.Spec.MetricsServer.DeploymentAnnotations["testCase"] = annotation + switch attr { case "namespace": kedaControllerInstance.Namespace = value case "logLevel": kedaControllerInstance.Spec.Operator.LogLevel = value + // TODO(jkyros): this breaks pattern with the rest of these cases but multiple operands have + // the same field, but we kind of bolted the admission tests on here without doing a refactor and + // this makes it work for now + case "logLevel-admission": + kedaControllerInstance.Spec.AdmissionWebhooks.LogLevel = value // metricsServer audit arguments case "auditLogFormat": kedaControllerInstance.Spec.MetricsServer.AuditConfig.LogFormat = value @@ -398,3 +477,22 @@ func changeAttribute(manifest mf.Manifest, attr string, value string, scheme *ru return manifest.Transform(transformer) } + +// deploymentHasRolledOut waits for the specified deployment to possess the specified annotation +// +//nolint:unparam +func deploymentHasRolledOut(deploymentName string, namespace string, deploymentAnnotation string) error { + u, err := getObject(ctx, "Deployment", deploymentName, namespace, k8sClient) + if err != nil { + return err + } + // The default manifest has no annotation, so I'm not checking whether it's present, only the value, + // because the default case will not have the annotation, and we want that to be okay in the case where we + // apply the default. + testcase := u.GetAnnotations()["testCase"] + if deploymentAnnotation == testcase { + By("Observing that the test case annotation is now: " + testcase) + return nil + } + return fmt.Errorf("Deployment has not rolled out, annotation is still '%s'", testcase) +}