Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 97384200D1F for ; Thu, 7 Sep 2017 17:47:55 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 967B11609DA; Thu, 7 Sep 2017 15:47:55 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 772521609BB for ; Thu, 7 Sep 2017 17:47:54 +0200 (CEST) Received: (qmail 86028 invoked by uid 500); 7 Sep 2017 15:47:53 -0000 Mailing-List: contact commits-help@brooklyn.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@brooklyn.apache.org Delivered-To: mailing list commits@brooklyn.apache.org Received: (qmail 86016 invoked by uid 99); 7 Sep 2017 15:47:52 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 07 Sep 2017 15:47:52 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 8E98FF5535; Thu, 7 Sep 2017 15:47:52 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: heneveld@apache.org To: commits@brooklyn.apache.org Date: Thu, 07 Sep 2017 15:47:52 -0000 Message-Id: <71728ecb70284e26949bd3e61819594f@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [01/11] brooklyn-server git commit: tests for uninstall behaviour errors archived-at: Thu, 07 Sep 2017 15:47:55 -0000 Repository: brooklyn-server Updated Branches: refs/heads/master 3d650c96b -> 36de666d2 tests for uninstall behaviour errors two tests, both of which fail in osgi subclasses only: - ensure failed bundle is uninstalled and rebind subsequently succeeds - ensure on failure previous items are left in place also adds placeholders where fixes are needed Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/3566ac0c Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/3566ac0c Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/3566ac0c Branch: refs/heads/master Commit: 3566ac0c06ce89d552029b2f20bf26bf9f80fd81 Parents: d961de3 Author: Alex Heneveld Authored: Wed Aug 16 09:36:41 2017 +0100 Committer: Alex Heneveld Committed: Wed Aug 16 09:49:01 2017 +0100 ---------------------------------------------------------------------- .../camp/brooklyn/AbstractYamlTest.java | 8 +++- .../brooklyn/camp/brooklyn/RebindOsgiTest.java | 28 +++++++++++++- .../brooklyn/catalog/CatalogYamlEntityTest.java | 39 ++++++++++++++++++++ .../catalog/internal/BasicBrooklynCatalog.java | 11 ++++-- .../core/catalog/internal/CatalogUtils.java | 1 + .../brooklyn/core/mgmt/ha/OsgiManager.java | 2 + .../rest/resources/CatalogResource.java | 1 + 7 files changed, 84 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3566ac0c/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java index 0300fc0..121413c 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java @@ -247,6 +247,7 @@ public abstract class AbstractYamlTest { Map> validation = mgmt.getCatalog().validateTypes( mgmt.getTypeRegistry().getMatching(RegisteredTypePredicates.containingBundle(b.get().getVersionedName())) ); if (!validation.isEmpty()) { + // TODO rollback throw Exceptions.propagate("Brooklyn failed to load types: "+validation.keySet(), Iterables.concat(validation.values())); } @@ -255,8 +256,11 @@ public abstract class AbstractYamlTest { } } - protected void deleteCatalogEntity(String catalogItem) { - ((BasicBrooklynTypeRegistry) mgmt().getTypeRegistry()).delete(new VersionedName(catalogItem, TEST_VERSION)); + protected void deleteCatalogEntity(String catalogItemSymbolicName) { + deleteCatalogEntity(catalogItemSymbolicName, TEST_VERSION); + } + protected void deleteCatalogEntity(String catalogItemSymbolicName, String version) { + ((BasicBrooklynTypeRegistry) mgmt().getTypeRegistry()).delete(new VersionedName(catalogItemSymbolicName, version)); } protected Logger getLogger() { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3566ac0c/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java index 07cab6d..730616b 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java @@ -32,6 +32,7 @@ import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.api.mgmt.ha.HighAvailabilityMode; import org.apache.brooklyn.api.policy.Policy; +import org.apache.brooklyn.api.typereg.ManagedBundle; import org.apache.brooklyn.core.catalog.internal.CatalogUtils; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.entity.Entities; @@ -43,6 +44,7 @@ import org.apache.brooklyn.core.mgmt.osgi.OsgiStandaloneTest; import org.apache.brooklyn.core.mgmt.osgi.OsgiVersionMoreEntityTest; import org.apache.brooklyn.core.sensor.Sensors; import org.apache.brooklyn.core.test.entity.TestEntity; +import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.test.support.TestResourceUnavailableException; import org.apache.brooklyn.util.core.ResourceUtils; import org.apache.brooklyn.util.core.osgi.Osgis; @@ -55,6 +57,7 @@ import org.osgi.framework.Bundle; import org.osgi.framework.launch.Framework; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.testng.Assert; import org.testng.annotations.BeforeMethod; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -490,7 +493,30 @@ public class RebindOsgiTest extends AbstractYamlRebindTest { Object policyConfigVal = origPolicy.config().get(ConfigKeys.newConfigKey(Object.class, OSGI_ENTITY_CONFIG_NAME)); assertEquals(getOsgiSimpleObjectsVal(policyConfigVal), "myPolicyVal"); } - + + @Test + public void testRebindAfterFailedInstall() throws Exception { + String appSymbolicName = "my.catalog.app.id.load"; + String appVersion = "0.1.0"; + Map oldBundles = origManagementContext.getOsgiManager().get().getManagedBundles(); + try { + addCatalogItems( + "brooklyn.catalog:", + " id: " + appSymbolicName, + " version: " + appVersion, + " itemType: entity", + " item:", + " type: DeliberatelyMissing"); + Asserts.shouldHaveFailedPreviously("Invalid plan was added"); + } catch (Exception e) { + Asserts.expectedFailureContains(e, "DeliberatelyMissing", appSymbolicName); + } + Map newBundles = origManagementContext.getOsgiManager().get().getManagedBundles(); + Assert.assertEquals(newBundles, oldBundles, "Bundles: "+newBundles); + + rebind(); + } + private Bundle getBundle(ManagementContext mgmt, final String symbolicName) throws Exception { OsgiManager osgiManager = ((ManagementContextInternal)mgmt).getOsgiManager().get(); Framework framework = osgiManager.getFramework(); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3566ac0c/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java index 4633817..b449f69 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java @@ -28,6 +28,7 @@ import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry; +import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind; import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest; import org.apache.brooklyn.config.ConfigKey; @@ -50,6 +51,8 @@ import com.google.common.collect.Iterables; public class CatalogYamlEntityTest extends AbstractYamlTest { + protected static final String TEST_VERSION_SNAPSHOT = TEST_VERSION + "-SNAPSHOT"; + @Test public void testAddCatalogItemVerySimple() throws Exception { String symbolicName = "my.catalog.app.id.load"; @@ -668,6 +671,42 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { deleteCatalogEntity(symbolicNameOuter); } + @Test + public void testReplacementFailureLeavesPreviousIntact() throws Exception { + String symbolicName = "my.catalog.app.id.load"; + addCatalogItems( + "brooklyn.catalog:", + " id: " + symbolicName, + " version: " + TEST_VERSION_SNAPSHOT, + " item: " + BasicEntity.class.getName()); + + RegisteredType item = mgmt().getTypeRegistry().get(symbolicName, TEST_VERSION_SNAPSHOT); + Assert.assertNotNull(item); + assertEquals(item.getKind(), RegisteredTypeKind.SPEC); + assertEquals(item.getSymbolicName(), symbolicName); + + try { + addCatalogItems( + "brooklyn.catalog:", + " id: " + symbolicName, + " version: " + TEST_VERSION_SNAPSHOT, + " item: " + "DeliberatelyMissing"); + Asserts.shouldHaveFailedPreviously(); + } catch (Exception e) { + Asserts.expectedFailureContains(e, "DeliberatelyMissing", symbolicName); + } + + RegisteredType item2 = mgmt().getTypeRegistry().get(symbolicName, TEST_VERSION_SNAPSHOT); + Assert.assertNotNull(item2, "Type was removed when broken item was added"); + assertEquals(item2.getSymbolicName(), symbolicName); + assertEquals(item2.getKind(), RegisteredTypeKind.SPEC, "Type was replaced by broken item"); + assertEquals(item2, item); + + deleteCatalogEntity(symbolicName, TEST_VERSION_SNAPSHOT); + RegisteredType item3 = mgmt().getTypeRegistry().get(symbolicName, TEST_VERSION_SNAPSHOT); + Assert.assertNull(item3, "Type should have been deleted"); + } + private void registerAndLaunchAndAssertSimpleEntity(String symbolicName, String serviceType) throws Exception { registerAndLaunchAndAssertSimpleEntity(symbolicName, serviceType, serviceType); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3566ac0c/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java index affb4e7..83d5028 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java @@ -299,8 +299,12 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { // thread local lets us call to other once then he calls us and we do other code path deletingCatalogItem.set(true); try { - ((BasicBrooklynTypeRegistry) mgmt.getTypeRegistry()).delete( - mgmt.getTypeRegistry().get(symbolicName, version) ); + RegisteredType item = mgmt.getTypeRegistry().get(symbolicName, version); + if (item==null) { + log.debug("Request to delete "+symbolicName+":"+version+" but nothing matching found; ignoring"); + } else { + ((BasicBrooklynTypeRegistry) mgmt.getTypeRegistry()).delete( item ); + } return; } finally { deletingCatalogItem.remove(); @@ -1385,10 +1389,11 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { } finally { bf.delete(); } - uninstallEmptyWrapperBundles(); if (result.getCode().isError()) { + // TODO rollback installation, restore others? throw new IllegalStateException(result.getMessage()); } + uninstallEmptyWrapperBundles(); return toLegacyCatalogItems(result.getCatalogItemsInstalled()); // if all items pertaining to an older anonymous catalog.bom bundle have been overridden http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3566ac0c/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java index 3e25384..f75a406 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java @@ -188,6 +188,7 @@ public class CatalogUtils { for (OsgiBundleInstallationResult r: results) { if (r.getDeferredStart()!=null) { r.getDeferredStart().run(); + // TODO on failure? } } if (log.isDebugEnabled()) { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3566ac0c/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java index 98c962e..b65894c 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java @@ -384,6 +384,8 @@ public class OsgiManager { catalogItems = null; } catch (RuntimeException ex) { + // TODO uninstall? + // TODO confirm -- as of May 2017 we no longer uninstall the bundle if install of catalog items fails; // caller needs to upgrade, or uninstall then reinstall // (this uninstall wouldn't have unmanaged it in brooklyn in any case) http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3566ac0c/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java ---------------------------------------------------------------------- diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java index fcb26f9..230c986 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java @@ -197,6 +197,7 @@ public class CatalogResource extends AbstractBrooklynRestResource implements Cat } if (result.hasError()) { + // TODO rollback installation? if (log.isTraceEnabled()) { log.trace("Unable to create from archive, returning 400: "+result.getError().getMessage(), result.getError()); }