Support ignoring actions for internal redeploy

10 files changed, 126 insertions, 47 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeAction.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeAction.java
index e453aaf5bdb..ea4b6a2b02d 100644
--- a/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeAction.java
+++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ConfigChangeAction.java
@@ -38,4 +38,7 @@ public interface ConfigChangeAction {
     /** Returns whether this change should be allowed */
     boolean allowed();
 
+    /** Returns whether this change should be ignored for internal redeploy */
+    boolean ignoreForInternalRedeploy();
+
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidator.java
index 5cee08ea9af..3176ad9f912 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidator.java
@@ -31,7 +31,7 @@ public class ContainerRestartValidator implements ChangeValidator {
     }
 
     private static ConfigChangeAction createConfigChangeAction(Container container) {
-        return new VespaRestartAction(createMessage(container), container.getServiceInfo());
+        return new VespaRestartAction(createMessage(container), container.getServiceInfo(), true);
     }
 
     private static String createMessage(Container container) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRefeedAction.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRefeedAction.java
index 0fd38e5dbdd..be43c6eddfb 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRefeedAction.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRefeedAction.java
@@ -6,7 +6,6 @@ import com.yahoo.config.model.api.ServiceInfo;
 import com.yahoo.config.application.api.ValidationOverrides;
 
 import java.time.Instant;
-import java.util.Collections;
 import java.util.List;
 
 /**
@@ -27,31 +26,29 @@ public class VespaRefeedAction extends VespaConfigChangeAction implements Config
 
     private final String documentType;
     private final boolean allowed;
-    private final Instant now;
 
-    private VespaRefeedAction(String name, String message, List<ServiceInfo> services, String documentType, boolean allowed, Instant now) {
+    private VespaRefeedAction(String name, String message, List<ServiceInfo> services, String documentType, boolean allowed) {
         super(message, services);
         this.name = name;
         this.documentType = documentType;
         this.allowed = allowed;
-        this.now = now;
     }
 
     /** Creates a refeed action with some missing information */
     // TODO: We should require document type or model its absence properly
     public static VespaRefeedAction of(String name, ValidationOverrides overrides, String message, Instant now) {
-        return new VespaRefeedAction(name, message, Collections.emptyList(), "", overrides.allows(name, now), now);
+        return new VespaRefeedAction(name, message, List.of(), "", overrides.allows(name, now));
     }
 
     /** Creates a refeed action */
     public static VespaRefeedAction of(String name, ValidationOverrides overrides, String message,
                                        List<ServiceInfo> services, String documentType, Instant now) {
-        return new VespaRefeedAction(name, message, services, documentType, overrides.allows(name, now), now);
+        return new VespaRefeedAction(name, message, services, documentType, overrides.allows(name, now));
     }
 
     @Override
     public VespaConfigChangeAction modifyAction(String newMessage, List<ServiceInfo> newServices, String documentType) {
-        return new VespaRefeedAction(name, newMessage, newServices, documentType, allowed, now);
+        return new VespaRefeedAction(name, newMessage, newServices, documentType, allowed);
     }
 
     @Override
@@ -64,6 +61,11 @@ public class VespaRefeedAction extends VespaConfigChangeAction implements Config
     public boolean allowed() { return allowed; }
 
     @Override
+    public boolean ignoreForInternalRedeploy() {
+        return false;
+    }
+
+    @Override
     public String toString() {
         return super.toString() + ", documentType='" + documentType + "'";
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRestartAction.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRestartAction.java
index d42db15d062..3ea18cac1d6 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRestartAction.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/VespaRestartAction.java
@@ -4,8 +4,6 @@ package com.yahoo.vespa.model.application.validation.change;
 import com.yahoo.config.model.api.ConfigChangeRestartAction;
 import com.yahoo.config.model.api.ServiceInfo;
 
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 /**
@@ -15,16 +13,24 @@ import java.util.List;
  */
 public class VespaRestartAction extends VespaConfigChangeAction implements ConfigChangeRestartAction {
 
+    private final boolean ignoreForInternalRedeploy;
+
     public VespaRestartAction(String message) {
-        super(message, new ArrayList<>());
+        this(message, List.of());
     }
 
     public VespaRestartAction(String message, ServiceInfo service) {
-        super(message, Collections.singletonList(service));
+        this(message, List.of(service));
+    }
+
+    public VespaRestartAction(String message, ServiceInfo services, boolean ignoreForInternalRedeploy) {
+        super(message, List.of(services));
+        this.ignoreForInternalRedeploy = ignoreForInternalRedeploy;
     }
 
     public VespaRestartAction(String message, List<ServiceInfo> services) {
         super(message, services);
+        this.ignoreForInternalRedeploy = false;
     }
 
     @Override
@@ -32,4 +38,25 @@ public class VespaRestartAction extends VespaConfigChangeAction implements Confi
         return new VespaRestartAction(newMessage, newServices);
     }
 
+    @Override
+    public boolean ignoreForInternalRedeploy() {
+        return ignoreForInternalRedeploy;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+
+        VespaRestartAction that = (VespaRestartAction) o;
+        return ignoreForInternalRedeploy == that.ignoreForInternalRedeploy;
+    }
+
+    @Override
+    public int hashCode() {
+        int result = super.hashCode();
+        result = 31 * result + (ignoreForInternalRedeploy ? 1 : 0);
+        return result;
+    }
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
index 57502973873..4670184c303 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
@@ -402,15 +402,11 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
                                   TimeoutBudget timeoutBudget,
                                   boolean force) {
         LocalSession localSession = getLocalSession(tenant, sessionId);
-        Deployment deployment = deployment(localSession, tenant, timeoutBudget.timeLeft(), force);
+        Deployment deployment = Deployment.prepared(localSession, this, hostProvisioner, tenant, logger, timeoutBudget.timeout(), clock, false, force);
         deployment.activate();
         return localSession.getApplicationId();
     }
 
-    private Deployment deployment(LocalSession session, Tenant tenant, Duration timeout, boolean force) {
-        return Deployment.prepared(session, this, hostProvisioner, tenant, logger, timeout, clock, false, force);
-    }
-
     public Transaction deactivateCurrentActivateNew(Session active, LocalSession prepared, boolean force) {
         Tenant tenant = tenantRepository.getTenant(prepared.getTenantName());
         Transaction transaction = tenant.getSessionRepository().createActivateTransaction(prepared);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RestartActions.java b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RestartActions.java
index 4db5a5e125f..326f146f64c 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RestartActions.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/configchange/RestartActions.java
@@ -5,6 +5,7 @@ import com.yahoo.config.model.api.ConfigChangeAction;
 import com.yahoo.config.model.api.ServiceInfo;
 
 import java.util.*;
+import java.util.stream.Collectors;
 
 /**
  * Represents all actions to restart services in order to handle a config change.
@@ -18,6 +19,7 @@ public class RestartActions {
         private final String clusterName;
         private final String clusterType;
         private final String serviceType;
+        private final boolean ignoreForInternalRedeploy;
         private final Set<ServiceInfo> services = new LinkedHashSet<>();
         private final Set<String> messages = new TreeSet<>();
 
@@ -31,10 +33,11 @@ public class RestartActions {
             return this;
         }
 
-        private Entry(String clusterName, String clusterType, String serviceType) {
+        private Entry(String clusterName, String clusterType, String serviceType, boolean ignoreForInternalRedeploy) {
             this.clusterName = clusterName;
             this.clusterType = clusterType;
             this.serviceType = serviceType;
+            this.ignoreForInternalRedeploy = ignoreForInternalRedeploy;
         }
 
         public String getClusterName() {
@@ -49,6 +52,10 @@ public class RestartActions {
             return serviceType;
         }
 
+        public boolean ignoreForInternalRedeploy() {
+            return ignoreForInternalRedeploy;
+        }
+
         public Set<ServiceInfo> getServices() {
             return services;
         }
@@ -59,28 +66,19 @@ public class RestartActions {
 
     }
 
-    private Entry addEntry(ServiceInfo service) {
-        String clusterName = service.getProperty("clustername").orElse("");
-        String clusterType = service.getProperty("clustertype").orElse("");
-        String entryId = clusterType + "." + clusterName + "." + service.getServiceType();
-        Entry entry = actions.get(entryId);
-        if (entry == null) {
-            entry = new Entry(clusterName, clusterType, service.getServiceType());
-            actions.put(entryId, entry);
-        }
-        return entry;
-    }
-
     private final Map<String, Entry> actions = new TreeMap<>();
 
-    public RestartActions() {
+    public RestartActions() { }
+
+    private RestartActions(Map<String, Entry> actions) {
+        this.actions.putAll(actions);
     }
 
     public RestartActions(List<ConfigChangeAction> actions) {
         for (ConfigChangeAction action : actions) {
             if (action.getType().equals(ConfigChangeAction.Type.RESTART)) {
                 for (ServiceInfo service : action.getServices()) {
-                    addEntry(service).
+                    addEntry(service, action.ignoreForInternalRedeploy()).
                             addService(service).
                             addMessage(action.getMessage());
                 }
@@ -88,6 +86,24 @@ public class RestartActions {
         }
     }
 
+    private Entry addEntry(ServiceInfo service, boolean ignoreForInternalRedeploy) {
+        String clusterName = service.getProperty("clustername").orElse("");
+        String clusterType = service.getProperty("clustertype").orElse("");
+        String entryId = clusterType + "." + clusterName + "." + service.getServiceType() + "." + ignoreForInternalRedeploy;
+        Entry entry = actions.get(entryId);
+        if (entry == null) {
+            entry = new Entry(clusterName, clusterType, service.getServiceType(), ignoreForInternalRedeploy);
+            actions.put(entryId, entry);
+        }
+        return entry;
+    }
+
+    public RestartActions withoutIgnoreForInternalRestart(boolean withoutIgnoreForInternalRestart) {
+        return new RestartActions(actions.entrySet().stream()
+                .filter(entry -> !withoutIgnoreForInternalRestart || !entry.getValue().ignoreForInternalRedeploy())
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)));
+    }
+
     public List<Entry> getEntries() {
         return new ArrayList<>(actions.values());
     }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
index 5f4f1298528..6d0ad2c1b98 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
@@ -52,12 +52,14 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
     private final Tenant tenant;
     private final DeployLogger deployLogger;
     private final Clock clock;
+    private final boolean internalRedeploy;
 
     private boolean prepared;
     private ConfigChangeActions configChangeActions;
 
     private Deployment(LocalSession session, ApplicationRepository applicationRepository, Supplier<PrepareParams> params,
-                       Optional<Provisioner> provisioner, Tenant tenant, DeployLogger deployLogger, Clock clock, boolean prepared) {
+                       Optional<Provisioner> provisioner, Tenant tenant, DeployLogger deployLogger, Clock clock,
+                       boolean internalRedeploy, boolean prepared) {
         this.session = session;
         this.applicationRepository = applicationRepository;
         this.params = params;
@@ -65,26 +67,27 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
         this.tenant = tenant;
         this.deployLogger = deployLogger;
         this.clock = clock;
+        this.internalRedeploy = internalRedeploy;
         this.prepared = prepared;
     }
 
     public static Deployment unprepared(LocalSession session, ApplicationRepository applicationRepository,
                                         Optional<Provisioner> provisioner, Tenant tenant, PrepareParams params, DeployLogger logger, Clock clock) {
-        return new Deployment(session, applicationRepository, () -> params, provisioner, tenant, logger, clock, false);
+        return new Deployment(session, applicationRepository, () -> params, provisioner, tenant, logger, clock, false, false);
     }
 
     public static Deployment unprepared(LocalSession session, ApplicationRepository applicationRepository,
                                         Optional<Provisioner> provisioner, Tenant tenant, DeployLogger logger,
                                         Duration timeout, Clock clock, boolean validate, boolean isBootstrap, boolean internalRestart) {
         Supplier<PrepareParams> params = createPrepareParams(clock, timeout, session, isBootstrap, !validate, false, internalRestart);
-        return new Deployment(session, applicationRepository, params, provisioner, tenant, logger, clock, false);
+        return new Deployment(session, applicationRepository, params, provisioner, tenant, logger, clock, true, false);
     }
 
     public static Deployment prepared(LocalSession session, ApplicationRepository applicationRepository,
                                       Optional<Provisioner> provisioner, Tenant tenant, DeployLogger logger,
                                       Duration timeout, Clock clock, boolean isBootstrap, boolean force) {
         Supplier<PrepareParams> params = createPrepareParams(clock, timeout, session, isBootstrap, false, force, false);
-        return new Deployment(session, applicationRepository, params, provisioner, tenant, logger, clock, true);
+        return new Deployment(session, applicationRepository, params, provisioner, tenant, logger, clock, false, true);
     }
 
     /** Prepares this. This does nothing if this is already prepared */
@@ -136,17 +139,21 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
                                 (previousActiveSession != null ? ". Based on session " + previousActiveSession.getSessionId() : "") +
                                 ". File references: " + applicationRepository.getFileReferences(applicationId));
 
-            if (params.internalRestart() && !configChangeActions.getRestartActions().isEmpty()) {
-                Set<String> hostnames = configChangeActions.getRestartActions().getEntries().stream()
-                        .flatMap(entry -> entry.getServices().stream())
-                        .map(ServiceInfo::getHostName)
-                        .collect(Collectors.toUnmodifiableSet());
+            if (params.internalRestart()) {
+                RestartActions restartActions = configChangeActions.getRestartActions().withoutIgnoreForInternalRestart(internalRedeploy);
 
-                provisioner.get().restart(applicationId, HostFilter.from(hostnames, Set.of(), Set.of(), Set.of()));
-                deployLogger.log(Level.INFO, String.format("Scheduled service restart of %d nodes: %s",
-                        hostnames.size(), hostnames.stream().sorted().collect(Collectors.joining(", "))));
+                if (!restartActions.isEmpty()) {
+                    Set<String> hostnames = restartActions.getEntries().stream()
+                            .flatMap(entry -> entry.getServices().stream())
+                            .map(ServiceInfo::getHostName)
+                            .collect(Collectors.toUnmodifiableSet());
 
-                this.configChangeActions = new ConfigChangeActions(new RestartActions(), configChangeActions.getRefeedActions());
+                    provisioner.get().restart(applicationId, HostFilter.from(hostnames, Set.of(), Set.of(), Set.of()));
+                    deployLogger.log(Level.INFO, String.format("Scheduled service restart of %d nodes: %s",
+                            hostnames.size(), hostnames.stream().sorted().collect(Collectors.joining(", "))));
+
+                    this.configChangeActions = new ConfigChangeActions(new RestartActions(), configChangeActions.getRefeedActions());
+                }
             }
 
             return session.getMetaData().getGeneration();
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java
index e2c3369d49e..ead1e79a416 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/ConfigChangeActionsBuilder.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.config.server.configchange;
 import com.google.common.collect.ImmutableMap;
 import com.yahoo.config.model.api.ConfigChangeAction;
 import com.yahoo.config.model.api.ServiceInfo;
+import com.yahoo.vespa.model.application.validation.change.VespaRestartAction;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -22,11 +23,17 @@ public class ConfigChangeActionsBuilder {
     }
 
     public ConfigChangeActionsBuilder restart(String message, String clusterName, String clusterType, String serviceType, String serviceName) {
-        actions.add(new MockRestartAction(message,
-                                          List.of(createService(clusterName, clusterType, serviceType, serviceName))));
+        return restart(message, clusterName, clusterType, serviceType, serviceName, false);
+    }
+
+    public ConfigChangeActionsBuilder restart(String message, String clusterName, String clusterType, String serviceType, String serviceName, boolean ignoreForInternalRedeploy) {
+        actions.add(new VespaRestartAction(message,
+                createService(clusterName, clusterType, serviceType, serviceName),
+                ignoreForInternalRedeploy));
         return this;
     }
 
+
     ConfigChangeActionsBuilder refeed(String name, boolean allowed, String message, String documentType, String clusterName, String serviceName) {
         actions.add(new MockRefeedAction(name,
                                          allowed,
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockRefeedAction.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockRefeedAction.java
index bdf63befd15..904ca10aa1c 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockRefeedAction.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/MockRefeedAction.java
@@ -30,6 +30,11 @@ public class MockRefeedAction extends MockConfigChangeAction implements ConfigCh
     public boolean allowed() { return allowed; }
 
     @Override
+    public boolean ignoreForInternalRedeploy() {
+        return false;
+    }
+
+    @Override
     public String getDocumentType() { return documentType; }
 
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RestartActionsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RestartActionsTest.java
index ee0180802af..27940383644 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RestartActionsTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/configchange/RestartActionsTest.java
@@ -5,8 +5,10 @@ import com.yahoo.config.model.api.ServiceInfo;
 import org.junit.Test;
 
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Collectors;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
@@ -81,4 +83,18 @@ public class RestartActionsTest {
         assertThat(toString(entries.get(0)), equalTo("content.foo.searchnode:[baz][change]"));
         assertThat(toString(entries.get(1)), equalTo("search.foo.searchnode:[baz][change]"));
     }
+
+    @Test
+    public void without_restart_i() {
+        ConfigChangeActions actions = new ConfigChangeActionsBuilder()
+                .restart(CHANGE_MSG, CLUSTER, CLUSTER_TYPE, SERVICE_TYPE, SERVICE_NAME)
+                .restart(CHANGE_MSG, CLUSTER, CLUSTER_TYPE_2, SERVICE_TYPE, SERVICE_NAME, true).build();
+
+        assertEquals(Set.of(CLUSTER_TYPE, CLUSTER_TYPE_2),
+                actions.getRestartActions().getEntries().stream().map(RestartActions.Entry::getClusterType).collect(Collectors.toSet()));
+        assertEquals(Set.of(CLUSTER_TYPE, CLUSTER_TYPE_2),
+                actions.getRestartActions().withoutIgnoreForInternalRestart(false).getEntries().stream().map(RestartActions.Entry::getClusterType).collect(Collectors.toSet()));
+        assertEquals(Set.of(CLUSTER_TYPE),
+                actions.getRestartActions().withoutIgnoreForInternalRestart(true).getEntries().stream().map(RestartActions.Entry::getClusterType).collect(Collectors.toSet()));
+    }
 }