Liron Aravot has uploaded a new change for review. Change subject: core: determine wether domain monitoring result is actual ......................................................................
core: determine wether domain monitoring result is actual When the domain monitoring results are reported back from vdsm, the first monitor run may haven't finished yet. In that case the returned status from vdsm is a dummy status and not an actual result. The non actual status could lead to a wrong decision making on the engine status (for example - decide that an inactive domain is now active). This patch uses the logic added on vdsm side (change I1fea5) to inspect whether the reported status is actual or not. Change-Id: Iddbba94d81694bd6cb1ce3648872eb81182a50ab Signed-off-by: lara...@redhat.com <lara...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java 8 files changed, 54 insertions(+), 17 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/37/38037/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java index ae9ac38..a196eb7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java @@ -275,7 +275,7 @@ } if (result.isSuccess()) { - Pair<Boolean, List<StorageDomainStatic>> vdsStatsResults = proceedVdsStats(!masterDomainInactiveOrUnknown); + Pair<Boolean, List<StorageDomainStatic>> vdsStatsResults = proceedVdsStats(!masterDomainInactiveOrUnknown, storagePool); result.setSuccess(vdsStatsResults.getFirst()); if (!result.isSuccess()) { result.setResultData(vdsStatsResults.getSecond()); @@ -302,14 +302,13 @@ return returnValue; } - private Pair<Boolean, List<StorageDomainStatic>> proceedVdsStats(boolean shouldCheckReportedDomains) { + private Pair<Boolean, List<StorageDomainStatic>> proceedVdsStats(boolean shouldCheckReportedDomains, StoragePool storagePool) { Pair<Boolean, List<StorageDomainStatic>> returnValue = new Pair<>(true, null); try { runVdsCommand(VDSCommandType.GetStats, new VdsIdAndVdsVDSCommandParametersBase(getVds())); if (shouldCheckReportedDomains) { List<Guid> problematicDomainsIds = - IrsBrokerCommand.fetchDomainsReportedAsProblematic(getVds().getStoragePoolId(), - getVds().getDomains()); + IrsBrokerCommand.fetchDomainsReportedAsProblematic(getVds().getDomains(), storagePool); for (Guid domainId : problematicDomainsIds) { StorageDomainStatic domainInfo = getStorageDomainStaticDAO().get(domainId); log.error("Storage Domain '{}' of pool '{}' is in problem in host '{}'", diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java index a85ab54..dad7682 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java @@ -471,4 +471,12 @@ return supportedInConfig(ConfigValues.GraphicsDeviceEnabled, version); } + /** + * @param version + * Compatibility version to check for. + * @return <code>true</code> if data center without spm is supported for the given version + */ + public static boolean reportWhetherDomainMonitoringResultIsActual(Version version) { + return supportedInConfig(ConfigValues.ReportWhetherDomainMonitoringResultIsActual, version); + } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java index e72b64c..fc44a86 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java @@ -9,6 +9,7 @@ private Guid privateDomainId; private double lastCheck; private double delay; + private boolean actual; public Guid getDomainId() { return privateDomainId; @@ -44,6 +45,14 @@ return delay; } + public boolean isActual() { + return actual; + } + + public void setActual(boolean actual) { + this.actual = actual; + } + public VDSDomainsData() { privateDomainId = Guid.Empty; } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java index a254905..0e75b73 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java @@ -2035,6 +2035,10 @@ @DefaultValueAttribute("true") GraphicsDeviceEnabled, + @TypeConverterAttribute(Boolean.class) + @DefaultValueAttribute("true") + ReportWhetherDomainMonitoringResultIsActual, + Invalid } diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java index 9a5ae6d..5ffd786 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java @@ -68,10 +68,10 @@ } } - public static List<Guid> fetchDomainsReportedAsProblematic(Guid storagePoolId, List<VDSDomainsData> vdsDomainsData) { - IrsProxyData proxy = _irsProxyData.get(storagePoolId); + public static List<Guid> fetchDomainsReportedAsProblematic(List<VDSDomainsData> vdsDomainsData, StoragePool storagePool) { + IrsProxyData proxy = _irsProxyData.get(storagePool.getId()); if (proxy != null) { - return proxy.obtainDomainsReportedAsProblematic(vdsDomainsData); + return proxy.obtainDomainsReportedAsProblematic(vdsDomainsData, storagePool); } return Collections.emptyList(); } diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java index 95b9e92..617768b 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java @@ -17,6 +17,7 @@ import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.common.AuditLogType; +import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.businessentities.AsyncTaskStatus; import org.ovirt.engine.core.common.businessentities.AsyncTaskStatusEnum; import org.ovirt.engine.core.common.businessentities.BusinessEntitiesDefinitions; @@ -1111,14 +1112,14 @@ // Unknown domains in pool for (VDSDomainsData tempData : data) { if (domainsInPool.contains(tempData.getDomainId())) { - DomainMonitoringResult domainMonitoringResult = analyzeDomainReport(tempData, false); + DomainMonitoringResult domainMonitoringResult = analyzeDomainReport(tempData, storagePool, false); if (domainMonitoringResult.invalid()) { domainsProblematicReportInfo.put(tempData.getDomainId(), domainMonitoringResult); } else if (tempData.getDelay() > Config.<Double> getValue(ConfigValues.MaxStorageVdsDelayCheckSec)) { logDelayedDomain(vdsId, tempData); } } else if (inActiveDomainsInPool.contains(tempData.getDomainId()) - && analyzeDomainReport(tempData, false).valid()) { + && analyzeDomainReport(tempData, storagePool, false).valid()) { log.warn("Storage Domain '{}' was reported by Host '{}' as Active in Pool '{}', moving to active status", getDomainIdTuple(tempData.getDomainId()), vdsName, @@ -1185,7 +1186,7 @@ AuditLogType.VDS_DOMAIN_DELAY_INTERVAL); } - protected List<Guid> obtainDomainsReportedAsProblematic(List<VDSDomainsData> vdsDomainsData) { + protected List<Guid> obtainDomainsReportedAsProblematic(List<VDSDomainsData> vdsDomainsData, StoragePool storagePool) { List<Guid> domainsInProblem = new LinkedList<>(); Set<Guid> domainsInPool = new HashSet<Guid>( DbFacade.getInstance().getStorageDomainStaticDao().getAllIds( @@ -1195,7 +1196,7 @@ List<Guid> domainWhichWereSeen = new ArrayList<Guid>(); for (VDSDomainsData vdsDomainData : vdsDomainsData) { if (domainsInPool.contains(vdsDomainData.getDomainId())) { - if (analyzeDomainReport(vdsDomainData, true).invalid()) { + if (analyzeDomainReport(vdsDomainData, storagePool, true).invalid()) { domainsInProblem.add(vdsDomainData.getDomainId()); } domainWhichWereSeen.add(vdsDomainData.getDomainId()); @@ -1212,24 +1213,34 @@ } private enum DomainMonitoringResult { - PROBLEMATIC(false), STORAGE_ACCCESS_ERROR(false), OK(true), NOT_REPORTED(false); + PROBLEMATIC(Boolean.FALSE), STORAGE_ACCCESS_ERROR(Boolean.FALSE), OK(Boolean.TRUE), NOT_REPORTED(Boolean.FALSE), NOT_ACTUAL(null); - private boolean valid; + private Boolean valid; - private DomainMonitoringResult(boolean valid) { + private DomainMonitoringResult(Boolean valid) { this.valid = valid; } public boolean valid() { - return valid; + return actual() && valid; } public boolean invalid() { - return !valid; + return actual() && !valid; } + + public boolean actual() {return this != NOT_ACTUAL;} } - private DomainMonitoringResult analyzeDomainReport(VDSDomainsData tempData, boolean isLog) { + private DomainMonitoringResult analyzeDomainReport(VDSDomainsData tempData, StoragePool storagePool, boolean isLog) { + if (!tempData.isActual() && + FeatureSupported.reportWhetherDomainMonitoringResultIsActual(storagePool.getCompatibilityVersion())) { + log.error("Domain '{}' report isn't an actual report '{}'", + getDomainIdTuple(tempData.getDomainId()), + tempData.getCode()); + return DomainMonitoringResult.NOT_ACTUAL; + } + if (tempData.getCode() != 0) { if (isLog) { log.error("Domain '{}' was reported with error code '{}'", diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java index 05268a8..3a76230 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java @@ -1019,6 +1019,11 @@ delay = Double.parseDouble((String) internalValue.get(VdsProperties.delay)); } data.setDelay(delay); + boolean actual = true; + if (internalValue.containsKey(VdsProperties.actual)) { + actual = Boolean.parseBoolean((String) internalValue.get(VdsProperties.actual)); + } + data.setActual(actual); domainsData.add(data); } catch (Exception e) { log.error("failed building domains: {}", e.getMessage()); diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java index c92b83b..0c7b25c 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java @@ -362,6 +362,7 @@ public static final String code = "code"; public static final String lastCheck = "lastCheck"; public static final String delay = "delay"; + public static final String actual = "actual"; public static final String DISK_STATS = "diskStats"; public static final String DISK_STATS_FREE = "free"; -- To view, visit http://gerrit.ovirt.org/38037 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iddbba94d81694bd6cb1ce3648872eb81182a50ab Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <lara...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches