From 1d3182f6e4eac5f2c07691a79f855ab8bd5a8b03 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Mon, 28 Sep 2020 15:05:29 +0200 Subject: Support ignoring actions for internal redeploy --- .../yahoo/config/model/api/ConfigChangeAction.java | 3 ++ .../change/ContainerRestartValidator.java | 2 +- .../validation/change/VespaRefeedAction.java | 16 ++++---- .../validation/change/VespaRestartAction.java | 35 ++++++++++++++-- .../vespa/config/server/ApplicationRepository.java | 6 +-- .../config/server/configchange/RestartActions.java | 46 +++++++++++++++------- .../vespa/config/server/deploy/Deployment.java | 33 ++++++++++------ .../configchange/ConfigChangeActionsBuilder.java | 11 +++++- .../server/configchange/MockRefeedAction.java | 5 +++ .../server/configchange/RestartActionsTest.java | 16 ++++++++ 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 services, String documentType, boolean allowed, Instant now) { + private VespaRefeedAction(String name, String message, List 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 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 newServices, String documentType) { - return new VespaRefeedAction(name, newMessage, newServices, documentType, allowed, now); + return new VespaRefeedAction(name, newMessage, newServices, documentType, allowed); } @Override @@ -63,6 +60,11 @@ public class VespaRefeedAction extends VespaConfigChangeAction implements Config @Override 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 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 services = new LinkedHashSet<>(); private final Set 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 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 actions = new TreeMap<>(); - public RestartActions() { + public RestartActions() { } + + private RestartActions(Map actions) { + this.actions.putAll(actions); } public RestartActions(List 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 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 params, - Optional provisioner, Tenant tenant, DeployLogger deployLogger, Clock clock, boolean prepared) { + Optional 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, 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, Tenant tenant, DeployLogger logger, Duration timeout, Clock clock, boolean validate, boolean isBootstrap, boolean internalRestart) { Supplier 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, Tenant tenant, DeployLogger logger, Duration timeout, Clock clock, boolean isBootstrap, boolean force) { Supplier 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 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 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 @@ -29,6 +29,11 @@ public class MockRefeedAction extends MockConfigChangeAction implements ConfigCh @Override 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())); + } } -- cgit v1.2.3