expand semantics of '*' for catalog remove headers, and more tests (prep for upgrades)
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/00df1d54 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/00df1d54 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/00df1d54 Branch: refs/heads/master Commit: 00df1d54369c0f5da8d1467e23a8d532a88ceb54 Parents: 9173183 Author: Alex Heneveld <[email protected]> Authored: Tue Oct 31 01:05:15 2017 +0000 Committer: Alex Heneveld <[email protected]> Committed: Tue Oct 31 01:07:44 2017 +0000 ---------------------------------------------------------------------- .../core/typereg/BundleUpgradeParser.java | 96 +++++++++------- .../core/typereg/BundleUpgradeParserTest.java | 10 +- .../BrooklynLauncherRebindCatalogOsgiTest.java | 113 +++++++++++++++++-- 3 files changed, 168 insertions(+), 51 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/00df1d54/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java index bfac6c6..24d905f 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java @@ -31,6 +31,7 @@ import java.util.Set; import org.apache.brooklyn.api.catalog.CatalogItem; import org.apache.brooklyn.api.typereg.RegisteredType; +import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.osgi.VersionedName; import org.apache.brooklyn.util.text.BrooklynVersionSyntax; @@ -60,16 +61,17 @@ public class BundleUpgradeParser { * <ul> * <li>Quoted {@code name:versionRange}, eg {@code "my-tomcat:[0,1)"}; * see {@link #MANIFEST_HEADER_FORCE_REMOVE_BUNDLES} for more information - * <li>Comma-separated list of quoted {@code name:versionRange}, eg {@code "my-tomcat:[0,1)","my-nginx:[0,1)"}; - * see {@link #MANIFEST_HEADER_FORCE_REMOVE_BUNDLES} for more information - * <li>{@code "*"} means all legacy items for things defined in this bundle, with version - * numbers lower than the version of the bundle. For example, if the bundle is - * version 1.0.0 and its catalog.bom contains items "foo" and "bar", then it is equivalent - * to writing {@code "foo:[0,1.0.0)","bar:[0,1.0.0)"}. + * <li>{@code *} means all legacy items for things defined in this bundle, with version + * numbers lower than the version of the bundle, + * and quoted {@code *:versionRange} means the indicated version(s) of types in this bundle. + * For example, if the bundle is version 1.0.0 and its catalog.bom contains items "foo" and "bar", then + * {@code *} is equivalent to writing {@code "foo:[0,1.0.0)","bar:[0,1.0.0)"}. * As per the comments on {@link #MANIFEST_HEADER_FORCE_REMOVE_BUNDLES}, * the version of the bundle and items being added by a bundle to replace legacy catalog items * should typically be larger in major/minor/point value, * as a qualifier bump can be quite complex due to ordering differences. + * <li>Comma-separated list of entries such as the above, eg {@code "my-tomcat:[0,1)","my-nginx:[0,1)"}; + * see {@link #MANIFEST_HEADER_FORCE_REMOVE_BUNDLES} for more information * </ul> */ @Beta @@ -95,15 +97,18 @@ public class BundleUpgradeParser { * as described in the "comma-separated list" format below. * This is good anyway because there are different conventions for release names * (e.g. "1.0.0" or "1.0.0.GA" or "1.0.0.2017-12") and any automation here is likely to cause surprises. - * <li>Comma-separated list of quoted {@code name:versionRange}. For example, + * <li>{@code *} as an entry, meaning all lower versions of this bundle, + * or quote {@code *:versionRange}, meaning the given version range on this bundle. + * For example, if the bundle is {@code org.example.mybundle:1.0.0}, + * then {@code *} is equivalent to writing {@code "org.example.mybundle:[0,1.0.0)"} + * and {@code "*:1-SNAPSHOT"} is equivalent to writing {@code "org.example.mybundle:1-SNAPSHOT"} + * (which when converted to OSGi is equivalent to {@code "org.example.mybundle:1.0.0.SNAPSHOT"}) + * <li>Comma-separated list of entries such as the above. For example, * {@code "org.example.mybundle:[0,1)","org.example.myotherbundle:[0,1)"} (useful for * when this bundle merges the contents of two previous bundles), or - * {@code "*","org.example.mybundle:1.0.0.SNAPSHOT","org.example.mybundle:1.0.0.rc1"} + * {@code "*","*:1.0.0.SNAPSHOT","*:1.0.0.rc1"} * when releasing {@code org.example.mybundle:1.0.0.GA} - * (to replace versions pre 1.0.0 as well as a snapshot and RC1) - * <li>{@code "*"} means all lower versions of this bundle. For example, if the bundle is - * {@code org.example.mybundle:1.0.0}, then it is equivalent to writing - * {@code "org.example.mybundle:[0,1.0.0)"} + * (to replace versions pre 1.0.0 as well as a snapshot and an rc1) * </ul> */ @Beta @@ -283,10 +288,16 @@ public class BundleUpgradeParser { } if (parts.length == 1 || Strings.isBlank(parts[1])) { throw new IllegalArgumentException("Identifier '"+val+"' must be of 'name:versionRange' syntax"); - } else if (singleVersionIsOsgiRange || (parts[1].startsWith("(") || parts[1].startsWith("["))) { - return new VersionRangedName(parts[0], parts[1]); + } + return new VersionRangedName(parts[0], parts[1], singleVersionIsOsgiRange); + } + + protected static String tidyVersionRange(String v, boolean singleVersionIsOsgiRange) { + if (v==null) return null; + if (singleVersionIsOsgiRange || (v.startsWith("(") || v.startsWith("["))) { + return v; } else { - return new VersionRangedName(parts[0], "["+parts[1]+","+parts[1]+"]"); + return "["+v+","+v+"]"; } } @@ -295,9 +306,9 @@ public class BundleUpgradeParser { this.v = checkNotNull(v, "versionRange").toString(); } - private VersionRangedName(String name, String v) { + private VersionRangedName(String name, String v, boolean singleVersionIsOsgiRange) { this.name = checkNotNull(name, "name"); - this.v = checkNotNull(v, "versionRange"); + this.v = tidyVersionRange(checkNotNull(v, "versionRange"), singleVersionIsOsgiRange); } @Override @@ -354,38 +365,32 @@ public class BundleUpgradeParser { @VisibleForTesting static List<VersionRangedName> parseForceRemoveLegacyItemsHeader(String input, Bundle bundle, Supplier<? extends Iterable<? extends RegisteredType>> typeSupplier) { if (input == null) return ImmutableList.of(); - if (stripQuotes(input.trim()).equals("*")) { - VersionRange versionRange = VersionRange.valueOf("[0,"+bundle.getVersion()+")"); - List<VersionRangedName> result = new ArrayList<>(); - for (RegisteredType item : typeSupplier.get()) { - result.add(new VersionRangedName(item.getSymbolicName(), versionRange)); - } - return result; - } else { - return parseVersionRangedNameList(input, false); + List<String> wildcardItems = MutableList.of(); + for (RegisteredType item : typeSupplier.get()) { + wildcardItems.add(item.getSymbolicName()); } + return parseVersionRangedNameList(input, false, wildcardItems, "[0,"+bundle.getVersion()+")"); } @VisibleForTesting static List<VersionRangedName> parseForceRemoveBundlesHeader(String input, Bundle bundle) { if (input == null) return ImmutableList.of(); - if (stripQuotes(input.trim()).equals("*")) { - String bundleVersion = bundle.getVersion().toString(); - String maxVersion; - if (BrooklynVersionSyntax.isSnapshot(bundleVersion)) { - maxVersion = BrooklynVersionSyntax.stripSnapshot(bundleVersion); - } else { - maxVersion = bundleVersion; - } - return ImmutableList.of(new VersionRangedName(bundle.getSymbolicName(), "[0,"+maxVersion+")")); + + String bundleVersion = bundle.getVersion().toString(); + String maxVersion; + if (BrooklynVersionSyntax.isSnapshot(bundleVersion)) { + maxVersion = BrooklynVersionSyntax.stripSnapshot(bundleVersion); } else { - return parseVersionRangedNameList(input, false); + maxVersion = bundleVersion; } + + return parseVersionRangedNameList(input, false, MutableList.of(bundle.getSymbolicName()), "[0,"+maxVersion+")"); } @VisibleForTesting - static List<VersionRangedName> parseVersionRangedNameList(String input, boolean singleVersionIsOsgiRange) { + static List<VersionRangedName> parseVersionRangedNameList(String input, boolean singleVersionIsOsgiRange, + List<String> wildcardNames, String wildcardVersion) { if (input == null) return ImmutableList.of(); List<String> vals = QuotedStringTokenizer.builder() @@ -397,7 +402,22 @@ public class BundleUpgradeParser { List<VersionRangedName> versionedItems = new ArrayList<>(); for (String val : vals) { try { - versionedItems.add(VersionRangedName.fromString(val.trim(), singleVersionIsOsgiRange)); + val = val.trim(); + if (val.startsWith("*")) { + String r; + if ("*".equals(val)) { + r = wildcardVersion; + } else if (val.startsWith("*:")) { + r = val.substring(2); + } else { + throw new IllegalArgumentException("Wildcard entry must be of the form \"*\" or \"*:range\""); + } + for (String item: wildcardNames) { + versionedItems.add(new VersionRangedName(item, r, false)); + } + } else { + versionedItems.add(VersionRangedName.fromString(val, singleVersionIsOsgiRange)); + } } catch (Exception e) { if (Strings.containsAny(val, "(", ")", "[", "]") && !Strings.containsAny(val, "'", "\"")) { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/00df1d54/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java b/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java index 26bd27a..5fc5188 100644 --- a/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java @@ -32,12 +32,14 @@ import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.core.typereg.BundleUpgradeParser.CatalogUpgrades; import org.apache.brooklyn.core.typereg.BundleUpgradeParser.VersionRangedName; import org.apache.brooklyn.test.Asserts; +import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.osgi.VersionedName; import org.apache.brooklyn.util.text.BrooklynVersionSyntax; import org.mockito.Mockito; import org.osgi.framework.Bundle; import org.osgi.framework.Version; import org.osgi.framework.VersionRange; +import org.testng.Assert; import org.testng.annotations.Test; import com.google.common.base.Supplier; @@ -113,6 +115,9 @@ public class BundleUpgradeParserTest { assertParseForceRemoveBundlesHeader("\"foo:0.1.0\"", bundle, ImmutableList.of(new VersionRangedName("foo", exactly0dot1))); assertParseForceRemoveBundlesHeader("\"*\"", bundle, ImmutableList.of(new VersionRangedName("foo.bar", from0lessThan1_2_3))); assertParseForceRemoveBundlesHeader("*", bundle, ImmutableList.of(new VersionRangedName("foo.bar", from0lessThan1_2_3))); + assertParseForceRemoveBundlesHeader("other:1, '*:[0,1)'", bundle, ImmutableList.of( + new VersionRangedName("other", VersionRange.valueOf("[1.0.0,1.0.0]")), + new VersionRangedName("foo.bar", new VersionRange('[', Version.valueOf("0"), Version.valueOf("1"), ')')))); } @Test @@ -133,6 +138,9 @@ public class BundleUpgradeParserTest { assertParseForceRemoveLegacyItemsHeader("\"foo:0.1.0\"", bundle, typeSupplier, ImmutableList.of(new VersionRangedName("foo", exactly0dot1))); assertParseForceRemoveLegacyItemsHeader("\"*\"", bundle, typeSupplier, ImmutableList.of(new VersionRangedName("foo", from0lessThan1), new VersionRangedName("bar", from0lessThan1))); assertParseForceRemoveLegacyItemsHeader("*", bundle, typeSupplier, ImmutableList.of(new VersionRangedName("foo", from0lessThan1), new VersionRangedName("bar", from0lessThan1))); + assertParseForceRemoveLegacyItemsHeader("*:1.0.0.SNAPSHOT, \"foo:[0.1,1)", bundle, typeSupplier, + ImmutableList.of(new VersionRangedName("foo", VersionRange.valueOf("[1.0.0.SNAPSHOT,1.0.0.SNAPSHOT]")), new VersionRangedName("bar", VersionRange.valueOf("[1.0.0.SNAPSHOT,1.0.0.SNAPSHOT]")), + new VersionRangedName("foo", VersionRange.valueOf("[0.1,1)")))); } @Test @@ -247,7 +255,7 @@ public class BundleUpgradeParserTest { } private void assertParseList(String input, List<VersionRangedName> expected) throws Exception { - List<VersionRangedName> actual = BundleUpgradeParser.parseVersionRangedNameList(input, false); + List<VersionRangedName> actual = BundleUpgradeParser.parseVersionRangedNameList(input, false, MutableList.of("WILCARD-NAME"), "0-WILDCARD_VERSION"); assertListsEqual(actual, expected); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/00df1d54/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java ---------------------------------------------------------------------- diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java index b67a29e..3f261df 100644 --- a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java +++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java @@ -44,8 +44,11 @@ import org.apache.brooklyn.core.mgmt.ha.OsgiManager; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.mgmt.rebind.RebindTestUtils; import org.apache.brooklyn.core.typereg.BundleUpgradeParser; +import org.apache.brooklyn.entity.stock.BasicEntity; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.test.support.TestResourceUnavailableException; +import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.osgi.Osgis; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.exceptions.ReferenceWithError; @@ -53,8 +56,6 @@ import org.apache.brooklyn.util.os.Os; import org.apache.brooklyn.util.osgi.OsgiTestResources; import org.apache.brooklyn.util.osgi.VersionedName; import org.apache.brooklyn.util.text.Identifiers; -import org.apache.brooklyn.util.time.Duration; -import org.apache.brooklyn.util.time.Time; import org.osgi.framework.Bundle; import org.osgi.framework.launch.Framework; import org.testng.Assert; @@ -313,6 +314,7 @@ public abstract class BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo } // Aled thought we supported version ranges in 'brooklyn.libraries', but doesn't work here. + // Alex confirms nope, not supported there yet (2017-10). @Test(groups="Broken") public void testInstallPreexistingBundleViaInitialBomBrooklynLibrariesReferenceWithVersionRange() throws Exception { runInstallPreexistingBundleViaInitialBomBrooklynLibrariesReference(true); @@ -617,7 +619,6 @@ public abstract class BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo } @Test - // might change this to fail in future, see RebindIteration.load public void testRebindRemovedItemWorksIfJavaClassFound() throws Exception { Set<VersionedName> bundleItemsV1 = ImmutableSet.of(VersionedName.fromString("one:1.0.0")); String bundleBomV1 = createCatalogYaml(ImmutableList.of(), bundleItemsV1); @@ -644,6 +645,90 @@ public abstract class BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo Assert.assertEquals(entity.getCatalogItemId(), bundleItemsV1.iterator().next()); } + @Test + public void testRebindRemovedItemButLeavingJavaSucceeds() throws Exception { + File initialBomFileV2 = prepForRebindRemovedItemTestReturningBomV2(true); + createAndStartApplication(launcherLast.getManagementContext(), + "services: [ { type: 'simple-entity:1.0.0' } ]"); + + // should start and promote fine, even though original catalog item ID not available + + // TODO when we switch to loading from type registry types instead of persisted java type (see RebindIteration.load) + // T2 startup should fail as the following test currently does, as we didn't allow the type to be upgraded + // (but note the following test should then succeed) + startT2(newLauncherForTests(initialBomFileV2.getAbsolutePath())); + promoteT2IfStandby(); + + Entity entity = Iterables.getOnlyElement( Iterables.getOnlyElement(launcherLast.getManagementContext().getApplications()).getChildren() ); + // TODO prep should set upgrade types and this should be 2.0.0 + Assert.assertEquals(entity.getCatalogItemId(), "simple-entity:1.0.0"); + } + @Test + public void testRebindRemovedItemIncludingJavaFails() throws Exception { + File initialBomFileV2 = prepForRebindRemovedItemTestReturningBomV2(true); + createAndStartApplication(launcherLast.getManagementContext(), + "services: [ { type: 'simple-entity:1.0.0' } ]"); + + // should have errors on promotion + + // TODO when we switch to loading from type registry types instead of persisted java type + // T2 startup should succeed as it knows the type is now a BasicEntity + startT2(newLauncherForTests(initialBomFileV2.getAbsolutePath())); + assertHotStandbyNow(launcherLast, false); + Assert.assertFalse( ((ManagementContextInternal)launcherLast.getManagementContext()).errors().isEmpty() ); + + promoteT2IfStandby(); + assertMasterEventually(launcherLast, false); + Assert.assertFalse( ((ManagementContextInternal)launcherLast.getManagementContext()).errors().isEmpty() ); + + // entity shouldn't be loaded + Asserts.assertSize( launcherLast.getManagementContext().getApplications(), 0 ); + } + + private File prepForRebindRemovedItemTestReturningBomV2(boolean removeSourceJavaBundle) throws Exception { + TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_COM_EXAMPLE_PATH); + + String initialBomV1 = Joiner.on("\n").join( + "brooklyn.catalog:", + " brooklyn.libraries:", + " - " + OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_COM_EXAMPLE_URL, + " items:", + " - id: simple-entity", + " item:", + " type: com.example.brooklyn.test.osgi.entities.SimpleEntity"); + VersionedName bundleNameV1 = new VersionedName("org.example.testRebindGetsInitialOsgiCatalog", "1.0.0"); + File bundleFileV1 = newTmpBundle(ImmutableMap.of(BasicBrooklynCatalog.CATALOG_BOM, initialBomV1.getBytes()), bundleNameV1); + File initialBomFileV1 = newTmpFile(createCatalogYaml(ImmutableList.of(bundleFileV1.toURI()), ImmutableList.of())); + + String initialBomV2 = Joiner.on("\n").join( + "brooklyn.catalog:", + " items:", + " - id: simple-entity", + " item:", + " type: "+BasicEntity.class.getName()); + VersionedName bundleNameV2 = new VersionedName("org.example.testRebindGetsInitialOsgiCatalog", "2.0.0"); + File bundleFileV2 = newTmpBundle(ImmutableMap.of(BasicBrooklynCatalog.CATALOG_BOM, initialBomV2.getBytes()), bundleNameV2, + ImmutableMap.of(BundleUpgradeParser.MANIFEST_HEADER_FORCE_REMOVE_BUNDLES, "*" + + (removeSourceJavaBundle ? ","+"'"+OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_COM_EXAMPLE_SYMBOLIC_NAME_FULL+":"+OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_VERSION+"'" + : ""))); + File initialBomFileV2 = newTmpFile(createCatalogYaml(ImmutableList.of(bundleFileV2.toURI()), ImmutableList.of())); + + startupAssertions = () -> { + String v = launcherT2==null ? "1.0.0" : "2.0.0"; + VersionedName bv = new VersionedName("org.example.testRebindGetsInitialOsgiCatalog", v); + VersionedName iv = new VersionedName("simple-entity", v); + assertManagedBundle(launcherLast, bv, MutableSet.of(iv)); + if (launcherT2==null) { + assertCatalogConsistsOfIds(launcherLast, MutableList.copyOf(COM_EXAMPLE_BUNDLE_CATALOG_IDS).append(iv)); + } else { + assertCatalogConsistsOfIds(launcherLast, MutableList.of(iv)); + } + }; + + startT1(newLauncherForTests(initialBomFileV1.getAbsolutePath())); + return initialBomFileV2; + } + protected void assertPersistedBundleListingEqualsEventually(BrooklynLauncher launcher, Set<VersionedName> bundles) { Asserts.succeedsEventually(new Runnable() { @Override public void run() { @@ -661,28 +746,32 @@ public abstract class BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo }}); } - protected void assertHotStandbyEventually(BrooklynLauncher launcher) { - Asserts.succeedsEventually(new Runnable() { - public void run() { - assertHotStandbyNow(launcher); - } - }); - } - protected void assertHotStandbyNow(BrooklynLauncher launcher) { + assertHotStandbyNow(launcher, true); + } + protected void assertHotStandbyNow(BrooklynLauncher launcher, boolean assertHealthy) { ManagementContext mgmt = launcher.getManagementContext(); assertTrue(mgmt.isStartupComplete()); assertTrue(mgmt.isRunning()); assertEquals(mgmt.getNodeState(), ManagementNodeState.HOT_STANDBY); + if (assertHealthy) { + Asserts.assertSize(((ManagementContextInternal)mgmt).errors(), 0); + } } protected void assertMasterEventually(BrooklynLauncher launcher) { + assertMasterEventually(launcher, true); + } + protected void assertMasterEventually(BrooklynLauncher launcher, boolean assertHealthy) { ManagementContext mgmt = launcher.getManagementContext(); Asserts.succeedsEventually(new Runnable() { public void run() { assertTrue(mgmt.isStartupComplete()); assertTrue(mgmt.isRunning()); assertEquals(mgmt.getNodeState(), ManagementNodeState.MASTER); + if (assertHealthy) { + Asserts.assertSize(((ManagementContextInternal)mgmt).errors(), 0); + } }}); } @@ -746,7 +835,7 @@ public abstract class BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo try { BrooklynLauncherRebindCatalogOsgiTest fixture = new LauncherRebindSubTests(); fixture.setUp(); - fixture.testRebindRemovedItemWorksIfJavaClassFound(); + fixture.testRebindRemovedItemIncludingJavaFails(); fixture.tearDown(); } catch (Exception e) { e.printStackTrace();
