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 'export-values' to requirements #3242

Closed
withnale opened this issue Dec 9, 2017 · 18 comments
Closed

Add 'export-values' to requirements #3242

withnale opened this issue Dec 9, 2017 · 18 comments

Comments

@withnale
Copy link

withnale commented Dec 9, 2017

There is a need for an export-values method when working with requirements.

At present there is already a mechanism for pulling values from subcharts into the parent chart using the 'import-values' method defined in docs/chart.md. However, there is no equivalent method for encapsulating parts of the values tree when wishing to provide specific trees to subcharts.

We use parent charts often with many subcharts, but often these subcharts are used by many parent charts (M:N). In this scenario there is often a set of values we wish to pass to various sets of charts.

The methods we have right now allow:

  1. We can put the value explicitly in the subcharts tree i.e. subchart1.myvalue - however, this doesn't scale too well, since we want to define this value once in the parent chart but if multiple subcharts require this value, we need to explicitly set for each subchart in the parents values. i.e. subchart1.myvalue, subchart2.myvalue, etc. When we spin up a release, we want to specify the minimal set of value overrides (using -f overrides.yaml) as possible and not have to specify this for each subchart - so this isn't really viable.
  2. Alternatively, we could use the globals namespace within the values tree. However, this isn't ideal either, since this tree is duplicated in all subcharts the values file quickly gets massive, and there are benefits in having the trees in use by each chart encapsulated specifically in the requirements. Managing a global namespace isn't ideal and can easily generate unforeseen side effects.

The attached PR approaches this by mimicking the import-values functionality but in the other direction - allowing you to specify the tree in the parent chart you wish to export into the values of the child chart.It also allows you to ringfence settings to a duplicate (aliased) subchart, as can be seen in the example below:

dependencies:
      - name: subchart1
        repository: http://localhost:10191
        version: 0.1.0
        export-values:
          - child: release
            parent: release.old

      - name: subchart1
        repository: http://localhost:10191
        version: 0.1.0
        export-values:
          - child: release
            parent: release.new
        alias: subchart1a

Obviously, this simple use case can be achieved through other methods - it's when there are many requirements using a mix & match of various parent values that this change become effective.

@vedit
Copy link

vedit commented Dec 18, 2017

I believe this list should be created automatically during a dependency installation with cli

@withnale
Copy link
Author

Maintainers - any update on the following? We have some complex scenarios we are trying to map out regarding parent charts and subcharts and this would really help simplify/encapsulate the 'interfaces' between charts

@jascott1
Copy link
Contributor

Hi @withnale

Thanks for the PR. Can you add an example to original proposal showing the values yaml and where the source tree for the dependency is take from? I want to understand how this works in a 3+ tier umbrella chart.

@kfox1111
Copy link

my $0.02

I think this is a subset of a larger issue. This does solve wanting to map values from a parent chart into differently named things in child charts. +1 for that.

But I also think you may want to map things within the same chart.

ie, you might want to have one value that has a default that two daemonsets use if a more specific value isn't set that is particular to each. you can do that one thing today with default and a bunch of logic, but maybe the same solution can be used for all use cases.

some way that does post value merging, pre templating value defaulting.

something like values.yaml:
c = 1

mapping.yaml
a = c
b = c

then daemonset-a.yaml can reference just .Values.a and daemonset-b can reference .Values.b

The user can override b directly without affecting a.

and the same mapping logic can handle the child chart too. say each was in its own subchart named subcharta and subchartb:
values.yaml:
c = 1

mapping.yaml:
subcharta.a = c
subchartb.b = c

or maybe mapping.yaml:
a = c
b = c
subcharta.a=a
subchartb.b=b

@withnale
Copy link
Author

@kfox1111 - yes there is definitely a need for more flexible mapping of the values tree within charts. This is always a pain point when authoring charts.

However, I tried to write this issue as an incremental change to meet the need whilst mirroring the existing import-values functionality.

On a separate note, it would be good to augment the existing best practices with feedback from complex chart writers. I often refer to the openstack-helm guys work for example. I'm aware of the vague wish to support different templating Is there a roadmap for helm templating improvements? (I guess I should ask this in slack rather than here

@withnale
Copy link
Author

Any update on this one?

@jascott1
Copy link
Contributor

We could continue with the PR and expand the export-values functionality later. @bacongobbler @SlickNik Any opinions on this one?

@kfox1111
Copy link

I think the solution to the bigger issue can solve the smaller issue, but if we solve the smaller issue in isolation, it will be a whole another set of code to have to maintain and for users to have to understand the working of.

I was hoping to discuss this during the helm summit more. is waiting till then an ok thing?

@kfox1111
Copy link

ie, if the user has to go to a webpage with a state diagragram of how variables can get pulled from subcharts, pushed from subcharts, pulled from requirements, defaulted in a defaults file via mapping rules or set in the values file of the package or the end users values or a subcharts all of the above....then I think we've lost.

@bacongobbler
Copy link
Member

bacongobbler commented Jan 27, 2018

Yes, I think it's fine to wait until then. I don't think we're planning on cutting a feature release before the summit.

@Tapppi
Copy link

Tapppi commented Apr 12, 2018

What is the status on this? Did the summit bring clarity?

This would be highly appreciated. We have platform charts with tens of microservices that use deployment specific defaults. Currently we have to resort to very specific semantic naming in global. This doesn't work when we don't control the subcharts, so it's a weird mix of duplicates for 3rd party charts and global configs for our own charts..

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@Tapppi
Copy link

Tapppi commented Aug 12, 2018

Another ping, has there been more conversations about the topic somewhere?

@swachter
Copy link

Is this feature still in consideration? It would allow to share values between multiple subcharts. I do not see another reasonable mechanism to accomplish this right now.

@withnale
Copy link
Author

bump.

I've come back to using helm subcharts after some time away and there still doesn't seem to be a clean way of doing this other than what I proposed. Any chance if I clean this up it will get a merge?

@kfox1111
Copy link

I still really suffer from not having a way to map variables too. so, I gotta do stuff like this, or worse constantly:

tenant=kfox
helm install namespace --name $tenant --namespace ${tenant}-admin --set magicnamespace.namespace=$tenant --set ingress.controller.scope.namespace=$tenant

@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@loewenstein
Copy link

@ilya-lesikov, are you considering to write a HIP for this, as this is apparently the recommended way forward by Helm maintainers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants