Roy Golan has uploaded a new change for review.

Change subject: core: VdsManager - cleanup
......................................................................

core: VdsManager - cleanup

* seperate fields from methods
* encapsulate important parts into methods
* extract interface from important methods interaction

Change-Id: Ib4062bc376d22253b045ad022c67f3f10da0b100
Signed-off-by: Roy Golan <[email protected]>
---
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
A 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManagerFacade.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsServerWrapper.java
4 files changed, 160 insertions(+), 91 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/74/27374/1

diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
index 8ed9d93..1cbe9e1 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
@@ -54,83 +54,32 @@
 import org.ovirt.engine.core.vdsbroker.vdsbroker.VdsServerWrapper;
 import org.ovirt.engine.core.vdsbroker.xmlrpc.XmlRpcUtils;
 
-public class VdsManager {
+public class VdsManager implements VdsManagerFacade {
+    private static Log log = LogFactory.getLog(VdsManager.class);
+    private static Map<Guid, String> recoveringJobIdMap = new 
ConcurrentHashMap<Guid, String>();
+    private final int _numberRefreshesBeforeSave = Config.<Integer> 
getValue(ConfigValues.NumberVmRefreshesBeforeSave);
+    private final Object _lockObj = new Object();
+    private final AtomicInteger mFailedToRunVmAttempts;
+    private final AtomicInteger mUnrespondedAttempts;
+    private final AtomicBoolean sshSoftFencingExecuted;
+    private final Guid _vdsId;
+    private final VdsMonitor vdsMonitor = new VdsMonitor();
     private VDS _vds;
     private long lastUpdate;
     private long updateStartTime;
     private long nextMaintenanceAttemptTime;
-
-    private static final Log log = LogFactory.getLog(VdsManager.class);
-
-    public boolean getRefreshStatistics() {
-        return (_refreshIteration == _numberRefreshesBeforeSave);
-    }
-
     private int VDS_REFRESH_RATE = Config.<Integer> 
getValue(ConfigValues.VdsRefreshRate) * 1000;
-
     private String onTimerJobId;
-
-    private final int _numberRefreshesBeforeSave = Config.<Integer> 
getValue(ConfigValues.NumberVmRefreshesBeforeSave);
     private int _refreshIteration = 1;
-
-    private final Object _lockObj = new Object();
-    private static Map<Guid, String> recoveringJobIdMap = new 
ConcurrentHashMap<Guid, String>();
     private boolean isSetNonOperationalExecuted;
     private MonitoringStrategy monitoringStrategy;
     private EngineLock monitoringLock;
-    public Object getLockObj() {
-        return _lockObj;
-    }
-
-    public static void cancelRecoveryJob(Guid vdsId) {
-        String jobId = recoveringJobIdMap.remove(vdsId);
-        if (jobId != null) {
-            log.infoFormat("Cancelling the recovery from crash timer for VDS 
{0} because vds started initializing", vdsId);
-            try {
-                SchedulerUtilQuartzImpl.getInstance().deleteJob(jobId);
-            } catch (Exception e) {
-                log.warnFormat("Failed deleting job {0} at cancelRecoveryJob", 
jobId);
-            }
-        }
-    }
-
-    private final AtomicInteger mFailedToRunVmAttempts;
-    private final AtomicInteger mUnrespondedAttempts;
-    private final AtomicBoolean sshSoftFencingExecuted;
-
     private int VDS_DURING_FAILURE_TIMEOUT_IN_MINUTES = Config
             .<Integer> 
getValue(ConfigValues.TimeToReduceFailedRunOnVdsInMinutes);
     private boolean privateInitialized;
-
-    public boolean getInitialized() {
-        return privateInitialized;
-    }
-
-    public void setInitialized(boolean value) {
-        privateInitialized = value;
-    }
-
     private IVdsServer _vdsProxy;
-
-    public IVdsServer getVdsProxy() {
-        return _vdsProxy;
-    }
-
-    public Guid getVdsId() {
-        return _vdsId;
-    }
-
     private boolean mBeforeFirstRefresh = true;
-
-    public boolean getbeforeFirstRefresh() {
-        return mBeforeFirstRefresh;
-    }
-
-    public void setbeforeFirstRefresh(boolean value) {
-        mBeforeFirstRefresh = value;
-    }
-
-    private final Guid _vdsId;
+    private VdsUpdateRunTimeInfo _vdsUpdater;
 
     private VdsManager(VDS vds) {
         log.info("Entered VdsManager constructor");
@@ -143,11 +92,14 @@
         monitoringLock = new 
EngineLock(Collections.singletonMap(_vdsId.toString(),
                 new Pair<String, String>(LockingGroup.VDS_INIT.name(), "")), 
null);
 
-        if (_vds.getStatus() == VDSStatus.PreparingForMaintenance) {
-            _vds.setPreviousStatus(_vds.getStatus());
-        } else {
-            _vds.setPreviousStatus(VDSStatus.Up);
-        }
+        handlePreviousStatus();
+        handleSecureSetup();
+        InitVdsBroker();
+        _vds = null;
+    }
+
+    @Override
+    public void handleSecureSetup() {
         // if ssl is on and no certificate file
         if (Config.<Boolean> getValue(ConfigValues.EncryptHostCommunication)
                 && !EngineEncryptionUtils.haveKey()) {
@@ -159,9 +111,15 @@
             AuditLogableBase logable = new AuditLogableBase(_vdsId);
             AuditLogDirector.log(logable, 
AuditLogType.CERTIFICATE_FILE_NOT_FOUND);
         }
-        InitVdsBroker();
-        _vds = null;
+    }
 
+    @Override
+    public void handlePreviousStatus() {
+        if (_vds.getStatus() == VDSStatus.PreparingForMaintenance) {
+            _vds.setPreviousStatus(_vds.getStatus());
+        } else {
+            _vds.setPreviousStatus(VDSStatus.Up);
+        }
     }
 
     public static VdsManager buildVdsManager(VDS vds) {
@@ -169,6 +127,7 @@
         return vdsManager;
     }
 
+    @Override
     public void schedulJobs() {
         SchedulerUtil sched = SchedulerUtilQuartzImpl.getInstance();
         // start with refresh statistics
@@ -180,29 +139,13 @@
 
     private void InitVdsBroker() {
         log.infoFormat("Initialize vdsBroker ({0},{1})", _vds.getHostName(), 
_vds.getPort());
-
-        // Get the values of the timeouts:
-        int clientTimeOut = Config.<Integer> getValue(ConfigValues.vdsTimeout) 
* 1000;
-        int connectionTimeOut = 
Config.<Integer>getValue(ConfigValues.vdsConnectionTimeout) * 1000;
-        int clientRetries = Config.<Integer>getValue(ConfigValues.vdsRetries);
-
-        Pair<VdsServerConnector, HttpClient> returnValue =
-                XmlRpcUtils.getConnection(_vds.getHostName(),
-                        _vds.getPort(),
-                        clientTimeOut,
-                        connectionTimeOut,
-                        clientRetries,
-                        VdsServerConnector.class,
-                        Config.<Boolean> 
getValue(ConfigValues.EncryptHostCommunication));
-        _vdsProxy = new VdsServerWrapper(returnValue.getFirst(), 
returnValue.getSecond());
+        _vdsProxy = new VdsServerWrapper(_vds.getHostName(), _vds.getPort());
     }
 
+    @Override
     public void updateVmDynamic(VmDynamic vmDynamic) {
         DbFacade.getInstance().getVmDynamicDao().update(vmDynamic);
     }
-
-    private VdsUpdateRunTimeInfo _vdsUpdater;
-    private final VdsMonitor vdsMonitor = new VdsMonitor();
 
     @OnTimerMethodAnnotation("onTimer")
     public void onTimer() {
@@ -306,6 +249,7 @@
                 ExceptionUtils.getMessage(ex));
     }
 
+    @Override
     public boolean isMonitoringNeeded() {
         return (monitoringStrategy.isMonitoringNeeded(_vds) &&
                 _vds.getStatus() != VDSStatus.Installing &&
@@ -364,6 +308,7 @@
      *
      * @param dynamicData
      */
+    @Override
     public void updateDynamicData(VdsDynamic dynamicData) {
         DbFacade.getInstance().getVdsDynamicDao().updateIfNeeded(dynamicData);
     }
@@ -373,10 +318,29 @@
      *
      * @param statisticsData
      */
+    @Override
     public void updateStatisticsData(VdsStatistics statisticsData) {
         DbFacade.getInstance().getVdsStatisticsDao().update(statisticsData);
     }
 
+<<<<<<< HEAD
+=======
+    @Override
+    public VDS activate() {
+        VDS vds = null;
+        try {
+            vds = DbFacade.getInstance().getVdsDao().get(getVdsId());
+            refreshHost(vds);
+        } catch (Exception e) {
+            log.infoFormat("Failed to activate VDS = {0} with error: {1}.",
+                    getVdsId(), e.getMessage());
+        }
+
+        return vds;
+    }
+
+    @Override
+>>>>>>> core: VdsManager - cleanup
     public void refreshHost(VDS vds) {
         try {
             refreshCapabilities(new AtomicBoolean(), vds);
@@ -393,6 +357,7 @@
         }
     }
 
+    @Override
     public void setStatus(VDSStatus status, VDS vds) {
         synchronized (getLockObj()) {
             if (vds == null) {
@@ -504,6 +469,7 @@
         ResourceManager.getInstance().succededToRunVm(vmId, _vds.getId());
     }
 
+    @Override
     public VDSStatus refreshCapabilities(AtomicBoolean 
processHardwareCapsNeeded, VDS vds) {
         log.debugFormat("monitoring: refresh {0} capabilities", vds);
         VDS oldVDS = vds.clone();
@@ -598,6 +564,7 @@
      * @param ex
      * @return
      */
+    @Override
     public boolean handleNetworkException(VDSNetworkException ex, VDS vds) {
         if (vds.getStatus() != VDSStatus.Down) {
             long timeoutToFence = calcTimeoutToFence(vds.getVmCount(), 
vds.getSpmStatus());
@@ -721,4 +688,48 @@
     public boolean isTimeToRetryMaintenance() {
         return System.currentTimeMillis() > nextMaintenanceAttemptTime;
     }
+
+    public boolean getInitialized() {
+        return privateInitialized;
+    }
+
+    public void setInitialized(boolean value) {
+        privateInitialized = value;
+    }
+
+    public IVdsServer getVdsProxy() {
+        return _vdsProxy;
+    }
+
+    public Guid getVdsId() {
+        return _vdsId;
+    }
+
+    public static void cancelRecoveryJob(Guid vdsId) {
+        String jobId = recoveringJobIdMap.remove(vdsId);
+        if (jobId != null) {
+            log.infoFormat("Cancelling the recovery from crash timer for VDS 
{0} because vds started initializing", vdsId);
+            try {
+                SchedulerUtilQuartzImpl.getInstance().deleteJob(jobId);
+            } catch (Exception e) {
+                log.warnFormat("Failed deleting job {0} at cancelRecoveryJob", 
jobId);
+            }
+        }
+    }
+
+    public boolean getRefreshStatistics() {
+        return (_refreshIteration == _numberRefreshesBeforeSave);
+    }
+
+    public Object getLockObj() {
+        return _lockObj;
+    }
+
+    public boolean getbeforeFirstRefresh() {
+        return mBeforeFirstRefresh;
+    }
+
+    public void setbeforeFirstRefresh(boolean value) {
+        mBeforeFirstRefresh = value;
+    }
 }
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManagerFacade.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManagerFacade.java
new file mode 100644
index 0000000..a9adf56
--- /dev/null
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManagerFacade.java
@@ -0,0 +1,39 @@
+package org.ovirt.engine.core.vdsbroker;
+
+import org.ovirt.engine.core.common.businessentities.VDS;
+import org.ovirt.engine.core.common.businessentities.VDSStatus;
+import org.ovirt.engine.core.common.businessentities.VdsDynamic;
+import org.ovirt.engine.core.common.businessentities.VdsStatistics;
+import org.ovirt.engine.core.common.businessentities.VmDynamic;
+import org.ovirt.engine.core.vdsbroker.vdsbroker.VDSNetworkException;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * Created by rgolan on 4/2/14.
+ */
+public interface VdsManagerFacade {
+    void handleSecureSetup();
+
+    void handlePreviousStatus();
+
+    void schedulJobs();
+
+    void updateVmDynamic(VmDynamic vmDynamic);
+
+    boolean isMonitoringNeeded();
+
+    void updateDynamicData(VdsDynamic dynamicData);
+
+    void updateStatisticsData(VdsStatistics statisticsData);
+
+    VDS activate();
+
+    void refreshHost(VDS vds);
+
+    void setStatus(VDSStatus status, VDS vds);
+
+    VDSStatus refreshCapabilities(AtomicBoolean processHardwareCapsNeeded, VDS 
vds);
+
+    boolean handleNetworkException(VDSNetworkException ex, VDS vds);
+}
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
index b923f79..8c470a2 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
@@ -536,7 +536,7 @@
         moveVDSToMaintenanceIfNeeded();
     }
 
-    private void refreshVdsStats() {
+    public void refreshVdsStats() {
         if (Config.<Boolean> getValue(ConfigValues.DebugTimerLogging)) {
             log.debugFormat("vdsManager::refreshVdsStats entered, vds = {0} : 
{1}", _vds.getId(),
                     _vds.getName());
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsServerWrapper.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsServerWrapper.java
index c3073cd..5d2a6aa 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsServerWrapper.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsServerWrapper.java
@@ -6,6 +6,9 @@
 import java.util.concurrent.FutureTask;
 
 import org.apache.commons.httpclient.HttpClient;
+import org.ovirt.engine.core.common.config.Config;
+import org.ovirt.engine.core.common.config.ConfigValues;
+import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.compat.Guid;
 import 
org.ovirt.engine.core.vdsbroker.gluster.GlusterHookContentInfoReturnForXmlRpc;
 import org.ovirt.engine.core.vdsbroker.gluster.GlusterHooksListReturnForXmlRpc;
@@ -22,6 +25,7 @@
 import org.ovirt.engine.core.vdsbroker.irsbroker.OneUuidReturnForXmlRpc;
 import 
org.ovirt.engine.core.vdsbroker.irsbroker.StoragePoolInfoReturnForXmlRpc;
 import org.ovirt.engine.core.vdsbroker.xmlrpc.XmlRpcRunTimeException;
+import org.ovirt.engine.core.vdsbroker.xmlrpc.XmlRpcUtils;
 
 @SuppressWarnings({"rawtypes", "unchecked"})
 public class VdsServerWrapper implements IVdsServer {
@@ -29,9 +33,24 @@
     private final VdsServerConnector vdsServer;
     private final HttpClient httpClient;
 
-    public VdsServerWrapper(VdsServerConnector innerImplementor, HttpClient 
httpClient) {
-        this.vdsServer = innerImplementor;
-        this.httpClient = httpClient;
+    public VdsServerWrapper(String hostname, int port) {
+        // Get the values of the timeouts:
+        int clientTimeOut = Config.<Integer> getValue(ConfigValues.vdsTimeout) 
* 1000;
+        int connectionTimeOut = 
Config.<Integer>getValue(ConfigValues.vdsConnectionTimeout) * 1000;
+        int clientRetries = Config.<Integer>getValue(ConfigValues.vdsRetries);
+
+        Pair<VdsServerConnector, HttpClient> returnValue =
+                XmlRpcUtils.getConnection(
+                        hostname,
+                        port,
+                        clientTimeOut,
+                        connectionTimeOut,
+                        clientRetries,
+                        VdsServerConnector.class,
+                        
Config.<Boolean>getValue(ConfigValues.EncryptHostCommunication));
+
+        this.vdsServer = returnValue.getFirst();
+        this.httpClient = returnValue.getSecond();
     }
 
     public HttpClient getHttpClient() {


-- 
To view, visit http://gerrit.ovirt.org/27374
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib4062bc376d22253b045ad022c67f3f10da0b100
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to