Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/197#discussion_r18288709
--- Diff:
core/src/main/java/brooklyn/management/internal/LocalLocationManager.java ---
@@ -158,50 +178,115 @@ public Location manage(Location loc) {
return loc;
}
- AccessController.Response access =
managementContext.getAccessManager().getAccessController().canManageLocation(loc);
- if (!access.isAllowed()) {
- throw new IllegalStateException("Access controller forbids
management of "+loc+": "+access.getMsg());
- }
-
Location parent = loc.getParent();
if (parent != null &&
!managementContext.getLocationManager().isManaged(parent)) {
-// throw new IllegalStateException("Can't manage "+e+" because
its parent is not yet managed ("+parent+")");
log.warn("Parent location "+parent+" of "+loc+" is not
managed; attempting to manage it (in future this may be disallowed)");
- manage(parent);
+ return manage(parent);
+ } else {
+ return manageRecursive(loc, ManagementTransitionMode.CREATING);
}
-
+ }
+
+ @Override
+ public void manageRebindedRoot(Location item) {
+ ManagementTransitionMode mode =
getLastManagementTransitionMode(item.getId());
+ Preconditions.checkNotNull(mode, "Mode not set for rebinding %s",
item);
+ manageRecursive(item, mode);
+ }
+
+ protected Location manageRecursive(Location loc, final
ManagementTransitionMode initialMode) {
+ AccessController.Response access =
managementContext.getAccessController().canManageLocation(loc);
+ if (!access.isAllowed()) {
+ throw new IllegalStateException("Access controller forbids
management of "+loc+": "+access.getMsg());
+ }
+
recursively(loc, new Predicate<AbstractLocation>() { public
boolean apply(AbstractLocation it) {
+ ManagementTransitionMode mode =
getLastManagementTransitionMode(it.getId());
+ if (mode==null) {
+ setManagementTransitionMode(it, mode = initialMode);
+ }
+
if (it.isManaged()) {
- return false;
- } else {
- boolean result = manageNonRecursive(it);
- if (result) {
- it.setManagementContext(managementContext);
+ if (mode==ManagementTransitionMode.CREATING) {
+ // silently bail out
+ return false;
+ } else {
+ // on rebind, we just replace, fall through to below
+ }
+ }
+
+ boolean result = manageNonRecursive(it, null);
+ if (result) {
+ it.setManagementContext(managementContext);
+ if (!mode.isReadOnly()) {
it.onManagementStarted();
recordLocationEvent(it, Lifecycle.CREATED);
-
managementContext.getRebindManager().getChangeListener().onManaged(it);
}
- return result;
+
managementContext.getRebindManager().getChangeListener().onManaged(it);
}
+ return result;
} });
return loc;
}
@Override
- public void unmanage(Location loc) {
+ public void unmanage(final Location loc) {
+ unmanage(loc, ManagementTransitionMode.DESTROYING);
+ }
+
+ public void unmanage(final Location loc, final
ManagementTransitionMode mode) {
+ unmanage(loc, mode, false);
+ }
+
+ private void unmanage(final Location loc, ManagementTransitionMode
mode, boolean hasBeenReplaced) {
if (shouldSkipUnmanagement(loc)) return;
-
- recursively(loc, new Predicate<AbstractLocation>() { public
boolean apply(AbstractLocation it) {
- if (shouldSkipUnmanagement(it)) return false;
- boolean result = unmanageNonRecursive(it);
- if (result) {
- it.onManagementStopped();
-
managementContext.getRebindManager().getChangeListener().onUnmanaged(it);
- recordLocationEvent(it, Lifecycle.DESTROYED);
- if (managementContext.gc != null)
managementContext.gc.onUnmanaged(it);
+
+ if (hasBeenReplaced) {
+ // we are unmanaging an old instance after having replaced it
+
+ if
(mode==ManagementTransitionMode.REBINDING_NO_LONGER_PRIMARY) {
+ // when migrating away, these all need to be called
+
managementContext.getRebindManager().getChangeListener().onUnmanaged(loc);
+ if (managementContext.gc != null)
managementContext.gc.onUnmanaged(loc);
+ } else {
+ // should be coming *from* read only; nothing needed
+ if (!mode.wasReadOnly())
+ log.warn("Should not be unmanaging "+loc+" in mode
"+mode);
--- End diff --
Is this a programming error? Should it throw an exception? There is a
similar case above.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---