Aled Sage created BROOKLYN-286:
----------------------------------

             Summary: merge config keys map values, where appropriate
                 Key: BROOKLYN-286
                 URL: https://issues.apache.org/jira/browse/BROOKLYN-286
             Project: Brooklyn
          Issue Type: New Feature
    Affects Versions: 0.9.0
            Reporter: Aled Sage


See the email discussion on [email protected], subject "[PROPOSAL] 
merging config keys", initially kicked off 25/05/2016, 12:12.

Below is a copy of that initial proposal, which has then been further discussed 
on the mailing list.

TL;DR: we should merge config when overriding entities/locations, where it's 
obvious that such behaviour is desired. For example, where an entity type 
defines shell.env, then a new entity extending this type should inherit and add 
to those values.


_*REQUIREMENTS*_

_*shell.env in entities*_

When extending an existing entity type in YAML, it is not possible to extend 
the set of environment variables. Instead, if the sub-type declares shell.env 
it will override the inherited values.

For example, consider the catalog items below:

   # Catalog
   brooklyn.catalog:
      items:
      - id: machine-with-env
        item:
          type:
   org.apache.brooklyn.entity.software.base.VanillaSoftwareProcess
          brooklyn.config:
            shell.env:
              ENV1: myEnv1


   # Blueprint
   location: ...
   services:
   - type: machine-with-env
      brooklyn.config:
        shell.env:
          ENV2: myEnv2
        launch.command: echo "ENV1=$ENV1, ENV2=$ENV2"

A user might well expect the launch.command to have myEnv1 and myEnv2. However, 
it does not get the ENV1 environment variable. This is a real pain when trying 
to customize stock blueprints.

We propose that the shell.env map should be *merged*.


_*provisioning.properties*_

An entity can be configured with provisioning.properties. These are passed to 
the location when obtaining a new machine. They supplement and override the 
values configured on the location. However, for templateOptions the 
expected/desired behaviour would be to merge the options.

Consider the blueprint below:_*
*_

   location:
      minCores: 1
      templateOptions:
        networks: myNetwork
   services:
   - type: org.apache.brooklyn.entity.machine.MachineEntity
      brooklyn.config:
        provisioning.properties:
          minRam: 2G
          templateOptions:
            tags: myTag

A user might well expect the VM to be created with the given networks and tags. 
However, currently the templateOptions in provisoining.properties will override 
the existing value, rather than being merged with it.

We propose that the templateOptions map should be *merged*.

Valentin made a start to fix this in 
https://github.com/apache/brooklyn-server/pull/151.


_*_*provisioning.properties in sub-entities*_
*_

A similar argument holds for when extending an entity-type in YAML.

If the super-type declares template options, then any additional 
provisioning.properties declared on the entity sub-type should be *merged* 
(including merging the templateOptions map contained within it).


_*files.preinstall, templates.preinstall, etc*_

The same applies for the map config for: files.preinstall, 
templates.preinstall, files.install, templates.install, files.runtime and 
templates.runtime.

We propose that these maps get *merged* with the value defined in the 
super-type.


_*Overriding default values*_

For default values in the super-type, we propose that this value *does* get 
overridden, rather than merged.

For example, in the blueprint below we suggest that the launch-command in the 
sub-type should have ENV2 but not ENV_IN_DEFAULT.

   brooklyn.catalog:
      items:
      - id: machine-with-env
        version: 1.0.0
        item:
          type:
   org.apache.brooklyn.entity.software.base.VanillaSoftwareProcess
          brooklyn.parameters:
          - name: shell.env
            default:
              ENV_IN_DEFAULT: myEnvInDefault
      - id: machine-with-env-2
        version: 1.0.0
        item:
          type: machine-with-env
          brooklyn.config:
            shell.env:
              ENV2: myEnv2
            launch.command: echo "ENV_IN_DEFAULT=$ENV_IN_DEFAULT,
   ENV2=$ENV2"

(Interestingly, the current behaviour of machine-with-env is that it gets the 
value for ENV_IN_DEFAULT but not for ENV2, so sometime strange is going on with 
re-defining the shell.env config key!)


_*Extending commands: deferred*_

Another scenario is where a super-type declares a value for `install.command`, 
and the sub-type wants to augment this by adding additional commands. Currently 
that is not possible. Instead the sub-type needs to use pre.install.command 
and/or post.install.command. But that leads to the same problem if a super-type 
also has a value defined for that key.

Svet suggested we could perhaps introduce something like $brooklyn:super().

Unless we can generalise that approach to also solve the merging of `shell.env` 
etc, then I suggest we defer the `install.command` use-case. That can be 
proposed and discussed in a different thread.

However, if we can solve these problems with clever explicit use of 
$brooklyn:super(), then that could provide an elegant solution to all of these 
problems!


_*Inheritance from parent entities*_

Things are made yet more complicated by the fact we inherit config from parent 
entities, in the entity hierarchy.

We propose that this behaviour is also configurable for the config key, but 
that the defaults stay as they are. The existing logic is applied to find the 
config value that applies to the given entity. That value is then merged with 
its super-type, as appropriate.

For example, in the blueprint below... machine1 would get ENV1 and ENV2 (i.e. 
the ENV1 definition overrides the ENV_IN_APP definition). However, machine2 
would get ENV1 and ENV_IN_APP (i.e. it inherits ENV_IN_APP from the parent, and 
this is meged with the super-type).

   services:
   - type: org.apache.brooklyn.entity.stock.BasicApplication
      brooklyn.config:
        shell.env:
          ENV_IN_APP: myEnvInApp
      brooklyn.children:
      - type: machine-with-env
        id: machine1
        brooklyn.config:
          shell.env:
            ENV2: myEnv2
      - type: machine-with-env
        id: machine2

The reasoning behind this is to figure out the inheritance/override rules 
incrementally. We leave the parent-inheritance as-is, and just focus on the 
sub-typing inheritance.

Note that there is already a ConfigInheritance defined on ConfigKey for 
controlling this kind of inheritance from the parent. The legal values for 
ConfigInheritance are currently just ALWAYS and NONE.


_*IMPLEMENTATION*_

Clearly we do not want to implement this piecemeal. We'll add a way to declare 
that a config key should be merged with that value from the super-type.

We'll change the Java ConfigKey code to be:

   public interface ConfigKey {
      /**
       * @since 0.10.0
       */
      @Nullable ConfigInheritance getParentInheritance();

      /**
       * @since 0.10.0
       */
   @Nullable ConfigInheritance getTypeInheritance();

      /**
       * @deprecated since 0.10.0; instead use {@link
   #getParentInheritance()}
       */
      @Nullable ConfigInheritance getInheritance();
   }

We'll add to ConfigInheritance support for MERGE. We'll change the name 
"ALWAYS" to OVERRIDE (deprecating the old value).

We'll change EntityConfigMap.getConfig to handle this new merge behaviour. And 
same for locations, policies and enrichers.




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to