modules/core/plugin-container-itest/src/test/java/org/rhq/core/pc/inventory/InventoryManagerTest.java | 34 ++++ modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java | 79 +++++----- modules/core/plugin-container/src/main/java/org/rhq/core/pc/plugin/PluginComponentFactory.java | 14 + 3 files changed, 88 insertions(+), 39 deletions(-)
New commits: commit 53a0def0333e9ac88d7301a9970dc853a617f145 Author: Ian Springer ian.springer@redhat.com Date: Thu May 24 13:13:01 2012 -0400
[BZ 824401] handle condition of a missing parent resourceContainer more gracefully in a few places, since it's normal in situations where the corresponding resource was just uninventoried - we now log a DEBUG message, rather than an ERROR message + stack trace; add a PC integration test that verifies Resource uninventory works (https://bugzilla.redhat.com/show_bug.cgi?id=824401) (cherry picked from commit 5c4322c3a5ac265cb9703b5bd3f58db5af645b02)
diff --git a/modules/core/plugin-container-itest/src/test/java/org/rhq/core/pc/inventory/InventoryManagerTest.java b/modules/core/plugin-container-itest/src/test/java/org/rhq/core/pc/inventory/InventoryManagerTest.java index 3d4a581..c367687 100644 --- a/modules/core/plugin-container-itest/src/test/java/org/rhq/core/pc/inventory/InventoryManagerTest.java +++ b/modules/core/plugin-container-itest/src/test/java/org/rhq/core/pc/inventory/InventoryManagerTest.java @@ -46,7 +46,7 @@ import static org.mockito.Matchers.any; import static org.mockito.Mockito.when;
/** - * A unit test for the {@link InventoryManager}. + * An integration test for the {@link InventoryManager}. * * @author Ian Springer */ @@ -115,6 +115,38 @@ public class InventoryManagerTest extends Arquillian { validatePluginContainerInventory(); }
+ /** + * Tests that uninventorying Resources works. + * + * @throws Exception if an error occurs + */ + @Test(groups = "pc.itest.inventorymanager", dependsOnMethods = "testSyncUnknownResources") + public void testUninventoryResources() throws Exception { + validatePluginContainerInventory(); + + Resource platform = pluginContainer.getInventoryManager().getPlatform(); + Resource server = platform.getChildResources().iterator().next(); + + // Uninventory the server. + fakeServerInventory.removeResource(server); + pluginContainer.getInventoryManager().uninventoryResource(server.getId()); + + platform = pluginContainer.getInventoryManager().getPlatform(); + Assert.assertNotNull(platform); + Assert.assertEquals(platform.getInventoryStatus(), InventoryStatus.COMMITTED); + + Assert.assertTrue(platform.getChildResources().isEmpty(), "Platform still has children: " + + platform.getChildResources()); + + // Now execute a full discovery. + System.out.println("Executing full discovery..."); + pluginContainer.getInventoryManager().executeServerScanImmediately(); + pluginContainer.getInventoryManager().executeServiceScanImmediately(); + + // Check that inventory is back to what it was before we uninventoried the server. + validatePluginContainerInventory(); + } + private void validatePluginContainerInventory() { System.out.println("Validating PC inventory...");
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 9811153..7288038 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 @@ -1498,13 +1498,7 @@ public class InventoryManager extends AgentService implements ContainerService, ResourceContainer resourceContainer = getResourceContainer(resource); if (resourceContainer == null) { PluginComponentFactory factory = PluginContainer.getInstance().getPluginComponentFactory(); - ClassLoader classLoader; - try { - classLoader = factory.getResourceClassloader(resource); - } catch (PluginContainerException e) { - log.error("Access to resource [" + resource + "] will fail due to missing classloader", e); - classLoader = null; - } + ClassLoader classLoader = getResourceClassLoader(resource); resourceContainer = new ResourceContainer(resource, classLoader); if (!this.configuration.isInsideAgent()) { // Auto-sync if the PC is running within the embedded JBossAS console. @@ -1514,20 +1508,29 @@ public class InventoryManager extends AgentService implements ContainerService, } else { // container already exists, but make sure the classloader exists too if (resourceContainer.getResourceClassLoader() == null) { - PluginComponentFactory factory = PluginContainer.getInstance().getPluginComponentFactory(); - ClassLoader classLoader; - try { - classLoader = factory.getResourceClassloader(resource); - } catch (PluginContainerException e) { - log.error("Access to resource [" + resource + "] will fail due to missing classloader!", e); - classLoader = null; - } + ClassLoader classLoader = getResourceClassLoader(resource); resourceContainer.setResourceClassLoader(classLoader); } } return resourceContainer; }
+ private ClassLoader getResourceClassLoader(Resource resource) { + PluginComponentFactory factory = PluginContainer.getInstance().getPluginComponentFactory(); + ClassLoader classLoader; + try { + classLoader = factory.getResourceClassloader(resource); + } catch (PluginContainerException e) { + if (log.isTraceEnabled()) { + log.trace("Access to resource [" + resource + "] will fail due to missing classloader.", e); + } else { + log.debug("Access to resource [" + resource + "] will fail due to missing classloader - cause: " + e); + } + classLoader = null; + } + return classLoader; + } + /** * This method prepares the resource and container for activation. * <p> @@ -1594,6 +1597,20 @@ public class InventoryManager extends AgentService implements ContainerService, + "]..."); }
+ ResourceContainer parentResourceContainer; + Resource parentResource = resource.getParentResource(); + if (parentResource == null) { + parentResourceContainer = null; + } else { + parentResourceContainer = getResourceContainer(parentResource); + if (parentResourceContainer == null) { + // The parent probably just got uninventoried - log a DEBUG message and abort. + log.debug(resource + " not being prepared for activation - container not found for parent " + + parentResource + "."); + return false; + } + } + // If the component does not even exist yet, we need to instantiate it and set it on the container. if (component == null) { if (log.isDebugEnabled()) { @@ -1601,28 +1618,17 @@ public class InventoryManager extends AgentService implements ContainerService, } try { component = PluginContainer.getInstance().getPluginComponentFactory().buildResourceComponent(resource); - } catch (Throwable e) { throw new PluginContainerException("Could not build component for Resource [" + resource + "]", e); } container.setResourceComponent(component); }
- // start the resource, but only if its parent component is running. If the parent is null, that means - // the resource is, itself, the root platform and we always activate that. - ResourceContainer parentResourceContainer; - boolean isParentStarted; - - if (resource.getParentResource() == null) { - parentResourceContainer = null; - isParentStarted = true; - } else { - parentResourceContainer = getResourceContainer(resource.getParentResource()); - isParentStarted = (parentResourceContainer.getResourceComponentState() == ResourceComponentState.STARTED); - } + // Start the resource, but only if its parent component is running. If the parent is null, that means + // the resource is, itself, the root platform, which we always activate. + boolean isParentStarted = (parentResourceContainer == null) || (parentResourceContainer.getResourceComponentState() == ResourceComponentState.STARTED);
if (isParentStarted) { - PluginComponentFactory factory = PluginContainer.getInstance().getPluginComponentFactory(); ResourceType type = resource.getResourceType(); ResourceDiscoveryComponent discoveryComponent = factory @@ -1642,8 +1648,8 @@ public class InventoryManager extends AgentService implements ContainerService,
ResourceComponent<?> parentComponent = null; ResourceContext<?> parentResourceContext = null; - if (resource.getParentResource() != null) { - ResourceContainer rc = getResourceContainer(resource.getParentResource()); + if (parentResource != null) { + ResourceContainer rc = getResourceContainer(parentResource);
parentComponent = rc.getResourceComponent(); parentResourceContext = rc.getResourceContext(); @@ -1652,8 +1658,8 @@ public class InventoryManager extends AgentService implements ContainerService, ResourceContext context = createResourceContext(resource, parentComponent, parentResourceContext, discoveryComponent); container.setResourceContext(context); - return true;
+ return true; } else { if (log.isDebugEnabled()) { log.debug("Resource [" + resource + "] not being prepared for activation; parent isn't started: " @@ -2806,15 +2812,20 @@ public class InventoryManager extends AgentService implements ContainerService, if (parentResourceContainer != null) { parentResource = parentResourceContainer.getResource(); } else { - parentResource = null; // TODO right thing to do? Or directly return? + log.debug("Skipping merge of " + resourceFromServer + + " into local inventory, since a container was not found for its parent " + + parentResourceFromServer + "."); + return; } } else { + // A null parent means this is the platform. parentResource = null; }
// See if the Resource already exists in our inventory. Resource existingResource = findMatchingChildResource(resourceFromServer, parentResource); - if (parentResource == null && existingResource == null) { + if ((existingResource == null) && + (resourceFromServer.getResourceType().getCategory() == ResourceCategory.PLATFORM)) { // This should never happen, but add a check so we'll know if it ever does. log.error("Existing platform [" + this.platform + "] has different Resource type and/or Resource key than " + "platform in Server inventory: " + resourceFromServer); diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/plugin/PluginComponentFactory.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/plugin/PluginComponentFactory.java index 5c2eab7..9866a64 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/plugin/PluginComponentFactory.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/plugin/PluginComponentFactory.java @@ -153,7 +153,14 @@ public class PluginComponentFactory implements ContainerService { }; } String className = getPluginManager().getMetadataManager().getComponentClass(resourceType); - ClassLoader resourceClassloader = getResourceClassloader(resource); + ResourceContainer resourceContainer = getInventoryManager().getResourceContainer(resource); + if (resourceContainer == null) { + throw new PluginContainerException("Resource container not found for " + resource + " - cannot create ResourceComponent."); + } + ClassLoader resourceClassloader = resourceContainer.getResourceClassLoader(); + if (resourceClassloader == null) { + throw new PluginContainerException("Resource classLoader not found for " + resource + " - cannot create ResourceComponent."); + } ResourceComponent component = (ResourceComponent) instantiateClass(resourceClassloader, className);
if (log.isDebugEnabled()) { @@ -173,7 +180,6 @@ public class PluginComponentFactory implements ContainerService { * @throws PluginContainerException if the resource's classloader could not be created */ public ClassLoader getResourceClassloader(Resource resource) throws PluginContainerException { - try { InventoryManager inventoryMgr = getInventoryManager(); PluginManager pluginMgr = getPluginManager(); @@ -193,8 +199,8 @@ public class PluginComponentFactory implements ContainerService { if (parentResource != null) { parentContainer = inventoryMgr.getResourceContainer(parentResource); if (parentContainer == null) { - throw new PluginContainerException("Missing parent resource container for parent resource=" - + parentResource); + throw new PluginContainerException("Missing container for parent " + parentResource + " of " + + resource + "."); } } else if (resource.equals(inventoryMgr.getPlatform())) { // the given resource is our top platform resource - just use its plugin classloader
rhq-commits@lists.fedorahosted.org