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

feat: add a helm chart with the releaser action #349

Merged
merged 3 commits into from
Oct 9, 2023
Merged

feat: add a helm chart with the releaser action #349

merged 3 commits into from
Oct 9, 2023

Conversation

allanger
Copy link
Contributor

@allanger allanger commented Oct 4, 2023

Hi!

I thought of starting to use sql_exporter on k8s, but I found out that there was no helm chart available, so I've decided to come up with one.

This PR contains the helm chart and the new GitHub action that will lint, test and release that chart. The release action requires GitHub pages enabled, because it's using it as a helm repository, you can check it in my fork: https://github.com/allanger/sql_exporter/tree/gh-pages

In this PR, release is enabled only when the action is triggered by pushing to the master, but since the action is checking if there were changes to the chart before releasing, it will require one more commit with a version bump to release the first version.

When the new chart version is released, it will be put to GitHub releases and GitHub pages branch will be updated. Having it in GitHub releases may not be a desired behavior, since it will conflict with actual release. If it's the case, I guess it's possible to disable it and put releases directly to the pages branch.

What about maintenance?

I can take care of it for a time being. But basically, if there are no breaking changed introduced to the tool, the whole maintenance will be just upgrading helm/Chart.yaml:appVersion to the new app version and helm/Chart.yaml/version to the following one.

helm/values.yaml Outdated Show resolved Hide resolved
@burningalchemist
Copy link
Owner

Hey @allanger, thanks for your contribution! No objections here. The reason there's no helm chart is mostly people use sql_exporter as a container and put into their integrations. However, if there's a case to deploy it as a separate helm chart, then let it be. 👍

@tonobo
Copy link

tonobo commented Oct 5, 2023

Howdy everyone, it's possibly a bit out of context, but i'm interested in having a kubernetes controller. At least for handling the queries / metrics. Quite similar how prometheus rules work in the end. Is this a bit over the top for the project itself or would you like to see a contribution in there.

@burningalchemist
Copy link
Owner

burningalchemist commented Oct 5, 2023

Hey @tonobo, would you mind creating a separate discussion in Discussions? I would be curious to learn more about the idea, etc. 👍 It's just out of topic for this particular PR, still quite interesting. 🙂

@burningalchemist burningalchemist added this to the v0.13 milestone Oct 5, 2023
Nikolai Rodionov added 2 commits October 6, 2023 14:39
Currently, Helm is the only package manager for Kubernetes, and it's
very widely used across many Kubernetes users. So I've thought that it
could make sense to start distributing sql_exporter with helm
This action will lint the chart on every change. If chart was updated,
but its version was not, the action will be failed, that should make
the delivery more smooth.

If commit was pushed to the main branch, the action will release a new
chart version. It will create a new GitHub release and push to the
gh-pages branch. This branch has to be configured as the one for serving
GitHub Pages. Then it will start action as a Helm repository.
helm/values.yaml Outdated Show resolved Hide resolved
helm/values.yaml Outdated Show resolved Hide resolved
helm/values.yaml Outdated Show resolved Hide resolved
@burningalchemist burningalchemist changed the title Add a helm chart with the releaser action feat: add a helm chart with the releaser action Oct 7, 2023
helm/values.yaml Outdated Show resolved Hide resolved
WIP: Switch to helm tests instead of the integration script test
@burningalchemist
Copy link
Owner

@allanger Thanks again for your contribution! I guess we're good to go. 👍🚀

@burningalchemist burningalchemist self-requested a review October 8, 2023 18:47
@allanger
Copy link
Contributor Author

allanger commented Oct 9, 2023

If this PR is scheduled for a release with the new version, once the new version is released and the chart is merged, we could update fields version and appVersion in the chart.yaml then the chart should be released as well and become available from gh-pages branch.

@burningalchemist
Copy link
Owner

I've just noticed that the workflow is set to run on every push event. We might need to update the condition there to tags and/or workflow_dispatch:

on:
  workflow_dispatch:
  push:
    tags:
    - '*.*.*'

Could you please make this change?

@allanger
Copy link
Contributor Author

allanger commented Oct 9, 2023

It's triggered on every push because of the design. Chart doesn't have the same workflow as the app. Tools that are used for linting, testing, and releasing are detecting changes comparing to the master branch. So if a change to the chart is required, you don't need to push tags, but make a commit with an updated chart version to the master

@burningalchemist
Copy link
Owner

Ok, sounds good to me. 👍

@burningalchemist burningalchemist merged commit eaccece into burningalchemist:master Oct 9, 2023
3 checks passed
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.

3 participants