diff options
50 files changed, 348 insertions, 151 deletions
diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSigner.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSigner.java index ddf56476c24..f188fba5074 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSigner.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ca/CertificateSigner.java @@ -5,7 +5,6 @@ import com.google.inject.Inject; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.config.provision.Zone; import com.yahoo.log.LogLevel; -import com.yahoo.vespa.defaults.Defaults; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.KeyProvider; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.config.AthenzProviderServiceConfig; import org.bouncycastle.asn1.ASN1ObjectIdentifier; @@ -73,9 +72,7 @@ public class CertificateSigner { ConfigserverConfig configserverConfig, AthenzProviderServiceConfig config, Zone zone) { - this(getPrivateKey(keyProvider, config, zone), - !configserverConfig.loadBalancerAddress().isEmpty() ? configserverConfig.loadBalancerAddress() : Defaults.getDefaults().vespaHostname(), // TODO Remove once vips in main is ready - Clock.systemUTC()); + this(getPrivateKey(keyProvider, config, zone), configserverConfig.loadBalancerAddress(), Clock.systemUTC()); } CertificateSigner(PrivateKey caPrivateKey, String loadBalancerAddress, Clock clock) { diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java b/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java index 77d7d557520..53d1bb09758 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java @@ -24,7 +24,7 @@ public interface FileDistribution { * @param hostName host which should be notified about file references to download * @param fileReferences set of file references to start downloading */ - // TODO: Remove when 6.170 is the last version in use + // TODO: Remove when 6.197 is the last version in use void startDownload(String hostName, Set<FileReference> fileReferences); /** diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java index c16d12f76d1..c95601f6bbf 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java @@ -187,7 +187,7 @@ public class TensorFlowFeatureConverter extends ExpressionTransformer<RankProfil public boolean hasTensorFlowModels() { try { - return application.getFileReference(ApplicationPackage.MODELS_DIR).exists(); + return application.getFile(ApplicationPackage.MODELS_DIR).exists(); } catch (UnsupportedOperationException e) { return false; // No files -> no TensorFlow models diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolver.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolver.java index f2d32f85a16..9b6a86ad13e 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolver.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolver.java @@ -2,7 +2,6 @@ package com.yahoo.searchdefinition.processing; import com.yahoo.config.application.api.DeployLogger; -import com.yahoo.document.TensorDataType; import com.yahoo.searchdefinition.DocumentReference; import com.yahoo.searchdefinition.DocumentReferences; import com.yahoo.searchdefinition.RankProfileRegistry; @@ -66,8 +65,6 @@ public class ImportedFieldsResolver extends Processor { fail(importedField, targetFieldAsString(targetFieldName, reference) + ": Is not an attribute field. Only attribute fields supported"); } else if (targetField.doesIndexing()) { fail(importedField, targetFieldAsString(targetFieldName, reference) + ": Is an index field. Not supported"); - } else if (targetField.getDataType() instanceof TensorDataType) { - fail(importedField, targetFieldAsString(targetFieldName, reference) + ": Is of type 'tensor'. Not supported"); } return targetField; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/SetLanguage.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/SetLanguage.java index 5b872e9db5e..38c1d1a94d3 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/SetLanguage.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/SetLanguage.java @@ -15,7 +15,7 @@ import java.util.List; /** * Check that no text field appears before a field that sets language. * - * @author <a href="mailto:gunnarga@yahoo-inc.com">Gunnar Gauslaa Bergem</a> + * @author Gunnar Gauslaa Bergem */ public class SetLanguage extends Processor { @@ -32,13 +32,12 @@ public class SetLanguage extends Processor { textFieldsWithoutLanguage.add(field.getName()); } if (field.containsExpression(SetLanguageExpression.class) && !textFieldsWithoutLanguage.isEmpty()) { - StringBuffer fieldString = new StringBuffer(); + StringBuilder fieldString = new StringBuilder(); for (String fieldName : textFieldsWithoutLanguage) { fieldString.append(fieldName).append(" "); } warn(search, field, "Field '" + field.getName() + "' sets the language for this document, " + - "and should be defined as the first field in the searchdefinition. If you have both header and body fields, this field "+ - "should be header, if you require it to affect subsequent header fields and/or any body fields. " + + "and should be defined as the first field in the searchdefinition." + "Preceding text fields that will not have their language set: " + fieldString.toString() + " (This warning is omitted for any subsequent fields that also do set_language.)"); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/multifieldresolver/RankProfileTypeSettingsProcessor.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/multifieldresolver/RankProfileTypeSettingsProcessor.java index fda850087df..292f01a3308 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/multifieldresolver/RankProfileTypeSettingsProcessor.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/multifieldresolver/RankProfileTypeSettingsProcessor.java @@ -10,6 +10,8 @@ import com.yahoo.searchdefinition.RankProfile; import com.yahoo.searchdefinition.RankProfileRegistry; import com.yahoo.searchdefinition.Search; import com.yahoo.searchdefinition.document.Attribute; +import com.yahoo.searchdefinition.document.ImportedField; +import com.yahoo.searchdefinition.document.ImportedFields; import com.yahoo.searchdefinition.document.SDField; import com.yahoo.searchdefinition.processing.Processor; import com.yahoo.vespa.model.container.search.QueryProfiles; @@ -17,6 +19,7 @@ import com.yahoo.vespa.model.container.search.QueryProfiles; import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.Optional; /** * Class that processes a search instance and sets type settings on all rank profiles. @@ -36,6 +39,7 @@ public class RankProfileTypeSettingsProcessor extends Processor { @Override public void process() { processAttributeFields(); + processImportedFields(); processQueryProfileTypes(); } @@ -49,6 +53,21 @@ public class RankProfileTypeSettingsProcessor extends Processor { } } + private void processImportedFields() { + Optional<ImportedFields> importedFields = search.importedFields(); + if (importedFields.isPresent()) { + importedFields.get().fields().forEach((fieldName, field) -> processImportedField(field)); + } + } + + private void processImportedField(ImportedField field) { + SDField targetField = field.targetField(); + Attribute attribute = targetField.getAttributes().get(targetField.getName()); + if (attribute != null && attribute.tensorType().isPresent()) { + addAttributeTypeToRankProfiles(field.fieldName(), attribute.tensorType().get().toString()); + } + } + private void addAttributeTypeToRankProfiles(String attributeName, String attributeType) { for (RankProfile profile : rankProfileRegistry.allRankProfiles()) { profile.addAttributeType(attributeName, attributeType); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidator.java index 6ae9883c082..520ff231921 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankingConstantsValidator.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.application.api.ApplicationPackage; +import com.yahoo.config.model.application.provider.FilesApplicationPackage; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.path.Path; import com.yahoo.searchdefinition.RankingConstant; @@ -17,7 +18,6 @@ import java.io.FileNotFoundException; * * @author Vegard Sjonfjell */ - public class RankingConstantsValidator extends Validator { private static class ExceptionMessageCollector { @@ -62,9 +62,14 @@ public class RankingConstantsValidator extends Validator { } } - private void validateRankingConstant(RankingConstant rankingConstant, ApplicationPackage applicationPackage) throws FileNotFoundException { - ApplicationFile tensorApplicationFile = applicationPackage.getFile(Path.fromString(rankingConstant.getFileName())); - new ConstantTensorJsonValidator().validate(rankingConstant.getFileName(), + private void validateRankingConstant(RankingConstant rankingConstant, ApplicationPackage application) throws FileNotFoundException { + String constantFile = rankingConstant.getFileName(); + if (application.getFileReference(Path.fromString("")).getAbsolutePath().endsWith(FilesApplicationPackage.preprocessed) && + constantFile.startsWith(FilesApplicationPackage.preprocessed)) + constantFile = constantFile.substring(FilesApplicationPackage.preprocessed.length()); + + ApplicationFile tensorApplicationFile = application.getFile(Path.fromString(constantFile)); + new ConstantTensorJsonValidator().validate(constantFile, rankingConstant.getTensorType(), tensorApplicationFile.createReader()); } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java index 9f7c7458738..7fb05c03cbf 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java @@ -38,18 +38,23 @@ public class ImportedFieldsResolverTestCase { @Rule public final ExpectedException exceptionRule = ExpectedException.none(); - @Test - public void valid_imported_fields_are_resolved() { + private void resolve_imported_field(String fieldName, String targetFieldName) { SearchModel model = new SearchModel(); - model.addImportedField("my_attribute_field", "ref", "attribute_field").resolve(); + model.addImportedField(fieldName, "ref", targetFieldName).resolve(); assertEquals(1, model.importedFields.fields().size()); - ImportedField myField = model.importedFields.fields().get("my_attribute_field"); + ImportedField myField = model.importedFields.fields().get(fieldName); assertNotNull(myField); - assertEquals("my_attribute_field", myField.fieldName()); + assertEquals(fieldName, myField.fieldName()); assertSame(model.childSearch.getConcreteField("ref"), myField.reference().referenceField()); assertSame(model.parentSearch, myField.reference().targetSearch()); - assertSame(model.parentSearch.getConcreteField("attribute_field"), myField.targetField()); + assertSame(model.parentSearch.getConcreteField(targetFieldName), myField.targetField()); + } + + @Test + public void valid_imported_fields_are_resolved() { + resolve_imported_field("my_attribute_field", "attribute_field"); + resolve_imported_field("my_tensor_field", "tensor_field"); } @Test @@ -88,17 +93,6 @@ public class ImportedFieldsResolverTestCase { } @Test - public void resolver_fails_if_imported_field_is_tensor_type() { - exceptionRule.expect(IllegalArgumentException.class); - exceptionRule.expectMessage( - "For search 'child', import field 'my_tensor_field': " + - "Field 'tensor_field' via reference field 'ref': Is of type 'tensor'. Not supported"); - new SearchModel() - .addImportedField("my_tensor_field", "ref", "tensor_field") - .resolve(); - } - - @Test public void resolver_fails_if_imported_field_is_also_an_imported_field() { exceptionRule.expect(IllegalArgumentException.class); exceptionRule.expectMessage( diff --git a/configdefinitions/src/vespa/configserver.def b/configdefinitions/src/vespa/configserver.def index 9072a20c006..958db866f3e 100644 --- a/configdefinitions/src/vespa/configserver.def +++ b/configdefinitions/src/vespa/configserver.def @@ -27,7 +27,7 @@ payloadCompressionType enum { UNCOMPRESSED, LZ4 } default=LZ4 serverId string default="localhost" hostedVespa bool default=false numParallelTenantLoaders int default=4 -zookeeperLocalhostAffinity bool default=false +zookeeperLocalhostAffinity bool default=true # Zone information environment string default="prod" diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java index 6643e9b3370..2c46f591037 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java @@ -261,6 +261,8 @@ public class ZKApplicationPackage implements ApplicationPackage { @Override public File getFileReference(Path pathRelativeToAppDir) { String fileName = liveApp.getData(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + pathRelativeToAppDir.getRelative()); + if (fileName == null) + return new File(pathRelativeToAppDir.getRelative()); // File does not exist: Manufacture a non-existing file return new File(fileName); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java index 2c677441c68..0f20ca3163b 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java @@ -38,7 +38,6 @@ import java.util.Optional; * Base class for session handler tests * * @author hmusum - * @since 5.1.14 */ public class SessionHandlerTest { @@ -233,7 +232,6 @@ public class SessionHandlerTest { @Override public void activate(NestedTransaction transaction, ApplicationId application, Collection<HostSpec> hosts) { - transaction.commit(); activated = true; lastApplicationId = application; lastHosts = hosts; diff --git a/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java b/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java index c436ce09d33..6a7af378e68 100644 --- a/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/statistics/StatisticsSearcher.java @@ -9,6 +9,7 @@ import com.yahoo.jdisc.Metric; import com.yahoo.log.LogLevel; import com.yahoo.metrics.simple.MetricSettings; import com.yahoo.metrics.simple.MetricReceiver; +import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Result; import com.yahoo.search.Searcher; import com.yahoo.search.result.ErrorHit; @@ -43,6 +44,7 @@ import static com.yahoo.container.protect.Error.*; @Before(PhaseNames.RAW_QUERY) public class StatisticsSearcher extends Searcher { + private static final CompoundName IGNORE_QUERY = new CompoundName("metrics.ignore"); private static final String MAX_QUERY_LATENCY_METRIC = "max_query_latency"; private static final String EMPTY_RESULTS_METRIC = "empty_results"; private static final String HITS_PER_QUERY_METRIC = "hits_per_query"; @@ -185,6 +187,10 @@ public class StatisticsSearcher extends Searcher { * 3) ..... */ public Result search(com.yahoo.search.Query query, Execution execution) { + if(query.properties().getBoolean(IGNORE_QUERY,false)){ + return execution.search(query); + } + Metric.Context metricContext = getChainMetricContext(execution.chain().getId().stringValue()); incrQueryCount(metricContext); diff --git a/container-search/src/main/java/com/yahoo/search/result/Hit.java b/container-search/src/main/java/com/yahoo/search/result/Hit.java index 3cc7378de5c..6adbac56dbe 100644 --- a/container-search/src/main/java/com/yahoo/search/result/Hit.java +++ b/container-search/src/main/java/com/yahoo/search/result/Hit.java @@ -405,7 +405,7 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi /** * Will preallocate in order to avoid resizing. - * @param minSize + * @param minSize The minimum size to reserve */ public void reserve(int minSize) { getFieldMap(minSize); 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 c79b467568a..2c2dcfe549b 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 @@ -233,8 +233,11 @@ public class ApplicationController { * @throws IllegalArgumentException if the application already exists */ public Application createApplication(ApplicationId id, Optional<NToken> token) { - if ( ! (id.instance().value().equals("default") || id.instance().value().matches("^default-pr\\d+$"))) // TODO: Support instances properly - throw new UnsupportedOperationException("Only the instance names 'default' and names starting with 'default-pr' are supported at the moment"); + // TODO: TLS: PR names change to prXXX. + if ( ! ( id.instance().value().equals("default") + || id.instance().value().matches("^default-pr\\d+$") // TODO: Remove when these no longer deploy. + || id.instance().value().matches("^\\d+$"))) // TODO: Support instances properly + throw new UnsupportedOperationException("Only the instance names 'default' and names starting with 'default-pr' or which are just the PR number are supported at the moment"); try (Lock lock = lock(id)) { com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId.validate(id.application().value()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java index 07d51b2b9c7..283d6a75178 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java @@ -145,10 +145,10 @@ public class ApplicationList { /** * Returns the subset of applications which are not pull requests: - * Pull requests changes the application instance name to default-pr[pull-request-number] + * Pull requests changes the application instance name to (default-pr)?[pull-request-number] */ public ApplicationList notPullRequest() { - return listOf(list.stream().filter(a -> ! a.id().instance().value().startsWith("default-pr"))); + return listOf(list.stream().filter(a -> ! a.id().instance().value().matches("^(default-pr)?\\d+$"))); } /** Returns the subset of applications which have at least one production deployment */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java index 2b7260d5ffa..ad373cf8e29 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmer.java @@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.api.integration.organization.OwnershipIssues; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; +import com.yahoo.vespa.hosted.controller.application.ApplicationList; import java.time.Duration; import java.util.NoSuchElementException; @@ -40,21 +41,24 @@ public class ApplicationOwnershipConfirmer extends Maintainer { /** File an ownership issue with the owners of all applications we know about. */ private void confirmApplicationOwnerships() { - for (Application application : controller().applications().asList()) - if (application.id().instance().value().startsWith("default-pr") || application.productionDeployments().isEmpty()) - store(null, application.id()); - else - try { - Tenant tenant = ownerOf(application.id()); - Optional<IssueId> ourIssueId = application.ownershipIssueId(); - ourIssueId = tenant.tenantType() == TenantType.USER - ? ownershipIssues.confirmOwnership(ourIssueId, application.id(), userFor(tenant)) - : ownershipIssues.confirmOwnership(ourIssueId, application.id(), propertyIdFor(tenant)); - ourIssueId.ifPresent(issueId -> store(issueId, application.id())); - } - catch (RuntimeException e) { // Catch errors due to wrong data in the controller, or issues client timeout. - log.log(Level.WARNING, "Exception caught when attempting to file an issue for " + application.id(), e); - } + ApplicationList.from(controller().applications().asList()) + .notPullRequest() + .hasProductionDeployment() + .asList() + .forEach(application -> { + try { + Tenant tenant = ownerOf(application.id()); + Optional<IssueId> ourIssueId = application.ownershipIssueId(); + ourIssueId = tenant.tenantType() == TenantType.USER + ? ownershipIssues.confirmOwnership(ourIssueId, application.id(), userFor(tenant)) + : ownershipIssues.confirmOwnership(ourIssueId, application.id(), propertyIdFor(tenant)); + ourIssueId.ifPresent(issueId -> store(issueId, application.id())); + } + catch (RuntimeException e) { // Catch errors due to wrong data in the controller, or issues client timeout. + log.log(Level.WARNING, "Exception caught when attempting to file an issue for " + application.id(), e); + } + }); + } /** Escalate ownership issues which have not been closed before a defined amount of time has passed. */ diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java index 5039777ec7d..b0954044c22 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ApplicationOwnershipConfirmerTest.java @@ -78,8 +78,6 @@ public class ApplicationOwnershipConfirmerTest { confirmer.maintain(); confirmer.maintain(); - assertEquals("Confirmation issue has been forgotten for application without production deployments.", Optional.empty(), userApp.get().ownershipIssueId()); - // Time has passed, and a new confirmation issue is in order for the property which is still in production. Optional<IssueId> issueId2 = Optional.of(IssueId.from("2")); issues.response = issueId2; @@ -87,8 +85,7 @@ public class ApplicationOwnershipConfirmerTest { confirmer.maintain(); assertEquals("A new confirmation issue id is stored when something is returned to the maintainer.", issueId2, propertyApp.get().ownershipIssueId()); - assertEquals("Confirmation issue for application without production deployments has not been filed.", Optional.empty(), userApp.get().ownershipIssueId()); - + assertEquals("Confirmation issue for application without production deployments has not been filed.", issueId, userApp.get().ownershipIssueId()); } private class MockOwnershipIssues implements OwnershipIssues { 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 1c1e2df2c94..7745cae1cb9 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 @@ -139,10 +139,13 @@ public class VersionStatusTest { // Application without deployment Application ignored0 = tester.createApplication("ignored0", "tenant1", 1000, 1000L); - // Pull request build - Application ignored1 = tester.controllerTester().createApplication(new TenantId("tenant1"), + // Pull request builds + tester.controllerTester().createApplication(new TenantId("tenant1"), "ignored1", "default-pr42", 1000); + tester.controllerTester().createApplication(new TenantId("tenant1"), + "ignored1", + "43", 1000); assertEquals("All applications running on this version: High", Confidence.high, confidence(tester.controller(), version0)); diff --git a/dist/vespa.spec b/dist/vespa.spec index b7d84bc6d89..b0d8ffd19a1 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -48,11 +48,11 @@ BuildRequires: llvm-devel >= 4.0 BuildRequires: boost-devel >= 1.63 %endif %if 0%{?fc27} -BuildRequires: llvm-devel >= 4.0 +BuildRequires: llvm4.0-devel >= 4.0 BuildRequires: boost-devel >= 1.64 %endif %if 0%{?fc28} -BuildRequires: llvm-devel >= 4.0 +BuildRequires: llvm4.0-devel >= 4.0 BuildRequires: boost-devel >= 1.64 %endif BuildRequires: zookeeper-devel >= 3.4.9 @@ -118,18 +118,22 @@ Requires: boost >= 1.63 %define _vespa_llvm_version 4.0 %endif %if 0%{?fc27} -Requires: llvm-libs >= 4.0 +Requires: llvm4.0-libs >= 4.0 Requires: boost >= 1.64 %define _vespa_llvm_version 4.0 +%define _vespa_llvm_link_directory /usr/lib64/llvm4.0/lib +%define _vespa_llvm_include_directory /usr/include/llvm4.0 %endif %if 0%{?fc28} -Requires: llvm-libs >= 4.0 +Requires: llvm4.0-libs >= 4.0 Requires: boost >= 1.64 %define _vespa_llvm_version 4.0 +%define _vespa_llvm_link_directory /usr/lib64/llvm4.0/lib +%define _vespa_llvm_include_directory /usr/include/llvm4.0 %endif Requires: zookeeper >= 3.4.9 -%define _extra_link_directory /opt/vespa-libtorrent/lib;/opt/vespa-cppunit/lib -%define _extra_include_directory /opt/vespa-libtorrent/include;/opt/vespa-cppunit/include +%define _extra_link_directory /opt/vespa-libtorrent/lib;/opt/vespa-cppunit/lib%{?_vespa_llvm_link_directory:;%{_vespa_llvm_link_directory}} +%define _extra_include_directory /opt/vespa-libtorrent/include;/opt/vespa-cppunit/include%{?_vespa_llvm_include_directory:;%{_vespa_llvm_include_directory}} %define _vespa_boost_lib_suffix %{nil} %endif Requires: java-1.8.0-openjdk diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java index bc94c39d135..00493e3e016 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java @@ -45,6 +45,10 @@ public interface Docker { Map<String, Object> getBlkioStats(); } + default boolean networkNATed() { + return false; + } + Optional<ContainerStats> getContainerStats(ContainerName containerName); void startContainer(ContainerName containerName); @@ -113,5 +117,5 @@ public interface Docker { */ ProcessResult executeInContainerAsRoot(ContainerName containerName, Long timeoutSeconds, String... command); - + String getGlobalIPv6Address(ContainerName name); } diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java index fa093e0b4dc..15e88f4f253 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.dockerapi; import com.github.dockerjava.api.DockerClient; import com.github.dockerjava.api.command.ExecCreateCmdResponse; import com.github.dockerjava.api.command.ExecStartCmd; +import com.github.dockerjava.api.command.InspectContainerCmd; import com.github.dockerjava.api.command.InspectContainerResponse; import com.github.dockerjava.api.command.InspectExecResponse; import com.github.dockerjava.api.command.InspectImageResponse; @@ -125,15 +126,23 @@ public class DockerImpl implements Docker { Duration minAgeToDelete = Duration.ofMinutes(config.imageGCMinTimeToLiveMinutes()); dockerImageGC = Optional.of(new DockerImageGarbageCollector(minAgeToDelete)); - try { - setupDockerNetworkIfNeeded(); - } catch (Exception e) { - throw new DockerException("Could not setup docker network", e); + + if (!config.networkNATed()) { + try { + setupDockerNetworkIfNeeded(); + } catch (Exception e) { + throw new DockerException("Could not setup docker network", e); + } } } } } + @Override + public boolean networkNATed() { + return config.networkNATed(); + } + static DefaultDockerClientConfig.Builder buildDockerClientConfig(DockerConfig config) { DefaultDockerClientConfig.Builder dockerConfigBuilder = new DefaultDockerClientConfig.Builder() .withDockerHost(config.uri()); @@ -393,6 +402,12 @@ public class DockerImpl implements Docker { return asContainer(containerName.asString()).findFirst(); } + @Override + public String getGlobalIPv6Address(ContainerName name) { + InspectContainerCmd cmd = dockerClient.inspectContainerCmd(name.asString()); + return cmd.exec().getNetworkSettings().getGlobalIPv6Address(); + } + private Stream<Container> asContainer(String container) { return inspectContainerCmd(container) .map(response -> diff --git a/docker-api/src/main/resources/configdefinitions/docker.def b/docker-api/src/main/resources/configdefinitions/docker.def index 5c6e52b2f63..b4585318cd8 100644 --- a/docker-api/src/main/resources/configdefinitions/docker.def +++ b/docker-api/src/main/resources/configdefinitions/docker.def @@ -14,3 +14,5 @@ readTimeoutMillis int default = 1800000 # 30 min isRunningLocally bool default = false imageGCMinTimeToLiveMinutes int default = 45 + +networkNATed bool default = false diff --git a/eval/src/tests/eval/value_cache/dense.tbf b/eval/src/tests/eval/value_cache/dense.tbf Binary files differnew file mode 100644 index 00000000000..61012a35ff9 --- /dev/null +++ b/eval/src/tests/eval/value_cache/dense.tbf diff --git a/eval/src/tests/eval/value_cache/tensor_loader_test.cpp b/eval/src/tests/eval/value_cache/tensor_loader_test.cpp index 5dd8caa6e27..8180a7daef8 100644 --- a/eval/src/tests/eval/value_cache/tensor_loader_test.cpp +++ b/eval/src/tests/eval/value_cache/tensor_loader_test.cpp @@ -67,6 +67,10 @@ TEST_F("require that lz4 compressed dense tensor can be loaded", ConstantTensorL TEST_DO(verify_tensor(make_dense_tensor(), f1.create(TEST_PATH("dense.json.lz4"), "tensor(x[2],y[2])"))); } +TEST_F("require that a binary tensor can be loaded", ConstantTensorLoader(SimpleTensorEngine::ref())) { + TEST_DO(verify_tensor(make_dense_tensor(), f1.create(TEST_PATH("dense.tbf"), "tensor(x[2],y[2])"))); +} + TEST_F("require that lz4 compressed sparse tensor can be loaded", ConstantTensorLoader(SimpleTensorEngine::ref())) { TEST_DO(verify_tensor(make_sparse_tensor(), f1.create(TEST_PATH("sparse.json.lz4"), "tensor(x{},y{})"))); } diff --git a/eval/src/vespa/eval/eval/value_cache/constant_tensor_loader.cpp b/eval/src/vespa/eval/eval/value_cache/constant_tensor_loader.cpp index 38d5bbc643b..afc8471bdb4 100644 --- a/eval/src/vespa/eval/eval/value_cache/constant_tensor_loader.cpp +++ b/eval/src/vespa/eval/eval/value_cache/constant_tensor_loader.cpp @@ -4,6 +4,7 @@ #include <vespa/eval/eval/tensor.h> #include <vespa/eval/eval/tensor_engine.h> #include <vespa/eval/eval/tensor_spec.h> +#include <vespa/vespalib/objects/nbostream.h> #include <vespa/vespalib/io/mapped_file_input.h> #include <vespa/vespalib/data/lz4_input_decoder.h> #include <vespa/vespalib/data/slime/slime.h> @@ -54,14 +55,14 @@ void decode_json(const vespalib::string &path, Slime &slime) { } else { if (ends_with(path, ".lz4")) { size_t buffer_size = 64 * 1024; - Lz4InputDecoder lz4_decoder(file, buffer_size); + Lz4InputDecoder lz4_decoder(file, buffer_size); decode_json(path, lz4_decoder, slime); if (lz4_decoder.failed()) { LOG(warning, "file contains lz4 errors (%s): %s", lz4_decoder.reason().c_str(), path.c_str()); } } else { - decode_json(path, file, slime); + decode_json(path, file, slime); } } } @@ -76,6 +77,12 @@ ConstantTensorLoader::create(const vespalib::string &path, const vespalib::strin LOG(warning, "invalid type specification: %s", type.c_str()); return std::make_unique<SimpleConstantValue>(_engine.from_spec(TensorSpec("double"))); } + if (ends_with(path, ".tbf")) { + vespalib::MappedFileInput file(path); + vespalib::Memory content = file.get(); + vespalib::nbostream stream(content.data, content.size); + return std::make_unique<SimpleConstantValue>(_engine.decode(stream)); + } Slime slime; decode_json(path, slime); std::set<vespalib::string> indexed; diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SetLanguageExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SetLanguageExpression.java index 0c0a83cdf1b..2f02c9fd19f 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SetLanguageExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/SetLanguageExpression.java @@ -2,11 +2,12 @@ package com.yahoo.vespa.indexinglanguage.expressions; import com.yahoo.document.DataType; -import com.yahoo.document.DocumentType; import com.yahoo.language.Language; /** - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a> + * Sets the language in the execution context. + * + * @author Simon Thoresen */ public class SetLanguageExpression extends Expression { @@ -47,4 +48,5 @@ public class SetLanguageExpression extends Expression { public int hashCode() { return getClass().hashCode(); } + } diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/SetLanguageTestCase.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/SetLanguageTestCase.java index c69af6ba0f2..270c6ab4386 100644 --- a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/SetLanguageTestCase.java +++ b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/SetLanguageTestCase.java @@ -13,7 +13,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; /** - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a> + * @author Simon Thoresen */ public class SetLanguageTestCase { @@ -34,10 +34,19 @@ public class SetLanguageTestCase { } @Test - public void requireThatLanguageIsSet() { + public void testsettingEnglish() { ExecutionContext ctx = new ExecutionContext(new SimpleTestAdapter()); ctx.setValue(new StringFieldValue("en")); new SetLanguageExpression().execute(ctx); assertEquals(Language.ENGLISH, ctx.getLanguage()); } + + @Test + public void testSettingUnknown() { + ExecutionContext ctx = new ExecutionContext(new SimpleTestAdapter()); + ctx.setValue(new StringFieldValue("unknown")); + new SetLanguageExpression().execute(ctx); + assertEquals(Language.UNKNOWN, ctx.getLanguage()); + } + } diff --git a/linguistics/src/main/java/com/yahoo/language/Language.java b/linguistics/src/main/java/com/yahoo/language/Language.java index df1685433fa..ab8bcd4459f 100644 --- a/linguistics/src/main/java/com/yahoo/language/Language.java +++ b/linguistics/src/main/java/com/yahoo/language/Language.java @@ -8,7 +8,7 @@ import java.util.Locale; import java.util.Map; /** - * @author rpito + * @author Rich Pito */ public enum Language { @@ -87,14 +87,14 @@ public enum Language { /** Language tag "chr". */ CHEROKEE("chr"), - /** + /** * Language tag "zh-hans". * * @see #fromLocale(Locale) */ CHINESE_SIMPLIFIED("zh-hans"), - /** + /** * Language tag "zh-hant". * * @see #fromLocale(Locale) @@ -173,7 +173,7 @@ public enum Language { /** Language tag "ha". */ HAUSA("ha"), - /** + /** * Language tag "he". * * @see #fromLocale(Locale) @@ -189,7 +189,7 @@ public enum Language { /** Language tag "is". */ ICELANDIC("is"), - /** + /** * Language tag "id". * * @see #fromLocale(Locale) @@ -298,7 +298,7 @@ public enum Language { /** Language tag "ne". */ NEPALI("ne"), - /** + /** * Language tag "nb". * * @see #fromLocale(Locale) @@ -476,7 +476,7 @@ public enum Language { /** Language tag "xh". */ XHOSA("xh"), - /** + /** * Language tag "yi". * * @see #fromLocale(Locale) diff --git a/linguistics/src/main/java/com/yahoo/language/LocaleFactory.java b/linguistics/src/main/java/com/yahoo/language/LocaleFactory.java index 552dd611b1f..2bc510bb038 100644 --- a/linguistics/src/main/java/com/yahoo/language/LocaleFactory.java +++ b/linguistics/src/main/java/com/yahoo/language/LocaleFactory.java @@ -4,7 +4,7 @@ package com.yahoo.language; import java.util.Locale; /** - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a> + * @author Simon Thoresen */ public final class LocaleFactory { @@ -15,7 +15,7 @@ public final class LocaleFactory { } /** - * <p>Implements a simple parser for RFC5646 language tags. The language tag is parsed into a Locale.</p> + * Implements a simple parser for RFC5646 language tags. The language tag is parsed into a Locale. * * @param tag The language tag to parse. * @return The corrseponding Locale. diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/config/package-info.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/config/package-info.java new file mode 100644 index 00000000000..15cfa99c749 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/config/package-info.java @@ -0,0 +1,5 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.vespa.hosted.node.admin.config; + +import com.yahoo.osgi.annotation.ExportPackage; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java index 1d11e221a9b..80978d30da5 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java @@ -15,7 +15,7 @@ public interface DockerOperations { void startContainer(ContainerName containerName, ContainerNodeSpec nodeSpec); - void removeContainer(Container existingContainer); + void removeContainer(Container existingContainer, ContainerNodeSpec nodeSpec); Optional<Container> getContainer(ContainerName containerName); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java index d2863468ee7..88f5c9acfed 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java @@ -12,6 +12,7 @@ import com.yahoo.vespa.hosted.dockerapi.DockerImpl; import com.yahoo.vespa.hosted.dockerapi.DockerNetworkCreator; import com.yahoo.vespa.hosted.dockerapi.ProcessResult; import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec; +import com.yahoo.vespa.hosted.node.admin.maintenance.acl.iptables.NATCommand; import com.yahoo.vespa.hosted.node.admin.util.Environment; import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; @@ -104,14 +105,13 @@ public class DockerOperationsImpl implements DockerOperations { String configServers = environment.getConfigServerUris().stream() .map(URI::getHost) .collect(Collectors.joining(",")); + Docker.CreateContainerCommand command = docker.createContainerCommand( nodeSpec.wantedDockerImage.get(), ContainerResources.from(nodeSpec.minCpuCores, nodeSpec.minMainMemoryAvailableGb), containerName, nodeSpec.hostname) .withManagedBy(MANAGER_NAME) - .withNetworkMode(DockerImpl.DOCKER_CUSTOM_MACVLAN_NETWORK_NAME) - .withIpAddress(nodeInetAddress) .withEnvironment("CONFIG_SERVER_ADDRESS", configServers) .withUlimit("nofile", 262_144, 262_144) .withUlimit("nproc", 32_768, 409_600) @@ -119,7 +119,13 @@ public class DockerOperationsImpl implements DockerOperations { .withAddCapability("SYS_PTRACE") // Needed for gcore, pstack etc. .withAddCapability("SYS_ADMIN"); // Needed for perf - command.withVolume("/etc/hosts", "/etc/hosts"); + if (!docker.networkNATed()) { + logger.info("Network not nated - setting up with specific ip address on a macvlan"); + command.withIpAddress(nodeInetAddress); + command.withNetworkMode(DockerImpl.DOCKER_CUSTOM_MACVLAN_NETWORK_NAME); + command.withVolume("/etc/hosts", "/etc/hosts"); // TODO This is probably not nessesary - review later + } + for (String pathInNode : DIRECTORIES_TO_MOUNT.keySet()) { String pathInHost = environment.pathInHostFromPathInNode(containerName, pathInNode).toString(); command.withVolume(pathInHost, pathInNode); @@ -137,11 +143,17 @@ public class DockerOperationsImpl implements DockerOperations { command.create(); if (isIPv6) { - docker.connectContainerToNetwork(containerName, "bridge"); + if (!docker.networkNATed()) { + docker.connectContainerToNetwork(containerName, "bridge"); + } + docker.startContainer(containerName); - setupContainerNetworkingWithScript(containerName); + setupContainerNetworkConnectivity(containerName, nodeInetAddress); } else { docker.startContainer(containerName); + if (docker.networkNATed()) { + setupContainerNetworkConnectivity(containerName, nodeInetAddress); + } } DIRECTORIES_TO_MOUNT.entrySet().stream().filter(Map.Entry::getValue).forEach(entry -> @@ -152,7 +164,7 @@ public class DockerOperationsImpl implements DockerOperations { } @Override - public void removeContainer(final Container existingContainer) { + public void removeContainer(final Container existingContainer, ContainerNodeSpec nodeSpec) { final ContainerName containerName = existingContainer.name; PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName); if (existingContainer.state.isRunning()) { @@ -162,6 +174,23 @@ public class DockerOperationsImpl implements DockerOperations { logger.info("Deleting container " + containerName.asString()); docker.deleteContainer(containerName); + + if (docker.networkNATed()) { + logger.info("Delete iptables NAT rules for " + containerName.asString()); + try { + InetAddress nodeInetAddress = environment.getInetAddressForHost(nodeSpec.hostname); + String ipv6Str = docker.getGlobalIPv6Address(containerName); + String drop = NATCommand.drop(nodeInetAddress, InetAddress.getByName(ipv6Str)); + Pair<Integer, String> result = processExecuter.exec(drop); + if (result.getFirst() != 0) { + // Might be because the one or two (out of three) rules where not present - debug log and ignore + // Trailing rules will always be overridden by a new one due the fact that we insert + logger.debug("Unable to drop NAT rule - error message: " + result.getSecond()); + } + } catch (IOException e) { + logger.warning("Unable to drop NAT rule for container " + containerName, e); + } + } } @Override @@ -191,13 +220,21 @@ public class DockerOperationsImpl implements DockerOperations { } /** + * For macvlan: * Due to a bug in docker (https://github.com/docker/libnetwork/issues/1443), we need to manually set * IPv6 gateway in containers connected to more than one docker network + * + * For nat: + * Setup iptables NAT rules to map the hosts public ips to the containers */ - private void setupContainerNetworkingWithScript(ContainerName containerName) throws IOException { - InetAddress hostDefaultGateway = DockerNetworkCreator.getDefaultGatewayLinux(true); - executeCommandInNetworkNamespace(containerName, - "route", "-A", "inet6", "add", "default", "gw", hostDefaultGateway.getHostAddress(), "dev", "eth1"); + private void setupContainerNetworkConnectivity(ContainerName containerName, InetAddress externalAddress) throws IOException { + if (docker.networkNATed()) { + insertNAT(containerName, externalAddress); + } else { + InetAddress hostDefaultGateway = DockerNetworkCreator.getDefaultGatewayLinux(true); + executeCommandInNetworkNamespace(containerName, + "route", "-A", "inet6", "add", "default", "gw", hostDefaultGateway.getHostAddress(), "dev", "eth1"); + } } @Override @@ -289,4 +326,25 @@ public class DockerOperationsImpl implements DockerOperations { public void deleteUnusedDockerImages() { docker.deleteUnusedDockerImages(); } + + /** + * Only insert NAT rules if they don't exist (or else they compounded) + */ + private void insertNAT(ContainerName containerName, InetAddress externalAddress) throws IOException { + PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName); + String ipv6Str = docker.getGlobalIPv6Address(containerName); + + // Check if exist + String checkCommand = NATCommand.check(externalAddress, InetAddress.getByName(ipv6Str)); + Pair<Integer, String> result = processExecuter.exec(checkCommand); + if (result.getFirst() == 0 ) return; + + // Setup NAT + String natCommand = NATCommand.insert(externalAddress, InetAddress.getByName(ipv6Str)); + logger.info("Setting up NAT rules: " + natCommand); + result = processExecuter.exec(checkCommand); + if (result.getFirst() != 0 ) { + throw new IOException("Unable to setup NAT rule - error message: " + result.getSecond()); + } + } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommand.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommand.java index 87bb5fddf23..7b380a0afd3 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommand.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommand.java @@ -5,10 +5,12 @@ import java.net.Inet6Address; import java.net.InetAddress; /** - * Creates two commands that: + * Creates three ip(6)tables commands customized to map docker containers on a private net to their + * respective public ip addresses that all are assign to the host * - * 1. replaces an external/public destination ip to an internal/private ip before routing it (pre-routing) - * 2. replaces an internal/private source ip to an external/public ip before writing it on the wire (post-routing) + * 1. DNAT PRE-ROUTING: replaces an external/public destination ip to an internal/private ip when it arrives on an interface + * 2. DNAT LOOPBACK OUTPUT: replaces an internal/public ip destination to an internal/private ip on the loopback device (for host to container communication) + * 3. SNAT POST-ROUTING: replaces an internal/private source ip to an external/public ip before writing it on the wire * * @author smorgrav */ @@ -16,27 +18,48 @@ public class NATCommand implements Command { private final String snatCommand; private final String dnatCommand; + private final String dnatLoopBackCommand; - NATCommand(InetAddress externalIp, InetAddress internalIp, String iface) { + public NATCommand(InetAddress externalIp, InetAddress internalIp, String chainCommand) { String command = externalIp instanceof Inet6Address ? "ip6tables" : "iptables"; - this.snatCommand = String.format("%s -t nat -A POSTROUTING -o %s -s %s -j SNAT --to %s", + this.snatCommand = String.format("%s -t nat %s POSTROUTING -s %s -j SNAT --to %s", command, - iface, + chainCommand, internalIp.getHostAddress(), externalIp.getHostAddress()); - - this.dnatCommand = String.format("%s -t nat -A PREROUTING -i %s -d %s -j DNAT --to-destination %s", + this.dnatLoopBackCommand = String.format("%s -t nat %s OUTPUT -o lo -d %s -j DNAT --to-destination %s", + command, + chainCommand, + externalIp.getHostAddress(), + internalIp.getHostAddress()); + this.dnatCommand = String.format("%s -t nat %s PREROUTING -d %s -j DNAT --to-destination %s", command, - iface, + chainCommand, externalIp.getHostAddress(), internalIp.getHostAddress()); } @Override public String asString() { - return snatCommand + "; " + dnatCommand; + return concat("&&"); } @Override public String asString(String commandName) { return asString(); } + + private String concat(String delimiter) { + return String.join(delimiter, snatCommand, dnatCommand, dnatLoopBackCommand); + } + + public static String insert(InetAddress externalIp, InetAddress internalIp) { + return new NATCommand(externalIp, internalIp, "-I").concat(" && "); + } + + public static String drop(InetAddress externalIp, InetAddress internalIp) { + return new NATCommand(externalIp, internalIp, "-D").concat("; "); + } + + public static String check(InetAddress externalIp, InetAddress internalIp) { + return new NATCommand(externalIp, internalIp, "-C").concat(" && "); + } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/DockerAdminComponent.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/DockerAdminComponent.java index db0313583db..7c3e04c18c1 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/DockerAdminComponent.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/DockerAdminComponent.java @@ -6,7 +6,7 @@ import com.yahoo.net.HostName; import com.yahoo.system.ProcessExecuter; import com.yahoo.vespa.hosted.dockerapi.Docker; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; -import com.yahoo.vespa.hosted.node.admin.ConfigServerConfig; +import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; import com.yahoo.vespa.hosted.node.admin.component.AdminComponent; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations; import com.yahoo.vespa.hosted.node.admin.docker.DockerOperationsImpl; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminMain.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminMain.java index 3e9cb239890..f412ec5ef2f 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminMain.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminMain.java @@ -8,7 +8,7 @@ import com.yahoo.log.LogLevel; import com.yahoo.vespa.defaults.Defaults; import com.yahoo.vespa.hosted.dockerapi.Docker; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; -import com.yahoo.vespa.hosted.node.admin.ConfigServerConfig; +import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; import com.yahoo.vespa.hosted.node.admin.component.AdminComponent; import com.yahoo.vespa.hosted.node.admin.provider.NodeAdminStateUpdater; 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 8ca3ace92e5..32f6186707a 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 @@ -345,7 +345,7 @@ public class NodeAgentImpl implements NodeAgent { } } if (currentFilebeatRestarter != null) currentFilebeatRestarter.cancel(true); - dockerOperations.removeContainer(existingContainer); + dockerOperations.removeContainer(existingContainer, nodeSpec); containerState = ABSENT; logger.info("Container successfully removed, new containerState is " + containerState); return Optional.empty(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java index a5146fcae09..fd8ca74644d 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java @@ -7,7 +7,7 @@ import com.yahoo.concurrent.classlock.ClassLocking; import com.yahoo.container.di.componentgraph.Provider; import com.yahoo.vespa.hosted.dockerapi.Docker; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; -import com.yahoo.vespa.hosted.node.admin.ConfigServerConfig; +import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; import com.yahoo.vespa.hosted.node.admin.component.AdminComponent; import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminMain; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/Environment.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/Environment.java index 72996e438b7..0f8baf9dabe 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/Environment.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/Environment.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.node.admin.util; import com.google.common.base.Strings; import com.yahoo.vespa.defaults.Defaults; import com.yahoo.vespa.hosted.dockerapi.ContainerName; -import com.yahoo.vespa.hosted.node.admin.ConfigServerConfig; +import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; import java.net.InetAddress; import java.net.URI; diff --git a/node-admin/src/main/resources/configdefinitions/config-server.def b/node-admin/src/main/resources/configdefinitions/config-server.def index f43bb221107..e4265e618a1 100644 --- a/node-admin/src/main/resources/configdefinitions/config-server.def +++ b/node-admin/src/main/resources/configdefinitions/config-server.def @@ -1,5 +1,5 @@ # Copyright 2018 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -namespace=vespa.hosted.node.admin +namespace=vespa.hosted.node.admin.config hosts[] string port int default=8080 range=[1,65535] diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java index 7a5d713936d..8a4c4fd8c88 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java @@ -162,6 +162,11 @@ public class DockerMock implements Docker { } + @Override + public String getGlobalIPv6Address(ContainerName name) { + return "2001:db8:1:2:0:242:ac13:2"; + } + public static class StartContainerCommandMock implements CreateContainerCommand { @Override public CreateContainerCommand withLabel(String name, String value) { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommandTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommandTest.java index c2a2575f6b1..b0df67b7b46 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommandTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/iptables/NATCommandTest.java @@ -20,19 +20,23 @@ public class NATCommandTest { public void sampleNATCommandIPv6() throws UnknownHostException{ InetAddress externalIP = Inet6Address.getByName("2001:db8::1"); InetAddress internalIP = Inet6Address.getByName("2001:db8::2"); - String iface = "eth0"; - NATCommand command = new NATCommand(externalIP, internalIP, iface); - Assert.assertEquals("ip6tables -t nat -A POSTROUTING -o eth0 -s 2001:db8:0:0:0:0:0:2 -j SNAT --to 2001:db8:0:0:0:0:0:1; ip6tables -t nat -A PREROUTING -i eth0 -d 2001:db8:0:0:0:0:0:1 -j DNAT --to-destination 2001:db8:0:0:0:0:0:2", command.asString()); + String insert = NATCommand.insert(externalIP, internalIP); + Assert.assertEquals("ip6tables -t nat -I POSTROUTING -s 2001:db8:0:0:0:0:0:2 -j SNAT --to 2001:db8:0:0:0:0:0:1 && ip6tables -t nat -I PREROUTING -d 2001:db8:0:0:0:0:0:1 -j DNAT --to-destination 2001:db8:0:0:0:0:0:2 && ip6tables -t nat -I OUTPUT -o lo -d 2001:db8:0:0:0:0:0:1 -j DNAT --to-destination 2001:db8:0:0:0:0:0:2", insert); + + String drop = NATCommand.drop(externalIP, internalIP); + Assert.assertEquals("ip6tables -t nat -D POSTROUTING -s 2001:db8:0:0:0:0:0:2 -j SNAT --to 2001:db8:0:0:0:0:0:1; ip6tables -t nat -D PREROUTING -d 2001:db8:0:0:0:0:0:1 -j DNAT --to-destination 2001:db8:0:0:0:0:0:2; ip6tables -t nat -D OUTPUT -o lo -d 2001:db8:0:0:0:0:0:1 -j DNAT --to-destination 2001:db8:0:0:0:0:0:2", drop); } @Test public void sampleNATCommandIPv4() throws UnknownHostException{ InetAddress externalIP = Inet4Address.getByName("192.168.0.1"); InetAddress internalIP = Inet4Address.getByName("192.168.0.2"); - String iface = "eth0"; - NATCommand command = new NATCommand(externalIP, internalIP, iface); - Assert.assertEquals("iptables -t nat -A POSTROUTING -o eth0 -s 192.168.0.2 -j SNAT --to 192.168.0.1; iptables -t nat -A PREROUTING -i eth0 -d 192.168.0.1 -j DNAT --to-destination 192.168.0.2", command.asString()); + String insert = NATCommand.insert(externalIP, internalIP); + Assert.assertEquals("iptables -t nat -I POSTROUTING -s 192.168.0.2 -j SNAT --to 192.168.0.1 && iptables -t nat -I PREROUTING -d 192.168.0.1 -j DNAT --to-destination 192.168.0.2 && iptables -t nat -I OUTPUT -o lo -d 192.168.0.1 -j DNAT --to-destination 192.168.0.2", insert); + + String drop = NATCommand.drop(externalIP, internalIP); + Assert.assertEquals("iptables -t nat -D POSTROUTING -s 192.168.0.2 -j SNAT --to 192.168.0.1; iptables -t nat -D PREROUTING -d 192.168.0.1 -j DNAT --to-destination 192.168.0.2; iptables -t nat -D OUTPUT -o lo -d 192.168.0.1 -j DNAT --to-destination 192.168.0.2", drop); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index cce000f858a..8067bf9ba69 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -115,7 +115,7 @@ public class NodeAgentImplTest { nodeAgent.converge(); - verify(dockerOperations, never()).removeContainer(any()); + verify(dockerOperations, never()).removeContainer(any(), any()); verify(orchestrator, never()).suspend(any(String.class)); verify(dockerOperations, never()).pullImageAsyncIfNeeded(any()); verify(storageMaintainer, never()).removeOldFilesFromNode(eq(containerName)); @@ -181,7 +181,7 @@ public class NodeAgentImplTest { nodeAgent.converge(); - verify(dockerOperations, never()).removeContainer(any()); + verify(dockerOperations, never()).removeContainer(any(), any()); verify(orchestrator, never()).suspend(any(String.class)); final InOrder inOrder = inOrder(dockerOperations, orchestrator, nodeRepository, aclMaintainer); @@ -223,7 +223,7 @@ public class NodeAgentImplTest { verify(orchestrator, never()).suspend(any(String.class)); verify(orchestrator, never()).resume(any(String.class)); - verify(dockerOperations, never()).removeContainer(any()); + verify(dockerOperations, never()).removeContainer(any(), any()); final InOrder inOrder = inOrder(dockerOperations); inOrder.verify(dockerOperations, times(1)).pullImageAsyncIfNeeded(eq(newDockerImage)); @@ -262,7 +262,7 @@ public class NodeAgentImplTest { inOrder.verify(orchestrator).resume(any(String.class)); inOrder.verify(orchestrator).resume(any(String.class)); inOrder.verify(orchestrator).suspend(any(String.class)); - inOrder.verify(dockerOperations).removeContainer(any()); + inOrder.verify(dockerOperations).removeContainer(any(), any()); inOrder.verify(dockerOperations).startContainer(eq(containerName), eq(thirdSpec)); inOrder.verify(orchestrator).resume(any(String.class)); } @@ -313,7 +313,7 @@ public class NodeAgentImplTest { nodeAgent.converge(); - verify(dockerOperations, never()).removeContainer(any()); + verify(dockerOperations, never()).removeContainer(any(), any()); verify(orchestrator, never()).resume(any(String.class)); verify(nodeRepository).updateNodeAttributes( hostName, new NodeAttributes() @@ -344,7 +344,7 @@ public class NodeAgentImplTest { // Should only be called once, when we initialize verify(dockerOperations, times(1)).getContainer(eq(containerName)); - verify(dockerOperations, never()).removeContainer(any()); + verify(dockerOperations, never()).removeContainer(any(), any()); verify(dockerOperations, never()).startContainer(eq(containerName), eq(nodeSpec)); verify(orchestrator, never()).resume(any(String.class)); verify(nodeRepository).updateNodeAttributes( @@ -378,7 +378,7 @@ public class NodeAgentImplTest { nodeAgent.converge(); final InOrder inOrder = inOrder(storageMaintainer, dockerOperations); - inOrder.verify(dockerOperations, never()).removeContainer(any()); + inOrder.verify(dockerOperations, never()).removeContainer(any(), any()); verify(orchestrator, never()).resume(any(String.class)); verify(nodeRepository).updateNodeAttributes( @@ -435,7 +435,7 @@ public class NodeAgentImplTest { nodeAgent.converge(); final InOrder inOrder = inOrder(storageMaintainer, dockerOperations, nodeRepository); - inOrder.verify(dockerOperations, times(1)).removeContainer(any()); + inOrder.verify(dockerOperations, times(1)).removeContainer(any(), any()); inOrder.verify(storageMaintainer, times(1)).cleanupNodeStorage(eq(containerName), eq(nodeSpec)); inOrder.verify(nodeRepository, times(1)).markNodeAvailableForNewAllocation(eq(hostName)); @@ -492,7 +492,7 @@ public class NodeAgentImplTest { nodeAgent.tick(); - verify(dockerOperations, times(1)).removeContainer(any()); + verify(dockerOperations, times(1)).removeContainer(any(), any()); verify(dockerOperations, times(1)).startContainer(eq(containerName), eq(nodeSpec)); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java index 031d56e3164..4ee56167c50 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirer.java @@ -5,6 +5,7 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -72,7 +73,7 @@ public class FailedExpirer extends Maintainer { .filter(node -> node.allocation().isPresent() && node.allocation().get().membership().cluster().type() == ClusterSpec.Type.container) .collect(Collectors.toList()); - List<Node> remainingNodes = getExpiredNodes(defaultExpiry); + List<Node> remainingNodes = getExpiredNodes(zone.system() == SystemName.cd ? containerExpiry : defaultExpiry); remainingNodes.removeAll(containerNodes); recycle(containerNodes); recycle(remainingNodes); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index d38c9179986..12a0f64aee1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -139,8 +139,8 @@ public class CuratorDatabaseClient { Map<Node.State, List<Node>> nodesByState = nodes.stream().collect(Collectors.groupingBy(Node::state)); for (Map.Entry<Node.State, List<Node>> entry : nodesByState.entrySet()) { writtenNodes.addAll(writeTo(entry.getKey(), entry.getValue(), agent, reason, nestedTransaction)); - nestedTransaction.commit(); } + nestedTransaction.commit(); } return writtenNodes; diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java index 720c5b05443..c3c32f2decb 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java @@ -96,7 +96,7 @@ public class FailedExpirerTest { .failNode(4, "node1") .failWithHardwareFailure("node2", "node3"); - scenario.clock().advance(Duration.ofDays(5)); + scenario.clock().advance(Duration.ofHours(2)); scenario.expirer().run(); scenario.assertNodesIn(Node.State.failed, "node1"); @@ -147,7 +147,7 @@ public class FailedExpirerTest { @Test public void ensure_failed_docker_host_is_not_parked_unless_all_children_are() { - FailureScenario scenario = new FailureScenario(SystemName.cd, Environment.prod) + FailureScenario scenario = new FailureScenario(SystemName.main, Environment.prod) .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent1") .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent2") .withNode(NodeType.host, FailureScenario.defaultFlavor, "parent3") diff --git a/vespajlib/src/main/java/com/yahoo/transaction/NestedTransaction.java b/vespajlib/src/main/java/com/yahoo/transaction/NestedTransaction.java index 8953263df20..af22cb77690 100644 --- a/vespajlib/src/main/java/com/yahoo/transaction/NestedTransaction.java +++ b/vespajlib/src/main/java/com/yahoo/transaction/NestedTransaction.java @@ -24,6 +24,9 @@ public final class NestedTransaction implements AutoCloseable { /** A list of (non-transactional) operations to execute after this transaction has committed successfully */ private final List<Runnable> onCommitted = new ArrayList<>(2); + /** Updated when commit() is done, to be able to track if someone tries to commit a second time */ + private boolean committed = false; + /** * Adds a transaction to this. * @@ -45,6 +48,8 @@ public final class NestedTransaction implements AutoCloseable { /** Perform a 2 phase commit */ public void commit() { + if (committed) throw new IllegalStateException("Transaction already committed"); + List<Transaction> organizedTransactions = organizeTransactions(transactions); // First phase @@ -75,6 +80,7 @@ public final class NestedTransaction implements AutoCloseable { log.log(Level.WARNING, "A committed task in " + this + " caused an exception", e); } } + committed = true; } public void onCommitted(Runnable runnable) { @@ -88,6 +94,11 @@ public final class NestedTransaction implements AutoCloseable { transaction.transaction.close(); } + @Override + public String toString() { + return String.join(",", transactions.stream().map(Object::toString).collect(Collectors.toList())); + } + private List<Transaction> organizeTransactions(List<ConstrainedTransaction> transactions) { return orderTransactions(combineTransactions(transactions), findOrderingConstraints(transactions)); } @@ -174,6 +185,11 @@ public final class NestedTransaction implements AutoCloseable { /** Returns transaction types which should commit after this */ public Class<? extends Transaction>[] before() { return before; } + @Override + public String toString() { + return transaction.toString(); + } + } private static class OrderingConstraint { diff --git a/vespajlib/src/test/java/com/yahoo/transaction/NestedTransactionTestCase.java b/vespajlib/src/test/java/com/yahoo/transaction/NestedTransactionTestCase.java index 4feb0869aa0..60facf7e4af 100644 --- a/vespajlib/src/test/java/com/yahoo/transaction/NestedTransactionTestCase.java +++ b/vespajlib/src/test/java/com/yahoo/transaction/NestedTransactionTestCase.java @@ -91,6 +91,19 @@ public class NestedTransactionTestCase { } } + @Test + public void testMoreThanOneCommitThrows() { + NestedTransaction t = new NestedTransaction(); + t.add(new TransactionTypeA("A1"), TransactionTypeB.class); + t.commit(); + try { + t.commit(); + fail("Expected exception"); + } + catch (IllegalStateException expected) { + } + } + private static class TransactionTypeA extends MockTransaction { public TransactionTypeA(String name) { super(name); } } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java index f875af307c5..0e0c7401e0e 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -71,7 +71,7 @@ public class Curator implements AutoCloseable { } private Curator(ConfigserverConfig configserverConfig, String zooKeeperEnsembleConnectionSpec) { - this((configserverConfig.zookeeperLocalhostAffinity() || configserverConfig.system().equals("cd")) ? + this((configserverConfig.zookeeperLocalhostAffinity()) ? createConnectionSpecForLocalhost(configserverConfig) : zooKeeperEnsembleConnectionSpec, zooKeeperEnsembleConnectionSpec); } @@ -139,7 +139,7 @@ public class Curator implements AutoCloseable { } throw new IllegalArgumentException("Unable to create connect string to localhost: " + - "There is no localhost servers specified in config: " + config); + "There is no localhost server specified in config: " + config); } private static void validateConnectionSpec(String connectionSpec) { diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/transaction/CuratorTransaction.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/transaction/CuratorTransaction.java index 9c9ba96a90f..5e11a650953 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/transaction/CuratorTransaction.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/transaction/CuratorTransaction.java @@ -6,6 +6,7 @@ import com.yahoo.vespa.curator.Curator; import org.apache.curator.framework.api.transaction.CuratorTransactionFinal; import java.util.List; +import java.util.stream.Collectors; /** * Transaction implementation against ZooKeeper. @@ -65,4 +66,9 @@ public class CuratorTransaction extends AbstractTransaction { } } + @Override + public String toString() { + return String.join(",", operations().stream().map(operation -> operation.toString()).collect(Collectors.toList())); + } + } diff --git a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java index c5227fcbaa5..89d4521a20f 100644 --- a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java +++ b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorTest.java @@ -20,6 +20,8 @@ import static org.junit.Assert.assertThat; */ public class CuratorTest { + private static String localhost = HostName.getLocalhost(); + private String spec1; private String spec2; private TestingServer test1; @@ -33,8 +35,8 @@ public class CuratorTest { port2 = allocatePort(); test1 = new TestingServer(port1); test2 = new TestingServer(port2); - spec1 = "localhost:" + port1; - spec2 = "localhost:" + port2; + spec1 = localhost + ":" + port1; + spec2 = localhost + ":" + port2; } private int allocatePort() { @@ -52,14 +54,7 @@ public class CuratorTest { @Test public void require_curator_is_created_from_config() { try (Curator curator = createCurator(createTestConfig())) { - assertThat(curator.connectionSpec(), is(spec1 + "," + spec2)); - } - } - - @Test - public void require_that_curator_can_produce_spec() { - try (Curator curator = createCurator(createTestConfig())) { - assertThat(curator.connectionSpec(), is(spec1 + "," + spec2)); + assertThat(curator.zooKeeperEnsembleConnectionSpec(), is(spec1 + "," + spec2)); assertThat(curator.zooKeeperEnsembleCount(), is(2)); } } @@ -67,7 +62,7 @@ public class CuratorTest { @Test public void require_that_server_count_is_correct() { ConfigserverConfig.Builder builder = new ConfigserverConfig.Builder(); - builder.zookeeperserver(createZKBuilder("localhost", port1)); + builder.zookeeperserver(createZKBuilder(localhost, port1)); try (Curator curator = createCurator(new ConfigserverConfig(builder))) { assertThat(curator.zooKeeperEnsembleCount(), is(1)); } @@ -91,8 +86,8 @@ public class CuratorTest { private ConfigserverConfig createTestConfig() { ConfigserverConfig.Builder builder = new ConfigserverConfig.Builder(); - builder.zookeeperserver(createZKBuilder("localhost", port1)); - builder.zookeeperserver(createZKBuilder("localhost", port2)); + builder.zookeeperserver(createZKBuilder(localhost, port1)); + builder.zookeeperserver(createZKBuilder(localhost, port2)); return new ConfigserverConfig(builder); } @@ -125,7 +120,7 @@ public class CuratorTest { private final static PortRange portRange = new PortRange(); // Get the next port from a pre-allocated range - public static int findAvailablePort() { + static int findAvailablePort() { return portRange.next(); } |