Merge pull request #29933 from vespa-engine/revert-29931-jonmv/reapply-defer-container-config-on-restart

Revert "Reapply defer container config changes when restarting"

14 files changed, 10 insertions, 69 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 04256ef2d9c..eb5942bd49c 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
@@ -43,7 +43,7 @@ public interface ModelContext {
     FileRegistry getFileRegistry();
     ExecutorService getExecutor();
     default Optional<? extends Reindexing> reindexing() { return Optional.empty(); }
-    default Set<ClusterSpec.Id> restartingClusters() { return Set.of(); }
+    default Set<ClusterSpec.Id> restartingClusters() { return Set.of(); } // TODO: Remove after 8.290 is gone.
     Properties properties();
     default Optional<File> appDir() { return Optional.empty(); }
     OnnxModelCost onnxModelCost();
diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
index 647745eba5d..f19341098f4 100644
--- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
+++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
@@ -27,7 +27,6 @@ import com.yahoo.config.model.application.provider.MockFileRegistry;
 import com.yahoo.config.model.provision.HostsXmlProvisioner;
 import com.yahoo.config.model.provision.SingleNodeProvisioner;
 import com.yahoo.config.model.test.MockApplicationPackage;
-import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.io.IOUtils;
@@ -93,7 +92,6 @@ public class DeployState implements ConfigDefinitionStore {
     private final Reindexing reindexing;
     private final ExecutorService executor;
     private final OnnxModelCost onnxModelCost;
-    private final Set<ClusterSpec.Id> restartingClusters;
 
     public static DeployState createTestState() {
         return new Builder().build();
@@ -129,8 +127,7 @@ public class DeployState implements ConfigDefinitionStore {
                         Optional<DockerImage> wantedDockerImageRepo,
                         Reindexing reindexing,
                         Optional<ValidationOverrides> validationOverrides,
-                        OnnxModelCost onnxModelCost,
-                        Set<ClusterSpec.Id> restartingClusters) {
+                        OnnxModelCost onnxModelCost) {
         this.logger = deployLogger;
         this.fileRegistry = fileRegistry;
         this.executor = executor;
@@ -159,7 +156,6 @@ public class DeployState implements ConfigDefinitionStore {
         this.wantedDockerImageRepo = wantedDockerImageRepo;
         this.reindexing = reindexing;
         this.onnxModelCost = onnxModelCost;
-        this.restartingClusters = Set.copyOf(restartingClusters);
     }
 
     public static HostProvisioner getDefaultModelHostProvisioner(ApplicationPackage applicationPackage) {
@@ -315,8 +311,6 @@ public class DeployState implements ConfigDefinitionStore {
 
     public OnnxModelCost onnxModelCost() { return onnxModelCost; }
 
-    public Set<ClusterSpec.Id> restartingClusters() { return restartingClusters; }
-
     public boolean isHostedTenantApplication(ApplicationType type) {
         boolean isTesterApplication = getProperties().applicationId().instance().isTester();
         return isHosted() && type == ApplicationType.DEFAULT && !isTesterApplication;
@@ -346,7 +340,6 @@ public class DeployState implements ConfigDefinitionStore {
         private Reindexing reindexing = null;
         private Optional<ValidationOverrides> validationOverrides = Optional.empty();
         private OnnxModelCost onnxModelCost = OnnxModelCost.disabled();
-        private Set<ClusterSpec.Id> restartingClusters = Set.of();
 
         public Builder() {}
 
@@ -466,11 +459,6 @@ public class DeployState implements ConfigDefinitionStore {
 
         public Builder onnxModelCost(OnnxModelCost instance) { this.onnxModelCost = instance; return this; }
 
-        public Builder restartingClusters(Set<ClusterSpec.Id> restartingClusters) {
-            this.restartingClusters = Set.copyOf(restartingClusters);
-            return this;
-        }
-
         public DeployState build() {
             return build(new ValidationParameters());
         }
@@ -504,8 +492,7 @@ public class DeployState implements ConfigDefinitionStore {
                                    wantedDockerImageRepo,
                                    reindexing,
                                    validationOverrides,
-                                   onnxModelCost,
-                                   restartingClusters);
+                                   onnxModelCost);
         }
 
     }
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 1b9d4004b5d..65572d07cc2 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
@@ -198,8 +198,7 @@ public class VespaModelFactory implements ModelFactory {
             .now(clock.instant())
             .wantedNodeVespaVersion(modelContext.wantedNodeVespaVersion())
             .wantedDockerImageRepo(modelContext.wantedDockerImageRepo())
-            .onnxModelCost(modelContext.onnxModelCost())
-            .restartingClusters(modelContext.restartingClusters());
+            .onnxModelCost(modelContext.onnxModelCost());
         modelContext.previousModel().ifPresent(builder::previousModel);
         modelContext.reindexing().ifPresent(builder::reindexing);
         return builder.build(validationParameters);
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 f36836092b8..ed7646b3066 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
@@ -50,7 +50,6 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-import java.util.logging.Level;
 import java.util.stream.Collectors;
 
 import static com.yahoo.vespa.model.container.docproc.DocprocChains.DOCUMENT_TYPE_MANAGER_CLASS;
@@ -152,10 +151,8 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
         registerApplicationBundles(deployState);
         registerUserConfiguredFiles(deployState);
         createEndpoints(deployState);
-        if (onnxModelCostCalculator.restartOnDeploy() || deployState.restartingClusters().contains(id())) {
-            deployState.getDeployLogger().log(Level.INFO, "Deferring config change until restart for cluster '" + id() + "'");
+        if (onnxModelCostCalculator.restartOnDeploy())
             setDeferChangesUntilRestart(true);
-        }
     }
 
     private void registerApplicationBundles(DeployState deployState) {
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 799e0bd62e8..aeb6c030a49 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
@@ -166,7 +166,7 @@ public abstract class ContainerCluster<CONTAINER extends Container>
     private String hostClusterId = null;
     private String jvmGCOptions = null;
 
-    private boolean deferChangesUntilRestart = false;
+    private volatile boolean deferChangesUntilRestart = false;
     private boolean clientsLegacyMode;
     private List<Client> clients = List.of();
 
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 c5449475136..22b2b581b44 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
@@ -22,7 +22,6 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.ClusterSpec;
-import com.yahoo.config.provision.ClusterSpec.Id;
 import com.yahoo.config.provision.DataplaneToken;
 import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.HostName;
@@ -40,7 +39,6 @@ import java.io.File;
 import java.net.URI;
 import java.security.cert.X509Certificate;
 import java.time.Duration;
-import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -69,7 +67,6 @@ public class ModelContextImpl implements ModelContext {
     private final ModelContext.Properties properties;
     private final Optional<File> appDir;
     private final OnnxModelCost onnxModelCost;
-    private final Set<ClusterSpec.Id> restartingClusters;
 
     private final Optional<DockerImage> wantedDockerImageRepository;
 
@@ -97,7 +94,6 @@ public class ModelContextImpl implements ModelContext {
                             ModelContext.Properties properties,
                             Optional<File> appDir,
                             OnnxModelCost onnxModelCost,
-                            Collection<Id> restartingClusters,
                             Optional<DockerImage> wantedDockerImageRepository,
                             Version modelVespaVersion,
                             Version wantedNodeVespaVersion) {
@@ -116,7 +112,6 @@ public class ModelContextImpl implements ModelContext {
         this.modelVespaVersion = modelVespaVersion;
         this.wantedNodeVespaVersion = wantedNodeVespaVersion;
         this.onnxModelCost = onnxModelCost;
-        this.restartingClusters = Set.copyOf(restartingClusters);
     }
 
     @Override
@@ -153,9 +148,6 @@ public class ModelContextImpl implements ModelContext {
     public Optional<? extends Reindexing> reindexing() { return  reindexing; }
 
     @Override
-    public Set<ClusterSpec.Id> restartingClusters() { return restartingClusters; }
-
-    @Override
     public ModelContext.Properties properties() { return properties; }
 
     @Override
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 02e49e3990c..ad785a33d5b 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
@@ -118,7 +118,6 @@ public class ActivatedModelsBuilder extends ModelsBuilder<Application> {
                 modelContextProperties,
                 Optional.empty(),
                 onnxModelCost,
-                zkClient.readActivationTriggers().restartingClusters(),
                 wantedDockerImageRepository,
                 modelFactory.version(),
                 wantedNodeVespaVersion);
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 894cedd0a34..fd8728ac655 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
@@ -128,7 +128,6 @@ public class PreparedModelsBuilder extends ModelsBuilder<PreparedModelsBuilder.P
                 createModelContextProperties(modelFactory.version(), applicationPackage),
                 getAppDir(applicationPackage),
                 onnxModelCost,
-                Set.of(), // Known after preparation and validation, passed from activated models builder only.
                 wantedDockerImageRepository,
                 modelVersion,
                 wantedNodeVespaVersion);
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 04db4b1b806..e704a36d21e 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
@@ -15,9 +15,9 @@ import java.util.List;
  *
  * @author jonmv
  */
-public record ActivationTriggers(List<NodeRestart> nodeRestarts, List<ClusterSpec.Id> restartingClusters, List<Reindexing> reindexings) {
+public record ActivationTriggers(List<NodeRestart> nodeRestarts, List<Reindexing> reindexings) {
 
-    private static final ActivationTriggers empty = new ActivationTriggers(List.of(), List.of(), List.of());
+    private static final ActivationTriggers empty = new ActivationTriggers(List.of(), List.of());
 
     public record NodeRestart(String hostname) { }
     public record Reindexing(String clusterId, String documentType) { }
@@ -30,11 +30,6 @@ public record ActivationTriggers(List<NodeRestart> nodeRestarts, List<ClusterSpe
                                                          .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 11a0f3cb935..4bad32ffee4 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
@@ -17,7 +17,6 @@ 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";
@@ -37,10 +36,6 @@ 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();
@@ -56,14 +51,11 @@ 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, restartingClusters, reindexings);
+        return new ActivationTriggers(nodeRestarts, reindexings);
     }
 
 }
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 27d82e90197..3289cc71357 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
@@ -16,7 +16,6 @@ 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.ClusterSpec.Id;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.container.jdisc.SecretStoreProvider;
 import com.yahoo.vespa.config.server.deploy.ModelContextImpl;
@@ -81,7 +80,6 @@ public class ModelContextImplTest {
                         List.of()),
                 Optional.empty(),
                 OnnxModelCost.disabled(),
-                List.of(Id.from("foobar")),
                 Optional.empty(),
                 new Version(7),
                 new Version(8));
@@ -90,7 +88,6 @@ public class ModelContextImplTest {
         assertFalse(context.previousModel().isPresent());
         assertTrue(context.getFileRegistry() instanceof MockFileRegistry);
         assertTrue(context.configDefinitionRepo() instanceof StaticConfigDefinitionRepo);
-        assertEquals(Set.of(Id.from("foobar")), context.restartingClusters());
         assertEquals(ApplicationId.defaultId(), context.properties().applicationId());
         assertTrue(context.properties().configServerSpecs().isEmpty());
         assertTrue(context.properties().multitenant());
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 a6f2eb38cc3..838b1b6b209 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
@@ -537,20 +537,6 @@ public class HostedDeployTest {
         assertEquals(Optional.of(ApplicationReindexing.empty()
                                                       .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 085fecaea16..5ef143198e8 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,6 +1,5 @@
 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;
@@ -18,7 +17,6 @@ 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 e182c3f557b..2fe46868562 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
@@ -178,7 +178,7 @@ public class SessionZooKeeperClientTest {
         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\":[],\"restartingClusters\":[],\"reindexings\":[]}"));
+                                 "\"activationTriggers\":{\"nodeRestarts\":[],\"reindexings\":[]}"));
     }
 
     private void assertApplicationIdParse(long sessionId, String idString, String expectedIdString) {