Do internal restart if provisioner is set

7 files changed, 57 insertions, 47 deletions

diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/HostFilter.java b/config-provisioning/src/main/java/com/yahoo/config/provision/HostFilter.java
index 95eec2ac0fb..ab84be4b66b 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/HostFilter.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/HostFilter.java
@@ -27,15 +27,11 @@ public class HostFilter {
                        Set<String> flavors,
                        Set<ClusterSpec.Type> clusterTypes,
                        Set<ClusterSpec.Id> clusterIds) {
-        Objects.requireNonNull(hostnames, "Hostnames cannot be null, use an empty list");
-        Objects.requireNonNull(flavors, "Flavors cannot be null, use an empty list");
-        Objects.requireNonNull(clusterTypes, "clusterTypes cannot be null, use an empty list");
-        Objects.requireNonNull(clusterIds, "clusterIds cannot be null, use an empty list");
-
-        this.hostnames = hostnames;
-        this.flavors = flavors;
-        this.clusterTypes = clusterTypes;
-        this.clusterIds = clusterIds;
+        this.hostnames = Objects.requireNonNull(hostnames, "Hostnames cannot be null, use an empty list");
+        this.flavors = Objects.requireNonNull(flavors, "Flavors cannot be null, use an empty list");
+        this.clusterTypes = Objects.requireNonNull(clusterTypes, "clusterTypes cannot be null, use an empty list");
+        this.clusterIds = Objects.requireNonNull(clusterIds, "clusterIds cannot be null, use an empty list");
+        System.out.println(hostnames);
     }
 
     /** Returns true if this filter matches the given host properties */
@@ -92,4 +88,25 @@ public class HostFilter {
                 StringUtilities.split(clusterIds).stream().map(ClusterSpec.Id::from).collect(Collectors.toSet()));
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+
+        HostFilter that = (HostFilter) o;
+
+        if (!hostnames.equals(that.hostnames)) return false;
+        if (!flavors.equals(that.flavors)) return false;
+        if (!clusterTypes.equals(that.clusterTypes)) return false;
+        return clusterIds.equals(that.clusterIds);
+    }
+
+    @Override
+    public int hashCode() {
+        int result = hostnames.hashCode();
+        result = 31 * result + flavors.hashCode();
+        result = 31 * result + clusterTypes.hashCode();
+        result = 31 * result + clusterIds.hashCode();
+        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 3e4f7a96e9b..958c39ff7a7 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
@@ -68,7 +68,6 @@ import com.yahoo.vespa.curator.stats.LockStats;
 import com.yahoo.vespa.curator.stats.ThreadLockStats;
 import com.yahoo.vespa.defaults.Defaults;
 import com.yahoo.vespa.flags.BooleanFlag;
-import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
@@ -132,7 +131,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
     private final LogRetriever logRetriever;
     private final TesterClient testerClient;
     private final Metric metric;
-    private final BooleanFlag deployWithInternalRestart;
     private final BooleanFlag acquireProvisionLock;
 
     @Inject
@@ -183,7 +181,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         this.clock = Objects.requireNonNull(clock);
         this.testerClient = Objects.requireNonNull(testerClient);
         this.metric = Objects.requireNonNull(metric);
-        this.deployWithInternalRestart = Flags.DEPLOY_WITH_INTERNAL_RESTART.bindTo(flagSource);
         this.acquireProvisionLock = Flags.ALWAYS_ACQUIRE_PROVISION_LOCK.bindTo(flagSource);
     }
 
@@ -394,10 +391,9 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         DeployLogger logger = new SilentDeployLogger();
         LocalSession newSession = sessionRepository.createSessionFromExisting(activeSession, logger, true, timeoutBudget);
         sessionRepository.addLocalSession(newSession);
-        boolean internalRestart = deployWithInternalRestart.with(FetchVector.Dimension.APPLICATION_ID, application.serializedForm()).value();
 
         return Optional.of(Deployment.unprepared(newSession, this, hostProvisioner, tenant, logger, timeout, clock,
-                                                 false /* don't validate as this is already deployed */, bootstrap, internalRestart));
+                                                 false /* don't validate as this is already deployed */, bootstrap));
     }
 
     @Override
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 ed5f1b2d056..e20f196202f 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
@@ -77,15 +77,15 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
 
     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);
+                                        Duration timeout, Clock clock, boolean validate, boolean isBootstrap) {
+        Supplier<PrepareParams> params = createPrepareParams(clock, timeout, session, isBootstrap, !validate, 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);
+        Supplier<PrepareParams> params = createPrepareParams(clock, timeout, session, isBootstrap, false, force);
         return new Deployment(session, applicationRepository, params, provisioner, tenant, logger, clock, false, true);
     }
 
@@ -94,9 +94,6 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
     public void prepare() {
         if (prepared) return;
         PrepareParams params = this.params.get();
-        if (params.internalRestart() && provisioner.isEmpty())
-            throw new IllegalArgumentException("Internal restart not supported without Provisioner");
-
         ApplicationId applicationId = params.getApplicationId();
         try (ActionTimer timer = applicationRepository.timerFor(applicationId, "deployment.prepareMillis")) {
             this.configChangeActions = tenant.getSessionRepository().prepareLocalSession(
@@ -138,7 +135,7 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
                                 (previousActiveSession != null ? ". Based on session " + previousActiveSession.getSessionId() : "") +
                                 ". File references: " + applicationRepository.getFileReferences(applicationId));
 
-            if (params.internalRestart()) {
+            if (configChangeActions != null && provisioner.isPresent()) {
                 RestartActions restartActions = configChangeActions.getRestartActions().useForInternalRestart(internalRedeploy);
 
                 if (!restartActions.isEmpty()) {
@@ -200,7 +197,7 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
      */
     private static Supplier<PrepareParams> createPrepareParams(
             Clock clock, Duration timeout, LocalSession session,
-            boolean isBootstrap, boolean ignoreValidationErrors, boolean force, boolean internalRestart) {
+            boolean isBootstrap, boolean ignoreValidationErrors, boolean force) {
 
         // Supplier because shouldn't/cant create this before validateSessionStatus() for prepared deployments
         // memoized because we want to create this once for unprepared deployments
@@ -213,8 +210,7 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
                     .timeoutBudget(timeoutBudget)
                     .ignoreValidationErrors(ignoreValidationErrors)
                     .isBootstrap(isBootstrap)
-                    .force(force)
-                    .internalRestart(internalRestart);
+                    .force(force);
             session.getDockerImageRepository().ifPresent(params::dockerImageRepository);
             session.getAthenzDomain().ifPresent(params::athenzDomain);
 
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java
index 914c927698f..d962218b63a 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java
@@ -45,7 +45,6 @@ public final class PrepareParams {
     static final String APPLICATION_CONTAINER_ROLE = "applicationContainerRole";
     static final String QUOTA_PARAM_NAME = "quota";
     static final String FORCE_PARAM_NAME = "force";
-    static final String INTERNAL_RESTART_PARAM_NAME = "internalRestart";
 
     private final ApplicationId applicationId;
     private final TimeoutBudget timeoutBudget;
@@ -54,7 +53,6 @@ public final class PrepareParams {
     private final boolean verbose;
     private final boolean isBootstrap;
     private final boolean force;
-    private final boolean internalRestart;
     private final Optional<Version> vespaVersion;
     private final List<ContainerEndpoint> containerEndpoints;
     private final Optional<String> tlsSecretsKeyName;
@@ -69,8 +67,7 @@ public final class PrepareParams {
                           List<ContainerEndpoint> containerEndpoints, Optional<String> tlsSecretsKeyName,
                           Optional<EndpointCertificateMetadata> endpointCertificateMetadata,
                           Optional<DockerImage> dockerImageRepository, Optional<AthenzDomain> athenzDomain,
-                          Optional<ApplicationRoles> applicationRoles, Optional<Quota> quota, boolean force,
-                          boolean internalRestart) {
+                          Optional<ApplicationRoles> applicationRoles, Optional<Quota> quota, boolean force) {
         this.timeoutBudget = timeoutBudget;
         this.applicationId = Objects.requireNonNull(applicationId);
         this.ignoreValidationErrors = ignoreValidationErrors;
@@ -86,7 +83,6 @@ public final class PrepareParams {
         this.applicationRoles = applicationRoles;
         this.quota = quota;
         this.force = force;
-        this.internalRestart = internalRestart;
     }
 
     public static class Builder {
@@ -96,7 +92,6 @@ public final class PrepareParams {
         private boolean verbose = false;
         private boolean isBootstrap = false;
         private boolean force = false;
-        private boolean internalRestart = false;
         private ApplicationId applicationId = null;
         private TimeoutBudget timeoutBudget = new TimeoutBudget(Clock.systemUTC(), Duration.ofSeconds(60));
         private Optional<Version> vespaVersion = Optional.empty();
@@ -213,16 +208,11 @@ public final class PrepareParams {
             return this;
         }
 
-        public Builder internalRestart(boolean internalRestart) {
-            this.internalRestart = internalRestart;
-            return this;
-        }
-
         public PrepareParams build() {
             return new PrepareParams(applicationId, timeoutBudget, ignoreValidationErrors, dryRun,
                                      verbose, isBootstrap, vespaVersion, containerEndpoints, tlsSecretsKeyName,
                                      endpointCertificateMetadata, dockerImageRepository, athenzDomain,
-                                     applicationRoles, quota, force, internalRestart);
+                                     applicationRoles, quota, force);
         }
     }
 
@@ -241,7 +231,6 @@ public final class PrepareParams {
                             .applicationRoles(ApplicationRoles.fromString(request.getProperty(APPLICATION_HOST_ROLE), request.getProperty(APPLICATION_CONTAINER_ROLE)))
                             .quota(request.getProperty(QUOTA_PARAM_NAME))
                             .force(request.getBooleanProperty(FORCE_PARAM_NAME))
-                            .internalRestart(request.getBooleanProperty(INTERNAL_RESTART_PARAM_NAME))
                             .build();
     }
 
@@ -293,8 +282,6 @@ public final class PrepareParams {
 
     public boolean force() { return force; }
 
-    public boolean internalRestart() { return internalRestart; }
-
     public TimeoutBudget getTimeoutBudget() {
         return timeoutBudget;
     }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
index fcc330235db..be1e96d246a 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
@@ -14,6 +14,7 @@ import com.yahoo.config.provision.AllocatedHosts;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.Deployment;
+import com.yahoo.config.provision.HostFilter;
 import com.yahoo.config.provision.HostSpec;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.NetworkPorts;
@@ -191,6 +192,21 @@ public class ApplicationRepositoryTest {
         prepareAndActivate(testAppJdiscOnly);
         PrepareResult result = prepareAndActivate(testAppJdiscOnlyRestart);
         assertTrue(result.configChangeActions().getRefeedActions().isEmpty());
+        assertTrue(result.configChangeActions().getRestartActions().isEmpty());
+        assertEquals(HostFilter.hostname("mytesthost"), provisioner.lastRestartFilter());
+    }
+
+    @Test
+    public void prepareAndActivateWithRestartWithoutProvisioner() {
+        applicationRepository = new ApplicationRepository.Builder()
+                .withTenantRepository(tenantRepository)
+                .withOrchestrator(orchestrator)
+                .withProvisioner(null)
+                .build();
+
+        prepareAndActivate(testAppJdiscOnly);
+        PrepareResult result = prepareAndActivate(testAppJdiscOnlyRestart);
+        assertTrue(result.configChangeActions().getRefeedActions().isEmpty());
         assertFalse(result.configChangeActions().getRestartActions().isEmpty());
     }
 
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/MockProvisioner.java b/configserver/src/test/java/com/yahoo/vespa/config/server/MockProvisioner.java
index 36198862c55..95c9788e62a 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/MockProvisioner.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/MockProvisioner.java
@@ -26,6 +26,7 @@ public class MockProvisioner implements Provisioner {
     private boolean restarted = false;
     private ApplicationId lastApplicationId;
     private Collection<HostSpec> lastHosts;
+    private HostFilter lastRestartFilter;
 
     private boolean transientFailureOnPrepare = false;
     private HostProvisioner hostProvisioner = null;
@@ -78,6 +79,7 @@ public class MockProvisioner implements Provisioner {
     public void restart(ApplicationId application, HostFilter filter) {
         restarted = true;
         lastApplicationId = application;
+        lastRestartFilter = filter;
     }
 
     @Override
@@ -105,4 +107,7 @@ public class MockProvisioner implements Provisioner {
         return lastApplicationId;
     }
 
+    public HostFilter lastRestartFilter() {
+        return lastRestartFilter;
+    }
 }
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 740b92366e2..ed8d043727a 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
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server.deploy;
 
-import com.google.common.collect.ImmutableSet;
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.component.Version;
 import com.yahoo.config.model.api.ConfigChangeAction;
@@ -21,7 +20,6 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.test.ManualClock;
-import com.yahoo.vespa.config.server.configchange.RestartActions;
 import com.yahoo.vespa.config.server.http.InvalidApplicationException;
 import com.yahoo.vespa.config.server.http.v2.PrepareResult;
 import com.yahoo.vespa.config.server.model.TestModelFactory;
@@ -45,11 +43,8 @@ import java.util.stream.IntStream;
 import static com.yahoo.vespa.config.server.deploy.DeployTester.CountingModelFactory;
 import static com.yahoo.vespa.config.server.deploy.DeployTester.createFailingModelFactory;
 import static com.yahoo.vespa.config.server.deploy.DeployTester.createHostedModelFactory;
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -387,9 +382,7 @@ public class HostedDeployTest {
         PrepareResult prepareResult = tester.deployApp("src/test/apps/hosted/", "6.2.0");
 
         assertEquals(4, tester.getAllocatedHostsOf(tester.applicationId()).getHosts().size());
-        List<RestartActions.Entry> actions = prepareResult.configChangeActions().getRestartActions().getEntries();
-        assertThat(actions.size(), is(1));
-        assertThat(actions.get(0).getMessages(), equalTo(ImmutableSet.of("change", "other change")));
+        assertTrue(prepareResult.configChangeActions().getRestartActions().isEmpty());
     }
 
     private ConfigserverConfig createConfigserverConfig() throws IOException {