nextcloud / helm

A community maintained helm chart for deploying Nextcloud on Kubernetes.

GNU Affero General Public License v3.0

295 stars 258 forks source link

fix(ci): improve lint and testing - WIP #523

Open wrenix opened 5 months ago

wrenix commented 5 months ago

Pull Request

Description of the change

Here some value files for chart-testing. it enables "all" features of the helmchart to find bugs like in #522

Benefits

it enables "all" features of the helm-chart during the chart-testing to find bugs like in #522

Possible drawbacks

Longer CI runtime

Additional information

TODO / Checklist (maybe moved to other PRs)

[ ] install CRDs of prometheus-operator (required)
[ ] lint error for "wrong" indent in sequence (array): https://github.com/helm/chart-testing/blob/afa25e92904181aad0674f9483780bf7a76b100c/etc/lintconf.yaml#L33
[ ] usage of {{- ... | nindent }} instatt of {{ ... | indent }}

Checklist

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

wrenix commented 5 months ago

PS: Current the pipeline should failed, till #521 is merged.

But maybe you like to run it, to see the errors :kissing:

wrenix commented 4 months ago

I believe we should merge that yet already for other and me ;) here related discussion:

wrenix commented 4 months ago

Failing works now: https://github.com/nextcloud/helm/actions/runs/7774014966

==> Linting charts/nextcloud
Error:  templates/metrics/deployment.yaml: unable to parse YAML: error converting YAML to JSON: yaml: line 49: did not find expected '-' indicator

Error: failed linting charts: failed processing charts
Error: 1 chart(s) linted, 1 chart(s) failed

------------------------------------------------------------------------------------------------------------------------
 ✖︎ nextcloud => (version: "4.5.17", path: "charts/nextcloud") > failed waiting for process: exit status 1
------------------------------------------------------------------------------------------------------------------------
failed linting charts: failed processing charts
Error: Process completed with exit code 1.

i rebase it to #521 / main

wrenix commented 4 months ago

first look at: https://github.com/nextcloud/helm/pull/517#issuecomment-1923611887

i found option to set indent of sequence here: https://github.com/helm/chart-testing/blob/afa25e92904181aad0674f9483780bf7a76b100c/etc/lintconf.yaml#L33 but on test i only has effect on the Chart.yaml -> maybe lets create an issue for later (and just merge that).

provokateurin commented 4 months ago

Still broken?

wrenix commented 4 months ago

Oh, thispipeline failed, because it try to install it on an k8s without an prometheus-operator CRDs are preinstalled: https://github.com/nextcloud/helm/actions/runs/7775024318/job/21200687969?pr=523

two options (in my head):

preinstall prometheus-operator
extends servicemonitor with if (.Capabilities.APIVersions.Has "monitoring.coreos.com/v1/ServiceMonitor") (where where lint/test does not check the ServiceMonitor)

provokateurin commented 4 months ago

Preinstalling sounds like it will test more cases, so let's go with that.

wrenix commented 4 months ago

No, that is expacted behaviour. (it should failed, that hrlm try to install something, what the current k8s-cluster does not support) (PS: I thing you know it - i just answer for history/ other reader).

okay lets check out, how it is possible to tune the github actions (i need some more time therefore - i am working more with gitlab-ci).

wrenix commented 4 months ago

i split it into a smaller PR #526 - so that the first part could already be merged till i solve an ServiceMonitor / Prometheus-Operator CRDs