search

k8ssandra / k8ssandra-operator

The Kubernetes operator for K8ssandra
https://k8ssandra.io/
Apache License 2.0
163 stars 75 forks source link

Update yq `eval-all` functionality #820

Open emerkle826 opened 1 year ago

emerkle826 commented 1 year ago

What happened? yq test is failing when using the eval-all option. Example:

/bin/yq ea .spec.cassandra.datacenters.[] as $item ireduce (0; . +1) ../testdata/fixtures/multi-dc/k8ssandra.yaml ../testdata/fixtures/single-dc/k8ssandra.yaml

--- FAIL: TestEval (0.10s)
    --- FAIL: TestEval/count_dcs_many_files_eval_all (0.01s)
        yq_test.go:99: 
                Error Trace:    /home/runner/work/k8ssandra-operator/k8ssandra-operator/test/yq/yq_test.go:99
                Error:          Not equal: 
                                expected: "3"
                                actual  : "3\n3"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1,2 @@
                                 3
                                +3
                Test:           TestEval/count_dcs_many_files_eval_all

Did you expect to see something different? The test expects all input files to be processed once. So in the example above, it's trying to count the number of datacenters defined in 2 different files. One file has 2 DCs defined, the other has only 1. With the eval-all option, the output is expected to be 3. With newer versions of yq the output is repeated for the number of input files, so we now get:

3
3

This test was passing as recently as yesterday (from the time of writing this ticket): https://github.com/k8ssandra/k8ssandra-operator/actions/runs/3956583819/jobs/6776181660

There seems to be a bug reporting this issue: https://github.com/mikefarah/yq/issues/1206

How to reproduce it (as minimally and precisely as possible): See the test failure above.

┆Issue is synchronized with this Jira Story by Unito

burmanm commented 1 year ago

I don't think that ticket you linked is the reason, as this test works fine 4.30.6. I'm not even sure if we've used the older version ever in these tests.

So, on my machine (tm) this test still works?

burmanm commented 1 year ago

Here's the manual output also:

➜  k8ssandra-operator git:(main) /usr/local/bin/yq ea '.spec.cassandra.datacenters.[] as $item ireduce (0; . +1)' test/testdata/fixtures/multi-dc/k8ssandra.yaml test/testdata/fixtures/single-dc/k8ssandra.yaml                    
3
➜  k8ssandra-operator git:(main) yq --version
yq (https://github.com/mikefarah/yq/) version v4.30.6
➜  k8ssandra-operator git:(main)
emerkle826 commented 1 year ago

Interesting. On my machine (yq version 4.30.8) I get the same duplicated output when using the ea option:

yq ea  '.spec.cassandra.datacenters.[] as $item ireduce (0; . +1)' test/testdata/fixtures/multi-dc/k8ssandra.yaml test/testdata/fixtures/single-dc/k8ssandra.yaml 
3
3
[emerkle@erik-datastax k8ssandra-operator]$ yq --version
yq (https://github.com/mikefarah/yq/) version v4.30.8
emerkle826 commented 1 year ago

Continuing the discussion from https://github.com/k8ssandra/k8ssandra-operator/pull/812

emerkle826 commented 1 year ago

This ticket was linked: https://github.com/mikefarah/yq/issues/1515

But that's not exactly the issue with the test. The test uses the eval-all option. It seems that the test expects the eval-all option to run the expression once over the merged set of input files. The ticket referenced does not use the eval-all option, so the expression is run on each input file.

I guess the question is how is the eval-all option supposed to work in the situation we are using it, and do we need to change how we are using it?