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

v3.9.0 fails on linting (v3.8.0 still works) #580

Closed
marcofranssen opened this issue Jul 19, 2023 · 10 comments
Closed

v3.9.0 fails on linting (v3.8.0 still works) #580

marcofranssen opened this issue Jul 19, 2023 · 10 comments

Comments

@marcofranssen
Copy link

Is this a BUG REPORT or FEATURE REQUEST? (choose one):

BUG REPORT

Version of Helm and Kubernetes:

  • ct v3.9.0
  • helm v3.12.0

What happened:

https://github.com/spiffe/helm-charts/actions/runs/5602051333/jobs/10246787989?pr=324#step:6:20

What you expected to happen:

To have the pipeline succeed just like it does with the v3.8.0 version of ct.

How to reproduce it (as minimally and precisely as possible):

  1. Use chart-testing 3.9.0
  2. Use the github action checkout action using fetch: 0
  3. run ct lint --config ct.yaml --target-branch $(TARGET_BRANCH) --check-version-increment=false
  4. now run the same with 3.8.0 and you will see it still works as expected.

Anything else we need to know:

See here the working variant with v3.8.0

@alacuku
Copy link

alacuku commented Jul 24, 2023

The issue seems to be related to: #552. Since it is passing down the extra-helm-args to helm lint it could happen that flags between differen helm commands are incompatible such as the --timeout flag which is a legit one for helm install but makes helm lint to fail.

djeebus added a commit to zapier/kubechecks that referenced this issue Jul 24, 2023
@joejulian
Copy link
Contributor

joejulian commented Aug 16, 2023

Perhaps we should have more than one helm-extra-args, ie: helm-install-extra-args, helm-upgrade-extra-args, helm-lint-extra-args, etc. Same as above, ct 3.9.0 now breaks because helm lint --timeout 900s is invalid.

What do you think, @AndersBennedsgaard

@joejulian
Copy link
Contributor

Which was also suggested in #540

@AndersBennedsgaard
Copy link
Contributor

@joejulian I agree, that sounds like a good solution going forward, but we should probably keep the --helm-extra-args such that you are able to reduce duplicate settings.

P.s. I haven't actually had issues with 3.9.0 yet, so you only just made me aware of this (sorry for the inconveniences my change has brought 😞 )

@odysseu
Copy link

odysseu commented Sep 12, 2023

TLDR :

Same issue

Have Time To Read :

my use case

Goal:

Trigger chart linting on merge request in gitlab.

Problem :

Won't read dynamic branch naming.

  • I have a CI stage that launches a script to test chart lint

  • the lint.sh :

#!/bin/bash

changed=$(ct list-changed --target-branch ${CI_MERGE_REQUEST_TARGET_BRANCH_NAME})
if [[ -n "$changed" ]]; then
    echo 'Something changed'
    ct lint --target-branch ${CI_MERGE_REQUEST_TARGET_BRANCH_NAME} --validate-maintainers=false
fi
  • CI logs :
...
Error: targetBranch 'main' does not exist

</details>
Something changed
Linting charts...
------------------------------------------------------------------------------------------------------------------------
No chart changes detected.
------------------------------------------------------------------------------------------------------------------------
Error: failed linting charts: failed identifying charts to process: targetBranch 'main' does not exist
failed linting charts: failed identifying charts to process: targetBranch 'main' does not exist

@jouve
Copy link
Contributor

jouve commented Oct 3, 2023

this looks like a duplicate of #577 , so I suggest the same workaround for gitlab-ci :

before_script:
  - git branch "$target_branch" "origin/$target_branch"

@cpanato
Copy link
Member

cpanato commented Oct 31, 2023

@cpanato cpanato closed this as completed Oct 31, 2023
@prashant-shahi
Copy link
Contributor

ct 3.9.0 now breaks because helm lint --timeout 900s is invalid

@joejulian @AndersBennedsgaard I am facing same issue. Fails in 3.9.0 and 3.10.0. How did you guys overcome this?

@joejulian
Copy link
Contributor

We're running off my fork for now. See our workflows in github.com/redpanda-data/helm-charts.

@cpanato
Copy link
Member

cpanato commented Nov 1, 2023

there is a fix for that #605

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

No branches or pull requests

8 participants