helm / chart-testing-action

A GitHub Action to lint and test Helm charts
https://github.com/helm/chart-testing
Apache License 2.0
251 stars 71 forks source link

No chart changes detected #108

Open SapiensAnatis opened 1 year ago

SapiensAnatis commented 1 year ago

Hey, I'm probably messing something up here, but every run of the action on my chart repo is saying that nothing has changed when it has.

Take for instance this run: https://github.com/SapiensAnatis/helm-charts/actions/runs/4033008969

which was triggered by this commit: https://github.com/SapiensAnatis/helm-charts/commit/47289587c35a1bad55d4e46ff6b3d1d59563fe2c

It says that nothing has changed when I've clearly updated one of the templates. Additionally, I think my directory structure is sound, with

Git repository
├───.github
│   └───workflows
└───charts
    └───dragalia-api
        ├───charts
        └───templates
            └───tests

Feel free to look at the repo itself but summarily the chart depends on two bitnami charts (redis and postgresql) and has the following ct.yaml

remote: origin
chart-dirs:
  - charts
chart-repos:
  - bitnami=https://charts.bitnami.com/bitnami
helm-extra-args: --timeout 600s

When I run the same command that the action is running, ct list-changed --target-branch master --config ct.yaml --remote=origin after making a change, I see the chart in the list of changes so not sure what's going on

cpanato commented 1 year ago

I took a look at your repo and cloned that and run locally the ct with a chart change and that works fine.

does that still an issue? otherwise lets close this issue, thanks!

MindTooth commented 1 year ago

I can verify that this affects me too.

Locally:

$ ct list-changed --target-branch master --config ct.yaml --remote=origin
containers/helm

But, initial PR where I try to use this action doesn't detect any changes. And no checking is done.

IMHO, it's not very obvious what I'm doing wrong. And it seems by this, I'm not the only that have problems.

cpanato commented 1 year ago

I can verify that this affects me too.

Locally:

$ ct list-changed --target-branch master --config ct.yaml --remote=origin
containers/helm

But, initial PR where I try to use this action doesn't detect any changes. And no checking is done.

IMHO, it's not very obvious what I'm doing wrong. And it seems by this, I'm not the only that have problems.

@MindTooth made a comment in your PR, that change should work, i've tested in a fork and works for me

cpanato commented 1 year ago

@SapiensAnatis i dont see the job in the repo to reproduce the issue

MindTooth commented 1 year ago

I can verify that this affects me too. Locally:

$ ct list-changed --target-branch master --config ct.yaml --remote=origin
containers/helm

But, initial PR where I try to use this action doesn't detect any changes. And no checking is done. IMHO, it's not very obvious what I'm doing wrong. And it seems by this, I'm not the only that have problems.

@MindTooth made a comment in your PR, that change should work, i've tested in a fork and works for me

I saw. Thank you! 🙏🏻 Any reason why I need --config ct.yaml? I thought that it was the standard file that ct parsed.

Notice that if no config file is specified, then ct.yaml (or any of the supported formats) is loaded from the current directory, $HOME/.ct, or /etc/ct, in that order, if found.

cpanato commented 1 year ago

you also can set CT_CONFIG_DIR

tirelibirefe commented 6 months ago

@SapiensAnatis is right; changes are not being detected.

      - name: Run chart-testing (list-changed)
        id: list-changed
        run: |
          changed=$(ct list-changed --target-branch ${{ github.event.repository.default_branch }})
          if [[ -n "$changed" ]]; then
            echo "changed=true" >> "$GITHUB_OUTPUT"
          fi

Maybe detected but although I replace "true" operators with "false", the steps don't work:

  ```
tirelibirefe commented 6 months ago

I added --config .github/ct.yaml, I think it works now.

znd4 commented 5 months ago

It seems like this is being caused by CT_CONFIG_DIR being set in ct.sh.

This feels like an anti-pattern, since chart-testing supports .ct/ct.yaml without further configuration.