diff options
9 files changed, 64 insertions, 51 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index 1cf698af9cb..05df332987e 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -31,7 +31,9 @@ public interface ModelContext { ApplicationPackage applicationPackage(); Optional<Model> previousModel(); Optional<ApplicationPackage> permanentApplicationPackage(); - Optional<HostProvisioner> hostProvisioner(); + // TODO: Remove after 7.338 has been released + default Optional<HostProvisioner> hostProvisioner() { return Optional.of(getHostProvisioner()); } + HostProvisioner getHostProvisioner(); Provisioned provisioned(); DeployLogger deployLogger(); ConfigDefinitionRepo configDefinitionRepo(); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java index 5de03f17958..d6673cd49e9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model; import ai.vespa.rankingexpression.importer.configmodelview.MlModelImporter; @@ -11,7 +11,6 @@ import com.yahoo.config.model.MapConfigModelRegistry; import com.yahoo.config.model.NullConfigModelRegistry; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ConfigModelPlugin; -import com.yahoo.config.model.api.HostProvisioner; import com.yahoo.config.model.api.Model; import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.api.ModelCreateResult; @@ -22,10 +21,8 @@ import com.yahoo.config.model.builder.xml.ConfigModelBuilder; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.TransientException; import com.yahoo.config.provision.Zone; -import com.yahoo.container.QrConfig; import com.yahoo.vespa.config.VespaVersion; import com.yahoo.vespa.model.application.validation.Validation; -import com.yahoo.vespa.model.container.ApplicationContainerCluster; import org.xml.sax.SAXException; import java.io.IOException; @@ -147,7 +144,7 @@ public class VespaModelFactory implements ModelFactory { .permanentApplicationPackage(modelContext.permanentApplicationPackage()) .properties(modelContext.properties()) .vespaVersion(version()) - .modelHostProvisioner(createHostProvisioner(modelContext)) + .modelHostProvisioner(modelContext.getHostProvisioner()) .provisioned(modelContext.provisioned()) .endpoints(modelContext.properties().endpoints()) .modelImporters(modelImporters) @@ -160,11 +157,6 @@ public class VespaModelFactory implements ModelFactory { return builder.build(validationParameters); } - private static HostProvisioner createHostProvisioner(ModelContext modelContext) { - return modelContext.hostProvisioner().orElse( - DeployState.getDefaultModelHostProvisioner(modelContext.applicationPackage())); - } - private void validateXML(ApplicationPackage applicationPackage, boolean ignoreValidationErrors) { try { applicationPackage.validateXML(); diff --git a/config-model/src/test/java/com/yahoo/config/model/MockModelContext.java b/config-model/src/test/java/com/yahoo/config/model/MockModelContext.java index f8ab3cc54c8..98cbd363bca 100644 --- a/config-model/src/test/java/com/yahoo/config/model/MockModelContext.java +++ b/config-model/src/test/java/com/yahoo/config/model/MockModelContext.java @@ -13,6 +13,7 @@ import com.yahoo.config.model.api.Provisioned; import com.yahoo.config.model.application.provider.BaseDeployLogger; import com.yahoo.config.model.application.provider.MockFileRegistry; import com.yahoo.config.model.application.provider.StaticConfigDefinitionRepo; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.deploy.TestProperties; import com.yahoo.config.model.test.MockApplicationPackage; @@ -54,6 +55,11 @@ public class MockModelContext implements ModelContext { } @Override + public HostProvisioner getHostProvisioner() { + return DeployState.getDefaultModelHostProvisioner(applicationPackage); + } + + @Override public Provisioned provisioned() { return new Provisioned(); } @Override diff --git a/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java b/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java index a9bf8bdcc49..33f9d715801 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java @@ -146,6 +146,9 @@ public class VespaModelFactoryTest { } @Override + public HostProvisioner getHostProvisioner() { return provisionerToOverride; } + + @Override public Properties properties() { return new TestProperties(); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index 14d156d39fb..5398a2cd190 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -49,7 +49,7 @@ public class ModelContextImpl implements ModelContext { private final DeployLogger deployLogger; private final ConfigDefinitionRepo configDefinitionRepo; private final FileRegistry fileRegistry; - private final Optional<HostProvisioner> hostProvisioner; + private final HostProvisioner hostProvisioner; private final Provisioned provisioned; private final Optional<? extends Reindexing> reindexing; private final ModelContext.Properties properties; @@ -76,7 +76,7 @@ public class ModelContextImpl implements ModelContext { ConfigDefinitionRepo configDefinitionRepo, FileRegistry fileRegistry, Optional<? extends Reindexing> reindexing, - Optional<HostProvisioner> hostProvisioner, + HostProvisioner hostProvisioner, Provisioned provisioned, ModelContext.Properties properties, Optional<File> appDir, @@ -112,9 +112,8 @@ public class ModelContextImpl implements ModelContext { * Returns the host provisioner to use, or empty to use the default provisioner, * creating hosts from the application package defined hosts */ - // TODO: Don't allow empty here but create the right provisioner when this is set up instead @Override - public Optional<HostProvisioner> hostProvisioner() { return hostProvisioner; } + public HostProvisioner getHostProvisioner() { return hostProvisioner; } @Override public Provisioned provisioned() { return provisioned; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java index 0a81c408ef4..fa058514d17 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ActivatedModelsBuilder.java @@ -102,9 +102,7 @@ public class ActivatedModelsBuilder extends ModelsBuilder<Application> { configDefinitionRepo, getForVersionOrLatest(applicationPackage.getFileRegistries(), modelFactory.version()).orElse(new MockFileRegistry()), new ApplicationCuratorDatabase(tenant, curator).readReindexingStatus(applicationId), - createStaticProvisioner(applicationPackage.getAllocatedHosts(), - modelContextProperties.applicationId(), - provisioned), + createStaticProvisioner(applicationPackage, modelContextProperties.applicationId(), provisioned), provisioned, modelContextProperties, Optional.empty(), diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java index 245b9db020b..823c82d3e14 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java @@ -1,22 +1,22 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.modelfactory; import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.cloud.config.ConfigserverConfig; +import com.yahoo.component.Version; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.api.HostProvisioner; import com.yahoo.config.model.api.ModelFactory; import com.yahoo.config.model.api.Provisioned; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationLockException; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.OutOfCapacityException; -import com.yahoo.component.Version; import com.yahoo.config.provision.TransientException; import com.yahoo.config.provision.Zone; import com.yahoo.lang.SettableOptional; -import java.util.logging.Level; import com.yahoo.vespa.config.server.http.InternalServerException; import com.yahoo.vespa.config.server.http.UnknownVespaVersionException; import com.yahoo.vespa.config.server.provision.HostProvisionerProvider; @@ -31,6 +31,7 @@ import java.util.List; import java.util.NoSuchElementException; import java.util.Optional; import java.util.Set; +import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -48,7 +49,7 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { protected final ConfigserverConfig configserverConfig; /** True if we are running in hosted mode */ - private final boolean hosted; + protected final boolean hosted; private final Zone zone; @@ -250,12 +251,20 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { * returns empty otherwise, which may either mean that no hosts are allocated or that we are running * non-hosted and should default to use hosts defined in the application package, depending on context */ - Optional<HostProvisioner> createStaticProvisioner(Optional<AllocatedHosts> allocatedHosts, - ApplicationId applicationId, - Provisioned provisioned) { + HostProvisioner createStaticProvisioner(ApplicationPackage applicationPackage, + ApplicationId applicationId, + Provisioned provisioned) { + Optional<AllocatedHosts> allocatedHosts = applicationPackage.getAllocatedHosts(); if (hosted && allocatedHosts.isPresent()) - return Optional.of(new StaticProvisioner(allocatedHosts.get(), createNodeRepositoryProvisioner(applicationId, provisioned).get())); - return Optional.empty(); + return createStaticProvisionerForHosted(allocatedHosts.get(), createNodeRepositoryProvisioner(applicationId, provisioned).get()); + return DeployState.getDefaultModelHostProvisioner(applicationPackage); + } + + /** + * Returns a host provisioner returning the previously allocated hosts + */ + HostProvisioner createStaticProvisionerForHosted(AllocatedHosts allocatedHosts, HostProvisioner nodeRepositoryProvisioner) { + return new StaticProvisioner(allocatedHosts, nodeRepositoryProvisioner); } Optional<HostProvisioner> createNodeRepositoryProvisioner(ApplicationId applicationId, Provisioned provisioned) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/PreparedModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/PreparedModelsBuilder.java index a064e8a9cac..7606aacff15 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/PreparedModelsBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/PreparedModelsBuilder.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.modelfactory; import com.yahoo.cloud.config.ConfigserverConfig; @@ -17,6 +17,7 @@ import com.yahoo.config.model.api.Provisioned; import com.yahoo.config.model.api.ValidationParameters; import com.yahoo.config.model.api.ValidationParameters.IgnoreValidationErrors; import com.yahoo.config.model.application.provider.FilesApplicationPackage; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.DockerImage; @@ -28,7 +29,6 @@ import com.yahoo.vespa.config.server.deploy.ModelContextImpl; import com.yahoo.vespa.config.server.filedistribution.FileDistributionProvider; import com.yahoo.vespa.config.server.host.HostValidator; import com.yahoo.vespa.config.server.provision.HostProvisionerProvider; -import com.yahoo.vespa.config.server.provision.StaticProvisioner; import com.yahoo.vespa.config.server.session.PrepareParams; import com.yahoo.vespa.curator.Curator; @@ -101,7 +101,7 @@ public class PreparedModelsBuilder extends ModelsBuilder<PreparedModelsBuilder.P configDefinitionRepo, fileDistributionProvider.getFileRegistry(), new ApplicationCuratorDatabase(applicationId.tenant(), curator).readReindexingStatus(applicationId), - createHostProvisioner(allocatedHosts, provisioned), + createHostProvisioner(applicationPackage, provisioned), provisioned, properties, getAppDir(applicationPackage), @@ -119,7 +119,7 @@ public class PreparedModelsBuilder extends ModelsBuilder<PreparedModelsBuilder.P log.log(Level.FINE, "Done building model " + modelVersion + " for " + applicationId); params.getTimeoutBudget().assertNotTimedOut(() -> "prepare timed out after building model " + modelVersion + " (timeout " + params.getTimeoutBudget().timeout() + "): " + applicationId); - return new PreparedModelsBuilder.PreparedModelResult(modelVersion, result.getModel(), fileDistributionProvider, result.getConfigChangeActions()); + return new PreparedModelResult(modelVersion, result.getModel(), fileDistributionProvider, result.getConfigChangeActions()); } private Optional<Model> modelOf(Version version) { @@ -127,22 +127,19 @@ public class PreparedModelsBuilder extends ModelsBuilder<PreparedModelsBuilder.P return currentActiveApplicationSet.get().get(version).map(Application::getModel); } - // This method is an excellent demonstration of what happens when one is too liberal with Optional - // -bratseth, who had to write the below :-\ - private Optional<HostProvisioner> createHostProvisioner(Optional<AllocatedHosts> allocatedHosts, - Provisioned provisioned) { - Optional<HostProvisioner> nodeRepositoryProvisioner = createNodeRepositoryProvisioner(properties.applicationId(), - provisioned); - if (allocatedHosts.isEmpty()) return nodeRepositoryProvisioner; - - Optional<HostProvisioner> staticProvisioner = createStaticProvisioner(allocatedHosts, - properties.applicationId(), - provisioned); - if (staticProvisioner.isEmpty()) return Optional.empty(); // Since we have hosts allocated this means we are on non-hosted - + private HostProvisioner createHostProvisioner(ApplicationPackage applicationPackage, Provisioned provisioned) { + HostProvisioner defaultHostProvisioner = DeployState.getDefaultModelHostProvisioner(applicationPackage); + // Note: nodeRepositoryProvisioner will always be present when hosted is true + Optional<HostProvisioner> nodeRepositoryProvisioner = createNodeRepositoryProvisioner(properties.applicationId(), provisioned); + Optional<AllocatedHosts> allocatedHosts = applicationPackage.getAllocatedHosts(); + + if (allocatedHosts.isEmpty()) return nodeRepositoryProvisioner.orElse(defaultHostProvisioner); + // Nodes are already allocated by a model and we should use them unless this model requests hosts from a // previously unallocated cluster. This allows future models to stop allocate certain clusters. - return Optional.of(new StaticProvisioner(allocatedHosts.get(), nodeRepositoryProvisioner.get())); + if (hosted) return createStaticProvisionerForHosted(allocatedHosts.get(), nodeRepositoryProvisioner.get()); + + return defaultHostProvisioner; } private Optional<File> getAppDir(ApplicationPackage applicationPackage) { @@ -166,11 +163,13 @@ public class PreparedModelsBuilder extends ModelsBuilder<PreparedModelsBuilder.P public final Version version; public final Model model; - public final com.yahoo.vespa.config.server.filedistribution.FileDistributionProvider fileDistributionProvider; + public final FileDistributionProvider fileDistributionProvider; public final List<ConfigChangeAction> actions; - public PreparedModelResult(Version version, Model model, - com.yahoo.vespa.config.server.filedistribution.FileDistributionProvider fileDistributionProvider, List<ConfigChangeAction> actions) { + public PreparedModelResult(Version version, + Model model, + FileDistributionProvider fileDistributionProvider, + List<ConfigChangeAction> actions) { this.version = version; this.model = model; this.fileDistributionProvider = fileDistributionProvider; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java index d094ad09bec..3e27e0b61ea 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelContextImplTest.java @@ -3,11 +3,14 @@ package com.yahoo.vespa.config.server; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.Version; +import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.api.ContainerEndpoint; +import com.yahoo.config.model.api.HostProvisioner; import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.api.Provisioned; import com.yahoo.config.model.application.provider.BaseDeployLogger; import com.yahoo.config.model.application.provider.MockFileRegistry; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Zone; @@ -45,15 +48,17 @@ public class ModelContextImplTest { .hostedVespa(false) .build(); + ApplicationPackage applicationPackage = MockApplicationPackage.createEmpty(); + HostProvisioner hostProvisioner = DeployState.getDefaultModelHostProvisioner(applicationPackage); ModelContext context = new ModelContextImpl( - MockApplicationPackage.createEmpty(), + applicationPackage, Optional.empty(), Optional.empty(), new BaseDeployLogger(), new StaticConfigDefinitionRepo(), new MockFileRegistry(), Optional.empty(), - Optional.empty(), + hostProvisioner, new Provisioned(), new ModelContextImpl.Properties( ApplicationId.defaultId(), @@ -72,7 +77,7 @@ public class ModelContextImplTest { new Version(7), new Version(8)); assertTrue(context.applicationPackage() instanceof MockApplicationPackage); - assertFalse(context.hostProvisioner().isPresent()); + assertEquals(hostProvisioner, context.getHostProvisioner()); assertFalse(context.permanentApplicationPackage().isPresent()); assertFalse(context.previousModel().isPresent()); assertTrue(context.getFileRegistry() instanceof MockFileRegistry); |