Stratos developers,

Please find attached, proposed diffs with further optimizations to the iaas 
specific code so that there is not impact to existing performance.

-Vanson


On 9/20/15, 2:36 AM, Vanson Lim wrote:
On 9/20/15, 2:09 AM, Akila Ravihansa Perera wrote:
Hi Vanson,

Thanks for reporting this. This has been a known issue causing problems when updating IaaS parameters. A work-around would be to redeploy the cartridge definition which forces it to re-build. I haven't tested it though. Your solution also works, but were you able to look into to the performance overhead of re-building every time?


Akila,

No I haven't had a chance to look at the performance overhead, but yes, the suggested diffs are inefficient. Alternatively, for only the partition update case, we could optimize each Iaas's partition validate routines to only call buidIaas() once after all the properties have been updated.

For Example, the revised function for the openstack IAAS is attached.

Currently only the partition validate routines make use of buildIaas:

https://github.com/apache/stratos/search?utf8=✓&q=buildIaas

I'll try updating the cartridge, but I have a hard time see how that works for partition definitions since it would execute the same codeflow below. Only the validate() function below attempts to parse the partition's zone property and propagate it into the IaasProvider definition.

Besides the cartridge update workaround,  do you have any suggestions for other 
ways we could fix this.

-Vanson










On Sun, Sep 20, 2015 at 10:59 AM, Vanson Lim <v...@cisco.com 
<mailto:v...@cisco.com>> wrote:

    Stratos developers,

    I found an issue where defining an availability "zone" property in a 
partition has no affect on controlling the placement of a VM
    launched under openstack.

    The code which builds the template (iaas.initialize) used by jclouds is 
executed only once before stratos appends the availability
    zone properties to the iaas datastructure.

    The problem is in the buildIaas() call shown below:

    
https://github.com/apache/stratos/blob/92ff7e9b5800d578a37d6aa82551d60fbdd66529/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/openstack/OpenstackPartitionValidator.java


    The issue also exists in the cloudstack, ec2, gce and docker variants of 
this function.

            @Override
                public IaasProvider validate(Partition partition, Properties 
properties) throws InvalidPartitionException {
                    try {
                        // validate the existence of the zone and hosts 
properties.
                        if (properties.containsKey(Scope.region.toString())) {
                            String region = 
properties.getProperty(Scope.region.toString());

                            if (iaasProvider.getImage() != null && 
!iaasProvider.getImage().contains(region)) {

                                String msg = "Invalid partition detected, invalid 
region: [partition-id] " + partition.getId() +
                                        " [region] " + region;
                                log.error(msg);
                                throw new InvalidPartitionException(msg);
                            }

                            iaas.isValidRegion(region);

                            IaasProvider updatedIaasProvider = new 
IaasProvider(iaasProvider);
            *                Iaas updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);*
            updatedIaas.setIaasProvider(updatedIaasProvider);

                            if (properties.containsKey(Scope.zone.toString())) {
                                String zone = 
properties.getProperty(Scope.zone.toString());
                                iaas.isValidZone(region, zone);

            
updatedIaasProvider.setProperty(CloudControllerConstants.AVAILABILITY_ZONE, 
zone);
            *                    updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);*
            updatedIaas.setIaasProvider(updatedIaasProvider);
                            }

            updateOtherProperties(updatedIaasProvider, properties);
                            return updatedIaasProvider;
                        } else {

                            return iaasProvider;
                        }
                    } catch (Exception e) {
                        String msg = "Invalid partition detected: [partition-id] 
" + partition.getId() + e.getMessage();
                        log.error(msg, e);
                        throw new InvalidPartitionException(msg, e);
                    }
                }


                private void updateOtherProperties(IaasProvider 
updatedIaasProvider,
                                                   Properties properties) {
                    Iaas updatedIaas;
                    try {
            *            updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);*

                        for (Object property : properties.keySet()) {
                            if (property instanceof String) {
                                String key = (String) property;
            updatedIaasProvider.setProperty(key,
            properties.getProperty(key));
                                if (log.isDebugEnabled()) {
                                    log.debug("Added property " + key
                                            + " to the IaasProvider.");
                                }
                            }
                        }
            *            updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);*
            updatedIaas.setIaasProvider(updatedIaasProvider);
                    } catch (InvalidIaasProviderException ignore) {
                    }

                }



buidIaas() calls getIaas() under the covers and only initializes the compute template the first time it is called. buildIaas() is called a second time after the availability_zone property is set, and then a third/fourth time in
            updateOtherProperties().

            As show below,   getIaas() only calls iaas.initilize() the first 
time it's called.

                public class CloudControllerServiceUtil {

                ...
                    public static Iaas buildIaas(IaasProvider iaasProvider) 
throws InvalidIaasProviderException {
                        return iaasProvider.getIaas();
                    }
                ....

                public class IaasProvider implements Serializable {
                ....
                    public Iaas getIaas() {
                        if (iaas == null) {
                            synchronized (IaasProvider.this) {
                                if (iaas == null) {
                                    try {
                                        iaas = 
CloudControllerUtil.createIaasInstance(this);
                                        iaas.initialize();
                                    } catch (InvalidIaasProviderException e) {
                                        throw new RuntimeException("Could not create 
IaaS instance", e);
                                    }
                                }
                            }
                        }
                        return iaas;
                    }
                ....

            I propose the attached diffs to define buildIaas() such that it 
forces the iaas datastructure to be reinitialized,  if this
            looks okay, I'll see about getting this pushed upstream.
            I've also validated this on my openstack setup.


            Regards,

            -Vanson




--
Akila Ravihansa Perera
WSO2 Inc.; http://wso2.com/

Blog: http://ravihansa3000.blogspot.com


diff --git 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/domain/IaasProvider.java
 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/domain/IaasProvider.java
index f4d7173..2ddc7a9 100644
--- 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/domain/IaasProvider.java
+++ 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/domain/IaasProvider.java
@@ -215,7 +215,7 @@ public class IaasProvider implements Serializable {
         this.className = className;
     }
 
-    public Iaas getIaas() {
+    public Iaas buildIaas() {
         if (iaas == null) {
             synchronized (IaasProvider.this) {
                 if (iaas == null) {
@@ -225,12 +225,27 @@ public class IaasProvider implements Serializable {
                     } catch (InvalidIaasProviderException e) {
                         throw new RuntimeException("Could not create IaaS 
instance", e);
                     }
+                } else {
+                    iaas.initialize();
                 }
             }
+        } else {
+            synchronized (IaasProvider.this) {
+                /** 
+                 * Force iaas to be reinitialized each time this funciton is 
called
+                 */ 
+                iaas.initialize();
+            }
         }
         return iaas;
     }
 
+    public Iaas getIaas() {
+        if (iaas == null) {
+            iaas = buildIaas();
+        }
+        return iaas;
+    }
 
     public byte[] getPayload() {
         return payload;
diff --git 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/cloudstack/CloudStackPartitionValidator.java
 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/cloudstack/CloudStackPartitionValidator.java
index e487f6d..ac9794f 100644
--- 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/cloudstack/CloudStackPartitionValidator.java
+++ 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/cloudstack/CloudStackPartitionValidator.java
@@ -51,22 +51,20 @@ public class CloudStackPartitionValidator implements 
PartitionValidator {
 
         try {
             IaasProvider updatedIaasProvider = new IaasProvider(iaasProvider);
-            Iaas updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-            updatedIaas.setIaasProvider(updatedIaasProvider);
 
             if (properties.containsKey(Scope.zone.toString())) {
                 String zone = properties.getProperty(Scope.zone.toString());
                 iaas.isValidZone(null, zone);
                 
updatedIaasProvider.setProperty(CloudControllerConstants.AVAILABILITY_ZONE, 
zone);
-                updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-                updatedIaas.setIaasProvider(updatedIaasProvider);
             }
 
+            Iaas updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
+            updatedIaas.setIaasProvider(updatedIaasProvider);
+            return updatedIaasProvider;
         } catch (Exception e) {
             String msg = "Invalid partition detected: [partition-id] " + 
partition.getId() + e.getMessage();
             log.error(msg, e);
             throw new InvalidPartitionException(msg, e);
         }
-        return iaasProvider;
     }
-}
\ No newline at end of file
+}
diff --git 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/docker/DockerPartitionValidator.java
 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/docker/DockerPartitionValidator.java
index 908398f..f8fa36e 100644
--- 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/docker/DockerPartitionValidator.java
+++ 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/docker/DockerPartitionValidator.java
@@ -47,34 +47,34 @@ public class DockerPartitionValidator implements 
PartitionValidator {
 
     @Override
     public IaasProvider validate(Partition partition, Properties properties) 
throws InvalidPartitionException {
-        // in Docker case currently we only update the custom properties 
passed via Partitions, and
-        // no validation done as of now.
-        IaasProvider updatedIaasProvider = new IaasProvider(iaasProvider);
-        updateOtherProperties(updatedIaasProvider, properties);
-        return updatedIaasProvider;
+        try {
+            // in Docker case currently we only update the custom properties 
passed via Partitions, and
+            // no validation done as of now.
+            IaasProvider updatedIaasProvider = new IaasProvider(iaasProvider);
+            updateOtherProperties(updatedIaasProvider, properties);
+
+            Iaas updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
+            updatedIaas.setIaasProvider(updatedIaasProvider);
+            return updatedIaasProvider;
+        } catch (Exception e) {
+            String msg = "Invalid partition detected: [partition-id] " + 
partition.getId() + e.getMessage();
+            log.error(msg, e);
+            throw new InvalidPartitionException(msg, e);
+        }
     }
 
     private void updateOtherProperties(IaasProvider updatedIaasProvider,
                                        Properties properties) {
-        Iaas updatedIaas;
-        try {
-            updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-
-            for (Object property : properties.keySet()) {
-                if (property instanceof String) {
-                    String key = (String) property;
-                    updatedIaasProvider.setProperty(key,
+        for (Object property : properties.keySet()) {
+            if (property instanceof String) {
+                String key = (String) property;
+                updatedIaasProvider.setProperty(key,
                             properties.getProperty(key));
-                    if (log.isDebugEnabled()) {
-                        log.debug("Added property " + key
+                if (log.isDebugEnabled()) {
+                    log.debug("Added property " + key
                                 + " to the IaasProvider.");
-                    }
                 }
             }
-            updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-            updatedIaas.setIaasProvider(updatedIaasProvider);
-        } catch (InvalidIaasProviderException ignore) {
         }
-
     }
 }
diff --git 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/ec2/EC2PartitionValidator.java
 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/ec2/EC2PartitionValidator.java
index 03f57df..1272698 100644
--- 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/ec2/EC2PartitionValidator.java
+++ 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/ec2/EC2PartitionValidator.java
@@ -60,18 +60,17 @@ public class EC2PartitionValidator implements 
PartitionValidator {
                 iaas.isValidRegion(region);
 
                 IaasProvider updatedIaasProvider = new 
IaasProvider(iaasProvider);
-                Iaas updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-                updatedIaas.setIaasProvider(updatedIaasProvider);
 
                 if (properties.containsKey(Scope.zone.toString())) {
                     String zone = 
properties.getProperty(Scope.zone.toString());
                     iaas.isValidZone(region, zone);
                     
updatedIaasProvider.setProperty(CloudControllerConstants.AVAILABILITY_ZONE, 
zone);
-                    updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-                    updatedIaas.setIaasProvider(updatedIaasProvider);
                 }
 
                 updateOtherProperties(updatedIaasProvider, properties);
+
+                Iaas updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
+                updatedIaas.setIaasProvider(updatedIaasProvider);
                 return updatedIaasProvider;
 
             } else {
@@ -87,26 +86,16 @@ public class EC2PartitionValidator implements 
PartitionValidator {
 
     private void updateOtherProperties(IaasProvider updatedIaasProvider,
                                        Properties properties) {
-        Iaas updatedIaas;
-        try {
-            updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-
-            for (Object property : properties.keySet()) {
-                if (property instanceof String) {
-                    String key = (String) property;
-                    updatedIaasProvider.setProperty(key,
-                            properties.getProperty(key));
-                    if (log.isDebugEnabled()) {
-                        log.debug("Added property " + key
-                                + " to the IaasProvider.");
-                    }
+        for (Object property : properties.keySet()) {
+            if (property instanceof String) {
+                String key = (String) property;
+                updatedIaasProvider.setProperty(key, 
properties.getProperty(key));
+                if (log.isDebugEnabled()) {
+                    log.debug("Added property " + key
+                              + " to the IaasProvider.");
                 }
             }
-            updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-            updatedIaas.setIaasProvider(updatedIaasProvider);
-        } catch (InvalidIaasProviderException ignore) {
         }
-
     }
 
     @Override
diff --git 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/openstack/OpenstackPartitionValidator.java
 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/openstack/OpenstackPartitionValidator.java
index 24f8e01..65ed16a 100644
--- 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/openstack/OpenstackPartitionValidator.java
+++ 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/iaases/openstack/OpenstackPartitionValidator.java
@@ -61,19 +61,18 @@ public class OpenstackPartitionValidator implements 
PartitionValidator {
                 iaas.isValidRegion(region);
 
                 IaasProvider updatedIaasProvider = new 
IaasProvider(iaasProvider);
-                Iaas updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-                updatedIaas.setIaasProvider(updatedIaasProvider);
 
                 if (properties.containsKey(Scope.zone.toString())) {
                     String zone = 
properties.getProperty(Scope.zone.toString());
                     iaas.isValidZone(region, zone);
 
                     
updatedIaasProvider.setProperty(CloudControllerConstants.AVAILABILITY_ZONE, 
zone);
-                    updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-                    updatedIaas.setIaasProvider(updatedIaasProvider);
                 }
 
                 updateOtherProperties(updatedIaasProvider, properties);
+
+                Iaas updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
+                updatedIaas.setIaasProvider(updatedIaasProvider);
                 return updatedIaasProvider;
             } else {
 
@@ -88,26 +87,17 @@ public class OpenstackPartitionValidator implements 
PartitionValidator {
 
     private void updateOtherProperties(IaasProvider updatedIaasProvider,
                                        Properties properties) {
-        Iaas updatedIaas;
-        try {
-            updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-
-            for (Object property : properties.keySet()) {
-                if (property instanceof String) {
-                    String key = (String) property;
-                    updatedIaasProvider.setProperty(key,
+        for (Object property : properties.keySet()) {
+            if (property instanceof String) {
+                String key = (String) property;
+                updatedIaasProvider.setProperty(key,
                             properties.getProperty(key));
-                    if (log.isDebugEnabled()) {
-                        log.debug("Added property " + key
+                if (log.isDebugEnabled()) {
+                    log.debug("Added property " + key
                                 + " to the IaasProvider.");
-                    }
                 }
             }
-            updatedIaas = 
CloudControllerServiceUtil.buildIaas(updatedIaasProvider);
-            updatedIaas.setIaasProvider(updatedIaasProvider);
-        } catch (InvalidIaasProviderException ignore) {
         }
-
     }
 
     @Override
diff --git 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/services/impl/CloudControllerServiceUtil.java
 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/services/impl/CloudControllerServiceUtil.java
index 37580eb..6533945 100644
--- 
a/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/services/impl/CloudControllerServiceUtil.java
+++ 
b/components/org.apache.stratos.cloud.controller/src/main/java/org/apache/stratos/cloud/controller/services/impl/CloudControllerServiceUtil.java
@@ -45,7 +45,7 @@ public class CloudControllerServiceUtil {
     private static final Log log = 
LogFactory.getLog(CloudControllerServiceUtil.class);
 
     public static Iaas buildIaas(IaasProvider iaasProvider) throws 
InvalidIaasProviderException {
-        return iaasProvider.getIaas();
+        return iaasProvider.buildIaas();
     }
 
     /**

Reply via email to