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

Nothing to do. No chart changes detected #36

Open
prologic opened this issue Aug 26, 2020 · 10 comments
Open

Nothing to do. No chart changes detected #36

prologic opened this issue Aug 26, 2020 · 10 comments

Comments

@prologic
Copy link

Hi,

I'm having a bit of trouble getting this workflow to work.

You can find my workflow here

Basically when I run this and make some chart changes I end up with:

Run helm/chart-releaser-action@v1.0.0
netdata-helmchart
Looking up latest tag...
Discovering changed charts since '1fe4c3b8e1f1385caf11c17e422c832c711078d3'...
Nothing to do. No chart changes detected.

See sample run

I'm not really sure what I'm doing wrong. The only way I was able ot get this to work was to put things in a directory layout like charts/<name>/... but we don't really want to change our helm chart repo structure.

Thanks!

@prologic
Copy link
Author

If I update my workflow to have charts_dir: . I get the following behaviour:

Run helm/chart-releaser-action@v1.0.0
  with:
    charts_dir: .
  env:
    CR_TOKEN: ***
netdata-helmchart
Looking up latest tag...
Discovering changed charts since '1fe4c3b8e1f1385caf11c17e422c832c711078d3'...
WARNING: .github/Chart.yaml is missing, assuming that '.github' is not a Helm chart. Skipping.
WARNING: sdconfig/Chart.yaml is missing, assuming that 'sdconfig' is not a Helm chart. Skipping.
WARNING: templates/Chart.yaml is missing, assuming that 'templates' is not a Helm chart. Skipping.
WARNING: tools/Chart.yaml is missing, assuming that 'tools' is not a Helm chart. Skipping.
Nothing to do. No chart changes detected.

it seems to me that this tool cannot understand a single chart at the top-level of a repo?

@prologic
Copy link
Author

I think the problem is with this action's workflow cr.sh script itself:

chart-releaser-action/cr.sh

Lines 186 to 188 in 9723794

[[ ! -d "$chart" ]] && continue
local file="$chart/Chart.yaml"
if [[ -f "$file" ]]; then

I can probably fix this if you would accept a PR to be a bit more flexible about the layout and not assume ./charts/<chart_name>/...?

@prologic
Copy link
Author

I made some modifications in this fork and now things are getting even weirder:

Discovering changed charts since '1fe4c3b8e1f1385caf11c17e422c832c711078d3'...
WARNING: .github/Chart.yaml is missing, assuming that '.github' is not a Helm chart. Skipping.
WARNING: sdconfig/Chart.yaml is missing, assuming that 'sdconfig' is not a Helm chart. Skipping.
WARNING: templates/Chart.yaml is missing, assuming that 'templates' is not a Helm chart. Skipping.
WARNING: tools/Chart.yaml is missing, assuming that 'tools' is not a Helm chart. Skipping.
Installing chart-releaser...
Chart 'Chart.yaml' no longer exists in repo. Skipping it...
Releasing charts...
Error: No charts found at .cr-release-packages.

Usage:
  cr upload [flags]

Flags:
  -c, --commit string           Target commit for release
  -b, --git-base-url string     GitHub Base URL (only needed for private GitHub) (default "https://api.github.com/")
  -r, --git-repo string         GitHub repository
  -u, --git-upload-url string   GitHub Upload URL (only needed for private GitHub) (default "https://uploads.github.com/")
  -h, --help                    help for upload
  -o, --owner string            GitHub username or organization
  -p, --package-path string     Path to directory with chart packages (default ".cr-release-packages")
  -t, --token string            GitHub Auth Token

Global Flags:
      --config string   Config file (default is $HOME/.cr.yaml)

Hmmm 🤔

@Knappek
Copy link

Knappek commented Sep 8, 2020

I am observing a similar problem. I have my chart in deployment/helm/my-awesome-chart and provide the charts_dir: deployment/helm. But it seems that charts_dir can not be nested. If I put my chart to deployment/my-awesome-chart and change charts_dir: deployment then it works. I think it is because of this line. It shouldn't filter on 1,2 but rather on every possible nested directory.

@lucernae
Copy link
Contributor

lucernae commented Sep 9, 2020

Nice, I found similar issues with mine.

My use case is kind of the same. I am creating a Rancher charts where the directory structures is:
charts/<chart-name>/<chart-version/Chart.yml

I was able to make it work by changing the filter to 1,2,3 of course.

In @prologic 's case, it failed because the top directory of the charts must stay inside the repo because they filter it by using git diff (not current directory structures). Since your Chart.yml is directly in top directory the function filter_charts missed it.

I can make PR for this, but since it is directly specified in the helm docs that the charts structure must be charts/<chart-name>/Chart.yaml, I was wondering if we can add this changes to this repo?
Personally I want to use it to detect nested charts directory like @Knappek 's said.

Possible implementation

  • Do not cut the path so it can recursively check nested folders (or allow options to do this).
  • Allow options to specify the depth of the search.

Can the maintainer comment on this?

@prologic
Copy link
Author

I can make PR for this, but since it is directly specified in the helm docs that the charts structure must be charts//Chart.yaml, I was wondering if we can add this changes to this repo?

I (to be honest) didn't know this! Q: Would it break/hurt anything to update our repo's structure to match this requirement after the fact now? Would any tooling out there we don't know about helm or otherwise b0rk?

@lucernae
Copy link
Contributor

@prologic yes, if you read the documentations, they are rather specific about this requirements: https://helm.sh/docs/chart_best_practices/conventions/

The directory that contains a chart MUST have the same name as the chart. Thus, the chart nginx-lego MUST be created in a directory called nginx-lego/. This is not merely a stylistic detail, but a requirement of the Helm Chart format.

this requirement is mostly for the tooling to make the tar.gz archive for helm repo deployment (to a webserver or package).

I'm not helm maintainer, but I suggest you make the directory structure match if you plan on creating official helm charts for public.
In my case, it's a little bit different, I want to create Rancher charts (slight difference in structure). I was wondering if the maintainer will allow this use case. If not, then I have to make my own github action (forked from this).

dfarrell07 added a commit to dfarrell07/submariner-charts that referenced this issue Nov 13, 2020
Attempting to get helm/charts-releaser to detect changes to our charts
and publish them. It seems the GHA expects this layout.

See discussion:

helm/chart-releaser-action#36

And default dir:

https://github.com/helm/chart-releaser-action/blob/master/action.yml#L14

Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
dfarrell07 added a commit to dfarrell07/submariner-charts that referenced this issue Nov 13, 2020
Attempting to get helm/charts-releaser to detect changes to our charts
and publish them. It seems the GHA expects this layout.

See discussion:

helm/chart-releaser-action#36

And default dir:

https://github.com/helm/chart-releaser-action/blob/master/action.yml#L14

Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
dfarrell07 added a commit to dfarrell07/submariner-charts that referenced this issue Nov 13, 2020
Attempting to get helm/charts-releaser to detect changes to our charts
and publish them. It seems the GHA expects this layout.

See discussion:

helm/chart-releaser-action#36

And default dir:

https://github.com/helm/chart-releaser-action/blob/master/action.yml#L14

Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
@tonsV2
Copy link

tonsV2 commented May 4, 2022

So this action simply doesn't work if the chart is in the root of the project? I tried the below resulting in the same "Nothing to do. No chart changes detected" as OP reports

  - name: Run chart-releaser
    uses: helm/chart-releaser-action@v1.4.0
    with:
      charts_dir: .
    env:
      CR_TOKEN: "${{ secrets.GITHUB_TOKEN }}"

tonsV2 added a commit to dhis2-sre/dhis2-core-helm that referenced this issue May 4, 2022
@toms-place
Copy link

In my case the Charts dir had a capital "C". The workflow is somehow case sensitive so I change it to:

  - name: Run chart-releaser
    uses: helm/chart-releaser-action@v1.4.0
    with:
      charts_dir: Charts
    env:
      CR_TOKEN: "${{ secrets.GITHUB_TOKEN }}"

@seanmorton
Copy link

For me, I forgot to set fetch-depth in the checkout step, which is necessary to fetch all git history for the tool to compare against:

- name: Checkout
        uses: actions/checkout@v2
        with:
          fetch-depth: 0 # important parameter

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

6 participants