On 25.05.2016 г., at 14:12, Aled Sage <aled.s...@gmail.com> wrote:
Hi all,
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.
Aled
