You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2016/10/06 16:18:24 UTC

[2/3] brooklyn-server git commit: pr371 tidies: instantiating jclouds customizers

pr371 tidies: instantiating jclouds customizers


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/a5470a8b
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/a5470a8b
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/a5470a8b

Branch: refs/heads/master
Commit: a5470a8b31baca9194353ffebaed8c9be8cedd06
Parents: c63a3c1
Author: Aled Sage <al...@gmail.com>
Authored: Thu Oct 6 11:04:04 2016 +0100
Committer: Duncan Grant <du...@cloudsoftcorp.com>
Committed: Thu Oct 6 14:53:11 2016 +0100

----------------------------------------------------------------------
 .../api/location/MachineLocationCustomizer.java |  5 ++
 .../ConfigLocationInheritanceYamlTest.java      |  2 +
 ...loudsCustomizerInstantiationYamlDslTest.java | 59 +++++++++++++++++---
 .../location/jclouds/JcloudsLocation.java       | 15 +++--
 .../location/jclouds/JcloudsLocationConfig.java |  4 +-
 .../jclouds/JcloudsLocationCustomizer.java      | 20 ++++---
 6 files changed, 81 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a5470a8b/api/src/main/java/org/apache/brooklyn/api/location/MachineLocationCustomizer.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/org/apache/brooklyn/api/location/MachineLocationCustomizer.java b/api/src/main/java/org/apache/brooklyn/api/location/MachineLocationCustomizer.java
index a9b4e2e..028efa1 100644
--- a/api/src/main/java/org/apache/brooklyn/api/location/MachineLocationCustomizer.java
+++ b/api/src/main/java/org/apache/brooklyn/api/location/MachineLocationCustomizer.java
@@ -25,6 +25,11 @@ import com.google.common.annotations.Beta;
  * <p>
  * Users are strongly encouraged to sub-class {@link BasicMachineLocationCustomizer}, to give
  * some protection against this {@link Beta} API changing in future releases.
+ * 
+ * Customizers can be instantiated on-demand, so the {@link #customize(MachineLocation)}
+ * and {@link #preRelease(MachineLocation)} methods may not be called on the same instance. 
+ * This is always true after a Brooklyn restart, and may be true at other times depending 
+ * how the customizer has been wired in.
  */
 @Beta
 public interface MachineLocationCustomizer {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a5470a8b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigLocationInheritanceYamlTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigLocationInheritanceYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigLocationInheritanceYamlTest.java
index 6d49947..db84b59 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigLocationInheritanceYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigLocationInheritanceYamlTest.java
@@ -141,6 +141,7 @@ public class ConfigLocationInheritanceYamlTest extends AbstractYamlTest {
     public static class RecordingJcloudsLocation extends JcloudsLocation {
         public final List<ConfigBag> templateConfigs = Lists.newCopyOnWriteArrayList();
         
+        @Override
         public Template buildTemplate(ComputeService computeService, ConfigBag config, Collection<JcloudsLocationCustomizer> customizers) {
             templateConfigs.add(config);
             return super.buildTemplate(computeService, config, customizers);
@@ -161,6 +162,7 @@ public class ConfigLocationInheritanceYamlTest extends AbstractYamlTest {
                 return "jclouds-config-test";
             }
 
+            @Override
             protected Class<? extends JcloudsLocation> getLocationClass() {
                 return RecordingJcloudsLocation.class;
             }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a5470a8b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/JcloudsCustomizerInstantiationYamlDslTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/JcloudsCustomizerInstantiationYamlDslTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/JcloudsCustomizerInstantiationYamlDslTest.java
index 9533280..7083fc2 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/JcloudsCustomizerInstantiationYamlDslTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/JcloudsCustomizerInstantiationYamlDslTest.java
@@ -43,6 +43,8 @@ import org.jclouds.compute.ComputeService;
 import org.jclouds.compute.domain.Template;
 import org.jclouds.compute.domain.TemplateBuilder;
 import org.jclouds.compute.options.TemplateOptions;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Joiner;
@@ -58,6 +60,8 @@ import com.google.common.collect.Lists;
  *
  * e.g.
  *
+ * <pre>
+ * {@code
  * brooklyn.config:
  *   provisioning.properties:
  *     customizers:
@@ -65,13 +69,31 @@ import com.google.common.collect.Lists;
  *       type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
  *       object.fields:
  *         - enabled: $brooklyn:config("kubernetes.sharedsecuritygroup.create")
- *
+ * }
+ * </pre>
  */
 @Test(groups = {"Live", "Live-sanity"})
 public class JcloudsCustomizerInstantiationYamlDslTest extends JcloudsRebindStubYamlTest {
 
     protected Entity origApp;
 
+    @BeforeMethod(alwaysRun=true)
+    @Override
+    public void setUp() throws Exception {
+        RecordingLocationCustomizer.clear();
+        super.setUp();
+    }
+    
+    @AfterMethod(alwaysRun=true)
+    @Override
+    public void tearDown() throws Exception {
+        try {
+            super.tearDown();
+        } finally {
+            RecordingLocationCustomizer.clear();
+        }
+    }
+    
     @Override
     protected JcloudsLocation newJcloudsLocation(ComputeServiceRegistry computeServiceRegistry) throws Exception {
         ByonComputeServiceStaticRef.setInstance(computeServiceRegistry);
@@ -102,7 +124,7 @@ public class JcloudsCustomizerInstantiationYamlDslTest extends JcloudsRebindStub
 
         EntitySpec<?> spec = mgmt().getTypeRegistry().createSpecFromPlan(CampTypePlanTransformer.FORMAT, yaml, RegisteredTypeLoadingContexts.spec(Application.class), EntitySpec.class);
         origApp = mgmt().getEntityManager().createEntity(spec);
-
+        
         return (JcloudsLocation) Iterables.getOnlyElement(origApp.getLocations());
     }
 
@@ -131,36 +153,57 @@ public class JcloudsCustomizerInstantiationYamlDslTest extends JcloudsRebindStub
     public static class RecordingLocationCustomizer extends BasicJcloudsLocationCustomizer {
 
         public static final List<CallParams> calls = Lists.newCopyOnWriteArrayList();
+
+        public static void clear() {
+            calls.clear();
+        }
+
         private Boolean enabled;
 
+        public void setEnabled(Boolean val) {
+            this.enabled = val;
+        }
+        
         @Override
         public void customize(JcloudsLocation location, ComputeService computeService, TemplateBuilder templateBuilder) {
-            calls.add(new CallParams(this, "customize", MutableList.of(location, computeService, templateBuilder)));
+            if (Boolean.TRUE.equals(enabled)) {
+                calls.add(new CallParams(this, "customize", MutableList.of(location, computeService, templateBuilder)));
+            }
         }
 
         @Override
         public void customize(JcloudsLocation location, ComputeService computeService, Template template) {
-            calls.add(new CallParams(this, "customize", MutableList.of(location, computeService, template)));
+            if (Boolean.TRUE.equals(enabled)) {
+                calls.add(new CallParams(this, "customize", MutableList.of(location, computeService, template)));
+            }
         }
 
         @Override
         public void customize(JcloudsLocation location, ComputeService computeService, TemplateOptions templateOptions) {
-            calls.add(new CallParams(this, "customize", MutableList.of(location, computeService, templateOptions)));
+            if (Boolean.TRUE.equals(enabled)) {
+                calls.add(new CallParams(this, "customize", MutableList.of(location, computeService, templateOptions)));
+            }
         }
 
         @Override
         public void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) {
-            calls.add(new CallParams(this, "customize", MutableList.of(location, computeService, machine)));
+            if (Boolean.TRUE.equals(enabled)) {
+                calls.add(new CallParams(this, "customize", MutableList.of(location, computeService, machine)));
+            }
         }
 
         @Override
         public void preRelease(JcloudsMachineLocation machine) {
-            calls.add(new CallParams(this, "preRelease", MutableList.of(machine)));
+            if (Boolean.TRUE.equals(enabled)) {
+                calls.add(new CallParams(this, "preRelease", MutableList.of(machine)));
+            }
         }
 
         @Override
         public void postRelease(JcloudsMachineLocation machine) {
-            calls.add(new CallParams(this, "postRelease", MutableList.of(machine)));
+            if (Boolean.TRUE.equals(enabled)) {
+                calls.add(new CallParams(this, "postRelease", MutableList.of(machine)));
+            }
         }
 
         public static class CallParams {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a5470a8b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
index 8469086..c58bdbd 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
@@ -692,6 +692,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
             Set<? extends NodeMetadata> nodes;
             Template template;
             Collection<JcloudsLocationCustomizer> customizers = getCustomizers(setup);
+            Collection<MachineLocationCustomizer> machineCustomizers = getMachineCustomizers(setup);
+            
             try {
                 // Setup the template
                 template = buildTemplate(computeService, setup, customizers);
@@ -1056,7 +1058,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 LOG.debug("Customizing machine {}, using customizer {}", machineLocation, customizer);
                 customizer.customize(this, computeService, machineLocation);
             }
-            for (MachineLocationCustomizer customizer : getMachineCustomizers(setup)) {
+            for (MachineLocationCustomizer customizer : machineCustomizers) {
                 LOG.debug("Customizing machine {}, using customizer {}", machineLocation, customizer);
                 customizer.customize(machineLocation);
             }
@@ -1640,7 +1642,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         }
         return BrooklynImageChooser.cloneFor(chooser, computeService, config);
     }
-    
+
     /** returns the jclouds Template which describes the image to be built, for the given config and compute service */
     public Template buildTemplate(ComputeService computeService, ConfigBag config, Collection<JcloudsLocationCustomizer> customizers) {
         TemplateBuilder templateBuilder = (TemplateBuilder) config.get(TEMPLATE_BUILDER);
@@ -2727,7 +2729,10 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
         Exception tothrow = null;
 
         ConfigBag setup = config().getBag();
-        for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) {
+        Collection<JcloudsLocationCustomizer> customizers = getCustomizers(setup);
+        Collection<MachineLocationCustomizer> machineCustomizers = getMachineCustomizers(setup);
+        
+        for (JcloudsLocationCustomizer customizer : customizers) {
             try {
                 customizer.preRelease(machine);
             } catch (Exception e) {
@@ -2737,7 +2742,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
                 if (tothrow==null) tothrow = e;
             }
         }
-        for (MachineLocationCustomizer customizer : getMachineCustomizers(setup)) {
+        for (MachineLocationCustomizer customizer : machineCustomizers) {
             customizer.preRelease(machine);
         }
 
@@ -2764,7 +2769,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
 
         removeChild(machine);
 
-        for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) {
+        for (JcloudsLocationCustomizer customizer : customizers) {
             try {
                 customizer.postRelease(machine);
             } catch (Exception e) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a5470a8b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
index d14c866..3c07f4b 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java
@@ -203,13 +203,13 @@ public interface JcloudsLocationConfig extends CloudLocationConfig {
     /** @deprecated since 0.7.0; use {@link #JCLOUDS_LOCATION_CUSTOMIZERS} instead */
     @Deprecated
     public static final ConfigKey<String> JCLOUDS_LOCATION_CUSTOMIZER_TYPE = ConfigKeys.newStringConfigKey(
-            "customizerType", "Optional location customizer type (to be class-loaded and constructed with no-arg constructor)");
+            "customizerType", "Optional location customizer type (to be class-loaded and constructed with either a ConfigBag or no-arg constructor)");
 
     /** @deprecated since 0.7.0; use {@link #JCLOUDS_LOCATION_CUSTOMIZERS} instead */
     @Deprecated
     public static final ConfigKey<String> JCLOUDS_LOCATION_CUSTOMIZERS_SUPPLIER_TYPE = ConfigKeys.newStringConfigKey(
             "customizersSupplierType", "Optional type of a Supplier<Collection<JcloudsLocationCustomizer>> " +
-            "(to be class-loaded and constructed with ConfigBag or no-arg constructor)");
+            "(to be class-loaded and constructed with either a ConfigBag or no-arg constructor)");
 
     public static final ConfigKey<String> LOCAL_TEMP_DIR = SshTool.PROP_LOCAL_TEMP_DIR;
     

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a5470a8b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationCustomizer.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationCustomizer.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationCustomizer.java
index 89e22bb..192077b 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationCustomizer.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationCustomizer.java
@@ -23,19 +23,21 @@ import org.jclouds.compute.domain.Template;
 import org.jclouds.compute.domain.TemplateBuilder;
 import org.jclouds.compute.options.TemplateOptions;
 
-import org.apache.brooklyn.util.core.config.ConfigBag;
-
 /**
  * Customization hooks to allow apps to perform specific customisation at each stage of jclouds machine provisioning.
  * For example, an app could attach an EBS volume to an EC2 node, or configure a desired availability zone.
  * <p>
- * Instances will be invoked with the {@link ConfigBag} being used to obtain a machine by the
- * {@link JcloudsLocation} if such a constructor exists. If not, the default no argument constructor
- * will be invoked.
- *
- * Customizers are not persisted so the pre and postRelease will not be called on the same instance as was used in
- * provisioning.  However the customize functions will be called on the same instance unless brooklyn is stopped, in which
- * case vm provisioning would fail anyway.
+ * Users are strongly encouraged to sub-class {@link org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer}, 
+ * to give some protection against this API changing in future releases.
+ * <p>
+ * Customizers can be instantiated on-demand, so the {@link #postRelease(JcloudsMachineLocation)}
+ * and {@link #postRelease(JcloudsMachineLocation)} methods may not be called on the same instance 
+ * as was used in provisioning. This is always true after a Brooklyn restart, and may be true at 
+ * other times depending how the customizer has been wired in.
+ * <p>
+ * However the customize functions will be called sequentially on the same instance during provisioning,
+ * unless Brooklyn is stopped (or fails over to a high-availability standby), in which case VM 
+ * provisioning would abort anyway.
  */
 public interface JcloudsLocationCustomizer {