diff options
79 files changed, 801 insertions, 1064 deletions
diff --git a/athenz-identity-provider-service/pom.xml b/athenz-identity-provider-service/pom.xml index 5d060838891..1b1edcc64b8 100644 --- a/athenz-identity-provider-service/pom.xml +++ b/athenz-identity-provider-service/pom.xml @@ -16,12 +16,45 @@ <dependencies> <!-- PROVIDED --> <dependency> + <groupId>com.google.inject</groupId> + <artifactId>guice</artifactId> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>component</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + <dependency> <groupId>com.yahoo.vespa</groupId> <artifactId>container-dev</artifactId> <version>${project.version}</version> <scope>provided</scope> </dependency> <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>jdisc_core</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>vespalog</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>com.fasterxml.jackson.core</groupId> + <artifactId>jackson-core</artifactId> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>com.fasterxml.jackson.core</groupId> + <artifactId>jackson-databind</artifactId> + <scope>provided</scope> + </dependency> + <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-server</artifactId> <scope>provided</scope> @@ -55,6 +88,12 @@ <version>${project.version}</version> <scope>provided</scope> </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>security-utils</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> <!-- COMPILE --> <dependency> @@ -68,20 +107,19 @@ <!-- TEST --> <dependency> - <groupId>junit</groupId> - <artifactId>junit</artifactId> + <groupId>com.yahoo.vespa</groupId> + <artifactId>zkfacade</artifactId> + <version>${project.version}</version> <scope>test</scope> </dependency> <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>container-test</artifactId> - <version>${project.version}</version> + <groupId>org.hamcrest</groupId> + <artifactId>hamcrest-core</artifactId> <scope>test</scope> </dependency> <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>testutil</artifactId> - <version>${project.version}</version> + <groupId>junit</groupId> + <artifactId>junit</artifactId> <scope>test</scope> </dependency> <dependency> diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java index 92ba8f0edfb..66ab5255b6f 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/identitydocument/IdentityDocumentGeneratorTest.java @@ -24,10 +24,8 @@ import com.yahoo.vespa.hosted.provision.node.Allocation; import com.yahoo.vespa.hosted.provision.node.Generation; import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.testutils.MockNodeFlavors; -import org.hamcrest.Matchers; import org.junit.Test; -import java.util.HashSet; import java.util.Optional; import java.util.Set; @@ -35,6 +33,7 @@ import static com.yahoo.vespa.hosted.athenz.instanceproviderservice.TestUtils.ge import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.hamcrest.core.IsCollectionContaining.hasItem; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -92,7 +91,7 @@ public class IdentityDocumentGeneratorTest { assertEquals(expectedProviderUniqueId, signedIdentityDocument.providerUniqueId()); // Validate that container ips are present - assertThat(signedIdentityDocument.ipAddresses(), Matchers.containsInAnyOrder("::1")); + assertThat(signedIdentityDocument.ipAddresses(), hasItem("::1")); IdentityDocumentSigner signer = new IdentityDocumentSigner(); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TlsSecretsKeys.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TlsSecretsKeys.java index eaa4916d8fc..78d39ef996b 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TlsSecretsKeys.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TlsSecretsKeys.java @@ -59,17 +59,15 @@ public class TlsSecretsKeys { } private Optional<TlsSecrets> readFromSecretStore(Optional<String> secretKeyname) { - if(secretKeyname.isEmpty()) return Optional.empty(); - TlsSecrets tlsSecretParameters = TlsSecrets.MISSING; + if (secretKeyname.isEmpty()) return Optional.empty(); try { String cert = secretStore.getSecret(secretKeyname.get() + "-cert"); String key = secretStore.getSecret(secretKeyname.get() + "-key"); - tlsSecretParameters = new TlsSecrets(cert, key); + return Optional.of(new TlsSecrets(cert, key)); } catch (RuntimeException e) { // Assume not ready yet -// log.log(LogLevel.DEBUG, "Could not fetch certificate/key with prefix: " + secretKeyname.get(), e); + return Optional.of(TlsSecrets.MISSING); } - return Optional.of(tlsSecretParameters); } /** Returns a transaction which deletes these tls secrets key if they exist */ diff --git a/container-search/src/main/java/com/yahoo/prelude/Index.java b/container-search/src/main/java/com/yahoo/prelude/Index.java index 8433939090b..65d5879b004 100644 --- a/container-search/src/main/java/com/yahoo/prelude/Index.java +++ b/container-search/src/main/java/com/yahoo/prelude/Index.java @@ -66,10 +66,16 @@ public class Index { private boolean numerical = false; private long predicateUpperBound = Long.MAX_VALUE; private long predicateLowerBound = Long.MIN_VALUE; + /** True if this is an <i>exact</i> index - which should match tokens containing any characters */ private boolean exact = false; + private boolean isNGram = false; - private int gramSize=2; + private int gramSize = 2; + + /** Whether implicit phrases should lead to a phrase item or an and item */ + private boolean phraseSegmenting = true; + /** The string terminating an exact token in this index, or null to use the default (space) */ private String exactTerminator = null; @@ -178,6 +184,10 @@ public class Index { setNumerical(true); } else if (commandString.startsWith("predicate-bounds ")) { setPredicateBounds(commandString.substring(17)); + } else if (commandString.equals("phrase-segmenting")) { + setPhraseSegmenting(true); + } else if (commandString.startsWith("phrase-segmenting ")) { + setPhraseSegmenting(Boolean.parseBoolean(commandString.substring("phrase-segmenting ".length()))); } else { commands.add(commandString); } @@ -307,6 +317,10 @@ public class Index { public long getPredicateLowerBound() { return predicateLowerBound; } + public boolean getPhraseSegmenting() { return phraseSegmenting; } + + public boolean setPhraseSegmenting(boolean phraseSegmenting) { return this.phraseSegmenting = phraseSegmenting; } + /** Returns all the literal command strings given as arguments to addCommand in this instance */ public List<String> allCommands() { return allCommands; } diff --git a/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java b/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java index 1fd8cce9889..76eef33d6c0 100644 --- a/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java +++ b/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java @@ -273,17 +273,13 @@ public class IndexFacts { return false; } - /** - * @return whether it is permissible to update this object - */ + /** Returns whether it is permissible to update this object */ public boolean isFrozen() { return frozen; } private void ensureNotFrozen() { - if (frozen) { - throw new IllegalStateException("Tried to modify frozen IndexFacts instance."); - } + if (frozen) throw new IllegalStateException("Tried to modify frozen IndexFacts instance."); } public String getDefaultPosition(String sdName) { @@ -307,7 +303,8 @@ public class IndexFacts { return new Session(sources, restrict); } - public Session newSession(Collection<String> sources, Collection<String> restrict, + public Session newSession(Collection<String> sources, + Collection<String> restrict, Set<String> candidateDocumentTypes) { return new Session(sources, restrict, candidateDocumentTypes); } diff --git a/container-search/src/main/java/com/yahoo/prelude/IndexModel.java b/container-search/src/main/java/com/yahoo/prelude/IndexModel.java index c83294efed7..062a514056b 100644 --- a/container-search/src/main/java/com/yahoo/prelude/IndexModel.java +++ b/container-search/src/main/java/com/yahoo/prelude/IndexModel.java @@ -24,7 +24,6 @@ public final class IndexModel { private static final Logger log = Logger.getLogger(IndexModel.class.getName()); - // Copied from MasterClustersInfoUpdater. It's a temporary workaround for IndexFacts private Map<String, List<String>> masterClusters; private Map<String, SearchDefinition> searchDefinitions; private SearchDefinition unionSearchDefinition; @@ -34,9 +33,6 @@ public final class IndexModel { this(Collections.emptyMap(), Collections.singleton(searchDefinition)); } - /** - * Create an index model. - */ public IndexModel(Map<String, List<String>> masterClusters, Collection<SearchDefinition> searchDefinitions) { this.masterClusters = masterClusters; this.searchDefinitions = searchDefinitions.stream().collect(Collectors.toMap(sd -> sd.getName(), sd -> sd)); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/IndexedSegmentItem.java b/container-search/src/main/java/com/yahoo/prelude/query/IndexedSegmentItem.java index bf032ec03a8..b459f0aa902 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/IndexedSegmentItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/IndexedSegmentItem.java @@ -38,12 +38,14 @@ public abstract class IndexedSegmentItem extends TaggableSegmentItem implements } // encode index bytes + @Override protected void encodeThis(ByteBuffer buffer) { super.encodeThis(buffer); putString(index, buffer); } /** Sets the name of the index to search */ + @Override public void setIndexName(String index) { if (index == null) { index = ""; @@ -59,17 +61,17 @@ public abstract class IndexedSegmentItem extends TaggableSegmentItem implements } } + @Override public boolean equals(Object object) { - if (!super.equals(object)) { - return false; - } + if ( ! super.equals(object)) return false; + IndexedItem other = (IndexedItem) object; // Ensured by superclass - if (!this.index.equals(other.getIndexName())) { - return false; - } + if ( ! this.index.equals(other.getIndexName())) return false; + return true; } + @Override public int hashCode() { return super.hashCode() + 31 * index.hashCode(); } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/PhraseItem.java b/container-search/src/main/java/com/yahoo/prelude/query/PhraseItem.java index 4de0af1f408..2d648557d9c 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/PhraseItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/PhraseItem.java @@ -76,15 +76,15 @@ public class PhraseItem extends CompositeIndexedItem { public void addItem(Item item) { if (item instanceof WordItem || item instanceof PhraseSegmentItem || item instanceof WordAlternativesItem) { addIndexedItem((IndexedItem) item); - } else if (item instanceof IntItem) { + } + else if (item instanceof IntItem) { addIndexedItem(convertIntToWord(item)); - } else if (item instanceof PhraseItem) { - PhraseItem phrase = (PhraseItem) item; - - for (Iterator<Item> i = phrase.getItemIterator(); i.hasNext();) { + } + else if (item instanceof PhraseItem || item instanceof AndSegmentItem) { + for (Iterator<Item> i = ((CompositeItem) item).getItemIterator(); i.hasNext();) addIndexedItem((IndexedItem) i.next()); - } - } else { + } + else { throw new IllegalArgumentException("Can not add " + item + " to a phrase"); } } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/PhraseSegmentItem.java b/container-search/src/main/java/com/yahoo/prelude/query/PhraseSegmentItem.java index 53a57a968f5..542f1393852 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/PhraseSegmentItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/PhraseSegmentItem.java @@ -52,7 +52,7 @@ public class PhraseSegmentItem extends IndexedSegmentItem { } public PhraseSegmentItem(String rawWord, String current, boolean isFromQuery, - boolean stemmed, Substring substring) { + boolean stemmed, Substring substring) { super(rawWord, current, isFromQuery, stemmed, substring); } @@ -146,16 +146,16 @@ public class PhraseSegmentItem extends IndexedSegmentItem { } - /** - * Returns false, no parenthezes for phrases - */ + /** Returns false, no parenthezes for phrases */ protected boolean shouldParenthize() { return false; } /** Segment phrase items uses a empty heading instead of "SPHRASE " */ + @Override protected void appendHeadingString(StringBuilder buffer) {} + @Override protected void appendBodyString(StringBuilder buffer) { appendIndexString(buffer); appendContentsString(buffer); @@ -175,14 +175,13 @@ public class PhraseSegmentItem extends IndexedSegmentItem { } // TODO: Must check all pertinent items + @Override public boolean equals(Object object) { - if (!super.equals(object)) { - return false; - } - // PhraseSegmentItem other = (PhraseSegmentItem) object; // Ensured by superclass + if ( ! super.equals(object)) return false; return true; } + @Override public String getIndexedString() { StringBuilder buf = new StringBuilder(); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/SegmentItem.java b/container-search/src/main/java/com/yahoo/prelude/query/SegmentItem.java index 1227a7f80cf..1c3eb261f90 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/SegmentItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/SegmentItem.java @@ -126,6 +126,7 @@ public abstract class SegmentItem extends CompositeItem implements BlockItem { // TODO: Add a getItemIterator which is safe for immutability /** Return a deep copy of this object */ + @Override public SegmentItem clone() { SegmentItem copy; synchronized(this) { diff --git a/container-search/src/main/java/com/yahoo/prelude/query/SimpleIndexedItem.java b/container-search/src/main/java/com/yahoo/prelude/query/SimpleIndexedItem.java index f230c76c954..a8dc9db1928 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/SimpleIndexedItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/SimpleIndexedItem.java @@ -38,6 +38,7 @@ public abstract class SimpleIndexedItem extends SimpleTaggableItem implements In } /** Sets the name of the index to search */ + @Override public void setIndexName(String index) { if (index == null) { index = ""; diff --git a/container-search/src/main/java/com/yahoo/prelude/query/SimpleTaggableItem.java b/container-search/src/main/java/com/yahoo/prelude/query/SimpleTaggableItem.java index 6cbd2286a8f..6f82f340f4b 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/SimpleTaggableItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/SimpleTaggableItem.java @@ -25,7 +25,7 @@ public abstract class SimpleTaggableItem extends Item implements TaggableItem { uniqueID = id; } - /** See {@link TaggableItem#setConnectivity} */ + @Override public void setConnectivity(Item item, double connectivity) { setHasUniqueID(true); item.setHasUniqueID(true); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/TaggableItem.java b/container-search/src/main/java/com/yahoo/prelude/query/TaggableItem.java index 03e85fa3260..8e2a472fd7c 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/TaggableItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/TaggableItem.java @@ -34,7 +34,6 @@ public interface TaggableItem { Item getConnectedItem(); double getConnectivity(); - /** * Used for setting explicit term significance (in the tf/idf sense) to a single term or phrase, * relative to the rest of the query. diff --git a/container-search/src/main/java/com/yahoo/prelude/query/TermItem.java b/container-search/src/main/java/com/yahoo/prelude/query/TermItem.java index 8fdf147ce0b..2c33e7a2630 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/TermItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/TermItem.java @@ -44,7 +44,7 @@ public abstract class TermItem extends SimpleIndexedItem implements BlockItem { this.origin = origin; } - final public int encode(ByteBuffer buffer) { + public final int encode(ByteBuffer buffer) { encodeThis(buffer); return 1; } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java index 8297a566a72..cd8579be7f0 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AbstractParser.java @@ -228,13 +228,13 @@ public abstract class AbstractParser implements CustomParser { protected abstract Item parseItems(); /** - * Assigns the default index to query terms having no default index The - * parser _should_ have done this, for some reason it doesn't + * Assigns the default index to query terms having no default index. The + * parser _should_ have done this, for some reason it doesn't. * - * @param defaultIndex The default index to assign. - * @param item The item to check. + * @param defaultIndex the default index to assign + * @param item the item to check */ - private static void assignDefaultIndex(final String defaultIndex, Item item) { + private static void assignDefaultIndex(String defaultIndex, Item item) { if (defaultIndex == null || item == null) return; if (item instanceof IndexedItem) { @@ -253,9 +253,6 @@ public abstract class AbstractParser implements CustomParser { /** * Unicode normalizes some piece of natural language text. The chosen form * is compatibility decomposition, canonical composition (NFKC). - * - * @param input The string to normalize. - * @return The normalized string. */ protected String normalize(String input) { if (input == null || input.length() == 0) return input; @@ -272,8 +269,8 @@ public abstract class AbstractParser implements CustomParser { /** * Tokenizes the given string and initializes tokens with the found tokens. * - * @param query the string to tokenize. - * @param defaultIndexName the name of the index to use as default. + * @param query the string to tokenize + * @param defaultIndexName the name of the index to use as default * @param indexFacts resolved information about the index we are searching * @param language the language set for this query, or null if none */ @@ -324,6 +321,13 @@ public abstract class AbstractParser implements CustomParser { } } + /** + * Segments a token + * + * @param indexName the index name which preceeded this token, or null if none + * @param token the token to segment + * @return the resulting item + */ // TODO: The segmenting stuff is a mess now, this will fix it: // - Make Segmenter a class which is instantiated per parsing // - Make the instance know the language, etc and do all dispatching internally @@ -331,13 +335,13 @@ public abstract class AbstractParser implements CustomParser { // TODO: Use segmenting for forced phrase searches? // // Language detection currently depends on tokenization (see generateLanguageDetectionTextFrom), but - // - the API's was originally not constructed for that, so a careful nd somewhat unsatisfactory dance - // most be carried out to make it work + // - the API's was originally not constructed for that, so a careful and somewhat unsatisfactory dance + // must be carried out to make it work // - it should really depend on parsing // This can be solved by making the segment method language independent by // always producing a query item containing the token text and resolve it to a WordItem or // SegmentItem after parsing and language detection. - protected Item segment(Token token) { + protected Item segment(String indexName, Token token) { String normalizedToken = normalize(token.toString()); if (token.isSpecial()) { @@ -361,13 +365,23 @@ public abstract class AbstractParser implements CustomParser { return new WordItem(segments.get(0), "", true, token.substring); } - CompositeItem composite = new PhraseSegmentItem(token.toString(), normalizedToken, true, false, token.substring); + CompositeItem composite; + if (indexFacts.getIndex(indexName).getPhraseSegmenting()) { + composite = new PhraseSegmentItem(token.toString(), normalizedToken, true, false, token.substring); + } + else { + composite = new AndSegmentItem(token.toString(), true, false); + } int n = 0; + WordItem previous = null; for (String segment : segments) { WordItem w = new WordItem(segment, "", true, token.substring); w.setFromSegmented(true); w.setSegmentIndex(n++); w.setStemmed(false); + if (previous != null) + previous.setConnectivity(w, 1.0); + previous = w; composite.addItem(w); } composite.lock(); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java index 2d7e55a10fc..6d4401aca04 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/PhraseParser.java @@ -38,7 +38,7 @@ public class PhraseParser extends AbstractParser { } // Note, this depends on segment never creating AndItems when quoted // (the second argument) is true. - Item newWord = segment(token); + Item newWord = segment(null, token); if (firstWord == null) { // First pass firstWord = newWord; diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java index 8c77b3d2130..ee4c0d4d9f0 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java @@ -63,12 +63,12 @@ abstract class StructuredParser extends AbstractParser { item = number(); if (item == null) { - item = phrase(); + item = phrase(indexName); } if (item == null && indexName != null) { if (wordsAhead()) { - item = phrase(); + item = phrase(indexName); } } @@ -407,12 +407,12 @@ abstract class StructuredParser extends AbstractParser { } /** Words for phrases also permits numerals as words */ - private Item phraseWord(boolean insidePhrase) { + private Item phraseWord(String indexName, boolean insidePhrase) { int position = tokens.getPosition(); Item item = null; try { - item = word(); + item = word(indexName); if (item == null && tokens.currentIs(NUMBER)) { Token t = tokens.next(); @@ -437,7 +437,7 @@ abstract class StructuredParser extends AbstractParser { * a WordItem or PhraseSegmentItem if this is a CJK query, * null if the current item is not a word */ - private Item word() { + private Item word(String indexName) { int position = tokens.getPosition(); Item item = null; @@ -452,7 +452,7 @@ abstract class StructuredParser extends AbstractParser { if (submodes.url) { item = new WordItem(word, true); } else { - item = segment(word); + item = segment(indexName, word); } if (submodes.url || submodes.site) { @@ -499,15 +499,16 @@ abstract class StructuredParser extends AbstractParser { * An phrase or word, either marked by quotes or by non-spaces between * words or by a combination. * + * @param indexName the index name which preceeded this phrase, or null if none * @return a word if there's only one word, a phrase if there is * several quoted or non-space-separated words, or null otherwise */ - private Item phrase() { + private Item phrase(String indexName) { int position = tokens.getPosition(); Item item = null; try { - item = phraseBody(); + item = phraseBody(indexName); return item; } finally { if (item == null) { @@ -516,8 +517,8 @@ abstract class StructuredParser extends AbstractParser { } } - /** Returns a word, a phrase or another composite */ - private Item phraseBody() { + /** Returns a word, a phrase, or another composite */ + private Item phraseBody(String indexName) { boolean quoted = false; PhraseItem phrase = null; Item firstWord = null; @@ -538,7 +539,7 @@ abstract class StructuredParser extends AbstractParser { quoted = !quoted; } - Item word = phraseWord((firstWord != null) || (phrase != null)); + Item word = phraseWord(indexName, (firstWord != null) || (phrase != null)); if (word == null) { if (tokens.skipMultiple(QUOTE)) { diff --git a/container-search/src/main/java/com/yahoo/search/query/parser/ParserEnvironment.java b/container-search/src/main/java/com/yahoo/search/query/parser/ParserEnvironment.java index ca437fb9def..9e53f9d8ea9 100644 --- a/container-search/src/main/java/com/yahoo/search/query/parser/ParserEnvironment.java +++ b/container-search/src/main/java/com/yahoo/search/query/parser/ParserEnvironment.java @@ -50,19 +50,17 @@ public final class ParserEnvironment { public static ParserEnvironment fromExecutionContext(Execution.Context context) { ParserEnvironment env = new ParserEnvironment(); - if (context == null) { - return env; - } - if (context.getIndexFacts() != null) { + if (context == null) return env; + + if (context.getIndexFacts() != null) env.setIndexFacts(context.getIndexFacts()); - } - if (context.getLinguistics() != null) { + + if (context.getLinguistics() != null) env.setLinguistics(context.getLinguistics()); - } - SpecialTokenRegistry registry = context.getTokenRegistry(); - if (registry != null) { - env.setSpecialTokens(registry.getSpecialTokens("default")); - } + + if (context.getTokenRegistry() != null) + env.setSpecialTokens(context.getTokenRegistry().getSpecialTokens("default")); + return env; } diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java b/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java index e2c79fa5a7e..5cc34ff5b28 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java @@ -59,15 +59,10 @@ public class Execution extends com.yahoo.processing.execution.Execution { */ public static final class Context { - /** - * Whether the search should perform detailed diagnostics. - */ + /** Whether the search should perform detailed diagnostics. */ private boolean detailedDiagnostics = false; - /** - * Whether the container was considered to be in a breakdown state when - * this query started. - */ + /** Whether the container was considered to be in a breakdown state when this query started. */ private boolean breakdown = false; /** @@ -79,19 +74,13 @@ public class Execution extends com.yahoo.processing.execution.Execution { private IndexFacts indexFacts = null; - /** - * The current set of special tokens. - */ + /** The current set of special tokens */ private SpecialTokenRegistry tokenRegistry = null; - /** - * The current template registry. - */ + /** The current template registry */ private RendererRegistry rendererRegistry = null; - /** - * The current linguistics. - */ + /** The current linguistics */ private Linguistics linguistics = null; /** Always set if this context belongs to an execution, never set if it does not. */ diff --git a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/parseindexinfo.cfg b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/parseindexinfo.cfg index 0d264e04799..4a4a5c3a04d 100644 --- a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/parseindexinfo.cfg +++ b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/parseindexinfo.cfg @@ -1,6 +1,6 @@ indexinfo[3] indexinfo[0].name one -indexinfo[0].command[44] +indexinfo[0].command[45] indexinfo[0].command[0].indexname url.all indexinfo[0].command[0].command fullurl indexinfo[0].command[1].indexname host.all @@ -89,6 +89,8 @@ indexinfo[0].command[42].indexname exactindex indexinfo[0].command[42].command index indexinfo[0].command[43].indexname exactindex indexinfo[0].command[43].command exact +indexinfo[0].command[44].indexname phraseSegment +indexinfo[0].command[44].command "phrase-segmenting false" indexinfo[1].name twoRanges indexinfo[1].command[2] @@ -109,3 +111,4 @@ indexinfo[2].command[3].indexname link indexinfo[2].command[3].command index indexinfo[2].command[4].indexname url indexinfo[2].command[4].command index + diff --git a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java index 45f270b2e37..fd29f2c12fe 100644 --- a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.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.search.test; +import com.google.common.collect.ImmutableList; import com.yahoo.component.chain.Chain; import com.yahoo.language.Language; import com.yahoo.language.Linguistics; @@ -42,7 +43,9 @@ import org.junit.Test; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -877,6 +880,57 @@ public class QueryTestCase { } } + @Test + public void testImplicitPhraseIsDefault() { + Query query = new Query(httpEncode("?query=it's fine")); + assertEquals("AND 'it s' fine", query.getModel().getQueryTree().toString()); + } + + @Test + public void testImplicitPhrase() { + Query query = new Query(httpEncode("?query=myfield:it's myfield:fine")); + + SearchDefinition test = new SearchDefinition("test"); + Index myField = new Index("myfield"); + myField.addCommand("phrase-segmenting true"); + assertTrue(myField.getPhraseSegmenting()); + test.addIndex(myField); + IndexModel indexModel = new IndexModel(test); + query.getModel().setExecution(new Execution(Execution.Context.createContextStub(new IndexFacts(indexModel)))); + + assertEquals("AND myfield:'it s' myfield:fine", query.getModel().getQueryTree().toString()); + } + + @Test + public void testImplicitAnd() { + Query query = new Query(httpEncode("?query=myfield:it's myfield:fine")); + + SearchDefinition test = new SearchDefinition("test"); + Index myField = new Index("myfield"); + myField.addCommand("phrase-segmenting false"); + assertFalse(myField.getPhraseSegmenting()); + test.addIndex(myField); + IndexModel indexModel = new IndexModel(test); + query.getModel().setExecution(new Execution(Execution.Context.createContextStub(new IndexFacts(indexModel)))); + + assertEquals("AND (SAND myfield:it myfield:s) myfield:fine", query.getModel().getQueryTree().toString()); + } + + @Test + public void testImplicitAndInPhrase() { + Query query = new Query(httpEncode("?query=myfield:\"it's fine\"")); + + SearchDefinition test = new SearchDefinition("test"); + Index myField = new Index("myfield"); + myField.addCommand("phrase-segmenting false"); + assertFalse(myField.getPhraseSegmenting()); + test.addIndex(myField); + IndexModel indexModel = new IndexModel(test); + query.getModel().setExecution(new Execution(Execution.Context.createContextStub(new IndexFacts(indexModel)))); + + assertEquals("myfield:\"it s fine\"", query.getModel().getQueryTree().toString()); + } + private void assertDetectionText(String expectedDetectionText, String queryString, String ... indexSpecs) { Query q = new Query(httpEncode("/?query=" + queryString)); SearchDefinition sd = new SearchDefinition("testSearchDefinition"); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java index 75d4e2036ab..57c71720962 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java @@ -82,6 +82,10 @@ public class NodeRepositoryNode { private Integer cost; @JsonProperty("minCpuCores") private Double minCpuCores; + @JsonProperty("bandwidth") + private Double bandwidth; + @JsonProperty("fastDisk") + private Boolean fastDisk; @JsonProperty("description") private String description; @JsonProperty("history") @@ -347,6 +351,22 @@ public class NodeRepositoryNode { this.minCpuCores = minCpuCores; } + public Double getBandwidth() { + return bandwidth; + } + + public void setBandwidth(Double bandwidth) { + this.bandwidth = bandwidth; + } + + public Boolean getFastDisk() { + return fastDisk; + } + + public void setFastDisk(Boolean fastDisk) { + this.fastDisk = fastDisk; + } + public String getDescription() { return description; } @@ -436,6 +456,8 @@ public class NodeRepositoryNode { ", minMainMemoryAvailableGb=" + minMainMemoryAvailableGb + ", cost=" + cost + ", minCpuCores=" + minCpuCores + + ", bandwidth=" + bandwidth + + ", fastDisk=" + fastDisk + ", description='" + description + '\'' + ", history=" + Arrays.toString(history) + ", allowedToBeDown=" + allowedToBeDown + 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 64f9d042121..d7a0465a8df 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 @@ -9,8 +9,8 @@ import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.athenz.api.AthenzIdentity; @@ -49,13 +49,10 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; 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.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.AssignedRotation; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.DeploymentSpecValidator; import com.yahoo.vespa.hosted.controller.application.Endpoint; -import com.yahoo.vespa.hosted.controller.application.EndpointId; -import com.yahoo.vespa.hosted.controller.application.EndpointList; import com.yahoo.vespa.hosted.controller.application.JobList; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.JobStatus.JobRun; @@ -66,8 +63,6 @@ import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority; import com.yahoo.vespa.hosted.controller.maintenance.RoutingPolicies; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; -import com.yahoo.vespa.hosted.controller.rotation.Rotation; -import com.yahoo.vespa.hosted.controller.rotation.RotationId; import com.yahoo.vespa.hosted.controller.rotation.RotationLock; import com.yahoo.vespa.hosted.controller.rotation.RotationRepository; import com.yahoo.vespa.hosted.controller.security.AccessControl; @@ -83,12 +78,13 @@ import java.security.Principal; import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.EnumSet; import java.util.HashMap; -import java.util.LinkedHashSet; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -99,7 +95,6 @@ import java.util.function.Consumer; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; -import java.util.stream.Stream; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.active; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.reserved; @@ -128,7 +123,6 @@ public class ApplicationController { private final RoutingGenerator routingGenerator; private final RoutingPolicies routingPolicies; private final Clock clock; - private final BooleanFlag useMultipleEndpoints; private final DeploymentTrigger deploymentTrigger; private final BooleanFlag provisionApplicationCertificate; private final ApplicationCertificateProvider applicationCertificateProvider; @@ -146,7 +140,6 @@ public class ApplicationController { this.routingGenerator = routingGenerator; this.routingPolicies = new RoutingPolicies(controller); this.clock = clock; - this.useMultipleEndpoints = Flags.MULTIPLE_GLOBAL_ENDPOINTS.bindTo(controller.flagSource()); this.artifactRepository = artifactRepository; this.applicationStore = applicationStore; @@ -233,7 +226,7 @@ public class ApplicationController { } /** Find the global endpoint of given deployment, if any */ - public Optional<RoutingEndpoint> findGlobalEndpoint(DeploymentId deployment) { + private Optional<RoutingEndpoint> findGlobalEndpoint(DeploymentId deployment) { return routingGenerator.endpoints(deployment).stream() .filter(RoutingEndpoint::isGlobal) .findFirst(); @@ -298,8 +291,8 @@ public class ApplicationController { Version platformVersion; ApplicationVersion applicationVersion; ApplicationPackage applicationPackage; - Set<String> legacyRotations = new LinkedHashSet<>(); - Set<ContainerEndpoint> endpoints = new LinkedHashSet<>(); + Set<ContainerEndpoint> endpoints; + Set<String> legacyRotations; Optional<ApplicationCertificate> applicationCertificate; try (Lock lock = lock(applicationId)) { @@ -338,41 +331,22 @@ public class ApplicationController { // TODO(jvenstad): Remove this when all packages are validated upon submission, as in ApplicationApiHandler.submit(...). verifyApplicationIdentityConfiguration(applicationId.tenant(), applicationPackage, deployingIdentity); - // Assign global rotation - // TODO(ogronnesby): Remove feature flag and replace calls to withRotationLegacy with withRotation - // TODO(mpolden): Remove all handling of legacy endpoints once withRotationLegacy disappears - if (useMultipleEndpoints.with(FetchVector.Dimension.APPLICATION_ID, application.get().id().serializedForm()).value()) { - application = withRotation(application, zone); - - // Include global DNS names - Application app = application.get(); - app.rotations().stream() - .filter(assignedRotation -> assignedRotation.regions().contains(zone.region())) - .map(assignedRotation -> { - return new ContainerEndpoint( - assignedRotation.clusterId().value(), - Stream.concat( - app.endpointsIn(controller.system(), assignedRotation.endpointId()).legacy(false).asList().stream().map(Endpoint::dnsName), - Stream.of(assignedRotation.rotationId().asString()) - ).collect(Collectors.toList()) - ); - }) - .forEach(endpoints::add); + // Assign and register endpoints + application = withRotation(application, zone); + endpoints = registerEndpointsInDns(application.get(), zone); + legacyRotations = endpoints.stream() + .map(ContainerEndpoint::names) + .flatMap(Collection::stream) + .collect(Collectors.toSet()); + + if (controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) { + // Get application certificate (provisions a new certificate if missing) + List<? extends ZoneApi> zones = controller.zoneRegistry().zones().all().zones(); + applicationCertificate = getApplicationCertificate(application.get()); } else { - application = withRotationLegacy(application, zone); - - // Add both the names we have in DNS for each endpoint as well as name of the rotation so healthchecks works - Application app = application.get(); - app.endpointsIn(controller.system()).asList().stream().map(Endpoint::dnsName).forEach(legacyRotations::add); - app.rotations().stream() - .map(AssignedRotation::rotationId) - .map(RotationId::asString) - .forEach(legacyRotations::add); + applicationCertificate = Optional.empty(); } - // Get application certificate (provisions a new certificate if missing) - applicationCertificate = getApplicationCertificate(application.get()); - // Update application with information from application package if ( ! preferOldestVersion && ! application.get().deploymentJobs().deployedInternally() @@ -479,77 +453,70 @@ public class ApplicationController { } /** Makes sure the application has a global rotation, if eligible. */ - private LockedApplication withRotationLegacy(LockedApplication application, ZoneId zone) { - if (zone.environment() == Environment.prod && application.get().deploymentSpec().globalServiceId().isPresent()) { - try (RotationLock rotationLock = rotationRepository.lock()) { - Rotation rotation = rotationRepository.getOrAssignRotation(application.get(), rotationLock); - application = application.with(createDefaultGlobalIdRotation(application.get(), rotation)); - store(application); // store assigned rotation even if deployment fails - - EndpointList globalEndpoints = application.get() - .endpointsIn(controller.system()) - .scope(Endpoint.Scope.global); - - globalEndpoints.main().ifPresent(mainEndpoint -> { - registerCname(mainEndpoint.dnsName(), rotation.name()); - globalEndpoints.legacy(true).asList().forEach(endpoint -> registerCname(endpoint.dnsName(), rotation.name())); - }); - } - } - return application; - } - - private List<AssignedRotation> createDefaultGlobalIdRotation(Application application, Rotation rotation) { - Set<RegionName> regions = application.deploymentSpec().zones().stream() - .filter(zone -> zone.environment().isProduction()) - .flatMap(zone -> zone.region().stream()) - .collect(Collectors.toSet()); - var assignment = new AssignedRotation( - ClusterSpec.Id.from(application.deploymentSpec().globalServiceId().get()), - EndpointId.default_(), - rotation.id(), - regions - ); - return List.of(assignment); - } - - /** Makes sure the application has a global rotation, if eligible. */ private LockedApplication withRotation(LockedApplication application, ZoneId zone) { if (zone.environment() == Environment.prod) { try (RotationLock rotationLock = rotationRepository.lock()) { var rotations = rotationRepository.getOrAssignRotations(application.get(), rotationLock); application = application.with(rotations); store(application); // store assigned rotation even if deployment fails - registerAssignedRotationCnames(application.get()); } } return application; } - private void registerAssignedRotationCnames(Application application) { - application.rotations().forEach(assignedRotation -> { + /** + * Register endpoints for rotations assigned to given application and zone in DNS. + * + * @return the registered endpoints + */ + private Set<ContainerEndpoint> registerEndpointsInDns(Application application, ZoneId zone) { + var containerEndpoints = new HashSet<ContainerEndpoint>(); + var registerLegacyNames = application.deploymentSpec().globalServiceId().isPresent(); + for (var assignedRotation : application.rotations()) { + var names = new ArrayList<String>(); var endpoints = application.endpointsIn(controller.system(), assignedRotation.endpointId()) .scope(Endpoint.Scope.global); - var maybeRotation = rotationRepository.getRotation(assignedRotation.rotationId()); - maybeRotation.ifPresent(rotation -> { - // For rotations assigned using <endpoints/> syntax, we only register the non-legacy name in DNS. - endpoints.main().ifPresent(mainEndpoint -> registerCname(mainEndpoint.dnsName(), rotation.name())); - }); - }); + + // Skip rotations which do not apply to this zone. Legacy names always point to all zones + if (!registerLegacyNames && !assignedRotation.regions().contains(zone.region())) { + continue; + } + + // Omit legacy DNS names when assigning rotations using <endpoints/> syntax + if (!registerLegacyNames) { + endpoints = endpoints.legacy(false); + } + + // Register names in DNS + var rotation = rotationRepository.getRotation(assignedRotation.rotationId()); + if (rotation.isPresent()) { + endpoints.asList().forEach(endpoint -> { + controller.nameServiceForwarder().createCname(RecordName.from(endpoint.dnsName()), + RecordData.fqdn(rotation.get().name()), + Priority.normal); + names.add(endpoint.dnsName()); + }); + } + + // Include rotation ID as a valid name of this container endpoint (required by global routing health checks) + names.add(assignedRotation.rotationId().asString()); + containerEndpoints.add(new ContainerEndpoint(assignedRotation.clusterId().value(), names)); + } + return Collections.unmodifiableSet(containerEndpoints); } private Optional<ApplicationCertificate> getApplicationCertificate(Application application) { + boolean provisionCertificate = provisionApplicationCertificate.with(FetchVector.Dimension.APPLICATION_ID, + application.id().serializedForm()).value(); + if (!provisionCertificate) { + return Optional.empty(); + } + // Re-use certificate if already provisioned Optional<ApplicationCertificate> applicationCertificate = curator.readApplicationCertificate(application.id()); if(applicationCertificate.isPresent()) return applicationCertificate; - // TODO(tokle): Verify that the application is deploying to a zone where certificate provisioning is enabled - boolean provisionCertificate = provisionApplicationCertificate.with(FetchVector.Dimension.APPLICATION_ID, - application.id().serializedForm()).value(); - if (!provisionCertificate) { - return Optional.empty(); - } ApplicationCertificate newCertificate = applicationCertificateProvider.requestCaSignedCertificate(application.id()); curator.writeApplicationCertificate(application.id(), newCertificate); @@ -610,11 +577,6 @@ public class ApplicationController { options.deployCurrentVersion); } - /** Register a CNAME record in DNS */ - private void registerCname(String name, String targetName) { - controller.nameServiceForwarder().createCname(RecordName.from(name), RecordData.fqdn(targetName), Priority.normal); - } - /** Returns the endpoints of the deployment, or empty if the request fails */ public List<URI> getDeploymentEndpoints(DeploymentId deploymentId) { if ( ! get(deploymentId.applicationId()) @@ -676,7 +638,7 @@ public class ApplicationController { */ public void deleteApplication(ApplicationId applicationId, Optional<Credentials> credentials) { Tenant tenant = controller.tenants().require(applicationId.tenant()); - if (tenant.type() != Tenant.Type.user && ! credentials.isPresent()) + if (tenant.type() != Tenant.Type.user && credentials.isEmpty()) throw new IllegalArgumentException("Could not delete application '" + applicationId + "': No credentials provided"); // Find all instances of the application diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java index a6775e19dd6..ef1ce19b167 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java @@ -309,5 +309,7 @@ public class Endpoint { } return new Endpoint(name, application, zone, system, port, legacy, directRouting, wildcard); } + } + } 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 7db75d73ea4..3392576643f 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 @@ -60,7 +60,6 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.Endpoint; import com.yahoo.vespa.hosted.controller.application.JobStatus; -import com.yahoo.vespa.hosted.controller.rotation.RotationState; import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; @@ -70,6 +69,9 @@ import com.yahoo.vespa.hosted.controller.restapi.ErrorResponse; import com.yahoo.vespa.hosted.controller.restapi.MessageResponse; import com.yahoo.vespa.hosted.controller.restapi.ResourceResponse; import com.yahoo.vespa.hosted.controller.restapi.SlimeJsonResponse; +import com.yahoo.vespa.hosted.controller.rotation.RotationId; +import com.yahoo.vespa.hosted.controller.rotation.RotationState; +import com.yahoo.vespa.hosted.controller.rotation.RotationStatus; import com.yahoo.vespa.hosted.controller.security.AccessControlRequests; import com.yahoo.vespa.hosted.controller.security.Credentials; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; @@ -195,7 +197,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service/{service}/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/nodes")) return nodes(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/logs")) return logs(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request.propertyMap()); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/global-rotation")) return rotationStatus(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/global-rotation")) return rotationStatus(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), Optional.ofNullable(request.getProperty("endpointId"))); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/global-rotation/override")) return getGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deployment(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deployment(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); @@ -204,7 +206,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service/{service}/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/nodes")) return nodes(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/logs")) return logs(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request.propertyMap()); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/global-rotation")) return rotationStatus(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/global-rotation")) return rotationStatus(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), Optional.ofNullable(request.getProperty("endpointId"))); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/global-rotation/override")) return getGlobalRotationOverride(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); return ErrorResponse.notFoundError("Nothing at " + path); } @@ -550,9 +552,19 @@ public class ApplicationApiHandler extends LoggingRequestHandler { for (Deployment deployment : deployments) { Cursor deploymentObject = instancesArray.addObject(); - if (!application.rotations().isEmpty() && deployment.zone().environment() == Environment.prod) { - // TODO(mpolden): Support retrieving status for multiple rotations - toSlime(application.rotationStatus().of(application.rotations().get(0).rotationId(), deployment), deploymentObject); + // Rotation status for this deployment + if (deployment.zone().environment() == Environment.prod) { + // 0 rotations: No fields written + // 1 rotation : Write legacy field and endpointStatus field + // >1 rotation : Write only endpointStatus field + if (application.rotations().size() == 1) { + // TODO(mpolden): Stop writing this field once clients stop expecting it + toSlime(application.rotationStatus().of(application.rotations().get(0).rotationId(), deployment), + deploymentObject); + } + if (!application.rotations().isEmpty()) { + toSlime(application.rotations(), application.rotationStatus(), deployment, deploymentObject); + } } if (recurseOverDeployments(request)) // List full deployment information when recursive. @@ -688,7 +700,16 @@ public class ApplicationApiHandler extends LoggingRequestHandler { private void toSlime(RotationState state, Cursor object) { Cursor bcpStatus = object.setObject("bcpStatus"); - bcpStatus.setString("rotationStatus", state.name().toUpperCase()); + bcpStatus.setString("rotationStatus", rotationStateString(state)); + } + + private void toSlime(List<AssignedRotation> rotations, RotationStatus status, Deployment deployment, Cursor object) { + var array = object.setArray("endpointStatus"); + for (var rotation : rotations) { + var statusObject = array.addObject(); + statusObject.setString("endpointId", rotation.endpointId().id()); + statusObject.setString("status", rotationStateString(status.of(rotation.rotationId(), deployment))); + } } private URI monitoringSystemUri(DeploymentId deploymentId) { @@ -763,13 +784,11 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return new SlimeJsonResponse(slime); } - private HttpResponse rotationStatus(String tenantName, String applicationName, String instanceName, String environment, String region) { + private HttpResponse rotationStatus(String tenantName, String applicationName, String instanceName, String environment, String region, Optional<String> endpointId) { ApplicationId applicationId = ApplicationId.from(tenantName, applicationName, instanceName); Application application = controller.applications().require(applicationId); ZoneId zone = ZoneId.from(environment, region); - if (application.rotations().isEmpty()) { - throw new NotExistsException("global rotation does not exist for " + application); - } + RotationId rotation = findRotationId(application, endpointId); Deployment deployment = application.deployments().get(zone); if (deployment == null) { throw new NotExistsException(application + " has no deployment in " + zone); @@ -777,8 +796,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { Slime slime = new Slime(); Cursor response = slime.setObject(); - // TODO(mpolden): Support retrieving status for multiple rotations - toSlime(application.rotationStatus().of(application.rotations().get(0).rotationId(), deployment), response); + toSlime(application.rotationStatus().of(rotation, deployment), response); return new SlimeJsonResponse(slime); } @@ -1541,4 +1559,29 @@ public class ApplicationApiHandler extends LoggingRequestHandler { return dataParts; } + private static RotationId findRotationId(Application application, Optional<String> endpointId) { + if (application.rotations().isEmpty()) { + throw new NotExistsException("global rotation does not exist for " + application); + } + if (endpointId.isPresent()) { + return application.rotations().stream() + .filter(r -> r.endpointId().id().equals(endpointId.get())) + .map(AssignedRotation::rotationId) + .findFirst() + .orElseThrow(() -> new NotExistsException("endpoint " + endpointId.get() + + " does not exist for " + application)); + } else if (application.rotations().size() > 1) { + throw new IllegalArgumentException(application + " has multiple rotations. Query parameter 'endpointId' must be given"); + } + return application.rotations().get(0).rotationId(); + } + + private static String rotationStateString(RotationState state) { + switch (state) { + case in: return "IN"; + case out: return "OUT"; + } + return "UNKNOWN"; + } + } 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 f451543f390..069b992290c 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 @@ -138,8 +138,7 @@ class JobControllerApiHandlerHelper { Cursor devJobsObject = responseObject.setObject("devJobs"); for (JobType type : JobType.allIn(controller.system())) if ( type.environment() != null - && type.environment().isManuallyDeployed() - && application.deployments().containsKey(type.zone(controller.system()))) + && type.environment().isManuallyDeployed()) controller.jobController().last(application.id(), type) .ifPresent(last -> { Cursor devJobObject = devJobsObject.setObject(type.jobName()); 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 965cc1a9b16..c16cf1e1997 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 @@ -242,21 +242,29 @@ public class VersionStatus { boolean isReleased, Collection<HostName> configServerHostnames, Controller controller) { - boolean isSystemVersion = statistics.version().equals(systemVersion); - boolean isControllerVersion = statistics.version().equals(controllerVersion.version()); - VespaVersion.Confidence confidence = controller.curator().readConfidenceOverrides().get(statistics.version()); + var isSystemVersion = statistics.version().equals(systemVersion); + var isControllerVersion = statistics.version().equals(controllerVersion.version()); + var confidence = controller.curator().readConfidenceOverrides().get(statistics.version()); // Compute confidence if there's no override if (confidence == null) { if (isSystemVersion || isControllerVersion) { // Always compute confidence for system and controller confidence = VespaVersion.confidenceFrom(statistics, controller); - } else { // This is an older version so we keep the existing confidence, if any + } else { // This is an older version so we preserve the existing confidence, if any confidence = confidenceFor(statistics.version(), controller) .orElseGet(() -> VespaVersion.confidenceFrom(statistics, controller)); } } + // Preserve existing commit details if we've previously computed status for this version + var commitSha = controllerVersion.commitSha(); + var commitDate = controllerVersion.commitDate(); + var previousStatus = controller.versionStatus().version(statistics.version()); + if (previousStatus != null) { + commitSha = previousStatus.releaseCommit(); + commitDate = previousStatus.committedAt(); + } return new VespaVersion(statistics, - controllerVersion.commitSha(), - controllerVersion.commitDate(), + commitSha, + commitDate, isControllerVersion, isSystemVersion, isReleased, @@ -268,9 +276,9 @@ public class VersionStatus { /** Returns the current confidence for the given version */ private static Optional<VespaVersion.Confidence> confidenceFor(Version version, Controller controller) { return controller.versionStatus().versions().stream() - .filter(v -> version.equals(v.versionNumber())) - .map(VespaVersion::confidence) - .findFirst(); + .filter(v -> version.equals(v.versionNumber())) + .map(VespaVersion::confidence) + .findFirst(); } } 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 6f8a10543e7..210b94737b0 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 @@ -279,13 +279,11 @@ public class ControllerTest { @Test public void testDnsAliasRegistration() { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); - Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) - .globalServiceId("foo") + .endpoint("default", "foo") .region("us-west-1") .region("us-central-1") // Two deployments should result in each DNS alias being registered once .build(); @@ -352,8 +350,6 @@ public class ControllerTest { @Test public void testDnsAliasRegistrationWithEndpoints() { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); - Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() @@ -409,8 +405,6 @@ public class ControllerTest { @Test public void testDnsAliasRegistrationWithChangingZones() { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); - Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() @@ -457,8 +451,6 @@ public class ControllerTest { @Test public void testUnassignRotations() { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); - Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() @@ -490,16 +482,14 @@ public class ControllerTest { ); } - @Test + @Test public void testUpdatesExistingDnsAlias() { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); - // Application 1 is deployed and deleted { Application app1 = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) - .globalServiceId("foo") + .endpoint("default", "foo") .region("us-west-1") .region("us-central-1") // Two deployments should result in each DNS alias being registered once .build(); @@ -545,7 +535,7 @@ public class ControllerTest { Application app2 = tester.createApplication("app2", "tenant2", 2, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) - .globalServiceId("foo") + .endpoint("default", "foo") .region("us-west-1") .region("us-central-1") .build(); @@ -564,7 +554,7 @@ public class ControllerTest { Application app1 = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) - .globalServiceId("foo") + .endpoint("default", "foo") .region("us-west-1") .region("us-central-1") .build(); @@ -706,7 +696,7 @@ public class ControllerTest { } @Test - public void testDeployProvisionsCertificate() { + public void testDeploySelectivelyProvisionsCertificate() { ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.PROVISION_APPLICATION_CERTIFICATE.id(), true); Function<Application, Optional<ApplicationCertificate>> certificate = (application) -> tester.controller().curator().readApplicationCertificate(application.id()); @@ -732,7 +722,7 @@ public class ControllerTest { tester.controller().applications().deploy(app2.id(), zone, Optional.of(applicationPackage), DeployOptions.none()); assertTrue("Application deployed and activated", tester.controllerTester().configServer().application(app2.id()).get().activated()); - assertTrue("Provisions certificate in " + Environment.dev, certificate.apply(app2).isPresent()); + assertFalse("Does not provision certificate in " + Environment.dev, certificate.apply(app2).isPresent()); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index 11ed95c3f15..46422188a01 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -117,9 +117,13 @@ public class DeploymentTester { controller().updateVersionStatus(VersionStatus.compute(controller())); } - /** Upgrade controller to given version */ public void upgradeController(Version version) { - controller().curator().writeControllerVersion(controller().hostname(), new ControllerVersion(version, "badc0ffee", Instant.EPOCH)); + upgradeController(version, "badc0ffee", Instant.EPOCH); + } + + /** Upgrade controller to given version */ + public void upgradeController(Version version, String commitSha, Instant commitDate) { + controller().curator().writeControllerVersion(controller().hostname(), new ControllerVersion(version, commitSha, commitDate)); computeVersionStatus(); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java index 4ef77573f93..903b1378438 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java @@ -3,8 +3,6 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus; import com.yahoo.vespa.hosted.controller.application.Deployment; @@ -27,7 +25,6 @@ public class RotationStatusUpdaterTest { @Test public void updates_rotation_status() { var tester = new DeploymentTester(); - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); var globalRotationService = tester.controllerTester().globalRoutingService(); var updater = new RotationStatusUpdater(tester.controller(), Duration.ofDays(1), new JobControl(tester.controller().curator())); 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 4fdce088853..363522700d1 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 @@ -11,6 +11,7 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.slime.Cursor; @@ -21,7 +22,6 @@ import com.yahoo.vespa.athenz.api.AthenzUser; import com.yahoo.vespa.athenz.api.OktaAccessToken; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Application; -import com.yahoo.vespa.hosted.controller.ApplicationController; import com.yahoo.vespa.hosted.controller.LockedTenant; import com.yahoo.vespa.hosted.controller.api.application.v4.EnvironmentResource; import com.yahoo.vespa.hosted.controller.api.identifiers.Property; @@ -59,6 +59,8 @@ import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.BuildJob; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; +import com.yahoo.vespa.hosted.controller.maintenance.JobControl; +import com.yahoo.vespa.hosted.controller.maintenance.RotationStatusUpdater; import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; @@ -75,6 +77,7 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.net.URI; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.Base64; @@ -378,6 +381,8 @@ public class ApplicationApiTest extends ControllerContainerTest { controllerTester.upgrader().overrideConfidence(Version.fromString("6.1"), VespaVersion.Confidence.broken); tester.computeVersionStatus(); setDeploymentMaintainedInfo(controllerTester); + setZoneInRotation("rotation-fqdn-1", ZoneId.from("prod", "us-central-1")); + // GET tenant application deployments tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1", GET) .userIdentity(USER_ID), @@ -754,6 +759,63 @@ public class ApplicationApiTest extends ControllerContainerTest { } @Test + public void multiple_endpoints() { + // Setup + tester.computeVersionStatus(); + createAthenzDomainWithAdmin(ATHENZ_TENANT_DOMAIN, USER_ID); + ApplicationPackage applicationPackage = new ApplicationPackageBuilder() + .region("us-west-1") + .region("us-east-3") + .region("eu-west-1") + .endpoint("eu", "default", "eu-west-1") + .endpoint("default", "default", "us-west-1", "us-east-3") + .build(); + + // Create tenant and deploy + ApplicationId id = createTenantAndApplication(); + long projectId = 1; + MultiPartStreamer deployData = createApplicationDeployData(Optional.empty(), false); + startAndTestChange(controllerTester, id, projectId, applicationPackage, deployData, 100); + for (var job : List.of(JobType.productionUsWest1, JobType.productionUsEast3, JobType.productionEuWest1)) { + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/" + job.zone(SystemName.main).region().value() + "/deploy", POST) + .data(deployData) + .screwdriverIdentity(SCREWDRIVER_ID), + new File("deploy-result.json")); + controllerTester.jobCompletion(job) + .application(id) + .projectId(projectId) + .submit(); + } + setZoneInRotation("rotation-fqdn-2", ZoneId.from("prod", "us-west-1")); + setZoneInRotation("rotation-fqdn-2", ZoneId.from("prod", "us-east-3")); + setZoneInRotation("rotation-fqdn-1", ZoneId.from("prod", "eu-west-1")); + + // GET global rotation status without specifying endpointId fails + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-west-1/global-rotation", GET) + .userIdentity(USER_ID), + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"application 'tenant1.application1.instance1' has multiple rotations. Query parameter 'endpointId' must be given\"}", + 400); + + // GET global rotation status for us-west-1 in default endpoint + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-west-1/global-rotation?endpointId=default", GET) + .userIdentity(USER_ID), + "{\"bcpStatus\":{\"rotationStatus\":\"IN\"}}", + 200); + + // GET global rotation status for us-west-1 in eu endpoint + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/us-west-1/global-rotation?endpointId=eu", GET) + .userIdentity(USER_ID), + "{\"bcpStatus\":{\"rotationStatus\":\"UNKNOWN\"}}", + 200); + + // GET global rotation status for eu-west-1 in eu endpoint + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/environment/prod/region/eu-west-1/global-rotation?endpointId=eu", GET) + .userIdentity(USER_ID), + "{\"bcpStatus\":{\"rotationStatus\":\"IN\"}}", + 200); + } + + @Test public void testDeployDirectly() { // Setup tester.computeVersionStatus(); @@ -865,7 +927,7 @@ public class ApplicationApiTest extends ControllerContainerTest { } @Test - public void testMeteringResponses() { + public void testMeteringResponses() { MockMeteringClient mockMeteringClient = (MockMeteringClient) controllerTester.controller().meteringClient(); // Mock response for MeteringClient @@ -1671,12 +1733,7 @@ public class ApplicationApiTest extends ControllerContainerTest { private void setZoneInRotation(String rotationName, ZoneId zone) { globalRoutingService().setStatus(rotationName, zone, com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus.IN); - ApplicationController applicationController = controllerTester.controller().applications(); - List<Application> applicationList = applicationController.asList(); - applicationList.forEach(application -> { - applicationController.lockIfPresent(application.id(), locked -> - applicationController.store(locked.with(rotationStatus(application)))); - }); + new RotationStatusUpdater(tester.controller(), Duration.ofDays(1), new JobControl(tester.controller().curator())).run(); } private RotationStatus rotationStatus(Application application) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json index 383a1b667f7..1f79e960782 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json @@ -239,6 +239,12 @@ "bcpStatus": { "rotationStatus": "IN" }, + "endpointStatus": [ + { + "endpointId": "default", + "status": "IN" + } + ], "environment": "prod", "region": "us-west-1", "instance": "instance1", @@ -248,6 +254,12 @@ "bcpStatus": { "rotationStatus": "UNKNOWN" }, + "endpointStatus": [ + { + "endpointId": "default", + "status": "UNKNOWN" + } + ], "environment": "prod", "region": "us-east-3", "instance": "instance1", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json index 1d719133ac3..65ea213ebbc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json @@ -231,8 +231,14 @@ }, { "bcpStatus": { - "rotationStatus": "UNKNOWN" + "rotationStatus": "IN" }, + "endpointStatus": [ + { + "endpointId": "default", + "status": "IN" + } + ], "environment": "prod", "region": "us-central-1", "instance": "instance1", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs.json index 5b2e529fa6d..709a4dc2de0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/jobs.json @@ -129,6 +129,27 @@ "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/job/production-us-west-1" } }, - "devJobs": {} + "devJobs": { + "dev-us-east-1": { + "runs": [ + { + "id": 1, + "status": "aborted", + "start": (ignore), + "wantedPlatform": "6.1", + "wantedApplication": { + "hash": "unknown" + }, + "steps": { + "deployReal": "unfinished", + "installReal": "unfinished" + }, + "tasks": {}, + "log": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/job/dev-us-east-1/run/1" + } + ], + "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/job/dev-us-east-1" + } + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json index 4810c8f92b2..bb68904bee6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json @@ -1,7 +1,13 @@ { "bcpStatus": { - "rotationStatus": "UNKNOWN" + "rotationStatus": "IN" }, + "endpointStatus": [ + { + "endpointId": "default", + "status": "IN" + } + ], "tenant": "tenant1", "application": "application1", "instance": "instance1", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index a9fdaac6ee7..7a57ebb65dc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java @@ -331,6 +331,37 @@ public class VersionStatusTest { .keySet().contains(version0)); } + @Test + public void testCommitDetailsPreservation() { + var tester = new DeploymentTester(); + + // Commit details are set for initial version + var version0 = new Version("6.2"); + var commitSha0 = "badc0ffee"; + var commitDate0 = Instant.EPOCH; + tester.upgradeSystem(version0); + assertEquals(version0, tester.controller().versionStatus().systemVersion().get().versionNumber()); + assertEquals(commitSha0, tester.controller().versionStatus().systemVersion().get().releaseCommit()); + assertEquals(commitDate0, tester.controller().versionStatus().systemVersion().get().committedAt()); + + // Deploy app on version0 to keep computing statistics for that version + tester.createAndDeploy("app", 1, "canary"); + + // Commit details are updated for new version + var version1 = new Version("6.3"); + var commitSha1 = "deadbeef"; + var commitDate1 = Instant.ofEpochMilli(123); + tester.upgradeController(version1, commitSha1, commitDate1); + tester.upgradeSystemApplications(version1); + assertEquals(version1, tester.controller().versionStatus().systemVersion().get().versionNumber()); + assertEquals(commitSha1, tester.controller().versionStatus().systemVersion().get().releaseCommit()); + assertEquals(commitDate1, tester.controller().versionStatus().systemVersion().get().committedAt()); + + // Commit details for previous version are preserved + assertEquals(commitSha0, tester.controller().versionStatus().version(version0).releaseCommit()); + assertEquals(commitDate0, tester.controller().versionStatus().version(version0).committedAt()); + } + private static void writeControllerVersion(HostName hostname, Version version, CuratorDb db) { db.writeControllerVersion(hostname, new ControllerVersion(version, "badc0ffee", Instant.EPOCH)); } diff --git a/dist/vespa.spec b/dist/vespa.spec index 50216f6fdde..285c54287dc 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -37,6 +37,7 @@ BuildRequires: llvm5.0-devel BuildRequires: vespa-boost-devel >= 1.59.0-6 BuildRequires: vespa-gtest >= 1.8.1-1 BuildRequires: vespa-protobuf-devel >= 3.7.0-4 +BuildRequires: vespa-openssl-devel >= 1.1.1c-1 %endif %if 0%{?fedora} BuildRequires: cmake >= 3.9.1 @@ -108,6 +109,7 @@ Requires: gdb Requires: net-tools %if 0%{?centos} Requires: llvm5.0 +Requires: vespa-openssl >= 1.1.1c-1 Requires: vespa-protobuf >= 3.7.0-4 %define _vespa_llvm_version 5.0 %define _extra_link_directory /usr/lib64/llvm5.0/lib;%{_vespa_deps_prefix}/lib64 @@ -131,7 +133,6 @@ Requires: llvm-libs >= 8.0.0 %define _extra_include_directory %{_vespa_deps_prefix}/include %endif Requires: java-11-openjdk -Requires: openssl Requires(pre): shadow-utils # Ugly workaround because vespamalloc/src/vespamalloc/malloc/mmap.cpp uses the private diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index cd5045054e2..7dcdda38765 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -145,12 +145,6 @@ public class Flags { "Takes effect on deployment through controller", APPLICATION_ID); - public static final UnboundBooleanFlag MULTIPLE_GLOBAL_ENDPOINTS = defineFeatureFlag( - "multiple-global-endpoints", false, - "Allow applications to use new endpoints syntax in deployment.xml", - "Takes effect on deployment through controller", - APPLICATION_ID); - public static final UnboundBooleanFlag ENABLE_GROUPING_SESSION_CACHE = defineFeatureFlag( "enable-grouping-session-cache", true, "Enable grouping session cache", diff --git a/node-admin/scripts/etc-hosts.sh b/node-admin/scripts/etc-hosts.sh deleted file mode 100755 index 68d882da22a..00000000000 --- a/node-admin/scripts/etc-hosts.sh +++ /dev/null @@ -1,106 +0,0 @@ -#!/bin/bash -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -set -e - -declare -r NETWORK_PREFIX=172.18 -declare -r CONFIG_SERVER_HOSTNAME=config-server -declare -r CONFIG_SERVER_IP="$NETWORK_PREFIX.1.1" - -declare -r APP_HOSTNAME_PREFIX=cnode- -declare -r APP_NETWORK_PREFIX="$NETWORK_PREFIX.2" -declare -r NUM_APP_CONTAINERS=20 # Statically allocated number of nodes. - -declare -r SYSTEM_TEST_HOSTNAME_PREFIX=stest- -declare -r SYSTEM_TEST_NETWORK_PREFIX="$NETWORK_PREFIX.3" -declare -r NUM_SYSTEM_TEST_CONTAINERS=5 # Statically allocated number of nodes. - -declare -r HOSTS_FILE=/etc/hosts -declare -r HOSTS_LINE_SUFFIX=" # Managed by etc-hosts.sh" - -function IsInHostsAlready { - local ip="$1" - local hostname="$2" - local file="$3" - - # TODO: Escape $ip to make sure it's matched as a literal in the regex. - local matching_ip_line - matching_ip_line=$(grep -E "^$ip[ \\t]" "$file") - - local -i num_ip_lines=0 - # This 'if' is needed because wc -l <<< "" is 1. - if [ -n "$matching_ip_line" ] - then - num_ip_lines=$(wc -l <<< "$matching_ip_line") - fi - - local matching_hostname_line - matching_hostname_line=$(grep -E "^[^#]*[ \\t]$hostname(\$|[ \\t])" "$file") - - local -i num_hostname_lines=0 - # This 'if' is needed because wc -l <<< "" is 1. - if [ -n "$matching_hostname_line" ] - then - num_hostname_lines=$(wc -l <<< "$matching_hostname_line") - fi - - if ((num_ip_lines == 1)) && ((num_hostname_lines == 1)) && - [ "$matching_ip_line" == "$matching_hostname_line" ] - then - return 0 - elif ((num_ip_lines == 0)) && ((num_hostname_lines == 0)) - then - return 1 - else - printf "$file contains a conflicting host specification for $hostname/$ip" - exit 1 - fi -} - -function AddHost { - local ip="$1" - local hostname="$2" - local file="$3" - - if IsInHostsAlready "$ip" "$hostname" "$file" - then - return - fi - - echo -n "Adding host $hostname ($ip) to $file... " - printf "%-11s %s%s\n" "$ip" "$hostname" "$HOSTS_LINE_SUFFIX" >> "$file" - echo done -} - -function Stop { - # TODO: Remove entries. - : -} - -function StartAsRoot { - # May need sudo - if [ ! -w "$HOSTS_FILE" ] - then - Fail "$HOSTS_FILE is not writeable (run script with sudo)" - fi - - AddHost "$CONFIG_SERVER_IP" "$CONFIG_SERVER_HOSTNAME" "$HOSTS_FILE" - - local -i index=1 - for ((; index <= NUM_APP_CONTAINERS; ++index)) - do - local ip="$APP_NETWORK_PREFIX.$index" - local container_name="$APP_HOSTNAME_PREFIX$index" - AddHost "$ip" "$container_name" "$HOSTS_FILE" - done - - local -i index=1 - for ((; index <= NUM_SYSTEM_TEST_CONTAINERS; ++index)) - do - local ip="$SYSTEM_TEST_NETWORK_PREFIX.$index" - local container_name="$SYSTEM_TEST_HOSTNAME_PREFIX$index" - AddHost "$ip" "$container_name" "$HOSTS_FILE" - done -} - -StartAsRoot "$@" diff --git a/node-admin/scripts/node-repo.sh b/node-admin/scripts/node-repo.sh deleted file mode 100755 index 82f99f28e72..00000000000 --- a/node-admin/scripts/node-repo.sh +++ /dev/null @@ -1,360 +0,0 @@ -#!/bin/bash -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -set -e - -declare -r VESPA_WEB_SERVICE_PORT=4080 - -# Output from InnerCurlNodeRepo, see there for details. -declare CURL_RESPONSE - -function Usage { - cat <<EOF -Usage: ${0##*/} <command> [<args>...] -Script for manipulating the Node Repository. - -Commands - add [-c <configserverhost>] -p <parent-hostname> [-f <parent-flavor>] - [-n <flavor> <hostname>...] - With -f, provision "host" node <parent-hostname> with flavor - <parent-flavor>. With -n, provision "tenant" nodes <hostname...> with - flavor <flavor> and parent host <parenthostname>. - reprovision [-c <configserverhost>] -p <parenthostname> <hostname>... - Fail node <hostname>, then rm and add. - rm [-c <configserverhost>] <hostname>... - Remove nodes from node repo. - set-state [-c <configserverhost>] <state> <hostname>... - Set the node repo state. - -By default, <configserverhost> is config-server. - -Example - To remove docker-1--1 through docker-1--5 from the node repo at configserver.com, - - ${0##*/} rm -c configserver.com \ - docker-1--{1,2,3,4,5}.dockerhosts.com -EOF - - exit 1 -} - -function Fail { - printf "%s\nRun '${0##*/} help' for usage\n" "$*" - exit 1 -} - -# Invoke curl such that: -# -# - Arguments to this function are passed to curl. -# -# - Additional arguments are passed to curl to filter noise (--silent -# --show-error). -# -# - The curl stdout (i.e. the response) is stored in the global CURL_RESPONSE -# variable on return of the function. -# -# - If curl returns 0 (i.e. a successful HTTP response) with a JSON response -# that contains an "error-code" key, this function will instead return 22. 22 -# does not conflict with a curl return code, because curl only ever returns -# 22 when --fail is specified. -# -# Except, if the JSON response contains a "message" of the form "Cannot add -# tmp-cnode-0: A node with this name already exists", InnerCurlNodeRepo -# returns 0 instead of 22, even if the HTTP response status code is an error. -# -# Note: Why not use --fail with curl? Because as of 2015-11-24, if the node -# exists when provisioning a node, the node repo returns a 400 Bad Request -# with a JSON body containing a "message" field as described above. With -# --fail, this would result in curl exiting with code 22, which is -# indistinguishable from other HTTP errors. Can the output from --show-error -# be used in combination with --fail? No, because that ends up saying "curl: -# (22) The requested URL returned error: 400 Bad Request" when the node -# exists, making it indistinguishable from other malformed request error -# messages. -# -# TODO: Make node repo return a unique HTTP error code when node already -# exists. It's also fragile to test for the error message in the response. -function InnerCurlNodeRepo { - # --show-error causes error message to be printed on error, even with - # --silent, which is useful when we print the error message to Fail. - local -a command=(curl --silent --show-error "$@") - - # We need the 'if' here, because a non-zero exit code of a command will - # exit the process, with 'set -e'. - if CURL_RESPONSE=$("${command[@]}" 2>&1) - then - # Match a JSON of the form: - # { - # "error-code": "BAD_REQUEST", - # "message": "Cannot add cnode-0: A node with this name already exists" - # } - if [[ "$CURL_RESPONSE" =~ '"error-code"' ]] - then - if [[ "$CURL_RESPONSE" =~ '"message"'[^\"]*\"(.*)\" ]] - then - local message="${BASH_REMATCH[1]}" - if [[ "$message" =~ 'already exists' ]] - then - return 0 - fi - fi - - return 22 - fi - - return 0 - else - # Do not move this statement outside of this else: $? gets cleared when - # the execution passes out of the else-fi block. - return $? - fi -} - -function CurlOrFail { - if InnerCurlNodeRepo "$@" - then - : # This form of if-else is used to preserve $?. - else - local error_code=$? - - # Terminate the current progress-bar-like line - printf ' failed\n' - - Fail "Error ($error_code) from the node repo at '$url': '$CURL_RESPONSE'" - fi -} - -function ProvisionDockerNode { - local config_server_hostname="$1" - local container_hostname="$2" - local parent_hostname="$3" - local flavor="$4" - - local json="[ - { - \"hostname\":\"$container_hostname\", - \"parentHostname\":\"$parent_hostname\", - \"openStackId\":\"fake-$container_hostname\", - \"flavor\":\"$flavor\", - \"type\":\"tenant\" - } - ]" - - ProvisionNode $config_server_hostname "$json" -} - - -# Docker host, the docker nodes points to this host in parentHostname in their node config -function ProvisionDockerHost { - local config_server_hostname="$1" - local docker_host_hostname="$2" - local flavor="$3" - - local json="[ - { - \"hostname\":\"$docker_host_hostname\", - \"openStackId\":\"$docker_host_hostname\", - \"flavor\":\"$flavor\", - \"type\":\"host\" - } - ]" - - ProvisionNode $config_server_hostname "$json" -} - -# Docker node in node repo (both docker hosts and docker nodes) -function ProvisionNode { - local config_server_hostname="$1" - local json="$2" - - local url="http://$config_server_hostname:$VESPA_WEB_SERVICE_PORT/nodes/v2/node" - - CurlOrFail -H "Content-Type: application/json" -X POST -d "$json" "$url" -} - -function SetNodeState { - local config_server_hostname="$1" - local hostname="$2" - local state="$3" - - local url="http://$config_server_hostname:$VESPA_WEB_SERVICE_PORT/nodes/v2/state/$state/$hostname" - CurlOrFail -X PUT "$url" -} - -function AddCommand { - local config_server_hostname=config-server - local parent_hostname= - - OPTIND=1 - local option - while getopts "c:p:f:n:" option - do - case "$option" in - c) config_server_hostname="$OPTARG" ;; - p) parent_hostname="$OPTARG" ;; - f) parent_host_flavor="$OPTARG" ;; - n) node_flavor="$OPTARG" ;; - ?) exit 1 ;; # E.g. option lacks argument, in case error has been - # already been printed - *) Fail "Unknown option '$option' with value '$OPTARG'" - esac - done - - if [ -z "$parent_hostname" ] - then - Fail "Parent hostname not specified (-p)" - fi - - shift $((OPTIND - 1)) - - if [ -n "$parent_host_flavor" ] - then - echo "Provisioning Docker host $parent_hostname with flavor $parent_host_flavor" - ProvisionDockerHost "$config_server_hostname" \ - "$parent_hostname" \ - "$parent_host_flavor" - fi - - if [ -n "$node_flavor" ] - then - echo -n "Provisioning $# nodes with parent host $parent_hostname" - local container_hostname - for container_hostname in "$@" - do - ProvisionDockerNode "$config_server_hostname" \ - "$container_hostname" \ - "$parent_hostname" \ - "$node_flavor" - echo -n . - done - - echo " done" - fi -} - -function ReprovisionCommand { - local config_server_hostname=config-server - local parent_hostname= - - OPTIND=1 - local option - while getopts "c:p:" option - do - case "$option" in - c) config_server_hostname="$OPTARG" ;; - p) parent_hostname="$OPTARG" ;; - ?) exit 1 ;; # E.g. option lacks argument, in case error has been - # already been printed - *) Fail "Unknown option '$option' with value '$OPTARG'" - esac - done - - if [ -z "$parent_hostname" ] - then - Fail "Parent hostname not specified (-p)" - fi - - shift $((OPTIND - 1)) - - if (($# == 0)) - then - Fail "No node hostnames were specified" - fi - - # Simulate calls to the following commands. - SetStateCommand -c "$config_server_hostname" failed "$@" - RemoveCommand -c "$config_server_hostname" "$@" - AddCommand -c "$config_server_hostname" -p "$parent_hostname" "$@" -} - -function RemoveCommand { - local config_server_hostname=config-server - - OPTIND=1 - local option - while getopts "c:" option - do - case "$option" in - c) config_server_hostname="$OPTARG" ;; - ?) exit 1 ;; # E.g. option lacks argument, in case error has been - # already been printed - *) Fail "Unknown option '$option' with value '$OPTARG'" - esac - done - - shift $((OPTIND - 1)) - - if (($# == 0)) - then - Fail "No nodes were specified" - fi - - echo -n "Removing $# nodes" - - local hostname - for hostname in "$@" - do - local url="http://$config_server_hostname:$VESPA_WEB_SERVICE_PORT/nodes/v2/node/$hostname" - CurlOrFail -X DELETE "$url" - echo -n . - done - - echo " done" -} - -function SetStateCommand { - local config_server_hostname=config-server - - OPTIND=1 - local option - while getopts "c:" option - do - case "$option" in - c) config_server_hostname="$OPTARG" ;; - ?) exit 1 ;; # E.g. option lacks argument, in case error has been - # already been printed - *) Fail "Unknown option '$option' with value '$OPTARG'" - esac - done - - shift $((OPTIND - 1)) - - if (($# <= 1)) - then - Fail "Too few arguments" - fi - - local state="$1" - shift - - echo -n "Setting $# nodes to $state" - - local hostname - for hostname in "$@" - do - SetNodeState "$config_server_hostname" "$hostname" "$state" - echo -n . - done - - echo " done" -} - -function Main { - if (($# == 0)) - then - Usage - fi - local command="$1" - shift - - case "$command" in - add) AddCommand "$@" ;; - reprovision) ReprovisionCommand "$@" ;; - rm) RemoveCommand "$@" ;; - set-state) SetStateCommand "$@" ;; - help) Usage "$@" ;; - *) Usage ;; - esac -} - -Main "$@" diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java index b83bd9895fc..233fe8318ae 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java @@ -1,9 +1,10 @@ // 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.node.admin.configserver.noderepository; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.host.FlavorOverrides; -import java.util.Collections; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -15,29 +16,29 @@ public class AddNode { public final String hostname; public final Optional<String> parentHostname; - public final String nodeFlavor; + public final Optional<String> nodeFlavor; + public final Optional<FlavorOverrides> flavorOverrides; + public final Optional<NodeResources> nodeResources; public final NodeType nodeType; public final Set<String> ipAddresses; public final Set<String> additionalIpAddresses; - /** - * Constructor for a host node (has no parent) - */ - public AddNode(String hostname, String nodeFlavor, NodeType nodeType, Set<String> ipAddresses, Set<String> additionalIpAddresses) { - this(hostname, Optional.empty(), nodeFlavor, nodeType, ipAddresses, additionalIpAddresses); + public static AddNode forHost(String hostname, String nodeFlavor, Optional<FlavorOverrides> flavorOverrides, NodeType nodeType, Set<String> ipAddresses, Set<String> additionalIpAddresses) { + return new AddNode(hostname, Optional.empty(), Optional.of(nodeFlavor), flavorOverrides, Optional.empty(), nodeType, ipAddresses, additionalIpAddresses); } - /** - * Constructor for a child node (Must set parentHostname, no additionalIpAddresses) - */ - public AddNode(String hostname, String parentHostname, String nodeFlavor, NodeType nodeType, Set<String> ipAddresses) { - this(hostname, Optional.of(parentHostname), nodeFlavor, nodeType, ipAddresses, Collections.emptySet()); + public static AddNode forNode(String hostname, String parentHostname, NodeResources nodeResources, NodeType nodeType, Set<String> ipAddresses) { + return new AddNode(hostname, Optional.of(parentHostname), Optional.empty(), Optional.empty(), Optional.of(nodeResources), nodeType, ipAddresses, Set.of()); } - public AddNode(String hostname, Optional<String> parentHostname, String nodeFlavor, NodeType nodeType, Set<String> ipAddresses, Set<String> additionalIpAddresses) { + private AddNode(String hostname, Optional<String> parentHostname, + Optional<String> nodeFlavor, Optional<FlavorOverrides> flavorOverrides, Optional<NodeResources> nodeResources, + NodeType nodeType, Set<String> ipAddresses, Set<String> additionalIpAddresses) { this.hostname = hostname; this.parentHostname = parentHostname; this.nodeFlavor = nodeFlavor; + this.flavorOverrides = flavorOverrides; + this.nodeResources = nodeResources; this.nodeType = nodeType; this.ipAddresses = ipAddresses; this.additionalIpAddresses = additionalIpAddresses; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeOwner.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeOwner.java deleted file mode 100644 index c41e050d534..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeOwner.java +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; - -import com.yahoo.config.provision.ApplicationId; - -/** - * @author freva - */ -public class NodeOwner { - private final String tenant; - private final String application; - private final String instance; - - public NodeOwner(String tenant, String application, String instance) { - this.tenant = tenant; - this.application = application; - this.instance = instance; - } - - public String tenant() { - return tenant; - } - - public String application() { - return application; - } - - public String instance() { - return instance; - } - - public ApplicationId asApplicationId() { - return ApplicationId.from(tenant, application, instance); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - NodeOwner owner = (NodeOwner) o; - - if (!tenant.equals(owner.tenant)) return false; - if (!application.equals(owner.application)) return false; - return instance.equals(owner.instance); - - } - - @Override - public int hashCode() { - int result = tenant.hashCode(); - result = 31 * result + application.hashCode(); - result = 31 * result + instance.hashCode(); - return result; - } - - public String toString() { - return "Owner {" + - " tenant = " + tenant + - " application = " + application + - " instance = " + instance + - " }"; - } -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java index 6fb6d44bd6f..c2e68bdb329 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java @@ -3,7 +3,9 @@ package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; import com.fasterxml.jackson.databind.JsonNode; import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.DockerImage; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import java.time.Instant; @@ -11,6 +13,9 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import static com.yahoo.config.provision.NodeResources.DiskSpeed.fast; +import static com.yahoo.config.provision.NodeResources.DiskSpeed.slow; + /** * @author stiankri */ @@ -19,7 +24,6 @@ public class NodeSpec { private final NodeState state; private final NodeType type; private final String flavor; - private final String canonicalFlavor; private final Optional<DockerImage> wantedDockerImage; private final Optional<DockerImage> currentDockerImage; @@ -43,15 +47,10 @@ public class NodeSpec { private final Optional<Boolean> allowedToBeDown; private final Optional<Boolean> wantToDeprovision; - private final Optional<NodeOwner> owner; + private final Optional<ApplicationId> owner; private final Optional<NodeMembership> membership; - private final double vcpus; - private final double memoryGb; - private final double diskGb; - - private final boolean fastDisk; - private final double bandwidth; + private final NodeResources resources; private final Set<String> ipAddresses; private final Set<String> additionalIpAddresses; @@ -66,14 +65,13 @@ public class NodeSpec { NodeState state, NodeType type, String flavor, - String canonicalFlavor, Optional<Version> wantedVespaVersion, Optional<Version> currentVespaVersion, Optional<Version> wantedOsVersion, Optional<Version> currentOsVersion, Optional<Boolean> allowedToBeDown, Optional<Boolean> wantToDeprovision, - Optional<NodeOwner> owner, + Optional<ApplicationId> owner, Optional<NodeMembership> membership, Optional<Long> wantedRestartGeneration, Optional<Long> currentRestartGeneration, @@ -82,11 +80,7 @@ public class NodeSpec { Optional<Instant> wantedFirmwareCheck, Optional<Instant> currentFirmwareCheck, Optional<String> modelName, - double vcpus, - double memoryGb, - double diskGb, - boolean fastDisk, - double bandwidth, + NodeResources resources, Set<String> ipAddresses, Set<String> additionalIpAddresses, NodeReports reports, @@ -104,7 +98,6 @@ public class NodeSpec { this.state = Objects.requireNonNull(state); this.type = Objects.requireNonNull(type); this.flavor = Objects.requireNonNull(flavor); - this.canonicalFlavor = canonicalFlavor; this.modelName = modelName; this.wantedVespaVersion = Objects.requireNonNull(wantedVespaVersion); this.currentVespaVersion = Objects.requireNonNull(currentVespaVersion); @@ -120,11 +113,7 @@ public class NodeSpec { this.currentRebootGeneration = currentRebootGeneration; this.wantedFirmwareCheck = Objects.requireNonNull(wantedFirmwareCheck); this.currentFirmwareCheck = Objects.requireNonNull(currentFirmwareCheck); - this.vcpus = vcpus; - this.memoryGb = memoryGb; - this.diskGb = diskGb; - this.fastDisk = fastDisk; - this.bandwidth = bandwidth; + this.resources = Objects.requireNonNull(resources); this.ipAddresses = Objects.requireNonNull(ipAddresses); this.additionalIpAddresses = Objects.requireNonNull(additionalIpAddresses); this.reports = Objects.requireNonNull(reports); @@ -147,10 +136,6 @@ public class NodeSpec { return flavor; } - public String canonicalFlavor() { - return canonicalFlavor; - } - public Optional<DockerImage> wantedDockerImage() { return wantedDockerImage; } @@ -211,7 +196,7 @@ public class NodeSpec { return wantToDeprovision; } - public Optional<NodeOwner> owner() { + public Optional<ApplicationId> owner() { return owner; } @@ -219,24 +204,28 @@ public class NodeSpec { return membership; } + public NodeResources resources() { + return resources; + } + public double vcpus() { - return vcpus; + return resources.vcpu(); } public double memoryGb() { - return memoryGb; + return resources.memoryGb(); } public double diskGb() { - return diskGb; + return resources.diskGb(); } public boolean isFastDisk() { - return fastDisk; + return resources.diskSpeed() == fast; } - public double bandwidth() { - return bandwidth; + public double bandwidthGbps() { + return resources.bandwidthGbps(); } public Set<String> ipAddresses() { @@ -266,7 +255,6 @@ public class NodeSpec { Objects.equals(state, that.state) && Objects.equals(type, that.type) && Objects.equals(flavor, that.flavor) && - Objects.equals(canonicalFlavor, that.canonicalFlavor) && Objects.equals(wantedVespaVersion, that.wantedVespaVersion) && Objects.equals(currentVespaVersion, that.currentVespaVersion) && Objects.equals(wantedOsVersion, that.wantedOsVersion) && @@ -281,11 +269,7 @@ public class NodeSpec { Objects.equals(currentRebootGeneration, that.currentRebootGeneration) && Objects.equals(wantedFirmwareCheck, that.wantedFirmwareCheck) && Objects.equals(currentFirmwareCheck, that.currentFirmwareCheck) && - Objects.equals(vcpus, that.vcpus) && - Objects.equals(memoryGb, that.memoryGb) && - Objects.equals(diskGb, that.diskGb) && - Objects.equals(fastDisk, that.fastDisk) && - Objects.equals(bandwidth, that.bandwidth) && + Objects.equals(resources, that.resources) && Objects.equals(ipAddresses, that.ipAddresses) && Objects.equals(additionalIpAddresses, that.additionalIpAddresses) && Objects.equals(reports, that.reports) && @@ -301,7 +285,6 @@ public class NodeSpec { state, type, flavor, - canonicalFlavor, wantedVespaVersion, currentVespaVersion, wantedOsVersion, @@ -316,11 +299,7 @@ public class NodeSpec { currentRebootGeneration, wantedFirmwareCheck, currentFirmwareCheck, - vcpus, - memoryGb, - diskGb, - fastDisk, - bandwidth, + resources, ipAddresses, additionalIpAddresses, reports, @@ -336,7 +315,6 @@ public class NodeSpec { + " state=" + state + " type=" + type + " flavor=" + flavor - + " canonicalFlavor=" + canonicalFlavor + " wantedVespaVersion=" + wantedVespaVersion + " currentVespaVersion=" + currentVespaVersion + " wantedOsVersion=" + wantedOsVersion @@ -345,17 +323,13 @@ public class NodeSpec { + " wantToDeprovision=" + wantToDeprovision + " owner=" + owner + " membership=" + membership - + " vcpus=" + vcpus + " wantedRestartGeneration=" + wantedRestartGeneration + " currentRestartGeneration=" + currentRestartGeneration + " wantedRebootGeneration=" + wantedRebootGeneration + " currentRebootGeneration=" + currentRebootGeneration + " wantedFirmwareCheck=" + wantedFirmwareCheck + " currentFirmwareCheck=" + currentFirmwareCheck - + " memoryGb=" + memoryGb - + " diskGb=" + diskGb - + " fastDisk=" + fastDisk - + " bandwidth=" + bandwidth + + " resources=" + resources + " ipAddresses=" + ipAddresses + " additionalIpAddresses=" + additionalIpAddresses + " reports=" + reports @@ -368,7 +342,6 @@ public class NodeSpec { private NodeState state; private NodeType type; private String flavor; - private String canonicalFlavor; private Optional<DockerImage> wantedDockerImage = Optional.empty(); private Optional<DockerImage> currentDockerImage = Optional.empty(); private Optional<Version> wantedVespaVersion = Optional.empty(); @@ -377,7 +350,7 @@ public class NodeSpec { private Optional<Version> currentOsVersion = Optional.empty(); private Optional<Boolean> allowedToBeDown = Optional.empty(); private Optional<Boolean> wantToDeprovision = Optional.empty(); - private Optional<NodeOwner> owner = Optional.empty(); + private Optional<ApplicationId> owner = Optional.empty(); private Optional<NodeMembership> membership = Optional.empty(); private Optional<Long> wantedRestartGeneration = Optional.empty(); private Optional<Long> currentRestartGeneration = Optional.empty(); @@ -386,11 +359,7 @@ public class NodeSpec { private Optional<Instant> wantedFirmwareCheck = Optional.empty(); private Optional<Instant> currentFirmwareCheck = Optional.empty(); private Optional<String> modelName = Optional.empty(); - private double vcpus; - private double memoryGb; - private double diskGb; - private boolean fastDisk; - private double bandwidth; + private NodeResources resources = new NodeResources(0, 0, 0, 0, slow); private Set<String> ipAddresses = Set.of(); private Set<String> additionalIpAddresses = Set.of(); private NodeReports reports = new NodeReports(); @@ -403,12 +372,7 @@ public class NodeSpec { state(node.state); type(node.type); flavor(node.flavor); - canonicalFlavor(node.canonicalFlavor); - vcpus(node.vcpus); - memoryGb(node.memoryGb); - diskGb(node.diskGb); - fastDisk(node.fastDisk); - bandwidth(node.bandwidth); + resources(node.resources); ipAddresses(node.ipAddresses); additionalIpAddresses(node.additionalIpAddresses); wantedRebootGeneration(node.wantedRebootGeneration); @@ -462,11 +426,6 @@ public class NodeSpec { return this; } - public Builder canonicalFlavor(String canonicalFlavor) { - this.canonicalFlavor = canonicalFlavor; - return this; - } - public Builder wantedVespaVersion(Version wantedVespaVersion) { this.wantedVespaVersion = Optional.of(wantedVespaVersion); return this; @@ -497,7 +456,7 @@ public class NodeSpec { return this; } - public Builder owner(NodeOwner owner) { + public Builder owner(ApplicationId owner) { this.owner = Optional.of(owner); return this; } @@ -537,29 +496,29 @@ public class NodeSpec { return this; } - public Builder vcpus(double minCpuCores) { - this.vcpus = minCpuCores; + public Builder resources(NodeResources resources) { + this.resources = resources; return this; } - public Builder memoryGb(double minMainMemoryAvailableGb) { - this.memoryGb = minMainMemoryAvailableGb; - return this; + public Builder vcpus(double vcpus) { + return resources(resources.withVcpu(vcpus)); } - public Builder diskGb(double minDiskAvailableGb) { - this.diskGb = minDiskAvailableGb; - return this; + public Builder memoryGb(double memoryGb) { + return resources(resources.withMemoryGb(memoryGb)); + } + + public Builder diskGb(double diskGb) { + return resources(resources.withDiskGb(diskGb)); } public Builder fastDisk(boolean fastDisk) { - this.fastDisk = fastDisk; - return this; + return resources(resources.withDiskSpeed(fastDisk ? fast : slow)); } - public Builder bandwidth(double bandwidth) { - this.bandwidth = bandwidth; - return this; + public Builder bandwidthGbps(double bandwidthGbps) { + return resources(resources.withBandwidthGbps(bandwidthGbps)); } public Builder ipAddresses(Set<String> ipAddresses) { @@ -627,10 +586,6 @@ public class NodeSpec { return flavor; } - public String canonicalFlavor() { - return canonicalFlavor; - } - public Optional<Version> wantedVespaVersion() { return wantedVespaVersion; } @@ -655,7 +610,7 @@ public class NodeSpec { return wantToDeprovision; } - public Optional<NodeOwner> owner() { + public Optional<ApplicationId> owner() { return owner; } @@ -679,24 +634,8 @@ public class NodeSpec { return currentRebootGeneration; } - public double vcpus() { - return vcpus; - } - - public double memoryGb() { - return memoryGb; - } - - public double diskGb() { - return diskGb; - } - - public boolean isFastDisk() { - return fastDisk; - } - - public double bandwidth() { - return bandwidth; + public NodeResources resources() { + return resources; } public Set<String> ipAddresses() { @@ -716,15 +655,13 @@ public class NodeSpec { } public NodeSpec build() { - return new NodeSpec(hostname, wantedDockerImage, currentDockerImage, state, type, - flavor, canonicalFlavor, + return new NodeSpec(hostname, wantedDockerImage, currentDockerImage, state, type, flavor, wantedVespaVersion, currentVespaVersion, wantedOsVersion, currentOsVersion, allowedToBeDown, wantToDeprovision, owner, membership, wantedRestartGeneration, currentRestartGeneration, wantedRebootGeneration, currentRebootGeneration, wantedFirmwareCheck, currentFirmwareCheck, modelName, - vcpus, memoryGb, diskGb, - fastDisk, bandwidth, ipAddresses, additionalIpAddresses, + resources, ipAddresses, additionalIpAddresses, reports, parentHostname); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index fe19b81614d..582ba3dd09c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -4,8 +4,11 @@ package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; import com.fasterxml.jackson.databind.JsonNode; import com.google.common.base.Strings; import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.DockerImage; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.host.FlavorOverrides; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; import com.yahoo.vespa.hosted.node.admin.configserver.HttpException; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.GetAclResponse; @@ -25,6 +28,9 @@ import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.yahoo.config.provision.NodeResources.DiskSpeed.fast; +import static com.yahoo.config.provision.NodeResources.DiskSpeed.slow; + /** * @author stiankri, dybis */ @@ -154,14 +160,13 @@ public class RealNodeRepository implements NodeRepository { nodeState, nodeType, node.flavor, - node.canonicalFlavor, Optional.ofNullable(node.wantedVespaVersion).map(Version::fromString), Optional.ofNullable(node.vespaVersion).map(Version::fromString), Optional.ofNullable(node.wantedOsVersion).map(Version::fromString), Optional.ofNullable(node.currentOsVersion).map(Version::fromString), Optional.ofNullable(node.allowedToBeDown), Optional.ofNullable(node.wantToDeprovision), - Optional.ofNullable(node.owner).map(o -> new NodeOwner(o.tenant, o.application, o.instance)), + Optional.ofNullable(node.owner).map(o -> ApplicationId.from(o.tenant, o.application, o.instance)), membership, Optional.ofNullable(node.restartGeneration), Optional.ofNullable(node.currentRestartGeneration), @@ -170,11 +175,12 @@ public class RealNodeRepository implements NodeRepository { Optional.ofNullable(node.wantedFirmwareCheck).map(Instant::ofEpochMilli), Optional.ofNullable(node.currentFirmwareCheck).map(Instant::ofEpochMilli), Optional.ofNullable(node.modelName), - node.minCpuCores, - node.minMainMemoryAvailableGb, - node.minDiskAvailableGb, - node.fastDisk, - node.bandwidth, + new NodeResources( + node.minCpuCores, + node.minMainMemoryAvailableGb, + node.minDiskAvailableGb, + node.bandwidth / 1000, + node.fastDisk ? fast : slow), node.ipAddresses, node.additionalIpAddresses, reports, @@ -186,7 +192,15 @@ public class RealNodeRepository implements NodeRepository { node.openStackId = "fake-" + addNode.hostname; node.hostname = addNode.hostname; node.parentHostname = addNode.parentHostname.orElse(null); - node.flavor = addNode.nodeFlavor; + addNode.nodeFlavor.ifPresent(f -> node.flavor = f); + addNode.flavorOverrides.flatMap(FlavorOverrides::diskGb).ifPresent(d -> node.minDiskAvailableGb = d); + addNode.nodeResources.ifPresent(resources -> { + node.minCpuCores = resources.vcpu(); + node.minMainMemoryAvailableGb = resources.memoryGb(); + node.minDiskAvailableGb = resources.diskGb(); + node.bandwidth = resources.bandwidthGbps() * 1000; + node.fastDisk = resources.diskSpeed() == NodeResources.DiskSpeed.fast; + }); node.type = addNode.nodeType.name(); node.ipAddresses = addNode.ipAddresses; node.additionalIpAddresses = addNode.additionalIpAddresses; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java index 2b4f9277fc7..90d70a94144 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java @@ -28,8 +28,6 @@ public class NodeRepositoryNode { public String openStackId; @JsonProperty("flavor") public String flavor; - @JsonProperty("canonicalFlavor") - public String canonicalFlavor; @JsonProperty("membership") public Membership membership; @JsonProperty("owner") @@ -98,7 +96,6 @@ public class NodeRepositoryNode { ", openStackId='" + openStackId + '\'' + ", modelName='" + modelName + '\'' + ", flavor='" + flavor + '\'' + - ", canonicalFlavor='" + canonicalFlavor + '\'' + ", membership=" + membership + ", owner=" + owner + ", restartGeneration=" + restartGeneration + diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java index f4355ed3afa..91f4924cefa 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java @@ -141,9 +141,9 @@ public class StorageMaintainer { context.node().parentHostname().ifPresent(parent -> attributes.put("parent_hostname", parent)); context.node().currentVespaVersion().ifPresent(version -> attributes.put("vespa_version", version.toFullString())); context.node().owner().ifPresent(owner -> { - attributes.put("tenant", owner.tenant()); - attributes.put("application", owner.application()); - attributes.put("instance", owner.instance()); + attributes.put("tenant", owner.tenant().value()); + attributes.put("application", owner.application().value()); + attributes.put("instance", owner.instance().value()); }); return Collections.unmodifiableMap(attributes); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index b7e7b97cdd8..161775b0702 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -14,7 +14,6 @@ import com.yahoo.vespa.hosted.dockerapi.ContainerResources; import com.yahoo.vespa.hosted.dockerapi.exception.ContainerNotFoundException; import com.yahoo.vespa.hosted.dockerapi.exception.DockerException; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes; -import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeOwner; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeState; @@ -345,7 +344,6 @@ public class NodeAgentImpl implements NodeAgent { double cpuCap = noCpuCap(context.zone()) ? 0 : context.node().owner() - .map(NodeOwner::asApplicationId) .map(appId -> containerCpuCap.with(FetchVector.Dimension.APPLICATION_ID, appId.serializedForm())) .orElse(containerCpuCap) .with(FetchVector.Dimension.HOSTNAME, context.node().hostname()) diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java index 0938eb23b49..e30572fc63e 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java @@ -1,11 +1,12 @@ // 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.node.admin.configserver.noderepository; import com.yahoo.application.Networking; import com.yahoo.application.container.JDisc; import com.yahoo.config.provision.DockerImage; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.host.FlavorOverrides; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApiImpl; import com.yahoo.vespa.hosted.provision.testutils.ContainerConfig; @@ -17,11 +18,10 @@ import java.io.IOException; import java.net.ServerSocket; import java.net.URI; import java.time.Instant; -import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; @@ -162,23 +162,22 @@ public class RealNodeRepositoryTest { @Test public void testAddNodes() { - AddNode host = new AddNode("host123.domain.tld", "default", NodeType.confighost, - Collections.singleton("::1"), new HashSet<>(Arrays.asList("::2", "::3"))); - - AddNode node = new AddNode("host123-1.domain.tld", "host123.domain.tld", "docker", NodeType.config, - new HashSet<>(Arrays.asList("::2", "::3"))); + AddNode host = AddNode.forHost("host123.domain.tld", "default", Optional.of(FlavorOverrides.ofDisk(123)), NodeType.confighost, + Set.of("::1"), Set.of("::2", "::3")); - List<AddNode> nodesToAdd = Arrays.asList(host, node); + NodeResources nodeResources = new NodeResources(1, 2, 3, 4, NodeResources.DiskSpeed.slow); + AddNode node = AddNode.forNode("host123-1.domain.tld", "host123.domain.tld", nodeResources, NodeType.config, Set.of("::2", "::3")); assertFalse(nodeRepositoryApi.getOptionalNode("host123.domain.tld").isPresent()); - nodeRepositoryApi.addNodes(nodesToAdd); - - NodeSpec hostSpecInNodeRepo = nodeRepositoryApi.getOptionalNode("host123.domain.tld") - .orElseThrow(RuntimeException::new); + nodeRepositoryApi.addNodes(List.of(host, node)); - assertEquals(host.nodeFlavor, hostSpecInNodeRepo.flavor()); - assertEquals(host.nodeType, hostSpecInNodeRepo.type()); + NodeSpec hostSpec = nodeRepositoryApi.getOptionalNode("host123.domain.tld").orElseThrow(); + assertEquals("default", hostSpec.flavor()); + assertEquals(123, hostSpec.diskGb(), 0); + assertEquals(NodeType.confighost, hostSpec.type()); - assertTrue(nodeRepositoryApi.getOptionalNode("host123-1.domain.tld").isPresent()); + NodeSpec nodeSpec = nodeRepositoryApi.getOptionalNode("host123-1.domain.tld").orElseThrow(); + assertEquals(nodeResources, nodeSpec.resources()); + assertEquals(NodeType.config, nodeSpec.type()); } } diff --git a/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp b/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp index 68c388111d3..76b6bf55534 100644 --- a/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp +++ b/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp @@ -235,7 +235,7 @@ MySearchableContext::MySearchableContext(IThreadingService &writeService, IBucketDBHandlerInitializer & bucketDBHandlerInitializer) : _fastUpdCtx(writeService, bucketDB, bucketDBHandlerInitializer), _queryLimiter(), _clock(), - _ctx(_fastUpdCtx._ctx, _queryLimiter, _clock, writeService.shared()) + _ctx(_fastUpdCtx._ctx, _queryLimiter, _clock, dynamic_cast<vespalib::SyncableThreadExecutor &>(writeService.shared())) {} MySearchableContext::~MySearchableContext() = default; diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/summarymanagerinitializer.h b/searchcore/src/vespa/searchcore/proton/docsummary/summarymanagerinitializer.h index e7cb37aea28..6c2d3e3d4cc 100644 --- a/searchcore/src/vespa/searchcore/proton/docsummary/summarymanagerinitializer.h +++ b/searchcore/src/vespa/searchcore/proton/docsummary/summarymanagerinitializer.h @@ -43,7 +43,7 @@ public: search::transactionlog::SyncProxy &tlSyncer, IBucketizerSP bucketizer, std::shared_ptr<SummaryManager::SP> result); - ~SummaryManagerInitializer(); + ~SummaryManagerInitializer() override; void run() override; }; diff --git a/searchcore/src/vespa/searchcore/proton/index/index_manager_initializer.cpp b/searchcore/src/vespa/searchcore/proton/index/index_manager_initializer.cpp index 9c680ec7768..e87f74e2258 100644 --- a/searchcore/src/vespa/searchcore/proton/index/index_manager_initializer.cpp +++ b/searchcore/src/vespa/searchcore/proton/index/index_manager_initializer.cpp @@ -15,7 +15,7 @@ IndexManagerInitializer(const vespalib::string &baseDir, search::SerialNum serialNum, searchcorespi::IIndexManager::Reconfigurer & reconfigurer, searchcorespi::index::IThreadingService & threadingService, - vespalib::ThreadExecutor & warmupExecutor, + vespalib::SyncableThreadExecutor & warmupExecutor, const search::TuneFileIndexManager & tuneFileIndexManager, const search::TuneFileAttributes &tuneFileAttributes, const search::common::FileHeaderContext & fileHeaderContext, diff --git a/searchcore/src/vespa/searchcore/proton/index/index_manager_initializer.h b/searchcore/src/vespa/searchcore/proton/index/index_manager_initializer.h index 1a14c8d2ea3..8e446e222b3 100644 --- a/searchcore/src/vespa/searchcore/proton/index/index_manager_initializer.h +++ b/searchcore/src/vespa/searchcore/proton/index/index_manager_initializer.h @@ -20,7 +20,7 @@ class IndexManagerInitializer : public initializer::InitializerTask search::SerialNum _serialNum; searchcorespi::IIndexManager::Reconfigurer &_reconfigurer; searchcorespi::index::IThreadingService &_threadingService; - vespalib::ThreadExecutor &_warmupExecutor; + vespalib::SyncableThreadExecutor &_warmupExecutor; const search::TuneFileIndexManager _tuneFileIndexManager; const search::TuneFileAttributes _tuneFileAttributes; const search::common::FileHeaderContext &_fileHeaderContext; @@ -33,7 +33,7 @@ public: search::SerialNum serialNum, searchcorespi::IIndexManager::Reconfigurer & reconfigurer, searchcorespi::index::IThreadingService & threadingService, - vespalib::ThreadExecutor & warmupExecutor, + vespalib::SyncableThreadExecutor & warmupExecutor, const search::TuneFileIndexManager & tuneFileIndexManager, const search::TuneFileAttributes & tuneFileAttributes, const search::common::FileHeaderContext & fileHeaderContext, diff --git a/searchcore/src/vespa/searchcore/proton/index/indexmanager.cpp b/searchcore/src/vespa/searchcore/proton/index/indexmanager.cpp index 8e838414015..6ffd3203019 100644 --- a/searchcore/src/vespa/searchcore/proton/index/indexmanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/index/indexmanager.cpp @@ -78,7 +78,7 @@ IndexManager::IndexManager(const vespalib::string &baseDir, SerialNum serialNum, Reconfigurer &reconfigurer, IThreadingService &threadingService, - vespalib::ThreadExecutor & warmupExecutor, + vespalib::SyncableThreadExecutor & warmupExecutor, const search::TuneFileIndexManager &tuneFileIndexManager, const search::TuneFileAttributes &tuneFileAttributes, const FileHeaderContext &fileHeaderContext) : diff --git a/searchcore/src/vespa/searchcore/proton/index/indexmanager.h b/searchcore/src/vespa/searchcore/proton/index/indexmanager.h index b14912239a3..7e305681f78 100644 --- a/searchcore/src/vespa/searchcore/proton/index/indexmanager.h +++ b/searchcore/src/vespa/searchcore/proton/index/indexmanager.h @@ -72,7 +72,7 @@ public: SerialNum serialNum, Reconfigurer &reconfigurer, searchcorespi::index::IThreadingService &threadingService, - vespalib::ThreadExecutor & warmupExecutor, + vespalib::SyncableThreadExecutor & warmupExecutor, const search::TuneFileIndexManager &tuneFileIndexManager, const search::TuneFileAttributes &tuneFileAttributes, const search::common::FileHeaderContext &fileHeaderContext); diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp index 5aacfa2678c..45784fc2683 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp @@ -119,7 +119,7 @@ DocumentDB::DocumentDB(const vespalib::string &baseDir, document::BucketSpace bucketSpace, const ProtonConfig &protonCfg, IDocumentDBOwner &owner, - vespalib::ThreadExecutor &warmupExecutor, + vespalib::SyncableThreadExecutor &warmupExecutor, vespalib::ThreadStackExecutorBase &sharedExecutor, search::transactionlog::Writer &tlsDirectWriter, MetricsWireService &metricsWireService, diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.h b/searchcore/src/vespa/searchcore/proton/server/documentdb.h index b4e58d6f178..d6448c0b515 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.h @@ -247,7 +247,7 @@ public: document::BucketSpace bucketSpace, const ProtonConfig &protonCfg, IDocumentDBOwner &owner, - vespalib::ThreadExecutor &warmupExecutor, + vespalib::SyncableThreadExecutor &warmupExecutor, vespalib::ThreadStackExecutorBase &sharedExecutor, search::transactionlog::Writer &tlsDirectWriter, MetricsWireService &metricsWireService, diff --git a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp index 682719436f1..74010391fcb 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.cpp @@ -34,7 +34,7 @@ DocumentSubDBCollection::DocumentSubDBCollection( const IGetSerialNum &getSerialNum, const DocTypeName &docTypeName, searchcorespi::index::IThreadingService &writeService, - vespalib::ThreadExecutor &warmupExecutor, + vespalib::SyncableThreadExecutor &warmupExecutor, const search::common::FileHeaderContext &fileHeaderContext, MetricsWireService &metricsWireService, DocumentDBTaggedMetrics &metrics, diff --git a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h index 05427f5e545..d09afb92cc8 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h +++ b/searchcore/src/vespa/searchcore/proton/server/documentsubdbcollection.h @@ -11,7 +11,7 @@ namespace vespalib { class Clock; - class ThreadExecutor; + class SyncableThreadExecutor; class ThreadStackExecutorBase; } @@ -100,7 +100,7 @@ public: const IGetSerialNum &getSerialNum, const DocTypeName &docTypeName, searchcorespi::index::IThreadingService &writeService, - vespalib::ThreadExecutor &warmupExecutor, + vespalib::SyncableThreadExecutor &warmupExecutor, const search::common::FileHeaderContext &fileHeaderContext, MetricsWireService &metricsWireService, DocumentDBTaggedMetrics &metrics, diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp index e6afb8a19b2..a2672cc7972 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp @@ -32,7 +32,7 @@ public: } MaintenanceController::MaintenanceController(IThreadService &masterThread, - vespalib::ThreadExecutor & defaultExecutor, + vespalib::SyncableThreadExecutor & defaultExecutor, const DocTypeName &docTypeName) : IBucketFreezeListener(), _masterThread(masterThread), diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h index 69416ff63d7..24c1c18959e 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h +++ b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h @@ -11,7 +11,7 @@ namespace vespalib { class Timer; - class ThreadExecutor; + class SyncableThreadExecutor; class Executor; } namespace searchcorespi { namespace index { struct IThreadService; }} @@ -34,7 +34,7 @@ public: using JobList = std::vector<std::shared_ptr<MaintenanceJobRunner>>; using UP = std::unique_ptr<MaintenanceController>; - MaintenanceController(IThreadService &masterThread, vespalib::ThreadExecutor & defaultExecutor, const DocTypeName &docTypeName); + MaintenanceController(IThreadService &masterThread, vespalib::SyncableThreadExecutor & defaultExecutor, const DocTypeName &docTypeName); virtual ~MaintenanceController(); void registerJobInMasterThread(IMaintenanceJob::UP job); @@ -73,7 +73,7 @@ private: using Guard = std::lock_guard<Mutex>; IThreadService &_masterThread; - vespalib::ThreadExecutor &_defaultExecutor; + vespalib::SyncableThreadExecutor &_defaultExecutor; MaintenanceDocumentSubDB _readySubDB; MaintenanceDocumentSubDB _remSubDB; MaintenanceDocumentSubDB _notReadySubDB; diff --git a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h index e39ada31673..f3cf95ebfa0 100644 --- a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h @@ -52,15 +52,15 @@ public: }; struct Context { - const FastAccessDocSubDB::Context _fastUpdCtx; - matching::QueryLimiter &_queryLimiter; - const vespalib::Clock &_clock; - vespalib::ThreadExecutor &_warmupExecutor; + const FastAccessDocSubDB::Context _fastUpdCtx; + matching::QueryLimiter &_queryLimiter; + const vespalib::Clock &_clock; + vespalib::SyncableThreadExecutor &_warmupExecutor; Context(const FastAccessDocSubDB::Context &fastUpdCtx, matching::QueryLimiter &queryLimiter, const vespalib::Clock &clock, - vespalib::ThreadExecutor &warmupExecutor) + vespalib::SyncableThreadExecutor &warmupExecutor) : _fastUpdCtx(fastUpdCtx), _queryLimiter(queryLimiter), _clock(clock), @@ -80,7 +80,7 @@ private: vespalib::eval::ConstantValueCache _constantValueCache; matching::ConstantValueRepo _constantValueRepo; SearchableDocSubDBConfigurer _configurer; - vespalib::ThreadExecutor &_warmupExecutor; + vespalib::SyncableThreadExecutor &_warmupExecutor; std::shared_ptr<GidToLidChangeHandler> _realGidToLidChangeHandler; DocumentDBFlushConfig _flushConfig; bool _nodeRetired; diff --git a/searchcorespi/src/vespa/searchcorespi/index/i_thread_service.h b/searchcorespi/src/vespa/searchcorespi/index/i_thread_service.h index 40d92010c9b..65f18ac2c79 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/i_thread_service.h +++ b/searchcorespi/src/vespa/searchcorespi/index/i_thread_service.h @@ -9,7 +9,7 @@ namespace searchcorespi::index { /** * Interface for a single thread used for write tasks. */ -struct IThreadService : public vespalib::ThreadExecutor +struct IThreadService : public vespalib::SyncableThreadExecutor { IThreadService(const IThreadService &) = delete; IThreadService & operator = (const IThreadService &) = delete; diff --git a/searchcorespi/src/vespa/searchcorespi/index/indexmaintainercontext.cpp b/searchcorespi/src/vespa/searchcorespi/index/indexmaintainercontext.cpp index 213d2cb8705..78b9930c69a 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/indexmaintainercontext.cpp +++ b/searchcorespi/src/vespa/searchcorespi/index/indexmaintainercontext.cpp @@ -11,7 +11,7 @@ namespace searchcorespi::index { IndexMaintainerContext::IndexMaintainerContext(IThreadingService &threadingService, IIndexManager::Reconfigurer &reconfigurer, const FileHeaderContext &fileHeaderContext, - vespalib::ThreadExecutor & warmupExecutor) + vespalib::SyncableThreadExecutor & warmupExecutor) : _threadingService(threadingService), _reconfigurer(reconfigurer), _fileHeaderContext(fileHeaderContext), diff --git a/searchcorespi/src/vespa/searchcorespi/index/indexmaintainercontext.h b/searchcorespi/src/vespa/searchcorespi/index/indexmaintainercontext.h index 972a16effc3..ab4a447168e 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/indexmaintainercontext.h +++ b/searchcorespi/src/vespa/searchcorespi/index/indexmaintainercontext.h @@ -15,15 +15,15 @@ namespace searchcorespi::index { class IndexMaintainerContext { private: IThreadingService &_threadingService; - searchcorespi::IIndexManager::Reconfigurer &_reconfigurer; + IIndexManager::Reconfigurer &_reconfigurer; const search::common::FileHeaderContext &_fileHeaderContext; - vespalib::ThreadExecutor & _warmupExecutor; + vespalib::SyncableThreadExecutor & _warmupExecutor; public: IndexMaintainerContext(IThreadingService &threadingService, - searchcorespi::IIndexManager::Reconfigurer &reconfigurer, + IIndexManager::Reconfigurer &reconfigurer, const search::common::FileHeaderContext &fileHeaderContext, - vespalib::ThreadExecutor & warmupExecutor); + vespalib::SyncableThreadExecutor & warmupExecutor); /** * Returns the treading service that encapsulates the thread model used for writing. @@ -35,7 +35,7 @@ public: /** * Returns the reconfigurer used to signal when the index maintainer has changed. */ - searchcorespi::IIndexManager::Reconfigurer &getReconfigurer() const { + IIndexManager::Reconfigurer &getReconfigurer() const { return _reconfigurer; } @@ -49,7 +49,7 @@ public: /** * @return The executor that should be used for warmup. */ - vespalib::ThreadExecutor & getWarmupExecutor() const { return _warmupExecutor; } + vespalib::SyncableThreadExecutor & getWarmupExecutor() const { return _warmupExecutor; } }; } diff --git a/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp b/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp index 911ff5f9eba..1786665c248 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp +++ b/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.cpp @@ -34,7 +34,7 @@ WarmupIndexCollection::WarmupIndexCollection(const WarmupConfig & warmupConfig, ISearchableIndexCollection::SP prev, ISearchableIndexCollection::SP next, IndexSearchable & warmup, - vespalib::ThreadExecutor & executor, + vespalib::SyncableThreadExecutor & executor, IWarmupDone & warmupDone) : _warmupConfig(warmupConfig), _prev(prev), diff --git a/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.h b/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.h index 229d1a7c31d..992cedb1057 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.h +++ b/searchcorespi/src/vespa/searchcorespi/index/warmupindexcollection.h @@ -30,9 +30,9 @@ public: ISearchableIndexCollection::SP prev, ISearchableIndexCollection::SP next, IndexSearchable & warmup, - vespalib::ThreadExecutor & executor, + vespalib::SyncableThreadExecutor & executor, IWarmupDone & warmupDone); - ~WarmupIndexCollection(); + ~WarmupIndexCollection() override; // Implements IIndexCollection const ISourceSelector &getSourceSelector() const override; size_t getSourceCount() const override; @@ -95,15 +95,15 @@ private: void fireWarmup(Task::UP task); bool handledBefore(uint32_t fieldId, const Node &term); - const WarmupConfig _warmupConfig; - ISearchableIndexCollection::SP _prev; - ISearchableIndexCollection::SP _next; - IndexSearchable & _warmup; - vespalib::ThreadExecutor & _executor; - IWarmupDone & _warmupDone; - fastos::TimeStamp _warmupEndTime; - std::mutex _lock; - std::unique_ptr<FieldTermMap> _handledTerms; + const WarmupConfig _warmupConfig; + ISearchableIndexCollection::SP _prev; + ISearchableIndexCollection::SP _next; + IndexSearchable & _warmup; + vespalib::SyncableThreadExecutor & _executor; + IWarmupDone & _warmupDone; + fastos::TimeStamp _warmupEndTime; + std::mutex _lock; + std::unique_ptr<FieldTermMap> _handledTerms; }; } // namespace searchcorespi diff --git a/searchlib/src/vespa/searchlib/docstore/compacter.cpp b/searchlib/src/vespa/searchlib/docstore/compacter.cpp index 4d154da9907..b53281b4dbb 100644 --- a/searchlib/src/vespa/searchlib/docstore/compacter.cpp +++ b/searchlib/src/vespa/searchlib/docstore/compacter.cpp @@ -7,8 +7,7 @@ #include <vespa/log/log.h> LOG_SETUP(".searchlib.docstore.compacter"); -namespace search { -namespace docstore { +namespace search::docstore { using vespalib::alloc::Alloc; @@ -19,7 +18,7 @@ Compacter::write(LockGuard guard, uint32_t chunkId, uint32_t lid, const void *bu _ds.write(std::move(guard), fileId, lid, buffer, sz); } -BucketCompacter::BucketCompacter(size_t maxSignificantBucketBits, const CompressionConfig & compression, LogDataStore & ds, ThreadExecutor & executor, const IBucketizer & bucketizer, FileId source, FileId destination) : +BucketCompacter::BucketCompacter(size_t maxSignificantBucketBits, const CompressionConfig & compression, LogDataStore & ds, Executor & executor, const IBucketizer & bucketizer, FileId source, FileId destination) : _unSignificantBucketBits((maxSignificantBucketBits > 8) ? (maxSignificantBucketBits - 8) : 0), _sourceFileId(source), _destinationFileId(destination), @@ -97,4 +96,3 @@ BucketCompacter::write(BucketId bucketId, uint32_t chunkId, uint32_t lid, const } } -} diff --git a/searchlib/src/vespa/searchlib/docstore/compacter.h b/searchlib/src/vespa/searchlib/docstore/compacter.h index 362896487aa..666943ed629 100644 --- a/searchlib/src/vespa/searchlib/docstore/compacter.h +++ b/searchlib/src/vespa/searchlib/docstore/compacter.h @@ -6,11 +6,9 @@ #include "storebybucket.h" #include <vespa/vespalib/data/memorydatastore.h> -namespace search { +namespace search { class LogDataStore; } -class LogDataStore; - -namespace docstore { +namespace search::docstore { /** * A simple write through implementation of the IWriteData interface. @@ -34,11 +32,11 @@ private: class BucketCompacter : public IWriteData, public StoreByBucket::IWrite { using CompressionConfig = vespalib::compression::CompressionConfig; - using ThreadExecutor = vespalib::ThreadExecutor; + using Executor = vespalib::Executor; public: using FileId = FileChunk::FileId; BucketCompacter(size_t maxSignificantBucketBits, const CompressionConfig & compression, LogDataStore & ds, - ThreadExecutor & exeutor, const IBucketizer & bucketizer, FileId source, FileId destination); + Executor & exeutor, const IBucketizer & bucketizer, FileId source, FileId destination); void write(LockGuard guard, uint32_t chunkId, uint32_t lid, const void *buffer, size_t sz) override ; void write(BucketId bucketId, uint32_t chunkId, uint32_t lid, const void *buffer, size_t sz) override; void close() override; @@ -60,4 +58,3 @@ private: }; } -} diff --git a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp index 46fcdafc585..e96628bdf4f 100644 --- a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp +++ b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp @@ -103,7 +103,6 @@ LogDataStore::~LogDataStore() { // Must be called before ending threads as there are sanity checks. _fileChunks.clear(); - _executor.sync(); _genHandler.updateFirstUsedGeneration(); _lidInfo.removeOldGenerations(_genHandler.getFirstUsedGeneration()); } diff --git a/searchlib/src/vespa/searchlib/docstore/storebybucket.cpp b/searchlib/src/vespa/searchlib/docstore/storebybucket.cpp index d0d2d1cdac2..13de5687ee0 100644 --- a/searchlib/src/vespa/searchlib/docstore/storebybucket.cpp +++ b/searchlib/src/vespa/searchlib/docstore/storebybucket.cpp @@ -12,20 +12,21 @@ using document::BucketId; using vespalib::makeTask; using vespalib::makeClosure; -StoreByBucket::StoreByBucket(MemoryDataStore & backingMemory, ThreadExecutor & executor, const CompressionConfig & compression) : - _chunkSerial(0), - _current(), - _where(), - _backingMemory(backingMemory), - _executor(executor), - _lock(), - _chunks(), - _compression(compression) +StoreByBucket::StoreByBucket(MemoryDataStore & backingMemory, Executor & executor, const CompressionConfig & compression) + : _chunkSerial(0), + _current(), + _where(), + _backingMemory(backingMemory), + _executor(executor), + _monitor(), + _numChunksPosted(0), + _chunks(), + _compression(compression) { createChunk().swap(_current); } -StoreByBucket::~StoreByBucket() { } +StoreByBucket::~StoreByBucket() = default; void StoreByBucket::add(BucketId bucketId, uint32_t chunkId, uint32_t lid, const void *buffer, size_t sz) @@ -33,6 +34,7 @@ StoreByBucket::add(BucketId bucketId, uint32_t chunkId, uint32_t lid, const void if ( ! _current->hasRoom(sz)) { Chunk::UP tmpChunk = createChunk(); _current.swap(tmpChunk); + incChunksPosted(); _executor.execute(makeTask(makeClosure(this, &StoreByBucket::closeChunk, std::move(tmpChunk)))); } Index idx(bucketId, _current->getId(), chunkId, lid); @@ -48,7 +50,7 @@ StoreByBucket::createChunk() size_t StoreByBucket::getChunkCount() const { - vespalib::LockGuard guard(_lock); + vespalib::LockGuard guard(_monitor); return _chunks.size(); } @@ -59,15 +61,33 @@ StoreByBucket::closeChunk(Chunk::UP chunk) chunk->pack(1, buffer, _compression); buffer.shrink(buffer.getDataLen()); ConstBufferRef bufferRef(_backingMemory.push_back(buffer.getData(), buffer.getDataLen()).data(), buffer.getDataLen()); - vespalib::LockGuard guard(_lock); + vespalib::MonitorGuard guard(_monitor); _chunks[chunk->getId()] = bufferRef; + if (_numChunksPosted == _chunks.size()) { + guard.signal(); + } +} + +void +StoreByBucket::incChunksPosted() { + vespalib::MonitorGuard guard(_monitor); + _numChunksPosted++; +} + +void +StoreByBucket::waitAllProcessed() { + vespalib::MonitorGuard guard(_monitor); + while (_numChunksPosted != _chunks.size()) { + guard.wait(); + } } void StoreByBucket::drain(IWrite & drainer) { + incChunksPosted(); _executor.execute(makeTask(makeClosure(this, &StoreByBucket::closeChunk, std::move(_current)))); - _executor.sync(); + waitAllProcessed(); std::vector<Chunk::UP> chunks; chunks.resize(_chunks.size()); for (const auto & it : _chunks) { diff --git a/searchlib/src/vespa/searchlib/docstore/storebybucket.h b/searchlib/src/vespa/searchlib/docstore/storebybucket.h index ac1f6fbe007..8be0610b588 100644 --- a/searchlib/src/vespa/searchlib/docstore/storebybucket.h +++ b/searchlib/src/vespa/searchlib/docstore/storebybucket.h @@ -5,13 +5,12 @@ #include "chunk.h" #include <vespa/document/bucket/bucketid.h> #include <vespa/vespalib/data/memorydatastore.h> -#include <vespa/vespalib/util/threadexecutor.h> +#include <vespa/vespalib/util/executor.h> #include <vespa/vespalib/util/sync.h> #include <vespa/vespalib/stllike/hash_map.h> #include <map> -namespace search { -namespace docstore { +namespace search::docstore { /** * StoreByBucket will organize the data you add to it by buckets. @@ -21,12 +20,12 @@ namespace docstore { class StoreByBucket { using MemoryDataStore = vespalib::MemoryDataStore; - using ThreadExecutor = vespalib::ThreadExecutor; + using Executor = vespalib::Executor; using ConstBufferRef = vespalib::ConstBufferRef; using CompressionConfig = vespalib::compression::CompressionConfig; public: StoreByBucket(vespalib::MemoryDataStore & backingMemory, const CompressionConfig & compression); - StoreByBucket(MemoryDataStore & backingMemory, ThreadExecutor & executor, const CompressionConfig & compression); + StoreByBucket(MemoryDataStore & backingMemory, Executor & executor, const CompressionConfig & compression); StoreByBucket(StoreByBucket &&) = default; ~StoreByBucket(); class IWrite { @@ -47,6 +46,8 @@ public: return lidCount; } private: + void incChunksPosted(); + void waitAllProcessed(); Chunk::UP createChunk(); void closeChunk(Chunk::UP chunk); struct Index { @@ -67,11 +68,11 @@ private: Chunk::UP _current; std::map<uint64_t, IndexVector> _where; MemoryDataStore & _backingMemory; - ThreadExecutor & _executor; - vespalib::Lock _lock; + Executor & _executor; + vespalib::Monitor _monitor; + size_t _numChunksPosted; vespalib::hash_map<uint64_t, ConstBufferRef> _chunks; CompressionConfig _compression; }; } -} diff --git a/searchlib/src/vespa/searchlib/transactionlog/domain.h b/searchlib/src/vespa/searchlib/transactionlog/domain.h index c0ee484926c..d6f964d5140 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/domain.h +++ b/searchlib/src/vespa/searchlib/transactionlog/domain.h @@ -39,7 +39,7 @@ class Domain { public: using SP = std::shared_ptr<Domain>; - using Executor = vespalib::ThreadExecutor; + using Executor = vespalib::SyncableThreadExecutor; Domain(const vespalib::string &name, const vespalib::string &baseDir, Executor & commitExecutor, Executor & sessionExecutor, uint64_t domainPartSize, DomainPart::Crc defaultCrcType, const common::FileHeaderContext &fileHeaderContext); diff --git a/security-utils/src/main/java/com/yahoo/security/KeyFormat.java b/security-utils/src/main/java/com/yahoo/security/KeyFormat.java new file mode 100644 index 00000000000..a04e7951dfe --- /dev/null +++ b/security-utils/src/main/java/com/yahoo/security/KeyFormat.java @@ -0,0 +1,11 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.security; + +/** + * Key format + * + * @author bjorncs + */ +public enum KeyFormat { + PKCS1, PKCS8 +} diff --git a/security-utils/src/main/java/com/yahoo/security/KeyUtils.java b/security-utils/src/main/java/com/yahoo/security/KeyUtils.java index f847e78f3c5..ed3b41d6e2a 100644 --- a/security-utils/src/main/java/com/yahoo/security/KeyUtils.java +++ b/security-utils/src/main/java/com/yahoo/security/KeyUtils.java @@ -141,10 +141,36 @@ public class KeyUtils { } } + // Note: Encoding using PKCS#1 as default as this is to be read by tools only supporting PKCS#1 + // Should ideally be PKCS#8 public static String toPem(PrivateKey privateKey) { + return toPem(privateKey, KeyFormat.PKCS1); + } + + public static String toPem(PrivateKey privateKey, KeyFormat format) { + switch (format) { + case PKCS1: + return toPkcs1Pem(privateKey); + case PKCS8: + return toPkcs8Pem(privateKey); + default: + throw new IllegalArgumentException("Unknown format: " + format); + } + } + + public static String toPem(PublicKey publicKey) { + try (StringWriter stringWriter = new StringWriter(); JcaPEMWriter pemWriter = new JcaPEMWriter(stringWriter)) { + pemWriter.writeObject(publicKey); + pemWriter.flush(); + return stringWriter.toString(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private static String toPkcs1Pem(PrivateKey privateKey) { try (StringWriter stringWriter = new StringWriter(); JcaPEMWriter pemWriter = new JcaPEMWriter(stringWriter)) { String algorithm = privateKey.getAlgorithm(); - // Note: Encoding using PKCS#1 as this is to be read by tools only supporting PKCS#1 String type; if (algorithm.equals(RSA.getAlgorithmName())) { type = "RSA PRIVATE KEY"; @@ -161,9 +187,9 @@ public class KeyUtils { } } - public static String toPem(PublicKey publicKey) { + private static String toPkcs8Pem(PrivateKey privateKey) { try (StringWriter stringWriter = new StringWriter(); JcaPEMWriter pemWriter = new JcaPEMWriter(stringWriter)) { - pemWriter.writeObject(publicKey); + pemWriter.writeObject(new PemObject("PRIVATE KEY", privateKey.getEncoded())); pemWriter.flush(); return stringWriter.toString(); } catch (IOException e) { diff --git a/security-utils/src/test/java/com/yahoo/security/KeyUtilsTest.java b/security-utils/src/test/java/com/yahoo/security/KeyUtilsTest.java index dc0c0a126ea..8a1c72b1e59 100644 --- a/security-utils/src/test/java/com/yahoo/security/KeyUtilsTest.java +++ b/security-utils/src/test/java/com/yahoo/security/KeyUtilsTest.java @@ -32,25 +32,23 @@ public class KeyUtilsTest { } @Test - public void can_serialize_and_deserialize_rsa_privatekey_using_pem_format() { - KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA); - String pem = KeyUtils.toPem(keyPair.getPrivate()); - assertThat(pem, containsString("BEGIN RSA PRIVATE KEY")); - assertThat(pem, containsString("END RSA PRIVATE KEY")); - PrivateKey deserializedKey = KeyUtils.fromPemEncodedPrivateKey(pem); - assertEquals(keyPair.getPrivate(), deserializedKey); - assertEquals(KeyAlgorithm.RSA.getAlgorithmName(), deserializedKey.getAlgorithm()); + public void can_serialize_and_deserialize_rsa_privatekey_using_pkcs1_pem_format() { + testPrivateKeySerialization(KeyAlgorithm.RSA, KeyFormat.PKCS1, "RSA PRIVATE KEY"); } @Test - public void can_serialize_and_deserialize_ec_privatekey_using_pem_format() { - KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.EC); - String pem = KeyUtils.toPem(keyPair.getPrivate()); - assertThat(pem, containsString("BEGIN EC PRIVATE KEY")); - assertThat(pem, containsString("END EC PRIVATE KEY")); - PrivateKey deserializedKey = KeyUtils.fromPemEncodedPrivateKey(pem); - assertEquals(keyPair.getPrivate(), deserializedKey); - assertEquals(KeyAlgorithm.EC.getAlgorithmName(), deserializedKey.getAlgorithm()); + public void can_serialize_and_deserialize_rsa_privatekey_using_pkcs8_pem_format() { + testPrivateKeySerialization(KeyAlgorithm.RSA, KeyFormat.PKCS8, "PRIVATE KEY"); + } + + @Test + public void can_serialize_and_deserialize_ec_privatekey_using_pkcs1_pem_format() { + testPrivateKeySerialization(KeyAlgorithm.EC, KeyFormat.PKCS1, "EC PRIVATE KEY"); + } + + @Test + public void can_serialize_and_deserialize_ec_privatekey_using_pkcs8_pem_format() { + testPrivateKeySerialization(KeyAlgorithm.EC, KeyFormat.PKCS8, "PRIVATE KEY"); } @Test @@ -75,4 +73,14 @@ public class KeyUtilsTest { assertEquals(KeyAlgorithm.EC.getAlgorithmName(), deserializedKey.getAlgorithm()); } + private static void testPrivateKeySerialization(KeyAlgorithm keyAlgorithm, KeyFormat keyFormat, String pemLabel) { + KeyPair keyPair = KeyUtils.generateKeypair(keyAlgorithm); + String pem = KeyUtils.toPem(keyPair.getPrivate(), keyFormat); + assertThat(pem, containsString("BEGIN " + pemLabel)); + assertThat(pem, containsString("END " + pemLabel)); + PrivateKey deserializedKey = KeyUtils.fromPemEncodedPrivateKey(pem); + assertEquals(keyPair.getPrivate(), deserializedKey); + assertEquals(keyAlgorithm.getAlgorithmName(), deserializedKey.getAlgorithm()); + } + } diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java index 5ad28730a8b..dc2621b822a 100644 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java +++ b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java @@ -84,10 +84,7 @@ public class TestRunner { static ProcessBuilder mavenProcessFrom(TestProfile profile, TestRunnerConfig config) { List<String> command = new ArrayList<>(); - if (Path.of("/opt/vespa").equals(Path.of(Defaults.getDefaults().vespaHome()))) - command.add("/opt/vespa/local/maven/bin/mvn"); - else - command.add("mvn"); + command.add("mvn"); // mvn must be in PATH of the jDisc containers command.add("test"); command.add("--batch-mode"); // Run in non-interactive (batch) mode (disables output color) diff --git a/vespalib/src/vespa/vespalib/util/threadexecutor.h b/vespalib/src/vespa/vespalib/util/threadexecutor.h index 158805288e9..2dcbb595bb3 100644 --- a/vespalib/src/vespa/vespalib/util/threadexecutor.h +++ b/vespalib/src/vespa/vespalib/util/threadexecutor.h @@ -2,16 +2,12 @@ #pragma once -#include <vespa/vespalib/util/executor.h> -#include <vespa/vespalib/util/syncable.h> +#include "executor.h" +#include "syncable.h" namespace vespalib { -/** - * Can both execute and sync - **/ -class ThreadExecutor : public Executor, - public Syncable +class ThreadExecutor : public Executor { public: /** @@ -21,5 +17,13 @@ public: virtual size_t getNumThreads() const = 0; }; +/** + * Can both execute and sync + **/ +class SyncableThreadExecutor : public ThreadExecutor, public Syncable +{ +public: +}; + } // namespace vespalib diff --git a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp index dfd835e0e8e..969b5e6f61e 100644 --- a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp +++ b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.cpp @@ -135,7 +135,9 @@ ThreadStackExecutorBase::run() ThreadStackExecutorBase::ThreadStackExecutorBase(uint32_t stackSize, uint32_t taskLimit, init_fun_t init_fun) - : _pool(std::make_unique<FastOS_ThreadPool>(stackSize)), + : SyncableThreadExecutor(), + Runnable(), + _pool(std::make_unique<FastOS_ThreadPool>(stackSize)), _monitor(), _stats(), _executorCompletion(), diff --git a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h index 8ee08ed3929..21a6e9cabe0 100644 --- a/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h +++ b/vespalib/src/vespa/vespalib/util/threadstackexecutorbase.h @@ -32,7 +32,7 @@ namespace thread { struct ThreadInit; } /** * An executor service that executes tasks in multiple threads. **/ -class ThreadStackExecutorBase : public ThreadExecutor, +class ThreadStackExecutorBase : public SyncableThreadExecutor, public Runnable { public: @@ -238,7 +238,7 @@ public: /** * Will invoke shutdown then sync. **/ - ~ThreadStackExecutorBase(); + ~ThreadStackExecutorBase() override; }; } // namespace vespalib |