Added ability to configure Helm chart repository accessible within cluster

[pedjak]

• Introduced HelmChartRepository top-level CR
• The corresponding enhancement proposal

[pedjak]

The default chart repository is https://redhat-developer.github.io/redhat-helm-charts

The following usecases require ability to configure the chart repo url:

• disconnected installs
• users wants to use their own repos

Motivation: openshift/enhancements#175
/cc @openshift/api-reviewers @benjaminapetersen

[soltysh]

I miss enhancement doc describing the purpose of this.

[pedjak]

@soltysh the motivation is openshift/enhancements#175

[soltysh]

@soltysh the motivation is openshift/enhancements#175

Enhancement freeze was last Friday since this is not merged the API can't merge either. Also look at my other comments.
/hold

[deads2k]

This approach appears to be based on a design that is console specific. I would expect the operator type itself to be extended (here probably: design appears to be console specific. You could start by moving here https://github.com/openshift/api/blob/master/operator/v1/types_console.go) instead of adjusting a top level configuration item. This seems to be a detail present today for extension that we may want an option to change in the future.

Doesn't OLM have a comparable configuration option? Can OLM look at helm?

The other thing that comes to mind is that this has value for an OLM managed console, which I think we want to someday have. Instead of payload only.

[benjaminapetersen]

All good points. It seems the final place for this config is not entirely known. This field on console config/operator may be later deprecated if helm was instead managed by OLM.

[sbose78]

Hi!

Enhancement freeze was last Friday since this is not merged the API can't merge either.

cc @derekwaynecarr @benjaminapetersen @spadgett

Significant parts of the enhancement are already in Console today. This bit effectively allows customers/partners/admins to specify custom/disconnected helm repositories in OpenShift. This is an additional feature as part of a larger set of features which are already in Console, which we wish to get in well before feature freeze so that Console UI changes could be put in before feature freeze.

We've presented the enhancement to Console stakeholders in early January when we first started designing and developing the "Helm in Console" support. I understand there's an enhancement lifecycle which hasn't been taken care of, yet :) We've tried to do our best to socialize the changes as much as possible.

[sbose78]

Hi @deads2k !

This field on console config/operator may be later deprecated if helm was instead managed by OLM.

When Console is managed by OLM, we would still have a Console CRD, right? I would expect these attributes to be present in the Console API there as well, to begin with.

Re: Having a separate resource to manage Helm

This is a really good discussion to have!
I'm not aware how always-on components would be lifecycle'd/designed when the management of such always-on components are done by OLM. I could definitely contribute a design if needed.

For the time being, I would envision the entire thing ( console and helm config ) to move as one thing to OLM. After that, if it makes sense, we can manage Helm components using a different resource.

Doesn't OLM have a comparable configuration option? Can OLM look at helm?

David, could you please elaborate on this, please? I didn't quite get you.

[pedjak]

@deads2k @soltysh - copied here the comment made on the enchancement #175, hoping that the motivation is much clearer. Looking forward to your suggestions.

Configuring Helm repository location can be modelled similar to OperatorSource. The resource needs to contain the following field:

url: http://my.chart-repo.org/stable

# optional and only needed for UI purposes
displayName: myChartRepo

# optional and only needed for UI purposes
description: my private chart repo

# set to true if need to be disabled temporarly
disabled: true

The resource could be standalone, a part of Console config, or a part of Console operator config

• If standalone, its could be named as HelmChartRepository and be placed in (cluster-wide config)[https://github.com/openshift/api/tree/master/config/v1] or console operator namespace. The console operator would watch for changes on instances and reconfigure the console deployment.
• If a part of existing resource, it should be possible to specify a list of chart repositories, so that future use cases (e.g. ODC-2994) could be implemented.

The console backend already implements a few helm endpoints (including the chart proxy), but the future might require to extract them into a separate service (e.g. to make openshift more modular). Hence, it would be good to define the configuration as the standalone resource (possibly cluster-wide or in a dedicated namespace, e.g. openshift-helm) detaching API from the implementation.

[sbose78]

@pedjak Nice summary!

[sbose78]

Minutes of the meeting

with @derekwaynecarr @spadgett @benjaminapetersen

• We'll be adding a cluster-scoped HelmRepository CRD, to be reconciled by the console-operator.
• In future, if a namespace-scoped HelmRepository CRD is needed, we'll consider it, based on use-cases.
• In future, if a cluster-scoped HelmConfig CRD is needed, we'll consider it, based on use-cases.

Action items:

• @pedjak to modify this PR to only have the HelmRepository API.

The Helm repository could be configured by going to the Cluster Settings -> Global Configuration page:

**

**

• In the upcoming releases, we'll support multiple Helm Repositories in Console with multiple HelmRepository CRs ( also, https://issues.redhat.com/browse/ODC-2994 )

[pedjak]

I have updated the PR, please rereview it.

[sbose78]

Looks okay to me @pedjak . @benjaminapetersen @spadgett Could you please review?

[benjaminapetersen]

I don't know if this has been clarified, if there is a problem with the helm config, should the console-operator go degraded? I would assume so since it will be the operator managing this/these resources (these, as I understand there may be several instances).

If there are several helm configs provided:

• helmA - fine
• helmB - fine
• helmC - bad for whatever reason

Then console-operator should go degraded, with some kind of HelmSyncErr reason, but it should also write the error on the offending helmC.Status or hemC.Status.Conditions[].

[deads2k]

I forgot the validation. lgtm otherwise, so I'll approve after validation. You should get someone else on the team to review and lgtm.

[akashshinde]

/lgtm
fyi: this API is being consumed in PR openshift/console#5933

[sbose78]

/lgtm

[deads2k]

/lgtm

[deads2k]

weird. I wonder if someone changed the bot

/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akashshinde, deads2k, pedjak, sbose78

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [deads2k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sbose78]

Is the hold by @soltysh blocking the merge?
Let me try..

/hold remove

[sbose78]

/hold cancel

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.