From fcb699ac64db02e455a43c9397437158c47d2d21 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Thu, 1 Jul 2021 02:30:53 +0200 Subject: Install model-evaluation + integration via config-model. - Only for application container clusters. --- .../vespa/model/container/ContainerModelEvaluation.java | 12 +++++++++--- .../vespa/model/container/xml/ContainerModelBuilder.java | 9 +++++++++ .../model/container/xml/ContainerModelBuilderTest.java | 15 +++++++++++---- .../container/xml/ContainerModelBuilderTestBase.java | 6 ++++++ 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java index 37bfb8821c3..65247f29281 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerModelEvaluation.java @@ -10,7 +10,9 @@ import com.yahoo.vespa.config.search.core.RankingConstantsConfig; import com.yahoo.vespa.config.search.core.RankingExpressionsConfig; import com.yahoo.vespa.model.container.component.Handler; import com.yahoo.vespa.model.container.component.SystemBindingPattern; +import com.yahoo.vespa.model.container.xml.PlatformBundles; +import java.nio.file.Path; import java.util.List; import java.util.Objects; @@ -26,17 +28,21 @@ public class ContainerModelEvaluation implements OnnxModelsConfig.Producer, RankingExpressionsConfig.Producer { - private final static String BUNDLE_NAME = "model-evaluation"; + private final static String EVALUATION_BUNDLE_NAME = "model-evaluation"; + private final static String INTEGRATION_BUNDLE_NAME = "model-integration"; private final static String EVALUATOR_NAME = ModelsEvaluator.class.getName(); private final static String REST_HANDLER_NAME = "ai.vespa.models.handler.ModelsEvaluationHandler"; private final static String REST_BINDING_PATH = "/model-evaluation/v1"; + public static final Path MODEL_EVALUATION_BUNDLE_FILE = PlatformBundles.absoluteBundlePath(EVALUATION_BUNDLE_NAME); + public static final Path MODEL_INTEGRATION_BUNDLE_FILE = PlatformBundles.absoluteBundlePath(INTEGRATION_BUNDLE_NAME); + /** Global rank profiles, aka models */ private final RankProfileList rankProfileList; public ContainerModelEvaluation(ApplicationContainerCluster cluster, RankProfileList rankProfileList) { this.rankProfileList = Objects.requireNonNull(rankProfileList, "rankProfileList cannot be null"); - cluster.addSimpleComponent(EVALUATOR_NAME, null, BUNDLE_NAME); + cluster.addSimpleComponent(EVALUATOR_NAME, null, EVALUATION_BUNDLE_NAME); cluster.addComponent(ContainerModelEvaluation.getHandler()); } @@ -64,7 +70,7 @@ public class ContainerModelEvaluation implements } public static Handler getHandler() { - Handler handler = new Handler<>(new ComponentModel(REST_HANDLER_NAME, null, BUNDLE_NAME)); + Handler handler = new Handler<>(new ComponentModel(REST_HANDLER_NAME, null, EVALUATION_BUNDLE_NAME)); handler.addServerBindings( SystemBindingPattern.fromHttpPath(REST_BINDING_PATH), SystemBindingPattern.fromHttpPath(REST_BINDING_PATH + "/*")); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 3b04d536300..5bf8aa5228e 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -187,6 +187,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder { throwUponRestApi(spec); // TODO: remove addServlets(deployState, spec, cluster); addModelEvaluation(spec, cluster, context); + addModelEvaluationBundles(cluster); addProcessing(deployState, spec, cluster); addSearch(deployState, spec, cluster); @@ -565,6 +566,14 @@ public class ContainerModelBuilder extends ConfigModelBuilder { cluster.setModelEvaluation(new ContainerModelEvaluation(cluster, profiles)); } + protected void addModelEvaluationBundles(ApplicationContainerCluster cluster) { + /* These bundles are added to all application container clusters, even if they haven't + * declared 'model-evaluation' in services.xml, because there are many public API packages + * in the model-evaluation bundle that could be used by customer code. */ + cluster.addPlatformBundle(ContainerModelEvaluation.MODEL_EVALUATION_BUNDLE_FILE); + cluster.addPlatformBundle(ContainerModelEvaluation.MODEL_INTEGRATION_BUNDLE_FILE); + } + private void addProcessing(DeployState deployState, Element spec, ApplicationContainerCluster cluster) { Element processingElement = XML.getChild(spec, "processing"); if (processingElement == null) return; diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index 543318f9224..ccee21c87dc 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -27,6 +27,7 @@ import com.yahoo.container.ComponentsConfig; import com.yahoo.container.QrConfig; import com.yahoo.container.core.ChainsConfig; import com.yahoo.container.core.VipStatusConfig; +import com.yahoo.container.di.config.PlatformBundlesConfig; import com.yahoo.container.handler.VipStatusHandler; import com.yahoo.container.handler.metrics.MetricsV2Handler; import com.yahoo.container.handler.observability.ApplicationStatusHandler; @@ -52,6 +53,7 @@ import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.ApplicationContainer; import com.yahoo.vespa.model.container.ApplicationContainerCluster; import com.yahoo.vespa.model.container.ContainerCluster; +import com.yahoo.vespa.model.container.ContainerModelEvaluation; import com.yahoo.vespa.model.container.SecretStore; import com.yahoo.vespa.model.container.component.Component; import com.yahoo.vespa.model.container.http.ConnectorFactory; @@ -123,6 +125,14 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { @Rule public TemporaryFolder applicationFolder = new TemporaryFolder(); + @Test + public void model_evaluation_bundles_are_deployed() { + createBasicContainerModel(); + PlatformBundlesConfig config = root.getConfig(PlatformBundlesConfig.class, "default"); + assertThat(config.bundlePaths(), hasItem(ContainerModelEvaluation.MODEL_EVALUATION_BUNDLE_FILE.toString())); + assertThat(config.bundlePaths(), hasItem(ContainerModelEvaluation.MODEL_INTEGRATION_BUNDLE_FILE.toString())); + } + @Test public void deprecated_jdisc_tag_is_allowed() { Element clusterElem = DomBuilderTest.parse( @@ -244,10 +254,7 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { @Test public void verify_bindings_for_builtin_handlers() { - Element clusterElem = DomBuilderTest.parse( - "" - ); - createModel(root, clusterElem); + createBasicContainerModel(); JdiscBindingsConfig config = root.getConfig(JdiscBindingsConfig.class, "default/container.0"); JdiscBindingsConfig.Handlers defaultRootHandler = config.handlers(BindingsOverviewHandler.class.getName()); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java index 9e02572737e..7034176da14 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.model.container.xml; import com.yahoo.collections.Pair; import com.yahoo.component.ComponentId; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.model.builder.xml.test.DomBuilderTest; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.test.MockRoot; import com.yahoo.container.ComponentsConfig; @@ -51,6 +52,11 @@ public abstract class ContainerModelBuilderTestBase { protected MockRoot root; + protected void createBasicContainerModel() { + Element clusterElem = DomBuilderTest.parse(""); + createModel(root, clusterElem); + } + public static void createModel(MockRoot root, DeployState deployState, VespaModel vespaModel, Element... containerElems) { for (Element containerElem : containerElems) { ContainerModel model = new ContainerModelBuilder(false, ContainerModelBuilder.Networking.enable) -- cgit v1.2.3 From f8e64bc63f7a0b33358526e816f1e34554c0ca28 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Thu, 1 Jul 2021 02:32:09 +0200 Subject: Preinstall model-evaluation bundles in standalone containers. - Configserver needs them because the config-model uses their apis --- .../container/xml/ConfigServerContainerModelBuilder.java | 6 ++++++ .../container/configserver/ConfigserverClusterTest.java | 12 ++++++++++++ standalone-container/pom.xml | 2 ++ 3 files changed, 20 insertions(+) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java index a37d0ef416f..a1f52cca9fd 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java @@ -6,6 +6,7 @@ import com.yahoo.config.model.deploy.DeployState; import com.yahoo.container.logging.FileConnectionLog; import com.yahoo.vespa.model.container.ApplicationContainerCluster; import com.yahoo.vespa.model.container.ContainerModel; +import com.yahoo.vespa.model.container.ContainerModelEvaluation; import com.yahoo.vespa.model.container.component.AccessLogComponent; import com.yahoo.vespa.model.container.component.ConnectionLogComponent; import com.yahoo.vespa.model.container.configserver.ConfigserverCluster; @@ -61,6 +62,11 @@ public class ConfigServerContainerModelBuilder extends ContainerModelBuilder { cluster.getHttp().getHttpServer().get().setHostedVespa(isHosted()); } + @Override + protected void addModelEvaluationBundles(ApplicationContainerCluster cluster) { + // Model evaluation bundles are pre-installed in the standalone container. + } + /** Note: using {@link CloudConfigOptions} as {@link DeployState#isHosted()} returns false for hosted configserver/controller */ private boolean isHosted() { return options.hostedVespa().orElse(Boolean.FALSE); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java index 2b55d7a3948..2144c3c9a66 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java @@ -9,6 +9,7 @@ import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducerRoot; import com.yahoo.config.model.test.MockRoot; import com.yahoo.container.StatisticsConfig; +import com.yahoo.container.di.config.PlatformBundlesConfig; import com.yahoo.container.jdisc.config.HealthMonitorConfig; import com.yahoo.net.HostName; import com.yahoo.text.XML; @@ -16,6 +17,7 @@ import com.yahoo.vespa.defaults.Defaults; import com.yahoo.vespa.model.HostResource; import com.yahoo.vespa.model.container.Container; import com.yahoo.vespa.model.container.ContainerModel; +import com.yahoo.vespa.model.container.ContainerModelEvaluation; import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions; import com.yahoo.vespa.model.container.xml.ConfigServerContainerModelBuilder; import org.junit.Test; @@ -27,6 +29,8 @@ import java.util.Optional; import java.util.function.Function; import java.util.stream.Collectors; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -124,6 +128,14 @@ public class ConfigserverClusterTest { assertTrue(config.zookeeperLocalhostAffinity()); } + @Test + public void model_evaluation_bundles_are_not_installed_via_config() { + // These bundles must be pre-installed because they are used by config-model. + PlatformBundlesConfig config = getConfig(PlatformBundlesConfig.class); + assertThat(config.bundlePaths(), not(hasItem(ContainerModelEvaluation.MODEL_INTEGRATION_BUNDLE_FILE.toString()))); + assertThat(config.bundlePaths(), not(hasItem(ContainerModelEvaluation.MODEL_EVALUATION_BUNDLE_FILE.toString()))); + } + @SuppressWarnings("varargs") private static void assertZookeeperServerProperty( List zkServers, Function propertyMapper, T... expectedProperties) { diff --git a/standalone-container/pom.xml b/standalone-container/pom.xml index 11d0406fcc5..4aa66e9b1a4 100644 --- a/standalone-container/pom.xml +++ b/standalone-container/pom.xml @@ -90,6 +90,8 @@ config-model-api-jar-with-dependencies.jar, config-model-jar-with-dependencies.jar, container-disc-jar-with-dependencies.jar, + model-evaluation-jar-with-dependencies.jar, + model-integration-jar-with-dependencies.jar, vespajlib.jar -- cgit v1.2.3 From 907b0c5eeb2478f4c1e2a6529e22a8f930865ec5 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Thu, 1 Jul 2021 02:33:24 +0200 Subject: Do not pre-install model-evaluation/integration from disc pom. - They are now installed via config-model for the appropriate container types. --- container-disc/pom.xml | 2 -- 1 file changed, 2 deletions(-) diff --git a/container-disc/pom.xml b/container-disc/pom.xml index 5446c9e1698..1acd6043498 100644 --- a/container-disc/pom.xml +++ b/container-disc/pom.xml @@ -176,8 +176,6 @@ docprocs-jar-with-dependencies.jar, hosted-zone-api-jar-with-dependencies.jar, jdisc-security-filters-jar-with-dependencies.jar, - model-evaluation-jar-with-dependencies.jar, - model-integration-jar-with-dependencies.jar, vespaclient-container-plugin-jar-with-dependencies.jar, vespa-athenz-jar-with-dependencies.jar, security-utils-jar-with-dependencies.jar, -- cgit v1.2.3 From 86d571c781d313dd3c89c5f3f8c7083a217ac09f Mon Sep 17 00:00:00 2001 From: gjoranv Date: Thu, 1 Jul 2021 22:56:19 +0200 Subject: Don't install physical bundles for test osgi frameworks. --- .../java/com/yahoo/container/core/config/HandlersConfigurerDi.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 9f6f5cf7ea1..08a468a3031 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 @@ -117,8 +117,11 @@ public class HandlersConfigurerDi { @Override public void installPlatformBundles(Collection bundlePaths) { - log.fine("Installing platform bundles."); - platformBundleLoader.useBundles(new ArrayList<>(bundlePaths)); + // Don't install physical bundles for test frameworks, where all platform bundles are on the classpath. + if (osgiFramework.isFelixFramework()) { + log.fine("Installing platform bundles."); + platformBundleLoader.useBundles(new ArrayList<>(bundlePaths)); + } } @Override -- cgit v1.2.3 From 181e6f43c3db62c7a2d19396400a5bce49450f2d Mon Sep 17 00:00:00 2001 From: gjoranv Date: Thu, 1 Jul 2021 23:00:06 +0200 Subject: Rename test cases for consistency. --- .../test/java/com/yahoo/application/container/ContainerTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/application/src/test/java/com/yahoo/application/container/ContainerTest.java b/application/src/test/java/com/yahoo/application/container/ContainerTest.java index 63347c2475a..29f895ce176 100644 --- a/application/src/test/java/com/yahoo/application/container/ContainerTest.java +++ b/application/src/test/java/com/yahoo/application/container/ContainerTest.java @@ -35,7 +35,7 @@ import static org.junit.Assert.fail; public class ContainerTest { @Test - public void jdisc_can_be_used_as_top_level_element() { + public void container_can_be_used_as_top_level_element() { try (JDisc container = fromServicesXml("" + // "" + // "", Networking.disable)) { @@ -44,7 +44,7 @@ public class ContainerTest { } @Test - public void jdisc_id_can_be_set() { + public void container_id_can_be_set() { try (JDisc container = fromServicesXml("" + // "" + // "", Networking.disable)) { @@ -53,7 +53,7 @@ public class ContainerTest { } @Test - public void jdisc_can_be_embedded_in_services_tag() { + public void container_can_be_embedded_in_services_tag() { try (JDisc container = fromServicesXml("" + // "" + // "" + // @@ -65,7 +65,7 @@ public class ContainerTest { @Test @SuppressWarnings("try") // container is unused inside the try block - public void multiple_jdisc_elements_gives_exception() { + public void multiple_container_elements_gives_exception() { try (JDisc container = fromServicesXml("" + // "" + // "" + // -- cgit v1.2.3