Github user aledsage commented on a diff in the pull request: https://github.com/apache/brooklyn-library/pull/147#discussion_r169175531 --- Diff: software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java --- @@ -425,6 +443,20 @@ public void update() { } } + @Override + public void changeServerPool(String groupId) { + Group newGroup = (Group) getManagementContext().getEntityManager().getEntity(groupId); + if (newGroup == null) { + throw new IllegalArgumentException("Group '"+groupId+"' not found"); + } + + config().set(SERVER_POOL, newGroup); + if (hasServerPoolMemberTrackingPolicy()) { --- End diff -- What happens is... If we don't already have a serverPoolMemberTrackingPolicy (e.g. because we haven't been started yet), then keep it that way (i.e. don't add one). In `hasServerPoolMemberTrackingPolicy`, it handles the case where the field has not been set - it is effectively transient because fields are not persisted (only attributes, config and adjuncts). When we then call `addServerPoolMemberTrackingPolicy`, it's into the code I didn't touch. Agreed that method is a bit weird - it'll remove it if `serverPoolMemberTrackerPolicy` is not null, but won't if a policy of that type already exists with the field being null (which presumably could happen on rebind). In conclusion, I agree that `addServerPoolMemberTrackingPolicy` is a bit weird. But I'm ok with the logic behind the code I've changed/added :-)
---