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

[PROPOSAL] Seperate the operator helm chart release with the actual operator release #830

Closed
prudhvigodithi opened this issue May 29, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request release

Comments

@prudhvigodithi
Copy link
Member

What/Why

What are you proposing?

Have have a seperate release cadence for operator helm chart. Example here is an open PR for a bug fix in helm chart #826, even though this is reviewed and merged, this wont be released until the next version of the operator release.

What problems are you trying to solve?

Currently, the release process for the operator helm charts is tightly coupled with the release of the operator, decouple the release cycles of helm charts and operator to allow independent and fast paced updates. Have a seperate version management to the helm charts.

What is the user and developer experience going to be?

The developers and users will have more control over the helm charts development and releases, having this a user/developer need not wait until the release of the operator to have an helm chart release. This improves and gives more control in fixing the bugs and improving the helm charts periodically.

Are there any security considerations?

No

Are there any breaking changes to the API

No

Are there breaking changes to the User Experience?

No

Why should it be built? Any reason not to?

The operator is mostly installed/used by the hem charts, having to implement this will have the following advantages
Faster Release Cycle: Helm chart updates can be released as soon as they are ready, without waiting for the next operator release cycle.
Increased Flexibility: Helm charts can be patched and improved independently, allowing quicker responses to bugs and user feedback.
Simplified Versioning: Separate versioning schemes for Helm charts and operators will make it easier to manage and ensure compatibility.

What will it take to execute?

There is already an open PR #754 contributed by @evheniyt review and merge this. Seperate the release process in https://github.com/opensearch-project/opensearch-k8s-operator/blob/main/.github/workflows/release.yaml, define the versioning and release process of helm charts. We can re-use some of the release workflows from the OpenSearch helm repo https://github.com/opensearch-project/helm-charts/blob/main/.github/workflows/release.yaml.
The helm release should be from the version in Chart.yaml,

  • Decouple the release workflow with helm chart release.
  • A developer along with the PR with the chart changes, have to increment the version in Chart.yaml (follow the SemVer)
  • Update and maintain the CHANGELOG.md.
  • The workflow should identify the the new version in Chart.yaml and do a helm release.

Note: Even though the logic to seperate the operator helm chart release with the actual operator release is needed, there should be an automation and mechanism to have a operator helm chart release soon after the the main operator release, this is to ensure the CRD's and other new files are synced that are part of the operator release.

Any remaining open questions?

N/A

@github-actions github-actions bot added the untriaged Issues that have not yet been triaged label May 29, 2024
@prudhvigodithi prudhvigodithi added enhancement New feature or request untriaged Issues that have not yet been triaged and removed untriaged Issues that have not yet been triaged labels May 29, 2024
@prudhvigodithi
Copy link
Member Author

Adding @dbason @swoehrl-mw @jochenkressin @pchmielnik @salyh @bbarani @getsaurabh02 @evheniyt to please take a look and share your thoughts.

@evheniyt
Copy link
Contributor

I think it's pretty clear that the release process of operator, and helm charts (both operator and cluster) should be separated. I think the main topic to discuss is versioning.

In operator-helm we have version and appVersion - nothing specific here, when helm is updated version field is bumped, when operator is updated - appVersion is set to the same tag as operator. Nothing specific here.

In cluster-helm for appVersion we could use either operator version or the version of Opensearch. If we use operator version, then we need to have a separate Opensearch version in values.yaml so the release process of operator should also bump appVersion for cluster-helm and Opensearch version in values.yaml (I think the second could be done manually too during release preparation).
If we use Opensearch version in appVersion then we need don't need to bump it during release pipeline (could be done manually if needed).

Another question is how to track/match operator-helm and cluster-helm version so we could know which version of operator-helm is compatible with cluster-helm. Maybe the easiest way is to keep the major and minor versions of these 2 charts the same and bump them only if there are changes like new CRD added, etc.

@swoehrl-mw
Copy link
Collaborator

One thing to keep in mind here are CRD changes in the operator chart. In the current process updates to the CRD yamls are added to the chart in the PR the changes are introduced. But if the operator chart release process is separate this can lead to problems if the chart is released with a changed CRD that the operator image does not yet support.

My opinion here is: Keep version and release process for the operator and operator helm chart in sync as is now, but decouple the release and version of the opensearch-cluster helm chart to allow faster pace there.

In cluster-helm for appVersion we could use either operator version or the version of Opensearch. If we use operator version, then we need to have a separate Opensearch version in values.yaml so the release process of operator should also bump appVersion for cluster-helm and Opensearch version in values.yaml (I think the second could be done manually too during release preparation).
If we use Opensearch version in appVersion then we need don't need to bump it during release pipeline (could be done manually if needed).

I like the first option: use the operator version as appVersion in the cluster chart to signal with which operator version it is compatible. We should keep the opensearch version out of there as both operator and chart support multiple and also older opensearch versions.

@prudhvigodithi prudhvigodithi removed the untriaged Issues that have not yet been triaged label Jun 20, 2024
@getsaurabh02 getsaurabh02 moved this from 🆕 New to Backlog in Engineering Effectiveness Board Jul 18, 2024
@prudhvigodithi prudhvigodithi moved this from Backlog to 🏗 In progress in Engineering Effectiveness Board Sep 17, 2024
prudhvigodithi added a commit that referenced this issue Nov 5, 2024
### Description
Decouple Helm Release with the Operator Release.

- Update the `release.yaml` to exclude the helm release.
- Onboard new helm release workflow `helm-release.yaml`.
- The `helm-release.yaml` use the
[helm/chart-releaser-action](https://github.com/helm/chart-releaser-action)
which identifies the chart version and releases incrementally.
- The `helm-release.yaml` runs once the PR is merged to `main` and
releases only once there is a change with chart `version`, inside the
[charts/
](https://github.com/opensearch-project/opensearch-k8s-operator/tree/main/charts)folder.

The expectation from the user is to create a PR with with updates
changes, along with modifying the `version` of the chart, once merged
the `helm/chart-releaser-action` will identify the new chart version and
does a helm release.

Whenever there is an operator version release, the `appVersion` along
with `version` has to be updated and thereafter the process is same
`helm/chart-releaser-action` will identify the new chart version and
does a helm release.


### Issues Resolved
Coming from
#830

### Check List
- [x] Commits are signed per the DCO using --signoff 
- [ ] Unittest added for the new/changed functionality and all unit
tests are successful
- [ ] Customer-visible features documented
- [ ] No linter warnings (`make lint`)

If CRDs are changed:
- [ ] CRD YAMLs updated (`make manifests`) and also copied into the helm
chart
- [ ] Changes to CRDs documented

Please refer to the [PR
guidelines](https://github.com/opensearch-project/opensearch-k8s-operator/blob/main/docs/developing.md#submitting-a-pr)
before submitting this pull request.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

Signed-off-by: Prudhvi Godithi <[email protected]>
prudhvigodithi added a commit that referenced this issue Nov 5, 2024
### Description
Coming from initial PR
#872.

Fix the error
https://github.com/opensearch-project/opensearch-k8s-operator/actions/runs/11685891300/job/32540910915
```
Looking up latest tag...
Discovering changed charts since 'v2.6.0'...
WARNING: charts/opensearch-cluster/templates/Chart.yaml is missing, assuming that 'charts/opensearch-cluster/templates' is not a Helm chart. Skipping.
Nothing to do. No chart changes detected.
```

Since moving from `stefanprodan/helm-gh-pages@master` to
`helm/[email protected]` added `fetch-depth` and
`skip_existing`. The `skip_existing` should not throw an error if the
chart with same `version` is already added to the `gh-pages` branch
`index.yaml` file.

#### Here are some tests done on my fork 
##### Release Helm Charts workflow

https://github.com/prudhvigodithi/opensearch-k8s-operator/actions/workflows/helm-release.yaml

##### Publish Release from tag workflow 

https://github.com/prudhvigodithi/opensearch-k8s-operator/actions/runs/11687219955




### Issues Resolved
Coming from
#830

### Check List
- [ ] Commits are signed per the DCO using --signoff 
- [ ] Unittest added for the new/changed functionality and all unit
tests are successful
- [ ] Customer-visible features documented
- [ ] No linter warnings (`make lint`)

If CRDs are changed:
- [ ] CRD YAMLs updated (`make manifests`) and also copied into the helm
chart
- [ ] Changes to CRDs documented

Please refer to the [PR
guidelines](https://github.com/opensearch-project/opensearch-k8s-operator/blob/main/docs/developing.md#submitting-a-pr)
before submitting this pull request.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

Signed-off-by: Prudhvi Godithi <[email protected]>
@prudhvigodithi
Copy link
Member Author

Works as expected https://github.com/opensearch-project/opensearch-k8s-operator/actions/runs/11780417815/job/32810969136
Screenshot 2024-11-11 at 11 34 13 AM
@evheniyt we should be able to move forward with this PR #754, once the changes are finalized incrementing the chart version should do a new helm chart release.
Thank you
@getsaurabh02 @peterzhuamazon @swoehrl-mw

@prudhvigodithi prudhvigodithi self-assigned this Nov 11, 2024
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Engineering Effectiveness Board Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request release
Projects
Status: ✅ Done
Development

No branches or pull requests

3 participants