modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java | 231 +++--- modules/core/plugin-container/src/main/java/org/rhq/core/pc/upgrade/DiscoverySuspendedException.java | 50 + modules/core/plugin-container/src/main/java/org/rhq/core/pc/upgrade/ResourceUpgradeDelegate.java | 57 - modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/FakeServerInventory.java | 2 modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ForwardingProxy.java | 72 ++ modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeFailureHandlingTest.java | 334 +++++++++- modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeTest.java | 205 +++--- modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseResourceComponent.java | 67 +- modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseResourceComponentInterface.java | 36 + modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseUpgradingDiscoveryComponent.java | 7 modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/NothingDiscoveringDiscoveryComponent.java | 42 + modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/sibling1/rhq-plugin.xml | 2 modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/sibling2/rhq-plugin.xml | 2 modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/v2/rhq-plugin.xml | 2 14 files changed, 854 insertions(+), 255 deletions(-)
New commits: commit e7fb6edacd4c172a236cfc39b19096f03cef85ee Author: Lukas Krejci lkrejci@redhat.com Date: Mon Oct 4 18:34:25 2010 +0200
Discovery now recurses to child resource types of the existing resources even if the discovery of the existing resources' type is disallowed due to upgrade failure. This means that no new siblings will be discovered but the existing resources "contents" will be up-to-date with the plugin version.
Added tests for this behaviour and updated the existing to reflect the changes.
diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java index 99a0513..ec40b0c 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java @@ -91,6 +91,7 @@ import org.rhq.core.pc.plugin.BlacklistedException; import org.rhq.core.pc.plugin.CanonicalResourceKey; import org.rhq.core.pc.plugin.PluginComponentFactory; import org.rhq.core.pc.plugin.PluginManager; +import org.rhq.core.pc.upgrade.DiscoverySuspendedException; import org.rhq.core.pc.upgrade.ResourceUpgradeDelegate; import org.rhq.core.pc.util.DiscoveryComponentProxyFactory; import org.rhq.core.pc.util.FacetLockType; @@ -289,18 +290,22 @@ public class InventoryManager extends AgentService implements ContainerService, * @param context the context for use by the discovery component * @return the details of all discovered resources, may be empty or <code>null</code> * + * @throws DiscoverySuspendedException if the discovery is suspended due to a resource upgrade failure * @throws Exception if the discovery component threw an exception */ public Set<DiscoveredResourceDetails> invokeDiscoveryComponent(ResourceContainer parentResourceContainer, - ResourceDiscoveryComponent component, ResourceDiscoveryContext context) throws Exception { + ResourceDiscoveryComponent component, ResourceDiscoveryContext context) throws DiscoverySuspendedException, Exception {
Resource parentResource = parentResourceContainer == null ? null : parentResourceContainer.getResource();
if (resourceUpgradeDelegate.hasUpgradeFailedInChildren(parentResource, context.getResourceType())) { - log.debug("Discovery of [" + context.getResourceType() + "] has been disallowed under " - + (parentResource == null ? " the platform " : parentResource) - + " because some of its siblings failed to upgrade."); - return null; + String message = "Discovery of [" + context.getResourceType() + "] has been suspended under " + + (parentResource == null ? " the platform " : parentResource) + + " because some of its siblings failed to upgrade."; + + log.debug(message); + + throw new DiscoverySuspendedException(message); }
long timeout = getDiscoveryComponentTimeout(context.getResourceType()); @@ -699,19 +704,29 @@ public class InventoryManager extends AgentService implements ContainerService,
// Ask the plugin's discovery component to find the new resource, throwing exceptions if it cannot be // found at all. - Set<DiscoveredResourceDetails> discoveredResources = invokeDiscoveryComponent(parentResourceContainer, - discoveryComponent, discoveryContext); - if ((discoveredResources == null) || discoveredResources.isEmpty()) { - log.info("Plugin Error: During manual add, discovery component method [" - + discoveryComponent.getClass().getName() + ".discoverResources()] returned " - + discoveredResources + " when passed a single plugin configuration " - + "(either the resource type was blacklisted or the plugin developer " - + "did not implement support for manually discovered resources correctly)."); - throw new PluginContainerException("The [" + resourceType.getPlugin() - + "] plugin does not properly support manual addition of [" + resourceType.getName() - + "] resources."); + try { + Set<DiscoveredResourceDetails> discoveredResources = invokeDiscoveryComponent(parentResourceContainer, + discoveryComponent, discoveryContext); + if ((discoveredResources == null) || discoveredResources.isEmpty()) { + log.info("Plugin Error: During manual add, discovery component method [" + + discoveryComponent.getClass().getName() + ".discoverResources()] returned " + + discoveredResources + " when passed a single plugin configuration " + + "(either the resource type was blacklisted or the plugin developer " + + "did not implement support for manually discovered resources correctly)."); + throw new PluginContainerException("The [" + resourceType.getPlugin() + + "] plugin does not properly support manual addition of [" + resourceType.getName() + + "] resources."); + } + discoveredResourceDetails = discoveredResources.iterator().next(); + } catch (DiscoverySuspendedException e) { + String message = "The discovery class [" + discoveryComponent.getClass().getName() + "]" + + " uses a legacy implementation of "manual add" functionality. Some of the child resources" + + " with the resource type [" + resourceType + "] under the parent resource [" + parentResourceContainer.getResource() + "]" + + " failed to upgrade, which makes it impossible to support the legacy manual-add implementation. Either upgrade the plugin [" + + resourceType.getPlugin() + "] to successfully upgrade all resources or consider implementing the ManuallAdd facet."; + log.info(message); + throw new PluginContainerException(message, e); } - discoveredResourceDetails = discoveredResources.iterator().next(); }
// Create the new Resource and add it to inventory if it isn't already there. @@ -1844,6 +1859,8 @@ public class InventoryManager extends AgentService implements ContainerService,
try { discoveredResources = invokeDiscoveryComponent(null, component, context); + } catch (DiscoverySuspendedException e) { + log.error("Discovery seems to be suspended for platforms due to upgrade error.", e); } catch (Throwable e) { log.warn("Platform plugin discovery failed - skipping", e); } @@ -2179,35 +2196,47 @@ public class InventoryManager extends AgentService implements ContainerService, parentResourceContext, SystemInfoFactory.createSystemInfo(), processScanResults, Collections.EMPTY_LIST, this.configuration.getContainerName(), this.configuration.getPluginContainerDeployment()); - Set<DiscoveredResourceDetails> discoveredResources = invokeDiscoveryComponent(parentContainer, - discoveryComponent, context); newResources = new HashSet<Resource>(); - if ((discoveredResources != null) && (!discoveredResources.isEmpty())) { - IdentityHashMap<Configuration, DiscoveredResourceDetails> pluginConfigObjects = new IdentityHashMap<Configuration, DiscoveredResourceDetails>(); - for (DiscoveredResourceDetails discoveredResource : discoveredResources) { - if (discoveredResource == null) { - throw new IllegalStateException("Plugin error: Discovery class " - + discoveryComponent.getClass().getName() - + " returned a Set containing one or more null items."); - } - if (!discoveredResource.getResourceType().equals(resourceType)) { - throw new IllegalStateException("Plugin error: Discovery class " - + discoveryComponent.getClass().getName() - + " returned a DiscoveredResourceDetails with an incorrect ResourceType (was " - + discoveredResource.getResourceType().getName() + " but should have been " - + resourceType.getName()); + try { + Set<DiscoveredResourceDetails> discoveredResources = invokeDiscoveryComponent(parentContainer, + discoveryComponent, context); + if ((discoveredResources != null) && (!discoveredResources.isEmpty())) { + IdentityHashMap<Configuration, DiscoveredResourceDetails> pluginConfigObjects = new IdentityHashMap<Configuration, DiscoveredResourceDetails>(); + for (DiscoveredResourceDetails discoveredResource : discoveredResources) { + if (discoveredResource == null) { + throw new IllegalStateException("Plugin error: Discovery class " + + discoveryComponent.getClass().getName() + + " returned a Set containing one or more null items."); + } + if (!discoveredResource.getResourceType().equals(resourceType)) { + throw new IllegalStateException("Plugin error: Discovery class " + + discoveryComponent.getClass().getName() + + " returned a DiscoveredResourceDetails with an incorrect ResourceType (was " + + discoveredResource.getResourceType().getName() + " but should have been " + + resourceType.getName()); + } + if (null != pluginConfigObjects + .put(discoveredResource.getPluginConfiguration(), discoveredResource)) { + throw new IllegalStateException("The plugin component " + + discoveryComponent.getClass().getName() + + " returned multiple resources that point to the same plugin configuration object on the " + + "resource type [" + resourceType + "]. This is not allowed, please use " + + "ResoureDiscoveryContext.getDefaultPluginConfiguration() " + + "for each discovered resource."); + } + Resource newResource = InventoryManager.createNewResource(discoveredResource); + newResources.add(newResource); } - if (null != pluginConfigObjects - .put(discoveredResource.getPluginConfiguration(), discoveredResource)) { - throw new IllegalStateException("The plugin component " - + discoveryComponent.getClass().getName() - + " returned multiple resources that point to the same plugin configuration object on the " - + "resource type [" + resourceType + "]. This is not allowed, please use " - + "ResoureDiscoveryContext.getDefaultPluginConfiguration() " - + "for each discovered resource."); + } + } catch (DiscoverySuspendedException e) { + //ok, the discovery is suspended for this resource type under this parent. + //but we can continue the discovery in the child resource types of the existing resources. + //we can therefore pretend that the discovery returned the existing resources so that + //we can recurse into their children up in the call-chain. + for(Resource existingResource : parentResource.getChildResources()) { + if (resourceType.equals(existingResource.getResourceType())) { + newResources.add(existingResource); } - Resource newResource = InventoryManager.createNewResource(discoveredResource); - newResources.add(newResource); } } } catch (Throwable e) { @@ -2654,14 +2683,14 @@ public class InventoryManager extends AgentService implements ContainerService, if (container != null) { if (container.getResourceComponentState() == ResourceComponentState.STARTED) { try { - resourceUpgradeDelegate.processAndQueue(container); + if (resourceUpgradeDelegate.processAndQueue(container)) { + for (Resource child : resource.getChildResources()) { + upgradeResource(child); + } + } } catch (PluginContainerException e) { log.error("Exception thrown while upgrading [" + resource + "].", e); } - - for (Resource child : resource.getChildResources()) { - upgradeResource(child); - } } else { String message = "The resource container for resource [" + resource + "] wasn't started while upgrading."; diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/upgrade/DiscoverySuspendedException.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/upgrade/DiscoverySuspendedException.java new file mode 100644 index 0000000..5cb695d --- /dev/null +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/upgrade/DiscoverySuspendedException.java @@ -0,0 +1,50 @@ +/* + * RHQ Management Platform + * Copyright (C) 2005-2010 Red Hat, Inc. + * All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +package org.rhq.core.pc.upgrade; + +import org.rhq.core.pc.inventory.InventoryManager; + +/** + * This exception is thrown from the {@link InventoryManager#invokeDiscoveryComponent(org.rhq.core.pc.inventory.ResourceContainer, org.rhq.core.pluginapi.inventory.ResourceDiscoveryComponent, org.rhq.core.pluginapi.inventory.ResourceDiscoveryContext)} if the + * discovery is suspended for given resource type due to a resource upgrade failure. + * + * @author Lukas Krejci + */ +public class DiscoverySuspendedException extends Exception { + + private static final long serialVersionUID = 1L; + + public DiscoverySuspendedException() { + super(); + } + + public DiscoverySuspendedException(String message, Throwable cause) { + super(message, cause); + } + + public DiscoverySuspendedException(String message) { + super(message); + } + + public DiscoverySuspendedException(Throwable cause) { + super(cause); + } + +} diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/upgrade/ResourceUpgradeDelegate.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/upgrade/ResourceUpgradeDelegate.java index 98a897e..06d2cd0 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/upgrade/ResourceUpgradeDelegate.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/upgrade/ResourceUpgradeDelegate.java @@ -26,6 +26,7 @@ import java.util.Set;
import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.rhq.core.clientapi.agent.PluginContainerException; import org.rhq.core.clientapi.agent.upgrade.ResourceUpgradeRequest; import org.rhq.core.domain.resource.Resource; @@ -61,7 +62,7 @@ public class ResourceUpgradeDelegate { private Set<Resource> failedResources = new HashSet<Resource>(); private Map<Resource, Set<ResourceType>> failedResourceTypesPerParent = new HashMap<Resource, Set<ResourceType>>(); private boolean mergeFailed; - + public ResourceUpgradeDelegate(InventoryManager inventoryManager) { this.inventoryManager = inventoryManager; } @@ -84,13 +85,17 @@ public class ResourceUpgradeDelegate { /** * Asks the resource's discovery component to upgrade the resource. * @param resourceContainer - * @return true if the resource was queued for upgrade, false otherwise + * @return true if the resource was queued for upgrade with no problems, + * false if there was some problem upgrading and the resource container was deactivated as + * a results of that. * @throws PluginContainerException on error */ - public void processAndQueue(ResourceContainer resourceContainer) throws PluginContainerException { + public boolean processAndQueue(ResourceContainer resourceContainer) throws PluginContainerException { if (enabled) { - executeResourceUpgradeFacetAndStoreRequest(resourceContainer); + return executeResourceUpgradeFacetAndStoreRequest(resourceContainer); } + + return true; }
/** @@ -99,7 +104,7 @@ public class ResourceUpgradeDelegate { public boolean hasUpgradeMergeFailed() { return mergeFailed; } - + /** * Tells whether given resource had a upgrade failure during the {@link #processAndQueue(ResourceContainer)} invocation. * @@ -109,7 +114,7 @@ public class ResourceUpgradeDelegate { public boolean hasUpgradeFailed(Resource resource) { return mergeFailed || failedResources.contains(resource); } - + /** * Tells whether at least one of the children with given resource type of the given parent resource * had an upgrade failure during {@link #processAndQueue(ResourceContainer)} invocation. @@ -120,35 +125,35 @@ public class ResourceUpgradeDelegate { if (mergeFailed) { return true; } - + Set<ResourceType> failedTypes = failedResourceTypesPerParent.get(parentResource); - + return failedTypes != null && failedTypes.contains(childrenResourceType); } - + public void sendRequests() throws Throwable { if (enabled && requests.size() > 0) { try { inventoryManager.mergeResourcesFromUpgrade(requests); } catch (Throwable t) { mergeFailed = true; - + //deactivate all the resources to be upgraded. We might have a problem //because they have not been upgraded because the merge failed. - for(ResourceUpgradeRequest request : requests) { + for (ResourceUpgradeRequest request : requests) { ResourceContainer container = inventoryManager.getResourceContainer(request.getResourceId()); if (container != null) { inventoryManager.deactivateResource(container.getResource()); } } - + throw t; } } }
@SuppressWarnings("unchecked") - private <T extends ResourceComponent> void executeResourceUpgradeFacetAndStoreRequest( + private <T extends ResourceComponent> boolean executeResourceUpgradeFacetAndStoreRequest( ResourceContainer resourceContainer) throws PluginContainerException {
ResourceComponent<T> parentResourceComponent = resourceContainer.getResourceContext() @@ -166,16 +171,16 @@ public class ResourceUpgradeDelegate {
if (!(discoveryComponent instanceof ResourceUpgradeFacet)) { //well, there's no point in continuing if the resource doesn't support the facet - return; + return true; }
ResourceUpgradeContext<ResourceComponent<T>> upgradeContext = inventoryManager.createResourceUpgradeContext( resource, parentResourceComponent, discoveryComponent);
ResourceUpgradeRequest request = new ResourceUpgradeRequest(resource.getId()); - + request.setTimestamp(System.currentTimeMillis()); - + ResourceUpgradeReport upgradeReport = null; try { upgradeReport = inventoryManager.invokeDiscoveryComponentResourceUpgradeFacet(resource.getResourceType(), @@ -189,13 +194,13 @@ public class ResourceUpgradeDelegate { String upgradeErrors = null; if ((upgradeErrors = checkUpgradeValid(resource, upgradeReport)) != null) { String errorString = "Upgrading the resource [" + resource + "] using these updates [" + upgradeReport - + "] would render the inventory invalid because of the following reasons: " + upgradeErrors; - + + "] would render the inventory invalid because of the following reasons: " + upgradeErrors; + log.error(errorString); - + IllegalStateException ex = new IllegalStateException(errorString); ex.fillInStackTrace(); - + request.setErrorProperties(ex); } else { request.fillInFromReport(upgradeReport); @@ -207,20 +212,24 @@ public class ResourceUpgradeDelegate { if (request.hasSomethingToUpgrade()) { requests.add(request); } - + if (request.getUpgradeErrorMessage() != null) { failedResources.add(resource); - + Set<ResourceType> failedResourceTypesInParent = failedResourceTypesPerParent.get(parentResource); if (failedResourceTypesInParent == null) { failedResourceTypesInParent = new HashSet<ResourceType>(); failedResourceTypesPerParent.put(parentResource, failedResourceTypesInParent); } - + failedResourceTypesInParent.add(resource.getResourceType()); - + inventoryManager.deactivateResource(resource); + + return false; } + + return true; }
private String checkUpgradeValid(Resource resource, ResourceUpgradeReport upgradeReport) { diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeFailureHandlingTest.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeFailureHandlingTest.java index 7eafafc..ebb73be 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeFailureHandlingTest.java +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeFailureHandlingTest.java @@ -156,9 +156,9 @@ public class ResourceUpgradeFailureHandlingTest extends ResourceUpgradeTestBase
//check that the resources are upgraded checkResourcesUpgraded(resources.get(PARENT_DEP_SIBLING_TYPE), 1); - checkResourcesUpgraded(resources.get(PARENT_TYPE), 2); - checkResourcesUpgraded(resources.get(SIBLING_TYPE), 30); - checkResourcesUpgraded(resources.get(TEST_TYPE), 30); + checkResourcesUpgraded(resources.get(PARENT_TYPE), 3); + checkResourcesUpgraded(resources.get(SIBLING_TYPE), 45); + checkResourcesUpgraded(resources.get(TEST_TYPE), 45); }
public Expectations getExpectations(Mockery context) throws Exception { @@ -206,19 +206,24 @@ public class ResourceUpgradeFailureHandlingTest extends ResourceUpgradeTestBase
checkNumberOfResources(resources, ROOT_TYPE, 1); checkNumberOfResources(resources, PARENT_DEP_TYPE, 1); - checkNumberOfResources(resources, PARENT_TYPE, 2);
- checkResourcesUpgraded(resources.get(PARENT_TYPE), 2); + checkResourcesUpgraded(resources.get(PARENT_TYPE), 3); checkResourcesUpgraded(resources.get(PARENT_DEP_SIBLING_TYPE), 1);
Resource parent0 = findResourceWithOrdinal(PARENT_TYPE, 0); Resource parent1 = findResourceWithOrdinal(PARENT_TYPE, 1); - - Set<Resource> siblingsUnderParent0 = filterResources(parent0.getChildResources(), SIBLING_TYPE); + Resource parent2 = findResourceWithOrdinal(PARENT_TYPE, 2); + Set<Resource> testsUnderParent0 = filterResources(parent0.getChildResources(), TEST_TYPE); - Set<Resource> siblingsUnderParent1 = filterResources(parent1.getChildResources(), SIBLING_TYPE); + Set<Resource> siblingsUnderParent0 = filterResources(parent0.getChildResources(), SIBLING_TYPE); Set<Resource> testsUnderParent1 = filterResources(parent1.getChildResources(), TEST_TYPE); - + Set<Resource> siblingsUnderParent1 = filterResources(parent1.getChildResources(), SIBLING_TYPE); + Set<Resource> testsUnderParent2 = filterResources(parent2.getChildResources(), TEST_TYPE); + Set<Resource> siblingsUnderParent2 = filterResources(parent2.getChildResources(), SIBLING_TYPE); + + //first check for the successful upgrades + checkResourcesUpgraded(testsUnderParent2, 15); + checkResourcesUpgraded(siblingsUnderParent2, 15); checkResourcesUpgraded(siblingsUnderParent0, 15); checkResourcesUpgraded(testsUnderParent1, 15);
@@ -257,10 +262,72 @@ public class ResourceUpgradeFailureHandlingTest extends ResourceUpgradeTestBase }
@Test - public void testFailureOnDependencies() { - //TODO implement - //check that stuff works if there is an upgrade failure on some of the resources - //in the plugin some "in the middle" of the plugin dep graph + public void testFailureOnDependencies() throws Exception { + setCurrentServerSideInventory(new FakeServerInventory()); + + executeTestWithPlugins(getAllDepsFor(TEST_V1_PLUGIN_NAME, PARENT_SIBLING_V1_PLUGIN_NAME), + new AbstractTestPayload(true, Collections.<ResType> emptyList()) { + public void test(Map<ResType, Set<Resource>> resourceUpgradeTestResources) { + //in here we set up the failures that are going to happen when + //the v2 plugins are run + + Resource parent = findResourceWithOrdinal(PARENT_DEP_TYPE, 0); + assertNotNull(parent, "Failed to find the parent to setup the failures for."); + + addChildrenToFail(parent, PARENT_TYPE, 0); + } + + public Expectations getExpectations(Mockery context) throws Exception { + return new Expectations() { + { + defineDefaultExpectations(this); + } + }; + } + }); + + executeTestWithPlugins(getAllDepsFor(TEST_V2_PLUGIN_NAME, PARENT_SIBLING_V2_PLUGIN_NAME), + new AbstractTestPayload(false, ALL_TYPES) { + public void test(Map<ResType, Set<Resource>> resources) { + checkPresenceOfResourceTypes(resources, getExpectedResourceTypes()); + + checkNumberOfResources(resources, ROOT_TYPE, 1); + checkNumberOfResources(resources, PARENT_DEP_TYPE, 1); + checkResourcesUpgraded(resources.get(PARENT_DEP_SIBLING_TYPE), 1); + + //check that the failed resources have the error attached to them + //we find the resource instance from the map provided to this method + //because that map contains the resources as found on the server-side + //(i.e. they include error objects). + Resource parent0 = getEqualFrom(resources.get(PARENT_TYPE), findResourceWithOrdinal(PARENT_TYPE, 0)); + Resource parent1 = getEqualFrom(resources.get(PARENT_TYPE), findResourceWithOrdinal(PARENT_TYPE, 1)); + + //v2 plugin discovers 3 resources but because parent0 failed to upgrade, + //the discovery shouldn't have occurred leaving us with the 2 already existing resources. + checkNumberOfResources(resources, PARENT_TYPE, 2); + checkResourceFailedUpgrade(parent0); + checkOthersUpgraded(resources.get(PARENT_TYPE), parent0); + + //parent1 upgraded ok, so discovering its children should have executed. + //this is v2, so we should find 15 of each. + checkResourcesUpgraded(filterResources(parent1.getChildResources(), TEST_TYPE), 15); + checkResourcesUpgraded(filterResources(parent1.getChildResources(), SIBLING_TYPE), 15); + + //these shouldn't have been upgraded. in v1 we had 10 resources of TEST_TYPE + //and 10 resources of SIBLING_TYPE and that's what we should be seeing + //now. + checkResourcesNotUpgraded(filterResources(parent0.getChildResources(), TEST_TYPE), 10); + checkResourcesNotUpgraded(filterResources(parent0.getChildResources(), SIBLING_TYPE), 10); + } + + public Expectations getExpectations(Mockery context) throws Exception { + return new Expectations() { + { + defineDefaultExpectations(this); + } + }; + } + }); }
@SuppressWarnings("unchecked") @@ -317,6 +384,22 @@ public class ResourceUpgradeFailureHandlingTest extends ResourceUpgradeTestBase } }
+ private static void checkResourcesNotUpgraded(Set<Resource> resources, int expectedSize) { + assertEquals(resources.size(), expectedSize, "The set of resources has unexpected size."); + for(Resource res : resources) { + assertFalse(res.getResourceKey().startsWith(UPGRADED_RESOURCE_KEY_PREFIX), "Resource " + res + + " seems to be upgraded even though it shouldn't."); + + ResourceContainer rc = PluginContainer.getInstance().getInventoryManager().getResourceContainer(res); + + assertEquals(rc.getResourceComponentState(), ResourceComponentState.STOPPED, + "A resource that has not been upgraded due to upgrade error in parent should be stopped."); + + //recurse, since the whole subtree under the failed resource should be not upgraded and stopped. + checkResourcesNotUpgraded(res.getChildResources(), res.getChildResources().size()); + } + } + private static void checkResourceFailedUpgrade(Resource resource) { assertFalse(resource.getResourceKey().startsWith(UPGRADED_RESOURCE_KEY_PREFIX), "Resource " + resource + " seems to be upgraded even though it shouldn't."); diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/v2/rhq-plugin.xml b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/v2/rhq-plugin.xml index 3d709b0..0f6671b 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/v2/rhq-plugin.xml +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/v2/rhq-plugin.xml @@ -21,7 +21,7 @@ </runs-inside>
<plugin-configuration> - <c:simple-property name="count" default="2" /> + <c:simple-property name="count" default="3" /> <c:simple-property name="key" default="UPGRADED-test-resource-parent-%n,parent-%p" /> </plugin-configuration>
commit 9533fc2898f18af7262f513049d7261516787f4f Author: Lukas Krejci lkrejci@redhat.com Date: Mon Oct 4 14:50:49 2010 +0200
Upgrade even the non-commited resources to prevent duplication of resources in the auto-discovery queue
diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java index c6f1169..99a0513 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java @@ -2650,11 +2650,6 @@ public class InventoryManager extends AgentService implements ContainerService, }
private void upgradeResource(Resource resource) { - //only process committed resources - if (resource.getInventoryStatus() != InventoryStatus.COMMITTED) { - return; - } - ResourceContainer container = getResourceContainer(resource); if (container != null) { if (container.getResourceComponentState() == ResourceComponentState.STARTED) { @@ -2668,8 +2663,17 @@ public class InventoryManager extends AgentService implements ContainerService, upgradeResource(child); } } else { - log.error("The resource container for resource [" + resource - + "] wasn't started during upgrade. This should not happen."); + String message = "The resource container for resource [" + resource + + "] wasn't started while upgrading."; + + if (resource.getChildResources().isEmpty()) { + log.info(message + + " If this is the first time the plugin container starts up and has completely empty inventory, you can ignore this message."); + } else { + log.error(message + + " This can potentially cause the discovery to find resources logically equivalent to already " + + "existing resources if the corresponding plugins support upgrade for that particular resource type."); + } } } else { log.error("Resource container not initialized for resource [" + resource diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/FakeServerInventory.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/FakeServerInventory.java index 39597ee..0dd3459 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/FakeServerInventory.java +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/FakeServerInventory.java @@ -80,8 +80,6 @@ public class FakeServerInventory { this.failing = failing; }
- //need to synchronize, because resource upgrade is async and can overlap with - //resource discovery. public synchronized CustomAction mergeInventoryReport(final InventoryStatus requiredInventoryStatus) { return new CustomAction("updateServerSideInventory") { public Object invoke(Invocation invocation) throws Throwable { diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeTest.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeTest.java index 04858c6..8b966c3 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeTest.java +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeTest.java @@ -71,7 +71,7 @@ public class ResourceUpgradeTest extends ResourceUpgradeTestBase { }
@Test - public void testIgnoreUncommittedResources() throws Exception { + public void testIncludeUncommittedResources() throws Exception { setCurrentServerSideInventory(new FakeServerInventory()); initialSyncAndDiscovery(InventoryStatus.NEW);
@@ -82,11 +82,12 @@ public class ResourceUpgradeTest extends ResourceUpgradeTestBase {
Resource discoveredResource = discoveredResources.get(SINGLETON_TYPE).iterator().next();
- assertEquals(discoveredResource.getResourceKey(), "resource-key-v1"); - assertEquals(discoveredResource.getName(), "resource-name-v1"); - assertEquals(discoveredResource.getDescription(), "resource-description-v1"); + assertEquals(discoveredResource.getResourceKey(), "resource-key-v2"); + assertEquals(discoveredResource.getName(), "resource-name-v2"); + assertEquals(discoveredResource.getDescription(), "resource-description-v2"); }
+ @SuppressWarnings("unchecked") public Expectations getExpectations(Mockery context) throws Exception { return new Expectations() { { @@ -95,6 +96,9 @@ public class ResourceUpgradeTest extends ResourceUpgradeTestBase { between(1, 4).of(getCurrentDiscoveryServerService()).mergeInventoryReport( with(any(InventoryReport.class))); will(getCurrentServerSideInventory().mergeInventoryReport(InventoryStatus.COMMITTED)); + + oneOf(getCurrentDiscoveryServerService()).upgradeResources(with(any(Set.class))); + will(getCurrentServerSideInventory().upgradeResources()); } }; }
commit 8ecbe7dec08b0e2e1f86ad8e1e56efd9f651cc7e Author: Lukas Krejci lkrejci@redhat.com Date: Mon Oct 4 14:32:55 2010 +0200
code formatting
diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java index de52cd8..c6f1169 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java @@ -242,16 +242,19 @@ public class InventoryManager extends AgentService implements ContainerService, // inside EmbJopr). if (configuration.isInsideAgent()) { // After an initial delay (5s by default), periodically run an availability check (every 1m by default). - availabilityThreadPoolExecutor.scheduleWithFixedDelay(availabilityExecutor, configuration - .getAvailabilityScanInitialDelay(), configuration.getAvailabilityScanPeriod(), TimeUnit.SECONDS); + availabilityThreadPoolExecutor.scheduleWithFixedDelay(availabilityExecutor, + configuration.getAvailabilityScanInitialDelay(), configuration.getAvailabilityScanPeriod(), + TimeUnit.SECONDS);
// After an initial delay (10s by default), periodically run a server discovery scan (every 15m by default). - inventoryThreadPoolExecutor.scheduleWithFixedDelay(serverScanExecutor, configuration - .getServerDiscoveryInitialDelay(), configuration.getServerDiscoveryPeriod(), TimeUnit.SECONDS); + inventoryThreadPoolExecutor.scheduleWithFixedDelay(serverScanExecutor, + configuration.getServerDiscoveryInitialDelay(), configuration.getServerDiscoveryPeriod(), + TimeUnit.SECONDS);
// After an initial delay (20s by default), periodically run a service discovery scan (every 1d by default). - inventoryThreadPoolExecutor.scheduleWithFixedDelay(serviceScanExecutor, configuration - .getServiceDiscoveryInitialDelay(), configuration.getServiceDiscoveryPeriod(), TimeUnit.SECONDS); + inventoryThreadPoolExecutor.scheduleWithFixedDelay(serviceScanExecutor, + configuration.getServiceDiscoveryInitialDelay(), configuration.getServiceDiscoveryPeriod(), + TimeUnit.SECONDS); } } finally { inventoryLock.writeLock().unlock(); @@ -288,22 +291,23 @@ public class InventoryManager extends AgentService implements ContainerService, * * @throws Exception if the discovery component threw an exception */ - public Set<DiscoveredResourceDetails> invokeDiscoveryComponent(ResourceContainer parentResourceContainer, ResourceDiscoveryComponent component, - ResourceDiscoveryContext context) throws Exception { + public Set<DiscoveredResourceDetails> invokeDiscoveryComponent(ResourceContainer parentResourceContainer, + ResourceDiscoveryComponent component, ResourceDiscoveryContext context) throws Exception {
Resource parentResource = parentResourceContainer == null ? null : parentResourceContainer.getResource(); - + if (resourceUpgradeDelegate.hasUpgradeFailedInChildren(parentResource, context.getResourceType())) { - log.debug("Discovery of [" + context.getResourceType() + "] has been disallowed under " + (parentResource == null ? " the platform " : parentResource) + - " because some of its siblings failed to upgrade."); + log.debug("Discovery of [" + context.getResourceType() + "] has been disallowed under " + + (parentResource == null ? " the platform " : parentResource) + + " because some of its siblings failed to upgrade."); return null; } - + long timeout = getDiscoveryComponentTimeout(context.getResourceType());
try { - ResourceDiscoveryComponent proxy = this.discoveryComponentProxyFactory.getDiscoveryComponentProxy(context - .getResourceType(), component, timeout); + ResourceDiscoveryComponent proxy = this.discoveryComponentProxyFactory.getDiscoveryComponentProxy( + context.getResourceType(), component, timeout); Set<DiscoveredResourceDetails> results = proxy.discoverResources(context); return results; } catch (TimeoutException te) { @@ -335,8 +339,8 @@ public class InventoryManager extends AgentService implements ContainerService, long timeout = getDiscoveryComponentTimeout(context.getResourceType());
try { - ManualAddFacet proxy = this.discoveryComponentProxyFactory.getDiscoveryComponentProxy(context - .getResourceType(), component, timeout, ManualAddFacet.class); + ManualAddFacet proxy = this.discoveryComponentProxyFactory.getDiscoveryComponentProxy( + context.getResourceType(), component, timeout, ManualAddFacet.class); DiscoveredResourceDetails result = proxy.discoverResource(pluginConfig, context); return result; } catch (TimeoutException te) { @@ -375,8 +379,8 @@ public class InventoryManager extends AgentService implements ContainerService, component, timeout, ClassLoaderFacet.class);
ResourceDiscoveryContext discoveryContext = new ResourceDiscoveryContext(resourceType, parentComponent, - parentResourceContext, SystemInfoFactory.createSystemInfo(), null, null, this.configuration - .getContainerName(), this.configuration.getPluginContainerDeployment()); + parentResourceContext, SystemInfoFactory.createSystemInfo(), null, null, + this.configuration.getContainerName(), this.configuration.getPluginContainerDeployment());
// Configurations are not immutable, so clone the plugin config, so the plugin will not be able to change the // actual PC-managed plugin config. @@ -384,8 +388,7 @@ public class InventoryManager extends AgentService implements ContainerService, // TODO (ips): Clone the ResourceType too for the same reason.
DiscoveredResourceDetails details = new DiscoveredResourceDetails(resourceType, resource.getResourceKey(), - resource.getName(), resource.getVersion(), resource.getDescription(), pluginConfigClone, - null); // TODO: I have a feeling we'll need process info, how to get it?? + resource.getName(), resource.getVersion(), resource.getDescription(), pluginConfigClone, null); // TODO: I have a feeling we'll need process info, how to get it??
List<URL> results = proxy.getAdditionalClasspathUrls(discoveryContext, details);
@@ -592,7 +595,8 @@ public class InventoryManager extends AgentService implements ContainerService,
@NotNull // TODO (ips): Perhaps refactor this so that it shares code with AvailablityExecutor.checkInventory(). - public Availability getCurrentAvailability(Resource resource) { + public + Availability getCurrentAvailability(Resource resource) { AvailabilityType availType = null; // i.e. UNKNOWN; ResourceContainer resourceContainer = getResourceContainer(resource); if (resourceContainer != null) { @@ -665,8 +669,8 @@ public class InventoryManager extends AgentService implements ContainerService, ResourceDiscoveryContext<ResourceComponent> discoveryContext = new ResourceDiscoveryContext<ResourceComponent>( resourceType, parentResourceComponent, parentResourceContainer.getResourceContext(), SystemInfoFactory.createSystemInfo(), new ArrayList<ProcessScanResult>(0), - new ArrayList<Configuration>(0), this.configuration.getContainerName(), this.configuration - .getPluginContainerDeployment()); + new ArrayList<Configuration>(0), this.configuration.getContainerName(), + this.configuration.getPluginContainerDeployment());
// Ask the plugin's discovery component to find the new resource, throwing exceptions if it cannot be // found at all. @@ -695,8 +699,8 @@ public class InventoryManager extends AgentService implements ContainerService,
// Ask the plugin's discovery component to find the new resource, throwing exceptions if it cannot be // found at all. - Set<DiscoveredResourceDetails> discoveredResources = invokeDiscoveryComponent(parentResourceContainer, discoveryComponent, - discoveryContext); + Set<DiscoveredResourceDetails> discoveredResources = invokeDiscoveryComponent(parentResourceContainer, + discoveryComponent, discoveryContext); if ((discoveredResources == null) || discoveredResources.isEmpty()) { log.info("Plugin Error: During manual add, discovery component method [" + discoveryComponent.getClass().getName() + ".discoverResources()] returned " @@ -850,8 +854,7 @@ public class InventoryManager extends AgentService implements ContainerService, if ((this.platform != null) && (this.platform.getInventoryStatus() == InventoryStatus.NEW) && newPlatformWasDeletedRecently) { // let's make sure we are registered; its probable that our platform was deleted and we need to re-register - log - .info("No committed resources to send in our availability report - the platform/agent was deleted, let's re-register again"); + log.info("No committed resources to send in our availability report - the platform/agent was deleted, let's re-register again"); registerWithServer(); newPlatformWasDeletedRecently = false; // we've tried to recover from our platform being deleted, let's not do it again } @@ -891,8 +894,8 @@ public class InventoryManager extends AgentService implements ContainerService, log.debug("Availability report content: " + report.toString(log.isTraceEnabled())); }
- boolean ok = configuration.getServerServices().getDiscoveryServerService().mergeAvailabilityReport( - report); + boolean ok = configuration.getServerServices().getDiscoveryServerService() + .mergeAvailabilityReport(report); if (!ok) { // I guess I could immediately call executeAvailabilityScanImmediately and pass its results to // mergeAvailabilityReport again right now, but what happens if we've queued up a bunch of @@ -955,8 +958,7 @@ public class InventoryManager extends AgentService implements ContainerService, if ((this.platform != null) && (this.platform.getInventoryStatus() == InventoryStatus.NEW) && newPlatformWasDeletedRecently) { // let's make sure we are registered; its probable that our platform was deleted and we need to re-register - log - .info("The inventory report was invalid probably because the platform/Agent was deleted; let's re-register..."); + log.info("The inventory report was invalid probably because the platform/Agent was deleted; let's re-register..."); registerWithServer(); newPlatformWasDeletedRecently = false; // we've tried to recover from our platform being deleted, let's not do it again } @@ -974,8 +976,8 @@ public class InventoryManager extends AgentService implements ContainerService, if (syncInfo != null) { synchInventory(syncInfo); } else { - purgeObsoleteResources(Collections.<String>emptySet()); - + purgeObsoleteResources(Collections.<String> emptySet()); + //can't live without a platform, but we just deleted it. Let's rediscover it. discoverPlatform(); } @@ -1113,7 +1115,7 @@ public class InventoryManager extends AgentService implements ContainerService, return; } boolean scan = removeResourceAndIndicateIfScanIsNeeded(resourceContainer.getResource()); - + //only actually schedule the scanning when we are finished with resource upgrade. The resource upgrade //happens before any scanning infrastructure is established. if (!resourceUpgradeDelegate.enabled() && scan) { @@ -1152,8 +1154,8 @@ public class InventoryManager extends AgentService implements ContainerService, parent.removeChildResource(resource); }
- PluginContainer.getInstance().getMeasurementManager().unscheduleCollection( - Collections.singleton(resource.getId())); + PluginContainer.getInstance().getMeasurementManager() + .unscheduleCollection(Collections.singleton(resource.getId()));
if (this.resourceContainers.remove(resource.getUuid()) == null) { if (log.isDebugEnabled()) { @@ -1258,8 +1260,8 @@ public class InventoryManager extends AgentService implements ContainerService, logMessage.append("description, "); }
- logMessage.replace(logMessage.length() - 1, logMessage.length(), "to become [").append( - existingResource.toString()).append("]"); + logMessage.replace(logMessage.length() - 1, logMessage.length(), "to become [") + .append(existingResource.toString()).append("]");
log.info(logMessage.toString()); } else { @@ -1429,7 +1431,7 @@ public class InventoryManager extends AgentService implements ContainerService, } return; } - + ResourceComponent component = container.getResourceComponent();
// if the component already exists and is started, and the resource's plugin config has not changed, there is @@ -1490,8 +1492,8 @@ public class InventoryManager extends AgentService implements ContainerService, log.warn("Cannot give activated resource its discovery component. Cause: " + e); }
- ConfigurationUtility.normalizeConfiguration(resource.getPluginConfiguration(), type - .getPluginConfigurationDefinition()); + ConfigurationUtility.normalizeConfiguration(resource.getPluginConfiguration(), + type.getPluginConfigurationDefinition());
ResourceComponent parentComponent = null; if (resource.getParentResource() != null) { @@ -1855,8 +1857,7 @@ public class InventoryManager extends AgentService implements ContainerService, } } else { // This is very strange - there are no platform types - we should never be missing the built-in platform plugin. - log - .error("Missing platform plugin(s) - falling back to dummy platform impl; this should only occur in tests!"); + log.error("Missing platform plugin(s) - falling back to dummy platform impl; this should only occur in tests!"); // TODO: Set sysprop (e.g. rhq.test.mode=true) in integration tests, // and throw a runtime exception here if that sysprop is not set. return getTestPlatform(); @@ -2049,7 +2050,7 @@ public class InventoryManager extends AgentService implements ContainerService, public boolean hasUpgradeMergeFailed() { return resourceUpgradeDelegate.hasUpgradeMergeFailed(); } - + /** * Always use this before accessing the event listeners because this ensures * thread safety. @@ -2176,9 +2177,10 @@ public class InventoryManager extends AgentService implements ContainerService, try { ResourceDiscoveryContext context = new ResourceDiscoveryContext(resourceType, parentComponent, parentResourceContext, SystemInfoFactory.createSystemInfo(), processScanResults, - Collections.EMPTY_LIST, this.configuration.getContainerName(), this.configuration - .getPluginContainerDeployment()); - Set<DiscoveredResourceDetails> discoveredResources = invokeDiscoveryComponent(parentContainer, discoveryComponent, context); + Collections.EMPTY_LIST, this.configuration.getContainerName(), + this.configuration.getPluginContainerDeployment()); + Set<DiscoveredResourceDetails> discoveredResources = invokeDiscoveryComponent(parentContainer, + discoveryComponent, context); newResources = new HashSet<Resource>(); if ((discoveredResources != null) && (!discoveredResources.isEmpty())) { IdentityHashMap<Configuration, DiscoveredResourceDetails> pluginConfigObjects = new IdentityHashMap<Configuration, DiscoveredResourceDetails>(); @@ -2403,8 +2405,8 @@ public class InventoryManager extends AgentService implements ContainerService, if (log.isDebugEnabled()) { log.debug("Merging [" + modifiedResourceIds.size() + "] modified Resources into local inventory..."); } - Set<Resource> modifiedResources = configuration.getServerServices().getDiscoveryServerService().getResources( - modifiedResourceIds, false); + Set<Resource> modifiedResources = configuration.getServerServices().getDiscoveryServerService() + .getResources(modifiedResourceIds, false); syncSchedules(modifiedResources); // RHQ-792, mtime is the indicator that schedules should be sync'ed too for (Resource modifiedResource : modifiedResources) { mergeResource(modifiedResource); @@ -2620,12 +2622,12 @@ public class InventoryManager extends AgentService implements ContainerService,
private void upgradeResources() { try { - + if (!configuration.isInsideAgent()) { log.debug("Skipping resource upgrade in embedded mode."); return; } - + log.debug("Executing resource upgrade.");
boolean syncResult = handleReport(new InventoryReport(getAgent()), true); @@ -2635,7 +2637,7 @@ public class InventoryManager extends AgentService implements ContainerService, }
upgradeResource(getPlatform()); - + log.debug("Sending the upgrade requests to the server."); resourceUpgradeDelegate.sendRequests();
commit 9050f4bfab5114ff31e19cb9c04b2d087dba2bff Author: Lukas Krejci lkrejci@redhat.com Date: Mon Oct 4 14:04:52 2010 +0200
Adding the test for handling the failure to upgrade some of the resources on the leaf of the plugin dependency graph.
diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ForwardingProxy.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ForwardingProxy.java new file mode 100644 index 0000000..7c7d277 --- /dev/null +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ForwardingProxy.java @@ -0,0 +1,72 @@ +/* + * RHQ Management Platform + * Copyright (C) 2005-2010 Red Hat, Inc. + * All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +package org.rhq.core.pc.upgrade; + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; + +/** + * This class provides a simple proxy that just forwards the calls to the + * actual object. + * <p> + * This seemingly pointless behavior is useful when dealing with the classes + * loaded in different classloaders. + * <p> + * The arguments passed to the method invocations of the proxy must unfortunately be "understandable" + * both in the "current" classloader and in the context of the classloader that loaded the target object. + * I don't know about a generic solution to this problem. + * A partial solution would be to serialize the arguments and deserialize them in the context + * of the target classloader but that doesn't cut it for non-serializable classes obviously. + * + * @author Lukas Krejci + */ +public class ForwardingProxy { + + private static class ForwardingHandler implements InvocationHandler { + private Object target; + + public ForwardingHandler(Object target) { + this.target = target; + } + + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + Method targetMethod = findMethodOnTarget(method); + + return targetMethod.invoke(target, args); + } + + private Method findMethodOnTarget(Method method) throws SecurityException, NoSuchMethodException { + Class<?>[] origParamTypes = method.getParameterTypes(); + + return target.getClass().getMethod(method.getName(), origParamTypes); + } + } + + public static <T> T forward(Object target, Class<T> targetInterface, Class<?>... additionalInterfaces) { + Class<?>[] interfaces = new Class<?>[additionalInterfaces.length + 1]; + interfaces[0] = targetInterface; + System.arraycopy(additionalInterfaces, 0, interfaces, 1, additionalInterfaces.length); + + Object proxy = Proxy.newProxyInstance(targetInterface.getClassLoader(), interfaces, new ForwardingHandler(target)); + return targetInterface.cast(proxy); + } +} diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeFailureHandlingTest.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeFailureHandlingTest.java index 5949b76..7eafafc 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeFailureHandlingTest.java +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeFailureHandlingTest.java @@ -20,14 +20,17 @@ package org.rhq.core.pc.upgrade;
import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail;
import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -36,10 +39,20 @@ import org.jmock.Expectations; import org.jmock.Mockery; import org.testng.annotations.Test;
+import org.rhq.core.clientapi.agent.PluginContainerException; +import org.rhq.core.clientapi.agent.discovery.InvalidPluginConfigurationClientException; import org.rhq.core.clientapi.server.discovery.InvalidInventoryReportException; import org.rhq.core.clientapi.server.discovery.InventoryReport; +import org.rhq.core.domain.configuration.Configuration; import org.rhq.core.domain.resource.InventoryStatus; import org.rhq.core.domain.resource.Resource; +import org.rhq.core.domain.resource.ResourceErrorType; +import org.rhq.core.domain.resource.ResourceType; +import org.rhq.core.pc.PluginContainer; +import org.rhq.core.pc.inventory.InventoryManager; +import org.rhq.core.pc.inventory.ResourceContainer; +import org.rhq.core.pc.inventory.ResourceContainer.ResourceComponentState; +import org.rhq.core.pc.upgrade.plugins.multi.base.BaseResourceComponentInterface; import org.rhq.core.pluginapi.upgrade.ResourceUpgradeFacet;
/** @@ -76,7 +89,7 @@ public class ResourceUpgradeFailureHandlingTest extends ResourceUpgradeTestBase private static final String TEST_V2_PLUGIN_NAME = "/resource-upgrade-test-plugin-multi-test-2.0.0.jar";
private static final String UPGRADED_RESOURCE_KEY_PREFIX = "UPGRADED"; - + private static final HashMap<String, List<String>> DEPS;
static { @@ -100,9 +113,12 @@ public class ResourceUpgradeFailureHandlingTest extends ResourceUpgradeTestBase private static final ResType SIBLING_TYPE = new ResType("TestResourceSibling", "test"); private static final ResType PARENT_TYPE = new ResType("TestResourceParent", "test"); private static final ResType PARENT_DEP_TYPE = new ResType("ParentDependency", "parentdep"); - private static final ResType PARENT_SIBLING_TYPE = new ResType("ParentDepSibling", "parentsibling"); + private static final ResType PARENT_DEP_SIBLING_TYPE = new ResType("ParentDepSibling", "parentsibling"); private static final ResType ROOT_TYPE = new ResType("Root", "root");
+ private static List<ResType> ALL_TYPES = Arrays.asList(TEST_TYPE, SIBLING_TYPE, PARENT_TYPE, PARENT_DEP_TYPE, + PARENT_DEP_SIBLING_TYPE, ROOT_TYPE); + protected Collection<String> getRequiredPlugins() { return Arrays.asList(BASE_PLUGIN_NAME, PARENT_DEP_V1_PLUGIN_NAME, PARENT_DEP_V2_PLUGIN_NAME, PARENT_SIBLING_V1_PLUGIN_NAME, PARENT_SIBLING_V2_PLUGIN_NAME, ROOT_PLUGIN_NAME, SIBLING_V1_PLUGIN_NAME, @@ -129,26 +145,20 @@ public class ResourceUpgradeFailureHandlingTest extends ResourceUpgradeTestBase });
//now let's run with v2 plugins and check the layout of the inventory - executeTestWithPlugins( - getAllDepsFor(TEST_V2_PLUGIN_NAME, PARENT_SIBLING_V2_PLUGIN_NAME), - new AbstractTestPayload(false, Arrays.asList(TEST_TYPE, SIBLING_TYPE, PARENT_TYPE, PARENT_DEP_TYPE, - PARENT_SIBLING_TYPE, ROOT_TYPE)) { + executeTestWithPlugins(getAllDepsFor(TEST_V2_PLUGIN_NAME, PARENT_SIBLING_V2_PLUGIN_NAME), + new AbstractTestPayload(false, ALL_TYPES) {
public void test(Map<ResType, Set<Resource>> resources) { checkPresenceOfResourceTypes(resources, getExpectedResourceTypes());
checkNumberOfResources(resources, ROOT_TYPE, 1); checkNumberOfResources(resources, PARENT_DEP_TYPE, 1); - checkNumberOfResources(resources, PARENT_SIBLING_TYPE, 1); - checkNumberOfResources(resources, PARENT_TYPE, 2); - checkNumberOfResources(resources, SIBLING_TYPE, 30); - checkNumberOfResources(resources, TEST_TYPE, 30); - + //check that the resources are upgraded - checkResourcesUpgraded(resources.get(PARENT_SIBLING_TYPE)); - checkResourcesUpgraded(resources.get(PARENT_TYPE)); - checkResourcesUpgraded(resources.get(SIBLING_TYPE)); - checkResourcesUpgraded(resources.get(TEST_TYPE)); + checkResourcesUpgraded(resources.get(PARENT_DEP_SIBLING_TYPE), 1); + checkResourcesUpgraded(resources.get(PARENT_TYPE), 2); + checkResourcesUpgraded(resources.get(SIBLING_TYPE), 30); + checkResourcesUpgraded(resources.get(TEST_TYPE), 30); }
public Expectations getExpectations(Mockery context) throws Exception { @@ -162,10 +172,88 @@ public class ResourceUpgradeFailureHandlingTest extends ResourceUpgradeTestBase }
@Test - public void testFailureOnLeaf() { - //TODO implement - //check that the system behaves correctly if there is an upgrade failure - //at the leaf node of the plugin dep graph + public void testFailureOnLeaf() throws Exception { + setCurrentServerSideInventory(new FakeServerInventory()); + + executeTestWithPlugins(getAllDepsFor(TEST_V1_PLUGIN_NAME, PARENT_SIBLING_V1_PLUGIN_NAME), + new AbstractTestPayload(true, Collections.<ResType> emptyList()) { + public void test(Map<ResType, Set<Resource>> resourceUpgradeTestResources) { + //in here we set up the failures that are going to happen when + //the v2 plugins are run + + Resource parent0 = findResourceWithOrdinal(PARENT_TYPE, 0); + assertNotNull(parent0, "Failed to find the parent to setup the failures for."); + Resource parent1 = findResourceWithOrdinal(PARENT_TYPE, 1); + assertNotNull(parent1, "Failed to find the parent to setup the failures for."); + + addChildrenToFail(parent0, TEST_TYPE, 1, 2); + addChildrenToFail(parent1, SIBLING_TYPE, 1); + } + + public Expectations getExpectations(Mockery context) throws Exception { + return new Expectations() { + { + defineDefaultExpectations(this); + } + }; + } + }); + + executeTestWithPlugins(getAllDepsFor(TEST_V2_PLUGIN_NAME, PARENT_SIBLING_V2_PLUGIN_NAME), + new AbstractTestPayload(false, ALL_TYPES) { + public void test(Map<ResType, Set<Resource>> resources) { + checkPresenceOfResourceTypes(resources, getExpectedResourceTypes()); + + checkNumberOfResources(resources, ROOT_TYPE, 1); + checkNumberOfResources(resources, PARENT_DEP_TYPE, 1); + checkNumberOfResources(resources, PARENT_TYPE, 2); + + checkResourcesUpgraded(resources.get(PARENT_TYPE), 2); + checkResourcesUpgraded(resources.get(PARENT_DEP_SIBLING_TYPE), 1); + + Resource parent0 = findResourceWithOrdinal(PARENT_TYPE, 0); + Resource parent1 = findResourceWithOrdinal(PARENT_TYPE, 1); + + Set<Resource> siblingsUnderParent0 = filterResources(parent0.getChildResources(), SIBLING_TYPE); + Set<Resource> testsUnderParent0 = filterResources(parent0.getChildResources(), TEST_TYPE); + Set<Resource> siblingsUnderParent1 = filterResources(parent1.getChildResources(), SIBLING_TYPE); + Set<Resource> testsUnderParent1 = filterResources(parent1.getChildResources(), TEST_TYPE); + + checkResourcesUpgraded(siblingsUnderParent0, 15); + checkResourcesUpgraded(testsUnderParent1, 15); + + //there should be no newly discovered sibling resources of the failed ones + assertEquals(testsUnderParent0.size(), 10); + assertEquals(siblingsUnderParent1.size(), 10); + + //check that the failed resources have the error attached to them + //we find the resource instance from the map provided to this method + //because that map contains the resources as found on the server-side + //(i.e. they include error objects). + Resource failedTest1 = getEqualFrom(resources.get(TEST_TYPE), + findResourceWithOrdinal(testsUnderParent0, 1)); + Resource failedTest2 = getEqualFrom(resources.get(TEST_TYPE), + findResourceWithOrdinal(testsUnderParent0, 2)); + + checkResourceFailedUpgrade(failedTest1); + checkResourceFailedUpgrade(failedTest2); + checkOthersUpgraded(testsUnderParent0, failedTest1, failedTest2); + + Resource failedSibling = getEqualFrom(resources.get(SIBLING_TYPE), + findResourceWithOrdinal(siblingsUnderParent1, 1)); + + checkResourceFailedUpgrade(failedSibling); + checkOthersUpgraded(siblingsUnderParent1, failedSibling); + } + + public Expectations getExpectations(Mockery context) throws Exception { + return new Expectations() { + { + defineDefaultExpectations(this); + } + }; + } + }); }
@Test @@ -180,13 +268,14 @@ public class ResourceUpgradeFailureHandlingTest extends ResourceUpgradeTestBase protected void defineDefaultExpectations(Expectations expectations) { super.defineDefaultExpectations(expectations); try { - expectations.allowing(getCurrentDiscoveryServerService()) - .mergeInventoryReport(expectations.with(Expectations.any(InventoryReport.class))); + expectations.allowing(getCurrentDiscoveryServerService()).mergeInventoryReport( + expectations.with(Expectations.any(InventoryReport.class))); expectations.will(getCurrentServerSideInventory().mergeInventoryReport(InventoryStatus.COMMITTED));
- expectations.allowing(getCurrentDiscoveryServerService()).upgradeResources(expectations.with(Expectations.any(Set.class))); + expectations.allowing(getCurrentDiscoveryServerService()).upgradeResources( + expectations.with(Expectations.any(Set.class))); expectations.will(getCurrentServerSideInventory().upgradeResources()); - + expectations.allowing(getCurrentDiscoveryServerService()).postProcessNewlyCommittedResources( expectations.with(Expectations.any(Set.class)));
@@ -214,10 +303,112 @@ public class ResourceUpgradeFailureHandlingTest extends ResourceUpgradeTestBase private static void checkNumberOfResources(Map<ResType, Set<Resource>> resources, ResType type, int count) { assertEquals(resources.get(type).size(), count, "Unexpected number of " + type + " discovered."); } - - private static void checkResourcesUpgraded(Set<Resource> resources) { - for(Resource res : resources) { - assertTrue(res.getResourceKey().startsWith(UPGRADED_RESOURCE_KEY_PREFIX), "Resource " + res + " doesn't seem to be upgraded even though it should."); + + private static void checkResourcesUpgraded(Set<Resource> resources, int expectedSize) { + assertEquals(resources.size(), expectedSize, "The set of resources has unexpected size."); + for (Resource res : resources) { + assertTrue(res.getResourceKey().startsWith(UPGRADED_RESOURCE_KEY_PREFIX), "Resource " + res + + " doesn't seem to be upgraded even though it should."); + + ResourceContainer rc = PluginContainer.getInstance().getInventoryManager().getResourceContainer(res); + + assertEquals(rc.getResourceComponentState(), ResourceComponentState.STARTED, + "A resource that successfully upgraded should be started."); + } + } + + private static void checkResourceFailedUpgrade(Resource resource) { + assertFalse(resource.getResourceKey().startsWith(UPGRADED_RESOURCE_KEY_PREFIX), "Resource " + resource + + " seems to be upgraded even though it shouldn't."); + assertTrue(resource.getResourceErrors(ResourceErrorType.UPGRADE).size() == 1, + "The failed resource should have an error associated with it."); + + ResourceContainer rc = PluginContainer.getInstance().getInventoryManager().getResourceContainer(resource); + + assertEquals(rc.getResourceComponentState(), ResourceComponentState.STOPPED, + "A resource that failed to upgrade should be stopped."); + } + + private static void checkOthersUpgraded(Set<Resource> resources, Resource... failedResource) { + Set<Resource> others = new HashSet<Resource>(resources); + others.removeAll(Arrays.asList(failedResource)); + checkResourcesUpgraded(others, others.size()); + } + + private void addChildrenToFail(Resource parent, ResType childResType, int... childrenOrdinals) { + InventoryManager inventoryManager = PluginContainer.getInstance().getInventoryManager(); + ResourceContainer parentContainer = inventoryManager.getResourceContainer(parent); + BaseResourceComponentInterface parentComponent = (BaseResourceComponentInterface) parentContainer + .getResourceComponent(); + + Map<String, Set<Integer>> childrenToFail = new HashMap<String, Set<Integer>>(); + Set<Integer> ordinals = new HashSet<Integer>(); + for (int i = 0; i < childrenOrdinals.length; ++i) { + ordinals.add(childrenOrdinals[i]); + } + + childrenToFail.put(childResType.getResourceTypeName(), ordinals); + + Configuration newPluginConfig = parentComponent.createPluginConfigurationWithMarkedFailures(childrenToFail); + + try { + int resourceId = parent.getId(); + inventoryManager.updatePluginConfiguration(resourceId, newPluginConfig); + } catch (InvalidPluginConfigurationClientException e) { + fail("Updating plugin configuration failed.", e); + } catch (PluginContainerException e) { + fail("Updating plugin configuration failed.", e); + } + } + + private Resource findResourceWithOrdinal(ResType resType, int ordinal) { + ResourceType resourceType = PluginContainer.getInstance().getPluginManager().getMetadataManager() + .getType(resType.getResourceTypeName(), resType.getResourceTypePluginName()); + + InventoryManager inventoryManager = PluginContainer.getInstance().getInventoryManager(); + Set<Resource> resources = inventoryManager.getResourcesWithType(resourceType); + + return findResourceWithOrdinal(resources, ordinal); + } + + private Resource findResourceWithOrdinal(Set<Resource> resources, int ordinal) { + for (Resource r : resources) { + Configuration pluginConfig = r.getPluginConfiguration(); + String ordinalString = pluginConfig.getSimpleValue("ordinal", null); + + if (ordinalString != null && Integer.parseInt(ordinalString) == ordinal) { + return r; + } + } + + return null; + } + + private Set<Resource> filterResources(Set<Resource> resources, ResType resType) { + Set<Resource> ret = new HashSet<Resource>(resources); + + Iterator<Resource> it = ret.iterator(); + + while (it.hasNext()) { + ResourceType resourceType = it.next().getResourceType(); + + if (!resourceType.getName().equals(resType.getResourceTypeName()) + || !resourceType.getPlugin().equals(resType.getResourceTypePluginName())) { + + it.remove(); + } } + + return ret; + } + + private static <T> T getEqualFrom(Collection<? extends T> collection, T object) { + for (T other : collection) { + if (object.equals(other)) { + return other; + } + } + + return null; } } diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeTest.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeTest.java index d47d00c..04858c6 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeTest.java +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/ResourceUpgradeTest.java @@ -37,12 +37,13 @@ import java.util.Set;
import org.jmock.Expectations; import org.jmock.Mockery; +import org.testng.annotations.Test; + import org.rhq.core.clientapi.server.discovery.InventoryReport; import org.rhq.core.domain.resource.InventoryStatus; import org.rhq.core.domain.resource.Resource; import org.rhq.core.pc.PluginContainer; import org.rhq.core.pc.inventory.ResourceContainer; -import org.testng.annotations.Test;
/** * Test cases for resource upgrade. @@ -55,234 +56,255 @@ public class ResourceUpgradeTest extends ResourceUpgradeTestBase { private static final String PLUGIN_V1_FILENAME = "/resource-upgrade-test-plugin-1.0.0.jar"; private static final String PLUGIN_V2_FILENAME = "/resource-upgrade-test-plugin-2.0.0.jar"; private static final String FAILING_PLUGIN_FILE_NAME = "/resource-upgrade-test-plugin-3.0.0.jar"; - + private static final String SINGLETON_RESOURCE_TYPE_NAME = "Resource"; private static final String SINGLETON_RESOURCE_TYPE_PLUGIN_NAME = "ResourceUpgradeTest"; - private static final ResType SINGLETON_TYPE = new ResType(SINGLETON_RESOURCE_TYPE_NAME, SINGLETON_RESOURCE_TYPE_PLUGIN_NAME); - + private static final ResType SINGLETON_TYPE = new ResType(SINGLETON_RESOURCE_TYPE_NAME, + SINGLETON_RESOURCE_TYPE_PLUGIN_NAME); + private static List<ResType> getMyExpectedTypes() { return Arrays.asList(SINGLETON_TYPE); } - + protected Collection<String> getRequiredPlugins() { return Arrays.asList(PLUGIN_V1_FILENAME, PLUGIN_V2_FILENAME, FAILING_PLUGIN_FILE_NAME); } - + @Test public void testIgnoreUncommittedResources() throws Exception { setCurrentServerSideInventory(new FakeServerInventory()); initialSyncAndDiscovery(InventoryStatus.NEW); - - TestPayload testNoChange = new AbstractTestPayload(false, getMyExpectedTypes()) { + + TestPayload testNoChange = new AbstractTestPayload(false, getMyExpectedTypes()) { public void test(Map<ResType, Set<Resource>> discoveredResources) { - assertEquals(discoveredResources.size(), 1, "Expected single test resource but multiple found."); - + assertEquals(discoveredResources.get(SINGLETON_TYPE).size(), 1, + "Expected single test resource but multiple found."); + Resource discoveredResource = discoveredResources.get(SINGLETON_TYPE).iterator().next();
assertEquals(discoveredResource.getResourceKey(), "resource-key-v1"); assertEquals(discoveredResource.getName(), "resource-name-v1"); assertEquals(discoveredResource.getDescription(), "resource-description-v1"); } - + public Expectations getExpectations(Mockery context) throws Exception { return new Expectations() { { defineDefaultExpectations(this); - - between(1, 4).of(getCurrentDiscoveryServerService()).mergeInventoryReport(with(any(InventoryReport.class))); + + between(1, 4).of(getCurrentDiscoveryServerService()).mergeInventoryReport( + with(any(InventoryReport.class))); will(getCurrentServerSideInventory().mergeInventoryReport(InventoryStatus.COMMITTED)); } }; } }; - + executeTestWithPlugins(Collections.singleton(PLUGIN_V2_FILENAME), testNoChange); } - + @Test public void testUpgradeData() throws Exception { setCurrentServerSideInventory(new FakeServerInventory()); upgradeTest(false); } - + @Test public void testInventoryReinitializationFromServerDuringUpgrade() throws Exception { setCurrentServerSideInventory(new FakeServerInventory()); upgradeTest(true); } - + @Test public void testSkipUpgradeWhenServerUnavailable() throws Exception { setCurrentServerSideInventory(new FakeServerInventory()); initialSyncAndDiscovery(InventoryStatus.COMMITTED); - + getCurrentServerSideInventory().setFailing(true);
- TestPayload test = new AbstractTestPayload(false, getMyExpectedTypes()) { + TestPayload test = new AbstractTestPayload(false, getMyExpectedTypes()) { public void test(Map<ResType, Set<Resource>> discoveredResources) { - assertEquals(discoveredResources.size(), 1, "Expected single test resource but multiple found."); - + assertEquals(discoveredResources.get(SINGLETON_TYPE).size(), 1, + "Expected single test resource but multiple found."); + Resource discoveredResource = discoveredResources.get(SINGLETON_TYPE).iterator().next(); - + assertEquals(discoveredResource.getResourceKey(), "resource-key-v1"); assertEquals(discoveredResource.getName(), "resource-name-v1"); assertEquals(discoveredResource.getDescription(), "resource-description-v1"); } - + @SuppressWarnings("unchecked") public Expectations getExpectations(Mockery context) throws Exception { return new Expectations() { { defineDefaultExpectations(this); - - between(1, 4).of(getCurrentDiscoveryServerService()).mergeInventoryReport(with(any(InventoryReport.class))); + + between(1, 4).of(getCurrentDiscoveryServerService()).mergeInventoryReport( + with(any(InventoryReport.class))); will(getCurrentServerSideInventory().mergeInventoryReport(InventoryStatus.COMMITTED)); - + never(getCurrentDiscoveryServerService()).upgradeResources(with(any(Set.class))); } }; } }; - + executeTestWithPlugins(Collections.singleton(PLUGIN_V2_FILENAME), test); } - + @Test public void testUpgradeWithPlatformDeletedOnServer() throws Exception { setCurrentServerSideInventory(new FakeServerInventory()); initialSyncAndDiscovery(InventoryStatus.COMMITTED); - - TestPayload test = new AbstractTestPayload(false, getMyExpectedTypes()) { + + TestPayload test = new AbstractTestPayload(false, getMyExpectedTypes()) { public void test(Map<ResType, Set<Resource>> discoveredResources) { - assertEquals(discoveredResources.size(), 1, "Expected single test resource but multiple found."); - + assertEquals(discoveredResources.get(SINGLETON_TYPE).size(), 1, + "Expected single test resource but multiple found."); + Resource discoveredResource = discoveredResources.get(SINGLETON_TYPE).iterator().next(); - + assertEquals(discoveredResource.getResourceKey(), "resource-key-v2"); assertEquals(discoveredResource.getName(), "resource-name-v2"); assertEquals(discoveredResource.getDescription(), "resource-description-v2");
- ResourceContainer container = PluginContainer.getInstance().getInventoryManager().getResourceContainer(discoveredResource); + ResourceContainer container = PluginContainer.getInstance().getInventoryManager() + .getResourceContainer(discoveredResource); File dataDir = container.getResourceContext().getDataDirectory(); - + File marker = new File(dataDir, "upgrade-succeeded"); - - assertFalse(marker.exists(), "The upgrade seems to have occured even though there shouldn't have been a resource to upgrade."); + + assertFalse(marker.exists(), + "The upgrade seems to have occured even though there shouldn't have been a resource to upgrade."); } - + @SuppressWarnings("unchecked") public Expectations getExpectations(Mockery context) throws Exception { return new Expectations() { { defineDefaultExpectations(this); - + //the first merge will be triggered from within the upgrade process and we are //going to report null sync. - oneOf(getCurrentDiscoveryServerService()).mergeInventoryReport(with(any(InventoryReport.class))); + oneOf(getCurrentDiscoveryServerService()) + .mergeInventoryReport(with(any(InventoryReport.class))); will(getCurrentServerSideInventory().clearPlatform()); - + //the rest of the inventory merges are executed by discoveries, so let's import the //discovered stuff into the server-side inventory. - between(1, 3).of(getCurrentDiscoveryServerService()).mergeInventoryReport(with(any(InventoryReport.class))); + between(1, 3).of(getCurrentDiscoveryServerService()).mergeInventoryReport( + with(any(InventoryReport.class))); will(getCurrentServerSideInventory().mergeInventoryReport(InventoryStatus.COMMITTED)); - + never(getCurrentDiscoveryServerService()).upgradeResources(with(any(Set.class))); } }; } }; - + executeTestWithPlugins(Collections.singleton(PLUGIN_V2_FILENAME), test); } - + @Test public void testUpgradeFailureHandling() throws Exception { setCurrentServerSideInventory(new FakeServerInventory()); initialSyncAndDiscovery(InventoryStatus.COMMITTED); - - TestPayload test = new AbstractTestPayload(false, getMyExpectedTypes()) { + + TestPayload test = new AbstractTestPayload(false, getMyExpectedTypes()) { public void test(Map<ResType, Set<Resource>> resourceUpgradeTestResources) { - assertEquals(resourceUpgradeTestResources.size(), 1, "Expected single test resource but multiple found."); - + assertEquals(resourceUpgradeTestResources.get(SINGLETON_TYPE).size(), 1, + "Expected single test resource but multiple found."); + Resource discoveredResource = resourceUpgradeTestResources.get(SINGLETON_TYPE).iterator().next(); - - assertTrue(discoveredResource.getResourceErrors().size() > 0, "There should be upgrade errors persisted on the server side."); - + + assertTrue(discoveredResource.getResourceErrors().size() > 0, + "There should be upgrade errors persisted on the server side."); + //the discovery of the failed resource mustn't have run - ResourceContainer container = PluginContainer.getInstance().getInventoryManager().getResourceContainer(discoveredResource); + ResourceContainer container = PluginContainer.getInstance().getInventoryManager() + .getResourceContainer(discoveredResource); File dataDir = container.getResourceContext().getDataDirectory(); - + File marker = new File(dataDir, "failing-discovery-ran"); - - assertFalse(marker.exists(), "The discovery of the resource type with a failed upgraded resource must not be executed but it was."); + + assertFalse(marker.exists(), + "The discovery of the resource type with a failed upgraded resource must not be executed but it was."); } - + @SuppressWarnings("unchecked") public Expectations getExpectations(Mockery context) throws Exception { return new Expectations() { { defineDefaultExpectations(this); - - between(1, 4).of(getCurrentDiscoveryServerService()).mergeInventoryReport(with(any(InventoryReport.class))); + + between(1, 4).of(getCurrentDiscoveryServerService()).mergeInventoryReport( + with(any(InventoryReport.class))); will(getCurrentServerSideInventory().mergeInventoryReport(InventoryStatus.COMMITTED)); - + oneOf(getCurrentDiscoveryServerService()).upgradeResources(with(any(Set.class))); will(getCurrentServerSideInventory().upgradeResources()); } }; } }; - + executeTestWithPlugins(Collections.singleton(FAILING_PLUGIN_FILE_NAME), test); } - + private void initialSyncAndDiscovery(final InventoryStatus requiredInventoryStatus) throws Exception { cleanDataDir(); - executeTestWithPlugins(Collections.singleton(PLUGIN_V1_FILENAME), new AbstractTestPayload(true, getMyExpectedTypes()) { + executeTestWithPlugins(Collections.singleton(PLUGIN_V1_FILENAME), new AbstractTestPayload(true, + getMyExpectedTypes()) { public void test(Map<ResType, Set<Resource>> discoveredResources) { - assertEquals(discoveredResources.size(), 1, "Expected single test resource but multiple found."); - + assertEquals(discoveredResources.get(SINGLETON_TYPE).size(), 1, + "Expected single test resource but multiple found."); + Resource discoveredResource = discoveredResources.get(SINGLETON_TYPE).iterator().next(); - + assertEquals(discoveredResource.getResourceKey(), "resource-key-v1"); assertEquals(discoveredResource.getName(), "resource-name-v1"); assertEquals(discoveredResource.getDescription(), "resource-description-v1"); } - + public Expectations getExpectations(Mockery context) throws Exception { return new Expectations() { { defineDefaultExpectations(this); - - between(1, 4).of(getCurrentDiscoveryServerService()).mergeInventoryReport(with(any(InventoryReport.class))); + + between(1, 4).of(getCurrentDiscoveryServerService()).mergeInventoryReport( + with(any(InventoryReport.class))); will(getCurrentServerSideInventory().mergeInventoryReport(requiredInventoryStatus)); } }; } - }); - + }); + }
- private void upgradeTest(boolean clearInventoryDat) throws Exception { initialSyncAndDiscovery(InventoryStatus.COMMITTED); - - executeTestWithPlugins(Collections.singleton(PLUGIN_V2_FILENAME), new AbstractTestPayload(clearInventoryDat, getMyExpectedTypes()) { + + executeTestWithPlugins(Collections.singleton(PLUGIN_V2_FILENAME), new AbstractTestPayload(clearInventoryDat, + getMyExpectedTypes()) { public void test(Map<ResType, Set<Resource>> discoveredResources) { - assertEquals(discoveredResources.size(), 1, "Expected single test resource but multiple found."); - + assertEquals(discoveredResources.get(SINGLETON_TYPE).size(), 1, + "Expected single test resource but multiple found."); + Resource discoveredResource = discoveredResources.get(SINGLETON_TYPE).iterator().next(); - + assertEquals(discoveredResource.getResourceKey(), "resource-key-v2"); assertEquals(discoveredResource.getName(), "resource-name-v2"); assertEquals(discoveredResource.getDescription(), "resource-description-v2"); - - ResourceContainer container = PluginContainer.getInstance().getInventoryManager().getResourceContainer(discoveredResource); + + ResourceContainer container = PluginContainer.getInstance().getInventoryManager() + .getResourceContainer(discoveredResource); File dataDir = container.getResourceContext().getDataDirectory(); - + File marker = new File(dataDir, "upgrade-succeeded"); - - assertTrue(marker.exists(), "The upgrade success marker file wasn't found. This means the upgrade didn't actually run."); + + assertTrue(marker.exists(), + "The upgrade success marker file wasn't found. This means the upgrade didn't actually run."); }
@SuppressWarnings("unchecked") @@ -290,10 +312,11 @@ public class ResourceUpgradeTest extends ResourceUpgradeTestBase { return new Expectations() { { defineDefaultExpectations(this); - - between(1, 4).of(getCurrentDiscoveryServerService()).mergeInventoryReport(with(any(InventoryReport.class))); + + between(1, 4).of(getCurrentDiscoveryServerService()).mergeInventoryReport( + with(any(InventoryReport.class))); will(getCurrentServerSideInventory().mergeInventoryReport(InventoryStatus.COMMITTED)); - + oneOf(getCurrentDiscoveryServerService()).upgradeResources(with(any(Set.class))); will(getCurrentServerSideInventory().upgradeResources()); } diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseResourceComponent.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseResourceComponent.java index c5bbf3a..04d3767 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseResourceComponent.java +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseResourceComponent.java @@ -19,11 +19,18 @@
package org.rhq.core.pc.upgrade.plugins.multi.base;
-import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; import java.util.Set;
import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.rhq.core.domain.configuration.Configuration; +import org.rhq.core.domain.configuration.Property; +import org.rhq.core.domain.configuration.PropertyList; +import org.rhq.core.domain.configuration.PropertyMap; +import org.rhq.core.domain.configuration.PropertySimple; import org.rhq.core.domain.measurement.AvailabilityType; import org.rhq.core.pluginapi.inventory.InvalidPluginConfigurationException; import org.rhq.core.pluginapi.inventory.ResourceComponent; @@ -33,13 +40,11 @@ import org.rhq.core.pluginapi.inventory.ResourceContext; * * @author Lukas Krejci */ -public class BaseResourceComponent<T extends ResourceComponent> implements ResourceComponent<T> { +public class BaseResourceComponent<T extends ResourceComponent> implements ResourceComponent<T>, BaseResourceComponentInterface {
private ResourceContext<T> context; private final Log log = LogFactory.getLog(getClass());
- private Set<Integer> childrenToFailUpgrade; - public AvailabilityType getAvailability() { return AvailabilityType.UP; } @@ -57,11 +62,57 @@ public class BaseResourceComponent<T extends ResourceComponent> implements Resou return context.getPluginConfiguration().getSimple("ordinal").getIntegerValue(); }
- public Set<Integer> getChildrenToFailUpgrade() { - return childrenToFailUpgrade == null ? Collections.<Integer>emptySet() : childrenToFailUpgrade; + public Map<String, Set<Integer>> getChildrenToFailUpgrade() { + + Map<String, Set<Integer>> ret = new HashMap<String, Set<Integer>>(); + + Configuration pluginConfig = context.getPluginConfiguration(); + PropertyList childrenToFail = pluginConfig.getList("childrenToFail"); + + if (childrenToFail != null) { + for(Property p : childrenToFail.getList()) { + PropertyMap entry = (PropertyMap) p; + + String type = entry.getSimpleValue("type", null); + + PropertyList typeList = entry.getList("children"); + + if (type != null && typeList != null) { + Set<Integer> ordinals = new HashSet<Integer>(); + + for(Property pp : typeList.getList()) { + ordinals.add(((PropertySimple)pp).getIntegerValue()); + } + + ret.put(type, ordinals); + } + } + } + + return ret; }
- public void setChildrenToFailUpgrade(Set<Integer> childrenToFailUpgrade) { - this.childrenToFailUpgrade = childrenToFailUpgrade; + public Configuration createPluginConfigurationWithMarkedFailures(Map<String, Set<Integer>> childrenToFailUpgrade) { + Configuration ret = context.getPluginConfiguration().clone(); + PropertyList list = new PropertyList("childrenToFail"); + + for(Map.Entry<String, Set<Integer>> entry : childrenToFailUpgrade.entrySet()) { + PropertyMap configEntry = new PropertyMap("childrenInType"); + configEntry.put(new PropertySimple("type", entry.getKey())); + + PropertyList typeList = new PropertyList("children"); + + for(Integer childOrdinal : entry.getValue()) { + typeList.add(new PropertySimple("ordinal", childOrdinal)); + } + + configEntry.put(typeList); + + list.add(configEntry); + } + + ret.put(list); + + return ret; } } diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseResourceComponentInterface.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseResourceComponentInterface.java new file mode 100644 index 0000000..1c9bef0 --- /dev/null +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseResourceComponentInterface.java @@ -0,0 +1,36 @@ +/* + * RHQ Management Platform + * Copyright (C) 2005-2010 Red Hat, Inc. + * All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +package org.rhq.core.pc.upgrade.plugins.multi.base; + +import java.util.Map; +import java.util.Set; + +import org.rhq.core.domain.configuration.Configuration; + +/** + * This is to support proxying to required methods from the tests. + * + * @author Lukas Krejci + */ +public interface BaseResourceComponentInterface { + + Configuration createPluginConfigurationWithMarkedFailures(Map<String, Set<Integer>> childrenToFailUpgrade); + +} diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseUpgradingDiscoveryComponent.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseUpgradingDiscoveryComponent.java index ec1cb20..5b8af94 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseUpgradingDiscoveryComponent.java +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/BaseUpgradingDiscoveryComponent.java @@ -19,6 +19,8 @@
package org.rhq.core.pc.upgrade.plugins.multi.base;
+import java.util.Set; + import org.rhq.core.domain.configuration.Configuration; import org.rhq.core.domain.resource.ResourceUpgradeReport; import org.rhq.core.pluginapi.upgrade.ResourceUpgradeContext; @@ -41,7 +43,10 @@ public class BaseUpgradingDiscoveryComponent<T extends BaseResourceComponent> ex boolean fail = Boolean.getBoolean(pluginConfig.getSimpleValue("failUpgrade", "false"));
if (!fail && parent != null) { - fail = parent.getChildrenToFailUpgrade().contains(Integer.valueOf(ordinal)); + String typeName = inventoriedResource.getResourceType().getName(); + Set<Integer> failingOrdinals = parent.getChildrenToFailUpgrade().get(typeName); + + fail = failingOrdinals != null && failingOrdinals.contains(Integer.valueOf(ordinal)); }
if (fail) { diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/NothingDiscoveringDiscoveryComponent.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/NothingDiscoveringDiscoveryComponent.java new file mode 100644 index 0000000..b6189e5 --- /dev/null +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/base/NothingDiscoveringDiscoveryComponent.java @@ -0,0 +1,42 @@ +/* + * RHQ Management Platform + * Copyright (C) 2005-2010 Red Hat, Inc. + * All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +package org.rhq.core.pc.upgrade.plugins.multi.base; + +import java.util.Collections; +import java.util.Set; + +import org.rhq.core.pluginapi.inventory.DiscoveredResourceDetails; +import org.rhq.core.pluginapi.inventory.InvalidPluginConfigurationException; +import org.rhq.core.pluginapi.inventory.ResourceComponent; +import org.rhq.core.pluginapi.inventory.ResourceDiscoveryComponent; +import org.rhq.core.pluginapi.inventory.ResourceDiscoveryContext; + +/** + * + * @author Lukas Krejci + */ +public class NothingDiscoveringDiscoveryComponent<T extends ResourceComponent> implements ResourceDiscoveryComponent<T> { + + public Set<DiscoveredResourceDetails> discoverResources(ResourceDiscoveryContext<T> context) + throws InvalidPluginConfigurationException, Exception { + + return Collections.emptySet(); + } +} diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/sibling1/rhq-plugin.xml b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/sibling1/rhq-plugin.xml index d3ea7b7..e3b4f9b 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/sibling1/rhq-plugin.xml +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/sibling1/rhq-plugin.xml @@ -11,7 +11,7 @@
<depends plugin="base" useClasses="true"/>
- <server name="Bogus" class="BaseDiscoveryComponent"> + <server name="Bogus" class="BaseDiscoveryComponent" discovery="NothingDiscoveringDiscoveryComponent"> <!-- This service is put in the type hierarchy in the v1/v2 test plugins --> <service name="TestResourceSibling" discovery="BaseDiscoveryComponent" diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/sibling2/rhq-plugin.xml b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/sibling2/rhq-plugin.xml index f7f7a2c..e7792b9 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/sibling2/rhq-plugin.xml +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/upgrade/plugins/multi/sibling2/rhq-plugin.xml @@ -10,7 +10,7 @@
<depends plugin="base" useClasses="true"/>
- <server name="Bogus" class="BaseDiscoveryComponent"> + <server name="Bogus" class="BaseDiscoveryComponent" discovery="NothingDiscoveringDiscoveryComponent"> <!-- This service is put in the type hierarchy in the v1/v2 test plugins --> <service name="TestResourceSibling" discovery="BaseUpgradingDiscoveryComponent"
rhq-commits@lists.fedorahosted.org