diff options
10 files changed, 66 insertions, 79 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 31ef1e75acd..9b75f4bcdda 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 @@ -44,7 +44,7 @@ public interface ModelContext { ExecutorService getExecutor(); default Optional<? extends Reindexing> reindexing() { return Optional.empty(); } Properties properties(); - default Optional<File> appDir() { return Optional.empty();} + default Optional<File> appDir() { return Optional.empty(); } OnnxModelCost onnxModelCost(); /** The Docker image repo we want to use for images for this deployment (optional, will use default if empty) */ diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java index dc7a2651e1f..56b7e6ee0c8 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java @@ -74,8 +74,6 @@ public class Validation { } else if (deployState.getPreviousModel().isPresent() && (deployState.getPreviousModel().get() instanceof VespaModel)) { validateChanges(execution); - // TODO: Why is this done here? It won't be done on more than one config server? - deferConfigChangesForClustersToBeRestarted(execution.actions, model); } execution.throwIfFailed(); @@ -138,22 +136,6 @@ public class Validation { new RestartOnDeployForOnnxModelChangesValidator().validate(execution); } - private static void deferConfigChangesForClustersToBeRestarted(List<ConfigChangeAction> actions, VespaModel model) { - Set<ClusterSpec.Id> clustersToBeRestarted = actions.stream() - .filter(action -> action.getType() == ConfigChangeAction.Type.RESTART) - .map(action -> action.clusterId()) - .collect(Collectors.toCollection(() -> new LinkedHashSet<>())); - for (var clusterToRestart : clustersToBeRestarted) { - var containerCluster = model.getContainerClusters().get(clusterToRestart.value()); - if (containerCluster != null) - containerCluster.setDeferChangesUntilRestart(true); - - var contentCluster = model.getContentClusters().get(clusterToRestart.value()); - if (contentCluster != null) - contentCluster.setDeferChangesUntilRestart(true); - } - } - public interface Context { /** Auxiliary deploy state of the application. */ DeployState deployState(); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartChangesDefersConfigChangesTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartChangesDefersConfigChangesTest.java deleted file mode 100644 index 35681095144..00000000000 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartChangesDefersConfigChangesTest.java +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.application.validation.change; - -import com.yahoo.config.model.provision.InMemoryProvisioner; -import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.NodeResources; -import com.yahoo.container.ComponentsConfig; -import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.ValidationTester; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - -/** - * @author bratseth - */ -public class RestartChangesDefersConfigChangesTest { - - @Test - void changes_requiring_restart_defers_config_changes() { - ValidationTester tester = new ValidationTester(new InMemoryProvisioner(5, - new NodeResources(1, 3, 9, 1), - true)); - VespaModel gen1 = tester.deploy(null, getServices(5, 3), Environment.prod, null).getFirst(); - - // Change node count - no restart - VespaModel gen2 = tester.deploy(gen1, getServices(4, 3), Environment.prod, null).getFirst(); - var config2 = new ComponentsConfig.Builder(); - gen2.getContainerClusters().get("default").getContainers().get(0).getConfig(config2); - assertFalse(config2.getApplyOnRestart()); - - // Change memory amount - requires restart - VespaModel gen3 = tester.deploy(gen2, getServices(4, 2), Environment.prod, null).getFirst(); - var config3 = new ComponentsConfig.Builder(); - gen3.getContainerClusters().get("default").getContainers().get(0).getConfig(config3); - assertTrue(config3.getApplyOnRestart()); - } - - private static String getServices(int nodes, int memory) { - return "<services version='1.0'>" + - " <container id='default' version='1.0'>" + - " <nodes count='" + nodes + "'><resources vcpu='1' memory='" + memory + "Gb' disk='9Gb'/></nodes>" + - " </container>" + - "</services>"; - } - -} 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 ed134db6452..64ccd910120 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 @@ -13,6 +13,7 @@ import com.yahoo.config.model.api.OnnxModelCost; import com.yahoo.config.model.api.Provisioned; import com.yahoo.config.model.application.provider.MockFileRegistry; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; @@ -33,6 +34,10 @@ import com.yahoo.vespa.config.server.tenant.EndpointCertificateRetriever; import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.model.VespaModel; +import com.yahoo.vespa.model.container.ApplicationContainerCluster; +import com.yahoo.vespa.model.content.cluster.ContentCluster; + import java.util.Comparator; import java.util.Map; import java.util.Optional; @@ -118,7 +123,7 @@ public class ActivatedModelsBuilder extends ModelsBuilder<Application> { wantedNodeVespaVersion); MetricUpdater applicationMetricUpdater = metrics.getOrCreateMetricUpdater(Metrics.createDimensions(applicationId)); ServerCache serverCache = new ServerCache(configDefinitionRepo, zkClient.getUserConfigDefinitions()); - return new Application(modelFactory.createModel(modelContext), + return new Application(withDeferredConfigForRestartingClusters(modelFactory.createModel(modelContext)), serverCache, applicationGeneration, modelFactory.version(), @@ -165,4 +170,15 @@ public class ActivatedModelsBuilder extends ModelsBuilder<Application> { zkClient.readDataplaneTokens()); } + private Model withDeferredConfigForRestartingClusters(Model model) { + if ( ! (model instanceof VespaModel vespaModel)) return model; + for (ClusterSpec.Id cluster : zkClient.readActivationTriggers().restartingClusters()) { + ApplicationContainerCluster containerCluster = vespaModel.getContainerClusters().get(cluster.value()); + if (containerCluster != null) containerCluster.setDeferChangesUntilRestart(true); + ContentCluster contentCluster = vespaModel.getContentClusters().get(cluster.value()); + if (contentCluster != null) contentCluster.setDeferChangesUntilRestart(true); + } + return model; + } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/ActivationTriggers.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/ActivationTriggers.java index 191f936c333..04db4b1b806 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/ActivationTriggers.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/ActivationTriggers.java @@ -1,5 +1,6 @@ package com.yahoo.vespa.config.server.session; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.config.server.configchange.ConfigChangeActions; import java.util.List; @@ -14,9 +15,9 @@ import java.util.List; * * @author jonmv */ -public record ActivationTriggers(List<NodeRestart> nodeRestarts, List<Reindexing> reindexings) { +public record ActivationTriggers(List<NodeRestart> nodeRestarts, List<ClusterSpec.Id> restartingClusters, List<Reindexing> reindexings) { - private static final ActivationTriggers empty = new ActivationTriggers(List.of(), List.of()); + private static final ActivationTriggers empty = new ActivationTriggers(List.of(), List.of(), List.of()); public record NodeRestart(String hostname) { } public record Reindexing(String clusterId, String documentType) { } @@ -29,6 +30,11 @@ public record ActivationTriggers(List<NodeRestart> nodeRestarts, List<Reindexing .hostnames().stream() .map(NodeRestart::new) .toList(), + configChangeActions.getRestartActions() + .useForInternalRestart(isInternalRedeployment) + .getEntries().stream() + .map(entry -> ClusterSpec.Id.from(entry.getClusterName())) + .toList(), configChangeActions.getReindexActions().getEntries().stream() .map(entry -> new Reindexing(entry.getClusterName(), entry.getDocumentType())) .toList()); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/ActivationTriggersSerializer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/ActivationTriggersSerializer.java index 5f67de0ccca..11a0f3cb935 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/ActivationTriggersSerializer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/ActivationTriggersSerializer.java @@ -1,5 +1,6 @@ package com.yahoo.vespa.config.server.session; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; @@ -16,6 +17,7 @@ import static com.yahoo.yolean.Exceptions.uncheck; public class ActivationTriggersSerializer { static final String NODE_RESTARTS = "nodeRestarts"; + static final String RESTARTING_CLUSTERS = "restartingClusters"; static final String REINDEXINGS = "reindexings"; static final String CLUSTER_NAME = "clusterName"; static final String DOCUMENT_TYPE = "documentType"; @@ -35,6 +37,10 @@ public class ActivationTriggersSerializer { for (NodeRestart nodeRestart : triggers.nodeRestarts()) nodeRestarts.addString(nodeRestart.hostname()); + Cursor restartingClusters = object.setArray(RESTARTING_CLUSTERS); + for (ClusterSpec.Id clusterId : triggers.restartingClusters()) + restartingClusters.addString(clusterId.value()); + Cursor reindexings = object.setArray(REINDEXINGS); for (Reindexing reindexing : triggers.reindexings()) { Cursor entry = reindexings.addObject(); @@ -50,11 +56,14 @@ public class ActivationTriggersSerializer { List<NodeRestart> nodeRestarts = SlimeUtils.entriesStream(object.field(NODE_RESTARTS)) .map(entry -> new NodeRestart(entry.asString())) .toList(); + List<ClusterSpec.Id> restartingClusters = SlimeUtils.entriesStream(object.field(RESTARTING_CLUSTERS)) + .map(entry -> ClusterSpec.Id.from(entry.asString())) + .toList(); List<Reindexing> reindexings = SlimeUtils.entriesStream(object.field(REINDEXINGS)) .map(entry -> new Reindexing(entry.field(CLUSTER_NAME).asString(), entry.field(DOCUMENT_TYPE).asString())) .toList(); - return new ActivationTriggers(nodeRestarts, reindexings); + return new ActivationTriggers(nodeRestarts, restartingClusters, reindexings); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java index e1b1f2b193f..24c98edd9aa 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java @@ -22,8 +22,11 @@ import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; +import com.yahoo.container.ComponentsConfig; import com.yahoo.slime.SlimeUtils; import com.yahoo.test.ManualClock; +import com.yahoo.vespa.config.GetConfigRequest; +import com.yahoo.vespa.config.search.core.ProtonConfig; import com.yahoo.vespa.config.server.MockConfigConvergenceChecker; import com.yahoo.vespa.config.server.application.ApplicationReindexing; import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker; @@ -33,6 +36,7 @@ import com.yahoo.vespa.config.server.http.UnknownVespaVersionException; import com.yahoo.vespa.config.server.http.v2.PrepareResult; import com.yahoo.vespa.config.server.model.TestModelFactory; import com.yahoo.vespa.config.server.session.PrepareParams; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.change.VespaReindexAction; import com.yahoo.vespa.model.application.validation.change.VespaRestartAction; import org.junit.Rule; @@ -48,6 +52,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.stream.IntStream; import static com.yahoo.vespa.config.server.deploy.DeployTester.CountingModelFactory; @@ -497,14 +502,15 @@ public class HostedDeployTest { @Test public void testThatAllowedConfigChangeActionsAreActedUpon() { List<Host> hosts = createHosts(9, "6.1.0"); - List<ServiceInfo> services = createServices(1); + List<ServiceInfo> searchServices = List.of(new ServiceInfo("proton", "searchnode", null, Map.of("clustername", "music"), "configid", "host")); + List<ServiceInfo> containerServices = List.of(new ServiceInfo("jdisc", "container", null, Map.of("clustername", "container"), "configid", "host")); ManualClock clock = new ManualClock(Instant.EPOCH); List<ModelFactory> modelFactories = List.of( new ConfigChangeActionsModelFactory(Version.fromString("6.1.0"), - VespaReindexAction.of(ClusterSpec.Id.from("test"), ValidationId.indexModeChange, - "reindex please", services, "music"), - new VespaRestartAction(ClusterSpec.Id.from("test"), "change", services))); + VespaReindexAction.of(ClusterSpec.Id.from("music"), ValidationId.indexModeChange, + "reindex please", searchServices, "music"), + new VespaRestartAction(ClusterSpec.Id.from("container"), "change", containerServices))); DeployTester tester = new DeployTester.Builder(temporaryFolder) .modelFactories(modelFactories) @@ -519,8 +525,22 @@ public class HostedDeployTest { assertEquals(9, tester.getAllocatedHostsOf(tester.applicationId()).getHosts().size()); assertTrue(prepareResult.configChangeActions().getRestartActions().isEmpty()); // Handled by deployment. assertEquals(Optional.of(ApplicationReindexing.empty() - .withPending("cluster0", "music", prepareResult.sessionId())), + .withPending("music", "music", prepareResult.sessionId())), tester.tenant().getApplicationRepo().database().readReindexingStatus(tester.applicationId())); + + VespaModel model = ((VespaModel) tester.tenant().getSessionRepository() + .activeApplicationVersions(tester.applicationId()).get().get(Version.fromString("6.1.0")).get() + .getModel()); + + // Config for the container cluster to be restarted has been deferred until after restart. + ComponentsConfig.Builder builder1 = new ComponentsConfig.Builder(); + model.getContainerClusters().get("container").getContainers().get(0).getConfig(builder1); + assertTrue(builder1.getApplyOnRestart()); + + // Config for the metricsproxy cluster, which is not restarted, has not been deferred until after restart. + ComponentsConfig.Builder builder2 = new ComponentsConfig.Builder(); + model.getAdmin().getMetricsProxyCluster().getContainers().get(0).getConfig(builder2); + assertFalse(builder2.getApplyOnRestart()); } @Test diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/ActivationTriggersSerializerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/ActivationTriggersSerializerTest.java index 97f079d9ee3..085fecaea16 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/ActivationTriggersSerializerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/ActivationTriggersSerializerTest.java @@ -1,5 +1,6 @@ package com.yahoo.vespa.config.server.session; +import com.yahoo.config.provision.ClusterSpec.Id; import com.yahoo.vespa.config.server.session.ActivationTriggers.NodeRestart; import com.yahoo.vespa.config.server.session.ActivationTriggers.Reindexing; import org.junit.jupiter.api.Test; @@ -7,7 +8,6 @@ import org.junit.jupiter.api.Test; import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.fail; /** * @author jonmv @@ -18,6 +18,7 @@ class ActivationTriggersSerializerTest { void testSerialization() { ActivationTriggers triggers = new ActivationTriggers(List.of(new NodeRestart("node1"), new NodeRestart("node2")), + List.of(Id.from("cluster1")), List.of(new Reindexing("cluster1", "type1"), new Reindexing("cluster1", "type2"), new Reindexing("cluster2", "type1"))); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClientTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClientTest.java index a7892bd4c56..e182c3f557b 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClientTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClientTest.java @@ -177,7 +177,8 @@ public class SessionZooKeeperClientTest { assertTrue(curator.exists(path)); String data = Utf8.toString(curator.getData(path).get()); assertTrue(data.contains("{\"applicationId\":\"default:default:default\",\"applicationPackageReference\":\"foo\",\"version\":\"8.195.1\",\"createTime\":")); - assertTrue(data.contains(",\"tenantSecretStores\":[],\"operatorCertificates\":[],\"dataplaneTokens\":[],\"activationTriggers\":{\"nodeRestarts\":[],\"reindexings\":[]}")); + assertTrue(data.contains(",\"tenantSecretStores\":[],\"operatorCertificates\":[],\"dataplaneTokens\":[]," + + "\"activationTriggers\":{\"nodeRestarts\":[],\"restartingClusters\":[],\"reindexings\":[]}")); } private void assertApplicationIdParse(long sessionId, String idString, String expectedIdString) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java index bdff94e011d..601fa528e34 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java @@ -43,7 +43,7 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer { try { return deployer().deployTime(application) .map(lastDeployTime -> lastDeployTime.isBefore(nodeRepository().clock().instant().minus(minTimeBetweenRedeployments)) - || deployer().readiedReindexingAfter(application, lastDeployTime)) + || deployer().readiedReindexingAfter(application, lastDeployTime)) .orElse(false); } catch (Exception e) { |