fix(ci): use default chart-testing logic for different testcases

[wrenix]

Pull Request

Description of the change

first chart-testing values (without ServiceMonitor will be solved in #523 ).

Additional information

Checklist

• DCO has been signed off on the commit.
• Chart version bumped in Chart.yaml according to semver.
• (optional) Variables are documented in the README.md

[wrenix]

This is a internal timeout error, i do not know what the exactly problem is

[jessebot]

In #598 we recently added more tests, but I do kind of like the idea of using values files instead of set args, but I'm not sure the order of prescience when using --helm-extra-args "--values charts/nextcloud/ci/ct-all-enabled-values.yaml" with ct, as in I'm not sure if it still processes the existing values.yaml and then merges the --values file on top of it, or if it ONLY processes the passed in --values file. The issue is that if it only uses the values file is passed in as an arg, and ignores the one sitting in the charts/nextcloud directory, we run the risk of any of our "special" test values files getting out of date and needing to be maintained along side the actual values.yaml.

That said, I do think it would be good to add a redis test to our set of tests we've defined here:

helm/.github/workflows/lint-test.yaml

Lines 69 to 83 in 34fc2df

	test_cases:
	# test the plain helm chart with nothing changed
	- name: 'Default - no custom values'
	# test the helm chart with postgresql subchart enabled
	- name: PostgreSQL Enabled
	helm_args: '--helm-extra-set-args "--set=postgresql.enabled=true --set=postgresql.global.postgresql.auth.password=testing123456 --set=internalDatabase.enabled=false --set=externalDatabase.enabled=True --set=externalDatabase.type=postgresql --set=externalDatabase.password=testing12345"'
	# test the helm chart with nginx container enabled
	- name: Nginx Enabled
	helm_args: '--helm-extra-set-args "--set=image.flavor=fpm --set=nginx.enabled=true"'
	# test the helm chart with horizontal pod autoscaling enabled
	- name: Horizontal Pod Autoscaling Enabled
	helm_args: '--helm-extra-set-args "--set=hpa.enabled=true --set=hpa.minPods=2 --set=hpa.maxPods=3 --set=hpa.targetCPUUtilizationPercentage=75"'

[wrenix]

To answer your question.
It will use the default charts/nextcloud/values.yaml like on a normal helm install and patch the given ct-*values.yaml (thats the reason, why i use an ct-empty-values.yaml to have an testcase with no changes).

but yes the testcase i wrote are a little bit stange (instatt of an nginx or postgresql testcase itself).
(i just tried to reduce compute time) ....

so i will rewrite / move the new existing testcases into a chart-testing way.

[wrenix]

Hmm, i change the ci/ct-*values.yaml the way you created testcases on github matrix way .... so have not seen / review an early state of #598 was it that idea?

---

edit: there is still something wrong with the s3 upload (and test) - i am not sure if we:

• should create for every testcase a complete new kind cluster (current setup) -> need a lot of compute time
• minio test with fixed namespace vs. cleanup correct (sorry my brain is tiered to express myself)

another unvalided thought: maybe we should move the upload test into the helm-chart so that everybody could test it with an helm post-install/-upgrade hook)

[jessebot]

Hmm, i change the ci/ct-*values.yaml the way you created testcases on github matrix way .... so have not seen / review an early state of #598 was it that idea?

Sorry, can you rephrase that a bit? I'm a little confused at what's being asked, but I want to help!

edit: there is still something wrong with the s3 upload (and test) - i am not sure if we:

* should create for every testcase a complete new kind cluster (current setup) -> need a lot of compute time

Yes, that's the issue with how I set it up currently: a lot of compute time. But we, kate and I, had earlier decided that was worth it, because it helped keep all the tests isolated.

* minio test with fixed namespace vs. cleanup correct (sorry my brain is tiered to express myself)

we could technically use different name spaces on the same cluster, and that would still keep the tests fairly isolated, yes, but we'd need still to clean up the old tests, as some of the things we test would still have similar names, so if we tested ingress, we couldn't have that be namespace specific, I think 🤔 @provokateurin do you want to chime in? @wrenix makes some valid points, but I'm not sure how much the nextcloud org is hurting for compute at this time.

another unvalided thought: maybe we should move the upload test into the helm-chart so that everybody could test it with an helm post-install/-upgrade hook)

that sounds like a kind thing to do :) do we want to include the vanilla minio chart as an optional subchart of our chart to facilitate that, or should we always require an external s3 server?

[provokateurin]

I'm not sure how much the nextcloud org is hurting for compute at this time.

Should be fine, as long as we don't spawn 20+ very long running jobs ;)