From 3674e80219d0be9e940bd948060fa10a467e4b7e Mon Sep 17 00:00:00 2001 From: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com> Date: Mon, 10 May 2021 04:47:42 -0300 Subject: [PATCH] Improve logs on SecondaryStorageManagerImpl and few refactors (#4955) Co-authored-by: Daniel Augusto Veronezi Salvador --- .../main/java/com/cloud/dc/DataCenterVO.java | 6 + .../main/java/com/cloud/vm/VMInstanceVO.java | 7 +- .../SecondaryStorageManagerImpl.java | 474 +++++++----------- .../SecondaryStorageManagerTest.java | 29 ++ 4 files changed, 222 insertions(+), 294 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/dc/DataCenterVO.java b/engine/schema/src/main/java/com/cloud/dc/DataCenterVO.java index d71f34cdc7e..38121a72d02 100644 --- a/engine/schema/src/main/java/com/cloud/dc/DataCenterVO.java +++ b/engine/schema/src/main/java/com/cloud/dc/DataCenterVO.java @@ -471,4 +471,10 @@ public class DataCenterVO implements DataCenter { public PartitionType partitionType() { return PartitionType.Zone; } + + @Override + public String toString() { + return String.format("Zone {\"id\": \"%s\", \"name\": \"%s\", \"uuid\": \"%s\"}", id, name, uuid); + } + } diff --git a/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java b/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java index 84f1d850cfd..47932c9fbbb 100644 --- a/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java +++ b/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java @@ -504,14 +504,9 @@ public class VMInstanceVO implements VirtualMachine, FiniteStateObject Destroyed -// Creating -> Stopped --> Starting -> Running -// HA -> Stopped -> Starting -> Running -// Migrating -> Running (if previous state is Running before it enters into Migrating state -// Migrating -> Stopped (if previous state is not Running before it enters into Migrating state) -// Running -> HA (if agent lost connection) -// Stopped -> Destroyed -// -// Creating state indicates of record creating and IP address allocation are ready, it is a transient -// state which will soon be switching towards Running if everything goes well. -// Stopped state indicates the readiness of being able to start (has storage and IP resources allocated) -// Starting state can only be entered from Stopped states -// -// Starting, HA, Migrating, Creating and Running state are all counted as "Open" for available capacity calculation -// because sooner or later, it will be driven into Running state -// +/** +* Class to manage secondary storages.

+* Possible secondary storage VM state transition cases:
+* - Creating -> Destroyed
+* - Creating -> Stopped -> Starting -> Running
+* - HA -> Stopped -> Starting -> Running
+* - Migrating -> Running (if previous state is Running before it enters into Migrating state
+* - Migrating -> Stopped (if previous state is not Running before it enters into Migrating state)
+* - Running -> HA (if agent lost connection)
+* - Stopped -> Destroyed

+* +* Creating state indicates of record creating and IP address allocation are ready, it is a transient state which will soon be switching towards Running if everything goes well.

+* Stopped state indicates the readiness of being able to start (has storage and IP resources allocated).

+* Starting state can only be entered from Stopped states.


+* +* Starting, HA, Migrating, Creating and Running states are all counted as Open for available capacity calculation because sooner or later, it will be driven into Running state. +*/ public class SecondaryStorageManagerImpl extends ManagerBase implements SecondaryStorageVmManager, VirtualMachineGuru, SystemVmLoadScanHandler, ResourceStateAdapter, Configurable { private static final Logger s_logger = Logger.getLogger(SecondaryStorageManagerImpl.class); - private static final int DEFAULT_CAPACITY_SCAN_INTERVAL = 30000; // 30 - // seconds - private static final int ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_SYNC = 180; // 3 - // minutes - - private static final int STARTUP_DELAY = 60000; // 60 seconds + private static final int DEFAULT_CAPACITY_SCAN_INTERVAL_IN_MILLISECONDS = 30000; + private static final int ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_SYNC_IN_SECONDS = 180; + private static final int STARTUP_DELAY_IN_MILLISECONDS = 60000; private int _mgmtPort = 8250; @@ -248,7 +246,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar @Inject private IndirectAgentLB indirectAgentLB; - private long _capacityScanInterval = DEFAULT_CAPACITY_SCAN_INTERVAL; + private long _capacityScanInterval = DEFAULT_CAPACITY_SCAN_INTERVAL_IN_MILLISECONDS; private int _secStorageVmMtuSize; private String _instance; @@ -258,7 +256,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar protected long _nodeId = ManagementServerNode.getManagementServerId(); private SystemVmLoadScanner _loadScanner; - private Map _zoneHostInfoMap; // map + private Map _zoneHostInfoMap; private final GlobalLock _allocLock = GlobalLock.getInternLock(getAllocLockName()); @@ -277,17 +275,8 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar SecondaryStorageVmVO secStorageVm = _secStorageVmDao.findById(secStorageVmId); _itMgr.advanceStart(secStorageVm.getUuid(), null, null); return _secStorageVmDao.findById(secStorageVm.getId()); - } catch (StorageUnavailableException e) { - s_logger.warn("Exception while trying to start secondary storage vm", e); - return null; - } catch (InsufficientCapacityException e) { - s_logger.warn("Exception while trying to start secondary storage vm", e); - return null; - } catch (ResourceUnavailableException e) { - s_logger.warn("Exception while trying to start secondary storage vm", e); - return null; - } catch (Exception e) { - s_logger.warn("Exception while trying to start secondary storage vm", e); + } catch (ConcurrentOperationException | InsufficientCapacityException | OperationTimedoutException | ResourceUnavailableException e) { + s_logger.warn(String.format("Unable to start secondary storage VM [%s] due to [%s].", secStorageVmId, e.getMessage()), e); return null; } } @@ -304,17 +293,18 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar HostVO cssHost = _hostDao.findById(ssHostId); Long zoneId = cssHost.getDataCenterId(); if (cssHost.getType() == Host.Type.SecondaryStorageVM) { + String hostName = cssHost.getName(); - SecondaryStorageVmVO secStorageVm = _secStorageVmDao.findByInstanceName(cssHost.getName()); + SecondaryStorageVmVO secStorageVm = _secStorageVmDao.findByInstanceName(hostName); if (secStorageVm == null) { - s_logger.warn("secondary storage VM " + cssHost.getName() + " doesn't exist"); + s_logger.warn(String.format("Secondary storage VM [%s] does not exist.", hostName)); return false; } List ssStores = _dataStoreMgr.getImageStoresByScope(new ZoneScope(zoneId)); for (DataStore ssStore : ssStores) { if (!(ssStore.getTO() instanceof NfsTO)) { - continue; // only do this for Nfs + continue; } String secUrl = ssStore.getUri(); SecStorageSetupCommand setupCmd = null; @@ -328,7 +318,6 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar String nfsVersion = imageStoreDetailsUtil.getNfsVersion(ssStore.getId()); setupCmd.setNfsVersion(nfsVersion); - //template/volume file upload key String postUploadKey = _configDao.getValue(Config.SSVMPSK.key()); setupCmd.setPostUploadKey(postUploadKey); @@ -336,43 +325,19 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar if (answer != null && answer.getResult()) { SecStorageSetupAnswer an = (SecStorageSetupAnswer)answer; if (an.get_dir() != null) { - // update the parent path in image_store table for this image store ImageStoreVO svo = _imageStoreDao.findById(ssStore.getId()); svo.setParent(an.get_dir()); _imageStoreDao.update(ssStore.getId(), svo); } - if (s_logger.isDebugEnabled()) { - s_logger.debug("Successfully programmed secondary storage " + ssStore.getName() + " in secondary storage VM " + secStorageVm.getInstanceName()); - } + + s_logger.debug(String.format("Successfully programmed secondary storage [%s] in secondary storage VM [%s].", ssStore.getName(), secStorageVm.getInstanceName())); } else { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Successfully programmed secondary storage " + ssStore.getName() + " in secondary storage VM " + secStorageVm.getInstanceName()); - } + s_logger.debug(String.format("Unable to program secondary storage [%s] in secondary storage VM [%s] due to [%s].", ssStore.getName(), secStorageVm.getInstanceName(), answer == null ? "null answer" : answer.getDetails())); return false; } } } - /* After removing SecondaryStorage entries from host table, control should never come here!! - else if( cssHost.getType() == Host.Type.SecondaryStorage ) { - List alreadyRunning = _secStorageVmDao.getSecStorageVmListInStates(SecondaryStorageVm.Role.templateProcessor, zoneId, State.Running); - String secUrl = cssHost.getStorageUrl(); - SecStorageSetupCommand setupCmd = new SecStorageSetupCommand(secUrl, null); - for ( SecondaryStorageVmVO ssVm : alreadyRunning ) { - HostVO host = _resourceMgr.findHostByName(ssVm.getInstanceName()); - Answer answer = _agentMgr.easySend(host.getId(), setupCmd); - if (answer != null && answer.getResult()) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Successfully programmed secondary storage " + host.getName() + " in secondary storage VM " + ssVm.getInstanceName()); - } - } else { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Successfully programmed secondary storage " + host.getName() + " in secondary storage VM " + ssVm.getInstanceName()); - } - return false; - } - } - } - */ + return true; } @@ -382,15 +347,16 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar if (ssAHost.getType() != Host.Type.SecondaryStorageVM) { return false; } - SecondaryStorageVmVO secStorageVm = _secStorageVmDao.findByInstanceName(ssAHost.getName()); + String ssvmName = ssAHost.getName(); + SecondaryStorageVmVO secStorageVm = _secStorageVmDao.findByInstanceName(ssvmName); if (secStorageVm == null) { - s_logger.warn("secondary storage VM " + ssAHost.getName() + " doesn't exist"); + s_logger.warn(String.format("Secondary storage VM [%s] does not exist.", ssvmName)); return false; } SecStorageVMSetupCommand setupCmd = new SecStorageVMSetupCommand(); if (_allowedInternalSites != null) { - List allowedCidrs = new ArrayList(); + List allowedCidrs = new ArrayList<>(); String[] cidrs = _allowedInternalSites.split(","); for (String cidr : cidrs) { if (NetUtils.isValidIp4Cidr(cidr) || NetUtils.isValidIp4(cidr) || !cidr.startsWith("0.0.0.0")) { @@ -402,15 +368,16 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar String copyPasswd = _configDao.getValue("secstorage.copy.password"); setupCmd.setCopyPassword(copyPasswd); setupCmd.setCopyUserName(TemplateConstants.DEFAULT_HTTP_AUTH_USER); + Answer answer = _agentMgr.easySend(ssAHostId, setupCmd); if (answer != null && answer.getResult()) { if (s_logger.isDebugEnabled()) { - s_logger.debug("Successfully programmed http auth into " + secStorageVm.getHostName()); + s_logger.debug(String.format("Successfully set HTTP auth into secondary storage VM [%s].", ssvmName)); } return true; } else { if (s_logger.isDebugEnabled()) { - s_logger.debug("failed to program http auth into secondary storage vm : " + secStorageVm.getHostName()); + s_logger.debug(String.format("Failed to set HTTP auth into secondary storage VM [%s] due to [%s].", ssvmName, answer == null ? "answer null" : answer.getDetails())); } return false; } @@ -427,10 +394,12 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar return true; } HostVO ssAHost = _hostDao.findById(ssAHostId); - SecondaryStorageVmVO thisSecStorageVm = _secStorageVmDao.findByInstanceName(ssAHost.getName()); + String hostName = ssAHost.getName(); + + SecondaryStorageVmVO thisSecStorageVm = _secStorageVmDao.findByInstanceName(hostName); if (thisSecStorageVm == null) { - s_logger.warn("secondary storage VM " + ssAHost.getName() + " doesn't exist"); + s_logger.warn(String.format("Secondary storage VM [%s] does not exist.", hostName)); return false; } @@ -446,14 +415,16 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar if (ssvm.getId() == ssAHostId) { continue; } + + hostName = ssvm.getName(); Answer answer = _agentMgr.easySend(ssvm.getId(), thiscpc); if (answer != null && answer.getResult()) { if (s_logger.isDebugEnabled()) { - s_logger.debug("Successfully programmed firewall rules into SSVM " + ssvm.getName()); + s_logger.debug(String.format("Successfully created firewall rules into secondary storage VM [%s].", hostName)); } } else { if (s_logger.isDebugEnabled()) { - s_logger.debug("failed to program firewall rules into secondary storage vm : " + ssvm.getName()); + s_logger.debug(String.format("Failed to create firewall rules into secondary storage VM [%s].", hostName)); } return false; } @@ -467,14 +438,16 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar allSSVMIpList.addPortConfig(ssvm.getPublicIpAddress(), copyPort, true, TemplateConstants.DEFAULT_TMPLT_COPY_INTF); } + hostName = thisSecStorageVm.getHostName(); + Answer answer = _agentMgr.easySend(ssAHostId, allSSVMIpList); if (answer != null && answer.getResult()) { if (s_logger.isDebugEnabled()) { - s_logger.debug("Successfully programmed firewall rules into " + thisSecStorageVm.getHostName()); + s_logger.debug(String.format("Successfully created firewall rules into secondary storage VM [%s].", hostName)); } } else { if (s_logger.isDebugEnabled()) { - s_logger.debug("failed to program firewall rules into secondary storage vm : " + thisSecStorageVm.getHostName()); + s_logger.debug(String.format("Failed to create firewall rules into secondary storage VM [%s] due to [%s].", hostName, answer == null ? "answer null" : answer.getDetails())); } return false; } @@ -497,36 +470,31 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar if (!isSecondaryStorageVmRequired(dataCenterId)) { if (s_logger.isDebugEnabled()) { - s_logger.debug("Secondary storage vm not required in zone " + dataCenterId + " acc. to zone config"); + s_logger.debug(String.format("Secondary storage VM not required in zone [%s] account to zone config.", dataCenterId)); } return null; } if (s_logger.isDebugEnabled()) { - s_logger.debug("Assign secondary storage vm from a newly started instance for request from data center : " + dataCenterId); + s_logger.debug(String.format("Assign secondary storage VM from a newly started instance for request from data center [%s].", dataCenterId)); } Map context = createSecStorageVmInstance(dataCenterId, role); long secStorageVmId = (Long)context.get("secStorageVmId"); if (secStorageVmId == 0) { - if (s_logger.isTraceEnabled()) { - s_logger.trace("Creating secondary storage vm instance failed, data center id : " + dataCenterId); - } - + s_logger.debug(String.format("Creating secondary storage VM instance failed on data center [%s].", dataCenterId)); return null; } SecondaryStorageVmVO secStorageVm = _secStorageVmDao.findById(secStorageVmId); - // SecondaryStorageVmVO secStorageVm = - // allocSecStorageVmStorage(dataCenterId, secStorageVmId); + if (secStorageVm != null) { SubscriptionMgr.getInstance().notifySubscribers(ALERT_SUBJECT, this, new SecStorageVmAlertEventArgs(SecStorageVmAlertEventArgs.SSVM_CREATED, dataCenterId, secStorageVmId, secStorageVm, null)); return secStorageVm; } else { if (s_logger.isDebugEnabled()) { - s_logger.debug("Unable to allocate secondary storage vm storage, remove the secondary storage vm record from DB, secondary storage vm id: " + - secStorageVmId); + s_logger.debug(String.format("Unable to allocate secondary storage VM [%s] due to it was not found on database.", secStorageVmId)); } SubscriptionMgr.getInstance().notifySubscribers(ALERT_SUBJECT, this, new SecStorageVmAlertEventArgs(SecStorageVmAlertEventArgs.SSVM_CREATE_FAILURE, dataCenterId, secStorageVmId, null, "Unable to allocate storage")); @@ -559,13 +527,13 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar */ protected NetworkVO getDefaultNetworkForAdvancedZone(DataCenter dc) { if (dc.getNetworkType() != NetworkType.Advanced) { - throw new CloudRuntimeException("Zone " + dc + " is not advanced."); + throw new CloudRuntimeException(String.format("%s is not advanced.", dc.toString())); } if (dc.isSecurityGroupEnabled()) { List networks = _networkDao.listByZoneSecurityGroup(dc.getId()); if (CollectionUtils.isEmpty(networks)) { - throw new CloudRuntimeException("Can not found security enabled network in SG Zone " + dc); + throw new CloudRuntimeException(String.format("Can not found security enabled network in SG %s.", dc.toString())); } return networks.get(0); @@ -573,9 +541,10 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar else { TrafficType defaultTrafficType = TrafficType.Public; List defaultNetworks = _networkDao.listByZoneAndTrafficType(dc.getId(), defaultTrafficType); - // api should never allow this situation to happen - if (defaultNetworks.size() != 1) { - throw new CloudRuntimeException("Found " + defaultNetworks.size() + " networks of type " + defaultTrafficType + " when expect to find 1"); + + int networksSize = defaultNetworks.size(); + if (networksSize != 1) { + throw new CloudRuntimeException(String.format("Found [%s] networks of type [%s] when expect to find 1.", networksSize, defaultTrafficType)); } return defaultNetworks.get(0); @@ -591,14 +560,15 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar */ protected NetworkVO getDefaultNetworkForBasicZone(DataCenter dc) { if (dc.getNetworkType() != NetworkType.Basic) { - throw new CloudRuntimeException("Zone " + dc + "is not basic."); + throw new CloudRuntimeException(String.format("%s is not basic.", dc.toString())); } TrafficType defaultTrafficType = TrafficType.Guest; List defaultNetworks = _networkDao.listByZoneAndTrafficType(dc.getId(), defaultTrafficType); - // api should never allow this situation to happen - if (defaultNetworks.size() != 1) { - throw new CloudRuntimeException("Found " + defaultNetworks.size() + " networks of type " + defaultTrafficType + " when expect to find 1"); + + int networksSize = defaultNetworks.size(); + if (networksSize != 1) { + throw new CloudRuntimeException(String.format("Found [%s] networks of type [%s] when expect to find 1.", networksSize, defaultTrafficType)); } return defaultNetworks.get(0); @@ -607,7 +577,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar protected Map createSecStorageVmInstance(long dataCenterId, SecondaryStorageVm.Role role) { DataStore secStore = _dataStoreMgr.getImageStoreWithFreeCapacity(dataCenterId); if (secStore == null) { - String msg = "No secondary storage available in zone " + dataCenterId + ", cannot create secondary storage vm"; + String msg = String.format("No secondary storage available in zone %s, cannot create secondary storage VM.", dataCenterId); s_logger.warn(msg); throw new CloudRuntimeException(msg); } @@ -627,26 +597,25 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar } else { offerings = _networkModel.getSystemAccountNetworkOfferings(NetworkOffering.SystemControlNetwork, NetworkOffering.SystemManagementNetwork); } - LinkedHashMap> networks = new LinkedHashMap>(offerings.size() + 1); + LinkedHashMap> networks = new LinkedHashMap<>(offerings.size() + 1); NicProfile defaultNic = new NicProfile(); defaultNic.setDefaultNic(true); defaultNic.setDeviceId(2); try { networks.put(_networkMgr.setupNetwork(systemAcct, _networkOfferingDao.findById(defaultNetwork.getNetworkOfferingId()), plan, null, null, false).get(0), - new ArrayList(Arrays.asList(defaultNic))); + new ArrayList<>(Arrays.asList(defaultNic))); for (NetworkOffering offering : offerings) { - networks.put(_networkMgr.setupNetwork(systemAcct, offering, plan, null, null, false).get(0), new ArrayList()); + networks.put(_networkMgr.setupNetwork(systemAcct, offering, plan, null, null, false).get(0), new ArrayList<>()); } } catch (ConcurrentOperationException e) { - s_logger.info("Unable to setup due to concurrent operation. " + e); - return new HashMap(); + s_logger.error(String.format("Unable to setup networks on %s due [%s].", dc.toString(), e.getMessage()), e); + return new HashMap<>(); } - VMTemplateVO template = null; HypervisorType availableHypervisor = _resourceMgr.getAvailableHypervisor(dataCenterId); - template = _templateDao.findSystemVMReadyTemplate(dataCenterId, availableHypervisor); + VMTemplateVO template = _templateDao.findSystemVMReadyTemplate(dataCenterId, availableHypervisor); if (template == null) { - throw new CloudRuntimeException("Not able to find the System templates or not downloaded in zone " + dataCenterId); + throw new CloudRuntimeException(String.format("Unable to find the system templates or it was not downloaded in %s.", dc.toString())); } ServiceOfferingVO serviceOffering = _serviceOffering; @@ -662,18 +631,17 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar _itMgr.allocate(name, template, serviceOffering, networks, plan, null); secStorageVm = _secStorageVmDao.findById(secStorageVm.getId()); } catch (InsufficientCapacityException e) { - s_logger.warn("InsufficientCapacity", e); - throw new CloudRuntimeException("Insufficient capacity exception", e); + String errorMessage = String.format("Unable to allocate secondary storage VM [%s] due to [%s].", name, e.getMessage()); + s_logger.warn(errorMessage, e); + throw new CloudRuntimeException(errorMessage, e); } - Map context = new HashMap(); + Map context = new HashMap<>(); context.put("secStorageVmId", secStorageVm.getId()); return context; } private SecondaryStorageVmAllocator getCurrentAllocator() { - - // for now, only one adapter is supported if (_ssVmAllocators.size() > 0) { return _ssVmAllocators.get(0); } @@ -686,50 +654,41 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar } public SecondaryStorageVmVO assignSecStorageVmFromRunningPool(long dataCenterId, SecondaryStorageVm.Role role) { - - if (s_logger.isTraceEnabled()) { - s_logger.trace("Assign secondary storage vm from running pool for request from data center : " + dataCenterId); - } + s_logger.debug(String.format("Assign secondary storage VM from running pool for request from zone [%s].", dataCenterId)); SecondaryStorageVmAllocator allocator = getCurrentAllocator(); assert (allocator != null); List runningList = _secStorageVmDao.getSecStorageVmListInStates(role, dataCenterId, State.Running); - if (runningList != null && runningList.size() > 0) { - if (s_logger.isTraceEnabled()) { - s_logger.trace("Running secondary storage vm pool size : " + runningList.size()); - for (SecondaryStorageVmVO secStorageVm : runningList) { - s_logger.trace("Running secStorageVm instance : " + secStorageVm.getHostName()); - } + if (CollectionUtils.isNotEmpty(runningList)) { + s_logger.debug(String.format("Running secondary storage VM pool size [%s].", runningList.size())); + for (SecondaryStorageVmVO secStorageVm : runningList) { + s_logger.debug(String.format("Running secondary storage %s.", secStorageVm.toString())); } - Map loadInfo = new HashMap(); + Map loadInfo = new HashMap<>(); return allocator.allocSecondaryStorageVm(runningList, loadInfo, dataCenterId); } else { - if (s_logger.isTraceEnabled()) { - s_logger.trace("Empty running secStorageVm pool for now in data center : " + dataCenterId); - } + s_logger.debug(String.format("There is no running secondary storage VM right now in the zone [%s].", dataCenterId)); } return null; } public SecondaryStorageVmVO assignSecStorageVmFromStoppedPool(long dataCenterId, SecondaryStorageVm.Role role) { - List l = _secStorageVmDao.getSecStorageVmListInStates(role, dataCenterId, State.Starting, State.Stopped, State.Migrating); - if (l != null && l.size() > 0) { - return l.get(0); + List secondaryStorageVms = _secStorageVmDao.getSecStorageVmListInStates(role, dataCenterId, State.Starting, State.Stopped, State.Migrating); + if (CollectionUtils.isNotEmpty(secondaryStorageVms)) { + return secondaryStorageVms.get(0); } return null; } public void allocCapacity(long dataCenterId, SecondaryStorageVm.Role role) { - if (s_logger.isTraceEnabled()) { - s_logger.trace("Allocate secondary storage vm standby capacity for data center : " + dataCenterId); - } + s_logger.debug(String.format("Allocate secondary storage VM standby capacity for zone [%s].", dataCenterId)); if (!isSecondaryStorageVmRequired(dataCenterId)) { if (s_logger.isDebugEnabled()) { - s_logger.debug("Secondary storage vm not required in zone " + dataCenterId + " according to zone config"); + s_logger.debug(String.format("Secondary storage VM not required in zone [%s] according to zone config.", dataCenterId)); } return; } @@ -740,10 +699,10 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar secStorageVm = assignSecStorageVmFromStoppedPool(dataCenterId, role); if (secStorageVm == null) { if (s_logger.isInfoEnabled()) { - s_logger.info("No stopped secondary storage vm is available, need to allocate a new secondary storage vm"); + s_logger.info("No stopped secondary storage VM is available, need to allocate a new secondary storage VM."); } - if (_allocLock.lock(ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_SYNC)) { + if (_allocLock.lock(ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_SYNC_IN_SECONDS)) { try { secStorageVm = startNew(dataCenterId, role); } finally { @@ -751,13 +710,13 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar } } else { if (s_logger.isInfoEnabled()) { - s_logger.info("Unable to acquire synchronization lock for secondary storage vm allocation, wait for next scan"); + s_logger.info("Unable to acquire synchronization lock for secondary storage VM allocation, wait for next scan."); } return; } } else { if (s_logger.isInfoEnabled()) { - s_logger.info("Found a stopped secondary storage vm, starting it. Vm id : " + secStorageVm.getId()); + s_logger.info(String.format("Found a stopped secondary storage %s, starting it.", secStorageVm.toString())); } secStorageVmFromStoppedPool = true; } @@ -766,7 +725,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar long secStorageVmId = secStorageVm.getId(); GlobalLock secStorageVmLock = GlobalLock.getInternLock(getSecStorageVmLockName(secStorageVmId)); try { - if (secStorageVmLock.lock(ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_SYNC)) { + if (secStorageVmLock.lock(ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_SYNC_IN_SECONDS)) { try { secStorageVm = startSecStorageVm(secStorageVmId); } finally { @@ -774,7 +733,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar } } else { if (s_logger.isInfoEnabled()) { - s_logger.info("Unable to acquire synchronization lock for starting secondary storage vm id : " + secStorageVm.getId()); + s_logger.info(String.format("Unable to acquire synchronization lock for starting secondary storage %s.", secStorageVm.toString())); } return; } @@ -784,7 +743,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar if (secStorageVm == null) { if (s_logger.isInfoEnabled()) { - s_logger.info("Unable to start secondary storage vm for standby capacity, vm id : " + secStorageVmId + ", will recycle it and start a new one"); + s_logger.info(String.format("Unable to start secondary storage VM [%s] for standby capacity, it will be recycled and will start a new one.", secStorageVmId)); } if (secStorageVmFromStoppedPool) { @@ -794,16 +753,14 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar SubscriptionMgr.getInstance().notifySubscribers(ALERT_SUBJECT, this, new SecStorageVmAlertEventArgs(SecStorageVmAlertEventArgs.SSVM_UP, dataCenterId, secStorageVmId, secStorageVm, null)); if (s_logger.isInfoEnabled()) { - s_logger.info("Secondary storage vm " + secStorageVm.getHostName() + " is started"); + s_logger.info(String.format("Secondary storage %s was started.", secStorageVm.toString())); } } } } catch (Exception e) { - errorString = e.getMessage(); + errorString = String.format("Unable to allocate capacity on zone [%s] due to [%s].", dataCenterId, errorString); throw e; } finally { - // TODO - For now put all the alerts as creation failure. Distinguish between creation vs start failure in future. - // Also add failure reason since startvm masks some of them. if(secStorageVm == null || secStorageVm.getState() != State.Running) SubscriptionMgr.getInstance().notifySubscribers(ALERT_SUBJECT, this, new SecStorageVmAlertEventArgs(SecStorageVmAlertEventArgs.SSVM_CREATE_FAILURE, dataCenterId, 0l, null, errorString)); @@ -816,37 +773,33 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar VMTemplateVO template = _templateDao.findSystemVMReadyTemplate(dataCenterId, HypervisorType.Any); if (template == null) { if (s_logger.isDebugEnabled()) { - s_logger.debug("System vm template is not ready at data center " + dataCenterId + ", wait until it is ready to launch secondary storage vm"); + s_logger.debug(String.format("System VM template is not ready at zone [%s], wait until it is ready to launch secondary storage VM.", dataCenterId)); } return false; } List stores = _dataStoreMgr.getImageStoresByScopeExcludingReadOnly(new ZoneScope(dataCenterId)); - if (stores.size() < 1) { - s_logger.debug("No image store added in zone " + dataCenterId + ", wait until it is ready to launch secondary storage vm"); + if (CollectionUtils.isEmpty(stores)) { + s_logger.debug(String.format("No image store added in zone [%s], wait until it is ready to launch secondary storage VM.", dataCenterId)); return false; } if (!template.isDirectDownload() && templateMgr.getImageStore(dataCenterId, template.getId()) == null) { if (s_logger.isDebugEnabled()) { - s_logger.debug("No secondary storage available in zone " + dataCenterId + ", wait until it is ready to launch secondary storage vm"); + s_logger.debug(String.format("No secondary storage available in zone [%s], wait until it is ready to launch secondary storage VM.", dataCenterId)); } return false; } - boolean useLocalStorage = false; - Boolean useLocal = ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dataCenterId); - if (useLocal != null) { - useLocalStorage = useLocal.booleanValue(); - } - List> l = _storagePoolHostDao.getDatacenterStoragePoolHostInfo(dataCenterId, !useLocalStorage); - if (l != null && l.size() > 0 && l.get(0).second().intValue() > 0) { + boolean useLocalStorage = BooleanUtils.toBoolean(ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dataCenterId)); + List> storagePoolHostInfos = _storagePoolHostDao.getDatacenterStoragePoolHostInfo(dataCenterId, !useLocalStorage); + if (CollectionUtils.isNotEmpty(storagePoolHostInfos) && storagePoolHostInfos.get(0).second() > 0) { return true; } else { if (s_logger.isDebugEnabled()) { - s_logger.debug("Primary storage is not ready, wait until it is ready to launch secondary storage vm. dcId: " + dataCenterId + - ", " + ConfigurationManagerImpl.SystemVMUseLocalStorage.key() + ": " + useLocalStorage + ". " + - "If you want to use local storage to start SSVM, need to set " + ConfigurationManagerImpl.SystemVMUseLocalStorage.key() + " to true"); + String configKey = ConfigurationManagerImpl.SystemVMUseLocalStorage.key(); + s_logger.debug(String.format("Primary storage is not ready, wait until it is ready to launch secondary storage VM. {\"dataCenterId\": %s, \"%s\": \"%s\"}. " + + "If you want to use local storage to start secondary storage VM, you need to set the configuration [%s] to \"true\".", dataCenterId, configKey, useLocalStorage, configKey)); } } @@ -856,11 +809,11 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar private synchronized Map getZoneHostInfo() { Date cutTime = DateUtil.currentGMTTime(); - List l = _hostDao.getRunningHostCounts(new Date(cutTime.getTime() - ClusterManager.HeartbeatThreshold.value())); + List runningHostCountInfos = _hostDao.getRunningHostCounts(new Date(cutTime.getTime() - ClusterManager.HeartbeatThreshold.value())); RunningHostInfoAgregator aggregator = new RunningHostInfoAgregator(); - if (l.size() > 0) { - for (RunningHostCountInfo countInfo : l) { + if (CollectionUtils.isNotEmpty(runningHostCountInfos)) { + for (RunningHostCountInfo countInfo : runningHostCountInfos) { aggregator.aggregate(countInfo); } } @@ -894,20 +847,11 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar Map configs = _configDao.getConfiguration("management-server", params); _secStorageVmMtuSize = NumbersUtil.parseInt(configs.get("secstorage.vm.mtu.size"), DEFAULT_SS_VM_MTUSIZE); - String useServiceVM = _configDao.getValue("secondary.storage.vm"); - boolean _useServiceVM = false; - if ("true".equalsIgnoreCase(useServiceVM)) { - _useServiceVM = true; - } + boolean _useServiceVM = BooleanUtils.toBoolean(_configDao.getValue("secondary.storage.vm")); + _useSSlCopy = BooleanUtils.toBoolean(_configDao.getValue("secstorage.encrypt.copy")); - String sslcopy = _configDao.getValue("secstorage.encrypt.copy"); - if ("true".equalsIgnoreCase(sslcopy)) { - _useSSlCopy = true; - } - - //default to HTTP in case of missing domain String ssvmUrlDomain = _configDao.getValue("secstorage.ssl.cert.domain"); - if(_useSSlCopy && (ssvmUrlDomain == null || ssvmUrlDomain.isEmpty())){ + if(_useSSlCopy && org.apache.commons.lang3.StringUtils.isEmpty(ssvmUrlDomain)){ s_logger.warn("Empty secondary storage url domain, explicitly disabling SSL"); _useSSlCopy = false; } @@ -915,7 +859,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar _allowedInternalSites = _configDao.getValue("secstorage.allowed.internal.sites"); String value = configs.get("secstorage.capacityscan.interval"); - _capacityScanInterval = NumbersUtil.parseLong(value, DEFAULT_CAPACITY_SCAN_INTERVAL); + _capacityScanInterval = NumbersUtil.parseLong(value, DEFAULT_CAPACITY_SCAN_INTERVAL_IN_MILLISECONDS); _instance = configs.get("instance.name"); if (_instance == null) { @@ -932,20 +876,22 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar _itMgr.registerGuru(VirtualMachine.Type.SecondaryStorageVm, this); - //check if there is a default service offering configured - String ssvmSrvcOffIdStr = configs.get(Config.SecondaryStorageServiceOffering.key()); + String configKey = Config.SecondaryStorageServiceOffering.key(); + String ssvmSrvcOffIdStr = configs.get(configKey); if (ssvmSrvcOffIdStr != null) { _serviceOffering = _offeringDao.findByUuid(ssvmSrvcOffIdStr); if (_serviceOffering == null) { try { + s_logger.debug(String.format("Unable to find a service offering by the UUID for secondary storage VM with the value [%s] set in the configuration [%s]. Trying to find by the ID.", ssvmSrvcOffIdStr, configKey)); _serviceOffering = _offeringDao.findById(Long.parseLong(ssvmSrvcOffIdStr)); + + if (_serviceOffering == null) { + s_logger.info(String.format("Unable to find a service offering by the UUID or ID for secondary storage VM with the value [%s] set in the configuration [%s]", ssvmSrvcOffIdStr, configKey)); + } } catch (NumberFormatException ex) { - s_logger.debug("The system service offering specified by global config is not id, but uuid=" + ssvmSrvcOffIdStr + " for secondary storage vm"); + s_logger.warn(String.format("Unable to find a service offering by the ID for secondary storage VM with the value [%s] set in the configuration [%s]. The value is not a valid integer number. Error: [%s].", ssvmSrvcOffIdStr, configKey, ex.getMessage()), ex); } } - if (_serviceOffering == null) { - s_logger.warn("Can't find system service offering specified by global config, uuid=" + ssvmSrvcOffIdStr + " for secondary storage vm"); - } } if (_serviceOffering == null || !_serviceOffering.isSystemUse()) { @@ -954,17 +900,17 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar List offerings = _offeringDao.createSystemServiceOfferings("System Offering For Secondary Storage VM", ServiceOffering.ssvmDefaultOffUniqueName, 1, ramSize, cpuFreq, null, null, false, null, Storage.ProvisioningType.THIN, true, null, true, VirtualMachine.Type.SecondaryStorageVm, true); - // this can sometimes happen, if DB is manually or programmatically manipulated + if (offerings == null || offerings.size() < 2) { - String msg = "Data integrity problem : System Offering For Secondary Storage VM has been removed?"; + String msg = "Unable to set a service offering for secondary storage VM. Verify if it was removed."; s_logger.error(msg); throw new ConfigurationException(msg); } } if (_useServiceVM) { - _loadScanner = new SystemVmLoadScanner(this); - _loadScanner.initScan(STARTUP_DELAY, _capacityScanInterval); + _loadScanner = new SystemVmLoadScanner<>(this); + _loadScanner.initScan(STARTUP_DELAY_IN_MILLISECONDS, _capacityScanInterval); } _httpProxy = configs.get(Config.SecStorageProxy.key()); @@ -973,8 +919,9 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar String errMsg = null; try { URI uri = new URI(_httpProxy); - if (!"http".equalsIgnoreCase(uri.getScheme())) { - errMsg = "Only support http proxy"; + String uriScheme = uri.getScheme(); + if (!"http".equalsIgnoreCase(uriScheme)) { + errMsg = String.format("[%s] is not supported, it only supports HTTP proxy", uriScheme); valid = false; } else if (uri.getHost() == null) { errMsg = "host can not be null"; @@ -984,16 +931,19 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar } } catch (URISyntaxException e) { errMsg = e.toString(); + valid = false; + s_logger.error(String.format("Unable to configure HTTP proxy [%s] on secondary storage VM manager [%s] due to [%s].", _httpProxy, name, errMsg), e); } finally { if (!valid) { - s_logger.debug("ssvm http proxy " + _httpProxy + " is invalid: " + errMsg); - throw new ConfigurationException("ssvm http proxy " + _httpProxy + "is invalid: " + errMsg); + String message = String.format("Unable to configure HTTP proxy [%s] on secondary storage VM manager [%s] due to [%s].", _httpProxy, name, errMsg); + s_logger.warn(message); + throw new ConfigurationException(message); } } } - if (s_logger.isInfoEnabled()) { - s_logger.info("Secondary storage vm Manager is configured."); - } + + s_logger.info(String.format("Secondary storage VM manager [%s] was configured.", name)); + _resourceMgr.registerResourceStateAdapter(this.getClass().getSimpleName(), this); return true; } @@ -1002,9 +952,8 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar public boolean stopSecStorageVm(long secStorageVmId) { SecondaryStorageVmVO secStorageVm = _secStorageVmDao.findById(secStorageVmId); if (secStorageVm == null) { - String msg = "Stopping secondary storage vm failed: secondary storage vm " + secStorageVmId + " no longer exists"; if (s_logger.isDebugEnabled()) { - s_logger.debug(msg); + s_logger.debug(String.format("Unable to stop secondary storage VM [%s] due to it no longer exists.", secStorageVmId)); } return false; } @@ -1012,7 +961,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar if (secStorageVm.getHostId() != null) { GlobalLock secStorageVmLock = GlobalLock.getInternLock(getSecStorageVmLockName(secStorageVm.getId())); try { - if (secStorageVmLock.lock(ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_SYNC)) { + if (secStorageVmLock.lock(ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_SYNC_IN_SECONDS)) { try { _itMgr.stop(secStorageVm.getUuid()); return true; @@ -1020,8 +969,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar secStorageVmLock.unlock(); } } else { - String msg = "Unable to acquire secondary storage vm lock : " + secStorageVm.toString(); - s_logger.debug(msg); + s_logger.debug(String.format("Unable to acquire secondary storage VM [%s] lock.", secStorageVm.toString())); return false; } } finally { @@ -1029,12 +977,9 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar } } - // vm was already stopped, return true return true; } catch (ResourceUnavailableException e) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Stopping secondary storage vm " + secStorageVm.getHostName() + " faled : exception " + e.toString()); - } + s_logger.error(String.format("Unable to stop secondary storage VM [%s] due to [%s].", secStorageVm.getHostName(), e.toString()), e); return false; } } @@ -1051,9 +996,11 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar final RebootCommand cmd = new RebootCommand(secStorageVm.getInstanceName(), _itMgr.getExecuteInSequence(secStorageVm.getHypervisorType())); final Answer answer = _agentMgr.easySend(secStorageVm.getHostId(), cmd); + String secondaryStorageVmName = secStorageVm.getHostName(); + if (answer != null && answer.getResult()) { if (s_logger.isDebugEnabled()) { - s_logger.debug("Successfully reboot secondary storage vm " + secStorageVm.getHostName()); + s_logger.debug(String.format("Successfully reboot secondary storage VM [%s].", secondaryStorageVmName)); } SubscriptionMgr.getInstance().notifySubscribers(ALERT_SUBJECT, this, @@ -1061,9 +1008,8 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar return true; } else { - String msg = "Rebooting Secondary Storage VM failed - " + secStorageVm.getHostName(); if (s_logger.isDebugEnabled()) { - s_logger.debug(msg); + s_logger.debug(String.format("Unable to reboot secondary storage VM [%s] due to [%s].", secondaryStorageVmName, answer == null ? "answer null" : answer.getDetails())); } return false; } @@ -1081,16 +1027,16 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar _secStorageVmDao.remove(ssvm.getId()); HostVO host = _hostDao.findByTypeNameAndZoneId(ssvm.getDataCenterId(), ssvm.getHostName(), Host.Type.SecondaryStorageVM); if (host != null) { - s_logger.debug("Removing host entry for ssvm id=" + vmId); + s_logger.debug(String.format("Removing host entry for secondary storage VM [%s].", vmId)); _hostDao.remove(host.getId()); - //Expire the download urls in the entire zone for templates and volumes. + _tmplStoreDao.expireDnldUrlsForZone(host.getDataCenterId()); _volumeStoreDao.expireDnldUrlsForZone(host.getDataCenterId()); return true; } return false; } catch (ResourceUnavailableException e) { - s_logger.warn("Unable to expunge " + ssvm, e); + s_logger.error(String.format("Unable to expunge secondary storage [%s] due to [%s].", ssvm.toString(), e.getMessage()), e); return false; } } @@ -1100,8 +1046,6 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar } private String getAllocLockName() { - // to improve security, it may be better to return a unique mashed - // name(for example MD5 hashed) return "secStorageVm.alloc"; } @@ -1117,7 +1061,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar DataStore secStore = _dataStoreMgr.getImageStoreWithFreeCapacity(dest.getDataCenter().getId()); if (secStore == null) { - s_logger.error(String.format("Unable to finalize virtual machine profile as no secondary storage available to satisfy storage needs for zone: %s", dest.getDataCenter().getUuid())); + s_logger.warn(String.format("Unable to finalize virtual machine profile [%s] as it has no secondary storage available to satisfy storage needs for zone [%s].", profile.toString(), dest.getDataCenter().getUuid())); return false; } @@ -1135,7 +1079,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar buf.append(" workers=").append(_configDao.getValue("workers")); if (_configDao.isPremium()) { - s_logger.debug("VmWare hypervisor configured, telling the ssvm to load the PremiumSecondaryStorageResource"); + s_logger.debug("VMWare hypervisor was configured, informing secondary storage VM to load the PremiumSecondaryStorageResource."); buf.append(" resource=com.cloud.storage.resource.PremiumSecondaryStorageResource"); } else { buf.append(" resource=org.apache.cloudstack.storage.resource.NfsSecondaryStorageResource"); @@ -1188,7 +1132,6 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar } } - /* External DHCP mode */ if (externalDhcp) { buf.append(" bootproto=dhcp"); } @@ -1207,7 +1150,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar String bootArgs = buf.toString(); if (s_logger.isDebugEnabled()) { - s_logger.debug("Boot Args for " + profile + ": " + bootArgs); + s_logger.debug(String.format("Boot args for machine profile [%s]: [%s].", profile.toString(), bootArgs)); } return true; @@ -1251,16 +1194,13 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar if (controlNic == null) { if (managementNic == null) { - s_logger.error("Management network doesn't exist for the secondaryStorageVm " + profile.getVirtualMachine()); + s_logger.warn(String.format("Management network does not exist for the secondary storage %s.", profile. toString())); return false; } controlNic = managementNic; } - // verify ssh access on management nic for system vm running on HyperV - if(profile.getHypervisorType() == HypervisorType.Hyperv) { - controlNic = managementNic; - } + controlNic = verifySshAccessOnManagementNicForSystemVm(profile, controlNic, managementNic); CheckSshCommand check = new CheckSshCommand(profile.getInstanceName(), controlNic.getIPv4Address(), 3922); cmds.addCommand("checkSsh", check); @@ -1268,26 +1208,32 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar return true; } + protected NicProfile verifySshAccessOnManagementNicForSystemVm(VirtualMachineProfile profile, NicProfile controlNic, NicProfile managementNic) { + if (profile.getHypervisorType() == HypervisorType.Hyperv) { + return managementNic; + } + + return controlNic; + } + @Override public boolean finalizeStart(VirtualMachineProfile profile, long hostId, Commands cmds, ReservationContext context) { CheckSshAnswer answer = (CheckSshAnswer)cmds.getAnswer("checkSsh"); if (!answer.getResult()) { - s_logger.warn("Unable to ssh to the VM: " + answer.getDetails()); + s_logger.warn(String.format("Unable to connect via SSH to the VM [%s] due to [%s] ", profile.toString(), answer.getDetails())); return false; } try { - //get system ip and create static nat rule for the vm in case of basic networking with EIP/ELB _rulesMgr.getSystemIpAndEnableStaticNatForVm(profile.getVirtualMachine(), false); IPAddressVO ipaddr = _ipAddressDao.findByAssociatedVmId(profile.getVirtualMachine().getId()); if (ipaddr != null && ipaddr.getSystem()) { SecondaryStorageVmVO secVm = _secStorageVmDao.findById(profile.getId()); - // override SSVM guest IP with EIP, so that download url's with be prepared with EIP secVm.setPublicIpAddress(ipaddr.getAddress().addr()); _secStorageVmDao.update(secVm.getId(), secVm); } - } catch (Exception ex) { - s_logger.warn("Failed to get system ip and enable static nat for the vm " + profile.getVirtualMachine() + " due to exception ", ex); + } catch (InsufficientAddressCapacityException ex) { + s_logger.error(String.format("Failed to get system IP and enable static NAT for the VM [%s] due to [%s].", profile.toString(), ex.getMessage()), ex); return false; } @@ -1296,15 +1242,13 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar @Override public void finalizeStop(VirtualMachineProfile profile, Answer answer) { - //release elastic IP here IPAddressVO ip = _ipAddressDao.findByAssociatedVmId(profile.getId()); if (ip != null && ip.getSystem()) { CallContext ctx = CallContext.current(); try { _rulesMgr.disableStaticNat(ip.getId(), ctx.getCallingAccount(), ctx.getCallingUserId(), true); - } catch (Exception ex) { - s_logger.warn("Failed to disable static nat and release system ip " + ip + " as a part of vm " + profile.getVirtualMachine() + " stop due to exception ", - ex); + } catch (ResourceUnavailableException ex) { + s_logger.error(String.format("Failed to disable static NAT and release system IP [%s] as a part of VM [%s] stop due to [%s].", ip, profile.toString(), ex.getMessage()), ex); } } } @@ -1348,27 +1292,22 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar } @Override - public boolean isPoolReadyForScan(Long pool) { - // pool is at zone basis - long dataCenterId = pool.longValue(); - + public boolean isPoolReadyForScan(Long dataCenterId) { if (!isZoneReady(_zoneHostInfoMap, dataCenterId)) { if (s_logger.isDebugEnabled()) { - s_logger.debug("Zone " + dataCenterId + " is not ready to launch secondary storage VM yet"); + s_logger.debug(String.format("Zone [%s] is not ready to launch secondary storage VM.", dataCenterId)); } return false; } if (s_logger.isDebugEnabled()) { - s_logger.debug("Zone " + dataCenterId + " is ready to launch secondary storage VM"); + s_logger.debug(String.format("Zone [%s] is ready to launch secondary storage VM.", dataCenterId)); } return true; } @Override - public Pair scanPool(Long pool) { - long dataCenterId = pool.longValue(); - + public Pair scanPool(Long dataCenterId) { List ssVms = _secStorageVmDao.getSecStorageVmListInStates(SecondaryStorageVm.Role.templateProcessor, dataCenterId, State.Running, State.Migrating, State.Starting, State.Stopped, State.Stopping); @@ -1376,16 +1315,15 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar List ssStores = _dataStoreMgr.getImageStoresByScopeExcludingReadOnly(new ZoneScope(dataCenterId)); int storeSize = (ssStores == null) ? 0 : ssStores.size(); if (storeSize > vmSize) { - s_logger.info("No secondary storage vms found in datacenter id=" + dataCenterId + ", starting a new one"); - return new Pair(AfterScanAction.expand, SecondaryStorageVm.Role.templateProcessor); + s_logger.info(String.format("No secondary storage VM found in zone [%s], starting a new one.", dataCenterId)); + return new Pair<>(AfterScanAction.expand, SecondaryStorageVm.Role.templateProcessor); } - return new Pair(AfterScanAction.nop, SecondaryStorageVm.Role.templateProcessor); + return new Pair<>(AfterScanAction.nop, SecondaryStorageVm.Role.templateProcessor); } @Override - public void expandPool(Long pool, Object actionArgs) { - long dataCenterId = pool.longValue(); + public void expandPool(Long dataCenterId, Object actionArgs) { allocCapacity(dataCenterId, (SecondaryStorageVm.Role)actionArgs); } @@ -1399,7 +1337,6 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar @Override public HostVO createHostVOForConnectedAgent(HostVO host, StartupCommand[] cmd) { - /* Called when Secondary Storage VM connected */ StartupCommand firstCmd = cmd[0]; if (!(firstCmd instanceof StartupSecondaryStorageCommand)) { return null; @@ -1409,57 +1346,18 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar return host; } + /** Used to be called when add secondary storage on UI through DummySecondaryStorageResource to update that host entry for Secondary Storage.

+ * Now since we move secondary storage from host table, this method, in this class, is not needed to be invoked anymore. + */ @Override public HostVO createHostVOForDirectConnectAgent(HostVO host, StartupCommand[] startup, ServerResource resource, Map details, List hostTags) { - // Used to be Called when add secondary storage on UI through DummySecondaryStorageResource to update that host entry for Secondary Storage. - // Now since we move secondary storage from host table, this code is not needed to be invoked anymore. - /* - StartupCommand firstCmd = startup[0]; - if (!(firstCmd instanceof StartupStorageCommand)) { - return null; - } - - com.cloud.host.Host.Type type = null; - StartupStorageCommand ssCmd = ((StartupStorageCommand) firstCmd); - if (ssCmd.getHostType() == Host.Type.SecondaryStorageCmdExecutor) { - type = ssCmd.getHostType(); - } else { - if (ssCmd.getResourceType() == Storage.StorageResourceType.SECONDARY_STORAGE) { - type = Host.Type.SecondaryStorage; - if (resource != null && resource instanceof DummySecondaryStorageResource) { - host.setResource(null); - } - } else if (ssCmd.getResourceType() == Storage.StorageResourceType.LOCAL_SECONDARY_STORAGE) { - type = Host.Type.LocalSecondaryStorage; - } else { - type = Host.Type.Storage; - } - - final Map hostDetails = ssCmd.getHostDetails(); - if (hostDetails != null) { - if (details != null) { - details.putAll(hostDetails); - } else { - details = hostDetails; - } - } - - host.setDetails(details); - host.setParent(ssCmd.getParent()); - host.setTotalSize(ssCmd.getTotalSize()); - host.setHypervisorType(HypervisorType.None); - host.setType(type); - if (ssCmd.getNfsShare() != null) { - host.setStorageUrl(ssCmd.getNfsShare()); - } - } - */ - return null; // no need to handle this event anymore since secondary storage is not in host table anymore. + return null; } + /** Since secondary storage is moved out of host table, this class should not handle delete secondary storage anymore. + */ @Override public DeleteHostAnswer deleteHost(HostVO host, boolean isForced, boolean isForceDeleteStorage) throws UnableDeleteHostException { - // Since secondary storage is moved out of host table, this class should not handle delete secondary storage anymore. return null; } diff --git a/services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerTest.java b/services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerTest.java index c42f2b130c8..98348fe2908 100644 --- a/services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerTest.java +++ b/services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerTest.java @@ -30,10 +30,16 @@ import org.mockito.MockitoAnnotations; import com.cloud.dc.DataCenterVO; import com.cloud.dc.DataCenter.NetworkType; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.hypervisor.Hypervisor; import com.cloud.network.Networks.TrafficType; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.NicProfile; +import com.cloud.vm.VirtualMachineProfile; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.any; @@ -189,4 +195,27 @@ public class SecondaryStorageManagerTest { _ssMgr.getDefaultNetworkForAdvancedZone(dc); } + + @Test + public void validateVerifySshAccessOnManagementNicForSystemVm(){ + Hypervisor.HypervisorType[] hypervisorTypesArray = Hypervisor.HypervisorType.values(); + List hypervisorTypesThatMustReturnManagementNic = new ArrayList<>(Arrays.asList(Hypervisor.HypervisorType.Hyperv)); + + for (Hypervisor.HypervisorType hypervisorType: hypervisorTypesArray) { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + NicProfile controlNic = Mockito.mock(NicProfile.class); + NicProfile managementNic = Mockito.mock(NicProfile.class); + + Mockito.doReturn(hypervisorType).when(virtualMachineProfileMock).getHypervisorType(); + + NicProfile expectedResult = controlNic; + if (hypervisorTypesThatMustReturnManagementNic.contains(hypervisorType)) { + expectedResult = managementNic; + } + + NicProfile result = _ssMgr.verifySshAccessOnManagementNicForSystemVm(virtualMachineProfileMock, controlNic, managementNic); + + Assert.assertEquals(expectedResult, result); + } + } }