dragonfly-operator
dragonfly-operator copied to clipboard
dragonflydb
feat/helmChart: initial support for installing the operator using helm charts
Hey there 👋
This PR adds initial support for helm chart https://github.com/dragonflydb/dragonfly-operator/issues/168
Thanks for tackling this @nujragan93 I don't see any workflow changes to publish the Helm chart (e.g. what is done in https://github.com/dragonflydb/dragonfly/blob/main/.github/workflows/docker-release.yml)
Thanks for tackling this @nujragan93 I don't see any workflow changes to publish the Helm chart (e.g. what is done in https://github.com/dragonflydb/dragonfly/blob/main/.github/workflows/docker-release.yml)
that seems like a OCI repo for dragonflyDb, does the operator need to have a separate repo ? do we need to add tests to the CI before pushing ?
that seems like a OCI repo for dragonflyDb, does the operator need to have a separate repo ?
They seem to be packaging up the Helm chart into a OCI artifact which could be done here as well and published under https://github.com/orgs/dragonflydb/packages?repo_name=dragonfly-operator
do we need to add tests to the CI before pushing ?
It looks like they have some CI tests in https://github.com/dragonflydb/dragonfly/tree/main/contrib/charts/dragonfly/ci so probably might be a good idea to have some here as well?
that seems like a OCI repo for dragonflyDb, does the operator need to have a separate repo ?
They seem to be packaging up the Helm chart into a OCI artifact which could be done here as well and published under https://github.com/orgs/dragonflydb/packages?repo_name=dragonfly-operator
do we need to add tests to the CI before pushing ?
It looks like they have some CI tests in https://github.com/dragonflydb/dragonfly/tree/main/contrib/charts/dragonfly/ci so probably might be a good idea to have some here as well?
@Abhra303 @Pothulapati what do you suggest ?
Hey @nujragan93, Thanks for working on this.
+1 to both of these questions. We can have similar CI linting, generation tests like Dragonfly. Feel free to add a simpler verison of that.
Yep, We should be using Github Packages to store and publish the Helm Chart.
(Reviewing the Helm Chart itself meanwhile)
Hey @nujragan93, Thanks for working on this.
+1 to both of these questions. We can have similar CI linting, generation tests like Dragonfly. Feel free to add a simpler verison of that.
Yep, We should be using Github Packages to store and publish the Helm Chart.
(Reviewing the Helm Chart itself meanwhile)
@Pothulapati I have made some workflow changes to push helm chart to github(dont know how to test it) This PR seems already very big, I feel the ci linting and tests should be made a separate PR.
@nujragan93 pipeline looks good. FWIW I agree we should separate the PRs for adding ci linting and tests after this is merged.
@Pothulapati any update here? I tried to cover most the review and it looks good from here.
@onedr0p Feel free to create a new branch from this branch and add your commits with the changes that I requested i.e test the chart. I'm happy to follow up with a review.
@Pothulapati you are a maintainer of this repo which gives you the ability to edit this PR with those said changes. If that's not something you want to do I'll go ahead and create a branch off this and open a PR with your requested changes.
@Pothulapati you are a maintainer of this repo which gives you the ability to edit this PR with those said changes.
Yes, but that does not mean I have enough bandwidth to work on everything :/
If that's not something you want to do I'll go ahead and create a branch off this and open a PR with your requested changes.
If you are interested, and want to help, Feel free!
@Pothulapati After looking this over... there's quite a bit of CI/GitHub workflow changes that need to be done to support what you ask because the workflow you have in this repo is only for releases / pushing to main. There is no testing workflow to expand upon like here https://github.com/dragonflydb/dragonfly/tree/main/.github/workflows
Why is it not possible to merge this PR and iterate on the changes in future updates? As @nujragan93 already mentioned this PR is pretty big already.
@Pothulapati After looking this over... there's quite a bit of CI/GitHub workflow changes that need to be done to support what you ask because the workflow you have in this repo is only for releases / pushing to main. There is no testing workflow to expand upon like here https://github.com/dragonflydb/dragonfly/tree/main/.github/workflows
Why is it not possible to merge this PR and iterate on the changes in future updates? As @nujragan93 already mentioned this PR is pretty big already.
I agree, if the workflows are setup, then I would jump in and iterate on tests with the helm charts.
Sorry folks but this is merged now! Looking forward to the next iterations!
Does this need a new operator release first? Seems like the package is not yet available in the ghcr
Just FYI, we had to remove the release changes (for now) as it wasn't working https://github.com/dragonflydb/dragonfly-operator/pull/201
I have a feeling something as simple as git checkout -b as appose to just git checkout may fix that issue.
Or something along those lines.
@Pothulapati do you need the commit back into the main branch? Assuming it's failing there, I think this might be fixed in 2 ways
Keep the chart and app separate, and have another workflow for helm, this also means having separate versions for the chart which might increase the complexity a bit as there would be 2 different artifacts that need to be maintained and released as part of the same repo with different versions.
Or keep it with matching versions and consider helm as part of the app. Any changes to helm are released with a new version of the app as well (common helm headache) and keep the workflow with sed for version change, login, package and push. For local install (from the cloned repo) there could be a note in the readme stating that the chart and app version need to be updated.
If Helm gets out of hand (which it might), this can be separated into its own repo where you only have the chart.
The fix is easy. Simply addfetch-depth: 0 to actions/checkout@v3 step and git push in the step where you update Chart.yaml file with the tag version.
But the concern here is that the Helm Chart version and Operator version are different, the former is at v1.x.x > and the former at v0.1.x. Not the same, I'd suggest working on improving the version management of both components. I'm opening another issue for this initiative.
