diff options
21 files changed, 181 insertions, 93 deletions
diff --git a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java index 169b1c8557b..f98f0524fea 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java @@ -169,11 +169,7 @@ class OverrideProcessor implements PreProcessor { } if ( ! elementRegions.isEmpty()) { // match region - // match region in multi-region environments only - if ( environment.isMultiRegion() && ! elementRegions.contains(region)) return false; - - // explicit region implies multi-region environment - if ( ! environment.isMultiRegion() && elementEnvironments.isEmpty() ) return false; + if ( ! elementRegions.contains(region)) return false; } if ( ! elementTags.isEmpty()) { // match tags diff --git a/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java index 2152a62228b..0c49e9f41d9 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java @@ -3,7 +3,11 @@ package com.yahoo.config.application; import java.util.logging.Level; import com.yahoo.text.XML; -import org.w3c.dom.*; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NamedNodeMap; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; import javax.xml.transform.TransformerException; import java.util.ArrayList; @@ -18,6 +22,7 @@ import java.util.logging.Logger; * @author hmusum */ class PropertiesProcessor implements PreProcessor { + private final static Logger log = Logger.getLogger(PropertiesProcessor.class.getName()); private final LinkedHashMap<String, String> properties; @@ -27,7 +32,7 @@ class PropertiesProcessor implements PreProcessor { public Document process(Document input) throws TransformerException { Document doc = Xml.copyDocument(input); - final Document document = buildProperties(doc); + Document document = buildProperties(doc); applyProperties(document.getDocumentElement()); return document; } @@ -36,11 +41,9 @@ class PropertiesProcessor implements PreProcessor { NodeList list = input.getElementsByTagNameNS(XmlPreProcessor.preprocessNamespaceUri, "properties"); while (list.getLength() > 0) { Element propertiesElement = (Element) list.item(0); - //System.out.println("prop=" + propertiesElement); Element parent = (Element) propertiesElement.getParentNode(); for (Node node : XML.getChildren(propertiesElement)) { - //System.out.println("Found " + node.getNodeName() + ", " + node.getTextContent()); - final String propertyName = node.getNodeName(); + String propertyName = node.getNodeName(); if (properties.containsKey(propertyName)) { log.log(Level.WARNING, "Duplicate definition for property '" + propertyName + "' detected"); } @@ -53,8 +56,7 @@ class PropertiesProcessor implements PreProcessor { } private void applyProperties(Element parent) { - // System.out.println("applying properties for " + parent.getNodeName()); - final NamedNodeMap attributes = parent.getAttributes(); + NamedNodeMap attributes = parent.getAttributes(); for (int i = 0; i < attributes.getLength(); i++) { Node a = attributes.item(i); if (hasProperty(a)) { @@ -84,7 +86,7 @@ class PropertiesProcessor implements PreProcessor { // Use a list with keys sorted by length (longest key first) // Needed for replacing values where you have overlapping keys ArrayList<String> keys = new ArrayList<>(properties.keySet()); - Collections.sort(keys, Collections.reverseOrder(Comparator.comparing(String::length))); + keys.sort(Collections.reverseOrder(Comparator.comparing(String::length))); for (String key : keys) { String value = properties.get(key); @@ -92,10 +94,10 @@ class PropertiesProcessor implements PreProcessor { // first, the else branch will only happen when there cannot be an exact // match, i.e. where you want to replace only parts of the attribute or node value if (propertyValue.equals("${" + key + "}")) { - final String regex = "\\$\\{" + key + "\\}"; + String regex = "\\$\\{" + key + "}"; return propertyValue.replaceAll(regex, value); } else if (propertyValue.contains(key)) { - return propertyValue.replaceAll("\\$\\{" + key + "\\}", value); + return propertyValue.replaceAll("\\$\\{" + key + "}", value); } } throw new IllegalArgumentException("Unable to find property replace in " + propertyValue); @@ -116,10 +118,11 @@ class PropertiesProcessor implements PreProcessor { } private static boolean hasProperty(String s) { - return s.matches("^.*\\$\\{.+\\}.*$"); + return s.matches("^.*\\$\\{.+}.*$"); } public LinkedHashMap<String, String> getProperties() { return properties; } + } diff --git a/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java index 5f430f70584..3dd2af0b6ea 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java @@ -15,4 +15,5 @@ public class ValidationProcessor implements PreProcessor { throw new UnsupportedOperationException("XInclude not supported, use preprocess:include instead"); return input; } + }
\ No newline at end of file diff --git a/config-application-package/src/main/java/com/yahoo/config/application/Xml.java b/config-application-package/src/main/java/com/yahoo/config/application/Xml.java index cf391f2dd0e..525da509de6 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/Xml.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/Xml.java @@ -32,19 +32,18 @@ import java.util.logging.Logger; * @author hmusum */ public class Xml { + private static final Logger log = Logger.getLogger(Xml.class.getPackage().toString()); // Access to this needs to be synchronized (as it is in getDocumentBuilder() below) private static final DocumentBuilderFactory factory = createDocumentBuilderFactory(); public static Document getDocument(Reader reader) { - Document doc; try { - doc = getDocumentBuilder().parse(new InputSource(reader)); + return getDocumentBuilder().parse(new InputSource(reader)); } catch (SAXException | IOException e) { throw new IllegalArgumentException(e); } - return doc; } private static DocumentBuilderFactory createDocumentBuilderFactory() { @@ -67,7 +66,7 @@ public class Xml { /** * Creates a new XML document builder. * - * @return A new DocumentBuilder instance, or null if we fail to get one. + * @return a new DocumentBuilder instance, or null if we fail to get one. */ private static synchronized DocumentBuilder getDocumentBuilder() { try { @@ -114,7 +113,7 @@ public class Xml { } /** - * Utility method to get an XML element from a reader + * Utility method to get an XML element from a reader. * * @param reader the {@link Reader} to get an xml element from */ @@ -122,9 +121,7 @@ public class Xml { return XML.getDocument(reader).getDocumentElement(); } - /** - * @return The root element of each xml file under pathFromAppRoot/ in the app package - */ + /** Returns the root element of each xml file under pathFromAppRoot/ in the app package. */ public static List<Element> allElemsFromPath(ApplicationPackage app, String pathFromAppRoot) { List<Element> ret = new ArrayList<>(); List<NamedReader> files = null; @@ -156,4 +153,5 @@ public class Xml { } return children; } + } diff --git a/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java index 697d8c208d3..d4c8884db11 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java @@ -45,6 +45,7 @@ public class IncludeProcessorTest { <adminserver hostalias="node1"/> </admin> <content id="foo" version="1.0"> + <thread count="128" deploy:region="us-central us-east"/> <redundancy>1</redundancy> <documents> <document mode="index" type="music.sd"/> diff --git a/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java index ca074ef9704..4d972d15716 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java @@ -287,38 +287,6 @@ public class OverrideProcessorTest { assertOverride(Environment.from("test"), RegionName.from("us-west"), expected); } - @Test - public void testParsingInheritEnvironment() throws TransformerException { - String expected = - "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + - "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"?\" version=\"1.0\">" + - " <admin version=\"2.0\">" + - " <adminserver hostalias=\"node0\"/>" + - " </admin>" + - " <content id=\"foo\" version=\"1.0\">" + - " <redundancy>1</redundancy>" + - " <documents>" + - " <document mode='index' type='music'/>\n" + - " <document mode='index' type='music2'/>\n" + - " <document type='music2' mode='index' />\n" + - " </documents>" + - " <nodes>" + - // node1 is specified for us-west but does not match because region overrides implies environment=prod - " <node distribution-key=\"0\" hostalias=\"node0\"/>" + - " </nodes>" + - " </content>" + - " <container id=\"stateless\" version=\"1.0\">" + - " <search/>" + - " <component id=\"foo\" class=\"MyFoo\" bundle=\"foobundle\" />" + - " <component id=\"bar\" class=\"TestBar\" bundle=\"foobundle\" />" + - " <nodes>" + - " <node hostalias=\"node0\"/>" + - " </nodes>" + - " </container>" + - "</services>"; - assertOverride(Environment.from("staging"), RegionName.from("us-west"), expected); - } - @Test(expected = IllegalArgumentException.class) public void testParsingDifferentEnvInParentAndChild() throws TransformerException { String in = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + diff --git a/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java index 49dbacbae3d..bbccc8343a1 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java @@ -57,14 +57,14 @@ public class XmlPreprocessorTest { Tags.empty()).run()); // Difference from dev: node1 - // Difference from dev: no TestBar + // Difference from dev: no TestBar String expectedStaging = """ <?xml version="1.0" encoding="UTF-8" standalone="no"?> <!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> <services xmlns:deploy="vespa" xmlns:preprocess="properties" version="1.0"> <admin version="2.0"> - <adminserver hostalias="node1"/> + <adminserver hostalias="node0"/> </admin> <content id="foo" version="1.0"> <redundancy>1</redundancy> @@ -91,7 +91,81 @@ public class XmlPreprocessorTest { RegionName.defaultName(), Tags.empty()).run()); - String expectedUsWest = + String expectedPerfUsWest = + """ + <?xml version="1.0" encoding="UTF-8" standalone="no"?> + <!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> + <services xmlns:deploy="vespa" xmlns:preprocess="properties" version="1.0"> + <admin version="2.0"> + <adminserver hostalias="node0"/> + </admin> + <content id="foo" version="1.0"> + <redundancy>1</redundancy> + <documents> + <document mode="index" type="music.sd"/> + </documents> + <nodes> + <node distribution-key="0" hostalias="node0"/> + </nodes> + </content> + <container id="stateless" version="1.0"> + <search/> + <component bundle="foobundle" class="MyFoo" id="foo"/> + <nodes> + <node hostalias="node0" baseport="5000"/> + </nodes> + </container> + </services>"""; + TestBase.assertDocument(expectedPerfUsWest, + new XmlPreProcessor(appDir, + services, + InstanceName.defaultName(), + Environment.perf, + RegionName.from("us-west"), + Tags.empty()).run()); + + String expectedPerfUsEastAndCentral = + """ + <?xml version="1.0" encoding="UTF-8" standalone="no"?> + <!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> + <services xmlns:deploy="vespa" xmlns:preprocess="properties" version="1.0"> + <admin version="2.0"> + <adminserver hostalias="node0"/> + </admin> + <content id="foo" version="1.0"> + <thread count="128"/> + <redundancy>1</redundancy> + <documents> + <document mode="index" type="music.sd"/> + </documents> + <nodes> + <node distribution-key="0" hostalias="node0"/> + </nodes> + </content> + <container id="stateless" version="1.0"> + <search/> + <component bundle="foobundle" class="MyFoo" id="foo"/> + <nodes> + <node hostalias="node0" baseport="5000"/> + </nodes> + </container> + </services>"""; + TestBase.assertDocument(expectedPerfUsEastAndCentral, + new XmlPreProcessor(appDir, + services, + InstanceName.defaultName(), + Environment.perf, + RegionName.from("us-east"), + Tags.empty()).run()); + TestBase.assertDocument(expectedPerfUsEastAndCentral, + new XmlPreProcessor(appDir, + services, + InstanceName.defaultName(), + Environment.perf, + RegionName.from("us-central"), + Tags.empty()).run()); + + String expectedProdUsWest = """ <?xml version="1.0" encoding="UTF-8" standalone="no"?> <!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> @@ -125,7 +199,7 @@ public class XmlPreprocessorTest { </nodes> </container> </services>"""; - TestBase.assertDocument(expectedUsWest, + TestBase.assertDocument(expectedProdUsWest, new XmlPreProcessor(appDir, services, InstanceName.defaultName(), @@ -133,7 +207,7 @@ public class XmlPreprocessorTest { RegionName.from("us-west"), Tags.empty()).run()); - String expectedUsEastAndCentral = + String expectedProdUsEastAndCentral = """ <?xml version="1.0" encoding="UTF-8" standalone="no"?> <!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> @@ -142,6 +216,7 @@ public class XmlPreprocessorTest { <adminserver hostalias="node1"/> </admin> <content id="foo" version="1.0"> + <thread count="128"/> <redundancy>1</redundancy> <documents> <document mode="index" type="music.sd"/> @@ -166,14 +241,14 @@ public class XmlPreprocessorTest { </nodes> </container> </services>"""; - TestBase.assertDocument(expectedUsEastAndCentral, + TestBase.assertDocument(expectedProdUsEastAndCentral, new XmlPreProcessor(appDir, services, InstanceName.defaultName(), Environment.prod, RegionName.from("us-east"), Tags.empty()).run()); - TestBase.assertDocument(expectedUsEastAndCentral, + TestBase.assertDocument(expectedProdUsEastAndCentral, new XmlPreProcessor(appDir, services, InstanceName.defaultName(), diff --git a/config-application-package/src/test/resources/multienvapp/services.xml b/config-application-package/src/test/resources/multienvapp/services.xml index 450291fa609..5c0ad50d87d 100644 --- a/config-application-package/src/test/resources/multienvapp/services.xml +++ b/config-application-package/src/test/resources/multienvapp/services.xml @@ -22,6 +22,7 @@ <preprocess:include file='jdisc.xml'/> <content version='1.0' id='foo'> + <thread deploy:region="us-central us-east" count="128"/> <preprocess:include file='content/content_foo.xml'/> </content> diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java index 2d3d5e062f7..bbec1f14c13 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java @@ -34,8 +34,9 @@ public enum Environment { /** Returns whether this environment is production (prod) */ public boolean isProduction() { return this == prod; } - /** Returns whether this environment can exist in multiple regions */ - public boolean isMultiRegion() { return this == prod || this == dev; } + /** Returns whether this environment can exist in multiple regions. @deprecated they all are */ + @Deprecated // TODO: Remove after June 2023 + public boolean isMultiRegion() { return true; } /** Returns the prod environment. This is useful for non-hosted properties where we just need any consistent value */ public static Environment defaultEnvironment() { return prod; } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java index 17010fe3fd3..0b56d811712 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java @@ -27,7 +27,7 @@ public class Zone { .name(CloudName.from(configserverConfig.cloud())) .dynamicProvisioning(cloudConfig.dynamicProvisioning()) .allowHostSharing(cloudConfig.allowHostSharing()) - .allowEnclave(cloudConfig.dynamicProvisioning()) + .allowEnclave(configserverConfig.cloud().equals("aws") || configserverConfig.cloud().equals("gcp")) .requireAccessControl(cloudConfig.requireAccessControl()) .account(CloudAccount.from(cloudConfig.account())) .build(), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java index cb779d847c3..9643de52c29 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializer.java @@ -32,6 +32,7 @@ public class VersionStatusSerializer { // VersionStatus fields private static final String versionsField = "versions"; + private static final String currentMajorField = "currentMajor"; // VespaVersion fields private static final String releaseCommitField = "releaseCommit"; @@ -61,12 +62,14 @@ public class VersionStatusSerializer { Slime slime = new Slime(); Cursor root = slime.setObject(); versionsToSlime(status.versions(), root.setArray(versionsField)); + root.setLong(currentMajorField, status.currentMajor()); return slime; } public VersionStatus fromSlime(Slime slime) { Inspector root = slime.get(); - return new VersionStatus(vespaVersionsFromSlime(root.field(versionsField))); + return new VersionStatus(vespaVersionsFromSlime(root.field(versionsField)), + (int) root.field(currentMajorField).asLong()); } private void versionsToSlime(List<VespaVersion> versions, Cursor array) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java index a00e80b17d4..9bee5623774 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/certificate/EndpointCertificatesHandler.java @@ -42,7 +42,7 @@ public class EndpointCertificatesHandler extends ThreadedHttpRequestHandler { public HttpResponse handle(HttpRequest request) { if (request.getMethod().equals(GET)) return listEndpointCertificates(); - if (request.getMethod().equals(POST)) return reRequestEndpointCertificateFor(request.getProperty("application")); + if (request.getMethod().equals(POST)) return reRequestEndpointCertificateFor(request.getProperty("application"), request.getProperty("ignoreExistingMetadata") != null); throw new RestApiException.MethodNotAllowed(request); } @@ -59,7 +59,7 @@ public class EndpointCertificatesHandler extends ThreadedHttpRequestHandler { return new StringResponse(requestsWithNames); } - public StringResponse reRequestEndpointCertificateFor(String instanceId) { + public StringResponse reRequestEndpointCertificateFor(String instanceId, boolean ignoreExistingMetadata) { ApplicationId applicationId = ApplicationId.fromFullString(instanceId); try (var lock = curator.lock(TenantAndApplicationId.from(applicationId))) { @@ -67,7 +67,7 @@ public class EndpointCertificatesHandler extends ThreadedHttpRequestHandler { .orElseThrow(() -> new RestApiException.NotFound("No certificate found for application " + applicationId.serializedForm())); EndpointCertificateMetadata reRequestedMetadata = endpointCertificateProvider.requestCaSignedCertificate( - applicationId, endpointCertificateMetadata.requestedDnsSans(), Optional.of(endpointCertificateMetadata)); + applicationId, endpointCertificateMetadata.requestedDnsSans(), ignoreExistingMetadata ? Optional.empty() : Optional.of(endpointCertificateMetadata)); curator.writeEndpointCertificateMetadata(applicationId, reRequestedMetadata); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java index ed7bd924ccc..732327e25f6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java @@ -35,13 +35,14 @@ import java.util.stream.Collectors; * @author bratseth * @author mpolden */ -public record VersionStatus(List<VespaVersion> versions) { +public record VersionStatus(List<VespaVersion> versions, int currentMajor) { private static final Logger log = Logger.getLogger(VersionStatus.class.getName()); /** Create a version status. DO NOT USE: Public for testing and serialization only */ - public VersionStatus(List<VespaVersion> versions) { + public VersionStatus(List<VespaVersion> versions, int currentMajor) { this.versions = List.copyOf(versions); + this.currentMajor = currentMajor; } /** Returns the current version of controllers in this system */ @@ -98,11 +99,12 @@ public record VersionStatus(List<VespaVersion> versions) { } /** Create the empty version status */ - public static VersionStatus empty() { return new VersionStatus(List.of()); } + public static VersionStatus empty() { return new VersionStatus(List.of(), -1); } /** Create a full, updated version status. This is expensive and should be done infrequently */ public static VersionStatus compute(Controller controller) { VersionStatus versionStatus = controller.readVersionStatus(); + int currentMajor = versionStatus.currentMajor(); List<NodeVersion> systemApplicationVersions = findSystemApplicationVersions(controller, versionStatus); Map<ControllerVersion, List<HostName>> controllerVersions = findControllerVersions(controller); @@ -164,6 +166,8 @@ public record VersionStatus(List<VespaVersion> versions) { controller, versionStatus); versions.add(vespaVersion); + if (vespaVersion.confidence().equalOrHigherThan(Confidence.high)) + currentMajor = Math.max(currentMajor, vespaVersion.versionNumber().getMajor()); } catch (IllegalArgumentException e) { log.log(Level.WARNING, "Unable to create VespaVersion for version " + statistics.version().toFullString(), e); @@ -172,7 +176,7 @@ public record VersionStatus(List<VespaVersion> versions) { Collections.sort(versions); - return new VersionStatus(versions); + return new VersionStatus(versions, currentMajor); } private static List<NodeVersion> findSystemApplicationVersions(Controller controller, VersionStatus versionStatus) { @@ -275,12 +279,7 @@ public record VersionStatus(List<VespaVersion> versions) { /** Whether no version on a newer major, with high confidence, can be deployed. */ public boolean isOnCurrentMajor(Version version) { - for (VespaVersion available : deployableVersions()) - if ( available.confidence().equalOrHigherThan(Confidence.high) - && available.versionNumber().getMajor() > version.getMajor()) - return false; - - return true; + return version.getMajor() >= currentMajor; } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdaterTest.java index e6f46e0630d..962288f9073 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdaterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdaterTest.java @@ -25,11 +25,10 @@ public class VersionStatusUpdaterTest { @Test void testVersionUpdating() { ControllerTester tester = new ControllerTester(); - tester.controller().updateVersionStatus(new VersionStatus(Collections.emptyList())); + tester.controller().updateVersionStatus(VersionStatus.empty()); assertFalse(tester.controller().readVersionStatus().systemVersion().isPresent()); - VersionStatusUpdater updater = new VersionStatusUpdater(tester.controller(), Duration.ofDays(1) - ); + VersionStatusUpdater updater = new VersionStatusUpdater(tester.controller(), Duration.ofDays(1)); updater.maintain(); assertTrue(tester.controller().readVersionStatus().systemVersion().isPresent()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java index 618c33835bd..7c3ba5aeadf 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/VersionStatusSerializerTest.java @@ -32,7 +32,7 @@ public class VersionStatusSerializerTest { vespaVersions.add(new VespaVersion(version, "cafe", Instant.now(), true, true, false, nodeVersions(Version.fromString("5.0"), Version.fromString("5.1"), "cfg1", "cfg2", "cfg3"), VespaVersion.Confidence.normal)); - VersionStatus status = new VersionStatus(vespaVersions); + VersionStatus status = new VersionStatus(vespaVersions, 5); VersionStatusSerializer serializer = new VersionStatusSerializer(new NodeVersionSerializer()); VersionStatus deserialized = serializer.fromSlime(serializer.toSlime(status)); @@ -49,6 +49,7 @@ public class VersionStatusSerializerTest { assertEquals(a.nodeVersions(), b.nodeVersions()); assertEquals(a.confidence(), b.confidence()); } + assertEquals(status.currentMajor(), deserialized.currentMajor()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java index c942a7ad63d..bcc1f27c89f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java @@ -105,7 +105,7 @@ public class DeploymentApiTest extends ControllerContainerTest { } censored.add(version); } - return new VersionStatus(censored); + return new VersionStatus(censored, versionStatus.currentMajor()); } } diff --git a/searchcore/src/tests/proton/flushengine/CMakeLists.txt b/searchcore/src/tests/proton/flushengine/CMakeLists.txt index a47b3f2c93f..8d186dfe189 100644 --- a/searchcore/src/tests/proton/flushengine/CMakeLists.txt +++ b/searchcore/src/tests/proton/flushengine/CMakeLists.txt @@ -10,5 +10,5 @@ vespa_add_executable(searchcore_flushengine_test_app TEST vespa_add_test( NAME searchcore_flushengine_test_app COMMAND searchcore_flushengine_test_app - ENVIRONMENT "VESPA_LOG_LEVEL=all;VESPA_LOG_TARGET=file:vlog.txt" + ENVIRONMENT "VESPA_LOG_LEVEL=all" ) diff --git a/storage/src/tests/distributor/removeoperationtest.cpp b/storage/src/tests/distributor/removeoperationtest.cpp index d88a0a574cc..d352d23bb8c 100644 --- a/storage/src/tests/distributor/removeoperationtest.cpp +++ b/storage/src/tests/distributor/removeoperationtest.cpp @@ -6,7 +6,8 @@ #include <vespa/storage/distributor/distributor_stripe.h> #include <vespa/storage/distributor/operations/external/removeoperation.h> #include <vespa/storageapi/message/persistence.h> -#include <vespa/vespalib/gtest/gtest.h> +#include <gtest/gtest.h> +#include <gmock/gmock.h> using documentapi::TestAndSetCondition; using document::test::makeDocumentBucket; @@ -119,6 +120,7 @@ void ExtRemoveOperationTest::set_up_tas_remove_with_2_nodes(ReplicaState replica auto remove = createRemove(docId); remove->setCondition(TestAndSetCondition("test.foo")); + remove->getTrace().setLevel(9); sendRemove(std::move(remove)); if (replica_state == ReplicaState::INCONSISTENT) { ASSERT_EQ("Get => 1,Get => 0", _sender.getCommands(true)); @@ -304,4 +306,41 @@ TEST_F(ExtRemoveOperationTest, failed_condition_probe_fails_op_with_returned_err _sender.getLastReply()); } +TEST_F(ExtRemoveOperationTest, trace_is_propagated_from_condition_probe_gets_ok_probe_case) { + ASSERT_NO_FATAL_FAILURE(set_up_tas_remove_with_2_nodes(ReplicaState::INCONSISTENT)); + + ASSERT_EQ(sent_get_command(0)->getTrace().getLevel(), 9); + auto get_reply = make_get_reply(0, 50, false, true); + MBUS_TRACE(get_reply->getTrace(), 1, "a foo walks into a bar"); + + op->receive(_sender, get_reply); + op->receive(_sender, make_get_reply(1, 50, false, true)); + + ASSERT_EQ("Get => 1,Get => 0,Remove => 1,Remove => 0", _sender.getCommands(true)); + reply_with(make_remove_reply(2, 50)); // remove from node 1 + reply_with(make_remove_reply(3, 50)); // remove from node 0 + ASSERT_EQ(_sender.replies().size(), 1); + auto remove_reply = sent_reply<api::RemoveReply>(0); + + auto trace_str = remove_reply->getTrace().toString(); + EXPECT_THAT(trace_str, HasSubstr("a foo walks into a bar")); +} + +TEST_F(ExtRemoveOperationTest, trace_is_propagated_from_condition_probe_gets_failed_probe_case) { + ASSERT_NO_FATAL_FAILURE(set_up_tas_remove_with_2_nodes(ReplicaState::INCONSISTENT)); + + auto get_reply = make_get_reply(0, 50, false, false); + MBUS_TRACE(get_reply->getTrace(), 1, "a foo walks into a zoo"); + + op->receive(_sender, get_reply); + op->receive(_sender, make_get_reply(1, 50, false, false)); + + ASSERT_EQ("Get => 1,Get => 0", _sender.getCommands(true)); + ASSERT_EQ(_sender.replies().size(), 1); + auto remove_reply = sent_reply<api::RemoveReply>(0); + + auto trace_str = remove_reply->getTrace().toString(); + EXPECT_THAT(trace_str, HasSubstr("a foo walks into a zoo")); +} + } // storage::distributor diff --git a/storage/src/vespa/storage/distributor/operations/external/check_condition.h b/storage/src/vespa/storage/distributor/operations/external/check_condition.h index 2a659c55081..062c9bb831d 100644 --- a/storage/src/vespa/storage/distributor/operations/external/check_condition.h +++ b/storage/src/vespa/storage/distributor/operations/external/check_condition.h @@ -136,7 +136,7 @@ public: const DistributorNodeContext& node_ctx, const DistributorStripeOperationContext& op_ctx, PersistenceOperationMetricSet& metric, - uint32_t trace_level = 0); // TODO remove default value + uint32_t trace_level); private: [[nodiscard]] bool replica_set_changed_after_get_operation() const; diff --git a/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp index d3001c37f7c..59ae4120fd6 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp @@ -48,13 +48,13 @@ void RemoveOperation::start_conditional_remove(DistributorStripeMessageSender& s document::Bucket bucket(_msg->getBucket().getBucketSpace(), _doc_id_bucket_id); _check_condition = CheckCondition::create_if_inconsistent_replicas(bucket, _bucket_space, _msg->getDocumentId(), _msg->getCondition(), _node_ctx, _op_ctx, - _temp_metric); + _temp_metric, _msg->getTrace().getLevel()); if (!_check_condition) { start_direct_remove_dispatch(sender); } else { // Inconsistent replicas; need write repair _check_condition->start_and_send(sender); - const auto& outcome = _check_condition->maybe_outcome(); // Might be done immediately + auto& outcome = _check_condition->maybe_outcome(); // Might be done immediately if (outcome) { on_completed_check_condition(*outcome, sender); } @@ -110,7 +110,7 @@ RemoveOperation::onReceive(DistributorStripeMessageSender& sender, const std::sh { if (_check_condition) { _check_condition->handle_reply(sender, msg); - const auto& outcome = _check_condition->maybe_outcome(); + auto& outcome = _check_condition->maybe_outcome(); if (!outcome) { return; // Condition check not done yet } @@ -131,9 +131,12 @@ RemoveOperation::onReceive(DistributorStripeMessageSender& sender, const std::sh _tracker.receiveReply(sender, reply); } -void RemoveOperation::on_completed_check_condition(const CheckCondition::Outcome& outcome, +void RemoveOperation::on_completed_check_condition(CheckCondition::Outcome& outcome, DistributorStripeMessageSender& sender) { + if (!outcome.trace().isEmpty()) { + _tracker.add_trace_tree_to_reply(outcome.steal_trace()); + } if (outcome.matched_condition()) { _msg->clear_condition(); // Transform to unconditional Remove start_direct_remove_dispatch(sender); diff --git a/storage/src/vespa/storage/distributor/operations/external/removeoperation.h b/storage/src/vespa/storage/distributor/operations/external/removeoperation.h index ba6d42c5108..349a6182937 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removeoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/removeoperation.h @@ -42,7 +42,7 @@ private: void start_direct_remove_dispatch(DistributorStripeMessageSender& sender); void start_conditional_remove(DistributorStripeMessageSender& sender); - void on_completed_check_condition(const CheckCondition::Outcome& outcome, + void on_completed_check_condition(CheckCondition::Outcome& outcome, DistributorStripeMessageSender& sender); [[nodiscard]] bool has_condition() const noexcept; }; |