Skip to content

Commit

Permalink
Merge pull request #233 from jkyros/make-loglevel-test-work
Browse files Browse the repository at this point in the history 
Fix functionality (audit, loglevel) tests to actually wait for changes
  • Loading branch information
@joelsmith
joelsmith authored Jun 25, 2024
2 parents 81ff7b1 + c926be8 commit 4967f87
Showing 1 changed file with 132 additions and 34 deletions.
166 changes: 132 additions & 34 deletions controllers/keda/kedacontroller_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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() {
Expand All @@ -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",
Expand All @@ -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))

})
}

})
})

Expand Down Expand Up @@ -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() {
Expand All @@ -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",
Expand All @@ -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)
Expand Down Expand Up @@ -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{}
Expand All @@ -317,7 +371,7 @@ var _ = Describe("Testing audit flags", func() {
value: "json",
},
{
argument: "auditMaxAge=",
argument: "auditMaxAge",
prefix: "--audit-log-maxage=",
value: "1",
},
Expand All @@ -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())
Expand Down Expand Up @@ -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
Expand All @@ -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)
}

0 comments on commit 4967f87

Please sign in to comment.