diff options
59 files changed, 490 insertions, 1410 deletions
diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java index 892eb9aac05..31c8049b0dd 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java @@ -57,12 +57,12 @@ public class GenerateOsgiManifestMojo extends AbstractGenerateOsgiManifestMojo { private String mainClass = null; @Parameter(defaultValue = "false") - private boolean buildVespaPlatformBundle; + private boolean buildLegacyVespaPlatformBundle; public void execute() throws MojoExecutionException { try { - if (discPreInstallBundle != null && ! buildVespaPlatformBundle) - throw new MojoExecutionException("The 'discPreInstallBundle' parameter can only be used by Vespa platform bundles."); + if (discPreInstallBundle != null && !buildLegacyVespaPlatformBundle) + throw new MojoExecutionException("The 'discPreInstallBundle' parameter can only be used by legacy Vespa platform bundles."); Artifacts.ArtifactSet artifactSet = Artifacts.getArtifacts(project); warnOnUnsupportedArtifacts(artifactSet.getNonJarArtifacts()); 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<ApplicationContainer> implements - BundlesConfig.Producer, + ApplicationBundlesConfig.Producer, QrStartConfig.Producer, RankProfilesConfig.Producer, RankingConstantsConfig.Producer, @@ -188,10 +188,9 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat public Optional<Integer> 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<CONTAINER extends Container> ContainerDocumentConfig.Producer, HealthMonitorConfig.Producer, ApplicationMetadataConfig.Producer, - BundlesConfig.Producer, + PlatformBundlesConfig.Producer, IndexInfoConfig.Producer, IlscriptsConfig.Producer, SchemamappingConfig.Producer, @@ -464,6 +464,7 @@ public abstract class ContainerCluster<CONTAINER extends Container> /** * 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<CONTAINER extends Container> } @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<String> installedBundles = bundleBuilder.build().bundle().stream().map(FileReference::value).collect(Collectors.toList()); + List<String> 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()); @@ -207,11 +204,6 @@ public class DocprocBuilderTest extends DomBuilderTest { } @Test - public void testBundlesConfig() { - assertTrue(bundlesConfig.bundle().isEmpty()); - } - - @Test public void testSchemaMappingConfig() { assertTrue(schemamappingConfig.fieldmapping().isEmpty()); } diff --git a/configdefinitions/src/vespa/configserver.def b/configdefinitions/src/vespa/configserver.def index c09054325ee..cde539f25f4 100644 --- a/configdefinitions/src/vespa/configserver.def +++ b/configdefinitions/src/vespa/configserver.def @@ -53,7 +53,7 @@ ztsUrl string default="" maintainerIntervalMinutes int default=30 # TODO: Default set to a high value (1 year) => maintainer will not run, change when maintainer verified out in prod tenantsMaintainerIntervalMinutes int default=525600 -keepUnusedFileReferencesHours int default=6 +keepUnusedFileReferencesHours int default=4 # Bootstrapping # How long bootstrapping can take before giving up (in seconds) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java index 77da56588ba..72e8e08e38f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java @@ -32,7 +32,7 @@ public class SessionsMaintainer extends ConfigServerMaintainer { // Expired remote sessions are sessions that belong to an application that have external deployments that // are no longer active if (hostedVespa) { - Duration expiryTime = Duration.ofDays(1); + Duration expiryTime = Duration.ofHours(12); int deleted = applicationRepository.deleteExpiredRemoteSessions(expiryTime); log.log(LogLevel.FINE, "Deleted " + deleted + " expired remote sessions, expiry time " + expiryTime); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java index 0f6ba37cf51..00e4005e771 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java @@ -571,11 +571,18 @@ public class SessionRepository { throw new IllegalArgumentException(sourceDir.getAbsolutePath() + " is not a directory"); // Copy app atomically: Copy to a temp dir and move to destination - java.nio.file.Path tempDestinationDir = Files.createTempDirectory(destinationDir.getParentFile().toPath(), "app-package"); - log.log(Level.FINE, "Copying dir " + sourceDir.getAbsolutePath() + " to " + tempDestinationDir.toFile().getAbsolutePath()); - IOUtils.copyDirectory(sourceDir, tempDestinationDir.toFile()); - log.log(Level.FINE, "Moving " + tempDestinationDir + " to " + destinationDir.getAbsolutePath()); - Files.move(tempDestinationDir, destinationDir.toPath(), StandardCopyOption.ATOMIC_MOVE); + java.nio.file.Path tempDestinationDir = null; + try { + tempDestinationDir = Files.createTempDirectory(destinationDir.getParentFile().toPath(), "app-package"); + log.log(Level.FINE, "Copying dir " + sourceDir.getAbsolutePath() + " to " + tempDestinationDir.toFile().getAbsolutePath()); + IOUtils.copyDirectory(sourceDir, tempDestinationDir.toFile()); + log.log(Level.FINE, "Moving " + tempDestinationDir + " to " + destinationDir.getAbsolutePath()); + Files.move(tempDestinationDir, destinationDir.toPath(), StandardCopyOption.ATOMIC_MOVE); + } finally { + // In case some of the operations above fail + if (tempDestinationDir != null) + IOUtils.recursiveDeleteDir(tempDestinationDir.toFile()); + } } /** 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<FileReference, Bundle> 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<Bundle> useBundles(List<FileReference> newFileReferences) { + + Set<FileReference> obsoleteReferences = getObsoleteFileReferences(newFileReferences); + Set<Bundle> 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<FileReference> getObsoleteFileReferences(List<FileReference> newReferences) { + Set<FileReference> 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<Bundle> getObsoleteBundles(Set<FileReference> obsoleteReferences) { + return obsoleteReferences.stream().map(reference2Bundle::get).collect(Collectors.toSet()); + } + + private void removeInactiveFileReferences(Set<FileReference> fileReferencesToRemove) { + fileReferencesToRemove.forEach(reference2Bundle::remove); + } + + private void installBundles(List<FileReference> references) { + Set<FileReference> 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<FileReference> bundlesToInstall, BundleInstaller bundleInstaller) { + for (FileReference reference : bundlesToInstall) { + try { + log.info("Installing bundle with reference '" + reference.value() + "'"); + List<Bundle> 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<FileReference> 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<FileReference, List<Bundle>> 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<Bundle> use(List<FileReference> newFileReferences) { - // Must be done before allowing duplicates because allowed duplicates affect osgi.getCurrentBundles - Set<Bundle> bundlesToUninstall = getObsoleteBundles(newFileReferences); - - Set<FileReference> 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<Bundle> getObsoleteBundles(List<FileReference> newReferences) { - Set<Bundle> 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<FileReference> getObsoleteFileReferences(List<FileReference> newReferences) { - Set<FileReference> 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<FileReference> obsoleteReferences) { - // The bundle at index 0 for each file reference always corresponds to the bundle at the file reference location - Set<Bundle> 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<FileReference> fileReferencesToRemove) { - // Clean up the map of active bundles - fileReferencesToRemove.forEach(reference2Bundles::remove); - } - - private void installBundles(List<FileReference> references) { - Set<FileReference> 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<FileReference> 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<FileReference> bundlesToInstall) { - for (FileReference reference : bundlesToInstall) { - try { - installBundleFromDisk(reference); - } - catch(Exception e) { - throw new RuntimeException("Could not install bundle '" + reference + "'", e); - } - } - } - - private void installBundlesFromFileDistribution(List<FileReference> 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<Bundle> bundles = bundleInstaller.installBundles(reference, osgi); - reference2Bundles.put(reference, bundles); - } - - private void installWithFileDistribution(List<FileReference> bundlesToInstall, BundleInstaller bundleInstaller) { - for (FileReference reference : bundlesToInstall) { - try { - log.info("Installing bundle with reference '" + reference.value() + "'"); - List<Bundle> 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<Bundle> 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<Bundle> 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<FileReference> 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<Bundle> 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<Bundle> useBundles(Collection<FileReference> bundles) { + public void installPlatformBundles(Collection<FileReference> bundles) { + log.fine("Installing platform bundles."); + platformBundleLoader.useBundles(new ArrayList<>(bundles)); + } + + @Override + public Set<Bundle> useApplicationBundles(Collection<FileReference> 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<FileReference> fileReferences) { + Set<Bundle> installedBundles = install(fileReferences); + BundleStarter.startBundles(installedBundles); + } + + private Set<Bundle> install(List<FileReference> bundlesToInstall) { + var installedBundles = new LinkedHashSet<Bundle>(); + 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<Bundle> installBundleFromDisk(FileReference reference) { + log.info("Installing bundle from disk with reference '" + reference.value() + "'"); + List<Bundle> 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 @@ -16,11 +16,6 @@ import static java.util.Collections.emptyList; public class MockOsgiWrapper implements OsgiWrapper { @Override - public List<Bundle> 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,15 +12,11 @@ import java.util.List; /** * @author Tony Vaagenes + * @author gjoranv */ public class MockOsgi extends NonWorkingOsgiFramework implements Osgi { @Override - public List<Bundle> 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<Bundle> getInitialBundles(); - Bundle[] getBundles(); /** Returns all bundles that have not been scheduled for uninstall. */ @@ -25,4 +24,7 @@ public interface Osgi { void allowDuplicateBundles(Collection<Bundle> 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()); @@ -42,11 +43,6 @@ public class OsgiImpl implements Osgi { } @Override - public List<Bundle> getInitialBundles() { - return initialBundles; - } - - @Override public Bundle[] getBundles() { List<Bundle> bundles = jdiscOsgi.bundles(); return bundles.toArray(new Bundle[bundles.size()]); @@ -155,6 +151,11 @@ public class OsgiImpl implements Osgi { jdiscOsgi.allowDuplicateBundles(bundles); } + @Override + public boolean hasFelixFramework() { + return jdiscOsgi.isFelixFramework(); + } + private static Bundle firstNonFrameworkBundle(List<Bundle> bundles) { for (Bundle b : bundles) { if (! (b instanceof Framework)) 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/ApplicationBundleLoaderTest.java index 414e6b05128..ee71e8a8f4c 100644 --- a/container-core/src/test/java/com/yahoo/container/core/config/BundleManagerTest.java +++ b/container-core/src/test/java/com/yahoo/container/core/config/ApplicationBundleLoaderTest.java @@ -16,27 +16,27 @@ import static org.junit.Assert.assertTrue; /** * @author gjoranv */ -public class BundleManagerTest { +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 BundleManager bundleLoader; + private ApplicationBundleLoader bundleLoader; private TestOsgi osgi; @Before public void setup() { osgi = new TestOsgi(testBundles()); var bundleInstaller = new TestBundleInstaller(); - bundleLoader = new BundleManager(osgi); + bundleLoader = new ApplicationBundleLoader(osgi); bundleLoader.useCustomBundleInstaller(bundleInstaller); } @Test public void bundles_are_installed_and_started() { - bundleLoader.use(List.of(BUNDLE_1_REF)); + bundleLoader.useBundles(List.of(BUNDLE_1_REF)); assertEquals(1, osgi.getInstalledBundles().size()); // The bundle is installed and started @@ -51,8 +51,8 @@ public class BundleManagerTest { @Test public void new_bundle_can_be_installed_in_reconfig() { - bundleLoader.use(List.of(BUNDLE_1_REF)); - Set<Bundle> obsoleteBundles = bundleLoader.use(List.of(BUNDLE_1_REF, BUNDLE_2_REF)); + bundleLoader.useBundles(List.of(BUNDLE_1_REF)); + Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_1_REF, BUNDLE_2_REF)); // No bundles are obsolete assertTrue(obsoleteBundles.isEmpty()); @@ -76,8 +76,8 @@ public class BundleManagerTest { @Test public void unused_bundle_is_marked_obsolete_after_reconfig() { - bundleLoader.use(List.of(BUNDLE_1_REF)); - Set<Bundle> obsoleteBundles = bundleLoader.use(List.of(BUNDLE_2_REF)); + bundleLoader.useBundles(List.of(BUNDLE_1_REF)); + Set<Bundle> obsoleteBundles = bundleLoader.useBundles(List.of(BUNDLE_2_REF)); // The returned set of obsolete bundles contains bundle-1 assertEquals(1, obsoleteBundles.size()); 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<BundlesConfig> bundlesConfigKey; + private ConfigKey<ApplicationBundlesConfig> applicationBundlesConfigKey; + private ConfigKey<PlatformBundlesConfig> platformBundlesConfigKey; private ConfigKey<ComponentsConfig> componentsConfigKey; private final ComponentDeconstructor componentDeconstructor; private final Osgi osgi; private final ConfigRetriever configurer; + private List<FileReference> 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<Bundle> bundlesToRemove = installBundles(snapshot.configs()); + if (graph.generation() == 0) { + platformBundles = getConfig(platformBundlesConfigKey, snapshot.configs()).bundles(); + osgi.installPlatformBundles(platformBundles); + } else { + throwIfPlatformBundlesChanged(snapshot); + } + Collection<Bundle> 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<ConfigKey<? extends ConfigInstance>, ConfigInstance> componentsConfigs, Injector fallbackInjector) { ComponentGraph componentGraph = createComponentsGraph(componentsConfigs, getComponentsGeneration(), fallbackInjector); @@ -151,9 +165,9 @@ public class Container { componentDeconstructor.deconstruct(oldComponents.keySet(), obsoleteBundles); } - private Set<Bundle> installBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) { - BundlesConfig bundlesConfig = getConfig(bundlesConfigKey, configsIncludingBootstrapConfigs); - return osgi.useBundles(bundlesConfig.bundle()); + private Set<Bundle> installApplicationBundles(Map<ConfigKey<? extends ConfigInstance>, ConfigInstance> configsIncludingBootstrapConfigs) { + ApplicationBundlesConfig applicationBundlesConfig = getConfig(applicationBundlesConfigKey, configsIncludingBootstrapConfigs); + return osgi.useApplicationBundles(applicationBundlesConfig.bundles()); } private ComponentGraph createComponentsGraph(Map<ConfigKey<? extends ConfigInstance>, 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<FileReference> 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<Bundle> useBundles(Collection<FileReference> bundles) { + default Set<Bundle> useApplicationBundles(Collection<FileReference> 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<Bundle> useBundles(Collection<FileReference> bundles) { + public Set<Bundle> useApplicationBundles(Collection<FileReference> 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/container-disc/pom.xml b/container-disc/pom.xml index 48872d0665b..1caf66e29dc 100644 --- a/container-disc/pom.xml +++ b/container-disc/pom.xml @@ -181,7 +181,8 @@ <artifactId>bundle-plugin</artifactId> <extensions>true</extensions> <configuration> - <discApplicationClass>com.yahoo.container.jdisc.ConfiguredApplication</discApplicationClass> + <discApplicationClass>com.yahoo.container.jdisc.ConfiguredApplication</discApplicationClass> + <buildLegacyVespaPlatformBundle>true</buildLegacyVespaPlatformBundle> <discPreInstallBundle> <!-- Vespa bundles --> config-bundle-jar-with-dependencies.jar, diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java index 523f3533a7f..2a42bd9c235 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java @@ -7,9 +7,6 @@ import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.curator.Lock; -import com.yahoo.vespa.flags.BooleanFlag; -import com.yahoo.vespa.flags.FetchVector; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer; @@ -48,12 +45,10 @@ public class RoutingPolicies { private final Controller controller; private final CuratorDb db; - private final BooleanFlag weightedDnsPerRegion; public RoutingPolicies(Controller controller) { this.controller = Objects.requireNonNull(controller, "controller must be non-null"); this.db = controller.curator(); - this.weightedDnsPerRegion = Flags.WEIGHTED_DNS_PER_REGION.bindTo(controller.flagSource()); try (var lock = db.lockRoutingPolicies()) { // Update serialized format for (var policy : db.readRoutingPolicies().entrySet()) { db.writeRoutingPolicies(policy.getKey(), policy.getValue()); @@ -92,7 +87,7 @@ public class RoutingPolicies { removeGlobalDnsUnreferencedBy(allocation, lock); storePoliciesOf(allocation, lock); removePoliciesUnreferencedBy(allocation, lock); - updateGlobalDnsOf(application, get(allocation.deployment.applicationId()).values(), inactiveZones, lock); + updateGlobalDnsOf(get(allocation.deployment.applicationId()).values(), inactiveZones, lock); } } @@ -102,8 +97,8 @@ public class RoutingPolicies { db.writeZoneRoutingPolicy(new ZoneRoutingPolicy(zone, GlobalRouting.status(status, GlobalRouting.Agent.operator, controller.clock().instant()))); Map<ApplicationId, Map<RoutingPolicyId, RoutingPolicy>> allPolicies = db.readRoutingPolicies(); - for (var kv : allPolicies.entrySet()) { - updateGlobalDnsOf(kv.getKey(), kv.getValue().values(), Set.of(), lock); + for (var applicationPolicies : allPolicies.values()) { + updateGlobalDnsOf(applicationPolicies.values(), Set.of(), lock); } } } @@ -120,56 +115,7 @@ public class RoutingPolicies { newPolicies.put(policy.id(), newPolicy); } db.writeRoutingPolicies(deployment.applicationId(), newPolicies); - updateGlobalDnsOf(deployment.applicationId(), newPolicies.values(), Set.of(), lock); - } - } - - /** Update global DNS record for given policies */ - private void legacyUpdateGlobalDnsOf(Collection<RoutingPolicy> routingPolicies, Set<ZoneId> inactiveZones, @SuppressWarnings("unused") Lock lock) { - // Create DNS record for each routing ID - var routingTable = routingTableFrom(routingPolicies); - for (Map.Entry<RoutingId, List<RoutingPolicy>> routeEntry : routingTable.entrySet()) { - var targets = new LinkedHashSet<AliasTarget>(); - var staleTargets = new LinkedHashSet<AliasTarget>(); - for (var policy : routeEntry.getValue()) { - if (policy.dnsZone().isEmpty()) continue; - if (!controller.zoneRegistry().routingMethods(policy.id().zone()).contains(RoutingMethod.exclusive)) continue; - var target = new LatencyAliasTarget(policy.canonicalName(), policy.dnsZone().get(), policy.id().zone()); - var zonePolicy = db.readZoneRoutingPolicy(policy.id().zone()); - // Remove target zone if global routing status is set out at: - // - zone level (ZoneRoutingPolicy) - // - deployment level (RoutingPolicy) - // - application package level (deployment.xml) - if (isConfiguredOut(policy, zonePolicy, inactiveZones)) { - staleTargets.add(target); - } else { - targets.add(target); - } - } - // If all targets are configured out, all targets are set in. We do this because otherwise removing 100% of - // the ALIAS records would cause the global endpoint to stop resolving entirely (NXDOMAIN). - if (targets.isEmpty() && !staleTargets.isEmpty()) { - targets.addAll(staleTargets); - staleTargets.clear(); - } - if (!targets.isEmpty()) { - var endpoints = controller.routing().endpointsOf(routeEntry.getKey().application()) - .named(routeEntry.getKey().endpointId()) - .not().requiresRotation(); - endpoints.forEach(endpoint -> controller.nameServiceForwarder().createAlias(RecordName.from(endpoint.dnsName()), targets, Priority.normal)); - } - staleTargets.forEach(t -> controller.nameServiceForwarder().removeRecords(Record.Type.ALIAS, - RecordData.fqdn(t.name().value()), - Priority.normal)); - } - } - - // TODO(mpolden): Remove and inline call to updateGlobalDnsOf when feature flag disappears - private void updateGlobalDnsOf(ApplicationId application, Collection<RoutingPolicy> routingPolicies, Set<ZoneId> inactiveZones, @SuppressWarnings("unused") Lock lock) { - if (useWeightedDnsPerRegion(application)) { - updateGlobalDnsOf(routingPolicies, inactiveZones, lock); - } else { - legacyUpdateGlobalDnsOf(routingPolicies, inactiveZones, lock); + updateGlobalDnsOf(newPolicies.values(), Set.of(), lock); } } @@ -245,7 +191,7 @@ public class RoutingPolicies { var policyId = new RoutingPolicyId(loadBalancer.application(), loadBalancer.cluster(), allocation.deployment.zoneId()); var existingPolicy = policies.get(policyId); var newPolicy = new RoutingPolicy(policyId, loadBalancer.hostname(), loadBalancer.dnsZone(), - allocation.endpointIdsOf(loadBalancer, useWeightedDnsPerRegion(loadBalancer.application())), + allocation.endpointIdsOf(loadBalancer), new Status(isActive(loadBalancer), GlobalRouting.DEFAULT_STATUS)); // Preserve global routing status for existing policy if (existingPolicy != null) { @@ -301,10 +247,10 @@ public class RoutingPolicies { } /** Compute routing IDs from given load balancers */ - private Set<RoutingId> routingIdsFrom(LoadBalancerAllocation allocation) { + private static Set<RoutingId> routingIdsFrom(LoadBalancerAllocation allocation) { Set<RoutingId> routingIds = new LinkedHashSet<>(); for (var loadBalancer : allocation.loadBalancers) { - for (var endpointId : allocation.endpointIdsOf(loadBalancer, useWeightedDnsPerRegion(loadBalancer.application()))) { + for (var endpointId : allocation.endpointIdsOf(loadBalancer)) { routingIds.add(new RoutingId(loadBalancer.application(), endpointId)); } } @@ -344,10 +290,6 @@ public class RoutingPolicies { return false; } - private boolean useWeightedDnsPerRegion(ApplicationId application) { - return weightedDnsPerRegion.with(FetchVector.Dimension.APPLICATION_ID, application.serializedForm()).value(); - } - /** Represents records for a region-wide endpoint */ private static class RegionEndpoint { @@ -409,7 +351,7 @@ public class RoutingPolicies { } /** Compute all endpoint IDs for given load balancer */ - private Set<EndpointId> endpointIdsOf(LoadBalancer loadBalancer, boolean useWeightedDnsPerRegion) { + private Set<EndpointId> endpointIdsOf(LoadBalancer loadBalancer) { if (!deployment.zoneId().environment().isProduction()) { // Only production deployments have configurable endpoints return Set.of(); } @@ -417,7 +359,7 @@ public class RoutingPolicies { if (instanceSpec.isEmpty()) { return Set.of(); } - if (useWeightedDnsPerRegion && instanceSpec.get().globalServiceId().filter(id -> id.equals(loadBalancer.cluster().value())).isPresent()) { + if (instanceSpec.get().globalServiceId().filter(id -> id.equals(loadBalancer.cluster().value())).isPresent()) { // Legacy assignment always has the default endpoint Id return Set.of(EndpointId.defaultId()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesLegacyTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesLegacyTest.java deleted file mode 100644 index 7ade8f31224..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesLegacyTest.java +++ /dev/null @@ -1,714 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.routing; - -import com.google.common.collect.Sets; -import com.yahoo.config.application.api.DeploymentSpec; -import com.yahoo.config.application.api.ValidationId; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.AthenzDomain; -import com.yahoo.config.provision.AthenzService; -import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.RegionName; -import com.yahoo.config.provision.SystemName; -import com.yahoo.config.provision.zone.RoutingMethod; -import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.InMemoryFlagSource; -import com.yahoo.vespa.hosted.controller.ControllerTester; -import com.yahoo.vespa.hosted.controller.Instance; -import com.yahoo.vespa.hosted.controller.RoutingController; -import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; -import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; -import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; -import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData; -import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; -import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.Endpoint; -import com.yahoo.vespa.hosted.controller.application.EndpointId; -import com.yahoo.vespa.hosted.controller.application.SystemApplication; -import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; -import com.yahoo.vespa.hosted.controller.integration.ServiceRegistryMock; -import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; -import com.yahoo.vespa.hosted.controller.maintenance.NameServiceDispatcher; -import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; -import org.junit.Test; - -import java.time.Duration; -import java.time.Instant; -import java.time.temporal.ChronoUnit; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; - -/** - * @author mortent - * @author mpolden - */ -// TODO(mpolden): Remove when weighted-dns-per-region flag is removed -public class RoutingPoliciesLegacyTest { - - private final ZoneId zone1 = ZoneId.from("prod", "us-west-1"); - private final ZoneId zone2 = ZoneId.from("prod", "us-central-1"); - private final ZoneId zone3 = ZoneId.from("prod", "us-east-3"); - - private final ApplicationPackage applicationPackage = applicationPackageBuilder().region(zone1.region()) - .region(zone2.region()) - .build(); - - @Test - public void global_routing_policies() { - var tester = new RoutingPoliciesTester(); - var context1 = tester.newDeploymentContext("tenant1", "app1", "default"); - var context2 = tester.newDeploymentContext("tenant1", "app2", "default"); - int clustersPerZone = 2; - int numberOfDeployments = 2; - var applicationPackage = applicationPackageBuilder() - .region(zone1.region()) - .region(zone2.region()) - .endpoint("r0", "c0") - .endpoint("r1", "c0", "us-west-1") - .endpoint("r2", "c1") - .build(); - tester.provisionLoadBalancers(clustersPerZone, context1.instanceId(), zone1, zone2); - - // Creates alias records - context1.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - tester.assertTargets(context1.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); - tester.assertTargets(context1.instanceId(), EndpointId.of("r1"), 0, zone1); - tester.assertTargets(context1.instanceId(), EndpointId.of("r2"), 1, zone1, zone2); - assertEquals("Routing policy count is equal to cluster count", - numberOfDeployments * clustersPerZone, - tester.policiesOf(context1.instance().id()).size()); - - // Applications gains a new deployment - ApplicationPackage applicationPackage2 = applicationPackageBuilder() - .region(zone1.region()) - .region(zone2.region()) - .region(zone3.region()) - .endpoint("r0", "c0") - .endpoint("r1", "c0", "us-west-1") - .endpoint("r2", "c1") - .build(); - numberOfDeployments++; - tester.provisionLoadBalancers(clustersPerZone, context1.instanceId(), zone3); - context1.submit(applicationPackage2).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - - // Endpoints are updated to contain cluster in new deployment - tester.assertTargets(context1.instanceId(), EndpointId.of("r0"), 0, zone1, zone2, zone3); - tester.assertTargets(context1.instanceId(), EndpointId.of("r1"), 0, zone1); - tester.assertTargets(context1.instanceId(), EndpointId.of("r2"), 1, zone1, zone2, zone3); - - // Another application is deployed with a single cluster and global endpoint - var endpoint4 = "r0.app2.tenant1.global.vespa.oath.cloud"; - tester.provisionLoadBalancers(1, context2.instanceId(), zone1, zone2); - var applicationPackage3 = applicationPackageBuilder() - .region(zone1.region()) - .region(zone2.region()) - .endpoint("r0", "c0") - .build(); - context2.submit(applicationPackage3).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - tester.assertTargets(context2.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); - - // All endpoints for app1 are removed - ApplicationPackage applicationPackage4 = applicationPackageBuilder() - .region(zone1.region()) - .region(zone2.region()) - .region(zone3.region()) - .allow(ValidationId.globalEndpointChange) - .build(); - context1.submit(applicationPackage4).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - tester.assertTargets(context1.instanceId(), EndpointId.of("r0"), 0); - tester.assertTargets(context1.instanceId(), EndpointId.of("r1"), 0); - tester.assertTargets(context1.instanceId(), EndpointId.of("r2"), 0); - var policies = tester.policiesOf(context1.instanceId()); - assertEquals(clustersPerZone * numberOfDeployments, policies.size()); - assertTrue("Rotation membership is removed from all policies", - policies.stream().allMatch(policy -> policy.endpoints().isEmpty())); - assertEquals("Rotations for " + context2.application() + " are not removed", 2, tester.aliasDataOf(endpoint4).size()); - } - - @Test - public void zone_routing_policies() { - zone_routing_policies(false); - zone_routing_policies(true); - } - - private void zone_routing_policies(boolean sharedRoutingLayer) { - var tester = new RoutingPoliciesTester(); - var context1 = tester.newDeploymentContext("tenant1", "app1", "default"); - var context2 = tester.newDeploymentContext("tenant1", "app2", "default"); - - // Deploy application - int clustersPerZone = 2; - tester.provisionLoadBalancers(clustersPerZone, context1.instanceId(), sharedRoutingLayer, zone1, zone2); - context1.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - - // Deployment creates records and policies for all clusters in all zones - Set<String> expectedRecords = Set.of( - "c0.app1.tenant1.us-west-1.vespa.oath.cloud", - "c1.app1.tenant1.us-west-1.vespa.oath.cloud", - "c0.app1.tenant1.us-central-1.vespa.oath.cloud", - "c1.app1.tenant1.us-central-1.vespa.oath.cloud" - ); - assertEquals(expectedRecords, tester.recordNames()); - assertEquals(4, tester.policiesOf(context1.instanceId()).size()); - - // Next deploy does nothing - context1.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - assertEquals(expectedRecords, tester.recordNames()); - assertEquals(4, tester.policiesOf(context1.instanceId()).size()); - - // Add 1 cluster in each zone and deploy - tester.provisionLoadBalancers(clustersPerZone + 1, context1.instanceId(), sharedRoutingLayer, zone1, zone2); - context1.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - expectedRecords = Set.of( - "c0.app1.tenant1.us-west-1.vespa.oath.cloud", - "c1.app1.tenant1.us-west-1.vespa.oath.cloud", - "c2.app1.tenant1.us-west-1.vespa.oath.cloud", - "c0.app1.tenant1.us-central-1.vespa.oath.cloud", - "c1.app1.tenant1.us-central-1.vespa.oath.cloud", - "c2.app1.tenant1.us-central-1.vespa.oath.cloud" - ); - assertEquals(expectedRecords, tester.recordNames()); - assertEquals(6, tester.policiesOf(context1.instanceId()).size()); - - // Deploy another application - tester.provisionLoadBalancers(clustersPerZone, context2.instanceId(), sharedRoutingLayer, zone1, zone2); - context2.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - expectedRecords = Set.of( - "c0.app1.tenant1.us-west-1.vespa.oath.cloud", - "c1.app1.tenant1.us-west-1.vespa.oath.cloud", - "c2.app1.tenant1.us-west-1.vespa.oath.cloud", - "c0.app1.tenant1.us-central-1.vespa.oath.cloud", - "c1.app1.tenant1.us-central-1.vespa.oath.cloud", - "c2.app1.tenant1.us-central-1.vespa.oath.cloud", - "c0.app2.tenant1.us-central-1.vespa.oath.cloud", - "c1.app2.tenant1.us-central-1.vespa.oath.cloud", - "c0.app2.tenant1.us-west-1.vespa.oath.cloud", - "c1.app2.tenant1.us-west-1.vespa.oath.cloud" - ); - assertEquals(expectedRecords.stream().sorted().collect(Collectors.toList()), tester.recordNames().stream().sorted().collect(Collectors.toList())); - assertEquals(4, tester.policiesOf(context2.instanceId()).size()); - - // Deploy removes cluster from app1 - tester.provisionLoadBalancers(clustersPerZone, context1.instanceId(), sharedRoutingLayer, zone1, zone2); - context1.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - expectedRecords = Set.of( - "c0.app1.tenant1.us-west-1.vespa.oath.cloud", - "c1.app1.tenant1.us-west-1.vespa.oath.cloud", - "c0.app1.tenant1.us-central-1.vespa.oath.cloud", - "c1.app1.tenant1.us-central-1.vespa.oath.cloud", - "c0.app2.tenant1.us-central-1.vespa.oath.cloud", - "c1.app2.tenant1.us-central-1.vespa.oath.cloud", - "c0.app2.tenant1.us-west-1.vespa.oath.cloud", - "c1.app2.tenant1.us-west-1.vespa.oath.cloud" - ); - assertEquals(expectedRecords, tester.recordNames()); - - // Remove app2 completely - tester.controllerTester().controller().applications().requireInstance(context2.instanceId()).deployments().keySet() - .forEach(zone -> { - tester.controllerTester().configServer().removeLoadBalancers(context2.instanceId(), zone); - tester.controllerTester().controller().applications().deactivate(context2.instanceId(), zone); - }); - context2.flushDnsUpdates(); - expectedRecords = Set.of( - "c0.app1.tenant1.us-west-1.vespa.oath.cloud", - "c1.app1.tenant1.us-west-1.vespa.oath.cloud", - "c0.app1.tenant1.us-central-1.vespa.oath.cloud", - "c1.app1.tenant1.us-central-1.vespa.oath.cloud" - ); - assertEquals(expectedRecords, tester.recordNames()); - assertTrue("Removes stale routing policies " + context2.application(), tester.routingPolicies().get(context2.instanceId()).isEmpty()); - assertEquals("Keeps routing policies for " + context1.application(), 4, tester.routingPolicies().get(context1.instanceId()).size()); - } - - @Test - public void global_routing_policies_in_rotationless_system() { - var tester = new RoutingPoliciesTester(new DeploymentTester(new ControllerTester(new RotationsConfig.Builder().build()))); - var context = tester.newDeploymentContext("tenant1", "app1", "default"); - tester.provisionLoadBalancers(1, context.instanceId(), zone1, zone2); - - var applicationPackage = applicationPackageBuilder() - .region(zone1.region().value()) - .endpoint("r0", "c0") - .build(); - context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - - var endpoint = "r0.app1.tenant1.global.vespa.oath.cloud"; - assertEquals(endpoint + " points to c0 in all regions", - List.of("latency/lb-0--tenant1:app1:default--prod.us-west-1/dns-zone-1/prod.us-west-1"), - tester.aliasDataOf(endpoint)); - assertTrue("No rotations assigned", context.application().instances().values().stream() - .map(Instance::rotations) - .allMatch(List::isEmpty)); - } - - @Test - public void manual_deployment_creates_routing_policy() { - // Empty application package is valid in manually deployed environments - var tester = new RoutingPoliciesTester(); - var context = tester.newDeploymentContext("tenant1", "app1", "default"); - var emptyApplicationPackage = new ApplicationPackageBuilder().build(); - var zone = ZoneId.from("dev", "us-east-1"); - var zoneApi = ZoneApiMock.from(zone.environment(), zone.region()); - tester.controllerTester().serviceRegistry().zoneRegistry() - .setZones(zoneApi) - .exclusiveRoutingIn(zoneApi); - - // Deploy to dev - tester.controllerTester().controller().applications().deploy(context.instanceId(), zone, Optional.of(emptyApplicationPackage), DeployOptions.none()); - assertEquals("DeploymentSpec is not persisted", DeploymentSpec.empty, context.application().deploymentSpec()); - context.flushDnsUpdates(); - - // Routing policy is created and DNS is updated - assertEquals(1, tester.policiesOf(context.instanceId()).size()); - assertEquals(Set.of("app1.tenant1.us-east-1.dev.vespa.oath.cloud"), tester.recordNames()); - } - - @Test - public void manual_deployment_creates_routing_policy_with_non_empty_spec() { - // Initial deployment - var tester = new RoutingPoliciesTester(); - var context = tester.newDeploymentContext("tenant1", "app1", "default"); - context.submit(applicationPackage).deploy(); - var zone = ZoneId.from("dev", "us-east-1"); - var zoneApi = ZoneApiMock.from(zone.environment(), zone.region()); - tester.controllerTester().serviceRegistry().zoneRegistry() - .setZones(zoneApi) - .exclusiveRoutingIn(zoneApi); - var prodRecords = Set.of("app1.tenant1.us-central-1.vespa.oath.cloud", "app1.tenant1.us-west-1.vespa.oath.cloud"); - assertEquals(prodRecords, tester.recordNames()); - - // Deploy to dev under different instance - var devInstance = context.application().id().instance("user"); - tester.controllerTester().controller().applications().deploy(devInstance, zone, Optional.of(applicationPackage), DeployOptions.none()); - assertEquals("DeploymentSpec is persisted", applicationPackage.deploymentSpec(), context.application().deploymentSpec()); - context.flushDnsUpdates(); - - // Routing policy is created and DNS is updated - assertEquals(1, tester.policiesOf(devInstance).size()); - assertEquals(Sets.union(prodRecords, Set.of("user.app1.tenant1.us-east-1.dev.vespa.oath.cloud")), tester.recordNames()); - } - - @Test - public void reprovisioning_load_balancer_preserves_cname_record() { - var tester = new RoutingPoliciesTester(); - var context = tester.newDeploymentContext("tenant1", "app1", "default"); - - // Initial load balancer is provisioned - tester.provisionLoadBalancers(1, context.instanceId(), zone1); - var applicationPackage = applicationPackageBuilder() - .region(zone1.region()) - .build(); - - // Application is deployed - context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - var expectedRecords = Set.of( - "c0.app1.tenant1.us-west-1.vespa.oath.cloud" - ); - assertEquals(expectedRecords, tester.recordNames()); - assertEquals(1, tester.policiesOf(context.instanceId()).size()); - - // Application is removed and the load balancer is deprovisioned - tester.controllerTester().controller().applications().deactivate(context.instanceId(), zone1); - tester.controllerTester().configServer().removeLoadBalancers(context.instanceId(), zone1); - - // Load balancer for the same application is provisioned again, but with a different hostname - var newHostname = HostName.from("new-hostname"); - var loadBalancer = new LoadBalancer("LB-0-Z-" + zone1.value(), - context.instanceId(), - ClusterSpec.Id.from("c0"), - newHostname, - LoadBalancer.State.active, - Optional.of("dns-zone-1")); - tester.controllerTester().configServer().putLoadBalancers(zone1, List.of(loadBalancer)); - - // Application redeployment preserves DNS record - context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - assertEquals(expectedRecords, tester.recordNames()); - assertEquals(1, tester.policiesOf(context.instanceId()).size()); - assertEquals("CNAME points to current load blancer", newHostname.value() + ".", - tester.cnameDataOf(expectedRecords.iterator().next()).get(0)); - } - - @Test - public void set_global_endpoint_status() { - var tester = new RoutingPoliciesTester(); - var context = tester.newDeploymentContext("tenant1", "app1", "default"); - - // Provision load balancers and deploy application - tester.provisionLoadBalancers(1, context.instanceId(), zone1, zone2); - var applicationPackage = applicationPackageBuilder() - .region(zone1.region()) - .region(zone2.region()) - .endpoint("r0", "c0", zone1.region().value(), zone2.region().value()) - .endpoint("r1", "c0", zone1.region().value(), zone2.region().value()) - .build(); - context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - - // Global DNS record is created - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); - tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1, zone2); - - // Global routing status is overridden in one zone - var changedAt = tester.controllerTester().clock().instant(); - tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.out, - GlobalRouting.Agent.tenant); - context.flushDnsUpdates(); - - // Inactive zone is removed from global DNS record - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone2); - tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone2); - - // Status details is stored in policy - var policy1 = tester.routingPolicies().get(context.deploymentIdIn(zone1)).values().iterator().next(); - assertEquals(GlobalRouting.Status.out, policy1.status().globalRouting().status()); - assertEquals(GlobalRouting.Agent.tenant, policy1.status().globalRouting().agent()); - assertEquals(changedAt.truncatedTo(ChronoUnit.MILLIS), policy1.status().globalRouting().changedAt()); - - // Other zone remains in - var policy2 = tester.routingPolicies().get(context.deploymentIdIn(zone2)).values().iterator().next(); - assertEquals(GlobalRouting.Status.in, policy2.status().globalRouting().status()); - assertEquals(GlobalRouting.Agent.system, policy2.status().globalRouting().agent()); - assertEquals(Instant.EPOCH, policy2.status().globalRouting().changedAt()); - - // Next deployment does not affect status - context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - context.flushDnsUpdates(); - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone2); - tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone2); - - // Deployment is set back in - tester.controllerTester().clock().advance(Duration.ofHours(1)); - changedAt = tester.controllerTester().clock().instant(); - tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.in, GlobalRouting.Agent.tenant); - context.flushDnsUpdates(); - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); - tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1, zone2); - - policy1 = tester.routingPolicies().get(context.deploymentIdIn(zone1)).values().iterator().next(); - assertEquals(GlobalRouting.Status.in, policy1.status().globalRouting().status()); - assertEquals(GlobalRouting.Agent.tenant, policy1.status().globalRouting().agent()); - assertEquals(changedAt.truncatedTo(ChronoUnit.MILLIS), policy1.status().globalRouting().changedAt()); - - // Deployment is set out through a new deployment.xml - var applicationPackage2 = applicationPackageBuilder() - .region(zone1.region()) - .region(zone2.region(), false) - .endpoint("r0", "c0", zone1.region().value(), zone2.region().value()) - .endpoint("r1", "c0", zone1.region().value(), zone2.region().value()) - .build(); - context.submit(applicationPackage2).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1); - tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1); - - // ... back in - var applicationPackage3 = applicationPackageBuilder() - .region(zone1.region()) - .region(zone2.region()) - .endpoint("r0", "c0", zone1.region().value(), zone2.region().value()) - .endpoint("r1", "c0", zone1.region().value(), zone2.region().value()) - .build(); - context.submit(applicationPackage3).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); - tester.assertTargets(context.instanceId(), EndpointId.of("r1"), 0, zone1, zone2); - } - - @Test - public void set_zone_global_endpoint_status() { - var tester = new RoutingPoliciesTester(); - var context1 = tester.newDeploymentContext("tenant1", "app1", "default"); - var context2 = tester.newDeploymentContext("tenant2", "app2", "default"); - var contexts = List.of(context1, context2); - - // Deploy applications - var applicationPackage = applicationPackageBuilder() - .region(zone1.region()) - .region(zone2.region()) - .endpoint("default", "c0", zone1.region().value(), zone2.region().value()) - .build(); - for (var context : contexts) { - tester.provisionLoadBalancers(1, context.instanceId(), zone1, zone2); - context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - tester.assertTargets(context.instanceId(), EndpointId.defaultId(), 0, zone1, zone2); - } - - // Set zone out - tester.routingPolicies().setGlobalRoutingStatus(zone2, GlobalRouting.Status.out); - context1.flushDnsUpdates(); - tester.assertTargets(context1.instanceId(), EndpointId.defaultId(), 0, zone1); - tester.assertTargets(context2.instanceId(), EndpointId.defaultId(), 0, zone1); - for (var context : contexts) { - var policies = tester.routingPolicies().get(context.instanceId()); - assertTrue("Global routing status for policy remains " + GlobalRouting.Status.in, - policies.values().stream() - .map(RoutingPolicy::status) - .map(Status::globalRouting) - .map(GlobalRouting::status) - .allMatch(status -> status == GlobalRouting.Status.in)); - } - var changedAt = tester.controllerTester().clock().instant(); - var zonePolicy = tester.controllerTester().controller().curator().readZoneRoutingPolicy(zone2); - assertEquals(GlobalRouting.Status.out, zonePolicy.globalRouting().status()); - assertEquals(GlobalRouting.Agent.operator, zonePolicy.globalRouting().agent()); - assertEquals(changedAt.truncatedTo(ChronoUnit.MILLIS), zonePolicy.globalRouting().changedAt()); - - // Setting status per deployment does not affect status as entire zone is out - tester.routingPolicies().setGlobalRoutingStatus(context1.deploymentIdIn(zone2), GlobalRouting.Status.in, GlobalRouting.Agent.tenant); - context1.flushDnsUpdates(); - tester.assertTargets(context1.instanceId(), EndpointId.defaultId(), 0, zone1); - tester.assertTargets(context2.instanceId(), EndpointId.defaultId(), 0, zone1); - - // Set single deployment out - tester.routingPolicies().setGlobalRoutingStatus(context1.deploymentIdIn(zone2), GlobalRouting.Status.out, GlobalRouting.Agent.tenant); - context1.flushDnsUpdates(); - - // Set zone back in. Deployment set explicitly out, remains out, the rest are in - tester.routingPolicies().setGlobalRoutingStatus(zone2, GlobalRouting.Status.in); - context1.flushDnsUpdates(); - tester.assertTargets(context1.instanceId(), EndpointId.defaultId(), 0, zone1); - tester.assertTargets(context2.instanceId(), EndpointId.defaultId(), 0, zone1, zone2); - } - - @Test - public void non_production_deployment_is_not_registered_in_global_endpoint() { - var tester = new RoutingPoliciesTester(SystemName.Public); - - // Configure the system to use the same region for test, staging and prod - var sharedRegion = RegionName.from("aws-us-east-1c"); - var prodZone = ZoneId.from(Environment.prod, sharedRegion); - var stagingZone = ZoneId.from(Environment.staging, sharedRegion); - var testZone = ZoneId.from(Environment.test, sharedRegion); - var zones = List.of(ZoneApiMock.from(prodZone), - ZoneApiMock.from(stagingZone), - ZoneApiMock.from(testZone)); - tester.controllerTester().zoneRegistry() - .setZones(zones) - .setRoutingMethod(zones, RoutingMethod.exclusive); - tester.controllerTester().configServer().bootstrap(List.of(prodZone, stagingZone, testZone), - SystemApplication.all()); - - var context = tester.tester.newDeploymentContext(); - var endpointId = EndpointId.of("r0"); - var applicationPackage = applicationPackageBuilder() - .trustDefaultCertificate() - .region(sharedRegion) - .endpoint(endpointId.id(), "default") - .build(); - - // Application starts deployment - context = context.submit(applicationPackage); - for (var testJob : List.of(JobType.systemTest, JobType.stagingTest)) { - context = context.runJob(testJob); - // Since runJob implicitly tears down the deployment and immediately deletes DNS records associated with the - // deployment, we consume only one DNS update at a time here - do { - context = context.flushDnsUpdates(1); - tester.assertTargets(context.instanceId(), endpointId, 0); - } while (!tester.recordNames().isEmpty()); - } - - // Deployment completes - context.completeRollout(); - tester.assertTargets(context.instanceId(), endpointId, 0, prodZone); - } - - @Test - public void changing_global_routing_status_never_removes_all_members() { - var tester = new RoutingPoliciesTester(); - var context = tester.newDeploymentContext("tenant1", "app1", "default"); - - // Provision load balancers and deploy application - tester.provisionLoadBalancers(1, context.instanceId(), zone1, zone2); - var applicationPackage = applicationPackageBuilder() - .region(zone1.region()) - .region(zone2.region()) - .endpoint("r0", "c0", zone1.region().value(), zone2.region().value()) - .build(); - context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - - // Global DNS record is created, pointing to all configured zones - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); - - // Global routing status is overridden for one deployment - tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.out, - GlobalRouting.Agent.tenant); - context.flushDnsUpdates(); - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone2); - - // Setting other deployment out implicitly sets all deployments in - tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone2), GlobalRouting.Status.out, - GlobalRouting.Agent.tenant); - context.flushDnsUpdates(); - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); - - // One inactive deployment is put back in. Global DNS record now points to the only active deployment - tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone1), GlobalRouting.Status.in, - GlobalRouting.Agent.tenant); - context.flushDnsUpdates(); - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1); - - // Setting zone (containing active deployment) out puts all deployments in - tester.routingPolicies().setGlobalRoutingStatus(zone1, GlobalRouting.Status.out); - context.flushDnsUpdates(); - assertEquals(GlobalRouting.Status.out, tester.routingPolicies().get(zone1).globalRouting().status()); - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); - - // Setting zone back in removes the currently inactive deployment - tester.routingPolicies().setGlobalRoutingStatus(zone1, GlobalRouting.Status.in); - context.flushDnsUpdates(); - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1); - - // Inactive deployment is set in - tester.routingPolicies().setGlobalRoutingStatus(context.deploymentIdIn(zone2), GlobalRouting.Status.in, - GlobalRouting.Agent.tenant); - context.flushDnsUpdates(); - for (var policy : tester.routingPolicies().get(context.instanceId()).values()) { - assertSame(GlobalRouting.Status.in, policy.status().globalRouting().status()); - } - tester.assertTargets(context.instanceId(), EndpointId.of("r0"), 0, zone1, zone2); - } - - @Test - public void config_server_routing_policy() { - var tester = new RoutingPoliciesTester(); - var app = SystemApplication.configServer.id(); - RecordName name = RecordName.from("cfg.prod.us-west-1.test.vip"); - - tester.provisionLoadBalancers(1, app, zone1); - tester.routingPolicies().refresh(app, DeploymentSpec.empty, zone1); - new NameServiceDispatcher(tester.tester.controller(), Duration.ofDays(1), Integer.MAX_VALUE).run(); - - List<Record> records = tester.controllerTester().nameService().findRecords(Record.Type.CNAME, name); - assertEquals(1, records.size()); - assertEquals(RecordData.from("lb-0--hosted-vespa:zone-config-servers:default--prod.us-west-1."), - records.get(0).data()); - } - - /** Returns an application package builder that satisfies requirements for a directly routed endpoint */ - private static ApplicationPackageBuilder applicationPackageBuilder() { - return new ApplicationPackageBuilder() - .athenzIdentity(AthenzDomain.from("domain"), AthenzService.from("service")) - .compileVersion(RoutingController.DIRECT_ROUTING_MIN_VERSION); - } - - private static List<LoadBalancer> createLoadBalancers(ZoneId zone, ApplicationId application, boolean shared, int count) { - List<LoadBalancer> loadBalancers = new ArrayList<>(); - for (int i = 0; i < count; i++) { - HostName lbHostname; - if (shared) { - lbHostname = HostName.from("shared-lb--" + zone.value()); - } else { - lbHostname = HostName.from("lb-" + i + "--" + application.serializedForm() + - "--" + zone.value()); - } - loadBalancers.add( - new LoadBalancer("LB-" + i + "-Z-" + zone.value(), - application, - ClusterSpec.Id.from("c" + i), - lbHostname, - LoadBalancer.State.active, - Optional.of("dns-zone-1"))); - } - return loadBalancers; - } - - private static class RoutingPoliciesTester { - - private final DeploymentTester tester; - - public RoutingPoliciesTester() { - this(SystemName.main); - } - - public RoutingPoliciesTester(SystemName system) { - this(new DeploymentTester(new ControllerTester(new ServiceRegistryMock(system)))); - } - - public RoutingPolicies routingPolicies() { - return tester.controllerTester().controller().routing().policies(); - } - - public DeploymentContext newDeploymentContext(String tenant, String application, String instance) { - return tester.newDeploymentContext(tenant, application, instance); - } - - public ControllerTester controllerTester() { - return tester.controllerTester(); - } - - public RoutingPoliciesTester(DeploymentTester tester) { - this.tester = tester; - // Make all zones directly routed - ((InMemoryFlagSource) tester.controllerTester().controller().flagSource()).withBooleanFlag(Flags.WEIGHTED_DNS_PER_REGION.id(), false); - tester.controllerTester().zoneRegistry().exclusiveRoutingIn(tester.controllerTester().zoneRegistry().zones().all().zones()); - } - - private void provisionLoadBalancers(int clustersPerZone, ApplicationId application, boolean shared, ZoneId... zones) { - for (ZoneId zone : zones) { - tester.configServer().removeLoadBalancers(application, zone); - tester.configServer().putLoadBalancers(zone, createLoadBalancers(zone, application, shared, clustersPerZone)); - } - } - - private void provisionLoadBalancers(int clustersPerZone, ApplicationId application, ZoneId... zones) { - provisionLoadBalancers(clustersPerZone, application, false, zones); - } - - private Collection<RoutingPolicy> policiesOf(ApplicationId instance) { - return tester.controller().curator().readRoutingPolicies(instance).values(); - } - - private Set<String> recordNames() { - return tester.controllerTester().nameService().records().stream() - .map(Record::name) - .map(RecordName::asString) - .collect(Collectors.toSet()); - } - - private List<String> aliasDataOf(String name) { - return tester.controllerTester().nameService().findRecords(Record.Type.ALIAS, RecordName.from(name)).stream() - .map(Record::data) - .map(RecordData::asString) - .collect(Collectors.toList()); - } - - private List<String> cnameDataOf(String name) { - return tester.controllerTester().nameService().findRecords(Record.Type.CNAME, RecordName.from(name)).stream() - .map(Record::data) - .map(RecordData::asString) - .collect(Collectors.toList()); - } - - private void assertTargets(ApplicationId application, EndpointId endpointId, int loadBalancerId, ZoneId ...zone) { - var endpoint = tester.controller().routing().endpointsOf(application) - .named(endpointId) - .targets(List.of(zone)) - .primary() - .map(Endpoint::dnsName) - .orElse("<none>"); - var zoneTargets = Arrays.stream(zone) - .map(z -> "latency/lb-" + loadBalancerId + "--" + application.serializedForm() + "--" + - z.value() + "/dns-zone-1/" + z.value()) - .collect(Collectors.toSet()); - assertEquals("Global endpoint " + endpoint + " points to expected zones", zoneTargets, - Set.copyOf(aliasDataOf(endpoint))); - } - - } - -} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java index 6679fc112a3..c58003cd781 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java @@ -16,8 +16,6 @@ import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.RoutingController; @@ -749,7 +747,6 @@ public class RoutingPoliciesTest { } tester.controllerTester().configServer().bootstrap(tester.controllerTester().zoneRegistry().zones().all().ids(), SystemApplication.all()); - ((InMemoryFlagSource) tester.controllerTester().controller().flagSource()).withBooleanFlag(Flags.WEIGHTED_DNS_PER_REGION.id(), true); } private void provisionLoadBalancers(int clustersPerZone, ApplicationId application, boolean shared, ZoneId... zones) { 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<String> mask, List<Bundle> out) throws BundleException { bundleLocation = BundleLocationResolver.resolve(bundleLocation); if (mask.contains(bundleLocation)) { diff --git a/jdisc_http_service/pom.xml b/jdisc_http_service/pom.xml index bd8c77bc9cc..b140e66f28a 100644 --- a/jdisc_http_service/pom.xml +++ b/jdisc_http_service/pom.xml @@ -140,6 +140,7 @@ <artifactId>bundle-plugin</artifactId> <extensions>true</extensions> <configuration> + <buildLegacyVespaPlatformBundle>true</buildLegacyVespaPlatformBundle> <discPreInstallBundle> javax.servlet-api-3.1.0.jar, jetty-continuation-${jetty.version}.jar, diff --git a/parent/pom.xml b/parent/pom.xml index a77750b9132..ce40eb464fc 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -255,7 +255,6 @@ <artifactId>bundle-plugin</artifactId> <version>${project.version}</version> <configuration> - <buildVespaPlatformBundle>true</buildVespaPlatformBundle> <configGenVersion>${project.version}</configGenVersion> <useCommonAssemblyIds>true</useCommonAssemblyIds> </configuration> diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp index 8edf85657dc..65959e6e6de 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp @@ -102,7 +102,7 @@ GeoLocationSpec process_location_term(ProtonLocationTerm &pterm) { void exchange_location_nodes(const string &location_str, Node::UP &query_tree, - std::vector<search::fef::Location> &fef_locations) + std::vector<GeoLocationSpec> &fef_locations) { std::vector<GeoLocationSpec> locationSpecs; @@ -118,13 +118,7 @@ void exchange_location_nodes(const string &location_str, } for (const GeoLocationSpec &spec : locationSpecs) { if (spec.location.has_point) { - search::fef::Location fef_loc; - fef_loc.setAttribute(spec.field_name); - fef_loc.setXPosition(spec.location.point.x); - fef_loc.setYPosition(spec.location.point.y); - fef_loc.setXAspect(spec.location.x_aspect.multiplier); - fef_loc.setValid(true); - fef_locations.push_back(fef_loc); + fef_locations.push_back(spec); } } if (parsed.location.can_limit()) { @@ -189,7 +183,7 @@ Query::extractTerms(vector<const ITermData *> &terms) } void -Query::extractLocations(vector<const search::fef::Location *> &locations) +Query::extractLocations(vector<const GeoLocationSpec *> &locations) { locations.clear(); for (const auto & loc : _locations) { diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.h b/searchcore/src/vespa/searchcore/proton/matching/query.h index 5a484d2ce1f..952b6260da1 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.h +++ b/searchcore/src/vespa/searchcore/proton/matching/query.h @@ -2,7 +2,7 @@ #pragma once -#include <vespa/searchlib/fef/location.h> +#include <vespa/searchlib/common/geo_location_spec.h> #include <vespa/searchlib/fef/itermdata.h> #include <vespa/searchlib/fef/matchdatalayout.h> #include <vespa/searchlib/fef/iindexenvironment.h> @@ -22,9 +22,12 @@ private: search::query::Node::UP _query_tree; Blueprint::UP _blueprint; Blueprint::UP _whiteListBlueprint; - std::vector<search::fef::Location> _locations; + std::vector<search::common::GeoLocationSpec> _locations; public: + /** Convenience typedef. */ + using GeoLocationSpecPtrs = std::vector<const search::common::GeoLocationSpec *>; + Query(); ~Query(); /** @@ -65,7 +68,7 @@ public: * * @param locs where to collect locations **/ - void extractLocations(std::vector<const search::fef::Location *> &locs); + void extractLocations(GeoLocationSpecPtrs &locs); /** * Reserve room for terms in the query in the given match data diff --git a/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.cpp b/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.cpp index 667b48d2acd..448ce14dd51 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.cpp @@ -5,7 +5,6 @@ using search::attribute::IAttributeContext; using search::fef::IIndexEnvironment; -using search::fef::Location; using search::fef::Properties; namespace proton::matching { diff --git a/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.h b/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.h index 426123599bd..6daf488297d 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.h +++ b/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.h @@ -4,7 +4,6 @@ #include <vespa/searchlib/fef/iqueryenvironment.h> #include <vespa/searchlib/fef/properties.h> -#include <vespa/searchlib/fef/location.h> namespace search::index { class IFieldLengthInspector; } @@ -19,7 +18,7 @@ private: const search::fef::IIndexEnvironment &_indexEnv; const search::attribute::IAttributeContext &_attrContext; search::fef::Properties _properties; - std::vector<const search::fef::Location *> _locations; + GeoLocationSpecPtrs _locations; std::vector<const search::fef::ITermData *> _terms; const search::index::IFieldLengthInspector &_field_length_inspector; @@ -56,7 +55,7 @@ public: * * @return modifiable list of location data pointers **/ - std::vector<const search::fef::Location *> &locations() { + GeoLocationSpecPtrs &locations() { return _locations; } @@ -70,7 +69,7 @@ public: const search::fef::ITermData *getTerm(uint32_t idx) const override; // inherited from search::fef::IQueryEnvironment - std::vector<const search::fef::Location *> getAllLocations() const override { + GeoLocationSpecPtrs getAllLocations() const override { return _locations; } diff --git a/searchlib/src/tests/features/prod_features.cpp b/searchlib/src/tests/features/prod_features.cpp index f886ba59c1c..25b5ba20d26 100644 --- a/searchlib/src/tests/features/prod_features.cpp +++ b/searchlib/src/tests/features/prod_features.cpp @@ -62,6 +62,8 @@ using search::StringAttribute; using search::SingleBoolAttribute; using search::WeightedSetStringExtAttribute; using search::attribute::WeightedEnumContent; +using search::common::GeoLocation; +using search::common::GeoLocationSpec; using AttributePtr = AttributeVector::SP; using AVC = search::attribute::Config; @@ -507,8 +509,8 @@ Test::assertCloseness(feature_t exp, const vespalib::string & attr, double dista int32_t x = 0; positions.emplace_back(x, x); setupForDistanceTest(ft, "pos", positions, false); - ft.getQueryEnv().getLocation().setXPosition((int)distance); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{int32_t(distance), 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{attr, p}); if (maxDistance > 0) { ft.getIndexEnv().getProperties().add(feature + ".maxDistance", vespalib::make_string("%u", (unsigned int)maxDistance)); @@ -857,13 +859,16 @@ Test::testDistance() { // non-existing attribute FtFeatureTest ft(_factory, "distance(pos)"); ft.getIndexEnv().getBuilder().addField(FieldType::ATTRIBUTE, CollectionType::SINGLE, DataType::INT64, "pos"); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{0, 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", p}); + ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().addScore("distance(pos)", 6400000000.0))); } { // label FtFeatureTest ft(_factory, "distance(label,foo)"); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{0, 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", p}); ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().addScore("distance(label,foo)", std::numeric_limits<feature_t>::max()))); } @@ -873,7 +878,8 @@ Test::testDistance() pos->commit(); ft.getIndexEnv().getAttributeMap().add(pos); ft.getIndexEnv().getBuilder().addField(FieldType::ATTRIBUTE, CollectionType::SINGLE, DataType::INT64, "pos"); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{0, 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", p}); ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().addScore("distance(pos)", 6400000000.0))); } @@ -883,7 +889,8 @@ Test::testDistance() pos->commit(); ft.getIndexEnv().getAttributeMap().add(pos); ft.getIndexEnv().getBuilder().addField(FieldType::ATTRIBUTE, CollectionType::SINGLE, DataType::INT64, "pos"); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{0, 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", p}); ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().addScore("distance(pos)", 6400000000.0))); } @@ -893,7 +900,8 @@ Test::testDistance() pos->commit(); ft.getIndexEnv().getAttributeMap().add(pos); ft.getIndexEnv().getBuilder().addField(FieldType::ATTRIBUTE, CollectionType::WEIGHTEDSET, DataType::INT64, "pos"); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{0, 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", p}); ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().addScore("distance(pos)", 6400000000.0))); } @@ -939,10 +947,9 @@ Test::assert2DZDistance(feature_t exp, const vespalib::string & positions, pos.emplace_back(x, y); } setupForDistanceTest(ft, "pos", pos, true); - ft.getQueryEnv().getLocation().setXPosition(xquery); - ft.getQueryEnv().getLocation().setYPosition(yquery); - ft.getQueryEnv().getLocation().setXAspect(xAspect); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{xquery, yquery}; + GeoLocation::Aspect aspect{xAspect}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", {p, aspect}}); ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().setEpsilon(1e-4). addScore("distance(pos)", exp))); diff --git a/searchlib/src/vespa/searchlib/features/distancefeature.cpp b/searchlib/src/vespa/searchlib/features/distancefeature.cpp index bd2c7becc81..6582bcae92a 100644 --- a/searchlib/src/vespa/searchlib/features/distancefeature.cpp +++ b/searchlib/src/vespa/searchlib/features/distancefeature.cpp @@ -2,7 +2,7 @@ #include "distancefeature.h" #include <vespa/searchcommon/common/schema.h> -#include <vespa/searchlib/fef/location.h> +#include <vespa/searchlib/common/geo_location_spec.h> #include <vespa/searchlib/fef/matchdata.h> #include <vespa/document/datatype/positiondatatype.h> #include <vespa/vespalib/geo/zcurve.h> @@ -100,19 +100,10 @@ DistanceExecutor::calculate2DZDistance(uint32_t docId) int32_t docy = 0; for (auto loc : _locations) { assert(loc); - assert(loc->isValid()); - int32_t loc_x = loc->getXPosition(); - int32_t loc_y = loc->getYPosition(); - uint64_t loc_a = loc->getXAspect(); - LOG(debug, "location: x=%u, y=%u, aspect=%zu", loc_x, loc_y, loc_a); + assert(loc->location.valid()); for (uint32_t i = 0; i < numValues; ++i) { vespalib::geo::ZCurve::decode(_intBuf[i], &docx, &docy); - uint32_t dx = (loc_x > docx) ? (loc_x - docx) : (docx - loc_x); - if (loc_a != 0) { - dx = (uint64_t(dx) * loc_a) >> 32; - } - uint32_t dy = (loc_y > docy) ? (loc_y - docy) : (docy - loc_y); - uint64_t sqdist = (uint64_t) dx * dx + (uint64_t) dy * dy; + uint64_t sqdist = loc->location.sq_distance_to({docx, docy}); if (sqdist < sqabsdist) { sqabsdist = sqdist; } @@ -121,7 +112,7 @@ DistanceExecutor::calculate2DZDistance(uint32_t docId) return static_cast<feature_t>(std::sqrt(static_cast<feature_t>(sqabsdist))); } -DistanceExecutor::DistanceExecutor(std::vector<const Location *> locations, +DistanceExecutor::DistanceExecutor(GeoLocationSpecPtrs locations, const search::attribute::IAttributeVector * pos) : FeatureExecutor(), _locations(locations), @@ -253,17 +244,17 @@ DistanceBlueprint::createExecutor(const IQueryEnvironment &env, vespalib::Stash } // expect geo pos: const search::attribute::IAttributeVector * pos = nullptr; - std::vector<const search::fef::Location *> matching_locs; - std::vector<const search::fef::Location *> other_locs; + GeoLocationSpecPtrs matching_locs; + GeoLocationSpecPtrs other_locs; for (auto loc_ptr : env.getAllLocations()) { - if (_use_geo_pos && loc_ptr && loc_ptr->isValid()) { - if (loc_ptr->getAttribute() == _arg_string) { + if (_use_geo_pos && loc_ptr && loc_ptr->location.valid()) { + if (loc_ptr->field_name == _arg_string) { LOG(debug, "found loc from query env matching '%s'", _arg_string.c_str()); matching_locs.push_back(loc_ptr); } else { LOG(debug, "found loc(%s) from query env not matching arg(%s)", - loc_ptr->getAttribute().c_str(), _arg_string.c_str()); + loc_ptr->field_name.c_str(), _arg_string.c_str()); other_locs.push_back(loc_ptr); } } diff --git a/searchlib/src/vespa/searchlib/features/distancefeature.h b/searchlib/src/vespa/searchlib/features/distancefeature.h index 024b9f37f31..ece139c6546 100644 --- a/searchlib/src/vespa/searchlib/features/distancefeature.h +++ b/searchlib/src/vespa/searchlib/features/distancefeature.h @@ -7,12 +7,15 @@ namespace search::features { +/** Convenience typedef. */ +using GeoLocationSpecPtrs = std::vector<const search::common::GeoLocationSpec *>; + /** * Implements the executor for the distance feature. */ class DistanceExecutor : public fef::FeatureExecutor { private: - std::vector<const fef::Location *> _locations; + GeoLocationSpecPtrs _locations; const attribute::IAttributeVector * _pos; attribute::IntegerContent _intBuf; @@ -26,7 +29,7 @@ public: * @param locations location objects associated with the query environment. * @param pos the attribute to use for positions (expects zcurve encoding). */ - DistanceExecutor(std::vector<const fef::Location *> locations, + DistanceExecutor(GeoLocationSpecPtrs locations, const attribute::IAttributeVector * pos); void execute(uint32_t docId) override; diff --git a/searchlib/src/vespa/searchlib/fef/CMakeLists.txt b/searchlib/src/vespa/searchlib/fef/CMakeLists.txt index 396775b20c5..178de1b8b87 100644 --- a/searchlib/src/vespa/searchlib/fef/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/fef/CMakeLists.txt @@ -16,7 +16,6 @@ vespa_add_library(searchlib_fef OBJECT filetablefactory.cpp functiontablefactory.cpp indexproperties.cpp - location.cpp matchdata.cpp matchdatalayout.cpp objectstore.cpp diff --git a/searchlib/src/vespa/searchlib/fef/fef.h b/searchlib/src/vespa/searchlib/fef/fef.h index 7dd24bd17ae..b677ba128c9 100644 --- a/searchlib/src/vespa/searchlib/fef/fef.h +++ b/searchlib/src/vespa/searchlib/fef/fef.h @@ -35,7 +35,6 @@ #include "itablemanager.h" #include "itermdata.h" #include "itermfielddata.h" -#include "location.h" #include "matchdata.h" #include "matchdatalayout.h" #include "parameter.h" diff --git a/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h b/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h index 811e7fd4616..7c6a84916f4 100644 --- a/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h +++ b/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h @@ -6,9 +6,10 @@ #include "objectstore.h" #include <vespa/searchcommon/attribute/iattributecontext.h> +namespace search::common { class GeoLocationSpec; } + namespace search::fef { -class Location; class Properties; class ITermData; @@ -24,6 +25,9 @@ public: **/ typedef std::shared_ptr<IQueryEnvironment> SP; + /** Convenience typedef. */ + using GeoLocationSpecPtrs = std::vector<const search::common::GeoLocationSpec *>; + /** * Obtain the set of properties associated with this query * environment. This set of properties is known through the system @@ -60,7 +64,7 @@ public: * * @return pointers to location objects. **/ - virtual std::vector<const Location *> getAllLocations() const = 0; + virtual GeoLocationSpecPtrs getAllLocations() const = 0; /** * Returns the attribute context for this query. diff --git a/searchlib/src/vespa/searchlib/fef/location.cpp b/searchlib/src/vespa/searchlib/fef/location.cpp deleted file mode 100644 index 978daa0f930..00000000000 --- a/searchlib/src/vespa/searchlib/fef/location.cpp +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "location.h" - -namespace search { -namespace fef { - -Location::Location() : - _attr(), - _xPos(0), - _yPos(0), - _xAspect(0), - _valid(false) -{ -} - -} // namespace fef -} // namespace search diff --git a/searchlib/src/vespa/searchlib/fef/location.h b/searchlib/src/vespa/searchlib/fef/location.h deleted file mode 100644 index 3bc693e11b4..00000000000 --- a/searchlib/src/vespa/searchlib/fef/location.h +++ /dev/null @@ -1,111 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include <vespa/vespalib/stllike/string.h> - -namespace search { -namespace fef { - -/** - * This class contains location data that is associated with a query. - **/ -class Location -{ -private: - vespalib::string _attr; - int32_t _xPos; - int32_t _yPos; - uint32_t _xAspect; - bool _valid; - -public: - /** - * Creates an empty object. - **/ - Location(); - - /** - * Sets the name of the attribute to use for x positions. - * - * @param xAttr the attribute name. - * @return this to allow chaining. - **/ - Location & - setAttribute(const vespalib::string & attr) - { - _attr = attr; - return *this; - } - - /** - * Returns the name of the attribute to use for positions. - * - * @return the attribute name. - **/ - const vespalib::string & getAttribute() const { return _attr; } - - /** - * Sets the x position of this location. - * - * @param xPos the x position. - * @return this to allow chaining. - **/ - Location & setXPosition(int32_t xPos) { _xPos = xPos; return *this; } - - /** - * Returns the x position of this location. - * - * @return the x position. - **/ - int32_t getXPosition() const { return _xPos; } - - /** - * Sets the y position of this location. - * - * @param yPos the y position. - * @return this to allow chaining. - **/ - Location & setYPosition(int32_t yPos) { _yPos = yPos; return *this; } - - /** - * Returns the y position of this location. - * - * @return the y position. - **/ - int32_t getYPosition() const { return _yPos; } - - /** - * Sets the x distance multiplier fraction. - * - * @param xAspect the x aspect. - * @return this to allow chaining. - **/ - Location & setXAspect(uint32_t xAspect) { _xAspect = xAspect; return *this; } - - /** - * Returns the x distance multiplier fraction. - * - * @return the x aspect. - **/ - uint32_t getXAspect() const { return _xAspect; } - - /** - * Sets whether this is a valid location object. - * - * @param valid true if this is valid. - * @return this to allow chaining. - **/ - Location & setValid(bool valid) { _valid = valid; return *this; } - - /** - * Returns whether this is a valid location object. - * - * @param true if this is a valid. - **/ - bool isValid() const { return _valid; } -}; - -} // namespace fef -} // namespace search - diff --git a/searchlib/src/vespa/searchlib/fef/phrase_splitter_query_env.h b/searchlib/src/vespa/searchlib/fef/phrase_splitter_query_env.h index 3a8cde99c06..90bf4e8955f 100644 --- a/searchlib/src/vespa/searchlib/fef/phrase_splitter_query_env.h +++ b/searchlib/src/vespa/searchlib/fef/phrase_splitter_query_env.h @@ -73,7 +73,7 @@ public: } const Properties & getProperties() const override { return _queryEnv.getProperties(); } - std::vector<const Location *> getAllLocations() const override { + GeoLocationSpecPtrs getAllLocations() const override { return _queryEnv.getAllLocations(); } const attribute::IAttributeContext & getAttributeContext() const override { return _queryEnv.getAttributeContext(); } diff --git a/searchlib/src/vespa/searchlib/fef/test/queryenvironment.cpp b/searchlib/src/vespa/searchlib/fef/test/queryenvironment.cpp index 4697675c071..d602e74ddfb 100644 --- a/searchlib/src/vespa/searchlib/fef/test/queryenvironment.cpp +++ b/searchlib/src/vespa/searchlib/fef/test/queryenvironment.cpp @@ -8,7 +8,7 @@ QueryEnvironment::QueryEnvironment(IndexEnvironment *env) : _indexEnv(env), _terms(), _properties(), - _location(), + _locations(), _attrCtx((env == nullptr) ? attribute::IAttributeContext::UP() : env->getAttributeMap().createContext()) { } diff --git a/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h b/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h index 4b9bec1ee68..d8d5c802360 100644 --- a/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h +++ b/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h @@ -4,12 +4,14 @@ #include "indexenvironment.h" #include <vespa/searchcommon/attribute/iattributecontext.h> #include <vespa/searchlib/fef/iqueryenvironment.h> -#include <vespa/searchlib/fef/location.h> #include <vespa/searchlib/fef/simpletermdata.h> +#include <vespa/searchlib/common/geo_location_spec.h> #include <unordered_map> namespace search::fef::test { +using search::common::GeoLocationSpec; + /** * Implementation of the IQueryEnvironment interface used for testing. */ @@ -22,7 +24,7 @@ private: IndexEnvironment *_indexEnv; std::vector<SimpleTermData> _terms; Properties _properties; - Location _location; + std::vector<GeoLocationSpec> _locations; search::attribute::IAttributeContext::UP _attrCtx; std::unordered_map<std::string, double> _avg_field_lengths; @@ -38,10 +40,12 @@ public: const Properties &getProperties() const override { return _properties; } uint32_t getNumTerms() const override { return _terms.size(); } const ITermData *getTerm(uint32_t idx) const override { return idx < _terms.size() ? &_terms[idx] : NULL; } - std::vector<const Location *> getAllLocations() const override { - std::vector<const Location *> retval; - retval.push_back(&_location); - return retval; + GeoLocationSpecPtrs getAllLocations() const override { + GeoLocationSpecPtrs locations; + for (const auto & loc : _locations) { + locations.push_back(&loc); + } + return locations; } const search::attribute::IAttributeContext &getAttributeContext() const override { return *_attrCtx; } double get_average_field_length(const vespalib::string& field_name) const override { @@ -86,7 +90,7 @@ public: Properties & getProperties() { return _properties; } /** Returns a reference to the location of this. */ - Location & getLocation() { return _location; } + void addLocation(const GeoLocationSpec &spec) { _locations.push_back(spec); } std::unordered_map<std::string, double>& get_avg_field_lengths() { return _avg_field_lengths; } }; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp index 42afadb386a..1f9b553b56a 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp @@ -15,6 +15,9 @@ #include <vespa/log/log.h> LOG_SETUP(".searchsummary.docsummary.docsumstate"); +using search::common::GeoLocationParser; +using search::common::GeoLocationSpec; + namespace search::docsummary { GetDocsumsState::GetDocsumsState(GetDocsumsStateCallback &callback) @@ -72,11 +75,11 @@ GetDocsumsState::parse_locations() using document::PositionDataType; assert(_parsedLocations.empty()); // only allowed to call this once if (! _args.getLocation().empty()) { - search::common::GeoLocationParser parser; + GeoLocationParser parser; if (parser.parseOldFormatWithField(_args.getLocation())) { auto view = parser.getFieldName(); auto attr_name = PositionDataType::getZCurveFieldName(view); - search::common::GeoLocationSpec spec{attr_name, parser.getGeoLocation()}; + GeoLocationSpec spec{attr_name, parser.getGeoLocation()}; _parsedLocations.push_back(spec); } else { LOG(warning, "could not parse location string '%s' from request", @@ -90,10 +93,10 @@ GetDocsumsState::parse_locations() if (iterator.getType() == search::ParseItem::ITEM_GEO_LOCATION_TERM) { vespalib::string view = iterator.getIndexName(); vespalib::string term = iterator.getTerm(); - search::common::GeoLocationParser parser; + GeoLocationParser parser; if (parser.parseOldFormat(term)) { auto attr_name = PositionDataType::getZCurveFieldName(view); - search::common::GeoLocationSpec spec{attr_name, parser.getGeoLocation()}; + GeoLocationSpec spec{attr_name, parser.getGeoLocation()}; _parsedLocations.push_back(spec); } else { LOG(warning, "could not parse location string '%s' from stack dump", diff --git a/standalone-container/pom.xml b/standalone-container/pom.xml index b9062b9284c..479a76e2fc6 100644 --- a/standalone-container/pom.xml +++ b/standalone-container/pom.xml @@ -81,6 +81,7 @@ <extensions>true</extensions> <configuration> <discApplicationClass>com.yahoo.container.standalone.StandaloneContainerApplication</discApplicationClass> + <buildLegacyVespaPlatformBundle>true</buildLegacyVespaPlatformBundle> <discPreInstallBundle> configdefinitions-jar-with-dependencies.jar, config-provisioning-jar-with-dependencies.jar, 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<ConfigInstance> bundlesKey = key("bundles"); + private static ConfigKey<ConfigInstance> platformBundlesKey = key("platform-bundles"); + private static ConfigKey<ConfigInstance> applicationBundlesKey = key("application-bundles"); private static ConfigKey<ConfigInstance> componentsKey = key("components"); private static ConfigKey<ConfigInstance> key(String name) { @@ -35,16 +37,19 @@ public class StandaloneSubscriberTest { public void standalone_subscriber() throws Exception { withContainerModel("<container version=\"1.0\"></container>", root -> { Set<ConfigKey<ConfigInstance>> keys = new HashSet<>(); - keys.add(bundlesKey); + keys.add(platformBundlesKey); + keys.add(applicationBundlesKey); keys.add(componentsKey); Subscriber subscriber = new StandaloneSubscriberFactory(root).getSubscriber(keys); Map<ConfigKey<ConfigInstance>, 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/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp index 6096a2faea4..06bdb9f69bb 100644 --- a/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp @@ -9,6 +9,8 @@ LOG_SETUP(".searchvisitor.queryenvironment"); using search::IAttributeManager; +using search::common::GeoLocationParser; +using search::common::GeoLocationSpec; using search::fef::Properties; using vespalib::string; @@ -16,28 +18,24 @@ namespace streaming { namespace { -search::fef::Location +std::vector<GeoLocationSpec> parseLocation(const string & location_str) { - search::fef::Location fefLocation; + std::vector<GeoLocationSpec> fefLocations; if (location_str.empty()) { - return fefLocation; + return fefLocations; } - search::common::GeoLocationParser locationParser; + GeoLocationParser locationParser; if (!locationParser.parseOldFormatWithField(location_str)) { LOG(warning, "Location parse error (location: '%s'): %s. Location ignored.", location_str.c_str(), locationParser.getParseError()); - return fefLocation; + return fefLocations; } - auto location = locationParser.getGeoLocation(); - if (location.has_point) { - fefLocation.setAttribute(locationParser.getFieldName()); - fefLocation.setXPosition(location.point.x); - fefLocation.setYPosition(location.point.y); - fefLocation.setXAspect(location.x_aspect.multiplier); - fefLocation.setValid(true); + auto loc = locationParser.getGeoLocation(); + if (loc.has_point) { + fefLocations.push_back(GeoLocationSpec{locationParser.getFieldName(), loc}); } - return fefLocation; + return fefLocations; } } @@ -50,7 +48,7 @@ QueryEnvironment::QueryEnvironment(const string & location_str, _properties(properties), _attrCtx(attrMgr->createContext()), _queryTerms(), - _location(parseLocation(location_str)) + _locations(parseLocation(location_str)) { } diff --git a/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h index e3da5a44167..df354d578e6 100644 --- a/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h +++ b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h @@ -6,7 +6,6 @@ #include <vespa/searchlib/attribute/iattributemanager.h> #include <vespa/searchlib/fef/iindexenvironment.h> #include <vespa/searchlib/fef/iqueryenvironment.h> -#include <vespa/searchlib/fef/location.h> #include <vespa/searchlib/fef/properties.h> #include "indexenvironment.h" @@ -23,7 +22,7 @@ private: const search::fef::Properties &_properties; search::attribute::IAttributeContext::UP _attrCtx; std::vector<const search::fef::ITermData *> _queryTerms; - search::fef::Location _location; + std::vector<search::common::GeoLocationSpec> _locations; public: typedef std::unique_ptr<QueryEnvironment> UP; @@ -49,10 +48,10 @@ public: } // inherit documentation - std::vector<const search::fef::Location *> getAllLocations() const override { - std::vector<const search::fef::Location *> retval; - if (_location.isValid()) { - retval.push_back(&_location); + GeoLocationSpecPtrs getAllLocations() const override { + GeoLocationSpecPtrs retval; + for (const auto & loc : _locations) { + retval.push_back(&loc); } return retval; } 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<E> { - public final List<E> falseValues; /// list of values where the predicate returned false - public final List<E> trueValues; /// list of values where the predicate returned true - - private PredicateSplit() { - falseValues = new ArrayList<E>(); - trueValues = new ArrayList<E>(); - } - - /** - * 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 <V> PredicateSplit<V> partition(Iterable<V> collection, Predicate<? super V> predicate) - { - PredicateSplit<V> r = new PredicateSplit<V>(); - 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<Integer> l = new ArrayList<Integer>(); - l.add(1); - l.add(6); - l.add(2); - l.add(4); - l.add(5); - PredicateSplit<Integer> 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); - } -} |