apache / couchdb-helm

Apache CouchDB Helm Chart
https://couchdb.apache.org/
Apache License 2.0
49 stars 64 forks source link

Run checks for chart version metadata update as GH Action #62

Closed kocolosk closed 2 years ago

kocolosk commented 2 years ago

We often have Pull Requests that could be merged but are missing the make publish step. At a minimum we can check for that as part of the PR validation.

colearendt commented 2 years ago

Worth noting that the helm project has some nice CI tooling available:

In practice, it has the benefit of avoiding the make publish requirement, and will fail if version conflicts, etc. automatically on your behalf.

It is worth noting that it does have a bit of a different deployment mechanism: using GitHub Releases rather than a directory of tarballs in the repository itself. If you want to see an example, we use here:

https://github.com/rstudio/helm/

colearendt commented 2 years ago

The more I think about this, the more I feel like having the tarballs in the git repository is less than ideal. There is no guarantee that any particular tarball matches the contents of what is in the repository at a given version/time (for instance). Much better to use the GitHub release mechanism to tie the GitHub sources to the released code.

I am happy to help get this set up if this seems like something that would be worthwhile. I suspect we could backfill older releases to ensure backwards compatibility as well.

kocolosk commented 2 years ago

I completely agree! I'd be quite happy to use Releases here, particularly if someone has already gone through the work of generating a Helm-friendly index that points to them.

We're operating in a bit of a gray area with respect to ASF Releases. This chart started out as something that lived on the Helm side of things, and so clearly wasn't in scope as part of Apache CouchDB. Now it's using an ASF-maintained GitHub org, but we still don't include it as part of the CouchDB source distro. I would say let's go ahead and pursue the GH Releases approach.

If you've got the time to hack on it, great! I'd appreciate it.

@willholley any concerns?

colearendt commented 2 years ago

Worth noting this commit: 930c909172470e39ae7bb9efc9abaac35176cea3

Which discusses moving from the gh-pages branch. It's not clear to me whether GitHub Actions and CI as a part of the wider helm community makes this a worthwhile "switch back" or not 😄

The plan that I imagine:

Any thoughts / feedback on this approach? I don't have access to do these things, so thought I would just PR with the appropriate bits and y'all can check / merge / run the manual pieces when convenient.

willholley commented 2 years ago

@colearendt thanks for digging into this. Moving to GH Actions and Releases to align with the Helm community totally makes sense. If you're able to PR the changes, I can run the manual steps as required.

colearendt commented 2 years ago

Sounds good! @willholley I created the scripts with the manual steps for backfill (here: https://github.com/apache/couchdb-helm/pull/79). My next step will be to PR the CI changes. One potential downside of this approach is that outstanding branches will likely need to change how they have handled the release process, and there will be a handful of docs changes needed 🙈 Not sure if docs are something you are comfortable merging piecemeal or if it should be handled in the same PR as the CI changes.

colearendt commented 2 years ago

Just added the PR for GitHub Actions additions too: https://github.com/apache/couchdb-helm/pull/80

Looking forward to discussing at your convenience!

willholley commented 2 years ago

@colearendt we're going to need to make some modifications to the workflows to align with ASF policy - apologies I hadn't picked up on this earlier. The lint/test stages are currently failing e.g. https://github.com/apache/couchdb-helm/actions/runs/2516645843

Broadly, I think we need to:

  1. Bring in the helm/chart-testing-action and helm/kind-action as submodules and update the workflows to use them.
  2. Specify the minimum permissions for each workflow
colearendt commented 2 years ago

Awesome! I’ll take a look - apologies for the trouble!

colearendt commented 2 years ago

Created PR to address w/ some discussion: https://github.com/apache/couchdb-helm/pull/87