diff options
115 files changed, 1243 insertions, 875 deletions
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 e5ab125227d..f1a93e58526 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 @@ -171,14 +171,16 @@ public class InstanceValidator { .orElse(false); } - // If/when we dont care about logging exactly whats wrong, this can be simplified + // If/when we don't 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) { + private boolean isSameIdentityAsInServicesXml(ApplicationId applicationId, String domain, String service) { Optional<ApplicationInfo> applicationInfo = superModelProvider.getSuperModel().getApplicationInfo(applicationId); if (!applicationInfo.isPresent()) { - log.info(String.format("Could not find application info for %s", applicationId.serializedForm())); + log.info(String.format("Could not find application info for %s, existing applications: %s", + applicationId.serializedForm(), + superModelProvider.getSuperModel().getAllApplicationInfos())); return false; } 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 486db205a4c..9407037eabd 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 @@ -25,4 +25,9 @@ public class ApplicationInfo { return model; } + @Override + public String toString() { + return applicationId + ", generation " + generation; + } + } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java index 20415343755..e9abcfd87a9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomConfigPayloadBuilder.java @@ -169,7 +169,9 @@ public class DomConfigPayloadBuilder { // Check for legacy (pre Vespa 6) usage throw new IllegalArgumentException("The 'index' attribute on config elements is not supported - use <item>"); } else if (element.hasAttribute("operation")) { - ConfigPayloadBuilder childPayloadBuilder = getBuilderForInnerArray(element, payloadBuilder, name); + // inner array, currently the only supported operation is 'append' + verifyLegalOperation(element); + ConfigPayloadBuilder childPayloadBuilder = payloadBuilder.getArray(name).append(); //Cursor array = node.setArray(name); for (Element child : children) { //Cursor struct = array.addObject(); @@ -238,24 +240,4 @@ public class DomConfigPayloadBuilder { + operation + "' at XML node '" + XML.getNodePath(currElem, " > ") + "'."); } - private ConfigPayloadBuilder getBuilderForInnerArray(Element element, ConfigPayloadBuilder payloadBuilder, String arrayName) { - // inner array, the supported operations are 'append' and 'clear' - String operation = element.getAttribute("operation").toLowerCase(); - ConfigPayloadBuilder arrayPayloadBuilder; - switch (operation) { - case "append": - arrayPayloadBuilder = payloadBuilder.getArray(arrayName).append(); - break; - case "clear": - // Clear array if it exists, use the existing builder - // Creating the array happens when handling the children ('item's) - payloadBuilder.removeArray(arrayName); - arrayPayloadBuilder = payloadBuilder; - break; - default: - throw new RuntimeException("Unknown operation '" + operation + "' at XML node '" + XML.getNodePath(element, " > ") + "'."); - } - return arrayPayloadBuilder; - } - } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java index 891f68041f1..6c7da668e93 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/ConfigserverCluster.java @@ -18,6 +18,7 @@ import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions; import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions.ConfigServer; import java.util.Optional; +import java.util.stream.IntStream; /** * Represents a config server cluster. @@ -53,17 +54,27 @@ public class ConfigserverCluster extends AbstractConfigProducer @Override public void getConfig(ZookeeperServerConfig.Builder builder) { + ConfigServer[] configServers = getConfigServers(); + int[] zookeeperIds = getConfigServerZookeeperIds(); + + if (configServers.length != zookeeperIds.length) { + throw new IllegalArgumentException(String.format("Number of provided config server hosts (%d) must be the " + + "same as number of provided config server zookeeper ids (%d)", + configServers.length, zookeeperIds.length)); + } + String myhostname = HostName.getLocalhost(); - int myid = 0; - int i = 0; - for (ConfigServer server : getConfigServers()) { - if (server.hostName.equals(myhostname)) { - myid = i; + for (int i = 0; i < configServers.length; i++) { + if (zookeeperIds[i] < 0) { + throw new IllegalArgumentException(String.format("Zookeeper ids cannot be negative, was %d for %s", + zookeeperIds[i], configServers[i].hostName)); + } + if (configServers[i].hostName.equals(myhostname)) { + builder.myid(zookeeperIds[i]); } - builder.server(getZkServer(server, i)); - i++; + builder.server(getZkServer(configServers[i], zookeeperIds[i])); } - builder.myid(myid); + if (options.zookeeperClientPort().isPresent()) { builder.clientPort(options.zookeeperClientPort().get()); } @@ -150,11 +161,15 @@ public class ConfigserverCluster extends AbstractConfigProducer } private ConfigServer[] getConfigServers() { - if (options.allConfigServers().length > 0) { - return options.allConfigServers(); - } else { - return new ConfigServer[]{new ConfigServer(HostName.getLocalhost(), Optional.<Integer>empty()) }; - } + return Optional.of(options.allConfigServers()) + .filter(configServers -> configServers.length > 0) + .orElseGet(() -> new ConfigServer[]{new ConfigServer(HostName.getLocalhost(), Optional.empty())}); + } + + private int[] getConfigServerZookeeperIds() { + return Optional.of(options.configServerZookeeperIds()) + .filter(ids -> ids.length > 0) + .orElseGet(() -> IntStream.range(0, getConfigServers().length).toArray()); } private ZookeeperServerConfig.Server.Builder getZkServer(ConfigServer server, int id) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/option/CloudConfigOptions.java b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/option/CloudConfigOptions.java index c8a39faa1d9..eafff2374cb 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/option/CloudConfigOptions.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/configserver/option/CloudConfigOptions.java @@ -24,6 +24,7 @@ public interface CloudConfigOptions { Optional<Boolean> hostedVespa(); ConfigServer[] allConfigServers(); + int[] configServerZookeeperIds(); Optional<Integer> zookeeperClientPort(); String[] configModelPluginDirs(); Optional<Long> sessionLifeTimeSecs(); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java index 623a963f77a..fe83148d7af 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java @@ -404,23 +404,24 @@ public class IndexedSearchCluster extends SearchCluster nodeBuilder.host(node.getHostName()); nodeBuilder.port(node.getRpcPort()); nodeBuilder.fs4port(node.getDispatchPort()); - if (tuning.dispatch.minActiveDocsCoverage != null) - builder.minActivedocsPercentage(tuning.dispatch.minActiveDocsCoverage); - if (tuning.dispatch.minGroupCoverage != null) - builder.minGroupCoverage(tuning.dispatch.minGroupCoverage); - if (tuning.dispatch.policy != null) { - switch (tuning.dispatch.policy) { - case RANDOM: - builder.distributionPolicy(DistributionPolicy.RANDOM); - break; - case ROUNDROBIN: - builder.distributionPolicy(DistributionPolicy.ROUNDROBIN); - break; - } - } - builder.maxNodesDownPerGroup(rootDispatch.getMaxNodesDownPerFixedRow()); builder.node(nodeBuilder); } + if (tuning.dispatch.minActiveDocsCoverage != null) + builder.minActivedocsPercentage(tuning.dispatch.minActiveDocsCoverage); + if (tuning.dispatch.minGroupCoverage != null) + builder.minGroupCoverage(tuning.dispatch.minGroupCoverage); + if (tuning.dispatch.policy != null) { + switch (tuning.dispatch.policy) { + case RANDOM: + builder.distributionPolicy(DistributionPolicy.RANDOM); + break; + case ROUNDROBIN: + builder.distributionPolicy(DistributionPolicy.ROUNDROBIN); + break; + } + } + builder.maxNodesDownPerGroup(rootDispatch.getMaxNodesDownPerFixedRow()); + builder.useMultilevelDispatch(useMultilevelDispatchSetup()); } @Override diff --git a/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java index 747ce93d86b..d1ef1010b73 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/builder/UserConfigBuilderTest.java @@ -8,6 +8,7 @@ import com.yahoo.config.model.deploy.ConfigDefinitionStore; import com.yahoo.test.SimpletypesConfig; import com.yahoo.config.model.producer.UserConfigRepo; import com.yahoo.config.model.builder.xml.XmlHelper; +import com.yahoo.vespa.config.ConfigDefinition; import com.yahoo.vespa.config.ConfigDefinitionKey; import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.ConfigPayloadBuilder; @@ -16,28 +17,30 @@ import org.junit.Test; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import javax.xml.parsers.ParserConfigurationException; +import java.io.IOException; import java.io.Reader; import java.io.StringReader; -import java.util.Arrays; -import java.util.Collections; import java.util.Optional; import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.*; /** * @author Ulf Lilleengen + * @since 5.1 */ public class UserConfigBuilderTest { - private final ConfigDefinitionStore configDefinitionStore = defKey -> Optional.empty(); + private final ConfigDefinitionStore configDefinitionStore = new ConfigDefinitionStore() { + @Override + public Optional<ConfigDefinition> getConfigDefinition(ConfigDefinitionKey defKey) { return Optional.empty(); } + }; @Test - public void require_that_simple_config_is_resolved() { + public void require_that_simple_config_is_resolved() throws ParserConfigurationException, IOException, SAXException { Element configRoot = getDocument("<config name=\"simpletypes\">" + " <intval>13</intval>" + "</config>" + @@ -53,59 +56,31 @@ public class UserConfigBuilderTest { assertThat(config.stringval(), is("foolio")); } - @Test - public void require_that_arrays_config_is_resolved() { - Element configRoot = getDocument("<config name=\"arraytypes\">" + - " <intarr operation=\"append\">13</intarr>" + - " <intarr operation=\"append\">10</intarr>" + - " <intarr operation=\"append\">1337</intarr>" + - "</config>"); - UserConfigRepo map = UserConfigBuilder.build(configRoot, configDefinitionStore, new BaseDeployLogger()); - assertFalse(map.isEmpty()); - ConfigDefinitionKey key = new ConfigDefinitionKey("arraytypes", "config"); - assertNotNull(map.get(key)); - ArraytypesConfig config = createConfig(ArraytypesConfig.class, map.get(key)); - assertEquals(Arrays.asList(13,10,1337), config.intarr()); + public static <ConfigType extends ConfigInstance> ConfigType createConfig(Class<ConfigType> clazz, ConfigPayloadBuilder builder) { + return ConfigPayload.fromBuilder(builder).toInstance(clazz, ""); } - @Test - public void require_that_array_with_items_is_resolved() { - ConfigDefinitionKey key = new ConfigDefinitionKey("arraytypes", "config"); - Element configRoot = getDocument("<config name='arraytypes'>" + - " <intarr operation='clear'>" + - " <item>0</item>" + - " </intarr>" + - "</config>"); + @Test + public void require_that_arrays_config_is_resolved() throws ParserConfigurationException, IOException, SAXException { + Element configRoot = getDocument("<config name=\"arraytypes\">" + + " <intarr operation=\"append\">13</intarr>" + + " <intarr operation=\"append\">10</intarr>" + + " <intarr operation=\"append\">1337</intarr>" + + "</config>"); UserConfigRepo map = UserConfigBuilder.build(configRoot, configDefinitionStore, new BaseDeployLogger()); assertFalse(map.isEmpty()); + ConfigDefinitionKey key = new ConfigDefinitionKey("arraytypes", "config"); assertNotNull(map.get(key)); ArraytypesConfig config = createConfig(ArraytypesConfig.class, map.get(key)); - assertEquals(Collections.singletonList(0), config.intarr()); - - Element configRoot2 = getDocument("<config name='arraytypes'>" + - " <intarr>" + - " <item>1</item>" + - " <item>2</item>" + - " </intarr>" + - "</config>"); - UserConfigRepo map2 = mergeAndCreateConfig(map, configRoot2); - ArraytypesConfig config2 = createConfig(ArraytypesConfig.class, map2.get(key)); - assertEquals(Arrays.asList(1, 2), config2.intarr()); - - - Element configRoot3 = getDocument("<config name='arraytypes'>" + - " <intarr operation='clear'>" + - " <item>3</item>" + - " </intarr>" + - "</config>"); - UserConfigRepo map3 = mergeAndCreateConfig(map2, configRoot3); - ArraytypesConfig config3 = createConfig(ArraytypesConfig.class, map3.get(key)); - assertEquals(Collections.singletonList(3), config3.intarr()); + assertThat(config.intarr().size(), is(3)); + assertThat(config.intarr(0), is(13)); + assertThat(config.intarr(1), is(10)); + assertThat(config.intarr(2), is(1337)); } @Test - public void require_that_arrays_of_structs_are_resolved() { + public void require_that_arrays_of_structs_are_resolved() throws ParserConfigurationException, IOException, SAXException { Element configRoot = getDocument( " <config name='vespa.configdefinition.specialtokens'>" + " <tokenlist operation='append'>" + @@ -132,7 +107,7 @@ public class UserConfigBuilderTest { } @Test - public void no_exception_when_config_class_does_not_exist() { + public void no_exception_when_config_class_does_not_exist() throws ParserConfigurationException, IOException, SAXException { Element configRoot = getDocument("<config name=\"unknown\">" + " <foo>1</foo>" + "</config>"); @@ -141,7 +116,7 @@ public class UserConfigBuilderTest { assertNotNull(builder); } - private Element getDocument(String xml) { + private Element getDocument(String xml) throws ParserConfigurationException { Reader xmlReader = new StringReader("<model>" + xml + "</model>"); Document doc; try { @@ -151,15 +126,4 @@ public class UserConfigBuilderTest { } return doc.getDocumentElement(); } - - private static <ConfigType extends ConfigInstance> ConfigType createConfig(Class<ConfigType> clazz, ConfigPayloadBuilder builder) { - return ConfigPayload.fromBuilder(builder).toInstance(clazz, ""); - } - - private UserConfigRepo mergeAndCreateConfig(UserConfigRepo original, Element newElement) { - UserConfigRepo map = UserConfigBuilder.build(newElement, configDefinitionStore, new BaseDeployLogger()); - original.merge(map); - return map; - } - } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java index b4ad2ddbd21..708d38d8ebb 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/ConfigserverClusterTest.java @@ -2,6 +2,8 @@ package com.yahoo.vespa.model.container.configserver; import com.yahoo.cloud.config.ConfigserverConfig; +import com.yahoo.cloud.config.ZookeeperServerConfig; +import com.yahoo.config.ConfigInstance; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducerRoot; import com.yahoo.config.model.test.MockRoot; @@ -11,11 +13,19 @@ import com.yahoo.jdisc.metrics.yamasconsumer.cloud.ScoreBoardConfig; import com.yahoo.net.HostName; import com.yahoo.text.XML; import com.yahoo.vespa.defaults.Defaults; +import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions; import com.yahoo.vespa.model.container.xml.ConfigServerContainerModelBuilder; -import org.junit.Before; import org.junit.Test; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.function.Function; +import java.util.stream.Collectors; + import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -25,33 +35,54 @@ import static org.junit.Assert.assertTrue; */ public class ConfigserverClusterTest { - private AbstractConfigProducerRoot root; + @Test + public void zookeeperConfig_default() { + ZookeeperServerConfig config = getConfig(ZookeeperServerConfig.class); + assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::hostname, "localhost"); + assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::id, 0); + assertEquals(0, config.myid()); + } - @Before - public void setupCluster() { - String services = "<jdisc id='standalone' version='1.0'>" - + " <http>" - + " <server port='1337' id='configserver' />" - + " </http>" - + "</jdisc>"; - root = new MockRoot(); - new ConfigServerContainerModelBuilder(new TestOptions().rpcPort(12345).useVespaVersionInRequest(true) - .hostedVespa(true).environment("test").region("bar") - .numParallelTenantLoaders(99)) - .build(new DeployState.Builder().build(), null, null, root, XML.getDocument(services).getDocumentElement()); - root.freezeModelTopology(); + @Test + public void zookeeperConfig_only_config_servers_set() { + TestOptions testOptions = createTestOptions(Arrays.asList("cfg1", "localhost", "cfg3"), Collections.emptyList()); + ZookeeperServerConfig config = getConfig(ZookeeperServerConfig.class, testOptions); + assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::hostname, "cfg1", "localhost", "cfg3"); + assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::id, 0, 1, 2); + assertEquals(1, config.myid()); + } + + @Test + public void zookeeperConfig_with_config_servers_and_zk_ids() { + TestOptions testOptions = createTestOptions(Arrays.asList("cfg1", "localhost", "cfg3"), Arrays.asList(4, 2, 3)); + ZookeeperServerConfig config = getConfig(ZookeeperServerConfig.class, testOptions); + assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::hostname, "cfg1", "localhost", "cfg3"); + assertZookeeperServerProperty(config.server(), ZookeeperServerConfig.Server::id, 4, 2, 3); + assertEquals(2, config.myid()); + } + + @Test(expected = IllegalArgumentException.class) + public void zookeeperConfig_uneven_number_of_config_servers_and_zk_ids() { + TestOptions testOptions = createTestOptions(Arrays.asList("cfg1", "localhost", "cfg3"), Collections.singletonList(1)); + getConfig(ZookeeperServerConfig.class, testOptions); + } + + @Test(expected = IllegalArgumentException.class) + public void zookeeperConfig_negative_zk_id() { + TestOptions testOptions = createTestOptions(Arrays.asList("cfg1", "localhost", "cfg3"), Arrays.asList(1, 2, -1)); + getConfig(ZookeeperServerConfig.class, testOptions); } @Test public void testStatisticsConfig() { - StatisticsConfig config = root.getConfig(StatisticsConfig.class, "configserver/standalone"); + StatisticsConfig config = getConfig(StatisticsConfig.class); assertThat((int) config.collectionintervalsec(), is(60)); assertThat((int) config.loggingintervalsec(), is(60)); } @Test public void testScoreBoardConfig() { - ScoreBoardConfig config = root.getConfig(ScoreBoardConfig.class, "configserver/standalone"); + ScoreBoardConfig config = getConfig(ScoreBoardConfig.class); assertThat(config.applicationName(), is("configserver")); assertThat(config.flushTime(), is(60)); assertThat(config.step(), is(60)); @@ -59,13 +90,13 @@ public class ConfigserverClusterTest { @Test public void testHealthMonitorConfig() { - HealthMonitorConfig config = root.getConfig(HealthMonitorConfig.class, "configserver/standalone"); + HealthMonitorConfig config = getConfig(HealthMonitorConfig.class); assertThat(((int) config.snapshot_interval()), is(60)); } @Test public void testConfigserverConfig() { - ConfigserverConfig config = root.getConfig(ConfigserverConfig.class, "configserver/standalone"); + ConfigserverConfig config = getConfig(ConfigserverConfig.class); assertThat(config.configModelPluginDir().size(), is(1)); assertThat(config.configModelPluginDir().get(0), is(Defaults.getDefaults().underVespaHome("lib/jars/config-models"))); assertThat(config.rpcport(), is(12345)); @@ -79,4 +110,53 @@ public class ConfigserverClusterTest { assertThat(config.region(), is("bar")); } + @SuppressWarnings("varargs") + private static <T> void assertZookeeperServerProperty( + List<ZookeeperServerConfig.Server> zkServers, Function<ZookeeperServerConfig.Server, T> properyMapper, T... expectedProperties) { + List<T> actualPropertyValues = zkServers.stream().map(properyMapper).collect(Collectors.toList()); + List<T> expectedPropertyValues = Arrays.asList(expectedProperties); + assertEquals(expectedPropertyValues, actualPropertyValues); + } + + private static TestOptions createTestOptions(List<String> configServerHostnames, List<Integer> configServerZkIds) { + TestOptions testOptions = new TestOptions() + .rpcPort(12345) + .useVespaVersionInRequest(true) + .hostedVespa(true) + .environment("test") + .region("bar") + .numParallelTenantLoaders(99); + + Optional.of(configServerHostnames) + .filter(hostnames -> !hostnames.isEmpty()) + .map(hostnames -> hostnames.stream() + .map(hostname -> new CloudConfigOptions.ConfigServer(hostname, Optional.empty())) + .toArray(CloudConfigOptions.ConfigServer[]::new)) + .ifPresent(testOptions::configServers); + + Optional.of(configServerZkIds) + .filter(zkIds -> !zkIds.isEmpty()) + .map(zkIds -> zkIds.stream().mapToInt(i -> i).toArray()) + .ifPresent(testOptions::configServerZookeeperIds); + + return testOptions; + } + + private static <CONFIGTYPE extends ConfigInstance> CONFIGTYPE getConfig(Class<CONFIGTYPE> clazz) { + return getConfig(clazz, createTestOptions(Collections.emptyList(), Collections.emptyList())); + } + + private static <CONFIGTYPE extends ConfigInstance> CONFIGTYPE getConfig(Class<CONFIGTYPE> clazz, TestOptions testOptions) { + AbstractConfigProducerRoot root = new MockRoot(); + String services = "<jdisc id='standalone' version='1.0'>" + + " <http>" + + " <server port='1337' id='configserver' />" + + " </http>" + + "</jdisc>"; + new ConfigServerContainerModelBuilder(testOptions) + .build(new DeployState.Builder().build(), null, null, root, XML.getDocument(services).getDocumentElement()); + root.freezeModelTopology(); + + return root.getConfig(clazz, "configserver/standalone"); + } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/TestOptions.java b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/TestOptions.java index 3c2f71fa2e1..07422b1f215 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/TestOptions.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/configserver/TestOptions.java @@ -9,6 +9,9 @@ import java.util.Optional; * @author Ulf Lilleengen */ public class TestOptions implements CloudConfigOptions { + + private ConfigServer[] configServers = new ConfigServer[0]; + private int[] configServerZookeeperIds = new int[0]; private Optional<Integer> rpcPort = Optional.empty(); private Optional<String> environment = Optional.empty(); private Optional<String> region = Optional.empty(); @@ -45,7 +48,12 @@ public class TestOptions implements CloudConfigOptions { @Override public ConfigServer[] allConfigServers() { - return new ConfigServer[0]; + return configServers; + } + + @Override + public int[] configServerZookeeperIds() { + return configServerZookeeperIds; } @Override @@ -121,6 +129,16 @@ public class TestOptions implements CloudConfigOptions { return Optional.empty(); } + public TestOptions configServers(ConfigServer[] configServers) { + this.configServers = configServers; + return this; + } + + public TestOptions configServerZookeeperIds(int[] configServerZookeeperIds) { + this.configServerZookeeperIds = configServerZookeeperIds; + return this; + } + public TestOptions numParallelTenantLoaders(int numLoaders) { this.numParallelTenantLoaders = Optional.of(numLoaders); return this; diff --git a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java index 863505af5b0..bb974ddae42 100644 --- a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java +++ b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadBuilder.java @@ -109,15 +109,6 @@ public class ConfigPayloadBuilder { return a; } - /** - * Removes an array - * - * @param name Name of array. - */ - public void removeArray(String name) { - arrayMap.remove(name); - } - private void validateArray(String name) { if (configDefinition != null) { configDefinition.verify(name); @@ -426,10 +417,6 @@ public class ConfigPayloadBuilder { } return this; } - - public void clear() { - elements.clear(); - } } private ConfigPayloadBuilder(ConfigPayloadBuilder other) { diff --git a/config/src/test/java/com/yahoo/vespa/config/classes/qr-logging.def b/config/src/test/java/com/yahoo/vespa/config/classes/qr-logging.def deleted file mode 100644 index 3fcab08a63b..00000000000 --- a/config/src/test/java/com/yahoo/vespa/config/classes/qr-logging.def +++ /dev/null @@ -1,33 +0,0 @@ -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -logger string default="com.yahoo" -# Either QueryAccessLog for a regular Vespa access log, or YApacheAccessLog for a log on yApache format -speciallog[].name string - -# Leave as "" -speciallog[].type string - -speciallog[].filehandler.name string default="" - -# File name patterns supporting the expected time variables -speciallog[].filehandler.pattern string default=".%Y%m%d%H%M%S" - -speciallog[].filehandler.rotation string default="0 60 ..." - -# Defines how file rotation is done. There are two options: -# -# "date" : -# The active log file is given the name resulting from pattern (but in this case "pattern" must yield a -# time-dependent name. In addition, a symlink is created pointing to the newest file. -# The symlink is given the name of the symlink parameter (or the name of this service -# if no parameter is given. -# -# "sequence" : -# The active log file is given the name -# defined by "pattern" (which in this case will likely just be a constant string). -# At rotation, this file is given the name pattern.N where N is 1 + the largest integer found by -# extracting the integers from all files ending by .Integer in the same directory -# -speciallog[].filehandler.rotatescheme string default="date" - -speciallog[].cachehandler.name string default="" -speciallog[].cachehandler.size int default=1000 diff --git a/config/src/test/resources/configs/def-files/chains-test.def b/config/src/test/resources/configs/def-files/chains-test.def deleted file mode 100644 index b7da79931d5..00000000000 --- a/config/src/test/resources/configs/def-files/chains-test.def +++ /dev/null @@ -1,42 +0,0 @@ -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -# Chains configuration -namespace=foo - -component[].id string - -# Configured functionality provided by this - comes in addition to those set in the code -component[].dependencies.provides[] string - -# Configured "before" dependencies provided by this - comes in addition to those set in the code -component[].dependencies.before[] string - -# Configured "after" dependencies provided by this - comes in addition to those set in the code -component[].dependencies.after[] string - -# The id of this chain. The id has the form name(:version)? -# where the version has the form 1(.2(.3(.identifier)?)?)?. -# The default chain must be called "default". -chain[].id string - -#The type of this chain -chain[].type enum {DOCPROC, SEARCH} default=SEARCH - -# The id of a component to include in this chain. -# The id has the form fullclassname(:version)? -# where the version has the form 1(.2(.3(.identifier)?)?)?. -chain[].component[] string - -# The optional list of chain ids this inherits. -# The ids has the form name(:version)? -# where the version has the form 1(.2(.3(.identifier)?)?)?. -# If the version is not specified the newest version is used. -chain[].inherit[] string - -# The optional list of component ids to exclude from this chain even if they exists in inherited chains -# If versions are specified in these ids, they are ignored. -chain[].exclude[] string - -# The phases for a chain -chain[].phase[].id string -chain[].phase[].before[] string -chain[].phase[].after[] string diff --git a/config/src/test/resources/configs/def-files/md5test.def b/config/src/test/resources/configs/def-files/md5test.def deleted file mode 100644 index da79d022648..00000000000 --- a/config/src/test/resources/configs/def-files/md5test.def +++ /dev/null @@ -1,25 +0,0 @@ -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -namespace=foo - -# Added empty line to see if we can confuse -# the server's md5 calculation - -#even adding a variable name starting with 'version' -versiontag int default=3 - -blabla string default="" -tabs string default=" " -test int - -# test multiple spaces/tabs -spaces int -singletab string -multitabs double - -# test enum -normal enum { VAL1, VAL2 } default=VAL1 -spacevalues enum { V1 , V2 , V3 , V4 } default=V3 - -# Comments and empty lines at the end - - diff --git a/config/src/test/resources/configs/def-files/standard.def b/config/src/test/resources/configs/def-files/standard.def deleted file mode 100644 index f4c6ced1e00..00000000000 --- a/config/src/test/resources/configs/def-files/standard.def +++ /dev/null @@ -1,8 +0,0 @@ -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -# Config containing only simple leaf types with default values, that can be used -# for testing individual types in detail. -namespace=foo - -basicStruct.intVal int default=0 -basicStruct.stringVal string default="s" -stringArr[] string diff --git a/configdefinitions/src/vespa/dispatch.def b/configdefinitions/src/vespa/dispatch.def index 602d3b17a8e..487f8ac24c3 100644 --- a/configdefinitions/src/vespa/dispatch.def +++ b/configdefinitions/src/vespa/dispatch.def @@ -16,6 +16,9 @@ maxNodesDownPerGroup int default=0 # Distribution policy for group selection distributionPolicy enum { ROUNDROBIN, RANDOM } default=ROUNDROBIN +# Is multi-level dispatch configured for this cluster +useMultilevelDispatch bool default=false + # The unique key of a search node node[].key int diff --git a/configgen/pom.xml b/configgen/pom.xml index be9cd733601..fc7be700373 100644 --- a/configgen/pom.xml +++ b/configgen/pom.xml @@ -12,7 +12,7 @@ <packaging>jar</packaging> <version>6-SNAPSHOT</version> <name>configgen</name> - <description>Config java code generation from defintion files for Java Vespa components.</description> + <description>Config java code generation from config definition files for Java Vespa components.</description> <dependencies> <dependency> <groupId>junit</groupId> @@ -51,9 +51,6 @@ <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-install-plugin</artifactId> - <configuration> - <updateReleaseInfo>true</updateReleaseInfo> - </configuration> </plugin> </plugins> </build> diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java index 51048db3cb7..f5d082635ab 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java @@ -8,9 +8,12 @@ import com.yahoo.search.Result; import com.yahoo.search.dispatch.FillInvoker; import com.yahoo.search.dispatch.InterleavedFillInvoker; import com.yahoo.search.dispatch.InterleavedSearchInvoker; +import com.yahoo.search.dispatch.SearchErrorInvoker; import com.yahoo.search.dispatch.SearchInvoker; import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.dispatch.searchcluster.SearchCluster; +import com.yahoo.search.result.Coverage; +import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.result.Hit; import java.util.ArrayList; @@ -70,19 +73,24 @@ public class FS4InvokerFactory { Map<Integer, SearchInvoker> invokers = new HashMap<>(); Set<Integer> failed = null; for (Node node : nodes) { + boolean nodeAdded = false; if (node.isWorking()) { Backend backend = fs4ResourcePool.getBackend(node.hostname(), node.fs4port()); if (backend.probeConnection()) { invokers.put(node.key(), new FS4SearchInvoker(searcher, query, backend.openChannel(), node)); - } else { - if(failed == null) { - failed = new HashSet<>(); - } - failed.add(node.key()); + nodeAdded = true; + } + } + + if (!nodeAdded) { + if (failed == null) { + failed = new HashSet<>(); } + failed.add(node.key()); } } - if (failed != null && ! acceptIncompleteCoverage) { + + if (failed != null) { List<Node> success = new ArrayList<>(nodes.size() - failed.size()); for (Node node : nodes) { if (!failed.contains(node.key())) { @@ -90,9 +98,14 @@ public class FS4InvokerFactory { } } if (!searchCluster.isPartialGroupCoverageSufficient(groupId, success)) { - return Optional.empty(); + if (acceptIncompleteCoverage) { + createCoverageErrorInvoker(invokers, nodes, failed); + } else { + return Optional.empty(); + } } } + if (invokers.size() == 1) { return Optional.of(invokers.values().iterator().next()); } else { @@ -100,6 +113,25 @@ public class FS4InvokerFactory { } } + private void createCoverageErrorInvoker(Map<Integer, SearchInvoker> invokers, List<Node> nodes, Set<Integer> failed) { + long activeDocuments = 0; + StringBuilder down = new StringBuilder("Connection failure on nodes with distribution-keys: "); + Integer key = null; + for (Node node : nodes) { + if (failed.contains(node.key())) { + activeDocuments += node.getActiveDocuments(); + if (key == null) { + key = node.key(); + } else { + down.append(", "); + } + down.append(node.key()); + } + } + Coverage coverage = new Coverage(0, activeDocuments, 0); + invokers.put(key, new SearchErrorInvoker(ErrorMessage.createBackendCommunicationError(down.toString()), coverage)); + } + public FillInvoker getFillInvoker(Query query, Node node) { return new FS4FillInvoker(searcher, query, fs4ResourcePool, node.hostname(), node.fs4port(), node.key()); } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java index 11b71c2c159..a98c34295ee 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java @@ -272,14 +272,20 @@ public class FastSearcher extends VespaBackEndSearcher { Result result = new Result(query); // keep a separate tally of coverage as the normal merge counts using // federated query rules - Coverage finalCoverage = new Coverage(0, 0); + Coverage finalCoverage = null; for (Result partialResult : results) { - finalCoverage.mergeWithPartition(partialResult.getCoverage(true)); + if(finalCoverage == null) { + finalCoverage = partialResult.getCoverage(true); + } else { + finalCoverage.mergeWithPartition(partialResult.getCoverage(true)); + } result.mergeWith(partialResult); result.hits().addAll(partialResult.hits().asUnorderedHits()); } - result.setCoverage(finalCoverage); + if (finalCoverage != null) { + result.setCoverage(finalCoverage); + } if (query.getOffset() != 0 || result.hits().size() > query.getHits()) { // with multiple results, each partial result is expected to have diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java index 0dd682dee0e..708d80d4969 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java @@ -14,6 +14,7 @@ import com.yahoo.search.dispatch.SearchPath.InvalidSearchPathException; import com.yahoo.search.dispatch.searchcluster.Group; import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.dispatch.searchcluster.SearchCluster; +import com.yahoo.search.result.ErrorMessage; import com.yahoo.vespa.config.search.DispatchConfig; import java.util.Arrays; @@ -46,12 +47,14 @@ public class Dispatcher extends AbstractComponent { private final LoadBalancer loadBalancer; private final RpcResourcePool rpcResourcePool; + private final boolean multilevelDispatch; public Dispatcher(DispatchConfig dispatchConfig, FS4ResourcePool fs4ResourcePool, int containerClusterSize, VipStatus vipStatus) { this.searchCluster = new SearchCluster(dispatchConfig, fs4ResourcePool, containerClusterSize, vipStatus); this.loadBalancer = new LoadBalancer(searchCluster, dispatchConfig.distributionPolicy() == DispatchConfig.DistributionPolicy.ROUNDROBIN); this.rpcResourcePool = new RpcResourcePool(dispatchConfig); + this.multilevelDispatch = dispatchConfig.useMultilevelDispatch(); } /** For testing */ @@ -59,6 +62,7 @@ public class Dispatcher extends AbstractComponent { this.searchCluster = null; this.loadBalancer = new LoadBalancer(searchCluster, true); this.rpcResourcePool = new RpcResourcePool(client, nodeConnections); + this.multilevelDispatch = false; } /** Returns the search cluster this dispatches to */ @@ -87,19 +91,20 @@ public class Dispatcher extends AbstractComponent { } public Optional<SearchInvoker> getSearchInvoker(Query query, FS4InvokerFactory fs4InvokerFactory) { - if (query.properties().getBoolean(dispatchInternal, false)) { - Optional<SearchInvoker> invoker = getSearchPathInvoker(query, fs4InvokerFactory::getSearchInvoker); + if (multilevelDispatch || ! query.properties().getBoolean(dispatchInternal, false)) { + return Optional.empty(); + } - if(! invoker.isPresent()) { - invoker = getInternalInvoker(query, fs4InvokerFactory::getSearchInvoker); - } - if(invoker.isPresent() && query.properties().getBoolean(com.yahoo.search.query.Model.ESTIMATE)) { - query.setHits(0); - query.setOffset(0); - } - return invoker; + Optional<SearchInvoker> invoker = getSearchPathInvoker(query, fs4InvokerFactory::getSearchInvoker); + + if (!invoker.isPresent()) { + invoker = getInternalInvoker(query, fs4InvokerFactory::getSearchInvoker); } - return Optional.empty(); + if (invoker.isPresent() && query.properties().getBoolean(com.yahoo.search.query.Model.ESTIMATE)) { + query.setHits(0); + query.setOffset(0); + } + return invoker; } @FunctionalInterface @@ -121,7 +126,7 @@ public class Dispatcher extends AbstractComponent { return invokerFactory.supply(query, -1, nodes, true); } } catch (InvalidSearchPathException e) { - return Optional.of(new SearchErrorInvoker(e.getMessage())); + return Optional.of(new SearchErrorInvoker(ErrorMessage.createIllegalQuery(e.getMessage()))); } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java index b716c182615..d5c505aa31b 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java @@ -5,6 +5,7 @@ import com.yahoo.fs4.QueryPacket; import com.yahoo.prelude.fastsearch.CacheKey; import com.yahoo.search.Query; import com.yahoo.search.Result; +import com.yahoo.search.result.Coverage; import com.yahoo.search.result.ErrorMessage; import java.io.IOException; @@ -12,17 +13,24 @@ import java.util.Arrays; import java.util.List; /** - * A search invoker that will immediately produce an error that occurred during invoker construction. - * Currently used for invalid searchpath values. + * A search invoker that will immediately produce an error that occurred during + * invoker construction. Currently used for invalid searchpath values and node + * failure * * @author ollivir */ public class SearchErrorInvoker extends SearchInvoker { - private final String message; + private final ErrorMessage message; private Query query; + private final Coverage coverage; - public SearchErrorInvoker(String message) { + public SearchErrorInvoker(ErrorMessage message, Coverage coverage) { this.message = message; + this.coverage = coverage; + } + + public SearchErrorInvoker(ErrorMessage message) { + this(message, null); } @Override @@ -32,7 +40,11 @@ public class SearchErrorInvoker extends SearchInvoker { @Override protected List<Result> getSearchResults(CacheKey cacheKey) throws IOException { - return Arrays.asList(new Result(query, ErrorMessage.createIllegalQuery(message))); + Result res = new Result(query, message); + if (coverage != null) { + res.setCoverage(coverage); + } + return Arrays.asList(res); } @Override diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java index b0e63d20931..b51620a978b 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java @@ -323,7 +323,7 @@ public class SearchCluster implements NodeManager<Node> { */ public boolean isPartialGroupCoverageSufficient(int groupId, List<Node> nodes) { if (orderedGroups.size() == 1) { - return true; + return nodes.size() >= groupSize() - maxNodesDownPerGroup; } long sumOfActiveDocuments = 0; int otherGroups = 0; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationStore.java index acf59b2a850..11886f7dd18 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationStore.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationStore.java @@ -2,22 +2,39 @@ package com.yahoo.vespa.hosted.controller.api.integration.deployment; import com.yahoo.config.provision.ApplicationId; + /** - * Store for the application package and application tester package. + * Store for the application and tester packages. + * + * This will replace ArtifactRepository for tenant applications. * - * This interface will take over most of the responsibility from the ArtifactRepository with time. + * @author smorgrav + * @author jonmv */ public interface ApplicationStore { /** Returns the tenant application package of the given version. */ - byte[] getApplicationPackage(ApplicationId application, String applicationVersion); + byte[] get(ApplicationId application, ApplicationVersion applicationVersion); /** Stores the given tenant application package of the given version. */ - void putApplicationPackage(ApplicationId application, String applicationVersion, byte[] applicationPackage); + void put(ApplicationId application, ApplicationVersion applicationVersion, byte[] applicationPackage); - /** Stores the given tester application package of the given version. Does NOT contain the services.xml. */ - void putTesterPackage(ApplicationId tester, String applicationVersion, byte[] testerPackage); + /** Removes applications older than the given version, for the given application, and returns whether something was removed. */ + boolean prune(ApplicationId application, ApplicationVersion olderThanVersion); + + /** Removes all application packages for the given application. */ + void removeAll(ApplicationId application); /** Returns the tester application package of the given version. Does NOT contain the services.xml. */ - byte[] getTesterPackage(ApplicationId tester, String applicationVersion); + byte[] get(TesterId tester, ApplicationVersion applicationVersion); + + /** Stores the given tester application package of the given version. Does NOT contain the services.xml. */ + void put(TesterId tester, ApplicationVersion applicationVersion, byte[] testerPackage); + + /** Removes tester packages older than the given version, for the given tester, and returns whether something was removed. */ + boolean prune(TesterId tester, ApplicationVersion olderThanVersion); + + /** Removes all tester packages for the given tester. */ + void removeAll(TesterId tester); + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationVersion.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java index 703a198be1e..0e14be29fc5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationVersion.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java @@ -1,5 +1,5 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.application; +package com.yahoo.vespa.hosted.controller.api.integration.deployment; import java.util.Objects; import java.util.Optional; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ArtifactRepository.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ArtifactRepository.java index 117dbd38a3b..13b16fc4cd1 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ArtifactRepository.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ArtifactRepository.java @@ -15,16 +15,7 @@ public interface ArtifactRepository { /** Returns the tenant application package of the given version. */ byte[] getApplicationPackage(ApplicationId application, String applicationVersion); - /** Stores the given tenant application package of the given version. */ - void putApplicationPackage(ApplicationId application, String applicationVersion, byte[] applicationPackage); - /** Returns the system application package of the given version. */ byte[] getSystemApplicationPackage(ApplicationId application, ZoneId zone, Version version); - /** Stores the given tester application package of the given version. Does NOT contain the services.xml. */ - void putTesterPackage(ApplicationId tester, String applicationVersion, byte[] testerPackage); - - /** Returns the tester application package of the given version. Does NOT contain the services.xml. */ - byte[] getTesterPackage(ApplicationId tester, String applicationVersion); - } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/RunId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/RunId.java index 85113d79d04..086bb8f2fb4 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/RunId.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/RunId.java @@ -13,17 +13,20 @@ import java.util.Objects; public class RunId { private final ApplicationId application; + private final TesterId tester; private final JobType type; private final long number; public RunId(ApplicationId application, JobType type, long number) { this.application = Objects.requireNonNull(application, "ApplicationId cannot be null!"); + this.tester = TesterId.of(application); this.type = Objects.requireNonNull(type, "JobType cannot be null!"); if (number <= 0) throw new IllegalArgumentException("Build number must be a positive integer!"); this.number = number; } public ApplicationId application() { return application; } + public TesterId tester() { return tester; } public JobType type() { return type; } public long number() { return number; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SourceRevision.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/SourceRevision.java index 9c10e0dc153..a9c1155c5ef 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SourceRevision.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/SourceRevision.java @@ -1,5 +1,5 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.application; +package com.yahoo.vespa.hosted.controller.api.integration.deployment; import java.util.Objects; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/TesterId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/TesterId.java new file mode 100644 index 00000000000..b586b9f3019 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/TesterId.java @@ -0,0 +1,32 @@ +package com.yahoo.vespa.hosted.controller.api.integration.deployment; + +import com.yahoo.config.provision.ApplicationId; + +/** + * Holds an application ID for a tester application. + * + * @author jonmv + */ +public class TesterId { + + public static final String suffix = "-t"; + + private final ApplicationId id; + + private TesterId(ApplicationId id) { + this.id = id; + } + + /** Creates a new TesterId for a tester of the given application. */ + public static TesterId of(ApplicationId id) { + return new TesterId(ApplicationId.from(id.tenant().value(), + id.application().value(), + id.instance().value() + suffix)); + } + + /** Returns the id of this tester application. */ + public ApplicationId id() { + return id; + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Contact.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Contact.java index e13b0f982da..9ca83673b8a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Contact.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Contact.java @@ -1,11 +1,12 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.tenant; +package com.yahoo.vespa.hosted.controller.api.integration.organization; import com.google.common.collect.ImmutableList; import java.net.URI; import java.util.List; import java.util.Objects; +import java.util.Optional; /** * Contact information for a tenant. @@ -18,12 +19,16 @@ public class Contact { private final URI propertyUrl; private final URI issueTrackerUrl; private final List<List<String>> persons; + private final String queue; + private final Optional<String> component; - public Contact(URI url, URI propertyUrl, URI issueTrackerUrl, List<List<String>> persons) { + public Contact(URI url, URI propertyUrl, URI issueTrackerUrl, List<List<String>> persons, String queue, Optional<String> component) { this.propertyUrl = Objects.requireNonNull(propertyUrl, "propertyUrl must be non-null"); this.url = Objects.requireNonNull(url, "url must be non-null"); this.issueTrackerUrl = Objects.requireNonNull(issueTrackerUrl, "issueTrackerUrl must be non-null"); this.persons = ImmutableList.copyOf(Objects.requireNonNull(persons, "persons must be non-null")); + this.queue = queue; + this.component = component; } /** URL to this */ @@ -46,6 +51,14 @@ public class Contact { return persons; } + public String queue() { + return queue; + } + + public Optional<String> component() { + return component; + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -54,7 +67,9 @@ public class Contact { return Objects.equals(url, contact.url) && Objects.equals(propertyUrl, contact.propertyUrl) && Objects.equals(issueTrackerUrl, contact.issueTrackerUrl) && - Objects.equals(persons, contact.persons); + Objects.equals(persons, contact.persons) && + Objects.equals(queue, contact.queue) && + Objects.equals(component, contact.component); } @Override @@ -69,6 +84,8 @@ public class Contact { ", propertyUrl=" + propertyUrl + ", issueTrackerUrl=" + issueTrackerUrl + ", persons=" + persons + + ", queue=" + queue + + (component.isPresent() ? ", component=" + component.get() : "") + '}'; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/ContactRetriever.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/ContactRetriever.java new file mode 100644 index 00000000000..9b05234d529 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/ContactRetriever.java @@ -0,0 +1,12 @@ +package com.yahoo.vespa.hosted.controller.api.integration.organization; + +import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; + +import java.util.Optional; + +/** + * @author olaa + */ +public interface ContactRetriever { + Contact getContact(Optional<PropertyId> propertyId); +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentIssues.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentIssues.java index 6888e8ac06d..bafedb543f6 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentIssues.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentIssues.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.controller.api.integration.organization; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import java.time.Duration; import java.util.Collection; @@ -16,12 +15,10 @@ import java.util.Optional; */ public interface DeploymentIssues { - IssueId fileUnlessOpen(Optional<IssueId> issueId, ApplicationId applicationId, PropertyId propertyId); - - IssueId fileUnlessOpen(Optional<IssueId> issueId, ApplicationId applicationId, User assignee); + IssueId fileUnlessOpen(Optional<IssueId> issueId, ApplicationId applicationId, User asignee, Contact contact); IssueId fileUnlessOpen(Collection<ApplicationId> applicationIds, Version version); - void escalateIfInactive(IssueId issueId, Optional<PropertyId> propertyId, Duration maxInactivity); + void escalateIfInactive(IssueId issueId, Duration maxInactivity, Optional<Contact> contact); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Issue.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Issue.java index a9bc7868f7a..7f42767d931 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Issue.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Issue.java @@ -22,52 +22,49 @@ public class Issue { private final String description; private final List<String> labels; private final User assignee; - private final PropertyId propertyId; private final Type type; + private final String queue; + private final Optional<String> component; - private Issue(String summary, String description, List<String> labels, User assignee, PropertyId propertyId, Type type) { + private Issue(String summary, String description, List<String> labels, User assignee, Type type, String queue, Optional<String> component) { if (summary.isEmpty()) throw new IllegalArgumentException("Issue summary can not be empty!"); if (description.isEmpty()) throw new IllegalArgumentException("Issue description can not be empty!"); - Objects.requireNonNull(propertyId, "An issue must belong to a property!"); this.summary = summary; this.description = description; this.labels = ImmutableList.copyOf(labels); this.assignee = assignee; - this.propertyId = propertyId; this.type = type; + this.queue = queue; + this.component = component; } - public Issue(String summary, String description, PropertyId propertyId) { - this(summary, description, Collections.emptyList(), null, propertyId, Type.defect); + public Issue(String summary, String description, String queue, Optional<String> component) { + this(summary, description, Collections.emptyList(), null, Type.defect, queue, component); } public Issue append(String appendage) { - return new Issue(summary, description + appendage, labels, assignee, propertyId, type); + return new Issue(summary, description + appendage, labels, assignee, type, queue, component); } public Issue with(String label) { List<String> labels = new ArrayList<>(this.labels); labels.add(label); - return new Issue(summary, description, labels, assignee, propertyId, type); + return new Issue(summary, description, labels, assignee, type, queue, component); } public Issue with(List<String> labels) { List<String> newLabels = new ArrayList<>(this.labels); newLabels.addAll(labels); - return new Issue(summary, description, newLabels, assignee, propertyId, type); + return new Issue(summary, description, newLabels, assignee, type, queue, component); } public Issue with(User assignee) { - return new Issue(summary, description, labels, assignee, propertyId, type); - } - - public Issue with(PropertyId propertyId) { - return new Issue(summary, description, labels, assignee, propertyId, type); + return new Issue(summary, description, labels, assignee, type, queue, component); } public Issue with(Type type) { - return new Issue(summary, description, labels, assignee, propertyId, type); + return new Issue(summary, description, labels, assignee, type, queue, component); } public String summary() { @@ -86,15 +83,17 @@ public class Issue { return Optional.ofNullable(assignee); } - public PropertyId propertyId() { - return propertyId; - } - public Type type() { return type; } + public String queue() { + return queue; + } + public Optional<String> component() { + return component; + } public enum Type { defect, // A defect which needs fixing. diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Organization.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/IssueHandler.java index 6dccaec3b7a..db4f0eb5c59 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Organization.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/IssueHandler.java @@ -1,20 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.organization; -import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; -import java.net.URI; import java.time.Duration; -import java.util.List; import java.util.Optional; /** - * Represents the humans who use this software, and their organization. - * Lets the software report issues to its caretakers, and provides other useful human resource lookups. - * * @author jonmv */ -public interface Organization { +public interface IssueHandler { /** * File an issue with its given property or the default, and with the specific assignee, if present. @@ -85,40 +79,8 @@ public interface Organization { * Escalate an issue filed with the given property. * * @param issueId ID of the issue to escalate. - * @param propertyId PropertyId of the tenant owning the application for which the issue was filed. * @return User that was assigned issue as a result of the escalation, if any */ - default Optional<User> escalate(IssueId issueId, PropertyId propertyId) { - List<? extends List<? extends User>> contacts = contactsFor(propertyId); - - Optional<User> assignee = assigneeOf(issueId); - int assigneeLevel = -1; - if (assignee.isPresent()) - for (int level = contacts.size(); --level > assigneeLevel; ) - if (contacts.get(level).contains(assignee.get())) - assigneeLevel = level; - - for (int level = assigneeLevel + 1; level < contacts.size(); level++) - for (User target : contacts.get(level)) - if (reassign(issueId, target)) - return Optional.of(target); - - return Optional.empty(); - } - - /** - * Returns a nested list where the entries have increasing rank, and where each entry is - * a list of the users of that rank, by decreasing relevance. - * - * @param propertyId ID of the property for which to list contacts. - * @return A sorted, nested, reverse sorted list of contacts. - */ - List<? extends List<? extends User>> contactsFor(PropertyId propertyId); - - URI issueCreationUri(PropertyId propertyId); - - URI contactsUri(PropertyId propertyId); - - URI propertyUri(PropertyId propertyId); + Optional<User> escalate(IssueId issueId, Contact contact); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java new file mode 100644 index 00000000000..55bad0b77ac --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockContactRetriever.java @@ -0,0 +1,36 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.organization; + +import com.yahoo.component.AbstractComponent; +import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; + +import java.net.URI; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +/** + * @author olaa + */ +public class MockContactRetriever extends AbstractComponent implements ContactRetriever{ + + private final Map<PropertyId, Contact> contacts = new HashMap<>(); + + + + @Override + public Contact getContact(Optional<PropertyId> propertyId) { + return contacts.getOrDefault(propertyId.get(), contact()); + } + + public void addContact(PropertyId propertyId, Contact contact) { + contacts.put(propertyId, contact); + } + + + public Contact contact() { + return new Contact(URI.create("contacts.tld"), URI.create("properties.tld"), URI.create("issues.tld"), Collections.emptyList(), "queue", Optional.of("component")); + } + +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockOrganization.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockIssueHandler.java index 82d3be596bc..674523ba26b 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockOrganization.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MockIssueHandler.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.api.integration.organization; import com.google.inject.Inject; -import com.yahoo.component.AbstractComponent; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import java.net.URI; @@ -13,34 +12,32 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.NoSuchElementException; import java.util.Optional; import java.util.concurrent.atomic.AtomicLong; +import java.util.stream.Collectors; /** * @author jvenstad */ -public class MockOrganization extends AbstractComponent implements Organization { +public class MockIssueHandler implements IssueHandler { private final Clock clock; private final AtomicLong counter = new AtomicLong(); private final Map<IssueId, MockIssue> issues = new HashMap<>(); - private final Map<PropertyId, PropertyInfo> properties = new HashMap<>(); @Inject @SuppressWarnings("unused") - public MockOrganization() { + public MockIssueHandler() { this(Clock.systemUTC()); } - public MockOrganization(Clock clock) { + public MockIssueHandler(Clock clock) { this.clock = clock; } @Override public IssueId file(Issue issue) { - if ( ! properties.containsKey(issue.propertyId())) - throw new NoSuchElementException("Unknown property '" + issue.propertyId() + "'!"); + if (!issue.assignee().isPresent()) throw new RuntimeException(); IssueId issueId = IssueId.from("" + counter.incrementAndGet()); issues.put(issueId, new MockIssue(issue)); return issueId; @@ -49,9 +46,9 @@ public class MockOrganization extends AbstractComponent implements Organization @Override public Optional<IssueId> findBySimilarity(Issue issue) { return issues.entrySet().stream() - .filter(entry -> entry.getValue().issue.summary().equals(issue.summary())) - .findFirst() - .map(Map.Entry::getKey); + .filter(entry -> entry.getValue().issue.summary().equals(issue.summary())) + .findFirst() + .map(Map.Entry::getKey); } @Override @@ -87,62 +84,55 @@ public class MockOrganization extends AbstractComponent implements Organization } @Override - public List<? extends List<? extends User>> contactsFor(PropertyId propertyId) { - return properties.getOrDefault(propertyId, new PropertyInfo()).contacts; - } - - @Override - public URI issueCreationUri(PropertyId propertyId) { - return properties.getOrDefault(propertyId, new PropertyInfo()).issueUrl; - } - - @Override - public URI contactsUri(PropertyId propertyId) { - return properties.getOrDefault(propertyId, new PropertyInfo()).contactsUrl; - } + public Optional<User> escalate(IssueId issueId, Contact contact) { + List<List<User>> contacts = getContactUsers(contact); + Optional<User> assignee = assigneeOf(issueId); + int assigneeLevel = -1; + if (assignee.isPresent()) + for (int level = contacts.size(); --level > assigneeLevel; ) + if (contacts.get(level).contains(assignee.get())) + assigneeLevel = level; - @Override - public URI propertyUri(PropertyId propertyId) { - return properties.getOrDefault(propertyId, new PropertyInfo()).propertyUrl; - } + for (int level = assigneeLevel + 1; level < contacts.size(); level++) + for (User target : contacts.get(level)) + if (reassign(issueId, target)) + return Optional.of(target); - public Map<IssueId, MockIssue> issues() { - return Collections.unmodifiableMap(issues); + return Optional.empty(); } - public MockOrganization close(IssueId issueId) { + public MockIssueHandler close(IssueId issueId) { issues.get(issueId).open = false; touch(issueId); return this; } - public MockOrganization setContactsFor(PropertyId propertyId, List<List<User>> contacts) { - properties.get(propertyId).contacts = contacts; - return this; + public Map<IssueId, MockIssue> issues() { + return issues; } - public MockOrganization setPropertyUrl(PropertyId propertyId, URI url) { - properties.get(propertyId).propertyUrl = url; - return this; + private List<List<User>> getContactUsers(Contact contact) { + return contact.persons().stream() + .map(userList -> + userList.stream().map(user -> + user.split(" ")[0]) + .map(User::from) + .collect(Collectors.toList()) + ).collect(Collectors.toList()); } - public MockOrganization setContactsUrl(PropertyId propertyId, URI url) { - properties.get(propertyId).contactsUrl = url; - return this; - } - public MockOrganization setIssueUrl(PropertyId propertyId, URI url) { - properties.get(propertyId).issueUrl = url; - return this; + private void touch(IssueId issueId) { + issues.get(issueId).updated = clock.instant(); } - public MockOrganization addProperty(PropertyId propertyId) { - properties.put(propertyId, new PropertyInfo()); - return this; - } + private class PropertyInfo { + + private List<List<User>> contacts = Collections.emptyList(); + private URI issueUrl = URI.create("issues.tld"); + private URI contactsUrl = URI.create("contacts.tld"); + private URI propertyUrl = URI.create("properties.tld"); - private void touch(IssueId issueId) { - issues.get(issueId).updated = clock.instant(); } public class MockIssue { @@ -165,14 +155,4 @@ public class MockOrganization extends AbstractComponent implements Organization } - private class PropertyInfo { - - private List<List<User>> contacts = Collections.emptyList(); - private URI issueUrl = URI.create("issues.tld"); - private URI contactsUrl = URI.create("contacts.tld"); - private URI propertyUrl = URI.create("properties.tld"); - - } - } - diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/OwnershipIssues.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/OwnershipIssues.java index ee17859c0fb..6a69eb54d2c 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/OwnershipIssues.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/OwnershipIssues.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.api.integration.organization; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import java.util.Optional; @@ -19,31 +18,21 @@ import java.util.Optional; public interface OwnershipIssues { /** - * Ensure ownership of the given application has been recently confirmed by the given property. - * - * @param issueId ID of the previous ownership issue filed for the given application. - * @param applicationId ID of the application for which to file an issue. - * @param propertyId ID of the property responsible for the given application. - * @return ID of the created issue, if one was created. - */ - Optional<IssueId> confirmOwnership(Optional<IssueId> issueId, ApplicationId applicationId, PropertyId propertyId); - - /** * Ensure ownership of the given application has been recently confirmed by the given user. * * @param issueId ID of the previous ownership issue filed for the given application. * @param applicationId ID of the application for which to file an issue. - * @param owner ID of the user responsible for the given application. + * @param asignee Issue asignee + * @param contact Contact info for the application tenant * @return ID of the created issue, if one was created. */ - Optional<IssueId> confirmOwnership(Optional<IssueId> issueId, ApplicationId applicationId, User owner); + Optional<IssueId> confirmOwnership(Optional<IssueId> issueId, ApplicationId applicationId, User asignee, Contact contact); /** * Make sure the given ownership confirmation request is acted upon, unless it is already acknowledged. - * * @param issueId ID of the ownership issue to escalate. - * @param propertyId ID of the property responsible for the issue, if any. + * @param contact Contact information of application tenant */ - void ensureResponse(IssueId issueId, Optional<PropertyId> propertyId); + void ensureResponse(IssueId issueId, Optional<Contact> contact); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/DummyOwnershipIssues.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/DummyOwnershipIssues.java index 6e4761d1cf8..14f252732fb 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/DummyOwnershipIssues.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/DummyOwnershipIssues.java @@ -2,7 +2,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.stubs; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.OwnershipIssues; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; @@ -12,17 +12,12 @@ import java.util.Optional; public class DummyOwnershipIssues implements OwnershipIssues { @Override - public Optional<IssueId> confirmOwnership(Optional<IssueId> issueId, ApplicationId applicationId, PropertyId propertyId) { + public Optional<IssueId> confirmOwnership(Optional<IssueId> issueId, ApplicationId applicationId, User asignee, Contact contact) { return Optional.empty(); } @Override - public Optional<IssueId> confirmOwnership(Optional<IssueId> issueId, ApplicationId applicationId, User owner) { - return Optional.empty(); - } - - @Override - public void ensureResponse(IssueId issueId, Optional<PropertyId> propertyId) { + public void ensureResponse(IssueId issueId, Optional<Contact> contact) { } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/LoggingDeploymentIssues.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/LoggingDeploymentIssues.java index c5efffd979a..5dfbeb5e756 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/LoggingDeploymentIssues.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/LoggingDeploymentIssues.java @@ -6,6 +6,7 @@ import com.google.inject.Inject; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.DeploymentIssues; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; @@ -54,12 +55,7 @@ public class LoggingDeploymentIssues implements DeploymentIssues { } @Override - public IssueId fileUnlessOpen(Optional<IssueId> issueId, ApplicationId applicationId, PropertyId propertyId) { - return fileUnlessPresent(issueId, applicationId); - } - - @Override - public IssueId fileUnlessOpen(Optional<IssueId> issueId, ApplicationId applicationId, User assignee) { + public IssueId fileUnlessOpen(Optional<IssueId> issueId, ApplicationId applicationId, User asignee, Contact contact) { return fileUnlessPresent(issueId, applicationId); } @@ -73,7 +69,7 @@ public class LoggingDeploymentIssues implements DeploymentIssues { } @Override - public void escalateIfInactive(IssueId issueId, Optional<PropertyId> propertyId, Duration maxInactivity) { + public void escalateIfInactive(IssueId issueId, Duration maxInactivity, Optional<Contact> contact) { if (issueUpdates.containsKey(issueId) && issueUpdates.get(issueId).isBefore(clock.instant().minus(maxInactivity))) escalateIssue(issueId); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java index 0c33307bf80..2705debf8ba 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java @@ -13,7 +13,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.MetricsService.Applicat import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationActivity; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index c69fe5021e7..ac317794725 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -29,6 +29,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareRes import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationStore; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ArtifactRepository; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId; import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService; import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordData; @@ -38,7 +39,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingEndpoint import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.JobList; import com.yahoo.vespa.hosted.controller.application.JobStatus; @@ -328,7 +329,7 @@ public class ApplicationController { try { applicationPackage = application.get().deploymentJobs().deployedInternally() - ? new ApplicationPackage(applicationStore.getApplicationPackage(application.get().id(), applicationVersion.id())) + ? new ApplicationPackage(applicationStore.get(application.get().id(), applicationVersion)) : new ApplicationPackage(artifactRepository.getApplicationPackage(application.get().id(), applicationVersion.id())); } catch (RuntimeException e) { // If application has switched deployment pipeline, artifacts stored prior to the switch are in the other artifact store. @@ -336,7 +337,7 @@ public class ApplicationController { + (application.get().deploymentJobs().deployedInternally() ? "internally" : "externally")); applicationPackage = application.get().deploymentJobs().deployedInternally() ? new ApplicationPackage(artifactRepository.getApplicationPackage(application.get().id(), applicationVersion.id())) - : new ApplicationPackage(applicationStore.getApplicationPackage(application.get().id(), applicationVersion.id())); + : new ApplicationPackage(applicationStore.get(application.get().id(), applicationVersion)); } validateRun(application.get(), zone, platformVersion, applicationVersion); } @@ -402,12 +403,9 @@ public class ApplicationController { } } - /** Assembles and deploys a tester application to the given zone. */ - public ActivateResult deployTester(ApplicationId tester, ApplicationPackage applicationPackage, ZoneId zone, DeployOptions options) { - if ( ! tester.instance().isTester()) - throw new IllegalArgumentException("'" + tester + "' is not a tester application!"); - - return deploy(tester, applicationPackage, zone, options, Collections.emptySet(), Collections.emptySet()); + /** Deploys the given tester application to the given zone. */ + public ActivateResult deployTester(TesterId tester, ApplicationPackage applicationPackage, ZoneId zone, DeployOptions options) { + return deploy(tester.id(), applicationPackage, zone, options, Collections.emptySet(), Collections.emptySet()); } private ActivateResult deploy(ApplicationId application, ApplicationPackage applicationPackage, @@ -569,6 +567,8 @@ public class ApplicationController { new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(id.application().value()), token.get()); } curator.removeApplication(id); + applicationStore.removeAll(id); + applicationStore.removeAll(TesterId.of(id)); log.info("Deleted " + application); })); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java index bd10a884213..5951ceb4792 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java @@ -13,7 +13,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.MetricsService.Applicat import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.ClusterInfo; import com.yahoo.vespa.hosted.controller.application.ClusterUtilization; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java index cb3f50d08c7..590b18f929d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java @@ -7,7 +7,9 @@ import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; -import com.yahoo.vespa.hosted.controller.tenant.Contact; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; +import com.yahoo.vespa.hosted.controller.tenant.Tenant; +import com.yahoo.vespa.hosted.controller.tenant.UserTenant; import java.util.Objects; import java.util.Optional; @@ -22,10 +24,11 @@ public class LockedTenant { private final Lock lock; private final TenantName name; - private final AthenzDomain domain; - private final Property property; - private final Optional<PropertyId> propertyId; + private AthenzDomain domain; + private Property property; + private Optional<PropertyId> propertyId; private final Optional<Contact> contact; + private final boolean isAthenzTenant; /** * Should never be constructed directly. @@ -37,6 +40,10 @@ public class LockedTenant { this(lock, tenant.name(), tenant.domain(), tenant.property(), tenant.propertyId(), tenant.contact()); } + LockedTenant(UserTenant tenant, Lock lock) { + this(lock, tenant.name(), tenant.contact()); + } + private LockedTenant(Lock lock, TenantName name, AthenzDomain domain, Property property, Optional<PropertyId> propertyId, Optional<Contact> contact) { this.lock = Objects.requireNonNull(lock, "lock must be non-null"); @@ -45,11 +52,20 @@ public class LockedTenant { this.property = Objects.requireNonNull(property, "property must be non-null"); this.propertyId = Objects.requireNonNull(propertyId, "propertyId must be non-null"); this.contact = Objects.requireNonNull(contact, "contact must be non-null"); + this.isAthenzTenant = true; + } + + private LockedTenant(Lock lock, TenantName name, Optional<Contact> contact) { + this.lock = Objects.requireNonNull(lock, "lock must be non-null"); + this.name = Objects.requireNonNull(name, "name must be non-null"); + this.contact = Objects.requireNonNull(contact, "contact must be non-null"); + this.isAthenzTenant = false; } /** Returns a read-only copy of this */ - public AthenzTenant get() { - return new AthenzTenant(name, domain, property, propertyId, contact); + public Tenant get() { + if (isAthenzTenant) return new AthenzTenant(name, domain, property, propertyId, contact); + else return new UserTenant(name, contact); } public LockedTenant with(AthenzDomain domain) { @@ -65,7 +81,8 @@ public class LockedTenant { } public LockedTenant with(Contact contact) { - return new LockedTenant(lock, name, domain, property, propertyId, Optional.of(contact)); + if (isAthenzTenant) return new LockedTenant(lock, name, domain, property, propertyId, Optional.of(contact)); + return new LockedTenant(lock, name, Optional.of(contact)); } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java index f0e13349fbf..78099fac34e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java @@ -102,7 +102,11 @@ public class TenantController { */ public void lockIfPresent(TenantName name, Consumer<LockedTenant> action) { try (Lock lock = lock(name)) { - athenzTenant(name).map(tenant -> new LockedTenant(tenant, lock)).ifPresent(action); + tenant(name).map(tenant -> { + tenant = tenant instanceof AthenzTenant ? (AthenzTenant) tenant : (UserTenant) tenant; + if (tenant instanceof AthenzTenant) return new LockedTenant((AthenzTenant) tenant, lock); + else return new LockedTenant((UserTenant) tenant, lock); + }).ifPresent(action); } } @@ -173,7 +177,8 @@ public class TenantController { /** Update Athenz domain for tenant. Returns the updated tenant which must be explicitly stored */ public LockedTenant withDomain(LockedTenant tenant, AthenzDomain newDomain, OktaAccessToken token) { - AthenzDomain existingDomain = tenant.get().domain(); + AthenzTenant athenzTenant = (AthenzTenant) tenant.get(); + AthenzDomain existingDomain = athenzTenant.domain(); if (existingDomain.equals(newDomain)) return tenant; Optional<Tenant> existingTenantWithNewDomain = tenantIn(newDomain); if (existingTenantWithNewDomain.isPresent()) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java index 0fb6459611c..ab16f84f628 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.component.Version; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import java.util.Objects; import java.util.Optional; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Deployment.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Deployment.java index 1016a5adf65..ed06ff5bddc 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Deployment.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Deployment.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.component.Version; import com.yahoo.config.provision.ClusterSpec.Id; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import java.time.Instant; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java index 6e9318042b5..7b02a45d23f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.application; import com.google.common.collect.ImmutableMap; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.api.integration.BuildService; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentMetrics.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentMetrics.java index 35bc86cac6c..1ee461cbb8d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentMetrics.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentMetrics.java @@ -1,6 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.application; +import java.time.Instant; +import java.util.Optional; + /** * Metrics for a deployment of an application. * @@ -15,14 +18,21 @@ public class DeploymentMetrics { private final double documentCount; private final double queryLatencyMillis; private final double writeLatencyMills; + private final Optional<Instant> instant; public DeploymentMetrics(double queriesPerSecond, double writesPerSecond, double documentCount, double queryLatencyMillis, double writeLatencyMills) { + this(queriesPerSecond, writesPerSecond, documentCount, queryLatencyMillis, writeLatencyMills, Optional.empty()); + } + + public DeploymentMetrics(double queriesPerSecond, double writesPerSecond, double documentCount, + double queryLatencyMillis, double writeLatencyMills, Optional<Instant> instant) { this.queriesPerSecond = queriesPerSecond; this.writesPerSecond = writesPerSecond; this.documentCount = documentCount; this.queryLatencyMillis = queryLatencyMillis; this.writeLatencyMills = writeLatencyMills; + this.instant = instant; } public double queriesPerSecond() { @@ -45,4 +55,8 @@ public class DeploymentMetrics { return writeLatencyMills; } + public Optional<Instant> instant() { + return instant; + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java index cd15556ba9b..dce40f1c583 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.application; import com.google.common.collect.ImmutableList; import com.yahoo.component.Version; import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.JobStatus.JobRun; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java index 59220d38821..5306cc4ae2b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.component.Version; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import java.time.Instant; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 69a42866a9e..d6182611fd5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -13,7 +13,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.BuildService; import com.yahoo.vespa.hosted.controller.api.integration.BuildService.JobState; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationList; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobReport; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index da1c2ec753b..633490b9299 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -28,7 +28,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.yolean.Exceptions; @@ -156,9 +156,9 @@ public class InternalStepRunner implements StepRunner { private Optional<RunStatus> deployTester(RunId id, DualLogger logger) { // TODO jvenstad: Consider deploying old version of tester for initial staging feeding? logger.log("Deploying the tester container ..."); - return deploy(JobController.testerOf(id.application()), + return deploy(id.tester().id(), id.type(), - () -> controller.applications().deployTester(JobController.testerOf(id.application()), + () -> controller.applications().deployTester(id.tester(), testerPackage(id), id.type().zone(controller.system()), new DeployOptions(true, @@ -263,7 +263,7 @@ public class InternalStepRunner implements StepRunner { } logger.log("Checking installation of tester container ..."); - if (servicesConverged(JobController.testerOf(id.application()), id.type(), logger)) { + if (servicesConverged(id.tester().id(), id.type(), logger)) { logger.log("Tester container successfully installed!"); return Optional.of(running); } @@ -409,7 +409,7 @@ public class InternalStepRunner implements StepRunner { private Optional<RunStatus> deactivateTester(RunId id, DualLogger logger) { logger.log("Deactivating tester of " + id.application() + " in " + id.type().zone(controller.system()) + " ..."); - controller.jobController().deactivateTester(id.application(), id.type()); + controller.jobController().deactivateTester(id.tester(), id.type()); return Optional.of(running); } @@ -452,7 +452,7 @@ public class InternalStepRunner implements StepRunner { private ApplicationPackage testerPackage(RunId id) { ApplicationVersion version = controller.jobController().run(id).get().versions().targetApplication(); - byte[] testPackage = controller.applications().applicationStore().getTesterPackage(JobController.testerOf(id.application()), version.id()); + byte[] testPackage = controller.applications().applicationStore().get(id.tester(), version); byte[] servicesXml = servicesXml(controller.system()); DeploymentSpec spec = controller.applications().require(id.application()).deploymentSpec(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index a75c3d2f8f3..480e7b3b0fe 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -13,17 +13,19 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.NoInstance import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.JobStatus; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.persistence.BufferedLogStore; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import java.net.URI; import java.util.Collections; +import java.util.Comparator; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -225,26 +227,28 @@ public class JobController { AtomicReference<ApplicationVersion> version = new AtomicReference<>(); controller.applications().lockOrThrow(id, application -> { if ( ! application.get().deploymentJobs().deployedInternally()) { + // TODO jvenstad: Remove when there are no more SDv3 pipelines. // Copy all current packages to the new application store application.get().deployments().values().stream() .map(Deployment::applicationVersion) .distinct() .forEach(appVersion -> { byte[] content = controller.applications().artifacts().getApplicationPackage(application.get().id(), appVersion.id()); - controller.applications().applicationStore().putApplicationPackage(application.get().id(), appVersion.id(), content); + controller.applications().applicationStore().put(application.get().id(), appVersion, content); }); } long run = nextBuild(id); version.set(ApplicationVersion.from(revision, run)); - controller.applications().applicationStore().putApplicationPackage(id, - version.get().id(), - packageBytes); - controller.applications().applicationStore().putTesterPackage(testerOf(id), - version.get().id(), - testPackageBytes); + controller.applications().applicationStore().put(id, + version.get(), + packageBytes); + controller.applications().applicationStore().put(TesterId.of(id), + version.get(), + testPackageBytes); + prunePackages(id); controller.applications().storeWithUpdatedConfig(application.withBuiltInternally(true), new ApplicationPackage(packageBytes)); notifyOfNewSubmission(id, projectId, revision, run); @@ -277,48 +281,42 @@ public class JobController { }); } - /** Deletes stale data and tester deployments for applications which are unknown, or no longer built internally. */ + /** Deletes run data, packages and tester deployments for applications which are unknown, or no longer built internally. */ public void collectGarbage() { Set<ApplicationId> applicationsToBuild = new HashSet<>(applications()); curator.applicationsWithJobs().stream() .filter(id -> ! applicationsToBuild.contains(id)) .forEach(id -> { try { + TesterId tester = TesterId.of(id); for (JobType type : jobs(id)) locked(id, type, deactivateTester, __ -> { try (Lock ___ = curator.lock(id, type)) { - deactivateTester(id, type); + deactivateTester(tester, type); curator.deleteRunData(id, type); logs.delete(id); } }); } catch (TimeoutException e) { - return; // Don't remove the data if we couldn't deactivate all testers. + return; // Don't remove the data if we couldn't clean up all resources. } curator.deleteRunData(id); }); } - public void deactivateTester(ApplicationId id, JobType type) { + public void deactivateTester(TesterId id, JobType type) { try { - controller.configServer().deactivate(new DeploymentId(testerOf(id), type.zone(controller.system()))); + controller.configServer().deactivate(new DeploymentId(id.id(), type.zone(controller.system()))); } catch (NoInstanceException ignored) { // Already gone -- great! } } - /** Returns the application id of the tester application for the real application with the given id. */ - public static ApplicationId testerOf(ApplicationId id) { - return ApplicationId.from(id.tenant().value(), - id.application().value(), - id.instance().value() + "-t"); - } - /** Returns a URI of the tester endpoint retrieved from the routing generator, provided it matches an expected form. */ Optional<URI> testerEndpoint(RunId id) { - ApplicationId tester = testerOf(id.application()); + ApplicationId tester = id.tester().id(); return controller.applications().getDeploymentEndpoints(new DeploymentId(tester, id.type().zone(controller.system()))) .flatMap(uris -> uris.stream() .filter(uri -> uri.getHost().contains(String.format("%s--%s--%s.", @@ -348,6 +346,18 @@ public class JobController { controller.applications().deploymentTrigger().notifyOfCompletion(report); } + private void prunePackages(ApplicationId id) { + controller.applications().lockIfPresent(id, application -> { + application.get().deployments().values().stream() + .map(Deployment::applicationVersion) + .min(Comparator.comparingLong(applicationVersion -> applicationVersion.buildNumber().getAsLong())) + .ifPresent(oldestDeployed -> { + controller.applications().applicationStore().prune(id, oldestDeployed); + controller.applications().applicationStore().prune(TesterId.of(id), oldestDeployed); + }); + }); + } + /** Locks and modifies the list of historic runs for the given application and job type. */ private void locked(ApplicationId id, JobType type, Consumer<SortedMap<RunId, Run>> modifications) { try (Lock __ = curator.lock(id, type)) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java index f146dd57885..b6b452bf606 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.yahoo.component.Version; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.JobStatus; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java index 219517cfd30..e260848d93a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.OwnershipIssues; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; @@ -14,7 +14,6 @@ import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.yolean.Exceptions; import java.time.Duration; -import java.util.NoSuchElementException; import java.util.Optional; import java.util.logging.Level; @@ -52,9 +51,7 @@ public class ApplicationOwnershipConfirmer extends Maintainer { try { Tenant tenant = ownerOf(application.id()); Optional<IssueId> ourIssueId = application.ownershipIssueId(); - ourIssueId = tenant instanceof AthenzTenant - ? ownershipIssues.confirmOwnership(ourIssueId, application.id(), propertyIdFor((AthenzTenant) tenant)) - : ownershipIssues.confirmOwnership(ourIssueId, application.id(), userFor(tenant)); + ourIssueId = ownershipIssues.confirmOwnership(ourIssueId, application.id(), userFor(tenant), tenant.contact().orElseThrow(RuntimeException::new)); ourIssueId.ifPresent(issueId -> store(issueId, application.id())); } catch (RuntimeException e) { // Catch errors due to wrong data in the controller, or issues client timeout. @@ -69,12 +66,11 @@ public class ApplicationOwnershipConfirmer extends Maintainer { for (Application application : controller().applications().asList()) application.ownershipIssueId().ifPresent(issueId -> { try { - Optional<PropertyId> propertyId = Optional.of(application.id()) - .map(this::ownerOf) - .filter(t -> t instanceof AthenzTenant) - .map(AthenzTenant.class::cast) - .flatMap(AthenzTenant::propertyId); - ownershipIssues.ensureResponse(issueId, propertyId); + Optional<Contact> contact = Optional.of(application.id()) + .map(this::ownerOf) + .filter(t -> t instanceof AthenzTenant) + .flatMap(Tenant::contact); + ownershipIssues.ensureResponse(issueId, contact); } catch (RuntimeException e) { log.log(Level.INFO, "Exception caught when attempting to escalate issue with id '" + issueId + "': " + Exceptions.toMessageString(e)); @@ -91,12 +87,6 @@ public class ApplicationOwnershipConfirmer extends Maintainer { return User.from(tenant.name().value().replaceFirst(Tenant.userPrefix, "")); } - protected PropertyId propertyIdFor(AthenzTenant tenant) { - return tenant.propertyId() - .orElseThrow(() -> new NoSuchElementException("No PropertyId is listed for non-user tenant " + - tenant)); - } - protected void store(IssueId issueId, ApplicationId applicationId) { controller().applications().lockIfPresent(applicationId, application -> controller().applications().store(application.withOwnershipIssueId(issueId))); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java index bf5743e2d3c..8bffa455b7e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainer.java @@ -1,23 +1,21 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.maintenance; +import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; +import com.yahoo.vespa.hosted.controller.api.integration.organization.ContactRetriever; import com.yahoo.config.provision.SystemName; import com.yahoo.log.LogLevel; import com.yahoo.vespa.hosted.controller.Controller; -import com.yahoo.vespa.hosted.controller.api.integration.organization.Organization; -import com.yahoo.vespa.hosted.controller.api.integration.organization.User; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; -import com.yahoo.vespa.hosted.controller.tenant.Contact; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.yolean.Exceptions; import java.time.Duration; import java.util.EnumSet; -import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.logging.Logger; -import java.util.stream.Collectors; /** * Periodically fetch and store contact information for tenants. @@ -28,23 +26,20 @@ public class ContactInformationMaintainer extends Maintainer { private static final Logger log = Logger.getLogger(ContactInformationMaintainer.class.getName()); - private final Organization organization; + private final ContactRetriever contactRetriever; - public ContactInformationMaintainer(Controller controller, Duration interval, JobControl jobControl, Organization organization) { + public ContactInformationMaintainer(Controller controller, Duration interval, JobControl jobControl, ContactRetriever contactRetriever) { super(controller, interval, jobControl, null, EnumSet.of(SystemName.cd, SystemName.main)); - this.organization = Objects.requireNonNull(organization, "organization must be non-null"); + this.contactRetriever = Objects.requireNonNull(contactRetriever, "organization must be non-null"); } @Override protected void maintain() { - for (Tenant t : controller().tenants().asList()) { - if (!(t instanceof AthenzTenant)) continue; // No contact information for non-Athenz tenants - AthenzTenant tenant = (AthenzTenant) t; - if (!tenant.propertyId().isPresent()) continue; // Can only update contact information if property ID is known - try { - findContact(tenant).ifPresent(contact -> { - controller().tenants().lockIfPresent(t.name(), lockedTenant -> controller().tenants().store(lockedTenant.with(contact))); - }); + for (Tenant tenant : controller().tenants().asList()) { + try{ + Optional<PropertyId> tenantPropertyId = tenant instanceof AthenzTenant ? ((AthenzTenant) tenant).propertyId() : Optional.empty(); + Contact contact = contactRetriever.getContact(tenantPropertyId); + controller().tenants().lockIfPresent(tenant.name(), lockedTenant -> controller().tenants().store(lockedTenant.with(contact))); } catch (Exception e) { log.log(LogLevel.WARNING, "Failed to update contact information for " + tenant + ": " + Exceptions.toMessageString(e) + ". Retrying in " + @@ -53,21 +48,5 @@ public class ContactInformationMaintainer extends Maintainer { } } - /** Find contact information for given tenant */ - private Optional<Contact> findContact(AthenzTenant tenant) { - if (!tenant.propertyId().isPresent()) { - return Optional.empty(); - } - List<List<String>> persons = organization.contactsFor(tenant.propertyId().get()) - .stream() - .map(personList -> personList.stream() - .map(User::displayName) - .collect(Collectors.toList())) - .collect(Collectors.toList()); - return Optional.of(new Contact(organization.contactsUri(tenant.propertyId().get()), - organization.propertyUri(tenant.propertyId().get()), - organization.issueCreationUri(tenant.propertyId().get()), - persons)); - } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java index 7b17f38bd78..f6978ef70ac 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java @@ -4,12 +4,12 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.AbstractComponent; import com.yahoo.jdisc.Metric; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.integration.organization.ContactRetriever; import com.yahoo.vespa.hosted.controller.authority.config.ApiAuthorityConfig; import com.yahoo.vespa.hosted.controller.api.integration.chef.Chef; import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryClientInterface; import com.yahoo.vespa.hosted.controller.api.integration.organization.DeploymentIssues; -import com.yahoo.vespa.hosted.controller.api.integration.organization.Organization; import com.yahoo.vespa.hosted.controller.api.integration.organization.OwnershipIssues; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.maintenance.config.MaintainerConfig; @@ -54,7 +54,7 @@ public class ControllerMaintenance extends AbstractComponent { JobControl jobControl, Metric metric, Chef chefClient, DeploymentIssues deploymentIssues, OwnershipIssues ownershipIssues, NameService nameService, NodeRepositoryClientInterface nodeRepositoryClient, - Organization organization) { + ContactRetriever contactRetriever) { Duration maintenanceInterval = Duration.ofMinutes(maintainerConfig.intervalMinutes()); this.jobControl = jobControl; deploymentExpirer = new DeploymentExpirer(controller, maintenanceInterval, jobControl); @@ -73,7 +73,7 @@ public class ControllerMaintenance extends AbstractComponent { jobRunner = new JobRunner(controller, Duration.ofMinutes(2), jobControl); osUpgraders = osUpgraders(controller, jobControl); osVersionStatusUpdater = new OsVersionStatusUpdater(controller, maintenanceInterval, jobControl); - contactInformationMaintainer = new ContactInformationMaintainer(controller, Duration.ofHours(12), jobControl, organization); + contactInformationMaintainer = new ContactInformationMaintainer(controller, Duration.ofHours(12), jobControl, contactRetriever); } public Upgrader upgrader() { return upgrader; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java index 1745e013a40..13733b32d86 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java @@ -7,6 +7,7 @@ import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.DeploymentIssues; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; @@ -18,7 +19,6 @@ import com.yahoo.yolean.Exceptions; import java.time.Duration; import java.util.Collection; import java.util.List; -import java.util.NoSuchElementException; import java.util.Optional; import java.util.Set; import java.util.logging.Level; @@ -113,20 +113,13 @@ public class DeploymentIssueReporter extends Maintainer { return User.from(tenant.name().value().replaceFirst(Tenant.userPrefix, "")); } - private PropertyId propertyIdFor(AthenzTenant tenant) { - return tenant.propertyId() - .orElseThrow(() -> new NoSuchElementException("No PropertyId is listed for non-user tenant " + - tenant)); - } - /** File an issue for applicationId, if it doesn't already have an open issue associated with it. */ private void fileDeploymentIssueFor(ApplicationId applicationId) { try { Tenant tenant = ownerOf(applicationId); + User asignee = userFor(tenant); Optional<IssueId> ourIssueId = controller().applications().require(applicationId).deploymentJobs().issueId(); - IssueId issueId = tenant instanceof AthenzTenant - ? deploymentIssues.fileUnlessOpen(ourIssueId, applicationId, propertyIdFor((AthenzTenant) tenant)) - : deploymentIssues.fileUnlessOpen(ourIssueId, applicationId, userFor(tenant)); + IssueId issueId = deploymentIssues.fileUnlessOpen(ourIssueId, applicationId, asignee, tenant.contact().get()); store(applicationId, issueId); } catch (RuntimeException e) { // Catch errors due to wrong data in the controller, or issues client timeout. @@ -138,12 +131,11 @@ public class DeploymentIssueReporter extends Maintainer { private void escalateInactiveDeploymentIssues(Collection<Application> applications) { applications.forEach(application -> application.deploymentJobs().issueId().ifPresent(issueId -> { try { - Optional<PropertyId> propertyId = Optional.of(application.id()) - .map(this::ownerOf) - .filter(t -> t instanceof AthenzTenant) - .map(AthenzTenant.class::cast) - .flatMap(AthenzTenant::propertyId); - deploymentIssues.escalateIfInactive(issueId, propertyId, maxInactivity); + AthenzTenant tenant = Optional.of(application.id()) + .map(this::ownerOf) + .filter(t -> t instanceof AthenzTenant) + .map(AthenzTenant.class::cast).orElseThrow(RuntimeException::new); + deploymentIssues.escalateIfInactive(issueId, maxInactivity, tenant.contact()); } catch (RuntimeException e) { log.log(Level.INFO, "Exception caught when attempting to escalate issue with id '" + issueId + "': " + Exceptions.toMessageString(e)); @@ -155,5 +147,4 @@ public class DeploymentIssueReporter extends Maintainer { controller().applications().lockIfPresent(id, application -> controller().applications().store(application.withDeploymentIssueId(issueId))); } - } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java index e324a76e1cf..56dbe9212b7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java @@ -16,6 +16,7 @@ import java.time.Duration; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.TreeMap; import java.util.concurrent.ForkJoinPool; import java.util.concurrent.TimeUnit; @@ -73,7 +74,8 @@ public class DeploymentMetricsMaintainer extends Maintainer { deploymentMetrics.writesPerSecond(), deploymentMetrics.documentCount(), deploymentMetrics.queryLatencyMillis(), - deploymentMetrics.writeLatencyMillis()); + deploymentMetrics.writeLatencyMillis(), + Optional.of(controller().clock().instant())); applications.lockIfPresent(application.id(), locked -> applications.store(locked.with(deployment.zone(), newMetrics) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 365d74babb5..c69e77a43e1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -17,7 +17,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.MetricsService.Applicat import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.ClusterInfo; import com.yahoo.vespa.hosted.controller.application.ClusterUtilization; @@ -28,7 +28,7 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.RotationStatus; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.rotation.RotationId; import java.time.Instant; @@ -130,6 +130,7 @@ public class ApplicationSerializer { private final String deploymentMetricsDocsField = "documentCount"; private final String deploymentMetricsQueryLatencyField = "queryLatencyMillis"; private final String deploymentMetricsWriteLatencyField = "writeLatencyMillis"; + private final String deploymentMetricsUpdateTime = "lastUpdated"; // ------------------ Serialization @@ -178,6 +179,7 @@ public class ApplicationSerializer { root.setDouble(deploymentMetricsDocsField, metrics.documentCount()); root.setDouble(deploymentMetricsQueryLatencyField, metrics.queryLatencyMillis()); root.setDouble(deploymentMetricsWriteLatencyField, metrics.writeLatencyMillis()); + metrics.instant().ifPresent(instant -> root.setLong(deploymentMetricsUpdateTime, instant.toEpochMilli())); } private void clusterInfoToSlime(Map<ClusterSpec.Id, ClusterInfo> clusters, Cursor object) { @@ -329,11 +331,15 @@ public class ApplicationSerializer { } private DeploymentMetrics deploymentMetricsFromSlime(Inspector object) { + Optional<Instant> instant = object.field(deploymentMetricsUpdateTime).valid() ? + Optional.of(Instant.ofEpochMilli(object.field(deploymentMetricsUpdateTime).asLong())) : + Optional.empty(); return new DeploymentMetrics(object.field(deploymentMetricsQPSField).asDouble(), object.field(deploymentMetricsWPSField).asDouble(), object.field(deploymentMetricsDocsField).asDouble(), object.field(deploymentMetricsQueryLatencyField).asDouble(), - object.field(deploymentMetricsWriteLatencyField).asDouble()); + object.field(deploymentMetricsWriteLatencyField).asDouble(), + instant); } private Map<HostName, RotationStatus> rotationStatusFromSlime(Inspector object) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 631aaa5a909..9c52d7a2244 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -280,7 +280,7 @@ public class CuratorDb { // -------------- Tenant -------------------------------------------------- - public void writeTenant(UserTenant tenant) { + public void writeTenant(Tenant tenant) { curator.set(tenantPath(tenant.name()), asJson(tenantSerializer.toSlime(tenant))); } @@ -288,10 +288,6 @@ public class CuratorDb { return readSlime(tenantPath(name)).map(tenantSerializer::userTenantFrom); } - public void writeTenant(AthenzTenant tenant) { - curator.set(tenantPath(tenant.name()), asJson(tenantSerializer.toSlime(tenant))); - } - public Optional<AthenzTenant> readAthenzTenant(TenantName name) { return readSlime(tenantPath(name)).map(tenantSerializer::athenzTenantFrom); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java index 5103dc244ed..0e5041215cb 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java @@ -10,8 +10,8 @@ import com.yahoo.slime.ObjectTraverser; import com.yahoo.slime.Slime; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.RunStatus; import com.yahoo.vespa.hosted.controller.deployment.Step; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java index 28400b85306..245cb0f4dae 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java @@ -12,7 +12,8 @@ import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; -import com.yahoo.vespa.hosted.controller.tenant.Contact; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; +import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.tenant.UserTenant; import java.net.URI; @@ -37,8 +38,15 @@ public class TenantSerializer { private static final String issueTrackerUrlField = "issueTrackerUrl"; private static final String personsField = "persons"; private static final String personField = "person"; + private static final String queueField = "queue"; + private static final String componentField = "component"; - public Slime toSlime(AthenzTenant tenant) { + public Slime toSlime(Tenant tenant) { + if (tenant instanceof AthenzTenant) return toSlime((AthenzTenant) tenant); + return toSlime((UserTenant) tenant); + } + + private Slime toSlime(AthenzTenant tenant) { Slime slime = new Slime(); Cursor root = slime.setObject(); root.setString(nameField, tenant.name().value()); @@ -46,26 +54,20 @@ public class TenantSerializer { root.setString(propertyField, tenant.property().id()); tenant.propertyId().ifPresent(propertyId -> root.setString(propertyIdField, propertyId.id())); tenant.contact().ifPresent(contact -> { - Cursor contactObject = root.setObject(contactField); - contactObject.setString(contactUrlField, contact.url().toString()); - contactObject.setString(propertyUrlField, contact.propertyUrl().toString()); - contactObject.setString(issueTrackerUrlField, contact.issueTrackerUrl().toString()); - Cursor personsArray = contactObject.setArray(personsField); - contact.persons().forEach(personList -> { - Cursor personArray = personsArray.addArray(); - personList.forEach(person -> { - Cursor personObject = personArray.addObject(); - personObject.setString(personField, person); - }); - }); + Cursor contactCursor = root.setObject(contactField); + writeContact(contact, contactCursor); }); return slime; } - public Slime toSlime(UserTenant tenant) { + private Slime toSlime(UserTenant tenant) { Slime slime = new Slime(); Cursor root = slime.setObject(); root.setString(nameField, tenant.name().value()); + tenant.contact().ifPresent(contact -> { + Cursor contactCursor = root.setObject(contactField); + writeContact(contact, contactCursor); + }); return slime; } @@ -82,17 +84,42 @@ public class TenantSerializer { public UserTenant userTenantFrom(Slime slime) { Inspector root = slime.get(); TenantName name = TenantName.from(root.field(nameField).asString()); - return new UserTenant(name); + Optional<Contact> contact = contactFrom(root.field(contactField)); + return new UserTenant(name, contact); } private Optional<Contact> contactFrom(Inspector object) { if (!object.valid()) { return Optional.empty(); } - return Optional.of(new Contact(URI.create(object.field(contactUrlField).asString()), - URI.create(object.field(propertyUrlField).asString()), - URI.create(object.field(issueTrackerUrlField).asString()), - personsFrom(object.field(personsField)))); + URI contactUrl = URI.create(object.field(contactUrlField).asString()); + URI propertyUrl = URI.create(object.field(propertyUrlField).asString()); + URI issueTrackerUrl = URI.create(object.field(issueTrackerUrlField).asString()); + List<List<String>> persons = personsFrom(object.field(personsField)); + String queue = object.field(queueField).asString(); + Optional<String> component = object.field(componentField).valid() ? Optional.of(object.field(componentField).asString()) : Optional.empty(); + return Optional.of(new Contact(contactUrl, + propertyUrl, + issueTrackerUrl, + persons, + queue, + component)); + } + + private void writeContact(Contact contact, Cursor contactCursor) { + contactCursor.setString(contactUrlField, contact.url().toString()); + contactCursor.setString(propertyUrlField, contact.propertyUrl().toString()); + contactCursor.setString(issueTrackerUrlField, contact.issueTrackerUrl().toString()); + Cursor personsArray = contactCursor.setArray(personsField); + contact.persons().forEach(personList -> { + Cursor personArray = personsArray.addArray(); + personList.forEach(person -> { + Cursor personObject = personArray.addObject(); + personObject.setString(personField, person); + }); + }); + contactCursor.setString(queueField, contact.queue()); + contact.component().ifPresent(component -> contactCursor.setString(componentField, component)); } private List<List<String>> personsFrom(Inspector array) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index dac65f16832..3a83ecfb9e2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -53,7 +53,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.ClusterCost; import com.yahoo.vespa.hosted.controller.application.ClusterUtilization; @@ -63,7 +63,7 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.RotationStatus; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.athenz.impl.ZmsClientFacade; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.restapi.ErrorResponse; @@ -97,7 +97,6 @@ import java.util.Map; import java.util.Optional; import java.util.Scanner; import java.util.logging.Level; -import java.util.stream.Collectors; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel.ALL; import static java.util.stream.Collectors.joining; @@ -563,6 +562,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { metricsObject.setDouble("documentCount", metrics.documentCount()); metricsObject.setDouble("queryLatencyMillis", metrics.queryLatencyMillis()); metricsObject.setDouble("writeLatencyMillis", metrics.writeLatencyMillis()); + metrics.instant().ifPresent(instant -> metricsObject.setLong("lastUpdated", instant.toEpochMilli())); } private void toSlime(ApplicationVersion applicationVersion, Cursor object) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index fa5d713d6b3..21640481e86 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -15,11 +15,11 @@ import com.yahoo.vespa.hosted.controller.NotExistsException; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.JobStatus; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.deployment.DeploymentSteps; import com.yahoo.vespa.hosted.controller.deployment.JobController; import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/contactinfo/ContactInfoHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/contactinfo/ContactInfoHandler.java index 7651381b810..cf9db8fd992 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/contactinfo/ContactInfoHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/contactinfo/ContactInfoHandler.java @@ -17,7 +17,7 @@ import com.yahoo.vespa.hosted.controller.restapi.ErrorResponse; import com.yahoo.vespa.hosted.controller.restapi.SlimeJsonResponse; import com.yahoo.vespa.hosted.controller.restapi.StringResponse; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; -import com.yahoo.vespa.hosted.controller.tenant.Contact; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.yolean.Exceptions; import java.io.IOException; @@ -117,6 +117,8 @@ public class ContactInfoHandler extends LoggingRequestHandler { sublist.addString(person); } } + cursor.setString("queue", contact.queue()); + contact.component().ifPresent(component -> cursor.setString("component", component)); return slime; } @@ -125,6 +127,8 @@ public class ContactInfoHandler extends LoggingRequestHandler { URI propertyUrl = URI.create(inspector.field("propertyUrl").asString()); URI url = URI.create(inspector.field("url").asString()); URI issueTrackerUrl = URI.create(inspector.field("issueTrackerUrl").asString()); + String queue = inspector.field("queue").asString(); + Optional<String> component = inspector.field("component").valid() ? Optional.of(inspector.field("component").asString()) : Optional.empty(); Inspector personInspector = inspector.field("persons"); List<List<String>> personList = new ArrayList<>(); personInspector.traverse((ArrayTraverser) (index, entry) -> { @@ -134,7 +138,7 @@ public class ContactInfoHandler extends LoggingRequestHandler { }); personList.add(subList); }); - return new Contact(url, propertyUrl, issueTrackerUrl, personList); + return new Contact(url, propertyUrl, issueTrackerUrl, personList, queue, component); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/AthenzTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/AthenzTenant.java index 8cbb4e06aca..3879e7f29ca 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/AthenzTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/AthenzTenant.java @@ -5,6 +5,7 @@ import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import java.util.Objects; import java.util.Optional; @@ -19,7 +20,7 @@ public class AthenzTenant extends Tenant { private final AthenzDomain domain; private final Property property; private final Optional<PropertyId> propertyId; - private final Optional<Contact> contact; + /** * This should only be used by serialization. @@ -27,11 +28,10 @@ public class AthenzTenant extends Tenant { * */ public AthenzTenant(TenantName name, AthenzDomain domain, Property property, Optional<PropertyId> propertyId, Optional<Contact> contact) { - super(name); + super(name, Objects.requireNonNull(contact, "contact must be non-null")); this.domain = Objects.requireNonNull(domain, "domain must be non-null"); this.property = Objects.requireNonNull(property, "property must be non-null"); this.propertyId = Objects.requireNonNull(propertyId, "propertyId must be non-null"); - this.contact = Objects.requireNonNull(contact, "contact must be non-null"); } /** Property name of this tenant */ @@ -44,11 +44,6 @@ public class AthenzTenant extends Tenant { return propertyId; } - /** Contact information for this, if any */ - public Optional<Contact> contact() { - return contact; - } - /** Athenz domain of this tenant */ public AthenzDomain domain() { return domain; @@ -70,6 +65,11 @@ public class AthenzTenant extends Tenant { return new AthenzTenant(requireName(requireNoPrefix(name)), domain, property, propertyId, Optional.empty()); } + public static AthenzTenant create(TenantName name, AthenzDomain domain, Property property, + Optional<PropertyId> propertyId, Optional<Contact> contact) { + return new AthenzTenant(requireName(requireNoPrefix(name)), domain, property, propertyId, contact); + } + private static TenantName requireNoPrefix(TenantName name) { if (name.value().startsWith(Tenant.userPrefix)) { throw new IllegalArgumentException("Athenz tenant name cannot have prefix '" + Tenant.userPrefix + "'"); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Tenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Tenant.java index aac3fa20d11..98950ca2632 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Tenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Tenant.java @@ -3,8 +3,11 @@ package com.yahoo.vespa.hosted.controller.tenant; import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; +import javax.swing.text.html.Option; import java.util.Objects; +import java.util.Optional; /** * A tenant in hosted Vespa. @@ -17,15 +20,31 @@ public abstract class Tenant { private final TenantName name; - Tenant(TenantName name) { + private Optional<Contact> contact; + + Tenant(TenantName name, Optional<Contact> contact) { this.name = name; + this.contact = contact; } + /*Tenant(TenantName name) { + this(name, Optional.empty()); + }*/ + /** Name of this tenant */ public TenantName name() { return name; } + public Optional<Contact> contact() { + return contact; + } + + public Tenant withContact(Optional<Contact> contact) { + this.contact = contact; + return this; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/UserTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/UserTenant.java index e110600639b..47e5580fbe4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/UserTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/tenant/UserTenant.java @@ -2,6 +2,9 @@ package com.yahoo.vespa.hosted.controller.tenant; import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; + +import java.util.Optional; /** * Represents an user tenant in hosted Vespa. @@ -14,8 +17,12 @@ public class UserTenant extends Tenant { * This should only be used by serialization. * Use {@link #create(String)}. * */ + public UserTenant(TenantName name, Optional<Contact> contact) { + super(name, contact); + } + public UserTenant(TenantName name) { - super(name); + super(name, Optional.empty()); } /** Returns true if this is the tenant for the given user name */ @@ -34,6 +41,11 @@ public class UserTenant extends Tenant { return new UserTenant(requireName(requireUser(name))); } + public static UserTenant create(String username, Optional<Contact> contact) { + TenantName name = TenantName.from(username); + return new UserTenant(requireName(requireUser(name)), contact); + } + /** Normalize given username. E.g. foo_bar becomes by-foo-bar */ public static String normalizeUser(String username) { int offset = 0; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 3351efff2dd..7a5c6ae1170 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -19,11 +19,11 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.Record; import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; import com.yahoo.vespa.hosted.controller.application.JobStatus; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.BuildJob; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index 8994c68acf3..639511959df 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -23,7 +23,9 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.MemoryNameService; import com.yahoo.vespa.hosted.controller.api.integration.entity.EntityService; import com.yahoo.vespa.hosted.controller.api.integration.entity.MemoryEntityService; import com.yahoo.vespa.hosted.controller.api.integration.github.GitHubMock; -import com.yahoo.vespa.hosted.controller.api.integration.organization.MockOrganization; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; +import com.yahoo.vespa.hosted.controller.api.integration.organization.MockContactRetriever; +import com.yahoo.vespa.hosted.controller.api.integration.organization.MockIssueHandler; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockBuildService; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockRunDataStore; @@ -78,7 +80,8 @@ public final class ControllerTester { private final MockBuildService buildService; private final MetricsServiceMock metricsService; private final RoutingGeneratorMock routingGenerator; - private final MockOrganization organization; + private final MockContactRetriever contactRetriever; + private final MockIssueHandler issueHandler; private Controller controller; @@ -88,7 +91,7 @@ public final class ControllerTester { new ZoneRegistryMock(), new GitHubMock(), curatorDb, rotationsConfig, new MemoryNameService(), new ArtifactRepositoryMock(), new ApplicationStoreMock(), new MemoryEntityService(), new MockBuildService(), - metricsService, new RoutingGeneratorMock(), new MockOrganization(clock)); + metricsService, new RoutingGeneratorMock(), new MockContactRetriever(), new MockIssueHandler(clock)); } public ControllerTester(ManualClock clock) { @@ -114,7 +117,7 @@ public final class ControllerTester { ApplicationStoreMock appStoreMock, EntityService entityService, MockBuildService buildService, MetricsServiceMock metricsService, RoutingGeneratorMock routingGenerator, - MockOrganization organization) { + MockContactRetriever contactRetriever, MockIssueHandler issueHandler) { this.athenzDb = athenzDb; this.clock = clock; this.configServer = configServer; @@ -129,7 +132,8 @@ public final class ControllerTester { this.buildService = buildService; this.metricsService = metricsService; this.routingGenerator = routingGenerator; - this.organization = organization; + this.contactRetriever = contactRetriever; + this.issueHandler = issueHandler; this.controller = createController(curator, rotationsConfig, configServer, clock, gitHub, zoneRegistry, athenzDb, nameService, artifactRepository, appStoreMock, entityService, buildService, metricsService, routingGenerator); @@ -178,8 +182,8 @@ public final class ControllerTester { public RoutingGeneratorMock routingGenerator() { return routingGenerator; } - public MockOrganization organization() { - return organization; + public MockContactRetriever contactRetriever() { + return contactRetriever; } /** Create a new controller instance. Useful to verify that controller state is rebuilt from persistence */ @@ -240,19 +244,23 @@ public final class ControllerTester { return domain; } - public TenantName createTenant(String tenantName, String domainName, Long propertyId) { + public TenantName createTenant(String tenantName, String domainName, Long propertyId, Optional<Contact> contact) { TenantName name = TenantName.from(tenantName); Optional<Tenant> existing = controller().tenants().tenant(name); if (existing.isPresent()) return name; AthenzTenant tenant = AthenzTenant.create(name, createDomain(domainName), new Property("app1Property"), Optional.ofNullable(propertyId) .map(Object::toString) - .map(PropertyId::new)); + .map(PropertyId::new), contact); controller().tenants().create(tenant, new OktaAccessToken("okta-token")); assertNotNull(controller().tenants().tenant(name)); return name; } + public TenantName createTenant(String tenantName, String domainName, Long propertyId) { + return createTenant(tenantName, domainName, propertyId, Optional.empty()); + } + public Application createApplication(TenantName tenant, String applicationName, String instanceName, long projectId) { ApplicationId applicationId = ApplicationId.from(tenant.value(), applicationName, instanceName); controller().applications().createApplication(applicationId, Optional.of(new OktaAccessToken("okta-token"))); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java index 0a36787e8f7..8c58e491bc4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java @@ -6,9 +6,9 @@ import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.integration.ArtifactRepositoryMock; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import java.util.Objects; import java.util.Optional; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index db58ef1830f..c75e5137bb9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -2,9 +2,7 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.yahoo.component.Version; -import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import com.yahoo.slime.Slime; import com.yahoo.test.ManualClock; @@ -18,11 +16,10 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockBuildService; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger.ChangesToCancel; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.maintenance.JobControl; import com.yahoo.vespa.hosted.controller.maintenance.ReadyJobsTrigger; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; @@ -34,7 +31,6 @@ import java.nio.file.Paths; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; import java.util.List; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java index a38bd1caf24..9e8ee16e0c1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java @@ -13,11 +13,12 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingEndpoint; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockTesterCloud; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; import com.yahoo.vespa.hosted.controller.integration.RoutingGeneratorMock; import com.yahoo.vespa.hosted.controller.maintenance.JobControl; @@ -29,7 +30,6 @@ import java.util.Collections; import java.util.Optional; import java.util.logging.Logger; -import static com.yahoo.vespa.hosted.controller.deployment.JobController.testerOf; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.aborted; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.unfinished; import static org.junit.Assert.assertEquals; @@ -45,6 +45,7 @@ public class InternalDeploymentTester { .parallel("us-west-1", "us-east-3") .build(); public static final ApplicationId appId = ApplicationId.from("tenant", "application", "default"); + public static final TesterId testerId = TesterId.of(appId); public static final String athenzDomain = "domain"; private final DeploymentTester tester; @@ -87,8 +88,8 @@ public class InternalDeploymentTester { */ public ApplicationVersion newSubmission() { ApplicationVersion version = jobs.submit(appId, BuildJob.defaultSourceRevision, 2, applicationPackage.zippedContent(), new byte[0]); - tester.applicationStore().putApplicationPackage(appId, version.id(), applicationPackage.zippedContent()); - tester.applicationStore().putTesterPackage(testerOf(appId), version.id(), new byte[0]); + tester.applicationStore().put(appId, version, applicationPackage.zippedContent()); + tester.applicationStore().put(testerId, version, new byte[0]); return version; } @@ -190,13 +191,13 @@ public class InternalDeploymentTester { assertEquals(Step.Status.succeeded, jobs.active(run.id()).get().steps().get(Step.installReal)); assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.installTester)); - tester.configServer().convergeServices(testerOf(appId), zone); + tester.configServer().convergeServices(testerId.id(), zone); runner.run(); assertEquals(Step.Status.succeeded, jobs.active(run.id()).get().steps().get(Step.installTester)); // All installation is complete. We now need endpoints, and the tests will then run, and cleanup finish. assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.startTests)); - setEndpoints(testerOf(appId), zone); + setEndpoints(testerId.id(), zone); runner.run(); if (!run.versions().sourceApplication().isPresent() || !type.isProduction()) { assertEquals(unfinished, jobs.active(run.id()).get().steps().get(Step.startTests)); @@ -211,20 +212,20 @@ public class InternalDeploymentTester { assertTrue(jobs.run(run.id()).get().hasEnded()); assertFalse(jobs.run(run.id()).get().hasFailed()); assertEquals(type.isProduction(), app().deployments().containsKey(zone)); - assertTrue(tester.configServer().nodeRepository().list(zone, testerOf(appId)).isEmpty()); + assertTrue(tester.configServer().nodeRepository().list(zone, testerId.id()).isEmpty()); if (!app().deployments().containsKey(zone)) routing.removeEndpoints(deployment); - routing.removeEndpoints(new DeploymentId(testerOf(appId), zone)); + routing.removeEndpoints(new DeploymentId(testerId.id(), zone)); } public RunId startSystemTestTests() { RunId id = newRun(JobType.systemTest); runner.run(); tester.configServer().convergeServices(appId, JobType.systemTest.zone(tester.controller().system())); - tester.configServer().convergeServices(testerOf(appId), JobType.systemTest.zone(tester.controller().system())); + tester.configServer().convergeServices(testerId.id(), JobType.systemTest.zone(tester.controller().system())); setEndpoints(appId, JobType.systemTest.zone(tester.controller().system())); - setEndpoints(testerOf(appId), JobType.systemTest.zone(tester.controller().system())); + setEndpoints(testerId.id(), JobType.systemTest.zone(tester.controller().system())); runner.run(); assertEquals(unfinished, jobs.run(id).get().steps().get(Step.endTests)); return id; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java index e1a557f26f1..8b8e21f4d33 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java @@ -6,9 +6,7 @@ import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.SystemName; import com.yahoo.slime.ArrayTraverser; -import com.yahoo.slime.Cursor; import com.yahoo.slime.Inspector; -import com.yahoo.slime.Slime; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ConfigChangeActions; import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RefeedAction; @@ -37,13 +35,13 @@ import java.util.Optional; import static com.yahoo.vespa.hosted.controller.api.integration.LogEntry.Type.debug; import static com.yahoo.vespa.hosted.controller.api.integration.LogEntry.Type.error; import static com.yahoo.vespa.hosted.controller.api.integration.LogEntry.Type.info; -import static com.yahoo.vespa.hosted.controller.deployment.JobController.testerOf; +import static com.yahoo.vespa.hosted.controller.deployment.InternalDeploymentTester.appId; +import static com.yahoo.vespa.hosted.controller.deployment.InternalDeploymentTester.testerId; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.failed; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.unfinished; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; /** * @author jonmv @@ -69,15 +67,15 @@ public class InternalStepRunnerTest { public void canSwitchFromScrewdriverAndBackAgain() { // Deploys a default application package with default build number. tester.tester().deployCompletely(tester.app(), InternalDeploymentTester.applicationPackage); - tester.setEndpoints(InternalDeploymentTester.appId, JobType.productionUsCentral1.zone(tester.tester().controller().system())); - tester.setEndpoints(InternalDeploymentTester.appId, JobType.productionUsWest1.zone(tester.tester().controller().system())); - tester.setEndpoints(InternalDeploymentTester.appId, JobType.productionUsEast3.zone(tester.tester().controller().system())); + tester.setEndpoints(appId, JobType.productionUsCentral1.zone(tester.tester().controller().system())); + tester.setEndpoints(appId, JobType.productionUsWest1.zone(tester.tester().controller().system())); + tester.setEndpoints(appId, JobType.productionUsEast3.zone(tester.tester().controller().system())); tester.deployNewSubmission(); tester.deployNewPlatform(new Version("7.1")); - tester.jobs().unregister(InternalDeploymentTester.appId); + tester.jobs().unregister(appId); try { tester.tester().deployCompletely(tester.app(), InternalDeploymentTester.applicationPackage, BuildJob.defaultBuildNumber + 1); throw new IllegalStateException("Component job should get ahead again with build numbers to produce a change."); @@ -90,7 +88,7 @@ public class InternalStepRunnerTest { public void testerHasAthenzIdentity() { tester.newRun(JobType.stagingTest); tester.runner().run(); - DeploymentSpec spec = tester.configServer().application(testerOf(InternalDeploymentTester.appId)).get().applicationPackage().deploymentSpec(); + DeploymentSpec spec = tester.configServer().application(InternalDeploymentTester.testerId.id()).get().applicationPackage().deploymentSpec(); assertEquals("domain", spec.athenzDomain().get().value()); ZoneId zone = JobType.stagingTest.zone(tester.tester().controller().system()); assertEquals("service", spec.athenzService(zone.environment(), zone.region()).get().value()); @@ -115,7 +113,7 @@ public class InternalStepRunnerTest { public void restartsServicesAndWaitsForRestartAndReboot() { RunId id = tester.newRun(JobType.productionUsCentral1); ZoneId zone = id.type().zone(tester.tester().controller().system()); - HostName host = tester.configServer().hostFor(InternalDeploymentTester.appId, zone); + HostName host = tester.configServer().hostFor(appId, zone); tester.configServer().setConfigChangeActions(new ConfigChangeActions(Collections.singletonList(new RestartAction("cluster", "container", "search", @@ -128,11 +126,11 @@ public class InternalStepRunnerTest { tester.runner().run(); assertEquals(succeeded, tester.jobs().run(id).get().steps().get(Step.deployReal)); - tester.configServer().convergeServices(InternalDeploymentTester.appId, zone); + tester.configServer().convergeServices(appId, zone); assertEquals(unfinished, tester.jobs().run(id).get().steps().get(Step.installReal)); - tester.configServer().nodeRepository().doRestart(new DeploymentId(InternalDeploymentTester.appId, zone), Optional.of(host)); - tester.configServer().nodeRepository().requestReboot(new DeploymentId(InternalDeploymentTester.appId, zone), Optional.of(host)); + tester.configServer().nodeRepository().doRestart(new DeploymentId(appId, zone), Optional.of(host)); + tester.configServer().nodeRepository().requestReboot(new DeploymentId(appId, zone), Optional.of(host)); tester.runner().run(); assertEquals(unfinished, tester.jobs().run(id).get().steps().get(Step.installReal)); @@ -146,50 +144,50 @@ public class InternalStepRunnerTest { tester.newRun(JobType.systemTest); tester.runner().run(); - tester.configServer().convergeServices(InternalDeploymentTester.appId, JobType.stagingTest.zone(tester.tester().controller().system())); + tester.configServer().convergeServices(appId, JobType.stagingTest.zone(tester.tester().controller().system())); tester.runner().run(); - tester.configServer().convergeServices(InternalDeploymentTester.appId, JobType.systemTest.zone(tester.tester().controller().system())); - tester.configServer().convergeServices(testerOf(InternalDeploymentTester.appId), JobType.systemTest.zone(tester.tester().controller().system())); - tester.configServer().convergeServices(InternalDeploymentTester.appId, JobType.stagingTest.zone(tester.tester().controller().system())); - tester.configServer().convergeServices(testerOf(InternalDeploymentTester.appId), JobType.stagingTest.zone(tester.tester().controller().system())); + tester.configServer().convergeServices(appId, JobType.systemTest.zone(tester.tester().controller().system())); + tester.configServer().convergeServices(testerId.id(), JobType.systemTest.zone(tester.tester().controller().system())); + tester.configServer().convergeServices(appId, JobType.stagingTest.zone(tester.tester().controller().system())); + tester.configServer().convergeServices(testerId.id(), JobType.stagingTest.zone(tester.tester().controller().system())); tester.runner().run(); // Tester fails to show up for system tests, and the real deployment for staging tests. - tester.setEndpoints(InternalDeploymentTester.appId, JobType.systemTest.zone(tester.tester().controller().system())); - tester.setEndpoints(testerOf(InternalDeploymentTester.appId), JobType.stagingTest.zone(tester.tester().controller().system())); + tester.setEndpoints(appId, JobType.systemTest.zone(tester.tester().controller().system())); + tester.setEndpoints(testerId.id(), JobType.stagingTest.zone(tester.tester().controller().system())); tester.clock().advance(InternalStepRunner.endpointTimeout.plus(Duration.ofSeconds(1))); tester.runner().run(); - assertEquals(failed, tester.jobs().last(InternalDeploymentTester.appId, JobType.systemTest).get().steps().get(Step.startTests)); - assertEquals(failed, tester.jobs().last(InternalDeploymentTester.appId, JobType.stagingTest).get().steps().get(Step.startTests)); + assertEquals(failed, tester.jobs().last(appId, JobType.systemTest).get().steps().get(Step.startTests)); + assertEquals(failed, tester.jobs().last(appId, JobType.stagingTest).get().steps().get(Step.startTests)); } @Test public void installationFailsIfDeploymentExpires() { tester.newRun(JobType.systemTest); tester.runner().run(); - tester.configServer().convergeServices(InternalDeploymentTester.appId, JobType.systemTest.zone(tester.tester().controller().system())); + tester.configServer().convergeServices(appId, JobType.systemTest.zone(tester.tester().controller().system())); tester.runner().run(); - assertEquals(succeeded, tester.jobs().last(InternalDeploymentTester.appId, JobType.systemTest).get().steps().get(Step.installReal)); + assertEquals(succeeded, tester.jobs().last(appId, JobType.systemTest).get().steps().get(Step.installReal)); - tester.applications().deactivate(InternalDeploymentTester.appId, JobType.systemTest.zone(tester.tester().controller().system())); + tester.applications().deactivate(appId, JobType.systemTest.zone(tester.tester().controller().system())); tester.runner().run(); - assertEquals(failed, tester.jobs().last(InternalDeploymentTester.appId, JobType.systemTest).get().steps().get(Step.installTester)); - assertTrue(tester.jobs().last(InternalDeploymentTester.appId, JobType.systemTest).get().hasEnded()); - assertTrue(tester.jobs().last(InternalDeploymentTester.appId, JobType.systemTest).get().hasFailed()); + assertEquals(failed, tester.jobs().last(appId, JobType.systemTest).get().steps().get(Step.installTester)); + assertTrue(tester.jobs().last(appId, JobType.systemTest).get().hasEnded()); + assertTrue(tester.jobs().last(appId, JobType.systemTest).get().hasFailed()); } @Test public void startTestsFailsIfDeploymentExpires() { tester.newRun(JobType.systemTest); tester.runner().run(); - tester.configServer().convergeServices(InternalDeploymentTester.appId, JobType.systemTest.zone(tester.tester().controller().system())); - tester.configServer().convergeServices(testerOf(InternalDeploymentTester.appId), JobType.systemTest.zone(tester.tester().controller().system())); + tester.configServer().convergeServices(appId, JobType.systemTest.zone(tester.tester().controller().system())); + tester.configServer().convergeServices(testerId.id(), JobType.systemTest.zone(tester.tester().controller().system())); tester.runner().run(); - tester.applications().deactivate(InternalDeploymentTester.appId, JobType.systemTest.zone(tester.tester().controller().system())); + tester.applications().deactivate(appId, JobType.systemTest.zone(tester.tester().controller().system())); tester.runner().run(); - assertEquals(unfinished, tester.jobs().last(InternalDeploymentTester.appId, JobType.systemTest).get().steps().get(Step.startTests)); + assertEquals(unfinished, tester.jobs().last(appId, JobType.systemTest).get().steps().get(Step.startTests)); } @Test @@ -233,16 +231,16 @@ public class InternalStepRunnerTest { RunId id = tester.startSystemTestTests(); tester.runner().run(); assertEquals(unfinished, tester.jobs().run(id).get().steps().get(Step.endTests)); - assertEquals(URI.create(tester.routing().endpoints(new DeploymentId(testerOf(InternalDeploymentTester.appId), JobType.systemTest.zone(tester.tester().controller().system()))).get(0).getEndpoint()), + assertEquals(URI.create(tester.routing().endpoints(new DeploymentId(testerId.id(), JobType.systemTest.zone(tester.tester().controller().system()))).get(0).getEndpoint()), tester.cloud().testerUrl()); Inspector configObject = SlimeUtils.jsonToSlime(tester.cloud().config()).get(); - assertEquals(InternalDeploymentTester.appId.serializedForm(), configObject.field("application").asString()); + assertEquals(appId.serializedForm(), configObject.field("application").asString()); assertEquals(JobType.systemTest.zone(tester.tester().controller().system()).value(), configObject.field("zone").asString()); assertEquals(tester.tester().controller().system().name(), configObject.field("system").asString()); assertEquals(1, configObject.field("endpoints").children()); assertEquals(1, configObject.field("endpoints").field(JobType.systemTest.zone(tester.tester().controller().system()).value()).entries()); configObject.field("endpoints").field(JobType.systemTest.zone(tester.tester().controller().system()).value()).traverse((ArrayTraverser) (__, endpoint) -> - assertEquals(tester.routing().endpoints(new DeploymentId(InternalDeploymentTester.appId, JobType.systemTest.zone(tester.tester().controller().system()))).get(0).getEndpoint(), endpoint.asString())); + assertEquals(tester.routing().endpoints(new DeploymentId(appId, JobType.systemTest.zone(tester.tester().controller().system()))).get(0).getEndpoint(), endpoint.asString())); long lastId = tester.jobs().details(id).get().lastId().getAsLong(); tester.cloud().add(new LogEntry(0, 123, info, "Ready!")); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java index dc6042e669b..bb9602903f4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java @@ -3,35 +3,69 @@ package com.yahoo.vespa.hosted.controller.integration; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationStore; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId; -import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import static java.util.Objects.requireNonNull; +import static org.junit.Assert.assertFalse; + +/** + * Threadsafe. + * + * @author jonmv + */ public class ApplicationStoreMock implements ApplicationStore { - Map<String, byte[]> store = new HashMap<>(); + private final Map<ApplicationId, Map<ApplicationVersion, byte[]>> store = new ConcurrentHashMap<>(); + + @Override + public byte[] get(ApplicationId application, ApplicationVersion applicationVersion) { + assertFalse(application.instance().isTester()); + return requireNonNull(store.get(application).get(applicationVersion)); + } + + @Override + public void put(ApplicationId application, ApplicationVersion applicationVersion, byte[] applicationPackage) { + assertFalse(application.instance().isTester()); + store.putIfAbsent(application, new ConcurrentHashMap<>()); + store.get(application).put(applicationVersion, applicationPackage); + } + + @Override + public boolean prune(ApplicationId application, ApplicationVersion oldestToRetain) { + assertFalse(application.instance().isTester()); + return store.containsKey(application) + && store.get(application).keySet().removeIf(version -> version.compareTo(oldestToRetain) < 0); + } @Override - public byte[] getApplicationPackage(ApplicationId application, String applicationVersion) { - return store.get(path(application, applicationVersion)); + public void removeAll(ApplicationId application) { + store.remove(application); } @Override - public void putApplicationPackage(ApplicationId application, String applicationVersion, byte[] applicationPackage) { - store.put(path(application, applicationVersion), applicationPackage); + public byte[] get(TesterId tester, ApplicationVersion applicationVersion) { + return requireNonNull(store.get(tester.id()).get(applicationVersion)); } @Override - public void putTesterPackage(ApplicationId tester, String applicationVersion, byte[] testerPackage) { - store.put(path(tester, applicationVersion), testerPackage); + public void put(TesterId tester, ApplicationVersion applicationVersion, byte[] testerPackage) { + store.putIfAbsent(tester.id(), new ConcurrentHashMap<>()); + store.get(tester.id()).put(applicationVersion, testerPackage); } @Override - public byte[] getTesterPackage(ApplicationId tester, String applicationVersion) { - return store.get(path(tester, applicationVersion)); + public boolean prune(TesterId tester, ApplicationVersion oldestToRetain) { + return store.containsKey(tester.id()) + && store.get(tester.id()).keySet().removeIf(version -> version.compareTo(oldestToRetain) < 0); } - String path(ApplicationId tester, String applicationVersion) { - return tester.toString() + applicationVersion; + @Override + public void removeAll(TesterId tester) { + store.remove(tester.id()); } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRepositoryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRepositoryMock.java index 955980e89d6..04c670cf136 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRepositoryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ArtifactRepositoryMock.java @@ -47,21 +47,6 @@ public class ArtifactRepositoryMock extends AbstractComponent implements Artifac } @Override - public void putApplicationPackage(ApplicationId application, String applicationVersion, byte[] applicationPackage) { - repository.put(artifactHash(application, applicationVersion), new Artifact(applicationPackage)); - } - - @Override - public byte[] getTesterPackage(ApplicationId tester, String applicationVersion) { - return getApplicationPackage(tester, applicationVersion); - } - - @Override - public void putTesterPackage(ApplicationId tester, String applicationVersion, byte[] testerPackage) { - putApplicationPackage(tester, applicationVersion, testerPackage); - } - - @Override public byte[] getSystemApplicationPackage(ApplicationId application, ZoneId zone, Version version) { return new byte[0]; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java index 2694e205a68..62653f29518 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.hosted.controller.Application; -import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.OwnershipIssues; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; @@ -40,18 +40,19 @@ public class ApplicationOwnershipConfirmerTest { @Test public void testConfirmation() { - TenantName property = tester.controllerTester().createTenant("property", "domain", 1L); + Optional<Contact> contact = Optional.of(tester.controllerTester().contactRetriever().contact()); + TenantName property = tester.controllerTester().createTenant("property", "domain", 1L, contact); tester.createAndDeploy(property, "application", 1, "default"); Supplier<Application> propertyApp = () -> tester.controller().applications().require(ApplicationId.from("property", "application", "default")); - UserTenant user = UserTenant.create("by-user"); + UserTenant user = UserTenant.create("by-user", contact); tester.controller().tenants().create(user); tester.createAndDeploy(user.name(), "application", 2, "default"); Supplier<Application> userApp = () -> tester.controller().applications().require(ApplicationId.from("by-user", "application", "default")); assertFalse("No issue is initially stored for a new application.", propertyApp.get().ownershipIssueId().isPresent()); assertFalse("No issue is initially stored for a new application.", userApp.get().ownershipIssueId().isPresent()); - assertFalse("No escalation has been attempted for a new application", issues.escalatedForProperty || issues.escalatedForUser); + assertFalse("No escalation has been attempted for a new application", issues.escalatedToContact || issues.escalatedToTerminator); // Set response from the issue mock, which will be obtained by the maintainer on issue filing. Optional<IssueId> issueId = Optional.of(IssueId.from("1")); @@ -68,7 +69,8 @@ public class ApplicationOwnershipConfirmerTest { assertEquals("Confirmation issue has been filed for property owned application.", issueId, propertyApp.get().ownershipIssueId()); assertEquals("Confirmation issue has been filed for user owned application.", issueId, userApp.get().ownershipIssueId()); - assertTrue("Both applications have had their responses ensured.", issues.escalatedForProperty && issues.escalatedForUser); + assertTrue(issues.escalatedToTerminator); + assertTrue("Both applications have had their responses ensured.", issues.escalatedToContact && issues.escalatedToTerminator); // No new issue is created, so return empty now. issues.response = Optional.empty(); @@ -99,23 +101,18 @@ public class ApplicationOwnershipConfirmerTest { private class MockOwnershipIssues implements OwnershipIssues { private Optional<IssueId> response; - private boolean escalatedForProperty = false; - private boolean escalatedForUser = false; + private boolean escalatedToContact = false; + private boolean escalatedToTerminator = false; @Override - public Optional<IssueId> confirmOwnership(Optional<IssueId> issueId, ApplicationId applicationId, PropertyId propertyId) { + public Optional<IssueId> confirmOwnership(Optional<IssueId> issueId, ApplicationId applicationId, User asignee, Contact contact) { return response; } @Override - public Optional<IssueId> confirmOwnership(Optional<IssueId> issueId, ApplicationId applicationId, User owner) { - return response; - } - - @Override - public void ensureResponse(IssueId issueId, Optional<PropertyId> propertyId) { - if (propertyId.isPresent()) escalatedForProperty = true; - else escalatedForUser = true; + public void ensureResponse(IssueId issueId, Optional<Contact> contact) { + if (contact.isPresent()) escalatedToContact = true; + else escalatedToTerminator = true; } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java index cbaa37b15e3..6a7096dbfae 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ContactInformationMaintainerTest.java @@ -6,7 +6,7 @@ import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; -import com.yahoo.vespa.hosted.controller.tenant.Contact; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import org.junit.Before; import org.junit.Test; @@ -15,6 +15,7 @@ import java.time.Duration; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -35,7 +36,7 @@ public class ContactInformationMaintainerTest { tester = new ControllerTester(); maintainer = new ContactInformationMaintainer(tester.controller(), Duration.ofDays(1), new JobControl(tester.controller().curator()), - tester.organization()); + tester.contactRetriever()); } @Test @@ -55,14 +56,7 @@ public class ContactInformationMaintainerTest { private void registerContact(long propertyId, Contact contact) { PropertyId p = new PropertyId(String.valueOf(propertyId)); - tester.organization().addProperty(p) - .setContactsUrl(p, contact.url()) - .setIssueUrl(p, contact.issueTrackerUrl()) - .setPropertyUrl(p, contact.propertyUrl()) - .setContactsFor(p, contact.persons().stream().map(persons -> persons.stream() - .map(User::from) - .collect(Collectors.toList())) - .collect(Collectors.toList())); + tester.contactRetriever().addContact(p, contact); } private static Contact testContact() { @@ -71,7 +65,9 @@ public class ContactInformationMaintainerTest { URI propertyUrl = URI.create("http://property1.test"); List<List<String>> persons = Arrays.asList(Collections.singletonList("alice"), Collections.singletonList("bob")); - return new Contact(contactUrl, propertyUrl, issueTrackerUrl, persons); + String queue = "queue"; + Optional<String> component = Optional.empty(); + return new Contact(contactUrl, propertyUrl, issueTrackerUrl, persons, queue, component); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java index dcfcb813de6..e97376d1f66 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporterTest.java @@ -5,6 +5,7 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; import com.yahoo.vespa.hosted.controller.Application; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.stubs.LoggingDeploymentIssues; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; @@ -18,6 +19,7 @@ import org.junit.Test; import java.time.Duration; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.component; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.productionCorpUsEast1; @@ -70,6 +72,11 @@ public class DeploymentIssueReporterTest { tester.upgradeSystem(Version.fromString("6.2")); + Optional<Contact> contact = Optional.of(tester.controllerTester().contactRetriever().contact()); + tester.controllerTester().createTenant("tenant1", "domain1", 1L, contact); + tester.controllerTester().createTenant("tenant2", "domain2", 1L, contact); + tester.controllerTester().createTenant("tenant3", "domain3", 1L, contact); + // Create and deploy one application for each of three tenants. Application app1 = tester.createApplication("application1", "tenant1", projectId1, propertyId1); Application app2 = tester.createApplication("application2", "tenant2", projectId2, propertyId2); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java index ca31eb52979..792eaab20e2 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java @@ -5,8 +5,8 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.athenz.api.OktaAccessToken; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.deployment.JobController; import com.yahoo.vespa.hosted.controller.deployment.Run; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java index bc22df7bf46..d9846dd37ee 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java @@ -8,7 +8,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.BuildService; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Change; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index e06578a545f..29ba2a5b6fd 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -13,7 +13,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.MetricsService; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.ClusterInfo; import com.yahoo.vespa.hosted.controller.application.ClusterUtilization; @@ -24,7 +24,7 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.RotationStatus; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.rotation.RotationId; import org.junit.Test; @@ -74,7 +74,7 @@ public class ApplicationSerializerTest { deployments.add(new Deployment(zone1, applicationVersion1, Version.fromString("1.2.3"), Instant.ofEpochMilli(3))); // One deployment without cluster info and utils deployments.add(new Deployment(zone2, applicationVersion2, Version.fromString("1.2.3"), Instant.ofEpochMilli(5), createClusterUtils(3, 0.2), createClusterInfo(3, 4), - new DeploymentMetrics(2, 3, 4, 5, 6), + new DeploymentMetrics(2, 3, 4, 5, 6, Optional.of(Instant.now())), DeploymentActivity.create(Optional.of(activityAt), Optional.of(activityAt), OptionalDouble.of(200), OptionalDouble.of(10)))); @@ -168,7 +168,7 @@ public class ApplicationSerializerTest { assertEquals(original.deployments().get(zone2).metrics().documentCount(), serialized.deployments().get(zone2).metrics().documentCount(), Double.MIN_VALUE); assertEquals(original.deployments().get(zone2).metrics().queryLatencyMillis(), serialized.deployments().get(zone2).metrics().queryLatencyMillis(), Double.MIN_VALUE); assertEquals(original.deployments().get(zone2).metrics().writeLatencyMillis(), serialized.deployments().get(zone2).metrics().writeLatencyMillis(), Double.MIN_VALUE); - + assertEquals(original.deployments().get(zone2).metrics().instant(), serialized.deployments().get(zone2).metrics().instant()); { // test more deployment serialization cases Application original2 = writable(original).withChange(Change.of(ApplicationVersion.from(new SourceRevision("repo1", "branch1", "commit1"), 42))).get(); Application serialized2 = applicationSerializer.fromSlime(applicationSerializer.toSlime(original2)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java index a6937a4f97e..9aefbc712ae 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializerTest.java @@ -7,8 +7,8 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.RunStatus; import com.yahoo.vespa.hosted.controller.deployment.Step; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java index 60ff556dca4..f3790e6d291 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java @@ -6,7 +6,7 @@ import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.hosted.controller.api.identifiers.Property; import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; -import com.yahoo.vespa.hosted.controller.tenant.Contact; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import com.yahoo.vespa.hosted.controller.tenant.UserTenant; import org.junit.Test; @@ -57,24 +57,30 @@ public class TenantSerializerTest { new AthenzDomain("domain1"), new Property("property1"), Optional.of(new PropertyId("1")), - Optional.of(new Contact( - URI.create("http://contact1.test"), - URI.create("http://property1.test"), - URI.create("http://issue-tracker-1.test"), - Arrays.asList( - Collections.singletonList("person1"), - Collections.singletonList("person2") - ) - ))); + Optional.of(contact())); AthenzTenant serialized = serializer.athenzTenantFrom(serializer.toSlime(tenant)); assertEquals(tenant.contact(), serialized.contact()); } @Test public void user_tenant() { - UserTenant tenant = UserTenant.create("by-foo"); + UserTenant tenant = UserTenant.create("by-foo", Optional.of(contact())); UserTenant serialized = serializer.userTenantFrom(serializer.toSlime(tenant)); assertEquals(tenant.name(), serialized.name()); + assertEquals(contact(), serialized.contact().get()); } + private Contact contact() { + return new Contact( + URI.create("http://contact1.test"), + URI.create("http://property1.test"), + URI.create("http://issue-tracker-1.test"), + Arrays.asList( + Collections.singletonList("person1"), + Collections.singletonList("person2") + ), + "queue", + Optional.empty() + ); + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java index 6044dd7b8f8..e908777a8b0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java @@ -71,7 +71,8 @@ public class ControllerContainerTest { " <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.LoggingDeploymentIssues'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.DummyOwnershipIssues'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.MockRunDataStore'/>\n" + - " <component id='com.yahoo.vespa.hosted.controller.api.integration.organization.MockOrganization'/>\n" + + " <component id='com.yahoo.vespa.hosted.controller.api.integration.organization.MockContactRetriever'/>\n" + + " <component id='com.yahoo.vespa.hosted.controller.api.integration.organization.MockIssueHandler'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.integration.ConfigServerMock'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.integration.NodeRepositoryClientMock'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock'/>\n" + diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index b82462ad595..8b260765423 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -32,8 +32,7 @@ import com.yahoo.vespa.hosted.controller.athenz.HostedAthenzIdentities; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; -import com.yahoo.vespa.hosted.controller.api.integration.organization.MockOrganization; -import com.yahoo.vespa.hosted.controller.api.integration.organization.User; +import com.yahoo.vespa.hosted.controller.api.integration.organization.MockContactRetriever; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Change; @@ -58,7 +57,7 @@ import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; -import com.yahoo.vespa.hosted.controller.tenant.Contact; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Contact; import org.apache.http.HttpEntity; import org.apache.http.entity.ContentType; import org.apache.http.entity.mime.MultipartEntityBuilder; @@ -1297,7 +1296,7 @@ public class ApplicationApiTest extends ControllerContainerTest { clusterInfo.put(ClusterSpec.Id.from("cluster1"), new ClusterInfo("flavor1", 37, 2, 4, 50, ClusterSpec.Type.content, hostnames)); Map<ClusterSpec.Id, ClusterUtilization> clusterUtils = new HashMap<>(); clusterUtils.put(ClusterSpec.Id.from("cluster1"), new ClusterUtilization(0.3, 0.6, 0.4, 0.3)); - DeploymentMetrics metrics = new DeploymentMetrics(1, 2, 3, 4, 5); + DeploymentMetrics metrics = new DeploymentMetrics(1, 2, 3, 4, 5, Optional.of(Instant.ofEpochMilli(123123))); lockedApplication = lockedApplication .withClusterInfo(deployment.zone(), clusterInfo) @@ -1314,8 +1313,8 @@ public class ApplicationApiTest extends ControllerContainerTest { return (MetricsServiceMock) tester.container().components().getComponent(MetricsServiceMock.class.getName()); } - private MockOrganization organization() { - return (MockOrganization) tester.container().components().getComponent(MockOrganization.class.getName()); + private MockContactRetriever contactRetriever() { + return (MockContactRetriever) tester.container().components().getComponent(MockContactRetriever.class.getName()); } private void setZoneInRotation(String rotationName, ZoneId zone) { @@ -1341,18 +1340,14 @@ public class ApplicationApiTest extends ControllerContainerTest { } private void updateContactInformation() { - Contact contact = new Contact(URI.create("www.contacts.tld/1234"), URI.create("www.properties.tld/1234"), URI.create("www.issues.tld/1234"), Arrays.asList(Arrays.asList("alice"), Arrays.asList("bob"))); + Contact contact = new Contact(URI.create("www.contacts.tld/1234"), URI.create("www.properties.tld/1234"), URI.create("www.issues.tld/1234"), Arrays.asList(Arrays.asList("alice"), Arrays.asList("bob")), "queue", Optional.empty()); tester.controller().tenants().lockIfPresent(TenantName.from("tenant2"), lockedTenant -> tester.controller().tenants().store(lockedTenant.with(contact))); } private void registerContact(long propertyId) { PropertyId p = new PropertyId(String.valueOf(propertyId)); - organization().addProperty(p) - .setIssueUrl(p, URI.create("www.issues.tld/" + p.id())) - .setContactsUrl(p, URI.create("www.contacts.tld/" + p.id())) - .setPropertyUrl(p, URI.create("www.properties.tld/" + p.id())) - .setContactsFor(p, Arrays.asList(Collections.singletonList(User.from("alice")), - Collections.singletonList(User.from("bob")))); + contactRetriever().addContact(p, new Contact(URI.create("www.issues.tld/" + p.id()), URI.create("www.contacts.tld/" + p.id()), URI.create("www.properties.tld/" + p.id()), Arrays.asList(Collections.singletonList("alice"), + Collections.singletonList("bob")), "queue", Optional.empty())); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java index 293a1f8a746..5fe725923a5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelperTest.java @@ -5,7 +5,7 @@ import com.yahoo.component.Version; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId; -import com.yahoo.vespa.hosted.controller.application.ApplicationVersion; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.deployment.InternalDeploymentTester; import org.json.JSONException; import org.json.JSONObject; @@ -28,7 +28,7 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobTy import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Status.FAILURE; import static com.yahoo.vespa.hosted.controller.deployment.InternalDeploymentTester.appId; -import static com.yahoo.vespa.hosted.controller.deployment.JobController.testerOf; +import static com.yahoo.vespa.hosted.controller.deployment.InternalDeploymentTester.testerId; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.deploymentFailed; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.installationFailed; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.running; @@ -66,9 +66,9 @@ public class JobControllerApiHandlerHelperTest { ZoneId usWest1 = productionUsWest1.zone(tester.tester().controller().system()); tester.configServer().convergeServices(appId, usWest1); - tester.configServer().convergeServices(testerOf(appId), usWest1); + tester.configServer().convergeServices(testerId.id(), usWest1); tester.setEndpoints(appId, usWest1); - tester.setEndpoints(testerOf(appId), usWest1); + tester.setEndpoints(testerId.id(), usWest1); tester.runner().run(); tester.cloud().set(FAILURE); tester.runner().run(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json index b790cec3912..203b67dce1a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json @@ -63,6 +63,7 @@ "writesPerSecond": 2.0, "documentCount": 3.0, "queryLatencyMillis": 4.0, - "writeLatencyMillis": 5.0 + "writeLatencyMillis": 5.0, + "lastUpdated": 123123 } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-west-1.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-west-1.json index f6eae19337a..65ea3925d8c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-west-1.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/dev-us-west-1.json @@ -62,6 +62,7 @@ "writesPerSecond": 2.0, "documentCount": 3.0, "queryLatencyMillis": 4.0, - "writeLatencyMillis": 5.0 + "writeLatencyMillis": 5.0, + "lastUpdated": 123123 } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-corp-us-east-1.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-corp-us-east-1.json index b35a53eb1a4..c495308aba1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-corp-us-east-1.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-corp-us-east-1.json @@ -69,6 +69,7 @@ "writesPerSecond": 2.0, "documentCount": 3.0, "queryLatencyMillis": 4.0, - "writeLatencyMillis": 5.0 + "writeLatencyMillis": 5.0, + "lastUpdated": 123123 } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/contactinfo/ContactInfoHandlerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/contactinfo/ContactInfoHandlerTest.java index 459b3a35f85..24506fda31a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/contactinfo/ContactInfoHandlerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/contactinfo/ContactInfoHandlerTest.java @@ -2,17 +2,11 @@ package com.yahoo.vespa.hosted.controller.restapi.contactinfo; import com.yahoo.application.container.handler.Request; import com.yahoo.application.container.handler.Response; -import com.yahoo.slime.Slime; -import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; -import com.yahoo.vespa.hosted.controller.tenant.Contact; import org.junit.Before; import org.junit.Test; -import java.net.URI; -import java.util.Arrays; - import static org.junit.Assert.*; public class ContactInfoHandlerTest extends ControllerContainerTest { @@ -25,7 +19,7 @@ public class ContactInfoHandlerTest extends ControllerContainerTest { } @Test - public void testGettingAndFeedingContactInfo() throws Exception { + public void testGettingAndFeedingContactInfo() { tester.createApplication(); // No contact information available yet @@ -33,8 +27,8 @@ public class ContactInfoHandlerTest extends ControllerContainerTest { assertResponse(new Request("http://localhost:8080/contactinfo/v1/tenant/tenant1"), 404, notFoundMessage); // Feed contact information for tenant1 - String contactInfo = "{\"url\":\"https://url:4444/\",\"issueTrackerUrl\":\"https://issueTrackerUrl:4444/\",\"propertyUrl\":\"https://propertyUrl:4444/\",\"persons\":[[\"foo\",\"bar\"]]}"; - String expectedResponseMessage = "Added contact info for tenant1 - Contact{url=https://url:4444/, propertyUrl=https://propertyUrl:4444/, issueTrackerUrl=https://issueTrackerUrl:4444/, persons=[[foo, bar]]}"; + String contactInfo = "{\"url\":\"https://url:4444/\",\"issueTrackerUrl\":\"https://issueTrackerUrl:4444/\",\"propertyUrl\":\"https://propertyUrl:4444/\",\"persons\":[[\"foo\",\"bar\"]],\"queue\":\"queue\",\"component\":\"component\"}"; + String expectedResponseMessage = "Added contact info for tenant1 - Contact{url=https://url:4444/, propertyUrl=https://propertyUrl:4444/, issueTrackerUrl=https://issueTrackerUrl:4444/, persons=[[foo, bar]], queue=queue, component=component}"; assertResponse(new Request("http://localhost:8080/contactinfo/v1/tenant/tenant1", contactInfo, Request.Method.POST), 200, expectedResponseMessage); // Get contact information for tenant1 diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java index 6e5e8a25e22..9f03df79cfe 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java @@ -5,7 +5,7 @@ import com.yahoo.application.container.handler.Request; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobReport; -import com.yahoo.vespa.hosted.controller.application.SourceRevision; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; import org.junit.Test; diff --git a/document/src/vespa/document/serialization/vespadocumentserializer.cpp b/document/src/vespa/document/serialization/vespadocumentserializer.cpp index f5059c233bd..ae03bcc0d3c 100644 --- a/document/src/vespa/document/serialization/vespadocumentserializer.cpp +++ b/document/src/vespa/document/serialization/vespadocumentserializer.cpp @@ -91,6 +91,13 @@ VespaDocumentSerializer::getContentCode(bool hasHeader, bool hasBody) const return content; } +static inline size_t wantChunks(bool hasHeader, bool hasBody) { + size_t res = 0; + if (hasHeader) ++res; + if (hasBody) ++res; + return res; +} + void VespaDocumentSerializer::write(const Document &value, DocSerializationMode mode) { nbostream doc_stream; @@ -101,12 +108,6 @@ void VespaDocumentSerializer::write(const Document &value, bool hasBody = false; const StructFieldValue::Chunks & chunks = value.getFields().getChunks(); - if (chunks.size() == 2) { - // we must assume both types of fields if the original serialization - // had that, even if config has changed since then. - hasHeader = true; - hasBody = true; - } for (const Field & field : value.getFields()) { if (field.isHeaderField()) { @@ -114,35 +115,29 @@ void VespaDocumentSerializer::write(const Document &value, } else { hasBody = true; } - if (hasHeader && hasBody) { break; } } - if (mode != COMPLETE) { hasBody = false; } - doc_stream << getContentCode(hasHeader, hasBody); doc_serializer.write(value.getType()); - if (!structNeedsReserialization(value.getFields())) { - // FIXME(vekterli): - // Currently assume legacy serialization; a chunk will only ever contain fields - // _either_ for the header _or_ for the body, never a mixture! - // This is to avoid horrible breakage whilst ripping out old guts. - - if (hasHeader) { + if (chunks.size() == wantChunks(hasHeader, hasBody) && + !structNeedsReserialization(value.getFields())) + { + // here we assume the receiver can handle whatever serialization the + // chunks contain, so we just send them as-is, even if some fields + // may have moved from header to body or vice versa. + if (hasHeader || hasBody) { assert(chunks.size() >= 1); doc_serializer.writeUnchanged(chunks[0]); - if (hasBody) { - assert(chunks.size() == 2); - doc_serializer.writeUnchanged(chunks[1]); - } - } else if (hasBody) { - assert(chunks.size() == 1); - doc_serializer.writeUnchanged(chunks[0]); + } + if (hasHeader && hasBody) { + assert(chunks.size() == 2); + doc_serializer.writeUnchanged(chunks[1]); } } else { if (hasHeader) { diff --git a/jrt/src/com/yahoo/jrt/MaybeTlsCryptoSocket.java b/jrt/src/com/yahoo/jrt/MaybeTlsCryptoSocket.java index 1cf3dfd1261..7c09b8b47ca 100644 --- a/jrt/src/com/yahoo/jrt/MaybeTlsCryptoSocket.java +++ b/jrt/src/com/yahoo/jrt/MaybeTlsCryptoSocket.java @@ -67,7 +67,9 @@ public class MaybeTlsCryptoSocket implements CryptoSocket { @Override public HandshakeResult handshake() throws IOException { if (factory != null) { - channel().read(buffer.getWritable(SNOOP_SIZE)); + if (channel().read(buffer.getWritable(SNOOP_SIZE)) == -1) { + throw new IOException("jrt: Connection closed by peer during tls detection"); + } if (buffer.bytes() < SNOOP_SIZE) { return HandshakeResult.NEED_READ; } diff --git a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/wire/HostStateChangeDenialReason.java b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/wire/HostStateChangeDenialReason.java index 3a1e8d6564d..054396a8eb9 100644 --- a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/wire/HostStateChangeDenialReason.java +++ b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/wire/HostStateChangeDenialReason.java @@ -56,4 +56,9 @@ public class HostStateChangeDenialReason { public int hashCode() { return Objects.hash(constraintName, message); } + + @Override + public String toString() { + return message + " [" + constraintName + "]"; + } } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.java index 52994db2b88..cc8a401ed78 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.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.vespa.orchestrator.resources; +import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.container.jaxrs.annotation.Component; import com.yahoo.log.LogLevel; import com.yahoo.vespa.applicationmodel.HostName; @@ -9,6 +10,7 @@ import com.yahoo.vespa.orchestrator.HostNameNotFoundException; import com.yahoo.vespa.orchestrator.OrchestrationException; import com.yahoo.vespa.orchestrator.Orchestrator; import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; +import com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy; import com.yahoo.vespa.orchestrator.restapi.HostApi; import com.yahoo.vespa.orchestrator.restapi.wire.GetHostResponse; import com.yahoo.vespa.orchestrator.restapi.wire.HostService; @@ -73,6 +75,9 @@ public class HostResource implements HostApi { host.getHostStatus().name(), applicationUri.toString(), hostServices); + } catch (UncheckedTimeoutException e) { + log.log(LogLevel.INFO, "Failed to get host " + hostName + ": " + e.getMessage()); + throw webExceptionFromTimeout("getHost", hostName, e); } catch (HostNameNotFoundException e) { log.log(LogLevel.INFO, "Host not found: " + hostName); throw new NotFoundException(e); @@ -96,6 +101,9 @@ public class HostResource implements HostApi { } catch (HostNameNotFoundException e) { log.log(LogLevel.INFO, "Host not found: " + hostName); throw new NotFoundException(e); + } catch (UncheckedTimeoutException e) { + log.log(LogLevel.INFO, "Failed to patch " + hostName + ": " + e.getMessage()); + throw webExceptionFromTimeout("patch", hostName, e); } catch (OrchestrationException e) { String message = "Failed to set " + hostName + " to " + state + ": " + e.getMessage(); log.log(LogLevel.INFO, message, e); @@ -116,6 +124,9 @@ public class HostResource implements HostApi { } catch (HostNameNotFoundException e) { log.log(LogLevel.INFO, "Host not found: " + hostName); throw new NotFoundException(e); + } catch (UncheckedTimeoutException e) { + log.log(LogLevel.INFO, "Failed to suspend " + hostName + ": " + e.getMessage()); + throw webExceptionFromTimeout("suspend", hostName, e); } catch (HostStateChangeDeniedException e) { log.log(LogLevel.INFO, "Failed to suspend " + hostName + ": " + e.getMessage()); throw webExceptionWithDenialReason("suspend", hostName, e); @@ -131,6 +142,9 @@ public class HostResource implements HostApi { } catch (HostNameNotFoundException e) { log.log(LogLevel.INFO, "Host not found: " + hostName); throw new NotFoundException(e); + } catch (UncheckedTimeoutException e) { + log.log(LogLevel.INFO, "Failed to resume " + hostName + ": " + e.getMessage()); + throw webExceptionFromTimeout("resume", hostName, e); } catch (HostStateChangeDeniedException e) { log.log(LogLevel.INFO, "Failed to resume " + hostName + ": " + e.getMessage()); throw webExceptionWithDenialReason("resume", hostName, e); @@ -138,15 +152,25 @@ public class HostResource implements HostApi { return new UpdateHostResponse(hostName.s(), null); } + private static WebApplicationException webExceptionFromTimeout(String operationDescription, HostName hostName, UncheckedTimeoutException e) { + return createWebException(operationDescription, hostName, e, HostedVespaPolicy.DEADLINE_CONSTRAINT, e.getMessage()); + } + private static WebApplicationException webExceptionWithDenialReason( String operationDescription, HostName hostName, HostStateChangeDeniedException e) { - HostStateChangeDenialReason hostStateChangeDenialReason = - new HostStateChangeDenialReason( - e.getConstraintName(), - operationDescription + " failed: " + e.getMessage()); - UpdateHostResponse response = new UpdateHostResponse(hostName.s(), hostStateChangeDenialReason); + return createWebException(operationDescription, hostName, e, e.getConstraintName(), e.getMessage()); + } + + private static WebApplicationException createWebException(String operationDescription, + HostName hostname, + Exception e, + String constraint, + String message) { + HostStateChangeDenialReason hostStateChangeDenialReason = new HostStateChangeDenialReason( + constraint, operationDescription + " failed: " + message); + UpdateHostResponse response = new UpdateHostResponse(hostname.s(), hostStateChangeDenialReason); return new WebApplicationException( hostStateChangeDenialReason.toString(), e, @@ -154,8 +178,6 @@ public class HostResource implements HostApi { .entity(response) .type(MediaType.APPLICATION_JSON_TYPE) .build()); - } - } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java index 6fa1e06520e..24d42264a51 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.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.vespa.orchestrator.resources; +import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.container.jaxrs.annotation.Component; import com.yahoo.log.LogLevel; import com.yahoo.vespa.applicationmodel.HostName; @@ -41,7 +42,7 @@ public class HostSuspensionResource implements HostSuspensionApi { List<HostName> hostnames = hostnamesAsStrings.stream().map(HostName::new).collect(Collectors.toList()); try { orchestrator.suspendAll(parentHostname, hostnames); - } catch (BatchHostStateChangeDeniedException e) { + } catch (BatchHostStateChangeDeniedException | UncheckedTimeoutException e) { log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e); throw createWebApplicationException(e.getMessage(), Response.Status.CONFLICT); } catch (BatchHostNameNotFoundException e) { diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java index 035d9cd686f..e943a4f105b 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.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.vespa.orchestrator.resources; +import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference; @@ -12,7 +13,10 @@ import com.yahoo.vespa.applicationmodel.ServiceInstance; import com.yahoo.vespa.applicationmodel.ServiceStatus; import com.yahoo.vespa.applicationmodel.ServiceType; import com.yahoo.vespa.applicationmodel.TenantId; +import com.yahoo.vespa.orchestrator.BatchHostNameNotFoundException; +import com.yahoo.vespa.orchestrator.BatchInternalErrorException; import com.yahoo.vespa.orchestrator.Host; +import com.yahoo.vespa.orchestrator.HostNameNotFoundException; import com.yahoo.vespa.orchestrator.InstanceLookupService; import com.yahoo.vespa.orchestrator.OrchestrationException; import com.yahoo.vespa.orchestrator.Orchestrator; @@ -20,10 +24,12 @@ import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.OrchestratorImpl; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactoryMock; import com.yahoo.vespa.orchestrator.model.ApplicationApi; +import com.yahoo.vespa.orchestrator.policy.BatchHostStateChangeDeniedException; import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; import com.yahoo.vespa.orchestrator.policy.Policy; import com.yahoo.vespa.orchestrator.restapi.wire.BatchOperationResult; import com.yahoo.vespa.orchestrator.restapi.wire.GetHostResponse; +import com.yahoo.vespa.orchestrator.restapi.wire.HostStateChangeDenialReason; import com.yahoo.vespa.orchestrator.restapi.wire.PatchHostRequest; import com.yahoo.vespa.orchestrator.restapi.wire.PatchHostResponse; import com.yahoo.vespa.orchestrator.restapi.wire.UpdateHostResponse; @@ -360,4 +366,33 @@ public class HostResourceTest { assertEquals("UP", response.services().get(0).serviceStatus); assertEquals("serviceType", response.services().get(0).serviceType); } + + @Test + public void throws_409_on_timeout() throws HostNameNotFoundException, HostStateChangeDeniedException { + Orchestrator orchestrator = mock(Orchestrator.class); + doThrow(new UncheckedTimeoutException("Timeout Message")).when(orchestrator).resume(any(HostName.class)); + + try { + HostResource hostResource = new HostResource(orchestrator, uriInfo); + hostResource.resume("hostname"); + fail(); + } catch (WebApplicationException w) { + assertThat(w.getResponse().getStatus()).isEqualTo(409); + assertEquals("resume failed: Timeout Message [deadline]", w.getMessage()); + } + } + + @Test + public void throws_409_on_suspendAll_timeout() throws BatchHostStateChangeDeniedException, BatchHostNameNotFoundException, BatchInternalErrorException { + Orchestrator orchestrator = mock(Orchestrator.class); + doThrow(new UncheckedTimeoutException("Timeout Message")).when(orchestrator).suspendAll(any(), any()); + + try { + HostSuspensionResource resource = new HostSuspensionResource(orchestrator); + resource.suspendAll("parenthost", Arrays.asList("h1", "h2", "h3")); + fail(); + } catch (WebApplicationException w) { + assertThat(w.getResponse().getStatus()).isEqualTo(409); + } + } } diff --git a/searchcore/CMakeLists.txt b/searchcore/CMakeLists.txt index 9d996d96dc7..d5d5544b88c 100644 --- a/searchcore/CMakeLists.txt +++ b/searchcore/CMakeLists.txt @@ -58,7 +58,7 @@ vespa_define_module( TESTS src/tests/applyattrupdates src/tests/fdispatch/randomrow - src/tests/fdispatch/search_path + src/tests/fdispatch/fnet_search src/tests/grouping src/tests/proton/attribute src/tests/proton/attribute/attribute_aspect_delayer diff --git a/searchcore/src/testlist.txt b/searchcore/src/testlist.txt index b74cd851052..f11d8d5c329 100644 --- a/searchcore/src/testlist.txt +++ b/searchcore/src/testlist.txt @@ -1,7 +1,7 @@ ?tests/proton/proton tests/applyattrupdates tests/fdispatch/randomrow -tests/fdispatch/search_path +tests/fdispatch/fnet_search tests/grouping tests/proton/attribute tests/proton/attribute/attribute_manager diff --git a/searchcore/src/tests/fdispatch/fnet_search/.gitignore b/searchcore/src/tests/fdispatch/fnet_search/.gitignore new file mode 100644 index 00000000000..b525d6fcd38 --- /dev/null +++ b/searchcore/src/tests/fdispatch/fnet_search/.gitignore @@ -0,0 +1,2 @@ +searchcore_search_path_test_app +searchcore_search_coverage_test_app diff --git a/searchcore/src/tests/fdispatch/search_path/CMakeLists.txt b/searchcore/src/tests/fdispatch/fnet_search/CMakeLists.txt index a3abaa868ff..c4e1608d6da 100644 --- a/searchcore/src/tests/fdispatch/search_path/CMakeLists.txt +++ b/searchcore/src/tests/fdispatch/fnet_search/CMakeLists.txt @@ -6,3 +6,13 @@ vespa_add_executable(searchcore_search_path_test_app TEST searchcore_fdispatch_search ) vespa_add_test(NAME searchcore_search_path_test_app COMMAND searchcore_search_path_test_app) + +vespa_add_executable(searchcore_search_coverage_test_app TEST + SOURCES + search_coverage_test.cpp + DEPENDS + searchcore_fdispatch_search + searchcore_fdcommon + searchcore_grouping +) +vespa_add_test(NAME searchcore_search_coverage_test_app COMMAND searchcore_search_coverage_test_app) diff --git a/searchcore/src/tests/fdispatch/search_path/DESC b/searchcore/src/tests/fdispatch/fnet_search/DESC index 4bc24883896..4bc24883896 100644 --- a/searchcore/src/tests/fdispatch/search_path/DESC +++ b/searchcore/src/tests/fdispatch/fnet_search/DESC diff --git a/searchcore/src/tests/fdispatch/search_path/FILES b/searchcore/src/tests/fdispatch/fnet_search/FILES index a38e13c26fd..a38e13c26fd 100644 --- a/searchcore/src/tests/fdispatch/search_path/FILES +++ b/searchcore/src/tests/fdispatch/fnet_search/FILES diff --git a/searchcore/src/tests/fdispatch/fnet_search/search_coverage_test.cpp b/searchcore/src/tests/fdispatch/fnet_search/search_coverage_test.cpp new file mode 100644 index 00000000000..d598583437d --- /dev/null +++ b/searchcore/src/tests/fdispatch/fnet_search/search_coverage_test.cpp @@ -0,0 +1,141 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <vespa/vespalib/testkit/testapp.h> + +#include <vespa/searchcore/fdispatch/search/fnet_search.h> +#include <vespa/searchlib/engine/searchreply.h> + +#include <vespa/log/log.h> +LOG_SETUP("search_coverage_test"); + +using namespace fdispatch; +using search::engine::SearchReply; + +std::vector<FastS_FNET_SearchNode> +createNodes(uint32_t count) { + std::vector<FastS_FNET_SearchNode> nodes; + nodes.reserve(count); + for (uint32_t partid(0); partid < count; partid++) { + nodes.emplace_back(nullptr, partid); + } + return nodes; +} + +void +query(FastS_FNET_SearchNode & node) { + node.DirtySetChannelOnlyForTesting((FNET_Channel *) 1); +} + +void +respond(FastS_FNET_SearchNode & node, size_t covered, size_t active, size_t soonActive, uint32_t degradeReason) { + node._qresult = new FS4Packet_QUERYRESULTX(); + node._qresult->_coverageDocs = covered; + node._qresult->_activeDocs = active; + node._qresult->_soonActiveDocs = soonActive; + node._qresult->_coverageDegradeReason = degradeReason; +} + +void +respond(FastS_FNET_SearchNode & node, size_t covered, size_t active, size_t soonActive) { + respond(node, covered, active, soonActive, 0); +} + +void disconnectNodes(std::vector<FastS_FNET_SearchNode> & nodes) { + for (auto & node : nodes) { + node.DirtySetChannelOnlyForTesting(nullptr); + } +} +TEST("testCoverageWhenAllNodesAreUp") { + std::vector<FastS_FNET_SearchNode> nodes = createNodes(4); + for (auto & node : nodes) { + query(node); + respond(node, 25, 30, 50); + } + FastS_SearchInfo si = FastS_FNET_Search::computeCoverage(nodes, 1, false); + EXPECT_EQUAL(4u, si._nodesQueried); + EXPECT_EQUAL(4u, si._nodesReplied); + EXPECT_EQUAL(100u, si._coverageDocs); + EXPECT_EQUAL(120u, si._activeDocs); + EXPECT_EQUAL(200u, si._soonActiveDocs); + EXPECT_EQUAL(0u, si._degradeReason); + disconnectNodes(nodes); +} + +TEST("testCoverageWhenNoNodesAreUp") { + std::vector<FastS_FNET_SearchNode> nodes = createNodes(4); + for (auto & node : nodes) { + query(node); + } + FastS_SearchInfo si = FastS_FNET_Search::computeCoverage(nodes, 1, false); + EXPECT_EQUAL(4u, si._nodesQueried); + EXPECT_EQUAL(0u, si._nodesReplied); + EXPECT_EQUAL(0u, si._coverageDocs); + EXPECT_EQUAL(0u, si._activeDocs); + EXPECT_EQUAL(0u, si._soonActiveDocs); + EXPECT_EQUAL(SearchReply::Coverage::TIMEOUT, si._degradeReason); + disconnectNodes(nodes); +} + +TEST("testCoverageWhenNoNodesAreUpWithAdaptiveTimeout") { + std::vector<FastS_FNET_SearchNode> nodes = createNodes(4); + for (auto & node : nodes) { + query(node); + } + FastS_SearchInfo si = FastS_FNET_Search::computeCoverage(nodes, 1, true); + EXPECT_EQUAL(4u, si._nodesQueried); + EXPECT_EQUAL(0u, si._nodesReplied); + EXPECT_EQUAL(0u, si._coverageDocs); + EXPECT_EQUAL(0u, si._activeDocs); + EXPECT_EQUAL(0u, si._soonActiveDocs); + EXPECT_EQUAL(SearchReply::Coverage::ADAPTIVE_TIMEOUT, si._degradeReason); + disconnectNodes(nodes); +} + +TEST("testCoverageWhen1NodesIsDown") { + std::vector<FastS_FNET_SearchNode> nodes = createNodes(4); + for (auto & node : nodes) { + query(node); + } + respond(nodes[0], 25, 30, 50); + respond(nodes[2], 25, 30, 50); + respond(nodes[3], 25, 30, 50); + + FastS_SearchInfo si = FastS_FNET_Search::computeCoverage(nodes, 1, false); + EXPECT_EQUAL(4u, si._nodesQueried); + EXPECT_EQUAL(3u, si._nodesReplied); + EXPECT_EQUAL(75u, si._coverageDocs); + EXPECT_EQUAL(120u, si._activeDocs); + EXPECT_EQUAL(200u, si._soonActiveDocs); + EXPECT_EQUAL(SearchReply::Coverage::TIMEOUT, si._degradeReason); + + // Do not trigger dirty magic when you still have enough coverage in theory + si = FastS_FNET_Search::computeCoverage(nodes, 2, false); + EXPECT_EQUAL(4u, si._nodesQueried); + EXPECT_EQUAL(3u, si._nodesReplied); + EXPECT_EQUAL(75u, si._coverageDocs); + EXPECT_EQUAL(90u, si._activeDocs); + EXPECT_EQUAL(150u, si._soonActiveDocs); + EXPECT_EQUAL(0u, si._degradeReason); + disconnectNodes(nodes); +} + +TEST("testCoverageWhen1NodeDoesnotReplyWithAdaptiveTimeout") { + std::vector<FastS_FNET_SearchNode> nodes = createNodes(4); + for (auto & node : nodes) { + query(node); + } + respond(nodes[0], 25, 30, 50); + respond(nodes[2], 25, 30, 50); + respond(nodes[3], 25, 30, 50); + + FastS_SearchInfo si = FastS_FNET_Search::computeCoverage(nodes, 1, true); + EXPECT_EQUAL(4u, si._nodesQueried); + EXPECT_EQUAL(3u, si._nodesReplied); + EXPECT_EQUAL(75u, si._coverageDocs); + EXPECT_EQUAL(120u, si._activeDocs); + EXPECT_EQUAL(200u, si._soonActiveDocs); + EXPECT_EQUAL(SearchReply::Coverage::ADAPTIVE_TIMEOUT, si._degradeReason); + disconnectNodes(nodes); +} + + +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/fdispatch/search_path/search_path_test.cpp b/searchcore/src/tests/fdispatch/fnet_search/search_path_test.cpp index b62fb8d14f2..b62fb8d14f2 100644 --- a/searchcore/src/tests/fdispatch/search_path/search_path_test.cpp +++ b/searchcore/src/tests/fdispatch/fnet_search/search_path_test.cpp diff --git a/searchcore/src/tests/fdispatch/search_path/.gitignore b/searchcore/src/tests/fdispatch/search_path/.gitignore deleted file mode 100644 index 7452ecf3ecc..00000000000 --- a/searchcore/src/tests/fdispatch/search_path/.gitignore +++ /dev/null @@ -1 +0,0 @@ -searchcore_search_path_test_app diff --git a/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp b/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp index 9dd4e81f91a..b85e706397d 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp +++ b/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp @@ -297,7 +297,7 @@ Fdispatch::Init() LOG(debug, "Creating FNET transport"); - _transport = std::make_unique<FNET_Transport>(_config->transportthreads); + _transport = std::make_unique<FNET_Transport>(std::make_shared<vespalib::NullCryptoEngine>(), _config->transportthreads); // disable encryption // grab node slowness limit defaults diff --git a/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.cpp b/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.cpp index 73b92eaa2b7..c85e432140e 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.cpp +++ b/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.cpp @@ -861,45 +861,60 @@ FastS_FNET_Search::MergeHits() } } -void -FastS_FNET_Search::CheckCoverage() +FastS_SearchInfo +FastS_FNET_Search::computeCoverage(const std::vector<FastS_FNET_SearchNode> & nodes, + uint32_t numSearchableCopies, bool adaptiveTimeout) { - uint64_t covDocs = 0; - uint64_t activeDocs = 0; - uint64_t soonActiveDocs = 0; - uint32_t degradedReason = 0; - uint16_t nodesQueried = 0; - uint16_t nodesReplied = 0; + FastS_SearchInfo si; size_t cntNone(0); size_t askedButNotAnswered(0); - for (const FastS_FNET_SearchNode & node : _nodes) { + for (const FastS_FNET_SearchNode & node : nodes) { if (node._qresult != nullptr) { - covDocs += node._qresult->_coverageDocs; - activeDocs += node._qresult->_activeDocs; - soonActiveDocs += node._qresult->_soonActiveDocs; - degradedReason |= node._qresult->_coverageDegradeReason; - nodesQueried += node._qresult->getNodesQueried(); - nodesReplied += node._qresult->getNodesReplied(); + si._coverageDocs += node._qresult->_coverageDocs; + si._activeDocs += node._qresult->_activeDocs; + si._soonActiveDocs += node._qresult->_soonActiveDocs; + si._degradeReason |= node._qresult->_coverageDegradeReason; + si._nodesQueried += node._qresult->getNodesQueried(); + si._nodesReplied += node._qresult->getNodesReplied(); } else { - nodesQueried++; + si._nodesQueried++; cntNone++; if (node.IsConnected()) { askedButNotAnswered++; } } } - bool missingReplies = (askedButNotAnswered != 0) || (nodesQueried != nodesReplied); - const ssize_t missingParts = cntNone - (_dataset->getSearchableCopies() - 1); - if (((missingParts > 0) || (missingReplies && useAdaptiveTimeout())) && (cntNone != _nodes.size())) { + bool missingReplies = (askedButNotAnswered != 0) || (si._nodesQueried != si._nodesReplied); + const ssize_t missingParts = cntNone - (numSearchableCopies - 1); + + if (missingReplies && adaptiveTimeout) { + // TODO This will not be correct when using multilevel dispatch and has timeout on anything, but leaf level. + // We can live with that as leaf level failures are the likely ones. + if (si._nodesReplied ) { + si._activeDocs += askedButNotAnswered * si._activeDocs/si._nodesReplied; + si._soonActiveDocs += askedButNotAnswered * si._soonActiveDocs/si._nodesReplied; + } + si._degradeReason |= search::engine::SearchReply::Coverage::ADAPTIVE_TIMEOUT; + } else if (missingParts > 0) { // TODO This is a dirty way of anticipating missing coverage. // It should be done differently - activeDocs += missingParts * activeDocs/(_nodes.size() - cntNone); - } - if (missingReplies && useAdaptiveTimeout()) { - degradedReason |= search::engine::SearchReply::Coverage::ADAPTIVE_TIMEOUT; + if ((cntNone != nodes.size())) { + si._activeDocs += missingParts * si._activeDocs/(nodes.size() - cntNone); + si._soonActiveDocs += missingParts * si._soonActiveDocs/(nodes.size() - cntNone); + } + si._degradeReason |= search::engine::SearchReply::Coverage::TIMEOUT; + } - _util.SetCoverage(covDocs, activeDocs, soonActiveDocs, degradedReason, nodesQueried, nodesReplied); + return si; +} + +void +FastS_FNET_Search::CheckCoverage() +{ + FastS_SearchInfo si = computeCoverage(_nodes, _dataset->getSearchableCopies(), useAdaptiveTimeout()); + _util.SetCoverage(si._coverageDocs, si._activeDocs, si._soonActiveDocs, + si._degradeReason, si._nodesQueried, si._nodesReplied); } diff --git a/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.h b/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.h index 81e84e92bbd..a77b984ca28 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.h +++ b/searchcore/src/vespa/searchcore/fdispatch/search/fnet_search.h @@ -121,6 +121,10 @@ public: } } + void DirtySetChannelOnlyForTesting(FNET_Channel * channel) { + _channel = channel; + } + void Disconnect() { @@ -284,6 +288,8 @@ public: bool ShouldLimitHitsPerNode() const; void MergeHits(); void CheckCoverage(); + static FastS_SearchInfo computeCoverage(const std::vector<FastS_FNET_SearchNode> & nodes, + uint32_t numSearchableCopies, bool adaptiveTimeout); void CheckQueryTimes(); void CheckDocsumTimes(); void CheckQueryTimeout(); diff --git a/searchcore/src/vespa/searchcore/proton/server/matchers.cpp b/searchcore/src/vespa/searchcore/proton/server/matchers.cpp index c2091410c16..a7df151f2f0 100644 --- a/searchcore/src/vespa/searchcore/proton/server/matchers.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/matchers.cpp @@ -14,13 +14,13 @@ Matchers::Matchers(const vespalib::Clock &clock, _default() { } -Matchers::~Matchers() { } +Matchers::~Matchers() = default; void Matchers::add(const vespalib::string &name, matching::Matcher::SP matcher) { _rpmap[name] = matcher; - if (name == "default" || _default.get() == 0) { + if ((name == "default") || ! _default) { _default = matcher; } } @@ -29,8 +29,8 @@ matching::MatchingStats Matchers::getStats() const { matching::MatchingStats stats; - for (Map::const_iterator it(_rpmap.begin()), mt(_rpmap.end()); it != mt; it++) { - stats.add(it->second->getStats()); + for (const auto & entry : _rpmap) { + stats.add(entry.second->getStats()); } return stats; } diff --git a/searchlib/src/vespa/searchlib/engine/transportserver.cpp b/searchlib/src/vespa/searchlib/engine/transportserver.cpp index 0fa7d44bbad..bc739a7bf48 100644 --- a/searchlib/src/vespa/searchlib/engine/transportserver.cpp +++ b/searchlib/src/vespa/searchlib/engine/transportserver.cpp @@ -359,7 +359,7 @@ TransportServer::TransportServer(SearchServer &searchServer, : _searchServer(searchServer), _docsumServer(docsumServer), _monitorServer(monitorServer), - _transport(), + _transport(std::make_shared<vespalib::NullCryptoEngine>(), 1), // disable encryption _ready(false), _failed(false), _doListen(true), diff --git a/searchlib/src/vespa/searchlib/fef/parametervalidator.cpp b/searchlib/src/vespa/searchlib/fef/parametervalidator.cpp index dda5ec0b719..b17592c8f74 100644 --- a/searchlib/src/vespa/searchlib/fef/parametervalidator.cpp +++ b/searchlib/src/vespa/searchlib/fef/parametervalidator.cpp @@ -51,7 +51,7 @@ ParameterValidator::Result::Result(size_t tag) : ParameterValidator::Result::Result(const Result &) = default; ParameterValidator::Result & ParameterValidator::Result::operator=(const Result &) = default; -ParameterValidator::Result::~Result() { } +ParameterValidator::Result::~Result() = default; void ParameterValidator::validateField(ParameterType::Enum type, diff --git a/standalone-container/src/main/java/com/yahoo/container/standalone/CloudConfigInstallVariables.java b/standalone-container/src/main/java/com/yahoo/container/standalone/CloudConfigInstallVariables.java index 0be4a55275c..2125fb0e499 100644 --- a/standalone-container/src/main/java/com/yahoo/container/standalone/CloudConfigInstallVariables.java +++ b/standalone-container/src/main/java/com/yahoo/container/standalone/CloudConfigInstallVariables.java @@ -33,6 +33,15 @@ public class CloudConfigInstallVariables implements CloudConfigOptions { } @Override + public int[] configServerZookeeperIds() { + return Optional.ofNullable(System.getenv("VESPA_CONFIGSERVER_ZOOKEEPER_IDS")) + .map(CloudConfigInstallVariables::multiValueParameterStream) + .orElseGet(Stream::empty) + .mapToInt(Integer::valueOf) + .toArray(); + } + + @Override public Optional<Long> zookeeperBarrierTimeout() { return getInstallVariable("zookeeper_barrier_timeout", Long::parseLong); } diff --git a/vespalib/src/vespa/vespalib/util/thread.cpp b/vespalib/src/vespa/vespalib/util/thread.cpp index d9341c7ec43..2d0118645ab 100644 --- a/vespalib/src/vespa/vespalib/util/thread.cpp +++ b/vespalib/src/vespa/vespalib/util/thread.cpp @@ -4,12 +4,12 @@ namespace vespalib { -__thread Thread *Thread::_currentThread = 0; +__thread Thread *Thread::_currentThread = nullptr; void Thread::Proxy::Run(FastOS_ThreadInterface *, void *) { - assert(_currentThread == 0); + assert(_currentThread == nullptr); _currentThread = &thread; start.await(); if (!cancel) { @@ -17,10 +17,10 @@ Thread::Proxy::Run(FastOS_ThreadInterface *, void *) runnable.run(); } assert(_currentThread == &thread); - _currentThread = 0; + _currentThread = nullptr; } -Thread::Proxy::~Proxy() { } +Thread::Proxy::~Proxy() = default; Thread::Thread(Runnable &runnable) : _proxy(*this, runnable), @@ -30,7 +30,7 @@ Thread::Thread(Runnable &runnable) _woken(false) { FastOS_ThreadInterface *thread = _pool.NewThread(&_proxy); - assert(thread != 0); + assert(thread != nullptr); (void)thread; } @@ -80,7 +80,7 @@ Thread & Thread::currentThread() { Thread *thread = _currentThread; - assert(thread != 0); + assert(thread != nullptr); return *thread; } diff --git a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp index 21d1de2a29b..ea657d4ec76 100644 --- a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp +++ b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp @@ -150,7 +150,7 @@ ThreadStackExecutorBase::start(uint32_t threads) assert(threads > 0); for (uint32_t i = 0; i < threads; ++i) { FastOS_ThreadInterface *thread = _pool->NewThread(_thread_init.get()); - assert(thread != 0); + assert(thread != nullptr); (void)thread; } } @@ -200,7 +200,7 @@ ThreadStackExecutorBase::execute(Task::UP task) } else { ++_stats.rejectedTasks; } - return std::move(task); + return task; } ThreadStackExecutorBase & |