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

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

Closed
kocolosk opened this issue Jan 7, 2022 · 10 comments · Fixed by #80 or #87
Closed

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

kocolosk opened this issue Jan 7, 2022 · 10 comments · Fixed by #80 or #87

Comments

@kocolosk
Copy link
Member

kocolosk commented Jan 7, 2022

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

colearendt commented May 10, 2022

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

colearendt commented May 11, 2022

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

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

colearendt commented May 25, 2022

Worth noting this commit: 930c909

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:

  • write a "backfill" script that will move all of the existing .tar.gz files into GitHub releases
    • my thought on how to do this is write a script that:
    • gets the last commit that a .tar.gz was added to docs/
    • presume that the git state for the charts/ directory was correct at that point (i.e. Chart.yaml updated, etc.). Plan to check that this is the case, but any other changes would be harder to detect. If there are concerns here, we could always SHA the files in the .tar.gz and compare to the git state
    • check out that commit, and use the chart-releaser to create a GitHub release with that tag, packaged .tar.gz, etc.
  • add the GitHub actions to maintain things going forwards
  • Sanity check whether the first run of the GitHub action will populate the index.yaml properly (w/ backfill). If not, we may need to build the index.yaml from scratch, as we do here

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

@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
Copy link
Contributor

colearendt commented May 27, 2022

Sounds good! @willholley I created the scripts with the manual steps for backfill (here: #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
Copy link
Contributor

Just added the PR for GitHub Actions additions too: #80

Looking forward to discussing at your convenience!

@willholley
Copy link
Member

@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

@willholley willholley reopened this Jun 17, 2022
@colearendt
Copy link
Contributor

colearendt commented Jun 17, 2022 via email

@colearendt colearendt mentioned this issue Jun 18, 2022
4 tasks
@colearendt
Copy link
Contributor

Created PR to address w/ some discussion: #87

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 a pull request may close this issue.

3 participants