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: allow import of child values via optional requirements field 'import-values' #1995

Closed
jascott1 opened this issue Feb 17, 2017 · 14 comments

Comments

@jascott1
Copy link
Contributor

jascott1 commented Feb 17, 2017

Problem

When working on complicated nested charts developers would like to create 'library charts' that contain common values that need to be accessible from the parent chart but there is no mechanism to do this. Developers resort to workarounds such as build scripts that merge values files which add more steps and are not helm native.

Proposed Solution

Allow the import of a child's values to parent values by adding a imports-values: field in requirements.yaml as in the example below

# requirements.yaml (simple list)
dependencies:
  - name: kolla-common
    repository: http://localhost:10191
    version: 0.1.0
    import-values:
      - foo.thing

The above would instruct helm to take the values found at path "foo.thing" in sub-chart "kolla-common" and place them at the root of the top parent's values. So that given a parent values of

# parent values
mything: yay

and child values

# child values
foo:
  thing:
    bar: true
    baz: false
    other:
      - 1
      - 2
      - 3

The resulting parent values would be

# result parent values
mything: yay
bar: true
baz: false
  other:
    - 1
    - 2
    - 3

Advanced usage

Additionally the import-values: field would support a child/parent dictionary value that would allow reparenting the imported values into a specific path in the parent values as in the example below:

# requirements (child/parent)
...
import-values:
  - child: foo.thing
    parent: new.key

In the above example, the values from the child path are placed at the specified location 'new.key' in the parent values as below:

# result values
mything: yay
new:
  key:
    bar: true
    baz: false
    other:
      - 1
      - 2
      - 3

Non-existent import values

If a import-values field contains child paths that do not exist, a warning will be issued.

Mixed string and child/parent

Mixing the simple list of strings and child/parent dictionaries is supported and the below would be valid:

# requirements.yaml
...
import-values:
  - common
  - child: foo.thing
    parent: new.key

Exports key convention

To facilitate the introspection of values intended to be consumed by a parent, it is suggested to follow a exports: key convention wherein those values are all contained in field named exports: as below:

# child/values.yaml

exports:
  foo: true
  bar: false
baz:
  pike: true
....

By following the above convention, tools will be able to better inspect and when necessary exclude the values intended for parent. Users that need the flexibility to ignore the convention are allowed to do so with the caveat that future analysis involving values intended for parent may be difficult or impossible.

Importing from Child to Parent to GrandParent

By using the imports-values mechanism at each level in the hierarchy, values can be imported from a child chart to its parent and then imported into the grandparent (and so on).

# parent requirements
imports-values:
  - child: data.foo
    parent: imported-from-child

# grandparent requirements
import-values:
  - child: imported-from-child
    parent: imported-from-child-via-parent


@kfox1111
Copy link

Also, this should suggest a convention that child charts place stuff under the "parent" section. The parent section can then be automatically excluded nicely when used by chart introspection tools.

@redbaron
Copy link
Contributor

what would be example of data passed this way in real-life?

@kfox1111
Copy link

ok. so maybe a more concrete example would help. in kolla kubernetes, we would use this feature like so I think:

in kolla-common/values.yaml, we would have something like:

parent:
    common:
        global:
            kolla:
                all:
                    docker_registry: docker.io
                    docker_namespace: kolla
                    base_distro: centos
                    install_type: binary
                    container_config_directory: /var/lib/kolla/config_files
                    image_tag: 2.0.2
                    fluentd_image_tag: 3.0.2
                    kolla_toolbox_image_tag: 3.0.2
                    kubernetes_entrypoint_image_tag: 4.0.0
                    selector_key: kolla_controller
                    selector_value: "true"
                    image_pull_policy: IfNotPresent
                    container_config_directory: /var/lib/kolla/config_files
                    kubernetes_entrypoint: false
                    kube_logger: true
    mariadb-connection:
        global:
            kolla:
                all:
                    database_host: mariadb
                    database_port: 3306
    keystone-admin:
        global:
            kolla:
                all:
                    keystone_admin_protocol: http
                    keystone_admin_svcname: keystone-admin
                    keystone_admin_port: 35357
                    keystone_admin_project: admin
                    keystone_admin_username: admin
                    keystone_admin_domain_name: Default
                    region: RegionOne

Then in chart: neutron-create-db-job
its requirements.yaml would have:

dependencies:
  - name: kolla-common
    repository: file://../../kolla-common
    version: 0.5.0-1
    include-values:
      - common
      - mariadb-connection

and then it sould behave the same way as if the following was in neutron-create-db-job values.yaml.

global:
    kolla:
        all:
            docker_registry: docker.io
            docker_namespace: kolla
            base_distro: centos
            install_type: binary
            container_config_directory: /var/lib/kolla/config_files
            image_tag: 2.0.2
            fluentd_image_tag: 3.0.2
            kolla_toolbox_image_tag: 3.0.2
            kubernetes_entrypoint_image_tag: 4.0.0
            selector_key: kolla_controller
            selector_value: "true"
            image_pull_policy: IfNotPresent
            container_config_directory: /var/lib/kolla/config_files
            kube_logger: true
            database_host: mariadb
            database_port: 3306

@jascott1
Copy link
Contributor Author

@technosophos Id like to implement this week. Do you have any reservations or required changes for this feature? Thanks!

@redbaron
Copy link
Contributor

@jascott1 , can you please have a look at #1965 and check if it solves you problem. It is more generic, seems to cover your case too.

@jascott1
Copy link
Contributor Author

jascott1 commented Feb 28, 2017

Im aware of #1965 but don't agree its 'more generic' and personally don't like having an exports file. The historic idea is that requirements.yaml expresses user intent (versions, repo, conditions/tags etc). What advantages do you perceive your approach has over the one in this proposal? I'd like to get this sorted so we can implement something asap.

@technosophos
Copy link
Member

@kfox1111 What if we suggested a convention where the child chart could declare things it thought were exportable.

Rather than using parent it'd do something like this:

exports:
  cache:
    port: 1337

Then the parent chart's requirements could import all of the exported variables at once:

include-values:
  - memcached.exports

And @jascott1 would it be easier to simplify the re-parenting aliasing by doing:

include-values:
  "ALIAS": "YAML_PATH"

So in the above case, if I wanted to reference the memcached.exports values as mcExports, it would be like this:

include-values:
  mcExports: memcached.exports

Or does that actually make the implementation harder?

@jascott1
Copy link
Contributor Author

jascott1 commented Mar 1, 2017

@technosophos Im fine with changing parent to exports but the reason we settled on the list with child and parent fields was because we have to quote the key if it has a dot in it in the simple key-value example.

@kfox1111
Copy link

kfox1111 commented Mar 1, 2017

@technosophos

s/parent/exports/ works for me.

include-values:
 - memcached.exports

seems verbose to me though. the include values is on the repo subsection, so specifying it again feels redundant. exports itself is also redundant I think as 99% of the time I think you will want to include just the groups of values inside. I think in kolla-kubernetes case, (and probably other libs), there will be more then one set of macro's, and you will most of the time only want to include the macro's values group rather then *.

For mapping, the concern with doing it "alias": "path" was the common case of wanting to include the values into the root of the parents values. would this work in yaml?

".": "YAML_PATH" ?

instead:

  - child: foo.thing
    parent: .

is more verbose, but maybe easier to understand?

@redbaron
Copy link
Contributor

redbaron commented Mar 1, 2017

@jascott1 ,

What advantages do you perceive your approach has over the one in this proposal?

Advantage is that not only you can share input values for the chart like in this proposal, you can explicitly export output values, ie final value of service name after all templating was done, therefore you are actually sharing precise values, not just inputs + naming conventions, which are error prone

@technosophos
Copy link
Member

@jascott1 and @kfox1111: Okay, I'm convinced of the value of the child/parent thing. In looking at the way the env section of the pod template is designed (which uses name:/value:), I see we get the additional possibility of introducing few features over time since the data is more structured.

@kfox1111
Copy link

kfox1111 commented Mar 1, 2017

@redbaron I'm a little concerned how passing the values down, then back up, and then maybe back down? will work in practice. Can you explain how that would work when you have multiple layers of templates N, where N > 3?

I think that use case can still mostly be handled by calling macro's from the child in the parent templates to get the processed values? We're doing that now in kolla-kubernetes.

The use case we're trying to solve here is getting values for templates that are stored in child libs to get the default values from the parents value space as thats where they execute. In this case at least, they can do their own processing on the values. They can pass the values back to the parent if needed like: {{ include "childs_value_thingy" . }}

@technosophos oh, I hadn't considered the need of adding more fields. That makes a lot of sense though. Gives us room to grow if/as needed.

@jascott1
Copy link
Contributor Author

jascott1 commented Mar 8, 2017

@kfox1111 @technosophos wrt to child->paren->grandparent values, I assumed we were thinking of importing imports all the way up from grandchild to parent to grandparent (to great grandparent etc.) in each requirements.yaml OR we could make everything relative to top parent root but that seems fraught with peril. If the explicit importing repeatedly sounds reasonable I will add it to the main proposal.

@kfox1111
Copy link

kfox1111 commented Mar 9, 2017

Yeah. I think child giving values to parent on parents request, followed by grandparent getting values from parent (that includes the stuff parent asked explicitly from child) is the right way to go. Values passed that way are always very explicit/intentional.

jascott1 added a commit to jascott1/helm that referenced this issue Mar 14, 2017
Implements a mechanism in requirements.yaml to allow the import
and re-parenting of value table from child chart.

Closes helm#1995
jascott1 added a commit to jascott1/helm that referenced this issue Mar 14, 2017
Implements a mechanism in requirements.yaml to allow the import
and re-parenting of value table from child chart.

Closes helm#1995
jascott1 added a commit to jascott1/helm that referenced this issue Mar 14, 2017
Implements a mechanism in requirements.yaml to allow the import
and re-parenting of value table from child chart.

Closes helm#1995
jascott1 added a commit to jascott1/helm that referenced this issue Mar 20, 2017
Implements a mechanism in requirements.yaml to allow the import
and re-parenting of value table from child chart.

Closes helm#1995
jascott1 added a commit to jascott1/helm that referenced this issue Mar 28, 2017
Implements a mechanism in requirements.yaml to allow the import
and re-parenting of value table from child chart.

Closes helm#1995
jascott1 added a commit to jascott1/helm that referenced this issue Mar 29, 2017
Implements a mechanism in requirements.yaml to allow the import
and re-parenting of value table from child chart.

Closes helm#1995
jascott1 added a commit to jascott1/helm that referenced this issue Mar 31, 2017
Implements a mechanism in requirements.yaml to allow the import
and re-parenting of value table from child chart.

Closes helm#1995
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

No branches or pull requests

4 participants