k8ssandra-operator icon indicating copy to clipboard operation
k8ssandra-operator copied to clipboard

Update yq `eval-all` functionality

Open emerkle826 opened this issue 2 years ago • 5 comments

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

emerkle826 avatar Jan 20 '23 23:01 emerkle826

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 avatar Jan 23 '23 06:01 burmanm

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)

burmanm avatar Jan 23 '23 06:01 burmanm

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 avatar Jan 23 '23 16:01 emerkle826

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

emerkle826 avatar Jan 26 '23 14:01 emerkle826

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?

emerkle826 avatar Jan 26 '23 14:01 emerkle826