You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by grkvlt <gi...@git.apache.org> on 2016/08/03 19:55:38 UTC
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
GitHub user grkvlt opened a pull request:
https://github.com/apache/brooklyn-server/pull/287
Add new member tracking policy with configurable SSH commands
Allows easy creation of new load balancer entities by defining tracking policies in YAML that execute the required commands over SSH when an entity is added or removed.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/grkvlt/brooklyn-server ssh-command-member-tracking-policy
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/287.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #287
----
commit a62eaca4996fcf6e6663d85db9db4d7d86e41c44
Author: Andrew Donald Kennedy <an...@cloudsoftcorp.com>
Date: 2016-08-03T19:46:05Z
Add new member tracking policy with configurable SSH commands
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73866537
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/SshCommandMembershipTrackingPolicy.java ---
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.entity.group;
+
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.config.MapConfigKey;
+import org.apache.brooklyn.core.effector.ssh.SshEffectorTasks;
+import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.location.Machines;
+import org.apache.brooklyn.core.sensor.ssh.SshCommandSensor;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
+import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.core.task.Tasks;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.text.Strings;
+
+/**
+ * Policy which tracks membership of a group, and executes SSH commands
+ * on MEMBER{ADDED,REMOVED} events, as well as SERVICE_UP {true,false} for those members.
+ */
+public class SshCommandMembershipTrackingPolicy extends AbstractMembershipTrackingPolicy {
+
+ private static final Logger LOG = LoggerFactory.getLogger(SshCommandMembershipTrackingPolicy.class);
+
+ public static final ConfigKey<String> EXECUTION_DIR = ConfigKeys.newStringConfigKey("executionDir", "Directory where the command should run; "
+ + "if not supplied, executes in the entity's run dir (or home dir if no run dir is defined); "
+ + "use '~' to always execute in the home dir, or 'custom-feed/' to execute in a custom-feed dir relative to the run dir");
+
+ public static final MapConfigKey<Object> SHELL_ENVIRONMENT = BrooklynConfigKeys.SHELL_ENVIRONMENT;
+
+ public static final ConfigKey<String> UPDATE_COMMAND = ConfigKeys.newStringConfigKey("update.command", "Command to run on membership change events");
+
+ /**
+ * Called when a member is updated or group membership changes.
+ */
+ @Override
+ protected void onEntityEvent(EventType type, Entity entity) {
+ LOG.trace("Event {} received for {} in {}", new Object[] { type, entity, getGroup() });
+ String command = config().get(UPDATE_COMMAND);
+ if (Strings.isNonBlank(command)) {
--- End diff --
Can you add `type` as an environment variable. Would be useful for scripts to know whether this is add/remove/change event.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73673522
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/SshCommandMembershipTrackingPolicy.java ---
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.entity.group;
+
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.config.MapConfigKey;
+import org.apache.brooklyn.core.effector.ssh.SshEffectorTasks;
+import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.location.Machines;
+import org.apache.brooklyn.core.sensor.ssh.SshCommandSensor;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
+import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.core.task.Tasks;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.text.Strings;
+
+/**
+ * Policy which tracks membership of a group, and executes SSH commands
+ * on MEMBER{ADDED,REMOVED} events, as well as SERVICE_UP {true,false} for those members.
+ */
+public class SshCommandMembershipTrackingPolicy extends AbstractMembershipTrackingPolicy {
--- End diff --
test? (preferably using `RecordingSshTool`, so it's a unit test).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73682812
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalPolicyFactory.java ---
@@ -125,7 +124,6 @@ public InternalPolicyFactory(ManagementContextInternal managementContext) {
for (Map.Entry<ConfigKey<?>, Object> entry : spec.getConfig().entrySet()) {
pol.config().set((ConfigKey)entry.getKey(), entry.getValue());
}
- ConfigConstraints.assertValid(pol);
--- End diff --
Because of unresolvable config that needs to know the entity context, which is a valid thing.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73682888
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/SshCommandMembershipTrackingPolicy.java ---
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.entity.group;
+
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.config.MapConfigKey;
+import org.apache.brooklyn.core.effector.ssh.SshEffectorTasks;
+import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.location.Machines;
+import org.apache.brooklyn.core.sensor.ssh.SshCommandSensor;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
+import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.core.task.Tasks;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.text.Strings;
+
+/**
+ * Policy which tracks membership of a group, and executes SSH commands
+ * on MEMBER{ADDED,REMOVED} events, as well as SERVICE_UP {true,false} for those members.
+ */
+public class SshCommandMembershipTrackingPolicy extends AbstractMembershipTrackingPolicy {
+
+ private static final Logger LOG = LoggerFactory.getLogger(SshCommandMembershipTrackingPolicy.class);
+
+ public static final ConfigKey<String> EXECUTION_DIR = ConfigKeys.newStringConfigKey("executionDir", "Directory where the command should run; "
+ + "if not supplied, executes in the entity's run dir (or home dir if no run dir is defined); "
+ + "use '~' to always execute in the home dir, or 'custom-feed/' to execute in a custom-feed dir relative to the run dir");
+
+ public static final MapConfigKey<Object> SHELL_ENVIRONMENT = BrooklynConfigKeys.SHELL_ENVIRONMENT;
+
+ public static final ConfigKey<String> UPDATE_COMMAND = ConfigKeys.newStringConfigKey("update.command", "Command to run on membership change events");
+
+ /**
+ * Called when a member is updated or group membership changes.
+ */
+ @Override
+ protected void onEntityEvent(EventType type, Entity entity) {
+ LOG.trace("Event {} received for {} in {}", new Object[] { type, entity, getGroup() });
+ String command = config().get(UPDATE_COMMAND);
+ if (Strings.isNonBlank(command)) {
+ execute(command);
+ }
+ }
+
+ public void execute(String command) {
+ Maybe<SshMachineLocation> machine = Machines.findUniqueMachineLocation(entity.getLocations(), SshMachineLocation.class);
+ if (machine.isAbsentOrNull()) {
+ throw new IllegalStateException("No machine available to execute command");
+ }
+ LOG.info("Executing command on {}: {}", machine.get(), command);
+ String executionDir = config().get(EXECUTION_DIR);
+ String sshCommand = SshCommandSensor.makeCommandExecutingInDirectory(command, executionDir, entity);
+
+ // Set things from the entities defined shell environment, overriding with our config
+ Map<String, Object> env = MutableMap.of();
+ env.putAll(MutableMap.copyOf(entity.config().get(BrooklynConfigKeys.SHELL_ENVIRONMENT)));
+ env.putAll(MutableMap.copyOf(config().get(BrooklynConfigKeys.SHELL_ENVIRONMENT)));
+
+ // Try to resolve the configuration in the env Map
+ try {
+ env = (Map<String, Object>) Tasks.resolveDeepValue(env, Object.class, ((EntityInternal) entity).getExecutionContext());
+ } catch (InterruptedException | ExecutionException e) {
+ Exceptions.propagateIfFatal(e);
--- End diff --
probably should be `throw Exceptions.propagate()` then?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73673275
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -421,6 +421,7 @@ public void setDisplayName(String name) {
public void setEntity(EntityLocal entity) {
if (destroyed.get()) throw new IllegalStateException("Cannot set entity on a destroyed entity adjunct");
this.entity = entity;
+ this.execution = ((EntityInternal) entity).getExecutionContext();
--- End diff --
I'm guessing you hit a problem that made you add this line - can we capture in a test what that problem was, rather than just fixing it?
I'm fine with merging this time, but for future reference I am really keen that we improve our test coverage for when we hit problems like this.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #287: Add new member tracking policy with configurable...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-server/pull/287
Thanks @neykov
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r74306037
--- Diff: core/src/test/java/org/apache/brooklyn/entity/group/SshCommandMembershipTrackingPolicyTest.java ---
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.entity.group;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+import java.util.Map;
+
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.ImmutableList;
+
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.location.LocationSpec;
+import org.apache.brooklyn.api.mgmt.EntityManager;
+import org.apache.brooklyn.api.policy.PolicySpec;
+import org.apache.brooklyn.core.entity.trait.Startable;
+import org.apache.brooklyn.core.location.SimulatedLocation;
+import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
+import org.apache.brooklyn.core.test.entity.TestEntity;
+import org.apache.brooklyn.entity.group.AbstractMembershipTrackingPolicy.EventType;
+import org.apache.brooklyn.entity.stock.BasicStartable;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool;
+import org.apache.brooklyn.util.time.Duration;
+
+public class SshCommandMembershipTrackingPolicyTest extends BrooklynAppUnitTestSupport {
+
+ private SimulatedLocation loc;
+ private EntityManager entityManager;
+ private BasicGroup group;
+ private BasicStartable entity;
+ private LocationSpec<SshMachineLocation> machine;
+
+ @BeforeMethod(alwaysRun=true)
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+
+ machine = LocationSpec.create(SshMachineLocation.class)
+ .configure("address", "1.2.3.4")
+ .configure("sshToolClass", RecordingSshTool.class.getName());
+ loc = mgmt.getLocationManager().createLocation(LocationSpec.create(SimulatedLocation.class));
+ entityManager = app.getManagementContext().getEntityManager();
+
+ group = app.createAndManageChild(EntitySpec.create(BasicGroup.class)
+ .configure("childrenAsMembers", true));
+
+ entity = app.createAndManageChild(EntitySpec.create(BasicStartable.class).location(machine));
+ entity.policies().add(PolicySpec.create(SshCommandMembershipTrackingPolicy.class)
+ .configure("group", group)
+ .configure("shell.env.TEST", "test")
+ .configure("update.command", "echo ignored"));
+
+ app.start(ImmutableList.of(loc));
+ }
+
+ @AfterMethod(alwaysRun=true)
+ @Override
+ public void tearDown() throws Exception {
+ super.tearDown();
+ RecordingSshTool.clear();
+ }
+
+ @Test
+ public void testCommandExecutedWithEnvironment() throws Exception {
+ TestEntity member = entityManager.createEntity(EntitySpec.create(TestEntity.class).parent(group));
+ Duration.seconds(1).countdownTimer().waitForExpiry();
--- End diff --
Why wait for a second? That might well not be long enough on Apache Jenkins for it to work every time! And it's too long on a normal laptop.
I'd have written:
```
protected void assertExecSizeEventually(final int expectedSize) {
Asserts.succeedsEventually(new Runnable() {
public void run() {
assertEquals(RecordingSshTool.getExecCmds().size(), expectedSize);
}});
}
```
Same below when you wait a second and assert that the size is 2 or 3. (hence why worth extracting it into a method).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73865586
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/SshCommandMembershipTrackingPolicy.java ---
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.entity.group;
+
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.config.MapConfigKey;
+import org.apache.brooklyn.core.effector.ssh.SshEffectorTasks;
+import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.location.Machines;
+import org.apache.brooklyn.core.sensor.ssh.SshCommandSensor;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
+import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.core.task.Tasks;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.text.Strings;
+
+/**
+ * Policy which tracks membership of a group, and executes SSH commands
+ * on MEMBER{ADDED,REMOVED} events, as well as SERVICE_UP {true,false} for those members.
+ */
+public class SshCommandMembershipTrackingPolicy extends AbstractMembershipTrackingPolicy {
+
+ private static final Logger LOG = LoggerFactory.getLogger(SshCommandMembershipTrackingPolicy.class);
+
+ public static final ConfigKey<String> EXECUTION_DIR = ConfigKeys.newStringConfigKey("executionDir", "Directory where the command should run; "
+ + "if not supplied, executes in the entity's run dir (or home dir if no run dir is defined); "
+ + "use '~' to always execute in the home dir, or 'custom-feed/' to execute in a custom-feed dir relative to the run dir");
+
+ public static final MapConfigKey<Object> SHELL_ENVIRONMENT = BrooklynConfigKeys.SHELL_ENVIRONMENT;
+
+ public static final ConfigKey<String> UPDATE_COMMAND = ConfigKeys.newStringConfigKey("update.command", "Command to run on membership change events");
+
+ /**
+ * Called when a member is updated or group membership changes.
+ */
+ @Override
+ protected void onEntityEvent(EventType type, Entity entity) {
+ LOG.trace("Event {} received for {} in {}", new Object[] { type, entity, getGroup() });
+ String command = config().get(UPDATE_COMMAND);
+ if (Strings.isNonBlank(command)) {
+ execute(command);
+ }
+ }
+
+ public void execute(String command) {
+ Maybe<SshMachineLocation> machine = Machines.findUniqueMachineLocation(entity.getLocations(), SshMachineLocation.class);
--- End diff --
Suggest to use `Locations.getLocationsCheckingAncestors(...)` here. For example when an entity is nested in a `SameServerEntity`.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/287
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r74337925
--- Diff: core/src/test/java/org/apache/brooklyn/entity/group/SshCommandMembershipTrackingPolicyTest.java ---
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.entity.group;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+import java.util.Map;
+
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.ImmutableList;
+
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.location.LocationSpec;
+import org.apache.brooklyn.api.mgmt.EntityManager;
+import org.apache.brooklyn.api.policy.PolicySpec;
+import org.apache.brooklyn.core.entity.trait.Startable;
+import org.apache.brooklyn.core.location.SimulatedLocation;
+import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
+import org.apache.brooklyn.core.test.entity.TestEntity;
+import org.apache.brooklyn.entity.group.AbstractMembershipTrackingPolicy.EventType;
+import org.apache.brooklyn.entity.stock.BasicStartable;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool;
+import org.apache.brooklyn.util.time.Duration;
+
+public class SshCommandMembershipTrackingPolicyTest extends BrooklynAppUnitTestSupport {
+
+ private SimulatedLocation loc;
+ private EntityManager entityManager;
+ private BasicGroup group;
+ private BasicStartable entity;
+ private LocationSpec<SshMachineLocation> machine;
+
+ @BeforeMethod(alwaysRun=true)
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+
+ machine = LocationSpec.create(SshMachineLocation.class)
+ .configure("address", "1.2.3.4")
+ .configure("sshToolClass", RecordingSshTool.class.getName());
+ loc = mgmt.getLocationManager().createLocation(LocationSpec.create(SimulatedLocation.class));
+ entityManager = app.getManagementContext().getEntityManager();
+
+ group = app.createAndManageChild(EntitySpec.create(BasicGroup.class)
+ .configure("childrenAsMembers", true));
+
+ entity = app.createAndManageChild(EntitySpec.create(BasicStartable.class).location(machine));
+ entity.policies().add(PolicySpec.create(SshCommandMembershipTrackingPolicy.class)
+ .configure("group", group)
+ .configure("shell.env.TEST", "test")
+ .configure("update.command", "echo ignored"));
+
+ app.start(ImmutableList.of(loc));
+ }
+
+ @AfterMethod(alwaysRun=true)
+ @Override
+ public void tearDown() throws Exception {
+ super.tearDown();
+ RecordingSshTool.clear();
+ }
+
+ @Test
+ public void testCommandExecutedWithEnvironment() throws Exception {
+ TestEntity member = entityManager.createEntity(EntitySpec.create(TestEntity.class).parent(group));
+ Duration.seconds(1).countdownTimer().waitForExpiry();
--- End diff --
Good idea, have done exactly this
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73682761
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -421,6 +421,7 @@ public void setDisplayName(String name) {
public void setEntity(EntityLocal entity) {
if (destroyed.get()) throw new IllegalStateException("Cannot set entity on a destroyed entity adjunct");
this.entity = entity;
+ this.execution = ((EntityInternal) entity).getExecutionContext();
--- End diff --
The problem was that `execution` is *NEVER* set otherwise, and it's used in `getContext()` that's what.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #287: Add new member tracking policy with configurable...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/287
LGTM from me as well, merging.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #287: Add new member tracking policy with configurable...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/287
LGTM - one very minor comment for how to dramatically reduce the test time from 3 seconds to a few milliseconds. It adds up to a surprisingly high number when too many of our tests did sleeps!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73682610
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java ---
@@ -1698,6 +1698,7 @@ public void add(Policy policy) {
CatalogUtils.setCatalogItemIdOnAddition(AbstractEntity.this, policy);
policiesInternal.add((AbstractPolicy)policy);
((AbstractPolicy)policy).setEntity(AbstractEntity.this);
+ ConfigConstraints.assertValid(policy);
--- End diff --
No, this asserts that the `ConfigConstraints` on the policy are valid. It had to be moved to cope with the situation where some config keys had values that were resolvable only using the *entities* context. So it must happen *after* `setEntity()` is called on the policy. I fixed enrichers too, just for luck.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73672524
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java ---
@@ -1698,6 +1698,7 @@ public void add(Policy policy) {
CatalogUtils.setCatalogItemIdOnAddition(AbstractEntity.this, policy);
policiesInternal.add((AbstractPolicy)policy);
((AbstractPolicy)policy).setEntity(AbstractEntity.this);
+ ConfigConstraints.assertValid(policy);
--- End diff --
Seems strange to add + activate the policy, and only then assert that it is valid. But I guess it might not be fully initialised before `setEntity` is called, for us to validate it?
Hopefully the policy was instantiated using the normal mechanism (so went through `InternalPolicyFactory.createPolicy`, which will have called `assertValid`.
Are there situations where this code catches something we were previously missing? If so, should we write a **test** for it?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73673387
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalPolicyFactory.java ---
@@ -125,7 +124,6 @@ public InternalPolicyFactory(ManagementContextInternal managementContext) {
for (Map.Entry<ConfigKey<?>, Object> entry : spec.getConfig().entrySet()) {
pol.config().set((ConfigKey)entry.getKey(), entry.getValue());
}
- ConfigConstraints.assertValid(pol);
--- End diff --
Ah, I see you've removed the `assertValid` here. Why? Is that because the policy/enricher won't have finished initialising until `setEntity` etc is called? Is that an indication of a poorly implemented policy? Or the right thing to do?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73673667
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/SshCommandMembershipTrackingPolicy.java ---
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.entity.group;
+
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.config.MapConfigKey;
+import org.apache.brooklyn.core.effector.ssh.SshEffectorTasks;
+import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.location.Machines;
+import org.apache.brooklyn.core.sensor.ssh.SshCommandSensor;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
+import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.core.task.Tasks;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.text.Strings;
+
+/**
+ * Policy which tracks membership of a group, and executes SSH commands
+ * on MEMBER{ADDED,REMOVED} events, as well as SERVICE_UP {true,false} for those members.
+ */
+public class SshCommandMembershipTrackingPolicy extends AbstractMembershipTrackingPolicy {
+
+ private static final Logger LOG = LoggerFactory.getLogger(SshCommandMembershipTrackingPolicy.class);
+
+ public static final ConfigKey<String> EXECUTION_DIR = ConfigKeys.newStringConfigKey("executionDir", "Directory where the command should run; "
+ + "if not supplied, executes in the entity's run dir (or home dir if no run dir is defined); "
+ + "use '~' to always execute in the home dir, or 'custom-feed/' to execute in a custom-feed dir relative to the run dir");
+
+ public static final MapConfigKey<Object> SHELL_ENVIRONMENT = BrooklynConfigKeys.SHELL_ENVIRONMENT;
+
+ public static final ConfigKey<String> UPDATE_COMMAND = ConfigKeys.newStringConfigKey("update.command", "Command to run on membership change events");
+
+ /**
+ * Called when a member is updated or group membership changes.
+ */
+ @Override
+ protected void onEntityEvent(EventType type, Entity entity) {
+ LOG.trace("Event {} received for {} in {}", new Object[] { type, entity, getGroup() });
+ String command = config().get(UPDATE_COMMAND);
+ if (Strings.isNonBlank(command)) {
+ execute(command);
+ }
+ }
+
+ public void execute(String command) {
+ Maybe<SshMachineLocation> machine = Machines.findUniqueMachineLocation(entity.getLocations(), SshMachineLocation.class);
+ if (machine.isAbsentOrNull()) {
+ throw new IllegalStateException("No machine available to execute command");
+ }
+ LOG.info("Executing command on {}: {}", machine.get(), command);
+ String executionDir = config().get(EXECUTION_DIR);
+ String sshCommand = SshCommandSensor.makeCommandExecutingInDirectory(command, executionDir, entity);
+
+ // Set things from the entities defined shell environment, overriding with our config
+ Map<String, Object> env = MutableMap.of();
+ env.putAll(MutableMap.copyOf(entity.config().get(BrooklynConfigKeys.SHELL_ENVIRONMENT)));
+ env.putAll(MutableMap.copyOf(config().get(BrooklynConfigKeys.SHELL_ENVIRONMENT)));
+
+ // Try to resolve the configuration in the env Map
+ try {
+ env = (Map<String, Object>) Tasks.resolveDeepValue(env, Object.class, ((EntityInternal) entity).getExecutionContext());
+ } catch (InterruptedException | ExecutionException e) {
+ Exceptions.propagateIfFatal(e);
--- End diff --
Never swallow exceptions. We should log the exception if we're not propagating it, yes?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73672833
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java ---
@@ -132,7 +132,11 @@ public EntityConfigMap(AbstractEntity entity, Map<ConfigKey<?>, Object> storage)
// Get own value
if (((ConfigKeySelfExtracting<T>)key).isSet(ownConfig)) {
- ownValue = Maybe.of(((ConfigKeySelfExtracting<T>)key).extractValue(ownConfig, exec));
+ Map<ConfigKey<?>, ?> ownCopy = null;
--- End diff --
Very minor: personal preference to not set its value to null when we're about to set its value immediately below that. Let the compiler tell us if someone changing it makes a mistake so the value no longer gets initialised.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #287: Add new member tracking policy with configurable...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-server/pull/287
@aledsage @neykov I added a test and implemented the suggested changes; should be ready for merge now
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73682680
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/internal/EntityConfigMap.java ---
@@ -132,7 +132,11 @@ public EntityConfigMap(AbstractEntity entity, Map<ConfigKey<?>, Object> storage)
// Get own value
if (((ConfigKeySelfExtracting<T>)key).isSet(ownConfig)) {
- ownValue = Maybe.of(((ConfigKeySelfExtracting<T>)key).extractValue(ownConfig, exec));
+ Map<ConfigKey<?>, ?> ownCopy = null;
--- End diff --
But the compiler won't compile if you don't? Or is that just my misleading Eclipse settings...?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #287: Add new member tracking policy with confi...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/287#discussion_r73682845
--- Diff: core/src/main/java/org/apache/brooklyn/entity/group/SshCommandMembershipTrackingPolicy.java ---
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.entity.group;
+
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.config.MapConfigKey;
+import org.apache.brooklyn.core.effector.ssh.SshEffectorTasks;
+import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.location.Machines;
+import org.apache.brooklyn.core.sensor.ssh.SshCommandSensor;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
+import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.core.task.Tasks;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.text.Strings;
+
+/**
+ * Policy which tracks membership of a group, and executes SSH commands
+ * on MEMBER{ADDED,REMOVED} events, as well as SERVICE_UP {true,false} for those members.
+ */
+public class SshCommandMembershipTrackingPolicy extends AbstractMembershipTrackingPolicy {
--- End diff --
OK!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---