Noam Slomianko has uploaded a new change for review. Change subject: core: external scheduler integration refactor ......................................................................
core: external scheduler integration refactor Changed the way we parse xml-rpc results to policy units To be simpler. There is no longer ExternalSchdulerDiscoveryUnits, the data is translated directly to PolicyUnits. Changed the comperison of the DB to be simpler as well. Change-Id: I77061dfccc0b1a113cf810ef6a77a55e07b7c597 Signed-off-by: Noam Slomianko <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerBroker.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerBrokerImpl.java D backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryResult.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryResultsParser.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryThread.java D backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryUnit.java 6 files changed, 162 insertions(+), 225 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/24/18324/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerBroker.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerBroker.java index 7541872..301e1b2 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerBroker.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerBroker.java @@ -3,11 +3,12 @@ import java.util.List; import java.util.Map; +import org.ovirt.engine.core.common.scheduling.PolicyUnit; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; public interface ExternalSchedulerBroker { - ExternalSchedulerDiscoveryResult runDiscover(); + List<PolicyUnit> runDiscover(); List<Guid> runFilters(List<String> filterNames, List<Guid> hostIDs, Guid vmID, Map<String, String> propertiesMap); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerBrokerImpl.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerBrokerImpl.java index d1f4a1a..49f2285 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerBrokerImpl.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerBrokerImpl.java @@ -11,6 +11,7 @@ import org.apache.xmlrpc.client.XmlRpcClientConfigImpl; import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; +import org.ovirt.engine.core.common.scheduling.PolicyUnit; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.utils.log.Log; @@ -43,25 +44,17 @@ } @Override - public ExternalSchedulerDiscoveryResult runDiscover() { + public List<PolicyUnit> runDiscover() { try { XmlRpcClient client = new XmlRpcClient(); client.setConfig(config); Object result = client.execute(DISCOVER, EMPTY); - return parseDiscoverResults(result); + return ExternalSchedulerDiscoveryResultsParser.parse(result); } catch (XmlRpcException e) { log.error("Could not communicate with the external scheduler while discovering", e); return null; } - } - - private ExternalSchedulerDiscoveryResult parseDiscoverResults(Object result) { - ExternalSchedulerDiscoveryResult retValue = new ExternalSchedulerDiscoveryResult(); - if (!retValue.populate(result)) { - return null; - } - return retValue; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryResult.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryResult.java deleted file mode 100644 index ca010ec..0000000 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryResult.java +++ /dev/null @@ -1,105 +0,0 @@ -package org.ovirt.engine.core.bll.scheduling.external; - -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; - -import org.apache.commons.lang.StringUtils; -import org.ovirt.engine.core.utils.customprop.SimpleCustomPropertiesUtil; -import org.ovirt.engine.core.utils.log.Log; -import org.ovirt.engine.core.utils.log.LogFactory; - - -public class ExternalSchedulerDiscoveryResult { - private static Log log = LogFactory.getLog(ExternalSchedulerDiscoveryResult.class); - private static final String FILTERS = "filters"; - private static final String SCORES = "scores"; - private static final String BALANCE = "balance"; - private List<ExternalSchedulerDiscoveryUnit> filters; - private List<ExternalSchedulerDiscoveryUnit> scores; - private List<ExternalSchedulerDiscoveryUnit> balance; - - ExternalSchedulerDiscoveryResult() { - filters = new LinkedList<ExternalSchedulerDiscoveryUnit>(); - scores = new LinkedList<ExternalSchedulerDiscoveryUnit>(); - balance = new LinkedList<ExternalSchedulerDiscoveryUnit>(); - } - - public boolean populate(Object xmlRpcRawResult) { - try { - if (xmlRpcRawResult == null || !(xmlRpcRawResult instanceof HashMap)) { - log.error("External scheduler error, malformed discover results"); - return false; - } - @SuppressWarnings("unchecked") - HashMap<String, HashMap<String, Object[]>> castedResult = (HashMap<String, HashMap<String, Object[]>>) xmlRpcRawResult; - - // keys will be filter, score and balance - for (String type : castedResult.keySet()) { - HashMap<String, Object[]> typeMap = castedResult.get(type); - List<ExternalSchedulerDiscoveryUnit> currentList = getRelevantList(type); - if (currentList == null) { - log.error("External scheduler error, got unknown type"); - return false; - } - // list of module names as keys and [description, regex] as value - for (String moduleName : typeMap.keySet()) { - Object[] singleModule = typeMap.get(moduleName); - // check custom properties format. - String customPropertiesRegex = singleModule[1].toString(); - if (!StringUtils.isEmpty(customPropertiesRegex) && SimpleCustomPropertiesUtil.getInstance() - .syntaxErrorInProperties(customPropertiesRegex)) { - log.error("module " + moduleName + " will not be loaded, wrong custom properties format (" - + customPropertiesRegex + ")"); - continue; - } - ExternalSchedulerDiscoveryUnit currentUnit = new ExternalSchedulerDiscoveryUnit(moduleName, - singleModule[0].toString(), - customPropertiesRegex); - currentList.add(currentUnit); - } - } - return true; - } catch (Exception e) { - log.error("External scheduler error, exception why parsing discovery results", e); - return false; - } - } - - private List<ExternalSchedulerDiscoveryUnit> getRelevantList(String type) { - switch (type) { - case FILTERS: - return filters; - case SCORES: - return scores; - case BALANCE: - return balance; - default: - return null; - } - } - - List<ExternalSchedulerDiscoveryUnit> getFilters() { - return filters; - } - - void setFilters(List<ExternalSchedulerDiscoveryUnit> filters) { - this.filters = filters; - } - - List<ExternalSchedulerDiscoveryUnit> getScores() { - return scores; - } - - void setScores(List<ExternalSchedulerDiscoveryUnit> scores) { - this.scores = scores; - } - - List<ExternalSchedulerDiscoveryUnit> getBalance() { - return balance; - } - - void setBalance(List<ExternalSchedulerDiscoveryUnit> balance) { - this.balance = balance; - } -} diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryResultsParser.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryResultsParser.java new file mode 100644 index 0000000..2120a3c --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryResultsParser.java @@ -0,0 +1,80 @@ +package org.ovirt.engine.core.bll.scheduling.external; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; + +import org.apache.commons.lang.StringUtils; +import org.ovirt.engine.core.common.scheduling.PolicyUnit; +import org.ovirt.engine.core.common.scheduling.PolicyUnitType; +import org.ovirt.engine.core.utils.customprop.SimpleCustomPropertiesUtil; +import org.ovirt.engine.core.utils.log.Log; +import org.ovirt.engine.core.utils.log.LogFactory; + +public class ExternalSchedulerDiscoveryResultsParser { + private static Log log = LogFactory.getLog(ExternalSchedulerDiscoveryResultsParser.class); + private static final String FILTERS = "filters"; + private static final String SCORES = "scores"; + private static final String BALANCE = "balance"; + + public static List<PolicyUnit> parse(Object xmlRpcRawResult) { + try { + if (xmlRpcRawResult == null || !(xmlRpcRawResult instanceof HashMap)) { + log.error("External scheduler error, malformed discover results"); + return null; + } + @SuppressWarnings("unchecked") + HashMap<String, HashMap<String, Object[]>> castedResult = + (HashMap<String, HashMap<String, Object[]>>) xmlRpcRawResult; + List<PolicyUnit> foundUnits = new ArrayList<PolicyUnit>(); + // keys will be filter, score and balance + for (String type : castedResult.keySet()) { + + HashMap<String, Object[]> typeMap = castedResult.get(type); + PolicyUnitType policyType = getType(type); + if (policyType == null) { + log.error("External scheduler error, got unknown type"); + return null; + } + // list of module names as keys and [description, regex] as value + for (String moduleName : typeMap.keySet()) { + Object[] singleModule = typeMap.get(moduleName); + // check custom properties format. + String customPropertiesRegex = singleModule[1].toString(); + if (!StringUtils.isEmpty(customPropertiesRegex) && SimpleCustomPropertiesUtil.getInstance() + .syntaxErrorInProperties(customPropertiesRegex)) { + log.error("module " + moduleName + " will not be loaded, wrong custom properties format (" + + customPropertiesRegex + ")"); + continue; + } + PolicyUnit currentUnit = new PolicyUnit(); + currentUnit.setName(moduleName); + currentUnit.setDescription(singleModule[0].toString()); + currentUnit.setEnabled(true); + currentUnit.setInternal(false); + currentUnit.setParameterRegExMap(SimpleCustomPropertiesUtil.getInstance() + .convertProperties(customPropertiesRegex)); + + foundUnits.add(currentUnit); + } + } + return foundUnits; + } catch (Exception e) { + log.error("External scheduler error, exception why parsing discovery results", e); + return null; + } + } + + private static PolicyUnitType getType(String type) { + switch (type) { + case FILTERS: + return PolicyUnitType.Filter; + case SCORES: + return PolicyUnitType.Weight; + case BALANCE: + return PolicyUnitType.LoadBalancing; + default: + return null; + } + } +} diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryThread.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryThread.java index a85630f..a185ea4 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryThread.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryThread.java @@ -1,18 +1,13 @@ package org.ovirt.engine.core.bll.scheduling.external; -import java.util.LinkedHashMap; -import java.util.LinkedList; +import java.util.ArrayList; import java.util.List; -import java.util.Map; -import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.scheduling.SchedulingManager; import org.ovirt.engine.core.common.scheduling.PolicyUnit; -import org.ovirt.engine.core.common.scheduling.PolicyUnitType; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dao.scheduling.PolicyUnitDao; -import org.ovirt.engine.core.utils.customprop.SimpleCustomPropertiesUtil; import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; @@ -22,7 +17,7 @@ @Override public void run() { - ExternalSchedulerDiscoveryResult discoveryResult = ExternalSchedulerFactory.getInstance().runDiscover(); + List<PolicyUnit> discoveryResult = ExternalSchedulerFactory.getInstance().runDiscover(); if (discoveryResult != null) { updateDB(discoveryResult); log.info("PolicyUnits updated"); @@ -31,100 +26,101 @@ } } - private void updateDB(ExternalSchedulerDiscoveryResult discoveryResult) { - List<PolicyUnit> allPolicyUnits = getPolicyUnitDao().getAll(); - List<PolicyUnit> foundInBoth = new LinkedList<PolicyUnit>(); - for (ExternalSchedulerDiscoveryUnit unit : discoveryResult.getFilters()) { - PolicyUnit found = compareToDB(allPolicyUnits, unit, PolicyUnitType.Filter); - if (found != null) { - foundInBoth.add(found); - } - } - for (ExternalSchedulerDiscoveryUnit unit : discoveryResult.getScores()) { - PolicyUnit found = compareToDB(allPolicyUnits, unit, PolicyUnitType.Weight); - if (found != null) { - foundInBoth.add(found); - } - } - for (ExternalSchedulerDiscoveryUnit unit : discoveryResult.getBalance()) { - PolicyUnit found = compareToDB(allPolicyUnits, unit, PolicyUnitType.LoadBalancing); - if (found != null) { - foundInBoth.add(found); - } + private void updateDB(List<PolicyUnit> discoveryResults) { + List<PolicyUnit> policyUnitsFromDB = getExternalPolicyUnitsFromDB(); + + List<PolicyUnit> policyUnitsToAddToDB = findNewPolicyUnitsFromDiscover(policyUnitsFromDB, discoveryResults); + List<PolicyUnit> policyUnitsToUpdate = findPolicyUnitsToUpdate(policyUnitsFromDB, discoveryResults); + List<PolicyUnit> policyUnitsToMarkAsDisable = + findPolicyUnitsFromDBNoLongerDiscovered(policyUnitsFromDB, discoveryResults); + + for (PolicyUnit policyUnit : policyUnitsToAddToDB) { + getPolicyUnitDao().save(policyUnit); } - allPolicyUnits.removeAll(foundInBoth); - // found in the db but not found in discovery, mark as such - markExternalPoliciesAsDisabled(allPolicyUnits); + for (PolicyUnit policyUnit : policyUnitsToUpdate) { + getPolicyUnitDao().update(policyUnit); + } + + for (PolicyUnit policyUnit : policyUnitsToMarkAsDisable) { + getPolicyUnitDao().update(policyUnit); + } SchedulingManager.getInstance().reloadPolicyUnits(); } - private void markExternalPoliciesAsDisabled(List<PolicyUnit> units) { - for (PolicyUnit policyUnit : units) { + private List<PolicyUnit> getExternalPolicyUnitsFromDB() { + List<PolicyUnit> allPolicyUnits = getPolicyUnitDao().getAll(); + List<PolicyUnit> externalPolicyUnits = new ArrayList<PolicyUnit>(); + for (PolicyUnit policyUnit : allPolicyUnits) { if (!policyUnit.isInternal()) { - policyUnit.setEnabled(false); - getPolicyUnitDao().update(policyUnit); + externalPolicyUnits.add(policyUnit); } } + return externalPolicyUnits; } - private PolicyUnit compareToDB(List<PolicyUnit> dbEnteries, - ExternalSchedulerDiscoveryUnit discoveryUnit, - PolicyUnitType type) { - for (PolicyUnit policyUnit : dbEnteries) { - if (policyUnit.isInternal()) { - continue; + private List<PolicyUnit> findNewPolicyUnitsFromDiscover(List<PolicyUnit> fromDB, List<PolicyUnit> fromDiscover) { + List<PolicyUnit> retList = new ArrayList<PolicyUnit>(); + for (PolicyUnit discoverPolicyUnit : fromDiscover) { + boolean found = false; + for (PolicyUnit dbPolicyUnit : fromDB) { + if (isTheSameUnit(dbPolicyUnit, discoverPolicyUnit)) { + found = true; + break; + } } - - if (policyUnit.getPolicyUnitType() != type) { - continue; + if (!found) { + discoverPolicyUnit.setId(Guid.newGuid()); + retList.add(discoverPolicyUnit); } - - if (!policyUnit.getName().equals(discoveryUnit.getName())) { - continue; - } - - Map<String, String> discoveryPropMap = - StringUtils.isEmpty(discoveryUnit.getRegex()) ? new LinkedHashMap<String, String>() : - SimpleCustomPropertiesUtil.getInstance().convertProperties(discoveryUnit.getRegex()); - if (!discoveryPropMap.equals(policyUnit.getParameterRegExMap()) || - !discoveryUnit.getDescription().equals(policyUnit.getDescription()) || - !policyUnit.isEnabled()) { - sendToDb(discoveryUnit, policyUnit.getId(), type); - } - - return policyUnit; - } - sendToDb(discoveryUnit, null, type); - return null; + return retList; } - private void sendToDb(ExternalSchedulerDiscoveryUnit discovery, Guid policyUnitId, PolicyUnitType type) { - PolicyUnit policy = createFromDiscoveryUnit(discovery, type); - if (policyUnitId != null) { - policy.setId(policyUnitId); - getPolicyUnitDao().update(policy); - } else { - policy.setId(Guid.newGuid()); - getPolicyUnitDao().save(policy); + private List<PolicyUnit> findPolicyUnitsToUpdate(List<PolicyUnit> fromDB, List<PolicyUnit> fromDiscover) { + List<PolicyUnit> retList = new ArrayList<PolicyUnit>(); + for (PolicyUnit discoverPolicyUnit : fromDiscover) { + for (PolicyUnit dbPolicyUnit : fromDB) { + if (isTheSameUnit(dbPolicyUnit, discoverPolicyUnit) + && !partialComapre(dbPolicyUnit, discoverPolicyUnit)) { + discoverPolicyUnit.setId(dbPolicyUnit.getId()); + retList.add(discoverPolicyUnit); + } + } } + return retList; } - private PolicyUnit createFromDiscoveryUnit(ExternalSchedulerDiscoveryUnit discoveryUnit, PolicyUnitType type) { - PolicyUnit policy = new PolicyUnit(); - policy.setInternal(false); - policy.setName(discoveryUnit.getName()); - policy.setPolicyUnitType(type); - policy.setDescription(discoveryUnit.getDescription()); - if (!StringUtils.isEmpty(discoveryUnit.getRegex())) { - policy.setParameterRegExMap(SimpleCustomPropertiesUtil.getInstance() - .convertProperties(discoveryUnit.getRegex())); - } else { - policy.setParameterRegExMap(new LinkedHashMap<String, String>()); + private List<PolicyUnit> findPolicyUnitsFromDBNoLongerDiscovered(List<PolicyUnit> fromDB, + List<PolicyUnit> fromDiscover) { + List<PolicyUnit> retList = new ArrayList<PolicyUnit>(); + for (PolicyUnit dbPolicyUnit : fromDB) { + boolean found = false; + for (PolicyUnit discoverPolicyUnit : fromDiscover) { + if (isTheSameUnit(dbPolicyUnit, discoverPolicyUnit)) { + found = true; + break; + } + } + if (!found) { + dbPolicyUnit.setEnabled(false); + retList.add(dbPolicyUnit); + } } - return policy; + return retList; + } + + private boolean isTheSameUnit(PolicyUnit fromDB, PolicyUnit fromDiscovery) { + return fromDB.getName().equals(fromDiscovery.getName()) + && fromDB.getPolicyUnitType().equals(fromDiscovery.getPolicyUnitType()); + } + + private boolean partialComapre(PolicyUnit fromDB, PolicyUnit fromDiscovery) { + return fromDB.getName().equals(fromDiscovery.getName()) + && fromDB.getDescription().equals(fromDiscovery.getDescription()) + && fromDB.getParameterRegExMap().equals(fromDiscovery.getParameterRegExMap()) + && fromDB.getPolicyUnitType().equals(fromDiscovery.getPolicyUnitType()); } public PolicyUnitDao getPolicyUnitDao() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryUnit.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryUnit.java deleted file mode 100644 index 8fcaa3b..0000000 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/external/ExternalSchedulerDiscoveryUnit.java +++ /dev/null @@ -1,28 +0,0 @@ -package org.ovirt.engine.core.bll.scheduling.external; - -public class ExternalSchedulerDiscoveryUnit{ - private String name; - private String description; - private String regex; - - public ExternalSchedulerDiscoveryUnit(String name, String description, String regex) { - this.name = name; - this.description = description; - this.regex = regex; - } - - String getName() { - return name; - } - - String getDescription() { - return description; - } - - String getRegex() { - return regex; - } - -} - - -- To view, visit http://gerrit.ovirt.org/18324 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I77061dfccc0b1a113cf810ef6a77a55e07b7c597 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Noam Slomianko <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
