kudobuilder / kuttl

KUbernetes Test TooL (kuttl)
https://kuttl.dev
Apache License 2.0
688 stars 85 forks source link

Allow partial comparison of slices in asserts #76

Open alembiewski opened 4 years ago

alembiewski commented 4 years ago

What would you like to be added:

It would be great to check only the subset of a slice in a test step assertion. Currently, it is required to specify all the array elements, otherwise, a test fails with slice length mismatch error.

Why is this needed:

Here are some real-life pain points:

kensipe commented 4 years ago

@alembiewski could you provide an example of what is required today and what you would like to see (yaml examples will help visualize). This seems valuable.

kensipe commented 4 years ago

we need enough info to create a kep

alembiewski commented 4 years ago

Here is the example: let's say we have a custom resource TrainingJob, which describes some machine-learning training job, and it has a status.conditions[] field (slice). The job goes through several conditions before it reaches the final state. In my test case, I want to check the job is succeeded. Currently, to work-around the slice length mismatch error, my assert file looks like this:

kind: TrainingJob
metadata:
  name: ml-job
status:
  conditions:
  - reason: JobCreated      # I have to provide all the intermediate conditions here
  - reason: JobRunning      # to make kuttl happy
  - reason: JobSucceeded   <-- this is what I actually want to test
    status: "True"
    type: Succeeded

This is how I would like my assert to be:

kind: TrainingJob
metadata:
  name: ml-job
status:
  conditions:
  - reason: JobSucceeded
    status: "True"
    type: Succeeded

So the problem is that I have to specify all the conditions (but this could be env variables, volumes - any property with array type).

kensipe commented 4 years ago

This is related to #75 Looking for better array support in asserts. Looking for:

  1. Disregard order to array
  2. Partial array support (only assert what is expressed)
porridge commented 4 years ago

Please note that the workaround mentioned in https://github.com/kudobuilder/kuttl/issues/76#issuecomment-632112138 is fragile: we've seen cases where the elements appear in a different order, and this causes the test to fail anyway:

 status:
   conditions:
-  - reason: JobCreated
-  - reason: JobRunning
-  - reason: JobSucceeded
+  - lastTransitionTime: "2020-07-17T19:13:59Z"
+    lastUpdateTime: "2020-07-17T19:13:59Z"
+    reason: JobCreated
+    status: "True"
+    type: Created
+  - lastTransitionTime: "2020-07-17T19:16:06Z"
+    lastUpdateTime: "2020-07-17T19:16:06Z"
+    reason: JobSucceeded         
     status: "True"
     type: Succeeded
+  - lastTransitionTime: "2020-07-17T19:16:06Z"
+    lastUpdateTime: "2020-07-17T19:16:06Z"
+    reason: JobRunning
+    status: "True"
+    type: Running
nic-hima commented 4 years ago

Hey, looking at this from a slightly different lense (Gomega), it almost seems like you need something like: ContainElement() .

This is related to #75 Looking for better array support in asserts. Looking for:

1. Disregard order to array

2. Partial array support (only assert what is expressed)

I have written quite a few tests imperatively (non-declarative) that boil down to something like https://github.com/kudobuilder/kuttl/issues/76#issuecomment-632112138 in code for operator testing. Could definitely see broader use-cases of this requested functionality!

owais commented 2 years ago

I'm writing an operator that injects a number of environment variables into pod container spec. The number of env vars it injects depends on a few factors which I might not want to necessarily test. Also some env vars have values that have some randomness them (e.g, a uuid). kuttl has no way to tell what the random value would be so it cannot test against the specific var. I'd like to leave out the random vars and test the rest with kuttl. This would give me enough confidence in an end-to-end test while I test fine grained logic in unit tests. Does this use case make sense?

owais commented 2 years ago

One more use case is where my operator injects the current namespace into the container spec as an environment variable. Right now my only workaround is to create a namespace with a well-known name and run the entire test inside that. This obviously adds other limitations. It would be nice if we could have some sort of variable substitution in asserts to test dynamic values like kuttl namespaces but still being able to test a slice partially would be even better as it solves many other such cases as well.

owais commented 2 years ago

Another case is volumeMounts. My operator mounts a specific volume on containers and I'd like to be able to test that with kuttl. However, k8s also appears to mount the following volume automatically:

    volumeMounts:
        - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
           name: kube-api-access-54d79
           readOnly: true

the volume name has a random component which cannot be predicted. I just want to be able to test that the operator injected a specific volume on a container. I don't care about what other volumes some other component may or may not be mounting. If the other volumes had predictable names, I could may be list them in my assertions even though they'd would make the tests look unclean and erode the intention of the assertions a bit but in this case I can't even do that as the name is randomly generated.

cbandy commented 2 years ago

The examples given above are all lists of objects following Kubernetes conventions. When accepting a patch, the API server treats these lists like maps. Other examples can be found in the API documentation with the note "Patch strategy: merge on key …" and the OpenAPI specification with x-kubernetes-patch-merge-key.

I'm not sure if or how KUTTL should incorporate this information.

Perhaps assertions need to be more expressive? Kubernetes is doing something with CEL to specify cross-field validation rules. Kyverno interprets special characters in YAML keys and values to match beyond simple equality. OPA has Rego, etc.

Related: #152

shanproofpoint commented 2 years ago

I am very surprised kuttl having such great google results has almost no support for non exact matches in the assertions and errors yamls.

gberche-orange commented 2 years ago

@shanproofpoint I'm using a TestAssert command as a workaround.

apiVersion: kuttl.dev/v1beta1
kind: TestAssert
commands:
- timeout: 40
  script: |
    kubectl wait --for=condition=Available deployment/spring-petclinic-medium --timeout 30s --namespace $NAMESPACE
---
# usual kuttl style assertion follows
apiVersion: apps/v1
kind: Deployment
metadata:
  name: spring-petclinic-medium
spec:
  template:
    spec:
      #! service binding operator injects a volume and volume mounts with the secret
      containers:
        - volumeMounts:
            - mountPath: /bindings/spring-petclinic-medium-postgresql
              name: spring-petclinic-medium-postgresql
[...]

This makes assertions harder to read. I'll try next to using the bats detik dsl for non-exact matches in TestAsserts as to be closer to "documentation by example" / bdd style asserts.

shanproofpoint commented 2 years ago

thanks @gberche-orange , my test suite is also littered with commands everywhere lol. i also have a scenario where cluster one creates cluster 2 and a need to switch kubeconfigs. not supported chuckle lol. so i embedded kubectl kuttl in a command to assert the additional resources lol

blakeromano commented 1 year ago

➕ 1️⃣ to this issue. Super needed!

kensipe commented 1 year ago

roger that... Marcin did some great work on this... I'll review this week and see if we can get this in

jesher commented 1 year ago

any update about this?

Miles-Garnsey commented 1 year ago

I'm chasing an update on this too.

We're seeing increasing issues as our configs get more complex. It is starting to make it really hard to sell the team on using kuttl.

shanproofpoint commented 1 year ago

i have made kuttl the defacto testing framework for our declarative resources. without these more flexible comparators, i am afraid it will stay a toy set. a lot of the steps have devolved into nothing more than kubectl and bash wrappers

gberche-orange commented 1 year ago

@kensipe @porridge I wonder whether inspiration from goss advanced matchers using gomega matchers could help here:

Goss supports advanced matchers by converting json input to gomega matchers.

Examples

Validate that user nobody has a uid that is less than 500 and that they are only a member of the nobody group.

user:
  nobody:
    exists: true
    uid:
      lt: 500
    groups:
      consist-of: [nobody]

Matchers can be nested for more complex logic, for example you can ensure that you have 3 kernel versions installed and none of them are 4.1.0:

package:
  kernel:
    installed: true
    versions:
      and:
        - have-len: 3
        - not:
            contain-element: "4.1.0"

Custom semver matcher is available under semver-constraint:

example:
  content:
    - 1.0.1
    - 1.9.9
  matches:
    semver-constraint: ">1.0.0 <2.0.0 !=1.5.0"

For more information see:

  • gomega_test.go - For a complete set of supported json -> Gomega mapping
  • gomega - Gomega matchers reference
  • semver - Semver constraint (or range) syntax
ZhiminXiang commented 1 year ago

Is there any update on this issue? It is a big blocker for us to adopt kuttl

ra0e commented 1 year ago

We have the same wish, it is always hard to work around this.

jeremymv2 commented 1 year ago

@kensipe any new updates on this?

jankaacc commented 11 months ago

hi I am also looking forward for this.