Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unittests with helm3 cause unwanted side effects #111

Closed
armingerten opened this issue May 11, 2021 · 7 comments · Fixed by #128
Closed

Unittests with helm3 cause unwanted side effects #111

armingerten opened this issue May 11, 2021 · 7 comments · Fixed by #128
Assignees

Comments

@armingerten
Copy link

armingerten commented May 11, 2021

We recently updated our helm charts to apiversion 2 and are therefore using the helm3 flag (i.e. helm unittest -3 .) to execute our test suites.

Unfortunately, when running multiple test jobs within a test suite, values provided via set in one test job are still present in every consecutive test job. This causes assertions of the consecutive jobs to fail, due to the set assumptions no longer being valid.

I tried debugging and noticed that the call to v3engine.Render(filteredChart, vals) actually manipulates the filteredChart.Values field on the v3chart.Chart struct. This is really counterintuitive because one might assume that values from a chart are immutable.

// Filter the files that needs to be validated
filteredChart := t.filterV3Chart(targetChart)
outputOfFiles, err := v3engine.Render(filteredChart, vals)

Since filteredChart.Values seems to be pointing to the same location as targetChart.Values, the next test job run in the suite will end up with values altered by previous test jobs.

@quintush
Copy link
Owner

Hello @armingerten,

Thanks for the feedback.

The implementation indeed should behave, as each test has immutable values.
Somehow i missed a testcase, validating that behaviour.

I will look into this and try to fix it.

Greetings,
@quintush

@quintush quintush self-assigned this May 11, 2021
@quintush
Copy link
Owner

Hello @armingerten,

I have verified some scenario's and with all the current testcases i'm not able to reproduce your situation.
As an example, when I use the following ingress testsuite in different order and run them, they all pass. As, based on my interpretation of the issue, some tests should fail as settings where already set by previous testjobs within the testsuite.

Perhaps you can help me with some examples, so i can understand the situation better.

Also here is some additional pointers on how the rendering of the tests are setup and the immutability of the values files, this may help understanding how the unittests work:

  • the v3engine.Render(filteredChart, vals) does indeed change the filteredChart.Values, the reason this happens is that in the render method the user values (defined in vals) do override the default (defined in filteredChart.Values)
  • within filterV3Chart method a copy of the targetChart is created and put in filteredChart, only partial files and the files defined in the suite are filtered for the unittests, this way only files are loaded that are required within the testsuite and as it is an copy the original targetChart will be untouched.

Greetings,
@quintush

@armingerten
Copy link
Author

Hi @quintush,

thanks for the quick reply!

I was also having trouble narrowing down the exact root cause for this issue. I renamed and stripped down my chart as much as possible until I ended up with this minimal setup to reproduce the issue.

When running helm unittest -3 ., the following output is displayed:

### Chart [ example ] .

 FAIL   tests/deployment_test.yaml
        - should only add annotation `hello` with value `world`

                - asserts[0] `equal` fail
                        Template:       example/templates/deployment.yaml
                        DocumentIndex:  0
                        Path:   spec.template.metadata.annotations
                        Expected to equal:
                                hello: world
                        Actual:
                                foo: bar
                                hello: world
                        Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1,2 @@
                                +foo: bar
                                 hello: world


Charts:      1 failed, 0 passed, 1 total
Test Suites: 1 failed, 0 passed, 1 total
Tests:       1 failed, 1 passed, 2 total
Snapshot:    0 passed, 0 total
Time:        15.017511ms

Error: plugin "unittest" exited with error

There is a couple of observations I made when testing, and now I am completely puzzled:

  • The issue seems to be related to the fact that the chart depends on another chart.
    • If I remove all "sub charts", the tests run successfully
    • If I only remove the "sub sub chart", the tests run successfully
  • The issue seems to be related to the fact that variables are used within global
    • If I change all values references from global.app.annotations to foo.app.annotations, the tests run successfully.
  • The issue seems to be related to the nesting of the configuration
    • If I change all values references from global.app.annotations to global.annotations, the tests run successfully.
  • I could only reproduce the issue if the dependency is improperly configured
    • If the reference to the "sub chart" is correct (correct version, name), the tests run successfully.
    • If the reference to the "sub chart" is deleted, the test fails with the described result.
    • In the example I specified an incorrect version (0.2.0 instead of 0.1.0)

@armingerten
Copy link
Author

armingerten commented Jun 7, 2021

Did you have any luck reproducing the issue?

My best guess is that it might be related to using values within global, since I could only reproduce the issue when sub charts were located in the charts/ folder and a template was referencing .Values.global.....

@quintush
Copy link
Owner

Hello @armingerten,

It took some time to investigate the actual situation.
The situation can be separated in 3 parts.

  1. Importing data from subsubcharts
  2. Global values
  3. Dependency version

1 There was an issue that importing data in sub-subcharts got corrupted (#115). This problem could relate to your first observation.

  1. The use of global values impact all charts, so when multiple charts are using the global section the filled global value in the override will be used

  2. Currently there is no validation of dependency versions. Together with the global value this can result in unpredictive behaviour. Although with the unittesting the unpredictive behaviour is a good thing, as the validation fails. The problem however is that the error is not telling the problem is related the dependency version.

Currently i'm not able to validate the versioning of all dependencies, but i did create some additional samples to have more support on this particular situation.

Greetings,
@quintush

@armingerten
Copy link
Author

armingerten commented Aug 16, 2021

Hello @quintush,

I also did some more investigation / testing and figured out a solution that works (for my case): Since the v3chart.Chart object seemed to carry unwanted state between individual job executions, I tried (re)loading the chart before every test job.

See PR #128

WDYT?

@mederel
Copy link

mederel commented Sep 21, 2021

Hi @quintush @armingerten

just wanted to share that on my side with quite complex helm v3 charts I encountered the same issue with states overlapping to other tests. Using the code of @armingerten 's PR and compiling the plugin again fixed my issue.

I am using helm v3.7.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants