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

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wrenix
Copy link
Contributor

@wrenix wrenix commented Feb 3, 2024

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)

Checklist

@wrenix wrenix force-pushed the fix/ct-values branch 2 times, most recently from 2fc9d68 to 16ff55e Compare February 3, 2024 22:23
@wrenix wrenix changed the title values-files for chart-testing fix(ci): values-files for chart-testing Feb 3, 2024
@wrenix
Copy link
Contributor Author

wrenix commented Feb 3, 2024

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

But maybe you like to run it, to see the errors 😗

@wrenix wrenix force-pushed the fix/ct-values branch 2 times, most recently from e99182b to 042586f Compare February 4, 2024 10:17
@wrenix
Copy link
Contributor Author

wrenix commented Feb 4, 2024

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

@wrenix
Copy link
Contributor Author

wrenix commented Feb 4, 2024

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 wrenix force-pushed the fix/ct-values branch 2 times, most recently from a15fa96 to 69d310c Compare February 4, 2024 15:21
@wrenix
Copy link
Contributor Author

wrenix commented Feb 4, 2024

first look at:
#517 (comment)

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
Copy link
Member

Still broken?

@wrenix
Copy link
Contributor Author

wrenix commented Feb 4, 2024

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
Copy link
Member

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

@wrenix
Copy link
Contributor Author

wrenix commented Feb 4, 2024

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 wrenix marked this pull request as draft February 5, 2024 00:35
@wrenix
Copy link
Contributor Author

wrenix commented Feb 5, 2024

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

@wrenix wrenix force-pushed the fix/ct-values branch 2 times, most recently from 266b68c to 8689b3e Compare February 5, 2024 01:08
Signed-off-by: WrenIX <dev.github@wrenix.eu>
Signed-off-by: WrenIX <dev.github@wrenix.eu>
@wrenix wrenix changed the title fix(ci): values-files for chart-testing fix(ci): improve lint and testing - WIP Feb 6, 2024
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 this pull request may close these issues.

None yet

2 participants