diff options
135 files changed, 1078 insertions, 864 deletions
diff --git a/athenz-identity-provider-service/pom.xml b/athenz-identity-provider-service/pom.xml index 982cb89f2bf..86d4defa861 100644 --- a/athenz-identity-provider-service/pom.xml +++ b/athenz-identity-provider-service/pom.xml @@ -131,14 +131,6 @@ <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-compiler-plugin</artifactId> - <configuration> - <compilerArgs> - <arg>-Xlint:all</arg> - <arg>-Xlint:-deprecation</arg> - <arg>-Xlint:-serial</arg> - <arg>-Werror</arg> - </compilerArgs> - </configuration> </plugin> </plugins> </build> diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGenerator.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGenerator.java index 59126fd023f..728406c297f 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGenerator.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGenerator.java @@ -7,7 +7,6 @@ import com.yahoo.net.HostName; import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; import com.yahoo.vespa.athenz.identityprovider.api.IdentityDocument; -import com.yahoo.vespa.athenz.identityprovider.api.IdentityType; import com.yahoo.vespa.athenz.identityprovider.api.SignedIdentityDocument; import com.yahoo.vespa.athenz.identityprovider.api.VespaUniqueInstanceId; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.KeyProvider; @@ -28,10 +27,7 @@ import java.util.Objects; import java.util.Set; /** - * Generates a signed identity document for a given hostname and type - * * @author mortent - * @author bjorncs */ public class IdentityDocumentGenerator { @@ -51,10 +47,10 @@ public class IdentityDocumentGenerator { this.keyProvider = keyProvider; } - public SignedIdentityDocument generateSignedIdentityDocument(String hostname, IdentityType identityType) { + public SignedIdentityDocument generateSignedIdentityDocument(String hostname) { Node node = nodeRepository.getNode(hostname).orElseThrow(() -> new RuntimeException("Unable to find node " + hostname)); try { - IdentityDocument identityDocument = generateIdDocument(node, identityType); + IdentityDocument identityDocument = generateIdDocument(node); String identityDocumentString = Utils.getMapper().writeValueAsString(EntityBindingsMapper.toIdentityDocumentEntity(identityDocument)); String encodedIdentityDocument = @@ -74,18 +70,13 @@ public class IdentityDocumentGenerator { toZoneDnsSuffix(zone, zoneConfig.certDnsSuffix()), new AthenzService(zoneConfig.domain(), zoneConfig.serviceName()), URI.create(zoneConfig.ztsUrl()), - SignedIdentityDocument.DEFAULT_DOCUMENT_VERSION, - identityDocument.configServerHostname(), - identityDocument.instanceHostname(), - identityDocument.createdAt(), - identityDocument.ipAddresses(), - identityType); + SignedIdentityDocument.DEFAULT_DOCUMENT_VERSION); } catch (Exception e) { throw new RuntimeException("Exception generating identity document: " + e.getMessage(), e); } } - private IdentityDocument generateIdDocument(Node node, IdentityType identityType) { + private IdentityDocument generateIdDocument(Node node) { Allocation allocation = node.allocation().orElseThrow(() -> new RuntimeException("No allocation for node " + node.hostname())); VespaUniqueInstanceId providerUniqueId = new VespaUniqueInstanceId( allocation.membership().index(), @@ -94,10 +85,17 @@ public class IdentityDocumentGenerator { allocation.owner().application().value(), allocation.owner().tenant().value(), zone.region().value(), - zone.environment().value(), - identityType); + zone.environment().value()); + // TODO: Hack to allow access from docker containers to non-ipv6 services. + // Remove when yca-bridge is no longer needed Set<String> ips = new HashSet<>(node.ipAddresses()); + if(node.parentHostname().isPresent()) { + String parentHostName = node.parentHostname().get(); + nodeRepository.getNode(parentHostName) + .map(Node::ipAddresses) + .ifPresent(ips::addAll); + } return new IdentityDocument( providerUniqueId, HostName.getLocalhost(), diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentResource.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentResource.java index 219e12c7223..93668006e26 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentResource.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentResource.java @@ -6,7 +6,6 @@ import com.yahoo.container.jaxrs.annotation.Component; import com.yahoo.jdisc.http.servlet.ServletRequest; import com.yahoo.log.LogLevel; import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; -import com.yahoo.vespa.athenz.identityprovider.api.IdentityType; import com.yahoo.vespa.athenz.identityprovider.api.bindings.IdentityDocumentApi; import com.yahoo.vespa.athenz.identityprovider.api.bindings.SignedIdentityDocumentEntity; import com.yahoo.vespa.hosted.provision.restapi.v2.filter.NodePrincipal; @@ -19,6 +18,7 @@ import javax.ws.rs.InternalServerErrorException; import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import java.util.logging.Logger; @@ -41,7 +41,15 @@ public class IdentityDocumentResource implements IdentityDocumentApi { this.request = request; } - private SignedIdentityDocumentEntity getIdentityDocument(String hostname, IdentityType identityType) { + /** + * @deprecated Use {@link #getNodeIdentityDocument(String)} and {@link #getTenantIdentityDocument(String)} instead. + */ + @GET + @Produces(MediaType.APPLICATION_JSON) + @Deprecated + @Override + // TODO Make this method private when the rest api is not longer in use + public SignedIdentityDocumentEntity getIdentityDocument(@QueryParam("hostname") String hostname) { if (hostname == null) { throw new BadRequestException("The 'hostname' query parameter is missing"); } @@ -59,7 +67,7 @@ public class IdentityDocumentResource implements IdentityDocumentApi { throw new ForbiddenException(); } try { - return EntityBindingsMapper.toSignedIdentityDocumentEntity(identityDocumentGenerator.generateSignedIdentityDocument(hostname, identityType)); + return EntityBindingsMapper.toSignedIdentityDocumentEntity(identityDocumentGenerator.generateSignedIdentityDocument(hostname)); } catch (Exception e) { String message = String.format("Unable to generate identity doument for '%s': %s", hostname, e.getMessage()); log.log(LogLevel.ERROR, message, e); @@ -72,7 +80,7 @@ public class IdentityDocumentResource implements IdentityDocumentApi { @Path("/node/{host}") @Override public SignedIdentityDocumentEntity getNodeIdentityDocument(@PathParam("host") String host) { - return getIdentityDocument(host, IdentityType.NODE); + return getIdentityDocument(host); } @GET @@ -80,7 +88,7 @@ public class IdentityDocumentResource implements IdentityDocumentApi { @Path("/tenant/{host}") @Override public SignedIdentityDocumentEntity getTenantIdentityDocument(@PathParam("host") String host) { - return getIdentityDocument(host, IdentityType.TENANT); + return getIdentityDocument(host); } } diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java index 0201c46b253..e457df37946 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java @@ -82,7 +82,6 @@ public class InstanceValidator { } // If/when we dont care about logging exactly whats wrong, this can be simplified - // TODO Use identity type to determine if this check should be performed boolean isSameIdentityAsInServicesXml(ApplicationId applicationId, String domain, String service) { Optional<ApplicationInfo> applicationInfo = superModelProvider.getSuperModel().getApplicationInfo(applicationId); diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java index 078ef1b7e39..d7b061ca2f1 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java @@ -15,7 +15,6 @@ import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; -import com.yahoo.vespa.athenz.identityprovider.api.IdentityType; import com.yahoo.vespa.athenz.identityprovider.api.SignedIdentityDocument; import com.yahoo.vespa.athenz.identityprovider.api.VespaUniqueInstanceId; import com.yahoo.vespa.athenz.identityprovider.api.bindings.SignedIdentityDocumentEntity; @@ -82,7 +81,7 @@ public class IdentityDocumentGeneratorTest { AthenzProviderServiceConfig config = getAthenzProviderConfig("domain", "service", dnsSuffix, ZONE); IdentityDocumentGenerator identityDocumentGenerator = new IdentityDocumentGenerator(config, nodeRepository, ZONE, keyProvider); - SignedIdentityDocument signedIdentityDocument = identityDocumentGenerator.generateSignedIdentityDocument(containerHostname, IdentityType.TENANT); + SignedIdentityDocument signedIdentityDocument = identityDocumentGenerator.generateSignedIdentityDocument(containerHostname); // Verify attributes assertEquals(containerHostname, signedIdentityDocument.identityDocument().instanceHostname()); @@ -93,11 +92,11 @@ public class IdentityDocumentGeneratorTest { assertEquals(expectedZoneDnsSuffix, signedIdentityDocument.dnsSuffix()); VespaUniqueInstanceId expectedProviderUniqueId = - new VespaUniqueInstanceId(0, "default", "default", "application", "tenant", region, environment, IdentityType.TENANT); + new VespaUniqueInstanceId(0, "default", "default", "application", "tenant", region, environment); assertEquals(expectedProviderUniqueId, signedIdentityDocument.providerUniqueId()); - // Validate that container ips are present - assertThat(signedIdentityDocument.identityDocument().ipAddresses(), Matchers.containsInAnyOrder("::1")); + // Validate that both parent and container ips are present + assertThat(signedIdentityDocument.identityDocument().ipAddresses(), Matchers.containsInAnyOrder("127.0.0.1", "::1")); SignedIdentityDocumentEntity signedIdentityDocumentEntity = EntityBindingsMapper.toSignedIdentityDocumentEntity(signedIdentityDocument); diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java index 54411b424eb..54786c86cd3 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java @@ -143,12 +143,7 @@ public class InstanceValidatorTest { "dnssuffix", "service", URI.create("http://localhost/zts"), - 1, - identityDocument.configServerHostname, - identityDocument.instanceHostname, - identityDocument.createdAt, - identityDocument.ipAddresses, - null)); // TODO Remove support for legacy representation without type + 1)); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java index f48dffecb27..cd13305c009 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/DeployData.java @@ -8,27 +8,37 @@ package com.yahoo.config.model.application.provider; */ public class DeployData { - /* Which user deployed */ + /** Which user deployed */ private final String deployedByUser; - /* Name of application given by user */ + /** Name of application given by user */ private final String applicationName; - /* The absolute path to the directory holding the application */ + /** The absolute path to the directory holding the application */ private final String deployedFromDir; - /* Timestamp when a deployment was made */ + /** Timestamp when a deployment was made */ private final long deployTimestamp; + /** Whether this is an internal redeploy, not caused by an application package change */ + private final boolean internalRedeploy; + /* Application generation. Incremented by one each time an application is deployed. */ private final long generation; private final long currentlyActiveGeneration; - public DeployData(String deployedByUser, String deployedFromDir, String applicationName, Long deployTimestamp, Long generation, long currentlyActiveGeneration) { + public DeployData(String deployedByUser, + String deployedFromDir, + String applicationName, + Long deployTimestamp, + boolean internalRedeploy, + Long generation, + long currentlyActiveGeneration) { this.deployedByUser = deployedByUser; this.deployedFromDir = deployedFromDir; this.applicationName = applicationName; this.deployTimestamp = deployTimestamp; + this.internalRedeploy = internalRedeploy; this.generation = generation; this.currentlyActiveGeneration = currentlyActiveGeneration; } @@ -45,6 +55,8 @@ public class DeployData { return deployTimestamp; } + public boolean isInternalRedeploy() { return internalRedeploy; } + public long getGeneration() { return generation; } diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java index 9bfcd4ecb6d..8fc871a1aa9 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java @@ -113,9 +113,13 @@ public class FilesApplicationPackage implements ApplicationPackage { } private static ApplicationMetaData metaDataFromDeployData(File appDir, DeployData deployData) { - return new ApplicationMetaData(deployData.getDeployedByUser(), deployData.getDeployedFromDir(), - deployData.getDeployTimestamp(), deployData.getApplicationName(), - computeCheckSum(appDir), deployData.getGeneration(), + return new ApplicationMetaData(deployData.getDeployedByUser(), + deployData.getDeployedFromDir(), + deployData.getDeployTimestamp(), + deployData.isInternalRedeploy(), + deployData.getApplicationName(), + computeCheckSum(appDir), + deployData.getGeneration(), deployData.getCurrentlyActiveGeneration()); } @@ -566,7 +570,7 @@ public class FilesApplicationPackage implements ApplicationPackage { } public static ApplicationMetaData readMetaData(File appDir) { - ApplicationMetaData defaultMetaData = new ApplicationMetaData(appDir, "n/a", "n/a", 0l, "", 0l, 0l); + ApplicationMetaData defaultMetaData = new ApplicationMetaData(appDir, "n/a", "n/a", 0l, false, "", 0l, 0l); File metaFile = new File(appDir, META_FILE_NAME); if (!metaFile.exists()) { return defaultMetaData; diff --git a/config-lib/src/main/java/com/yahoo/config/ConfigInstance.java b/config-lib/src/main/java/com/yahoo/config/ConfigInstance.java index ebe93f16738..04405839a9b 100644 --- a/config-lib/src/main/java/com/yahoo/config/ConfigInstance.java +++ b/config-lib/src/main/java/com/yahoo/config/ConfigInstance.java @@ -14,22 +14,25 @@ import java.util.Map; public abstract class ConfigInstance extends InnerNode { public interface Builder extends ConfigBuilder { + /** * Dispatches a getConfig() call if this instance's producer is of the right type * @param producer a config producer * @return true if this instance's producer was the correct type, and hence a getConfig call was dispatched */ - public boolean dispatchGetConfig(Producer producer); + boolean dispatchGetConfig(Producer producer); + + String getDefName(); + String getDefNamespace(); + String getDefMd5(); - public String getDefName(); - public String getDefNamespace(); - public String getDefMd5(); } public interface Producer {} private String configMd5 = ""; + @SuppressWarnings("unused") // Used by reflection from ConfigInstanceUtil String configId; /** diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationMetaData.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationMetaData.java index 41a0feff5d4..a3769299cf8 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationMetaData.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationMetaData.java @@ -11,27 +11,32 @@ import java.io.*; * Metadata about an application package. * * @author hmusum - * @since 5.0 */ public class ApplicationMetaData { + private final String deployedByUser; private final String deployedFromDir; private final long deployTimestamp; + private final boolean internalRedeploy; private final long generation; private final long previousActiveGeneration; private final String checkSum; private final String appName; public ApplicationMetaData(File appDir, String deployedByUser, String deployedFromDir, Long deployTimestamp, + boolean internalRedeploy, String checkSum, Long generation, long previousActiveGeneration) { - this(deployedByUser, deployedFromDir, deployTimestamp, appDir.getName(), checkSum, generation, previousActiveGeneration); + this(deployedByUser, deployedFromDir, deployTimestamp, internalRedeploy, + appDir.getName(), checkSum, generation, previousActiveGeneration); } - public ApplicationMetaData(String deployedByUser, String deployedFromDir, Long deployTimestamp, String applicationName, String checkSum, Long generation, long previousActiveGeneration) { + public ApplicationMetaData(String deployedByUser, String deployedFromDir, Long deployTimestamp, boolean internalRedeploy, + String applicationName, String checkSum, Long generation, long previousActiveGeneration) { this.appName = applicationName; this.deployedByUser = deployedByUser; this.deployedFromDir = deployedFromDir; this.deployTimestamp = deployTimestamp; + this.internalRedeploy = internalRedeploy; this.checkSum = checkSum; this.generation = generation; this.previousActiveGeneration = previousActiveGeneration; @@ -88,6 +93,12 @@ public class ApplicationMetaData { } /** + * Returns whether this application generation was produced by a system internal redeployment, + * not an application package change + */ + public boolean isInternalRedeploy() { return internalRedeploy; } + + /** * Returns an md5 hash of the contents of the application package * @return an md5sum of the application package */ @@ -115,7 +126,14 @@ public class ApplicationMetaData { Inspector root = data.get(); Inspector deploy = root.field("deploy"); Inspector app = root.field("application"); - return new ApplicationMetaData(deploy.field("user").asString(), deploy.field("from").asString(), deploy.field("timestamp").asLong(), app.field("name").asString(), app.field("checksum").asString(), app.field("generation").asLong(), app.field("previousActiveGeneration").asLong()); + return new ApplicationMetaData(deploy.field("user").asString(), + deploy.field("from").asString(), + deploy.field("timestamp").asLong(), + booleanField("internalRedeploy", false, deploy), + app.field("name").asString(), + app.field("checksum").asString(), + app.field("generation").asLong(), + app.field("previousActiveGeneration").asLong()); } catch (Exception e) { throw new IllegalArgumentException("Error parsing json metadata", e); } @@ -128,6 +146,7 @@ public class ApplicationMetaData { deploy.setString("user", deployedByUser); deploy.setString("from", deployedFromDir); deploy.setLong("timestamp", deployTimestamp); + deploy.setBool("internalRedeploy", internalRedeploy); Cursor app = meta.setObject("application"); app.setString("name", appName); app.setString("checksum", checkSum); @@ -136,6 +155,12 @@ public class ApplicationMetaData { return slime; } + private static boolean booleanField(String fieldName, boolean defaultValue, Inspector object) { + Inspector value = object.field(fieldName); + if ( ! value.valid()) return defaultValue; + return value.asBool(); + } + public String asJsonString() { Slime slime = getSlime(); ByteArrayOutputStream baos = new ByteArrayOutputStream(); diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java index fe6f7da2092..dd54fe11c39 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java @@ -224,9 +224,10 @@ public interface ApplicationPackage { /** * Gets the ApplicationMetaData instance for this application package. + * * @return an ApplicationMetaData instance */ - default ApplicationMetaData getMetaData() { return null; } + ApplicationMetaData getMetaData(); default File getFileReference(Path pathRelativeToAppDir) { throw new UnsupportedOperationException("This application package cannot return file references"); diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationInfo.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationInfo.java index c6a72ebb3ff..486db205a4c 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationInfo.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ApplicationInfo.java @@ -4,6 +4,7 @@ package com.yahoo.config.model.api; import com.yahoo.config.provision.ApplicationId; public class ApplicationInfo { + private final ApplicationId applicationId; private final long generation; private final Model model; // NOT immutable @@ -23,4 +24,5 @@ public class ApplicationInfo { public Model getModel() { return model; } + } diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java index 8ca0e5c501c..0cfde3c655c 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.model.test; +import com.yahoo.config.application.api.ApplicationMetaData; import com.yahoo.config.application.api.ComponentInfo; import com.yahoo.config.application.api.UnparsedConfigDefinition; import com.yahoo.config.application.api.ApplicationFile; @@ -39,6 +40,7 @@ public class MockApplicationPackage implements ApplicationPackage { private final Optional<String> validationOverrides; private final boolean failOnValidateXml; private final QueryProfileRegistry queryProfileRegistry; + private final ApplicationMetaData applicationMetaData; protected MockApplicationPackage(String hosts, String services, List<String> searchDefinitions, String searchDefinitionDir, String deploymentSpec, String validationOverrides, boolean failOnValidateXml, @@ -52,6 +54,7 @@ public class MockApplicationPackage implements ApplicationPackage { this.failOnValidateXml = failOnValidateXml; queryProfileRegistry = new QueryProfileXMLReader().read(asNamedReaderList(queryProfileType), asNamedReaderList(queryProfile)); + applicationMetaData = new ApplicationMetaData("user", "dir", 0L, false, "application", "checksum", 0L, 0L); } @Override @@ -133,6 +136,8 @@ public class MockApplicationPackage implements ApplicationPackage { public QueryProfileRegistry getQueryProfiles() { return queryProfileRegistry; } + public ApplicationMetaData getMetaData() { return applicationMetaData; } + @Override public Reader getRankingExpression(String name) { File expressionFile = new File(searchDefinitionDir, name); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java b/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java index 7444224258e..9801eab9f2b 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java @@ -27,7 +27,6 @@ import java.util.TreeSet; /** * @author Einar M R Rosenvinge - * @since 5.1.11 */ public class ContainerDocumentApi implements FeederConfig.Producer { diff --git a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java index f113b10f508..d3e8136cdec 100644 --- a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java @@ -20,7 +20,6 @@ import com.yahoo.vespa.config.ConfigDefinition; import com.yahoo.vespa.config.ConfigDefinitionKey; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.search.SearchDefinition; -import org.json.JSONException; import org.junit.After; import org.junit.Rule; import org.junit.Test; @@ -290,7 +289,7 @@ public class ApplicationDeployTest { } @Test - public void testConfigDefinitionsFromJars() throws IOException { + public void testConfigDefinitionsFromJars() { String appName = "src/test/cfg//application/app1"; FilesApplicationPackage app = FilesApplicationPackage.fromFile(new File(appName), false); Map<ConfigDefinitionKey, UnparsedConfigDefinition> defs = app.getAllExistingConfigDefs(); @@ -298,31 +297,31 @@ public class ApplicationDeployTest { } @Test - public void testMetaData() throws IOException, JSONException { + public void testMetaData() throws IOException { File tmp = Files.createTempDir(); String appPkg = TESTDIR + "app1"; IOUtils.copyDirectory(new File(appPkg), tmp); - final DeployData deployData = new DeployData("foo", "bar", "baz", 13l, 1337l, 3l); + DeployData deployData = new DeployData("foo", "bar", "baz", 13l, false, 1337l, 3l); FilesApplicationPackage app = FilesApplicationPackage.fromFileWithDeployData(tmp, deployData); app.writeMetaData(); FilesApplicationPackage newApp = FilesApplicationPackage.fromFileWithDeployData(tmp, deployData); ApplicationMetaData meta = newApp.getMetaData(); assertThat(meta.getDeployedByUser(), is("foo")); assertThat(meta.getDeployPath(), is("bar")); - assertThat(meta.getDeployTimestamp(), is(13l)); - assertThat(meta.getGeneration(), is(1337l)); - assertThat(meta.getPreviousActiveGeneration(), is(3l)); - final String checkSum = meta.getCheckSum(); + assertThat(meta.getDeployTimestamp(), is(13L)); + assertThat(meta.getGeneration(), is(1337L)); + assertThat(meta.getPreviousActiveGeneration(), is(3L)); + String checkSum = meta.getCheckSum(); assertNotNull(checkSum); assertTrue((new File(tmp, "hosts.xml")).delete()); FilesApplicationPackage app2 = FilesApplicationPackage.fromFileWithDeployData(tmp, deployData); - final String app2CheckSum = app2.getMetaData().getCheckSum(); + String app2CheckSum = app2.getMetaData().getCheckSum(); assertThat(app2CheckSum, is(not(checkSum))); assertTrue((new File(tmp, "files/foo.json")).delete()); FilesApplicationPackage app3 = FilesApplicationPackage.fromFileWithDeployData(tmp, deployData); - final String app3CheckSum = app3.getMetaData().getCheckSum(); + String app3CheckSum = app3.getMetaData().getCheckSum(); assertThat(app3CheckSum, is(not(app2CheckSum))); } diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java index 68985bd598a..f80c2699da2 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java @@ -314,7 +314,7 @@ public class ConfigProxyRpcServer implements Runnable, TargetWatcher, RpcServer public void returnOkResponse(JRTServerConfigRequest request, RawConfig config) { request.getRequestTrace().trace(TRACELEVEL, "Config proxy returnOkResponse()"); - request.addOkResponse(config.getPayload(), config.getGeneration(), config.getConfigMd5()); + request.addOkResponse(config.getPayload(), config.getGeneration(), config.isInternalRedeploy(), config.getConfigMd5()); log.log(LogLevel.DEBUG, () -> "Return response: " + request.getShortDescription() + ",configMd5=" + config.getConfigMd5() + ",generation=" + config.getGeneration()); log.log(LogLevel.SPAM, () -> "Config payload in response for " + request.getShortDescription() + ":" + config.getPayload()); diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigTester.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigTester.java index c16d6ccbcf5..33e798da15e 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigTester.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigTester.java @@ -44,11 +44,11 @@ public class ConfigTester { String defMd5 = ConfigUtils.getDefMd5(defContent); String configMd5 = ConfigUtils.getMd5(fooConfigPayload); fooConfig = new RawConfig(configKey, defMd5, fooPayload, configMd5, - generation, defContent, Optional.empty()); + generation, false, defContent, Optional.empty()); String defName2 = "bar"; barConfig = new RawConfig(new ConfigKey<>(defName2, configId, namespace), defMd5, fooPayload, configMd5, - generation, defContent, Optional.empty()); + generation, false, defContent, Optional.empty()); } JRTServerConfigRequest createRequest(RawConfig config) { diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheTest.java index 2f2aac2a6bb..16bc2c66a94 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MemoryCacheTest.java @@ -63,9 +63,9 @@ public class MemoryCacheTest { slime.setString("bar \"value2\""); payloadDifferentMd5 = Payload.from(new ConfigPayload(slime)); - config = new RawConfig(configKey, defMd5, payload, configMd5, generation, defContent, Optional.empty()); - config2 = new RawConfig(configKey2, defMd52, payload2, configMd5, generation, defContent, Optional.empty()); - configDifferentMd5 = new RawConfig(configKey, differentDefMd5, payloadDifferentMd5, configMd5, generation, defContent, Optional.empty()); + config = new RawConfig(configKey, defMd5, payload, configMd5, generation, false, defContent, Optional.empty()); + config2 = new RawConfig(configKey2, defMd52, payload2, configMd5, generation, false, defContent, Optional.empty()); + configDifferentMd5 = new RawConfig(configKey, differentDefMd5, payloadDifferentMd5, configMd5, generation, false, defContent, Optional.empty()); cacheKey = new ConfigCacheKey(configKey, config.getDefMd5()); cacheKey2 = new ConfigCacheKey(configKey2, config2.getDefMd5()); diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java index 3cd0f1043cc..513a5caa08d 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java @@ -18,7 +18,6 @@ import static org.junit.Assert.*; /** * @author hmusum - * @since 5.1.9 */ public class ProxyServerTest { @@ -35,7 +34,7 @@ public class ProxyServerTest { private static final ConfigKey<?> errorConfigKey = new ConfigKey<>("error", fooConfig.getConfigId(), fooConfig.getNamespace()); static final RawConfig errorConfig = new RawConfig(errorConfigKey, fooConfig.getDefMd5(), fooConfig.getPayload(), fooConfig.getConfigMd5(), - fooConfig.getGeneration(), ErrorCode.UNKNOWN_DEFINITION, fooConfig.getDefContent(), Optional.empty()); + fooConfig.getGeneration(), false, ErrorCode.UNKNOWN_DEFINITION, fooConfig.getDefContent(), Optional.empty()); @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @@ -223,7 +222,8 @@ public class ProxyServerTest { private static RawConfig createConfigWithNextConfigGeneration(RawConfig config, int errorCode, Payload payload) { return new RawConfig(config.getKey(), config.getDefMd5(), payload, config.getConfigMd5(), - config.getGeneration() + 1, errorCode, config.getDefContent(), Optional.empty()); + config.getGeneration() + 1, false, + errorCode, config.getDefContent(), Optional.empty()); } } diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigHandle.java b/config/src/main/java/com/yahoo/config/subscription/ConfigHandle.java index bb29e329121..c0eb3e98157 100644 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigHandle.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigHandle.java @@ -11,7 +11,6 @@ import com.yahoo.config.subscription.impl.ConfigSubscription; * * @param <T> the type of the config * @author vegardh - * @since 5.1 */ public class ConfigHandle<T extends ConfigInstance> { diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java b/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java index 18d13aac12c..cce3ce0a0c5 100644 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigInstanceUtil.java @@ -14,7 +14,6 @@ import com.yahoo.vespa.config.*; /** * @author gjoranv - * @since 5.1.6 */ public class ConfigInstanceUtil { @@ -34,8 +33,8 @@ public class ConfigInstanceUtil { setter.invoke(destination, source); setter.setAccessible(false); } catch (Exception e) { - throw new ConfigurationRuntimeException("Could not set values on config builder." - + destination.getClass().getName(), e); + throw new ConfigurationRuntimeException("Could not set values on config builder." + + destination.getClass().getName(), e); } } diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigSource.java b/config/src/main/java/com/yahoo/config/subscription/ConfigSource.java index 886d8994d15..d0eda9d27cc 100644 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigSource.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSource.java @@ -3,9 +3,8 @@ package com.yahoo.config.subscription; /** * A type of source of config - * @author vegardh - * @since 5.1 * + * @author vegardh */ public interface ConfigSource { diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigSourceSet.java b/config/src/main/java/com/yahoo/config/subscription/ConfigSourceSet.java index bb8d5a87916..c799186435c 100755 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigSourceSet.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSourceSet.java @@ -14,10 +14,10 @@ import java.util.logging.Logger; * Two sets are said to be equal if they contain the same sources, independent of order, * upper/lower-casing and whitespaces. * - * @author <a href="gv@yahoo-inc.com">G. Voldengen</a> + * @author gjoranv */ -public class ConfigSourceSet implements ConfigSource -{ +public class ConfigSourceSet implements ConfigSource { + private static final Logger log = Logger.getLogger(ConfigSourceSet.class.getName()); private final Set<String> sources = new LinkedHashSet<>(); @@ -124,4 +124,5 @@ public class ConfigSourceSet implements ConfigSource } return sourceSet; } + } diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java index b7c45de9f5b..47048fa16c7 100644 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java @@ -24,7 +24,6 @@ import com.yahoo.vespa.config.TimingValues; * {@link ConfigHandle} which {@link #subscribe(Class, String)} returned. * * @author vegardh - * @since 5.1 */ public class ConfigSubscriber { @@ -32,8 +31,13 @@ public class ConfigSubscriber { private State state = State.OPEN; protected List<ConfigHandle<? extends ConfigInstance>> subscriptionHandles = new ArrayList<>(); private final ConfigSource source; + + /** The last complete config generation received by this */ private long generation = -1; + /** Whether the last generation received was due to a system-internal redeploy, not an application package change */ + private boolean internalRedeploy = false; + /** * Reuse requesters for equal source sets, limit number if many subscriptions. */ @@ -217,10 +221,11 @@ public class ConfigSubscriber { /** * Acquire a snapshot of all configs with the same generation within a timeout. + * * @param timeoutInMillis timeout to wait in milliseconds * @param requireChange if set, at least one config have to change * @return true, if a new config generation has been found for all configs (additionally requires - * that at lest one of them has changed if <code>requireChange</code> is true), false otherwise + * that at lest one of them has changed if <code>requireChange</code> is true), false otherwise */ private boolean acquireSnapshot(long timeoutInMillis, boolean requireChange) { if (state == State.CLOSED) return false; @@ -235,20 +240,22 @@ public class ConfigSubscriber { h.setChanged(false); // Reset this flag, if it was set, the user should have acted on it the last time this method returned true. } boolean reconfigDue; + boolean internalRedeployOnly = true; do { // Keep on polling the subscriptions until we have a new generation across the board, or it times out for (ConfigHandle<? extends ConfigInstance> h : subscriptionHandles) { ConfigSubscription<? extends ConfigInstance> subscription = h.subscription(); - if (!subscription.nextConfig(timeLeftMillis)) { + if ( ! subscription.nextConfig(timeLeftMillis)) { // This subscriber has no new state and we know it has exhausted all time return false; } throwIfExceptionSet(subscription); ConfigSubscription.ConfigState<? extends ConfigInstance> config = subscription.getConfigState(); if (currentGen == null) currentGen = config.getGeneration(); - if (!currentGen.equals(config.getGeneration())) allGenerationsTheSame = false; + if ( ! currentGen.equals(config.getGeneration())) allGenerationsTheSame = false; allGenerationsChanged = allGenerationsChanged && config.isGenerationChanged(); if (config.isConfigChanged()) anyConfigChanged = true; + internalRedeployOnly = internalRedeployOnly && config.isInternalRedeploy(); timeLeftMillis = timeLeftMillis - (System.currentTimeMillis() - started); } reconfigDue = (anyConfigChanged || !requireChange) && allGenerationsChanged && allGenerationsTheSame; @@ -260,6 +267,7 @@ public class ConfigSubscriber { // This indicates the clients will possibly reconfigure their services, so "reset" changed-logic in subscriptions. // Also if appropriate update the changed flag on the handler, which clients use. markSubsChangedSeen(currentGen); + internalRedeploy = internalRedeployOnly; generation = currentGen; } return reconfigDue; @@ -423,6 +431,12 @@ public class ConfigSubscriber { } /** + * Whether the current config generation received by this was due to a system-internal redeploy, + * not an application package change + */ + public boolean isInternalRedeploy() { return internalRedeploy; } + + /** * Convenience interface for clients who only subscribe to one config. Implement this, and pass it to {@link ConfigSubscriber#subscribe(SingleSubscriber, Class, String)}. * * @author vegardh diff --git a/config/src/main/java/com/yahoo/config/subscription/RawSource.java b/config/src/main/java/com/yahoo/config/subscription/RawSource.java index f9d79561c1c..d68b503eb74 100644 --- a/config/src/main/java/com/yahoo/config/subscription/RawSource.java +++ b/config/src/main/java/com/yahoo/config/subscription/RawSource.java @@ -3,11 +3,11 @@ package com.yahoo.config.subscription; /** * Source specifying raw config, where payload is given programmatically - * @author vegardh - * @since 5.1 * + * @author vegardh */ public class RawSource implements ConfigSource { + public final String payload; /** diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java index 02c455bf1c3..76241c560e4 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/ConfigSubscription.java @@ -23,9 +23,9 @@ import com.yahoo.vespa.config.protocol.DefContent; * Represents one active subscription to one config * * @author vegardh - * @since 5.1 */ public abstract class ConfigSubscription<T extends ConfigInstance> { + protected static Logger log = Logger.getLogger(ConfigSubscription.class.getName()); protected final ConfigSubscriber subscriber; private final AtomicReference<ConfigState<T>> config = new AtomicReference<>(); @@ -35,34 +35,49 @@ public abstract class ConfigSubscription<T extends ConfigInstance> { private State state = State.OPEN; public static class ConfigState<T extends ConfigInstance> { + private final boolean configChanged; private final boolean generationChanged; private final T config; private final Long generation; - private ConfigState(boolean generationChanged, Long generation, boolean configChanged, T config) { - this.configChanged = configChanged; - this.config = config; + private final boolean internalRedeploy; + + private ConfigState(boolean generationChanged, Long generation, boolean internalRedeploy, boolean configChanged, T config) { this.generationChanged = generationChanged; this.generation = generation; + this.internalRedeploy = internalRedeploy; + this.configChanged = configChanged; + this.config = config; } + private ConfigState(Long generation, T config) { - this(false, generation,false, config); + this(false, generation, false, false, config); } + private ConfigState() { - this(false, 0L, false, null); + this(false, 0L, false, false, null); } + private ConfigState<T> createUnchanged() { return new ConfigState<T>(generation, config); } public boolean isConfigChanged() { return configChanged; } public boolean isGenerationChanged() { return generationChanged; } public Long getGeneration() { return generation; } + + /** + * Returns whether this config generation was caused by a system-internal redeploy, + * not an application package change + */ + public boolean isInternalRedeploy() { return internalRedeploy; } + public T getConfig() { return config; } + } + /** * If non-null: The user has set this generation explicitly. nextConfig should take this into account. * Access to these variables _must_ be synchronized, as nextConfig and reload() is likely to be run from * independent threads. */ - private final AtomicReference<Long> reloadedGeneration = new AtomicReference<>(); enum State { @@ -168,30 +183,28 @@ public abstract class ConfigSubscription<T extends ConfigInstance> { } void setConfig(Long generation, T config) { - this.config.set(new ConfigState<>(true, generation, true, config)); + this.config.set(new ConfigState<>(true, generation, false, true, config)); } // Only used by {@link FileConfigSubscription} protected void setConfigIncGen(T config) { ConfigState<T> prev = this.config.get(); - setConfig(prev.getGeneration() + 1, config); + this.config.set(new ConfigState<>(true, prev.getGeneration() + 1, prev.isInternalRedeploy(), true, config)); } - // Only used by {@link FileConfigSubscription} and {@link ConfigSetSubscription} - protected void setConfigIfChangedIncGen(T config) { - ConfigState<T> prev = this.config.get(); - this.config.set(new ConfigState<>(true, prev.getGeneration() + 1, - !config.equals(prev.getConfig()), config)); - } protected void setConfigIfChanged(T config) { ConfigState<T> prev = this.config.get(); - this.config.set(new ConfigState<>(true, prev.getGeneration(), - !config.equals(prev.getConfig()), config)); + this.config.set(new ConfigState<>(true, prev.getGeneration(), prev.isInternalRedeploy(), !config.equals(prev.getConfig()), config)); } void setGeneration(Long generation) { - ConfigState<T> c = config.get(); - this.config.set(new ConfigState<>(true, generation, c.isConfigChanged(), c.getConfig())); + ConfigState<T> prev = config.get(); + this.config.set(new ConfigState<>(true, generation, prev.isInternalRedeploy(), prev.isConfigChanged(), prev.getConfig())); + } + + void setInternalRedeploy(boolean internalRedeploy) { + ConfigState<T> prev = config.get(); + this.config.set(new ConfigState<>(prev.isGenerationChanged(), prev.getGeneration(), prev.isConfigChanged(), internalRedeploy, prev.getConfig())); } /** diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/FileConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/FileConfigSubscription.java index 8c3f87d0702..bcee06cd667 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/FileConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/FileConfigSubscription.java @@ -17,9 +17,8 @@ import com.yahoo.log.LogLevel; /** * Subscription used when config id is file:... - * @author vegardh - * @since 5.1 * + * @author vegardh */ public class FileConfigSubscription<T extends ConfigInstance> extends ConfigSubscription<T> { diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java index 068b24cdf1a..69c57120577 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/GenericJRTConfigSubscription.java @@ -15,6 +15,7 @@ import com.yahoo.vespa.config.protocol.JRTClientConfigRequest; /** * A JRT subscription which does not use the config class, but {@link com.yahoo.vespa.config.RawConfig} instead. * Used by config proxy. + * * @author vegardh * */ diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java index 3eea87ab5ba..d95b11533d1 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java @@ -22,13 +22,14 @@ import com.yahoo.vespa.config.protocol.Payload; * A JRT config subscription uses one {@link JRTConfigRequester} to fetch config using Vespa RPC from a config source, typically proxy or server * * @author vegardh - * @since 5.1 */ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubscription<T> { + private JRTConfigRequester requester; private TimingValues timingValues; + // Last time we got an OK JRT callback for this - private long lastOK=0; + private long lastOK = 0; /** * The queue containing either nothing or the one (newest) request that has got callback from JRT, @@ -67,8 +68,11 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc } /** - * Polls the callback queue and <em>maybe</em> sets the following (caller must check): generation, generation changed, config, config changed - * Important: it never <em>resets</em> those flags, we must persist that state until the {@link ConfigSubscriber} clears it + * Polls the callback queue and <em>maybe</em> sets the following (caller must check): + * generation, generation changed, config, config changed + * Important: it never <em>resets</em> those flags, we must persist that state until the + * {@link ConfigSubscriber} clears it + * * @param timeoutMillis timeout when polling (returns after at most this time) * @return true if it got anything off the queue and <em>maybe</em> changed any state, false if timed out taking from queue */ @@ -85,6 +89,7 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc return false; } if (jrtReq.hasUpdatedGeneration()) { + setInternalRedeploy(jrtReq.responseIsInternalRedeploy()); if (jrtReq.hasUpdatedConfig()) { setNewConfig(jrtReq); } else { diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JarConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/JarConfigSubscription.java index 4870d69bb23..1b2daba32a4 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JarConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JarConfigSubscription.java @@ -22,10 +22,9 @@ import com.yahoo.vespa.config.ConfigPayload; * * @author vegardh * @author gjoranv - * @since 5.1 - * */ public class JarConfigSubscription<T extends ConfigInstance> extends ConfigSubscription<T> { + private final String jarName; private final String path; private ZipEntry zipEntry = null; diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/MockConnection.java b/config/src/main/java/com/yahoo/config/subscription/impl/MockConnection.java index cb623437dba..9ad8f5c6ba2 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/MockConnection.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/MockConnection.java @@ -111,12 +111,14 @@ public class MockConnection implements ConnectionPool, com.yahoo.vespa.config.Co } static class OKResponseHandler extends AbstractResponseHandler { + protected void createResponse() { JRTServerConfigRequestV3 jrtReq = JRTServerConfigRequestV3.createFromRequest(request); Payload payload = Payload.from(ConfigPayload.empty()); long generation = 1; - jrtReq.addOkResponse(payload, generation, ConfigUtils.getMd5(payload.getData())); + jrtReq.addOkResponse(payload, generation, false, ConfigUtils.getMd5(payload.getData())); } + } public interface ResponseHandler extends Runnable { @@ -131,6 +133,7 @@ public class MockConnection implements ConnectionPool, com.yahoo.vespa.config.Co } public abstract static class AbstractResponseHandler implements ResponseHandler { + private RequestWaiter requestWaiter; protected Request request; diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/RawConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/RawConfigSubscription.java index 0d3f04361aa..60811dd38d1 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/RawConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/RawConfigSubscription.java @@ -16,10 +16,9 @@ import com.yahoo.vespa.config.ConfigPayload; * Config is the actual text given after the config id, with newlines * * @author vegardh - * @since 5.1 - * */ public class RawConfigSubscription<T extends ConfigInstance> extends ConfigSubscription<T> { + final String inputPayload; String payload; diff --git a/config/src/main/java/com/yahoo/vespa/config/RawConfig.java b/config/src/main/java/com/yahoo/vespa/config/RawConfig.java index 75c5161103e..ae4431e5195 100755 --- a/config/src/main/java/com/yahoo/vespa/config/RawConfig.java +++ b/config/src/main/java/com/yahoo/vespa/config/RawConfig.java @@ -31,6 +31,7 @@ public class RawConfig extends ConfigInstance { private final String configMd5; private final Optional<VespaVersion> vespaVersion; private long generation; + private boolean internalRedeploy; /** * Constructor for an empty config (not yet resolved). @@ -38,27 +39,29 @@ public class RawConfig extends ConfigInstance { * @param defMd5 The md5 sum of the .def-file. */ public RawConfig(ConfigKey<?> key, String defMd5) { - this(key, defMd5, null, "", 0L, 0, Collections.emptyList(), Optional.empty()); + this(key, defMd5, null, "", 0L, false, 0, Collections.emptyList(), Optional.empty()); } public RawConfig(ConfigKey<?> key, String defMd5, Payload payload, String configMd5, long generation, - List<String> defContent, Optional<VespaVersion> vespaVersion) { - this(key, defMd5, payload, configMd5, generation, 0, defContent, vespaVersion); + boolean internalRedeploy, List<String> defContent, Optional<VespaVersion> vespaVersion) { + this(key, defMd5, payload, configMd5, generation, internalRedeploy, 0, defContent, vespaVersion); } /** Copy constructor */ public RawConfig(RawConfig rawConfig) { this(rawConfig.key, rawConfig.defMd5, rawConfig.payload, rawConfig.configMd5, - rawConfig.generation, rawConfig.errorCode, rawConfig.defContent, rawConfig.getVespaVersion()); + rawConfig.generation, rawConfig.internalRedeploy, rawConfig.errorCode, + rawConfig.defContent, rawConfig.getVespaVersion()); } public RawConfig(ConfigKey<?> key, String defMd5, Payload payload, String configMd5, long generation, - int errorCode, List<String> defContent, Optional<VespaVersion> vespaVersion) { + boolean internalRedeploy, int errorCode, List<String> defContent, Optional<VespaVersion> vespaVersion) { this.key = key; this.defMd5 = ConfigUtils.getDefMd5FromRequest(defMd5, defContent); this.payload = payload; this.configMd5 = configMd5; this.generation = generation; + this.internalRedeploy = internalRedeploy; this.errorCode = errorCode; this.defContent = defContent; this.vespaVersion = vespaVersion; @@ -69,8 +72,15 @@ public class RawConfig extends ConfigInstance { * @param req a {@link JRTClientConfigRequest} */ public static RawConfig createFromResponseParameters(JRTClientConfigRequest req) { - return new RawConfig(req.getConfigKey(), req.getConfigKey().getMd5(), req.getNewPayload(), req.getNewConfigMd5(), - req.getNewGeneration(), 0, req.getDefContent().asList(), req.getVespaVersion()); + return new RawConfig(req.getConfigKey(), + req.getConfigKey().getMd5(), + req.getNewPayload(), + req.getNewConfigMd5(), + req.getNewGeneration(), + req.responseIsInternalRedeploy(), + 0, + req.getDefContent().asList(), + req.getVespaVersion()); } /** @@ -78,58 +88,47 @@ public class RawConfig extends ConfigInstance { * @param req a {@link JRTClientConfigRequest} */ public static RawConfig createFromServerRequest(JRTServerConfigRequest req) { - return new RawConfig(req.getConfigKey(), req.getConfigKey().getMd5() , Payload.from(new Utf8String(""), CompressionInfo.uncompressed()), req.getRequestConfigMd5(), - req.getRequestGeneration(), 0, req.getDefContent().asList(), req.getVespaVersion()); + return new RawConfig(req.getConfigKey(), + req.getConfigKey().getMd5() , + Payload.from(new Utf8String(""), CompressionInfo.uncompressed()), + req.getRequestConfigMd5(), + req.getRequestGeneration(), + req.isInternalRedeploy(), + 0, + req.getDefContent().asList(), + req.getVespaVersion()); } - public ConfigKey<?> getKey() { - return key; - } + public ConfigKey<?> getKey() { return key; } - public String getName() { - return key.getName(); - } + public String getName() { return key.getName(); } - public String getNamespace() { - return key.getNamespace(); - } + public String getNamespace() { return key.getNamespace(); } - public String getConfigId() { - return key.getConfigId(); - } + public String getConfigId() { return key.getConfigId(); } - public String getConfigMd5() { - return configMd5; - } + public String getConfigMd5() { return configMd5; } - public String getDefMd5() { - return defMd5; - } + public String getDefMd5() { return defMd5; } - public long getGeneration() { - return generation; - } + public long getGeneration() { return generation; } - public void setGeneration(long generation) { - this.generation = generation; - } + public void setGeneration(long generation) { this.generation = generation; } - public Payload getPayload() { - return payload; - } + /** + * Returns whether this config generation was created by a system internal redeploy, not an + * application package change. + */ + public boolean isInternalRedeploy() { return internalRedeploy; } - public int errorCode() { - return errorCode; - } + public Payload getPayload() { return payload; } - public String getDefNamespace() { - return key.getNamespace(); - } + public int errorCode() { return errorCode; } - public Optional<VespaVersion> getVespaVersion() { - return vespaVersion; - } + public String getDefNamespace() { return key.getNamespace(); } + + public Optional<VespaVersion> getVespaVersion() { return vespaVersion; } /** * Returns true if this config is equal to the config (same payload md5) in the given request. diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java b/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java index 5666417a50a..c07be8337fe 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java @@ -13,8 +13,7 @@ import java.util.List; * of this must be thread safe, because a response may be cached and, the methods below should be callable * from multiple request handler threads. * - * @author lulf - * @since 5.1.14 + * @author Ulf Lilleengen */ public interface ConfigResponse { @@ -24,6 +23,8 @@ public interface ConfigResponse { long getGeneration(); + boolean isInternalRedeploy(); + String getConfigMd5(); void serialize(OutputStream os, CompressionType uncompressed) throws IOException; diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequest.java b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequest.java index 8e554dc45d8..25fecba425c 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequest.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequest.java @@ -4,12 +4,13 @@ package com.yahoo.vespa.config.protocol; /** * Interface for config requests used by clients. * - * @author lulf - * @since 5.3 + * @author Ulf Lilleengen */ public interface JRTClientConfigRequest extends JRTConfigRequest { + /** * Validate config response given by the server. If none is given, or an error occurred, this should return false. + * * @return true if valid response, false if not. */ boolean validateResponse(); @@ -17,19 +18,22 @@ public interface JRTClientConfigRequest extends JRTConfigRequest { /** * Test whether ot not the returned config has an updated generation. This should return false if no response have * been given. + * * @return true if generation is updated, false if not. */ boolean hasUpdatedGeneration(); /** * Return the payload in the response given by the server. The payload will be empty if no response was given. + * * @return the config payload. */ Payload getNewPayload(); /** - * Create a new {@link JRTClientConfigRequest} based on this request based on the same request parameters, but having - * the timeout changed. + * Create a new {@link JRTClientConfigRequest} based on this request based on the same request parameters, + * but having the timeout changed. + * * @param timeout server timeout of the new request. * @return a new {@link JRTClientConfigRequest} instance. */ @@ -37,44 +41,57 @@ public interface JRTClientConfigRequest extends JRTConfigRequest { /** * Test whether or not the returned request is an error. + * * @return true if error, false if not. */ boolean isError(); /** * Get the generation of the newly provided config. If none has been given, 0 should be returned. + * * @return the new generation. */ long getNewGeneration(); + /** Returns whether this config change is due to an internal change not an application package change */ + boolean responseIsInternalRedeploy(); + /** * Get the config md5 of the config returned by the server. Return an empty string if no response has been returned. + * * @return a config md5. */ String getNewConfigMd5(); /** - * Test whether or not the payload is contained in this response or not. Should return false for error responses as well. + * Test whether or not the payload is contained in this response or not. + * Should return false for error responses as well. + * * @return true if empty, false if not. */ boolean containsPayload(); /** - * Test whether or not the response contains an updated config or not. False if no response has been returned. + * Test whether or not the response contains an updated config or not. + * False if no response has been returned. + * * @return true if config is updated, false if not. */ boolean hasUpdatedConfig(); /** - * Get the {@link Trace} given in the response by the server. The {@link Trace} can be used to add further tracing - * and later printed to provide useful debug info. + * Get the {@link Trace} given in the response by the server. + * The {@link Trace} can be used to add further tracing and later printed to provide useful debug info. + * * @return a {@link Trace}. */ Trace getResponseTrace(); /** * Get config definition content. + * * @return def as lines. */ DefContent getDefContent(); + } diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java index 3d7c0dc1e80..1c842a4d1b0 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTClientConfigRequestV3.java @@ -19,8 +19,7 @@ import java.util.Optional; * * See {@link JRTServerConfigRequestV3} for protocol details. * - * @author lulf - * @since 5.19 + * @author Ulf Lilleengen */ public class JRTClientConfigRequestV3 extends SlimeClientConfigRequest { @@ -71,7 +70,10 @@ public class JRTClientConfigRequestV3 extends SlimeClientConfigRequest { requestData.getVespaVersion()); } - public static <T extends ConfigInstance> JRTClientConfigRequest createFromSub(JRTConfigSubscription<T> sub, Trace trace, CompressionType compressionType, Optional<VespaVersion> vespaVersion) { + public static <T extends ConfigInstance> JRTClientConfigRequest createFromSub(JRTConfigSubscription<T> sub, + Trace trace, + CompressionType compressionType, + Optional<VespaVersion> vespaVersion) { String hostname = ConfigUtils.getCanonicalHostName(); ConfigKey<T> key = sub.getKey(); ConfigSubscription.ConfigState<T> configState = sub.getConfigState(); @@ -88,7 +90,11 @@ public class JRTClientConfigRequestV3 extends SlimeClientConfigRequest { } - public static JRTClientConfigRequest createFromRaw(RawConfig config, long serverTimeout, Trace trace, CompressionType compressionType, Optional<VespaVersion> vespaVersion) { + public static JRTClientConfigRequest createFromRaw(RawConfig config, + long serverTimeout, + Trace trace, + CompressionType compressionType, + Optional<VespaVersion> vespaVersion) { String hostname = ConfigUtils.getCanonicalHostName(); return createWithParams(config.getKey(), DefContent.fromList(config.getDefContent()), diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequest.java b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequest.java index 54607ec4502..763f672a513 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequest.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequest.java @@ -6,13 +6,14 @@ import com.yahoo.vespa.config.GetConfigRequest; /** * Interface for config requests at the server end point. * - * @author lulf - * @since 5.3 + * @author Ulf Lilleengen */ public interface JRTServerConfigRequest extends JRTConfigRequest, GetConfigRequest { + /** * Notify this request that its delayed due to no new config being available at this point. The value * provided in this function should be returned when calling {@link #isDelayedResponse()}. + * * @param delayedResponse true if response is delayed, false if not. */ void setDelayedResponse(boolean delayedResponse); @@ -20,6 +21,7 @@ public interface JRTServerConfigRequest extends JRTConfigRequest, GetConfigReque /** * Signal error when handling this request. The error should be reflected in the request state and propagated * back to the client. + * * @param errorCode error code, as described in {@link com.yahoo.vespa.config.ErrorCode}. * @param message message to display for this error, typically printed by client. */ @@ -27,33 +29,46 @@ public interface JRTServerConfigRequest extends JRTConfigRequest, GetConfigReque /** * Signal that the request was handled and provide return values typically needed by a client. + * * @param payload The config payload that the client should receive. * @param generation The config generation of the given payload. + * @param internalRedeployment whether this payload was generated from an internal redeployment not an + * application package change * @param configMd5 The md5sum of the given payload. */ - void addOkResponse(Payload payload, long generation, String configMd5); + void addOkResponse(Payload payload, long generation, boolean internalRedeployment, String configMd5); /** * Get the current config md5 of the client config. + * * @return a config md5. */ String getRequestConfigMd5(); /** * Get the current config generation of the client config. + * * @return the current config generation. */ long getRequestGeneration(); /** * Check whether or not this request is delayed. + * * @return true if delayed, false if not. */ boolean isDelayedResponse(); /** + * Returns whether the response config was created by a system internal redeploy, not an application + * package change + */ + boolean isInternalRedeploy(); + + /** * Get the request trace for this request. The trace can be used to trace config execution to provide useful * debug info in production environments. + * * @return a {@link Trace} instance. */ Trace getRequestTrace(); @@ -65,4 +80,5 @@ public interface JRTServerConfigRequest extends JRTConfigRequest, GetConfigReque * @return A {@link Payload} that satisfies this request format. */ Payload payloadFromResponse(ConfigResponse response); + } diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java index b0096444820..f70ebf39a28 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java @@ -20,17 +20,21 @@ import java.io.IOException; * The implementation of addOkResponse is optimized for doing as little copying of payload data as possible, ensuring * that we get a lower memory footprint. * - * @author lulf - * @since 5.19 + * @author Ulf Lilleengen */ +// TODO: Merge with parent public class JRTServerConfigRequestV3 extends SlimeServerConfigRequest { + /** Response field */ + private boolean internalRedeploy = false; + protected JRTServerConfigRequestV3(Request request) { super(request); } @Override - public void addOkResponse(Payload payload, long generation, String configMd5) { + public void addOkResponse(Payload payload, long generation, boolean internalRedeploy, String configMd5) { + this.internalRedeploy = internalRedeploy; boolean changedConfig = !configMd5.equals(getRequestConfigMd5()); boolean changedConfigAndNewGeneration = changedConfig && ConfigUtils.isGenerationNewer(generation, getRequestGeneration()); Payload responsePayload = payload.withCompression(getCompressionType()); @@ -41,6 +45,7 @@ public class JRTServerConfigRequestV3 extends SlimeServerConfigRequest { addCommonReturnValues(jsonGenerator); setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_MD5, configMd5); setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_GENERATION, generation); + setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_INTERNAL_REDEPLOY, internalRedeploy); jsonGenerator.writeObjectFieldStart(SlimeResponseData.RESPONSE_COMPRESSION_INFO); if (responsePayload == null) { throw new RuntimeException("Payload is null for ' " + this + ", not able to create response"); @@ -73,7 +78,11 @@ public class JRTServerConfigRequestV3 extends SlimeServerConfigRequest { return 3; } + @Override + public boolean isInternalRedeploy() { return internalRedeploy; } + public static JRTServerConfigRequestV3 createFromRequest(Request req) { return new JRTServerConfigRequestV3(req); } + } diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeClientConfigRequest.java b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeClientConfigRequest.java index 19323fb1a85..386757d2afc 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeClientConfigRequest.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeClientConfigRequest.java @@ -16,8 +16,7 @@ import java.util.logging.Logger; * Base class for new generation of config requests based on {@link Slime}. Allows for some customization of * payload encoding and decoding, as well as adding extra request/response fields. * - * @author lulf - * @since 5.18 + * @author Ulf Lilleengen */ public abstract class SlimeClientConfigRequest implements JRTClientConfigRequest { @@ -82,6 +81,7 @@ public abstract class SlimeClientConfigRequest implements JRTClientConfigRequest .append(",").append(getVespaVersion()).append("'\n"); sb.append("response='").append(getNewConfigMd5()) .append(",").append(getNewGeneration()) + .append(",").append(responseIsInternalRedeploy()) .append("'\n"); return sb.toString(); } @@ -203,6 +203,11 @@ public abstract class SlimeClientConfigRequest implements JRTClientConfigRequest } @Override + public boolean responseIsInternalRedeploy() { + return responseData.getResponseInternalRedeployment(); + } + + @Override public long getRequestGeneration() { return requestData.getRequestGeneration(); } diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeConfigResponse.java b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeConfigResponse.java index e7aae3251d0..566e3597269 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeConfigResponse.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeConfigResponse.java @@ -15,8 +15,7 @@ import java.util.List; /** * Class for serializing config responses based on {@link com.yahoo.slime.Slime} implementing the {@link ConfigResponse} interface. * - * @author lulf - * @since 5.1 + * @author Ulf Lilleengen */ public class SlimeConfigResponse implements ConfigResponse { @@ -24,17 +23,27 @@ public class SlimeConfigResponse implements ConfigResponse { private final CompressionInfo compressionInfo; private final InnerCNode targetDef; private final long generation; + private final boolean internalRedeploy; private final String configMd5; - public static SlimeConfigResponse fromConfigPayload(ConfigPayload payload, InnerCNode targetDef, long generation, String configMd5) { + public static SlimeConfigResponse fromConfigPayload(ConfigPayload payload, InnerCNode targetDef, long generation, + boolean internalRedeploy, String configMd5) { Utf8Array data = payload.toUtf8Array(true); - return new SlimeConfigResponse(data, targetDef, generation, configMd5, CompressionInfo.create(CompressionType.UNCOMPRESSED, data.getByteLength())); + return new SlimeConfigResponse(data, targetDef, generation, internalRedeploy, + configMd5, + CompressionInfo.create(CompressionType.UNCOMPRESSED, data.getByteLength())); } - public SlimeConfigResponse(Utf8Array payload, InnerCNode targetDef, long generation, String configMd5, CompressionInfo compressionInfo) { + public SlimeConfigResponse(Utf8Array payload, + InnerCNode targetDef, + long generation, + boolean internalRedeploy, + String configMd5, + CompressionInfo compressionInfo) { this.payload = payload; this.targetDef = targetDef; this.generation = generation; + this.internalRedeploy = internalRedeploy; this.configMd5 = configMd5; this.compressionInfo = compressionInfo; } @@ -62,6 +71,13 @@ public class SlimeConfigResponse implements ConfigResponse { return generation; } + /** + * Returns whether this application instance was produced by an internal redeployment, + * not an application package change + */ + @Override + public boolean isInternalRedeploy() { return internalRedeploy; } + @Override public String getConfigMd5() { return configMd5; @@ -81,4 +97,5 @@ public class SlimeConfigResponse implements ConfigResponse { @Override public CompressionInfo getCompressionInfo() { return compressionInfo; } + } diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeResponseData.java b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeResponseData.java index 04f6df7a45b..6add29074d1 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeResponseData.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeResponseData.java @@ -11,10 +11,10 @@ import com.yahoo.text.Utf8; * Contains response data for a slime response and methods for decoding the response data that * are common to all {@link Slime} based config requests. * - * @author lulf - * @since 5.18 + * @author Ulf Lilleengen */ class SlimeResponseData { + static final String RESPONSE_VERSION = "version"; static final String RESPONSE_DEF_NAME = "defName"; static final String RESPONSE_DEF_NAMESPACE = "defNamespace"; @@ -24,6 +24,7 @@ class SlimeResponseData { static final String RESPONSE_TRACE = "trace"; static final String RESPONSE_CONFIG_MD5 = "configMD5"; static final String RESPONSE_CONFIG_GENERATION = "generation"; + static final String RESPONSE_INTERNAL_REDEPLOY = "internalRedeploy"; static final String RESPONSE_COMPRESSION_INFO = "compressionInfo"; private final Request request; @@ -66,4 +67,10 @@ class SlimeResponseData { CompressionInfo getCompressionInfo() { return CompressionInfo.fromSlime(getResponseField(RESPONSE_COMPRESSION_INFO)); } + + boolean getResponseInternalRedeployment() { + Inspector inspector = getResponseField(RESPONSE_INTERNAL_REDEPLOY); + return inspector.valid() ? inspector.asBool() : false; + } + } diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeServerConfigRequest.java b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeServerConfigRequest.java index 662eacc4eea..41bf38ef1dc 100644 --- a/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeServerConfigRequest.java +++ b/config/src/main/java/com/yahoo/vespa/config/protocol/SlimeServerConfigRequest.java @@ -19,8 +19,7 @@ import java.util.logging.Logger; * payload encoding and decoding, as well as adding extra request/response fields. Used by both V2 and V3 * config protocol. * - * @author lulf - * @since 5.18 + * @author Ulf Lilleengen */ abstract class SlimeServerConfigRequest implements JRTServerConfigRequest { @@ -164,6 +163,10 @@ abstract class SlimeServerConfigRequest implements JRTServerConfigRequest { jsonGenerator.writeNumberField(fieldName, value); } + protected static void setResponseField(JsonGenerator jsonGenerator, String fieldName, boolean value) throws IOException { + jsonGenerator.writeBooleanField(fieldName, value); + } + @Override public long getRequestGeneration() { return requestData.getRequestGeneration(); diff --git a/config/src/test/java/com/yahoo/config/subscription/ConfigGetterTest.java b/config/src/test/java/com/yahoo/config/subscription/ConfigGetterTest.java index b9787f76e0c..731a5f50816 100644 --- a/config/src/test/java/com/yahoo/config/subscription/ConfigGetterTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/ConfigGetterTest.java @@ -39,7 +39,7 @@ public class ConfigGetterTest { assertThat(config, is(serviceConfig)); } -@Test + @Test public void testGetFromRawSource() { ConfigGetter<AppConfig> getter = new ConfigGetter<>(new RawSource("message \"one\""), AppConfig.class); AppConfig config = getter.getConfig("test"); diff --git a/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java b/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java index 8acab56d838..d17a2ff61f4 100644 --- a/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java @@ -85,7 +85,7 @@ public class ConfigSetSubscriptionTest { assertEquals(hA1.getConfig().times(), 89); assertEquals(hS.getConfig().stringVal(), "StringVal"); - //Reconfigure all configs, generation should change + // Reconfigure all configs, generation should change a0builder.message("A new message, 0").times(880); a1builder.message("A new message, 1").times(890); barBuilder.stringVal("new StringVal"); diff --git a/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java b/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java index 586ad7857f4..3aa422eb116 100644 --- a/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java @@ -3,6 +3,7 @@ package com.yahoo.config.subscription; import com.yahoo.config.ConfigInstance; import com.yahoo.config.ConfigurationRuntimeException; +import com.yahoo.config.subscription.impl.GenericConfigHandle; import com.yahoo.foo.SimpletypesConfig; import com.yahoo.foo.AppConfig; import com.yahoo.config.subscription.impl.ConfigSubscription; diff --git a/config/src/test/java/com/yahoo/vespa/config/RawConfigTest.java b/config/src/test/java/com/yahoo/vespa/config/RawConfigTest.java index 33560ff666d..b19da2c1689 100644 --- a/config/src/test/java/com/yahoo/vespa/config/RawConfigTest.java +++ b/config/src/test/java/com/yahoo/vespa/config/RawConfigTest.java @@ -60,14 +60,14 @@ public class RawConfigTest { assertThat(config.hashCode(), is(not(new RawConfig(key, "a").hashCode()))); // different def md5 // different generation - config = new RawConfig(key, defMd5, payload, configMd5, generation, defContent, Optional.empty()); - RawConfig config2 = new RawConfig(key, defMd5, payload, configMd5, 2L, defContent, Optional.empty()); + config = new RawConfig(key, defMd5, payload, configMd5, generation, false, defContent, Optional.empty()); + RawConfig config2 = new RawConfig(key, defMd5, payload, configMd5, 2L, false, defContent, Optional.empty()); assertThat(config, is(not(config2))); assertThat(config.hashCode(), is(not(config2.hashCode()))); // different config md5 and with vespa version final VespaVersion vespaVersion = VespaVersion.fromString("5.37.38"); - RawConfig config3 = new RawConfig(key, defMd5, payload, "9999", generation, defContent, Optional.of(vespaVersion)); + RawConfig config3 = new RawConfig(key, defMd5, payload, "9999", generation, false, defContent, Optional.of(vespaVersion)); assertThat(config, is(not(config3))); assertThat(config.hashCode(), is(not(config3.hashCode()))); // Check that vespa version is set correctly @@ -81,19 +81,19 @@ public class RawConfigTest { assertFalse(config.equals(key)); // errors - RawConfig errorConfig1 = new RawConfig(key, defMd5, payload, configMd5, generation, 1, defContent, Optional.empty()); + RawConfig errorConfig1 = new RawConfig(key, defMd5, payload, configMd5, generation, false, 1, defContent, Optional.empty()); assertThat(errorConfig1, is(errorConfig1)); assertThat(config, is(not(errorConfig1))); assertThat(config.hashCode(), is(not(errorConfig1.hashCode()))); assertThat(errorConfig1, is(errorConfig1)); - RawConfig errorConfig2 = new RawConfig(key, defMd5, payload, configMd5, generation, 2, defContent, Optional.empty()); + RawConfig errorConfig2 = new RawConfig(key, defMd5, payload, configMd5, generation, false, 2, defContent, Optional.empty()); assertThat(errorConfig1, is(not(errorConfig2))); assertThat(errorConfig1.hashCode(), is(not(errorConfig2.hashCode()))); } @Test public void payload() { - RawConfig config = new RawConfig(key, defMd5, payload, configMd5, generation, defContent, Optional.empty()); + RawConfig config = new RawConfig(key, defMd5, payload, configMd5, generation, false, defContent, Optional.empty()); assertThat(config.getPayload(), is(payload)); assertThat(config.getConfigMd5(), is(configMd5)); assertThat(config.getGeneration(), is(generation)); @@ -104,19 +104,19 @@ public class RawConfigTest { public void require_correct_defmd5() { final String defMd5ForEmptyDefContent = "d41d8cd98f00b204e9800998ecf8427e"; - RawConfig config = new RawConfig(key, null, payload, configMd5, generation, defContent, Optional.empty()); + RawConfig config = new RawConfig(key, null, payload, configMd5, generation, false, defContent, Optional.empty()); assertThat(config.getDefMd5(), is(defMd5)); - config = new RawConfig(key, "", payload, configMd5, generation, defContent, Optional.empty()); + config = new RawConfig(key, "", payload, configMd5, generation, false, defContent, Optional.empty()); assertThat(config.getDefMd5(), is(defMd5)); - config = new RawConfig(key, defMd5, payload, configMd5, generation, defContent, Optional.empty()); + config = new RawConfig(key, defMd5, payload, configMd5, generation, false, defContent, Optional.empty()); assertThat(config.getDefMd5(), is(defMd5)); - config = new RawConfig(key, null, payload, configMd5, generation, null, Optional.empty()); + config = new RawConfig(key, null, payload, configMd5, generation, false, null, Optional.empty()); assertNull(config.getDefMd5()); - config = new RawConfig(key, null, payload, configMd5, generation, Arrays.asList(""), Optional.empty()); + config = new RawConfig(key, null, payload, configMd5, generation, false, Arrays.asList(""), Optional.empty()); assertThat(config.getDefMd5(), is(defMd5ForEmptyDefContent)); - config = new RawConfig(key, "", payload, configMd5, generation, null, Optional.empty()); + config = new RawConfig(key, "", payload, configMd5, generation, false, null, Optional.empty()); assertThat(config.getDefMd5(), is("")); - config = new RawConfig(key, "", payload, configMd5, generation, Arrays.asList(""), Optional.empty()); + config = new RawConfig(key, "", payload, configMd5, generation, false, Arrays.asList(""), Optional.empty()); assertThat(config.getDefMd5(), is(defMd5ForEmptyDefContent)); } diff --git a/config/src/test/java/com/yahoo/vespa/config/protocol/ConfigResponseTest.java b/config/src/test/java/com/yahoo/vespa/config/protocol/ConfigResponseTest.java index 9a7b216d6b0..91adc544d88 100644 --- a/config/src/test/java/com/yahoo/vespa/config/protocol/ConfigResponseTest.java +++ b/config/src/test/java/com/yahoo/vespa/config/protocol/ConfigResponseTest.java @@ -20,8 +20,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; /** - * @author lulf - * @since 5.1 + * @author Ulf Lilleengen */ public class ConfigResponseTest { @@ -30,7 +29,7 @@ public class ConfigResponseTest { ConfigPayload configPayload = ConfigPayload.fromInstance(new SimpletypesConfig(new SimpletypesConfig.Builder())); DefParser dParser = new DefParser(SimpletypesConfig.getDefName(), new StringReader(StringUtilities.implode(SimpletypesConfig.CONFIG_DEF_SCHEMA, "\n"))); InnerCNode targetDef = dParser.getTree(); - ConfigResponse response = SlimeConfigResponse.fromConfigPayload(configPayload, targetDef, 3, "mymd5"); + ConfigResponse response = SlimeConfigResponse.fromConfigPayload(configPayload, targetDef, 3, false, "mymd5"); List<String> payload = response.getLegacyPayload(); assertNotNull(payload); assertThat(payload.size(), is(6)); @@ -45,13 +44,12 @@ public class ConfigResponseTest { @Test public void require_that_slime_response_decompresses_on_serialize() throws IOException { - ConfigPayload configPayload = ConfigPayload.fromInstance(new SimpletypesConfig(new SimpletypesConfig.Builder())); DefParser dParser = new DefParser(SimpletypesConfig.getDefName(), new StringReader(StringUtilities.implode(SimpletypesConfig.CONFIG_DEF_SCHEMA, "\n"))); InnerCNode targetDef = dParser.getTree(); Utf8Array data = configPayload.toUtf8Array(true); Utf8Array bytes = new Utf8Array(new LZ4PayloadCompressor().compress(data.getBytes())); - ConfigResponse response = new SlimeConfigResponse(bytes, targetDef, 3, "mymd5", CompressionInfo.create(CompressionType.LZ4, data.getByteLength())); + ConfigResponse response = new SlimeConfigResponse(bytes, targetDef, 3, false, "mymd5", CompressionInfo.create(CompressionType.LZ4, data.getByteLength())); List<String> payload = response.getLegacyPayload(); assertNotNull(payload); assertThat(payload.size(), is(6)); @@ -63,4 +61,5 @@ public class ConfigResponseTest { response.serialize(baos, CompressionType.UNCOMPRESSED); assertThat(baos.toString(), is("{\"boolval\":false,\"doubleval\":0.0,\"enumval\":\"VAL1\",\"intval\":0,\"longval\":0,\"stringval\":\"s\"}")); } + } diff --git a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java index 1f5274b832b..cdaaf2061f4 100644 --- a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java +++ b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java @@ -125,7 +125,7 @@ public abstract class JRTConfigRequestBase { @Test public void next_request_when_error_is_correct() { - serverReq.addOkResponse(createPayload(), 999999, "newmd5"); + serverReq.addOkResponse(createPayload(), 999999, false, "newmd5"); serverReq.addErrorResponse(ErrorCode.OUTDATED_CONFIG, "error message"); System.out.println(serverReq); JRTClientConfigRequest next = clientReq.nextRequest(6); @@ -141,7 +141,7 @@ public abstract class JRTConfigRequestBase { Payload payload = createPayload("vale"); String md5 = ConfigUtils.getMd5(payload.getData()); long generation = 4L; - serverReq.addOkResponse(payload, generation, md5); + serverReq.addOkResponse(payload, generation, false, md5); assertTrue(clientReq.validateResponse()); assertThat(clientReq.getNewPayload().withCompression(CompressionType.UNCOMPRESSED).getData().toString(), is(payload.getData().toString())); assertThat(clientReq.getNewGeneration(), is(4L)); @@ -168,7 +168,7 @@ public abstract class JRTConfigRequestBase { @Test public void generation_only_is_updated() { Payload payload = createPayload(); - serverReq.addOkResponse(payload, 4L, ConfigUtils.getMd5(payload.getData())); + serverReq.addOkResponse(payload, 4L, false, ConfigUtils.getMd5(payload.getData())); boolean value = clientReq.validateResponse(); assertTrue(clientReq.errorMessage(), value); assertFalse(clientReq.hasUpdatedConfig()); @@ -188,7 +188,7 @@ public abstract class JRTConfigRequestBase { @Test public void nothing_is_updated() { Payload payload = createPayload(); - serverReq.addOkResponse(payload, currentGeneration, configMd5); + serverReq.addOkResponse(payload, currentGeneration, false, configMd5); assertTrue(clientReq.validateResponse()); assertFalse(clientReq.hasUpdatedConfig()); assertFalse(clientReq.hasUpdatedGeneration()); @@ -199,7 +199,7 @@ public abstract class JRTConfigRequestBase { Payload payload = Payload.from(ConfigPayload.empty()); clientReq = createReq(payload); serverReq = createReq(clientReq.getRequest()); - serverReq.addOkResponse(payload, currentGeneration, ConfigUtils.getMd5(payload.getData())); + serverReq.addOkResponse(payload, currentGeneration, false, ConfigUtils.getMd5(payload.getData())); boolean val = clientReq.validateResponse(); assertTrue(clientReq.errorMessage(), val); assertFalse(clientReq.hasUpdatedConfig()); @@ -238,7 +238,7 @@ public abstract class JRTConfigRequestBase { @Override protected void createResponse() { JRTServerConfigRequest serverRequest = createReq(request); - serverRequest.addOkResponse(createPayload(), currentGeneration, configMd5); + serverRequest.addOkResponse(createPayload(), currentGeneration, false, configMd5); } }); diff --git a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java index ccb68da5f85..d1879c9318b 100644 --- a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java +++ b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestV3Test.java @@ -11,6 +11,7 @@ import org.junit.Test; import java.util.Arrays; import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -70,10 +71,11 @@ public class JRTConfigRequestV3Test extends JRTConfigRequestBase { @Test public void emptypayload() { ConfigPayload payload = ConfigPayload.empty(); - SlimeConfigResponse response = SlimeConfigResponse.fromConfigPayload(payload, null, 0, ConfigUtils.getMd5(payload)); - serverReq.addOkResponse(serverReq.payloadFromResponse(response), response.getGeneration(), response.getConfigMd5()); + SlimeConfigResponse response = SlimeConfigResponse.fromConfigPayload(payload, null, 0, false, ConfigUtils.getMd5(payload)); + serverReq.addOkResponse(serverReq.payloadFromResponse(response), response.getGeneration(), false, response.getConfigMd5()); assertTrue(clientReq.validateResponse()); assertTrue(clientReq.hasUpdatedGeneration()); assertThat(clientReq.getNewPayload().withCompression(CompressionType.UNCOMPRESSED).getData().toString(), is("{}")); + assertFalse(clientReq.responseIsInternalRedeploy()); } } 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 e0b69c78cf9..f8e1bcec0c3 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 @@ -184,6 +184,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye /** * Creates a new deployment from the active application, if available. + * This is used for system internal redeployments, not on application package changes. * * @param application the active application to be redeployed * @return a new deployment from the local active, or empty if a local active application @@ -196,6 +197,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye /** * Creates a new deployment from the active application, if available. + * This is used for system internal redeployments, not on application package changes. * * @param application the active application to be redeployed * @param timeout the timeout to use for each individual deployment operation @@ -210,7 +212,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye LocalSession activeSession = getActiveSession(tenant, application); if (activeSession == null) return Optional.empty(); TimeoutBudget timeoutBudget = new TimeoutBudget(clock, timeout); - LocalSession newSession = tenant.getSessionFactory().createSessionFromExisting(activeSession, logger, timeoutBudget); + LocalSession newSession = tenant.getSessionFactory().createSessionFromExisting(activeSession, logger, true, timeoutBudget); tenant.getLocalSessionRepo().addSession(newSession); // Keep manually deployed tenant applications on the latest version, don't change version otherwise @@ -400,12 +402,15 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye throw new IllegalStateException("Session not prepared: " + sessionId); } - public long createSessionFromExisting(ApplicationId applicationId, DeployLogger logger, TimeoutBudget timeoutBudget) { + public long createSessionFromExisting(ApplicationId applicationId, + DeployLogger logger, + boolean internalRedeploy, + TimeoutBudget timeoutBudget) { Tenant tenant = tenantRepository.getTenant(applicationId.tenant()); LocalSessionRepo localSessionRepo = tenant.getLocalSessionRepo(); SessionFactory sessionFactory = tenant.getSessionFactory(); LocalSession fromSession = getExistingSession(tenant, applicationId); - LocalSession session = sessionFactory.createSessionFromExisting(fromSession, logger, timeoutBudget); + LocalSession session = sessionFactory.createSessionFromExisting(fromSession, logger, internalRedeploy, timeoutBudget); localSessionRepo.addSession(session); return session.getSessionId(); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/GetConfigContext.java b/configserver/src/main/java/com/yahoo/vespa/config/server/GetConfigContext.java index ea14b9194e8..6c9b7216e59 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/GetConfigContext.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/GetConfigContext.java @@ -9,8 +9,7 @@ import com.yahoo.vespa.config.server.tenant.TenantRepository; /** * Contains the context for serving getconfig requests so that this information does not have to be looked up multiple times. * - * @author lulf - * @since 5.8 + * @author Ulf Lilleengen */ public class GetConfigContext { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelController.java b/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelController.java index 8e865f96db3..f723ca2fe91 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelController.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelController.java @@ -51,7 +51,7 @@ public class SuperModelController { ConfigKey<?> configKey = request.getConfigKey(); InnerCNode targetDef = getConfigDefinition(request.getConfigKey(), request.getDefContent()); ConfigPayload payload = model.getConfig(configKey); - return responseFactory.createResponse(payload, targetDef, generation); + return responseFactory.createResponse(payload, targetDef, generation, false); } private InnerCNode getConfigDefinition(ConfigKey<?> configKey, DefContent defContent) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java index a73fc95eb05..64123420622 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java @@ -32,23 +32,26 @@ import java.util.Set; * a Vespa application, i.e. generation, model and zookeeper data, as well as methods for resolving config * and other queries against the model. * - * @author Harald Musum + * @author hmusum */ public class Application implements ModelResult { private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(Application.class.getName()); private final long appGeneration; // The generation of the set of configs belonging to an application + private final boolean internalRedeploy; private final Version vespaVersion; private final Model model; private final ServerCache cache; private final MetricUpdater metricUpdater; private final ApplicationId app; - public Application(Model model, ServerCache cache, long appGeneration, Version vespaVersion, MetricUpdater metricUpdater, ApplicationId app) { + public Application(Model model, ServerCache cache, long appGeneration, boolean internalRedeploy, + Version vespaVersion, MetricUpdater metricUpdater, ApplicationId app) { Objects.requireNonNull(model, "The model cannot be null"); this.model = model; this.cache = cache; this.appGeneration = appGeneration; + this.internalRedeploy = internalRedeploy; this.vespaVersion = vespaVersion; this.metricUpdater = metricUpdater; this.app = app; @@ -106,7 +109,7 @@ public class Application implements ModelResult { debug("Resolving config " + cacheKey); } - if (!req.noCache()) { + if ( ! req.noCache()) { ConfigResponse config = cache.get(cacheKey); if (config != null) { if (logDebug()) { @@ -131,9 +134,9 @@ public class Application implements ModelResult { throw new ConfigurationRuntimeException("Unable to resolve config " + configKey); } - ConfigResponse configResponse = responseFactory.createResponse(payload, def.getCNode(), appGeneration); + ConfigResponse configResponse = responseFactory.createResponse(payload, def.getCNode(), appGeneration, internalRedeploy); metricUpdater.incrementProcTime(System.currentTimeMillis() - start); - if (!req.noCache()) { + if ( ! req.noCache()) { cache.put(cacheKey, configResponse, configResponse.getConfigMd5()); metricUpdater.setCacheConfigElems(cache.configElems()); metricUpdater.setCacheChecksumElems(cache.checkSumElems()); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployer.java index 22ce952481d..68c129d1e42 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperDeployer.java @@ -13,7 +13,7 @@ import java.util.Map; * Interface for initializing zookeeper and deploying an application package to zookeeper. * Initialize must be called before each deploy. * - * @author lulf + * @author Ulf Lilleengen */ public class ZooKeeperDeployer { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/HostHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/HostHandler.java index 13933544ad1..d8385456866 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/HostHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/HostHandler.java @@ -7,7 +7,6 @@ import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.container.logging.AccessLog; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.application.BindingMatch; import com.yahoo.log.LogLevel; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandler.java index 74c85829ef2..26ef79ebe02 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandler.java @@ -58,7 +58,7 @@ public class SessionCreateHandler extends SessionHandler { long sessionId; if (request.hasProperty("from")) { ApplicationId applicationId = getFromApplicationId(request); - sessionId = applicationRepository.createSessionFromExisting(applicationId, logger, timeoutBudget); + sessionId = applicationRepository.createSessionFromExisting(applicationId, logger, false, timeoutBudget); } else { validateDataAndHeader(request); String name = getNameProperty(request, logger); 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 9d583d27341..d83548dcc3d 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 @@ -88,7 +88,9 @@ public class ActivatedModelsBuilder extends ModelsBuilder<Application> { new com.yahoo.component.Version(modelFactory.getVersion().toString()), wantedNodeVespaVersion); MetricUpdater applicationMetricUpdater = metrics.getOrCreateMetricUpdater(Metrics.createDimensions(applicationId)); - return new Application(modelFactory.createModel(modelContext), cache, appGeneration, modelFactory.getVersion(), + return new Application(modelFactory.createModel(modelContext), cache, appGeneration, + applicationPackage.getMetaData().isInternalRedeploy(), + modelFactory.getVersion(), applicationMetricUpdater, applicationId); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java index 07e07fa2595..247ae388639 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java @@ -9,17 +9,19 @@ import com.yahoo.vespa.config.protocol.ConfigResponse; * Represents a component that creates config responses from a payload. Different implementations * can do transformations of the payload such as compression. * - * @author lulf - * @since 5.19 + * @author Ulf Lilleengen */ public interface ConfigResponseFactory { /** * Create a {@link ConfigResponse} for a given payload and generation. + * * @param payload The {@link com.yahoo.vespa.config.ConfigPayload} to put in the response. * @param defFile The {@link com.yahoo.config.codegen.InnerCNode} def file for this config. * @param generation The payload generation. @return A {@link ConfigResponse} that can be sent to the client. + * @param internalRedeployment whether this config generation was produced by an internal redeployment, + * not a change to the application package */ - ConfigResponse createResponse(ConfigPayload payload, InnerCNode defFile, long generation); + ConfigResponse createResponse(ConfigPayload payload, InnerCNode defFile, long generation, boolean internalRedeployment); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java index b37d013cc6b..970fe9f169b 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java @@ -22,7 +22,6 @@ import java.util.logging.Logger; /** * @author hmusum -* @since 5.1 */ class GetConfigProcessor implements Runnable { @@ -30,8 +29,9 @@ class GetConfigProcessor implements Runnable { private static final String localHostName = HostName.getLocalhost(); private final JRTServerConfigRequest request; + /* True only when this request has expired its server timeout and we need to respond to the client */ - private boolean forceResponse = false; + private final boolean forceResponse; private final RpcServer rpcServer; private String logPre = ""; @@ -64,7 +64,7 @@ class GetConfigProcessor implements Runnable { // TODO: Increment statistics (Metrics) failed counters when requests fail public void run() { //Request has already been detached - if (!request.validateParameters()) { + if ( ! request.validateParameters()) { // Error code is set in verifyParameters if parameters are not OK. log.log(LogLevel.WARNING, "Parameters for request " + request + " did not validate: " + request.errorCode() + " : " + request.errorMessage()); respond(request); @@ -121,7 +121,7 @@ class GetConfigProcessor implements Runnable { // config == null is not an error, but indicates that the config will be returned later. if ((config != null) && (!config.hasEqualConfig(request) || config.hasNewerGeneration(request) || forceResponse)) { // debugLog(trace, "config response before encoding:" + config.toString()); - request.addOkResponse(request.payloadFromResponse(config), config.getGeneration(), config.getConfigMd5()); + request.addOkResponse(request.payloadFromResponse(config), config.getGeneration(), config.isInternalRedeploy(), config.getConfigMd5()); if (logDebug(trace)) { debugLog(trace, "return response: " + request.getShortDescription()); } @@ -146,8 +146,8 @@ class GetConfigProcessor implements Runnable { private void returnEmpty(JRTServerConfigRequest request) { ConfigPayload emptyPayload = ConfigPayload.empty(); String configMd5 = ConfigUtils.getMd5(emptyPayload); - ConfigResponse config = SlimeConfigResponse.fromConfigPayload(emptyPayload, null, 0, configMd5); - request.addOkResponse(request.payloadFromResponse(config), config.getGeneration(), config.getConfigMd5()); + ConfigResponse config = SlimeConfigResponse.fromConfigPayload(emptyPayload, null, 0, false, configMd5); + request.addOkResponse(request.payloadFromResponse(config), config.getGeneration(), false, config.getConfigMd5()); respond(request); } @@ -161,4 +161,5 @@ class GetConfigProcessor implements Runnable { trace.trace(RpcServer.TRACELEVEL_DEBUG, logPre + message); } } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/LZ4ConfigResponseFactory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/LZ4ConfigResponseFactory.java index 609f1d5f79f..ff15eb7bfa7 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/LZ4ConfigResponseFactory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/LZ4ConfigResponseFactory.java @@ -14,19 +14,22 @@ import com.yahoo.vespa.config.util.ConfigUtils; /** * Compressor that compresses config payloads to lz4. * - * @author lulf - * @since 5.19 + * @author Ulf Lilleengen */ public class LZ4ConfigResponseFactory implements ConfigResponseFactory { private static LZ4PayloadCompressor compressor = new LZ4PayloadCompressor(); @Override - public ConfigResponse createResponse(ConfigPayload payload, InnerCNode defFile, long generation) { + public ConfigResponse createResponse(ConfigPayload payload, + InnerCNode defFile, + long generation, + boolean internalRedeployment) { Utf8Array rawPayload = payload.toUtf8Array(true); String configMd5 = ConfigUtils.getMd5(rawPayload); CompressionInfo info = CompressionInfo.create(CompressionType.LZ4, rawPayload.getByteLength()); Utf8Array compressed = new Utf8Array(compressor.compress(rawPayload.getBytes())); - return new SlimeConfigResponse(compressed, defFile, generation, configMd5, info); + return new SlimeConfigResponse(compressed, defFile, generation, internalRedeployment, configMd5, info); } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/UncompressedConfigResponseFactory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/UncompressedConfigResponseFactory.java index ac3cfa2fda1..995e981e0f3 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/UncompressedConfigResponseFactory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/UncompressedConfigResponseFactory.java @@ -13,17 +13,19 @@ import com.yahoo.vespa.config.util.ConfigUtils; /** * Simply returns an uncompressed payload. * - * @author lulf - * @since 5.19 + * @author Ulf Lilleengen */ public class UncompressedConfigResponseFactory implements ConfigResponseFactory { @Override - public ConfigResponse createResponse(ConfigPayload payload, InnerCNode defFile, long generation) { + public ConfigResponse createResponse(ConfigPayload payload, + InnerCNode defFile, + long generation, + boolean internalRedeployment) { Utf8Array rawPayload = payload.toUtf8Array(true); String configMd5 = ConfigUtils.getMd5(rawPayload); CompressionInfo info = CompressionInfo.create(CompressionType.UNCOMPRESSED, rawPayload.getByteLength()); - return new SlimeConfigResponse(rawPayload, defFile, generation, configMd5, info); + return new SlimeConfigResponse(rawPayload, defFile, generation, internalRedeployment, configMd5, info); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java index 653df81f296..3978a1f25f8 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java @@ -9,8 +9,7 @@ import com.yahoo.vespa.config.server.tenant.TenantRepository; * class represents the common stuff between sessions working on the local file * system ({@link LocalSession}s) and sessions working on zookeeper {@link RemoteSession}s. * - * @author lulf - * @since 5.1 + * @author Ulf Lilleengen */ public abstract class Session { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java index bd9da36a2ba..a3dea83d50c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java @@ -10,7 +10,7 @@ import java.io.File; /** * A session factory responsible for creating deploy sessions. * - * @author lulf + * @author Ulf Lilleengen */ public interface SessionFactory { @@ -31,9 +31,11 @@ public interface SessionFactory { * * @param existingSession The session to use as base * @param logger a deploy logger where the deploy log will be written. + * @param internalRedeploy if this session is for a system internal redeploy not an application package change * @param timeoutBudget Timeout for creating session and waiting for other servers. * @return a new session */ - LocalSession createSessionFromExisting(LocalSession existingSession, DeployLogger logger, TimeoutBudget timeoutBudget); + LocalSession createSessionFromExisting(LocalSession existingSession, DeployLogger logger, + boolean internalRedeploy, TimeoutBudget timeoutBudget); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java index 7285ff905ff..10590a26690 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java @@ -74,9 +74,12 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { this.clock = globalComponentRegistry.getClock(); } + /** Create a session for a true application package change */ @Override - public LocalSession createSession(File applicationFile, ApplicationId applicationId, TimeoutBudget timeoutBudget) { - return create(applicationFile, applicationId, nonExistingActiveSession, timeoutBudget); + public LocalSession createSession(File applicationFile, + ApplicationId applicationId, + TimeoutBudget timeoutBudget) { + return create(applicationFile, applicationId, nonExistingActiveSession, false, timeoutBudget); } private void ensureZKPathDoesNotExist(Path sessionPath) { @@ -89,13 +92,14 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { File configApplicationDir, String applicationName, long sessionId, - long currentlyActiveSession) { + long currentlyActiveSession, + boolean internalRedeploy) { long deployTimestamp = System.currentTimeMillis(); String user = System.getenv("USER"); if (user == null) { user = "unknown"; } - DeployData deployData = new DeployData(user, userDir.getAbsolutePath(), applicationName, deployTimestamp, sessionId, currentlyActiveSession); + DeployData deployData = new DeployData(user, userDir.getAbsolutePath(), applicationName, deployTimestamp, internalRedeploy, sessionId, currentlyActiveSession); return FilesApplicationPackage.fromFileWithDeployData(configApplicationDir, deployData); } @@ -119,19 +123,21 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { @Override public LocalSession createSessionFromExisting(LocalSession existingSession, DeployLogger logger, + boolean internalRedeploy, TimeoutBudget timeoutBudget) { File existingApp = getSessionAppDir(existingSession.getSessionId()); ApplicationId existingApplicationId = existingSession.getApplicationId(); long liveApp = getLiveApp(existingApplicationId); logger.log(LogLevel.DEBUG, "Create from existing application id " + existingApplicationId + ", live app for it is " + liveApp); - LocalSession session = create(existingApp, existingApplicationId, liveApp, timeoutBudget); + LocalSession session = create(existingApp, existingApplicationId, liveApp, internalRedeploy, timeoutBudget); session.setApplicationId(existingApplicationId); session.setVespaVersion(existingSession.getVespaVersion()); return session; } - private LocalSession create(File applicationFile, ApplicationId applicationId, long currentlyActiveSession, TimeoutBudget timeoutBudget) { + private LocalSession create(File applicationFile, ApplicationId applicationId, long currentlyActiveSession, + boolean internalRedeploy, TimeoutBudget timeoutBudget) { long sessionId = sessionCounter.nextSessionId(); Path sessionIdPath = sessionsPath.append(String.valueOf(sessionId)); log.log(LogLevel.DEBUG, TenantRepository.logPre(tenant) + "Next session id is " + sessionId + " , sessionIdPath=" + sessionIdPath.getAbsolute()); @@ -145,8 +151,12 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { nodeFlavors); File userApplicationDir = tenantFileSystemDirs.getUserApplicationDir(sessionId); IOUtils.copyDirectory(applicationFile, userApplicationDir); - ApplicationPackage applicationPackage = createApplication(applicationFile, userApplicationDir, - applicationId.application().value(), sessionId, currentlyActiveSession); + ApplicationPackage applicationPackage = createApplication(applicationFile, + userApplicationDir, + applicationId.application().value(), + sessionId, + currentlyActiveSession, + internalRedeploy); applicationPackage.writeMetaData(); return createSessionFromApplication(applicationPackage, sessionId, sessionZooKeeperClient, timeoutBudget, clock); } catch (Exception e) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantHandlerProvider.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantHandlerProvider.java index 8c774fd35ce..b19f8bf8d0b 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantHandlerProvider.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantHandlerProvider.java @@ -7,8 +7,7 @@ import com.yahoo.vespa.config.server.RequestHandler; /** * Represents something that can provide request and reload handlers of a tenant. * - * @author lulf - * @since 5.3 + * @author Ulf Lilleengen */ public interface TenantHandlerProvider { diff --git a/configserver/src/main/resources/configserver-app/services.xml b/configserver/src/main/resources/configserver-app/services.xml index 9e01f594484..2eeefda63e7 100644 --- a/configserver/src/main/resources/configserver-app/services.xml +++ b/configserver/src/main/resources/configserver-app/services.xml @@ -10,7 +10,7 @@ <initialStatus>initializing</initialStatus> </config> - <accesslog type="vespa" fileNamePattern="logs/vespa/configserver/access.log.%Y%m%d%H%M%S" rotationScheme="date" symlinkName="access.log" /> + <accesslog type="vespa" fileNamePattern="logs/vespa/configserver/access.log.%Y%m%d%H%M%S" rotationScheme="date" compressOnRotation="true" symlinkName="access.log" /> <preprocess:include file='access-logging.xml' required='false' /> <component id="com.yahoo.vespa.config.server.ConfigServerBootstrap" bundle="configserver" /> diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ServerCacheTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ServerCacheTest.java index c1db543d386..b93b3fa28e0 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ServerCacheTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ServerCacheTest.java @@ -50,9 +50,9 @@ public class ServerCacheTest { cache.addDef(fooBimDefKey, new ConfigDefinition("mynode", new String[0])); - cache.put(fooBarCacheKey, SlimeConfigResponse.fromConfigPayload(ConfigPayload.empty(), payload.getCNode(), 2, configMd5), configMd5); - cache.put(bazQuuxCacheKey, SlimeConfigResponse.fromConfigPayload(ConfigPayload.empty(), payload.getCNode(), 2, configMd5), configMd5); - cache.put(fooBarCacheKeyDifferentMd5, SlimeConfigResponse.fromConfigPayload(ConfigPayload.empty(), payload_2.getCNode(), 2, configMd5_2), configMd5_2); + cache.put(fooBarCacheKey, SlimeConfigResponse.fromConfigPayload(ConfigPayload.empty(), payload.getCNode(), 2, false, configMd5), configMd5); + cache.put(bazQuuxCacheKey, SlimeConfigResponse.fromConfigPayload(ConfigPayload.empty(), payload.getCNode(), 2, false, configMd5), configMd5); + cache.put(fooBarCacheKeyDifferentMd5, SlimeConfigResponse.fromConfigPayload(ConfigPayload.empty(), payload_2.getCNode(), 2, false, configMd5_2), configMd5_2); } @Test diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelRequestHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelRequestHandlerTest.java index bc07ac7d79c..2b72db63d55 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelRequestHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/SuperModelRequestHandlerTest.java @@ -122,12 +122,11 @@ public class SuperModelRequestHandlerTest { } private static class TestApplication extends Application { - private long version = 0; public TestApplication(VespaModel vespaModel, ServerCache cache, long appGeneration, ApplicationId app, long version) { - super(vespaModel, cache, appGeneration, Version.fromIntValues(1, 2, 3), MetricUpdater.createTestUpdater(), app); - this.version = version; + super(vespaModel, cache, appGeneration, false, Version.fromIntValues(1, 2, 3), MetricUpdater.createTestUpdater(), app); } + } public static NodeFlavors emptyNodeFlavors() { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java index 1d692e8f78f..3c3edae7914 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java @@ -28,7 +28,7 @@ import static org.junit.Assert.assertThat; import static com.yahoo.vespa.config.server.application.ApplicationConvergenceChecker.ServiceResponse; /** - * @author lulf + * @author Ulf Lilleengen */ public class ApplicationConvergenceCheckerTest { @@ -43,7 +43,12 @@ public class ApplicationConvergenceCheckerTest { @Before public void setup() { Model mockModel = MockModel.createContainer("localhost", 1337); - application = new Application(mockModel, new ServerCache(), 3, Version.fromIntValues(0, 0, 0), MetricUpdater.createTestUpdater(), appId); + application = new Application(mockModel, + new ServerCache(), + 3, + false, + Version.fromIntValues(0, 0, 0), + MetricUpdater.createTestUpdater(), appId); } @Test diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationMapperTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationMapperTest.java index a83f2676a4d..233c0e99ed8 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationMapperTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationMapperTest.java @@ -31,9 +31,9 @@ public class ApplicationMapperTest { vespaVersions.add(Version.fromString("1.2.3")); vespaVersions.add(Version.fromString("1.2.4")); vespaVersions.add(Version.fromString("1.2.5")); - applications.add(new Application(new ModelStub(), null, 0, vespaVersions.get(0), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); - applications.add(new Application(new ModelStub(), null, 0, vespaVersions.get(1), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); - applications.add(new Application(new ModelStub(), null, 0, vespaVersions.get(2), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); + applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(0), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); + applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(1), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); + applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(2), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); } @Test diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationSetTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationSetTest.java index 98bedb76599..94bb81021dc 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationSetTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationSetTest.java @@ -29,9 +29,9 @@ public class ApplicationSetTest { vespaVersions.add(Version.fromString("1.2.3")); vespaVersions.add(Version.fromString("1.2.4")); vespaVersions.add(Version.fromString("1.2.5")); - applications.add(new Application(new ModelStub(), null, 0, vespaVersions.get(0), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); - applications.add(new Application(new ModelStub(), null, 0, vespaVersions.get(1), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); - applications.add(new Application(new ModelStub(), null, 0, vespaVersions.get(2), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); + applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(0), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); + applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(1), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); + applications.add(new Application(new ModelStub(), null, 0, false, vespaVersions.get(2), MetricUpdater.createTestUpdater(), ApplicationId.defaultId())); } @Test diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationTest.java index 02cf6303ba8..90a27c39736 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationTest.java @@ -42,8 +42,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; /** - * @author lulf - * @since 5.1.14 + * @author Ulf Lilleengen */ public class ApplicationTest { @@ -53,7 +52,7 @@ public class ApplicationTest { ApplicationName.from("foobar"), InstanceName.defaultName()); ServerCache cache = new ServerCache(); Version vespaVersion = Version.fromIntValues(1, 2, 3); - Application app = new Application(new ModelStub(), cache, 1337, vespaVersion, MetricUpdater.createTestUpdater(), appId); + Application app = new Application(new ModelStub(), cache, 1337L, false, vespaVersion, MetricUpdater.createTestUpdater(), appId); assertThat(app.getApplicationGeneration(), is(1337l)); assertNotNull(app.getModel()); assertThat(app.getCache(), is(cache)); @@ -71,24 +70,24 @@ public class ApplicationTest { File testApp = new File("src/test/apps/app"); ServerCache cache = createCacheAndAddContent(); VespaModel model = new VespaModel(FilesApplicationPackage.fromFile(testApp)); - final ApplicationId applicationId = new ApplicationId.Builder().tenant("foo").applicationName("foo").build(); - handler = new Application(model, cache, 1, Version.fromIntValues(1, 2, 3), + ApplicationId applicationId = new ApplicationId.Builder().tenant("foo").applicationName("foo").build(); + handler = new Application(model, cache, 1L, false, Version.fromIntValues(1, 2, 3), new MetricUpdater(Metrics.createTestMetrics(), Metrics.createDimensions(applicationId)), applicationId); } private static ServerCache createCacheAndAddContent() { ServerCache cache = new ServerCache(); - final ConfigDefinitionKey key = new ConfigDefinitionKey(SimpletypesConfig.CONFIG_DEF_NAME, SimpletypesConfig.CONFIG_DEF_NAMESPACE); + ConfigDefinitionKey key = new ConfigDefinitionKey(SimpletypesConfig.CONFIG_DEF_NAME, SimpletypesConfig.CONFIG_DEF_NAMESPACE); com.yahoo.vespa.config.buildergen.ConfigDefinition def = getDef(key, SimpletypesConfig.CONFIG_DEF_SCHEMA); // TODO Why do we have to use empty def md5 here? cache.addDef(key, def); - final ConfigDefinitionKey key2 = new ConfigDefinitionKey(SlobroksConfig.CONFIG_DEF_NAME, SlobroksConfig.CONFIG_DEF_NAMESPACE); + ConfigDefinitionKey key2 = new ConfigDefinitionKey(SlobroksConfig.CONFIG_DEF_NAME, SlobroksConfig.CONFIG_DEF_NAMESPACE); com.yahoo.vespa.config.buildergen.ConfigDefinition def2 = getDef(key2, SlobroksConfig.CONFIG_DEF_SCHEMA); cache.addDef(key2, def2); - final ConfigDefinitionKey key3 = new ConfigDefinitionKey(LogdConfig.CONFIG_DEF_NAME, LogdConfig.CONFIG_DEF_NAMESPACE); + ConfigDefinitionKey key3 = new ConfigDefinitionKey(LogdConfig.CONFIG_DEF_NAME, LogdConfig.CONFIG_DEF_NAMESPACE); com.yahoo.vespa.config.buildergen.ConfigDefinition def3 = getDef(key3, LogdConfig.CONFIG_DEF_SCHEMA); cache.addDef(key3, def3); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/FileDistributionStatusTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/FileDistributionStatusTest.java index 76204d5c5f2..c2ddec7e795 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/FileDistributionStatusTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/FileDistributionStatusTest.java @@ -161,7 +161,13 @@ public class FileDistributionStatusTest { private Application createApplication(List<String> hostnames) { Model mockModel = MockModel.createConfigProxies(hostnames, 1337); - return new Application(mockModel, new ServerCache(), 3, Version.fromIntValues(0, 0, 0), MetricUpdater.createTestUpdater(), appId); + return new Application(mockModel, + new ServerCache(), + 3, + false, + Version.fromIntValues(0, 0, 0), + MetricUpdater.createTestUpdater(), + appId); } HttpResponse getStatus(FileDistributionStatus fileDistributionStatus, Application application) { 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 9b40784018a..b151ac57352 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 @@ -15,6 +15,7 @@ import com.yahoo.config.provision.Zone; import com.yahoo.test.ManualClock; import static com.yahoo.vespa.config.server.deploy.DeployTester.CountingModelFactory; +import com.yahoo.vespa.config.server.session.LocalSession; import org.junit.Ignore; import org.junit.Test; @@ -50,12 +51,18 @@ public class HostedDeployTest { @Test public void testRedeploy() { DeployTester tester = new DeployTester("src/test/apps/hosted/", createConfigserverConfig()); - tester.deployApp("myApp", Instant.now()); + ApplicationId appId = tester.deployApp("myApp", Instant.now()); + LocalSession s1 = tester.applicationRepository().getActiveSession(appId); + System.out.println("First session: " + s1.getSessionId()); + assertFalse(tester.applicationRepository().getActiveSession(appId).getMetaData().isInternalRedeploy()); Optional<com.yahoo.config.provision.Deployment> deployment = tester.redeployFromLocalActive(); assertTrue(deployment.isPresent()); deployment.get().prepare(); deployment.get().activate(); + LocalSession s2 = tester.applicationRepository().getActiveSession(appId); + System.out.println("Second session: " + s2.getSessionId()); + assertTrue(tester.applicationRepository().getActiveSession(appId).getMetaData().isInternalRedeploy()); } @Test diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java index bf7f7038c1a..945c7d60750 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java @@ -43,7 +43,14 @@ public class ZooKeeperClientTest extends TestWithCurator { public void setupZK() throws IOException { this.zk = ConfigCurator.create(curator); ZooKeeperClient zkc = new ZooKeeperClient(zk, new BaseDeployLogger(), true, Path.fromString(appPath)); - ApplicationPackage app = FilesApplicationPackage.fromFileWithDeployData(new File("src/test/apps/zkfeed"), new DeployData("foo", "/bar/baz", "appName", 1345l, 3l, 2l)); + ApplicationPackage app = FilesApplicationPackage.fromFileWithDeployData(new File("src/test/apps/zkfeed"), + new DeployData("foo", + "/bar/baz", + "appName", + 1345L, + true, + 3L, + 2L)); Map<Version, FileRegistry> fileRegistries = createFileRegistries(); app.writeMetaData(); zkc.setupZooKeeper(); @@ -97,7 +104,7 @@ public class ZooKeeperClientTest extends TestWithCurator { // TODO: Evaluate if we want this or not @Test @Ignore - public void testFeedComponentsFileReferencesToZooKeeper() throws IOException { + public void testFeedComponentsFileReferencesToZooKeeper() { final String appDir = "src/test/apps/app_sdbundles"; ConfigCurator zk = ConfigCurator.create(new MockCurator()); BaseDeployLogger logger = new BaseDeployLogger(); @@ -120,6 +127,7 @@ public class ZooKeeperClientTest extends TestWithCurator { ApplicationMetaData metaData = ApplicationMetaData.fromJsonString(zk.getData(appPath, ConfigCurator.META_ZK_PATH)); assertThat(metaData.getApplicationName(), is("appName")); assertTrue(metaData.getCheckSum().length() > 0); + assertTrue(metaData.isInternalRedeploy()); assertThat(metaData.getDeployedByUser(), is("foo")); assertThat(metaData.getDeployPath(), is("/bar/baz")); assertThat(metaData.getDeployTimestamp(), is(1345l)); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpConfigResponseTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpConfigResponseTest.java index 9b82beb860e..5b166155a45 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpConfigResponseTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpConfigResponseTest.java @@ -29,7 +29,7 @@ public class HttpConfigResponseTest { // TODO: Hope to be able to remove this mess soon. DefParser dParser = new DefParser(SimpletypesConfig.getDefName(), new StringReader(StringUtilities.implode(SimpletypesConfig.CONFIG_DEF_SCHEMA, "\n"))); InnerCNode targetDef = dParser.getTree(); - ConfigResponse configResponse = SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, "mymd5"); + ConfigResponse configResponse = SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, false, "mymd5"); HttpConfigResponse response = HttpConfigResponse.createFromConfig(configResponse); assertThat(SessionHandlerTest.getRenderedString(response), is("{\"boolval\":false,\"doubleval\":0.0,\"enumval\":\"VAL1\",\"intval\":0,\"longval\":0,\"stringval\":\"s\"}")); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java index b19d6e2e257..f3800d66330 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java @@ -55,7 +55,7 @@ public class HttpGetConfigHandlerTest { final long generation = 1L; ConfigPayload payload = ConfigPayload.fromInstance(new SimpletypesConfig(new SimpletypesConfig.Builder())); InnerCNode targetDef = getInnerCNode(); - mockRequestHandler.responses.put(ApplicationId.defaultId(), SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, "mymd5")); + mockRequestHandler.responses.put(ApplicationId.defaultId(), SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, false, "mymd5")); HttpResponse response = handler.handle(HttpRequest.createTestRequest(configUri, GET)); assertThat(SessionHandlerTest.getRenderedString(response), is("{\"boolval\":false,\"doubleval\":0.0,\"enumval\":\"VAL1\",\"intval\":0,\"longval\":0,\"stringval\":\"s\"}")); } @@ -82,7 +82,7 @@ public class HttpGetConfigHandlerTest { long generation = 1L; ConfigPayload payload = ConfigPayload.fromInstance(new SimpletypesConfig(new SimpletypesConfig.Builder())); InnerCNode targetDef = getInnerCNode(); - mockRequestHandler.responses.put(ApplicationId.defaultId(), SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, "mymd5")); + mockRequestHandler.responses.put(ApplicationId.defaultId(), SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, false, "mymd5")); final HttpRequest request = HttpRequest.createTestRequest(configUri, GET, null, Collections.singletonMap("nocache", "true")); HttpResponse response = handler.handle(request); assertThat(SessionHandlerTest.getRenderedString(response), is("{\"boolval\":false,\"doubleval\":0.0,\"enumval\":\"VAL1\",\"intval\":0,\"longval\":0,\"stringval\":\"s\"}")); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java index 930ae98c9e9..eb5dc7a2abf 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java @@ -191,7 +191,8 @@ public class SessionHandlerTest { public String applicationName; @Override - public LocalSession createSession(File applicationDirectory, ApplicationId applicationId, TimeoutBudget timeoutBudget) { + public LocalSession createSession(File applicationDirectory, ApplicationId applicationId, + TimeoutBudget timeoutBudget) { createCalled = true; this.applicationName = applicationId.application().value(); if (doThrow) { @@ -208,7 +209,8 @@ public class SessionHandlerTest { } @Override - public LocalSession createSessionFromExisting(LocalSession existingSession, DeployLogger logger, TimeoutBudget timeoutBudget) { + public LocalSession createSessionFromExisting(LocalSession existingSession, DeployLogger logger, + boolean internalRedeploy, TimeoutBudget timeoutBudget) { if (doThrow) { throw new RuntimeException("foo"); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java index 5226ff38ce3..bc583c64206 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HttpGetConfigHandlerTest.java @@ -60,7 +60,7 @@ public class HttpGetConfigHandlerTest { ConfigPayload payload = ConfigPayload.fromInstance(new SimpletypesConfig(new SimpletypesConfig.Builder())); InnerCNode targetDef = getInnerCNode(); mockRequestHandler.responses.put(new ApplicationId.Builder().tenant(tenant).applicationName("myapplication").build(), - SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, "mymd5")); + SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, false, "mymd5")); HttpResponse response = handler.handle(HttpRequest.createTestRequest(configUri, GET)); assertThat(SessionHandlerTest.getRenderedString(response), is(EXPECTED_RENDERED_STRING)); } @@ -75,7 +75,7 @@ public class HttpGetConfigHandlerTest { mockRequestHandler.responses.put(new ApplicationId.Builder() .tenant(tenant) .applicationName("myapplication").instanceName("myinstance").build(), - SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, "mymd5")); + SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, false, "mymd5")); HttpResponse response = handler.handle(HttpRequest.createTestRequest(uriLongAppId, GET)); assertThat(SessionHandlerTest.getRenderedString(response), is(EXPECTED_RENDERED_STRING)); } @@ -123,7 +123,7 @@ public class HttpGetConfigHandlerTest { ConfigPayload payload = ConfigPayload.fromInstance(new SimpletypesConfig(new SimpletypesConfig.Builder())); InnerCNode targetDef = getInnerCNode(); mockRequestHandler.responses.put(new ApplicationId.Builder().tenant(tenant).applicationName("myapplication").build(), - SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, "mymd5")); + SlimeConfigResponse.fromConfigPayload(payload, targetDef, generation, false, "mymd5")); final HttpRequest request = HttpRequest.createTestRequest(configUri, GET, null, Collections.singletonMap("nocache", "true")); HttpResponse response = handler.handle(request); assertThat(SessionHandlerTest.getRenderedString(response), is(EXPECTED_RENDERED_STRING)); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java index 030963aa9a1..fba4e40000d 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java @@ -285,7 +285,13 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { ActivateRequest(long sessionId, long previousSessionId, Session.Status initialStatus, String subPath, Clock clock) { this.sessionId = sessionId; this.initialStatus = initialStatus; - this.deployData = new DeployData("foo", "bar", appName, 0l, sessionId, previousSessionId); + this.deployData = new DeployData("foo", + "bar", + appName, + 0l, + false, + sessionId, + previousSessionId); this.subPath = subPath; this.clock = clock; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java index 9193c2409c7..7c1d5fa8dbc 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java @@ -13,38 +13,38 @@ import org.junit.Test; import java.io.StringReader; -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertEquals; /** - * @author lulf - * @since 5.19 + * @author Ulf Lilleengen */ public class ConfigResponseFactoryTest { - private InnerCNode def; + private InnerCNode def; @Before public void setup() { - DefParser dParser = new DefParser(SimpletypesConfig.getDefName(), new StringReader(StringUtilities.implode(SimpletypesConfig.CONFIG_DEF_SCHEMA, "\n"))); + DefParser dParser = new DefParser(SimpletypesConfig.getDefName(), + new StringReader(StringUtilities.implode(SimpletypesConfig.CONFIG_DEF_SCHEMA, "\n"))); def = dParser.getTree(); } @Test public void testUncompressedFacory() { UncompressedConfigResponseFactory responseFactory = new UncompressedConfigResponseFactory(); - ConfigResponse response = responseFactory.createResponse(ConfigPayload.empty(), def, 3); - assertThat(response.getCompressionInfo().getCompressionType(), is(CompressionType.UNCOMPRESSED)); - assertThat(response.getGeneration(), is(3l)); - assertThat(response.getPayload().getByteLength(), is(2)); + ConfigResponse response = responseFactory.createResponse(ConfigPayload.empty(), def, 3, false); + assertEquals(CompressionType.UNCOMPRESSED, response.getCompressionInfo().getCompressionType()); + assertEquals(3L,response.getGeneration()); + assertEquals(2, response.getPayload().getByteLength()); } @Test public void testLZ4CompressedFacory() { LZ4ConfigResponseFactory responseFactory = new LZ4ConfigResponseFactory(); - ConfigResponse response = responseFactory.createResponse(ConfigPayload.empty(), def, 3); - assertThat(response.getCompressionInfo().getCompressionType(), is(CompressionType.LZ4)); - assertThat(response.getGeneration(), is(3l)); - assertThat(response.getPayload().getByteLength(), is(3)); + ConfigResponse response = responseFactory.createResponse(ConfigPayload.empty(), def, 3, false); + assertEquals(CompressionType.LZ4, response.getCompressionInfo().getCompressionType()); + assertEquals(3L, response.getGeneration()); + assertEquals(3, response.getPayload().getByteLength()); } + } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessorTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessorTest.java index aa4678fb01b..1a4d04d0323 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessorTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessorTest.java @@ -108,6 +108,9 @@ public class GetConfigProcessorTest { } @Override + public boolean isInternalRedeploy() { return false; } + + @Override public String getConfigMd5() { return "mymd5"; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRequestHandler.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRequestHandler.java index 62ff13093ea..ebdee8f58e5 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRequestHandler.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRequestHandler.java @@ -15,15 +15,15 @@ import java.util.*; /** * Test utility class - * @author lulf - * @since 5.25 + * + * @author Ulf Lilleengen */ public class MockRequestHandler implements RequestHandler, ReloadHandler, TenantHandlerProvider { volatile boolean throwException = false; private Set<ConfigKey<?>> allConfigs = new HashSet<>(); public volatile ConfigResponse responseConfig = null; // for some v1 mocking - public Map<ApplicationId, ConfigResponse> responses = new LinkedHashMap<>(); // for v2 mocking + public Map<ApplicationId, ConfigResponse> responses = new LinkedHashMap<>(); // for v3 mocking private final boolean pretendToHaveLoadedAnyApplication; public MockRequestHandler() { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java index 3cfe6aa7c6d..9807045e122 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java @@ -32,8 +32,7 @@ import static org.hamcrest.core.Is.is; import static org.junit.Assert.*; /** - * @author lulf - * @since 5.1 + * @author Ulf Lilleengen */ public class RpcServerTest extends TestWithRpc { @@ -70,10 +69,15 @@ public class RpcServerTest extends TestWithRpc { return clientReq; } - private void testEnabled() throws IOException, SAXException { generationCounter.increment(); - Application app = new Application(new VespaModel(MockApplicationPackage.createEmpty()), new ServerCache(), 2l, Version.fromIntValues(1, 2, 3), MetricUpdater.createTestUpdater(), ApplicationId.defaultId()); + Application app = new Application(new VespaModel(MockApplicationPackage.createEmpty()), + new ServerCache(), + 2L, + false, + Version.fromIntValues(1, 2, 3), + MetricUpdater.createTestUpdater(), + ApplicationId.defaultId()); ApplicationSet appSet = ApplicationSet.fromSingle(app); rpcServer.configActivated(TenantName.defaultName(), appSet); ConfigKey<?> key = new ConfigKey<>(LbServicesConfig.class, "*"); @@ -95,12 +99,17 @@ public class RpcServerTest extends TestWithRpc { public void testGetConfig() { ((MockRequestHandler)tenantProvider.getRequestHandler()).throwException = false; ConfigKey<?> key = new ConfigKey<>(SimpletypesConfig.class, "brim"); - ((MockRequestHandler)tenantProvider.getRequestHandler()).responses.put(ApplicationId.defaultId(), createResponse()); - JRTClientConfigRequest req = JRTClientConfigRequestV3.createFromRaw(new RawConfig(key, SimpletypesConfig.CONFIG_DEF_MD5), 120_000, Trace.createDummy(), CompressionType.UNCOMPRESSED, Optional.empty()); + ((MockRequestHandler)tenantProvider.getRequestHandler()).responses.put(ApplicationId.defaultId(), createResponse(true)); + JRTClientConfigRequest req = JRTClientConfigRequestV3.createFromRaw(new RawConfig(key, SimpletypesConfig.CONFIG_DEF_MD5), + 120_000, + Trace.createDummy(), + CompressionType.UNCOMPRESSED, + Optional.empty()); assertTrue(req.validateParameters()); performRequest(req.getRequest()); assertThat(req.errorCode(), is(0)); assertTrue(req.validateResponse()); + assertTrue(req.responseIsInternalRedeploy()); ConfigPayload payload = ConfigPayload.fromUtf8Array(req.getNewPayload().getData()); assertNotNull(payload); SimpletypesConfig.Builder builder = new SimpletypesConfig.Builder(); @@ -109,13 +118,19 @@ public class RpcServerTest extends TestWithRpc { assertThat(config.intval(), is(123)); } - public ConfigResponse createResponse() { + public ConfigResponse createResponse(boolean internalRedeploy) { SimpletypesConfig.Builder builder = new SimpletypesConfig.Builder(); builder.intval(123); SimpletypesConfig responseConfig = new SimpletypesConfig(builder); ConfigPayload responsePayload = ConfigPayload.fromInstance(responseConfig); - InnerCNode targetDef = new DefParser(SimpletypesConfig.CONFIG_DEF_NAME, new StringReader(Joiner.on("\n").join(SimpletypesConfig.CONFIG_DEF_SCHEMA))).getTree(); - return SlimeConfigResponse.fromConfigPayload(responsePayload, targetDef, 3l, ConfigUtils.getMd5(responsePayload)); + InnerCNode targetDef = new DefParser(SimpletypesConfig.CONFIG_DEF_NAME, + new StringReader(Joiner.on("\n").join(SimpletypesConfig.CONFIG_DEF_SCHEMA))) + .getTree(); + return SlimeConfigResponse.fromConfigPayload(responsePayload, + targetDef, + 3L, + internalRedeploy, + ConfigUtils.getMd5(responsePayload)); } public void testPrintStatistics() { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionFactoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionFactoryTest.java index 531a2e3745b..0ca487cfb67 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionFactoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionFactoryTest.java @@ -31,8 +31,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; /** - * @author lulf - * @since 5.1 + * @author Ulf Lilleengen */ public class SessionFactoryTest extends TestWithTenant { private SessionFactory factory; @@ -64,10 +63,13 @@ public class SessionFactoryTest extends TestWithTenant { public void require_that_session_can_be_created_from_existing() throws IOException { LocalSession session = getLocalSession(); assertNotNull(session); - assertThat(session.getSessionId(), is(2l)); - LocalSession session2 = factory.createSessionFromExisting(session, new BaseDeployLogger(), TimeoutBudgetTest.day()); + assertThat(session.getSessionId(), is(2L)); + LocalSession session2 = factory.createSessionFromExisting(session, + new BaseDeployLogger(), + false, + TimeoutBudgetTest.day()); assertNotNull(session2); - assertThat(session2.getSessionId(), is(3l)); + assertThat(session2.getSessionId(), is(3L)); } @Test(expected = RuntimeException.class) diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/MockTenantProvider.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/MockTenantProvider.java index 77505006b77..4d01f8a609d 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/MockTenantProvider.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/MockTenantProvider.java @@ -7,8 +7,7 @@ import com.yahoo.vespa.config.server.ReloadHandler; import com.yahoo.vespa.config.server.RequestHandler; /** - * @author lulf - * @since 5. + * @author Ulf Lilleengen */ public class MockTenantProvider implements TenantHandlerProvider { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java index 9a4b0b05186..f47ed69ad14 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java @@ -65,8 +65,13 @@ public class TenantRepositoryTest extends TestWithCurator { @Test public void testListenersAdded() throws IOException, SAXException { tenantRepository.getTenant(tenant1).getReloadHandler().reloadConfig(ApplicationSet.fromSingle( - new Application(new VespaModel(MockApplicationPackage.createEmpty()), new ServerCache(), 4l, - Version.fromIntValues(1, 2, 3), MetricUpdater.createTestUpdater(), ApplicationId.defaultId()))); + new Application(new VespaModel(MockApplicationPackage.createEmpty()), + new ServerCache(), + 4L, + false, + Version.fromIntValues(1, 2, 3), + MetricUpdater.createTestUpdater(), + ApplicationId.defaultId()))); assertThat(listener.reloaded.get(), is(1)); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java index ef320f0f084..cecbab2d9ec 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java @@ -3,8 +3,10 @@ package com.yahoo.vespa.config.server.tenant; import com.yahoo.config.ConfigInstance; import com.yahoo.config.SimpletypesConfig; +import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.NullConfigModelRegistry; import com.yahoo.config.model.application.provider.BaseDeployLogger; +import com.yahoo.config.model.application.provider.DeployData; import com.yahoo.config.model.application.provider.FilesApplicationPackage; import com.yahoo.config.model.application.provider.MockFileRegistry; import com.yahoo.config.provision.ApplicationName; @@ -54,8 +56,7 @@ import static org.hamcrest.core.Is.is; import static org.junit.Assert.*; /** - * @author lulf - * @since 5.1 + * @author Ulf Lilleengen */ public class TenantRequestHandlerTest extends TestWithCurator { @@ -70,9 +71,13 @@ public class TenantRequestHandlerTest extends TestWithCurator { @Rule public TemporaryFolder tempFolder = new TemporaryFolder(); + private ApplicationId defaultApp() { + return new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(tenant).build(); + } + @Before - public void setUp() throws IOException, SAXException { - feedApp(app1, 1); + public void setUp() throws IOException { + feedApp(app1, 1, defaultApp(), false); Metrics sh = Metrics.createTestMetrics(); List<ReloadListener> listeners = new ArrayList<>(); listeners.add(listener); @@ -80,11 +85,7 @@ public class TenantRequestHandlerTest extends TestWithCurator { componentRegistry = new TestComponentRegistry.Builder().curator(curator).modelFactoryRegistry(createRegistry()).build(); } - private void feedApp(File appDir, long sessionId) throws IOException { - feedApp(appDir, sessionId, new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(tenant).build()); - } - - private void feedApp(File appDir, long sessionId, ApplicationId appId) throws IOException { + private void feedApp(File appDir, long sessionId, ApplicationId appId, boolean internalRedeploy) throws IOException { SessionZooKeeperClient zkc = new SessionZooKeeperClient(curator, configCurator, TenantRepository.getSessionsPath(tenant).append(String.valueOf(sessionId)), new TestConfigDefinitionRepo(), @@ -93,7 +94,17 @@ public class TenantRequestHandlerTest extends TestWithCurator { File app = tempFolder.newFolder(); IOUtils.copyDirectory(appDir, app); ZooKeeperDeployer deployer = zkc.createDeployer(new BaseDeployLogger()); - deployer.deploy(FilesApplicationPackage.fromFile(appDir), Collections.singletonMap(vespaVersion, new MockFileRegistry()), AllocatedHosts.withHosts(Collections.emptySet())); + DeployData deployData = new DeployData("user", + appDir.toString(), + appId.application().toString(), + 0L, + internalRedeploy, + 0L, + 0L); + ApplicationPackage appPackage = FilesApplicationPackage.fromFileWithDeployData(appDir, deployData); + deployer.deploy(appPackage, + Collections.singletonMap(vespaVersion, new MockFileRegistry()), + AllocatedHosts.withHosts(Collections.emptySet())); } private ApplicationSet reloadConfig(long id, Clock clock) { @@ -115,12 +126,21 @@ public class TenantRequestHandlerTest extends TestWithCurator { new TestModelFactory(Version.fromIntValues(3, 2, 1)))); } - public <T extends ConfigInstance> T resolve(Class<T> clazz, TenantRequestHandler tenantRequestHandler, String configId) { - return resolve(clazz, tenantRequestHandler, new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(tenant).build(), vespaVersion, configId); + public <T extends ConfigInstance> T resolve(Class<T> clazz, + TenantRequestHandler tenantRequestHandler, + ApplicationId appId, + Version vespaVersion, + String configId) { + ConfigResponse response = getConfigResponse(clazz, tenantRequestHandler, appId, vespaVersion, configId); + return ConfigPayload.fromUtf8Array(response.getPayload()).toInstance(clazz, configId); } - public <T extends ConfigInstance> T resolve(final Class<T> clazz, TenantRequestHandler tenantRequestHandler, ApplicationId appId, Version vespaVersion, final String configId) { - ConfigResponse response = tenantRequestHandler.resolveConfig(appId, new GetConfigRequest() { + public <T extends ConfigInstance> ConfigResponse getConfigResponse(Class<T> clazz, + TenantRequestHandler tenantRequestHandler, + ApplicationId appId, + Version vespaVersion, + String configId) { + return tenantRequestHandler.resolveConfig(appId, new GetConfigRequest() { @Override public ConfigKey<T> getConfigKey() { return new ConfigKey<T>(clazz, configId); @@ -141,23 +161,24 @@ public class TenantRequestHandlerTest extends TestWithCurator { return false; } }, Optional.empty()); - return ConfigPayload.fromUtf8Array(response.getPayload()).toInstance(clazz, configId); } @Test - public void testReloadConfig() throws IOException, SAXException { + public void testReloadConfig() throws IOException { Clock clock = Clock.systemUTC(); ApplicationId applicationId = new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(tenant).build(); server.reloadConfig(reloadConfig(1, clock)); assertThat(listener.reloaded.get(), is(1)); // Using only payload list for this simple test - SimpletypesConfig config = resolve(SimpletypesConfig.class, server, ""); + SimpletypesConfig config = resolve(SimpletypesConfig.class, server, defaultApp(), vespaVersion, ""); assertThat(config.intval(), is(1337)); assertThat(server.getApplicationGeneration(applicationId, Optional.of(vespaVersion)), is(1l)); - server.reloadConfig(reloadConfig(1l, clock)); - config = resolve(SimpletypesConfig.class, server, ""); + server.reloadConfig(reloadConfig(1L, clock)); + ConfigResponse configResponse = getConfigResponse(SimpletypesConfig.class, server, defaultApp(), vespaVersion, ""); + assertFalse(configResponse.isInternalRedeploy()); + config = resolve(SimpletypesConfig.class, server, defaultApp(), vespaVersion, ""); assertThat(config.intval(), is(1337)); assertThat(listener.reloaded.get(), is(2)); assertThat(server.getApplicationGeneration(applicationId, Optional.of(vespaVersion)), is(1l)); @@ -165,9 +186,11 @@ public class TenantRequestHandlerTest extends TestWithCurator { assertThat(server.resolveApplicationId("mytesthost"), is(applicationId)); listener.reloaded.set(0); - feedApp(app2, 2); - server.reloadConfig(reloadConfig(2l, clock)); - config = resolve(SimpletypesConfig.class, server, ""); + feedApp(app2, 2, defaultApp(), true); + server.reloadConfig(reloadConfig(2L, clock)); + configResponse = getConfigResponse(SimpletypesConfig.class, server, defaultApp(), vespaVersion, ""); + assertTrue(configResponse.isInternalRedeploy()); + config = resolve(SimpletypesConfig.class, server, defaultApp(), vespaVersion,""); assertThat(config.intval(), is(1330)); assertThat(listener.reloaded.get(), is(1)); assertThat(server.getApplicationGeneration(applicationId, Optional.of(vespaVersion)), is(2l)); @@ -183,7 +206,7 @@ public class TenantRequestHandlerTest extends TestWithCurator { @Test public void testResolveForAppId() { - long id = 1l; + long id = 1L; SessionZooKeeperClient zkc = new SessionZooKeeperClient(curator, configCurator, TenantRepository.getSessionsPath(tenant).append(String.valueOf(id)), new TestConfigDefinitionRepo(), @@ -199,7 +222,7 @@ public class TenantRequestHandlerTest extends TestWithCurator { } @Test - public void testResolveMultipleApps() throws IOException, SAXException { + public void testResolveMultipleApps() throws IOException { ApplicationId appId1 = new ApplicationId.Builder() .tenant(tenant) .applicationName("myapp1").instanceName("myinst1").build(); @@ -228,7 +251,7 @@ public class TenantRequestHandlerTest extends TestWithCurator { } private void feedAndReloadApp(File appDir, long sessionId, ApplicationId appId) throws IOException { - feedApp(appDir, sessionId, appId); + feedApp(appDir, sessionId, appId, false); SessionZooKeeperClient zkc = new SessionZooKeeperClient(curator, TenantRepository.getSessionsPath(tenant).append(String.valueOf(sessionId))); zkc.writeApplicationId(appId); RemoteSession session = new RemoteSession(tenant, sessionId, componentRegistry, zkc, Clock.systemUTC()); @@ -263,7 +286,8 @@ public class TenantRequestHandlerTest extends TestWithCurator { public void testHasApplication() throws IOException, SAXException { assertdefaultAppNotFound(); server.reloadConfig(reloadConfig(1l, Clock.systemUTC())); - assertTrue(server.hasApplication(new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(tenant).build(), Optional.of(vespaVersion))); + assertTrue(server.hasApplication(new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(tenant).build(), + Optional.of(vespaVersion))); } private void assertdefaultAppNotFound() { @@ -286,18 +310,17 @@ public class TenantRequestHandlerTest extends TestWithCurator { @Test public void testListConfigs() throws IOException, SAXException { assertdefaultAppNotFound(); - /*assertTrue(server.allConfigIds(ApplicationId.defaultId()).isEmpty()); - assertTrue(server.allConfigsProduced(ApplicationId.defaultId()).isEmpty()); - assertTrue(server.listConfigs(ApplicationId.defaultId(), false).isEmpty()); - assertTrue(server.listConfigs(ApplicationId.defaultId(), true).isEmpty());*/ VespaModel model = new VespaModel(FilesApplicationPackage.fromFile(new File("src/test/apps/app"))); - server.reloadConfig(ApplicationSet.fromSingle(new Application(model, new ServerCache(), 1, vespaVersion, MetricUpdater.createTestUpdater(), ApplicationId.defaultId()))); + server.reloadConfig(ApplicationSet.fromSingle(new Application(model, + new ServerCache(), + 1, + false, + vespaVersion, + MetricUpdater.createTestUpdater(), + ApplicationId.defaultId()))); Set<ConfigKey<?>> configNames = server.listConfigs(ApplicationId.defaultId(), Optional.of(vespaVersion), false); assertTrue(configNames.contains(new ConfigKey<>("sentinel", "hosts", "cloud.config"))); - //for (ConfigKey<?> ck : configNames) { - // assertTrue(!"".equals(ck.getConfigId())); - //} configNames = server.listConfigs(ApplicationId.defaultId(), Optional.of(vespaVersion), true); System.out.println(configNames); diff --git a/container-core/src/main/java/com/yahoo/container/Server.java b/container-core/src/main/java/com/yahoo/container/Server.java index 207050a8d88..2eec2e36c47 100644 --- a/container-core/src/main/java/com/yahoo/container/Server.java +++ b/container-core/src/main/java/com/yahoo/container/Server.java @@ -19,7 +19,7 @@ public class Server { private ConfigSubscriber subscriber = new ConfigSubscriber(); /** The OSGi container instance of this server */ - private Container container=Container.get(); + private Container container = Container.get(); /** A short string which is different for all the qrserver instances on a given node. */ private String localServerDiscriminator = "qrserver.0"; @@ -48,7 +48,6 @@ public class Server { return instance; } - private void initRpcServer(Rpc rpcConfig) { if (rpcConfig.enabled()) { ContainerRpcAdaptor rpcAdaptor = container.getRpcAdaptor(); @@ -57,8 +56,7 @@ public class Server { } } - /** Ugly hack, see Container.resetInstance - **/ + /** Ugly hack, see Container.resetInstance */ static void resetInstance() { instance = new Server(); } diff --git a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java index 49cacb3a09b..cb4a21137a2 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java @@ -86,7 +86,7 @@ public class HandlersConfigurerDi { container = new Container(subscriberFactory, configId, deconstructor, osgiWrapper); try { - runOnceAndEnsureRegistryHackRun(discInjector); + getNewComponentGraph(discInjector, false); } catch (InterruptedException e) { throw new RuntimeException("Interrupted while setting up handlers for the first time."); } @@ -138,11 +138,13 @@ public class HandlersConfigurerDi { } } - public void runOnceAndEnsureRegistryHackRun(Injector discInjector) throws InterruptedException { - currentGraph = container.runOnce(currentGraph, createFallbackInjector(vespaContainer, discInjector)); + /** + * Wait for new config to arrive and produce the new graph + */ + public void getNewComponentGraph(Injector discInjector, boolean restartOnRedeploy) throws InterruptedException { + currentGraph = container.getNewComponentGraph(currentGraph, createFallbackInjector(vespaContainer, discInjector), restartOnRedeploy); - RegistriesHack registriesHack = currentGraph.getInstance(RegistriesHack.class); - assert (registriesHack != null); + assert (currentGraph.getInstance(RegistriesHack.class) != null); // TODO: Remove, seems quite pointless? } @SuppressWarnings("deprecation") diff --git a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java index 89c73e19fe3..afbf163500f 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java @@ -112,7 +112,7 @@ public class HandlersConfigurerTestWrapper { public void reloadConfig() { configurer.reloadConfig(++lastGeneration); try { - configurer.runOnceAndEnsureRegistryHackRun(Guice.createInjector()); + configurer.getNewComponentGraph(Guice.createInjector(), false); } catch (InterruptedException e) { throw new RuntimeException(e); } diff --git a/container-di/src/main/java/com/yahoo/container/di/config/Subscriber.java b/container-di/src/main/java/com/yahoo/container/di/config/Subscriber.java index 9fd30f888b9..0feab7779ad 100644 --- a/container-di/src/main/java/com/yahoo/container/di/config/Subscriber.java +++ b/container-di/src/main/java/com/yahoo/container/di/config/Subscriber.java @@ -14,6 +14,7 @@ public interface Subscriber { long waitNextGeneration(); long generation(); + boolean internalRedeploy(); boolean configChanged(); Map<ConfigKey<ConfigInstance>, ConfigInstance> config(); diff --git a/container-di/src/main/scala/com/yahoo/container/di/CloudSubscriberFactory.scala b/container-di/src/main/scala/com/yahoo/container/di/CloudSubscriberFactory.scala index afef3e96821..0f3fab93e80 100644 --- a/container-di/src/main/scala/com/yahoo/container/di/CloudSubscriberFactory.scala +++ b/container-di/src/main/scala/com/yahoo/container/di/CloudSubscriberFactory.scala @@ -16,9 +16,8 @@ import scala.language.existentials /** * @author Tony Vaagenes */ +class CloudSubscriberFactory(configSource: ConfigSource) extends SubscriberFactory { -class CloudSubscriberFactory(configSource: ConfigSource) extends SubscriberFactory -{ private var testGeneration: Option[Long] = None private val activeSubscribers = new java.util.WeakHashMap[CloudSubscriber, Int]() @@ -50,9 +49,12 @@ object CloudSubscriberFactory { private val handles: Map[ConfigKeyT, ConfigHandle[_ <: ConfigInstance]] = keys.map(subscribe).toMap - //if waitNextGeneration has not yet been called, -1 should be returned + // if waitNextGeneration has not yet been called, -1 should be returned var generation: Long = -1 + // True if this reconfiguration was caused by a system-internal redeploy, not an external application change + var internalRedeploy: Boolean = false + private def subscribe(key: ConfigKeyT) = (key, subscriber.subscribe(key.getConfigClass, key.getConfigId)) override def configChanged = handles.values.exists(_.isChanged) @@ -79,13 +81,14 @@ object CloudSubscriberFactory { case e: IllegalArgumentException => numExceptions += 1 log.log(Level.WARNING, "Got exception from the config system (please ignore the exception if you just removed " - + "a component from your application that used the mentioned config): ", e) + + "a component from your application that used the mentioned config): ", e) if (numExceptions >= 5) throw new IllegalArgumentException("Failed retrieving the next config generation.", e) } } generation = subscriber.getGeneration + internalRedeploy = subscriber.isInternalRedeploy generation } diff --git a/container-di/src/main/scala/com/yahoo/container/di/ConfigRetriever.scala b/container-di/src/main/scala/com/yahoo/container/di/ConfigRetriever.scala index dc94d789f7b..aad9e17acb2 100644 --- a/container-di/src/main/scala/com/yahoo/container/di/ConfigRetriever.scala +++ b/container-di/src/main/scala/com/yahoo/container/di/ConfigRetriever.scala @@ -27,27 +27,43 @@ final class ConfigRetriever(bootstrapKeys: Set[ConfigKeyT], private var componentSubscriber: Subscriber = subscribe(Set()) private var componentSubscriberKeys: Set[ConfigKeyT] = Set() - + /** Loop forever until we get config */ @tailrec - final def getConfigs(componentConfigKeys: Set[ConfigKeyT], leastGeneration: Long): ConfigSnapshot = { + final def getConfigs(componentConfigKeys: Set[ConfigKeyT], leastGeneration: Long, restartOnRedeploy: Boolean = false): ConfigSnapshot = { require(componentConfigKeys intersect bootstrapKeys isEmpty) log.log(DEBUG, "getConfigs: " + componentConfigKeys) setupComponentSubscriber(componentConfigKeys ++ bootstrapKeys) - getConfigsOptional(leastGeneration) match { + getConfigsOptional(leastGeneration, restartOnRedeploy) match { case Some(snapshot) => resetComponentSubscriberIfBootstrap(snapshot); snapshot - case None => getConfigs(componentConfigKeys, leastGeneration) + case None => getConfigs(componentConfigKeys, leastGeneration, restartOnRedeploy) + } + } + + + /** Try to get config just once */ + final def getConfigsOnce(componentConfigKeys: Set[ConfigKeyT], leastGeneration: Long, restartOnRedeploy: Boolean = false): Option[ConfigSnapshot] = { + require(componentConfigKeys intersect bootstrapKeys isEmpty) + log.log(DEBUG, "getConfigsOnce: " + componentConfigKeys) + + setupComponentSubscriber(componentConfigKeys ++ bootstrapKeys) + + getConfigsOptional(leastGeneration, restartOnRedeploy) match { + case Some(snapshot) => resetComponentSubscriberIfBootstrap(snapshot); Some(snapshot) + case None => None; } } - private def getConfigsOptional(leastGeneration: Long): Option[ConfigSnapshot] = { + private def getConfigsOptional(leastGeneration: Long, restartOnRedeploy: Boolean): Option[ConfigSnapshot] = { val newestComponentGeneration = componentSubscriber.waitNextGeneration() log.log(DEBUG, s"getConfigsOptional: new component generation: $newestComponentGeneration") // leastGeneration is only used to ensure newer generation when the previous generation was invalidated due to an exception if (newestComponentGeneration < leastGeneration) { None + } else if (restartOnRedeploy && ! componentSubscriber.internalRedeploy()) { // Don't reconfig - wait for restart + None } else if (bootstrapSubscriber.generation < newestComponentGeneration) { val newestBootstrapGeneration = bootstrapSubscriber.waitNextGeneration() log.log(DEBUG, s"getConfigsOptional: new bootstrap generation: ${bootstrapSubscriber.generation}") diff --git a/container-di/src/main/scala/com/yahoo/container/di/Container.scala b/container-di/src/main/scala/com/yahoo/container/di/Container.scala index 5ea6422ddad..2a185d41a6c 100644 --- a/container-di/src/main/scala/com/yahoo/container/di/Container.scala +++ b/container-di/src/main/scala/com/yahoo/container/di/Container.scala @@ -44,9 +44,9 @@ class Container( var leastGeneration = -1L @throws(classOf[InterruptedException]) - def runOnce( - oldGraph: ComponentGraph = new ComponentGraph, - fallbackInjector: GuiceInjector = Guice.createInjector()): ComponentGraph = { + def getNewComponentGraph(oldGraph: ComponentGraph = new ComponentGraph, + fallbackInjector: GuiceInjector = Guice.createInjector(), + restartOnRedeploy: Boolean = false): ComponentGraph = { def deconstructObsoleteComponents(oldGraph: ComponentGraph, newGraph: ComponentGraph) { val oldComponents = new IdentityHashMap[AnyRef, AnyRef]() @@ -56,7 +56,7 @@ class Container( } try { - val newGraph = createNewGraph(oldGraph, fallbackInjector) + val newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, restartOnRedeploy) newGraph.reuseNodes(oldGraph) constructComponents(newGraph) deconstructObsoleteComponents(oldGraph, newGraph) @@ -113,10 +113,12 @@ class Container( } } - final def createNewGraph(graph: ComponentGraph = new ComponentGraph, - fallbackInjector: Injector): ComponentGraph = { + final def getConfigAndCreateGraph(graph: ComponentGraph = new ComponentGraph, + fallbackInjector: Injector, + restartOnRedeploy: Boolean): ComponentGraph = { + + val snapshot = configurer.getConfigs(graph.configKeys, leastGeneration, restartOnRedeploy) - val snapshot = configurer.getConfigs(graph.configKeys, leastGeneration) log.log(DEBUG, """createNewGraph: |graph.configKeys = %s @@ -138,9 +140,8 @@ class Container( |previous generation: %d""" .format(getBootstrapGeneration, getComponentsGeneration, previousConfigGeneration).stripMargin) installBundles(configs) - createNewGraph( - createComponentsGraph(configs, getBootstrapGeneration,fallbackInjector), - fallbackInjector) + getConfigAndCreateGraph( + createComponentsGraph(configs, getBootstrapGeneration,fallbackInjector), fallbackInjector, restartOnRedeploy) case ComponentsConfigs(configs) => log.log(DEBUG, """Got components configs, diff --git a/container-di/src/test/java/demo/ContainerTestBase.java b/container-di/src/test/java/demo/ContainerTestBase.java index 426033ea101..9c2415c3514 100644 --- a/container-di/src/test/java/demo/ContainerTestBase.java +++ b/container-di/src/test/java/demo/ContainerTestBase.java @@ -12,12 +12,9 @@ import com.yahoo.container.di.Osgi; import com.yahoo.container.di.componentgraph.core.ComponentGraph; import org.junit.Before; import org.osgi.framework.Bundle; -import scala.collection.*; -import scala.collection.immutable.*; import scala.collection.immutable.Set; import java.util.Collection; -import java.util.List; /** * @author tonytv @@ -62,7 +59,7 @@ public class ContainerTestBase extends ContainerTest { throw new UnsupportedOperationException("getBundle not supported."); } }); - componentGraph = container.runOnce(componentGraph, Guice.createInjector()); + componentGraph = container.getNewComponentGraph(componentGraph, Guice.createInjector(), false); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/container-di/src/test/scala/com/yahoo/container/di/ConfigRetrieverTest.scala b/container-di/src/test/scala/com/yahoo/container/di/ConfigRetrieverTest.scala index 93618f90e92..7f1d9a73a82 100644 --- a/container-di/src/test/scala/com/yahoo/container/di/ConfigRetrieverTest.scala +++ b/container-di/src/test/scala/com/yahoo/container/di/ConfigRetrieverTest.scala @@ -18,11 +18,13 @@ import scala.collection.JavaConverters._ * @author tonytv */ class ConfigRetrieverTest { + var dirConfigSource: DirConfigSource = null @Before def setup() { dirConfigSource = new DirConfigSource("ConfigRetrieverTest-") } + @After def cleanup() { dirConfigSource.cleanup() } @Test @@ -49,6 +51,22 @@ class ConfigRetrieverTest { } } + @Test + def require_no_reconfig_when_restart_on_redeploy() { + // TODO + writeConfigs() + val retriever = createConfigRetriever() + val bootstrapConfigs = retriever.getConfigs(Set(), 0) + + val testConfigKey = new ConfigKey(classOf[TestConfig], dirConfigSource.configId) + val componentsConfigs = retriever.getConfigsOnce(Set(testConfigKey), 0, true) + + componentsConfigs match { + case Some(snapshot) => fail("Expected no configs") + case _ => // ok + } + } + @Test(expected = classOf[IllegalArgumentException]) @Ignore def require_exception_upon_modified_components_keys_without_bootstrap() { diff --git a/container-di/src/test/scala/com/yahoo/container/di/ContainerTest.scala b/container-di/src/test/scala/com/yahoo/container/di/ContainerTest.scala index 8e7fbde3f5e..9f07acc7dc9 100644 --- a/container-di/src/test/scala/com/yahoo/container/di/ContainerTest.scala +++ b/container-di/src/test/scala/com/yahoo/container/di/ContainerTest.scala @@ -44,7 +44,7 @@ class ContainerTest { val container = newContainer(dirConfigSource) - val component = createComponentTakingConfig(container.runOnce()) + val component = createComponentTakingConfig(container.getNewComponentGraph()) assertThat(component.config.stringVal(), is("myString")) container.shutdownConfigurer() @@ -57,7 +57,7 @@ class ContainerTest { val container = newContainer(dirConfigSource) - val componentGraph = container.runOnce() + val componentGraph = container.getNewComponentGraph() val component = createComponentTakingConfig(componentGraph) assertThat(component.config.stringVal(), is("original")) @@ -66,7 +66,7 @@ class ContainerTest { dirConfigSource.writeConfig("test", """stringVal "reconfigured" """) container.reloadConfig(2) - val newComponentGraph = container.runOnce(componentGraph) + val newComponentGraph = container.getNewComponentGraph(componentGraph) val component2 = createComponentTakingConfig(newComponentGraph) assertThat(component2.config.stringVal(), is("reconfigured")) @@ -80,7 +80,7 @@ class ContainerTest { val container = newContainer(dirConfigSource) - val graph = container.runOnce() + val graph = container.getNewComponentGraph() val component = createComponentTakingConfig(graph) assertThat(component.getId.toString, is("id1")) @@ -89,7 +89,7 @@ class ContainerTest { ("id2", classOf[ComponentTakingConfig]))) container.reloadConfig(2) - val newGraph = container.runOnce(graph) + val newGraph = container.getNewComponentGraph(graph) assertThat(ComponentGraph.getNode(newGraph, "id1"), notNullValue(classOf[Node])) assertThat(ComponentGraph.getNode(newGraph, "id2"), notNullValue(classOf[Node])) @@ -107,12 +107,12 @@ class ContainerTest { val container = newContainer(dirConfigSource) - val oldGraph = container.runOnce() + val oldGraph = container.getNewComponentGraph() val componentToDestruct = oldGraph.getInstance(classOf[DestructableComponent]) writeBootstrapConfigs("id2", classOf[DestructableComponent]) container.reloadConfig(2) - container.runOnce(oldGraph) + container.getNewComponentGraph(oldGraph) assertTrue(componentToDestruct.deconstructed) } @@ -123,7 +123,7 @@ class ContainerTest { val container = newContainer(dirConfigSource) var currentGraph: ComponentGraph = null try { - currentGraph = container.runOnce() + currentGraph = container.getNewComponentGraph() fail("Expected to log and die.") } catch { case _: Throwable => fail("Expected to log and die") @@ -136,14 +136,14 @@ class ContainerTest { writeBootstrapConfigs(Array(simpleComponentEntry)) val container = newContainer(dirConfigSource) - var currentGraph = container.runOnce() + var currentGraph = container.getNewComponentGraph() val simpleComponent = currentGraph.getInstance(classOf[SimpleComponent]) writeBootstrapConfigs("thrower", classOf[ComponentThrowingExceptionInConstructor]) container.reloadConfig(2) try { - currentGraph = container.runOnce(currentGraph) + currentGraph = container.getNewComponentGraph(currentGraph) fail("Expected exception") } catch { case _: ComponentConstructorException => // Expected, do nothing @@ -156,7 +156,7 @@ class ContainerTest { dirConfigSource.writeConfig("test", """stringVal "myString" """) writeBootstrapConfigs(Array(simpleComponentEntry, componentTakingConfigEntry)) container.reloadConfig(3) - currentGraph = container.runOnce(currentGraph) + currentGraph = container.getNewComponentGraph(currentGraph) assertEquals(3, currentGraph.generation) assertSame(simpleComponent, currentGraph.getInstance(classOf[SimpleComponent])) @@ -169,7 +169,7 @@ class ContainerTest { writeBootstrapConfigs(Array(simpleComponentEntry)) val container = newContainer(dirConfigSource) - var currentGraph = container.runOnce() + var currentGraph = container.getNewComponentGraph() val simpleComponent = currentGraph.getInstance(classOf[SimpleComponent]) @@ -177,7 +177,7 @@ class ContainerTest { dirConfigSource.writeConfig("test", """stringVal "myString" """) container.reloadConfig(2) try { - currentGraph = container.runOnce(currentGraph) + currentGraph = container.getNewComponentGraph(currentGraph) fail("Expected exception") } catch { case _: IllegalArgumentException => // Expected, do nothing @@ -192,20 +192,20 @@ class ContainerTest { writeBootstrapConfigs("myId", classOf[ComponentTakingConfig]) val container = newContainer(dirConfigSource) - var currentGraph = container.runOnce() + var currentGraph = container.getNewComponentGraph() writeBootstrapConfigs("thrower", classOf[ComponentThrowingExceptionForMissingConfig]) container.reloadConfig(2) try { - currentGraph = container.runOnce(currentGraph) + currentGraph = container.getNewComponentGraph(currentGraph) fail("expected exception") } catch { case e: Exception => } val newGraph = Future { - currentGraph = container.runOnce(currentGraph) + currentGraph = container.getNewComponentGraph(currentGraph) currentGraph } @@ -230,7 +230,7 @@ class ContainerTest { dirConfigSource.writeConfig("jersey-injection", """inject[0]" """) val container = newContainer(dirConfigSource) - val componentGraph = container.runOnce() + val componentGraph = container.getNewComponentGraph() val restApiContext = componentGraph.getInstance(clazz) assertNotNull(restApiContext) @@ -278,7 +278,7 @@ class ContainerTest { dirConfigSource.writeConfig("jersey-injection", injectionConfig) val container = newContainer(dirConfigSource) - val componentGraph = container.runOnce() + val componentGraph = container.getNewComponentGraph() val restApiContext = componentGraph.getInstance(restApiClass) } @@ -328,12 +328,12 @@ class ContainerTest { val container = newContainer(dirConfigSource, deconstructor) - val oldGraph = container.runOnce() + val oldGraph = container.getNewComponentGraph() val destructableEntity = oldGraph.getInstance(classOf[DestructableEntity]) writeBootstrapConfigs("id2", classOf[DestructableProvider]) container.reloadConfig(2) - container.runOnce(oldGraph) + container.getNewComponentGraph(oldGraph) assertTrue(destructableEntity.deconstructed) } diff --git a/container-di/src/test/scala/com/yahoo/container/di/DirConfigSource.scala b/container-di/src/test/scala/com/yahoo/container/di/DirConfigSource.scala index 5afa1bc418e..4f80b25a247 100644 --- a/container-di/src/test/scala/com/yahoo/container/di/DirConfigSource.scala +++ b/container-di/src/test/scala/com/yahoo/container/di/DirConfigSource.scala @@ -7,14 +7,12 @@ import java.util.Random import org.junit.rules.TemporaryFolder import com.yahoo.config.subscription.{ConfigSource, ConfigSourceSet} -// TODO: Make this a junit rule. Does not yet work. Look out for junit updates -// (@Rule def configSourceRule = dirConfigSource) - /** * @author tonytv * @author gjoranv */ class DirConfigSource(val testSourcePrefix: String) { + private val tempFolder = createTemporaryFolder() val configSource : ConfigSource = new ConfigSourceSet(testSourcePrefix + new Random().nextLong) @@ -32,9 +30,11 @@ class DirConfigSource(val testSourcePrefix: String) { def cleanup() { tempFolder.delete() } + } private object DirConfigSource { + def printFile(f: File, content: String) { var out: OutputStream = new FileOutputStream(f) out.write(content.getBytes("UTF-8")) @@ -46,4 +46,5 @@ private object DirConfigSource { folder.create() folder } + } diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java index 1977b934253..932d31c0036 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java @@ -128,7 +128,7 @@ public final class ConfiguredApplication implements Application { configureComponents(builder.guiceModules().activate()); intitializeAndActivateContainer(builder); - if (! qrConfig.restartOnDeploy()) startReconfigurerThread(); + startReconfigurerThread(); portWatcher = new Thread(this::watchPortChange); portWatcher.setDaemon(true); portWatcher.start(); @@ -199,7 +199,9 @@ public final class ConfiguredApplication implements Application { while ( ! Thread.interrupted()) { try { ContainerBuilder builder = createBuilderWithGuiceBindings(); - configurer.runOnceAndEnsureRegistryHackRun(builder.guiceModules().activate()); + + // Block until new config arrives, and it should be applied + configurer.getNewComponentGraph(builder.guiceModules().activate(), qrConfig.restartOnDeploy()); intitializeAndActivateContainer(builder); } catch (ConfigInterruptedException | InterruptedException e) { break; diff --git a/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java b/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java index 70e9357e7cf..3d596cc7d34 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java @@ -52,9 +52,32 @@ public class SameElementItem extends CompositeItem { Validator.ensureNonEmpty("Struct fieldname", asTerm.getIndexName()); Validator.ensureNonEmpty("Query term", asTerm.getIndexedString()); Validator.ensure("Struct fieldname starts with '" + getFieldName() + ".'", - !asTerm.getIndexName().startsWith(fieldName+".")); - item.setIndexName(fieldName + '.' + asTerm.getIndexName()); + !asTerm.getIndexName().startsWith(fieldName+".") || (item.getParent() != null)); + super.adding(item); } + + private void expandChild(Item item) { + item.setIndexName(fieldName + '.' + ((TermItem)item).getIndexName()); + } + @Override + public void addItem(int index, Item item) { + super.addItem(index, item); + expandChild(item); + } + + @Override + public void addItem(Item item) { + super.addItem(item); + expandChild(item); + } + + @Override + public Item setItem(int index, Item item) { + Item prev = super.setItem(index, item); + expandChild(item); + return prev; + } + @Override public ItemType getItemType() { return ItemType.SAME_ELEMENT; diff --git a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java index c6097b1bc73..0b9f79537d0 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java +++ b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java @@ -102,6 +102,10 @@ public class YqlParser implements Parser { NEVER, POSSIBLY, ALWAYS; } + private static class IndexNameExpander { + public String expand(String leaf) { return leaf; } + } + private static final Integer DEFAULT_HITS = 10; private static final Integer DEFAULT_OFFSET = 0; private static final Integer DEFAULT_TARGET_NUM_HITS = 10; @@ -194,6 +198,7 @@ public class YqlParser implements Parser { private Query userQuery; private Parsable currentlyParsing; private IndexFacts.Session indexFactsSession; + private IndexNameExpander indexNameExpander = new IndexNameExpander(); private Set<String> docTypes; private Sorting sorting; private String segmenterBackend; @@ -534,14 +539,28 @@ public class YqlParser implements Parser { return leafStyleSettings(ast, out); } + private static class PrefixExpander extends IndexNameExpander { + private final String prefix; + public PrefixExpander(String prefix) { + this.prefix = prefix + "."; + } + + @Override + public String expand(String leaf) { + return prefix + leaf; + } + } @NonNull private Item instantiateSameElementItem(String field, OperatorNode<ExpressionOperator> ast) { assertHasFunctionName(ast, SAME_ELEMENT); SameElementItem sameElement = new SameElementItem(field); - for (OperatorNode<ExpressionOperator> word : ast.<List<OperatorNode<ExpressionOperator>>> getArgument(1)) { - sameElement.addItem(buildTermSearch(word)); + // All terms below sameElement are relative to this. + IndexNameExpander prev = swapIndexCreator(new PrefixExpander(field)); + for (OperatorNode<ExpressionOperator> term : ast.<List<OperatorNode<ExpressionOperator>>> getArgument(1)) { + sameElement.addItem(convertExpression(term)); } + swapIndexCreator(prev); return sameElement; } @@ -896,8 +915,16 @@ public class YqlParser implements Parser { @NonNull private static String fetchFieldRead(OperatorNode<ExpressionOperator> ast) { - assertHasOperator(ast, ExpressionOperator.READ_FIELD); - return ast.getArgument(1); + switch (ast.getOperator()) { + case READ_FIELD: + return ast.getArgument(1); + case PROPREF: + return new StringBuilder(fetchFieldRead(ast.getArgument(0))) + .append('.').append(ast.getArgument(1).toString()).toString(); + default: + throw newUnexpectedArgumentException(ast.getOperator(), + ExpressionOperator.READ_FIELD, ExpressionOperator.PROPREF); + } } @NonNull @@ -967,14 +994,12 @@ public class YqlParser implements Parser { OperatorNode<ExpressionOperator> lhs = ast.getArgument(0); OperatorNode<ExpressionOperator> rhs = ast.getArgument(1); if (lhs.getOperator() == ExpressionOperator.LITERAL || lhs.getOperator() == ExpressionOperator.NEGATE) { - assertHasOperator(rhs, ExpressionOperator.READ_FIELD); return getIndex(rhs); } if (rhs.getOperator() == ExpressionOperator.LITERAL || rhs.getOperator() == ExpressionOperator.NEGATE) { - assertHasOperator(lhs, ExpressionOperator.READ_FIELD); return getIndex(lhs); } - throw new IllegalArgumentException("Expected LITERAL and READ_FIELD, got " + lhs.getOperator() + + throw new IllegalArgumentException("Expected LITERAL and READ_FIELD/PROPREF, got " + lhs.getOperator() + " and " + rhs.getOperator() + "."); } @@ -990,28 +1015,24 @@ public class YqlParser implements Parser { } @NonNull - private static String fetchConditionWord( - OperatorNode<ExpressionOperator> ast) { + private static String fetchConditionWord(OperatorNode<ExpressionOperator> ast) { OperatorNode<ExpressionOperator> lhs = ast.getArgument(0); OperatorNode<ExpressionOperator> rhs = ast.getArgument(1); - if (lhs.getOperator() == ExpressionOperator.LITERAL - || lhs.getOperator() == ExpressionOperator.NEGATE) { - assertHasOperator(rhs, ExpressionOperator.READ_FIELD); + if (lhs.getOperator() == ExpressionOperator.LITERAL || lhs.getOperator() == ExpressionOperator.NEGATE) { + assertFieldName(rhs); return getNumberAsString(lhs); } - if (rhs.getOperator() == ExpressionOperator.LITERAL - || rhs.getOperator() == ExpressionOperator.NEGATE) { - assertHasOperator(lhs, ExpressionOperator.READ_FIELD); + if (rhs.getOperator() == ExpressionOperator.LITERAL || rhs.getOperator() == ExpressionOperator.NEGATE) { + assertFieldName(lhs); return getNumberAsString(rhs); } - throw new IllegalArgumentException( - "Expected LITERAL/NEGATE and READ_FIELD, got " + throw new IllegalArgumentException("Expected LITERAL/NEGATE and READ_FIELD/PROPREF, got " + lhs.getOperator() + " and " + rhs.getOperator() + "."); } - private static boolean isIndexOnLeftHandSide( - OperatorNode<ExpressionOperator> ast) { - return ast.getArgument(0, OperatorNode.class).getOperator() == ExpressionOperator.READ_FIELD; + private static boolean isIndexOnLeftHandSide(OperatorNode<ExpressionOperator> ast) { + OperatorNode node = ast.getArgument(0, OperatorNode.class); + return node.getOperator() == ExpressionOperator.READ_FIELD || node.getOperator() == ExpressionOperator.PROPREF; } @NonNull @@ -1539,6 +1560,12 @@ public class YqlParser implements Parser { expectedFunctionName, names.get(0)); } + private static void assertFieldName(OperatorNode<?> ast) { + Preconditions.checkArgument(ast.getOperator() == ExpressionOperator.READ_FIELD || + ast.getOperator() == ExpressionOperator.PROPREF, + "Expected operator READ_FIELD or PRPPREF, got %s.", ast.getOperator()); + } + private static void addItems(OperatorNode<ExpressionOperator> ast, WeightedSetItem out) { switch (ast.getOperator()) { case MAP: @@ -1619,21 +1646,25 @@ public class YqlParser implements Parser { } } + private IndexNameExpander swapIndexCreator(IndexNameExpander newExpander) { + IndexNameExpander old = indexNameExpander; + indexNameExpander = newExpander; + return old; + } @NonNull private String getIndex(OperatorNode<ExpressionOperator> operatorNode) { String index = fetchFieldRead(operatorNode); - Preconditions.checkArgument(indexFactsSession.isIndex(index), "Field '%s' does not exist.", index); + String expanded = indexNameExpander.expand(index); + Preconditions.checkArgument(indexFactsSession.isIndex(expanded), "Field '%s' does not exist.", expanded); return indexFactsSession.getCanonicName(index); } private Substring getOrigin(OperatorNode<ExpressionOperator> ast) { - Map<?, ?> origin = getAnnotation(ast, ORIGIN, Map.class, null, - ORIGIN_DESCRIPTION); + Map<?, ?> origin = getAnnotation(ast, ORIGIN, Map.class, null, ORIGIN_DESCRIPTION); if (origin == null) { return null; } - String original = getMapValue(ORIGIN, origin, ORIGIN_ORIGINAL, - String.class); + String original = getMapValue(ORIGIN, origin, ORIGIN_ORIGINAL, String.class); int offset = getMapValue(ORIGIN, origin, ORIGIN_OFFSET, Integer.class); int length = getMapValue(ORIGIN, origin, ORIGIN_LENGTH, Integer.class); return new Substring(offset, length + offset, original); diff --git a/container-search/src/test/java/com/yahoo/prelude/query/test/SameElementItemTestCase.java b/container-search/src/test/java/com/yahoo/prelude/query/test/SameElementItemTestCase.java index 01c03fcd802..0e4b994fe99 100644 --- a/container-search/src/test/java/com/yahoo/prelude/query/test/SameElementItemTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/query/test/SameElementItemTestCase.java @@ -16,6 +16,16 @@ public class SameElementItemTestCase { s.addItem(new WordItem("d", "f3")); assertEquals("structa:{f1:b f2:c f3:d}", s.toString()); } + @Test + public void testClone() { + SameElementItem s = new SameElementItem("structa"); + s.addItem(new WordItem("b", "f1")); + s.addItem(new WordItem("c", "f2")); + s.addItem(new WordItem("d", "f3")); + assertEquals("structa:{f1:b f2:c f3:d}", s.toString()); + SameElementItem c = (SameElementItem)s.clone(); + assertEquals("structa:{f1:b f2:c f3:d}", c.toString()); + } @Test(expected = IllegalArgumentException.class) public void requireAllChildrenHaveStructMemberNameSet() { SameElementItem s = new SameElementItem("structa"); diff --git a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java index 9d04dc545e1..8bbe43ee3d4 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java @@ -118,6 +118,17 @@ public class YqlParserTestCase { } @Test + public void testDottedFieldNames() { + assertParse("select foo from bar where my.nested.title contains \"madonna\";", + "my.nested.title:madonna"); + } + @Test + public void testDottedNestedFieldNames() { + assertParse("select foo from bar where my.title contains \"madonna\";", + "my.title:madonna"); + } + + @Test public void testOr() { assertParse("select foo from bar where title contains \"madonna\" or title contains \"saint\";", "OR title:madonna title:saint"); @@ -264,6 +275,10 @@ public class YqlParserTestCase { public void testSameElement() { assertParse("select foo from bar where baz contains sameElement(f1 contains \"a\", f2 contains \"b\");", "baz:{f1:a f2:b}"); + assertParse("select foo from bar where baz contains sameElement(f1 contains \"a\", f2 = 10);", + "baz:{f1:a f2:10}"); + assertParse("select foo from bar where baz contains sameElement(key contains \"a\", value.f2 = 10);", + "baz:{key:a value.f2:10}"); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java index 9d5fcb31288..a81c4adcb2e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java @@ -85,7 +85,7 @@ public class ApplicationPackageBuilder { public ApplicationPackageBuilder allow(ValidationId validationId) { validationOverridesBody.append(" <allow until='"); - validationOverridesBody.append(asIso8601Date(Instant.now().plus(Duration.ofDays(29)))); + validationOverridesBody.append(asIso8601Date(Instant.now().plus(Duration.ofDays(28)))); validationOverridesBody.append("'>"); validationOverridesBody.append(validationId.value()); validationOverridesBody.append("</allow>\n"); diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerActivator.java b/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerActivator.java index 9d1b613e23c..59db453e2c4 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerActivator.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerActivator.java @@ -13,27 +13,28 @@ import com.yahoo.jdisc.Container; * #newContainerBuilder()}, 2) configure the returned {@link ContainerBuilder}, and 3) pass the builder to the {@link * #activateContainer(ContainerBuilder)} method.</p> * - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a> + * @author Simon Thoresen */ public interface ContainerActivator { /** - * <p>This method creates and returns a new {@link ContainerBuilder} object that has the necessary references to the - * application and its internal components.</p> + * This method creates and returns a new {@link ContainerBuilder} object that has the necessary references to the + * application and its internal components. * * @return The created builder. */ - public ContainerBuilder newContainerBuilder(); + ContainerBuilder newContainerBuilder(); /** - * <p>Creates and activates a {@link Container} based on the provided {@link ContainerBuilder}. By providing a + * Creates and activates a {@link Container} based on the provided {@link ContainerBuilder}. By providing a * <em>null</em> argument, this method can be used to deactivate the current Container. The returned object can be - * used to schedule a cleanup task that is executed once the the deactivated Container has terminated.</p> + * used to schedule a cleanup task that is executed once the the deactivated Container has terminated. * * @param builder The builder to activate. * @return The previous container, if any. * @throws ApplicationNotReadyException If this method is called before {@link Application#start()} or after {@link * Application#stop()}. */ - public DeactivatedContainer activateContainer(ContainerBuilder builder); + DeactivatedContainer activateContainer(ContainerBuilder builder); + } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java index ef5f2e60220..8cd877b25e5 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java @@ -91,6 +91,14 @@ public class DockerOperationsImpl implements DockerOperations { command.withVolume("/var/lib/sia", "/var/lib/sia"); } + // TODO When rolling out host-admin on-prem: Always map in /var/zpe from host + make sure zpu is configured on host + if (environment.getCloud().equalsIgnoreCase("yahoo")) { + Path pathInNode = environment.pathInNodeUnderVespaHome("var/zpe"); + command.withVolume(environment.pathInHostFromPathInNode(containerName, pathInNode).toString(), pathInNode.toString()); + } else if (environment.getNodeType() == NodeType.host) { + command.withVolume("/var/zpe", environment.pathInNodeUnderVespaHome("var/zpe").toString()); + } + if (environment.getNodeType() == NodeType.proxyhost) { command.withVolume("/opt/yahoo/share/ssl/certs/", "/opt/yahoo/share/ssl/certs/"); } @@ -354,7 +362,6 @@ public class DockerOperationsImpl implements DockerOperations { directoriesToMount.put(environment.pathInNodeUnderVespaHome("var/yca"), true); directoriesToMount.put(environment.pathInNodeUnderVespaHome("var/ycore++"), false); directoriesToMount.put(environment.pathInNodeUnderVespaHome("var/zookeeper"), false); - directoriesToMount.put(environment.pathInNodeUnderVespaHome("var/zpe"), false); directoriesToMount.put(environment.pathInNodeUnderVespaHome("tmp"), false); directoriesToMount.put(environment.pathInNodeUnderVespaHome("var/container-data"), false); if (environment.getNodeType() == NodeType.proxyhost) diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java index bd75368a0dc..f7e9c3ca1d8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java @@ -148,7 +148,6 @@ public class AthenzCredentialsMaintainer { } } - @SuppressWarnings("deprecation") private VespaUniqueInstanceId getVespaUniqueInstanceId(NodeSpec nodeSpec) { NodeSpec.Membership membership = nodeSpec.getMembership().get(); NodeSpec.Owner owner = nodeSpec.getOwner().get(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/NodeIdentifierTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/NodeIdentifierTest.java index 11c7832091b..c0cead74f5f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/NodeIdentifierTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/NodeIdentifierTest.java @@ -29,7 +29,6 @@ import java.security.cert.X509Certificate; import java.time.Instant; import java.util.Optional; -import static com.yahoo.vespa.athenz.identityprovider.api.IdentityType.*; import static com.yahoo.vespa.athenz.tls.KeyAlgorithm.RSA; import static com.yahoo.vespa.athenz.tls.SignatureAlgorithm.SHA256_WITH_RSA; import static java.util.Collections.emptySet; @@ -162,7 +161,7 @@ public class NodeIdentifierTest { Pkcs10Csr csr = Pkcs10CsrBuilder .fromKeypair(new X500Principal("CN=" + TENANT_NODE_IDENTITY), KEYPAIR, SHA256_WITH_RSA) .build(); - VespaUniqueInstanceId vespaUniqueInstanceId = new VespaUniqueInstanceId(clusterIndex, clusterId, INSTANCE_ID, application, tenant, region, environment, NODE); + VespaUniqueInstanceId vespaUniqueInstanceId = new VespaUniqueInstanceId(clusterIndex, clusterId, INSTANCE_ID, application, tenant, region, environment); X509Certificate certificate = X509CertificateBuilder .fromCsr(csr, ATHENZ_YAHOO_CA_CERT.getSubjectX500Principal(), Instant.EPOCH, Instant.EPOCH.plusSeconds(60), KEYPAIR.getPrivate(), SHA256_WITH_RSA, 1) .addSubjectAlternativeName(vespaUniqueInstanceId.asDottedString() + ".instanceid.athenz.provider-name.vespa.yahoo.cloud") diff --git a/searchcore/src/tests/proton/matching/termdataextractor_test.cpp b/searchcore/src/tests/proton/matching/termdataextractor_test.cpp index 43d1d43a9d6..2570e64dbe2 100644 --- a/searchcore/src/tests/proton/matching/termdataextractor_test.cpp +++ b/searchcore/src/tests/proton/matching/termdataextractor_test.cpp @@ -44,6 +44,7 @@ class Test : public vespalib::TestApp { void requireThatAViewWithTwoFieldsGivesOneTermDataPerTerm(); void requireThatUnrankedTermsAreSkipped(); void requireThatNegativeTermsAreSkipped(); + void requireThatSameElementIsSkipped(); public: int Main() override; @@ -58,6 +59,7 @@ Test::Main() TEST_DO(requireThatAViewWithTwoFieldsGivesOneTermDataPerTerm()); TEST_DO(requireThatUnrankedTermsAreSkipped()); TEST_DO(requireThatNegativeTermsAreSkipped()); + TEST_DO(requireThatSameElementIsSkipped()); TEST_DONE(); } @@ -161,6 +163,24 @@ Test::requireThatNegativeTermsAreSkipped() EXPECT_EQUAL(id[1], term_data[1]->getUniqueId()); } +void +Test::requireThatSameElementIsSkipped() +{ + QueryBuilder<ProtonNodeTypes> query_builder; + query_builder.addAnd(2); + query_builder.addSameElement(2, field); + query_builder.addStringTerm("term1", field, id[0], Weight(1)); + query_builder.addStringTerm("term2", field, id[1], Weight(1)); + query_builder.addStringTerm("term3", field, id[2], Weight(1)); + Node::UP node = query_builder.build(); + + vector<const ITermData *> term_data; + TermDataExtractor::extractTerms(*node, term_data); + EXPECT_EQUAL(1u, term_data.size()); + ASSERT_TRUE(term_data.size() >= 1); + EXPECT_EQUAL(id[2], term_data[0]->getUniqueId()); +} + } // namespace TEST_APPHOOK(Test); diff --git a/searchcore/src/vespa/searchcore/proton/matching/termdataextractor.cpp b/searchcore/src/vespa/searchcore/proton/matching/termdataextractor.cpp index 6a9857e1172..596cc7fce1e 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/termdataextractor.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/termdataextractor.cpp @@ -41,6 +41,8 @@ public: // XXX: unranked equiv not supported _term_data.push_back(&n); } + + virtual void visit(ProtonNodeTypes::SameElement &) override {} }; } // namespace diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp index a0f6ee98b71..273b97542ef 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp @@ -319,7 +319,6 @@ StoreOnlyFeedView::updateAttributes(SerialNum, Lid, FutureDoc, bool, OnOperation void StoreOnlyFeedView::updateIndexedFields(SerialNum, Lid, FutureDoc, bool, OnOperationDoneType) { - abort(); // Should never be called. } void diff --git a/searchlib/src/tests/queryeval/same_element/same_element_test.cpp b/searchlib/src/tests/queryeval/same_element/same_element_test.cpp index e0c20949c8e..d89883bc417 100644 --- a/searchlib/src/tests/queryeval/same_element/same_element_test.cpp +++ b/searchlib/src/tests/queryeval/same_element/same_element_test.cpp @@ -58,7 +58,7 @@ TEST("require that children must match within same element") { auto a = make_result({{5, {1,3,7}}}); auto b = make_result({{5, {2,5,10}}}); SimpleResult result = find_matches({a, b}); - SimpleResult expect({}); + SimpleResult expect; EXPECT_EQUAL(result, expect); } diff --git a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java index 882bb5709f4..6ea2671b05b 100644 --- a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java +++ b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java @@ -27,6 +27,7 @@ public class StandaloneSubscriberFactory implements SubscriberFactory { } private class StandaloneSubscriber implements Subscriber { + private final Set<ConfigKey<ConfigInstance>> configKeys; private long generation = -1L; @@ -35,6 +36,9 @@ public class StandaloneSubscriberFactory implements SubscriberFactory { } @Override + public boolean internalRedeploy() { return false; } + + @Override public boolean configChanged() { return generation == 0; } diff --git a/storage/src/tests/visiting/visitormanagertest.cpp b/storage/src/tests/visiting/visitormanagertest.cpp index 8b17e851868..b9d4f24072d 100644 --- a/storage/src/tests/visiting/visitormanagertest.cpp +++ b/storage/src/tests/visiting/visitormanagertest.cpp @@ -19,9 +19,14 @@ #include <vespa/documentapi/messagebus/messages/removedocumentmessage.h> #include <vespa/documentapi/messagebus/messages/visitor.h> #include <vespa/config/common/exceptions.h> +#include <optional> +#include <thread> +#include <chrono> using document::test::makeDocumentBucket; using document::test::makeBucketSpace; +using documentapi::Priority; +using namespace std::chrono_literals; namespace storage { namespace { @@ -78,8 +83,9 @@ public: std::vector<document::Document::SP >& docs, std::vector<document::DocumentId>& docIds, api::ReturnCode::Result returnCode = api::ReturnCode::OK, - documentapi::Priority::Value priority = documentapi::Priority::PRI_NORMAL_4); + std::optional<Priority::Value> priority = documentapi::Priority::PRI_NORMAL_4); uint32_t getMatchingDocuments(std::vector<document::Document::SP >& docs); + void finishAndWaitForVisitorSessionCompletion(uint32_t sessionIndex); void testNormalUsage(); void testResending(); @@ -185,8 +191,7 @@ VisitorManagerTest::initializeTest() for (uint32_t i=0; i<10; ++i) { document::BucketId bid(16, i); - std::shared_ptr<api::CreateBucketCommand> cmd( - new api::CreateBucketCommand(makeDocumentBucket(bid))); + auto cmd = std::make_shared<api::CreateBucketCommand>(makeDocumentBucket(bid)); cmd->setAddress(address); cmd->setSourceIndex(0); _top->sendDown(cmd); @@ -202,8 +207,7 @@ VisitorManagerTest::initializeTest() for (uint32_t i=0; i<docCount; ++i) { document::BucketId bid(16, i); - std::shared_ptr<api::PutCommand> cmd( - new api::PutCommand(makeDocumentBucket(bid), _documents[i], i+1)); + auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), _documents[i], i+1); cmd->setAddress(address); _top->sendDown(cmd); _top->waitForMessages(1, 60); @@ -226,45 +230,40 @@ VisitorManagerTest::addSomeRemoves(bool removeAll) for (uint32_t i=0; i<docCount; i += (removeAll ? 1 : 4)) { // Add it to the database document::BucketId bid(16, i % 10); - std::shared_ptr<api::RemoveCommand> cmd( - new api::RemoveCommand( - makeDocumentBucket(bid), _documents[i]->getId(), clock.getTimeInMicros().getTime() + docCount + i + 1)); + auto cmd = std::make_shared<api::RemoveCommand>( + makeDocumentBucket(bid), _documents[i]->getId(), clock.getTimeInMicros().getTime() + docCount + i + 1); cmd->setAddress(address); _top->sendDown(cmd); _top->waitForMessages(1, 60); const msg_ptr_vector replies = _top->getRepliesOnce(); - CPPUNIT_ASSERT_EQUAL((size_t) 1, replies.size()); - std::shared_ptr<api::RemoveReply> reply( - std::dynamic_pointer_cast<api::RemoveReply>( - replies[0])); + CPPUNIT_ASSERT_EQUAL(size_t(1), replies.size()); + auto reply = std::dynamic_pointer_cast<api::RemoveReply>(replies[0]); CPPUNIT_ASSERT(reply.get()); - CPPUNIT_ASSERT_EQUAL(api::ReturnCode(api::ReturnCode::OK), - reply->getResult()); + CPPUNIT_ASSERT_EQUAL(api::ReturnCode(api::ReturnCode::OK), reply->getResult()); } } void VisitorManagerTest::tearDown() { - if (_top.get() != 0) { + if (_top) { + assert(_top->getNumReplies() == 0); _top->close(); _top->flush(); - _top.reset(0); + _top.reset(); } - _node.reset(0); - _messageSessionFactory.reset(0); - _manager = 0; + _node.reset(); + _messageSessionFactory.reset(); + _manager = nullptr; } TestVisitorMessageSession& VisitorManagerTest::getSession(uint32_t n) { // Wait until we have started the visitor - const std::vector<TestVisitorMessageSession*>& sessions( - _messageSessionFactory->_visitorSessions); + const std::vector<TestVisitorMessageSession*>& sessions(_messageSessionFactory->_visitorSessions); framework::defaultimplementation::RealClock clock; - framework::MilliSecTime endTime( - clock.getTimeInMillis() + framework::MilliSecTime(30 * 1000)); + framework::MilliSecTime endTime(clock.getTimeInMillis() + framework::MilliSecTime(30 * 1000)); while (true) { { vespalib::LockGuard lock(_messageSessionFactory->_accessLock); @@ -276,7 +275,7 @@ VisitorManagerTest::getSession(uint32_t n) throw vespalib::IllegalStateException( "Timed out waiting for visitor session", VESPA_STRLOC); } - FastOS_Thread::Sleep(10); + std::this_thread::sleep_for(10ms); } throw std::logic_error("unreachable"); } @@ -288,7 +287,7 @@ VisitorManagerTest::getMessagesAndReply( std::vector<document::Document::SP >& docs, std::vector<document::DocumentId>& docIds, api::ReturnCode::Result result, - documentapi::Priority::Value priority) + std::optional<Priority::Value> priority) { for (int i = 0; i < expectedCount; i++) { session.waitForMessages(i + 1); @@ -296,8 +295,10 @@ VisitorManagerTest::getMessagesAndReply( { vespalib::MonitorGuard guard(session.getMonitor()); - CPPUNIT_ASSERT_EQUAL(priority, - session.sentMessages[i]->getPriority()); + if (priority) { + CPPUNIT_ASSERT_EQUAL(*priority, + session.sentMessages[i]->getPriority()); + } switch (session.sentMessages[i]->getType()) { case documentapi::DocumentProtocol::MESSAGE_PUTDOCUMENT: @@ -340,8 +341,7 @@ VisitorManagerTest::verifyCreateVisitorReply( CPPUNIT_ASSERT_EQUAL(api::MessageType::VISITOR_CREATE_REPLY, msg->getType()); - std::shared_ptr<api::CreateVisitorReply> reply( - std::dynamic_pointer_cast<api::CreateVisitorReply>(msg)); + auto reply = std::dynamic_pointer_cast<api::CreateVisitorReply>(msg); CPPUNIT_ASSERT(reply.get()); CPPUNIT_ASSERT_EQUAL(expectedResult, reply->getResult().getResult()); @@ -410,8 +410,7 @@ VisitorManagerTest::testNormalUsage() { initializeTest(); api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", "testvis", "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->setAddress(address); cmd->setControlDestination("foo/bar"); @@ -436,8 +435,7 @@ VisitorManagerTest::testResending() { initializeTest(); api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", "testvis", "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->setAddress(address); cmd->setControlDestination("foo/bar"); @@ -486,8 +484,7 @@ VisitorManagerTest::testVisitEmptyBucket() initializeTest(); addSomeRemoves(true); api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", "testvis", "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->setAddress(address); @@ -502,8 +499,7 @@ VisitorManagerTest::testMultiBucketVisit() { initializeTest(); api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", "testvis", "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); for (uint32_t i=0; i<10; ++i) { cmd->addBucketToBeVisited(document::BucketId(16, i)); } @@ -527,8 +523,7 @@ VisitorManagerTest::testNoBuckets() { initializeTest(); api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", "testvis", "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->setAddress(address); _top->sendDown(cmd); @@ -536,15 +531,12 @@ VisitorManagerTest::testNoBuckets() // Should get one reply; a CreateVisitorReply with error since no // buckets where specified in the CreateVisitorCommand _top->waitForMessages(1, 60); - const msg_ptr_vector replies = _top->getRepliesOnce(); + const msg_ptr_vector replies = _top->getRepliesOnce(); CPPUNIT_ASSERT_EQUAL((size_t) 1, replies.size()); - std::shared_ptr<api::CreateVisitorReply> reply( - std::dynamic_pointer_cast<api::CreateVisitorReply>( - replies[0])); + auto reply = std::dynamic_pointer_cast<api::CreateVisitorReply>(replies[0]); // Verify that cast went ok => it was a CreateVisitorReply message CPPUNIT_ASSERT(reply.get()); - api::ReturnCode ret(api::ReturnCode::ILLEGAL_PARAMETERS, - "No buckets specified"); + api::ReturnCode ret(api::ReturnCode::ILLEGAL_PARAMETERS, "No buckets specified"); CPPUNIT_ASSERT_EQUAL(ret, reply->getResult()); } @@ -553,8 +545,7 @@ void VisitorManagerTest::testVisitPutsAndRemoves() initializeTest(); addSomeRemoves(); api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", "testvis", "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->setAddress(address); cmd->setVisitRemoves(); for (uint32_t i=0; i<10; ++i) { @@ -581,9 +572,7 @@ void VisitorManagerTest::testVisitWithTimeframeAndSelection() { initializeTest(); api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", "testvis", - "testdoctype1.headerval < 2")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", "testdoctype1.headerval < 2"); cmd->setFromTime(3); cmd->setToTime(8); for (uint32_t i=0; i<10; ++i) { @@ -613,9 +602,8 @@ void VisitorManagerTest::testVisitWithTimeframeAndBogusSelection() { initializeTest(); api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", "testvis", - "DocType(testdoctype1---///---) XXX BAD Field(headerval) < 2")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", + "DocType(testdoctype1---///---) XXX BAD Field(headerval) < 2"); cmd->setFromTime(3); cmd->setToTime(8); for (uint32_t i=0; i<10; ++i) { @@ -628,11 +616,9 @@ void VisitorManagerTest::testVisitWithTimeframeAndBogusSelection() const msg_ptr_vector replies = _top->getRepliesOnce(); CPPUNIT_ASSERT_EQUAL((size_t) 1, replies.size()); - api::StorageReply* reply = dynamic_cast<api::StorageReply*>( - replies.front().get()); + auto* reply = dynamic_cast<api::StorageReply*>(replies.front().get()); CPPUNIT_ASSERT(reply); - CPPUNIT_ASSERT_EQUAL(api::ReturnCode::ILLEGAL_PARAMETERS, - reply->getResult().getResult()); + CPPUNIT_ASSERT_EQUAL(api::ReturnCode::ILLEGAL_PARAMETERS, reply->getResult().getResult()); } void @@ -641,8 +627,7 @@ VisitorManagerTest::testVisitorCallbacks() initializeTest(); std::ostringstream replydata; api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "TestVisitor", "testvis", "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "TestVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->addBucketToBeVisited(document::BucketId(16, 5)); cmd->setAddress(address); @@ -659,8 +644,8 @@ VisitorManagerTest::testVisitorCallbacks() CPPUNIT_ASSERT_EQUAL((uint32_t)documentapi::DocumentProtocol::MESSAGE_MAPVISITOR, session.sentMessages[i]->getType()); - documentapi::MapVisitorMessage* mapvisitormsg( - static_cast<documentapi::MapVisitorMessage*>(session.sentMessages[i].get())); + auto* mapvisitormsg = dynamic_cast<documentapi::MapVisitorMessage*>(session.sentMessages[i].get()); + CPPUNIT_ASSERT(mapvisitormsg != nullptr); replydata << mapvisitormsg->getData().get("msg"); @@ -690,8 +675,7 @@ VisitorManagerTest::testVisitorCleanup() for (uint32_t i=0; i<10; ++i) { std::ostringstream ost; ost << "testvis" << i; - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "InvalidVisitor", ost.str(), "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "InvalidVisitor", ost.str(), ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->setAddress(address); cmd->setQueueTimeout(0); @@ -703,28 +687,27 @@ VisitorManagerTest::testVisitorCleanup() for (uint32_t i=0; i<10; ++i) { std::ostringstream ost; ost << "testvis" << (i + 10); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", ost.str(), "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", ost.str(), ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->setAddress(address); cmd->setQueueTimeout(0); _top->sendDown(cmd); } - - // Should get 14 immediate replies - 10 failures and 4 busy + // Should get 16 immediate replies - 10 failures and 6 busy { - _top->waitForMessages(14, 60); + const int expected_total = 16; + _top->waitForMessages(expected_total, 60); const msg_ptr_vector replies = _top->getRepliesOnce(); + CPPUNIT_ASSERT_EQUAL(size_t(expected_total), replies.size()); int failures = 0; int busy = 0; - for (uint32_t i=0; i< 14; ++i) { + for (uint32_t i=0; i< expected_total; ++i) { std::shared_ptr<api::StorageMessage> msg(replies[i]); CPPUNIT_ASSERT_EQUAL(api::MessageType::VISITOR_CREATE_REPLY, msg->getType()); - std::shared_ptr<api::CreateVisitorReply> reply( - std::dynamic_pointer_cast<api::CreateVisitorReply>(msg)); + auto reply = std::dynamic_pointer_cast<api::CreateVisitorReply>(msg); CPPUNIT_ASSERT(reply.get()); if (i < 10) { @@ -741,9 +724,11 @@ VisitorManagerTest::testVisitorCleanup() } CPPUNIT_ASSERT_EQUAL(10, failures); - CPPUNIT_ASSERT_EQUAL(4, busy); + CPPUNIT_ASSERT_EQUAL(expected_total - 10, busy); } + // 4 pending + // Finish a visitor std::vector<document::Document::SP > docs; std::vector<document::DocumentId> docIds; @@ -753,22 +738,25 @@ VisitorManagerTest::testVisitorCleanup() // Should get a reply for the visitor. verifyCreateVisitorReply(api::ReturnCode::OK); + // 3 pending + // Fail a visitor getMessagesAndReply(1, getSession(1), docs, docIds, api::ReturnCode::INTERNAL_FAILURE); // Should get a reply for the visitor. verifyCreateVisitorReply(api::ReturnCode::INTERNAL_FAILURE); - while (_manager->getActiveVisitorCount() > 2) { - FastOS_Thread::Sleep(10); + // Wait until there are 2 pending. Visitor threads might not have completed + // cleanup of existing visitors yet. + while (_manager->getActiveVisitorCount() != 2) { + std::this_thread::sleep_for(10ms); } // Start a bunch of more visitors for (uint32_t i=0; i<10; ++i) { std::ostringstream ost; ost << "testvis" << (i + 24); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", ost.str(), "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", ost.str(), ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->setAddress(address); cmd->setQueueTimeout(0); @@ -778,17 +766,21 @@ VisitorManagerTest::testVisitorCleanup() // Should now get 8 busy. _top->waitForMessages(8, 60); const msg_ptr_vector replies = _top->getRepliesOnce(); - CPPUNIT_ASSERT_EQUAL(8, (int)replies.size()); + CPPUNIT_ASSERT_EQUAL(size_t(8), replies.size()); for (uint32_t i=0; i< replies.size(); ++i) { std::shared_ptr<api::StorageMessage> msg(replies[i]); CPPUNIT_ASSERT_EQUAL(api::MessageType::VISITOR_CREATE_REPLY, msg->getType()); - std::shared_ptr<api::CreateVisitorReply> reply( - std::dynamic_pointer_cast<api::CreateVisitorReply>(msg)); + auto reply = std::dynamic_pointer_cast<api::CreateVisitorReply>(msg); CPPUNIT_ASSERT(reply.get()); CPPUNIT_ASSERT_EQUAL(api::ReturnCode::BUSY, reply->getResult().getResult()); } + + for (uint32_t i = 0; i < 4; ++i) { + getMessagesAndReply(1, getSession(i + 2), docs, docIds); + verifyCreateVisitorReply(api::ReturnCode::OK); + } } void @@ -798,18 +790,13 @@ VisitorManagerTest::testAbortOnFailedVisitorInfo() api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); { - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", "testvis", "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->setAddress(address); cmd->setQueueTimeout(0); _top->sendDown(cmd); } - uint32_t visitorRepliesReceived = 0; - uint32_t oki = 0; - uint32_t failed = 0; - std::vector<document::Document::SP > docs; std::vector<document::DocumentId> docIds; @@ -823,37 +810,13 @@ VisitorManagerTest::testAbortOnFailedVisitorInfo() mbus::Reply::UP reply = cmd->createReply(); - CPPUNIT_ASSERT_EQUAL((uint32_t)documentapi::DocumentProtocol::MESSAGE_VISITORINFO, session.sentMessages[1]->getType()); + CPPUNIT_ASSERT_EQUAL(uint32_t(documentapi::DocumentProtocol::MESSAGE_VISITORINFO), session.sentMessages[1]->getType()); reply->swapState(*session.sentMessages[1]); reply->setMessage(mbus::Message::UP(session.sentMessages[1].release())); reply->addError(mbus::Error(api::ReturnCode::NOT_CONNECTED, "Me no ready")); session.reply(std::move(reply)); } - - _top->waitForMessages(1, 60); - const msg_ptr_vector replies = _top->getRepliesOnce(); - for (uint32_t i=0; i< replies.size(); ++i) { - std::shared_ptr<api::StorageMessage> msg(replies[i]); - if (msg->getType() == api::MessageType::VISITOR_CREATE_REPLY) - { - ++visitorRepliesReceived; - std::shared_ptr<api::CreateVisitorReply> reply( - std::dynamic_pointer_cast<api::CreateVisitorReply>(msg)); - CPPUNIT_ASSERT(reply.get()); - if (reply->getResult().success()) { - ++oki; - std::cerr << "\n" << reply->toString(true) << "\n"; - } else { - ++failed; - } - } - } - - std::ostringstream errmsg; - errmsg << "oki " << oki << ", failed " << failed; - - CPPUNIT_ASSERT_EQUAL_MSG(errmsg.str(), 0u, oki); - CPPUNIT_ASSERT_EQUAL_MSG(errmsg.str(), 1u, failed); + verifyCreateVisitorReply(api::ReturnCode::NOT_CONNECTED); } void @@ -863,10 +826,8 @@ VisitorManagerTest::testAbortOnFieldPathError() api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); // Use bogus field path to force error to happen - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", - "testvis", - "testdoctype1.headerval{bogus} == 1234")); + auto cmd = std::make_shared<api::CreateVisitorCommand>( + makeBucketSpace(), "DumpVisitor", "testvis", "testdoctype1.headerval{bogus} == 1234"); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->setAddress(address); cmd->setQueueTimeout(0); @@ -885,8 +846,7 @@ VisitorManagerTest::testVisitorQueueTimeout() { vespalib::MonitorGuard guard(_manager->getThread(0).getQueueMonitor()); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", "testvis", "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->setAddress(address); cmd->setQueueTimeout(1); @@ -897,18 +857,13 @@ VisitorManagerTest::testVisitorQueueTimeout() } // Don't answer any messages. Make sure we timeout anyways. - uint32_t visitorRepliesReceived = 0; - _top->waitForMessages(1, 60); const msg_ptr_vector replies = _top->getRepliesOnce(); std::shared_ptr<api::StorageMessage> msg(replies[0]); CPPUNIT_ASSERT_EQUAL(api::MessageType::VISITOR_CREATE_REPLY, msg->getType()); - ++visitorRepliesReceived; - std::shared_ptr<api::CreateVisitorReply> reply( - std::dynamic_pointer_cast<api::CreateVisitorReply>(msg)); - CPPUNIT_ASSERT_EQUAL(api::ReturnCode(api::ReturnCode::BUSY, - "Visitor timed out in visitor queue"), + auto reply = std::dynamic_pointer_cast<api::CreateVisitorReply>(msg); + CPPUNIT_ASSERT_EQUAL(api::ReturnCode(api::ReturnCode::BUSY, "Visitor timed out in visitor queue"), reply->getResult()); } @@ -918,8 +873,7 @@ VisitorManagerTest::testVisitorProcessingTimeout() initializeTest(); api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", "testvis", "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->setAddress(address); cmd->setQueueTimeout(0); @@ -932,19 +886,7 @@ VisitorManagerTest::testVisitorProcessingTimeout() _node->getClock().addSecondsToTime(1000); - // Don't answer any messages. Make sure we timeout anyways. - uint32_t visitorRepliesReceived = 0; - - _top->waitForMessages(1, 60); - const msg_ptr_vector replies = _top->getRepliesOnce(); - std::shared_ptr<api::StorageMessage> msg(replies[0]); - - CPPUNIT_ASSERT_EQUAL(api::MessageType::VISITOR_CREATE_REPLY, msg->getType()); - ++visitorRepliesReceived; - std::shared_ptr<api::CreateVisitorReply> reply( - std::dynamic_pointer_cast<api::CreateVisitorReply>(msg)); - CPPUNIT_ASSERT_EQUAL(api::ReturnCode::ABORTED, - reply->getResult().getResult()); + verifyCreateVisitorReply(api::ReturnCode::ABORTED); } namespace { @@ -955,8 +897,7 @@ namespace { std::ostringstream ost; ost << "testvis" << ++nextVisitor; api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::shared_ptr<api::CreateVisitorCommand> cmd( - new api::CreateVisitorCommand(makeBucketSpace(), "DumpVisitor", ost.str(), "")); + auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", ost.str(), ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->setAddress(address); cmd->setQueueTimeout(timeout); @@ -1002,14 +943,25 @@ VisitorManagerTest::testPrioritizedVisitorQueing() // Finish the first visitor std::vector<document::Document::SP > docs; std::vector<document::DocumentId> docIds; - getMessagesAndReply(1, getSession(0), docs, docIds, api::ReturnCode::OK, - documentapi::Priority::PRI_HIGHEST); + getMessagesAndReply(1, getSession(0), docs, docIds, api::ReturnCode::OK, Priority::PRI_HIGHEST); verifyCreateVisitorReply(api::ReturnCode::OK); // We should now start the highest priority visitor. - getMessagesAndReply(1, getSession(4), docs, docIds, api::ReturnCode::OK, - documentapi::Priority::PRI_VERY_HIGH); + getMessagesAndReply(1, getSession(4), docs, docIds, api::ReturnCode::OK, Priority::PRI_VERY_HIGH); CPPUNIT_ASSERT_EQUAL(ids[9], verifyCreateVisitorReply(api::ReturnCode::OK)); + + // 3 pending, 3 in queue. Clean them up + std::vector<uint32_t> pending_sessions = {1, 2, 3, 5, 6, 7}; + for (auto session : pending_sessions) { + finishAndWaitForVisitorSessionCompletion(session); + } +} + +void VisitorManagerTest::finishAndWaitForVisitorSessionCompletion(uint32_t sessionIndex) { + std::vector<document::Document::SP > docs; + std::vector<document::DocumentId> docIds; + getMessagesAndReply(1, getSession(sessionIndex), docs, docIds, api::ReturnCode::OK, std::optional<Priority::Value>()); + verifyCreateVisitorReply(api::ReturnCode::OK); } void @@ -1135,6 +1087,9 @@ VisitorManagerTest::testVisitorQueingZeroQueueSize() { sendCreateVisitor(1000, *_top, 100 - i); verifyCreateVisitorReply(api::ReturnCode::BUSY); } + for (uint32_t session = 0; session < 4; ++session) { + finishAndWaitForVisitorSessionCompletion(session); + } } void @@ -1148,8 +1103,10 @@ VisitorManagerTest::testStatusPage() { sendCreateVisitor(1000000, *_top, 1); sendCreateVisitor(1000000, *_top, 128); - TestVisitorMessageSession& session = getSession(0); - session.waitForMessages(1); + { + TestVisitorMessageSession& session = getSession(0); + session.waitForMessages(1); + } std::ostringstream ss; static_cast<framework::HtmlStatusReporter&>(*_manager).reportHtmlStatus(ss, path); @@ -1162,6 +1119,10 @@ VisitorManagerTest::testStatusPage() { CPPUNIT_ASSERT(str.find("Visitor thread 0") != std::string::npos); CPPUNIT_ASSERT(str.find("Disconnected visitor timeout") != std::string::npos); // verbose per thread CPPUNIT_ASSERT(str.find("Message #1 <b>putdocumentmessage</b>") != std::string::npos); // 1 active + + for (uint32_t session = 0; session < 2 ; ++session){ + finishAndWaitForVisitorSessionCompletion(session); + } } } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/EntityBindingsMapper.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/EntityBindingsMapper.java index 12389712976..1504119d9cc 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/EntityBindingsMapper.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/EntityBindingsMapper.java @@ -12,8 +12,6 @@ import com.yahoo.vespa.athenz.utils.AthenzIdentities; import java.util.Base64; -import static com.yahoo.vespa.athenz.identityprovider.api.VespaUniqueInstanceId.*; - /** * Utility class for mapping objects model types and their Jackson binding versions. * @@ -35,7 +33,7 @@ public class EntityBindingsMapper { public static VespaUniqueInstanceId toVespaUniqueInstanceId(VespaUniqueInstanceIdEntity entity) { return new VespaUniqueInstanceId( - entity.clusterIndex, entity.clusterId, entity.instance, entity.application, entity.tenant, entity.region, entity.environment, entity.type != null ? IdentityType.fromId(entity.type) : null); // TODO Remove support for legacy representation without type + entity.clusterIndex, entity.clusterId, entity.instance, entity.application, entity.tenant, entity.region, entity.environment); } public static IdentityDocument toIdentityDocument(IdentityDocumentEntity entity) { @@ -52,22 +50,17 @@ public class EntityBindingsMapper { toIdentityDocument(entity.identityDocument), entity.signature, entity.signingKeyVersion, - fromDottedString(entity.providerUniqueId), + VespaUniqueInstanceId.fromDottedString(entity.providerUniqueId), entity.dnsSuffix, (AthenzService) AthenzIdentities.from(entity.providerService), entity.ztsEndpoint, - entity.documentVersion, - entity.configServerHostname, - entity.instanceHostname, - entity.createdAt, - entity.ipAddresses, - entity.identityType != null ? IdentityType.fromId(entity.identityType) : null); // TODO Remove support for legacy representation without type + entity.documentVersion); } public static VespaUniqueInstanceIdEntity toVespaUniqueInstanceIdEntity(VespaUniqueInstanceId model) { return new VespaUniqueInstanceIdEntity( model.tenant(), model.application(), model.environment(), model.region(), - model.instance(), model.clusterId(), model.clusterIndex(), model.type() != null ? model.type().id() : null); // TODO Remove support for legacy representation without type + model.instance(), model.clusterId(), model.clusterIndex()); } public static IdentityDocumentEntity toIdentityDocumentEntity(IdentityDocument model) { @@ -91,12 +84,7 @@ public class EntityBindingsMapper { model.dnsSuffix(), model.providerService().getFullName(), model.ztsEndpoint(), - model.documentVersion(), - model.configServerHostname(), - model.instanceHostname(), - model.createdAt(), - model.ipAddresses(), - model.identityType() != null ? model.identityType().id() : null); // TODO Remove support for legacy representation without type + model.documentVersion()); } catch (JsonProcessingException e) { throw new RuntimeException(e); } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/IdentityDocument.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/IdentityDocument.java index 82d0a3d622c..8da2bd0a343 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/IdentityDocument.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/IdentityDocument.java @@ -8,9 +8,7 @@ import java.util.Set; * The identity document that contains the instance specific information * * @author bjorncs - * @deprecated Will soon be inlined into {@link SignedIdentityDocument} */ -@Deprecated public class IdentityDocument { private final VespaUniqueInstanceId providerUniqueId; private final String configServerHostname; diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/IdentityType.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/IdentityType.java deleted file mode 100644 index 4ca2e34a618..00000000000 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/IdentityType.java +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.athenz.identityprovider.api; - -import java.util.Arrays; - -/** - * Represents the types of identities that the configserver can provide. - * - * @author bjorncs - */ -public enum IdentityType {TENANT("tenant"), NODE("node"); - private final String id; - - IdentityType(String id) { this.id = id; } - - public String id() { return id; } - - public static IdentityType fromId(String id) { - return Arrays.stream(values()) - .filter(v -> v.id.equals(id)) - .findFirst() - .orElseThrow(() -> new IllegalArgumentException("Invalid id: " + id)); - } -} - diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/SignedIdentityDocument.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/SignedIdentityDocument.java index 60be42544c7..d184efc0221 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/SignedIdentityDocument.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/SignedIdentityDocument.java @@ -4,8 +4,6 @@ package com.yahoo.vespa.athenz.identityprovider.api; import com.yahoo.vespa.athenz.api.AthenzService; import java.net.URI; -import java.time.Instant; -import java.util.Set; /** * A signed identity document which contains a {@link IdentityDocument} @@ -24,11 +22,6 @@ public class SignedIdentityDocument { private final AthenzService providerService; private final URI ztsEndpoint; private final int documentVersion; - private final String configServerHostname; - private final String instanceHostname; - private final Instant createdAt; - private final Set<String> ipAddresses; - private final IdentityType identityType; public SignedIdentityDocument(IdentityDocument identityDocument, String signature, @@ -37,12 +30,7 @@ public class SignedIdentityDocument { String dnsSuffix, AthenzService providerService, URI ztsEndpoint, - int documentVersion, - String configServerHostname, - String instanceHostname, - Instant createdAt, - Set<String> ipAddresses, - IdentityType identityType) { + int documentVersion) { this.identityDocument = identityDocument; this.signature = signature; this.signingKeyVersion = signingKeyVersion; @@ -51,11 +39,6 @@ public class SignedIdentityDocument { this.providerService = providerService; this.ztsEndpoint = ztsEndpoint; this.documentVersion = documentVersion; - this.configServerHostname = configServerHostname; - this.instanceHostname = instanceHostname; - this.createdAt = createdAt; - this.ipAddresses = ipAddresses; - this.identityType = identityType; } public IdentityDocument identityDocument() { @@ -89,24 +72,4 @@ public class SignedIdentityDocument { public int documentVersion() { return documentVersion; } - - public String configServerHostname() { - return configServerHostname; - } - - public String instanceHostname() { - return instanceHostname; - } - - public Instant createdAt() { - return createdAt; - } - - public Set<String> ipAddresses() { - return ipAddresses; - } - - public IdentityType identityType() { - return identityType; - } } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/VespaUniqueInstanceId.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/VespaUniqueInstanceId.java index be94cc59691..5539ba53882 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/VespaUniqueInstanceId.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/VespaUniqueInstanceId.java @@ -4,8 +4,6 @@ package com.yahoo.vespa.athenz.identityprovider.api; import java.util.Objects; /** - * Represents the unique instance id as used in Vespa's integration with Athenz Copper Argos - * * @author bjorncs */ public class VespaUniqueInstanceId { @@ -17,7 +15,6 @@ public class VespaUniqueInstanceId { private final String tenant; private final String region; private final String environment; - private final IdentityType type; public VespaUniqueInstanceId(int clusterIndex, String clusterId, @@ -25,8 +22,7 @@ public class VespaUniqueInstanceId { String application, String tenant, String region, - String environment, - IdentityType type) { + String environment) { this.clusterIndex = clusterIndex; this.clusterId = clusterId; this.instance = instance; @@ -34,43 +30,21 @@ public class VespaUniqueInstanceId { this.tenant = tenant; this.region = region; this.environment = environment; - this.type = type; } - // TODO Remove support for legacy representation without type - @Deprecated - public VespaUniqueInstanceId(int clusterIndex, - String clusterId, - String instance, - String application, - String tenant, - String region, - String environment) { - this(clusterIndex, clusterId, instance, application, tenant, region, environment, null); - } - - - // TODO Remove support for legacy representation without type public static VespaUniqueInstanceId fromDottedString(String instanceId) { String[] tokens = instanceId.split("\\."); - if (tokens.length != 7 && tokens.length != 8) { + if (tokens.length != 7) { throw new IllegalArgumentException("Invalid instance id: " + instanceId); } return new VespaUniqueInstanceId( - Integer.parseInt(tokens[0]), tokens[1], tokens[2], tokens[3], tokens[4], tokens[5], tokens[6], tokens.length == 8 ? IdentityType.fromId(tokens[7]) : null); + Integer.parseInt(tokens[0]), tokens[1], tokens[2], tokens[3], tokens[4], tokens[5], tokens[6]); } - // TODO Remove support for legacy representation without type public String asDottedString() { - if (type != null) { - return String.format( - "%d.%s.%s.%s.%s.%s.%s.%s", - clusterIndex, clusterId, instance, application, tenant, region, environment, type.id()); - } else { - return String.format( - "%d.%s.%s.%s.%s.%s.%s", - clusterIndex, clusterId, instance, application, tenant, region, environment); - } + return String.format( + "%d.%s.%s.%s.%s.%s.%s", + clusterIndex, clusterId, instance, application, tenant, region, environment); } public int clusterIndex() { @@ -101,8 +75,6 @@ public class VespaUniqueInstanceId { return environment; } - public IdentityType type() { return type; } - @Override public String toString() { return "VespaUniqueInstanceId{" + @@ -113,7 +85,6 @@ public class VespaUniqueInstanceId { ", tenant='" + tenant + '\'' + ", region='" + region + '\'' + ", environment='" + environment + '\'' + - ", type=" + type + '}'; } @@ -128,12 +99,11 @@ public class VespaUniqueInstanceId { Objects.equals(application, that.application) && Objects.equals(tenant, that.tenant) && Objects.equals(region, that.region) && - Objects.equals(environment, that.environment) && - type == that.type; + Objects.equals(environment, that.environment); } @Override public int hashCode() { - return Objects.hash(clusterIndex, clusterId, instance, application, tenant, region, environment, type); + return Objects.hash(clusterIndex, clusterId, instance, application, tenant, region, environment); } } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/IdentityDocumentApi.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/IdentityDocumentApi.java index fc5392411c1..775a49349a3 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/IdentityDocumentApi.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/IdentityDocumentApi.java @@ -5,6 +5,7 @@ import javax.ws.rs.GET; import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.core.MediaType; /** @@ -15,6 +16,11 @@ public interface IdentityDocumentApi { @GET @Produces(MediaType.APPLICATION_JSON) + @Deprecated + SignedIdentityDocumentEntity getIdentityDocument(@QueryParam("hostname") String hostname); + + @GET + @Produces(MediaType.APPLICATION_JSON) @Path("/node/{host}") SignedIdentityDocumentEntity getNodeIdentityDocument(@PathParam("host") String host); diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/IdentityDocumentEntity.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/IdentityDocumentEntity.java index b4b2e82ab0e..58a4f1e24bf 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/IdentityDocumentEntity.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/IdentityDocumentEntity.java @@ -10,10 +10,8 @@ import java.util.Set; /** * @author bjorncs - * @deprecated Will soon be inlined into {@link SignedIdentityDocumentEntity} */ @JsonIgnoreProperties(ignoreUnknown = true) -@Deprecated public class IdentityDocumentEntity { @JsonProperty("provider-unique-id") diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/SignedIdentityDocumentEntity.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/SignedIdentityDocumentEntity.java index aa514b3caf3..e397b81ef9e 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/SignedIdentityDocumentEntity.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/SignedIdentityDocumentEntity.java @@ -11,10 +11,8 @@ import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import java.io.IOException; import java.io.UncheckedIOException; import java.net.URI; -import java.time.Instant; import java.util.Base64; import java.util.Objects; -import java.util.Set; /** * @author bjorncs @@ -33,11 +31,6 @@ public class SignedIdentityDocumentEntity { @JsonProperty("provider-service") public final String providerService; @JsonProperty("zts-endpoint") public final URI ztsEndpoint; @JsonProperty("document-version") public final int documentVersion; - @JsonProperty("configserver-hostname") public final String configServerHostname; - @JsonProperty("instance-hostname") public final String instanceHostname; - @JsonProperty("created-at") public final Instant createdAt; - @JsonProperty("ip-addresses") public final Set<String> ipAddresses; - @JsonProperty("identity-type") public final String identityType; @JsonCreator public SignedIdentityDocumentEntity(@JsonProperty("identity-document") String rawIdentityDocument, @@ -47,12 +40,7 @@ public class SignedIdentityDocumentEntity { @JsonProperty("dns-suffix") String dnsSuffix, @JsonProperty("provider-service") String providerService, @JsonProperty("zts-endpoint") URI ztsEndpoint, - @JsonProperty("document-version") int documentVersion, - @JsonProperty("configserver-hostname") String configServerHostname, - @JsonProperty("instance-hostname") String instanceHostname, - @JsonProperty("created-at") Instant createdAt, - @JsonProperty("ip-addresses") Set<String> ipAddresses, - @JsonProperty("identity-type") String identityType) { + @JsonProperty("document-version") int documentVersion) { this.rawIdentityDocument = rawIdentityDocument; this.identityDocument = parseIdentityDocument(rawIdentityDocument); this.signature = signature; @@ -62,11 +50,6 @@ public class SignedIdentityDocumentEntity { this.providerService = providerService; this.ztsEndpoint = ztsEndpoint; this.documentVersion = documentVersion; - this.configServerHostname = configServerHostname; - this.instanceHostname = instanceHostname; - this.createdAt = createdAt; - this.ipAddresses = ipAddresses; - this.identityType = identityType; } private static IdentityDocumentEntity parseIdentityDocument(String rawIdentityDocument) { @@ -90,16 +73,7 @@ public class SignedIdentityDocumentEntity { ", identityDocument=" + identityDocument + ", signature='" + signature + '\'' + ", signingKeyVersion=" + signingKeyVersion + - ", providerUniqueId='" + providerUniqueId + '\'' + - ", dnsSuffix='" + dnsSuffix + '\'' + - ", providerService='" + providerService + '\'' + - ", ztsEndpoint=" + ztsEndpoint + ", documentVersion=" + documentVersion + - ", configServerHostname='" + configServerHostname + '\'' + - ", instanceHostname='" + instanceHostname + '\'' + - ", createdAt=" + createdAt + - ", ipAddresses=" + ipAddresses + - ", identityType=" + identityType + '}'; } @@ -112,20 +86,11 @@ public class SignedIdentityDocumentEntity { documentVersion == that.documentVersion && Objects.equals(rawIdentityDocument, that.rawIdentityDocument) && Objects.equals(identityDocument, that.identityDocument) && - Objects.equals(signature, that.signature) && - Objects.equals(providerUniqueId, that.providerUniqueId) && - Objects.equals(dnsSuffix, that.dnsSuffix) && - Objects.equals(providerService, that.providerService) && - Objects.equals(ztsEndpoint, that.ztsEndpoint) && - Objects.equals(configServerHostname, that.configServerHostname) && - Objects.equals(instanceHostname, that.instanceHostname) && - Objects.equals(createdAt, that.createdAt) && - Objects.equals(ipAddresses, that.ipAddresses) && - Objects.equals(identityType, identityType); + Objects.equals(signature, that.signature); } @Override public int hashCode() { - return Objects.hash(rawIdentityDocument, identityDocument, signature, signingKeyVersion, providerUniqueId, dnsSuffix, providerService, ztsEndpoint, documentVersion, configServerHostname, instanceHostname, createdAt, ipAddresses, identityType); + return Objects.hash(rawIdentityDocument, identityDocument, signature, signingKeyVersion, documentVersion); } } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/VespaUniqueInstanceIdEntity.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/VespaUniqueInstanceIdEntity.java index 103c087638d..3127752ac7d 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/VespaUniqueInstanceIdEntity.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/VespaUniqueInstanceIdEntity.java @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.athenz.identityprovider.api.bindings; -import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import java.util.Objects; @@ -25,18 +24,14 @@ public class VespaUniqueInstanceIdEntity { public final String clusterId; @JsonProperty("cluster-index") public final int clusterIndex; - @JsonProperty("type") - public final String type; - @JsonCreator public VespaUniqueInstanceIdEntity(@JsonProperty("tenant") String tenant, @JsonProperty("application") String application, @JsonProperty("environment") String environment, @JsonProperty("region") String region, @JsonProperty("instance") String instance, @JsonProperty("cluster-id") String clusterId, - @JsonProperty("cluster-index") int clusterIndex, - @JsonProperty("type") String type) { + @JsonProperty("cluster-index") int clusterIndex) { this.tenant = tenant; this.application = application; this.environment = environment; @@ -44,21 +39,8 @@ public class VespaUniqueInstanceIdEntity { this.instance = instance; this.clusterId = clusterId; this.clusterIndex = clusterIndex; - this.type = type; } - @Deprecated - public VespaUniqueInstanceIdEntity(String tenant, - String application, - String environment, - String region, - String instance, - String clusterId, - int clusterIndex) { - this(tenant, application, environment, region, instance, clusterId, clusterIndex, null); - } - - @Override public String toString() { return "VespaUniqueInstanceIdEntity{" + @@ -69,7 +51,6 @@ public class VespaUniqueInstanceIdEntity { ", instance='" + instance + '\'' + ", clusterId='" + clusterId + '\'' + ", clusterIndex=" + clusterIndex + - ", type='" + type + '\'' + '}'; } @@ -84,12 +65,11 @@ public class VespaUniqueInstanceIdEntity { Objects.equals(environment, that.environment) && Objects.equals(region, that.region) && Objects.equals(instance, that.instance) && - Objects.equals(clusterId, that.clusterId) && - Objects.equals(type, that.type); + Objects.equals(clusterId, that.clusterId); } @Override public int hashCode() { - return Objects.hash(tenant, application, environment, region, instance, clusterId, clusterIndex, type); + return Objects.hash(tenant, application, environment, region, instance, clusterId, clusterIndex); } } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java index e8ef2d9f97e..96e93ca419d 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.athenz.identityprovider.client; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import com.yahoo.container.core.identity.IdentityConfig; import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; @@ -29,7 +28,7 @@ import static com.yahoo.vespa.athenz.tls.KeyStoreType.JKS; */ class AthenzCredentialsService { - private static final ObjectMapper mapper = new ObjectMapper().registerModule(new JavaTimeModule()); + private static final ObjectMapper mapper = new ObjectMapper(); private final IdentityConfig identityConfig; private final IdentityDocumentClient identityDocumentClient; diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/DefaultIdentityDocumentClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/DefaultIdentityDocumentClient.java index f92956f7961..90d1312c9f9 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/DefaultIdentityDocumentClient.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/DefaultIdentityDocumentClient.java @@ -2,11 +2,14 @@ package com.yahoo.vespa.athenz.identityprovider.client; import com.fasterxml.jackson.databind.ObjectMapper; +import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider; import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; import com.yahoo.vespa.athenz.identityprovider.api.IdentityDocumentClient; import com.yahoo.vespa.athenz.identityprovider.api.SignedIdentityDocument; +import com.yahoo.vespa.athenz.identityprovider.api.VespaUniqueInstanceId; import com.yahoo.vespa.athenz.identityprovider.api.bindings.SignedIdentityDocumentEntity; +import com.yahoo.vespa.athenz.utils.AthenzIdentities; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.methods.RequestBuilder; @@ -79,7 +82,15 @@ public class DefaultIdentityDocumentClient implements IdentityDocumentClient { String responseContent = EntityUtils.toString(response.getEntity()); if (HttpStatus.isSuccess(response.getStatusLine().getStatusCode())) { SignedIdentityDocumentEntity entity = objectMapper.readValue(responseContent, SignedIdentityDocumentEntity.class); - return EntityBindingsMapper.toSignedIdentityDocument(entity); + return new SignedIdentityDocument( + EntityBindingsMapper.toIdentityDocument(entity.identityDocument), + entity.signature, + entity.signingKeyVersion, + VespaUniqueInstanceId.fromDottedString(entity.providerUniqueId), + entity.dnsSuffix, + (AthenzService) AthenzIdentities.from(entity.providerService), + entity.ztsEndpoint, + entity.documentVersion); } else { throw new RuntimeException( String.format( diff --git a/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/api/VespaUniqueInstanceIdTest.java b/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/api/VespaUniqueInstanceIdTest.java index 86b6c566987..8c4e4c1262d 100644 --- a/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/api/VespaUniqueInstanceIdTest.java +++ b/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/api/VespaUniqueInstanceIdTest.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.athenz.identityprovider.api; import org.junit.Test; -import static com.yahoo.vespa.athenz.identityprovider.api.IdentityType.*; import static org.junit.Assert.*; /** @@ -13,18 +12,6 @@ public class VespaUniqueInstanceIdTest { @Test public void can_serialize_to_and_deserialize_from_string() { VespaUniqueInstanceId id = - new VespaUniqueInstanceId(1, "cluster-id", "instance", "application", "tenant", "region", "environment", TENANT); - String stringRepresentation = id.asDottedString(); - String expectedStringRepresentation = "1.cluster-id.instance.application.tenant.region.environment.tenant"; - assertEquals(expectedStringRepresentation, stringRepresentation); - VespaUniqueInstanceId deserializedId = VespaUniqueInstanceId.fromDottedString(stringRepresentation); - assertEquals(id, deserializedId); - } - - // TODO Remove support for legacy representation without type - @Test - public void supports_legacy_representation_without_type() { - VespaUniqueInstanceId id = new VespaUniqueInstanceId(1, "cluster-id", "instance", "application", "tenant", "region", "environment"); String stringRepresentation = id.asDottedString(); String expectedStringRepresentation = "1.cluster-id.instance.application.tenant.region.environment"; diff --git a/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImplTest.java b/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImplTest.java index 7ad465a7d80..2e9b29f5327 100644 --- a/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImplTest.java +++ b/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImplTest.java @@ -11,7 +11,6 @@ import com.yahoo.test.ManualClock; import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; import com.yahoo.vespa.athenz.identityprovider.api.IdentityDocument; -import com.yahoo.vespa.athenz.identityprovider.api.IdentityType; import com.yahoo.vespa.athenz.identityprovider.api.SignedIdentityDocument; import com.yahoo.vespa.athenz.identityprovider.api.VespaUniqueInstanceId; import com.yahoo.vespa.athenz.tls.KeyStoreBuilder; @@ -133,7 +132,7 @@ public class AthenzIdentityProviderImplTest { } private static String getIdentityDocument() throws JsonProcessingException { - VespaUniqueInstanceId instanceId = new VespaUniqueInstanceId(0, "default", "default", "application", "tenant", "us-north-1", "dev", IdentityType.TENANT); + VespaUniqueInstanceId instanceId = new VespaUniqueInstanceId(0, "default", "default", "application", "tenant", "us-north-1", "dev"); SignedIdentityDocument signedIdentityDocument = new SignedIdentityDocument( new IdentityDocument(instanceId, "localhost", "x.y.com", Instant.EPOCH, Collections.emptySet()), "dummysignature", @@ -142,12 +141,7 @@ public class AthenzIdentityProviderImplTest { "dev-us-north-1.vespa.cloud", new AthenzService("vespa.vespa.provider_dev_us-north-1"), URI.create("https://zts:4443/zts/v1"), - 1, - "localhost", - "x.y.com", - Instant.EPOCH, - Collections.emptySet(), - IdentityType.TENANT); + 1); return new ObjectMapper().registerModule(new JavaTimeModule()) .writeValueAsString(EntityBindingsMapper.toSignedIdentityDocumentEntity(signedIdentityDocument)); diff --git a/vespalib/src/apps/vespa-detect-hostname/detect_hostname.cpp b/vespalib/src/apps/vespa-detect-hostname/detect_hostname.cpp index 057d45e8ef5..5c84790a669 100644 --- a/vespalib/src/apps/vespa-detect-hostname/detect_hostname.cpp +++ b/vespalib/src/apps/vespa-detect-hostname/detect_hostname.cpp @@ -4,6 +4,7 @@ #include <stdlib.h> #include <vespa/vespalib/net/socket_address.h> #include <vespa/vespalib/stllike/string.h> +#include <vespa/vespalib/util/stringfmt.h> #include <set> using vespalib::SocketAddress; @@ -22,14 +23,17 @@ vespalib::string get_hostname() { return SocketAddress::normalize(&result[0]); } -bool check(const vespalib::string &name, const std::set<vespalib::string> &ip_set) { +bool check(const vespalib::string &name, const std::set<vespalib::string> &ip_set, vespalib::string &error_msg) { auto addr_list = SocketAddress::resolve(80, name.c_str()); if (addr_list.empty()) { + error_msg = vespalib::make_string("hostname '%s' could not be resolved", name.c_str()); return false; } for (const SocketAddress &addr: addr_list) { vespalib::string ip_addr = addr.ip_address(); if (ip_set.count(ip_addr) == 0) { + error_msg = vespalib::make_string("hostname '%s' resolves to ip address not owned by this host (%s)", + name.c_str(), ip_addr.c_str()); return false; } } @@ -38,14 +42,21 @@ bool check(const vespalib::string &name, const std::set<vespalib::string> &ip_se int main(int, char **) { auto my_ip_set = make_ip_set(); - std::vector<vespalib::string> list({get_hostname(), "localhost", "127.0.0.1", "::1"}); - for (const vespalib::string &name: list) { - if (check(name, my_ip_set)) { - fprintf(stdout, "%s\n", name.c_str()); - return 0; - } + vespalib::string my_hostname = get_hostname(); + vespalib::string my_hostname_error; + vespalib::string localhost = "localhost"; + vespalib::string localhost_error; + if (check(my_hostname, my_ip_set, my_hostname_error)) { + fprintf(stdout, "%s\n", my_hostname.c_str()); + } else if (check(localhost, my_ip_set, localhost_error)) { + fprintf(stdout, "%s\n", localhost.c_str()); + } else { + fprintf(stderr, "FATAL: hostname detection failed\n"); + fprintf(stderr, " INFO: canonical hostname (from gethostname/getaddrinfo): %s\n", my_hostname.c_str()); + fprintf(stderr, " ERROR: %s\n", my_hostname_error.c_str()); + fprintf(stderr, " INFO: falling back to local hostname: %s\n", localhost.c_str()); + fprintf(stderr, " ERROR: %s\n", localhost_error.c_str()); + return 1; } - fprintf(stderr, "FATAL: hostname detection failed\n"); - // XXX we should explain why it failed - return 1; + return 0; } diff --git a/vespalib/src/apps/vespa-validate-hostname/validate_hostname.cpp b/vespalib/src/apps/vespa-validate-hostname/validate_hostname.cpp index c97927884f8..88c2d07dd29 100644 --- a/vespalib/src/apps/vespa-validate-hostname/validate_hostname.cpp +++ b/vespalib/src/apps/vespa-validate-hostname/validate_hostname.cpp @@ -25,19 +25,6 @@ vespalib::string normalize(const vespalib::string &hostname) { return canon_name; } -void check_reverse(const vespalib::string &hostname, const SocketAddress &addr) { - std::set<vespalib::string> seen({hostname}); - vespalib::string reverse = addr.reverse_lookup(); - for (size_t i = 0; !reverse.empty() && (i < 10); ++i) { - if (seen.count(reverse) == 0) { - seen.insert(reverse); - fprintf(stderr, "warning: hostname validation: found conflicting reverse lookup: '%s' -> %s -> '%s'\n", - hostname.c_str(), addr.ip_address().c_str(), reverse.c_str()); - } - reverse = addr.reverse_lookup(); - } -} - int usage(const char *self) { fprintf(stderr, "usage: %s <hostname>\n", self); return 1; @@ -62,8 +49,6 @@ int main(int argc, char **argv) { valid = false; fprintf(stderr, "FATAL: hostname validation failed: '%s' resolves to ip address not owned by this host (%s)\n", hostname.c_str(), ip_addr.c_str()); - } else { - check_reverse(hostname, addr); } } return valid ? 0 : 1; diff --git a/vespalib/src/tests/guard/guard_test.cpp b/vespalib/src/tests/guard/guard_test.cpp index a9d5d5f894c..9889f466edb 100644 --- a/vespalib/src/tests/guard/guard_test.cpp +++ b/vespalib/src/tests/guard/guard_test.cpp @@ -35,7 +35,8 @@ Test::testFilePointer() FilePointer file(fopen("filept.txt", "r")); EXPECT_TRUE(file.valid()); char tmp[128]; - fgets(tmp, sizeof(tmp), file); + char *fgetsres = fgets(tmp, sizeof(tmp), file); + ASSERT_EQUAL(tmp, fgetsres); EXPECT_TRUE(strcmp(tmp, "Hello") == 0); } { @@ -57,7 +58,8 @@ Test::testFilePointer() file.reset(fopen("filept.txt", "r")); EXPECT_TRUE(file.valid()); char tmp[128]; - fgets(tmp, sizeof(tmp), file.fp()); + char *fgetsres = fgets(tmp, sizeof(tmp), file.fp()); + ASSERT_EQUAL(tmp, fgetsres); EXPECT_TRUE(strcmp(tmp, "World") == 0); FILE *ref = file.fp(); diff --git a/vespalib/src/tests/testapp-debug/testapp-debug.cpp b/vespalib/src/tests/testapp-debug/testapp-debug.cpp index 8c75a104cd8..0083200ac51 100644 --- a/vespalib/src/tests/testapp-debug/testapp-debug.cpp +++ b/vespalib/src/tests/testapp-debug/testapp-debug.cpp @@ -4,8 +4,12 @@ using namespace vespalib; TEST_MAIN() { - system("./vespalib_debug_test_app"); - system("diff lhs.out rhs.out > diff.out"); + int status = system("./vespalib_debug_test_app"); + ASSERT_FALSE(WIFSIGNALED(status)); + EXPECT_NOT_EQUAL(0, WEXITSTATUS(status)); + status = system("diff lhs.out rhs.out > diff.out"); + ASSERT_FALSE(WIFSIGNALED(status)); + EXPECT_NOT_EQUAL(0, WEXITSTATUS(status)); std::string diff_cmd("diff diff.out "); diff --git a/vespalib/src/tests/testapp-state/testapp-state.cpp b/vespalib/src/tests/testapp-state/testapp-state.cpp index eda67a15d60..683476a5525 100644 --- a/vespalib/src/tests/testapp-state/testapp-state.cpp +++ b/vespalib/src/tests/testapp-state/testapp-state.cpp @@ -4,8 +4,12 @@ using namespace vespalib; TEST_MAIN() { - system("./vespalib_state_test_app > out.txt 2>&1 out.txt"); - system("cat out.txt | grep STATE | sed 's/([^)].*\\//(/' > actual.txt"); + int status = system("./vespalib_state_test_app > out.txt 2>&1 out.txt"); + ASSERT_FALSE(WIFSIGNALED(status)); + EXPECT_NOT_EQUAL(0, WEXITSTATUS(status)); + status = system("cat out.txt | grep STATE | sed 's/([^)].*\\//(/' > actual.txt"); + ASSERT_FALSE(WIFSIGNALED(status)); + EXPECT_EQUAL(0, WEXITSTATUS(status)); std::string diff_cmd("diff -u actual.txt "); diff_cmd += TEST_PATH("expect.txt"); diff --git a/vespalib/src/vespa/vespalib/net/selector.cpp b/vespalib/src/vespa/vespalib/net/selector.cpp index 6fad2e71444..e59638b6144 100644 --- a/vespalib/src/vespa/vespalib/net/selector.cpp +++ b/vespalib/src/vespa/vespalib/net/selector.cpp @@ -45,14 +45,14 @@ void WakeupPipe::write_token() { char token = 'T'; - write(_pipe[1], &token, 1); + [[maybe_unused]] ssize_t res = write(_pipe[1], &token, 1); } void WakeupPipe::read_tokens() { char token_trash[128]; - read(_pipe[0], token_trash, sizeof(token_trash)); + [[maybe_unused]] ssize_t res = read(_pipe[0], token_trash, sizeof(token_trash)); } //----------------------------------------------------------------------------- diff --git a/vespamalloc/src/vespamalloc/CMakeLists.txt b/vespamalloc/src/vespamalloc/CMakeLists.txt index d43d779217d..ac7fd300828 100644 --- a/vespamalloc/src/vespamalloc/CMakeLists.txt +++ b/vespamalloc/src/vespamalloc/CMakeLists.txt @@ -5,6 +5,7 @@ vespa_add_library(vespamalloc $<TARGET_OBJECTS:vespamalloc_util> INSTALL lib64/vespa/malloc DEPENDS + atomic dl ) vespa_add_library(vespamallocd @@ -13,6 +14,7 @@ vespa_add_library(vespamallocd $<TARGET_OBJECTS:vespamalloc_util> INSTALL lib64/vespa/malloc DEPENDS + atomic dl ) vespa_add_library(vespamallocdst16 @@ -21,6 +23,7 @@ vespa_add_library(vespamallocdst16 $<TARGET_OBJECTS:vespamalloc_util> INSTALL lib64/vespa/malloc DEPENDS + atomic dl ) vespa_add_library(vespamallocdst16_nl @@ -29,6 +32,7 @@ vespa_add_library(vespamallocdst16_nl $<TARGET_OBJECTS:vespamalloc_util> INSTALL lib64/vespa/malloc DEPENDS + atomic dl ) vespa_add_library(vespammap @@ -36,4 +40,5 @@ vespa_add_library(vespammap $<TARGET_OBJECTS:vespamalloc_mmap> INSTALL lib64/vespa/malloc DEPENDS + dl ) |