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

Add release logic #35

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

miles-w-3
Copy link
Collaborator

@miles-w-3 miles-w-3 commented Jan 4, 2025

Closes #30

A snapshot of what I did:

Create my pages branch

  • git checkout --orphan gh-pages
  • remove all files from this branch:
    • find . -name ".*" -not -name "." -not -name ".." -not -name ".git" -maxdepth 1 -exec rm -rf {} \;
  • Add a readme, example:
    # AWX Operator Helm Release
    
    This branch is used to manage github-pages releases for the AWX Operator Helm Chart
  • git add README.md
  • git commit -am "chart releaser pages config"
    • -a is important to ensure all files are removed
  • git push --set-upstream origin gh-pages

Create configs in main

  • go back to main and create the workflow configs: git switch main
    • git switch -c feature/chart-releaser-to-main
  • Applied various fixes to Makefile to make it work as expected
  • add the release.yml action to .workflows
  • Added some documentation

Remaining design decisions

A huge consequence of this approach is that the charts directory can't be gitignored, since chart-releaser uses it to detect changes. A consequence of this is that contributors need to make sure that they don't commit into charts/ on release. I think a natural solution to this is to make a development make target for helm generation that goes to a /test folder, and then have a release make target which goes to charts and is used only in the release pipeline. That will keep this directory clean from accidental commits.

There's probably another approach we could take of permanently populating charts with enough stubs to fool chart-releaser into always wanting to make a release, but I'm not sure if we want to do that either - imo there is value in allowing non-chart-specific commits like readme and other top-level file updates which should not trigger a release.

Blockers for merge

I think that the current gh-pages branch is already in a good spot. It points at all of the old releases in the index file, and chart-releaser will seamlessly add releases onto it. @rooftopcellist can probably confirm, but I'm guessing the custom url for the page has already been set up.

Therefore, I don't think there are any remaining blockers beyond figuring out how to limit dumping in charts/. In this branch, I set the chart version at 3.0.0, since @oraNod as you pointed out helm recommends using semver.

One last thought, we may want a PR validation workflow that generates the chart and checks if there are file changes in charts/. If there are, it should make sure that the generated chart has a new version.

@miles-w-3 miles-w-3 changed the title Add release logic - updated pr for posterity Add release logic Jan 4, 2025
@miles-w-3 miles-w-3 requested a review from oraNod January 4, 2025 04:34
Copy link
Contributor

@oraNod oraNod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this one @miles-w-3 I'll find some time this week to go through and see these changes in action. It's great to see some progress made towards the next release though.

About protecting the charts directory from accidental commits, a .git/hooks/pre-commit hook could be a good option to prevent direct commits. Something like this might work:

#!/bin/bash
if git diff --cached --name-only | grep -q "^charts/"; then
    echo "Direct commits to the charts directory not allowed"
    echo "Use the release process instead"
    exit 1
fi

Cheers.

python-versions: "${{ matrix.python-versions }}"
- name: "Run nox -s ${{ matrix.session }}"
run: |
nox -s "${{ matrix.session }}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miles-w-3 Should we really delete this workflow? At the moment it only runs the docs build session but we might decide to add other nox sessions for things un-related to the release, like linting, spell checks, etc.

@@ -5,7 +5,7 @@ gh-pages/
/bundle
/bundle_tmp*
/bundle.Dockerfile
/charts

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure we want an empty line here 😄

@@ -1,14 +1,18 @@
# Include the Makefile from the ansible/awx-operator repository.
# This Makefile is created with the clone-awx-operator.py script.
include Makefile.awx-operator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miles-w-3 I've only been able to take a bit of a cursory look at these changes but if we no longer need the makefile from the awx-operator repo should we also update the clone-awx-operator.py script so it doesn't create it?

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 this pull request may close these issues.

Helm chart release process
2 participants