Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Add support for multi-path to the Helm Operator #1348

Closed
jasaltvik opened this issue Sep 7, 2018 · 12 comments
Closed

Add support for multi-path to the Helm Operator #1348

jasaltvik opened this issue Sep 7, 2018 · 12 comments

Comments

@jasaltvik
Copy link

Now that you have multi-path support for Flux through --git-path, it would be great to have the same for the Helm Operator through the helmOperator.git.chartsPath argument so that you could have Helm Operator 1 looking at something like charts/common and charts/cluster1, and Helm Operator 2 looking at something like charts/common and charts/cluster2.

@squaremo
Copy link
Member

Technically there's no reason to provide a path to the helm operator at all -- each FluxHelmRelease should just give the full path from the top of the repo.

@squaremo
Copy link
Member

Having said that, I am curious about the use case suggested in your example. Do you want to reuse the same FluxHelmRelease resources in different places, with the helm operator pointing at distinct directories containing charts that have the same name but otherwise differ?

@stephenmoloney
Copy link
Contributor

I'm interested in this too for reasons similar to @kozejonaz

@squaremo

Can you give an example of this

Technically there's no reason to provide a path to the helm operator at all -- each FluxHelmRelease should just give the full path from the top of the repo.

Not sure what the full path from the top of the repo. means exactly

@squaremo
Copy link
Member

I mean there's no especial reason to supply --git-chart-dir to the helm operator, then a path relative to that in every FluxHelmRelease. You may as well just supply the full path in each FluxHelmRelease.

There is the one reason -- and now I read through again, I see that's what @kozejonaz what's getting at. It does seem rather counter to the point of charts, which is to have a single template to which you refer in many places. I can see how referring to a chart in a git repo means you can only run one version of it, and perhaps that's where the motivation arises for having charts in more than one place.

@jasaltvik
Copy link
Author

@squaremo Sorry for the late reply! Before I created this I was under the impression that the Helm operator would need to be able to read several chart paths to be able to use several Helm operators against the same branch. After some discussion in Slack (start at https://weave-community.slack.com/archives/C4U5ATZ9S/p1536325875000100 and read on) @geoah pointed out that I could achieve what I wanted by having one location for Helm charts. This is achieved by pointing all Helm operators to the single charts path using helmOperator.git.chartsPath (e.g. flux/charts), and then using git.path in each Helm operator to point to the namespaces and FluxHelmReleases that should be controlled by that specific Helm operator (e.g. flux/helmop1 and flux/helmop2). This has worked well for me.

I can't think of many good reasons to do this anymore, except for consolidating how the daemon and the Helm operator uses the paths maybe? If you wanted to use several versions of a chart, I guess you could add both versions as separate charts as well. You could sub-chart them to avoid too much copy+paste. But maybe this is a potential argument for doing this.

I should have been a bit more descriptive when I logged this.

@squaremo
Copy link
Member

@kozejonaz Thanks for this update, it is really helpful.
@stephenmoloney, are you trying to do something similar, and would the advice above help you too?

@stephenmoloney
Copy link
Contributor

As some background info, I am experimenting with the structure of the charts folder. Rather than drop all the charts in a flat structure in the ./charts dir, I want subdirectories for organisational reasons.

At the moment, I'm trying to move from a flat charts structure to one with nomenclature which will make it easier to know what's what... It's a WIP.

Before:

charts
├── app
├── cert-issuer
├── cert-manager
├── contour
├── flux
├── kubernetes-dashboard
├── metrics-server
├── openebs
├── postgresql
├── sealed-secrets
└── weave-scope

After:

./addons/charts/
├── kube-system-addons
│   ├── cert-issuer
│   ├── cert-manager
│   ├── contour
│   ├── flux
│   ├── kubernetes-dashboard
│   ├── metrics-server
│   ├── openebs
│   └── sealed-secrets
└── microservices
    ├── app
    └── postgresql

@squaremo

This is achieved by pointing all Helm operators to the single charts path using helmOperator.git.chartsPath (e.g. flux/charts), and then using git.path in each Helm operator to point to the namespaces and FluxHelmReleases that should be controlled by that specific Helm operator (e.g. flux/helmop1 and flux/helmop2). This has worked well for me.

This pretty much answers my original question about the full path from the top of the repo, thanks @kozejonaz

So for the same reason then, I do not need this feature either, I can specify it in the CRD.

So to give an example in code, I am presuming the following is the correct way to achieve what I want to try:

    helm install \
    --name flux \
    --namespace kube-system-addons \
    --set rbac.create=true \
    --set helmOperator.create=true \
    --set helmOperator.git.chartsPath=addons/charts \
    weaveworks/flux 
apiVersion: helm.integrations.flux.weave.works/v1alpha2
kind: FluxHelmRelease
metadata:
  name: kubernetes-dashboard
  namespace: kube-system-addons
  labels:
    chart: kubernetes-dashboard
spec:
  chartGitPath: kube-system-addons/kubernetes-dashboard
  releaseName: kubernetes-dashboard

So changing chartGitPath: kubernetes-dashboard to chartGitPath: kube-system-addons/kubernetes-dashboard is what I need to do, right ?

@jasaltvik
Copy link
Author

@stephenmoloney That is exactly right :)

@squaremo
Copy link
Member

squaremo commented Sep 27, 2018

@kozejonaz @stephenmoloney It sounds like both of you can get by without this feature -- shall we close it?

@jasaltvik
Copy link
Author

@squaremo That is perfectly fine by me.

@stephenmoloney
Copy link
Contributor

@squaremo yes, can close this. I have a working solution as per #1348 (comment)

@squaremo
Copy link
Member

squaremo commented Oct 8, 2018

Thanks both!

@squaremo squaremo closed this as completed Oct 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants