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 Helm as a bundle MediaType #25

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

Conversation

Bowenislandsong
Copy link
Member

This enhancement extend OLM's ability to accept helm charts as bundle mediatype where raw helm charts can be consumed by OLM.

Copy link
Member

@njhale njhale 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 writing this up. Some initial comments.

## Open Questions [optional]

- How to reflect helm's [chart dependencies](https://helm.sh/docs/topics/charts/#chart-dependencies) either `from a registry` or `provided`?
- Is it OLM's responsibility to deploy provided dependent charts?
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Is it OLM's responsibility to deploy provided dependent charts?
- Is it OLM's responsibility to deploy chart dependencies?


- How to reflect helm's [chart dependencies](https://helm.sh/docs/topics/charts/#chart-dependencies) either `from a registry` or `provided`?
- Is it OLM's responsibility to deploy provided dependent charts?
- Charts deprecation and where to reflect that in the bundle? [Deprecate Charts]() [Deprecate APIs](https://helm.sh/docs/topics/kubernetes_apis/)
Copy link
Member

Choose a reason for hiding this comment

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

We first need to ask whether bundles support this feature.

- How to reflect helm's [chart dependencies](https://helm.sh/docs/topics/charts/#chart-dependencies) either `from a registry` or `provided`?
- Is it OLM's responsibility to deploy provided dependent charts?
- Charts deprecation and where to reflect that in the bundle? [Deprecate Charts]() [Deprecate APIs](https://helm.sh/docs/topics/kubernetes_apis/)
- CRDs from helm charts are all provided with Helm 3 with the deprecation of crd-install hooks, we should not need to support versions before Helm 3.
Copy link
Member

Choose a reason for hiding this comment

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

this is a statement

- Is it OLM's responsibility to deploy provided dependent charts?
- Charts deprecation and where to reflect that in the bundle? [Deprecate Charts]() [Deprecate APIs](https://helm.sh/docs/topics/kubernetes_apis/)
- CRDs from helm charts are all provided with Helm 3 with the deprecation of crd-install hooks, we should not need to support versions before Helm 3.
- Not all Helm capabilities are supported. Creating methods of vetting if a helm chart is usable for OLM. Is there anything else to consider other than the provided kinds of Kube manifests and the existence of required CSV information?
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Not all Helm capabilities are supported. Creating methods of vetting if a helm chart is usable for OLM. Is there anything else to consider other than the provided kinds of Kube manifests and the existence of required CSV information?
- What Helm chart features will be supported by bundles?

- Charts deprecation and where to reflect that in the bundle? [Deprecate Charts]() [Deprecate APIs](https://helm.sh/docs/topics/kubernetes_apis/)
- CRDs from helm charts are all provided with Helm 3 with the deprecation of crd-install hooks, we should not need to support versions before Helm 3.
- Not all Helm capabilities are supported. Creating methods of vetting if a helm chart is usable for OLM. Is there anything else to consider other than the provided kinds of Kube manifests and the existence of required CSV information?
- How to avoid OLM/Registry from previous version to install/add Helm charts so that no error should be thrown rather not allowed or able to do so? UX experience.
Copy link
Member

Choose a reason for hiding this comment

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

grammar


## Summary

This enhancement adds support to accept `helm chart` media type for [Operator Bundle spec](https://github.com/openshift/enhancements/blob/master/enhancements/olm/operator-bundle.md), [operator-registry](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go), and [OLM](https://github.com/operator-framework/operator-lifecycle-manager) and enables deploying helm-based operators.
Copy link
Member

Choose a reason for hiding this comment

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

Is there a schema/convention for media types? i.e. helm+v3 over helm chart.


## Summary

This enhancement adds support to accept `helm chart` media type for [Operator Bundle spec](https://github.com/openshift/enhancements/blob/master/enhancements/olm/operator-bundle.md), [operator-registry](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go), and [OLM](https://github.com/operator-framework/operator-lifecycle-manager) and enables deploying helm-based operators.
Copy link
Member

Choose a reason for hiding this comment

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

This enhancement should cover the design/UX for for opm, and potentially pipelines, too.


## Summary

This enhancement adds support to accept `helm chart` media type for [Operator Bundle spec](https://github.com/openshift/enhancements/blob/master/enhancements/olm/operator-bundle.md), [operator-registry](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go), and [OLM](https://github.com/operator-framework/operator-lifecycle-manager) and enables deploying helm-based operators.
Copy link
Member

Choose a reason for hiding this comment

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

and enables deploying helm-based operators.

and enables deployment/upgrading helm-based operators.

Helm 3


### User Stories by Persona
Copy link
Member

Choose a reason for hiding this comment

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

We're missing the Operator Author.


Not all Helm charts with their provided resources can be supported by OLM. The list of supported Kube manifests kinds is available [here](https://github.com/operator-framework/operator-registry/blob/7437af6d40bd9992ddede1aaee6223677b3b896f/pkg/lib/bundle/supported_resources.go#L23) which OLM will be in charge of creating them.

### UX Experience
Copy link
Member

Choose a reason for hiding this comment

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

The UX for opm should be a priority. We should write it up README-style.


## Summary

This enhancement adds support to accept `helm chart` media type for [Operator Bundle spec](https://github.com/openshift/enhancements/blob/master/enhancements/olm/operator-bundle.md), [operator-registry](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go), and [OLM](https://github.com/operator-framework/operator-lifecycle-manager) and enables deployment/upgrading helm-based operators. This enhancement covers the design/UX for opm and OLM, and pipeline for building, adding, and installing helm-based operators that follow the [conventions](https://helm.sh/docs/chart_best_practices/conventions/) of `Helm+v3`.
Copy link
Member

Choose a reason for hiding this comment

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

the operator-registry link is a link to the CSV type definition

This enhancement adds support to accept `helm chart` media type for [Operator Bundle spec](https://github.com/openshift/enhancements/blob/master/enhancements/olm/operator-bundle.md), [operator-registry](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go), and [OLM](https://github.com/operator-framework/operator-lifecycle-manager) and enables deployment/upgrading helm-based operators. This enhancement covers the design/UX for opm and OLM, and pipeline for building, adding, and installing helm-based operators that follow the [conventions](https://helm.sh/docs/chart_best_practices/conventions/) of `Helm+v3`.

## Motivation
Operator authors and cluster admins may find themselves in the split between choosing to support/install operators via OLM or Helm. Helm users hoping to have OLM's operator management experience would have to convert their existing chart into OLM bundle. As many similarities as there are between the two projects, supporting/installing through either mechanism expects different packaging methods and pipeline. By supporting [Helm chart](https://hub.helm.sh/) as a bundle media type, OLM would become approachable to the Helm community without requiring operator authors to explicitly support OLM. It also gives cluster admins the ability to bring Helm-based operators on clusters using OLM.
Copy link
Member

Choose a reason for hiding this comment

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

Have we thought about what helm users lose from converting their bundle to an OLM bundle? Helm has a concept of revisions, which allow you to track what was deployed to the cluster and when. By converting to an OLM bundle, we do enable manifests to get on the cluster, but lose other helm related metadata. Have we thought about importing helm libraries and providing a more first-class experience for existing helm users?


- Support, update, and lifecycle existing operators installed via Helm.
- Building a bundle from helm charts without putting any restrictions on the contents of [templates](https://helm.sh/docs/chart_best_practices/templates/#helm) directory and metadata.
- Support/Replace all capabilities of Helm in deploying Helm charts.
Copy link
Member

Choose a reason for hiding this comment

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

This seems reasonable, but should a goal be to explicitly list what we are/aren't going to support as a result of this?


Special mappings from [Chart.yaml](https://helm.sh/docs/topics/charts/#the-chartyaml-file) -> `olm.yaml`:
```
kubeVersion -> minKubeVersion # KubeVersion describes a range of compatible Kube versions which olm takes the minimum value
Copy link
Member

Choose a reason for hiding this comment

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

OLM doesn't currently do anything with minKubeVersion, no? Would we actively support it as part of this conversion?

This enhancement extend OLM's ability to accept helm charts as bundle mediatype where raw helm charts can be consumed by OLM.

## Summary

This enhancement adds support to accept `helm chart` media type for [Operator Bundle spec](https://github.com/openshift/enhancements/blob/master/enhancements/olm/operator-bundle.md), [operator-registry](https://github.com/operator-framework/operator-registry), and [OLM](https://github.com/operator-framework/operator-lifecycle-manager) to enable deployment/upgrading helm-based operators. This enhancement covers the design/UX for opm and OLM, and pipeline for building, adding, and installing helm-based operators that follow the [conventions](https://helm.sh/docs/chart_best_practices/conventions/) of `Helm+v3`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Operator-SDK allows users to create operators based on the helm-charts and it provides the features for them to work with as build the bundle. So, why it would be required?

Copy link
Member

Choose a reason for hiding this comment

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

That requires the helm chart developer to convert their chart into an operator bundle. IMO this would be more valuable if there was a way for OLM to consume helm charts directly rather than have a manual conversion take place before they could be consumed (which adding a bundle type as is described above doesn't accomplish)

Copy link
Contributor

@camilamacedo86 camilamacedo86 Sep 1, 2020

Choose a reason for hiding this comment

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

IMO it is not part of the domain of responsibility of OLM. OLM has the goal to distribute operators projects which can be done via the bundle. Also, will OLM generate the CRD's? will OLM generate a controller using helm native implementation to manage the charts?

c/c @njhale

Copy link
Member

Choose a reason for hiding this comment

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

That's true today, but that's just because OLM has currently limited itself to deploying and installing operators. It seems reasonable for OLM to potentially branch out to other types of applications on a cluster outside the scope of the strict definition of operators defined by the operator framework's bundle format.

It may be that in the case of helm charts we can't easily define a UX that enables the templating system that helm defines. But at a high level one of the killer features that I see OLM provides is it's dependency resolution. Why should the dependency resolver be limited just to other bundles if there was a convenient entryway for non-operators (but still applications or kube objects) to be applied in addition to operators in the strictest sense of the world. We already allow users to add certain additional kube types to their bundles in OLM, why do those kube types need be included in the bundle as a yaml object itself?

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.

6 participants