Merge pull request #714 from rafaelweingartner/master-lrg-cs-hackday-003

Changed variable s_logger to non-static and fixed its name in com.cloud.utils.component.ComponentLifecycleBase and its subclassesHi guys,
We have noticed that every single class that is a subclass of ComponentLifecycleBase instantiate their on logger manually and uses a nonstandard name. We fixed that by changing the variable in ComponentLifecycleBase to protected and non-static and instantiated it using the method getClass from Object class. Therefore, we can reduce the code in a few hundred lines and use a more intuitive name for the logger variable.

During that process we found a static method that used the s_logger variable in classes:
com.cloud.network.element.VirtualRouterElement
org.apache.cloudstack.network.element.InternalLoadBalancerElement

To fix that we had to create a new class com.cloud.network.element.HAProxyLBRule, instantiate it with @componente and inject into the aforementioned classes.

The class that we create is com.cloud.network.element.HAProxyLBRule and has the following methods:
com.cloud.network.element.HAProxyLBRule.containsOnlyNumbers(String, String)
com.cloud.network.element.HAProxyLBRule.validateHAProxyLBRule(LoadBalancingRule)

Sadly we could not write test cases to it; hence we did not fully understand those methods. However, if anyone out there understands it, we would appreciate some code to be added to it.

As minor this change may seem; we believe that it enhances a little bit the ACS code by using standard name to logger variable.

* pr/714:
  Solved jira ticket: CLOUDSTACK-8750

Signed-off-by: Daan Hoogland <daan@onecht.net>

Author: DaanHoogland

core/src/com/cloud/storage/template/IsoProcessor.java

@@ -25,29 +25,27 @@
 import javax.ejb.Local;
 import javax.naming.ConfigurationException;
 
-import org.apache.log4j.Logger;
 
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.storage.StorageLayer;
 import com.cloud.utils.component.AdapterBase;
 
 @Local(value = Processor.class)
 public class IsoProcessor extends AdapterBase implements Processor {
-    private static final Logger s_logger = Logger.getLogger(IsoProcessor.class);
 
     StorageLayer _storage;
 
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) {
         if (format != null) {
-            s_logger.debug("We don't handle conversion from " + format + " to ISO.");
+            logger.debug("We don't handle conversion from " + format + " to ISO.");
             return null;
         }
 
         String isoPath = templatePath + File.separator + templateName + "." + ImageFormat.ISO.getFileExtension();
 
         if (!_storage.exists(isoPath)) {
-            s_logger.debug("Unable to find the iso file: " + isoPath);
+            logger.debug("Unable to find the iso file: " + isoPath);
             return null;
         }
 

core/src/com/cloud/storage/template/OVAProcessor.java

@@ -26,7 +26,6 @@
 import javax.naming.ConfigurationException;
 import javax.xml.parsers.DocumentBuilderFactory;
 
-import org.apache.log4j.Logger;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 
@@ -38,39 +37,38 @@
 
 @Local(value = Processor.class)
 public class OVAProcessor extends AdapterBase implements Processor {
-    private static final Logger s_logger = Logger.getLogger(OVAProcessor.class);
 
     StorageLayer _storage;
 
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
         if (format != null) {
-            if (s_logger.isInfoEnabled()) {
-                s_logger.info("We currently don't handle conversion from " + format + " to OVA.");
+            if (logger.isInfoEnabled()) {
+                logger.info("We currently don't handle conversion from " + format + " to OVA.");
             }
             return null;
         }
 
-        s_logger.info("Template processing. templatePath: " + templatePath + ", templateName: " + templateName);
+        logger.info("Template processing. templatePath: " + templatePath + ", templateName: " + templateName);
         String templateFilePath = templatePath + File.separator + templateName + "." + ImageFormat.OVA.getFileExtension();
         if (!_storage.exists(templateFilePath)) {
-            if (s_logger.isInfoEnabled()) {
-                s_logger.info("Unable to find the vmware template file: " + templateFilePath);
+            if (logger.isInfoEnabled()) {
+                logger.info("Unable to find the vmware template file: " + templateFilePath);
             }
             return null;
         }
 
-        s_logger.info("Template processing - untar OVA package. templatePath: " + templatePath + ", templateName: " + templateName);
+        logger.info("Template processing - untar OVA package. templatePath: " + templatePath + ", templateName: " + templateName);
         String templateFileFullPath = templatePath + File.separator + templateName + "." + ImageFormat.OVA.getFileExtension();
         File templateFile = new File(templateFileFullPath);
 
-        Script command = new Script("tar", 0, s_logger);
+        Script command = new Script("tar", 0, logger);
         command.add("--no-same-owner");
         command.add("-xf", templateFileFullPath);
         command.setWorkDir(templateFile.getParent());
         String result = command.execute();
         if (result != null) {
-            s_logger.info("failed to untar OVA package due to " + result + ". templatePath: " + templatePath + ", templateName: " + templateName);
+            logger.info("failed to untar OVA package due to " + result + ". templatePath: " + templatePath + ", templateName: " + templateName);
             return null;
         }
 
@@ -91,7 +89,7 @@ public long getVirtualSize(File file) {
             long size = getTemplateVirtualSize(file.getParent(), file.getName());
             return size;
         } catch (Exception e) {
-            s_logger.info("[ignored]"
+            logger.info("[ignored]"
                     + "failed to get virtual template size for ova: " + e.getLocalizedMessage());
         }
         return file.length();
@@ -105,7 +103,7 @@ public long getTemplateVirtualSize(String templatePath, String templateName) thr
         String ovfFileName = getOVFFilePath(templateFileFullPath);
         if (ovfFileName == null) {
             String msg = "Unable to locate OVF file in template package directory: " + templatePath;
-            s_logger.error(msg);
+            logger.error(msg);
             throw new InternalErrorException(msg);
         }
         try {
@@ -130,7 +128,7 @@ public long getTemplateVirtualSize(String templatePath, String templateName) thr
             return virtualSize;
         } catch (Exception e) {
             String msg = "Unable to parse OVF XML document to get the virtual disk size due to" + e;
-            s_logger.error(msg);
+            logger.error(msg);
             throw new InternalErrorException(msg);
         }
     }

core/src/com/cloud/storage/template/QCOW2Processor.java

@@ -27,7 +27,6 @@
 import javax.ejb.Local;
 import javax.naming.ConfigurationException;
 
-import org.apache.log4j.Logger;
 
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.storage.StorageLayer;
@@ -36,22 +35,21 @@
 
 @Local(value = Processor.class)
 public class QCOW2Processor extends AdapterBase implements Processor {
-    private static final Logger s_logger = Logger.getLogger(QCOW2Processor.class);
     private static final int VIRTUALSIZE_HEADER_LOCATION = 24;
 
     private StorageLayer _storage;
 
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) {
         if (format != null) {
-            s_logger.debug("We currently don't handle conversion from " + format + " to QCOW2.");
+            logger.debug("We currently don't handle conversion from " + format + " to QCOW2.");
             return null;
         }
 
         String qcow2Path = templatePath + File.separator + templateName + "." + ImageFormat.QCOW2.getFileExtension();
 
         if (!_storage.exists(qcow2Path)) {
-            s_logger.debug("Unable to find the qcow2 file: " + qcow2Path);
+            logger.debug("Unable to find the qcow2 file: " + qcow2Path);
             return null;
         }
 
@@ -66,7 +64,7 @@ public FormatInfo process(String templatePath, ImageFormat format, String templa
         try {
             info.virtualSize = getVirtualSize(qcow2File);
         } catch (IOException e) {
-            s_logger.error("Unable to get virtual size from " + qcow2File.getName());
+            logger.error("Unable to get virtual size from " + qcow2File.getName());
             return null;
         }
 

core/src/com/cloud/storage/template/RawImageProcessor.java

@@ -25,7 +25,6 @@
 import javax.ejb.Local;
 import javax.naming.ConfigurationException;
 
-import org.apache.log4j.Logger;
 
 import com.cloud.exception.InternalErrorException;
 import com.cloud.storage.Storage.ImageFormat;
@@ -34,7 +33,6 @@
 
 @Local(value = Processor.class)
 public class RawImageProcessor extends AdapterBase implements Processor {
-    private static final Logger s_logger = Logger.getLogger(RawImageProcessor.class);
     StorageLayer _storage;
 
     @Override
@@ -50,21 +48,21 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
         if (format != null) {
-            s_logger.debug("We currently don't handle conversion from " + format + " to raw image.");
+            logger.debug("We currently don't handle conversion from " + format + " to raw image.");
             return null;
         }
 
         String imgPath = templatePath + File.separator + templateName + "." + ImageFormat.RAW.getFileExtension();
         if (!_storage.exists(imgPath)) {
-            s_logger.debug("Unable to find raw image:" + imgPath);
+            logger.debug("Unable to find raw image:" + imgPath);
             return null;
         }
         FormatInfo info = new FormatInfo();
         info.format = ImageFormat.RAW;
         info.filename = templateName + "." + ImageFormat.RAW.getFileExtension();
         info.size = _storage.getSize(imgPath);
         info.virtualSize = info.size;
-        s_logger.debug("Process raw image " + info.filename + " successfully");
+        logger.debug("Process raw image " + info.filename + " successfully");
         return info;
     }
 

core/src/com/cloud/storage/template/TARProcessor.java

@@ -22,7 +22,6 @@
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.storage.StorageLayer;
 import com.cloud.utils.component.AdapterBase;
-import org.apache.log4j.Logger;
 
 import javax.ejb.Local;
 import javax.naming.ConfigurationException;
@@ -31,21 +30,20 @@
 
 @Local(value = Processor.class)
 public class TARProcessor extends AdapterBase implements Processor {
-    private static final Logger s_logger = Logger.getLogger(TARProcessor.class);
 
     private StorageLayer _storage;
 
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) {
         if (format != null) {
-            s_logger.debug("We currently don't handle conversion from " + format + " to TAR.");
+            logger.debug("We currently don't handle conversion from " + format + " to TAR.");
             return null;
         }
 
         String tarPath = templatePath + File.separator + templateName + "." + ImageFormat.TAR.getFileExtension();
 
         if (!_storage.exists(tarPath)) {
-            s_logger.debug("Unable to find the tar file: " + tarPath);
+            logger.debug("Unable to find the tar file: " + tarPath);
             return null;
         }
 

core/src/com/cloud/storage/template/VhdProcessor.java

@@ -27,7 +27,6 @@
 import javax.ejb.Local;
 import javax.naming.ConfigurationException;
 
-import org.apache.log4j.Logger;
 
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.storage.StorageLayer;
@@ -43,7 +42,6 @@
 @Local(value = Processor.class)
 public class VhdProcessor extends AdapterBase implements Processor {
 
-    private static final Logger s_logger = Logger.getLogger(VhdProcessor.class);
     StorageLayer _storage;
     private int vhdFooterSize = 512;
     private int vhdFooterCreatorAppOffset = 28;
@@ -54,13 +52,13 @@ public class VhdProcessor extends AdapterBase implements Processor {
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) {
         if (format != null) {
-            s_logger.debug("We currently don't handle conversion from " + format + " to VHD.");
+            logger.debug("We currently don't handle conversion from " + format + " to VHD.");
             return null;
         }
 
         String vhdPath = templatePath + File.separator + templateName + "." + ImageFormat.VHD.getFileExtension();
         if (!_storage.exists(vhdPath)) {
-            s_logger.debug("Unable to find the vhd file: " + vhdPath);
+            logger.debug("Unable to find the vhd file: " + vhdPath);
             return null;
         }
 
@@ -74,7 +72,7 @@ public FormatInfo process(String templatePath, ImageFormat format, String templa
         try {
             info.virtualSize = getVirtualSize(vhdFile);
         } catch (IOException e) {
-            s_logger.error("Unable to get the virtual size for " + vhdPath);
+            logger.error("Unable to get the virtual size for " + vhdPath);
             return null;
         }
 

core/src/com/cloud/storage/template/VmdkProcessor.java

@@ -31,7 +31,6 @@
 import javax.ejb.Local;
 import javax.naming.ConfigurationException;
 
-import org.apache.log4j.Logger;
 
 import com.cloud.exception.InternalErrorException;
 import com.cloud.storage.Storage.ImageFormat;
@@ -40,24 +39,23 @@
 
 @Local(value = Processor.class)
 public class VmdkProcessor extends AdapterBase implements Processor {
-    private static final Logger s_logger = Logger.getLogger(VmdkProcessor.class);
 
     StorageLayer _storage;
 
     @Override
     public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
         if (format != null) {
-            if (s_logger.isInfoEnabled()) {
-                s_logger.info("We currently don't handle conversion from " + format + " to VMDK.");
+            if (logger.isInfoEnabled()) {
+                logger.info("We currently don't handle conversion from " + format + " to VMDK.");
             }
             return null;
         }
 
-        s_logger.info("Template processing. templatePath: " + templatePath + ", templateName: " + templateName);
+        logger.info("Template processing. templatePath: " + templatePath + ", templateName: " + templateName);
         String templateFilePath = templatePath + File.separator + templateName + "." + ImageFormat.VMDK.getFileExtension();
         if (!_storage.exists(templateFilePath)) {
-            if (s_logger.isInfoEnabled()) {
-                s_logger.info("Unable to find the vmware template file: " + templateFilePath);
+            if (logger.isInfoEnabled()) {
+                logger.info("Unable to find the vmware template file: " + templateFilePath);
             }
             return null;
         }
@@ -77,7 +75,7 @@ public long getVirtualSize(File file) {
             long size = getTemplateVirtualSize(file.getParent(), file.getName());
             return size;
         } catch (Exception e) {
-            s_logger.info("[ignored]"
+            logger.info("[ignored]"
                     + "failed to get template virtual size for vmdk: " + e.getLocalizedMessage());
         }
         return file.length();
@@ -103,15 +101,15 @@ public long getTemplateVirtualSize(String templatePath, String templateName) thr
             }
         } catch(FileNotFoundException ex) {
             String msg = "Unable to open file '" + templateFileFullPath + "' " + ex.toString();
-            s_logger.error(msg);
+            logger.error(msg);
             throw new InternalErrorException(msg);
         } catch(IOException ex) {
             String msg = "Unable read open file '" + templateFileFullPath + "' " + ex.toString();
-            s_logger.error(msg);
+            logger.error(msg);
             throw new InternalErrorException(msg);
         }
 
-        s_logger.debug("vmdk file had size="+virtualSize);
+        logger.debug("vmdk file had size="+virtualSize);
         return virtualSize;
     }