[
https://issues.apache.org/jira/browse/BROOKLYN-286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15312260#comment-15312260
]
ASF GitHub Bot commented on BROOKLYN-286:
-----------------------------------------
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/173#discussion_r65534125
--- Diff:
camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
---
@@ -257,37 +261,63 @@ private void configureEntityConfig(EntitySpec<?>
spec, Set<String> encounteredRe
// attrs will contain only brooklyn.xxx properties when coming
from BrooklynEntityMatcher.
// Any top-level flags will go into "brooklyn.flags". When
resolving a spec from $brooklyn:entitySpec
// top level flags remain in place. Have to support both cases.
-
+ //
+ // For config values inherited from the super-type (be that the
Java type or another catalog item
+ // being extended), we lookup the config key to find out if the
values should be merged, overridden or
+ // cleared.
+
ConfigBag bag = ConfigBag.newInstance((Map<Object, Object>)
attrs.getStringKey(BrooklynCampReservedKeys.BROOKLYN_CONFIG));
ConfigBag bagFlags = ConfigBag.newInstanceCopying(attrs);
if (attrs.containsKey(BrooklynCampReservedKeys.BROOKLYN_FLAGS)) {
bagFlags.putAll((Map<String, Object>)
attrs.getStringKey(BrooklynCampReservedKeys.BROOKLYN_FLAGS));
}
- Collection<FlagConfigKeyAndValueRecord> topLevelApparentConfig =
findAllFlagsAndConfigKeys(spec, bagFlags);
+ Collection<FlagConfigKeyAndValueRecord> topLevelApparentConfig =
findAllFlagsAndConfigKeyValues(spec, bagFlags);
for (FlagConfigKeyAndValueRecord r: topLevelApparentConfig) {
if (r.getConfigKeyMaybeValue().isPresent())
bag.putIfAbsent((ConfigKey)r.getConfigKey(),
r.getConfigKeyMaybeValue().get());
if (r.getFlagMaybeValue().isPresent())
bag.putAsStringKeyIfAbsent(r.getFlagName(),
r.getFlagMaybeValue().get());
}
+
// now set configuration for all the items in the bag
- Collection<FlagConfigKeyAndValueRecord> records =
findAllFlagsAndConfigKeys(spec, bag);
+ Map<String, ConfigKey<?>> entityConfigKeys =
findAllConfigKeys(spec);
+
+ Collection<FlagConfigKeyAndValueRecord> records =
findAllFlagsAndConfigKeyValues(spec, bag);
Set<String> keyNamesUsed = new LinkedHashSet<String>();
for (FlagConfigKeyAndValueRecord r: records) {
if (r.getFlagMaybeValue().isPresent()) {
- Object transformed = new SpecialFlagsTransformer(loader,
encounteredRegisteredTypeIds).apply(r.getFlagMaybeValue().get());
- spec.configure(r.getFlagName(), transformed);
- keyNamesUsed.add(r.getFlagName());
+ String flag = r.getFlagName();
+ Object ownVal = new SpecialFlagsTransformer(loader,
encounteredRegisteredTypeIds).apply(r.getFlagMaybeValue().get());
+ Maybe<?> superVal = spec.getFlags().containsKey(flag) ?
Maybe.of(spec.getFlags().get(flag)) : Maybe.absent();
+ Object combinedVal =
combineValues(entityConfigKeys.get(flag), spec, Maybe.of(ownVal),
superVal).get();
+ spec.configure(flag, combinedVal);
+ keyNamesUsed.add(flag);
}
if (r.getConfigKeyMaybeValue().isPresent()) {
- Object transformed = new SpecialFlagsTransformer(loader,
encounteredRegisteredTypeIds).apply(r.getConfigKeyMaybeValue().get());
- spec.configure((ConfigKey<Object>)r.getConfigKey(),
transformed);
- keyNamesUsed.add(r.getConfigKey().getName());
+ ConfigKey<Object> key = (ConfigKey<Object>)
r.getConfigKey();
+ Object ownVal = new SpecialFlagsTransformer(loader,
encounteredRegisteredTypeIds).apply(r.getConfigKeyMaybeValue().get());
+ Maybe<?> superVal = spec.getConfig().containsKey(key) ?
Maybe.of(spec.getConfig().get(key)) : Maybe.absent();
+ Object combinedVal =
combineValues(entityConfigKeys.get(key.getName()), spec, Maybe.of(ownVal),
superVal).get();
+ spec.configure(key, combinedVal);
+ keyNamesUsed.add(key.getName());
}
}
+ // For anything that should not be inherited, clear if from the
spec
+ for (Map.Entry<String, ConfigKey<?>> entry :
entityConfigKeys.entrySet()) {
+ if (keyNamesUsed.contains(entry.getKey())) {
+ continue;
+ }
+ ConfigKey<?> key = entry.getValue();
+ InheritanceMode mode = getInheritanceMode(key, spec);
+ if (mode == InheritanceMode.NONE) {
+ spec.removeConfig(key);
+ spec.removeFlag(key.getName());
+ }
+ }
+
--- End diff --
Should do analogous merging for locations?
> 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)
