From 5627a766d1d34159ee126b4980dc817654bc7508 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 17 Jul 2020 14:23:41 +0200 Subject: Reapply "Load platform bundles separately 3" This reverts commit 1c69b4e72b3212e3ce989a8675db08ac51b7f79d. --- .../ClusterControllerContainer.java | 10 +- .../container/ApplicationContainerCluster.java | 9 +- .../vespa/model/container/ContainerCluster.java | 9 +- .../MetricsProxyContainerClusterTest.java | 11 +- .../model/container/ContainerClusterTest.java | 6 +- .../model/container/xml/DocprocBuilderTest.java | 14 +- .../core/config/ApplicationBundleLoader.java | 142 +++++++++++++ .../yahoo/container/core/config/BundleManager.java | 236 --------------------- .../yahoo/container/core/config/BundleStarter.java | 40 ++++ .../core/config/HandlersConfigurerDi.java | 18 +- .../core/config/PlatformBundleLoader.java | 54 +++++ .../testutil/HandlersConfigurerTestWrapper.java | 3 +- .../core/config/testutil/MockOsgiWrapper.java | 5 - .../src/main/java/com/yahoo/osgi/MockOsgi.java | 6 +- .../src/main/java/com/yahoo/osgi/Osgi.java | 6 +- .../src/main/java/com/yahoo/osgi/OsgiImpl.java | 11 +- .../core/config/ApplicationBundleLoaderTest.java | 106 +++++++++ .../container/core/config/BundleManagerTest.java | 106 --------- container-di/CMakeLists.txt | 2 + .../java/com/yahoo/container/di/Container.java | 52 +++-- .../src/main/java/com/yahoo/container/di/Osgi.java | 6 +- .../configdefinitions/application-bundles.def | 5 + .../main/resources/configdefinitions/bundles.def | 2 +- .../configdefinitions/platform-bundles.def | 5 + .../java/com/yahoo/container/di/ContainerTest.java | 3 +- .../com/yahoo/container/di/ContainerTestBase.java | 5 +- .../com/yahoo/jdisc/application/OsgiFramework.java | 7 + .../java/com/yahoo/jdisc/core/FelixFramework.java | 5 + .../standalone/StandaloneSubscriberTest.java | 15 +- .../java/com/yahoo/collections/PredicateSplit.java | 40 ---- .../yahoo/collections/PredicateSplitTestCase.java | 30 --- 31 files changed, 471 insertions(+), 498 deletions(-) create mode 100644 container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java delete mode 100644 container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java create mode 100644 container-core/src/main/java/com/yahoo/container/core/config/BundleStarter.java create mode 100644 container-core/src/main/java/com/yahoo/container/core/config/PlatformBundleLoader.java create mode 100644 container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java delete mode 100644 container-core/src/test/java/com/yahoo/container/core/config/BundleManagerTest.java create mode 100644 container-di/src/main/resources/configdefinitions/application-bundles.def create mode 100644 container-di/src/main/resources/configdefinitions/platform-bundles.def delete mode 100644 vespajlib/src/main/java/com/yahoo/collections/PredicateSplit.java delete mode 100644 vespajlib/src/test/java/com/yahoo/collections/PredicateSplitTestCase.java diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java index 547c05d2c9b..9cc83bb4275 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java @@ -5,8 +5,8 @@ import com.yahoo.cloud.config.ZookeeperServerConfig; import com.yahoo.component.ComponentSpecification; import com.yahoo.config.model.api.container.ContainerServiceType; import com.yahoo.config.model.producer.AbstractConfigProducer; -import com.yahoo.container.BundlesConfig; import com.yahoo.container.bundle.BundleInstantiationSpecification; +import com.yahoo.container.di.config.PlatformBundlesConfig; import com.yahoo.osgi.provider.model.ComponentModel; import com.yahoo.vespa.config.content.FleetcontrollerConfig; import com.yahoo.vespa.model.application.validation.RestartConfigs; @@ -25,7 +25,7 @@ import static com.yahoo.vespa.defaults.Defaults.getDefaults; */ @RestartConfigs({FleetcontrollerConfig.class, ZookeeperServerConfig.class}) public class ClusterControllerContainer extends Container implements - BundlesConfig.Producer, + PlatformBundlesConfig.Producer, ZookeeperServerConfig.Producer { private static final ComponentSpecification CLUSTERCONTROLLER_BUNDLE = new ComponentSpecification("clustercontroller-apps"); @@ -102,10 +102,8 @@ public class ClusterControllerContainer extends Container implements } @Override - public void getConfig(BundlesConfig.Builder builder) { - for (String bundle : bundles) { - builder.bundle(bundle); - } + public void getConfig(PlatformBundlesConfig.Builder builder) { + bundles.forEach(builder::bundles); } @Override diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java index 9ce5fdfcc04..b0ac02d0fe8 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java @@ -8,8 +8,8 @@ import com.yahoo.config.FileReference; import com.yahoo.config.application.api.ComponentInfo; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; -import com.yahoo.container.BundlesConfig; import com.yahoo.container.bundle.BundleInstantiationSpecification; +import com.yahoo.container.di.config.ApplicationBundlesConfig; import com.yahoo.container.handler.ThreadpoolConfig; import com.yahoo.container.handler.metrics.MetricsProxyApiConfig; import com.yahoo.container.handler.metrics.MetricsV2Handler; @@ -46,7 +46,7 @@ import java.util.stream.Stream; * @author gjoranv */ public final class ApplicationContainerCluster extends ContainerCluster implements - BundlesConfig.Producer, + ApplicationBundlesConfig.Producer, QrStartConfig.Producer, RankProfilesConfig.Producer, RankingConstantsConfig.Producer, @@ -188,10 +188,9 @@ public final class ApplicationContainerCluster extends ContainerCluster getMemoryPercentage() { return Optional.ofNullable(memoryPercentage); } @Override - public void getConfig(BundlesConfig.Builder builder) { + public void getConfig(ApplicationBundlesConfig.Builder builder) { applicationBundles.stream().map(FileReference::value) - .forEach(builder::bundle); - super.getConfig(builder); + .forEach(builder::bundles); } @Override diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java index 5127616ad5e..e2d0868cb9d 100755 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java @@ -12,12 +12,12 @@ import com.yahoo.config.model.ApplicationConfigProducerRoot; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.config.provision.Zone; -import com.yahoo.container.BundlesConfig; import com.yahoo.container.ComponentsConfig; import com.yahoo.container.QrSearchersConfig; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.container.core.ApplicationMetadataConfig; import com.yahoo.container.core.document.ContainerDocumentConfig; +import com.yahoo.container.di.config.PlatformBundlesConfig; import com.yahoo.container.handler.ThreadpoolConfig; import com.yahoo.container.jdisc.JdiscBindingsConfig; import com.yahoo.container.jdisc.config.HealthMonitorConfig; @@ -87,7 +87,7 @@ public abstract class ContainerCluster ContainerDocumentConfig.Producer, HealthMonitorConfig.Producer, ApplicationMetadataConfig.Producer, - BundlesConfig.Producer, + PlatformBundlesConfig.Producer, IndexInfoConfig.Producer, IlscriptsConfig.Producer, SchemamappingConfig.Producer, @@ -464,6 +464,7 @@ public abstract class ContainerCluster /** * Adds a bundle present at a known location at the target container nodes. + * Note that the set of platform bundles cannot change during the jdisc container's lifetime. * * @param bundlePath usually an absolute path, e.g. '$VESPA_HOME/lib/jars/foo.jar' */ @@ -472,9 +473,9 @@ public abstract class ContainerCluster } @Override - public void getConfig(BundlesConfig.Builder builder) { + public void getConfig(PlatformBundlesConfig.Builder builder) { platformBundles.stream() .map(ContainerCluster::toFileReferenceString) - .forEach(builder::bundle); + .forEach(builder::bundles); } private static String toFileReferenceString(Path path) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java index bed77bd5c77..754f4b66070 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java @@ -15,8 +15,8 @@ import ai.vespa.metricsproxy.metric.dimensions.PublicDimensions; import com.yahoo.component.ComponentSpecification; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.provision.Zone; -import com.yahoo.container.BundlesConfig; import com.yahoo.container.core.ApplicationMetadataConfig; +import com.yahoo.container.di.config.PlatformBundlesConfig; import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames; @@ -44,7 +44,6 @@ import static org.hamcrest.CoreMatchers.endsWith; import static org.hamcrest.CoreMatchers.hasItem; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -56,11 +55,11 @@ public class MetricsProxyContainerClusterTest { @Test public void metrics_proxy_bundle_is_included_in_bundles_config() { VespaModel model = getModel(servicesWithAdminOnly(), self_hosted); - var builder = new BundlesConfig.Builder(); + var builder = new PlatformBundlesConfig.Builder(); model.getConfig(builder, CLUSTER_CONFIG_ID); - BundlesConfig config = builder.build(); - assertEquals(1, config.bundle().size()); - assertThat(config.bundle(0).value(), endsWith(METRICS_PROXY_BUNDLE_FILE.toString())); + PlatformBundlesConfig config = builder.build(); + assertEquals(1, config.bundles().size()); + assertThat(config.bundles(0).value(), endsWith(METRICS_PROXY_BUNDLE_FILE.toString())); } @Test diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java index 03c05af1145..07b6cd72bdb 100755 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java @@ -16,7 +16,7 @@ import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.config.provisioning.FlavorsConfig; -import com.yahoo.container.BundlesConfig; +import com.yahoo.container.di.config.PlatformBundlesConfig; import com.yahoo.container.handler.ThreadpoolConfig; import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.Host; @@ -292,9 +292,9 @@ public class ContainerClusterTest { .zone(zone).build(); MockRoot root = new MockRoot("foo", state); ApplicationContainerCluster cluster = new ApplicationContainerCluster(root, "container0", "container1", state); - BundlesConfig.Builder bundleBuilder = new BundlesConfig.Builder(); + var bundleBuilder = new PlatformBundlesConfig.Builder(); cluster.getConfig(bundleBuilder); - List installedBundles = bundleBuilder.build().bundle().stream().map(FileReference::value).collect(Collectors.toList()); + List installedBundles = bundleBuilder.build().bundles().stream().map(FileReference::value).collect(Collectors.toList()); assertEquals(expectedBundleNames.size(), installedBundles.size()); assertThat(installedBundles, containsInAnyOrder( diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java index eda90b03147..e9048cf7863 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java @@ -3,17 +3,16 @@ package com.yahoo.vespa.model.container.xml; import com.yahoo.config.docproc.DocprocConfig; import com.yahoo.config.docproc.SchemamappingConfig; -import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.builder.xml.test.DomBuilderTest; -import com.yahoo.container.BundlesConfig; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.container.ComponentsConfig; import com.yahoo.container.core.ChainsConfig; import com.yahoo.container.jdisc.ContainerMbusConfig; import com.yahoo.document.config.DocumentmanagerConfig; import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.HostPorts; -import com.yahoo.vespa.model.container.ApplicationContainerCluster; import com.yahoo.vespa.model.container.ApplicationContainer; +import com.yahoo.vespa.model.container.ApplicationContainerCluster; import com.yahoo.vespa.model.container.ContainerModel; import com.yahoo.vespa.model.container.docproc.DocprocChain; import com.yahoo.vespa.model.container.docproc.DocumentProcessor; @@ -30,8 +29,8 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.core.IsNull.notNullValue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** @@ -46,7 +45,6 @@ public class DocprocBuilderTest extends DomBuilderTest { private ContainerMbusConfig containerMbusConfig; private ComponentsConfig componentsConfig; private ChainsConfig chainsConfig; - private BundlesConfig bundlesConfig; private SchemamappingConfig schemamappingConfig; private DocprocConfig docprocConfig; private QrStartConfig qrStartConfig; @@ -64,7 +62,6 @@ public class DocprocBuilderTest extends DomBuilderTest { cluster.getConfigId() + "/component/com.yahoo.docproc.jdisc.DocumentProcessingHandler"); documentmanagerConfig = root.getConfig(DocumentmanagerConfig.class, cluster.getConfigId()); - bundlesConfig = root.getConfig(BundlesConfig.class, cluster.getConfigId()); schemamappingConfig = root.getConfig(SchemamappingConfig.class, cluster.getContainers().get(0).getConfigId()); qrStartConfig = root.getConfig(QrStartConfig.class, cluster.getConfigId()); docprocConfig = root.getConfig(DocprocConfig.class, cluster.getConfigId()); @@ -206,11 +203,6 @@ public class DocprocBuilderTest extends DomBuilderTest { assertThat(chainsMap.get("chein").phases().size(), is(0)); } - @Test - public void testBundlesConfig() { - assertTrue(bundlesConfig.bundle().isEmpty()); - } - @Test public void testSchemaMappingConfig() { assertTrue(schemamappingConfig.fieldmapping().isEmpty()); diff --git a/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java b/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java new file mode 100644 index 00000000000..5236daf0302 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/core/config/ApplicationBundleLoader.java @@ -0,0 +1,142 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import com.yahoo.container.Container; +import com.yahoo.filedistribution.fileacquirer.FileAcquirer; +import com.yahoo.osgi.Osgi; +import org.osgi.framework.Bundle; +import org.osgi.framework.wiring.BundleRevision; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.logging.Logger; +import java.util.stream.Collectors; + +/** + * Manages the set of installed and active/inactive bundles. + * + * @author gjoranv + * @author Tony Vaagenes + */ +public class ApplicationBundleLoader { + + /* Map of file refs of active bundles (not scheduled for uninstall) to the installed bundle. + * + * Used to: + * 1. Avoid installing already installed bundles. Just an optimization, installing the same bundle location is a NOP + * 2. Start bundles (all are started every time) + * 3. Calculate the set of bundles to uninstall + */ + private final Map reference2Bundle = new LinkedHashMap<>(); + + private final Logger log = Logger.getLogger(ApplicationBundleLoader.class.getName()); + private final Osgi osgi; + + // A custom bundle installer for non-disk bundles, to be used for testing + private BundleInstaller customBundleInstaller = null; + + public ApplicationBundleLoader(Osgi osgi) { + this.osgi = osgi; + } + + /** + * Installs the given set of bundles and returns the set of bundles that is no longer used + * by the application, and should therefore be scheduled for uninstall. + */ + public synchronized Set useBundles(List newFileReferences) { + + Set obsoleteReferences = getObsoleteFileReferences(newFileReferences); + Set bundlesToUninstall = getObsoleteBundles(obsoleteReferences); + log.info("Bundles to schedule for uninstall: " + bundlesToUninstall); + + osgi.allowDuplicateBundles(bundlesToUninstall); + removeInactiveFileReferences(obsoleteReferences); + + installBundles(newFileReferences); + BundleStarter.startBundles(reference2Bundle.values()); + log.info(installedBundlesMessage()); + + return bundlesToUninstall; + } + + private Set getObsoleteFileReferences(List newReferences) { + Set obsoleteReferences = new HashSet<>(reference2Bundle.keySet()); + obsoleteReferences.removeAll(newReferences); + return obsoleteReferences; + } + + + /** + * Returns the bundles that will not be retained by the new application generation. + */ + private Set getObsoleteBundles(Set obsoleteReferences) { + return obsoleteReferences.stream().map(reference2Bundle::get).collect(Collectors.toSet()); + } + + private void removeInactiveFileReferences(Set fileReferencesToRemove) { + fileReferencesToRemove.forEach(reference2Bundle::remove); + } + + private void installBundles(List references) { + Set bundlesToInstall = new HashSet<>(references); + + // This is just an optimization, as installing a bundle with the same location id returns the already installed bundle. + bundlesToInstall.removeAll(reference2Bundle.keySet()); + + if (!bundlesToInstall.isEmpty()) { + FileAcquirer fileAcquirer = Container.get().getFileAcquirer(); + boolean hasFileDistribution = (fileAcquirer != null); + if (hasFileDistribution) { + installWithFileDistribution(bundlesToInstall, new FileAcquirerBundleInstaller(fileAcquirer)); + } else if (customBundleInstaller != null) { + installWithFileDistribution(bundlesToInstall, customBundleInstaller); + } else { + log.warning("Can't retrieve bundles since file distribution is disabled."); + } + } + } + + private void installWithFileDistribution(Set bundlesToInstall, BundleInstaller bundleInstaller) { + for (FileReference reference : bundlesToInstall) { + try { + log.info("Installing bundle with reference '" + reference.value() + "'"); + List bundles = bundleInstaller.installBundles(reference, osgi); + + // Throw if more than one bundle was installed, which means that the X-JDisc-Preinstall-Bundle header was used. + // However, if the OSGi framework is only a test framework, this rule does not apply. + if (bundles.size() > 1 && osgi.hasFelixFramework()) { + throw new RuntimeException("Bundle '" + bundles.get(0).getSymbolicName() + "' tried to pre-install other bundles."); + } + reference2Bundle.put(reference, bundles.get(0)); + } + catch(Exception e) { + throw new RuntimeException("Could not install bundle with reference '" + reference + "'", e); + } + } + } + + private String installedBundlesMessage() { + StringBuilder sb = new StringBuilder("Installed bundles: {" ); + for (Bundle b : osgi.getBundles()) + sb.append("[" + b.getBundleId() + "]" + b.getSymbolicName() + ":" + b.getVersion() + ", "); + sb.setLength(sb.length() - 2); + sb.append("}"); + return sb.toString(); + } + + // Only for testing + void useCustomBundleInstaller(BundleInstaller bundleInstaller) { + customBundleInstaller = bundleInstaller; + } + + // Only for testing + List getActiveFileReferences() { + return new ArrayList<>(reference2Bundle.keySet()); + } + +} diff --git a/container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java b/container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java deleted file mode 100644 index 406d68408e3..00000000000 --- a/container-core/src/main/java/com/yahoo/container/core/config/BundleManager.java +++ /dev/null @@ -1,236 +0,0 @@ -// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.container.core.config; - -import com.yahoo.collections.PredicateSplit; -import com.yahoo.config.FileReference; -import com.yahoo.container.Container; -import com.yahoo.filedistribution.fileacquirer.FileAcquirer; -import com.yahoo.osgi.Osgi; -import org.osgi.framework.Bundle; -import org.osgi.framework.wiring.BundleRevision; - -import java.util.ArrayList; -import java.util.HashSet; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.logging.Logger; -import java.util.stream.Collectors; - -import static com.yahoo.collections.PredicateSplit.partition; -import static com.yahoo.container.core.BundleLoaderProperties.DISK_BUNDLE_PREFIX; - -/** - * Manages the set of installed and active/inactive bundles. - * - * @author gjoranv - * @author Tony Vaagenes - */ -public class BundleManager { - - /* Map of file refs of active bundles (not scheduled for uninstall) to a list of all bundles that were installed - * (pre-install directive) by the bundle pointed to by the file ref (including itself). - * - * Used to: - * 1. Avoid installing already installed bundles. Just an optimization, installing the same bundle location is a NOP - * 2. Start bundles (all are started every time) - * 3. Calculate the set of bundles to uninstall - */ - private final Map> reference2Bundles = new LinkedHashMap<>(); - - private final Logger log = Logger.getLogger(BundleManager.class.getName()); - private final Osgi osgi; - - // A custom bundle installer for non-disk bundles, to be used for testing - private BundleInstaller customBundleInstaller = null; - - public BundleManager(Osgi osgi) { - this.osgi = osgi; - } - - /** - * Installs the given set of bundles and returns the set of bundles that is no longer used - * by the application, and should therefore be scheduled for uninstall. - */ - public synchronized Set use(List newFileReferences) { - // Must be done before allowing duplicates because allowed duplicates affect osgi.getCurrentBundles - Set bundlesToUninstall = getObsoleteBundles(newFileReferences); - - Set obsoleteReferences = getObsoleteFileReferences(newFileReferences); - allowDuplicateBundles(obsoleteReferences); - removeInactiveFileReferences(obsoleteReferences); - - installBundles(newFileReferences); - startBundles(); - - bundlesToUninstall.removeAll(allActiveBundles()); - log.info("Bundles to schedule for uninstall: " + bundlesToUninstall); - - log.info(installedBundlesMessage()); - return bundlesToUninstall; - } - - /** - * Returns the bundles that are not assumed to be retained by the new application generation. - * Note that at this point we don't yet know the full set of new bundles, because of the potential - * pre-install directives in the new bundles. However, only "disk bundles" (file:) can be listed - * in the pre-install directive, so we know about all the obsolete application bundles. - */ - private Set getObsoleteBundles(List newReferences) { - Set bundlesToRemove = new HashSet<>(osgi.getCurrentBundles()); - - for (FileReference fileReferenceToKeep : newReferences) { - if (reference2Bundles.containsKey(fileReferenceToKeep)) { - bundlesToRemove.removeAll(reference2Bundles.get(fileReferenceToKeep)); - } - } - bundlesToRemove.removeAll(osgi.getInitialBundles()); - return bundlesToRemove; - } - - - private Set getObsoleteFileReferences(List newReferences) { - Set obsoleteReferences = new HashSet<>(reference2Bundles.keySet()); - obsoleteReferences.removeAll(newReferences); - return obsoleteReferences; - } - - /** - * Allow duplicates (bsn+version) for each bundle that corresponds to obsolete file references, - * and avoid allowing duplicates for bundles that were installed via the - * X-JDisc-Preinstall-Bundle directive. These bundles are always "disk bundles" (library - * bundles installed on the node, and not transferred via file distribution). - * Such bundles will never have duplicates because they always have the same location id. - */ - private void allowDuplicateBundles(Set obsoleteReferences) { - // The bundle at index 0 for each file reference always corresponds to the bundle at the file reference location - Set allowedDuplicates = obsoleteReferences.stream() - .filter(reference -> ! isDiskBundle(reference)) - .map(reference -> reference2Bundles.get(reference).get(0)) - .collect(Collectors.toSet()); - - log.info(() -> allowedDuplicates.isEmpty() ? "" : "Adding bundles to allowed duplicates: " + allowedDuplicates); - osgi.allowDuplicateBundles(allowedDuplicates); - } - - /** - * Cleans up the map of active file references - */ - private void removeInactiveFileReferences(Set fileReferencesToRemove) { - // Clean up the map of active bundles - fileReferencesToRemove.forEach(reference2Bundles::remove); - } - - private void installBundles(List references) { - Set bundlesToInstall = new HashSet<>(references); - - // This is just an optimization, as installing a bundle with the same location id returns the already installed bundle. - bundlesToInstall.removeAll(reference2Bundles.keySet()); - - PredicateSplit bundlesToInstall_isDisk = partition(bundlesToInstall, BundleManager::isDiskBundle); - installBundlesFromDisk(bundlesToInstall_isDisk.trueValues); - installBundlesFromFileDistribution(bundlesToInstall_isDisk.falseValues); - } - - private static boolean isDiskBundle(FileReference fileReference) { - return fileReference.value().startsWith(DISK_BUNDLE_PREFIX); - } - - private void installBundlesFromDisk(List bundlesToInstall) { - for (FileReference reference : bundlesToInstall) { - try { - installBundleFromDisk(reference); - } - catch(Exception e) { - throw new RuntimeException("Could not install bundle '" + reference + "'", e); - } - } - } - - private void installBundlesFromFileDistribution(List bundlesToInstall) { - if (!bundlesToInstall.isEmpty()) { - FileAcquirer fileAcquirer = Container.get().getFileAcquirer(); - boolean hasFileDistribution = (fileAcquirer != null); - if (hasFileDistribution) { - installWithFileDistribution(bundlesToInstall, new FileAcquirerBundleInstaller(fileAcquirer)); - } else if (customBundleInstaller != null) { - installWithFileDistribution(bundlesToInstall, customBundleInstaller); - } else { - log.warning("Can't retrieve bundles since file distribution is disabled."); - } - } - } - - private void installBundleFromDisk(FileReference reference) { - log.info("Installing bundle from disk with reference '" + reference.value() + "'"); - - var bundleInstaller = new DiskBundleInstaller(); - List bundles = bundleInstaller.installBundles(reference, osgi); - reference2Bundles.put(reference, bundles); - } - - private void installWithFileDistribution(List bundlesToInstall, BundleInstaller bundleInstaller) { - for (FileReference reference : bundlesToInstall) { - try { - log.info("Installing bundle with reference '" + reference.value() + "'"); - List bundles = bundleInstaller.installBundles(reference, osgi); - reference2Bundles.put(reference, bundles); - } - catch(Exception e) { - throw new RuntimeException("Could not install bundle '" + reference + "'", e); - } - } - } - - /** - * Resolves and starts (calls the Bundles BundleActivator) all bundles. Bundle resolution must take place - * after all bundles are installed to ensure that the framework can resolve dependencies between bundles. - */ - private void startBundles() { - for (List bundles : reference2Bundles.values()) { - for (Bundle bundle : bundles) { - try { - if ( ! isFragment(bundle)) - bundle.start(); // NOP for already ACTIVE bundles - } catch(Exception e) { - throw new RuntimeException("Could not start bundle '" + bundle.getSymbolicName() + "'", e); - } - } - } - } - - private boolean isFragment(Bundle bundle) { - BundleRevision bundleRevision = bundle.adapt(BundleRevision.class); - if (bundleRevision == null) - throw new NullPointerException("Null bundle revision means that bundle has probably been uninstalled: " + - bundle.getSymbolicName() + ":" + bundle.getVersion()); - return (bundleRevision.getTypes() & BundleRevision.TYPE_FRAGMENT) != 0; - } - - private Set allActiveBundles() { - return reference2Bundles.keySet().stream() - .flatMap(reference -> reference2Bundles.get(reference).stream()) - .collect(Collectors.toSet()); - } - - private String installedBundlesMessage() { - StringBuilder sb = new StringBuilder("Installed bundles: {" ); - for (Bundle b : osgi.getBundles()) - sb.append("[" + b.getBundleId() + "]" + b.getSymbolicName() + ":" + b.getVersion() + ", "); - sb.setLength(sb.length() - 2); - sb.append("}"); - return sb.toString(); - } - - // Only for testing - void useCustomBundleInstaller(BundleInstaller bundleInstaller) { - customBundleInstaller = bundleInstaller; - } - - // Only for testing - List getActiveFileReferences() { - return new ArrayList<>(reference2Bundles.keySet()); - } - -} diff --git a/container-core/src/main/java/com/yahoo/container/core/config/BundleStarter.java b/container-core/src/main/java/com/yahoo/container/core/config/BundleStarter.java new file mode 100644 index 00000000000..4a87c27b990 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/core/config/BundleStarter.java @@ -0,0 +1,40 @@ +package com.yahoo.container.core.config; + +import org.osgi.framework.Bundle; +import org.osgi.framework.wiring.BundleRevision; + +import java.util.Collection; + +/** + * Utility to start a collection of bundles. + * + * @author gjoranv + */ +public class BundleStarter { + + private BundleStarter() { } + + /** + * Resolves and starts (calls the Bundles BundleActivator) all bundles. Bundle resolution must take place + * after all bundles are installed to ensure that the framework can resolve dependencies between bundles. + */ + static void startBundles(Collection bundles) { + for (var bundle : bundles) { + try { + if ( ! isFragment(bundle)) + bundle.start(); // NOP for already ACTIVE bundles + } catch(Exception e) { + throw new RuntimeException("Could not start bundle '" + bundle.getSymbolicName() + "'", e); + } + } + } + + private static boolean isFragment(Bundle bundle) { + BundleRevision bundleRevision = bundle.adapt(BundleRevision.class); + if (bundleRevision == null) + throw new NullPointerException("Null bundle revision means that bundle has probably been uninstalled: " + + bundle.getSymbolicName() + ":" + bundle.getVersion()); + return (bundleRevision.getTypes() & BundleRevision.TYPE_FRAGMENT) != 0; + } + +} diff --git a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java index b983cbcfe37..80c39bef2cb 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java @@ -101,12 +101,16 @@ public class HandlersConfigurerDi { private static class ContainerAndDiOsgi extends OsgiImpl implements OsgiWrapper { private final OsgiFramework osgiFramework; - private final BundleManager bundleManager; + private final ApplicationBundleLoader applicationBundleLoader; + private final PlatformBundleLoader platformBundleLoader; public ContainerAndDiOsgi(OsgiFramework osgiFramework) { super(osgiFramework); this.osgiFramework = osgiFramework; - bundleManager = new BundleManager(new OsgiImpl(osgiFramework)); + + OsgiImpl osgi = new OsgiImpl(osgiFramework); + applicationBundleLoader = new ApplicationBundleLoader(osgi); + platformBundleLoader = new PlatformBundleLoader(osgi); } @@ -131,9 +135,15 @@ public class HandlersConfigurerDi { } @Override - public Set useBundles(Collection bundles) { + public void installPlatformBundles(Collection bundles) { + log.fine("Installing platform bundles."); + platformBundleLoader.useBundles(new ArrayList<>(bundles)); + } + + @Override + public Set useApplicationBundles(Collection bundles) { log.info("Installing bundles from the latest application"); - return bundleManager.use(new ArrayList<>(bundles)); + return applicationBundleLoader.useBundles(new ArrayList<>(bundles)); } } diff --git a/container-core/src/main/java/com/yahoo/container/core/config/PlatformBundleLoader.java b/container-core/src/main/java/com/yahoo/container/core/config/PlatformBundleLoader.java new file mode 100644 index 00000000000..f922bc0cb25 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/core/config/PlatformBundleLoader.java @@ -0,0 +1,54 @@ +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import com.yahoo.osgi.Osgi; +import org.osgi.framework.Bundle; + +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.logging.Logger; + +/** + * Installs all platform bundles, using the {@link DiskBundleInstaller}. + * All platform bundles reside on disk, and they are never uninstalled. + * + * @author gjoranv + */ +public class PlatformBundleLoader { + private static final Logger log = Logger.getLogger(PlatformBundleLoader.class.getName()); + + private final Osgi osgi; + private final DiskBundleInstaller installer; + + public PlatformBundleLoader(Osgi osgi) { + this.osgi = osgi; + installer = new DiskBundleInstaller(); + } + + public void useBundles(List fileReferences) { + Set installedBundles = install(fileReferences); + BundleStarter.startBundles(installedBundles); + } + + private Set install(List bundlesToInstall) { + var installedBundles = new LinkedHashSet(); + for (FileReference reference : bundlesToInstall) { + try { + installedBundles.addAll(installBundleFromDisk(reference)); + } + catch(Exception e) { + throw new RuntimeException("Could not install bundle '" + reference + "'", e); + } + } + return installedBundles; + } + + private List installBundleFromDisk(FileReference reference) { + log.info("Installing bundle from disk with reference '" + reference.value() + "'"); + List bundles = installer.installBundles(reference, osgi); + log.fine("Installed " + bundles.size() + " bundles for file reference " + reference); + return bundles; + } + +} diff --git a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java index 9f49b016b68..503bf2f2db1 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java @@ -41,7 +41,8 @@ public class HandlersConfigurerTestWrapper { private final static String testFiles[] = { "components.cfg", "handlers.cfg", - "bundles.cfg", + "platform-bundles.cfg", + "application-bundles.cfg", "string.cfg", "int.cfg", "renderers.cfg", diff --git a/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java b/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java index ac0fbd71671..98c927b8efd 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/testutil/MockOsgiWrapper.java @@ -15,11 +15,6 @@ import static java.util.Collections.emptyList; */ public class MockOsgiWrapper implements OsgiWrapper { - @Override - public List getInitialBundles() { - return emptyList(); - } - @Override public Bundle[] getBundles() { return new Bundle[0]; diff --git a/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java b/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java index d809c493565..6a700a65a03 100644 --- a/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java +++ b/container-core/src/main/java/com/yahoo/osgi/MockOsgi.java @@ -12,14 +12,10 @@ import java.util.List; /** * @author Tony Vaagenes + * @author gjoranv */ public class MockOsgi extends NonWorkingOsgiFramework implements Osgi { - @Override - public List getInitialBundles() { - return Collections.emptyList(); - } - @Override public Bundle[] getBundles() { return new Bundle[0]; diff --git a/container-core/src/main/java/com/yahoo/osgi/Osgi.java b/container-core/src/main/java/com/yahoo/osgi/Osgi.java index 8f0acf41f30..513e7883594 100644 --- a/container-core/src/main/java/com/yahoo/osgi/Osgi.java +++ b/container-core/src/main/java/com/yahoo/osgi/Osgi.java @@ -9,11 +9,10 @@ import java.util.List; /** * @author Tony Vaagenes + * @author gjoranv */ public interface Osgi { - List getInitialBundles(); - Bundle[] getBundles(); /** Returns all bundles that have not been scheduled for uninstall. */ @@ -25,4 +24,7 @@ public interface Osgi { void allowDuplicateBundles(Collection bundles); + default boolean hasFelixFramework() { + return false; + } } diff --git a/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java b/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java index ed93d15c975..b34442d50a9 100644 --- a/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java +++ b/container-core/src/main/java/com/yahoo/osgi/OsgiImpl.java @@ -16,6 +16,7 @@ import java.util.logging.Logger; /** * @author Tony Vaagenes * @author bratseth + * @author gjoranv */ public class OsgiImpl implements Osgi { private static final Logger log = Logger.getLogger(OsgiImpl.class.getName()); @@ -41,11 +42,6 @@ public class OsgiImpl implements Osgi { log.info("Using " + alwaysCurrentBundle + " to lookup current bundles."); } - @Override - public List getInitialBundles() { - return initialBundles; - } - @Override public Bundle[] getBundles() { List bundles = jdiscOsgi.bundles(); @@ -155,6 +151,11 @@ public class OsgiImpl implements Osgi { jdiscOsgi.allowDuplicateBundles(bundles); } + @Override + public boolean hasFelixFramework() { + return jdiscOsgi.isFelixFramework(); + } + private static Bundle firstNonFrameworkBundle(List bundles) { for (Bundle b : bundles) { if (! (b instanceof Framework)) diff --git a/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java b/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java new file mode 100644 index 00000000000..ee71e8a8f4c --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java @@ -0,0 +1,106 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.core.config; + +import com.yahoo.config.FileReference; +import org.junit.Before; +import org.junit.Test; +import org.osgi.framework.Bundle; + +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * @author gjoranv + */ +public class ApplicationBundleLoaderTest { + + private static final FileReference BUNDLE_1_REF = new FileReference("bundle-1"); + private static final Bundle BUNDLE_1 = new TestBundle(BUNDLE_1_REF.value()); + private static final FileReference BUNDLE_2_REF = new FileReference("bundle-2"); + private static final Bundle BUNDLE_2 = new TestBundle(BUNDLE_2_REF.value()); + + private ApplicationBundleLoader bundleLoader; + private TestOsgi osgi; + + @Before + public void setup() { + osgi = new TestOsgi(testBundles()); + var bundleInstaller = new TestBundleInstaller(); + bundleLoader = new ApplicationBundleLoader(osgi); + bundleLoader.useCustomBundleInstaller(bundleInstaller); + } + + @Test + public void bundles_are_installed_and_started() { + bundleLoader.useBundles(List.of(BUNDLE_1_REF)); + assertEquals(1, osgi.getInstalledBundles().size()); + + // The bundle is installed and started + TestBundle installedBundle = (TestBundle)osgi.getInstalledBundles().get(0); + assertEquals(BUNDLE_1.getSymbolicName(), installedBundle.getSymbolicName()); + assertTrue(installedBundle.started); + + // The file reference is active + assertEquals(1, bundleLoader.getActiveFileReferences().size()); + assertEquals(BUNDLE_1_REF, bundleLoader.getActiveFileReferences().get(0)); + } + + @Test + public void new_bundle_can_be_installed_in_reconfig() { + bundleLoader.useBundles(List.of(BUNDLE_1_REF)); + Set obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_1_REF, BUNDLE_2_REF)); + + // No bundles are obsolete + assertTrue(obsoleteBundles.isEmpty()); + + // Both bundles are installed + assertEquals(2, osgi.getInstalledBundles().size()); + assertEquals(BUNDLE_1.getSymbolicName(), osgi.getInstalledBundles().get(0).getSymbolicName()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getInstalledBundles().get(1).getSymbolicName()); + + // Both bundles are current + assertEquals(2, osgi.getCurrentBundles().size()); + assertEquals(BUNDLE_1.getSymbolicName(), osgi.getCurrentBundles().get(0).getSymbolicName()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getCurrentBundles().get(1).getSymbolicName()); + + + // Both file references are active + assertEquals(2, bundleLoader.getActiveFileReferences().size()); + assertEquals(BUNDLE_1_REF, bundleLoader.getActiveFileReferences().get(0)); + assertEquals(BUNDLE_2_REF, bundleLoader.getActiveFileReferences().get(1)); + } + + @Test + public void unused_bundle_is_marked_obsolete_after_reconfig() { + bundleLoader.useBundles(List.of(BUNDLE_1_REF)); + Set obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_2_REF)); + + // The returned set of obsolete bundles contains bundle-1 + assertEquals(1, obsoleteBundles.size()); + assertEquals(BUNDLE_1.getSymbolicName(), obsoleteBundles.iterator().next().getSymbolicName()); + + // Both bundles are installed + assertEquals(2, osgi.getInstalledBundles().size()); + assertEquals(BUNDLE_1.getSymbolicName(), osgi.getInstalledBundles().get(0).getSymbolicName()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getInstalledBundles().get(1).getSymbolicName()); + + // Only bundle-2 is current + assertEquals(1, osgi.getCurrentBundles().size()); + assertEquals(BUNDLE_2.getSymbolicName(), osgi.getCurrentBundles().get(0).getSymbolicName()); + + // Only the bundle-2 file reference is active + assertEquals(1, bundleLoader.getActiveFileReferences().size()); + assertEquals(BUNDLE_2_REF, bundleLoader.getActiveFileReferences().get(0)); + } + + + private static Map testBundles() { + return Map.of(BUNDLE_1_REF.value(), BUNDLE_1, + BUNDLE_2_REF.value(), BUNDLE_2); + } + +} diff --git a/container-core/src/test/java/com/yahoo/container/core/config/BundleManagerTest.java b/container-core/src/test/java/com/yahoo/container/core/config/BundleManagerTest.java deleted file mode 100644 index 414e6b05128..00000000000 --- a/container-core/src/test/java/com/yahoo/container/core/config/BundleManagerTest.java +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.container.core.config; - -import com.yahoo.config.FileReference; -import org.junit.Before; -import org.junit.Test; -import org.osgi.framework.Bundle; - -import java.util.List; -import java.util.Map; -import java.util.Set; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -/** - * @author gjoranv - */ -public class BundleManagerTest { - - private static final FileReference BUNDLE_1_REF = new FileReference("bundle-1"); - private static final Bundle BUNDLE_1 = new TestBundle(BUNDLE_1_REF.value()); - private static final FileReference BUNDLE_2_REF = new FileReference("bundle-2"); - private static final Bundle BUNDLE_2 = new TestBundle(BUNDLE_2_REF.value()); - - private BundleManager bundleLoader; - private TestOsgi osgi; - - @Before - public void setup() { - osgi = new TestOsgi(testBundles()); - var bundleInstaller = new TestBundleInstaller(); - bundleLoader = new BundleManager(osgi); - bundleLoader.useCustomBundleInstaller(bundleInstaller); - } - - @Test - public void bundles_are_installed_and_started() { - bundleLoader.use(List.of(BUNDLE_1_REF)); - assertEquals(1, osgi.getInstalledBundles().size()); - - // The bundle is installed and started - TestBundle installedBundle = (TestBundle)osgi.getInstalledBundles().get(0); - assertEquals(BUNDLE_1.getSymbolicName(), installedBundle.getSymbolicName()); - assertTrue(installedBundle.started); - - // The file reference is active - assertEquals(1, bundleLoader.getActiveFileReferences().size()); - assertEquals(BUNDLE_1_REF, bundleLoader.getActiveFileReferences().get(0)); - } - - @Test - public void new_bundle_can_be_installed_in_reconfig() { - bundleLoader.use(List.of(BUNDLE_1_REF)); - Set obsoleteBundles = bundleLoader.use(List.of(BUNDLE_1_REF, BUNDLE_2_REF)); - - // No bundles are obsolete - assertTrue(obsoleteBundles.isEmpty()); - - // Both bundles are installed - assertEquals(2, osgi.getInstalledBundles().size()); - assertEquals(BUNDLE_1.getSymbolicName(), osgi.getInstalledBundles().get(0).getSymbolicName()); - assertEquals(BUNDLE_2.getSymbolicName(), osgi.getInstalledBundles().get(1).getSymbolicName()); - - // Both bundles are current - assertEquals(2, osgi.getCurrentBundles().size()); - assertEquals(BUNDLE_1.getSymbolicName(), osgi.getCurrentBundles().get(0).getSymbolicName()); - assertEquals(BUNDLE_2.getSymbolicName(), osgi.getCurrentBundles().get(1).getSymbolicName()); - - - // Both file references are active - assertEquals(2, bundleLoader.getActiveFileReferences().size()); - assertEquals(BUNDLE_1_REF, bundleLoader.getActiveFileReferences().get(0)); - assertEquals(BUNDLE_2_REF, bundleLoader.getActiveFileReferences().get(1)); - } - - @Test - public void unused_bundle_is_marked_obsolete_after_reconfig() { - bundleLoader.use(List.of(BUNDLE_1_REF)); - Set obsoleteBundles = bundleLoader.use(List.of(BUNDLE_2_REF)); - - // The returned set of obsolete bundles contains bundle-1 - assertEquals(1, obsoleteBundles.size()); - assertEquals(BUNDLE_1.getSymbolicName(), obsoleteBundles.iterator().next().getSymbolicName()); - - // Both bundles are installed - assertEquals(2, osgi.getInstalledBundles().size()); - assertEquals(BUNDLE_1.getSymbolicName(), osgi.getInstalledBundles().get(0).getSymbolicName()); - assertEquals(BUNDLE_2.getSymbolicName(), osgi.getInstalledBundles().get(1).getSymbolicName()); - - // Only bundle-2 is current - assertEquals(1, osgi.getCurrentBundles().size()); - assertEquals(BUNDLE_2.getSymbolicName(), osgi.getCurrentBundles().get(0).getSymbolicName()); - - // Only the bundle-2 file reference is active - assertEquals(1, bundleLoader.getActiveFileReferences().size()); - assertEquals(BUNDLE_2_REF, bundleLoader.getActiveFileReferences().get(0)); - } - - - private static Map testBundles() { - return Map.of(BUNDLE_1_REF.value(), BUNDLE_1, - BUNDLE_2_REF.value(), BUNDLE_2); - } - -} diff --git a/container-di/CMakeLists.txt b/container-di/CMakeLists.txt index c2b033baa92..02b2b0d34d9 100644 --- a/container-di/CMakeLists.txt +++ b/container-di/CMakeLists.txt @@ -1,5 +1,7 @@ # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. install_config_definition(src/main/resources/configdefinitions/bundles.def container.bundles.def) +install_config_definition(src/main/resources/configdefinitions/application-bundles.def com.yahoo.container.di.config.application-bundles.def) +install_config_definition(src/main/resources/configdefinitions/platform-bundles.def com.yahoo.container.di.config.platform-bundles.def) install_config_definition(src/main/resources/configdefinitions/components.def container.components.def) install_config_definition(src/main/resources/configdefinitions/jersey-bundles.def container.di.config.jersey-bundles.def) install_config_definition(src/main/resources/configdefinitions/jersey-injection.def container.di.config.jersey-injection.def) diff --git a/container-di/src/main/java/com/yahoo/container/di/Container.java b/container-di/src/main/java/com/yahoo/container/di/Container.java index ef7813ce368..672cef22010 100644 --- a/container-di/src/main/java/com/yahoo/container/di/Container.java +++ b/container-di/src/main/java/com/yahoo/container/di/Container.java @@ -4,8 +4,8 @@ package com.yahoo.container.di; import com.google.inject.Injector; import com.yahoo.config.ConfigInstance; import com.yahoo.config.ConfigurationRuntimeException; +import com.yahoo.config.FileReference; import com.yahoo.config.subscription.ConfigInterruptedException; -import com.yahoo.container.BundlesConfig; import com.yahoo.container.ComponentsConfig; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.container.di.ConfigRetriever.BootstrapConfigs; @@ -14,6 +14,8 @@ import com.yahoo.container.di.componentgraph.core.ComponentGraph; import com.yahoo.container.di.componentgraph.core.ComponentNode; import com.yahoo.container.di.componentgraph.core.JerseyNode; import com.yahoo.container.di.componentgraph.core.Node; +import com.yahoo.container.di.config.ApplicationBundlesConfig; +import com.yahoo.container.di.config.PlatformBundlesConfig; import com.yahoo.container.di.config.RestApiContext; import com.yahoo.container.di.config.SubscriberFactory; import com.yahoo.vespa.config.ConfigKey; @@ -23,6 +25,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.IdentityHashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.logging.Level; @@ -40,12 +43,14 @@ public class Container { private static final Logger log = Logger.getLogger(Container.class.getName()); private final SubscriberFactory subscriberFactory; - private ConfigKey bundlesConfigKey; + private ConfigKey applicationBundlesConfigKey; + private ConfigKey platformBundlesConfigKey; private ConfigKey componentsConfigKey; private final ComponentDeconstructor componentDeconstructor; private final Osgi osgi; private final ConfigRetriever configurer; + private List platformBundles; // Used to verify that platform bundles don't change. private long previousConfigGeneration = -1L; private long leastGeneration = -1L; @@ -54,9 +59,10 @@ public class Container { this.componentDeconstructor = componentDeconstructor; this.osgi = osgi; - bundlesConfigKey = new ConfigKey<>(BundlesConfig.class, configId); + applicationBundlesConfigKey = new ConfigKey<>(ApplicationBundlesConfig.class, configId); + platformBundlesConfigKey = new ConfigKey<>(PlatformBundlesConfig.class, configId); componentsConfigKey = new ConfigKey<>(ComponentsConfig.class, configId); - var bootstrapKeys = Set.of(bundlesConfigKey, componentsConfigKey); + var bootstrapKeys = Set.of(applicationBundlesConfigKey, platformBundlesConfigKey, componentsConfigKey); this.configurer = new ConfigRetriever(bootstrapKeys, subscriberFactory::getSubscriber); } @@ -74,7 +80,6 @@ public class Container { deconstructObsoleteComponents(oldGraph, newGraph, obsoleteBundles); return newGraph; } catch (Throwable t) { - // TODO: Wrap ComponentConstructorException in an Error when generation==0 (+ unit test that Error is thrown) invalidateGeneration(oldGraph.generation(), t); throw t; } @@ -98,13 +103,15 @@ public class Container { "Got bootstrap configs out of sequence for old config generation %d.\n" + "Previous config generation is %d", getBootstrapGeneration(), previousConfigGeneration)); } - log.log(FINE, - String.format( - "Got new bootstrap generation\n" + "bootstrap generation = %d\n" + "components generation: %d\n" - + "previous generation: %d\n", - getBootstrapGeneration(), getComponentsGeneration(), previousConfigGeneration)); + log.log(FINE, "Got new bootstrap generation\n" + configGenerationsString()); - Collection bundlesToRemove = installBundles(snapshot.configs()); + if (graph.generation() == 0) { + platformBundles = getConfig(platformBundlesConfigKey, snapshot.configs()).bundles(); + osgi.installPlatformBundles(platformBundles); + } else { + throwIfPlatformBundlesChanged(snapshot); + } + Collection bundlesToRemove = installApplicationBundles(snapshot.configs()); obsoleteBundles.addAll(bundlesToRemove); graph = createComponentsGraph(snapshot.configs(), getBootstrapGeneration(), fallbackInjector); @@ -115,11 +122,7 @@ public class Container { break; } } - log.log(FINE, - String.format( - "Got components configs,\n" + "bootstrap generation = %d\n" + "components generation: %d\n" - + "previous generation: %d", - getBootstrapGeneration(), getComponentsGeneration(), previousConfigGeneration)); + log.log(FINE, "Got components configs,\n" + configGenerationsString()); return createAndConfigureComponentsGraph(snapshot.configs(), fallbackInjector); } @@ -131,6 +134,17 @@ public class Container { return configurer.getComponentsGeneration(); } + private String configGenerationsString() { + return String.format("bootstrap generation = %d\n" + "components generation: %d\n" + "previous generation: %d", + getBootstrapGeneration(), getComponentsGeneration(), previousConfigGeneration); + } + + private void throwIfPlatformBundlesChanged(ConfigSnapshot snapshot) { + var checkPlatformBundles = getConfig(platformBundlesConfigKey, snapshot.configs()).bundles(); + if (! checkPlatformBundles.equals(platformBundles)) + throw new RuntimeException("Platform bundles are not allowed to change!\nOld: " + platformBundles + "\nNew: " + checkPlatformBundles); + } + private ComponentGraph createAndConfigureComponentsGraph(Map, ConfigInstance> componentsConfigs, Injector fallbackInjector) { ComponentGraph componentGraph = createComponentsGraph(componentsConfigs, getComponentsGeneration(), fallbackInjector); @@ -151,9 +165,9 @@ public class Container { componentDeconstructor.deconstruct(oldComponents.keySet(), obsoleteBundles); } - private Set installBundles(Map, ConfigInstance> configsIncludingBootstrapConfigs) { - BundlesConfig bundlesConfig = getConfig(bundlesConfigKey, configsIncludingBootstrapConfigs); - return osgi.useBundles(bundlesConfig.bundle()); + private Set installApplicationBundles(Map, ConfigInstance> configsIncludingBootstrapConfigs) { + ApplicationBundlesConfig applicationBundlesConfig = getConfig(applicationBundlesConfigKey, configsIncludingBootstrapConfigs); + return osgi.useApplicationBundles(applicationBundlesConfig.bundles()); } private ComponentGraph createComponentsGraph(Map, ConfigInstance> configsIncludingBootstrapConfigs, diff --git a/container-di/src/main/java/com/yahoo/container/di/Osgi.java b/container-di/src/main/java/com/yahoo/container/di/Osgi.java index ab7da7665b6..c9ca256b5e0 100644 --- a/container-di/src/main/java/com/yahoo/container/di/Osgi.java +++ b/container-di/src/main/java/com/yahoo/container/di/Osgi.java @@ -25,11 +25,15 @@ public interface Osgi { return new BundleClasses(new MockBundle(), Collections.emptySet()); } + default void installPlatformBundles(Collection bundles) { + System.out.println("installPlatformBundles " + bundles.stream().map(Object::toString).collect(Collectors.joining(", "))); + } + /** * Returns the set of bundles that is not used by the current application generation, * and therefore should be scheduled for uninstalling. */ - default Set useBundles(Collection bundles) { + default Set useApplicationBundles(Collection bundles) { System.out.println("useBundles " + bundles.stream().map(Object::toString).collect(Collectors.joining(", "))); return emptySet(); } diff --git a/container-di/src/main/resources/configdefinitions/application-bundles.def b/container-di/src/main/resources/configdefinitions/application-bundles.def new file mode 100644 index 00000000000..7e03b1e3ac8 --- /dev/null +++ b/container-di/src/main/resources/configdefinitions/application-bundles.def @@ -0,0 +1,5 @@ +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package=com.yahoo.container.di.config + +# References to user bundles to install. +bundles[] file diff --git a/container-di/src/main/resources/configdefinitions/bundles.def b/container-di/src/main/resources/configdefinitions/bundles.def index 9e10d863106..79e24742398 100644 --- a/container-di/src/main/resources/configdefinitions/bundles.def +++ b/container-di/src/main/resources/configdefinitions/bundles.def @@ -1,5 +1,5 @@ # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. namespace=container -# References to all 3rd-party bundles to be installed. +# References to both application and platform bundles to install. bundle[] file diff --git a/container-di/src/main/resources/configdefinitions/platform-bundles.def b/container-di/src/main/resources/configdefinitions/platform-bundles.def new file mode 100644 index 00000000000..9b72bf6831e --- /dev/null +++ b/container-di/src/main/resources/configdefinitions/platform-bundles.def @@ -0,0 +1,5 @@ +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package=com.yahoo.container.di.config + +# References to platform bundles to install. +bundles[] file diff --git a/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java b/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java index eac64c20274..19f277ff8fb 100644 --- a/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java +++ b/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java @@ -267,7 +267,8 @@ public class ContainerTest extends ContainerTestBase { "inject[0].forClass \"" + injectedClass.getName() + "\"\n"; dirConfigSource.writeConfig("components", componentsConfig); - dirConfigSource.writeConfig("bundles", ""); + dirConfigSource.writeConfig("platform-bundles", ""); + dirConfigSource.writeConfig("application-bundles", ""); dirConfigSource.writeConfig("jersey-bundles", "bundles[0].spec \"mock-entry-to-enforce-a-MockBundle\""); dirConfigSource.writeConfig("jersey-injection", injectionConfig); diff --git a/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java b/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java index 18236a6bde9..f1f3c4a2ae4 100644 --- a/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java +++ b/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java @@ -61,7 +61,7 @@ public class ContainerTestBase { } @Override - public Set useBundles(Collection bundles) { + public Set useApplicationBundles(Collection bundles) { return emptySet(); } @@ -81,7 +81,8 @@ public class ContainerTestBase { } protected void writeBootstrapConfigs(ComponentEntry... componentEntries) { - dirConfigSource.writeConfig("bundles", ""); + dirConfigSource.writeConfig("platform-bundles", ""); + dirConfigSource.writeConfig("application-bundles", ""); StringBuilder components = new StringBuilder(); for (int i = 0; i < componentEntries.length; i++) { components.append(componentEntries[i].asConfig(i)); diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java b/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java index b1aceb81bc6..12168663205 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/application/OsgiFramework.java @@ -113,4 +113,11 @@ public interface OsgiFramework { */ void stop() throws BundleException; + /** + * Returns true if this is a Felix based framework and not e.g. a test framework. + */ + default boolean isFelixFramework() { + return false; + } + } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java index c14e513fb98..bd189f8b898 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/FelixFramework.java @@ -169,6 +169,11 @@ public class FelixFramework implements OsgiFramework { collisionHook.allowDuplicateBundles(bundles); } + @Override + public boolean isFelixFramework() { + return true; + } + private void installBundle(String bundleLocation, Set mask, List out) throws BundleException { bundleLocation = BundleLocationResolver.resolve(bundleLocation); if (mask.contains(bundleLocation)) { diff --git a/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneSubscriberTest.java b/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneSubscriberTest.java index dd755e8e6dd..9ba1a56083a 100644 --- a/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneSubscriberTest.java +++ b/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneSubscriberTest.java @@ -2,8 +2,9 @@ package com.yahoo.container.standalone; import com.yahoo.config.ConfigInstance; -import com.yahoo.container.BundlesConfig; import com.yahoo.container.ComponentsConfig; +import com.yahoo.container.di.config.ApplicationBundlesConfig; +import com.yahoo.container.di.config.PlatformBundlesConfig; import com.yahoo.container.di.config.Subscriber; import com.yahoo.vespa.config.ConfigKey; import org.junit.Ignore; @@ -23,7 +24,8 @@ import static org.junit.Assert.assertThat; * @author ollivir */ public class StandaloneSubscriberTest { - private static ConfigKey bundlesKey = key("bundles"); + private static ConfigKey platformBundlesKey = key("platform-bundles"); + private static ConfigKey applicationBundlesKey = key("application-bundles"); private static ConfigKey componentsKey = key("components"); private static ConfigKey key(String name) { @@ -35,16 +37,19 @@ public class StandaloneSubscriberTest { public void standalone_subscriber() throws Exception { withContainerModel("", root -> { Set> keys = new HashSet<>(); - keys.add(bundlesKey); + keys.add(platformBundlesKey); + keys.add(applicationBundlesKey); keys.add(componentsKey); Subscriber subscriber = new StandaloneSubscriberFactory(root).getSubscriber(keys); Map, ConfigInstance> config = subscriber.config(); assertThat(config.size(), is(2)); - BundlesConfig bundlesConfig = (BundlesConfig) config.get(bundlesKey); + PlatformBundlesConfig platformBundlesConfig = (PlatformBundlesConfig) config.get(platformBundlesKey); + ApplicationBundlesConfig applicationBundlesConfig = (ApplicationBundlesConfig) config.get(applicationBundlesKey); ComponentsConfig componentsConfig = (ComponentsConfig) config.get(componentsKey); - assertThat(bundlesConfig.bundle().size(), is(0)); + assertThat(platformBundlesConfig.bundles().size(), is(0)); + assertThat(applicationBundlesConfig.bundles().size(), is(0)); assertThat(componentsConfig.components().size(), greaterThan(10)); return null; }); diff --git a/vespajlib/src/main/java/com/yahoo/collections/PredicateSplit.java b/vespajlib/src/main/java/com/yahoo/collections/PredicateSplit.java deleted file mode 100644 index 1b3941df7bf..00000000000 --- a/vespajlib/src/main/java/com/yahoo/collections/PredicateSplit.java +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.collections; - -import java.util.List; -import java.util.ArrayList; -import java.util.function.Predicate; - -/** - * Class holding the result of a partition-by-predicate operation. - **/ -public class PredicateSplit { - public final List falseValues; /// list of values where the predicate returned false - public final List trueValues; /// list of values where the predicate returned true - - private PredicateSplit() { - falseValues = new ArrayList(); - trueValues = new ArrayList(); - } - - /** - * Perform a partition-by-predicate operation. - * Each value in the input is tested by the predicate and - * added to either the falseValues list or the trueValues list. - * @param collection The input collection. - * @param predicate A test for selecting the target list. - * @return Two lists bundled in an object. - **/ - public static PredicateSplit partition(Iterable collection, Predicate predicate) - { - PredicateSplit r = new PredicateSplit(); - for (V value : collection) { - if (predicate.test(value)) { - r.trueValues.add(value); - } else { - r.falseValues.add(value); - } - } - return r; - } -} diff --git a/vespajlib/src/test/java/com/yahoo/collections/PredicateSplitTestCase.java b/vespajlib/src/test/java/com/yahoo/collections/PredicateSplitTestCase.java deleted file mode 100644 index 05a719d6a4d..00000000000 --- a/vespajlib/src/test/java/com/yahoo/collections/PredicateSplitTestCase.java +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.collections; - -import org.junit.Test; - -import java.util.List; -import java.util.ArrayList; - -import static org.junit.Assert.assertEquals; - -public class PredicateSplitTestCase { - @Test - public void requireThatSplitWorks() { - List l = new ArrayList(); - l.add(1); - l.add(6); - l.add(2); - l.add(4); - l.add(5); - PredicateSplit result = PredicateSplit.partition(l, x -> (x % 2 == 0)); - assertEquals((long) result.falseValues.size(), 2L); - assertEquals((long) result.falseValues.get(0), 1L); - assertEquals((long) result.falseValues.get(1), 5L); - - assertEquals((long) result.trueValues.size(), 3L); - assertEquals((long) result.trueValues.get(0), 6L); - assertEquals((long) result.trueValues.get(1), 2L); - assertEquals((long) result.trueValues.get(2), 4L); - } -} -- cgit v1.2.3